Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1317620 > unrolled thread

Re: [PATCH] i2c: imx: add slave support

Started byDmitriy Baranov <dbaranov@dev.rtsoft.ru>
First post2016-01-26 09:30 +0100
Last post2016-01-26 16:20 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] i2c: imx: add slave support Dmitriy Baranov <dbaranov@dev.rtsoft.ru> - 2016-01-26 09:30 +0100
    Re: [PATCH] i2c: imx: add slave support Wolfram Sang <wsa@the-dreams.de> - 2016-01-26 09:40 +0100
      Re: [PATCH] i2c: imx: add slave support Dmitriy Baranov <dbaranov@dev.rtsoft.ru> - 2016-01-26 11:00 +0100
        Re: [PATCH] i2c: imx: add slave support Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-01-26 14:40 +0100
          Re: [PATCH] i2c: imx: add slave support Dmitriy Baranov <dbaranov@dev.rtsoft.ru> - 2016-01-26 15:20 +0100
            Re: [PATCH] i2c: imx: add slave support Wolfram Sang <wsa@the-dreams.de> - 2016-01-26 16:20 +0100

#1317620 — Re: [PATCH] i2c: imx: add slave support

FromDmitriy Baranov <dbaranov@dev.rtsoft.ru>
Date2016-01-26 09:30 +0100
SubjectRe: [PATCH] i2c: imx: add slave support
Message-ID<qV7aG-1qe-17@gated-at.bofh.it>
Thank you for testing our patch.

Due to using the generic slave interface, It should be enabled in the 
config file.
Please add the following in the config file:
CONFIG_I2C_SLAVE=y



On 25.01.2016 21:09, kbuild test robot wrote:
> Hi Dmitriy,
>
> [auto build test ERROR on wsa/i2c/for-next]
> [also build test ERROR on v4.5-rc1 next-20160125]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Dmitriy-Baranov/i2c-imx-add-slave-support/20160125-225538
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
> config: arm-imx_v6_v7_defconfig (attached as .config)
> reproduce:
>          wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm
>
> All error/warnings (new ones prefixed by >>):
>
>     drivers/i2c/busses/i2c-imx.c: In function 'i2c_imx_slave_threadfn':
>>> drivers/i2c/busses/i2c-imx.c:696:6: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration]
>           i2c_slave_event(i2c_imx->slave,
>           ^
>>> drivers/i2c/busses/i2c-imx.c:697:7: error: 'I2C_SLAVE_READ_REQUESTED' undeclared (first use in this function)
>            I2C_SLAVE_READ_REQUESTED, &data);
>            ^
>     drivers/i2c/busses/i2c-imx.c:697:7: note: each undeclared identifier is reported only once for each function it appears in
>>> drivers/i2c/busses/i2c-imx.c:706:7: error: 'I2C_SLAVE_WRITE_REQUESTED' undeclared (first use in this function)
>            I2C_SLAVE_WRITE_REQUESTED, &data);
>            ^
>>> drivers/i2c/busses/i2c-imx.c:719:8: error: 'I2C_SLAVE_READ_PROCESSED' undeclared (first use in this function)
>             I2C_SLAVE_READ_PROCESSED, &data);
>             ^
>>> drivers/i2c/busses/i2c-imx.c:744:7: error: 'I2C_SLAVE_WRITE_RECEIVED' undeclared (first use in this function)
>            I2C_SLAVE_WRITE_RECEIVED, &data);
>            ^
>>> drivers/i2c/busses/i2c-imx.c:755:37: error: 'I2C_SLAVE_STOP' undeclared (first use in this function)
>          i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
>                                          ^
>     drivers/i2c/busses/i2c-imx.c: At top level:
>>> drivers/i2c/busses/i2c-imx.c:1283:2: error: unknown field 'reg_slave' specified in initializer
>       .reg_slave = i2c_imx_reg_slave,
>       ^
>>> drivers/i2c/busses/i2c-imx.c:1283:2: warning: excess elements in struct initializer
>     drivers/i2c/busses/i2c-imx.c:1283:2: warning: (near initialization for 'i2c_imx_algo')
>>> drivers/i2c/busses/i2c-imx.c:1284:2: error: unknown field 'unreg_slave' specified in initializer
>       .unreg_slave = i2c_imx_unreg_slave,
>       ^
>     drivers/i2c/busses/i2c-imx.c:1284:2: warning: excess elements in struct initializer
>     drivers/i2c/busses/i2c-imx.c:1284:2: warning: (near initialization for 'i2c_imx_algo')
>     cc1: some warnings being treated as errors
>
> vim +/i2c_slave_event +696 drivers/i2c/busses/i2c-imx.c
>
>     690				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>     691				ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>     692	
>     693				if (status & I2SR_IAAS) {
>     694					if (status & I2SR_SRW) {
>     695						/* master wants to read from us */
>   > 696						i2c_slave_event(i2c_imx->slave,
>   > 697							I2C_SLAVE_READ_REQUESTED, &data);
>     698						ctl |= I2CR_MTX;
>     699						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     700	
>     701						/*send data */
>     702						imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
>     703					} else {
>     704						dev_dbg(&i2c_imx->adapter.dev, "write requested");
>     705						i2c_slave_event(i2c_imx->slave,
>   > 706							I2C_SLAVE_WRITE_REQUESTED, &data);
>     707						/*slave receive */
>     708						ctl &= ~I2CR_MTX;
>     709						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     710	
>     711						/*dummy read */
>     712						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     713					}
>     714				} else {
>     715					/* slave send */
>     716					if (ctl & I2CR_MTX) {
>     717						if (!(status & I2SR_RXAK)) {	/*ACK received */
>     718							i2c_slave_event(i2c_imx->slave,
>   > 719								I2C_SLAVE_READ_PROCESSED, &data);
>     720							ctl |= I2CR_MTX;
>     721							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     722							/*send data */
>     723							imx_i2c_write_reg(data, i2c_imx, IMX_I2C_I2DR);
>     724						} else {
>     725							/*no ACK. */
>     726							/*dummy read */
>     727							dev_dbg(&i2c_imx->adapter.dev, "read requested");
>     728							i2c_slave_event(i2c_imx->slave,
>     729								I2C_SLAVE_READ_REQUESTED, &data);
>     730	
>     731							ctl &= ~I2CR_MTX;
>     732							imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     733							imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     734						}
>     735					} else {	/*read */
>     736						ctl &= ~I2CR_MTX;
>     737						imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>     738	
>     739						/*read */
>     740						data = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>     741						dev_dbg(&i2c_imx->adapter.dev, "received %x",
>     742							(unsigned int) data);
>     743						i2c_slave_event(i2c_imx->slave,
>   > 744							I2C_SLAVE_WRITE_RECEIVED, &data);
>     745					}
>     746				}
>     747			}
>     748	
>     749			if (atomic_read(&i2c_imx->slave_state) == I2C_IMX_SLAVE_POLLING) {
>     750				udelay(50);
>     751				status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>     752	
>     753				if ((status & I2SR_IBB) == 0) {
>     754					pr_debug("end of package");
>   > 755					i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &data);
>     756					atomic_set(&i2c_imx->slave_state, I2C_IMX_SLAVE_IDLE);
>     757					timeout	= HZ;
>     758				}
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [next] | [standalone]

#1317635

FromWolfram Sang <wsa@the-dreams.de>
Date2016-01-26 09:40 +0100
Message-ID<qV7km-1uA-27@gated-at.bofh.it>
In reply to#1317620

[Multipart message — attachments visible in raw view] — view raw

On Tue, Jan 26, 2016 at 11:22:24AM +0300, Dmitriy Baranov wrote:
> Thank you for testing our patch.
> 
> Due to using the generic slave interface, It should be enabled in the config
> file.
> Please add the following in the config file:
> CONFIG_I2C_SLAVE=y

For now, you need to select it to make randconfigs built.

[toc] | [prev] | [next] | [standalone]

#1317689

FromDmitriy Baranov <dbaranov@dev.rtsoft.ru>
Date2016-01-26 11:00 +0100
Message-ID<qV8zN-2fZ-33@gated-at.bofh.it>
In reply to#1317635
Sorry, we should have added selecting this in our patch.
The following fixes it:

Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the 
generic slave interface.

Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
---
  drivers/i2c/busses/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0299dfa..bc7cbfd 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -587,6 +587,7 @@ config I2C_IMG
  config I2C_IMX
      tristate "IMX I2C interface"
      depends on ARCH_MXC || ARCH_LAYERSCAPE
