snd_hdac_device_unregister is called twice on ASoC driver

snd_hdac_device_unregister is called twice on ASoC driver

[Liao, Bard]

Hi Takashi and all,

We meet a problem that snd_hdac_device_unregister will be called twice
on ASoC driver. We will call snd_hdac_ext_bus_device_init ->
snd_hdac_device_register and snd_hdac_ext_bus_device_remove ->
snd_hdac_device_unregister on ASoC driver. It looks even to me.
However, hdac_hda_codec_probe will call snd_hda_codec_device_new
to create a hda codec device. And it will assign snd_hda_codec_dev_free
to the .dev_free ops where snd_hdac_device_unregister will be called.
That's why snd_hdac_device_unregister will be call twice on ASoC driver.
Below patch can "fix" this issue, but I don't know if it is the right way
to fix it. Could you take a look and give me some advice?

--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -841,7 +841,13 @@ static int snd_hda_codec_dev_free(struct snd_device *device)
        struct hda_codec *codec = device->device_data;

        codec->in_freeing = 1;
-       snd_hdac_device_unregister(&codec->core);
+       /*
+        * snd_hda_codec_device_new() is used by legacy HDA and ASoC driver.
+        * We can't unregister ASoC device since it will be unregistered in
+        * snd_hdac_ext_bus_device_remove().
+        */
+       if (codec->core.type == HDA_DEV_LEGACY)
+               snd_hdac_device_unregister(&codec->core);
        codec_display_power(codec, false);
        put_device(hda_codec_dev(codec));
        return 0;

--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -328,6 +328,12 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
                dev_err(&hdev->dev, "failed to create hda codec %d\n", ret);
                goto error_no_pm;
        }
+       /*
+        * Overwrite type to HDA_DEV_ASOC since it is a ASoC driver
+        * hda_codec.c will check this flag to determine if unregister
+        * device is needed.
+        */
+       hdev->type = HDA_DEV_ASOC;

        /*
         * snd_hda_codec_device_new decrements the usage count so call get pm

Re: snd_hdac_device_unregister is called twice on ASoC driver

[Takashi Iwai]

On Sun, 28 Apr 2019 03:46:56 +0200,
Liao, Bard wrote:
> 
> Hi Takashi and all,
> 
> We meet a problem that snd_hdac_device_unregister will be called twice
> 
> on ASoC driver. We will call snd_hdac_ext_bus_device_init ->
> 
> snd_hdac_device_register and snd_hdac_ext_bus_device_remove ->
> 
> snd_hdac_device_unregister on ASoC driver. It looks even to me.
> 
> However, hdac_hda_codec_probe will call snd_hda_codec_device_new
> 
> to create a hda codec device. And it will assign snd_hda_codec_dev_free
> 
> to the .dev_free ops where snd_hdac_device_unregister will be called.
> 
> That’s why snd_hdac_device_unregister will be call twice on ASoC driver.
> 
> Below patch can “fix” this issue, but I don’t know if it is the right way
> 
> to fix it. Could you take a look and give me some advice?

The change looks OK to me.
Care to cook up a proper patch?


thanks,

Takashi


> 
> --- a/sound/pci/hda/hda_codec.c
> 
> +++ b/sound/pci/hda/hda_codec.c
> 
> @@ -841,7 +841,13 @@ static int snd_hda_codec_dev_free(struct snd_device
> *device)
> 
>         struct hda_codec *codec = device->device_data;
> 
>         codec->in_freeing = 1;
> 
> -       snd_hdac_device_unregister(&codec->core);
> 
> +       /*
> 
> +        * snd_hda_codec_device_new() is used by legacy HDA and ASoC driver.
> 
> +        * We can't unregister ASoC device since it will be unregistered in
> 
> +        * snd_hdac_ext_bus_device_remove().
> 
> +        */
> 
> +       if (codec->core.type == HDA_DEV_LEGACY)
> 
> +               snd_hdac_device_unregister(&codec->core);
> 
>         codec_display_power(codec, false);
> 
>         put_device(hda_codec_dev(codec));
> 
>         return 0;
> 
> --- a/sound/soc/codecs/hdac_hda.c
> 
> +++ b/sound/soc/codecs/hdac_hda.c
> 
> @@ -328,6 +328,12 @@ static int hdac_hda_codec_probe(struct snd_soc_component
> *component)
> 
>                 dev_err(&hdev->dev, "failed to create hda codec %d\n", ret);
> 
>                 goto error_no_pm;
> 
>         }
> 
> +       /*
> 
> +        * Overwrite type to HDA_DEV_ASOC since it is a ASoC driver
> 
> +        * hda_codec.c will check this flag to determine if unregister
> 
> +        * device is needed.
> 
> +        */
> 
> +       hdev->type = HDA_DEV_ASOC;
> 
>         /*
> 
>          * snd_hda_codec_device_new decrements the usage count so call get pm
> 
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel