| Message ID | 20220509205847.607076-1-nfraprado@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ASoC: dt-bindings: mediatek: mt8192: Add i2s-share properties | expand |
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
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
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 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
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
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
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(+)