[PATCH v2 0/3] ASoC: hdac_hda: Fix module unbind/bind via sysfs

[PATCH v2 0/3] ASoC: hdac_hda: Fix module unbind/bind via sysfs

[Peter Ujfalusi]

Hi,

Changes since v1:
- Add pm_runtime_set_active() after snd_hda_codec_device_new() in
  hdac_hda_codec_probe() function as reported and suggested by
  Sashiko Bot.

Expanded test plan run on the device to cover the previously missed
scenario.

While looking at the reconfig lockup reported in
https://bugzilla.kernel.org/show_bug.cgi?id=222182
I checked whether the same can be done by unbinding and binding the HDA
codec driver via sysfs:

  echo -n ehdaudio0D0 > /sys/bus/hdaudio/drivers/snd_hda_codec_alc882/unbind
  echo -n ehdaudio0D0 > /sys/bus/hdaudio/drivers/snd_hda_codec_alc882/bind

The unbind works and the SOF card is removed, but the codec cannot be
bound again. This has been broken for a long time with hdac_hda; the
reconfig lockup itself is addressed by a separate patch.

Patch 1 and 2 fix the bind: the codec private data was lost on unbind
and the codec device type was left as HDA_DEV_ASOC, so no HDA codec
driver could match it anymore. Patch 2 depends on patch 1, otherwise
the bind would dereference NULL drvdata instead of failing.

Patch 3 fixes an unbalanced runtime PM disable in the component remove,
which leaves the codec with runtime PM disabled after the card is
re-created. This is also visible with machine driver unbind/bind.

Tested on PTL (ALC888 + iDisp): repeated codec and machine driver
unbind/bind, playback/capture, runtime suspend and module reload.

Regards,
Peter
---
Peter Ujfalusi (3):
  ASoC: hdac_hda: Allocate the private data at codec driver bind
  ASoC: hdac_hda: Restore the codec device type on unbind
  ASoC: hdac_hda: Fix codec runtime PM across remove and probe

 sound/soc/codecs/hdac_hda.c     | 36 ++++++++++++++++++++++++++++++++-
 sound/soc/sof/intel/hda-codec.c | 23 ++++-----------------
 2 files changed, 39 insertions(+), 20 deletions(-)

-- 
2.56.0

[PATCH v2 1/3] ASoC: hdac_hda: Allocate the private data at codec driver bind

[Peter Ujfalusi]

The SOF HDA code allocates struct hdac_hda_priv and stores it as drvdata
of the codec device before the codec driver is bound. The driver core
clears drvdata on unbind, so after unbinding the codec driver via sysfs
a subsequent bind dereferences a NULL pointer in hdac_hda_dev_probe().

All information in the private data can be derived from the codec
device itself, so allocate and set it up in hdac_hda_dev_probe() as a
device managed resource, which is released on unbind.

Remove the allocation from SOF, keeping the check for iDisp codecs
without display audio component.

