[PATCH v3 0/5] ARM: Add GPIO support

[PATCH v3 0/5] ARM: Add GPIO support

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP SoC supports GPIO on multiple interfaces. The interfaces are
CPLD and Host. The GPIOs is a combination of both physical and virtual
I/O across the interfaces. The gpio-gxp driver specifically covers the
CSM(physical), FN2(virtual), and VUHC(virtual) which are the host. The
gpio-gxp-pl driver covers the CPLD which takes physical I/O from the
board and shares it with GXP via a propriety interface that maps the I/O
onto a specific register area of the GXP. The drivers both support
interrupts but from different interrupt parents.

The gxp-fan-ctrl driver in HWMON no longer will report fan presence
or fan failure states as these GPIOs providing this information will be
consumed by the host. It will be the hosts function to keep track of
fan presence and status.

---
Changes since v2:
 *Removed shared fan variables between HWMON and GPIO based on feedback
 *Removed reporting fan presence and failure from hwmon gxp-fan-ctrl
  driver
 *Removed GPIO dependency from gxp-fan-ctrl driver
 *Changed description and title for hpe,gxp-gpio binding
 *Corrected indention on example for hpe,gxp-gpio binding
 *Removed additional example from hpe,gxp-gpio binding
 
Changes since v1:
 *Removed ARM device tree changes and defconfig changes to reduce
  patchset size
 *Removed GXP PSU changes to reduce patchset size
 *Corrected hpe,gxp-gpio YAML file based on feedback
 *Created new gpio-gxp-pl file to reduce complexity
 *Separated code into two files to keep size down: gpio-gxp.c and
  gpio-gxp-pl.c
 *Fixed Kconfig indentation as well as add new entry for gpio-gxp-pl
 *Removed use of linux/of.h and linux/of_device.h
 *Added mod_devicetable.h and property.h
 *Fixed indentation of defines and uses consistent number of digits
 *Corrected defines with improper GPIO_ namespace.
 *For masks now use BIT()
 *Added comment for PLREG offsets
 *Move gpio_chip to be first in structure
 *Calculate offset for high and low byte GPIO reads instead of having
  H(High) and L(Low) letters added to the variables.
 *Removed repeditive use of "? 1 : 0"
 *Switched to handle_bad_irq()
 *Removed improper bailout on gpiochip_add_data
 *Used GENMASK to arm interrupts
 *Removed use of of_match_device
 *fixed sizeof in devm_kzalloc
 *Added COMPILE_TEST to Kconfig
 *Added dev_err_probe where applicable
 *Removed unecessary parent and compatible checks

Nick Hawkins (5):
  dt-bindings: gpio: Add HPE GXP GPIO
  gpio: gxp: Add HPE GXP GPIO
  dt-bindings: hwmon: hpe,gxp-fan-ctrl: remove fn2 and pl registers
  hwmon: (gxp_fan_ctrl) Provide fan info via gpio
  MAINTAINERS: hpe: Add GPIO

 .../bindings/gpio/hpe,gxp-gpio.yaml           | 139 ++++
 .../bindings/hwmon/hpe,gxp-fan-ctrl.yaml      |  16 +-
 MAINTAINERS                                   |   2 +
 drivers/gpio/Kconfig                          |  18 +
 drivers/gpio/Makefile                         |   2 +
 drivers/gpio/gpio-gxp-pl.c                    | 519 ++++++++++++++
 drivers/gpio/gpio-gxp.c                       | 637 ++++++++++++++++++
 drivers/hwmon/Kconfig                         |   2 +-
 drivers/hwmon/gxp-fan-ctrl.c                  | 108 +--
 9 files changed, 1324 insertions(+), 119 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
 create mode 100644 drivers/gpio/gpio-gxp-pl.c
 create mode 100644 drivers/gpio/gpio-gxp.c

-- 
2.17.1

[PATCH v3 1/5] dt-bindings: gpio: Add HPE GXP GPIO

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Provide access to the register regions and interrupt for GPIO. There
will be two drivers available. The first driver under the hpe,gxp-gpio
binding will provide GPIO information for the VUHC, CSM, and FN2
host interfaces. The second driver under the hpe,gxp-gpio-pl will
provide GPIO information from the CPLD interface. The main difference
and need for two separate bindings is they have different interrupt
parents. The other is hpe,gxp-gpio is a combination of physical
and virtual GPIOs where as hpe,gxp-gpio-pl are all physical
GPIOs from the CPLD.

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

---

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           | 139 ++++++++++++++++++
 1 file changed, 139 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..72cfff4d3e26
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
@@ -0,0 +1,139 @@
+# 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:
+    enum:
+      - hpe,gxp-gpio
+      - hpe,gxp-gpio-pl
+
+  reg:
+    minItems: 2
+    maxItems: 6
+
+  reg-names:
+    minItems: 2
+    maxItems: 6
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-line-names:
+    minItems: 80
+    maxItems: 300
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - hpe,gxp-gpio
+    then:
+      properties:
+        reg:
+          items:
+            - description: CSM GPIO interface
+            - description: fn2 virtual button GPIO
+            - description: fn2 system status GPIO
+            - description: vuhc GPIO status interface
+        reg-names:
+          items:
+            - const: csm
+            - const: fn2-vbtn
+            - const: fn2-stat
+            - const: vuhc
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - hpe,gxp-gpio-pl
+    then:
+      properties:
+        reg:
+          items:
+            - description: Programmable logic device GPIO
+            - description: Programmable logic device interrupt GPIO
+        reg-names:
+          items:
+            - const: base
+            - const: interrupt
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio@0 {
+        compatible = "hpe,gxp-gpio";
+        reg = <0x0 0x400>, <0x200046 0x1>, <0x200070 0x08>, <0x400064 0x80>;
+        reg-names = "csm", "fn2-vbtn", "fn2-stat", "vuhc";
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-parent = <&vic0>;
+        interrupts = <10>;
+        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

[PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP SoC supports GPIO on multiple interfaces. The interfaces are
CPLD and Host. The GPIOs is a combination of both physical and virtual
I/O across the interfaces. The gpio-gxp driver specifically covers the
CSM(physical), FN2(virtual), and VUHC(virtual) which are the host. The
gpio-gxp-pl driver covers the CPLD which takes physical I/O from the
board and shares it with GXP via a propriety interface that maps the I/O
onto a specific register area of the GXP. The drivers both support
interrupts but from different interrupt parents.

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

---

v3:
 *Remove shared variables with gxp-fan-ctrl
v2:
 *Separated code into two files to keep size down: gpio-gxp.c and
  gpio-gxp-pl.c
 *Fixed Kconfig indentation as well as add new entry for gpio-gxp-pl
 *Removed use of linux/of.h and linux/of_device.h
 *Added mod_devicetable.h and property.h
 *Fixed indentation of defines and uses consistent number of digits
 *Corrected defines with improper GPIO_ namespace.
 *For masks now use BIT()
 *Added comment for PLREG offsets
 *Move gpio_chip to be first in structure
 *Calculate offset for high and low byte GPIO reads instead of having
  H(High) and L(Low) letters added to the variables.
 *Removed repeditive use of "? 1 : 0"
 *Switched to handle_bad_irq()
 *Removed improper bailout on gpiochip_add_data
 *Used GENMASK to arm interrupts
 *Removed use of of_match_device
 *fixed sizeof in devm_kzalloc
 *Added COMPILE_TEST to Kconfig
 *Added dev_err_probe
 *Removed unecessary parent and compatible checks
---
 drivers/gpio/Kconfig       |  18 ++
 drivers/gpio/Makefile      |   2 +
 drivers/gpio/gpio-gxp-pl.c | 519 ++++++++++++++++++++++++++++++
 drivers/gpio/gpio-gxp.c    | 637 +++++++++++++++++++++++++++++++++++++
 4 files changed, 1176 insertions(+)
 create mode 100644 drivers/gpio/gpio-gxp-pl.c
 create mode 100644 drivers/gpio/gpio-gxp.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 13be729710f2..b0a24ef18392 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1235,6 +1235,24 @@ config HTC_EGPIO
 	  several HTC phones.  It provides basic support for input
 	  pins, output pins, and IRQs.
 
+config GPIO_GXP
+	tristate "GXP GPIO support"
+	depends on ARCH_HPE_GXP || COMPILE_TEST
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y here to support GXP GPIO controllers. It provides
+	  support for the multiple GPIO interfaces available to be
+	  available to the Host.
+
+config GPIO_GXP_PL
+	tristate "GXP GPIO PL support"
+	depends on ARCH_HPE_GXP || COMPILE_TEST
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y here to support GXP GPIO PL controller. It provides
+	  support for the GPIO PL interface available to be
+	  available to the Host.
+
 config GPIO_JANZ_TTL
 	tristate "Janz VMOD-TTL Digital IO Module"
 	depends on MFD_JANZ_CMODIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c048ba003367..a401dd472c93 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -63,6 +63,8 @@ obj-$(CONFIG_GPIO_FTGPIO010)		+= gpio-ftgpio010.o
 obj-$(CONFIG_GPIO_GE_FPGA)		+= gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM)		+= gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)		+= gpio-grgpio.o
