[PATCH] iio: ssp: Serialize watchdog timer state changes

[PATCH] iio: ssp: Serialize watchdog timer state changes

[Runyu Xiao]

The SSP watchdog timer rearms itself from its callback, but the driver uses
timer_delete_sync() when the last sensor is disabled and during suspend.
Those operations do not prevent a concurrent enable or callback from
rearming the timer after the deletion has completed.  The final remove path
also used timer_delete_sync(), which does not provide the shutdown
guarantee needed before releasing the device state.

Protect the watchdog state and enable reference count with a mutex.  The
callback checks a state flag before rearming. Reusable stops clear the flag
before deleting the timer. Use timer_shutdown_sync() for the final remove
path so that any later rearm attempt is rejected permanently.

Cancel watchdog work after releasing wdt_lock because the reset work can
wait for the threaded IRQ handler, which may synchronously wait for refresh
work that re-enables sensors and takes wdt_lock. Remove the MFD children
before destroying the locks because IIO child teardown can disable an
active sensor.

Fixes: 50dd64d57eee ("iio: common: ssp_sensors: Add sensorhub driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/iio/common/ssp_sensors/ssp.h     |  4 ++
 drivers/iio/common/ssp_sensors/ssp_dev.c | 56 ++++++++++++++++++++----
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp.h b/drivers/iio/common/ssp_sensors/ssp.h
index f649cdecc2774..2aa70eff06fc4 100644
--- a/drivers/iio/common/ssp_sensors/ssp.h
+++ b/drivers/iio/common/ssp_sensors/ssp.h
@@ -143,6 +143,8 @@ struct ssp_sensorhub_info {
  * @spi:		spi device
  * @sensorhub_info:	info about sensorhub board specific features
  * @wdt_timer:		watchdog timer
+ * @wdt_lock:		lock protecting watchdog timer state
+ * @wdt_enabled:		watchdog timer is allowed to rearm
  * @work_wdt:		watchdog work
  * @work_firmware:	firmware upgrade work queue
  * @work_refresh:	refresh work queue for reset request from MCU
@@ -180,6 +182,8 @@ struct ssp_data {
 	struct spi_device *spi;
 	const struct ssp_sensorhub_info *sensorhub_info;
 	struct timer_list wdt_timer;
+	struct mutex wdt_lock; /* protects watchdog timer state */
+	bool wdt_enabled;
 	struct work_struct work_wdt;
 	struct delayed_work work_refresh;
 
diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index 828fcfe1d4f10..44dfafcaeff71 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -179,17 +179,46 @@ static void ssp_wdt_timer_func(struct timer_list *t)
 	    data->com_fail_cnt > SSP_LIMIT_RESET_CNT)
 		queue_work(system_power_efficient_wq, &data->work_wdt);
 _mod:
+	if (READ_ONCE(data->wdt_enabled))
+		mod_timer(&data->wdt_timer,
+			  jiffies + msecs_to_jiffies(SSP_WDT_TIME));
+}
+
+static void __ssp_enable_wdt_timer(struct ssp_data *data)
+{
+	WRITE_ONCE(data->wdt_enabled, true);
 	mod_timer(&data->wdt_timer, jiffies + msecs_to_jiffies(SSP_WDT_TIME));
 }
 
+static void __ssp_stop_wdt_timer(struct ssp_data *data, bool shutdown)
+{
+	WRITE_ONCE(data->wdt_enabled, false);
+	if (shutdown)
+		timer_shutdown_sync(&data->wdt_timer);
+	else
+		timer_delete_sync(&data->wdt_timer);
+}
+
 static void ssp_enable_wdt_timer(struct ssp_data *data)
 {
-	mod_timer(&data->wdt_timer, jiffies + msecs_to_jiffies(SSP_WDT_TIME));
+	mutex_lock(&data->wdt_lock);
+	__ssp_enable_wdt_timer(data);
+	mutex_unlock(&data->wdt_lock);
 }
 
 static void ssp_disable_wdt_timer(struct ssp_data *data)
 {
-	timer_delete_sync(&data->wdt_timer);
+	mutex_lock(&data->wdt_lock);
+	__ssp_stop_wdt_timer(data, false);
+	mutex_unlock(&data->wdt_lock);
+	cancel_work_sync(&data->work_wdt);
+}
+
+static void ssp_shutdown_wdt_timer(struct ssp_data *data)
+{
+	mutex_lock(&data->wdt_lock);
+	__ssp_stop_wdt_timer(data, true);
+	mutex_unlock(&data->wdt_lock);
 	cancel_work_sync(&data->work_wdt);
 }
 
@@ -258,8 +287,10 @@ int ssp_enable_sensor(struct ssp_data *data, enum ssp_sensor_type type,
 
 	data->delay_buf[type] = delay;
 
+	mutex_lock(&data->wdt_lock);
 	if (atomic_inc_return(&data->enable_refcount) == 1)
-		ssp_enable_wdt_timer(data);
+		__ssp_enable_wdt_timer(data);
+	mutex_unlock(&data->wdt_lock);
 
 	return 0;
 
@@ -310,6 +341,7 @@ EXPORT_SYMBOL_NS(ssp_change_delay, "IIO_SSP_SENSORS");
 int ssp_disable_sensor(struct ssp_data *data, enum ssp_sensor_type type)
 {
 	int ret;
+	bool stop_wdt = false;
 	__le32 command;
 
 	if (data->sensor_enable & BIT(type)) {
@@ -329,8 +361,14 @@ int ssp_disable_sensor(struct ssp_data *data, enum ssp_sensor_type type)
 
 	data->check_status[type] = SSP_ADD_SENSOR_STATE;
 
+	mutex_lock(&data->wdt_lock);
 	if (atomic_dec_and_test(&data->enable_refcount))
-		ssp_disable_wdt_timer(data);
+		stop_wdt = true;
+	if (stop_wdt)
+		__ssp_stop_wdt_timer(data, false);
+	mutex_unlock(&data->wdt_lock);
+	if (stop_wdt)
+		cancel_work_sync(&data->work_wdt);
 
 	return 0;
 }
@@ -510,6 +548,7 @@ static int ssp_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, data);
 
 	mutex_init(&data->comm_lock);
+	mutex_init(&data->wdt_lock);
 
 	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
 		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
@@ -566,6 +605,7 @@ static int ssp_probe(struct spi_device *spi)
 	free_irq(data->spi->irq, data);
 err_setup_irq:
 	mutex_destroy(&data->pending_lock);
+	mutex_destroy(&data->wdt_lock);
 	mutex_destroy(&data->comm_lock);
 err_setup_spi:
 	mfd_remove_devices(&spi->dev);
@@ -584,20 +624,18 @@ static void ssp_remove(struct spi_device *spi)
 			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
 
 	ssp_enable_mcu(data, false);
-	ssp_disable_wdt_timer(data);
+	ssp_shutdown_wdt_timer(data);
 
 	ssp_clean_pending_list(data);
 
 	free_irq(data->spi->irq, data);
 	cancel_delayed_work_sync(&data->work_refresh);
 
-	timer_delete_sync(&data->wdt_timer);
-	cancel_work_sync(&data->work_wdt);
+	mfd_remove_devices(&spi->dev);
 
 	mutex_destroy(&data->comm_lock);
+	mutex_destroy(&data->wdt_lock);
 	mutex_destroy(&data->pending_lock);
-
-	mfd_remove_devices(&spi->dev);
 }
 
 static int ssp_suspend(struct device *dev)
-- 
2.34.1

Re: [PATCH] iio: ssp: Serialize watchdog timer state changes

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 03:03:19PM +0800, Runyu Xiao wrote:
> The SSP watchdog timer rearms itself from its callback, but the driver uses
> timer_delete_sync() when the last sensor is disabled and during suspend.
> Those operations do not prevent a concurrent enable or callback from
> rearming the timer after the deletion has completed.  The final remove path
> also used timer_delete_sync(), which does not provide the shutdown
> guarantee needed before releasing the device state.
> 
> Protect the watchdog state and enable reference count with a mutex.  The
> callback checks a state flag before rearming. Reusable stops clear the flag
> before deleting the timer. Use timer_shutdown_sync() for the final remove
> path so that any later rearm attempt is rejected permanently.
> 
> Cancel watchdog work after releasing wdt_lock because the reset work can
> wait for the threaded IRQ handler, which may synchronously wait for refresh
> work that re-enables sensors and takes wdt_lock. Remove the MFD children
> before destroying the locks because IIO child teardown can disable an
> active sensor.

...

> struct ssp_data {

>  	struct spi_device *spi;
>  	const struct ssp_sensorhub_info *sensorhub_info;
>  	struct timer_list wdt_timer;
> +	struct mutex wdt_lock; /* protects watchdog timer state */
> +	bool wdt_enabled;
>  	struct work_struct work_wdt;
>  	struct delayed_work work_refresh;

Does `pahole` agree with the given layout?

>  _mod:
> +	if (READ_ONCE(data->wdt_enabled))

What are we going to do if just after this wdt_enabled becomes false?
(Is it a possible case?)

> +		mod_timer(&data->wdt_timer,
> +			  jiffies + msecs_to_jiffies(SSP_WDT_TIME));
> +}

Same Q to all the below. Hmm... It seems they are all protected by the mutex?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: ssp: Serialize watchdog timer state changes

[Runyu Xiao]

Hi Andy,

Thanks for the review.

On Wed, Sep 30, 2026 at 03:03:19PM +0800, Runyu Xiao wrote:
> The SSP watchdog timer rearms itself from its callback, but the driver uses
> timer_delete_sync() when the last sensor is disabled and during suspend.
> Those operations do not prevent a concurrent enable or callback from
> rearming the timer after the deletion has completed. The final remove path
> also used timer_delete_sync(), which does not provide the shutdown
> guarantee needed before releasing the device state.
>
> Protect the watchdog state and enable reference count with a mutex. The
> callback checks a state flag before rearming. Reusable stops clear the
> flag before deleting the timer. Use timer_shutdown_sync() for the final
> remove path so that any later rearm attempt is rejected permanently.
>
> struct ssp_data {
>     struct timer_list wdt_timer;
>     struct mutex wdt_lock;
>     bool wdt_enabled;
>     struct work_struct work_wdt;
> };

On the x86_64 build, pahole reports wdt_timer at offset 16, wdt_lock at
56, wdt_enabled at 80, and work_wdt at 88; sizeof(struct ssp_data) is
816 bytes. There is a 7-byte hole after wdt_enabled.

> _mod:
> + if (READ_ONCE(data->wdt_enabled))
> +     mod_timer(&data->wdt_timer, ...);
>
> What are we going to do if just after this wdt_enabled becomes false?
> (Is it a possible case?)

Yes, that is possible. The callback may read true before the stop path
writes false, then rearm the timer once. timer_delete_sync() waits for the
running callback and removes that rearmed instance before returning.

> Same Q to all the below. Hmm... It seems they are all protected by the
> mutex?

All process-context start and stop paths use wdt_lock. The timer callback
is the exception because it cannot take a mutex; the state check and
synchronous deletion handle its in-flight rearm. The mutex serializes
operations, but a start that runs after a reusable stop releases the lock
can arm the timer again. That is expected after an ordinary sensor enable.

There is also a suspend interleaving. A pending work_refresh can call
ssp_sync_available_sensors() and ssp_enable_sensor() after ssp_suspend()
checks enable_refcount. If the count is zero, the first successful sensor
enable can arm the timer. The first version did not close this path.

In v2, suspend state is tracked under wdt_lock. Sensor enables can update the
count while suspended but cannot arm the timer. It is restarted only after
resume succeeds, or if suspend fails. Watchdog work is synchronized before
the suspend command. I will send v2 separately.

Final removal uses timer_shutdown_sync() to reject later rearm attempts.

Thanks,
Runyu

Re: [PATCH] iio: ssp: Serialize watchdog timer state changes

[Andriy Shevchenko]

On Sun, Oct 04, 2026 at 01:39:15PM +0800, Runyu Xiao wrote:
> On Wed, Sep 30, 2026 at 03:03:19PM +0800, Runyu Xiao wrote:

...

> > struct ssp_data {
> >     struct timer_list wdt_timer;
> >     struct mutex wdt_lock;
> >     bool wdt_enabled;
> >     struct work_struct work_wdt;
> > };
> 
> On the x86_64 build, pahole reports wdt_timer at offset 16, wdt_lock at
> 56, wdt_enabled at 80, and work_wdt at 88; sizeof(struct ssp_data) is
> 816 bytes. There is a 7-byte hole after wdt_enabled.

Right, so you can make it 3, correct?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: ssp: Serialize watchdog timer state changes

[Runyu Xiao]

On Sun, Oct 04, 2026 at 11:37:59AM +0300, Andriy Shevchenko wrote:
> General rule of thumb is to defer a new version until the discussion is settled
> down in the previous round(s).
>
> Same Q here, can you reduce the gap by 4 bytes by rearranging the new members?

Yes. I checked both layouts with pahole on x86_64. In v2, wdt_enabled and
wdt_suspended are at offsets 80 and 81, and work_wdt starts at 88, leaving a
six-byte hole; sizeof(struct ssp_data) is 816 bytes.

Moving the two flags after time_syncing and before timestamp puts work_wdt at
offset 80 and the flags at offsets 203 and 204. There are then three bytes of
padding before timestamp at offset 208, and sizeof(struct ssp_data) is 808
bytes. This reduces the structure size by eight bytes, exceeding the four-byte
reduction requested. I will include this layout change with the other agreed
review changes in the next revision.

Thanks,
Runyu

[PATCH v2] iio: ssp: Serialize watchdog timer state changes

[Runyu Xiao]

The SSP watchdog timer rearms itself from its callback, but the driver uses
timer_delete_sync() when the last sensor is disabled and during suspend.
Those operations do not prevent a concurrent enable or callback from
rearming the timer after deletion. The final remove path also used
timer_delete_sync(), which does not permanently prevent rearming before the
device state is released.

Protect watchdog state and the enable reference count with wdt_lock. The
timer callback rearms only while the timer is enabled, and synchronous
deletion drains an in-flight callback. Use timer_shutdown_sync() for final
removal so later rearm attempts are rejected permanently.

A refresh work item can call ssp_sync_available_sensors() and
ssp_enable_sensor() during suspend. If the enable count is zero when
suspend checks it, that work can otherwise start the watchdog after the
suspend stop. Track the suspended state under wdt_lock. Sensor enables may
update the count without restarting the timer. Stop the watchdog before
sending the suspend command, and restart it only after resume succeeds or
suspend fails.

Cancel watchdog work after releasing wdt_lock because reset work can wait
for the threaded IRQ handler, which may synchronously wait for refresh work
that enables sensors. Remove MFD children before destroying the locks;
IIO child teardown can disable an active sensor.

Fixes: 50dd64d57eee ("iio: common: ssp_sensors: Add sensorhub driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Changes in v2:
- Keep the watchdog stopped while refresh work can re-enable sensors during
  suspend.
- Restart it only after resume succeeds or suspend fails.

 drivers/iio/common/ssp_sensors/ssp.h     |  6 ++
 drivers/iio/common/ssp_sensors/ssp_dev.c | 78 ++++++++++++++++++------
 2 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp.h b/drivers/iio/common/ssp_sensors/ssp.h
index f649cdecc..2e356a6bb 100644
--- a/drivers/iio/common/ssp_sensors/ssp.h
+++ b/drivers/iio/common/ssp_sensors/ssp.h
@@ -143,6 +143,9 @@ struct ssp_sensorhub_info {
  * @spi:		spi device
  * @sensorhub_info:	info about sensorhub board specific features
  * @wdt_timer:		watchdog timer
+ * @wdt_lock:		lock protecting watchdog timer state
+ * @wdt_enabled:		watchdog timer is allowed to rearm
+ * @wdt_suspended:	watchdog timer is stopped for system suspend
  * @work_wdt:		watchdog work
  * @work_firmware:	firmware upgrade work queue
  * @work_refresh:	refresh work queue for reset request from MCU
@@ -180,6 +183,9 @@ struct ssp_data {
 	struct spi_device *spi;
 	const struct ssp_sensorhub_info *sensorhub_info;
 	struct timer_list wdt_timer;
+	struct mutex wdt_lock; /* protects watchdog timer state */
+	bool wdt_enabled;
+	bool wdt_suspended;
 	struct work_struct work_wdt;
 	struct delayed_work work_refresh;
 
diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index 828fcfe1d..94d0dc2d3 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -179,17 +179,53 @@ static void ssp_wdt_timer_func(struct timer_list *t)
 	    data->com_fail_cnt > SSP_LIMIT_RESET_CNT)
 		queue_work(system_power_efficient_wq, &data->work_wdt);
 _mod:
-	mod_timer(&data->wdt_timer, jiffies + msecs_to_jiffies(SSP_WDT_TIME));
+	if (READ_ONCE(data->wdt_enabled))
+		mod_timer(&data->wdt_timer,
+			  jiffies + msecs_to_jiffies(SSP_WDT_TIME));
 }
 
-static void ssp_enable_wdt_timer(struct ssp_data *data)
+static void __ssp_enable_wdt_timer(struct ssp_data *data)
 {
+	if (data->wdt_suspended)
+		return;
+
+	WRITE_ONCE(data->wdt_enabled, true);
 	mod_timer(&data->wdt_timer, jiffies + msecs_to_jiffies(SSP_WDT_TIME));
 }
 
-static void ssp_disable_wdt_timer(struct ssp_data *data)
+static void __ssp_stop_wdt_timer(struct ssp_data *data, bool shutdown)
+{
+	WRITE_ONCE(data->wdt_enabled, false);
+	if (shutdown)
+		timer_shutdown_sync(&data->wdt_timer);
+	else
+		timer_delete_sync(&data->wdt_timer);
+}
+
+static void ssp_suspend_wdt_timer(struct ssp_data *data)
 {
-	timer_delete_sync(&data->wdt_timer);
+	mutex_lock(&data->wdt_lock);
+	data->wdt_suspended = true;
+	__ssp_stop_wdt_timer(data, false);
+	mutex_unlock(&data->wdt_lock);
+	cancel_work_sync(&data->work_wdt);
+}
+
+static void ssp_resume_wdt_timer(struct ssp_data *data)
+{
+	mutex_lock(&data->wdt_lock);
+	data->wdt_suspended = false;
+	if (atomic_read(&data->enable_refcount) > 0)
+		__ssp_enable_wdt_timer(data);
+	mutex_unlock(&data->wdt_lock);
+}
+
+static void ssp_shutdown_wdt_timer(struct ssp_data *data)
+{
+	mutex_lock(&data->wdt_lock);
+	data->wdt_suspended = true;
+	__ssp_stop_wdt_timer(data, true);
+	mutex_unlock(&data->wdt_lock);
 	cancel_work_sync(&data->work_wdt);
 }
 
@@ -258,8 +294,10 @@ int ssp_enable_sensor(struct ssp_data *data, enum ssp_sensor_type type,
 
 	data->delay_buf[type] = delay;
 
+	mutex_lock(&data->wdt_lock);
 	if (atomic_inc_return(&data->enable_refcount) == 1)
-		ssp_enable_wdt_timer(data);
+		__ssp_enable_wdt_timer(data);
+	mutex_unlock(&data->wdt_lock);
 
 	return 0;
 
@@ -310,6 +348,7 @@ EXPORT_SYMBOL_NS(ssp_change_delay, "IIO_SSP_SENSORS");
 int ssp_disable_sensor(struct ssp_data *data, enum ssp_sensor_type type)
 {
 	int ret;
+	bool stop_wdt = false;
 	__le32 command;
 
 	if (data->sensor_enable & BIT(type)) {
@@ -329,8 +368,14 @@ int ssp_disable_sensor(struct ssp_data *data, enum ssp_sensor_type type)
 
 	data->check_status[type] = SSP_ADD_SENSOR_STATE;
 
+	mutex_lock(&data->wdt_lock);
 	if (atomic_dec_and_test(&data->enable_refcount))
-		ssp_disable_wdt_timer(data);
+		stop_wdt = true;
+	if (stop_wdt)
+		__ssp_stop_wdt_timer(data, false);
+	mutex_unlock(&data->wdt_lock);
+	if (stop_wdt)
+		cancel_work_sync(&data->work_wdt);
 
 	return 0;
 }
@@ -510,6 +555,7 @@ static int ssp_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, data);
 
 	mutex_init(&data->comm_lock);
+	mutex_init(&data->wdt_lock);
 
 	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
 		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
@@ -566,6 +612,7 @@ static int ssp_probe(struct spi_device *spi)
 	free_irq(data->spi->irq, data);
 err_setup_irq:
 	mutex_destroy(&data->pending_lock);
+	mutex_destroy(&data->wdt_lock);
 	mutex_destroy(&data->comm_lock);
 err_setup_spi:
 	mfd_remove_devices(&spi->dev);
@@ -584,20 +631,18 @@ static void ssp_remove(struct spi_device *spi)
 			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
 
 	ssp_enable_mcu(data, false);
-	ssp_disable_wdt_timer(data);
+	ssp_shutdown_wdt_timer(data);
 
 	ssp_clean_pending_list(data);
 
 	free_irq(data->spi->irq, data);
 	cancel_delayed_work_sync(&data->work_refresh);
 
-	timer_delete_sync(&data->wdt_timer);
-	cancel_work_sync(&data->work_wdt);
+	mfd_remove_devices(&spi->dev);
 
 	mutex_destroy(&data->comm_lock);
+	mutex_destroy(&data->wdt_lock);
 	mutex_destroy(&data->pending_lock);
-
-	mfd_remove_devices(&spi->dev);
 }
 
 static int ssp_suspend(struct device *dev)
@@ -607,15 +652,14 @@ static int ssp_suspend(struct device *dev)
 
 	data->last_resume_state = SSP_MSG2SSP_AP_STATUS_SUSPEND;
 
-	if (atomic_read(&data->enable_refcount) > 0)
-		ssp_disable_wdt_timer(data);
+	ssp_suspend_wdt_timer(data);
 
 	ret = ssp_command(data, SSP_MSG2SSP_AP_STATUS_SUSPEND, 0);
 	if (ret < 0) {
 		dev_err(&data->spi->dev,
 			"%s SSP_MSG2SSP_AP_STATUS_SUSPEND failed\n", __func__);
 
-		ssp_enable_wdt_timer(data);
+		ssp_resume_wdt_timer(data);
 		return ret;
 	}
 
@@ -632,17 +676,15 @@ static int ssp_resume(struct device *dev)
 
 	enable_irq(data->spi->irq);
 
-	if (atomic_read(&data->enable_refcount) > 0)
-		ssp_enable_wdt_timer(data);
-
 	ret = ssp_command(data, SSP_MSG2SSP_AP_STATUS_RESUME, 0);
 	if (ret < 0) {
 		dev_err(&data->spi->dev,
 			"%s SSP_MSG2SSP_AP_STATUS_RESUME failed\n", __func__);
-		ssp_disable_wdt_timer(data);
 		return ret;
 	}
 
+	ssp_resume_wdt_timer(data);
+
 	/* timesyncing is set by MCU */
 	data->last_resume_state = SSP_MSG2SSP_AP_STATUS_RESUME;
 

Re: [PATCH v2] iio: ssp: Serialize watchdog timer state changes

[Andriy Shevchenko]

On Sun, Oct 04, 2026 at 01:39:27PM +0800, Runyu Xiao wrote:
> The SSP watchdog timer rearms itself from its callback, but the driver uses
> timer_delete_sync() when the last sensor is disabled and during suspend.
> Those operations do not prevent a concurrent enable or callback from
> rearming the timer after deletion. The final remove path also used
> timer_delete_sync(), which does not permanently prevent rearming before the
> device state is released.
> 
> Protect watchdog state and the enable reference count with wdt_lock. The
> timer callback rearms only while the timer is enabled, and synchronous
> deletion drains an in-flight callback. Use timer_shutdown_sync() for final
> removal so later rearm attempts are rejected permanently.
> 
> A refresh work item can call ssp_sync_available_sensors() and
> ssp_enable_sensor() during suspend. If the enable count is zero when
> suspend checks it, that work can otherwise start the watchdog after the
> suspend stop. Track the suspended state under wdt_lock. Sensor enables may
> update the count without restarting the timer. Stop the watchdog before
> sending the suspend command, and restart it only after resume succeeds or
> suspend fails.
> 
> Cancel watchdog work after releasing wdt_lock because reset work can wait
> for the threaded IRQ handler, which may synchronously wait for refresh work
> that enables sensors. Remove MFD children before destroying the locks;
> IIO child teardown can disable an active sensor.

General rule of thumb is to defer a new version until the discussion is settled
down in the previous round(s).

...

> struct ssp_data {

>  	struct spi_device *spi;
>  	const struct ssp_sensorhub_info *sensorhub_info;
>  	struct timer_list wdt_timer;
> +	struct mutex wdt_lock; /* protects watchdog timer state */
> +	bool wdt_enabled;
> +	bool wdt_suspended;
>  	struct work_struct work_wdt;
>  	struct delayed_work work_refresh;

Same Q here, can you reduce the gap by 4 bytes by rearranging the new members?

-- 
With Best Regards,
Andy Shevchenko