* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 17:44 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2021-10-15 17:44 UTC (permalink / raw)
To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Rob Herring,
Heiko Stuebner
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> The behaviour described in the bindings is exactly the opposite
> of what it should be, so this patch fixes the description and
> adjusts the if conditions for adding rockchip,cru to required.
>
> Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
> .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> index ce3e18b50230..93f5221e9381 100644
> --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> @@ -86,7 +86,7 @@ properties:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> The phandle of the cru.
> - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are specified.
> + Required if either trcm-sync-tx-only or trcm-sync-rx-only are specified.
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> @@ -147,8 +147,13 @@ required:
> allOf:
> - if:
> properties:
> - rockchip,trcm-sync-tx-only: false
> - rockchip,trcm-sync-rx-only: false
> + rockchip,trcm-sync-tx-only: true
> + then:
> + required:
> + - rockchip,cru
> + - if:
> + properties:
> + rockchip,trcm-sync-rx-only: true
FWIW, isn't this just a long-winded way to say
if:
anyOf:
- properties:
rockchip,trcm-sync-tx-only: true
- properties:
rockchip,trcm-sync-rx-only: true
which itself could still be more concisely
if:
not:
properties:
rockchip,trcm-sync-tx-only: false
rockchip,trcm-sync-rx-only: false
?
Robin.
> then:
> required:
> - rockchip,cru
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 17:44 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2021-10-15 17:44 UTC (permalink / raw)
To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Rob Herring,
Heiko Stuebner
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> The behaviour described in the bindings is exactly the opposite
> of what it should be, so this patch fixes the description and
> adjusts the if conditions for adding rockchip,cru to required.
>
> Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
> .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> index ce3e18b50230..93f5221e9381 100644
> --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> @@ -86,7 +86,7 @@ properties:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> The phandle of the cru.
> - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are specified.
> + Required if either trcm-sync-tx-only or trcm-sync-rx-only are specified.
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> @@ -147,8 +147,13 @@ required:
> allOf:
> - if:
> properties:
> - rockchip,trcm-sync-tx-only: false
> - rockchip,trcm-sync-rx-only: false
> + rockchip,trcm-sync-tx-only: true
> + then:
> + required:
> + - rockchip,cru
> + - if:
> + properties:
> + rockchip,trcm-sync-rx-only: true
FWIW, isn't this just a long-winded way to say
if:
anyOf:
- properties:
rockchip,trcm-sync-tx-only: true
- properties:
rockchip,trcm-sync-rx-only: true
which itself could still be more concisely
if:
not:
properties:
rockchip,trcm-sync-tx-only: false
rockchip,trcm-sync-rx-only: false
?
Robin.
> then:
> required:
> - rockchip,cru
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 17:44 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2021-10-15 17:44 UTC (permalink / raw)
To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Rob Herring,
Heiko Stuebner
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> The behaviour described in the bindings is exactly the opposite
> of what it should be, so this patch fixes the description and
> adjusts the if conditions for adding rockchip,cru to required.
>
> Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
> .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> index ce3e18b50230..93f5221e9381 100644
> --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> @@ -86,7 +86,7 @@ properties:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> The phandle of the cru.
> - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are specified.
> + Required if either trcm-sync-tx-only or trcm-sync-rx-only are specified.
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> @@ -147,8 +147,13 @@ required:
> allOf:
> - if:
> properties:
> - rockchip,trcm-sync-tx-only: false
> - rockchip,trcm-sync-rx-only: false
> + rockchip,trcm-sync-tx-only: true
> + then:
> + required:
> + - rockchip,cru
> + - if:
> + properties:
> + rockchip,trcm-sync-rx-only: true
FWIW, isn't this just a long-winded way to say
if:
anyOf:
- properties:
rockchip,trcm-sync-tx-only: true
- properties:
rockchip,trcm-sync-rx-only: true
which itself could still be more concisely
if:
not:
properties:
rockchip,trcm-sync-tx-only: false
rockchip,trcm-sync-rx-only: false
?
Robin.
> then:
> required:
> - rockchip,cru
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
2021-10-15 17:44 ` Robin Murphy
(?)
(?)
@ 2021-10-15 21:33 ` Nicolas Frattaroli
-1 siblings, 0 replies; 12+ messages in thread
From: Nicolas Frattaroli @ 2021-10-15 21:33 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Heiko Stuebner,
Robin Murphy
Cc: Rob Herring, alsa-devel, devicetree, linux-kernel, linux-rockchip,
linux-arm-kernel
On Freitag, 15. Oktober 2021 19:44:56 CEST Robin Murphy wrote:
> On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> > The behaviour described in the bindings is exactly the opposite
> > of what it should be, so this patch fixes the description and
> > adjusts the if conditions for adding rockchip,cru to required.
> >
> > Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > ---
> >
> > .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml index
> > ce3e18b50230..93f5221e9381 100644
> > --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> >
> > @@ -86,7 +86,7 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > description:
> > The phandle of the cru.
> >
> > - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are
> > specified. + Required if either trcm-sync-tx-only or
> > trcm-sync-rx-only are specified.>
> > rockchip,grf:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > @@ -147,8 +147,13 @@ required:
> > allOf:
> > - if:
> > properties:
> > - rockchip,trcm-sync-tx-only: false
> > - rockchip,trcm-sync-rx-only: false
> > + rockchip,trcm-sync-tx-only: true
> > + then:
> > + required:
> > + - rockchip,cru
> > + - if:
> > + properties:
> > + rockchip,trcm-sync-rx-only: true
>
> FWIW, isn't this just a long-winded way to say
>
> if:
> anyOf:
> - properties:
> rockchip,trcm-sync-tx-only: true
> - properties:
> rockchip,trcm-sync-rx-only: true
>
> which itself could still be more concisely
>
> if:
> not:
> properties:
> rockchip,trcm-sync-tx-only: false
> rockchip,trcm-sync-rx-only: false
>
> ?
Hi Robin,
you are absolutely correct. I'm not very familiar with dtschema yet,
so I mostly just did what I know would work, as I didn't know about
"not" or "anyOf" in this context.
However I don't think this patch is going to get a V2 because I may
have found a way to easily get rid of the CRU property altogether,
making this patch sort of obsolete in the very near future.
Thank you for your review nonetheless, I learned some more dtschema
today thanks to it. :)
>
> Robin.
>
> > then:
> > required:
> > - rockchip,cru
Regards,
Nicolas Frattaroli
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 21:33 ` Nicolas Frattaroli
0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Frattaroli @ 2021-10-15 21:33 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Heiko Stuebner,
Robin Murphy
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On Freitag, 15. Oktober 2021 19:44:56 CEST Robin Murphy wrote:
> On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> > The behaviour described in the bindings is exactly the opposite
> > of what it should be, so this patch fixes the description and
> > adjusts the if conditions for adding rockchip,cru to required.
> >
> > Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > ---
> >
> > .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml index
> > ce3e18b50230..93f5221e9381 100644
> > --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> >
> > @@ -86,7 +86,7 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > description:
> > The phandle of the cru.
> >
> > - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are
> > specified. + Required if either trcm-sync-tx-only or
> > trcm-sync-rx-only are specified.>
> > rockchip,grf:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > @@ -147,8 +147,13 @@ required:
> > allOf:
> > - if:
> > properties:
> > - rockchip,trcm-sync-tx-only: false
> > - rockchip,trcm-sync-rx-only: false
> > + rockchip,trcm-sync-tx-only: true
> > + then:
> > + required:
> > + - rockchip,cru
> > + - if:
> > + properties:
> > + rockchip,trcm-sync-rx-only: true
>
> FWIW, isn't this just a long-winded way to say
>
> if:
> anyOf:
> - properties:
> rockchip,trcm-sync-tx-only: true
> - properties:
> rockchip,trcm-sync-rx-only: true
>
> which itself could still be more concisely
>
> if:
> not:
> properties:
> rockchip,trcm-sync-tx-only: false
> rockchip,trcm-sync-rx-only: false
>
> ?
Hi Robin,
you are absolutely correct. I'm not very familiar with dtschema yet,
so I mostly just did what I know would work, as I didn't know about
"not" or "anyOf" in this context.
However I don't think this patch is going to get a V2 because I may
have found a way to easily get rid of the CRU property altogether,
making this patch sort of obsolete in the very near future.
Thank you for your review nonetheless, I learned some more dtschema
today thanks to it. :)
>
> Robin.
>
> > then:
> > required:
> > - rockchip,cru
Regards,
Nicolas Frattaroli
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 21:33 ` Nicolas Frattaroli
0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Frattaroli @ 2021-10-15 21:33 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Heiko Stuebner,
Robin Murphy
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On Freitag, 15. Oktober 2021 19:44:56 CEST Robin Murphy wrote:
> On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> > The behaviour described in the bindings is exactly the opposite
> > of what it should be, so this patch fixes the description and
> > adjusts the if conditions for adding rockchip,cru to required.
> >
> > Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > ---
> >
> > .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml index
> > ce3e18b50230..93f5221e9381 100644
> > --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> >
> > @@ -86,7 +86,7 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > description:
> > The phandle of the cru.
> >
> > - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are
> > specified. + Required if either trcm-sync-tx-only or
> > trcm-sync-rx-only are specified.>
> > rockchip,grf:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > @@ -147,8 +147,13 @@ required:
> > allOf:
> > - if:
> > properties:
> > - rockchip,trcm-sync-tx-only: false
> > - rockchip,trcm-sync-rx-only: false
> > + rockchip,trcm-sync-tx-only: true
> > + then:
> > + required:
> > + - rockchip,cru
> > + - if:
> > + properties:
> > + rockchip,trcm-sync-rx-only: true
>
> FWIW, isn't this just a long-winded way to say
>
> if:
> anyOf:
> - properties:
> rockchip,trcm-sync-tx-only: true
> - properties:
> rockchip,trcm-sync-rx-only: true
>
> which itself could still be more concisely
>
> if:
> not:
> properties:
> rockchip,trcm-sync-tx-only: false
> rockchip,trcm-sync-rx-only: false
>
> ?
Hi Robin,
you are absolutely correct. I'm not very familiar with dtschema yet,
so I mostly just did what I know would work, as I didn't know about
"not" or "anyOf" in this context.
However I don't think this patch is going to get a V2 because I may
have found a way to easily get rid of the CRU property altogether,
making this patch sort of obsolete in the very near future.
Thank you for your review nonetheless, I learned some more dtschema
today thanks to it. :)
>
> Robin.
>
> > then:
> > required:
> > - rockchip,cru
Regards,
Nicolas Frattaroli
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement
@ 2021-10-15 21:33 ` Nicolas Frattaroli
0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Frattaroli @ 2021-10-15 21:33 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Heiko Stuebner,
Robin Murphy
Cc: Rob Herring, linux-rockchip, alsa-devel, devicetree,
linux-arm-kernel, linux-kernel
On Freitag, 15. Oktober 2021 19:44:56 CEST Robin Murphy wrote:
> On 2021-10-15 15:31, Nicolas Frattaroli wrote:
> > The behaviour described in the bindings is exactly the opposite
> > of what it should be, so this patch fixes the description and
> > adjusts the if conditions for adding rockchip,cru to required.
> >
> > Fixes: 510f1c133aed ("ASoC: dt-bindings: rockchip: add i2s-tdm bindings")
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > ---
> >
> > .../devicetree/bindings/sound/rockchip,i2s-tdm.yaml | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml index
> > ce3e18b50230..93f5221e9381 100644
> > --- a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> > +++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> >
> > @@ -86,7 +86,7 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > description:
> > The phandle of the cru.
> >
> > - Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are
> > specified. + Required if either trcm-sync-tx-only or
> > trcm-sync-rx-only are specified.>
> > rockchip,grf:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > @@ -147,8 +147,13 @@ required:
> > allOf:
> > - if:
> > properties:
> > - rockchip,trcm-sync-tx-only: false
> > - rockchip,trcm-sync-rx-only: false
> > + rockchip,trcm-sync-tx-only: true
> > + then:
> > + required:
> > + - rockchip,cru
> > + - if:
> > + properties:
> > + rockchip,trcm-sync-rx-only: true
>
> FWIW, isn't this just a long-winded way to say
>
> if:
> anyOf:
> - properties:
> rockchip,trcm-sync-tx-only: true
> - properties:
> rockchip,trcm-sync-rx-only: true
>
> which itself could still be more concisely
>
> if:
> not:
> properties:
> rockchip,trcm-sync-tx-only: false
> rockchip,trcm-sync-rx-only: false
>
> ?
Hi Robin,
you are absolutely correct. I'm not very familiar with dtschema yet,
so I mostly just did what I know would work, as I didn't know about
"not" or "anyOf" in this context.
However I don't think this patch is going to get a V2 because I may
have found a way to easily get rid of the CRU property altogether,
making this patch sort of obsolete in the very near future.
Thank you for your review nonetheless, I learned some more dtschema
today thanks to it. :)
>
> Robin.
>
> > then:
> > required:
> > - rockchip,cru
Regards,
Nicolas Frattaroli
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 12+ messages in thread