Re: [PATCH] iio: imu: st_lsm6dsx: deselect shub page before reading whoami

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 5 Jun 2026 13:44:59 +0000
Andreas Kempe <andreas.kempe@actia.se> wrote:

> On Fri, Jun 05, 2026 at 01:15:18PM +0100, Jonathan Cameron wrote:
> > On Thu, 4 Jun 2026 16:23:27 +0000
> > Andreas Kempe <andreas.kempe@actia.se> wrote:  
> > > On Thu, Jun 04, 2026 at 06:14:11PM +0200, David Lechner wrote:  
> > > >
> > > > How about setting the SW_RESET bit in the CTRL3_C register during
> > > > probe too to ensure the rest of the registers are in a known state?
> > > >  
> > >
> > > While the shub register file is selected, the reset register is
> > > shadowed so it can't be written without first selecting the normal
> > > register file. A reset is already called from probe in
> > > st_lsm6dsx_init_device() after the whoami check has passed.  
> > 
> > The fix is fine, but I think it shouldn't be buried in the whoami
> > check as it's a bit of a weird side effect.  I'd like a top level
> > function that we can see in probe(). That can do the read + write
> > if necessary sequence you have in this patch.
> > 
> > This driver currently hard rejects unknown wai values (which it probably
> > should not given fallback compatibles should work). Lets assume that will
> > get resolved at somepoint and so the wai gate is advisory only. That
> > means that the dance to avoid writing to a device that doesn't match will
> > no be useful anyway.
> > 
> > As such, I'd prefer this 'reset of the register file mux selector' was
> > part of the reset function and that was simply called before checking
> > WAI.  It fits more logically there than in the whoami check function
> > which is just the first place the unexpected setting causes problems.
> > That is put it in st_lsm6dsx_reset_device() and call that before
> > st_lsm6dsx_check_whoami().  Maybe with a comment to justify that.
> >   
> 
> I agree fully that its current placement in st_lsm6dsx_check_whoami()
> is not optimal.
> 
> The reason I put the fix where it is, is because 
> st_lsm6dsx_check_whoami() is what assigns hw->settings needed for
> st_lsm6dsx_set_page() to work. I moved that assignment to before the
> whoami check for that very reason.
> 
> As I see it, moving my fix to a more reasonable place would mean a
> larger restructuring of the driver or making the whoami check advisory
> per your suggestion.

That feels unwise (even though it would work) as a fix for this issue.
People would get very confused at the informational print.

Ok. I guess maybe this is the best we can do.

Lets see if Lorenzo is happy with earlier part of this thread

Jonathan


> 
> Best regards,
> Andreas Kempe