linux-kernel - Re: [PATCH v5 09/22] dt-bindings: qcom-qce: Move 'clocks' to optional properties

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YZbobzlBNWIRNn/e@robh.at.kernel.org>
Date:   Thu, 18 Nov 2021 17:57:35 -0600
From:   Rob Herring <robh@...nel.org>
To:     Bhupesh Sharma <bhupesh.sharma@...aro.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-crypto@...r.kernel.org,
        bhupesh.linux@...il.com, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, agross@...nel.org,
        herbert@...dor.apana.org.au, davem@...emloft.net,
        stephan@...hold.net, Thara Gopinath <thara.gopinath@...aro.org>
Subject: Re: [PATCH v5 09/22] dt-bindings: qcom-qce: Move 'clocks' to
 optional properties

On Mon, Nov 15, 2021 at 11:04:31AM +0530, Bhupesh Sharma wrote:
> Hi Bjorn,
> 
> On Sun, 14 Nov 2021 at 01:32, Bjorn Andersson
> <bjorn.andersson@...aro.org> wrote:
> >
> > On Wed 10 Nov 04:59 CST 2021, Bhupesh Sharma wrote:
> >
> > > QCom QCE block on some SoCs like ipq6018 don't
> > > require clock as the required property, so the properties
> > > 'clocks' and 'clock-names' can be moved instead in the dt-bindings
> > > to the 'optional' properties section.
> > >
> > > Otherwise, running 'make dtbs_check' leads to the following
> > > errors:
> > >
> > > dma-controller@...4000: clock-names:0: 'bam_clk' was expected
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@...4000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@...4000: clock-names: ['iface_clk', 'bam_clk'] is too long
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@...4000: clocks: [[9, 138], [9, 137]] is too long
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > Cc: Thara Gopinath <thara.gopinath@...aro.org>
> > > Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
> > > Cc: Rob Herring <robh+dt@...nel.org>
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@...aro.org>
> > > ---
> > >  Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > index 30deaa0fa93d..f35bdb9ee7a8 100644
> > > --- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > +++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
> > > @@ -53,8 +53,6 @@ properties:
> > >  required:
> > >    - compatible
> > >    - reg
> > > -  - clocks
> > > -  - clock-names
> >
> > I would prefer that we make this conditional on the compatible. That
> > said, if this only applies to ipq6018 I think we should double check the
> > fact that there's no clock there...
> >
> > For the sake of making progress on the series, I think you should omit
> > this patch from the next version.
> 
> Without this patch, 'make dtbs_check' fails with the following error:
> dma-controller@...4000: clock-names:0: 'bam_clk' was expected
>         arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> 
> dma-controller@...4000: clock-names: Additional items are not allowed
> ('bam_clk' was unexpected)
>         arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml

Those errors do not correspond to the change here. Adding something to 
'required' would never solve any error (other than a driver requires a 
property to function).


> which I think is making Rob bot-check fail.

dtbs_check don't have to be fixed as the message says.

> So, I think instead of dropping the patch, let's try and understand
> from the 'ipq6018 qce' documentation if the clocks are really
> 'optional' there for the qce block (as clock properties are not
> mentioned in the dts from the very first upstream version). If not, we
> can try and fix the 'ipq6018 qce' dts node itself.
> 
> Regards,
> Bhupesh
>