[PATCH] dt-bindings: hwlock: Convert to dtschema

[PATCH] dt-bindings: hwlock: Convert to dtschema

[Siddharth Menon]

From: BiscuitBobby <simeddon@gmail.com>

Convert the generic hwspinlock bindings to DT schema.
---
 This is my first time converting bindings to dt schema, please let me
 know if I have overlooked anything.
 .../devicetree/bindings/hwlock/hwlock.txt     | 59 -----------------
 .../devicetree/bindings/hwlock/hwlock.yaml    | 65 +++++++++++++++++++
 2 files changed, 65 insertions(+), 59 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
 create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.yaml

diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt
deleted file mode 100644
index 085d1f5c916a..000000000000
--- a/Documentation/devicetree/bindings/hwlock/hwlock.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-Generic hwlock bindings
-=======================
-
-Generic bindings that are common to all the hwlock platform specific driver
-implementations.
-
-Please also look through the individual platform specific hwlock binding
-documentations for identifying any additional properties specific to that
-platform.
-
-hwlock providers:
-=================
-
-Required properties:
-- #hwlock-cells:        Specifies the number of cells needed to represent a
-                        specific lock.
-
-hwlock users:
-=============
-
-Consumers that require specific hwlock(s) should specify them using the
-property "hwlocks", and an optional "hwlock-names" property.
-
-Required properties:
-- hwlocks:              List of phandle to a hwlock provider node and an
-                        associated hwlock args specifier as indicated by
-                        #hwlock-cells. The list can have just a single hwlock
-                        or multiple hwlocks, with each hwlock represented by
-                        a phandle and a corresponding args specifier.
-
-Optional properties:
-- hwlock-names:         List of hwlock name strings defined in the same order
-                        as the hwlocks, with one name per hwlock. Consumers can
-                        use the hwlock-names to match and get a specific hwlock.
-
-
-1. Example of a node using a single specific hwlock:
-
-The following example has a node requesting a hwlock in the bank defined by
-the node hwlock1. hwlock1 is a hwlock provider with an argument specifier
-of length 1.
-
-	node {
-		...
-		hwlocks = <&hwlock1 2>;
-		...
-	};
-
-2. Example of a node using multiple specific hwlocks:
-
-The following example has a node requesting two hwlocks, a hwlock within
-the hwlock device node 'hwlock1' with #hwlock-cells value of 1, and another
-hwlock within the hwlock device node 'hwlock2' with #hwlock-cells value of 2.
-
-	node {
-		...
-		hwlocks = <&hwlock1 2>, <&hwlock2 0 3>;
-		...
-	};
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.yaml b/Documentation/devicetree/bindings/hwlock/hwlock.yaml
new file mode 100644
index 000000000000..2492fdad3c6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwlock/hwlock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic Hardware Lock (hwlock)
+
+description: |
+  Generic bindings that are common to all the hwlock platform specific driver
+  implementations.
+  Please also look through the individual platform specific hwlock binding
+  documentations for identifying any additional properties specific to that
+  platform.
+
+maintainers:
+  - Bjorn Andersson <andersson@kernel.org>
+  - Rob Herring <robh@kernel.org>
+  - Krzysztof Kozlowski <krzk+dt@kernel.org>
+  - Conor Dooley <conor+dt@kernel.org>
+
+properties:
+  $nodename:
+    pattern: "^hwlock(@.*)?"
+
+  "#hwlock-cells":
+    description: |
+      Specifies the number of cells needed to represent a specific lock.
+    minimum: 1
+
+  hwlocks:
+    description: |
+      List of phandle to a hwlock provider node and an associated hwlock args
+      specifier as indicated by #hwlock-cells. The list can have just a single
+      hwlock or multiple hwlocks, with each hwlock represented by a phandle and
+      a corresponding args specifier.
+
+  hwlock-names:
+    description: |
+      List of hwlock name strings defined in the same order as the hwlocks,
+      with one name per hwlock. Consumers can use the hwlock-names to match
+      and get a specific hwlock.
+
+patternProperties:
+  "^hwlock@[0-9a-f]+$":
+    type: object
+    description: Hardware lock provider node
+
+required:
+  - "#hwlock-cells"
+
+additionalProperties: true
+
+examples:
+  # Example 1: A node using a single specific hwlock
+  - |
+    node {
+      hwlocks = <&hwlock1 2>;
+    };
+
+  # Example 2: A node using multiple specific hwlocks
+  - |
+    node {
+      hwlocks = <&hwlock1 2>, <&hwlock2 0 3>;
+    };
-- 
2.48.1

Re: [PATCH] dt-bindings: hwlock: Convert to dtschema

[Krzysztof Kozlowski]

On 18/02/2025 17:09, Siddharth Menon wrote:
> From: BiscuitBobby <simeddon@gmail.com>
> 
> Convert the generic hwspinlock bindings to DT schema.

Thank you for your patch. There is something to discuss/improve.

Please run scripts/checkpatch.pl and fix reported warnings. After that,
run also `scripts/checkpatch.pl --strict` and (probably) fix more
warnings. Some warnings can be ignored, especially from --strict run,
but the code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

Also you must use your full name, not nicknames.


