From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Cc: jic23@kernel.org, dlechner@baylibre.com, nuno.sa@analog.com,
andy@kernel.org, mingo@kernel.org, christophe.jaillet@wanadoo.fr,
nabijaczleweli@nabijaczleweli.xyz, kees@kernel.org,
kyungmin.park@samsung.com, k.wrona@samsung.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 6/9] iio: ssp_sensors: Drop duplicated wdt timer and work cleanup
Date: Mon, 27 Apr 2026 11:17:08 +0300 [thread overview]
Message-ID: <ae8bhC34KE4-Kguw@ashevche-desk.local> (raw)
In-Reply-To: <20260426091710.3722035-7-sanjayembedded@gmail.com>
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
next prev parent reply other threads:[~2026-04-27 8:17 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-26 9:17 [PATCH v7 0/9] iio: ssp_sensors: improve resource cleanup Sanjay Chitroda
2026-04-26 9:17 ` [PATCH v7 1/9] iio: ssp_sensors: cleanup codestyle warning Sanjay Chitroda
2026-04-26 13:53 ` Jonathan Cameron
2026-04-29 18:12 ` Sanjay Chitroda
2026-04-26 9:17 ` [PATCH v7 2/9] iio: ssp_sensors: factor out pending list add/remove helper(s) Sanjay Chitroda
2026-04-26 14:08 ` Jonathan Cameron
2026-04-27 8:05 ` Andy Shevchenko
2026-05-03 11:23 ` Sanjay Chitroda
2026-05-05 16:38 ` Jonathan Cameron
2026-05-06 7:37 ` Andy Shevchenko
2026-05-06 14:37 ` Jonathan Cameron
2026-04-26 9:17 ` [PATCH v7 3/9] iio: ssp_sensors: cancel delayed work_refresh on remove Sanjay Chitroda
2026-04-26 14:09 ` Jonathan Cameron
2026-04-29 18:06 ` Sanjay Chitroda
2026-04-29 18:09 ` Jonathan Cameron
2026-04-26 9:17 ` [PATCH v7 4/9] iio: ssp_sensors: factor out mcu enable/disable helper(s) Sanjay Chitroda
2026-04-26 14:13 ` Jonathan Cameron
2026-04-26 9:17 ` [PATCH v7 5/9] iio: ssp_sensors: use local struct device Sanjay Chitroda
2026-04-26 14:16 ` Jonathan Cameron
2026-04-27 8:09 ` Andy Shevchenko
2026-05-03 12:06 ` Sanjay Chitroda
2026-04-26 9:17 ` [PATCH v7 6/9] iio: ssp_sensors: Drop duplicated wdt timer and work cleanup Sanjay Chitroda
2026-04-27 8:17 ` Andy Shevchenko [this message]
2026-05-03 13:06 ` Sanjay Chitroda
2026-04-26 9:17 ` [PATCH v7 7/9] iio: ssp_sensors: convert probe and teardown to devm-managed resources Sanjay Chitroda
2026-04-26 9:17 ` [PATCH v7 8/9] iio: ssp_sensors: Use dev_err_probe Sanjay Chitroda
2026-04-26 14:24 ` Jonathan Cameron
2026-04-27 8:20 ` Andy Shevchenko
2026-04-27 8:19 ` Andy Shevchenko
2026-04-26 9:17 ` [PATCH v7 9/9] iio: ssp_sensors: reuse embedded RX buffer for SPI transfers Sanjay Chitroda
2026-04-26 14:31 ` Jonathan Cameron
2026-05-03 15:02 ` Sanjay Chitroda
2026-04-26 14:35 ` Jonathan Cameron
2026-05-03 14:17 ` Sanjay Chitroda
2026-05-04 8:41 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ae8bhC34KE4-Kguw@ashevche-desk.local \
--to=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=k.wrona@samsung.com \
--cc=kees@kernel.org \
--cc=kyungmin.park@samsung.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=nabijaczleweli@nabijaczleweli.xyz \
--cc=nuno.sa@analog.com \
--cc=sanjayembeddedse@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox