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 <[email protected]>
---
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/[email protected]
v1: https://lore.kernel.org/all/[email protected]/
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
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 <[email protected]>
>
> ---
> 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/[email protected]
>
> v1: https://lore.kernel.org/all/[email protected]/
>
> 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
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 <[email protected]>
>
> ---
> 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/[email protected]
>
> v1: https://lore.kernel.org/all/[email protected]/
>
> 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
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 <[email protected]>
> >
> > ---
> > 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/[email protected]
> >
> > v1: https://lore.kernel.org/all/[email protected]/
> >
> > 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
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.
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 <[email protected]>
>>>
>>> ---
>>> 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/[email protected]
>>>
>>> v1: https://lore.kernel.org/all/[email protected]/
>>>
>>> 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