2020-12-23 11:37:59

by Wilken Gottwalt

[permalink] [raw]
Subject: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

Adds documentation on how to use the sun6i_hwspinlock driver for sun6i
compatible SoCs.

Signed-off-by: Wilken Gottwalt <[email protected]>
---
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#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SUN6I hardware spinlock driver for Allwinner sun6i compatible SoCs
+
+maintainers:
+ - Wilken Gottwalt <[email protected]>
+
+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@1c18000 {
+ compatible = "allwinner,sun6i-a31-hwspinlock";
+ reg = <0x01c18000 0x1000>;
+ clocks = <&ccu CLK_BUS_SPINLOCK>;
+ resets = <&ccu RST_BUS_SPINLOCK>;
+ };
--
2.29.2


2020-12-23 22:51:24

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
<[email protected]> 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.

> Signed-off-by: Wilken Gottwalt <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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@1c18000 {

hwlock@...

> + 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.

> + };
> --
> 2.29.2
>

2021-01-06 10:16:55

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

Hi,

On Wed, Dec 23, 2020 at 03:49:19PM -0700, Rob Herring wrote:
> On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
> <[email protected]> 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.
>
> > Signed-off-by: Wilken Gottwalt <[email protected]>
> > ---
> > 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.

Also, the name should be the whole compatible, so
"allwinner,sun6i-a31-hwspinlock.yaml".

> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SUN6I hardware spinlock driver for Allwinner sun6i compatible SoCs
> > +
> > +maintainers:
> > + - Wilken Gottwalt <[email protected]>
> > +
> > +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

And this is not indented properly, it should be at the same level than
compatible, not under it. Doesn't the meta-schema catch this?

Maxime


Attachments:
(No filename) (2.44 kB)
signature.asc (235.00 B)
Download all attachments

2021-01-07 08:38:26

by Wilken Gottwalt

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

On Wed, 6 Jan 2021 11:14:36 +0100
Maxime Ripard <[email protected]> wrote:

> Hi,
>
> On Wed, Dec 23, 2020 at 03:49:19PM -0700, Rob Herring wrote:
> > On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
> > <[email protected]> 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.
> >
> > > Signed-off-by: Wilken Gottwalt <[email protected]>
> > > ---
> > > 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.
>
> Also, the name should be the whole compatible, so
> "allwinner,sun6i-a31-hwspinlock.yaml".

Yes, I mixed it up a bit. I was a bit to tired this day. And changing the
names/symbols in the last minute didn't go well either.

> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: SUN6I hardware spinlock driver for Allwinner sun6i compatible SoCs
> > > +
> > > +maintainers:
> > > + - Wilken Gottwalt <[email protected]>
> > > +
> > > +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
>
> And this is not indented properly, it should be at the same level than
> compatible, not under it. Doesn't the meta-schema catch this?

Hmm, yeah, this is odd. I need to check my scripts. The bindings stuff is
clearly not checked.

> Maxime

2021-02-07 09:09:24

by Wilken Gottwalt

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: hwlock: add sun6i_hwspinlock

On Wed, 23 Dec 2020 15:49:19 -0700
Rob Herring <[email protected]> wrote:

> On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
> <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > +
> > +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@1c18000 {
>
> hwlock@...

sprd, stm32 and omap using hwspinlock. Why is it okay there and not okay in
my version?

> > + 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?

> > + };
> > --
> > 2.29.2
> >

2021-02-09 20:58:22

by Maxime Ripard

[permalink] [raw]
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 <[email protected]> wrote:
>
> > On Wed, Dec 23, 2020 at 4:34 AM Wilken Gottwalt
> > <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
> > > +
> > > +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@1c18000 {
> >
> > 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


Attachments:
(No filename) (4.03 kB)
signature.asc (235.00 B)
Download all attachments