Re: [PATCH 05/14] dt-bindings: gpio: Convert Broadcom STB GPIO to YAML
From: Gregory Fong <hidden>
Date: 2021-12-02 16:01:13
Also in:
linux-arm-kernel, linux-crypto, linux-devicetree, linux-ide, linux-mmc, linux-pm, linux-pwm, linux-rtc, linux-usb, lkml
Hi Florian, I haven't kept up with the new yaml format, so not entirely sure I know what I'm talking about yet, but here are a few comments: On Wed, Dec 1, 2021 at 12:51 PM Florian Fainelli [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Convert the Broadcom STB GPIO Device Tree binding to YAML to help with validation. Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- .../bindings/gpio/brcm,brcmstb-gpio.txt | 83 -------------- .../bindings/gpio/brcm,brcmstb-gpio.yaml | 104 ++++++++++++++++++ MAINTAINERS | 2 +- 3 files changed, 105 insertions(+), 84 deletions(-) delete mode 100644 Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt create mode 100644 Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yamldiff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt deleted file mode 100644 index 5d468ecd1809..000000000000 --- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt +++ /dev/null@@ -1,83 +0,0 @@[snip] - -- interrupts-extended: - Alternate form of specifying interrupts and parents that allows for - multiple parents. This takes precedence over 'interrupts' and - 'interrupt-parent'. Wakeup-capable GPIO controllers often route their - wakeup interrupt lines through a different interrupt controller than the - primary interrupt line, making this property necessary.
It looks like interrupts-extended was removed from the new docs, I'm assuming that was intentional?
quoted hunk ↗ jump to hunk
[snip]diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml new file mode 100644 index 000000000000..4b7309dc74dc --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml@@ -0,0 +1,104 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/brcm,brcmstb-gpio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom STB "UPG GIO" GPIO controller + +description: > + The controller's registers are organized as sets of eight 32-bit + registers with each set controlling a bank of up to 32 pins. A single + interrupt is shared for all of the banks handled by the controller. + +maintainers: + - Doug Berger <opendmb@gmail.com> + - Florian Fainelli <f.fainelli@gmail.com> + +properties: + compatible: + oneOf: + - items: + - enum: + - brcm,bcm7445-gpio + - const: brcm,brcmstb-gpio + + reg: + maxItems: 1 + description:
Missing folded block scalar marker ('>') above
+ Define the base and range of the I/O address space containing + the brcmstb GPIO controller registers + + "#gpio-cells": + const: 2 + description: > + The first cell is the pin number (within the controller's + pin space), and the second is used for the following: + bit[0]: polarity (0 for active-high, 1 for active-low) + + gpio-controller: true + + "brcm,gpio-bank-widths": + $ref: /schemas/types.yaml#/definitions/uint32-array + description:
Same here
+ Number of GPIO lines for each bank. Number of elements must + correspond to number of banks suggested by the 'reg' property. + + interrupts: + maxItems: 1 + description:
While it's not necessary while this is only one line, consider adding '>' here too.
+ The interrupt shared by all GPIO lines for this controller. + + "#interrupt-cells": + const: 2 + description: >
This next block could get formatted strangely with '>'; recommend using '|' instead
+ The first cell is the GPIO number, the second should specify + flags. The following subset of flags is supported: + - bits[3:0] trigger type and level flags + 1 = low-to-high edge triggered + 2 = high-to-low edge triggered + 4 = active high level-sensitive + 8 = active low level-sensitive + Valid combinations are 1, 2, 3, 4, 8. + + interrupt-controller: true + + wakeup-source: + type: boolean + description: > + GPIOs for this controller can be used as a wakeup source + +required: + - compatible + - reg + - gpio-controller + - "#gpio-cells"
Need to add required property "brcm,gpio-bank-widths"
quoted hunk ↗ jump to hunk
+ +additionalProperties: false + +examples: + - | + upg_gio: gpio@f040a700 { + #gpio-cells = <2>; + #interrupt-cells = <2>; + compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio"; + gpio-controller; + interrupt-controller; + reg = <0xf040a700 0x80>; + interrupt-parent = <&irq0_intc>; + interrupts = <0x6>; + brcm,gpio-bank-widths = <32 32 32 24>; + }; + + upg_gio_aon: gpio@f04172c0 { + #gpio-cells = <2>; + #interrupt-cells = <2>; + compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio"; + gpio-controller; + interrupt-controller; + reg = <0xf04172c0 0x40>; + interrupt-parent = <&irq0_aon_intc>; + interrupts = <0x6>; + wakeup-source; + brcm,gpio-bank-widths = <18 4>; + };diff --git a/MAINTAINERS b/MAINTAINERS index 913856599623..78161abc384f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -3772,7 +3772,7 @@ BROADCOM BRCMSTB GPIO DRIVER M: Gregory Fong <gregory.0xf0@gmail.com>
Not really related to this patch, but I should probably update this entry to reflect current reality. Should that be you and/or Doug?
L: bcm-kernel-feedback-list@broadcom.com S: Supported -F: Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt +F: Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml F: drivers/gpio/gpio-brcmstb.c BROADCOM BRCMSTB I2C DRIVER -- 2.25.1
Best regards, Gregory