Message-ID:
 <DM4PR12MB610930805348D91ACAE876A18C25A@DM4PR12MB6109.namprd12.prod.outlook.com>
Date: Tue, 29 Jul 2025 12:02:56 +0000
From: "Guntupalli, Manikanta" <manikanta.guntupalli@....com>
To: Jorge Marques <gastmaier@...il.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

[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.

Shall we proceed with only the software reset changes along with an improved description, or do you recommend any additional modifications?

Thanks,
Manikanta.

Powered by blists - more mailing lists