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

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3158 bytes --]

> 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

I am fine with fixing the issue in st_lsm6dsx_check_whoami().
For simplicity, I suggest to always disable page_mux if supported by the hw.
Something like:

static int st_lsm6dsx_check_whoami()
{
	...
	if (hw->settings->shub_settings.page_mux.addr) {
		/* Please add a comment here explaining the issue */
		err = st_lsm6dsx_set_page(hw, false);
		...
	}

Regards,
Lorenzo

> 
> Jonathan
> 
> 
> > 
> > Best regards,
> > Andreas Kempe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]