On 18/02/2025 17:09, Siddharth Menon wrote: > From: BiscuitBobby <simeddon@xxxxxxxxx> > > 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@xxxxxxxxxx> > + - Rob Herring <robh@xxxxxxxxxx> > + - Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx> > + - Conor Dooley <conor+dt@xxxxxxxxxx> 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
