Re: [PATCH RFC v2 05/11] ASoC: dt-bindings: amlogic: add schema for audin-formatter and audin-toddr

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/04/2026 23:51, Valerio Setti wrote:
> Hi,
> 
> thanks for your review and the feedbacks.
> 
>>> +examples:
>>> +  - |
>>> +    audio-controller@a040 {
>>> +      compatible = "amlogic,meson-gxbb-audin-decoder-i2s",
>>> +                   "amlogic,meson-gx-audin-decoder-i2s";
>>> +      #sound-dai-cells = <0>;
>>> +      sound-name-prefix = "AUDIN I2S Decoder";
>>> +      reg = <0xa040 0x4>;
>>
>> One register-wide block? I have doubts this is a separate device
> 
> I understand that this might look as weird configuration, so please let 
> me explain what's going on here.
> 
> In this SoC the audio part is split into 2 blocks: AIU for the audio 
> output (already supported in the kernel) and AUDIN for the input (which 
> is what I'm trying to add with this patch series). Unfortunately from 
> the clock management point of view the two of them are not indipendent 
> and interface clocks are in AIU register range. Moreover the two systems 

Two devices of one-register-interface having mixed clock management is a
CLEAR sign these are one device.

> are not in a continguous memory range, so creating a single audio 
> component that implements both is not feasible (unless we want to add 
> some dirty tricks with multiple regmaps, etc).

OK, but then what is between these? Again, register ranges ARE NOT
4-bytes wide on a SoC. On several SoCs they are aligned per page or
0x1000 or more. I have no clue how it is here, but 4 bytes is just plain
warning sign.

> This is where the AXG design comes into play: we use the backend DAI 
> provided from AIU for both playback and capture and then we attach 
> formatters (i.e. the audin-decoder-i2s we're discussing about) to 
> properly format the data.
> This explains why this is a relatively simple device with very few (1) 
> register. To be noted that for example also similar component on the AXG 
> platform (axg-tdmin.c) claims to have a larger register range, but in 
> fact is almost entirely using the 1st register with only 2 occurences 
> for the 2nd and 3rd. IMHO this is not that different from what I'm 
> trying to achieve in this series for the GX platform.
> Also looking at the implementation of "audin-decoder-i2s.c", even though 
> it uses a single register, it really provides functionalities i.e. it's 
> not a useless device and it can also be expanded in the future to 
> support 24-bit sample format.

I understand it is not useless. I claim this is writing bindings per
driver. I claim this is not a correct representation of hardware,
because hardware is not a device with 4 bytes of address space.

> 
> Then the question might be: why not merging this together with 
> audin-fifo? Becuase these are 3 indipendent instances from the interface 
> and each of which can receive data from the interface. Each of them has 
> its own registers range (and optionally also an interrupt - which is not 

4 bytes is not "registers range". It is one register of someone else's
range.


Best regards,
Krzysztof

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic