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