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

[Sanjay Chitroda]

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