+    select I2C_SLAVE
      help
        Say Y here if you want to use the IIC bus controller on
        the Freescale i.MX/MXC or Layerscape processors.
-- 
2.5.0




On 26.01.2016 11:36, Wolfram Sang wrote:
> On Tue, Jan 26, 2016 at 11:22:24AM +0300, Dmitriy Baranov wrote:
>> Thank you for testing our patch.
>>
>> Due to using the generic slave interface, It should be enabled in the config
>> file.
>> Please add the following in the config file:
>> CONFIG_I2C_SLAVE=y
> For now, you need to select it to make randconfigs built.
>

[toc] | [prev] | [next] | [standalone]

#1317916

FromVladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date2016-01-26 14:40 +0100
Message-ID<qVc0F-4LK-3@gated-at.bofh.it>
In reply to#1317689
On 26.01.2016 11:54, Dmitriy Baranov wrote:
> Sorry, we should have added selecting this in our patch.
> The following fixes it:
> 
> Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the 
> generic slave interface.
> 
> Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
> Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
> ---
>   drivers/i2c/busses/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 0299dfa..bc7cbfd 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -587,6 +587,7 @@ config I2C_IMG
>   config I2C_IMX
>       tristate "IMX I2C interface"
>       depends on ARCH_MXC || ARCH_LAYERSCAPE
> +    select I2C_SLAVE
>       help
>         Say Y here if you want to use the IIC bus controller on
>         the Freescale i.MX/MXC or Layerscape processors.
> 

Could you please squash the changes (care about indentation in Kconfig also)
and resend it as v2 (probably you may want to wait for review comments some
more time)?

Two more formal questions, why patch submitter's Signed-off-by: is not the
last in the list and why Maxim has Signed-off-by tag here? Who is the author
of the change?

See Documentation/SubmittingPatches "Sign your work" for details.

Best wishes,
Vladimir

[toc] | [prev] | [next] | [standalone]

#1317947

FromDmitriy Baranov <dbaranov@dev.rtsoft.ru>
Date2016-01-26 15:20 +0100
Message-ID<qVcDn-5i7-5@gated-at.bofh.it>
In reply to#1317916
Vladimir,

Thanks for the comment.
We are both the authors of this patch.

> Could you please squash the changes (care about indentation in Kconfig also)
> and resend it as v2 (probably you may want to wait for review comments some
> more time)?
I suppose that after review, there will be many other things to fix in 
this patch.
Thus, if you don`t mind, we wont send fixed version for now.




On 26.01.2016 16:37, Vladimir Zapolskiy wrote:
> On 26.01.2016 11:54, Dmitriy Baranov wrote:
>> Sorry, we should have added selecting this in our patch.
>> The following fixes it:
>>
>> Subject: [PATCH] Select I2C_SLAVE for i2c-imx driver because it uses the
>> generic slave interface.
>>
>> Signed-off-by: Dmitriy Baranov <dbaranov@dev.rtsoft.ru>
>> Signed-off-by: Maxim Syrchin <syrchin@dev.rtsoft.ru>
>> ---
>>    drivers/i2c/busses/Kconfig | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 0299dfa..bc7cbfd 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -587,6 +587,7 @@ config I2C_IMG
>>    config I2C_IMX
>>        tristate "IMX I2C interface"
>>        depends on ARCH_MXC || ARCH_LAYERSCAPE
>> +    select I2C_SLAVE
>>        help
>>          Say Y here if you want to use the IIC bus controller on
>>          the Freescale i.MX/MXC or Layerscape processors.
>>
> Could you please squash the changes (care about indentation in Kconfig also)
> and resend it as v2 (probably you may want to wait for review comments some
> more time)?
>
> Two more formal questions, why patch submitter's Signed-off-by: is not the
> last in the list and why Maxim has Signed-off-by tag here? Who is the author
> of the change?
>
> See Documentation/SubmittingPatches "Sign your work" for details.
>
> Best wishes,
> Vladimir

[toc] | [prev] | [next] | [standalone]

#1318046

FromWolfram Sang <wsa@the-dreams.de>
Date2016-01-26 16:20 +0100
Message-ID<qVdzt-5Vw-27@gated-at.bofh.it>
In reply to#1317947

[Multipart message — attachments visible in raw view] — view raw

> Thus, if you don`t mind, we wont send fixed version for now.

Perfect!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web