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

[Rob Herring]

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@xxxxxxxxxx> 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@7984000: clock-names:0: 'bam_clk' was expected
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clock-names: Additional items are not allowed ('bam_clk' was unexpected)
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clock-names: ['iface_clk', 'bam_clk'] is too long
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > dma-controller@7984000: clocks: [[9, 138], [9, 137]] is too long
> > >       arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> > >
> > > Cc: Thara Gopinath <thara.gopinath@xxxxxxxxxx>
> > > Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>
> > > ---
> > >  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@7984000: clock-names:0: 'bam_clk' was expected
>         arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dt.yaml
> 
> dma-controller@7984000: 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
>