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

[Nuno Sá]

On Tue, 2025-07-22 at 07:19 +0000, Guntupalli, Manikanta wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi @Andy Shevchenko,
> 
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > Sent: Monday, July 21, 2025 5:10 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@xxxxxxx>
> > Cc: git (AMD-Xilinx) <git@xxxxxxx>; Simek, Michal <michal.simek@xxxxxxx>;
> > lorenzo@xxxxxxxxxx; jic23@xxxxxxxxxx; dlechner@xxxxxxxxxxxx;
> > nuno.sa@xxxxxxxxxx; andy@xxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@xxxxxxx>; Goud, Srinivas <srinivas.goud@xxxxxxx>;
> > manion05gk@xxxxxxxxx
> > Subject: Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for
> > I3C
> > interface
> > 
> > On Mon, Jul 21, 2025 at 02:38:42PM +0300, 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.
> > > 
> > > > To support this, the previously static st_lsm6dsx_reset_device()
> > > > function is now exported via EXPORT_SYMBOL_NS() under the
> > > > IIO_LSM6DSX namespace, allowing it to be invoked from the I3C-specific
> > > > driver.
> > > 
> > > Why system suspend callback can't do this?
> > 
> > Ah, and why only I3C is important? Doesn't I2C or SPI also broken in this
> > sense?
> 
> There is no device enumeration process involved for I2C and SPI, so they are
> not impacted.
> 
> However, for I3C, device enumeration does occur. During this process, the
> device PID and BCR/DCR values are compared against the entries defined 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 there is a match, the probe function will be called.
> 
> Additionally, the sensor reset logic is implemented inside the probe.
> Therefore, to ensure the sensor responds correctly during device enumeration
> after a reboot (such as after kexec), it is necessary to reset the sensor
> during the shutdown phase.
> 

Hmm I see. I was going to ask why can't we just do this during probe().

- Nuno Sá

> Thanks,
> Manikanta.