Fixes: 3fd63658caed ("ASoC: Intel: Drop hdac_ext usage for codec device creation")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/codecs/hdac_hda.c     | 13 ++++++++++++-
 sound/soc/sof/intel/hda-codec.c | 23 ++++-------------------
 2 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 7414be08d3d5..46617fc88277 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -619,10 +619,21 @@ static const struct snd_soc_component_driver hdac_hda_hdmi_codec = {
 
 static int hdac_hda_dev_probe(struct hdac_device *hdev)
 {
-	struct hdac_hda_priv *hda_pvt = dev_get_drvdata(&hdev->dev);
+	struct hdac_hda_priv *hda_pvt;
 	struct hdac_ext_link *hlink;
 	int ret;
 
+	hda_pvt = devm_kzalloc(&hdev->dev, sizeof(*hda_pvt), GFP_KERNEL);
+	if (!hda_pvt)
+		return -ENOMEM;
+
+	hda_pvt->codec = dev_to_hda_codec(&hdev->dev);
+	hda_pvt->dev_index = hdev->addr;
+	/* Intel iDisp (HDMI/DP) codec */
+	if (((hdev->vendor_id >> 16) & 0xFFFF) == 0x8086)
+		hda_pvt->need_display_power = true;
+	dev_set_drvdata(&hdev->dev, hda_pvt);
+
 	/* hold the ref while we probe */
 	hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev));
 	if (!hlink) {
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index 91f7ca22a4a7..a2b2ecf1e7e6 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -15,8 +15,6 @@
 #include "hda.h"
 
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
-#include "../../codecs/hdac_hda.h"
-
 #define CODEC_PROBE_RETRIES	3
 
 #define IDISP_VID_INTEL	0x80860000
@@ -148,7 +146,6 @@ static struct hda_codec *hda_codec_device_init(struct hdac_bus *bus, int addr, i
 /* probe individual codec */
 static int hda_codec_probe(struct snd_sof_dev *sdev, int address)
 {
-	struct hdac_hda_priv *hda_priv;
 	struct hda_bus *hbus = sof_to_hbus(sdev);
 	struct hda_codec *codec;
 	u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) |
@@ -168,27 +165,15 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address)
 	dev_dbg(sdev->dev, "HDA codec #%d probed OK: response: %x\n",
 		address, resp);
 
-	hda_priv = devm_kzalloc(sdev->dev, sizeof(*hda_priv), GFP_KERNEL);
-	if (!hda_priv)
-		return -ENOMEM;
-
 	codec = hda_codec_device_init(&hbus->core, address, HDA_DEV_LEGACY);
 	ret = PTR_ERR_OR_ZERO(codec);
 	if (ret < 0)
 		return ret;
 
-	hda_priv->codec = codec;
-	hda_priv->dev_index = address;
-	dev_set_drvdata(&codec->core.dev, hda_priv);
-
-	if ((resp & 0xFFFF0000) == IDISP_VID_INTEL) {
-		if (!hbus->core.audio_component) {
-			dev_dbg(sdev->dev,
-				"iDisp hw present but no driver\n");
-			ret = -ENOENT;
-			goto out;
-		}
-		hda_priv->need_display_power = true;
+	if ((resp & 0xFFFF0000) == IDISP_VID_INTEL && !hbus->core.audio_component) {
+		dev_dbg(sdev->dev, "iDisp hw present but no driver\n");
+		ret = -ENOENT;
+		goto out;
 	}
 
 	if (is_generic_config(hbus))
-- 
2.56.0

[PATCH v2 2/3] ASoC: hdac_hda: Restore the codec device type on unbind

[Peter Ujfalusi]

hdac_hda_codec_probe() changes the type of the codec device to
HDA_DEV_ASOC to prevent snd_hda_codec_unregister() from unregistering
the device when the card is freed.

The type is never restored, and hda_bus_match() requires the device and
driver types to be equal. After unbinding the codec driver via sysfs the
HDA_DEV_LEGACY codec drivers no longer match the device and bind fails
with -ENODEV.

Restore HDA_DEV_LEGACY with a device managed action registered before
the component, so it runs after the component and the card are gone.

Fixes: b60ee2e281b6 ("ASoC: hdac_hda: overwrite hdev type to HDA_DEV_ASOC")
Cc: stable@vger.kernel.org
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/codecs/hdac_hda.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 46617fc88277..cae5711e979c 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -617,6 +617,13 @@ static const struct snd_soc_component_driver hdac_hda_hdmi_codec = {
 	.endianness		= 1,
 };
 
+static void hdac_hda_restore_dev_type(void *data)
+{
+	struct hdac_device *hdev = data;
+
+	hdev->type = HDA_DEV_LEGACY;
+}
+
 static int hdac_hda_dev_probe(struct hdac_device *hdev)
 {
 	struct hdac_hda_priv *hda_pvt;
@@ -634,6 +641,11 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev)
 		hda_pvt->need_display_power = true;
 	dev_set_drvdata(&hdev->dev, hda_pvt);
 
+	/* Allow the legacy codec driver to match again after unbind */
+	ret = devm_add_action(&hdev->dev, hdac_hda_restore_dev_type, hdev);
+	if (ret)
+		return ret;
+
 	/* hold the ref while we probe */
 	hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev));
 	if (!hlink) {
-- 
2.56.0

[PATCH v2 3/3] ASoC: hdac_hda: Fix codec runtime PM across remove and probe

[Peter Ujfalusi]

hdac_hda_codec_remove() disables runtime PM of the codec device before
releasing the HDA link and calling the codec driver's remove callback,
so that no runtime resume can race with the teardown and access the
link or the already freed codec driver data.
snd_hda_codec_cleanup_for_unbind() then disables runtime PM again.

On the next probe snd_hda_codec_register() enables runtime PM only once,
leaving it disabled after the ASoC card or the codec driver has been
unbound and bound again: the codec stays powered and runtime_status
reads 'unsupported'.

Re-enable runtime PM at the end of hdac_hda_codec_remove() to balance
its own disable. The codec remains disabled until it is registered again.

With runtime PM enabled again the codec still does not suspend: the
runtime status is RPM_SUSPENDED after the remove, while
snd_hda_codec_device_new() powers the codec to D0 on probe. The
pm_runtime_put()/snd_hda_power_down() calls see an already suspended
device and the codec, its HDA link and the display power reference
stay powered, preventing the controller from runtime suspending.
Mark the device active after the codec has been powered up.

Fixes: c3ec8ac82105 ("ASoC: hdac_hda: fix memleak on module unload")
Cc: stable@vger.kernel.org
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/codecs/hdac_hda.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index cae5711e979c..fe5796cad1ff 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -435,6 +435,13 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
 		goto error_no_pm;
 	}
 
