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

[PATCH v5 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 are a combination of both physical and virtual
I/O across the interfaces. The gpio-gxp-pl driver covers the CPLD which
takes physical I/O from the board and shares it with GXP via a proprietary
interface that maps the I/O onto a specific register area of the GXP.
The CPLD interface supports interrupts.

Notes:

Based on previous feedback the gpio-gxp.c driver has been discarded in
favor of it going into a separate larger patchset. This leaves behind
only the gpio-gxp-pl.c driver.

After exploring the recommendation of using regmap_gpio it does not seem
like a good fit. Some of the GPIOs are a combination of several bits in
a byte where others are not contiguous blocks of GPIOs.

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. There was an excellent suggestion to have linux
handle the entire thermal management of the system however the HPE
OpenBMC developers prefer to handle this inside OpenBMC stack instead.

---

Changes since v4:
 *Removed gpio-gxp.c patch as it requires a much larger patchset based
  on feedback.
 *Modified MAINTAINERS Removing gpio-gxp.c and adding missing
  gpio-gxp-pl.c
 *Modified hpe,gxp-gpio.yaml by removing hpe,gxp-gpio compatible
  reference for now in favor of adding it later with separate patchset.
 *Modified cover letter to explain that although there is a suggestion
  to have the kernel handle all thermal matters the HPE OpenBMC developers
  prefer to handle it there instead.
 *Modified cover letter to explain gpio-gxp.c removal

Changes since v3:
 *Added called with debugfs to read server id
 *Added reviewed-by: tags to hwmon fan driver and fan yaml
 *Changed maxItems to be 4 instead of 6 on reg and reg-names in gpio
  yaml
 *Moved gpio-gxp-pl.c to be in a separate patch/commit.
 *Moved regmap_config out of function in both gpio drivers to turn into
  static const
 *Removed unnecesary variables and redundant conditionals
 *Modified regmap_read switch statements to calculate offset and mask
  then read at end
 *Removed use of -EOPNOTSUPP in favor of -ENOTSUPP
 *Removed redundant casting
 *Switched generic_handle_irq for generic_handle_domain_irq
 *Used GENMASK where applicable
 *Used bitmap_xor and for_each_bit_set in PL PSU interrupt
 *Made GPIO chip const and marked as a template (in the name)
 *Made irq_chip const and immutable
 *Corrected check on devm_gpiochip_add_data
 *Removed dev_err_probe on platform_get_irq
 *Changed return 0 to devm_request_irq

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 PL
  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           |  71 +++
 .../bindings/hwmon/hpe,gxp-fan-ctrl.yaml      |  16 +-
 MAINTAINERS                                   |   2 +
 drivers/gpio/Kconfig                          |   9 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-gxp-pl.c                    | 582 ++++++++++++++++++
 drivers/hwmon/Kconfig                         |   2 +-
 drivers/hwmon/gxp-fan-ctrl.c                  | 106 +---
 8 files changed, 671 insertions(+), 118 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
 create mode 100644 drivers/gpio/gpio-gxp-pl.c

-- 
2.17.1

[PATCH v5 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. 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

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", "", "", "", "",
+          "", "", "", "", "", "", "", "", "", "";
+        };
-- 
2.17.1

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

[nick.hawkins]

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

The GXP SoC supports GPIO on multiple interfaces. The interfaces are
CPLD and Host. The gpio-gxp-pl driver covers the CPLD which takes
physical I/O from the board and shares it with GXP via a proprietary
interface that maps the I/O onto a specific register area of the GXP.
This driver supports interrupts from the CPLD.

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

---
v5:
 *No change
v4:
 *Moved gpio-gxp-pl.c to a separate commit.
 *Moved regmap_config out of function and made it static const
 *Removed unnecessary variables
 *Removed redundant conditional
 *Modified regmap_read switch statements to calculate offset and mask
  then read at end.
 *Removed use of -EOPNOTSUPP
 *Removed redundant casting
 *Switched generic_handle_irq -> generic_handle_domain_irq
 *Used GENMASK where applicable
 *Used bitmap_xor and for_each_bit_set
 *Made GPIO chip const and marked as a template (in the name)
 *Made irq_chip const and immutable
 *Removed casting in one case
 *Corrected check on devm_gpiochip_add_data
 *Remove dev_err_probe on platform_get_irq
 *Changed return 0 to devm_request_irq
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       |   9 +
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-gxp-pl.c | 582 +++++++++++++++++++++++++++++++++++++
 3 files changed, 592 insertions(+)
 create mode 100644 drivers/gpio/gpio-gxp-pl.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5521f060d58e..2e0b003ccefb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1263,6 +1263,15 @@ config HTC_EGPIO
 	  several HTC phones.  It provides basic support for input
 	  pins, output pins, and IRQs.
 
+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_ELKHARTLAKE
 	tristate "Intel Elkhart Lake PSE GPIO support"
 	depends on X86 || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 20036af3acb1..8903f446ef1b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_GPIO_FXL6408)		+= gpio-fxl6408.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_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..8506e2a96da4
