Re: [PATCH] ASoC: cs35l56: Use devres to destroy workqueue

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 4/5/26 12:07, Richard Fitzgerald wrote:
> In cs35l56_dsp_init() use devm_add_action_or_reset() to add a devres
> cleanup function that flushes and destroys the workqueue. This replaces
> manually calling destroy_workqueue().
> 
> The error path in cs35l56_common_probe() did not call destroy_workqueue().
> Using devres keeps the destroy_workqueue() automatically ordered relative
> to all the other devres-managed cleanup.
> 
> The call to destroy_workqueue() in cs35l56_remove() has been deleted.
> 
> The use of devres cleanup was suggested by https://sashiko.dev to avoid a
> small cleanup inversion window if destroy_workqueue() is called in the
> error path of cs35l56_common_probe().
> 
> Fixes: e49611252900 ("ASoC: cs35l56: Add driver for Cirrus Logic CS35L56")
> Closes: https://sashiko.dev/#/patchset/20260501103002.2843735-1-rf%40opensource.cirrus.com
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>   sound/soc/codecs/cs35l56.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
> index 378017fcea10..030051292534 100644
> --- a/sound/soc/codecs/cs35l56.c
> +++ b/sound/soc/codecs/cs35l56.c
> @@ -1627,6 +1627,14 @@ static int cs35l56_control_add_nop(struct wm_adsp *dsp, struct cs_dsp_coeff_ctl
>   	return 0;
>   }
>   
> +static void cs35l56_dsp_workqueue_destroy(void *data)
> +{
> +	struct workqueue_struct *wq = data;
> +
> +	flush_workqueue(wq);
> +	destroy_workqueue(wq);
> +}
> +
>   static int cs35l56_dsp_init(struct cs35l56_private *cs35l56)
>   {
>   	struct wm_adsp *dsp;
> @@ -1636,6 +1644,12 @@ static int cs35l56_dsp_init(struct cs35l56_private *cs35l56)
>   	if (!cs35l56->dsp_wq)
>   		return -ENOMEM;
>   
> +	ret = devm_add_action_or_reset(cs35l56->base.dev,
> +				       cs35l56_dsp_workqueue_destroy,
> +				       cs35l56->dsp_wq);
> +	if (ret)
> +		return ret;
> +
>   	INIT_WORK(&cs35l56->dsp_work, cs35l56_dsp_work);
>   
>   	dsp = &cs35l56->dsp;
> @@ -2066,8 +2080,6 @@ void cs35l56_remove(struct cs35l56_private *cs35l56)
>   	if (cs35l56->base.irq)
>   		devm_free_irq(cs35l56->base.dev, cs35l56->base.irq, &cs35l56->base);
>   
> -	destroy_workqueue(cs35l56->dsp_wq);
> -

Sashiko points out that this should be replaced by a flush so the
workqueue is empty before the following cleanup steps.

>   	pm_runtime_dont_use_autosuspend(cs35l56->base.dev);
>   	pm_runtime_suspend(cs35l56->base.dev);
>   	pm_runtime_disable(cs35l56->base.dev);