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

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

[Peter Ujfalusi]

Hi,

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: Balance the runtime PM disable in codec remove

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

-- 
2.56.0

[PATCH 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 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 3/3] ASoC: hdac_hda: Balance the runtime PM disable in codec remove

[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.

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index cae5711e979c..bd4b113401a8 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -554,6 +554,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 +562,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 3/3] ASoC: hdac_hda: Balance the runtime PM disable in codec remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: hdac_hda: codec stays powered in D0 on re-probe due to RPM_SUSPENDED state
--

commit fc19b5dab0635a3fee4ca467d334fb76459c8230
Author: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: hdac_hda: Balance the runtime PM disable in codec remove

This patch re-enables runtime PM at the end of hdac_hda_codec_remove() to
balance its earlier pm_runtime_disable() call so that runtime PM does not
remain disabled after snd_hda_codec_register() runs on a subsequent probe.

> diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
> index cae5711e979c3..bd4b113401a8d 100644
> --- a/sound/soc/codecs/hdac_hda.c
> +++ b/sound/soc/codecs/hdac_hda.c
> @@ -554,6 +554,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 +562,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);
>  }

[Severity: Medium]
Does the codec hardware still stay powered in D0 after an unbind and re-bind
cycle even with this balanced pm_runtime_enable() call?

On initial device creation, snd_hdac_device_init() sets runtime_status to
RPM_ACTIVE while runtime PM is disabled (disable_depth == 1). However, when
hdac_hda_codec_remove() finishes, runtime_status is left as RPM_SUSPENDED:
either the codec was already runtime-suspended before an ASoC card unbind,
or on a codec driver unbind __device_release_driver() calls
device_unbind_cleanup() -> pm_runtime_reinit(), which sees
!pm_runtime_enabled(dev) and calls pm_runtime_set_suspended(dev):

drivers/base/power/runtime.c:pm_runtime_reinit() {
	if (pm_runtime_enabled(dev))
		return;

	if (dev->power.runtime_status == RPM_ACTIVE)
		pm_runtime_set_suspended(dev);
    ...
}

When the codec is probed again, hdac_hda_codec_probe() enters with
disable_depth == 1, usage_count == 1, and runtime_status == RPM_SUSPENDED.
It acquires the HDA link via snd_hdac_ext_bus_link_get() and calls
snd_hda_codec_device_new(), which powers the hardware up to AC_PWRST_D0 and
calls pm_runtime_forbid(). Because runtime PM is still disabled at that
point, rpm_resume() returns -EACCES and leaves runtime_status as
RPM_SUSPENDED.

At the end of hdac_hda_codec_probe(), it assumes runtime_status is still
RPM_ACTIVE from initial device creation and never calls
pm_runtime_set_active(&hdev->dev):

sound/soc/codecs/hdac_hda.c:hdac_hda_codec_probe() {
    ...
	/*
	 * hdac_device core already sets the state to active and calls
	 * get_noresume. So enable runtime and set the device to suspend.
	 * pm_runtime_enable is also called during codec registration
	 */
	pm_runtime_put(&hdev->dev);
	pm_runtime_suspend(&hdev->dev);
}

When snd_hda_codec_register() subsequently runs, it enables runtime PM and
calls snd_hda_power_down():

