On Thu, 2022-05-05 at 12:25 -0400, Nícolas F. R. A. Prado wrote: > > > On Thu, May 05, 2022 at 10:52:45AM +0200, AngeloGioacchino Del Regno > wrote: > > 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@xxxxxxxxxxxxx> > > > > > --- > > > > > > > > > > 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. > > > > > Hi Jiaxin, > > thank you for the insights. > > > > > 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. > > So, the situation seems different at least on mt8192-asurada- > spherion. > Here, I2S8 is used for the headset microphone and I2S9 for the > headset audio. > Even for input and odd for output agree with Jiaxin's description. > However, the > input bus seems to be the main one, that is, disabling I2S8: > > amixer cset name='UL2_CH1 I2S8_CH1' 0 > amixer cset name='UL2_CH2 I2S8_CH2' 0 > > not only disables the microphone but also the audio on the headset. > If I add > > i2s9-share = "I2S8"; > > on the DT, then everything works, I can disable I2S8 without > impacting the > headset audio. So the pattern for the property on this platform is > the opposite > that Jiaxin mentioned. This tells me that we should keep the binding > more > generic (not assume where odds and evens go). I will still apply the > other > suggestions mentioned though. > > Thanks, > Nícolas > Hi Nícolas, >From software point, I2S8 and I2S9 belong to different hardware, so if you turn off I2S8 with CMD1, of course it will not affect I2S9. CMD1: amixer cset name='UL2_CH1 I2S8_CH1' 0 amixer cset name='UL2_CH2 I2S8_CH2' 0 Frome hardware point, I2S9 will use(share) I2S8's clock. If we don't want the user to perceive this, the driver need to help do something. So this property 'i2s9-share = "I2S8";' will be added to inform the driver. Best Regards, Jiaxin Yu > > Nicolas, take note! :-) :-) > > > > Thanks, > > Angelo > > > > To unsubscribe, send mail to > > kernel-unsubscribe@xxxxxxxxxxxxxxxxxxxxx.
