Re: [PATCH v3 03/12] gpio: Add GPIO driver for Intel EG20T

[Yao Zi <ziyao@disroot.org>]

On Tue, Jul 29, 2025 at 04:22:43PM +0000, Uros Stajic wrote:
> From: Chao-ying Fu <cfu@mips.com>
> 
> Add a GPIO driver for the Intel EG20T Platform Controller Hub, which
> exposes a set of 12 GPIOs via PCI MMIO.
> 
> The driver implements basic GPIO operations (input/output direction,
> value read/write, and function query) using the U-Boot driver model
> infrastructure. It maps the required BAR1 region via `dm_pci_map_bar`
> and uses internal registers to control pin state and direction.
> 
> This driver is required for platforms using EG20T, such as P8700-based
> systems, to access GPIOs through the standard U-Boot DM GPIO framework.
> 
> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> Signed-off-by: Uros Stajic <uros.stajic@htecgroup.com>
> ---
>  board/mips/boston-riscv/MAINTAINERS |   1 +
>  drivers/gpio/Kconfig                |   7 ++
>  drivers/gpio/Makefile               |   1 +
>  drivers/gpio/eg20t-gpio.c           | 138 ++++++++++++++++++++++++++++
>  4 files changed, 147 insertions(+)
>  create mode 100644 drivers/gpio/eg20t-gpio.c
> 
> diff --git a/board/mips/boston-riscv/MAINTAINERS b/board/mips/boston-riscv/MAINTAINERS
> index e350121395e..bc59a628c79 100644
> --- a/board/mips/boston-riscv/MAINTAINERS
> +++ b/board/mips/boston-riscv/MAINTAINERS
> @@ -7,3 +7,4 @@ F:  arch/riscv/cpu/p8700/
>  F:  arch/riscv/include/asm/arch-p8700/
>  F:  configs/boston-p8700_defconfig
>  F:  arch/riscv/dts/boston-p8700.dts
> +F:  drivers/gpio/eg20t-gpio.c
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 58e464106a3..26040947a69 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -726,3 +726,10 @@ config MPFS_GPIO
>  		Enable to support the GPIO driver on Polarfire SoC
>  
>  endif
> +
> +config EG20T_GPIO
> +        bool "Intel EG20T GPIO driver"
> +        depends on DM_GPIO && DM_PCI
> +        help
> +          Enable this to support the GPIO controller found in the Intel EG20T
> +          Platform Controller Hub.
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 83e10c79b91..e44122cb9aa 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -81,3 +81,4 @@ obj-$(CONFIG_FTGPIO010)		+= ftgpio010.o
>  obj-$(CONFIG_$(PHASE_)ADP5585_GPIO)	+= adp5585_gpio.o
>  obj-$(CONFIG_RZG2L_GPIO)	+= rzg2l-gpio.o
>  obj-$(CONFIG_MPFS_GPIO)	+= mpfs_gpio.o
> +obj-$(CONFIG_EG20T_GPIO)	+= eg20t-gpio.o
> diff --git a/drivers/gpio/eg20t-gpio.c b/drivers/gpio/eg20t-gpio.c
> new file mode 100644
> index 00000000000..d41ca4bfb17
> --- /dev/null
> +++ b/drivers/gpio/eg20t-gpio.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2016 Imagination Technologies
> + */
> +
> +#include <dm.h>
> +#include <errno.h>
> +#include <pci.h>
> +#include <asm/io.h>
> +#include <asm/gpio.h>
> +#include <log.h>
> +
> +enum {
> +	REG_IEN		= 0x00,
> +	REG_ISTATUS	= 0x04,
> +	REG_IDISP	= 0x08,
> +	REG_ICLR	= 0x0c,
> +	REG_IMASK	= 0x10,
> +	REG_IMASKCLR	= 0x14,
> +	REG_PO		= 0x18,
> +	REG_PI		= 0x1c,
> +	REG_PM		= 0x20,
> +};
> +
> +struct eg20t_gpio_priv {
> +	void *base;
> +};
> +
> +static int eg20t_gpio_get_value(struct udevice *dev, unsigned int offset)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	u32 pm, pval;
> +
> +	pm = readl(priv->base + REG_PM);
> +	if ((pm >> offset) & 0x1)
> +		pval = readl(priv->base + REG_PO);
> +	else
> +		pval = readl(priv->base + REG_PI);
> +
> +	return (pval >> offset) & 0x1;
> +}
> +
> +static int eg20t_gpio_set_value(struct udevice *dev, unsigned int offset,
> +				int value)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	u32 po;
> +
> +	po = readl(priv->base + REG_PO);
> +	if (value)
> +		po |= 1 << offset;
> +	else
> +		po &= ~(1 << offset);
> +	writel(po, priv->base + REG_PO);
> +	return 0;
> +}

This file contains inconsistent usage of empty lines between statements
and the final return among functions. I think it's better to keep the
style aligned.

Regards,
Yao Zi

> +
> +static int eg20t_gpio_direction_input(struct udevice *dev, unsigned int offset)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	u32 pm;
> +
> +	pm = readl(priv->base + REG_PM);
> +	pm &= ~(1 << offset);
> +	writel(pm, priv->base + REG_PM);
> +	return 0;
> +}
> +
> +static int eg20t_gpio_direction_output(struct udevice *dev, unsigned int offset,
> +				       int value)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	u32 pm;
> +
> +	pm = readl(priv->base + REG_PM);
> +	pm |= 1 << offset;
> +	writel(pm, priv->base + REG_PM);
> +
> +	return eg20t_gpio_set_value(dev, offset, value);
> +}
> +
> +static int eg20t_gpio_get_function(struct udevice *dev, unsigned int offset)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	u32 pm;
> +
> +	pm = readl(priv->base + REG_PM);
> +
> +	if ((pm >> offset) & 0x1)
> +		return GPIOF_OUTPUT;
> +
> +	return GPIOF_INPUT;
> +}
> +
> +static const struct dm_gpio_ops eg20t_gpio_ops = {
> +	.direction_input	= eg20t_gpio_direction_input,
> +	.direction_output	= eg20t_gpio_direction_output,
> +	.get_value		= eg20t_gpio_get_value,
> +	.set_value		= eg20t_gpio_set_value,
> +	.get_function		= eg20t_gpio_get_function,
> +};
> +
> +static int eg20t_gpio_probe(struct udevice *dev)
> +{
> +	struct eg20t_gpio_priv *priv = dev_get_priv(dev);
> +	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
> +
> +	priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM);
> +	if (!priv->base) {
> +		debug("failed to map GPIO registers\n");
> +		return -EINVAL;
> +	}
> +
> +	uc_priv->gpio_count = 12;
> +	uc_priv->bank_name = "eg20t";
> +	return 0;
> +}
> +
> +static const struct udevice_id eg20t_gpio_ids[] = {
> +	{ .compatible = "intel,eg20t-gpio" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(eg20t_gpio) = {
> +	.name	= "eg20t-gpio",
> +	.id	= UCLASS_GPIO,
> +	.of_match = eg20t_gpio_ids,
> +	.probe	= eg20t_gpio_probe,
> +	.priv_auto_alloc_size = sizeof(struct eg20t_gpio_priv),
> +	.ops	= &eg20t_gpio_ops,
> +};
> +
> +static struct pci_device_id eg20t_gpio_supported[] = {
> +	{ PCI_VENDOR_ID_INTEL, 0x8803 },
> +	{ },
> +};
> +
> +U_BOOT_PCI_DEVICE(eg20t_gpio, eg20t_gpio_supported);
> -- 
> 2.34.1