[alsa-devel] [RFC v2 00/11] Enable HDA Codec support on Intel Platforms (Series2)

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Takashi Iwai [mailto:tiwai at suse.de]
>Sent: Tuesday, December 12, 2017 12:19 AM
>To: Ughreja, Rakesh A <rakesh.a.ughreja at intel.com>
>Cc: alsa-devel at alsa-project.org; broonie at kernel.org;
>liam.r.girdwood at linux.intel.com; pierre-louis.bossart at linux.intel.com; Koul,
>Vinod <vinod.koul at intel.com>; Patches Audio <patches.audio at intel.com>
>Subject: Re: [RFC v2 00/11] Enable HDA Codec support on Intel Platforms
>(Series2)

>
>The registration should be triggered from the top-level, i.e. the
>controller driver, once after all codecs get probed.  Otherwise, it
>becomes racy.  The user-space will start poking the device before all
>the stuff becomes ready.

Thank you very much, I understand now.

>
>That said, at the point when the codec is probed, the card shouldn't
>be registered yet.  The card gets registered after the all components
>are bound and become ready -- which is done a single
>snd_card_register() call.  At this point, the access points to
>user-space are provided.

So based on your suggestion when I relooked at the code, I could
remove the new function that I added. I could completely reuse the 
existing function with little change.

Does it look okay ?

Following is the patch, which I tested and it works for me.
I will include this in next series but for now it's only for quick reference.

I am calling this API from the hdac_hda codec driver with argument
HDA_DEV_ASOC to make sure that it does not register with the bus
again. I am not able to call this API from ASoC Platform driver because
I don't have snd_card pointer at the time of machine driver probe.

Following is the patch. 
---------------------------
Modify snd_hda_codec_new API so that it can work for legacy as well as
ASoC codec drivers. The API takes an additional argument dev_type as
HDA_DEV_LEGACY or HDA_DEV_ASOC to distinguish the caller.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja at intel.com>
---
 sound/pci/hda/hda_codec.c      | 30 +++++++++++++++++++-----------
 sound/pci/hda/hda_codec.h      |  3 ++-
 sound/pci/hda/hda_controller.c |  2 +-
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 085fd9e..252779c 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -865,7 +865,8 @@ static void snd_hda_codec_dev_release(struct device *dev)
  * Returns 0 if successful, or a negative error code.
  */
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
-		      unsigned int codec_addr, struct hda_codec **codecp)
+		      unsigned int codec_addr, struct hda_codec **codecp,
+			unsigned int dev_type)
 {
 	struct hda_codec *codec;
 	char component[31];
@@ -882,20 +883,27 @@ int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 	if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
 		return -EINVAL;
 
-	codec = kzalloc(sizeof(*codec), GFP_KERNEL);
-	if (!codec)
-		return -ENOMEM;
+	if (dev_type == HDA_DEV_LEGACY) {
 
-	sprintf(component, "hdaudioC%dD%d", card->number, codec_addr);
-	err = snd_hdac_device_init(&codec->core, &bus->core, component,
-				   codec_addr);
-	if (err < 0) {
-		kfree(codec);
-		return err;
+		codec = kzalloc(sizeof(*codec), GFP_KERNEL);
+		if (!codec)
+			return -ENOMEM;
+
+		sprintf(component, "hdaudioC%dD%d", card->number, codec_addr);
+		err = snd_hdac_device_init(&codec->core, &bus->core, component,
+						codec_addr);
+		if (err < 0) {
+			kfree(codec);
+			return err;
+		}
+
+		codec->core.type = HDA_DEV_LEGACY;
+
+	} else {
+		codec = *codecp;
 	}
 
 	codec->core.dev.release = snd_hda_codec_dev_release;
-	codec->core.type = HDA_DEV_LEGACY;
 	codec->core.exec_verb = codec_exec_verb;
 
 	codec->bus = bus;
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index d3099db..92815b7 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -306,7 +306,8 @@ struct hda_codec {
  * constructors
  */
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
-		      unsigned int codec_addr, struct hda_codec **codecp);
+			unsigned int codec_addr, struct hda_codec **codecp,
+			unsigned int dev_type);
 int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
 
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index d1eb148..ec887d7 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -1318,7 +1318,7 @@ int azx_probe_codecs(struct azx *chip, unsigned int max_slots)
 	for (c = 0; c < max_slots; c++) {
 		if ((bus->codec_mask & (1 << c)) & chip->codec_probe_mask) {
 			struct hda_codec *codec;
-			err = snd_hda_codec_new(&chip->bus, chip->card, c, &codec);
+			err = snd_hda_codec_new(&chip->bus, chip->card, c, &codec, HDA_DEV_LEGACY);
 			if (err < 0)
 				continue;
 			codec->jackpoll_interval = get_jackpoll_interval(chip);
-- 
2.7.4