--- /dev/null
+++ b/drivers/gpio/gpio-gxp-pl.c
@@ -0,0 +1,582 @@
+// 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_GRP5_STATE	0x32
+#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_SERVER_ID		0x01 /* 2 Bytes */
+#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 PLREG_SIDEBAND		0x40
+#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,
+	SIDEBAND_SEL_L = 63,
+	SIDEBAND_SEL_H = 64
+};
+
+/*
+ * When an interrupt fires for a PSU config change
+ * there is a need to know the previous PSU configuration
+ * so that the appropriate gpio line is interrupted for
+ * the correct PSU. In order to keep this variable up to
+ * date it is global so that it can be set at init and
+ * each time the interrupt fires.
+ */
+u8 psu_presence;
+
+struct gxp_gpio_drvdata {
+	struct gpio_chip chip;
+	struct regmap *base;
+	struct regmap *interrupt;
+	int irq;
+};
+
+static const struct regmap_config gxp_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x80,
+	.name = "gxp-gpio-pl",
+};
+
+static const struct regmap_config gxp_int_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x7f,
+	.name = "gxp-gpio-pl-int",
+};
+
+static struct regmap *gxp_gpio_init_regmap(struct platform_device *pdev,
+					   char *reg_name, bool is_interrupt)
+{
+	void __iomem *base;
+
+	base = devm_platform_ioremap_resource_byname(pdev, reg_name);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	if (is_interrupt)
+		return devm_regmap_init_mmio(&pdev->dev, base, &gxp_int_regmap_config);
+	else
+		return devm_regmap_init_mmio(&pdev->dev, base, &gxp_regmap_config);
+}
+
+#ifdef CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
+
+static int gxp_gpio_serverid_show(struct seq_file *s, void *unused)
+{
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(s->private);
+	unsigned int server_id_l;
+	unsigned int server_id_h;
+
+	regmap_read(drvdata->base, PLREG_SERVER_ID, &server_id_l);
+	regmap_read(drvdata->base, PLREG_SERVER_ID + 0x01, &server_id_h);
+
+	seq_printf(s, "%02x %02x", server_id_h, server_id_l);
+
+	return 0;
+}
+
+static void gxp_gpio_debuginit(struct platform_device *pdev)
+{
+	debugfs_create_devm_seqfile(&pdev->dev, "gxp_gpio_serverid", NULL,
+				    gxp_gpio_serverid_show);
+}
+
+#else
+
+static inline void gxp_gpio_debuginit(struct platform_device *pdev)
+{
+}
+
+#endif
+
+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;
+	unsigned int reg_offset;
+	u8 reg_mask;
+	bool is_active_low = false;
+
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+		reg_offset = PLREG_IOP_LED;
+		reg_mask = BIT(offset);
+		break;
+	case FAN1_INST ...FAN8_INST:
+		regmap_read(drvdata->base, PLREG_FAN_INST, &val);
+		reg_mask = BIT(offset - FAN1_INST);
+		break;
+	case FAN1_FAIL ... FAN8_FAIL:
+		regmap_read(drvdata->base, PLREG_FAN_FAIL, &val);
+		reg_mask = BIT(offset - FAN1_FAIL);
+		break;
+	case PWR_BTN_INT ... SLP_INT:
+		/* Note this is active low */
+		reg_offset = PLREG_INT_GRP5_STATE;
+		reg_mask = BIT(offset - PWR_BTN_INT);
+		is_active_low = true;
+		break;
+	case  PSU1_INST ... PSU8_INST:
+		reg_offset = PLREG_PSU_INST;
+		reg_mask = BIT(offset - PSU1_INST);
+		break;
+	case PSU1_AC ... PSU8_AC:
+		reg_offset = PLREG_PSU_AC;
+		reg_mask = BIT(offset - PSU1_AC);
+		break;
+	case PSU1_DC ... PSU8_DC:
+		reg_offset = PLREG_PSU_DC;
+		reg_mask = BIT(offset - PSU1_DC);
+		break;
+	case LED_IDENTIFY:
+		reg_offset = PLREG_IDENT_LED;
+		reg_mask = BIT(1);
+		break;
+	case LED_HEALTH_RED:
+		reg_offset = PLREG_HEALTH_LED;
+		reg_mask = GENMASK(5, 4); /* Bit 5 set, bit 4 clear */
+		break;
+	case LED_HEALTH_AMBER:
+		reg_offset = PLREG_HEALTH_LED;
+		reg_mask = GENMASK(5, 4); /* Bit 5, bit 4 set */
+		break;
+	case SIDEBAND_SEL_L:
+		reg_offset = PLREG_SIDEBAND;
+		reg_mask = BIT(0);
+		break;
+	case SIDEBAND_SEL_H:
+		reg_offset = PLREG_SIDEBAND;
+		reg_mask = BIT(1);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	regmap_read(drvdata->base, reg_offset, &val);
+
+	/* Special case: Check two bits for Health LED */
+	if (offset == LED_HEALTH_RED)
+		/* Bit 5 set, bit 4 not set */
+		return ((val & reg_mask) == BIT(5) ? 1 : 0);
+	else if (offset == LED_HEALTH_AMBER)
+		/* Bit 5 and 4 set */
+		return ((val & reg_mask) == reg_mask ? 1 : 0);
+
+	val = val & reg_mask;
+
+	if (is_active_low)
+		return (val ? 0 : 1);
+	else
+		return (val ? 1 : 0);
+}
+
+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,
+				   GENMASK(7, 6),
+				   value == 0 ? BIT(7) : GENMASK(7, 6));
+		break;
+	case LED_HEALTH_RED:
+		regmap_update_bits(drvdata->base,
+				   PLREG_HEALTH_LED,
+				   GENMASK(7, 6),
+				   value == 0 ? 0 : BIT(7));
+		break;
+	case LED_HEALTH_AMBER:
+		regmap_update_bits(drvdata->base,
+				   PLREG_HEALTH_LED,
+				   GENMASK(7, 6),
+				   value == 0 ? 0 : BIT(6));
+		break;
+	case SIDEBAND_SEL_L ... SIDEBAND_SEL_H:
+		regmap_update_bits(drvdata->base,
+				   PLREG_SIDEBAND,
+				   BIT(offset - SIDEBAND_SEL_L),
+				   value == 0 ? 0 : BIT(offset - SIDEBAND_SEL_L));
+		break;
+	default:
+		break;
+	}
+}
+
+static int gxp_gpio_pl_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+	case LED_IDENTIFY ... LED_HEALTH_AMBER:
+	case ACM_FORCE_OFF:
+	case ACM_REQ_N:
+	case SIDEBAND_SEL_L ... SIDEBAND_SEL_H:
+		return GXP_GPIO_DIR_OUT;
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+static int gxp_gpio_pl_direction_input(struct gpio_chip *chip,
+				       unsigned int offset)
+{
+	switch (offset) {
+	case FAN1_INST ... FAN8_FAIL:
+		return GXP_GPIO_DIR_OUT;
+	case PWR_BTN_INT ... SLP_INT:
+		return GXP_GPIO_DIR_OUT;
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+static int gxp_gpio_pl_direction_output(struct gpio_chip *chip,
+					unsigned int offset, int value)
+{
+	switch (offset) {
+	case IOP_LED1 ... IOP_LED8:
+	case LED_IDENTIFY ... LED_HEALTH_AMBER:
+	case ACM_FORCE_OFF:
+	case ACM_REQ_N:
+	case SIDEBAND_SEL_L ... SIDEBAND_SEL_H:
+		gxp_gpio_pl_set(chip, offset, value);
+		return GXP_GPIO_DIR_OUT;
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+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 = _drvdata;
+	unsigned int val, i;
+	unsigned long temp;
+
+	/* Check group 5 interrupts */
+
+	regmap_read(drvdata->base, PLREG_INT_GRP5_FLAG, &val);
+
+	temp = (unsigned long)val;
+	for_each_set_bit(i, &temp, 3) {
+		generic_handle_domain_irq(drvdata->chip.irq.domain,
+					  i + PWR_BTN_INT);
+	}
+
+		/* Clear latched interrupt */
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP5_FLAG,
+			   GENMASK(7, 0), GENMASK(7, 0));
+	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 = val;
+
+		if (old_psu != psu_presence) {
+			/* Identify all bits which differs */
+			unsigned long current_val = psu_presence;
+			unsigned long old_val = old_psu;
+			unsigned long changed_bits;
+
+			bitmap_xor(&changed_bits, &current_val, &old_val, 8);
+
+			for_each_set_bit(i, &changed_bits, 8) {
+				/* PSU state has changed */
+				generic_handle_domain_irq(drvdata->chip.irq.domain,
+							  i + PSU1_INST);
+			}
+		}
+	}
+
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_FLAG,
+			   GENMASK(7, 0), GENMASK(7, 0));
+	regmap_update_bits(drvdata->interrupt, PLREG_INT_GRP6_BASE,
+			   BIT(2), 0);
+
+	return IRQ_HANDLED;
+}
+
+static const struct gpio_chip template_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 const 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,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+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;
+	struct gpio_irq_chip *girq;
+	unsigned int val;
+
+	/* Initialize global vars */
+	psu_presence = 0;
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, drvdata);
+
+	drvdata->base = gxp_gpio_init_regmap(pdev, "base", false);
+	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", true);
+	if (IS_ERR(drvdata->interrupt))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->interrupt),
+				     "failed to map interrupt base\n");
+
+	/* Necessary to read the server id */
+	gxp_gpio_debuginit(pdev);
+
+	/* Initialize psu_presence variable */
+	regmap_read(drvdata->base, PLREG_PSU_INST, &val);
+	psu_presence = val;
+
+	drvdata->chip = template_chip;
+	drvdata->chip.ngpio = 80;
+	drvdata->chip.parent = &pdev->dev;
+
+	girq = &drvdata->chip.irq;
+	gpio_irq_chip_set_chip(girq, &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)
+		return dev_err_probe(&pdev->dev, ret, "Could not register gpiochip for plreg\n");
+
+	regmap_update_bits(drvdata->interrupt,
+			   PLREG_INT_HI_PRI_EN,
+			   PLREG_GRP4_GRP5_MASK,
+			   PLREG_GRP4_GRP5_MASK);
+	regmap_update_bits(drvdata->interrupt,
+			   PLREG_INT_GRP_STAT_MASK,
+			   PLREG_GRP4_GRP5_MASK,
+			   0x00);
+
+	regmap_read(drvdata->interrupt, PLREG_INT_HI_PRI_EN, &val);
+	regmap_read(drvdata->interrupt, PLREG_INT_GRP_STAT_MASK, &val);
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		return ret;
+
+	drvdata->irq = ret;
+
+	return devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
+				IRQF_SHARED, "gxp-pl", drvdata);
+}
+
+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");
-- 
2.17.1

