Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement

Nicolas Frattaroli
2021-10-15T21:33:40Z

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