diff mbox series

[2/4] ALSA: hda: Make snd_hda_codec_device_init() the only codec constructor

Message ID 20220720130622.146973-3-cezary.rojewski@intel.com (mailing list archive)
State Superseded
Headers show
Series ALSA: hda: Unify codec construction | expand

Commit Message

Cezary Rojewski July 20, 2022, 1:06 p.m. UTC
Refactor snd_hdac_ext_bus_device_init() so that it makes use of
snd_hda_codec_device_init() to create and initialize new codec device.
Causes the latter to become the sole codec device constructor.

Users of the refactored function are updated accordingly and now also
take responsibility for assigning driver's private data as that task is
no longer performed by hdac_hda_dev_probe().

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/hdaudio_ext.h     |  4 ++--
 sound/hda/ext/hdac_ext_bus.c    | 34 +++++++++++++++------------------
 sound/soc/intel/skylake/skl.c   | 24 ++++++++++-------------
 sound/soc/sof/intel/hda-codec.c | 29 ++++++++++++----------------
 4 files changed, 39 insertions(+), 52 deletions(-)

Comments

Takashi Iwai July 20, 2022, 1:13 p.m. UTC | #1
On Wed, 20 Jul 2022 15:06:20 +0200,
Cezary Rojewski wrote:
> 
> Refactor snd_hdac_ext_bus_device_init() so that it makes use of
> snd_hda_codec_device_init() to create and initialize new codec device.
> Causes the latter to become the sole codec device constructor.
> 
> Users of the refactored function are updated accordingly and now also
> take responsibility for assigning driver's private data as that task is
> no longer performed by hdac_hda_dev_probe().

Hrm, this doesn't look really right.  It means you'll introduce a hard
dependency chain in a reverse order: snd-hda-ext-core ->
snd-hda-codec.

Originally, the ext bus code was written completely independent from
the legacy HD-audio implementations, and hdac-hda driver was a kind of
wrapper / bridge for the legacy codec over the ext bus.  If we want
change this rule and make the legacy HD-audio codec always tied with
the ext bus, a likely better way would be to call
snd_hda_codec_device_init() in the caller's side (e.g. skl or sof),
then pass the newly created codec object to
snd_hdac_ext_bus_device_init() for further initialization.


thanks,

Takashi

