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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2236749.301RoY5kYM@archbook>
Date:   Fri, 15 Oct 2021 23:33:34 +0200
From:   Nicolas Frattaroli <frattaroli.nicolas@...il.com>
To:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiko Stuebner <heiko@...ech.de>,
        Robin Murphy <robin.murphy@....com>
Cc:     Rob Herring <robh@...nel.org>, linux-rockchip@...ts.infradead.org,
        alsa-devel@...a-project.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: dt-bindings: rockchip: i2s-tdm: fix rockchip, cru requirement

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@...il.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