[v1,1/9] gpio: gxp: Add HPE GXP GPIO

Commit Message

Hawkins, Nick April 18, 2023, 3:28 p.m. UTC

From: Nick Hawkins <[e-mail]>

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 <[e-mail]>
---
 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

Comments

Krzysztof Kozlowski April 18, 2023, 5:02 p.m. UTC | #1

On 18/04/2023 17:28, [e-mail] wrote:
> From: Nick Hawkins <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>  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

Guenter Roeck April 18, 2023, 5:17 p.m. UTC | #2

On 4/18/23 08:28, [e-mail] wrote:
> From: Nick Hawkins <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>   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

Hawkins, Nick April 27, 2023, 2:53 p.m. UTC | #3

> 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

Andy Shevchenko April 27, 2023, 4:28 p.m. UTC | #4

Tue, Apr 18, 2023 at 10:28:16AM -0500, [e-mail] kirjoitti:
> From: Nick Hawkins <[e-mail]>
> 
> 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;

Hawkins, Nick April 27, 2023, 7:01 p.m. UTC | #5

> 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

Andy Shevchenko April 28, 2023, 6:51 a.m. UTC | #6

On Thu, Apr 27, 2023 at 10:01 PM Hawkins, Nick <[e-mail]> 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.

Guenter Roeck April 28, 2023, 1:32 p.m. UTC | #7

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

Hawkins, Nick May 12, 2023, 4:08 p.m. UTC | #8

> 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

Patch

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, &regmap_config);
+}
+
+static void gxp_gpio_fn2_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct gxp_gpio_drvdata *drvdata = dev_get_drvdata(chip->parent);
+	unsigned int val;
+
+	/* Read latched interrupt */
+	regmap_read(drvdata->fn2_stat, 0, &val);
+	/* Clear latched interrupt */
+	regmap_update_bits(drvdata->fn2_stat, 0,
+			   0xFFFF, 0xFFFF);
+}
+
+#define FN2_SEVMASK 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 <[e-mail]>");
+MODULE_DESCRIPTION("GPIO interface for GXP");
+MODULE_LICENSE("GPL");