On Mon, May 09, 2022 at 09:22:42PM +0200, Kirill Marinushkin wrote:
> On 5/9/22 10:37 AM, Charles Keepax 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`.

> 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?

If doing this for the CODEC side of the link causes an issue it's just
exposing an existing bug on the CPU side of the link which may already
be affecting other systems - like Charles says the CODEC is expecting a
fixed bit order regardless of the memory layout of the data.

> > 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?

On the CODEC driver side it is meaningless.  On the CPU side it controls
the in memory layout of the data.

> > 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.

If the CPU side of the link is fine then there should be no problem, we
simply start supporting both endian settings all the way through the
chain, if userspace chooses something that wasn't supported before then
the CPU side driver will look at what's being configured and set up the
hardware appropriately.