sound/hda/common/codec.c:snd_hda_codec_register() {
    ...
	if (device_is_registered(hda_codec_dev(codec))) {
		snd_hda_codec_display_power(codec, true);
		pm_runtime_enable(hda_codec_dev(codec));
		/* it was powered up in snd_hda_codec_new(), now all done */
		snd_hda_power_down(codec);
    ...
}

Because dev->power.runtime_status is already RPM_SUSPENDED, rpm_suspend()
inside snd_hda_power_down() returns 1 immediately without invoking
hda_codec_runtime_suspend() or scheduling autosuspend. Doesn't this leave the
codec hardware powered in AC_PWRST_D0, the HDA link reference from
hdac_hda_codec_probe() held, and the display power reference from
snd_hda_codec_register() held, unless pm_runtime_set_active(&hdev->dev) is
also called in hdac_hda_codec_probe() (similar to hda_codec_probe_complete()
in sound/soc/codecs/hda.c)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027083840.28813-1-peter.ujfalusi@linux.intel.com?part=3

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

[Mark Brown]

On Tue, 27 Oct 2026 10:38:37 +0200, Peter Ujfalusi wrote:
> ASoC: hdac_hda: Fix module unbind/bind via sysfs
> 
> Hi,
> 
> 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:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

[1/3] ASoC: hdac_hda: Allocate the private data at codec driver bind
      https://git.kernel.org/broonie/sound/c/7d357eae973a
[2/3] ASoC: hdac_hda: Restore the codec device type on unbind
      https://git.kernel.org/broonie/sound/c/08a44bde821d
[3/3] ASoC: hdac_hda: Balance the runtime PM disable in codec remove
      https://git.kernel.org/broonie/sound/c/f26c0d5f91d9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 3/3] ASoC: hdac_hda: Balance the runtime PM disable in codec remove

[Péter Ujfalusi]

On 27/10/2026 11:12, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] ASoC: hdac_hda: codec stays powered in D0 on re-probe due to RPM_SUSPENDED state
> --
> 
> commit fc19b5dab0635a3fee4ca467d334fb76459c8230
> Author: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> 
> ASoC: hdac_hda: Balance the runtime PM disable in codec remove
> 
> This patch re-enables runtime PM at the end of hdac_hda_codec_remove() to
> balance its earlier pm_runtime_disable() call so that runtime PM does not
> remain disabled after snd_hda_codec_register() runs on a subsequent probe.
> 
>> diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
>> index cae5711e979c3..bd4b113401a8d 100644
>> --- a/sound/soc/codecs/hdac_hda.c
>> +++ b/sound/soc/codecs/hdac_hda.c
>> @@ -554,6 +554,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 +562,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);
>>  }
> 
> [Severity: Medium]
> Does the codec hardware still stay powered in D0 after an unbind and re-bind
> cycle even with this balanced pm_runtime_enable() call?
> 
> On initial device creation, snd_hdac_device_init() sets runtime_status to
> RPM_ACTIVE while runtime PM is disabled (disable_depth == 1). However, when
> hdac_hda_codec_remove() finishes, runtime_status is left as RPM_SUSPENDED:
> either the codec was already runtime-suspended before an ASoC card unbind,
> or on a codec driver unbind __device_release_driver() calls
> device_unbind_cleanup() -> pm_runtime_reinit(), which sees
> !pm_runtime_enabled(dev) and calls pm_runtime_set_suspended(dev):
> 
> drivers/base/power/runtime.c:pm_runtime_reinit() {
> 	if (pm_runtime_enabled(dev))
> 		return;
> 
> 	if (dev->power.runtime_status == RPM_ACTIVE)
> 		pm_runtime_set_suspended(dev);
>     ...
> }
> 
> When the codec is probed again, hdac_hda_codec_probe() enters with
> disable_depth == 1, usage_count == 1, and runtime_status == RPM_SUSPENDED.
> It acquires the HDA link via snd_hdac_ext_bus_link_get() and calls
> snd_hda_codec_device_new(), which powers the hardware up to AC_PWRST_D0 and
> calls pm_runtime_forbid(). Because runtime PM is still disabled at that
> point, rpm_resume() returns -EACCES and leaves runtime_status as
> RPM_SUSPENDED.
> 
> At the end of hdac_hda_codec_probe(), it assumes runtime_status is still
> RPM_ACTIVE from initial device creation and never calls
> pm_runtime_set_active(&hdev->dev):
> 
> sound/soc/codecs/hdac_hda.c:hdac_hda_codec_probe() {
>     ...
> 	/*
> 	 * hdac_device core already sets the state to active and calls
> 	 * get_noresume. So enable runtime and set the device to suspend.
> 	 * pm_runtime_enable is also called during codec registration
> 	 */
> 	pm_runtime_put(&hdev->dev);
> 	pm_runtime_suspend(&hdev->dev);
> }
> 
> When snd_hda_codec_register() subsequently runs, it enables runtime PM and
> calls snd_hda_power_down():
> 
> sound/hda/common/codec.c:snd_hda_codec_register() {
>     ...
> 	if (device_is_registered(hda_codec_dev(codec))) {
> 		snd_hda_codec_display_power(codec, true);
> 		pm_runtime_enable(hda_codec_dev(codec));
> 		/* it was powered up in snd_hda_codec_new(), now all done */
> 		snd_hda_power_down(codec);
>     ...
> }
> 
> Because dev->power.runtime_status is already RPM_SUSPENDED, rpm_suspend()
> inside snd_hda_power_down() returns 1 immediately without invoking
> hda_codec_runtime_suspend() or scheduling autosuspend. Doesn't this leave the
> codec hardware powered in AC_PWRST_D0, the HDA link reference from
> hdac_hda_codec_probe() held, and the display power reference from
> snd_hda_codec_register() held, unless pm_runtime_set_active(&hdev->dev) is
> also called in hdac_hda_codec_probe() (similar to hda_codec_probe_complete()
> in sound/soc/codecs/hda.c)?

Yes, this is valid, I will add the pm_runtime_set_active() call.
Now tested on device and it is reflecting correct state across the
expanded test matrix.

> 

-- 
Péter