[alsa-devel] [PATCH 0/4] A few more HD-audio core fixes / cleanups
Hi,
this is a series of yet another patches for HD-audio core stuff. The first two are workarounds for possible races in the driver registration and other two are merely cleanups.
Takashi
===
Takashi Iwai (4): ALSA: hda - Drop unsol event handler for Intel HDMI codecs ALSA: hda: Add codec on bus address table lately ALSA: hda: Drop export of snd_hdac_bus_add/remove_device() ALSA: hda: Unexport a few more stuff
include/sound/hdaudio.h | 9 --------- sound/hda/hdac_bus.c | 8 ++++---- sound/hda/hdac_device.c | 27 ++++++++++++++------------- sound/hda/hdac_regmap.c | 1 + sound/hda/local.h | 7 +++++++ sound/pci/hda/patch_hdmi.c | 9 ++++++++- 6 files changed, 34 insertions(+), 27 deletions(-)
We don't need to deal with the unsol events for Intel chips that are tied with the graphics via audio component notifier. Although the presence of the audio component is checked at the beginning of hdmi_unsol_event(), better to short cut by dropping unsol_event ops.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204565 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 2096993eaf28..933c7bf47ef6 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2760,6 +2760,8 @@ static void i915_pin_cvt_fixup(struct hda_codec *codec, /* precondition and allocation for Intel codecs */ static int alloc_intel_hdmi(struct hda_codec *codec) { + int err; + /* requires i915 binding */ if (!codec->bus->core.audio_component) { codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n"); @@ -2768,7 +2770,12 @@ static int alloc_intel_hdmi(struct hda_codec *codec) return -ENODEV; }
- return alloc_generic_hdmi(codec); + err = alloc_generic_hdmi(codec); + if (err < 0) + return err; + /* no need to handle unsol events */ + codec->patch_ops.unsol_event = NULL; + return 0; }
/* parse and post-process for Intel codecs */
The call of snd_hdac_bus_add_device() is needed only for registering the codec onto the bus caddr_tbl[] that is referred essentially only in the unsol event handler. That is, the reason of this call and the release by the counter-part function snd_hdac_bus_remove_device() is just to assure that the unsol event gets notified to the codec.
But the current implementation of the unsol notification wouldn't work properly when the codec is still in a premature init state. So this patch tries to work around it by delaying the caddr_tbl[] registration at the point of snd_hdac_device_register().
Also, the order of snd_hdac_bus_remove_device() and device_del() calls are shuffled to make sure that the unsol event is masked before deleting the device.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=204565 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/hda/hdac_device.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index b26cc93e7e10..033bcef8751a 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -61,10 +61,6 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, pm_runtime_get_noresume(&codec->dev); atomic_set(&codec->in_pm, 0);
- err = snd_hdac_bus_add_device(bus, codec); - if (err < 0) - goto error; - /* fill parameters */ codec->vendor_id = snd_hdac_read_parm(codec, AC_NODE_ROOT, AC_PAR_VENDOR_ID); @@ -143,15 +139,22 @@ int snd_hdac_device_register(struct hdac_device *codec) err = device_add(&codec->dev); if (err < 0) return err; + err = snd_hdac_bus_add_device(codec->bus, codec); + if (err < 0) + goto error; mutex_lock(&codec->widget_lock); err = hda_widget_sysfs_init(codec); mutex_unlock(&codec->widget_lock); - if (err < 0) { - device_del(&codec->dev); - return err; - } + if (err < 0) + goto error_remove;
return 0; + + error_remove: + snd_hdac_bus_remove_device(codec->bus, codec); + error: + device_del(&codec->dev); + return err; } EXPORT_SYMBOL_GPL(snd_hdac_device_register);
@@ -165,8 +168,8 @@ void snd_hdac_device_unregister(struct hdac_device *codec) mutex_lock(&codec->widget_lock); hda_widget_sysfs_exit(codec); mutex_unlock(&codec->widget_lock); - device_del(&codec->dev); snd_hdac_bus_remove_device(codec->bus, codec); + device_del(&codec->dev); } } EXPORT_SYMBOL_GPL(snd_hdac_device_unregister);
snd_hdac_bus_add_device() and snd_hdac_remove_device() are called only internally in hda-core. Let's drop the exports of them and move the declarations into local.h.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/hdaudio.h | 3 --- sound/hda/hdac_bus.c | 3 +-- sound/hda/local.h | 4 ++++ 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index 4af4af55e854..edb176a265c7 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -374,9 +374,6 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr, unsigned int cmd, unsigned int *res); void snd_hdac_bus_queue_event(struct hdac_bus *bus, u32 res, u32 res_ex);
-int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec); -void snd_hdac_bus_remove_device(struct hdac_bus *bus, - struct hdac_device *codec); void snd_hdac_bus_process_unsol_events(struct work_struct *work);
static inline void snd_hdac_codec_link_up(struct hdac_device *codec) diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c index cd25e2b3f7f2..18ed3185df82 100644 --- a/sound/hda/hdac_bus.c +++ b/sound/hda/hdac_bus.c @@ -9,6 +9,7 @@ #include <linux/module.h> #include <linux/export.h> #include <sound/hdaudio.h> +#include "local.h" #include "trace.h"
static const struct hdac_bus_ops default_ops = { @@ -196,7 +197,6 @@ int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec) bus->num_codecs++; return 0; } -EXPORT_SYMBOL_GPL(snd_hdac_bus_add_device);
/** * snd_hdac_bus_remove_device - Remove a codec from bus @@ -215,7 +215,6 @@ void snd_hdac_bus_remove_device(struct hdac_bus *bus, bus->num_codecs--; flush_work(&bus->unsol_work); } -EXPORT_SYMBOL_GPL(snd_hdac_bus_remove_device);
#ifdef CONFIG_SND_HDA_ALIGNED_MMIO /* Helpers for aligned read/write of mmio space, for Tegra */ diff --git a/sound/hda/local.h b/sound/hda/local.h index 877631e39373..3a4e303169a6 100644 --- a/sound/hda/local.h +++ b/sound/hda/local.h @@ -33,4 +33,8 @@ int hda_widget_sysfs_reinit(struct hdac_device *codec, hda_nid_t start_nid, int num_nodes); void hda_widget_sysfs_exit(struct hdac_device *codec);
+int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec); +void snd_hdac_bus_remove_device(struct hdac_bus *bus, + struct hdac_device *codec); + #endif /* __HDAC_LOCAL_H */
Drop EXPORT_SYMBOL*() from a few more stuff in HD-audio core that aren't used outside. Particular the unsol event handler can be staticized now because the recent change removed all external callers.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/hdaudio.h | 6 ------ sound/hda/hdac_bus.c | 5 +++-- sound/hda/hdac_device.c | 6 ++---- sound/hda/hdac_regmap.c | 1 + sound/hda/local.h | 3 +++ 5 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index edb176a265c7..b260c5fd2337 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -122,10 +122,6 @@ int snd_hdac_codec_modalias(struct hdac_device *hdac, char *buf, size_t size);
int snd_hdac_refresh_widgets(struct hdac_device *codec);
-unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid, - unsigned int verb, unsigned int parm); -int snd_hdac_exec_verb(struct hdac_device *codec, unsigned int cmd, - unsigned int flags, unsigned int *res); int snd_hdac_read(struct hdac_device *codec, hda_nid_t nid, unsigned int verb, unsigned int parm, unsigned int *res); int _snd_hdac_read_parm(struct hdac_device *codec, hda_nid_t nid, int parm, @@ -374,8 +370,6 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr, unsigned int cmd, unsigned int *res); void snd_hdac_bus_queue_event(struct hdac_bus *bus, u32 res, u32 res_ex);
-void snd_hdac_bus_process_unsol_events(struct work_struct *work); - static inline void snd_hdac_codec_link_up(struct hdac_device *codec) { set_bit(codec->addr, &codec->bus->codec_powered); diff --git a/sound/hda/hdac_bus.c b/sound/hda/hdac_bus.c index 18ed3185df82..8f19876244eb 100644 --- a/sound/hda/hdac_bus.c +++ b/sound/hda/hdac_bus.c @@ -12,6 +12,8 @@ #include "local.h" #include "trace.h"
+static void snd_hdac_bus_process_unsol_events(struct work_struct *work); + static const struct hdac_bus_ops default_ops = { .command = snd_hdac_bus_send_cmd, .get_response = snd_hdac_bus_get_response, @@ -149,7 +151,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_bus_queue_event); /* * process queued unsolicited events */ -void snd_hdac_bus_process_unsol_events(struct work_struct *work) +static void snd_hdac_bus_process_unsol_events(struct work_struct *work) { struct hdac_bus *bus = container_of(work, struct hdac_bus, unsol_work); struct hdac_device *codec; @@ -172,7 +174,6 @@ void snd_hdac_bus_process_unsol_events(struct work_struct *work) drv->unsol_event(codec, res); } } -EXPORT_SYMBOL_GPL(snd_hdac_bus_process_unsol_events);
/** * snd_hdac_bus_add_device - Add a codec to bus diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index 033bcef8751a..bf83d7062ef6 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -221,8 +221,8 @@ EXPORT_SYMBOL_GPL(snd_hdac_codec_modalias); * * Return an encoded command verb or -1 for error. */ -unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid, - unsigned int verb, unsigned int parm) +static unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid, + unsigned int verb, unsigned int parm) { u32 val, addr;
@@ -240,7 +240,6 @@ unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid, val |= parm; return val; } -EXPORT_SYMBOL_GPL(snd_hdac_make_cmd);
/** * snd_hdac_exec_verb - execute an encoded verb @@ -261,7 +260,6 @@ int snd_hdac_exec_verb(struct hdac_device *codec, unsigned int cmd, return codec->exec_verb(codec, cmd, flags, res); return snd_hdac_bus_exec_verb(codec->bus, codec->addr, cmd, res); } -EXPORT_SYMBOL_GPL(snd_hdac_exec_verb);
/** diff --git a/sound/hda/hdac_regmap.c b/sound/hda/hdac_regmap.c index f399a1552e73..286361ecd640 100644 --- a/sound/hda/hdac_regmap.c +++ b/sound/hda/hdac_regmap.c @@ -21,6 +21,7 @@ #include <sound/core.h> #include <sound/hdaudio.h> #include <sound/hda_regmap.h> +#include "local.h"
static int codec_pm_lock(struct hdac_device *codec) { diff --git a/sound/hda/local.h b/sound/hda/local.h index 3a4e303169a6..5b935219352f 100644 --- a/sound/hda/local.h +++ b/sound/hda/local.h @@ -37,4 +37,7 @@ int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec); void snd_hdac_bus_remove_device(struct hdac_bus *bus, struct hdac_device *codec);
+int snd_hdac_exec_verb(struct hdac_device *codec, unsigned int cmd, + unsigned int flags, unsigned int *res); + #endif /* __HDAC_LOCAL_H */
participants (1)
-
Takashi Iwai