+	/* The codec is in D0, the status is RPM_SUSPENDED after a previous unbind */
+	ret = pm_runtime_set_active(&hdev->dev);
+	if (ret < 0) {
+		dev_err(&hdev->dev, "%s: failed to set runtime PM active %d\n", __func__, ret);
+		goto error_no_pm;
+	}
+
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
 	if (loadable_patch[hda_pvt->dev_index] && *loadable_patch[hda_pvt->dev_index]) {
 		const struct firmware *fw __free(firmware) = NULL;
@@ -554,6 +561,7 @@ static void hdac_hda_codec_remove(struct snd_soc_component *component)
 		return;
 	}
 
+	/* prevent runtime PM callbacks while the link and the codec are torn down */
 	pm_runtime_disable(&hdev->dev);
 	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
 
@@ -561,6 +569,9 @@ static void hdac_hda_codec_remove(struct snd_soc_component *component)
 		driver->ops->remove(codec);
 
 	snd_hda_codec_cleanup_for_unbind(codec);
+
+	/* Balances the disable above, cleanup_for_unbind keeps runtime PM disabled */
+	pm_runtime_enable(&hdev->dev);
 }
 
 static const struct snd_soc_dapm_route hdac_hda_dapm_routes[] = {
-- 
2.56.0

Re: [PATCH v2 0/3] ASoC: hdac_hda: Fix module unbind/bind via sysfs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On Tue, Oct 27, 2026 at 02:54:45PM +0200, Peter Ujfalusi wrote:
> Hi,
> 
> Changes since v1:
> - Add pm_runtime_set_active() after snd_hda_codec_device_new() in
>   hdac_hda_codec_probe() function as reported and suggested by
>   Sashiko Bot.

Please send an incremental change, I already put this into CI.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/3] ASoC: hdac_hda: Fix module unbind/bind via sysfs

[Péter Ujfalusi]

On 27/10/2026 15:34, Mark Brown wrote:
> On Tue, Oct 27, 2026 at 02:54:45PM +0200, Peter Ujfalusi wrote:
>> Hi,
>>
>> Changes since v1:
>> - Add pm_runtime_set_active() after snd_hda_codec_device_new() in
>>   hdac_hda_codec_probe() function as reported and suggested by
>>   Sashiko Bot.
> 
> Please send an incremental change, I already put this into CI.

I don't see them in for-7.4, for-linus or for-next branches, the
followup should have a Fixes tag to be backported alongside this patch.

I can use the same commit id on the other had for this as well:
Fixes: c3ec8ac82105 ("ASoC: hdac_hda: fix memleak on module unload")

-- 
Péter

Re: [PATCH v2 0/3] ASoC: hdac_hda: Fix module unbind/bind via sysfs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

On Tue, Oct 27, 2026 at 04:25:00PM +0200, Péter Ujfalusi wrote:
> On 27/10/2026 15:34, Mark Brown wrote:

> > Please send an incremental change, I already put this into CI.

> I don't see them in for-7.4, for-linus or for-next branches, the
> followup should have a Fixes tag to be backported alongside this patch.

CI means I am testing them (and other stuff), I don't want to have to
redo all the testing for a rebase.

> I can use the same commit id on the other had for this as well:
> Fixes: c3ec8ac82105 ("ASoC: hdac_hda: fix memleak on module unload")

That's probably fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]