[PATCH v5 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>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---
v5:
 *No change
v4:
 *No change, added reviewed by
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 v5 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>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

---

v5:
 *No change
v4:
 *No change, Added Reviewed-by:
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 | 106 +----------------------------------
 2 files changed, 3 insertions(+), 105 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index fc640201a2de..599162c36a53 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 2e05bc2f627a..cd23ea596415 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;
 };
 
-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 * const 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 v5 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>

---

v5:
 *Remove gpio-gxp.c reference as it has been discarded for separate
  commit
 *Added missing gpio-gxp-pl.c reference
v4:
 *No change
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 27ef11624748..559d4ecb65e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2241,6 +2241,7 @@ M:	Jean-Marie Verdun <verdun@hpe.com>
 M:	Nick Hawkins <nick.hawkins@hpe.com>
 S:	Maintained
 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
@@ -2250,6 +2251,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-pl.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 v5 1/5] dt-bindings: gpio: Add HPE GXP GPIO

[Krzysztof Kozlowski]

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

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

[Hawkins, Nick]

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

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

[Rob Herring]

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

Re: [PATCH v5 0/5] ARM: Add GPIO support

[Linus Walleij]

On Wed, Jul 5, 2023 at 9:49 PM <nick.hawkins@hpe.com> wrote:

