>-----Original Message----- >From: Takashi Iwai [mailto:tiwai@xxxxxxx] >Sent: Monday, December 11, 2017 8:51 PM >To: Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx> >Cc: alsa-devel@xxxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; >liam.r.girdwood@xxxxxxxxxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; Koul, Vinod ><vinod.koul@xxxxxxxxx>; Patches Audio <patches.audio@xxxxxxxxx> >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 ? _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
