Re: [PATCH 01/38] ASoC: soc-component: Add comment for the endianness flag

[Kirill Marinushkin <kmarinushkin@birdec.com>]

Hello Charles,

On 5/9/22 10:37 AM, Charles Keepax wrote:
> On Sun, May 08, 2022 at 10:34:12PM +0200, Kirill Marinushkin wrote:
>> In the [PATCH 00/38] of this patch set, you write:
>>> 2) Devices behind non-audio buses, SPI just moves bits and doesn't
>>> really define an endian for audio data on the bus. Thus it seems the
>>> CODEC probably can care about the endian. The only devices that fall
>>> into this group (mostly for AoV) are: rt5514-spi.c, rt5677-spi.c,
>>> cros_ec_codec.c (only the AoV).
>>  From my experience with some PCM codecs by TI, they can not care about the
>> endianness. For example, in driver [1], if I allow BE format as
>> well, and configure
>> the sound card to use BE, it will not work. I have no sound with BE.
>> In these cases, the codec HW supports *only* LE. That's why their
>> `snd_soc_dai_driver`
>> structures provide only LE in the `format` field.
>> If such drivers specify `endianness = 1`, then `soc-core` will
>> extend their supported
>> formats with BE counter-parts, see [2]. AFAIU, it will have the same
>> effect, as if the
>> drivers themselves provided their formats in both LE | BE.
>>
>> I don't think adding `endianness = 1` to such codecs will work as expected.
>> Unfortunately, I don't have an easy access to HW today, to confirm
>> or disprove
>> this understanding.
>>
> This sounds like an error on the CPU side of the DAI link rather
> than the CODEC side of the DAI link. The formats that will be
> supported on the link are the union of the CPU and CODEC supported
> formats, ie. a format must be supported on both for the DAI to
> support it.


Yes, agree, both sides of the DAI link should provide only endianness 
they support.

This works currently, but, from my understending, it will break, after 
we set `endianness = 1`.

As soon as we start setting `endianness = 1`, the function 
`convert_endianness_formats()` will extend LE to (LE | BE), right?

If so, setting `endianness = 1` is the source of an error, right?


I could be missing something. Let's see what other reviewers think.


> The CPU I2S hardware should be sending out the bits in the same
> order regardless of if the data you feed it is BE or LE, as I2S
> specifies an ordering for the bits.


What does the endianness in the driver configure, then?


> If this is not the case then
> the host I2S controller is claiming to support an endian it does
> not, and the problem should be fixed on that side by removing the
> supported endian.


I think we have a misundersanding of my example.

In my example, i don't mean, that my CPU part of the DAI link is broken.

What i tried to demonstrate - is that if i set the unsupported 
endianness, i wouldn't expect that "the CODEC probably can care about 
the endian", as the message in [PATCH 00/38] suggests. I would expect, 
that i will have no sound.


Best regards,

Kirill

>
> Thanks,
> Charles