Re: [PATCH v2 0/2] iio: humidity: si7020: Check device property for skipping reset in probe

Jonathan Cameron
2022-05-14T13:34:49Z

On Sat, 14 May 2022 00:48:51 +0200
Peter Rosin <peda@axentia.se> wrote:

> Hi!
>
> 2022-05-13 at 18:45, Jonathan Cameron wrote:
> > On Thu, 12 May 2022 12:08:07 -0500
> > Eddie James <eajames@linux.ibm.com> wrote:
> >
> >> On 5/12/22 11:48, Jonathan Cameron wrote:
> >>> On Thu, 12 May 2022 11:20:18 -0500
> >>> Eddie James <eajames@linux.ibm.com> wrote:
> >>>
> >>>> I2C commands issued after the SI7020 is starting up or after reset
> >>>> can potentially upset the startup sequence. Therefore, the host
> >>>> needs to wait for the startup sequence to finish before issuing
> >>>> further i2c commands. This is impractical in cases where the SI7020
> >>>> is on a shared bus or behind a mux, which may switch channels at
> >>>> any time (generating I2C traffic). Therefore, check for a device
> >>>> property that indicates that the driver should skip resetting the
> >>>> device when probing.
> >>> Why not lock the bus? It's not ideal, but then not resetting and hence
> >>> potentially ending up in an unknown state isn't great either.
> >>
> >>
> >> Agreed, but locking the bus doesn't work in the case where the chip is
> >> behind a mux. The mux core driver deselects the mux immediately after
> >> the transfer to reset the si7020, causing some i2c traffic, breaking the
> >> si7020. So it would also be a requirement to configure the mux to idle
> >> as-is... That's why I went with the optional skipping of the reset.
> >> Maybe I should add the bus lock too?
> >>
> >
> > +Cc Peter and linux-i2c for advice as we should resolve any potential
> > issue with the mux side of things rather than hiding it in the driver
> > (if possible!)
>
> IIUC, the chip in question cannot handle *any* action on the I2C bus
> for 15ms (or so) after a "soft reset", or something bad<tm> happens
> (or at least may happen).
>
> If that's the case, then providing a means of skipping the reset is
> insufficient. If you don't lock the bus, you would need to *always*
> skip the reset, because you don't know for certain if something else
> does I2C xfers.
>
> So, in order to make the soft reset not be totally dangerous even in
> a normal non-muxed environment, the bus must be locked for the 15ms.
>
> However, Eddie is correct in that the I2C mux code may indeed do its
> muxing xfer right after the soft reset command. There is currently
> no way to avoid that muxing xfer. However, it should be noted that
> there are ways to mux an I2C bus without using xfers on the bus
> itself, so it's not problematic for *all* mux variants.
>
> It can be debated if the problem should be worked around with extra
> dt properties like this, or if a capability should be added to delay
> a trailing muxing xfer.
>
> I bet there are other broken chips that have drivers that do in fact
> lock the bus to give the chip a break, but then it all stumbles
> because of the unexpected noise if there's a (wrong kind of) mux in
> the mix.

Ok, so for now I think we need the bus lock for the reset + either
a work around similar to this series, or additions to the i2c mux code
to stop it doing a muxing xfer if the bus is locked?

Jonathan

>
> Cheers,
> Peter
>
> >
> > Jonathan
> >
> >
> >
> >
> >>
> >> Thanks,
> >>
> >> Eddie
> >>
> >>
> >>>
> >>> Jonathan
> >>>
> >>>> Changes since v1:
> >>>> - Fix dt binding document
> >>>>
> >>>> Eddie James (2):
> >>>> dt-bindings: iio: humidity: Add si7020 bindings
> >>>> iio: humidity: si7020: Check device property for skipping reset in probe
> >>>>
> >>>> .../bindings/iio/humidity/silabs,si7020.yaml | 47 +++++++++++++++++++
> >>>> .../devicetree/bindings/trivial-devices.yaml | 2 -
> >>>> drivers/iio/humidity/si7020.c | 14 +++---
> >>>> 3 files changed, 55 insertions(+), 8 deletions(-)
> >>>> create mode 100644 Documentation/devicetree/bindings/iio/humidity/silabs,si7020.yaml
> >>>>
> >