linux-kernel - Re: [PATCH 1/3] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 May 2022 10:52:45 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Jiaxin Yu <jiaxin.yu@...iatek.com>,
        Nícolas F. R. A. Prado 
        <nfraprado@...labora.com>, Mark Brown <broonie@...nel.org>
Cc:     kernel@...labora.com,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Shane Chien <shane.chien@...iatek.com>,
        alsa-devel@...a-project.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 1/3] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share
 properties

Il 05/05/22 10:48, Jiaxin Yu ha scritto:
> On Thu, 2022-05-05 at 10:08 +0200, AngeloGioacchino Del Regno wrote:
>> Il 29/04/22 22:30, Nícolas F. R. A. Prado ha scritto:
>>> The Mediatek AFE PCM controller for MT8192 allows sharing of an I2S
>>> bus
>>> between two busses. Add a pattern for these properties in the
>>> dt-binding.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
>>> ---
>>>
>>>    Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml | 5
>>> +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/mt8192-afe-
>>> pcm.yaml b/Documentation/devicetree/bindings/sound/mt8192-afe-
>>> pcm.yaml
>>> index 7a25bc9b8060..5b03c8dbf318 100644
>>> --- a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
>>> @@ -54,6 +54,11 @@ properties:
>>>          - const: aud_infra_clk
>>>          - const: aud_infra_26m_clk
>>>    
>>> +patternProperties:
>>> +  "^i2s[0-35-9]-share$":
>>> +    description: Name of the I2S bus that is shared with this bus
>>> +    pattern: "^I2S[0-35-9]$"
>>> +
>>>    required:
>>>      - compatible
>>>      - interrupts
>>>
>>
>> The only other way of doing this would be to complicate this in the
>> driver
>> so that we can do something like
>>
>> "i2s-share = <0 2>";  instead of  i2s0-share = "I2S2";
>>
>> ...and I don't think that this would be any more straightforward than
>> the
>> provided way.
>>
>> There's an improvement that we can do to that pattern description
>> though,
>> which would be explaining that declaring 'i2s0-share = "I2S2"' means
>> that
>> I2S2's data pin will be used as DATA-OUT, while i2s0 is DATA-IN.
>>
>> Another thing that comes to mind here is that this is a MediaTek
>> specific
>> property and *not* a generic one, which means that both the driver
>> and
>> this binding should be fixed to get a "mediatek," prefix, so, this
>> property
>> should - in reality - be "mediatek,i2s[0-35-9]-share" instead.
>>
>> I think that everyone agrees about that, but let's see what the
>> others say.
>>
>> Cheers,
>> Angelo
> 
> Hi Angelo,
> 
> 'i2s0-share = "I2S2"' means that if we want use I2S0, there need open
> I2S2 to provide clock. Conversely, if we want to use I2S2, we don't
> need to open I2S0. However, MediaTek I2S0 and I2S2 hardware are
> generally designed as input. So usually we use 'i2s0-share = "I2S1"'.
> Even numbers represent input, odd numbers represent output.
> 
> Yes, I think adding the "mediatek," prefix is the right way to define a
> non-generic property.
> 

Hello Jiaxin,

if I get this correctly, i2s0-share = "I2S2" would be *invalid*... as you
just explained, i2sX, where:

X = even number -> always DATA IN
X = odd number  -> always DATA OUT

...this means that the dt-binding needs a pattern to specify that only odd
can be assigned to only even.

Nicolas, take note! :-) :-)

Thanks,
Angelo