+obj-$(CONFIG_GPIO_GXP)                  += gpio-gxp.o
+obj-$(CONFIG_GPIO_GXP_PL)		+= gpio-gxp-pl.o
 obj-$(CONFIG_GPIO_GW_PLD)		+= gpio-gw-pld.o
 obj-$(CONFIG_GPIO_HISI)                 += gpio-hisi.o
 obj-$(CONFIG_GPIO_HLWD)			+= gpio-hlwd.o
diff --git a/drivers/gpio/gpio-gxp-pl.c b/drivers/gpio/gpio-gxp-pl.c
new file mode 100644
index 000000000000..eaa2ffe2edfa
--- /dev/null
+++ b/drivers/gpio/gpio-gxp-pl.c
@@ -0,0 +1,519 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+/* Specific offsets in CPLD registers for interrupts */
+#define PLREG_INT_GRP_STAT_MASK	0x08
+#define PLREG_INT_HI_PRI_EN	0x0C
+#define PLREG_INT_GRP5_BASE	0x31
+#define PLREG_INT_GRP6_BASE	0x35
+#define PLREG_INT_GRP5_FLAG	0x30
+#define PLREG_INT_GRP6_FLAG	0x34
+
+/* Specific bits to enable Group 4 and Group 5 interrupts */
+#define PLREG_GRP4_GRP5_MASK	GENMASK(5, 4)
+
+/* Specific offsets in CPLD registers */
+#define PLREG_IOP_LED		0x04
+#define PLREG_IDENT_LED		0x05
+#define PLREG_HEALTH_LED	0x0D
+#define PLREG_PSU_INST		0x19
+#define PLREG_PSU_AC		0x1B
+#define PLREG_PSU_DC		0x1C
+#define PLREG_FAN_INST		0x27
+#define PLREG_FAN_FAIL		0x29
+
+#define GXP_GPIO_DIR_OUT        0x00
+#define GXP_GPIO_DIR_IN         0x01
+
+enum pl_gpio_pn {
+	IOP_LED1 = 0,
+	IOP_LED2 = 1,
+	IOP_LED3 = 2,
+	IOP_LED4 = 3,
+	IOP_LED5 = 4,
+	IOP_LED6 = 5,
+	IOP_LED7 = 6,
+	IOP_LED8 = 7,
+	FAN1_INST = 8,
+	FAN2_INST = 9,
+	FAN3_INST = 10,
+	FAN4_INST = 11,
+	FAN5_INST = 12,
+	FAN6_INST = 13,
+	FAN7_INST = 14,
+	FAN8_INST = 15,
+	FAN1_FAIL = 16,
+	FAN2_FAIL = 17,
+	FAN3_FAIL = 18,
+	FAN4_FAIL = 19,
+	FAN5_FAIL = 20,
+	FAN6_FAIL = 21,
+	FAN7_FAIL = 22,
+	FAN8_FAIL = 23,
+	LED_IDENTIFY = 24,
+	LED_HEALTH_RED = 25,
+	LED_HEALTH_AMBER = 26,
+	PWR_BTN_INT = 27,
+	UID_PRESS_INT = 28,
+	SLP_INT = 29,
+	ACM_FORCE_OFF = 30,
+	ACM_REMOVED = 31,
+	ACM_REQ_N = 32,
+	PSU1_INST = 33,
+	PSU2_INST = 34,
+	PSU3_INST = 35,
+	PSU4_INST = 36,
+	PSU5_INST = 37,
+	PSU6_INST = 38,
+	PSU7_INST = 39,
+	PSU8_INST = 40,
+	PSU1_AC = 41,
+	PSU2_AC = 42,
+	PSU3_AC = 43,
+	PSU4_AC = 44,
+	PSU5_AC = 45,
+	PSU6_AC = 46,
+	PSU7_AC = 47,
+	PSU8_AC = 48,
+	PSU1_DC = 49,
+	PSU2_DC = 50,
+	PSU3_DC = 51,
+	PSU4_DC = 52,
+	PSU5_DC = 53,
+	PSU6_DC = 54,
+	PSU7_DC = 55,
+	PSU8_DC = 56,
+	RESET = 57,
+	NMI_OUT = 58,
+	VPBTN = 59,
+	PGOOD = 60,
+	PERST = 61,
+	POST_COMPLETE = 62,
+};
+
+/* Remember last PSU config */
+u8 psu_presence;
+
+struct gxp_gpio_drvdata {
+	struct regmap *base;
+	struct regmap *interrupt;
+	struct gpio_chip chip;
+	int irq;
+};
+
+static struct regmap *gxp_gpio_init_regmap(struct platform_device *pdev,
+					   char *reg_name, u8 bits)
+{
+	struct regmap_config regmap_config = {
+		.reg_bits = 32,
+		.reg_stride = 4,
+		.val_bits = 32,
+	};
+	void __iomem *base;
+
+	if (bits == 8) {
+		regmap_config.reg_bits = 8;
+		regmap_config.reg_stride = 1;
+		regmap_config.val_bits = 8;
+		regmap_config.max_register = 0xff;
+	}
+
+	base = devm_platform_ioremap_resource_byname(pdev, reg_name);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	regmap_config.name = reg_name;
+
+	return devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
+}
+
+static int gxp_gpio_pl_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+	int ret = 0;
+
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+		regmap_read(drvdata->base, PLREG_IOP_LED, &val);
+		ret = (val & BIT(offset)) ? 1 : 0;
+		break;
+	case FAN1_INST ...FAN8_INST:
+		regmap_read(drvdata->base, PLREG_FAN_INST, &val);
+		ret = (val & BIT((offset - FAN1_INST))) ? 1 : 0;
+		break;
+	case FAN1_FAIL ... FAN8_FAIL:
+		regmap_read(drvdata->base, PLREG_FAN_FAIL, &val);
+		ret = (val & BIT((offset - FAN1_FAIL))) ? 1 : 0;
+		break;
+	case PWR_BTN_INT ... SLP_INT:
+		regmap_read(drvdata->base, PLREG_INT_GRP5_FLAG, &val);
+		/* Active low */
+		ret = (val & BIT((offset - PWR_BTN_INT) + 16)) ? 0 : 1;
+		break;
+	case  PSU1_INST ... PSU8_INST:
+		regmap_read(drvdata->base, PLREG_PSU_INST, &val);
+		psu_presence = (u8)val;
+		ret = (psu_presence & BIT((offset - PSU1_INST))) ? 1 : 0;
+		break;
+	case PSU1_AC ... PSU8_AC:
+		regmap_read(drvdata->base, PLREG_PSU_AC, &val);
+		ret = (val & BIT((offset - PSU1_AC))) ? 1 : 0;
+		break;
+	case PSU1_DC ... PSU8_DC:
+		regmap_read(drvdata->base, PLREG_PSU_DC, &val);
+		ret = (val & BIT((offset - PSU1_DC))) ? 1 : 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static void gxp_gpio_pl_set(struct gpio_chip *chip,
+			    unsigned int offset, int value)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+		regmap_update_bits(drvdata->base,
+				   PLREG_IOP_LED,
+				   BIT(offset),
+				   value == 0 ? 0 : BIT(offset));
+		break;
+	case LED_IDENTIFY:
+		regmap_update_bits(drvdata->base,
+				   PLREG_IDENT_LED,
+				   BIT(7) | BIT(6),
+				   value == 0 ? BIT(7) : BIT(7) | BIT(6));
+		break;
+	case LED_HEALTH_RED:
+		regmap_update_bits(drvdata->base,
+				   PLREG_HEALTH_LED,
+				   BIT(7),
+				   value == 0 ? 0 : BIT(7));
+		break;
+	case LED_HEALTH_AMBER:
+		regmap_update_bits(drvdata->base,
+				   PLREG_HEALTH_LED,
+				   BIT(6),
+				   value == 0 ? 0 : BIT(6));
+		break;
+	default:
+		break;
+	}
+}
+
+static int gxp_gpio_pl_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	int ret = GXP_GPIO_DIR_IN;
+
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+	case LED_IDENTIFY ... LED_HEALTH_AMBER:
+	case ACM_FORCE_OFF:
+	case ACM_REQ_N:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_pl_direction_input(struct gpio_chip *chip,
+				       unsigned int offset)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case 8 ... 55:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	case 59 ... 65:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_pl_direction_output(struct gpio_chip *chip,
+					unsigned int offset, int value)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+	case LED_IDENTIFY ... LED_HEALTH_AMBER:
+	case ACM_FORCE_OFF:
+	case ACM_REQ_N:
+		gxp_gpio_pl_set(chip, offset, value);
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static void gxp_gpio_pl_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+
+	/* Read latched interrupt for group 5 */
+	regmap_read(drvdata->interrupt, PLREG_INT_GRP5_FLAG, &val);
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_FLAG,
+			   0xFF, 0xFF);
+
+	/* Read latched interrupt for group 6 */
+	regmap_read(drvdata->interrupt, PLREG_INT_GRP6_FLAG, &val);
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_FLAG,
+			   0xFF, 0xFF);
+}
+
+static void gxp_gpio_pl_irq_set_mask(struct irq_data *d, bool set)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_BASE,
+			   BIT(0) | BIT(2), set ? 0 : BIT(0) | BIT(2));
+
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_BASE,
+			   BIT(2), set ? 0 : BIT(2));
+}
+
+static void gxp_gpio_pl_irq_mask(struct irq_data *d)
+{
+	gxp_gpio_pl_irq_set_mask(d, false);
+}
+
+static void gxp_gpio_pl_irq_unmask(struct irq_data *d)
+{
+	gxp_gpio_pl_irq_set_mask(d, true);
+}
+
+static int gxp_gpio_irq_init_hw(struct gpio_chip *chip)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_BASE,
+			   BIT(0) | BIT(2), 0);
+
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_BASE,
+			   BIT(2), 0);
+
+	return 0;
+}
+
+static int gxp_gpio_pl_set_type(struct irq_data *d, unsigned int type)
+{
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else
+		irq_set_handler_locked(d, handle_edge_irq);
+
+	return 0;
+}
+
+static irqreturn_t gxp_gpio_pl_irq_handle(int irq, void *_drvdata)
+{
+	struct gxp_gpio_drvdata *drvdata = (struct gxp_gpio_drvdata *)_drvdata;
+	unsigned int val, girq, i;
+
+	/* Check group 5 interrupts */
+
+	regmap_read(drvdata->base, PLREG_INT_GRP5_FLAG, &val);
+
+	if (val) {
+		for_each_set_bit(i, (unsigned long *)&val, 3) {
+			girq = irq_find_mapping(drvdata->chip.irq.domain,
+						i + PWR_BTN_INT);
+			generic_handle_irq(girq);
+		}
+
+		/* Clear latched interrupt */
+		regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_FLAG,
+				   0xFF, 0xFF);
+		regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_BASE,
+				   BIT(0) | BIT(2), 0);
+	}
+
+	/* Check group 6 interrupts */
+
+	regmap_read(drvdata->base, PLREG_INT_GRP6_FLAG, &val);
+
+	if (val & BIT(2)) {
+		u8 old_psu = psu_presence;
+
+		regmap_read(drvdata->base, PLREG_PSU_INST, &val);
+		psu_presence = (u8)val;
+
+		if (old_psu != psu_presence) {
+			/* Identify all bits which differs */
+			u8 current_val = psu_presence;
+			u8 old_val = old_psu;
+
+			for (i = 0 ; i < 8 ; i++) {
+				if ((current_val & 0x1) != (old_val & 0x1)) {
+				/* PSU state has changed */
+					girq = irq_find_mapping(drvdata->chip.irq.domain,
+								i + PSU1_INST);
+					if (girq)
+						generic_handle_irq(girq);
+				}
+				current_val = current_val >> 1;
+				old_val = old_val >> 1;
+			}
+		}
+	}
+
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_FLAG,
+			   0xFF, 0xFF);
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_BASE,
+			   BIT(2), 0);
+
+	return IRQ_HANDLED;
+}
+
+static struct gpio_chip plreg_chip = {
+	.label			= "gxp_gpio_plreg",
+	.owner			= THIS_MODULE,
+	.get			= gxp_gpio_pl_get,
+	.set			= gxp_gpio_pl_set,
+	.get_direction = gxp_gpio_pl_get_direction,
+	.direction_input = gxp_gpio_pl_direction_input,
+	.direction_output = gxp_gpio_pl_direction_output,
+	.base = -1,
+};
+
+static struct irq_chip gxp_plreg_irqchip = {
+	.name		= "gxp_plreg",
+	.irq_ack	= gxp_gpio_pl_irq_ack,
+	.irq_mask	= gxp_gpio_pl_irq_mask,
+	.irq_unmask	= gxp_gpio_pl_irq_unmask,
+	.irq_set_type	= gxp_gpio_pl_set_type,
+};
+
+static int gxp_gpio_pl_init(struct platform_device *pdev)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
+	struct gpio_irq_chip *girq;
+	int ret;
+	unsigned int val;
+
+	drvdata->base = gxp_gpio_init_regmap(pdev, "base", 8);
+	if (IS_ERR(drvdata->base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->base),
+				     "failed to map base\n");
+
+	drvdata->interrupt = gxp_gpio_init_regmap(pdev, "interrupt", 8);
+	if (IS_ERR(drvdata->interrupt))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->interrupt),
+				     "failed to map interrupt base\n");
+
+	regmap_read(drvdata->base, PLREG_PSU_INST, &val);
+	psu_presence = (u8)val;
+
+	drvdata->chip = plreg_chip;
+	drvdata->chip.ngpio = 57;
+	drvdata->chip.parent = &pdev->dev;
+
+	girq = &drvdata->chip.irq;
+	girq->chip = &gxp_plreg_irqchip;
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	girq->init_hw = gxp_gpio_irq_init_hw;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
+	if (ret < 0)
+		dev_err_probe(&pdev->dev, ret, "Could not register gpiochip for plreg\n");
+
+	regmap_update_bits(drvdata->base,
+			   PLREG_INT_HI_PRI_EN,
+			   PLREG_GRP4_GRP5_MASK,
+			   PLREG_GRP4_GRP5_MASK);
+	regmap_update_bits(drvdata->base,
+			   PLREG_INT_GRP_STAT_MASK,
+			   PLREG_GRP4_GRP5_MASK,
+			   0x00);
+	regmap_read(drvdata->base, PLREG_INT_HI_PRI_EN, &val);
+	regmap_read(drvdata->base, PLREG_INT_GRP_STAT_MASK, &val);
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "Get irq from platform fail\n");
+
+	drvdata->irq = ret;
+
+	ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
+			       IRQF_SHARED, "gxp-pl", drvdata);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id gxp_gpio_of_match[] = {
+	{ .compatible = "hpe,gxp-gpio-pl" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gxp_gpio_of_match);
+
+static int gxp_gpio_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct gxp_gpio_drvdata *drvdata;
+
+	/* Initialize global vars */
+	psu_presence = 0;
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drvdata);
+
+	ret = gxp_gpio_pl_init(pdev);
+
+	return ret;
+}
+
+static struct platform_driver gxp_gpio_driver = {
+	.driver = {
+		.name = "gxp-gpio-pl",
+		.of_match_table = gxp_gpio_of_match,
+	},
+	.probe = gxp_gpio_probe,
+};
+module_platform_driver(gxp_gpio_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>");
+MODULE_DESCRIPTION("GPIO PL interface for GXP");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpio/gpio-gxp.c b/drivers/gpio/gpio-gxp.c
new file mode 100644
index 000000000000..ed6d8577e6b7
--- /dev/null
+++ b/drivers/gpio/gpio-gxp.c
@@ -0,0 +1,637 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define GPIDAT		0x040
+#define GPODAT		0x0b0
+#define GPODAT2		0x0f8
+#define GPOOWN		0x110
+#define GPOOWN2		0x118
+#define ASR_OFS		0x05c
+
+#define GXP_GPIO_DIR_OUT	0
+#define GXP_GPIO_DIR_IN		1
+
+#define PGOOD_MASK	BIT(0)
+
+struct gxp_gpio_drvdata {
+	struct gpio_chip chip;
+	struct regmap *csm_map;
+	void __iomem *fn2_vbtn;
+	struct regmap *fn2_stat;
+	struct regmap *vuhc0_map;
+	int irq;
+};
+
+/*
+ * Note: Instead of definining all PINs here are the select few that
+ * are specifically defined in DTS and offsets are used here.
+ */
+enum gxp_gpio_pn {
+	RESET = 192,
+	VPBTN = 210, /* aka POWER_OK */
+	PGOOD = 211, /* aka PS_PWROK */
+	PERST = 212, /* aka PCIERST */
+	POST_COMPLETE = 213,
+};
+
+static int gxp_gpio_csm_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	int ret = 0;
+
+	switch (offset) {
+	case 0 ... 31:
+		regmap_read(drvdata->csm_map, GPIDAT, &ret);
+		ret = (ret & BIT(offset));
+		break;
+	case 32 ... 63:
+		regmap_read(drvdata->csm_map, GPIDAT + 0x20, &ret);
+		ret = (ret & BIT(offset - 32));
+		break;
+	case 64 ... 95:
+		regmap_read(drvdata->csm_map, GPODAT, &ret);
+		ret = (ret & BIT(offset - 64));
+		break;
+	case 96 ... 127:
+		regmap_read(drvdata->csm_map, GPODAT + 0x04, &ret);
+		ret = (ret & BIT(offset - 96));
+		break;
+	case 128 ...  159:
+		regmap_read(drvdata->csm_map, GPODAT2, &ret);
+		ret = (ret & BIT(offset - 128));
+		break;
+	case 160 ... 191:
+		regmap_read(drvdata->csm_map, GPODAT2 + 0x04, &ret);
+		ret = (ret & BIT(offset - 160));
+		break;
+	case RESET:
+		/* SW_RESET */
+		regmap_read(drvdata->csm_map, ASR_OFS, &ret);
+		ret = (ret & BIT(15));
+		break;
+	default:
+		break;
+	}
+
+	/* Return either 1 or 0 */
+	return (ret ? 1 : 0);
+}
+
+static void gxp_gpio_csm_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	u32 tmp;
+
+	switch (offset) {
+	case 64 ... 95:
+		/* Keep ownership setting */
+		regmap_read(drvdata->csm_map, GPOOWN, &tmp);
+		tmp = (tmp & BIT(offset - 64)) ? 1 : 0;
+
+		regmap_update_bits(drvdata->csm_map, GPOOWN,
+				   BIT(offset - 64), BIT(offset - 64));
+		regmap_update_bits(drvdata->csm_map, GPODAT,
+				   BIT(offset - 64), value ? BIT(offset - 64) : 0);
+
+		/* Restore ownership setting */
+		regmap_update_bits(drvdata->csm_map, GPOOWN,
+				   BIT(offset - 64), tmp ? BIT(offset - 64) : 0);
+		break;
+	case 96 ... 127:
+		/* Keep ownership setting */
+		regmap_read(drvdata->csm_map, GPOOWN + 0x04, &tmp);
+		tmp = (tmp & BIT(offset - 96)) ? 1 : 0;
+
+		regmap_update_bits(drvdata->csm_map, GPOOWN + 0x04,
+				   BIT(offset - 96), BIT(offset - 96));
+		regmap_update_bits(drvdata->csm_map, GPODAT + 0x04,
+				   BIT(offset - 96), value ? BIT(offset - 96) : 0);
+
+		/* Restore ownership setting */
+		regmap_update_bits(drvdata->csm_map, GPOOWN + 0x04,
+				   BIT(offset - 96), tmp ? BIT(offset - 96) : 0);
+		break;
+	case 128 ... 159:
+		/* Keep ownership setting */
+		regmap_read(drvdata->csm_map, GPOOWN2, &tmp);
+		tmp = (tmp & BIT(offset - 128)) ? 1 : 0;
+
+		regmap_update_bits(drvdata->csm_map, GPOOWN2,
+				   BIT(offset - 128), BIT(offset - 128));
+		regmap_update_bits(drvdata->csm_map, GPODAT2,
+				   BIT(offset - 128), value ? BIT(offset - 128) : 0);
+
+		/* Restore ownership setting */
+		regmap_update_bits(drvdata->csm_map, GPOOWN2,
+				   BIT(offset - 128), tmp ? BIT(offset - 128) : 0);
+		break;
+	case 160 ... 191:
+		/* Keep ownership setting */
+		regmap_read(drvdata->csm_map, GPOOWN2 + 0x04,	&tmp);
+		tmp = (tmp & BIT(offset - 160)) ? 1 : 0;
+
+		regmap_update_bits(drvdata->csm_map, GPOOWN2 + 0x04,
+				   BIT(offset - 160), BIT(offset - 160));
+		regmap_update_bits(drvdata->csm_map, GPODAT2 + 0x04,
+				   BIT(offset - 160), value ? BIT(offset - 160) : 0);
+
+		/* Restore ownership setting */
+		regmap_update_bits(drvdata->csm_map, GPOOWN2 + 0x04,
+				   BIT(offset - 160), tmp ? BIT(offset - 160) : 0);
+		break;
+	case 192:
+		if (value) {
+			regmap_update_bits(drvdata->csm_map, ASR_OFS,
+					   BIT(0), BIT(0));
+			regmap_update_bits(drvdata->csm_map, ASR_OFS,
+					   BIT(15), BIT(15));
+		} else {
+			regmap_update_bits(drvdata->csm_map, ASR_OFS,
+					   BIT(15), 0);
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static int gxp_gpio_csm_get_direction(struct gpio_chip *chip,
+				      unsigned int offset)
+{
+	int ret = 0;
+
+	switch (offset) {
+	case 0 ... 63:
+		ret = GXP_GPIO_DIR_IN;
+		break;
+	case 64 ... 191:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	case 192 ... 193:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	case 194:
+		ret = GXP_GPIO_DIR_IN;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_csm_direction_input(struct gpio_chip *chip,
+					unsigned int offset)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case 0 ... 63:
+		ret = 0;
+		break;
+	case 194:
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_csm_direction_output(struct gpio_chip *chip,
+					 unsigned int offset, int value)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case 64 ... 191:
+	case 192 ... 193:
+		gxp_gpio_csm_set(chip, offset, value);
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_vuhc_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+	int ret = 0;
+
+	if (offset < 8) {
+		regmap_read(drvdata->vuhc0_map, 0x64 + 4 * offset,   &val);
+		ret = (val & BIT(13)) ? 1 : 0;
+	}
+
+	return ret;
+}
+
+static void gxp_gpio_vuhc_set(struct gpio_chip *chip, unsigned int offset,
+			      int value)
+{
+	switch (offset) {
+	default:
+		break;
+	}
+}
+
+static int gxp_gpio_vuhc_get_direction(struct gpio_chip *chip,
+				       unsigned int offset)
+{
+	int ret = 0;
+
+	switch (offset) {
+	case 0:
+	case 1:
+	case 2:
+		ret = GXP_GPIO_DIR_IN;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_vuhc_direction_input(struct gpio_chip *chip,
+					 unsigned int offset)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case 0:
+	case 1:
+	case 2:
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_vuhc_direction_output(struct gpio_chip *chip,
+					  unsigned int offset, int value)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_fn2_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+	int ret = 0;
+
+	switch (offset) {
+	case PGOOD:
+		regmap_read(drvdata->fn2_stat, 0, &val);
+		ret = (val & BIT(24));
+
+		break;
+	case PERST:
+		regmap_read(drvdata->fn2_stat, 0, &val);
+		ret = (val & BIT(25));
+
+		break;
+	default:
+		break;
+	}
+
+	/* Return either 1 or 0 */
+	return (ret ? 1 : 0);
+}
+
+static void gxp_gpio_fn2_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+
+	switch (offset) {
+	case VPBTN:
+		writeb(1, drvdata->fn2_vbtn);
+		break;
+	default:
+		break;
+	}
+}
+
+static int gxp_gpio_fn2_get_direction(struct gpio_chip *chip,
+				      unsigned int offset)
+{
+	int ret = GXP_GPIO_DIR_IN;
+
+	switch (offset) {
+	case VPBTN:
+		ret = GXP_GPIO_DIR_OUT;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_fn2_direction_input(struct gpio_chip *chip,
+					unsigned int offset)
+{
+	int ret = -EOPNOTSUPP;
+
+	switch (offset) {
+	case PGOOD:
+	case PERST:
+	case POST_COMPLETE:
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int gxp_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	int ret = 0;
+
+	if (offset < 200)
+		ret = gxp_gpio_csm_get(chip, offset);
+	else if (offset >= 250 && offset < 300)
+		ret = gxp_gpio_vuhc_get(chip, offset - 250);
+	else if (offset >= 300)
+		ret = gxp_gpio_fn2_get(chip, offset);
+
+	return ret;
+}
+
+static void gxp_gpio_set(struct gpio_chip *chip,
+			 unsigned int offset, int value)
+{
+	if (offset < 200)
+		gxp_gpio_csm_set(chip, offset, value);
+	else if (offset >= 250 && offset < 300)
+		gxp_gpio_vuhc_set(chip, offset - 250, value);
+	else if (offset >= 300)
+		gxp_gpio_fn2_set(chip, offset, value);
+}
+
+static int gxp_gpio_get_direction(struct gpio_chip *chip,
+				  unsigned int offset)
+{
+	int ret = 0;
+
+	if (offset < 200)
+		ret = gxp_gpio_csm_get_direction(chip, offset);
+	else if (offset >= 250 && offset < 300)
+		ret = gxp_gpio_vuhc_get_direction(chip, offset - 250);
+	else if (offset >= 300)
+		ret = gxp_gpio_fn2_get_direction(chip, offset);
+
+	return ret;
+}
+
+static int gxp_gpio_direction_input(struct gpio_chip *chip,
+				    unsigned int offset)
+{
+	int ret = 0;
+
+	if (offset < 200)
+		ret = gxp_gpio_csm_direction_input(chip, offset);
+	else if (offset >= 250 && offset < 300)
+		ret = gxp_gpio_vuhc_direction_input(chip, offset - 250);
+	else if (offset >= 300)
+		ret = gxp_gpio_fn2_direction_input(chip, offset);
+
+	return ret;
+}
+
+static int gxp_gpio_direction_output(struct gpio_chip *chip,
+				     unsigned int offset, int value)
+{
+	int ret = 0;
+
+	if (offset < 200)
+		ret = gxp_gpio_csm_direction_output(chip, offset, value);
+	else if (offset >= 250 && offset < 300)
+		ret = gxp_gpio_vuhc_direction_output(chip, offset - 250, value);
+	return ret;
+}
+
+static struct regmap *gxp_gpio_init_regmap(struct platform_device *pdev,
+					   char *reg_name, u8 bits)
+{
+	struct regmap_config regmap_config = {
+		.reg_bits = 32,
+		.reg_stride = 4,
+		.val_bits = 32,
+	};
+	void __iomem *base;
+
+	if (bits == 8) {
+		regmap_config.reg_bits = 8;
+		regmap_config.reg_stride = 1;
+		regmap_config.val_bits = 8;
+		regmap_config.max_register = 0xff;
+	}
+
+	base = devm_platform_ioremap_resource_byname(pdev, reg_name);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	regmap_config.name = reg_name;
+
+	return devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
+}
+
+static void gxp_gpio_fn2_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+
+	/* Read latched interrupt */
+	regmap_read(drvdata->fn2_stat, 0, &val);
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->fn2_stat, 0,
+			   0xFFFF, 0xFFFF);
+}
+
+#define FN2_SEVMASK BIT(2)
+static void gxp_gpio_fn2_irq_set_mask(struct irq_data *d, bool set)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+
+	regmap_update_bits(drvdata->fn2_stat, FN2_SEVMASK,
+			   BIT(0), set ? BIT(0) : 0);
+}
+
+static void gxp_gpio_fn2_irq_mask(struct irq_data *d)
+{
+	gxp_gpio_fn2_irq_set_mask(d, false);
+}
+
+static void gxp_gpio_fn2_irq_unmask(struct irq_data *d)
+{
+	gxp_gpio_fn2_irq_set_mask(d, true);
+}
+
+static int gxp_gpio_fn2_set_type(struct irq_data *d, unsigned int type)
+{
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else
+		irq_set_handler_locked(d, handle_edge_irq);
+
+	return 0;
+}
+
+static irqreturn_t gxp_gpio_fn2_irq_handle(int irq, void *_drvdata)
+{
+	struct gxp_gpio_drvdata *drvdata = (struct gxp_gpio_drvdata *)_drvdata;
+	unsigned int val, girq;
+
+	regmap_read(drvdata->fn2_stat, 0, &val);
+
+	if (val & PGOOD_MASK) {
+		girq = irq_find_mapping(drvdata->chip.irq.domain, PGOOD);
+		generic_handle_irq(girq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct irq_chip gxp_gpio_irqchip = {
+	.name		= "gxp_fn2",
+	.irq_ack	= gxp_gpio_fn2_irq_ack,
+	.irq_mask	= gxp_gpio_fn2_irq_mask,
+	.irq_unmask	= gxp_gpio_fn2_irq_unmask,
+	.irq_set_type	= gxp_gpio_fn2_set_type,
+};
+
+static struct gpio_chip common_chip = {
+	.label			= "gxp_gpio",
+	.owner                  = THIS_MODULE,
+	.get                    = gxp_gpio_get,
+	.set                    = gxp_gpio_set,
+	.get_direction		= gxp_gpio_get_direction,
+	.direction_input	= gxp_gpio_direction_input,
+	.direction_output	= gxp_gpio_direction_output,
+	.base = 0,
+};
+
+static int gxp_gpio_init(struct platform_device *pdev)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
+	struct gpio_irq_chip *girq;
+	int ret;
+
+	drvdata->csm_map = gxp_gpio_init_regmap(pdev, "csm", 32);
+	if (IS_ERR(drvdata->csm_map))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->csm_map),
+				     "failed to map csm_handle\n");
+
+	drvdata->fn2_vbtn = devm_platform_ioremap_resource_byname(pdev, "fn2-vbtn");
+	if (IS_ERR(drvdata->fn2_vbtn))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->fn2_vbtn),
+				     "failed to map fn2_vbtn\n");
+
+	drvdata->fn2_stat = gxp_gpio_init_regmap(pdev, "fn2-stat", 32);
+	if (IS_ERR(drvdata->fn2_stat))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->fn2_stat),
+				     "failed to map fn2_stat\n");
+
+	drvdata->vuhc0_map = gxp_gpio_init_regmap(pdev, "vuhc", 32);
+	if (IS_ERR(drvdata->vuhc0_map))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->vuhc0_map),
+				     "failed to map vuhc0_map\n");
+
+	girq = &drvdata->chip.irq;
+	girq->chip = &gxp_gpio_irqchip;
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Get irq from platform fail\n");
+
+	drvdata->irq = ret;
+
+	ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_fn2_irq_handle,
+			       IRQF_SHARED, "gxp-fn2", drvdata);
+	if (ret < 0)
+		return ret;
+
+	drvdata->chip = common_chip;
+	drvdata->chip.ngpio = 220;
+
+	drvdata->chip.parent = &pdev->dev;
+	ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, NULL);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+			"Could not register gpiochip for fn2\n");
+
+	return 0;
+}
+
+static const struct of_device_id gxp_gpio_of_match[] = {
+	{ .compatible = "hpe,gxp-gpio" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gxp_gpio_of_match);
+
+static int gxp_gpio_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct gxp_gpio_drvdata *drvdata;
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drvdata);
+
+	ret = gxp_gpio_init(pdev);
+
+	return ret;
+}
+
+static struct platform_driver gxp_gpio_driver = {
+	.driver = {
+		.name = "gxp-gpio",
+		.of_match_table = gxp_gpio_of_match,
+	},
+	.probe = gxp_gpio_probe,
+};
+module_platform_driver(gxp_gpio_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>");
+MODULE_DESCRIPTION("GPIO interface for GXP");
+MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH v3 3/5] dt-bindings: hwmon: hpe,gxp-fan-ctrl: remove fn2 and pl registers

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Reduce the hpe,gxp-fan-ctrl register references from 3 to 1. The
function2 (fn2) and programmable logic (pl) references are removed.
The purpose of removal being their functionality will be consumed by a
new GPIO driver.

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

