Re: [PATCH v6 4/7] iio: ssp_sensors: drop duplicated wdt timer and work cleanup

[Sanjay Chitroda <sanjayembeddedse@gmail.com>]

On 15 April 2026 2:59:55 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Wed, Apr 15, 2026 at 10:37:46AM +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.
>> 
>> Remove the duplicated timer_delete_sync() and cancel_work_sync()
>> calls and rely on ssp_disable_wdt_timer() as the single cleanup helper
>> to simplify the remove path.
>> 
>> No functional change intended.
>
>...
>
>>  	ssp_enable_mcu(data, false);
>> -	ssp_disable_wdt_timer(data);
>> -
>>  	ssp_clean_pending_list(data);
>>  
>>  	free_irq(data->spi->irq, data);
>>  
>> -	timer_delete_sync(&data->wdt_timer);
>> -	cancel_work_sync(&data->work_wdt);
>> +	ssp_disable_wdt_timer(data);
>
>Hmm... I'm not sure it doesn't change the behaviour. First of all, the
>difference is that IRQ handler may be involved in the timer and/or work.
>
>This patch needs much better investigation conducted and commit message
>provided with explanation of all possible race conditions and why they
>don't happen. Also I am not sure if this dup is a mistake or deliberate
>action (again, it's all related to the timings of IRQ handler, timer job,
>and work and how they are relies on each other data/flow).
>

Thank you Andy for the input.
You are right patch has changed teardown ordering by moving watchdog stop after free_irq(), so "no functional change" is wrong.

I will send next series change by removing duplicate timer/work cleanup after free_irq().

Race analysis,
- wdt_timer can queue work_wdt
- free_irq() handle IRQ, but does not cancel timer/work. Therefore timer/work needs to explicitly cancelled before IRQ release.

Along with change I will also add analysis in commit message.

>