| Message ID | 20230418152824.110823-1-nick.hawkins@hpe.com |
|---|---|
| Headers | show |
| Series | ARM: Add GPIO and PSU Support | expand |
On 18/04/2023 17:28, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > Provide access to the registers and interrupts for GPIO. The GPIO > will have two driver instances: One for host, the other for CPLD. Are these different devices? What does it mean here "instance"? What are the differences? > > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> > --- > .../bindings/gpio/hpe,gxp-gpio.yaml | 137 ++++++++++++++++++ > 1 file changed, 137 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..1cf4cff26d5f > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/hpe,gxp-gpio.yaml > @@ -0,0 +1,137 @@ > +# 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 s/gpio/GPIO/ > + > +maintainers: > + - Nick Hawkins <nick.hawkins@hpe.com> > + > +description: > + Interruptable GPIO drivers for the HPE GXP that covers multiple interfaces. > + > +properties: > + compatible: > + oneOf: Drop oneOf. > + - items: And items. You do not have here multiple choices and items. > + - enum: > + - hpe,gxp-gpio > + - hpe,gxp-gpio-pl > + > + reg: > + minItems: 3 > + maxItems: 6 > + > + reg-names: > + minItems: 3 > + maxItems: 6 > + > + gpio-controller: true > + > + '#gpio-cells': > + const: 2 > + > + gpio-line-names: > + minItems: 1 > + maxItems: 300 Hm, shouldn't line-names match all GPIOs? If someone provides just one name, how do you know for which GPIO is it? > + > + interrupts: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - reg-names > + - gpio-controller > + - "#gpio-cells" Use consistent quotes. Either ' or " > + > +additionalProperties: false Put it after allOf: block. > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - hpe,gxp-gpio > + then: > + properties: > + reg: > + items: > + - description: CSM > + - description: fn2 virtual button > + - description: fn2 system status > + - description: vuhc status > + - description: external virtual button I have doubts you describe actual one GPIO controller... > + reg-names: > + items: > + - const: csm > + - const: fn2-vbtn > + - const: fn2-stat > + - const: vuhc > + - const: vbtn > + - if: > + properties: > + compatible: > + contains: > + enum: > + - hpe,gxp-gpio-pl > + then: > + properties: > + reg: > + items: > + - description: Programmable logic led > + - description: Programmable logic health led > + - description: Programmable logic interrupt interface > + reg-names: > + items: > + - const: pl-led > + - const: pl-health > + - const: pl-int > + > +examples: > + - | > + gpio@0 { Weird indentation. Use 4 spaces for example indentation. > + 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>; > + interrupt-parent = <&vic0>; > + interrupts = <10>; > + gpio-line-names = > + "IOP_LED1", "IOP_LED2", "IOP_LED3", "IOP_LED4", "IOP_LED5", "IOP_LED6", "IOP_LED7", "IOP_LED8", Broken indentation and unnecessary line break before. > + "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
On 18/04/2023 17:28, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > Provide i2c register information and CPLD register information to the > driver. > > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> > --- > .../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 <nick.hawkins@hpe.com> > + > +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, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > 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 <nick.hawkins@hpe.com> > --- > 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
From: Nick Hawkins <nick.hawkins@hpe.com> 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