[alsa-devel] [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

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