---

v3:
 *Modify the subject.
 *Remove mention of fan driver receiving data from GPIO as it is no
  longer applicable
v2:
 *Added more detailed subject and patch description
---
 .../bindings/hwmon/hpe,gxp-fan-ctrl.yaml         | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
index 4a52aac6be72..963aa640dc05 100644
--- a/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
+++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
@@ -18,21 +18,12 @@ properties:
     const: hpe,gxp-fan-ctrl
 
   reg:
-    items:
-      - description: Fan controller PWM
-      - description: Programmable logic
-      - description: Function 2
-
-  reg-names:
-    items:
-      - const: base
-      - const: pl
-      - const: fn2
+    description: Fan controller PWM
+    maxItems: 1
 
 required:
   - compatible
   - reg
-  - reg-names
 
 additionalProperties: false
 
@@ -40,6 +31,5 @@ examples:
   - |
     fan-controller@1000c00 {
       compatible = "hpe,gxp-fan-ctrl";
-      reg = <0x1000c00 0x200>, <0xd1000000 0xff>, <0x80200000 0x100000>;
-      reg-names = "base", "pl", "fn2";
+      reg = <0x1000c00 0x200>;
     };
-- 
2.17.1

[PATCH v3 4/5] hwmon: (gxp_fan_ctrl) Provide fan info via gpio

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The fan driver now is independent of the fan plreg GPIO information.
Therefore there will no longer be presence or fail information available
from the driver. Part of the changes includes removing a system power check
as the GPIO driver needs it to report power state to host.

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

