Subject: [PATCH v2 1/2] dt-bindings: improve wb,mbl-gpio binding documentation.

From: Niall Leonard <[email protected]>

Convert existing wd,mbl-gpio binding documentation to YAML and add
new optional propery "no-input".

Signed-off-by: Niall Leonard <[email protected]>
---
.../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -----------
.../devicetree/bindings/gpio/wd,mbl-gpio.yaml | 78 ++++++++++++++++++++++
2 files changed, 78 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
deleted file mode 100644
index 038c3a6a1f4d..000000000000
--- a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
+++ /dev/null
@@ -1,38 +0,0 @@
-Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
-
-The Western Digital MyBook Live has two memory-mapped GPIO controllers.
-Both GPIO controller only have a single 8-bit data register, where GPIO
-state can be read and/or written.
-
-Required properties:
- - compatible: should be "wd,mbl-gpio"
- - reg-names: must contain
- "dat" - data register
- - reg: address + size pairs describing the GPIO register sets;
- order must correspond with the order of entries in reg-names
- - #gpio-cells: must be set to 2. The first cell is the pin number and
- the second cell is used to specify the gpio polarity:
- 0 = active high
- 1 = active low
- - gpio-controller: Marks the device node as a gpio controller.
-
-Optional properties:
- - no-output: GPIOs are read-only.
-
-Examples:
- gpio0: gpio0@e0000000 {
- compatible = "wd,mbl-gpio";
- reg-names = "dat";
- reg = <0xe0000000 0x1>;
- #gpio-cells = <2>;
- gpio-controller;
- };
-
- gpio1: gpio1@e0100000 {
- compatible = "wd,mbl-gpio";
- reg-names = "dat";
- reg = <0xe0100000 0x1>;
- #gpio-cells = <2>;
- gpio-controller;
- no-output;
- };
diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
new file mode 100644
index 000000000000..d1c72a42c5bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/wd,mbl-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Western Digital's MyBook Live memory-mapped GPIO controllers.
+
+maintainers:
+ - Niall Leonard <[email protected]>
+
+description: |+
+ Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
+
+ The Western Digital MyBook Live has two memory-mapped GPIO controllers.
+ Both GPIO controller only have a single 8-bit data register, where GPIO
+ state can be read and/or written.
+
+properties:
+ compatible:
+ enum:
+ - wd,mbl-gpio
+
+ reg-names:
+ items:
+ - const: dat
+
+ reg:
+ maxItems: 1
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-controller: true
+
+ no-output:
+ description: GPIOs are read-only.
+
+ no-input:
+ description: GPIOs are write-only.
+
+required:
+ - compatible
+ - reg-names
+ - reg
+ - '#gpio-cells'
+ - gpio-controller
+
+additionalProperties: false
+
+examples:
+ - |
+ gpio0: gpio0@e0000000 {
+ compatible = "wd,mbl-gpio";
+ reg-names = "dat";
+ reg = <0xe0000000 0x1>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ };
+
+ gpio1: gpio1@e0100000 {
+ compatible = "wd,mbl-gpio";
+ reg-names = "dat";
+ reg = <0xe0100000 0x1>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ no-output;
+ };
+
+ gpio2: gpio1@e0200000 {
+ compatible = "wd,mbl-gpio";
+ reg-names = "dat";
+ reg = <0xe0200000 0x1>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ no-input;
+ };
+

--
2.34.1



2023-02-01 08:27:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: improve wb,mbl-gpio binding documentation.

On 31/01/2023 14:49, Niall Leonard via B4 Submission Endpoint wrote:
> From: Niall Leonard <[email protected]>
>
> Convert existing wd,mbl-gpio binding documentation to YAML and add
> new optional propery "no-input".

Subject: drop full stop

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

Subject: improve is vague. Instead: convert to DT schema


