| Message ID | 20230126-gpio-mmio-fix-v2-1-38397aace340@ncr.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce new optional property to mark port as write only. | expand |
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 <nl250060@ncr.com> >> >> 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 <nl250060@ncr.com> >> --- >> .../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 <nl250060@ncr.com> >> + >> +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.
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
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 <nl250060@ncr.com> + +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; + }; +