[v5,1/5] dt-bindings: gpio: Add HPE GXP GPIO

Message ID 20230705194544.100370-2-nick.hawkins@hpe.com
State New
Headers
Series ARM: Add GPIO support |

Commit Message

Hawkins, Nick July 5, 2023, 7:45 p.m. UTC
  From: Nick Hawkins <nick.hawkins@hpe.com>

Provide access to the register regions and interrupt for GPIO. The
driver under the hpe,gxp-gpio-pl will provide GPIO information from the
CPLD interface. The CPLD interface represents all physical GPIOs. The
GPIO interface with the CPLD allows use of interrupts.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---

v5:
 *Removed use of gpio-gxp in favor of just supporting
  hpe,gxp-gpio-pl for now as the full gpio-gxp will
  require a much larger patchset
 *Modified commit description to reflect removal of
  hpe,gxp-gpio
v4:
 *Fix min and max values for regs
v3:
 *Remove extra example in examples
 *Actually fixed indentation on example - Aligned
  GPIO line names with " above.
v2:
 *Put binding patch before the driver in the series
 *Improved patch description
 *Removed oneOf and items in compatible definition
 *Moved additionalProperties definition to correct spot in file
 *Fixed indentation on example
 *Improved description in .yaml
---
 .../bindings/gpio/hpe,gxp-gpio.yaml           | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
  

Comments

Krzysztof Kozlowski July 6, 2023, 6:22 a.m. UTC | #1
On 05/07/2023 21:45, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Provide access to the register regions and interrupt for GPIO. The
> driver under the hpe,gxp-gpio-pl will provide GPIO information from the
> CPLD interface. The CPLD interface represents all physical GPIOs. The
> GPIO interface with the CPLD allows use of interrupts.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> 
> v5:
>  *Removed use of gpio-gxp in favor of just supporting
>   hpe,gxp-gpio-pl for now as the full gpio-gxp will
>   require a much larger patchset

Bindings describe hardware, not drivers, and should be rather complete.


>  *Modified commit description to reflect removal of
>   hpe,gxp-gpio
> v4:
>  *Fix min and max values for regs
> v3:
>  *Remove extra example in examples
>  *Actually fixed indentation on example - Aligned
>   GPIO line names with " above.
> v2:
>  *Put binding patch before the driver in the series
>  *Improved patch description
>  *Removed oneOf and items in compatible definition
>  *Moved additionalProperties definition to correct spot in file
>  *Fixed indentation on example
>  *Improved description in .yaml
> ---
>  .../bindings/gpio/hpe,gxp-gpio.yaml           | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
> new file mode 100644
> index 000000000000..799643c1a0c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/hpe,gxp-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP gpio controllers

GPIO
> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +
> +description:
> +  Interruptable GPIO drivers for the HPE GXP that covers multiple interfaces

"drivers" as Linux drivers? If so, then drop and rephrase to describe
hardware.

> +  of both physical and virtual GPIO pins.
> +
> +properties:
> +  compatible:
> +    const: hpe,gxp-gpio-pl> +
> +  reg:
> +    items:
> +      - description: pl base gpio
> +      - description: pl interrupt gpio
> +
> +  reg-names:
> +    items:
> +      - const: base
> +      - const: interrupt
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-line-names:
> +    maxItems: 80
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - gpio-controller
> +  - "#gpio-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        gpio@51000300 {

Wrong indentation. Use 2 or 4 (preferred) spaces, not 8.

> +          compatible = "hpe,gxp-gpio-pl";
> +          reg = <0x51000300 0x7f>, <0x51000380 0x20>;
> +          reg-names = "base", "interrupt";
> +          gpio-controller;
> +          #gpio-cells = <2>;
> +          interrupt-parent = <&vic0>;
> +          interrupts = <24>;
> +          gpio-line-names =
> +          "IOP_LED1", "IOP_LED2", "IOP_LED3", "IOP_LED4", "IOP_LED5", "IOP_LED6", "IOP_LED7", "IOP_LED8",

And this is even worse.

> +          "FAN1_INST", "FAN2_INST", "FAN3_INST", "FAN4_INST", "FAN5_INST", "FAN6_INST", "FAN7_INST",
> +          "FAN8_INST", "FAN1_FAIL", "FAN2_FAIL", "FAN3_FAIL", "FAN4_FAIL", "FAN5_FAIL", "FAN6_FAIL",
> +          "FAN7_FAIL", "FAN8_FAIL", "FAN1_ID", "FAN2_ID", "FAN3_ID", "FAN4_ID", "FAN5_ID", "FAN6_ID",
> +          "FAN7_ID", "FAN8_ID", "IDENTIFY", "HEALTH_RED", "HEALTH_AMBER", "POWER_BUTTON", "UID_PRESS",
> +          "SLP", "NMI_BUTTON", "RESET_BUTTON", "SIO_S5", "SO_ON_CONTROL", "PSU1_INST", "PSU2_INST",
> +          "PSU3_INST", "PSU4_INST", "PSU5_INST", "PSU6_INST", "PSU7_INST", "PSU8_INST", "PSU1_AC",
> +          "PSU2_AC", "PSU3_AC", "PSU4_AC", "PSU5_AC", "PSU6_AC", "PSU7_AC", "PSU8_AC", "PSU1_DC",
> +          "PSU2_DC", "PSU3_DC", "PSU4_DC", "PSU5_DC", "PSU6_DC", "PSU7_DC", "PSU8_DC", "", "", "", "",
> +          "", "", "", "", "", "", "", "", "", "";
> +        };

