Re: [PATCH v3 0/4] P2040/P2041 i2c recovery erratum

[Joakim Tjernlund]

On Wed, 2021-05-12 at 01:48 +0000, Chris Packham wrote:
> On 12/05/21 10:10 am, Joakim Tjernlund wrote:
> > On Wed, 2021-05-12 at 09:20 +1200, Chris Packham wrote:
> > > The P2040/P2041 has an erratum where the i2c recovery scheme
> > > documented in the reference manual (and currently implemented
> > > in the i2c-mpc.c driver) does not work. The errata document
> > > provides an alternative that does work. This series implements
> > > that alternative and uses a property in the devicetree to
> > > decide when the alternative mechanism is needed.
> > > 
> > > Chris Packham (4):
> > >    dt-bindings: i2c: mpc: Add fsl,i2c-erratum-a004447 flag
> > >    powerpc/fsl: set fsl,i2c-erratum-a004447 flag for P2041 i2c
> > >      controllers
> > >    powerpc/fsl: set fsl,i2c-erratum-a004447 flag for P1010 i2c
> > >      controllers
> > >    i2c: mpc: implement erratum A-004447 workaround
> > > 
> > >   .../devicetree/bindings/i2c/i2c-mpc.yaml      |  7 ++
> > >   arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |  8 ++
> > >   arch/powerpc/boot/dts/fsl/p2041si-post.dtsi   | 16 ++++
> > >   drivers/i2c/busses/i2c-mpc.c                  | 81 ++++++++++++++++++-
> > >   4 files changed, 110 insertions(+), 2 deletions(-)
> > > 
> > This now reminds me about the current I2C reset procedure, it didn't work for us and I came up with this one:
> >    https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-i2c%2Fmsg29490.html&data=04%7C01%7CJoakim.Tjernlund%40infinera.com%7Cb85a6e9c3c8b469572da08d914e816b5%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637563809322419998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=4cwujNmAVlBa08Tt79hLYGJfJtn7wdz1Kgz0eW2VX9U%3D&reserved=0
> > it never got in but we are still using it.
> 
> For those reading along the v2 mentioned in that thread was posted as 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-i2c%2F20170511122033.22471-1-joakim.tjernlund%40infinera.com%2F&data=04%7C01%7CJoakim.Tjernlund%40infinera.com%7Cb85a6e9c3c8b469572da08d914e816b5%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637563809322419998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=YDTX5L6J2ocHep5XutVN46jUpvJj7h1aDbHHwMqlrAs%3D&reserved=0 
> there was a bit of discussion but it seemed to die out without reaching 
> a conclusion.
> 
> The i2c-mpc driver is now using the generic recovery mechanism so that 
> addresses one bit of feedback from the original thread.
> 
> I do wonder if the reason the recovery wasn't working for your case was 
> because of the erratum. Do you happen to remember which SoC your issue 
> was on?

It could only be P2010 or MPC8321, I think it was MPC8321, you could try my solution on your
CPU if you want to make sure.

> 
> I've been doing my recent work with a P2040 and prior to that I did test 
> out the recovery on a T2081 (which isn't documented to have this 
> erratum) when I was re-working the driver. The "new" recovery actually 
> seems better but I don't have a reliably faulty i2c device so that's 
> only based on me writing some code to manually trigger the recovery 
> (using the snippet below) and observing it with an oscilloscope.

You don't need a faulty device, just an aborted I2C read/write op.
You could force one such I2C op. by py pulling down the clock/SDA in the middle of a byte transfer.

 Jocke