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

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

[Richard Fitzgerald]

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 changed to
flush_workqueue(), as pointed out by Sashiko:
https://sashiko.dev/#/patchset/20260504110743.3341869-1-rf%40opensource.cirrus.com

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>
---
Change in V2:
- Call flush_workqueue() in cs35l56_remove().

 sound/soc/codecs/cs35l56.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 378017fcea10..26fa94c98775 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,7 +2080,7 @@ 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);
+	flush_workqueue(cs35l56->dsp_wq);
 
 	pm_runtime_dont_use_autosuspend(cs35l56->base.dev);
 	pm_runtime_suspend(cs35l56->base.dev);
-- 
2.47.3

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

[Christophe JAILLET]

Le 04/05/2026 à 17:10, Richard Fitzgerald a écrit :
> 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 changed to
> flush_workqueue(), as pointed out by Sashiko:
> https://sashiko.dev/#/patchset/20260504110743.3341869-1-rf%40opensource.cirrus.com
> 
> 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>
> ---
> Change in V2:
> - Call flush_workqueue() in cs35l56_remove().
> 
>   sound/soc/codecs/cs35l56.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
> index 378017fcea10..26fa94c98775 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);

I don't think that an explicit flush_workqueue() is needed here.

destroy_workqueue() already does the job ( destroy_workqueue() --> 
drain_workqueue() --> __flush_workqueue())

> +	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,

so, I guess that destroy_workqueue() could even be called directly 
without the need of cs35l56_dsp_workqueue_destroy(). Not sure it is cleaner.

CJ

> +				       cs35l56->dsp_wq);
> +	if (ret)
> +		return ret;
> +
>   	INIT_WORK(&cs35l56->dsp_work, cs35l56_dsp_work);
>   
>   	dsp = &cs35l56->dsp;
> @@ -2066,7 +2080,7 @@ 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);
> +	flush_workqueue(cs35l56->dsp_wq);
>   
>   	pm_runtime_dont_use_autosuspend(cs35l56->base.dev);
>   	pm_runtime_suspend(cs35l56->base.dev);

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

[Richard Fitzgerald]

On 4/5/26 17:46, Christophe JAILLET wrote:
> Le 04/05/2026 à 17:10, Richard Fitzgerald a écrit :
>> 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 changed to
>> flush_workqueue(), as pointed out by Sashiko:
>> https://sashiko.dev/#/patchset/20260504110743.3341869-1- 
>> rf%40opensource.cirrus.com
>>
>> 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>
>> ---
>> Change in V2:
>> - Call flush_workqueue() in cs35l56_remove().
>>
>>   sound/soc/codecs/cs35l56.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
>> index 378017fcea10..26fa94c98775 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);
> 
> I don't think that an explicit flush_workqueue() is needed here.
> 
> destroy_workqueue() already does the job ( destroy_workqueue() --> 
> drain_workqueue() --> __flush_workqueue())
> 
>> +    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,
> 
> so, I guess that destroy_workqueue() could even be called directly 
> without the need of cs35l56_dsp_workqueue_destroy(). Not sure it is 
> cleaner.
> 
> CJ

Ok, I will change to manually destroying the ASoC component in the
driver remove(), instead of using devm_snd_soc_register_component().
That would make it safe to directly call destroy_workqueue() from
driver remove().