linux-kernel - Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5pmqumpue7h4us265co6pya37434t4jvf3b655gtjcohlyhash@3ggx7e2maud6>
Date: Tue, 29 Jul 2025 14:49:23 +0200
From: Jorge Marques <gastmaier@...il.com>
To: "Guntupalli, Manikanta" <manikanta.guntupalli@....com>
Cc: David Lechner <dlechner@...libre.com>, 
	Andy Shevchenko <andriy.shevchenko@...el.com>, "git (AMD-Xilinx)" <git@....com>, 
	"Simek, Michal" <michal.simek@....com>, "lorenzo@...nel.org" <lorenzo@...nel.org>, 
	"jic23@...nel.org" <jic23@...nel.org>, "nuno.sa@...log.com" <nuno.sa@...log.com>, 
	"andy@...nel.org" <andy@...nel.org>, "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>, 
	"Goud, Srinivas" <srinivas.goud@....com>, "manion05gk@...il.com" <manion05gk@...il.com>
Subject: Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
 interface

On Tue, Jul 29, 2025 at 12:02:56PM +0000, Guntupalli, Manikanta wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi @Jorge Marques,
> 
> > -----Original Message-----
> > From: Jorge Marques <gastmaier@...il.com>
> > Sent: Tuesday, July 22, 2025 1:27 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@....com>
> > Cc: David Lechner <dlechner@...libre.com>; Andy Shevchenko
> > <andriy.shevchenko@...el.com>; git (AMD-Xilinx) <git@....com>; Simek, Michal
> > <michal.simek@....com>; lorenzo@...nel.org; jic23@...nel.org;
> > nuno.sa@...log.com; andy@...nel.org; linux-iio@...r.kernel.org; linux-
> > kernel@...r.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@....com>; Goud, Srinivas <srinivas.goud@....com>;
> > manion05gk@...il.com
> > Subject: Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
> > interface
> >
> > On Tue, Jul 22, 2025 at 07:32:54AM +0000, Guntupalli, Manikanta wrote:
> > > [AMD Official Use Only - AMD Internal Distribution Only]
> > >
> > > Hi @David Lechner,
> > >
> > > > -----Original Message-----
> > > > From: David Lechner <dlechner@...libre.com>
> > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > To: Andy Shevchenko <andriy.shevchenko@...el.com>; Guntupalli,
> > > > Manikanta <manikanta.guntupalli@....com>
> > > > Cc: git (AMD-Xilinx) <git@....com>; Simek, Michal
> > > > <michal.simek@....com>; lorenzo@...nel.org; jic23@...nel.org;
> > > > nuno.sa@...log.com; andy@...nel.org; linux-iio@...r.kernel.org;
> > > > linux-kernel@...r.kernel.org; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@....com>; Goud, Srinivas
> > > > <srinivas.goud@....com>; manion05gk@...il.com
> > > > Subject: Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback
> > > > support for I3C interface
> > > >
> > > > On 7/21/25 6:38 AM, Andy Shevchenko wrote:
> > > > > On Mon, Jul 21, 2025 at 04:37:41PM +0530, Manikanta Guntupalli wrote:
> > > > >> Add a shutdown handler for the ST LSM6DSx I3C driver to perform a
> > > > >> hardware reset during system shutdown. This ensures the sensor is
> > > > >> placed in a well-defined reset state, preventing issues during
> > > > >> subsequent reboots, such as kexec, where the device may fail to
> > > > >> respond correctly during enumeration.
> > > > >
> > > > > Do you imply that tons of device drivers missing this? I don't
> > > > > think we have even 5% of the drivers implementing the feature.
> > > > >
> > > > In the IIO drivers I've worked on, we always do reset in the probe()
> > > > function. The
> > > > shutdown() function might not run, e.g. if the board loses power, so
> > > > it doesn't fix 100% of the cases.
> > >
> > > Thank you for the input.
> > >
> > > You're absolutely right — shutdown() may not cover all cases like power loss.
> > However, in scenarios such as a warm reboot (kexec), the situation is different.
> > >
> > > Before the probe is called in the next boot, device enumeration takes place. During
> > this process, the I3C framework compares the device’s PID, BCR, and DCR values
> > against the ones registered in the driver:
> > >
> > > static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > >         I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > >         I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> > >         { }
> > > };
> > >
> > > Only if this matching succeeds, the probe will be invoked.
> > >
> > > Since the sensor reset logic is placed inside the probe, the device must be in a
> > responsive state during enumeration. In the case of kexec, we observed that the
> > sensor does not respond correctly unless it is explicitly reset during shutdown().
> > Hence, adding the reset in shutdown() addresses this specific case where the probe
> > isn't reached due to failed enumeration.
> > >
> > Hi Manikanta,
> >
> > During i3c bus init, the CCC RSTDAA is emitted to reset all DAs of all devices in the
> > bus (drivers/i3c/master.c@..._master_bus_init -> i3c_master_rstdaa_locked). Is the
> > LSM6DSX not compliant with that?
> LSM6DSX is compliant with RSTDAA CCC.
> 
> >
> > I get your solution but find odd to use the same method as in the probe.
> > In the probe, you would, in general, reset the device logic, but leave the i3c
> > peripheral logic intact, because you don't want to undo whatever the controller has
> > set-up for the current bus attached devices (ibi config, da, max devices speed, all the
> > good i3c stuff).
> > For this device, the st_lsm6dsx_reset_device seems to flush a FIFO, do a software
> > reset, and reload a trimming parameter; which are necessary to solve the bug you
> > are observed?
> Only software reset necessary to solve the bug.
> 
> >
> > If possible, please explain better why the device won't enumerate correctly after a
> > reboot without the reset. If it is a device bug, explicitly state that and that it is not
> > compliant. Also, take a look at fig.100 of the i3c spec basic 1.1.1.
> >
> > Thank you for looking into this, this type of corner case is usually overlooked.
> It appears that the sensor device is entering a deep sleep or low-power state and is not responding to CCC commands. However, after a software reset, the sensor starts responding to CCCs as expected.
It should, from the silicon pov, definitely respond to CCCs, even on
low-power states.
Could you confirm with stm32 the behaviour you are observing?
Inform them if it occurs under under i2c/i3c dual support, only i3c, or
both.
It sounds a little the messages are being filtered by the spike filter
when it shouldn't?
> 
> Shall we proceed with only the software reset changes along with an improved description, or do you recommend any additional modifications?
Confirm if this is a silicon issue first, if so, a note from st should
be issued also.
> 
> Thanks,
> Manikanta.

Best regards,
Jorge