---

v3:
 *Removed shared variable
 *Removed GPIO dependency on Kconfig
 *Removed present and failure checks surrounding Fans sysfs
v2:
 *Removed use of shared functions to GPIO in favor of a shared variable
 *Added build dependency on GXP GPIO driver.
---
 drivers/hwmon/Kconfig        |   2 +-
 drivers/hwmon/gxp-fan-ctrl.c | 108 +----------------------------------
 2 files changed, 4 insertions(+), 106 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5b3b76477b0e..196ce88d2db9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -721,7 +721,7 @@ config SENSORS_GXP_FAN_CTRL
 	  If you say yes here you get support for GXP fan control functionality.
 
 	  The GXP controls fan function via the CPLD through the use of PWM
-	  registers. This driver reports status and pwm setting of the fans.
+	  registers. This driver enables pwm setting of the fans.
 
 config SENSORS_HIH6130
 	tristate "Honeywell Humidicon HIH-6130 humidity/temperature sensor"
diff --git a/drivers/hwmon/gxp-fan-ctrl.c b/drivers/hwmon/gxp-fan-ctrl.c
index 0014b8b0fd41..55a10c7fc9d6 100644
--- a/drivers/hwmon/gxp-fan-ctrl.c
+++ b/drivers/hwmon/gxp-fan-ctrl.c
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
 
