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

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

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().