Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

On 12/13/2013 04:18 PM, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 03:57:24PM +0200, Peter Ujfalusi wrote:
>> On 12/13/2013 03:34 PM, Mark Brown wrote:
> 
>>> No, I'd expect the wire behaviour to be identical for any 24 bit samples
>>> (that's certainly what most drivers are written for).  The memory layout
>>> differences shouldn't be visible to CODEC drivers.
> 
>> We can not change the HW... for example:
> 
> This isn't a hardware issue, this is a software issue.
> 
>> twl4030/twl5030: 32 clock cycle/channel and 24 bits used out of that.
> 
> These shouldn't be advertising themselves as supporting 24 bit format
> for ASoC then since they need 32 bit samples on the bus - this is
> actually pretty standard for things that say they support 32 bits, they
> usually just discard some of the lower bits but can cope with 32 bit on
> the interface.
> 
> The reason everything says 24_LE right now is that we're being dumb
> about how we do the constraints and CPUs do care about the memory
> layout, not because the CODECs require the blank cycles.  Of course this
> means that almost anything that does I2S should be able to advertise
> arbatrary sample size support which is one of the resons we ought to be
> doing stuff in the core to make this nicer.

Hrm, not sure about other SoCs but on OMAP most of the formats from memory
ends up on the I2S bus without any bits added/removed so the format actually
applies to the codec side as well since the data arriving to the codec will
have the same layout as it had in the memory.
But, yes some level of logic in the core might help.

>> tlv320aic3106: 24 clock cycle/channel for 24 bit audio.
> 
> This is normal ASoC behaviour.
> 
>> The wire behavior is different and this need to be known by the CPU side as well.
> 
> This isn't the way to do it, though - if the TWL drivers were ever used
> with other CPUs (admittedly unlikely) they'd run into trouble.

The twl drivers (and the dac33) is doing the correct thing IMHO: reporting
S32_LE with set .sig_bits = 24. Just to let the user space know that thay
should not use the whole 32bit range.

-- 
Péter