linux-kernel - Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i

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]

Date:   Tue, 9 Feb 2021 19:48:43 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Wilken Gottwalt <wilken.gottwalt@...teo.net>
Cc:     Rob Herring <robh+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Baolin Wang <baolin.wang7@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>
Subject: Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

On Sun, Feb 07, 2021 at 10:07:31AM +0100, Wilken Gottwalt wrote:
> On Wed, 23 Dec 2020 15:49:19 -0700
> Rob Herring <robh+dt@...nel.org> wrote:
> 
> > On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
> > <wilken.gottwalt@...teo.net> wrote:
> > >
> > > Adds documentation on how to use the sun6i_hwspinlock driver for sun6i
> > > compatible SoCs.
> > 
> > Please resend to DT list so that automated checks run and it's in my
> > queue (PW). You need to run 'make dt_binding_check' as there are
> > several issues.
> 
> Mentioning somewhere, that yamllint is required would have helped here a lot.
> Without it I always ended up with that, what was quite misleading:
> 
> ERROR: dtschema minimum version is v2020.8.1
> make[1]: *** [Documentation/devicetree/bindings/Makefile:12: check_dtschema_version] Error 1
> make: *** [Makefile:1370: dt_binding_check] Error 2
> 
> > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@...teo.net>
> > > ---
> > > Changes in v5:
> > >   - changed binding to earliest known supported SoC sun6i-a31
> > >   - dropped unnecessary entries
> > >
> > > Changes in v4:
> > >   - changed binding to sun8i-a33-hwpinlock
> > >   - added changes suggested by Maxime Ripard
> > >
> > > Changes in v3:
> > >   - changed symbols from sunxi to sun8i
> > >
> > > Changes in v2:
> > >   - fixed memory ranges
> > > ---
> > >  .../bindings/hwlock/sun6i-a31-hwspinlock.yaml | 44 +++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/hwlock/sun6i-a31-hwspinlock.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwlock/sun6i-a31-hwspinlock.yaml
> > > b/Documentation/devicetree/bindings/hwlock/sun6i-a31-hwspinlock.yaml new file mode 100644
> > > index 000000000000..481c5c995ad7
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwlock/sun6i-a31-hwspinlock.yaml
> > > @@ -0,0 +1,44 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwlock/sun6i-hwspinlock.yaml#
> > 
> > This will fail checks. Wrong filename.
> > 
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: SUN6I hardware spinlock driver for Allwinner sun6i compatible SoCs
> > > +
> > > +maintainers:
> > > +  - Wilken Gottwalt <wilken.gottwalt@...teo.net>
> > > +
> > > +description:
> > > +  The hardware unit provides semaphores between the ARM cores and the embedded
> > > +  companion core on the SoC.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: allwinner,sun6i-a31-hwspinlock
> > > +
> > > +    reg:
> > > +      maxItems: 1
> > > +
> > > +    clocks:
> > > +      maxItems: 1
> > > +
> > > +    resets:
> > > +      maxItems: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - resets
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    hwspinlock@...8000 {
> > 
> > hwlock@...
> 
> sprd, stm32 and omap using hwspinlock. Why is it okay there and not okay in
> my version?

The spec mandates hwlock:

https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst#generic-names-recommendation

They probably introduced their nodes before it was standardized (or
didn't care).

> > > +      compatible = "allwinner,sun6i-a31-hwspinlock";
> > > +      reg = <0x01c18000 0x1000>;
> > > +      clocks = <&ccu CLK_BUS_SPINLOCK>;
> > > +      resets = <&ccu RST_BUS_SPINLOCK>;
> > 
> > You need an include for these defines.
> 
> So I guess it is needed because I the clocks/resets are used, right? But why
> is it not the case for the sprd example, which also uses clocks?

If you're talking about sprd,hwspinlock-r3p0, it doesn't use any define
(and thus doesn't need any include), and it's a binding in a text format
that wouldn't compile the example (this is only done for yaml bindings).
Even if it was wrong for them to do so, they wouldn't notice.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)