Re: [PATCH 0/4] hw/i2c: smbus: Reset fixes

[Joe Komlodi <komlodi@google.com>]

On Thu, Jan 11, 2024 at 6:03 AM Corey Minyard <minyard@acm.org> wrote:
>
> On Wed, Jan 10, 2024 at 09:26:37PM +0000, Joe Komlodi wrote:
> > Hi all,
> >
> > This series adds some resets for SMBus and for the I2C core. Along with
> > it, we make SMBus slave error printing a little more helpful.
> >
> > These reset issues were very infrequent, they would maybe occur in 1 out
> > of hundreds of resets in our testing, but the way they happen is pretty
> > straightforward.
> > Basically as long as a reset happens in the middle of a transaction, the
> > state of the old transaction would still partially be there after the
> > reset. Once a new transaction comes in, the partial stale state can
> > cause the new transaction to incorrectly fail.
>
> This seems wrong to me.  In a real system, the reset would be done on
> the smbus master and not necessarily on the mux (though I looked at a
> few of the PCA954x devices and they appear to have reset lines, but
> different systems may drive that reset differently).
>
> It seems to me that the bug is the smbus master device isn't getting
> reset in a system reset.  Just adding the reset logic there would be
> easier and more consistent with the real hardware.
>
> -corey
>
Oops, sorry, missed this in my inbox.

That sounds good to me, I'll send up a v2 that resets the SMBus master
instead of the mux.

Thanks,
Joe

> >
> > Thanks,
> > Joe
> >
> > Joe Komlodi (4):
> >   hw/i2c: core: Add reset
> >   hw/i2c/smbus_slave: Add object path on error prints
> >   hw/i2c: smbus_slave: Reset state on reset
> >   hw/i2c: smbus: mux: Reset SMBusDevice state on reset
> >
> >  hw/i2c/core.c                | 30 +++++++++++++++++++++++++-----
> >  hw/i2c/i2c_mux_pca954x.c     |  5 +++++
> >  hw/i2c/smbus_slave.c         | 20 ++++++++++++++++++--
> >  include/hw/i2c/i2c.h         |  6 +++++-
> >  include/hw/i2c/smbus_slave.h |  1 +
> >  5 files changed, 54 insertions(+), 8 deletions(-)
> >
> > --
> > 2.43.0.472.g3155946c3a-goog
> >
> >