Messages in this thread | | | Date | Thu, 5 May 2022 10:52:45 +0200 | Subject | Re: [PATCH 1/3] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties | From | AngeloGioacchino Del Regno <> |
| |
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@collabora.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
| |