alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Vinod Koul <vinod.koul@intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: liam.r.girdwood@linux.intel.com, patches.audio@intel.com,
	alsa-devel@alsa-project.org, Mark Brown <broonie@kernel.org>,
	Jeeja KP <jeeja.kp@intel.com>
Subject: Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function
Date: Sat, 23 May 2015 11:21:56 +0530	[thread overview]
Message-ID: <20150523055156.GZ3140@localhost> (raw)
In-Reply-To: <s5hsiao32g6.wl-tiwai@suse.de>

On Fri, May 22, 2015 at 08:13:45PM +0200, Takashi Iwai wrote:
> At Fri, 22 May 2015 18:41:06 +0100,
> Mark Brown wrote:
> > 
> > On Fri, May 22, 2015 at 03:35:03PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > 
> > > > I'm sorry but I still don't entirely understand what this is supposed to
> > > > do.  It *looks* like it's trying to create a bus for HDA with this:
> > 
> > > > but it's not actually defining a driver model bus type (though it does
> > > > use driver_register()) and I'd expect the bus type to be provided by the
> > > > generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
> > > > it's not entirely obvious how it all fits together and the changelog for
> > > > that just talks about moving code around.
> > 
> > > The bus is already defined in sound/hda.  There, the actual binding is
> > 
> > I see there's a bus_type defined there but this is calling raw
> > device_register() and providing a match function so it seems like
> > there's something missing or an abstraction problem.  I'd expect a bus
> > to be providing things like match functions and bus specific
> > registration functions.
> 
> This was intentionally left in that way, so that the upper core layer
> (HDA ASoC or legacy core parts) can implement the matching method more
> specifically.  It can be done in the hdac_bus itself, but currently
> still leaves more room until we see the exact matching condition for
> ASoC.
> 
> Once when the matching method can be unified, we can move the it to
> hdac_bus.  But it's not necessarily done now.
> 
> > This is all setting off alarm bells, both the code and the very
> > aggressive pushing.
> > 
> > > done in each hda driver specifics, i.e. in sound/soc/hda and
> > > sound/pci/hda.  It's the way to allow binding completely different
> > > drivers for the very same PCI ID.
> > 
> > I don't really understand this, sorry.  What impact would HDA bus
> > implementation have on PCI device drivers (by the time you're
> > registering HDA devices presumably the PCI device will already be
> > enumerated)?  
> >
> > Having two drivers for the same PCI function doesn't sound like an
> > *obviously* good idea (as we discussed recently) but that's a bit of a
> > separate thing.
> 
> Suggesting two PCI IDs might have been confusing, sorry.
> 
> The point is that a HD-audio object can be inherited to two different
> level of objects, legacy and ASoC.  Both are bound on the same bus,
> but to the corresponding drivers.  Both objects use the very same bus
> ops, thus they share the same hdac_bus.
Thanks a bunch Takashi for clarifying :)

Mark,

I guess one clarification may still help, The SKL Audio controller is PCI
device, the PCM driver in this series will load against the PCI device.
Now the driver will go ahead and initialize the HDA bus. Also we provide our
own matching function here.

The matching function is for matching the devices on HDA link. The probing
of the bus will find the HDA codecs present in the bus and we will match
them based on Vendor ID and Device ID in match function above.

Let us know if you have more questions

-- 
~Vinod

  reply	other threads:[~2015-05-23  5:50 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-11 10:53 [PATCH v4 0/7] ASoC: intel - add skylake PCM driver Vinod Koul
2015-05-11 10:53 ` [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function Vinod Koul
2015-05-22 12:56   ` Mark Brown
2015-05-22 13:35     ` Takashi Iwai
2015-05-22 17:41       ` Mark Brown
2015-05-22 18:13         ` Takashi Iwai
2015-05-23  5:51           ` Vinod Koul [this message]
2015-05-25 10:48             ` Mark Brown
2015-05-25 11:21               ` Vinod Koul
2015-05-25 11:55                 ` Takashi Iwai
2015-05-25 13:58                   ` Mark Brown
2015-05-26  5:24                     ` Takashi Iwai
2015-05-26 13:32                       ` Mark Brown
2015-05-26 13:41                         ` Takashi Iwai
2015-05-26 19:43                           ` Mark Brown
2015-05-27  6:05                             ` Takashi Iwai
2015-05-27 18:34                               ` Mark Brown
2015-05-27 19:17                                 ` Takashi Iwai
2015-05-28 19:53                                   ` Mark Brown
2015-05-29  4:58                                     ` Takashi Iwai
2015-05-29  8:15                                       ` Vinod Koul
2015-05-29 17:35                                         ` Mark Brown
2015-06-01  5:05                                           ` Vinod Koul
2015-06-02 10:38                                             ` Mark Brown
2015-06-02 12:25                                               ` Vinod Koul
2015-05-11 10:54 ` [PATCH v4 2/7] ALSA: hda - add new HDA registers Vinod Koul
2015-05-22 12:58   ` Mark Brown
2015-05-22 13:32     ` Takashi Iwai
2015-05-11 10:54 ` [PATCH v4 3/7] ASoC: hda - add asoc hda core bus, controller and stream helpers Vinod Koul
2015-05-26 18:51   ` Mark Brown
2015-05-27  5:40     ` Vinod Koul
2015-05-11 10:54 ` [PATCH v4 4/7] ASoC: intel - add Skylake HDA platform driver Vinod Koul
2015-05-11 10:54 ` [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver Vinod Koul
2015-05-29 17:41   ` Mark Brown
2015-05-29 18:25     ` Takashi Iwai
2015-06-02 10:45       ` Mark Brown
2015-06-02 10:53         ` Takashi Iwai
2015-06-02 11:07           ` Mark Brown
2015-06-02 11:57             ` Takashi Iwai
2015-06-02 12:39             ` Vinod Koul
2015-06-02 14:30               ` Mark Brown
2015-06-01  5:13     ` Vinod Koul
2015-06-01  5:32       ` Takashi Iwai
2015-06-02 10:42         ` Mark Brown
2015-06-02 10:48           ` Takashi Iwai
2015-06-02 11:10             ` Mark Brown
2015-06-02 11:44               ` Takashi Iwai
2015-06-02 12:29                 ` Vinod Koul
2015-05-11 10:54 ` [PATCH v4 6/7] ASoC: intel - add makefile support for SKL driver Vinod Koul
2015-05-11 10:54 ` [PATCH v4 7/7] ASoC: intel - adds support for decoupled mode in skl driver Vinod Koul
2015-05-22 12:20 ` [PATCH v4 0/7] ASoC: intel - add skylake PCM driver Vinod Koul
2015-05-22 13:12   ` Mark Brown
2015-05-25  6:57 ` Takashi Iwai
2015-05-25 11:24   ` Vinod Koul
2015-05-25 11:58     ` Takashi Iwai
2015-05-26  4:14       ` Vinod Koul
2015-05-26  5:27         ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150523055156.GZ3140@localhost \
    --to=vinod.koul@intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=jeeja.kp@intel.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=patches.audio@intel.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).