Re: [PATCH v7 6/9] iio: ssp_sensors: Drop duplicated wdt timer and work cleanup

[Andy Shevchenko]

On Sun, Apr 26, 2026 at 02:47:07PM +0530, Sanjay Chitroda wrote:

> The SSP remove path cleans up the watchdog timer and associated work
> both via ssp_disable_wdt_timer() and through explicit timer and work
> teardown.
> 
> ssp_disable_wdt_timer() already performs a synchronous teardown of the
> watchdog timer and watchdog work, guaranteeing that no timer callbacks
> or watchdog work can be running or requeued once it returns.
> 
> In addition, the remove path disables interrupts and frees IRQ handler
> using ssp_disable_mcu() and free_irq(). The refresh work is also
> cancelled, preventing wdt_timer being re-armed before teardown. This
> ensures that no new refresh or watchdog activity can be triggered from
> the IRQ thread and refresh workqueue.
> 
> As a result, the additional timer and work cancellation is redundant
> and does not provide additional ordering or race protection. Remove the
> duplicated cleanup and rely on the centralized watchdog disable helper.

Nice...

...

>  	ssp_disable_mcu(data);
> -	ssp_disable_wdt_timer(data);
> -

...but this is left unexplained. The commit message as I read it relies on the
fact of this call to be present here and not being moved below.

>  	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);
> +	ssp_disable_wdt_timer(data);

Now it's still possible that watchdog will fire and we might have a callback
run while the IRQs are already being disabled. Which means that if WDT callback
needs the HW to be able to generate IRQs, it won't happen.

Note, that IRQ can be still fired in between ssp_disable_mcu() and free_irq().
Only the latter call makes sure you want see any IRQs from HW.

>  	mutex_destroy(&data->comm_lock);
>  	mutex_destroy(&data->pending_lock);

-- 
With Best Regards,
Andy Shevchenko