2023-07-05 20:02:35

by Hawkins, Nick

[permalink] [raw]
Subject: [PATCH v5 1/5] dt-bindings: gpio: Add HPE GXP GPIO

From: Nick Hawkins <[email protected]>

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 <[email protected]>

---

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

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 <[email protected]>
+
+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", "", "", "", "",
+ "", "", "", "", "", "", "", "", "", "";
+ };
--
2.17.1



2023-07-06 06:40:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] dt-bindings: gpio: Add HPE GXP GPIO

On 05/07/2023 21:45, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> 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 <[email protected]>
>
> ---
>
> 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 <[email protected]>
> +
> +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


2023-07-06 15:03:39

by Hawkins, Nick

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] dt-bindings: gpio: Add HPE GXP GPIO

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

2023-07-06 19:08:30

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] dt-bindings: gpio: Add HPE GXP GPIO

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