[alsa-devel] [PATCH 14/17] ASoC: tegra: add tegra30-ahub driver

[Stephen Warren]

On 03/31/2012 02:14 PM, Mark Brown wrote:
> On Fri, Mar 30, 2012 at 05:07:29PM -0600, Stephen Warren wrote:
> 
>> +int tegra30_ahub_is_probed(void)
>> +{
>> +	return probed;
>> +}
>> +EXPORT_SYMBOL(tegra30_ahub_is_probed);
> 
> This looks like a fairly clear sign that this should be a CODEC driver,

Certainly it should be a CODEC in the long-term, and most likely the
Tegra20 DAS driver too.

My aim here was to get something basic in place before I looked into all
the "SoC DSP" and separate FE/BE stuuf, and how to express the AHUB and
DAS as codecs.

> or possibly the DMA driver - if nothing else even if you didn't change
> anything else about the implementation it'd ensure that you didn't need
> to open code this probe checking.

Maybe just because I haven't looked into this at all, but I'm not sure
how this would solve the probing issue; I can see that the machine
driver wouldn't (ASoC rather than pdev) probe until the AHUB driver had
(pdev) probed, but how would that affect the dependent device probes? I
suppose it'd work out if the dependent devices didn't touch HW until
their ASoC probes?

> All your EXPORT_SYMBOLs should really be EXPORT_SYMBOL_GPL as you're
> using the ASoC APIs.

Yes, just a think-o (like a typo)

>> +/*
>> + * clk_apbif isn't required for a theoretical I2S<->I2S configuration where
>> + * no PCM data is read from or sent to memory. However, that's an unlikely
>> + * use-case, and not something the rest of the driver supports right now, so
>> + * we'll just treat the two clocks as one for now.
> 
> I rather suspect you'll find your system architects view that as a
> resonable use case for things like voice calls where the audio for the
> call would go from the baseband to the CODEC or bluetooth without going
> into main memory.  Of course many CODECs can take this path directly
> allowing the CPU to be powered off so...

Yes, the roots of this patch were actually written long ago before I was
really aware of that use-case. I should probably update the comment.