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] [day] [month] [year] [list]

Message-ID: <3d7w3rczrdics77nt7lig5rsj2bmfubpwzhffarzlxmo5w2g4a@baewpltdovhk>
Date: Tue, 22 Jul 2025 09:56:41 +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 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?

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?

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.

Best regards,
Jorge

> Thanks,
> Manikanta.