Re: [PATCH] iio: ssp: Serialize watchdog timer state changes

[Runyu Xiao]

Hi Andy,

Thanks for the review.

On Wed, Sep 30, 2026 at 03:03:19PM +0800, Runyu Xiao wrote:
> The SSP watchdog timer rearms itself from its callback, but the driver uses
> timer_delete_sync() when the last sensor is disabled and during suspend.
> Those operations do not prevent a concurrent enable or callback from
> rearming the timer after the deletion has completed. The final remove path
> also used timer_delete_sync(), which does not provide the shutdown
> guarantee needed before releasing the device state.
>
> Protect the watchdog state and enable reference count with a mutex. The
> callback checks a state flag before rearming. Reusable stops clear the
> flag before deleting the timer. Use timer_shutdown_sync() for the final
> remove path so that any later rearm attempt is rejected permanently.
>
> struct ssp_data {
>     struct timer_list wdt_timer;
>     struct mutex wdt_lock;
>     bool wdt_enabled;
>     struct work_struct work_wdt;
> };

On the x86_64 build, pahole reports wdt_timer at offset 16, wdt_lock at
56, wdt_enabled at 80, and work_wdt at 88; sizeof(struct ssp_data) is
816 bytes. There is a 7-byte hole after wdt_enabled.

> _mod:
> + if (READ_ONCE(data->wdt_enabled))
> +     mod_timer(&data->wdt_timer, ...);
>
> What are we going to do if just after this wdt_enabled becomes false?
> (Is it a possible case?)

Yes, that is possible. The callback may read true before the stop path
writes false, then rearm the timer once. timer_delete_sync() waits for the
running callback and removes that rearmed instance before returning.

> Same Q to all the below. Hmm... It seems they are all protected by the
> mutex?

All process-context start and stop paths use wdt_lock. The timer callback
is the exception because it cannot take a mutex; the state check and
synchronous deletion handle its in-flight rearm. The mutex serializes
operations, but a start that runs after a reusable stop releases the lock
can arm the timer again. That is expected after an ordinary sensor enable.

There is also a suspend interleaving. A pending work_refresh can call
ssp_sync_available_sensors() and ssp_enable_sensor() after ssp_suspend()
checks enable_refcount. If the count is zero, the first successful sensor
enable can arm the timer. The first version did not close this path.

In v2, suspend state is tracked under wdt_lock. Sensor enables can update the
count while suspended but cannot arm the timer. It is restarted only after
resume succeeds, or if suspend fails. Watchdog work is synchronized before
the suspend command. I will send v2 separately.

Final removal uses timer_shutdown_sync() to reject later rearm attempts.

Thanks,
Runyu