-#include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/hwmon.h>
 #include <linux/io.h>
@@ -9,52 +8,10 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 
-#define OFS_FAN_INST	0 /* Is 0 because plreg base will be set at INST */
-#define OFS_FAN_FAIL	2 /* Is 2 bytes after base */
-#define OFS_SEVSTAT	0 /* Is 0 because fn2 base will be set at SEVSTAT */
-#define POWER_BIT	24
-
 struct gxp_fan_ctrl_drvdata {
-	void __iomem	*base;
-	void __iomem	*plreg;
-	void __iomem	*fn2;
+	void __iomem *base;
 };
 
-static bool fan_installed(struct device *dev, int fan)
-{
-	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u8 val;
-
-	val = readb(drvdata->plreg + OFS_FAN_INST);
-
-	return !!(val & BIT(fan));
-}
-
-static long fan_failed(struct device *dev, int fan)
-{
-	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u8 val;
-
-	val = readb(drvdata->plreg + OFS_FAN_FAIL);
-
-	return !!(val & BIT(fan));
-}
-
-static long fan_enabled(struct device *dev, int fan)
-{
-	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u32 val;
-
-	/*
-	 * Check the power status as if the platform is off the value
-	 * reported for the PWM will be incorrect. Report fan as
-	 * disabled.
-	 */
-	val = readl(drvdata->fn2 + OFS_SEVSTAT);
-
-	return !!((val & BIT(POWER_BIT)) && fan_installed(dev, fan));
-}
-
 static int gxp_pwm_write(struct device *dev, u32 attr, int channel, long val)
 {
 	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
@@ -81,37 +38,11 @@ static int gxp_fan_ctrl_write(struct device *dev, enum hwmon_sensor_types type,
 	}
 }
 
-static int gxp_fan_read(struct device *dev, u32 attr, int channel, long *val)
-{
-	switch (attr) {
-	case hwmon_fan_enable:
-		*val = fan_enabled(dev, channel);
-		return 0;
-	case hwmon_fan_fault:
-		*val = fan_failed(dev, channel);
-		return 0;
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
 static int gxp_pwm_read(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
-	u32 reg;
 
-	/*
-	 * Check the power status of the platform. If the platform is off
-	 * the value reported for the PWM will be incorrect. In this case
-	 * report a PWM of zero.
-	 */
-
-	reg = readl(drvdata->fn2 + OFS_SEVSTAT);
-
-	if (reg & BIT(POWER_BIT))
-		*val = fan_installed(dev, channel) ? readb(drvdata->base + channel) : 0;
-	else
-		*val = 0;
+	*val = readb(drvdata->base + channel);
 
 	return 0;
 }
@@ -120,8 +51,6 @@ static int gxp_fan_ctrl_read(struct device *dev, enum hwmon_sensor_types type,
 			     u32 attr, int channel, long *val)
 {
 	switch (type) {
-	case hwmon_fan:
-		return gxp_fan_read(dev, attr, channel, val);
 	case hwmon_pwm:
 		return gxp_pwm_read(dev, attr, channel, val);
 	default:
@@ -136,16 +65,6 @@ static umode_t gxp_fan_ctrl_is_visible(const void *_data,
 	umode_t mode = 0;
 
 	switch (type) {
-	case hwmon_fan:
-		switch (attr) {
-		case hwmon_fan_enable:
-		case hwmon_fan_fault:
-			mode = 0444;
-			break;
-		default:
-			break;
-		}
-		break;
 	case hwmon_pwm:
 		switch (attr) {
 		case hwmon_pwm_input:
@@ -169,15 +88,6 @@ static const struct hwmon_ops gxp_fan_ctrl_ops = {
 };
 
 static const struct hwmon_channel_info *gxp_fan_ctrl_info[] = {
-	HWMON_CHANNEL_INFO(fan,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE,
-			   HWMON_F_FAULT | HWMON_F_ENABLE),
 	HWMON_CHANNEL_INFO(pwm,
 			   HWMON_PWM_INPUT,
 			   HWMON_PWM_INPUT,
@@ -212,18 +122,6 @@ static int gxp_fan_ctrl_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(drvdata->base),
 				     "failed to map base\n");
 
-	drvdata->plreg = devm_platform_ioremap_resource_byname(pdev,
-							       "pl");
-	if (IS_ERR(drvdata->plreg))
-		return dev_err_probe(dev, PTR_ERR(drvdata->plreg),
-				     "failed to map plreg\n");
-
-	drvdata->fn2 = devm_platform_ioremap_resource_byname(pdev,
-							     "fn2");
-	if (IS_ERR(drvdata->fn2))
-		return dev_err_probe(dev, PTR_ERR(drvdata->fn2),
-				     "failed to map fn2\n");
-
 	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
 							 "hpe_gxp_fan_ctrl",
 							 drvdata,
-- 
2.17.1

[PATCH v3 5/5] MAINTAINERS: hpe: Add GPIO

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

List the files added for GPIO.

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

---

v3:
 *No change
v2:
 *Removed reference to PSU changes as they have been discarded.
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a3b14ec33830..6157d9466a58 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2239,6 +2239,7 @@ M:	Nick Hawkins <nick.hawkins@hpe.com>
 S:	Maintained
 F:	Documentation/hwmon/gxp-fan-ctrl.rst
 F:	Documentation/devicetree/bindings/arm/hpe,gxp.yaml
+F:	Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
 F:	Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
 F:	Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
 F:	Documentation/devicetree/bindings/spi/hpe,gxp-spifi.yaml
@@ -2247,6 +2248,7 @@ F:	arch/arm/boot/dts/hpe-bmc*
 F:	arch/arm/boot/dts/hpe-gxp*
 F:	arch/arm/mach-hpe/
 F:	drivers/clocksource/timer-gxp.c
+F:	drivers/gpio/gpio-gxp.c
 F:	drivers/hwmon/gxp-fan-ctrl.c
 F:	drivers/i2c/busses/i2c-gxp.c
 F:	drivers/spi/spi-gxp.c
-- 
2.17.1

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[andy.shevchenko]

Mon, Jun 05, 2023 at 08:42:31PM -0500, nick.hawkins@hpe.com kirjoitti:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The GXP SoC supports GPIO on multiple interfaces. The interfaces are
> CPLD and Host. The GPIOs is a combination of both physical and virtual
> I/O across the interfaces. The gpio-gxp driver specifically covers the
> CSM(physical), FN2(virtual), and VUHC(virtual) which are the host. The
> gpio-gxp-pl driver covers the CPLD which takes physical I/O from the
> board and shares it with GXP via a propriety interface that maps the I/O
> onto a specific register area of the GXP. The drivers both support
> interrupts but from different interrupt parents.

Thank you.
This needs some work to be done. See my comments below.

...

> +/* Remember last PSU config */

Would be nice to add why this is a global variable.

> +u8 psu_presence;

...

> +struct gxp_gpio_drvdata {
> +	struct regmap *base;
> +	struct regmap *interrupt;

> +	struct gpio_chip chip;

Making this the first member might save a few bytes of code.

> +	int irq;
> +};

> +static struct regmap *gxp_gpio_init_regmap(struct platform_device *pdev,
> +					   char *reg_name, u8 bits)
> +{
> +	struct regmap_config regmap_config = {
> +		.reg_bits = 32,
> +		.reg_stride = 4,
> +		.val_bits = 32,
> +	};

Move it out and make static const.

> +	void __iomem *base;

> +	if (bits == 8) {
> +		regmap_config.reg_bits = 8;
> +		regmap_config.reg_stride = 1;
> +		regmap_config.val_bits = 8;
> +		regmap_config.max_register = 0xff;
> +	}

Just make two regmap configs and choose them based on the bits.

> +	base = devm_platform_ioremap_resource_byname(pdev, reg_name);
> +	if (IS_ERR(base))
> +		return ERR_CAST(base);
> +
> +	regmap_config.name = reg_name;
> +
> +	return devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);

Why are you not using gpio-regmap?

> +}

...

> +static int gxp_gpio_pl_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
> +	unsigned int val;

> +	int ret = 0;

Unneeded.

> +	switch (offset) {
> +	case IOP_LED1 ... IOP_LED8:
> +		regmap_read(drvdata->base, PLREG_IOP_LED, &val);
> +		ret = (val & BIT(offset)) ? 1 : 0;
> +		break;
> +	case FAN1_INST ...FAN8_INST:
> +		regmap_read(drvdata->base, PLREG_FAN_INST, &val);
> +		ret = (val & BIT((offset - FAN1_INST))) ? 1 : 0;
> +		break;
> +	case FAN1_FAIL ... FAN8_FAIL:
> +		regmap_read(drvdata->base, PLREG_FAN_FAIL, &val);
> +		ret = (val & BIT((offset - FAN1_FAIL))) ? 1 : 0;
> +		break;
> +	case PWR_BTN_INT ... SLP_INT:
> +		regmap_read(drvdata->base, PLREG_INT_GRP5_FLAG, &val);
> +		/* Active low */
> +		ret = (val & BIT((offset - PWR_BTN_INT) + 16)) ? 0 : 1;
> +		break;
> +	case  PSU1_INST ... PSU8_INST:
> +		regmap_read(drvdata->base, PLREG_PSU_INST, &val);
> +		psu_presence = (u8)val;
> +		ret = (psu_presence & BIT((offset - PSU1_INST))) ? 1 : 0;
> +		break;
> +	case PSU1_AC ... PSU8_AC:
> +		regmap_read(drvdata->base, PLREG_PSU_AC, &val);
> +		ret = (val & BIT((offset - PSU1_AC))) ? 1 : 0;
> +		break;
> +	case PSU1_DC ... PSU8_DC:
> +		regmap_read(drvdata->base, PLREG_PSU_DC, &val);
> +		ret = (val & BIT((offset - PSU1_DC))) ? 1 : 0;
> +		break;
> +	default:
> +		break;
> +	}

Obviously what needs to be done in a switch case is to assing offset adjustment
and register name. Then here you read and return the value based on that.

Same approach may be used in other switch-cases in the driver.

> +	return ret;
> +}

...

> +static int gxp_gpio_pl_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +	int ret = GXP_GPIO_DIR_IN;

Useless variable. Return directly.

> +	switch (offset) {
> +	case IOP_LED1 ... IOP_LED8:
> +	case LED_IDENTIFY ... LED_HEALTH_AMBER:
> +	case ACM_FORCE_OFF:
> +	case ACM_REQ_N:
> +		ret = GXP_GPIO_DIR_OUT;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}

...

> +static int gxp_gpio_pl_direction_input(struct gpio_chip *chip,
> +				       unsigned int offset)
> +{
> +	int ret = -EOPNOTSUPP;

Ditto.
Also note, GPIOLIB uses ENOTSUPP.

> +	switch (offset) {
> +	case 8 ... 55:
> +		ret = GXP_GPIO_DIR_OUT;
> +		break;
> +	case 59 ... 65:
> +		ret = GXP_GPIO_DIR_OUT;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}

Same comments as above may be applied to other function implementations in the
driver.

...

> +static irqreturn_t gxp_gpio_pl_irq_handle(int irq, void *_drvdata)
> +{
> +	struct gxp_gpio_drvdata *drvdata = (struct gxp_gpio_drvdata *)_drvdata;

Redundant casting.

> +	unsigned int val, girq, i;
> +
> +	/* Check group 5 interrupts */
> +
> +	regmap_read(drvdata->base, PLREG_INT_GRP5_FLAG, &val);

No error check? So it will spit a spurious interrupts here and there?

> +	if (val) {

Redundant conditional.

> +		for_each_set_bit(i, (unsigned long *)&val, 3) {

This casting is red flag. It has 3 issues. So, no. Just make the copy in the
unsigned long type.

> +			girq = irq_find_mapping(drvdata->chip.irq.domain,
> +						i + PWR_BTN_INT);
> +			generic_handle_irq(girq);

generic_handle_domain_irq()

> +		}
> +
> +		/* Clear latched interrupt */
> +		regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_FLAG,
> +				   0xFF, 0xFF);

GENMASK()

> +		regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_BASE,
> +				   BIT(0) | BIT(2), 0);
> +	}
> +
> +	/* Check group 6 interrupts */
> +
> +	regmap_read(drvdata->base, PLREG_INT_GRP6_FLAG, &val);
> +
> +	if (val & BIT(2)) {
> +		u8 old_psu = psu_presence;
> +
> +		regmap_read(drvdata->base, PLREG_PSU_INST, &val);
> +		psu_presence = (u8)val;
> +
> +		if (old_psu != psu_presence) {
> +			/* Identify all bits which differs */
> +			u8 current_val = psu_presence;
> +			u8 old_val = old_psu;

> +			for (i = 0 ; i < 8 ; i++) {
> +				if ((current_val & 0x1) != (old_val & 0x1)) {

Make them unsigned long, use bitmap_xor() or just ^ followed by
for_each_set_bit().

> +				/* PSU state has changed */
> +					girq = irq_find_mapping(drvdata->chip.irq.domain,
> +								i + PSU1_INST);
> +					if (girq)
> +						generic_handle_irq(girq);

See above.

> +				}

> +				current_val = current_val >> 1;
> +				old_val = old_val >> 1;

No need with the above suggestion.

> +			}
> +		}
> +	}
> +
> +	/* Clear latched interrupt */
> +	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_FLAG,
> +			   0xFF, 0xFF);

GENMASK()

> +	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_BASE,
> +			   BIT(2), 0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct gpio_chip plreg_chip = {

Make it const and mark as a template (in the name).

> +	.label			= "gxp_gpio_plreg",
> +	.owner			= THIS_MODULE,
> +	.get			= gxp_gpio_pl_get,
> +	.set			= gxp_gpio_pl_set,
> +	.get_direction = gxp_gpio_pl_get_direction,
> +	.direction_input = gxp_gpio_pl_direction_input,
> +	.direction_output = gxp_gpio_pl_direction_output,
> +	.base = -1,
> +};
> +
> +static struct irq_chip gxp_plreg_irqchip = {

Make it const and immutable.

> +	.name		= "gxp_plreg",
> +	.irq_ack	= gxp_gpio_pl_irq_ack,
> +	.irq_mask	= gxp_gpio_pl_irq_mask,
> +	.irq_unmask	= gxp_gpio_pl_irq_unmask,
> +	.irq_set_type	= gxp_gpio_pl_set_type,
> +};

...

> +	psu_presence = (u8)val;

Why casting?

...

> +	ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
> +	if (ret < 0)

What is the meaning of positive returned value? Why do we not care about it?

> +		dev_err_probe(&pdev->dev, ret, "Could not register gpiochip for plreg\n");

...

> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "Get irq from platform fail\n");

No need to repeat what the API already messages to the user.

...

> +	ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
> +			       IRQF_SHARED, "gxp-pl", drvdata);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;

	return devm_request_irq(...);

> +}


...

> +static int gxp_gpio_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct gxp_gpio_drvdata *drvdata;
> +
> +	/* Initialize global vars */
> +	psu_presence = 0;

> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, drvdata);

> +	ret = gxp_gpio_pl_init(pdev);
> +
> +	return ret;

	return gxp_...;

But why that function is separate?

> +}