> From: Nick Hawkins <nick.hawkins@hpe.com>
>
> The GXP SoC supports GPIO on multiple interfaces. The interfaces are
> CPLD and Host. The GPIOs are a combination of both physical and virtual
> I/O across the interfaces. The gpio-gxp-pl driver covers the CPLD which
> takes physical I/O from the board and shares it with GXP via a proprietary
> interface that maps the I/O onto a specific register area of the GXP.
> The CPLD interface supports interrupts.
>
> Notes:
>
> Based on previous feedback the gpio-gxp.c driver has been discarded in
> favor of it going into a separate larger patchset. This leaves behind
> only the gpio-gxp-pl.c driver.

The kernel certainly looks better after this change than before, so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

[Bartosz Golaszewski]

On Wed, Jul 5, 2023 at 9:49 PM <nick.hawkins@hpe.com> wrote:
>
> From: Nick Hawkins <nick.hawkins@hpe.com>
>
> The GXP SoC supports GPIO on multiple interfaces. The interfaces are
> CPLD and Host. The gpio-gxp-pl driver covers the CPLD which takes
> physical I/O from the board and shares it with GXP via a proprietary
> interface that maps the I/O onto a specific register area of the GXP.
> This driver supports interrupts from the CPLD.
>
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
>

[snip]

