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

Sanjay Chitroda
2026-05-03T14:13:47Z

On 27 April 2026 1:47:08 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>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.
>>

Calling ssp_disable_wdt_timer() after IRQ teardown is safe, as the watchdog disable helper does not depend on interrupt delivery and ensures all watchdog activity is fully synchronized before returning.

>> 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.
>

Thank you for the review.

I will add a section in commit message as above to justify wdt cleanup shifted to specific position.

>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);
>