RE: [EXT] Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after last read

[Carlos Song]

> -----Original Message-----
> From: Andi Shyti <andi.shyti@xxxxxxxxxx>
> Sent: Friday, March 20, 2026 7:10 AM
> To: Carlos Song <carlos.song@xxxxxxx>
> Cc: Frank Li <frank.li@xxxxxxx>; Stefan Eichenberger <eichest@xxxxxxxxx>;
> o.rempel@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; stefan.eichenberger@xxxxxxxxxxx; Francesco Dolcini
> <francesco.dolcini@xxxxxxxxxxx>; linux-i2c@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH v1 2/2] i2c: imx: ensure no clock is generated after
> last read
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Carlos,
> 
> > > > > > > When reading from the I2DR register, right after releasing
> > > > > > > the bus by clearing MSTA and MTX, the I2C controller might
> > > > > > > still generate an additional clock cycle which can cause
> > > > > > > devices to misbehave. Ensure to
> > > > > >
> > > > > > Do you means SCL have additional toggle? You capture waveform?
> > > > > >
> > > > >
> > > > > Yes exactly. We were able to capture the waveform when the issue
> > > > > happens. It doesn't always happen though, it depends on how much
> > > > > time passes between clearing MSTA and MTX and reading from I2DR.
> > > > >
> > > > > If you want to see the waveform, I uploaded it to our server:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > >
> 2Fshare.toradex.com%2Fdwnhcrl6b9toib6&data=05%7C02%7Ccarlos.song
> > > > > %40nxp.com%7C9567b791c35345a75eba08de860c9954%7C686ea1d
> 3bc2b4c6f
> > > > >
> a92cd99c5c301635%7C0%7C0%7C639095585800985569%7CUnknown%7CT
> WFpbG
> > > > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMi
> > > > >
> IsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=AK6DGztAek
> > > > > 8Dq0M8G98PO%2F68UhcwfJPxsLSJQZ4jumE%3D&reserved=0
> > > > > You can see the additional clock at the right end, after "0x17 + NAK".
> > > >
> > > > Have you had a chance to look at the waveform? Do you have any
> > > > concerns about the proposed solution?
> > >
> > > I am fine. Add carlos, who did many work about I2C.
> > >
> > > Frank
> >
> > Hi,
> >
> > Just review this series, looks this series patch make this fix for the limitation[1]
> safer:
> > "It must generate STOP before read I2DR to prevent controller from
> generating another clock cycle".
> >
> > Previous patch[2] has done this to avoid the limitation. However according to
> the waveform, I2C controller still generated an additional clock cycle
> sometime.
> >
> > The key of patch is ensure to read the last bytes after the bus is not busy
> anymore to avoid this another clock cycle.So these patches are fine to me also.
> >
> > [1] 054b62d9f25c ("i2c: imx: fix the i2c bus hang issue when do repeat
> > restart") [2] 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma
> > mode")
> 
> Sorry, I'm not understanding your point here. Are you suggesting to change the
> Fixes tag to [1]?
> 

Hi, Andi

No, don't need to do anything, . I agreed with this patch totally, it is reasonable.

> Thanks,
> Andi