Re: [PATCH 15/17] ASoC: tegra: add tegra30-i2s driver

[Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On Sat, Mar 31, 2012 at 08:17:10PM -0600, Stephen Warren wrote:
> On 03/31/2012 03:34 PM, Mark Brown wrote:

> >> +#ifdef CONFIG_DEBUG_FS
> >> +static int tegra30_i2s_show(struct seq_file *s, void *unused)
> >> +{

> > Abstraction please - this is open coded in several of your drivers.

> There's very little code here; unifying it would require passing the
> clock enable/disable and register read functions to any common code,
> which would end up making everything rather more complex. Sometimes

If you move the clock management into runtime PM that'd provide a
convenient abstraction for keeping the clocks and whatever else is
needed on without much complexity.  There's some other (non-ASoC)
drivers that do this, it works pretty well.

> there are multiple register read functions for different chunks of the
> address space. I guess if we did have regmap for MMIO, these debugfs
> files would pretty much come for free. Is that what you're looking for here?

That's one of the ideas, yes.  It'd need some work to make it happen
(mainly due to the use of mutexes), or you could mark things that need
to be attacked from register context

> > Looking at this it seems like all that's required is to propagate stream
> > events back up the chain and do the routing, there doesn't seem to be
> > much other interaction between the AHUB and the interfaces?

> I suspect that's pretty much what representing AHUB as a CODEC would
> turn into, or very similar anyway. Is it OK to get the basic

Yes, the above question was aimed at trying to figure out which way of
representing the device would work best - looking at this code it seems
like a CODEC.

> functionality in first and then refactor this, or would you prefer going
> straight to the complete solution?

It seems best to at least hook things in so machine drivers know about
the audio hub (in their DT bindings if nothing else) even if the actual
implementation is something like you've got now.  That way any churn is
limited to the CPU drivers as much as possible.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]