ALSA: hda - Fix registration of beep input device

The beep input device is registered via input_register_device(), but
this is called in snd_hda_attach_beep_device() where the sound devices
aren't registered yet.  This leads to the binding to non-existing
object, thus results in failure.  And, even if the binding worked
(against the PCI object), it's still racy; the input device appears
before the sound objects.

For fixing this, register the input device properly at dev_register
ops of the codec object it's bound with.  Also, call
snd_hda_detach_beep_device() at dev_disconnection so that it's
detached at the right timing.  As a bonus, since it's called in the
codec's ops, we can get rid of the further call from the other codec
drivers.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index 88bb084..8c6c50a 100644 (file)
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -139,7 +139,10 @@ static void turn_off_beep(struct hda_beep *beep)
 
 static void snd_hda_do_detach(struct hda_beep *beep)
 {
-       input_unregister_device(beep->dev);
+       if (beep->registered)
+               input_unregister_device(beep->dev);
+       else
+               input_free_device(beep->dev);
        beep->dev = NULL;
        turn_off_beep(beep);
 }
@@ -148,7 +151,6 @@ static int snd_hda_do_attach(struct hda_beep *beep)
 {
        struct input_dev *input_dev;
        struct hda_codec *codec = beep->codec;
-       int err;
 
        input_dev = input_allocate_device();
        if (!input_dev)
@@ -169,12 +171,6 @@ static int snd_hda_do_attach(struct hda_beep *beep)
        input_dev->dev.parent = &codec->dev;
        input_set_drvdata(input_dev, beep);
 
-       err = input_register_device(input_dev);
-       if (err < 0) {
-               input_free_device(input_dev);
-               codec_err(codec, "hda_beep: unable to register input device\n");
-               return err;
-       }
        beep->dev = input_dev;
        return 0;
 }
@@ -244,6 +240,27 @@ void snd_hda_detach_beep_device(struct hda_codec *codec)
 }
 EXPORT_SYMBOL_GPL(snd_hda_detach_beep_device);
 
+int snd_hda_register_beep_device(struct hda_codec *codec)
+{
+       struct hda_beep *beep = codec->beep;
+       int err;
+
+       if (!beep || !beep->dev)
+               return 0;
+
+       err = input_register_device(beep->dev);
+       if (err < 0) {
+               codec_err(codec, "hda_beep: unable to register input device\n");
+               input_free_device(beep->dev);
+               codec->beep = NULL;
+               kfree(beep);
+               return err;
+       }
+       beep->registered = true;
+       return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hda_register_beep_device);
+
 static bool ctl_has_mute(struct snd_kcontrol *kcontrol)
 {
        struct hda_codec *codec = snd_kcontrol_chip(kcontrol);

diff --git a/sound/pci/hda/hda_beep.h b/sound/pci/hda/hda_beep.h
index cb88464..a63b5e0 100644 (file)
--- a/sound/pci/hda/hda_beep.h
+++ b/sound/pci/hda/hda_beep.h
@@ -34,6 +34,7 @@ struct hda_beep {
        char phys[32];
        int tone;
        hda_nid_t nid;
+       unsigned int registered:1;
        unsigned int enabled:1;
        unsigned int linear_tone:1;     /* linear tone for IDT/STAC codec */
        unsigned int playing:1;
@@ -45,6 +46,7 @@ struct hda_beep {
 int snd_hda_enable_beep_device(struct hda_codec *codec, int enable);
 int snd_hda_attach_beep_device(struct hda_codec *codec, int nid);
 void snd_hda_detach_beep_device(struct hda_codec *codec);
+int snd_hda_register_beep_device(struct hda_codec *codec);
 #else
 static inline int snd_hda_attach_beep_device(struct hda_codec *codec, int nid)
 {
@@ -53,5 +55,9 @@ static inline int snd_hda_attach_beep_device(struct hda_codec *codec, int nid)
 static inline void snd_hda_detach_beep_device(struct hda_codec *codec)
 {
 }
+static inline int snd_hda_register_beep_device(struct hda_codec *codec)
+{
+       return 0;
+}
 #endif
 #endif

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 6db2dbc..4c20277 100644 (file)
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1379,14 +1379,19 @@ static unsigned int hda_set_power_state(struct hda_codec *codec,
 static int snd_hda_codec_dev_register(struct snd_device *device)
 {
        struct hda_codec *codec = device->device_data;
+       int err = device_add(&codec->dev);
 
-       return device_add(&codec->dev);
+       if (err < 0)
+               return err;
+       snd_hda_register_beep_device(codec);
+       return 0;
 }
 
 static int snd_hda_codec_dev_disconnect(struct snd_device *device)
 {
        struct hda_codec *codec = device->device_data;
 
+       snd_hda_detach_beep_device(codec);
        device_del(&codec->dev);
        return 0;
 }
@@ -2692,6 +2697,7 @@ int snd_hda_codec_reset(struct hda_codec *codec)
                                  bus->pcm_dev_bits);
                }
        }
+       snd_hda_detach_beep_device(codec);
        if (codec->patch_ops.free)
                codec->patch_ops.free(codec);
        memset(&codec->patch_ops, 0, sizeof(codec->patch_ops));

diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 9e0609a..1613388 100644 (file)
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -5350,7 +5350,6 @@ EXPORT_SYMBOL_GPL(snd_hda_gen_init);
 void snd_hda_gen_free(struct hda_codec *codec)
 {
        snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_FREE);
-       snd_hda_detach_beep_device(codec);
        snd_hda_gen_spec_free(codec->spec);
        kfree(codec->spec);
        codec->spec = NULL;

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 09d6d0d..1dc7e97 100644 (file)
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -445,9 +445,7 @@ static int conexant_init(struct hda_codec *codec)
 
 static void conexant_free(struct hda_codec *codec)
 {
-       struct conexant_spec *spec = codec->spec;
-       snd_hda_detach_beep_device(codec);
-       kfree(spec);
+       kfree(codec->spec);
 }
 
 static const struct snd_kcontrol_new cxt_capture_mixers[] = {