linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
@ 2025-07-21 11:07 Manikanta Guntupalli
  2025-07-21 11:38 ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Manikanta Guntupalli @ 2025-07-21 11:07 UTC (permalink / raw)
  To: git, michal.simek, lorenzo, jic23, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel
  Cc: radhey.shyam.pandey, srinivas.goud, manion05gk,
	Manikanta Guntupalli

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.

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.

Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  | 14 ++++++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index c225b246c8a5..42c0dcfbad49 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -466,6 +466,7 @@ extern const struct dev_pm_ops st_lsm6dsx_pm_ops;
 
 int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 		     struct regmap *regmap);
+int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw);
 int st_lsm6dsx_sensor_set_enable(struct st_lsm6dsx_sensor *sensor,
 				 bool enable);
 int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw);
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index c65ad49829e7..929b30985d41 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -2267,7 +2267,7 @@ static int st_lsm6dsx_init_hw_timer(struct st_lsm6dsx_hw *hw)
 	return 0;
 }
 
-static int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw)
+int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw)
 {
 	const struct st_lsm6dsx_reg *reg;
 	int err;
@@ -2302,6 +2302,7 @@ static int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw)
 
 	return 0;
 }
+EXPORT_SYMBOL_NS(st_lsm6dsx_reset_device, "IIO_LSM6DSX");
 
 static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
index cb5c5d7e1f3d..f3d9cdd5a743 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
@@ -41,10 +41,24 @@ static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
 	return st_lsm6dsx_probe(dev, 0, (uintptr_t)id->data, regmap);
 }
 
+static void st_lsm6dsx_i3c_shutdown(struct device *dev)
+{
+	struct st_lsm6dsx_hw *hw = dev_get_drvdata(dev);
+
+	/*
+	 * Perform device reset to ensure the sensor is in a known
+	 * good state for subsequent re-initialization or power cycles.
+	 * This addresses issues where the sensor might not enumerate
+	 * correctly after a warm reboot (e.g., kexec).
+	 */
+	st_lsm6dsx_reset_device(hw);
+}
+
 static struct i3c_driver st_lsm6dsx_driver = {
 	.driver = {
 		.name = "st_lsm6dsx_i3c",
 		.pm = pm_sleep_ptr(&st_lsm6dsx_pm_ops),
+		.shutdown = st_lsm6dsx_i3c_shutdown,
 	},
 	.probe = st_lsm6dsx_i3c_probe,
 	.id_table = st_lsm6dsx_i3c_ids,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-21 11:07 [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface Manikanta Guntupalli
@ 2025-07-21 11:38 ` Andy Shevchenko
  2025-07-21 11:39   ` Andy Shevchenko
  2025-07-21 21:01   ` David Lechner
  0 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2025-07-21 11:38 UTC (permalink / raw)
  To: Manikanta Guntupalli
  Cc: git, michal.simek, lorenzo, jic23, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel, radhey.shyam.pandey, srinivas.goud,
	manion05gk

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?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-21 11:38 ` Andy Shevchenko
@ 2025-07-21 11:39   ` Andy Shevchenko
  2025-07-22  7:19     ` Guntupalli, Manikanta
  2025-07-21 21:01   ` David Lechner
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2025-07-21 11:39 UTC (permalink / raw)
  To: Manikanta Guntupalli
  Cc: git, michal.simek, lorenzo, jic23, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel, radhey.shyam.pandey, srinivas.goud,
	manion05gk

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?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-21 11:38 ` Andy Shevchenko
  2025-07-21 11:39   ` Andy Shevchenko
@ 2025-07-21 21:01   ` David Lechner
  2025-07-22  7:32     ` Guntupalli, Manikanta
  1 sibling, 1 reply; 15+ messages in thread
From: David Lechner @ 2025-07-21 21:01 UTC (permalink / raw)
  To: Andy Shevchenko, Manikanta Guntupalli
  Cc: git, michal.simek, lorenzo, jic23, nuno.sa, andy, linux-iio,
	linux-kernel, radhey.shyam.pandey, srinivas.goud, manion05gk

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-21 11:39   ` Andy Shevchenko
@ 2025-07-22  7:19     ` Guntupalli, Manikanta
  2025-09-19 11:10       ` Nuno Sá
  0 siblings, 1 reply; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-07-22  7:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: git (AMD-Xilinx), Simek, Michal, lorenzo@kernel.org,
	jic23@kernel.org, dlechner@baylibre.com, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

[AMD Official Use Only - AMD Internal Distribution Only]

Hi @Andy Shevchenko,

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Monday, July 21, 2025 5:10 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>;
> lorenzo@kernel.org; jic23@kernel.org; dlechner@baylibre.com;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> 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.

Thanks,
Manikanta.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-21 21:01   ` David Lechner
@ 2025-07-22  7:32     ` Guntupalli, Manikanta
  2025-07-22  7:56       ` Jorge Marques
  0 siblings, 1 reply; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-07-22  7:32 UTC (permalink / raw)
  To: David Lechner, Andy Shevchenko
  Cc: git (AMD-Xilinx), Simek, Michal, lorenzo@kernel.org,
	jic23@kernel.org, nuno.sa@analog.com, andy@kernel.org,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pandey, Radhey Shyam, Goud, Srinivas, manion05gk@gmail.com

[AMD Official Use Only - AMD Internal Distribution Only]

Hi @David Lechner,

> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Tuesday, July 22, 2025 2:31 AM
> To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli, Manikanta
> <manikanta.guntupalli@amd.com>
> Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>;
> lorenzo@kernel.org; jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.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.

Thanks,
Manikanta.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-22  7:32     ` Guntupalli, Manikanta
@ 2025-07-22  7:56       ` Jorge Marques
  2025-07-29 12:02         ` Guntupalli, Manikanta
  0 siblings, 1 reply; 15+ messages in thread
From: Jorge Marques @ 2025-07-22  7:56 UTC (permalink / raw)
  To: Guntupalli, Manikanta
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

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@baylibre.com>
> > Sent: Tuesday, July 22, 2025 2:31 AM
> > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli, Manikanta
> > <manikanta.guntupalli@amd.com>
> > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>;
> > lorenzo@kernel.org; jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.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@i3c_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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-22  7:56       ` Jorge Marques
@ 2025-07-29 12:02         ` Guntupalli, Manikanta
  2025-07-29 12:49           ` Jorge Marques
  2025-09-19 11:18           ` Nuno Sá
  0 siblings, 2 replies; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-07-29 12:02 UTC (permalink / raw)
  To: Jorge Marques
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

[AMD Official Use Only - AMD Internal Distribution Only]

Hi @Jorge Marques,

> -----Original Message-----
> From: Jorge Marques <gastmaier@gmail.com>
> Sent: Tuesday, July 22, 2025 1:27 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.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@baylibre.com>
> > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > Manikanta <manikanta.guntupalli@amd.com>
> > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> > > <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-29 12:02         ` Guntupalli, Manikanta
@ 2025-07-29 12:49           ` Jorge Marques
  2025-07-30  6:27             ` Guntupalli, Manikanta
  2025-09-19 11:18           ` Nuno Sá
  1 sibling, 1 reply; 15+ messages in thread
From: Jorge Marques @ 2025-07-29 12:49 UTC (permalink / raw)
  To: Guntupalli, Manikanta
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

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@gmail.com>
> > Sent: Tuesday, July 22, 2025 1:27 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.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@baylibre.com>
> > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > > Manikanta <manikanta.guntupalli@amd.com>
> > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > > <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-29 12:49           ` Jorge Marques
@ 2025-07-30  6:27             ` Guntupalli, Manikanta
  2025-09-05  5:29               ` Guntupalli, Manikanta
  0 siblings, 1 reply; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-07-30  6:27 UTC (permalink / raw)
  To: Jorge Marques
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

[AMD Official Use Only - AMD Internal Distribution Only]

Hi,

> -----Original Message-----
> From: Jorge Marques <gastmaier@gmail.com>
> Sent: Tuesday, July 29, 2025 6:19 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.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@gmail.com>
> > > Sent: Tuesday, July 22, 2025 1:27 PM
> > > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>;
> > > Simek, Michal <michal.simek@amd.com>; lorenzo@kernel.org;
> > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > linux-iio@vger.kernel.org; linux- kernel@vger.kernel.org; Pandey,
> > > Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > <srinivas.goud@amd.com>; manion05gk@gmail.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@baylibre.com>
> > > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > > > Manikanta <manikanta.guntupalli@amd.com>
> > > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > > > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org;
> > > > > linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> > > > > <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > > > <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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.
We are unable to verify the behavior on any silicon other than ours. It would be helpful if someone with access to a different silicon could confirm this behavior.

Thanks,
Manikanta.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-30  6:27             ` Guntupalli, Manikanta
@ 2025-09-05  5:29               ` Guntupalli, Manikanta
  2025-09-18  7:22                 ` Mario TESI
  0 siblings, 1 reply; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-09-05  5:29 UTC (permalink / raw)
  To: Jorge Marques, mario.tesi@st.com
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

[Public]

+ @mario.tesi@st.com

Hi @mario.tesi,
We are observing an enumeration issue with the LSM6DSX sensor on I3C,
since enumeration occurs only there. During a kexec reboot, the LSM6DSX
does not respond to CCC commands during enumeration.
Adding a software reset in shutdown() resolves this and allows enumeration to succeed.

Could you please confirm this behavior of the LSM6DSX on I3C from your side ?
Your confirmation will help us decide whether to proceed with just the software
reset in the driver or consider additional steps.

> -----Original Message-----
> From: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Sent: Wednesday, July 30, 2025 11:58 AM
> To: Jorge Marques <gastmaier@gmail.com>
> Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> Subject: RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
> interface
>
>
> Hi,
>
> > -----Original Message-----
> > From: Jorge Marques <gastmaier@gmail.com>
> > Sent: Tuesday, July 29, 2025 6:19 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek,
> > Michal <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.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@gmail.com>
> > > > Sent: Tuesday, July 22, 2025 1:27 PM
> > > > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > > > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>;
> > > > Simek, Michal <michal.simek@amd.com>; lorenzo@kernel.org;
> > > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > > linux-iio@vger.kernel.org; linux- kernel@vger.kernel.org; Pandey,
> > > > Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > > <srinivas.goud@amd.com>; manion05gk@gmail.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@baylibre.com>
> > > > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > > > > Manikanta <manikanta.guntupalli@amd.com>
> > > > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > > > > nuno.sa@analog.com; andy@kernel.org;
> > > > > > linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > > Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud,
> > > > > > Srinivas <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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.
> We are unable to verify the behavior on any silicon other than ours. It would be
> helpful if someone with access to a different silicon could confirm this behavior.
>

Thanks,
Manikanta.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-09-05  5:29               ` Guntupalli, Manikanta
@ 2025-09-18  7:22                 ` Mario TESI
  2025-09-19  9:22                   ` Guntupalli, Manikanta
  0 siblings, 1 reply; 15+ messages in thread
From: Mario TESI @ 2025-09-18  7:22 UTC (permalink / raw)
  To: Guntupalli, Manikanta, Jorge Marques
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

Hi Manikanta,

The mainline lsm6dsx driver supports several IMUs, but on the I3C bus, it supports two: lam6dso and lam6dsr.
This is very strange because the software reset doesn't reset the dynamic address on such devices; only an RSTDAA or a power cycle can reset it. Looking at how the I3C master works, the first thing it requests during the probe phase is an RSTDAA, which is sufficient to reset the I3C address. Therefore, the IMU sensors should then participate in the new assignment with the ENTDAA or SETDASA command (in case of assigned address). The software reset you perform during shutdown likely deactivates the sensor, which solves any issues that may exist with the various I3C drivers (many of which are under development or debugging). It would be interesting to evaluate the I3C trace when the issue occurs and also a register dump to better understand where to focus.

Best Regards,
Mario

________________________________________
Da: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
Inviato: venerdì 5 settembre 2025 07:29:18
A: Jorge Marques; Mario TESI
Cc: David Lechner; Andy Shevchenko; git (AMD-Xilinx); Simek, Michal; lorenzo@kernel.org; jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandey, Radhey Shyam; Goud, Srinivas; manion05gk@gmail.com
Oggetto: RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface

[Public]

+ @mario.tesi@st.com

Hi @mario.tesi,
We are observing an enumeration issue with the LSM6DSX sensor on I3C,
since enumeration occurs only there. During a kexec reboot, the LSM6DSX
does not respond to CCC commands during enumeration.
Adding a software reset in shutdown() resolves this and allows enumeration to succeed.

Could you please confirm this behavior of the LSM6DSX on I3C from your side ?
Your confirmation will help us decide whether to proceed with just the software
reset in the driver or consider additional steps.

> -----Original Message-----
> From: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Sent: Wednesday, July 30, 2025 11:58 AM
> To: Jorge Marques <gastmaier@gmail.com>
> Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> Subject: RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
> interface
>
>
> Hi,
>
> > -----Original Message-----
> > From: Jorge Marques <gastmaier@gmail.com>
> > Sent: Tuesday, July 29, 2025 6:19 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek,
> > Michal <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.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@gmail.com>
> > > > Sent: Tuesday, July 22, 2025 1:27 PM
> > > > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > > > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>;
> > > > Simek, Michal <michal.simek@amd.com>; lorenzo@kernel.org;
> > > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > > linux-iio@vger.kernel.org; linux- kernel@vger.kernel.org; Pandey,
> > > > Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > > <srinivas.goud@amd.com>; manion05gk@gmail.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@baylibre.com>
> > > > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > > > > Manikanta <manikanta.guntupalli@amd.com>
> > > > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > > > > nuno.sa@analog.com; andy@kernel.org;
> > > > > > linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > > Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud,
> > > > > > Srinivas <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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.
> We are unable to verify the behavior on any silicon other than ours. It would be
> helpful if someone with access to a different silicon could confirm this behavior.
>

Thanks,
Manikanta.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-09-18  7:22                 ` Mario TESI
@ 2025-09-19  9:22                   ` Guntupalli, Manikanta
  0 siblings, 0 replies; 15+ messages in thread
From: Guntupalli, Manikanta @ 2025-09-19  9:22 UTC (permalink / raw)
  To: Mario TESI, Jorge Marques
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

[Public]

Hi @Mario TESI,
Thanks for the response.

We don't have the flexibility to provide traces or register dumps. However, the issue is straightforward to reproduce with a kexec reboot.

On the first boot, everything works fine: the sensor responds to the full enumeration sequence, including RSTDAA to reset the dynamic address and ENTDAA for assigning a new dynamic address. At this point, the sensor driver probe is called successfully.

On the subsequent boot via kexec reboot, the sensor driver probe is not called because the sensor does not respond to the enumeration process (RSTDAA and ENTDAA).

When we add a software reset in the shutdown handler of the sensor driver, the sensor once again responds correctly to RSTDAA and ENTDAA, and the driver probe is called as expected during kexec reboot.

Note: We don't observe this behavior with other I3C devices, which helps isolate the issue to this specific sensor.

I hope this provides better clarity.

Thanks,
Manikanta.

> -----Original Message-----
> From: Mario TESI <mario.tesi@st.com>
> Sent: Thursday, September 18, 2025 12:53 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; Jorge Marques
> <gastmaier@gmail.com>
> Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> manion05gk@gmail.com
> Subject: Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
> interface
>
> Hi Manikanta,
>
> The mainline lsm6dsx driver supports several IMUs, but on the I3C bus, it supports
> two: lam6dso and lam6dsr.
> This is very strange because the software reset doesn't reset the dynamic address
> on such devices; only an RSTDAA or a power cycle can reset it. Looking at how the
> I3C master works, the first thing it requests during the probe phase is an RSTDAA,
> which is sufficient to reset the I3C address. Therefore, the IMU sensors should then
> participate in the new assignment with the ENTDAA or SETDASA command (in case
> of assigned address). The software reset you perform during shutdown likely
> deactivates the sensor, which solves any issues that may exist with the various I3C
> drivers (many of which are under development or debugging). It would be interesting
> to evaluate the I3C trace when the issue occurs and also a register dump to better
> understand where to focus.
>
> Best Regards,
> Mario
>
> ________________________________________
> Da: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> Inviato: venerdì 5 settembre 2025 07:29:18
> A: Jorge Marques; Mario TESI
> Cc: David Lechner; Andy Shevchenko; git (AMD-Xilinx); Simek, Michal;
> lorenzo@kernel.org; jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandey, Radhey Shyam;
> Goud, Srinivas; manion05gk@gmail.com
> Oggetto: RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C
> interface
>
> [Public]
>
> + @mario.tesi@st.com
>
> Hi @mario.tesi,
> We are observing an enumeration issue with the LSM6DSX sensor on I3C, since
> enumeration occurs only there. During a kexec reboot, the LSM6DSX does not
> respond to CCC commands during enumeration.
> Adding a software reset in shutdown() resolves this and allows enumeration to
> succeed.
>
> Could you please confirm this behavior of the LSM6DSX on I3C from your side ?
> Your confirmation will help us decide whether to proceed with just the software reset
> in the driver or consider additional steps.
>
> > -----Original Message-----
> > From: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Sent: Wednesday, July 30, 2025 11:58 AM
> > To: Jorge Marques <gastmaier@gmail.com>
> > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek,
> > Michal <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.com
> > Subject: RE: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support
> > for I3C interface
> >
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Jorge Marques <gastmaier@gmail.com>
> > > Sent: Tuesday, July 29, 2025 6:19 PM
> > > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>;
> > > Simek, Michal <michal.simek@amd.com>; lorenzo@kernel.org;
> > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > linux-iio@vger.kernel.org; linux- kernel@vger.kernel.org; Pandey,
> > > Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > <srinivas.goud@amd.com>; manion05gk@gmail.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@gmail.com>
> > > > > Sent: Tuesday, July 22, 2025 1:27 PM
> > > > > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > > > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > > > > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>;
> > > > > Simek, Michal <michal.simek@amd.com>; lorenzo@kernel.org;
> > > > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > > > linux-iio@vger.kernel.org; linux- kernel@vger.kernel.org;
> > > > > Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud,
> > > > > Srinivas <srinivas.goud@amd.com>; manion05gk@gmail.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@baylibre.com>
> > > > > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>;
> > > > > > > Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > > > > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > > > > <michal.simek@amd.com>; lorenzo@kernel.org;
> > > > > > > jic23@kernel.org; nuno.sa@analog.com; andy@kernel.org;
> > > > > > > linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > > > Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Goud,
> > > > > > > Srinivas <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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.
> > We are unable to verify the behavior on any silicon other than ours.
> > It would be helpful if someone with access to a different silicon could confirm this
> behavior.
> >
>
> Thanks,
> Manikanta.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-22  7:19     ` Guntupalli, Manikanta
@ 2025-09-19 11:10       ` Nuno Sá
  0 siblings, 0 replies; 15+ messages in thread
From: Nuno Sá @ 2025-09-19 11:10 UTC (permalink / raw)
  To: Guntupalli, Manikanta, Andy Shevchenko
  Cc: git (AMD-Xilinx), Simek, Michal, lorenzo@kernel.org,
	jic23@kernel.org, dlechner@baylibre.com, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

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@intel.com>
> > Sent: Monday, July 21, 2025 5:10 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>;
> > lorenzo@kernel.org; jic23@kernel.org; dlechner@baylibre.com;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.com
> > 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface
  2025-07-29 12:02         ` Guntupalli, Manikanta
  2025-07-29 12:49           ` Jorge Marques
@ 2025-09-19 11:18           ` Nuno Sá
  1 sibling, 0 replies; 15+ messages in thread
From: Nuno Sá @ 2025-09-19 11:18 UTC (permalink / raw)
  To: Guntupalli, Manikanta, Jorge Marques
  Cc: David Lechner, Andy Shevchenko, git (AMD-Xilinx), Simek, Michal,
	lorenzo@kernel.org, jic23@kernel.org, nuno.sa@analog.com,
	andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pandey, Radhey Shyam,
	Goud, Srinivas, manion05gk@gmail.com

On Tue, 2025-07-29 at 12:02 +0000, Guntupalli, Manikanta wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi @Jorge Marques,
> 
> > -----Original Message-----
> > From: Jorge Marques <gastmaier@gmail.com>
> > Sent: Tuesday, July 22, 2025 1:27 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>
> > Cc: David Lechner <dlechner@baylibre.com>; Andy Shevchenko
> > <andriy.shevchenko@intel.com>; git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; Goud, Srinivas <srinivas.goud@amd.com>;
> > manion05gk@gmail.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@baylibre.com>
> > > > Sent: Tuesday, July 22, 2025 2:31 AM
> > > > To: Andy Shevchenko <andriy.shevchenko@intel.com>; Guntupalli,
> > > > Manikanta <manikanta.guntupalli@amd.com>
> > > > Cc: git (AMD-Xilinx) <git@amd.com>; Simek, Michal
> > > > <michal.simek@amd.com>; lorenzo@kernel.org; jic23@kernel.org;
> > > > nuno.sa@analog.com; andy@kernel.org; linux-iio@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@amd.com>; Goud, Srinivas
> > > > <srinivas.goud@amd.com>; manion05gk@gmail.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@i3c_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 seems the device has some PM ops implemented [1]. Can it be that those are
being called somehow? I don't think they should be called during shutdown but it
would at least explain the low power state.

[1]: https://elixir.bootlin.com/linux/v6.17-rc6/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L2730

- Nuno Sá

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-09-19 11:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21 11:07 [PATCH] iio: imu: lsm6dsx: Add shutdown callback support for I3C interface Manikanta Guntupalli
2025-07-21 11:38 ` Andy Shevchenko
2025-07-21 11:39   ` Andy Shevchenko
2025-07-22  7:19     ` Guntupalli, Manikanta
2025-09-19 11:10       ` Nuno Sá
2025-07-21 21:01   ` David Lechner
2025-07-22  7:32     ` Guntupalli, Manikanta
2025-07-22  7:56       ` Jorge Marques
2025-07-29 12:02         ` Guntupalli, Manikanta
2025-07-29 12:49           ` Jorge Marques
2025-07-30  6:27             ` Guntupalli, Manikanta
2025-09-05  5:29               ` Guntupalli, Manikanta
2025-09-18  7:22                 ` Mario TESI
2025-09-19  9:22                   ` Guntupalli, Manikanta
2025-09-19 11:18           ` Nuno Sá

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).