> 
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  include/sound/hdaudio_ext.h     |  4 ++--
>  sound/hda/ext/hdac_ext_bus.c    | 34 +++++++++++++++------------------
>  sound/soc/intel/skylake/skl.c   | 24 ++++++++++-------------
>  sound/soc/sof/intel/hda-codec.c | 29 ++++++++++++----------------
>  4 files changed, 39 insertions(+), 52 deletions(-)
> 
> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> index d26234f9ee46..25c7b13db278 100644
> --- a/include/sound/hdaudio_ext.h
> +++ b/include/sound/hdaudio_ext.h
> @@ -11,8 +11,8 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
>  		      const struct hdac_ext_bus_ops *ext_ops);
>  
>  void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
> -				struct hdac_device *hdev, int type);
> +struct hda_codec *
> +snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, int type);
>  void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
>  void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);
>  
> diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
> index 765c40a6ccba..bd3c7124aca1 100644
> --- a/sound/hda/ext/hdac_ext_bus.c
> +++ b/sound/hda/ext/hdac_ext_bus.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> +#include <sound/hda_codec.h>
>  #include <sound/hdaudio_ext.h>
>  
>  MODULE_DESCRIPTION("HDA extended core");
> @@ -67,39 +68,34 @@ static void default_release(struct device *dev)
>  
>  /**
>   * snd_hdac_ext_bus_device_init - initialize the HDA extended codec base device
> - * @bus: hdac bus to attach to
> + * @bus: hda bus to attach to
>   * @addr: codec address
> - * @hdev: hdac device to init
>   * @type: codec type (HDAC_DEV_*) to use for this device
>   *
> - * Returns zero for success or a negative error code.
> + * Returns pointer to newly created codec or ERR_PTR.
>   */
> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
> -				 struct hdac_device *hdev, int type)
> +struct hda_codec *
> +snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, int type)
>  {
> -	char name[15];
> +	struct hda_codec *codec;
>  	int ret;
>  
> -	hdev->bus = bus;
> -
> -	snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
> -
> -	ret  = snd_hdac_device_init(hdev, bus, name, addr);
> -	if (ret < 0) {
> +	codec = snd_hda_codec_device_init(to_hda_bus(bus), addr, "ehdaudio%dD%d", bus->idx, addr);
> +	if (IS_ERR(codec)) {
>  		dev_err(bus->dev, "device init failed for hdac device\n");
> -		return ret;
> +		return codec;
>  	}
> -	hdev->type = type;
> -	hdev->dev.release = default_release;
> +	codec->core.type = type;
> +	codec->core.dev.release = default_release;
>  
> -	ret = snd_hdac_device_register(hdev);
> +	ret = snd_hdac_device_register(&codec->core);
>  	if (ret) {
>  		dev_err(bus->dev, "failed to register hdac device\n");
> -		snd_hdac_ext_bus_device_exit(hdev);
> -		return ret;
> +		snd_hdac_ext_bus_device_exit(&codec->core);
> +		return ERR_PTR(ret);
>  	}
>  
> -	return 0;
> +	return codec;
>  }
>  EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
>  
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 7e573a887118..5637292c2aa9 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -700,9 +700,8 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>  	struct skl_dev *skl = bus_to_skl(bus);
>  #if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
>  	struct hdac_hda_priv *hda_codec;
> -	int err;
>  #endif
> -	struct hdac_device *hdev;
> +	struct hda_codec *codec;
>  
>  	mutex_lock(&bus->cmd_mutex);
>  	snd_hdac_bus_send_cmd(bus, cmd);
> @@ -718,25 +717,22 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>  	if (!hda_codec)
>  		return -ENOMEM;
>  
> -	hda_codec->codec->bus = skl_to_hbus(skl);
> -	hdev = &hda_codec->codec->core;
> +	codec = snd_hdac_ext_bus_device_init(bus, addr, HDA_DEV_ASOC);
> +	if (IS_ERR(codec))
> +		return PTR_ERR(codec);
>  
> -	err = snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
> -	if (err < 0)
> -		return err;
> +	hda_codec->codec = codec;
> +	dev_set_drvdata(&codec->core.dev, hda_codec);
>  
>  	/* use legacy bus only for HDA codecs, idisp uses ext bus */
>  	if ((res & 0xFFFF0000) != IDISP_INTEL_VENDOR_ID) {
> -		hdev->type = HDA_DEV_LEGACY;
> -		load_codec_module(hda_codec->codec);
> +		codec->core.type = HDA_DEV_LEGACY;
> +		load_codec_module(codec);
>  	}
>  	return 0;
>  #else
> -	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
> -	if (!hdev)
> -		return -ENOMEM;
> -
> -	return snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
> +	codec = snd_hdac_ext_bus_device_init(bus, addr, HDA_DEV_ASOC);
> +	return PTR_ERR_OR_ZERO(codec);
>  #endif /* CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC */
>  }
>  
> diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> index de7c53224ac3..7c3ea4a12d63 100644
> --- a/sound/soc/sof/intel/hda-codec.c
> +++ b/sound/soc/sof/intel/hda-codec.c
> @@ -115,11 +115,10 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
>  {
>  #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
>  	struct hdac_hda_priv *hda_priv;
> -	struct hda_codec *codec;
>  	int type = HDA_DEV_LEGACY;
>  #endif
>  	struct hda_bus *hbus = sof_to_hbus(sdev);
> -	struct hdac_device *hdev;
> +	struct hda_codec *codec;
>  	u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) |
>  		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
>  	u32 resp = -1;
> @@ -142,20 +141,19 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
>  	if (!hda_priv)
>  		return -ENOMEM;
>  
> -	hda_priv->codec->bus = hbus;
> -	hdev = &hda_priv->codec->core;
> -	codec = hda_priv->codec;
> -
>  	/* only probe ASoC codec drivers for HDAC-HDMI */
>  	if (!hda_codec_use_common_hdmi && (resp & 0xFFFF0000) == IDISP_VID_INTEL)
>  		type = HDA_DEV_ASOC;
>  
> -	ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, type);
> -	if (ret < 0)
> -		return ret;
> +	codec = snd_hdac_ext_bus_device_init(&hbus->core, address, type);
> +	if (IS_ERR(codec))
> +		return PTR_ERR(codec);
> +
> +	hda_priv->codec = codec;
> +	dev_set_drvdata(&codec->core.dev, hda_priv);
>  
>  	if ((resp & 0xFFFF0000) == IDISP_VID_INTEL) {
> -		if (!hdev->bus->audio_component) {
> +		if (!hbus->core.audio_component) {
>  			dev_dbg(sdev->dev,
>  				"iDisp hw present but no driver\n");
>  			ret = -ENOENT;
> @@ -181,15 +179,12 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
>  
>  out:
>  	if (ret < 0) {
> -		snd_hdac_device_unregister(hdev);
> -		put_device(&hdev->dev);
> +		snd_hdac_device_unregister(&codec->core);
> +		put_device(&codec->core.dev);
>  	}
>  #else
> -	hdev = devm_kzalloc(sdev->dev, sizeof(*hdev), GFP_KERNEL);
> -	if (!hdev)
> -		return -ENOMEM;
> -
> -	ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, HDA_DEV_ASOC);
> +	codec = snd_hdac_ext_bus_device_init(&hbus->core, address, HDA_DEV_ASOC);
> +	ret = PTR_ERR_OR_ZERO(codec);
>  #endif
>  
>  	return ret;
> -- 
> 2.25.1
>
Cezary Rojewski July 20, 2022, 3:39 p.m. UTC | #2
On 2022-07-20 3:13 PM, Takashi Iwai wrote:
> On Wed, 20 Jul 2022 15:06:20 +0200,
> Cezary Rojewski wrote:
>>
>> Refactor snd_hdac_ext_bus_device_init() so that it makes use of
>> snd_hda_codec_device_init() to create and initialize new codec device.
>> Causes the latter to become the sole codec device constructor.
>>
>> Users of the refactored function are updated accordingly and now also
>> take responsibility for assigning driver's private data as that task is
>> no longer performed by hdac_hda_dev_probe().
> 
> Hrm, this doesn't look really right.  It means you'll introduce a hard
> dependency chain in a reverse order: snd-hda-ext-core ->
> snd-hda-codec.
> 
> Originally, the ext bus code was written completely independent from
> the legacy HD-audio implementations, and hdac-hda driver was a kind of
> wrapper / bridge for the legacy codec over the ext bus.  If we want
> change this rule and make the legacy HD-audio codec always tied with
> the ext bus, a likely better way would be to call
> snd_hda_codec_device_init() in the caller's side (e.g. skl or sof),
> then pass the newly created codec object to
> snd_hdac_ext_bus_device_init() for further initialization.


Agree. That or drop the snd_hdac_ext_bus_device_init() entirely. Will 
send an update soon.

In regard to the other subject, my plan:
- separate code used by both ALSA/ASoC into sound/hda (this includes 
many hda_codec functions)
- combine hda_bus and hdac_bus
- combine hda_codec and hdac_device
- drop HDA_DEV_ASOC
- drop hdac_hda/hdac_hdmi (once skylake-driver is gone; sof will be 
updated accordingly)
<story does not end here>


Regards,
Czarek
Pierre-Louis Bossart July 20, 2022, 4 p.m. UTC | #3
On 7/20/22 10:39, Cezary Rojewski wrote:
> On 2022-07-20 3:13 PM, Takashi Iwai wrote:
>> On Wed, 20 Jul 2022 15:06:20 +0200,
>> Cezary Rojewski wrote:
>>>
>>> Refactor snd_hdac_ext_bus_device_init() so that it makes use of
>>> snd_hda_codec_device_init() to create and initialize new codec device.
>>> Causes the latter to become the sole codec device constructor.
>>>
>>> Users of the refactored function are updated accordingly and now also
>>> take responsibility for assigning driver's private data as that task is
>>> no longer performed by hdac_hda_dev_probe().
>>
>> Hrm, this doesn't look really right.  It means you'll introduce a hard
>> dependency chain in a reverse order: snd-hda-ext-core ->
>> snd-hda-codec.
>>
>> Originally, the ext bus code was written completely independent from
>> the legacy HD-audio implementations, and hdac-hda driver was a kind of
>> wrapper / bridge for the legacy codec over the ext bus.  If we want
>> change this rule and make the legacy HD-audio codec always tied with
>> the ext bus, a likely better way would be to call
>> snd_hda_codec_device_init() in the caller's side (e.g. skl or sof),
>> then pass the newly created codec object to
>> snd_hdac_ext_bus_device_init() for further initialization.
> 
> 
> Agree. That or drop the snd_hdac_ext_bus_device_init() entirely. Will
> send an update soon.
> 
> In regard to the other subject, my plan:
> - separate code used by both ALSA/ASoC into sound/hda (this includes
> many hda_codec functions)
> - combine hda_bus and hdac_bus
> - combine hda_codec and hdac_device
> - drop HDA_DEV_ASOC
> - drop hdac_hda/hdac_hdmi (once skylake-driver is gone; sof will be
> updated accordingly)

the skylake driver cannot be removed until you have evidence that users
have switched, and SOF has other priorities that will likely conflict
with that goal. I don't even know what this 'drop hdac_hda' idea means
in detail, we need to keep an ASoC-based codec and the split between
platform/codec/machine. We are not going to move the HDaudio codec
management logic inside the SOF driver if that was the intent. The SOF
driver will focus on host/controller/DSP handling.

> <story does not end here>

I strongly recommend that we add no dependencies between hdac_ext and
hda_codec. To be clearer, we don't want to limit the hdac_ext bus and
stream management to platforms with HDaudio codecs.
Cezary Rojewski July 20, 2022, 4:18 p.m. UTC | #4
On 2022-07-20 6:00 PM, Pierre-Louis Bossart wrote:
> On 7/20/22 10:39, Cezary Rojewski wrote:
>> On 2022-07-20 3:13 PM, Takashi Iwai wrote:
>>> On Wed, 20 Jul 2022 15:06:20 +0200,
>>> Cezary Rojewski wrote:
>>>>
>>>> Refactor snd_hdac_ext_bus_device_init() so that it makes use of
>>>> snd_hda_codec_device_init() to create and initialize new codec device.
>>>> Causes the latter to become the sole codec device constructor.
>>>>
>>>> Users of the refactored function are updated accordingly and now also
>>>> take responsibility for assigning driver's private data as that task is
>>>> no longer performed by hdac_hda_dev_probe().
>>>
>>> Hrm, this doesn't look really right.  It means you'll introduce a hard
>>> dependency chain in a reverse order: snd-hda-ext-core ->
>>> snd-hda-codec.
>>>
>>> Originally, the ext bus code was written completely independent from
>>> the legacy HD-audio implementations, and hdac-hda driver was a kind of
>>> wrapper / bridge for the legacy codec over the ext bus.  If we want
>>> change this rule and make the legacy HD-audio codec always tied with
>>> the ext bus, a likely better way would be to call
>>> snd_hda_codec_device_init() in the caller's side (e.g. skl or sof),
>>> then pass the newly created codec object to
>>> snd_hdac_ext_bus_device_init() for further initialization.
>>
>>
>> Agree. That or drop the snd_hdac_ext_bus_device_init() entirely. Will
>> send an update soon.
>>
>> In regard to the other subject, my plan:
>> - separate code used by both ALSA/ASoC into sound/hda (this includes
>> many hda_codec functions)
>> - combine hda_bus and hdac_bus
>> - combine hda_codec and hdac_device
>> - drop HDA_DEV_ASOC
>> - drop hdac_hda/hdac_hdmi (once skylake-driver is gone; sof will be
>> updated accordingly)
> 
> the skylake driver cannot be removed until you have evidence that users
> have switched, and SOF has other priorities that will likely conflict
> with that goal. I don't even know what this 'drop hdac_hda' idea means
> in detail, we need to keep an ASoC-based codec and the split between
> platform/codec/machine. We are not going to move the HDaudio codec
> management logic inside the SOF driver if that was the intent. The SOF
> driver will focus on host/controller/DSP handling.


The evidence will be there : )
Also, there is nothing stopping us from adjusting skylake-driver however 
we see fit along the road. sound/soc/codecs/hda.c is a clear winner here.

And SOF is just breaking compatibility in several places due to IPC4 and 
stuff, no? There is no reason not to update sof along the road too - so 
it makes use of the aforementioned codec driver.

>> <story does not end here>
> 
> I strongly recommend that we add no dependencies between hdac_ext and
> hda_codec. To be clearer, we don't want to limit the hdac_ext bus and
> stream management to platforms with HDaudio codecs.

Yeah, we all agree here - snd_hdac_ext_bus_device_init() will be 
removed. snd_hda_codec_device_init() + registration will be used 
directly instead.
diff mbox series

Patch

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index d26234f9ee46..25c7b13db278 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -11,8 +11,8 @@  int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
 		      const struct hdac_ext_bus_ops *ext_ops);
 
 void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
