diff mbox series

[v2] i2c: omap: Fix standard mode false ACK readings

Message ID 20230426194956.689756-1-reidt@ti.com (mailing list archive)
State New, archived
Headers show
Series [v2] i2c: omap: Fix standard mode false ACK readings | expand

Commit Message

reidt April 26, 2023, 7:49 p.m. UTC
Using standard mode, rare false ACK responses were appearing with
i2cdetect tool. This was happening due to NACK interrupt triggering
ISR thread before register access interrupt was ready. Removing the
NACK interrupt's ability to trigger ISR thread lets register access
ready interrupt do this instead.

Cc: <stable@vger.kernel.org> # v3.7+
Fixes: 3b2f8f82dad7 ("i2c: omap: switch to threaded IRQ support")
Signed-off-by: Reid Tonking <reidt@ti.com>
---
 drivers/i2c/busses/i2c-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vignesh Raghavendra April 27, 2023, 1:18 p.m. UTC | #1
On 4/27/2023 1:19 AM, Reid Tonking wrote:
> Using standard mode, rare false ACK responses were appearing with
> i2cdetect tool. This was happening due to NACK interrupt triggering
> ISR thread before register access interrupt was ready. Removing the
> NACK interrupt's ability to trigger ISR thread lets register access
> ready interrupt do this instead.
> 
> Cc: <stable@vger.kernel.org> # v3.7+
> Fixes: 3b2f8f82dad7 ("i2c: omap: switch to threaded IRQ support")
> Signed-off-by: Reid Tonking <reidt@ti.com>

Acked-by: Vignesh Raghavendra <vigneshr@ti.com>


Regards
Vignesh
Tony Lindgren April 28, 2023, 7:43 a.m. UTC | #2
* Raghavendra, Vignesh <vigneshr@ti.com> [230427 13:18]:
> On 4/27/2023 1:19 AM, Reid Tonking wrote:
> > Using standard mode, rare false ACK responses were appearing with
> > i2cdetect tool. This was happening due to NACK interrupt triggering
> > ISR thread before register access interrupt was ready. Removing the
> > NACK interrupt's ability to trigger ISR thread lets register access
> > ready interrupt do this instead.

So is it safe to leave NACK interrupt unhandled until we get the next
interrupt, does the ARDY always trigger after hitting this?

Regards,

Tony
reidt April 28, 2023, 6:30 p.m. UTC | #3
On 10:43-20230428, Tony Lindgren wrote:
> * Raghavendra, Vignesh <vigneshr@ti.com> [230427 13:18]:
> > On 4/27/2023 1:19 AM, Reid Tonking wrote:
> > > Using standard mode, rare false ACK responses were appearing with
> > > i2cdetect tool. This was happening due to NACK interrupt triggering
> > > ISR thread before register access interrupt was ready. Removing the
> > > NACK interrupt's ability to trigger ISR thread lets register access
> > > ready interrupt do this instead.
> 
> So is it safe to leave NACK interrupt unhandled until we get the next
> interrupt, does the ARDY always trigger after hitting this?
> 
> Regards,
> 
> Tony

Yep, the ARDY always gets set after a new command when register access is ready so there's no need for NACK interrupt to control this. 

-Reid
Wolfram Sang April 30, 2023, 5:52 a.m. UTC | #4
On Wed, Apr 26, 2023 at 02:49:56PM -0500, Reid Tonking wrote:
> Using standard mode, rare false ACK responses were appearing with
> i2cdetect tool. This was happening due to NACK interrupt triggering
> ISR thread before register access interrupt was ready. Removing the
> NACK interrupt's ability to trigger ISR thread lets register access
> ready interrupt do this instead.
> 
> Cc: <stable@vger.kernel.org> # v3.7+
> Fixes: 3b2f8f82dad7 ("i2c: omap: switch to threaded IRQ support")
> Signed-off-by: Reid Tonking <reidt@ti.com>

Applied to for-current, thanks!
Tony Lindgren May 3, 2023, 6:03 a.m. UTC | #5
* Reid Tonking <reidt@ti.com> [230428 18:30]:
> On 10:43-20230428, Tony Lindgren wrote:
> > * Raghavendra, Vignesh <vigneshr@ti.com> [230427 13:18]:
> > > On 4/27/2023 1:19 AM, Reid Tonking wrote:
> > > > Using standard mode, rare false ACK responses were appearing with
> > > > i2cdetect tool. This was happening due to NACK interrupt triggering
> > > > ISR thread before register access interrupt was ready. Removing the
> > > > NACK interrupt's ability to trigger ISR thread lets register access
> > > > ready interrupt do this instead.
> > 
> > So is it safe to leave NACK interrupt unhandled until we get the next
> > interrupt, does the ARDY always trigger after hitting this?
> > 
> > Regards,
> > 
> > Tony
> 
> Yep, the ARDY always gets set after a new command when register access is ready so there's no need for NACK interrupt to control this. 

OK thanks, looks good to me:

Reviewed-by: Tony Lindgren <tony@atomide.com>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 2b4e2be51318..4199f57a6bf2 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1058,7 +1058,7 @@  omap_i2c_isr(int irq, void *dev_id)
 	u16 stat;
 
 	stat = omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG);
-	mask = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
+	mask = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG) & ~OMAP_I2C_STAT_NACK;
 
 	if (stat & mask)
 		ret = IRQ_WAKE_THREAD;