>
> Signed-off-by: Niall Leonard <[email protected]>
> ---
> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -----------
> .../devicetree/bindings/gpio/wd,mbl-gpio.yaml | 78 ++++++++++++++++++++++
> 2 files changed, 78 insertions(+), 38 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
> deleted file mode 100644
> index 038c3a6a1f4d..000000000000
> --- a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
> -
> -The Western Digital MyBook Live has two memory-mapped GPIO controllers.
> -Both GPIO controller only have a single 8-bit data register, where GPIO
> -state can be read and/or written.
> -
> -Required properties:
> - - compatible: should be "wd,mbl-gpio"
> - - reg-names: must contain
> - "dat" - data register
> - - reg: address + size pairs describing the GPIO register sets;
> - order must correspond with the order of entries in reg-names
> - - #gpio-cells: must be set to 2. The first cell is the pin number and
> - the second cell is used to specify the gpio polarity:
> - 0 = active high
> - 1 = active low
> - - gpio-controller: Marks the device node as a gpio controller.
> -
> -Optional properties:
> - - no-output: GPIOs are read-only.
> -
> -Examples:
> - gpio0: gpio0@e0000000 {
> - compatible = "wd,mbl-gpio";
> - reg-names = "dat";
> - reg = <0xe0000000 0x1>;
> - #gpio-cells = <2>;
> - gpio-controller;
> - };
> -
> - gpio1: gpio1@e0100000 {
> - compatible = "wd,mbl-gpio";
> - reg-names = "dat";
> - reg = <0xe0100000 0x1>;
> - #gpio-cells = <2>;
> - gpio-controller;
> - no-output;
> - };
> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
> new file mode 100644
> index 000000000000..d1c72a42c5bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/wd,mbl-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Western Digital's MyBook Live memory-mapped GPIO controllers.

Drop full stop.

> +
> +maintainers:
> + - Niall Leonard <[email protected]>
> +
> +description: |+
> + Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.

Drop "Bindings for". Actually drop entire line - it's redundant, you
repeat the title.

> +
> + The Western Digital MyBook Live has two memory-mapped GPIO controllers.
> + Both GPIO controller only have a single 8-bit data register, where GPIO
> + state can be read and/or written.
> +
> +properties:
> + compatible:
> + enum:
> + - wd,mbl-gpio
> +
> + reg-names:
> + items:
> + - const: dat
> +
> + reg:
> + maxItems: 1
> +
> + "#gpio-cells":
> + const: 2
> +
> + gpio-controller: true
> +
> + no-output:
> + description: GPIOs are read-only.
> +
> + no-input:
> + description: GPIOs are write-only.


Split adding new property into separate patch. Each patch should do one
logical change. New feature is another logical change. Conversion is
that logical change (with any fixes needed for successful conversion,
but not with new features).


> +
> +required:
> + - compatible
> + - reg-names
> + - reg
> + - '#gpio-cells'

Use consistent quotes - either ' or "

> + - gpio-controller
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + gpio0: gpio0@e0000000 {

gpio@

(0 is not correct name of node)

> + compatible = "wd,mbl-gpio";
> + reg-names = "dat";
> + reg = <0xe0000000 0x1>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + };
> +
> + gpio1: gpio1@e0100000 {

gpio@

> + compatible = "wd,mbl-gpio";
> + reg-names = "dat";
> + reg = <0xe0100000 0x1>;
> + #gpio-cells = <2>;
> + gpio-controller;
> + no-output;
> + };
> +

Drop all examples below, they are not needed. Actually even these two
above could be combined as they differ with only one property.

Best regards,
Krzysztof


2023-02-07 09:50:14

by Leonard, Niall

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: improve wb,mbl-gpio binding documentation.