> ---
>  This is my first time converting bindings to dt schema, please let me
>  know if I have overlooked anything.
>  .../devicetree/bindings/hwlock/hwlock.txt     | 59 -----------------
>  .../devicetree/bindings/hwlock/hwlock.yaml    | 65 +++++++++++++++++++
>  2 files changed, 65 insertions(+), 59 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
>  create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.yaml
> 

You leave now incorrect paths in the kernel.

If you decide to convert the generic subsystem binding, you must take
extra care and change/fix/update/improve all bindings using it.


> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.yaml b/Documentation/devicetree/bindings/hwlock/hwlock.yaml
> new file mode 100644
> index 000000000000..2492fdad3c6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwlock/hwlock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic Hardware Lock (hwlock)
> +
> +description: |
> +  Generic bindings that are common to all the hwlock platform specific driver
> +  implementations.
> +  Please also look through the individual platform specific hwlock binding
> +  documentations for identifying any additional properties specific to that
> +  platform.
> +
> +maintainers:
> +  - Bjorn Andersson <andersson@kernel.org>
> +  - Rob Herring <robh@kernel.org>
> +  - Krzysztof Kozlowski <krzk+dt@kernel.org>
> +  - Conor Dooley <conor+dt@kernel.org>

Subsystem maintainer only.

> +
> +properties:
> +  $nodename:
> +    pattern: "^hwlock(@.*)?"

Why .* in the pattern if you do not anchor it with $?

> +
> +  "#hwlock-cells":
> +    description: |
> +      Specifies the number of cells needed to represent a specific lock.
> +    minimum: 1
> +
> +  hwlocks:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      List of phandle to a hwlock provider node and an associated hwlock args
> +      specifier as indicated by #hwlock-cells. The list can have just a single
> +      hwlock or multiple hwlocks, with each hwlock represented by a phandle and
> +      a corresponding args specifier.

Missing type, here and  other places, unless this is already covered by
dtschema? But then why this binding is needed?

> +
> +  hwlock-names:
> +    description: |
> +      List of hwlock name strings defined in the same order as the hwlocks,
> +      with one name per hwlock. Consumers can use the hwlock-names to match
> +      and get a specific hwlock.

Hm? I don't think you understood the binding. The provider does not have
consumer properties. Read again original binding.

> +
> +patternProperties:
> +  "^hwlock@[0-9a-f]+$":
> +    type: object
> +    description: Hardware lock provider node

This makes no sense. hwlock within hwlock?

> +
> +required:
> +  - "#hwlock-cells"
> +
> +additionalProperties: true
> +
> +examples:
> +  # Example 1: A node using a single specific hwlock


Drop all examples, not really useful.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: hwlock: Convert to dtschema

[Rob Herring (Arm)]

On Tue, 18 Feb 2025 21:39:35 +0530, Siddharth Menon wrote:
> From: BiscuitBobby <simeddon@gmail.com>
> 
> Convert the generic hwspinlock bindings to DT schema.
> ---
>  This is my first time converting bindings to dt schema, please let me
>  know if I have overlooked anything.
>  .../devicetree/bindings/hwlock/hwlock.txt     | 59 -----------------
>  .../devicetree/bindings/hwlock/hwlock.yaml    | 65 +++++++++++++++++++
>  2 files changed, 65 insertions(+), 59 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
>  create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:


doc reference errors (make refcheckdocs):
Warning: Documentation/devicetree/bindings/hwlock/ti,omap-hwspinlock.yaml references a file that doesn't exist: Documentation/devicetree/bindings/hwlock/hwlock.txt
Documentation/devicetree/bindings/hwlock/ti,omap-hwspinlock.yaml: Documentation/devicetree/bindings/hwlock/hwlock.txt

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250218161352.269237-1-simeddon@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH] dt-bindings: hwlock: Convert to dtschema

[Rob Herring]

On Tue, Feb 18, 2025 at 09:39:35PM +0530, Siddharth Menon wrote:
> From: BiscuitBobby <simeddon@gmail.com>
> 
> Convert the generic hwspinlock bindings to DT schema.
> ---
>  This is my first time converting bindings to dt schema, please let me
>  know if I have overlooked anything.
>  .../devicetree/bindings/hwlock/hwlock.txt     | 59 -----------------
>  .../devicetree/bindings/hwlock/hwlock.yaml    | 65 +++++++++++++++++++
>  2 files changed, 65 insertions(+), 59 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
>  create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.yaml

The consumer side lives in dtschema already. Please add the provider 
side there too. Patches to devicetree-spec@vger.kernel.org or GH PR are 
fine.

For the descriptions, you'll need to relicense the text in hwlock.txt to 
dual GPL/BSD. You will need TI's permission for that.

Rob

Re: [PATCH] dt-bindings: hwlock: Convert to dtschema

[Siddharth Menon]

On Wed, 19 Feb 2025 at 02:42, Rob Herring <robh@kernel.org> wrote:
>
> The consumer side lives in dtschema already. Please add the provider
> side there too. Patches to devicetree-spec@vger.kernel.org or GH PR are
> fine.

Thank you and Krzysztof for the feedback, I shall address the issues in the
next patch set.

> For the descriptions, you'll need to relicense the text in hwlock.txt to
> dual GPL/BSD. You will need TI's permission for that.

Regarding the relicensing, should I first submit a patch to relicense
the hwlock.txt
binding, then follow up with a patch to replace it with the YAML file
and correct
incorrect paths?

Regards,
Siddharth Menon