> +
> +/*
> + * When an interrupt fires for a PSU config change
> + * there is a need to know the previous PSU configuration
> + * so that the appropriate gpio line is interrupted for
> + * the correct PSU. In order to keep this variable up to
> + * date it is global so that it can be set at init and
> + * each time the interrupt fires.
> + */
> +u8 psu_presence;

I'm not buying it. There's no user of this variable outside of this
compilation unit, is there? If there was - the name should have some
prefix but even then, I don't see a need for this to be global. Why
don't you put it in struct gxp_gpio_drvdata?

Bart

[snip]

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

[Hawkins, Nick]

> > +
> > +/*
> > + * When an interrupt fires for a PSU config change
> > + * there is a need to know the previous PSU configuration
> > + * so that the appropriate gpio line is interrupted for
> > + * the correct PSU. In order to keep this variable up to
> > + * date it is global so that it can be set at init and
> > + * each time the interrupt fires.
> > + */
> > +u8 psu_presence;

> I'm not buying it. There's no user of this variable outside of this
> compilation unit, is there? If there was - the name should have some
> prefix but even then, I don't see a need for this to be global. Why
> don't you put it in struct gxp_gpio_drvdata?

Hi Bart,

You are correct this should not be global. It will be placed in 
gxp_gpio_drvdata.

Thank you for catching this and the assistance,

-Nick Hawkins