Best regards,
Krzysztof
  
Hawkins, Nick July 6, 2023, 2:12 p.m. UTC | #2
Greetings Krzysztof,

Thank you for the feedback. I see that due to a patch conflict I
reintroduced some of the alignment issues you had me fix in
a previous version. This was a mistake and I will correct this.

> > v5:
> > *Removed use of gpio-gxp in favor of just supporting
> > hpe,gxp-gpio-pl for now as the full gpio-gxp will
> > require a much larger patchset

> Bindings describe hardware, not drivers, and should be rather complete.

This patch is intended to still cover the hardware interface between our
BMC and our CPLD which gathers GPIO for us. The part of the binding I
removed was a completely separate interface with different mechanisms
for reading GPIOs. With that said I could keep these two interfaces
separate in yaml files: Having a yaml for hpe,gxp-gpio and another for
hpe,gxp-gpio-pl. Would this be a better approach?

Thank you,

-Nick Hawkins
  
Rob Herring July 6, 2023, 7:05 p.m. UTC | #3
On Thu, Jul 06, 2023 at 02:12:12PM +0000, Hawkins, Nick wrote:
> Greetings Krzysztof,
> 
> Thank you for the feedback. I see that due to a patch conflict I
> reintroduced some of the alignment issues you had me fix in
> a previous version. This was a mistake and I will correct this.
> 
> > > v5:
> > > *Removed use of gpio-gxp in favor of just supporting
> > > hpe,gxp-gpio-pl for now as the full gpio-gxp will
> > > require a much larger patchset
> 
> > Bindings describe hardware, not drivers, and should be rather complete.
> 
> This patch is intended to still cover the hardware interface between our
> BMC and our CPLD which gathers GPIO for us. The part of the binding I
> removed was a completely separate interface with different mechanisms
> for reading GPIOs. With that said I could keep these two interfaces
> separate in yaml files: Having a yaml for hpe,gxp-gpio and another for
> hpe,gxp-gpio-pl. Would this be a better approach?

If they are independent (and it sounds like they are), then yes.

Rob
  

Patch

diff --git a/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
new file mode 100644
index 000000000000..799643c1a0c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
@@ -0,0 +1,71 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/hpe,gxp-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP gpio controllers
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+
+description:
+  Interruptable GPIO drivers for the HPE GXP that covers multiple interfaces
+  of both physical and virtual GPIO pins.
+
+properties:
+  compatible:
+    const: hpe,gxp-gpio-pl
+
+  reg:
+    items:
+      - description: pl base gpio
+      - description: pl interrupt gpio
+
+  reg-names:
+    items:
+      - const: base
+      - const: interrupt
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-line-names:
+    maxItems: 80
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+        gpio@51000300 {
+          compatible = "hpe,gxp-gpio-pl";
+          reg = <0x51000300 0x7f>, <0x51000380 0x20>;
+          reg-names = "base", "interrupt";
+          gpio-controller;
+          #gpio-cells = <2>;
+          interrupt-parent = <&vic0>;
+          interrupts = <24>;
+          gpio-line-names =
+          "IOP_LED1", "IOP_LED2", "IOP_LED3", "IOP_LED4", "IOP_LED5", "IOP_LED6", "IOP_LED7", "IOP_LED8",
+          "FAN1_INST", "FAN2_INST", "FAN3_INST", "FAN4_INST", "FAN5_INST", "FAN6_INST", "FAN7_INST",
+          "FAN8_INST", "FAN1_FAIL", "FAN2_FAIL", "FAN3_FAIL", "FAN4_FAIL", "FAN5_FAIL", "FAN6_FAIL",
+          "FAN7_FAIL", "FAN8_FAIL", "FAN1_ID", "FAN2_ID", "FAN3_ID", "FAN4_ID", "FAN5_ID", "FAN6_ID",
+          "FAN7_ID", "FAN8_ID", "IDENTIFY", "HEALTH_RED", "HEALTH_AMBER", "POWER_BUTTON", "UID_PRESS",
+          "SLP", "NMI_BUTTON", "RESET_BUTTON", "SIO_S5", "SO_ON_CONTROL", "PSU1_INST", "PSU2_INST",
+          "PSU3_INST", "PSU4_INST", "PSU5_INST", "PSU6_INST", "PSU7_INST", "PSU8_INST", "PSU1_AC",
+          "PSU2_AC", "PSU3_AC", "PSU4_AC", "PSU5_AC", "PSU6_AC", "PSU7_AC", "PSU8_AC", "PSU1_DC",
+          "PSU2_DC", "PSU3_DC", "PSU4_DC", "PSU5_DC", "PSU6_DC", "PSU7_DC", "PSU8_DC", "", "", "", "",
+          "", "", "", "", "", "", "", "", "", "";
+        };