| Message ID | 20240221193013.14233-2-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] i2c: imx: when being a target, mark the last read as processed | expand |
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 >
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")
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
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!
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
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; }