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

[Nicolas Frattaroli <frattaroli.nicolas@gmail.com>]

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