From: Nick Hawkins <[email protected]>
The GXP has multiple interfaces that provide I/O to it. There is GPIO
coming from the host and from the cpld. Both of these interfaces are
interruptable.
The GXP is able to monitor PSU's via I2C. There is support for up to 8
PSUs. The GXP gets presence information from I/O with the cpld.
The fan controller and the psu monitor consume I/O from the CPLD.
Thus for the GXP to be able to report this GPIO to the OpenBMC stack
calls have been enabled for the GPIO driver to use.
Nick Hawkins (9):
gpio: gxp: Add HPE GXP GPIO
hwmon: (gxp_fan_ctrl) Give GPIO access to fan data
hwmon: (gxp-psu) Add driver to read HPE PSUs
dt-bindings: hwmon: Modify hpe,gxp-fan-ctrl
dt-bindings: gpio: Add HPE GXP GPIO
dt-bindings: hwmon: Add HPE GXP PSU Support
ARM: dts: gxp: add psu, i2c, gpio
ARM: multi_v7_defconfig: Add PSU, GPIO, and I2C
MAINTAINERS: hpe: Add GPIO, PSU
.../bindings/gpio/hpe,gxp-gpio.yaml | 137 +++
.../bindings/hwmon/hpe,gxp-fan-ctrl.yaml | 6 +-
.../bindings/hwmon/hpe,gxp-psu.yaml | 45 +
MAINTAINERS | 4 +
arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 161 +++
arch/arm/boot/dts/hpe-gxp.dtsi | 197 ++-
arch/arm/configs/multi_v7_defconfig | 5 +-
drivers/gpio/Kconfig | 9 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-gxp.c | 1056 +++++++++++++++++
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/gxp-fan-ctrl.c | 58 +-
drivers/hwmon/gxp-psu.c | 773 ++++++++++++
14 files changed, 2404 insertions(+), 59 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml
create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
create mode 100644 drivers/gpio/gpio-gxp.c
create mode 100644 drivers/hwmon/gxp-psu.c
--
2.17.1
From: Nick Hawkins <[email protected]>
Add support for the GXP I2C, PSU, and GPIO
drivers. As well as adjust register ranges to be
correct.
Signed-off-by: Nick Hawkins <[email protected]>
---
arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 161 ++++++++++++++++++
arch/arm/boot/dts/hpe-gxp.dtsi | 197 ++++++++++++++++++++---
2 files changed, 338 insertions(+), 20 deletions(-)
diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
index 3a7382ce40ef..487b6485a832 100644
--- a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
+++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
@@ -23,4 +23,165 @@
device_type = "memory";
reg = <0x40000000 0x20000000>;
};
+
+ i2cmux@4 {
+ compatible = "i2c-mux-reg";
+ i2c-parent = <&i2c4>;
+ reg = <0xd1000374 0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@3 {
+ reg = <3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@4 {
+ reg = <4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ i2cmux@6 {
+ compatible = "i2c-mux-reg";
+ i2c-parent = <&i2c6>;
+ reg = <0xd1000376 0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@3 {
+ reg = <3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@4 {
+ reg = <4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@5 {
+ reg = <5>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&i2c1 {
+ status = "okay";
+};
+
+&i2c2 {
+ status = "okay";
+ eeprom@50 {
+ compatible = "atmel,24c02";
+ pagesize = <8>;
+ reg = <0x50>;
+ };
+};
+
+&i2c3 {
+ status = "okay";
+};
+
+&i2c4 {
+ status = "okay";
+};
+
+&i2c5 {
+ status = "okay";
+};
+
+&i2c6 {
+ status = "okay";
+};
+
+&i2c7 {
+ status = "okay";
+ psu@58 {
+ compatible = "hpe,gxp-psu";
+ reg = <0x58>;
+ hpe,sysreg = <&sysreg_system_controller2>;
+ };
+
+ psu@59 {
+ compatible = "hpe,gxp-psu";
+ reg = <0x59>;
+ hpe,sysreg = <&sysreg_system_controller2>;
+ };
+};
+
+&i2c8 {
+ status = "okay";
+};
+
+&i2c9 {
+ status = "okay";
+};
+
+&gpio {
+ gpio-line-names =
+ "", "", "", "", "", "", "", "", "", "", /*0 - 9*/
+ "", "", "", "", "", "", "", "", "", "", /*10 - 19*/
+ "", "", "", "", "", "", "", "", "", "", /*20 - 29*/
+ "", "", "", "", "", "", "", "", "", "", /*30 - 39*/
+ "", "", "", "", "", "", "", "", "", "", /*40 - 49*/
+ "", "", "", "", "", "", "", "", "", "", /*50 - 59*/
+ "", "", "", "", "", "", "", "", "", "", /*60 - 69*/
+ "", "", "", "", "", "", "", "", "", "", /*70 - 79*/
+ "", "", "", "", "", "", "", "", "", "", /*80 - 89*/
+ "", "", "", "", "", "", "", "", "", "", /*90 - 99*/
+ "", "", "", "", "", "", "", "", "", "", /*100 - 109*/
+ "", "", "", "", "", "", "", "", "", "", /*110 - 119*/
+ "", "", "", "", "", "", "", "", "", "", /*120 - 129*/
+ "", "", "", "", "", "", "", "", "", "", /*130 - 139*/
+ "", "", "", "", "", "", "", "", "", "", /*140 - 149*/
+ "", "", "", "", "", "", "", "", "", "", /*150 - 159*/
+ "", "", "", "", "", "", "", "", "", "", /*160 - 169*/
+ "", "", "", "", "", "", "", "", "", "", /*170 - 179*/
+ "", "", "", "", "", "", "", "", "", "", /*180 - 189*/
+ "", "", "RESET_OUT", "NMI_OUT", "", "", "", "", "", "", /*190 - 199*//*GPIO*/
+ "", "", "", "", "", "", "", "", "", "", /*Vuhc 200-209*/
+ "POWER_OUT", "PS_PWROK", "PCIERST", "POST_COMPLETE", "", "", "", "", "", ""; /*210 - 219*/
+};
+
+&gpio1 {
+ 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", "", "", "", "",
+ "", "", "", "", "", "", "", "", "", "";
};
diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
index cf735b3c4f35..8a8faf7fbd60 100644
--- a/arch/arm/boot/dts/hpe-gxp.dtsi
+++ b/arch/arm/boot/dts/hpe-gxp.dtsi
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Device Tree file for HPE GXP
+ * Device Tree for HPE GXP
*/
/dts-v1/;
@@ -52,76 +52,233 @@
cache-level = <2>;
};
- ahb@c0000000 {
+ ahb@80000000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
- ranges = <0x0 0xc0000000 0x30000000>;
- dma-ranges;
+ ranges = <0x0 0x80000000 0x20000000>,
+ <0x40000000 0xc0000000 0x3fff0000>;
- vic0: interrupt-controller@eff0000 {
+ vic0: interrupt-controller@4eff0000 {
compatible = "arm,pl192-vic";
- reg = <0xeff0000 0x1000>;
+ reg = <0x4eff0000 0x1000>;
interrupt-controller;
#interrupt-cells = <1>;
};
- vic1: interrupt-controller@80f00000 {
+ vic1: interrupt-controller@f00000 {
compatible = "arm,pl192-vic";
- reg = <0x80f00000 0x1000>;
+ reg = <0xf00000 0x1000>;
interrupt-controller;
#interrupt-cells = <1>;
};
- uarta: serial@e0 {
+ uarta: serial@400000e0 {
compatible = "ns16550a";
- reg = <0xe0 0x8>;
+ reg = <0x400000e0 0x8>;
interrupts = <17>;
interrupt-parent = <&vic0>;
clock-frequency = <1846153>;
reg-shift = <0>;
};
- uartb: serial@e8 {
+ uartb: serial@400000e8 {
compatible = "ns16550a";
- reg = <0xe8 0x8>;
+ reg = <0x400000e8 0x8>;
interrupts = <18>;
interrupt-parent = <&vic0>;
clock-frequency = <1846153>;
reg-shift = <0>;
};
- uartc: serial@f0 {
+ uartc: serial@400000f0 {
compatible = "ns16550a";
- reg = <0xf0 0x8>;
+ reg = <0x400000f0 0x8>;
interrupts = <19>;
interrupt-parent = <&vic0>;
clock-frequency = <1846153>;
reg-shift = <0>;
};
- usb0: usb@efe0000 {
+ usb0: usb@4efe0000 {
compatible = "hpe,gxp-ehci", "generic-ehci";
- reg = <0xefe0000 0x100>;
+ reg = <0x4efe0000 0x100>;
interrupts = <7>;
interrupt-parent = <&vic0>;
};
- st: timer@80 {
+ st: timer@40000080 {
compatible = "hpe,gxp-timer";
- reg = <0x80 0x16>;
+ reg = <0x40000080 0x16>;
interrupts = <0>;
interrupt-parent = <&vic0>;
clocks = <&iopclk>;
clock-names = "iop";
};
- usb1: usb@efe0100 {
+ usb1: usb@4efe0100 {
compatible = "hpe,gxp-ohci", "generic-ohci";
- reg = <0xefe0100 0x110>;
+ reg = <0x4efe0100 0x110>;
interrupts = <6>;
interrupt-parent = <&vic0>;
};
+
+ sysreg_system_controller: syscon@400000f8 {
+ compatible = "hpe,gxp-sysreg", "syscon";
+ reg = <0x400000f8 0x8>;
+ };
+
+ sysreg_system_controller2: syscon@51000319 {
+ compatible = "hpe,gxp-sysreg", "syscon";
+ reg = <0x51000319 0x4>;
+ };
+
+ i2c0: i2c@40002000 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002000 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c1: i2c@40002100 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002100 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c2: i2c@40002200 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002200 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c3: i2c@40002300 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002300 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c4: i2c@40002400 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002400 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c5: i2c@40002500 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002500 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c6: i2c@40002600 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002600 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c7: i2c@40002700 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002700 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c8: i2c@40002800 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002800 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ i2c9: i2c@40002900 {
+ compatible = "hpe,gxp-i2c";
+ reg = <0x40002900 0x70>;
+ interrupts = <9>;
+ interrupt-parent = <&vic0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ hpe,sysreg = <&sysreg_system_controller>;
+ clock-frequency = <100000>;
+ };
+
+ fan-controller@40000c10 { /* 0xc0000c10 */
+ compatible = "hpe,gxp-fan-ctrl";
+ reg = <0x40000c10 0x8>, <0x51000327 0x06>;
+ reg-names = "base", "pl";
+ };
+
+ gpio: gpio@0 {
+ compatible = "hpe,gxp-gpio";
+ reg = <0x0 0x400>, <0x200046 0x1>, <0x200070 0x08>,
+ <0x400064 0x80>, <0x5100030f 0x1>;
+ reg-names = "csm", "fn2-vbtn", "fn2-stat", "vuhc", "vbtn";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupts = <0>;
+ interrupt-parent = <&vic1>;
+ };
+
+ gpio1: gpio@51000304 {
+ compatible = "hpe,gxp-gpio-pl";
+ reg = <0x51000304 0x2>, <0x5100030d 0x01>, <0x51000380 0x7f>;
+ reg-names = "pl-led", "pl-health", "pl-int";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupts = <26>;
+ interrupt-parent = <&vic0>;
+ };
};
};
};
--
2.17.1
From: Nick Hawkins <[email protected]>
List the files added for GPIO and PSU support.
Signed-off-by: Nick Hawkins <[email protected]>
---
MAINTAINERS | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index a3b14ec33830..6df959ebf523 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2239,7 +2239,9 @@ M: Nick Hawkins <[email protected]>
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/hwmon/hpe,gxp-psu.yaml
F: Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
F: Documentation/devicetree/bindings/spi/hpe,gxp-spifi.yaml
F: Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
@@ -2247,7 +2249,9 @@ 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/hwmon/gxp-psu.c
F: drivers/i2c/busses/i2c-gxp.c
F: drivers/spi/spi-gxp.c
F: drivers/watchdog/gxp-wdt.c
--
2.17.1
From: Nick Hawkins <[email protected]>
The GXP SoC supports GPIO on multiple interfaces: Host, CPLD and Soc
pins. The interface from CPLD and Host are interruptable from vic0
and vic1. This driver allows support for both of these interfaces
through the use of different compatible bindings.
Signed-off-by: Nick Hawkins <[email protected]>
---
drivers/gpio/Kconfig | 9 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-gxp.c | 1056 +++++++++++++++++++++++++++++++++++++++
3 files changed, 1066 insertions(+)
create mode 100644 drivers/gpio/gpio-gxp.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 13be729710f2..47435307698c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1235,6 +1235,15 @@ 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
+ 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_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..a7ce0ab097aa 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -63,6 +63,7 @@ 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_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.c b/drivers/gpio/gpio-gxp.c
new file mode 100644
index 000000000000..86f69174434d
--- /dev/null
+++ b/drivers/gpio/gpio-gxp.c
@@ -0,0 +1,1056 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define GPIDATL 0x40
+#define GPIDATH 0x60
+#define GPODATL 0xb0
+#define GPODATH 0xb4
+#define GPODAT2L 0xf8
+#define GPODAT2H 0xfc
+#define GPOOWNL 0x110
+#define GPOOWNH 0x114
+#define GPOOWN2L 0x118
+#define GPOOWN2H 0x11c
+
+#define GPIO_DIR_OUT 0
+#define GPIO_DIR_IN 1
+
+#define PGOOD_MASK 1
+
+#define PLREG_INT_GRP_STAT_MASK 0x8
+#define PLREG_INT_HI_PRI_EN 0xC
+#define PLREG_INT_GRP5_BASE 0x31
+#define PLREG_INT_GRP6_BASE 0x35
+#define PLREG_INT_GRP5_FLAG 0x30
+#define PLREG_INT_GRP6_FLAG 0x34
+#define PLREG_INT_GRP5_PIN_BASE 59
+#define PLREG_INT_GRP6_PIN_BASE 90
+
+enum pl_gpio_pn {
+ IOP_LED1 = 0,
+ IOP_LED2,
+ IOP_LED3,
+ IOP_LED4,
+ IOP_LED5,
+ IOP_LED6,
+ IOP_LED7,
+ IOP_LED8,
+ FAN1_INST = 8,
+ 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,
+ LED_IDENTIFY = 56,
+ LED_HEALTH_RED,
+ LED_HEALTH_AMBER,
+ PWR_BTN_INT = 59,
+ UID_PRESS_INT,
+ SLP_INT,
+ ACM_FORCE_OFF = 70,
+ ACM_REMOVED,
+ ACM_REQ_N,
+ 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
+};
+
+enum plreg_gpio_pn {
+ RESET = 192,
+ NMI_OUT = 193,
+ VPBTN = 210,
+ PGOOD,
+ PERST,
+ POST_COMPLETE,
+};
+
+struct gxp_gpio_drvdata {
+ struct regmap *csm_map;
+ void __iomem *fn2_vbtn;
+ struct regmap *fn2_stat;
+ struct regmap *vuhc0_map;
+ void __iomem *vbtn;
+ struct regmap *pl_led;
+ struct regmap *pl_health;
+ struct regmap *pl_int;
+ struct gpio_chip chip;
+ int irq;
+};
+
+extern u8 get_psu_inst(void);
+extern u8 get_psu_ac(void);
+extern u8 get_psu_dc(void);
+extern u8 get_fans_installed(void);
+extern u8 get_fans_failed(void);
+
+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, GPIDATL, &ret);
+ ret = (ret & BIT(offset)) ? 1 : 0;
+ break;
+ case 32 ... 63:
+ regmap_read(drvdata->csm_map, GPIDATH, &ret);
+ ret = (ret & BIT(offset - 32)) ? 1 : 0;
+ break;
+ case 64 ... 95:
+ regmap_read(drvdata->csm_map, GPODATL, &ret);
+ ret = (ret & BIT(offset - 64)) ? 1 : 0;
+ break;
+ case 96 ... 127:
+ regmap_read(drvdata->csm_map, GPODATH, &ret);
+ ret = (ret & BIT(offset - 96)) ? 1 : 0;
+ break;
+ case 128 ... 159:
+ regmap_read(drvdata->csm_map, GPODAT2L, &ret);
+ ret = (ret & BIT(offset - 128)) ? 1 : 0;
+ break;
+ case 160 ... 191:
+ regmap_read(drvdata->csm_map, GPODAT2H, &ret);
+ ret = (ret & BIT(offset - 160)) ? 1 : 0;
+ break;
+ case 192:
+ /* SW_RESET */
+ regmap_read(drvdata->csm_map, 0x5C, &ret);
+ ret = (ret & BIT(15)) ? 1 : 0;
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+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, GPOOWNL, &tmp);
+ tmp = (tmp & BIT(offset - 64)) ? 1 : 0;
+
+ regmap_update_bits(drvdata->csm_map, GPOOWNL,
+ BIT(offset - 64), BIT(offset - 64));
+ regmap_update_bits(drvdata->csm_map, GPODATL,
+ BIT(offset - 64), value ? BIT(offset - 64) : 0);
+
+ /* Restore ownership setting */
+ regmap_update_bits(drvdata->csm_map, GPOOWNL,
+ BIT(offset - 64), tmp ? BIT(offset - 64) : 0);
+ break;
+ case 96 ... 127:
+ /* Keep ownership setting */
+ regmap_read(drvdata->csm_map, GPOOWNH, &tmp);
+ tmp = (tmp & BIT(offset - 96)) ? 1 : 0;
+
+ regmap_update_bits(drvdata->csm_map, GPOOWNH,
+ BIT(offset - 96), BIT(offset - 96));
+ regmap_update_bits(drvdata->csm_map, GPODATH,
+ BIT(offset - 96), value ? BIT(offset - 96) : 0);
+
+ /* Restore ownership setting */
+ regmap_update_bits(drvdata->csm_map, GPOOWNH,
+ BIT(offset - 96), tmp ? BIT(offset - 96) : 0);
+ break;
+ case 128 ... 159:
+ /* Keep ownership setting */
+ regmap_read(drvdata->csm_map, GPOOWN2L, &tmp);
+ tmp = (tmp & BIT(offset - 128)) ? 1 : 0;
+
+ regmap_update_bits(drvdata->csm_map, GPOOWN2L,
+ BIT(offset - 128), BIT(offset - 128));
+ regmap_update_bits(drvdata->csm_map, GPODAT2L,
+ BIT(offset - 128), value ? BIT(offset - 128) : 0);
+
+ /* Restore ownership setting */
+ regmap_update_bits(drvdata->csm_map, GPOOWN2L,
+ BIT(offset - 128), tmp ? BIT(offset - 128) : 0);
+ break;
+ case 160 ... 191:
+ /* Keep ownership setting */
+ regmap_read(drvdata->csm_map, GPOOWN2H, &tmp);
+ tmp = (tmp & BIT(offset - 160)) ? 1 : 0;
+
+ regmap_update_bits(drvdata->csm_map, GPOOWN2H,
+ BIT(offset - 160), BIT(offset - 160));
+ regmap_update_bits(drvdata->csm_map, GPODAT2H,
+ BIT(offset - 160), value ? BIT(offset - 160) : 0);
+
+ /* Restore ownership setting */
+ regmap_update_bits(drvdata->csm_map, GPOOWN2H,
+ BIT(offset - 160), tmp ? BIT(offset - 160) : 0);
+ break;
+ case 192:
+ if (value) {
+ regmap_update_bits(drvdata->csm_map, 0x5C,
+ BIT(0), BIT(0));
+ regmap_update_bits(drvdata->csm_map, 0x5C,
+ BIT(15), BIT(15));
+ } else {
+ regmap_update_bits(drvdata->csm_map, 0x5C,
+ 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 = GPIO_DIR_IN;
+ break;
+ case 64 ... 191:
+ ret = GPIO_DIR_OUT;
+ break;
+ case 192 ... 193:
+ ret = GPIO_DIR_OUT;
+ break;
+ case 194:
+ ret = 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 = 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)) ? 1 : 0;
+
+ break;
+ case PERST:
+ regmap_read(drvdata->fn2_stat, 0, &val);
+ ret = (val & BIT(25)) ? 1 : 0;
+
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+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(4, drvdata->vbtn);
+ writeb(1, drvdata->fn2_vbtn);
+ break;
+ default:
+ break;
+ }
+}
+
+static int gxp_gpio_fn2_get_direction(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ int ret = GPIO_DIR_IN;
+
+ switch (offset) {
+ case VPBTN:
+ ret = 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, ®map_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 4
+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 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->pl_led, 0x00, &val);
+ ret = (val & BIT(offset)) ? 1 : 0;
+ break;
+ case FAN1_INST ...FAN8_INST:
+ ret = (get_fans_installed() & BIT((offset - FAN1_INST))) ? 1 : 0;
+ break;
+ case FAN1_FAIL ... FAN8_FAIL:
+ ret = (get_fans_failed() & BIT((offset - FAN1_FAIL))) ? 1 : 0;
+ break;
+ case PWR_BTN_INT ... SLP_INT:
+ regmap_read(drvdata->pl_int, PLREG_INT_GRP5_FLAG, &val);
+ /* Active low */
+ ret = (val & BIT((offset - PWR_BTN_INT) + 16)) ? 0 : 1;
+ break;
+ case PSU1_INST ... PSU8_INST:
+ ret = (get_psu_inst() & BIT((offset - PSU1_INST))) ? 1 : 0;
+ break;
+ case PSU1_AC ... PSU8_AC:
+ ret = (get_psu_ac() & BIT((offset - PSU1_AC))) ? 1 : 0;
+ break;
+ case PSU1_DC ... PSU8_DC:
+ ret = (get_psu_dc() & 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->pl_led, 0x00, BIT(offset),
+ value == 0 ? 0 : BIT(offset));
+ break;
+ case LED_IDENTIFY:
+ regmap_update_bits(drvdata->pl_led, 0x01, BIT(7) | BIT(6),
+ value == 0 ? BIT(7) : BIT(7) | BIT(6));
+ break;
+ case LED_HEALTH_RED:
+ regmap_update_bits(drvdata->pl_health, 0x0, BIT(7),
+ value == 0 ? 0 : BIT(7));
+ break;
+ case LED_HEALTH_AMBER:
+ regmap_update_bits(drvdata->pl_health, 0x0, 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 = 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 = 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 = 0;
+ break;
+ case 59 ... 65:
+ ret = 0;
+ 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 = 0;
+ 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->pl_int, PLREG_INT_GRP5_FLAG, &val);
+ /* Clear latched interrupt */
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP5_FLAG,
+ 0xFF, 0xFF);
+
+ /* Read latched interrupt for group 6 */
+ regmap_read(drvdata->pl_int, PLREG_INT_GRP6_FLAG, &val);
+ /* Clear latched interrupt */
+ regmap_update_bits(drvdata->pl_int, 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->pl_int, PLREG_INT_GRP5_BASE,
+ BIT(0) | BIT(2), set ? 0 : BIT(0) | BIT(2));
+
+ regmap_update_bits(drvdata->pl_int, 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->pl_int, PLREG_INT_GRP5_BASE,
+ BIT(0) | BIT(2), 0);
+
+ regmap_update_bits(drvdata->pl_int, 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->pl_int, 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 + PLREG_INT_GRP5_PIN_BASE);
+ generic_handle_irq(girq);
+ }
+
+ /* Clear latched interrupt */
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP5_FLAG,
+ 0xFF, 0xFF);
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP5_BASE,
+ BIT(0) | BIT(2), 0);
+ }
+
+ /* Check group 6 interrupts */
+
+ regmap_read(drvdata->pl_int, PLREG_INT_GRP6_FLAG, &val);
+
+ if (val) {
+ for_each_set_bit(i, (unsigned long *)&val, 3) {
+ girq = irq_find_mapping(drvdata->chip.irq.domain,
+ i + PLREG_INT_GRP6_PIN_BASE);
+ generic_handle_irq(girq);
+ }
+
+ /* Clear latched interrupt */
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP6_FLAG,
+ 0xFF, 0xFF);
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP6_BASE,
+ BIT(2), 0);
+ }
+
+ 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 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_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");
+
+ drvdata->vbtn = devm_platform_ioremap_resource_byname(pdev, "vbtn");
+ if (IS_ERR(drvdata->vbtn))
+ return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->vbtn),
+ "failed to map vbtn\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_edge_irq;
+
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Get irq from platform fail - %d\n", ret);
+ return ret;
+ }
+ drvdata->irq = ret;
+
+ ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_fn2_irq_handle,
+ IRQF_SHARED, "gxp-fn2", drvdata);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "IRQ handler failed - %d\n", ret);
+ return ret;
+ }
+ drvdata->chip = common_chip;
+ drvdata->chip.ngpio = 300;
+
+ drvdata->chip.parent = &pdev->dev;
+ ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, NULL);
+ if (ret < 0)
+ dev_err(&pdev->dev,
+ "Could not register gpiochip for fn2, %d\n", ret);
+ dev_info(&pdev->dev, "HPE GXP FN2 driver loaded.\n");
+
+ return 0;
+}
+
+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->pl_led = gxp_gpio_init_regmap(pdev, "pl-led", 8);
+ if (IS_ERR(drvdata->pl_led))
+ return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
+ "failed to map pl_led\n");
+
+ drvdata->pl_health = gxp_gpio_init_regmap(pdev, "pl-health", 8);
+ if (IS_ERR(drvdata->pl_health))
+ return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
+ "failed to map pl_health\n");
+
+ drvdata->pl_int = gxp_gpio_init_regmap(pdev, "pl-int", 8);
+ if (IS_ERR(drvdata->pl_int))
+ return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
+ "failed to map pl_int\n");
+
+ drvdata->chip = plreg_chip;
+ drvdata->chip.ngpio = 100;
+ 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_edge_irq;
+
+ girq->init_hw = gxp_gpio_irq_init_hw;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
+ if (ret < 0)
+ dev_err(&pdev->dev, "Could not register gpiochip for plreg, %d\n",
+ ret);
+
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_HI_PRI_EN,
+ BIT(4) | BIT(5), BIT(4) | BIT(5));
+ regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK,
+ BIT(4) | BIT(5), 0x00);
+ regmap_read(drvdata->pl_int, PLREG_INT_HI_PRI_EN, &val);
+ regmap_read(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK, &val);
+
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Get irq from platform fail - %d\n", ret);
+ return ret;
+ }
+
+ drvdata->irq = ret;
+
+ ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
+ IRQF_SHARED, "gxp-pl", drvdata);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "IRQ handler failed - %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id gxp_gpio_of_match[] = {
+ { .compatible = "hpe,gxp-gpio"},
+ { .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 device *dev = &pdev->dev;
+ struct device *parent;
+ const struct of_device_id *match = of_match_device(gxp_gpio_of_match, dev);
+
+ if (!match) {
+ dev_err(dev, "device is not compatible");
+ return -EINVAL;
+ }
+
+ parent = dev->parent;
+ if (!parent) {
+ dev_err(dev, "no parent\n");
+ return -ENODEV;
+ }
+
+ drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_gpio_drvdata),
+ GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, drvdata);
+
+ if (strcmp(match->compatible, "hpe,gxp-gpio-pl") == 0)
+ ret = gxp_gpio_pl_init(pdev);
+ else
+ 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 <[email protected]>");
+MODULE_DESCRIPTION("GPIO interface for GXP");
+MODULE_LICENSE("GPL");
--
2.17.1
From: Nick Hawkins <[email protected]>
Add the CONFIG_I2C_GXP, CONFIG_GPIO_GXP, and CONFIG_SENSORS_GXP_PSU
symbols. Make CONFIG_SENSORS_GXP_FAN_CTRL=y
Signed-off-by: Nick Hawkins <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 084cc612ea23..fcfbcd233fb8 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -405,6 +405,7 @@ CONFIG_I2C_DAVINCI=y
CONFIG_I2C_DESIGNWARE_PLATFORM=y
CONFIG_I2C_DIGICOLOR=m
CONFIG_I2C_EMEV2=m
+CONFIG_I2C_GXP=m
CONFIG_I2C_IMX=y
CONFIG_I2C_MESON=y
CONFIG_I2C_MV64XXX=y
@@ -478,6 +479,7 @@ CONFIG_GPIO_ASPEED_SGPIO=y
CONFIG_GPIO_DAVINCI=y
CONFIG_GPIO_DWAPB=y
CONFIG_GPIO_EM=y
+CONFIG_GPIO_GXP=y
CONFIG_GPIO_MPC8XXX=y
CONFIG_GPIO_MXC=y
CONFIG_GPIO_RCAR=y
@@ -527,7 +529,8 @@ CONFIG_SENSORS_NTC_THERMISTOR=m
CONFIG_SENSORS_PWM_FAN=m
CONFIG_SENSORS_RASPBERRYPI_HWMON=m
CONFIG_SENSORS_INA2XX=m
-CONFIG_SENSORS_GXP_FAN_CTRL=m
+CONFIG_SENSORS_GXP_FAN_CTRL=y
+CONFIG_SENSORS_GXP_PSU=y
CONFIG_CPU_THERMAL=y
CONFIG_DEVFREQ_THERMAL=y
CONFIG_IMX_THERMAL=y
--
2.17.1
From: Nick Hawkins <[email protected]>
Provide i2c register information and CPLD register information to the
driver.
Signed-off-by: Nick Hawkins <[email protected]>
---
.../bindings/hwmon/hpe,gxp-psu.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
new file mode 100644
index 000000000000..60ca0f6ace46
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/hpe,gxp-psu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP psu controller
+
+maintainers:
+ - Nicholas Hawkins <[email protected]>
+
+properties:
+ compatible:
+ const: hpe,gxp-psu
+ interrupts:
+ maxItems: 1
+
+ reg:
+ maxItems: 1
+
+ hpe,sysreg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the global status registers shared between each psu
+ controller instance.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ psu@48 {
+ compatible = "hpe,gxp-psu";
+ reg = <0x48>;
+ hpe,sysreg = <&sysreg_system_controller2>;
+ };
+ };
--
2.17.1
On 18/04/2023 17:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> The GXP SoC supports GPIO on multiple interfaces: Host, CPLD and Soc
> pins. The interface from CPLD and Host are interruptable from vic0
> and vic1. This driver allows support for both of these interfaces
> through the use of different compatible bindings.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> drivers/gpio/Kconfig | 9 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-gxp.c | 1056 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1066 insertions(+)
> create mode 100644 drivers/gpio/gpio-gxp.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 13be729710f2..47435307698c 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1235,6 +1235,15 @@ 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
(...)
> +
> + drvdata->chip.parent = &pdev->dev;
> + ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, NULL);
> + if (ret < 0)
> + dev_err(&pdev->dev,
> + "Could not register gpiochip for fn2, %d\n", ret);
> + dev_info(&pdev->dev, "HPE GXP FN2 driver loaded.\n");
Drop non-informative probe successes. They are not needed.
> +
> + return 0;
> +}
> +
> +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->pl_led = gxp_gpio_init_regmap(pdev, "pl-led", 8);
> + if (IS_ERR(drvdata->pl_led))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_led\n");
> +
> + drvdata->pl_health = gxp_gpio_init_regmap(pdev, "pl-health", 8);
> + if (IS_ERR(drvdata->pl_health))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_health\n");
> +
> + drvdata->pl_int = gxp_gpio_init_regmap(pdev, "pl-int", 8);
> + if (IS_ERR(drvdata->pl_int))
> + return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->pl_int),
> + "failed to map pl_int\n");
> +
> + drvdata->chip = plreg_chip;
> + drvdata->chip.ngpio = 100;
> + 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_edge_irq;
> +
> + girq->init_hw = gxp_gpio_irq_init_hw;
> +
> + ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
> + if (ret < 0)
> + dev_err(&pdev->dev, "Could not register gpiochip for plreg, %d\n",
> + ret);
> +
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_HI_PRI_EN,
> + BIT(4) | BIT(5), BIT(4) | BIT(5));
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK,
> + BIT(4) | BIT(5), 0x00);
> + regmap_read(drvdata->pl_int, PLREG_INT_HI_PRI_EN, &val);
> + regmap_read(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK, &val);
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Get irq from platform fail - %d\n", ret);
> + return ret;
return dev_err_probe
> + }
> +
> + drvdata->irq = ret;
> +
> + ret = devm_request_irq(&pdev->dev, drvdata->irq, gxp_gpio_pl_irq_handle,
> + IRQF_SHARED, "gxp-pl", drvdata);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "IRQ handler failed - %d\n", ret);
return dev_err_probe
Actually other applicable places as well...
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id gxp_gpio_of_match[] = {
> + { .compatible = "hpe,gxp-gpio"},
> + { .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 device *dev = &pdev->dev;
> + struct device *parent;
> + const struct of_device_id *match = of_match_device(gxp_gpio_of_match, dev);
> +
> + if (!match) {
> + dev_err(dev, "device is not compatible");
It is not possible.
> + return -EINVAL;
> + }
> +
> + parent = dev->parent;
> + if (!parent) {
> + dev_err(dev, "no parent\n");
Why do you need this check?
> + return -ENODEV;
> + }
> +
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_gpio_drvdata),
sizeof(*)
> + GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, drvdata);
> +
> + if (strcmp(match->compatible, "hpe,gxp-gpio-pl") == 0)
No, use match data. Anyway this is open-coding of OF functions...
> + ret = gxp_gpio_pl_init(pdev);
> + else
> + ret = gxp_gpio_init(pdev);
> +
> + return ret;
Best regards,
Krzysztof
On 18/04/2023 17:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> Provide i2c register information and CPLD register information to the
> driver.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> .../bindings/hwmon/hpe,gxp-psu.yaml | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> new file mode 100644
> index 000000000000..60ca0f6ace46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/hpe,gxp-psu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP psu controller
> +
> +maintainers:
> + - Nicholas Hawkins <[email protected]>
> +
> +properties:
> + compatible:
> + const: hpe,gxp-psu
Missing blank line.
> + interrupts:
> + maxItems: 1
> +
> + reg:
> + maxItems: 1
All your bindings are written with different style... reg is second in
your previous patchset, so what order did you choose here?
> +
> + hpe,sysreg:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the global status registers shared between each psu
> + controller instance.
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
Drop blank line.
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + psu@48 {
> + compatible = "hpe,gxp-psu";
> + reg = <0x48>;
Add also interrupts.
Best regards,
Krzysztof
On 18/04/2023 17:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> Add the CONFIG_I2C_GXP, CONFIG_GPIO_GXP, and CONFIG_SENSORS_GXP_PSU
Why?
> symbols. Make CONFIG_SENSORS_GXP_FAN_CTRL=y
Why?
Please briefly provide rationale in the commit msg.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> arch/arm/configs/multi_v7_defconfig | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 084cc612ea23..fcfbcd233fb8 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -405,6 +405,7 @@ CONFIG_I2C_DAVINCI=y
> CONFIG_I2C_DESIGNWARE_PLATFORM=y
> CONFIG_I2C_DIGICOLOR=m
> CONFIG_I2C_EMEV2=m
> +CONFIG_I2C_GXP=m
> CONFIG_I2C_IMX=y
> CONFIG_I2C_MESON=y
> CONFIG_I2C_MV64XXX=y
> @@ -478,6 +479,7 @@ CONFIG_GPIO_ASPEED_SGPIO=y
> CONFIG_GPIO_DAVINCI=y
> CONFIG_GPIO_DWAPB=y
> CONFIG_GPIO_EM=y
> +CONFIG_GPIO_GXP=y
> CONFIG_GPIO_MPC8XXX=y
> CONFIG_GPIO_MXC=y
> CONFIG_GPIO_RCAR=y
> @@ -527,7 +529,8 @@ CONFIG_SENSORS_NTC_THERMISTOR=m
> CONFIG_SENSORS_PWM_FAN=m
> CONFIG_SENSORS_RASPBERRYPI_HWMON=m
> CONFIG_SENSORS_INA2XX=m
> -CONFIG_SENSORS_GXP_FAN_CTRL=m
> +CONFIG_SENSORS_GXP_FAN_CTRL=y
No, we want it to be module.
> +CONFIG_SENSORS_GXP_PSU=y
Same here.
> CONFIG_CPU_THERMAL=y
> CONFIG_DEVFREQ_THERMAL=y
> CONFIG_IMX_THERMAL=y
Best regards,
Krzysztof
On 4/18/23 08:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> The GXP SoC supports GPIO on multiple interfaces: Host, CPLD and Soc
> pins. The interface from CPLD and Host are interruptable from vic0
> and vic1. This driver allows support for both of these interfaces
> through the use of different compatible bindings.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> drivers/gpio/Kconfig | 9 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-gxp.c | 1056 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1066 insertions(+)
> create mode 100644 drivers/gpio/gpio-gxp.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 13be729710f2..47435307698c 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1235,6 +1235,15 @@ 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
> + 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_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..a7ce0ab097aa 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -63,6 +63,7 @@ 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_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.c b/drivers/gpio/gpio-gxp.c
> new file mode 100644
> index 000000000000..86f69174434d
> --- /dev/null
> +++ b/drivers/gpio/gpio-gxp.c
> @@ -0,0 +1,1056 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
> +
> +#include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define GPIDATL 0x40
> +#define GPIDATH 0x60
> +#define GPODATL 0xb0
> +#define GPODATH 0xb4
> +#define GPODAT2L 0xf8
> +#define GPODAT2H 0xfc
> +#define GPOOWNL 0x110
> +#define GPOOWNH 0x114
> +#define GPOOWN2L 0x118
> +#define GPOOWN2H 0x11c
> +
> +#define GPIO_DIR_OUT 0
> +#define GPIO_DIR_IN 1
> +
> +#define PGOOD_MASK 1
> +
> +#define PLREG_INT_GRP_STAT_MASK 0x8
> +#define PLREG_INT_HI_PRI_EN 0xC
> +#define PLREG_INT_GRP5_BASE 0x31
> +#define PLREG_INT_GRP6_BASE 0x35
> +#define PLREG_INT_GRP5_FLAG 0x30
> +#define PLREG_INT_GRP6_FLAG 0x34
> +#define PLREG_INT_GRP5_PIN_BASE 59
> +#define PLREG_INT_GRP6_PIN_BASE 90
> +
> +enum pl_gpio_pn {
> + IOP_LED1 = 0,
> + IOP_LED2,
> + IOP_LED3,
> + IOP_LED4,
> + IOP_LED5,
> + IOP_LED6,
> + IOP_LED7,
> + IOP_LED8,
> + FAN1_INST = 8,
> + 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,
> + LED_IDENTIFY = 56,
> + LED_HEALTH_RED,
> + LED_HEALTH_AMBER,
> + PWR_BTN_INT = 59,
> + UID_PRESS_INT,
> + SLP_INT,
> + ACM_FORCE_OFF = 70,
> + ACM_REMOVED,
> + ACM_REQ_N,
> + 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
> +};
> +
> +enum plreg_gpio_pn {
> + RESET = 192,
> + NMI_OUT = 193,
> + VPBTN = 210,
> + PGOOD,
> + PERST,
> + POST_COMPLETE,
> +};
> +
> +struct gxp_gpio_drvdata {
> + struct regmap *csm_map;
> + void __iomem *fn2_vbtn;
> + struct regmap *fn2_stat;
> + struct regmap *vuhc0_map;
> + void __iomem *vbtn;
> + struct regmap *pl_led;
> + struct regmap *pl_health;
> + struct regmap *pl_int;
> + struct gpio_chip chip;
> + int irq;
> +};
> +
> +extern u8 get_psu_inst(void);
> +extern u8 get_psu_ac(void);
> +extern u8 get_psu_dc(void);
> +extern u8 get_fans_installed(void);
> +extern u8 get_fans_failed(void);
> +
This is not information which should be reported through a gpio driver.
Besides, the functions don't exist at this point in the series,
and there should be no extern declarations in source files.
If you want to model fan or psu information through gpio, drop
the hwmon drivers and implement reading the status here, then use
the existing gpio-fan hwmon driver to report it in the hwmon subsystem.
Guenter
On 18/04/2023 17:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> Add support for the GXP I2C, PSU, and GPIO
> drivers. As well as adjust register ranges to be
> correct.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 161 ++++++++++++++++++
> arch/arm/boot/dts/hpe-gxp.dtsi | 197 ++++++++++++++++++++---
> 2 files changed, 338 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> index 3a7382ce40ef..487b6485a832 100644
> --- a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> @@ -23,4 +23,165 @@
> device_type = "memory";
> reg = <0x40000000 0x20000000>;
> };
> +
> + i2cmux@4 {
> + compatible = "i2c-mux-reg";
> + i2c-parent = <&i2c4>;
> + reg = <0xd1000374 0x1>;
Keep reg as second property. Run dtbs_check W=1, doesn't it scream about
mistake in unit address?
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@3 {
> + reg = <3>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@4 {
> + reg = <4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + i2cmux@6 {
> + compatible = "i2c-mux-reg";
> + i2c-parent = <&i2c6>;
> + reg = <0xd1000376 0x1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@3 {
> + reg = <3>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@4 {
> + reg = <4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@5 {
> + reg = <5>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +};
> +
> +&i2c0 {
> + status = "okay";
> +};
> +
> +&i2c1 {
> + status = "okay";
> +};
> +
> +&i2c2 {
> + status = "okay";
> + eeprom@50 {
> + compatible = "atmel,24c02";
> + pagesize = <8>;
> + reg = <0x50>;
Keep reg as second property. In other places you have it correctly.
> + };
> +};
> +
> +&i2c3 {
> + status = "okay";
> +};
> +
> +&i2c4 {
> + status = "okay";
> +};
> +
> +&i2c5 {
> + status = "okay";
> +};
> +
> +&i2c6 {
> + status = "okay";
> +};
> +
> +&i2c7 {
> + status = "okay";
> + psu@58 {
> + compatible = "hpe,gxp-psu";
> + reg = <0x58>;
> + hpe,sysreg = <&sysreg_system_controller2>;
> + };
> +
> + psu@59 {
> + compatible = "hpe,gxp-psu";
> + reg = <0x59>;
> + hpe,sysreg = <&sysreg_system_controller2>;
> + };
> +};
> +
> +&i2c8 {
> + status = "okay";
> +};
> +
> +&i2c9 {
> + status = "okay";
> +};
> +
> +&gpio {
> + gpio-line-names =
> + "", "", "", "", "", "", "", "", "", "", /*0 - 9*/
That's very odd indentation. Usually it is one of:
gpio-line-names = "foo", "bar"
"baz", "zab";
gpio-line-names =
"foo", "bar"
"baz", "zab";
Where first one is preferred.
> + "", "", "", "", "", "", "", "", "", "", /*10 - 19*/
> + "", "", "", "", "", "", "", "", "", "", /*20 - 29*/
> + "", "", "", "", "", "", "", "", "", "", /*30 - 39*/
> + "", "", "", "", "", "", "", "", "", "", /*40 - 49*/
> + "", "", "", "", "", "", "", "", "", "", /*50 - 59*/
> + "", "", "", "", "", "", "", "", "", "", /*60 - 69*/
> + "", "", "", "", "", "", "", "", "", "", /*70 - 79*/
> + "", "", "", "", "", "", "", "", "", "", /*80 - 89*/
> + "", "", "", "", "", "", "", "", "", "", /*90 - 99*/
> + "", "", "", "", "", "", "", "", "", "", /*100 - 109*/
> + "", "", "", "", "", "", "", "", "", "", /*110 - 119*/
> + "", "", "", "", "", "", "", "", "", "", /*120 - 129*/
> + "", "", "", "", "", "", "", "", "", "", /*130 - 139*/
> + "", "", "", "", "", "", "", "", "", "", /*140 - 149*/
> + "", "", "", "", "", "", "", "", "", "", /*150 - 159*/
> + "", "", "", "", "", "", "", "", "", "", /*160 - 169*/
> + "", "", "", "", "", "", "", "", "", "", /*170 - 179*/
> + "", "", "", "", "", "", "", "", "", "", /*180 - 189*/
> + "", "", "RESET_OUT", "NMI_OUT", "", "", "", "", "", "", /*190 - 199*//*GPIO*/
> + "", "", "", "", "", "", "", "", "", "", /*Vuhc 200-209*/
> + "POWER_OUT", "PS_PWROK", "PCIERST", "POST_COMPLETE", "", "", "", "", "", ""; /*210 - 219*/
> +};
> +
> +&gpio1 {
> + 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", "", "", "", "",
> + "", "", "", "", "", "", "", "", "", "";
> };
> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> index cf735b3c4f35..8a8faf7fbd60 100644
> --- a/arch/arm/boot/dts/hpe-gxp.dtsi
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Device Tree file for HPE GXP
> + * Device Tree for HPE GXP
Not really related to this change,
> */
>
> /dts-v1/;
> @@ -52,76 +52,233 @@
> cache-level = <2>;
> };
>
> - ahb@c0000000 {
> + ahb@80000000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> - ranges = <0x0 0xc0000000 0x30000000>;
> - dma-ranges;
> + ranges = <0x0 0x80000000 0x20000000>,
> + <0x40000000 0xc0000000 0x3fff0000>;
Wrong indentation.
>
> - vic0: interrupt-controller@eff0000 {
> + vic0: interrupt-controller@4eff0000 {
You need to better organize your changes and split some refactorings
from new devices. I don't understand why eff0000 becomes 4eff0000 -
whether this is a bug being fixed, incorrect design etc. Commit msg just
says "to be correct", so this is was a bug. Bugfixes cannot mixed with
new features/code/refactorings. Anyway this is very vague. Explain what
is not correct, why it has to be fixed.
> compatible = "arm,pl192-vic";
> - reg = <0xeff0000 0x1000>;
> + reg = <0x4eff0000 0x1000>;
> interrupt-controller;
> #interrupt-cells = <1>;
> };
>
> - vic1: interrupt-controller@80f00000 {
> + vic1: interrupt-controller@f00000 {
> compatible = "arm,pl192-vic";
> - reg = <0x80f00000 0x1000>;
> + reg = <0xf00000 0x1000>;
> interrupt-controller;
> #interrupt-cells = <1>;
> };
>
> - uarta: serial@e0 {
> + uarta: serial@400000e0 {
> compatible = "ns16550a";
> - reg = <0xe0 0x8>;
> + reg = <0x400000e0 0x8>;
> interrupts = <17>;
> interrupt-parent = <&vic0>;
> clock-frequency = <1846153>;
> reg-shift = <0>;
> };
>
> - uartb: serial@e8 {
> + uartb: serial@400000e8 {
> compatible = "ns16550a";
> - reg = <0xe8 0x8>;
> + reg = <0x400000e8 0x8>;
> interrupts = <18>;
> interrupt-parent = <&vic0>;
> clock-frequency = <1846153>;
> reg-shift = <0>;
> };
>
> - uartc: serial@f0 {
> + uartc: serial@400000f0 {
> compatible = "ns16550a";
> - reg = <0xf0 0x8>;
> + reg = <0x400000f0 0x8>;
> interrupts = <19>;
> interrupt-parent = <&vic0>;
> clock-frequency = <1846153>;
> reg-shift = <0>;
> };
>
> - usb0: usb@efe0000 {
> + usb0: usb@4efe0000 {
> compatible = "hpe,gxp-ehci", "generic-ehci";
> - reg = <0xefe0000 0x100>;
> + reg = <0x4efe0000 0x100>;
> interrupts = <7>;
> interrupt-parent = <&vic0>;
> };
>
> - st: timer@80 {
> + st: timer@40000080 {
> compatible = "hpe,gxp-timer";
> - reg = <0x80 0x16>;
> + reg = <0x40000080 0x16>;
> interrupts = <0>;
> interrupt-parent = <&vic0>;
> clocks = <&iopclk>;
> clock-names = "iop";
> };
>
> - usb1: usb@efe0100 {
> + usb1: usb@4efe0100 {
> compatible = "hpe,gxp-ohci", "generic-ohci";
> - reg = <0xefe0100 0x110>;
> + reg = <0x4efe0100 0x110>;
> interrupts = <6>;
> interrupt-parent = <&vic0>;
> };
> +
> + sysreg_system_controller: syscon@400000f8 {
> + compatible = "hpe,gxp-sysreg", "syscon";
> + reg = <0x400000f8 0x8>;
> + };
> +
> + sysreg_system_controller2: syscon@51000319 {
> + compatible = "hpe,gxp-sysreg", "syscon";
> + reg = <0x51000319 0x4>;
> + };
> +
> + i2c0: i2c@40002000 {
> + compatible = "hpe,gxp-i2c";
> + reg = <0x40002000 0x70>;
> + interrupts = <9>;
> + interrupt-parent = <&vic0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
Status is last property.
> + hpe,sysreg = <&sysreg_system_controller>;
> + clock-frequency = <100000>;
> + };
(...)
> +
> + fan-controller@40000c10 { /* 0xc0000c10 */
> + compatible = "hpe,gxp-fan-ctrl";
> + reg = <0x40000c10 0x8>, <0x51000327 0x06>;
> + reg-names = "base", "pl";
> + };
> +
> + gpio: gpio@0 {
> + compatible = "hpe,gxp-gpio";
> + reg = <0x0 0x400>, <0x200046 0x1>, <0x200070 0x08>,
> + <0x400064 0x80>, <0x5100030f 0x1>;
This looks randomly indented...
Best regards,
Krzysztof
On 18/04/2023 17:28, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> List the files added for GPIO and PSU support.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> MAINTAINERS | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a3b14ec33830..6df959ebf523 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2239,7 +2239,9 @@ M: Nick Hawkins <[email protected]>
> 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
Since the drivers are going through subsystems, your patchset is not
bisectable.
Squash respective changes with respective patches.
I also suggest do not mix three different subsystems - GPIO, hwmon and
ARM SoC - in one patchset.
Best regards,
Krzysztof
On Tue, Apr 18, 2023 at 10:28:21AM -0500, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> Provide i2c register information and CPLD register information to the
> driver.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> .../bindings/hwmon/hpe,gxp-psu.yaml | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> new file mode 100644
> index 000000000000..60ca0f6ace46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/hpe,gxp-psu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP psu controller
> +
> +maintainers:
> + - Nicholas Hawkins <[email protected]>
> +
> +properties:
> + compatible:
> + const: hpe,gxp-psu
> + interrupts:
> + maxItems: 1
> +
> + reg:
> + maxItems: 1
> +
> + hpe,sysreg:
Why is this optional?
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the global status registers shared between each psu
> + controller instance.
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + psu@48 {
> + compatible = "hpe,gxp-psu";
> + reg = <0x48>;
> + hpe,sysreg = <&sysreg_system_controller2>;
> + };
> + };
> --
> 2.17.1
>
> This is not information which should be reported through a gpio driver.
> Besides, the functions don't exist at this point in the series,
> and there should be no extern declarations in source files.
> If you want to model fan or psu information through gpio, drop
> the hwmon drivers and implement reading the status here, then use
> the existing gpio-fan hwmon driver to report it in the hwmon subsystem.
Thank you for the feedback Guenter,
I see how it is possible to use gpio-fan for the fan. As for the gxp-psu
Hwmon driver can I model the gpio-fan driver to get the necessary
gpio information for power supplies?
-Nick Hawkins
> You need to better organize your changes and split some refactorings
> from new devices. I don't understand why eff0000 becomes 4eff0000 -
> whether this is a bug being fixed, incorrect design etc. Commit msg just
> says "to be correct", so this is was a bug. Bugfixes cannot mixed with
> new features/code/refactorings. Anyway this is very vague. Explain what
> is not correct, why it has to be fixed.
Thank you for all of the feedback you have provided Krzysztof,
It indeed is a bug that was introduced early on. I attempted
previously to correct this issue separately here:
https://lore.kernel.org/all/[email protected]/
I see though that I have made some of the mistakes you mentioned above.
I will resubmit.
> > + gpio: gpio@0 {
> > + compatible = "hpe,gxp-gpio";
> > + reg = <0x0 0x400>, <0x200046 0x1>, <0x200070 0x08>,
> > + <0x400064 0x80>, <0x5100030f 0x1>;
> This looks randomly indented...
Although the design is likely to change I will use a format similar to
gpio-line-names you mentioned previously.
Thanks,
-Nick Hawkins
Tue, Apr 18, 2023 at 10:28:16AM -0500, [email protected] kirjoitti:
> From: Nick Hawkins <[email protected]>
>
> The GXP SoC supports GPIO on multiple interfaces: Host, CPLD and Soc
> pins. The interface from CPLD and Host are interruptable from vic0
> and vic1. This driver allows support for both of these interfaces
> through the use of different compatible bindings.
Thank you for your contribution. To begin with, I don't believe a simple GPIO
driver needs 1000+ LoCs. But see more comments below.
...
> +config GPIO_GXP
> + tristate "GXP GPIO support"
> + depends on ARCH_HPE_GXP
> + 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.
The indentation is rather problematic. Had you run checkpatch.pl?
...
> +#include <linux/gpio.h>
No, new code may not include this header.
...
> +#include <linux/of.h>
> +#include <linux/of_device.h>
Why? I haven't seen much evidence of needing of these. What you need are
rather mod_devicetable.h and property.h (see also below).
...
> +#define GPIDATL 0x40
> +#define GPIDATH 0x60
> +#define GPODATL 0xb0
> +#define GPODATH 0xb4
> +#define GPODAT2L 0xf8
> +#define GPODAT2H 0xfc
Use same size of the values, e.g. 0x0fc
> +#define GPOOWNL 0x110
> +#define GPOOWNH 0x114
> +#define GPOOWN2L 0x118
> +#define GPOOWN2H 0x11c
To me this sounds like a 2 8-lines bank GPIO array. Drop those H and L suffixes
and use proper offset calculation based on the bank number. There are plenty of
existing GPIO drivers that do that way.
Also why gpio-regmap is not in use?
...
> +#define GPIO_DIR_OUT 0
> +#define GPIO_DIR_IN 1
Oh, what is this?! You must not interfere with the GPIO_ namespace.
...
> +#define PGOOD_MASK 1
For mask use BIT() / GENMASK()
> +#define PLREG_INT_GRP_STAT_MASK 0x8
Ditto.
...
> +#define PLREG_INT_HI_PRI_EN 0xC
Is it register offset or value?
> +#define PLREG_INT_GRP5_BASE 0x31
> +#define PLREG_INT_GRP6_BASE 0x35
> +#define PLREG_INT_GRP5_FLAG 0x30
> +#define PLREG_INT_GRP6_FLAG 0x34
These need more expanation what they are for and why these specific values are
being used.
...
> +#define PLREG_INT_GRP5_PIN_BASE 59
> +#define PLREG_INT_GRP6_PIN_BASE 90
What are these for?
...
> +enum plreg_gpio_pn {
> + RESET = 192,
> + NMI_OUT = 193,
> + VPBTN = 210,
> + PGOOD,
> + PERST,
> + POST_COMPLETE,
Why these numbers? If it comes from hardware specification, make _all_ them
explicitly assigned.
> +};
...
> +struct gxp_gpio_drvdata {
> + struct regmap *csm_map;
> + void __iomem *fn2_vbtn;
> + struct regmap *fn2_stat;
> + struct regmap *vuhc0_map;
> + void __iomem *vbtn;
> + struct regmap *pl_led;
> + struct regmap *pl_health;
> + struct regmap *pl_int;
> + struct gpio_chip chip;
Move it to be the first member in the structure. Might save a few bytes in the
code.
> + int irq;
> +};
...
> +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, GPIDATL, &ret);
> + ret = (ret & BIT(offset)) ? 1 : 0;
> + break;
> + case 32 ... 63:
> + regmap_read(drvdata->csm_map, GPIDATH, &ret);
> + ret = (ret & BIT(offset - 32)) ? 1 : 0;
> + break;
> + case 64 ... 95:
> + regmap_read(drvdata->csm_map, GPODATL, &ret);
> + ret = (ret & BIT(offset - 64)) ? 1 : 0;
> + break;
> + case 96 ... 127:
> + regmap_read(drvdata->csm_map, GPODATH, &ret);
> + ret = (ret & BIT(offset - 96)) ? 1 : 0;
> + break;
> + case 128 ... 159:
> + regmap_read(drvdata->csm_map, GPODAT2L, &ret);
> + ret = (ret & BIT(offset - 128)) ? 1 : 0;
> + break;
> + case 160 ... 191:
> + regmap_read(drvdata->csm_map, GPODAT2H, &ret);
> + ret = (ret & BIT(offset - 160)) ? 1 : 0;
> + break;
> + case 192:
> + /* SW_RESET */
> + regmap_read(drvdata->csm_map, 0x5C, &ret);
> + ret = (ret & BIT(15)) ? 1 : 0;
> + break;
Besides redundant " ? 1 : 0" parts, see what I wrote above and calculate offset
properly and then apply. Ditto for other functions.
> + default:
> + break;
> + }
> +
> + return ret;
> +}
I stopped here, this driver requires a lot more work to be done before
considering for upstream, sorry.
...
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_edge_irq;
Should be handle_bad_irq() by default.
...
> + ret = devm_gpiochip_add_data(&pdev->dev, &drvdata->chip, drvdata);
> + if (ret < 0)
> + dev_err(&pdev->dev, "Could not register gpiochip for plreg, %d\n",
> + ret);
Huh?! No bailing out?
...
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_HI_PRI_EN,
> + BIT(4) | BIT(5), BIT(4) | BIT(5));
GENMASK(), but do it as a defined value with meaningful name.
> + regmap_update_bits(drvdata->pl_int, PLREG_INT_GRP_STAT_MASK,
> + BIT(4) | BIT(5), 0x00);
Ditto.
...
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Get irq from platform fail - %d\n", ret);
No. The message is already printed by platform code. You are not supposed to
pollute logs with duplicative noise.
> + return ret;
> + }
...
> +static const struct of_device_id gxp_gpio_of_match[] = {
> + { .compatible = "hpe,gxp-gpio"},
> + { .compatible = "hpe,gxp-gpio-pl"},
Missing spaces in above two lines.
> + {}
> +};
...
> + const struct of_device_id *match = of_match_device(gxp_gpio_of_match, dev);
device_get_match_data()
...
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_gpio_drvdata),
> + GFP_KERNEL);
sizeof(*drvdata) and make it one line.
> + if (!drvdata)
> + return -ENOMEM;
--
With Best Regards,
Andy Shevchenko
> Thank you for your contribution. To begin with, I don't believe a simple GPIO
> driver needs 1000+ LoCs. But see more comments below.
Andy,
Thank you for your feedback. I will apply all the input you have provided.
I will need to rewrite this code and I am considering the need to
perhaps create two files instead of one to keep code length down. As
implied by the description I was trying to have one file handle two
different compatible strings.
I believe that one file will need to be the regular IO from the host and
memory mapped IO pins of our SoC. The other will need to be the memory
mapped IO pins coming from our CPLD. Both of these sources are interruptible
which does cause some complexity.
Please let me know if what I have described above is not a good approach to
take with GPIO drivers. Any guidance would be greatly appreciated.
Best Regards,
-Nick Hawkins
On Thu, Apr 27, 2023 at 10:01 PM Hawkins, Nick <[email protected]> wrote:
> > Thank you for your contribution. To begin with, I don't believe a simple GPIO
> > driver needs 1000+ LoCs. But see more comments below.
>
> Andy,
>
> Thank you for your feedback. I will apply all the input you have provided.
You are welcome!
> I will need to rewrite this code and I am considering the need to
> perhaps create two files instead of one to keep code length down. As
> implied by the description I was trying to have one file handle two
> different compatible strings.
>
> I believe that one file will need to be the regular IO from the host and
> memory mapped IO pins of our SoC. The other will need to be the memory
> mapped IO pins coming from our CPLD. Both of these sources are interruptible
> which does cause some complexity.
>
> Please let me know if what I have described above is not a good approach to
> take with GPIO drivers. Any guidance would be greatly appreciated.
I don't know your hardware, otherwise what you wrote above sounds good to me.
--
With Best Regards,
Andy Shevchenko
On 4/27/23 07:53, Hawkins, Nick wrote:
>
>
>
>> This is not information which should be reported through a gpio driver.
>> Besides, the functions don't exist at this point in the series,
>> and there should be no extern declarations in source files.
>
>> If you want to model fan or psu information through gpio, drop
>> the hwmon drivers and implement reading the status here, then use
>> the existing gpio-fan hwmon driver to report it in the hwmon subsystem.
>
> Thank you for the feedback Guenter,
>
> I see how it is possible to use gpio-fan for the fan. As for the gxp-psu
> Hwmon driver can I model the gpio-fan driver to get the necessary
> gpio information for power supplies?
>
Sorry, I don't understand. Looking into the code again, the major problem
I see is that you want to model fan install status and fan fault
status as gpio pins. The same is true for psu information (installed,
ac, dc flags).
If you want to do this, fine, but then get the status from the gpio
driver and don't export anything to the gpio driver. The kernel supports
means to do that (look at gpiod_get and similar functions). It makes the
code more complex, but I assume you know what you are doing.
Guenter
> Sorry, I don't understand. Looking into the code again, the major problem
> I see is that you want to model fan install status and fan fault
> status as gpio pins. The same is true for psu information (installed,
> ac, dc flags).
> If you want to do this, fine, but then get the status from the gpio
> driver and don't export anything to the gpio driver. The kernel supports
> means to do that (look at gpiod_get and similar functions). It makes the
> code more complex, but I assume you know what you are doing.
Greetings Guenter and Andy,
I have pursued this approach and I have found that while the PSU and
FAN drivers can consume the presence and fail info from the GPIO
driver, the host is unable to read the read only GPIOs.
In OpenBMC we have a GPIO consumer that sits and waits for
GPIOs changes then takes action on it. To resolve this issue would
it be acceptable for the GPIO driver to poll the relevant GPIOs and
share the necessary fan presence/failure and psu presence/failure
information via a global shared variable? This would be the alternative
to the drivers consuming GPIOs.
Thanks you for your time and assistance,
-Nick Hawkins