-int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
-				struct hdac_device *hdev, int type);
+struct hda_codec *
+snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, int type);
 void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
 void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);
 
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 765c40a6ccba..bd3c7124aca1 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -12,6 +12,7 @@ 
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <sound/hda_codec.h>
 #include <sound/hdaudio_ext.h>
 
 MODULE_DESCRIPTION("HDA extended core");
@@ -67,39 +68,34 @@  static void default_release(struct device *dev)
 
 /**
  * snd_hdac_ext_bus_device_init - initialize the HDA extended codec base device
- * @bus: hdac bus to attach to
+ * @bus: hda bus to attach to
  * @addr: codec address
- * @hdev: hdac device to init
  * @type: codec type (HDAC_DEV_*) to use for this device
  *
- * Returns zero for success or a negative error code.
+ * Returns pointer to newly created codec or ERR_PTR.
  */
-int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
-				 struct hdac_device *hdev, int type)
+struct hda_codec *
+snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, int type)
 {
-	char name[15];
+	struct hda_codec *codec;
 	int ret;
 
-	hdev->bus = bus;
-
-	snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
-
-	ret  = snd_hdac_device_init(hdev, bus, name, addr);
-	if (ret < 0) {
+	codec = snd_hda_codec_device_init(to_hda_bus(bus), addr, "ehdaudio%dD%d", bus->idx, addr);
+	if (IS_ERR(codec)) {
 		dev_err(bus->dev, "device init failed for hdac device\n");
-		return ret;
+		return codec;
 	}
-	hdev->type = type;
-	hdev->dev.release = default_release;
+	codec->core.type = type;
+	codec->core.dev.release = default_release;
 
-	ret = snd_hdac_device_register(hdev);
+	ret = snd_hdac_device_register(&codec->core);
 	if (ret) {
 		dev_err(bus->dev, "failed to register hdac device\n");
-		snd_hdac_ext_bus_device_exit(hdev);
-		return ret;
+		snd_hdac_ext_bus_device_exit(&codec->core);
+		return ERR_PTR(ret);
 	}
 
-	return 0;
+	return codec;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
 
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 7e573a887118..5637292c2aa9 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -700,9 +700,8 @@  static int probe_codec(struct hdac_bus *bus, int addr)
 	struct skl_dev *skl = bus_to_skl(bus);
 #if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC)
 	struct hdac_hda_priv *hda_codec;
-	int err;
 #endif
-	struct hdac_device *hdev;
+	struct hda_codec *codec;
 
 	mutex_lock(&bus->cmd_mutex);
 	snd_hdac_bus_send_cmd(bus, cmd);
@@ -718,25 +717,22 @@  static int probe_codec(struct hdac_bus *bus, int addr)
 	if (!hda_codec)
 		return -ENOMEM;
 
-	hda_codec->codec->bus = skl_to_hbus(skl);
-	hdev = &hda_codec->codec->core;
+	codec = snd_hdac_ext_bus_device_init(bus, addr, HDA_DEV_ASOC);
+	if (IS_ERR(codec))
+		return PTR_ERR(codec);
 
-	err = snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
-	if (err < 0)
-		return err;
+	hda_codec->codec = codec;
+	dev_set_drvdata(&codec->core.dev, hda_codec);
 
 	/* use legacy bus only for HDA codecs, idisp uses ext bus */
 	if ((res & 0xFFFF0000) != IDISP_INTEL_VENDOR_ID) {
-		hdev->type = HDA_DEV_LEGACY;
-		load_codec_module(hda_codec->codec);
+		codec->core.type = HDA_DEV_LEGACY;
+		load_codec_module(codec);
 	}
 	return 0;
 #else
-	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
-	if (!hdev)
-		return -ENOMEM;
-
-	return snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC);
+	codec = snd_hdac_ext_bus_device_init(bus, addr, HDA_DEV_ASOC);
+	return PTR_ERR_OR_ZERO(codec);
 #endif /* CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC */
 }
 
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index de7c53224ac3..7c3ea4a12d63 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -115,11 +115,10 @@  static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
 {
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
 	struct hdac_hda_priv *hda_priv;
-	struct hda_codec *codec;
 	int type = HDA_DEV_LEGACY;
 #endif
 	struct hda_bus *hbus = sof_to_hbus(sdev);
-	struct hdac_device *hdev;
+	struct hda_codec *codec;
 	u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) |
 		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
 	u32 resp = -1;