...

> +++ b/drivers/gpio/gpio-gxp.c

Take all above comments and apply here as well.

...

Split this to two patches, one per driver. It makes easier reviews and flexible
handling of the driver maintenance.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/5] dt-bindings: gpio: Add HPE GXP GPIO

[Krzysztof Kozlowski]

On 06/06/2023 03:42, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Provide access to the register regions and interrupt for GPIO. There
> will be two drivers available. The first driver under the hpe,gxp-gpio
> binding will provide GPIO information for the VUHC, CSM, and FN2
> host interfaces. The second driver under the hpe,gxp-gpio-pl will
> provide GPIO information from the CPLD interface. The main difference
> and need for two separate bindings is they have different interrupt
> parents. The other is hpe,gxp-gpio is a combination of physical
> and virtual GPIOs where as hpe,gxp-gpio-pl are all physical
> GPIOs from the CPLD.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

Thank you for your patch. There is something to discuss/improve.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - hpe,gxp-gpio
> +      - hpe,gxp-gpio-pl
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 6

Why 6? You have 4 items in allOf:if:then.

> +
> +  reg-names:
> +    minItems: 2
> +    maxItems: 6
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-line-names:
> +    minItems: 80
> +    maxItems: 300
> +
> +  interrupts:
> +    maxItems: 1
> +

Best regards,
Krzysztof

Re: [PATCH v3 3/5] dt-bindings: hwmon: hpe,gxp-fan-ctrl: remove fn2 and pl registers

[Krzysztof Kozlowski]

On 06/06/2023 03:42, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Reduce the hpe,gxp-fan-ctrl register references from 3 to 1. The
> function2 (fn2) and programmable logic (pl) references are removed.
> The purpose of removal being their functionality will be consumed by a
> new GPIO driver.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 4/5] hwmon: (gxp_fan_ctrl) Provide fan info via gpio

[Guenter Roeck]

On Mon, Jun 05, 2023 at 08:42:33PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The fan driver now is independent of the fan plreg GPIO information.
> Therefore there will no longer be presence or fail information available
> from the driver. Part of the changes includes removing a system power check
> as the GPIO driver needs it to report power state to host.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Let me know if you want me to apply this patch now or if I should wait
for the gpio patches to be accepted.

Guenter

RE: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Hawkins, Nick]

Hi Andy,

Thanks for the feedback. I have a question below:

> > +	base = devm_platform_ioremap_resource_byname(pdev, reg_name);
> > +	if (IS_ERR(base))
> > +		return ERR_CAST(base);
> > +
> > +	regmap_config.name = reg_name;
> > +
> > +	return devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);

> Why are you not using gpio-regmap?

Is there are good example or previous commit you would recommend
looking at that shows how to convert from regmap to gpio-regmap?
Later in the code I am using regmap_read and regmap_update_bits
with large differences in offset registers, and not so much a
contiguous block.

Thank you for the review and assistance!

-Nick Hawkins

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Andy Shevchenko]

On Tue, Jun 6, 2023 at 9:51 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:

...

> > Why are you not using gpio-regmap?
>
> Is there are good example or previous commit you would recommend
> looking at that shows how to convert from regmap to gpio-regmap?
> Later in the code I am using regmap_read and regmap_update_bits
> with large differences in offset registers, and not so much a
> contiguous block.

I don't know how good these are, but that's what we have currently as
most prominent use of gpio-regmap

1) (ongoing) https://lore.kernel.org/linux-gpio/20230606092107.764621-6-jiawenwu@trustnetic.com/
2) (in the repo)
https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-i8255.c
3) (in the repo)
https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-104-idi-48.c

2) & 3) were converted, so you may see by executing respective `git
log -p -- drivers/gpio/...`.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Hawkins, Nick]

> > > Why are you not using gpio-regmap?
> >
> > Is there are good example or previous commit you would recommend
> > looking at that shows how to convert from regmap to gpio-regmap?
> > Later in the code I am using regmap_read and regmap_update_bits
> > with large differences in offset registers, and not so much a
> > contiguous block.


> I don't know how good these are, but that's what we have currently as
> most prominent use of gpio-regmap


> 1) (ongoing) https://lore.kernel.org/linux-gpio/20230606092107.764621-6-jiawenwu@trustnetic.com <mailto:20230606092107.764621-6-jiawenwu@trustnetic.com>/
> 2) (in the repo)
> https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-i8255.c <https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-i8255.c>
> 3) (in the repo)
> https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-104-idi-48.c <https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-104-idi-48.c>


> 2) & 3) were converted, so you may see by executing respective `git
> log -p -- drivers/gpio/...`.

Greetings Andy,

Thank you for those links, I have observed the gpio_regmap code
they have implemented in that case. It appears that the regmap
code is opening the entire range of memory to be read. For my
particular purpose I am not wanting to expose all the 0-0xff byte
range of the GPIOs. In my case is it still necessary to use the
gpio_regmap code?

If gpio_regmap is required, how do I create a direct correlation
between a specific gpio-line and a register offset? For example, in
gpio-gxp-pl.c. Gpio-line at offset 0 (IOPLED) is at register 0x04. The
gpio-line at offset 8 (FAN_INST) is at register 0x27. 

Additionally, is it required to remove gpio_chip if gpio_regmap is
used?

Thank you for the assistance,

-Nick Hawkins

Re: [PATCH v3 4/5] hwmon: (gxp_fan_ctrl) Provide fan info via gpio

[Hawkins, Nick]

> > The fan driver now is independent of the fan plreg GPIO information.
> > Therefore there will no longer be presence or fail information available
> > from the driver. Part of the changes includes removing a system power check
> > as the GPIO driver needs it to report power state to host.
> > 
> > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com <mailto:nick.hawkins@hpe.com>>


> For my reference:


> Reviewed-by: Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>


> Let me know if you want me to apply this patch now or if I should wait
> for the gpio patches to be accepted.

Greetings Guenter,

Please wait until GPIO patches are accepted.

Thanks!

-Nick Hawkins

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Andy Shevchenko]

On Wed, Jun 7, 2023 at 7:07 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:

...

> > > > Why are you not using gpio-regmap?
> > >
> > > Is there are good example or previous commit you would recommend
> > > looking at that shows how to convert from regmap to gpio-regmap?
> > > Later in the code I am using regmap_read and regmap_update_bits
> > > with large differences in offset registers, and not so much a
> > > contiguous block.
>
>
> > I don't know how good these are, but that's what we have currently as
> > most prominent use of gpio-regmap
>
>
> > 1) (ongoing) https://lore.kernel.org/linux-gpio/20230606092107.764621-6-jiawenwu@trustnetic.com <mailto:20230606092107.764621-6-jiawenwu@trustnetic.com>/
> > 2) (in the repo)
> > https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-i8255.c <https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-i8255.c>
> > 3) (in the repo)
> > https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-104-idi-48.c <https://elixir.bootlin.com/linux/v6.4-rc4/source/drivers/gpio/gpio-104-idi-48.c>
>
>
> > 2) & 3) were converted, so you may see by executing respective `git
> > log -p -- drivers/gpio/...`.
>
> Greetings Andy,
>
> Thank you for those links, I have observed the gpio_regmap code
> they have implemented in that case. It appears that the regmap
> code is opening the entire range of memory to be read. For my
> particular purpose I am not wanting to expose all the 0-0xff byte
> range of the GPIOs.

This is also supported by regmap (and regmap has caches for the sparse
registers as well).

> In my case is it still necessary to use the
> gpio_regmap code?

It does care about things the average GPIO controller driver needs to
repeat. So at least you may try and see how it will look.

> If gpio_regmap is required, how do I create a direct correlation
> between a specific gpio-line and a register offset? For example, in
> gpio-gxp-pl.c. Gpio-line at offset 0 (IOPLED) is at register 0x04. The
> gpio-line at offset 8 (FAN_INST) is at register 0x27.

You may remap registers. See, for example, gpio-pca953x, where some of
the registers (with high bit set) are actually virtual rather than
real offsets. Similar idea can be used in your case.

> Additionally, is it required to remove gpio_chip if gpio_regmap is
> used?

I don't think gpio_chip will be needed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Hawkins, Nick]

> It does care about things the average GPIO controller driver needs to
> repeat. So at least you may try and see how it will look.


> > If gpio_regmap is required, how do I create a direct correlation
> > between a specific gpio-line and a register offset? For example, in
> > gpio-gxp-pl.c. Gpio-line at offset 0 (IOPLED) is at register 0x04. The
> > gpio-line at offset 8 (FAN_INST) is at register 0x27.


> You may remap registers. See, for example, gpio-pca953x, where some of
> the registers (with high bit set) are actually virtual rather than
> real offsets. Similar idea can be used in your case.

Greetings Andy,

Is there any documents available describing how regmap_gpio
populates the GPIO lines? Does it automatically go through and add lines
for each successful regmap_read and bits per byte?

I have taken your advice and used the additional readable and writeable
on regmap_config to limit the number of accessible registers.

static const struct regmap_config gxp_regmap_config = {
        .reg_bits = 8,
        .reg_stride = 1,
        .val_bits = 8,
        .readable_reg = gxp_readable_register,
        .writeable_reg = gxp_writeable_register,
        .max_register = 0x80,
        .name = "gxp-gpio-pl",
};

static const struct regmap_config gxp_int_regmap_config = {
        .reg_bits = 8,
        .reg_stride = 1,
        .val_bits = 8,
        .readable_reg = gxp_read_write_int_register,
        .writeable_reg = gxp_read_write_int_register,
        .max_register = 0x7f
        .name = "gxp-gpio-pl-int"
};

Thanks,

-Nick Hawkins

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Andy Shevchenko]

On Wed, Jun 7, 2023 at 11:45 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
>
> > It does care about things the average GPIO controller driver needs to
> > repeat. So at least you may try and see how it will look.

...

> Is there any documents available describing how regmap_gpio
> populates the GPIO lines? Does it automatically go through and add lines
> for each successful regmap_read and bits per byte?

Nope, it assumes one bit per register or something different if xlate
callback is defined. This is my understanding. That said, it might be
that this is a limitation which does not allow you to switch to that
library.

...

> static const struct regmap_config gxp_int_regmap_config = {
>         .reg_bits = 8,

>         .reg_stride = 1,

AFAIU 0 is the same as 1, so this can be dropped.

>         .val_bits = 8,
>         .readable_reg = gxp_read_write_int_register,
>         .writeable_reg = gxp_read_write_int_register,
>         .max_register = 0x7f
>         .name = "gxp-gpio-pl-int"
> };

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Hawkins, Nick]

> > Is there any documents available describing how regmap_gpio
> > populates the GPIO lines? Does it automatically go through and add lines
> > for each successful regmap_read and bits per byte?


> Nope, it assumes one bit per register or something different if xlate
> callback is defined. This is my understanding. That said, it might be
> that this is a limitation which does not allow you to switch to that
> library.


Greetings Andy,

Thank you for this feedback. After exploring the gpio_regmap it seems
it does not fit my needs. Some of the GPIOs are a combination of
several bits in a byte. For instance the Health LED or Identify LED have
more than 2 states. If acceptable I believe the gxp-gpio-pl.c file should
not use the gpio_regmap.

Thanks,

-Nick Hawkins

Re: [PATCH v3 2/5] gpio: gxp: Add HPE GXP GPIO

[Andy Shevchenko]

On Thu, Jun 8, 2023 at 5:58 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
> > > Is there any documents available describing how regmap_gpio
> > > populates the GPIO lines? Does it automatically go through and add lines
> > > for each successful regmap_read and bits per byte?
>
> > Nope, it assumes one bit per register or something different if xlate
> > callback is defined. This is my understanding. That said, it might be
> > that this is a limitation which does not allow you to switch to that
> > library.
>
> Thank you for this feedback. After exploring the gpio_regmap it seems
> it does not fit my needs. Some of the GPIOs are a combination of
> several bits in a byte. For instance the Health LED or Identify LED have
> more than 2 states. If acceptable I believe the gxp-gpio-pl.c file should
> not use the gpio_regmap.

Yes, just mention this reasoning in the cover letter.

-- 
With Best Regards,
Andy Shevchenko