On Mon, May 09, 2022 at 09:22:42PM +0200, Kirill Marinushkin wrote:
On 5/9/22 10:37 AM, Charles Keepax wrote:
On Sun, May 08, 2022 at 10:34:12PM +0200, Kirill Marinushkin wrote: 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`.
It will break if the CPU side claims it supports endians which it doesn't which I believe is what you have in your system.
As soon as we start setting `endianness = 1`, the function `convert_endianness_formats()` will extend LE to (LE | BE), right?
Correct.
If so, setting `endianness = 1` is the source of an error, right?
No, potentially it exposes an error but it is not the source of the error. Endian should have no meaning once you cross an I2S bus.
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.
Why are you sure the CPU part of the DAI link isn't broken? I2S defines the order bits should be sent out on the bus, this means if the CPU side recieves big endian or little endian data it doesn't matter, the actual bus signals should be identical in both cases. If only one endian works the actual signals on the bus must have been different depending on which endian you played in which case the host side doesn't match the I2S specification.
My point is that what you are seeing is exactly what you get if your CPU DAI claimed to support both big and little endian but just puts that data directly out on the bus, rather than adapting to the endian of data it receives.
And to be fair this is exactly what I am trying to tackle with this series. The fact we have so many CODECs that are incorrectly not specifying the endianness flag means that it is very easy for the host side guys to do testing and not realise that the CODEC is the one limiting the supported endians on the DAI link and thus miss they have claimed to support endians they don't.
I believe you are right that there is a reasonable chance will see some breakage after these patches, but that breakage should be exposing host side bugs which we need to fix anyway.
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.
That comment in my original post was in respect to CODECs that recieve/transmit audio directly over a SPI bus rather than a normal audio bus. The is no standard governing how audio data is transmitted over the SPI bus, thus endian could be important, although I need to do some more work to analyse how people are using these links. Often they are implemented effectively as a CPU side DAI anyway.
Thanks, Charles