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

[Valerio Setti <vsetti@baylibre.com>]

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

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 
implemented due the documented limitations in the driver) so the most 
natural solution here is to implement them as 3 different devices. Of 
course then none of these should own the data formatter exclusively.

I apologize for the lengthy reply, but I tried to provide you a the full 
picture to answer your question.

Best regards,

-- 
Valerio


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