[PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Nícolas F. R. A. Prado]

The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
share the same clock and act as a single interface with both input and
output. Add patterns for these properties in the dt-binding. The
property is split into two patterns in order to allow all valid
interface pairings.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---
The series from v1 of this patch was merged although some changes were
still needed in this patch, so the v1 of this patch was reverted [1] and
this standalone commit addresses the feedback from v1 and readds the
property.

[1] https://lore.kernel.org/all/20220509185625.580811-1-nfraprado@collabora.com

v1: https://lore.kernel.org/all/20220429203039.2207848-2-nfraprado@collabora.com/

Changes in v2:
- Added "mediatek," prefix to property
- Rewrote and added more information to property description
- Split into two patterns to validate that output-input pairings are
  done

 .../bindings/sound/mt8192-afe-pcm.yaml           | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
index 7a25bc9b8060..2abf43c6c2c3 100644
--- a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
+++ b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
@@ -54,6 +54,22 @@ properties:
       - const: aud_infra_clk
       - const: aud_infra_26m_clk
 
+patternProperties:
+  "^mediatek,i2s[13579]-share$":
+    description:
+      Each I2S interface has a single data line, input if its index is even or
+      output if the index is odd. An input and an output I2S interface can be
+      used together as if they were a single I2S interface with both input and
+      output data lines by sharing the same clock. This property represents this
+      pairing. The value should be the name of the interface whose clock is
+      used, and the property name the other interface that depends on this
+      clock.
+    pattern: "^I2S[0268]$"
+
+  "^mediatek,i2s[0268]-share$":
+    description: Same as above.
+    pattern: "^I2S[13579]$"
+
 required:
   - compatible
   - interrupts
-- 
2.36.0


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

Re: [PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Nícolas F. R. A. Prado]

On Mon, May 09, 2022 at 04:58:47PM -0400, Nícolas F. R. A. Prado wrote:
> The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
> share the same clock and act as a single interface with both input and
> output. Add patterns for these properties in the dt-binding. The
> property is split into two patterns in order to allow all valid
> interface pairings.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> ---
> The series from v1 of this patch was merged although some changes were
> still needed in this patch, so the v1 of this patch was reverted [1] and
> this standalone commit addresses the feedback from v1 and readds the
> property.
> 
> [1] https://lore.kernel.org/all/20220509185625.580811-1-nfraprado@collabora.com
> 
> v1: https://lore.kernel.org/all/20220429203039.2207848-2-nfraprado@collabora.com/
> 
> Changes in v2:
> - Added "mediatek," prefix to property
> - Rewrote and added more information to property description
> - Split into two patterns to validate that output-input pairings are
>   done
> 
>  .../bindings/sound/mt8192-afe-pcm.yaml           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Gentle ping on this one. Any feedback for v2?

Thanks,
Nícolas

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

Re: [PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1187 bytes --]

On Wed, May 25, 2022 at 04:53:46PM -0400, Nícolas F. R. A. Prado wrote:
> On Mon, May 09, 2022 at 04:58:47PM -0400, Nícolas F. R. A. Prado wrote:
> > The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
> > share the same clock and act as a single interface with both input and
> > output. Add patterns for these properties in the dt-binding. The

> Gentle ping on this one. Any feedback for v2?

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Krzysztof Kozlowski]

On 09/05/2022 22:58, Nícolas F. R. A. Prado wrote:
> The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
> share the same clock and act as a single interface with both input and
> output. Add patterns for these properties in the dt-binding. The
> property is split into two patterns in order to allow all valid
> interface pairings.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> ---
> The series from v1 of this patch was merged although some changes were
> still needed in this patch, so the v1 of this patch was reverted [1] and
> this standalone commit addresses the feedback from v1 and readds the
> property.
> 
> [1] https://lore.kernel.org/all/20220509185625.580811-1-nfraprado@collabora.com
> 
> v1: https://lore.kernel.org/all/20220429203039.2207848-2-nfraprado@collabora.com/
> 
> Changes in v2:
> - Added "mediatek," prefix to property
> - Rewrote and added more information to property description
> - Split into two patterns to validate that output-input pairings are
>   done
> 
>  .../bindings/sound/mt8192-afe-pcm.yaml           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> index 7a25bc9b8060..2abf43c6c2c3 100644
> --- a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> +++ b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> @@ -54,6 +54,22 @@ properties:
>        - const: aud_infra_clk
>        - const: aud_infra_26m_clk
>  
> +patternProperties:
> +  "^mediatek,i2s[13579]-share$":
> +    description:
> +      Each I2S interface has a single data line, input if its index is even or
> +      output if the index is odd. An input and an output I2S interface can be
> +      used together as if they were a single I2S interface with both input and
> +      output data lines by sharing the same clock. This property represents this
> +      pairing. The value should be the name of the interface whose clock is
> +      used, and the property name the other interface that depends on this
> +      clock.
> +    pattern: "^I2S[0268]$"
> +
> +  "^mediatek,i2s[0268]-share$":
> +    description: Same as above.
> +    pattern: "^I2S[13579]$"

Rob's question is still valid - why these are not phandles?

In any case you miss $ref.


Best regards,
Krzysztof

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

Re: [PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Nícolas F. R. A. Prado]

On Thu, May 26, 2022 at 08:49:39AM +0200, Krzysztof Kozlowski wrote:
> On 09/05/2022 22:58, Nícolas F. R. A. Prado wrote:
> > The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
> > share the same clock and act as a single interface with both input and
> > output. Add patterns for these properties in the dt-binding. The
> > property is split into two patterns in order to allow all valid
> > interface pairings.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > 
> > ---
> > The series from v1 of this patch was merged although some changes were
> > still needed in this patch, so the v1 of this patch was reverted [1] and
> > this standalone commit addresses the feedback from v1 and readds the
> > property.
> > 
> > [1] https://lore.kernel.org/all/20220509185625.580811-1-nfraprado@collabora.com
> > 
> > v1: https://lore.kernel.org/all/20220429203039.2207848-2-nfraprado@collabora.com/
> > 
> > Changes in v2:
> > - Added "mediatek," prefix to property
> > - Rewrote and added more information to property description
> > - Split into two patterns to validate that output-input pairings are
> >   done
> > 
> >  .../bindings/sound/mt8192-afe-pcm.yaml           | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> > index 7a25bc9b8060..2abf43c6c2c3 100644
> > --- a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> > +++ b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
> > @@ -54,6 +54,22 @@ properties:
> >        - const: aud_infra_clk
> >        - const: aud_infra_26m_clk
> >  
> > +patternProperties:
> > +  "^mediatek,i2s[13579]-share$":
> > +    description:
> > +      Each I2S interface has a single data line, input if its index is even or
> > +      output if the index is odd. An input and an output I2S interface can be
> > +      used together as if they were a single I2S interface with both input and
> > +      output data lines by sharing the same clock. This property represents this
> > +      pairing. The value should be the name of the interface whose clock is
> > +      used, and the property name the other interface that depends on this
> > +      clock.
> > +    pattern: "^I2S[0268]$"
> > +
> > +  "^mediatek,i2s[0268]-share$":
> > +    description: Same as above.
> > +    pattern: "^I2S[13579]$"
> 
> Rob's question is still valid - why these are not phandles?

So, instead of having

	i2s9-share = "I2S8";

on the DT, you want us to have something like this:

        afe_i2s8: mediatek,i2s8 { };

        mediatek,i2s9 {
          mediatek,share-clock = <&afe_i2s8>;
        };

Or do you mean something else?

It seems like a lot more syntax to express the same thing (and the empty node
seems awkward), but if that's the DT way, I can change it no problem.

> 
> In any case you miss $ref.

Indeed, sorry, I'll add it in next version.

Thanks,
Nícolas

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

Re: [PATCH v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties

[Krzysztof Kozlowski]

On 27/05/2022 21:05, Nícolas F. R. A. Prado wrote:
> On Thu, May 26, 2022 at 08:49:39AM +0200, Krzysztof Kozlowski wrote:
>> On 09/05/2022 22:58, Nícolas F. R. A. Prado wrote:
>>> The Mediatek AFE PCM controller for MT8192 allows two I2S interfaces to
>>> share the same clock and act as a single interface with both input and
>>> output. Add patterns for these properties in the dt-binding. The
>>> property is split into two patterns in order to allow all valid
>>> interface pairings.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>>
>>> ---
>>> The series from v1 of this patch was merged although some changes were
>>> still needed in this patch, so the v1 of this patch was reverted [1] and
>>> this standalone commit addresses the feedback from v1 and readds the
>>> property.
>>>
>>> [1] https://lore.kernel.org/all/20220509185625.580811-1-nfraprado@collabora.com
>>>
>>> v1: https://lore.kernel.org/all/20220429203039.2207848-2-nfraprado@collabora.com/
>>>
>>> Changes in v2:
>>> - Added "mediatek," prefix to property
>>> - Rewrote and added more information to property description
>>> - Split into two patterns to validate that output-input pairings are
>>>   done
>>>
>>>  .../bindings/sound/mt8192-afe-pcm.yaml           | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
>>> index 7a25bc9b8060..2abf43c6c2c3 100644
>>> --- a/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/mt8192-afe-pcm.yaml
>>> @@ -54,6 +54,22 @@ properties:
>>>        - const: aud_infra_clk
>>>        - const: aud_infra_26m_clk
>>>  
>>> +patternProperties:
>>> +  "^mediatek,i2s[13579]-share$":
>>> +    description:
>>> +      Each I2S interface has a single data line, input if its index is even or
>>> +      output if the index is odd. An input and an output I2S interface can be
>>> +      used together as if they were a single I2S interface with both input and
>>> +      output data lines by sharing the same clock. This property represents this
>>> +      pairing. The value should be the name of the interface whose clock is
>>> +      used, and the property name the other interface that depends on this
>>> +      clock.
>>> +    pattern: "^I2S[0268]$"
>>> +
>>> +  "^mediatek,i2s[0268]-share$":
>>> +    description: Same as above.
>>> +    pattern: "^I2S[13579]$"
>>
>> Rob's question is still valid - why these are not phandles?
> 
> So, instead of having
> 
> 	i2s9-share = "I2S8";
> 
> on the DT, you want us to have something like this:
> 
>         afe_i2s8: mediatek,i2s8 { };
> 
>         mediatek,i2s9 {
>           mediatek,share-clock = <&afe_i2s8>;
>         };
> 
> Or do you mean something else?

Not entirely. This is a binding for mediatek,mt8192-audio, not for I2S9,
so the property should be in this binding referencing your existing I2S
devices.

> 
> It seems like a lot more syntax to express the same thing (and the empty node
> seems awkward), but if that's the DT way, I can change it no problem.

Hm, then the follow up question: why you do not have i2s8 defined in DTS?


Best regards,
Krzysztof

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