On 01/02/2023 08:27, Krzysztof Kozlowski wrote:
> *External Message* - Use caution before opening links or attachments
>
> On 31/01/2023 14:49, Niall Leonard via B4 Submission Endpoint wrote:
>> From: Niall Leonard <[email protected]>
>>
>> Convert existing wd,mbl-gpio binding documentation to YAML and add
>> new optional propery "no-input".
>
> Subject: drop full stop
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>
> Subject: improve is vague. Instead: convert to DT schema
>
>
>>
>> Signed-off-by: Niall Leonard <[email protected]>
>> ---
>> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -----------
>> .../devicetree/bindings/gpio/wd,mbl-gpio.yaml | 78 ++++++++++++++++++++++
>> 2 files changed, 78 insertions(+), 38 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>> deleted file mode 100644
>> index 038c3a6a1f4d..000000000000
>> --- a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>> +++ /dev/null
>> @@ -1,38 +0,0 @@
>> -Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
>> -
>> -The Western Digital MyBook Live has two memory-mapped GPIO controllers.
>> -Both GPIO controller only have a single 8-bit data register, where GPIO
>> -state can be read and/or written.
>> -
>> -Required properties:
>> - - compatible: should be "wd,mbl-gpio"
>> - - reg-names: must contain
>> - "dat" - data register
>> - - reg: address + size pairs describing the GPIO register sets;
>> - order must correspond with the order of entries in reg-names
>> - - #gpio-cells: must be set to 2. The first cell is the pin number and
>> - the second cell is used to specify the gpio polarity:
>> - 0 = active high
>> - 1 = active low
>> - - gpio-controller: Marks the device node as a gpio controller.
>> -
>> -Optional properties:
>> - - no-output: GPIOs are read-only.
>> -
>> -Examples:
>> - gpio0: gpio0@e0000000 {
>> - compatible = "wd,mbl-gpio";
>> - reg-names = "dat";
>> - reg = <0xe0000000 0x1>;
>> - #gpio-cells = <2>;
>> - gpio-controller;
>> - };
>> -
>> - gpio1: gpio1@e0100000 {
>> - compatible = "wd,mbl-gpio";
>> - reg-names = "dat";
>> - reg = <0xe0100000 0x1>;
>> - #gpio-cells = <2>;
>> - gpio-controller;
>> - no-output;
>> - };
>> diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
>> new file mode 100644
>> index 000000000000..d1c72a42c5bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.yaml
>> @@ -0,0 +1,78 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/wd,mbl-gpio.yaml*__;Iw!!In4Qlw!qBa7N0ccPrNQmjMeU9Hi-Ao3KxY4Fj2fXQutcNmpGxlXF7GLy7qjRk7k2ABk3yN7oi7tio6PoZyY-lsOTTreIaCxp-hI$
>> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!In4Qlw!qBa7N0ccPrNQmjMeU9Hi-Ao3KxY4Fj2fXQutcNmpGxlXF7GLy7qjRk7k2ABk3yN7oi7tio6PoZyY-lsOTTreIbT2TzBT$
>> +
>> +title: Western Digital's MyBook Live memory-mapped GPIO controllers.
>
> Drop full stop.
>
>> +
>> +maintainers:
>> + - Niall Leonard <[email protected]>
>> +
>> +description: |+
>> + Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
>
> Drop "Bindings for". Actually drop entire line - it's redundant, you
> repeat the title.
>
>> +
>> + The Western Digital MyBook Live has two memory-mapped GPIO controllers.
>> + Both GPIO controller only have a single 8-bit data register, where GPIO
>> + state can be read and/or written.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - wd,mbl-gpio
>> +
>> + reg-names:
>> + items:
>> + - const: dat
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#gpio-cells":
>> + const: 2
>> +
>> + gpio-controller: true
>> +
>> + no-output:
>> + description: GPIOs are read-only.
>> +
>> + no-input:
>> + description: GPIOs are write-only.
>
>
> Split adding new property into separate patch. Each patch should do one
> logical change. New feature is another logical change. Conversion is
> that logical change (with any fixes needed for successful conversion,
> but not with new features).
>
>
>> +
>> +required:
>> + - compatible
>> + - reg-names
>> + - reg
>> + - '#gpio-cells'
>
> Use consistent quotes - either ' or "
>
>> + - gpio-controller
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + gpio0: gpio0@e0000000 {
>
> gpio@
>
> (0 is not correct name of node)
>
>> + compatible = "wd,mbl-gpio";
>> + reg-names = "dat";
>> + reg = <0xe0000000 0x1>;
>> + #gpio-cells = <2>;
>> + gpio-controller;
>> + };
>> +
>> + gpio1: gpio1@e0100000 {
>
> gpio@
>
>> + compatible = "wd,mbl-gpio";
>> + reg-names = "dat";
>> + reg = <0xe0100000 0x1>;
>> + #gpio-cells = <2>;
>> + gpio-controller;
>> + no-output;
>> + };
>> +
>
> Drop all examples below, they are not needed. Actually even these two
> above could be combined as they differ with only one property.
>
> Best regards,
> Krzysztof
>
Thanks for reviewing. I'll make the suggested changes.

You might note that I added myself as maintainer of this file (as it
wouldn't pass the dt checks otherwise) but that can't really be the case
going forwards. I have only converted it from txt to yaml. Who should be
the maintainer ? There was no-one listed as maintainer previously ?

Regards,
Niall.

2023-02-07 10:48:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: improve wb,mbl-gpio binding documentation.

On 07/02/2023 10:49, Leonard, Niall wrote:
>> Drop all examples below, they are not needed. Actually even these two
>> above could be combined as they differ with only one property.
>>
>> Best regards,
>> Krzysztof
>>
> Thanks for reviewing. I'll make the suggested changes.
>
> You might note that I added myself as maintainer of this file (as it
> wouldn't pass the dt checks otherwise) but that can't really be the case
> going forwards. I have only converted it from txt to yaml. Who should be
> the maintainer ? There was no-one listed as maintainer previously ?

If you are not interested in maintaining this binding, then as last
resort add here subsystem maintainers (Linus and Bartosz) but it isn't
really correct... just better than nothing.

Best regards,
Krzysztof