-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, December 11, 2017 8:51 PM To: Ughreja, Rakesh A rakesh.a.ughreja@intel.com Cc: alsa-devel@alsa-project.org; broonie@kernel.org; liam.r.girdwood@linux.intel.com; pierre-louis.bossart@linux.intel.com; Koul, Vinod vinod.koul@intel.com; Patches Audio patches.audio@intel.com Subject: Re: [RFC v2 00/11] Enable HDA Codec support on Intel Platforms (Series2)
On Mon, 11 Dec 2017 16:10:32 +0100, Ughreja, Rakesh A wrote:
good in one side. OTOH, it is unacceptably messy, in special, because the device creation and registration phases are mixed up.
I am not sure if I understand this fully. "Device creation and registration phases are mixed up". If you can explain bit more, I can try to improve it.
e.g. in the patch 7, snd_hda_asoc_codec_new() has the code like:
+int snd_hda_asoc_codec_new(struct hda_bus *bus, struct snd_card *card,
unsigned int codec_addr, struct hda_codec **codecp)+{
- struct hda_codec *codec = NULL;
....
- err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);
which means that the device is being created in this function. At the same time, however, this function tries to register that codec device itself soon after the above:
- list_for_each_entry(snd_dev, &card->devices, list) {
if (snd_dev->type == SNDRV_DEV_CODEC) {void *device_data = snd_dev->device_data;err = snd_device_register(card, device_data);if (err < 0)This is wrong. The registration must happen at a later stage once after all components get ready. After all, that's the reason we have the dev_register individual callback in snd_dev_ops.
So when you say "after all the components get ready" what exactly you are referring to ?
I am calling snd_hda_asoc_codec_new function from probe function of hdac_driver. So I was thinking that since the codec is already enumerated from the bus point of view, I can register. What am I missing here ?
The exception is a case with multiple probe-bindings like USB-audio that matches multiple times per interface. But it's an exception, and not applied to the normal probe. (And if any, you should just register the whole via snd_card_register() call.)
And exporting each codec table doesn't look nice. We need to find a better way...
Do you think accessing the codec table via some EXPORT function is a better way ?
We need more consideration. I don't think we have done it fully enough. At least, exporting each table sounds like a bad idea. Ideally, we should re-use the whole codec driver as is; i.e. the single codec probe should work for both legacy and ext controllers.
The generic hdac_hda driver uses the legacy driver APIs once the hdac_dev is enumerated on the hdac_bus. While the legacy driver APIs are used by controller driver.
Do you think it's a wrong path to go ? Should the legacy driver APIs be called directly from ASoC platform driver ?