At Tue, 26 May 2015 20:43:16 +0100, Mark Brown wrote:
On Tue, May 26, 2015 at 03:41:32PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
What are these objects supposed to be then and why does the bus for HDA CODECs know about the differences between them? They're both called HDA CODECs which might lead one to think that they are supposed to represent the same thing.
Both legacy and ASoC drivers contain own codec drivers, but their classes inherit the same HD-audio core object class that is managed by HD-audio core bus. Meanwhile, since the id table is (still) not embedded into HD-audio core object, the match method is still placed in each (asoc and legacy) driver core code.
This is still as clear as mud, sorry - why is the ID table not something that the bus handles? It seems like a core part of what a bus abstracts. Having two completely separate sets of controller and device drivers that can't interact with each other (which seems like what we're ending up with here) seems really worrying and I don't think I've seen any plan for unification either. It's not just the match function stuff by the way, some of the later patches also made me wonder about the mutiple implementations of the bus thing.
HDA bus is the place for communication between a device and a controller. This is common, no matter whether asoc or legacy. That is, an ASoC HDA controller may communicate even with an HDA legacy codec device, if really wanted. There is no distinction there.
The fact you might be missing from the beginning of the story is that we need to implement two different driver sets for ASoC, both for controller and codec drivers. They can't be unified with legacy HDA unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into ALSA core). Also ASoC has the different ways of registration, too.
For managing these two sets of stuff better, HDA bus provides the place for controller/codec communication. All verbs are executed via snd_hdac_bus_exec_verb(), basically. Also, it provides dozens of helper codes to implement PCM and codec/controller/stream management codes. Then both HDA legacy and asoc drivers are built up on them. This is the current standing point.
The lack of id table in hdac_bus object is just because of the current form of the legacy HDA *codec* driver. They aren't translated to a normal struct device_driver yet. It's not done because such a translation is a large amount even though it's done systematically, and there is always a risk of breakage by that. But the plan is to achieve it in 4.3 kernel. So you can see the unified match method sooner or later. No reason to stick with it.
I think I'm really not understanding what this bus is supposed to be doing. It's possible that there is something I'm missing here (I imagine there's been some off-list discussions of this) but right now what I'm seeing is that either the bus is not yet at a point where it abstracts enough to really be a bus (it's not clear to me how something would use the bus) or the existing HDA code has been converted to use the new bus before we're ready to do that.
What you all seem to be saying is that the existing code for HDA is too fragile to touch much so we don't want to move much of its functionality to the new bus yet. I can believe that but I think I'd be a lot happier if we were handling that by having the existing HDA code override bus functionality rather than by implementing key bus functionality in random places. It feels like trying to use the bus now is adding technical debt.
Well, the patchset looks more complicated because lots of codes you're seeing in this patchset are specific to SKL hardware extension, and it doesn't (maybe won't) exist in HDA legacy. For example, patch 3 contains many such codes, which is likely the point where you stopped reading the rest patches.
If you look through the patches, most of codes are wrappers, just calling snd_hdac_*(). PCM and PCI driver stuff need more codes than that because of ASoC-specific data and ops (dai_ops, dai_driver, etc) and to be an individual PCI driver. Some codes might be able to be unified in future, but not everything, as long as they are implemented as individual drivers.
Takashi