[PATCH v3] i2c: imx: when being a target, mark the last read as processed

Wolfram Sang posted 1 patch 1 year, 11 months ago
drivers/i2c/busses/i2c-imx.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Wolfram Sang 1 year, 11 months ago
From: Corey Minyard <minyard@acm.org>

When being a target, NAK from the controller means that all bytes have
been transferred. So, the last byte needs also to be marked as
'processed'. Otherwise index registers of backends may not increase.

Signed-off-by: Corey Minyard <minyard@acm.org>
Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
[wsa: fixed comment and commit message to properly describe the case]
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since v2:
* updated commit message and comment

In the stalled discussion[1], it seems I couldn't make my suggestions
clear. So, here are the changes how I meant them. I hope this can be
agreed on.

[1] http://patchwork.ozlabs.org/project/linux-i2c/patch/20211112133956.655179-3-minyard@acm.org/

 drivers/i2c/busses/i2c-imx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 88a053987403..60e813137f84 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -803,6 +803,11 @@ static irqreturn_t i2c_imx_slave_handle(struct imx_i2c_struct *i2c_imx,
 		ctl &= ~I2CR_MTX;
 		imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
 		imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+
+		/* flag the last byte as processed */
+		i2c_imx_slave_event(i2c_imx,
+				    I2C_SLAVE_READ_PROCESSED, &value);
+
 		i2c_imx_slave_finish_op(i2c_imx);
 		return IRQ_HANDLED;
 	}
-- 
2.43.0
Re: [PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Andi Shyti 1 year, 11 months ago
Hi

On Wed, 21 Feb 2024 20:27:13 +0100, Wolfram Sang wrote:
> When being a target, NAK from the controller means that all bytes have
> been transferred. So, the last byte needs also to be marked as
> 'processed'. Otherwise index registers of backends may not increase.
> 
> 

Applied to i2c/i2c-host-fixes on

git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git

Thank you,
Andi

Patches applied
===============
[1/1] i2c: imx: when being a target, mark the last read as processed
      commit: cf8281b1aeab93a03c87033a741075c39ace80d4
Re: [PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Andi Shyti 1 year, 11 months ago
Hi Wolfram and Corey,

On Wed, Feb 21, 2024 at 08:27:13PM +0100, Wolfram Sang wrote:
> From: Corey Minyard <minyard@acm.org>
> 
> When being a target, NAK from the controller means that all bytes have
> been transferred. So, the last byte needs also to be marked as
> 'processed'. Otherwise index registers of backends may not increase.
> 
> Signed-off-by: Corey Minyard <minyard@acm.org>
> Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
> Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> [wsa: fixed comment and commit message to properly describe the case]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

is this a fix?

Andi

> ---
> 
> Changes since v2:
> * updated commit message and comment
> 
> In the stalled discussion[1], it seems I couldn't make my suggestions
> clear. So, here are the changes how I meant them. I hope this can be
> agreed on.
> 
> [1] http://patchwork.ozlabs.org/project/linux-i2c/patch/20211112133956.655179-3-minyard@acm.org/
> 
>  drivers/i2c/busses/i2c-imx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 88a053987403..60e813137f84 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -803,6 +803,11 @@ static irqreturn_t i2c_imx_slave_handle(struct imx_i2c_struct *i2c_imx,
>  		ctl &= ~I2CR_MTX;
>  		imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>  		imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +
> +		/* flag the last byte as processed */
> +		i2c_imx_slave_event(i2c_imx,
> +				    I2C_SLAVE_READ_PROCESSED, &value);
> +
>  		i2c_imx_slave_finish_op(i2c_imx);
>  		return IRQ_HANDLED;
>  	}
> -- 
> 2.43.0
>
Re: [PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Wolfram Sang 1 year, 11 months ago
On Wed, Feb 21, 2024 at 09:58:23PM +0100, Andi Shyti wrote:
> Hi Wolfram and Corey,
> 
> On Wed, Feb 21, 2024 at 08:27:13PM +0100, Wolfram Sang wrote:
> > From: Corey Minyard <minyard@acm.org>
> > 
> > When being a target, NAK from the controller means that all bytes have
> > been transferred. So, the last byte needs also to be marked as
> > 'processed'. Otherwise index registers of backends may not increase.
> > 
> > Signed-off-by: Corey Minyard <minyard@acm.org>
> > Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
> > Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
> > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > [wsa: fixed comment and commit message to properly describe the case]
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> is this a fix?

In deed, it is:

Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")

Re: [PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Oleksij Rempel 1 year, 11 months ago
Hi Wolfram,

On Wed, Feb 21, 2024 at 11:54:54PM +0100, Wolfram Sang wrote:
> On Wed, Feb 21, 2024 at 09:58:23PM +0100, Andi Shyti wrote:
> > Hi Wolfram and Corey,
> > 
> > On Wed, Feb 21, 2024 at 08:27:13PM +0100, Wolfram Sang wrote:
> > > From: Corey Minyard <minyard@acm.org>
> > > 
> > > When being a target, NAK from the controller means that all bytes have
> > > been transferred. So, the last byte needs also to be marked as
> > > 'processed'. Otherwise index registers of backends may not increase.
> > > 
> > > Signed-off-by: Corey Minyard <minyard@acm.org>
> > > Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
> > > Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
> > > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > [wsa: fixed comment and commit message to properly describe the case]
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > is this a fix?
> 
> In deed, it is:
> 
> Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")

Looks good :)
Are any action needed on my side?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Re: [PATCH v3] i2c: imx: when being a target, mark the last read as processed
Posted by Wolfram Sang 1 year, 11 months ago
On Thu, Feb 22, 2024 at 08:56:00AM +0100, Oleksij Rempel wrote:

> > > > When being a target, NAK from the controller means that all bytes have
> > > > been transferred. So, the last byte needs also to be marked as
> > > > 'processed'. Otherwise index registers of backends may not increase.
> > > > 
> > > > Signed-off-by: Corey Minyard <minyard@acm.org>
> > > > Tested-by: Andrew Manley <andrew.manley@sealingtech.com>
> > > > Reviewed-by: Andrew Manley <andrew.manley@sealingtech.com>
> > > > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > [wsa: fixed comment and commit message to properly describe the case]
> > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > 
> > > is this a fix?
> > 
> > In deed, it is:
> > 
> > Fixes: f7414cd6923f ("i2c: imx: support slave mode for imx I2C driver")
> 
> Looks good :)
> Are any action needed on my side?

Nope. All tags are still valid, I'd say, because I didn't change any code.

Thanks!