@@ -142,20 +141,19 @@  static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
 	if (!hda_priv)
 		return -ENOMEM;
 
-	hda_priv->codec->bus = hbus;
-	hdev = &hda_priv->codec->core;
-	codec = hda_priv->codec;
-
 	/* only probe ASoC codec drivers for HDAC-HDMI */
 	if (!hda_codec_use_common_hdmi && (resp & 0xFFFF0000) == IDISP_VID_INTEL)
 		type = HDA_DEV_ASOC;
 
-	ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, type);
-	if (ret < 0)
-		return ret;
+	codec = snd_hdac_ext_bus_device_init(&hbus->core, address, type);
+	if (IS_ERR(codec))
+		return PTR_ERR(codec);
+
+	hda_priv->codec = codec;
+	dev_set_drvdata(&codec->core.dev, hda_priv);
 
 	if ((resp & 0xFFFF0000) == IDISP_VID_INTEL) {
-		if (!hdev->bus->audio_component) {
+		if (!hbus->core.audio_component) {
 			dev_dbg(sdev->dev,
 				"iDisp hw present but no driver\n");
 			ret = -ENOENT;
@@ -181,15 +179,12 @@  static int hda_codec_probe(struct snd_sof_dev *sdev, int address,
 
 out:
 	if (ret < 0) {
-		snd_hdac_device_unregister(hdev);
-		put_device(&hdev->dev);
+		snd_hdac_device_unregister(&codec->core);
+		put_device(&codec->core.dev);
 	}
 #else
-	hdev = devm_kzalloc(sdev->dev, sizeof(*hdev), GFP_KERNEL);
-	if (!hdev)
-		return -ENOMEM;
-
-	ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, HDA_DEV_ASOC);
+	codec = snd_hdac_ext_bus_device_init(&hbus->core, address, HDA_DEV_ASOC);
+	ret = PTR_ERR_OR_ZERO(codec);
 #endif
 
 	return ret;