Re: [RFC PATCH] ASoC: rt700-sdw: always drain jack work on remove

[Pierre-Louis Bossart]

On 6/19/26 10:03, Runyu Xiao wrote:
> rt700_sdw_remove() drains jack_detect_work and jack_btn_check_work only
> when rt700->hw_init is true. That state bit is cleared by
> rt700_update_status() when the SoundWire slave becomes UNATTACHED, but a
> jack work item can already have been queued by rt700_interrupt_callback()
> or rt700_jack_init() while the device was initialized.
> 
> Do not use hw_init as the remove-time guard for draining these work
> objects. The delayed works are initialized during rt700_init(), so remove
> can cancel them unconditionally and pair the object lifetime with the
> codec-private data lifetime instead of a mutable hardware state bit.
> 
> This issue was found by our static analysis tool and then confirmed by
> manual review of the SoundWire status, interrupt and remove paths. The
> remove path should drain work based on whether the work object exists, not
> on a runtime hardware state bit that can change after the work was queued.
> 
> A QEMU PoC queued jack_detect_work, simulated SDW_SLAVE_UNATTACHED, and
> then entered remove. DEBUG_OBJECTS reported an active timer/work object
> associated with the rt700 jack work path after remove skipped the cancel.
> 
> This is sent as an RFC because the practical trigger depends on SoundWire
> core remove ordering after an UNATTACHED status update. If remove cannot
> run after hw_init has been cleared while jack work is still pending, this
> is a defensive lifecycle cleanup rather than a reachable race on current
> systems.

The SoundWire core doesn't deal well with devices going off the bus anyways, so this feels like an academic improvement IMHO.
In addition this codec isn't used in any production systems to the best of my knowledge, it's likely this driver could be removed without impacts.


> Fixes: 737ee8bdf682 ("ASoC: rt700-sdw: use cancel_work_sync() in .remove as well as .suspend")
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>  sound/soc/codecs/rt700-sdw.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
> index 44543c0da177..f7bd793e3e67 100644
> --- a/sound/soc/codecs/rt700-sdw.c
> +++ b/sound/soc/codecs/rt700-sdw.c
> @@ -459,10 +459,8 @@ static int rt700_sdw_remove(struct sdw_slave *slave)
>  {
>  	struct rt700_priv *rt700 = dev_get_drvdata(&slave->dev);
>  
> -	if (rt700->hw_init) {
> -		cancel_delayed_work_sync(&rt700->jack_detect_work);
> -		cancel_delayed_work_sync(&rt700->jack_btn_check_work);
> -	}
> +	cancel_delayed_work_sync(&rt700->jack_detect_work);
> +	cancel_delayed_work_sync(&rt700->jack_btn_check_work);
>  
>  	pm_runtime_disable(&slave->dev);
>