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