linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/1] Polarfire SoC GPIO support
@ 2024-10-24  9:20 Conor Dooley
  2024-10-24  9:20 ` [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support Conor Dooley
  0 siblings, 1 reply; 4+ messages in thread
From: Conor Dooley @ 2024-10-24  9:20 UTC (permalink / raw)
  To: linux-gpio
  Cc: conor, Conor Dooley, Daire McNamara, valentina.fernandezalanis,
	Linus Walleij, Bartosz Golaszewski

From: Conor Dooley <conor.dooley@microchip.com>

Hey all,

Lewis is no longer at Microchip, so I've taken over the GPIO controller
patchset that he had been working on prior to that:
https://lore.kernel.org/linux-gpio/20220815120834.1562544-1-lewis.hanly@microchip.com/

One thing that was wrong with Lewis' series was that it could only,
depending on the iteration of the series, support GPIOs that had their
interrupts muxed or GPIOs that had dedicated interrupts at the
parent interrupt controller. I found that to be problematic, because the
hardware itself always has a mix of muxed and dedicated interrupts and
so there was always a controller rendered unusable for interrupts.
I attempted to fix this by remodelling how the interrupt hierarchy in
the devicetree is described, with a mux added between the GPIO
controllers and the platform's interrupt controller. v7 introduced an
irqchip driver for the mux between the GPIO controllers and PLIC to handle
that problem.

After some discussion with Linus on v7, I've opted to strip out the
interrupt handling entirely, in order to upstream this piecemeal.
Interrupt controller support will be added at a later date, when I've
sorted out the bits that Thomas did not approve of.

The binding for this GPIO controller is already upstream, so there's
just one patch here now.

Cheers,
Conor.

v7: https://lore.kernel.org/linux-gpio/20240723-supervise-drown-d5d3b303e7fd@wendy/

CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: valentina.fernandezalanis@microchip.com
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Bartosz Golaszewski <brgl@bgdev.pl>
CC: <linux-gpio@vger.kernel.org>

Lewis Hanly (1):
  gpio: mpfs: add polarfire soc gpio support

 drivers/gpio/Kconfig     |   7 ++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-mpfs.c | 178 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+)
 create mode 100644 drivers/gpio/gpio-mpfs.c

-- 
2.45.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support
  2024-10-24  9:20 [PATCH v8 0/1] Polarfire SoC GPIO support Conor Dooley
@ 2024-10-24  9:20 ` Conor Dooley
  2024-10-24  9:57   ` Bartosz Golaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: Conor Dooley @ 2024-10-24  9:20 UTC (permalink / raw)
  To: linux-gpio
  Cc: conor, Conor Dooley, Daire McNamara, valentina.fernandezalanis,
	Linus Walleij, Bartosz Golaszewski, Lewis Hanly

From: Lewis Hanly <lewis.hanly@microchip.com>

Add a driver to support the Polarfire SoC gpio controller. Interrupt
controller support is unavailable for now and will be added at a later
date.

Signed-off-by: Lewis Hanly <lewis.hanly@microchip.com>
Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
v8:
- drop interrupt support
- replace regular mmio acesses with regmap (nice complexity reduction)
- change mpfs_gpio_get() so that it can report non-zero when the line
  direction is output.
---
 drivers/gpio/Kconfig     |   7 ++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-mpfs.c | 178 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+)
 create mode 100644 drivers/gpio/gpio-mpfs.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d93cd4f722b40..fab9c4f73f62b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -549,6 +549,13 @@ config GPIO_PL061
 	help
 	  Say yes here to support the PrimeCell PL061 GPIO device.
 
+config GPIO_POLARFIRE_SOC
+	bool "Microchip FPGA GPIO support"
+	depends on OF_GPIO
+	select REGMAP_MMIO
+	help
+	  Say yes here to support the GPIO controllers on Microchip FPGAs.
+
 config GPIO_PXA
 	bool "PXA GPIO support"
 	depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 1429e8c0229b9..fc66e6388c76c 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_GPIO_PCI_IDIO_16)		+= gpio-pci-idio-16.o
 obj-$(CONFIG_GPIO_PISOSR)		+= gpio-pisosr.o
 obj-$(CONFIG_GPIO_PL061)		+= gpio-pl061.o
 obj-$(CONFIG_GPIO_PMIC_EIC_SPRD)	+= gpio-pmic-eic-sprd.o
+obj-$(CONFIG_GPIO_POLARFIRE_SOC)	+= gpio-mpfs.o
 obj-$(CONFIG_GPIO_PXA)			+= gpio-pxa.o
 obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)	+= gpio-raspberrypi-exp.o
 obj-$(CONFIG_GPIO_RC5T583)		+= gpio-rc5t583.o
diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c
new file mode 100644
index 0000000000000..eaf65ddb6ad73
--- /dev/null
+++ b/drivers/gpio/gpio-mpfs.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/*
+ * Microchip PolarFire SoC (MPFS) GPIO controller driver
+ *
+ * Copyright (c) 2018-2024 Microchip Technology Inc. and its subsidiaries
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+
+#define MPFS_GPIO_CTRL(i)		(0x4 * (i))
+#define MAX_NUM_GPIO			32
+#define MPFS_GPIO_EN_INT		3
+#define MPFS_GPIO_EN_OUT_BUF		BIT(2)
+#define MPFS_GPIO_EN_IN			BIT(1)
+#define MPFS_GPIO_EN_OUT		BIT(0)
+#define MPFS_GPIO_DIR_MASK		GENMASK(2, 0)
+
+#define MPFS_GPIO_TYPE_INT_EDGE_BOTH	0x80
+#define MPFS_GPIO_TYPE_INT_EDGE_NEG	0x60
+#define MPFS_GPIO_TYPE_INT_EDGE_POS	0x40
+#define MPFS_GPIO_TYPE_INT_LEVEL_LOW	0x20
+#define MPFS_GPIO_TYPE_INT_LEVEL_HIGH	0x00
+#define MPFS_GPIO_TYPE_INT_MASK		GENMASK(7, 5)
+#define MPFS_IRQ_REG			0x80
+#define MPFS_INP_REG			0x84
+#define MPFS_OUTP_REG			0x88
+
+struct mpfs_gpio_chip {
+	struct clk *clk;
+	struct regmap *regs;
+	struct gpio_chip gc;
+};
+
+static const struct regmap_config mpfs_gpio_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+};
+
+static int mpfs_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio_index)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+
+	regmap_update_bits(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index),
+			   MPFS_GPIO_DIR_MASK, MPFS_GPIO_EN_IN);
+
+	return 0;
+}
+
+static int mpfs_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio_index, int value)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+
+	regmap_update_bits(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index),
+			   MPFS_GPIO_DIR_MASK, MPFS_GPIO_EN_IN);
+	regmap_update_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index),
+			   value << gpio_index);
+
+	return 0;
+}
+
+static int mpfs_gpio_get_direction(struct gpio_chip *gc,
+				   unsigned int gpio_index)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+	unsigned int gpio_cfg;
+
+	regmap_read(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index), &gpio_cfg);
+	if (gpio_cfg & MPFS_GPIO_EN_IN)
+		return GPIO_LINE_DIRECTION_IN;
+
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mpfs_gpio_get(struct gpio_chip *gc, unsigned int gpio_index)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+
+	if (mpfs_gpio_get_direction(gc, gpio_index) == GPIO_LINE_DIRECTION_OUT)
+		return regmap_test_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index));
+	else
+		return regmap_test_bits(mpfs_gpio->regs, MPFS_INP_REG, BIT(gpio_index));
+}
+
+static void mpfs_gpio_set(struct gpio_chip *gc, unsigned int gpio_index, int value)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+
+	mpfs_gpio_get(gc, gpio_index);
+
+	regmap_update_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index),
+			   value << gpio_index);
+
+	mpfs_gpio_get(gc, gpio_index);
+}
+
+static int mpfs_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mpfs_gpio_chip *mpfs_gpio;
+	struct clk *clk;
+	void __iomem *base;
+	int ret, ngpios;
+
+	mpfs_gpio = devm_kzalloc(dev, sizeof(*mpfs_gpio), GFP_KERNEL);
+	if (!mpfs_gpio)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return dev_err_probe(dev, PTR_ERR(base), "failed to ioremap memory resource\n");
+
+	mpfs_gpio->regs = devm_regmap_init_mmio(dev, base, &mpfs_gpio_regmap_config);
+	if (IS_ERR(mpfs_gpio->regs))
+		return dev_err_probe(dev, PTR_ERR(mpfs_gpio->regs),
+				     "failed to initialise regmap\n");
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "failed to get and enable clock\n");
+
+	mpfs_gpio->clk = clk;
+
+	ngpios = MAX_NUM_GPIO;
+	device_property_read_u32(dev, "ngpios", &ngpios);
+	if (ngpios > MAX_NUM_GPIO)
+		ngpios = MAX_NUM_GPIO;
+
+	mpfs_gpio->gc.direction_input = mpfs_gpio_direction_input;
+	mpfs_gpio->gc.direction_output = mpfs_gpio_direction_output;
+	mpfs_gpio->gc.get_direction = mpfs_gpio_get_direction;
+	mpfs_gpio->gc.get = mpfs_gpio_get;
+	mpfs_gpio->gc.set = mpfs_gpio_set;
+	mpfs_gpio->gc.base = -1;
+	mpfs_gpio->gc.ngpio = ngpios;
+	mpfs_gpio->gc.label = dev_name(dev);
+	mpfs_gpio->gc.parent = dev;
+	mpfs_gpio->gc.owner = THIS_MODULE;
+
+	ret = gpiochip_add_data(&mpfs_gpio->gc, mpfs_gpio);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, mpfs_gpio);
+
+	return 0;
+}
+
+static void mpfs_gpio_remove(struct platform_device *pdev)
+{
+	struct mpfs_gpio_chip *mpfs_gpio = platform_get_drvdata(pdev);
+
+	gpiochip_remove(&mpfs_gpio->gc);
+}
+
+static const struct of_device_id mpfs_gpio_of_ids[] = {
+	{ .compatible = "microchip,mpfs-gpio", },
+	{ /* end of list */ }
+};
+
+static struct platform_driver mpfs_gpio_driver = {
+	.probe = mpfs_gpio_probe,
+	.driver = {
+		.name = "microchip,mpfs-gpio",
+		.of_match_table = mpfs_gpio_of_ids,
+	},
+	.remove = mpfs_gpio_remove,
+};
+builtin_platform_driver(mpfs_gpio_driver);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support
  2024-10-24  9:20 ` [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support Conor Dooley
@ 2024-10-24  9:57   ` Bartosz Golaszewski
  2024-10-24 10:32     ` Conor Dooley
  0 siblings, 1 reply; 4+ messages in thread
From: Bartosz Golaszewski @ 2024-10-24  9:57 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-gpio, Conor Dooley, Daire McNamara,
	valentina.fernandezalanis, Linus Walleij, Lewis Hanly

On Thu, Oct 24, 2024 at 11:21 AM Conor Dooley <conor@kernel.org> wrote:
>
> From: Lewis Hanly <lewis.hanly@microchip.com>
>
> Add a driver to support the Polarfire SoC gpio controller. Interrupt
> controller support is unavailable for now and will be added at a later
> date.
>
> Signed-off-by: Lewis Hanly <lewis.hanly@microchip.com>
> Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> v8:
> - drop interrupt support
> - replace regular mmio acesses with regmap (nice complexity reduction)
> - change mpfs_gpio_get() so that it can report non-zero when the line
>   direction is output.
> ---
>  drivers/gpio/Kconfig     |   7 ++
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-mpfs.c | 178 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 186 insertions(+)
>  create mode 100644 drivers/gpio/gpio-mpfs.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d93cd4f722b40..fab9c4f73f62b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -549,6 +549,13 @@ config GPIO_PL061
>         help
>           Say yes here to support the PrimeCell PL061 GPIO device.
>
> +config GPIO_POLARFIRE_SOC
> +       bool "Microchip FPGA GPIO support"
> +       depends on OF_GPIO

You can drop this. Even if you were using symbols from
drivers/gpio/gpiolib-of.h, they are hidden behind stubs for !OF_GPIO.

> +       select REGMAP_MMIO
> +       help
> +         Say yes here to support the GPIO controllers on Microchip FPGAs.
> +
>  config GPIO_PXA
>         bool "PXA GPIO support"
>         depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 1429e8c0229b9..fc66e6388c76c 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -133,6 +133,7 @@ obj-$(CONFIG_GPIO_PCI_IDIO_16)              += gpio-pci-idio-16.o
>  obj-$(CONFIG_GPIO_PISOSR)              += gpio-pisosr.o
>  obj-$(CONFIG_GPIO_PL061)               += gpio-pl061.o
>  obj-$(CONFIG_GPIO_PMIC_EIC_SPRD)       += gpio-pmic-eic-sprd.o
> +obj-$(CONFIG_GPIO_POLARFIRE_SOC)       += gpio-mpfs.o
>  obj-$(CONFIG_GPIO_PXA)                 += gpio-pxa.o
>  obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)     += gpio-raspberrypi-exp.o
>  obj-$(CONFIG_GPIO_RC5T583)             += gpio-rc5t583.o
> diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c
> new file mode 100644
> index 0000000000000..eaf65ddb6ad73
> --- /dev/null
> +++ b/drivers/gpio/gpio-mpfs.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: (GPL-2.0)
> +/*
> + * Microchip PolarFire SoC (MPFS) GPIO controller driver
> + *
> + * Copyright (c) 2018-2024 Microchip Technology Inc. and its subsidiaries
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/spinlock.h>
> +
> +#define MPFS_GPIO_CTRL(i)              (0x4 * (i))
> +#define MAX_NUM_GPIO                   32

Please use the MPFS prefix everywhere, otherwise it's not clear
whether the define comes from the driver or from GPIO core.

> +#define MPFS_GPIO_EN_INT               3
> +#define MPFS_GPIO_EN_OUT_BUF           BIT(2)
> +#define MPFS_GPIO_EN_IN                        BIT(1)
> +#define MPFS_GPIO_EN_OUT               BIT(0)
> +#define MPFS_GPIO_DIR_MASK             GENMASK(2, 0)
> +
> +#define MPFS_GPIO_TYPE_INT_EDGE_BOTH   0x80
> +#define MPFS_GPIO_TYPE_INT_EDGE_NEG    0x60
> +#define MPFS_GPIO_TYPE_INT_EDGE_POS    0x40
> +#define MPFS_GPIO_TYPE_INT_LEVEL_LOW   0x20
> +#define MPFS_GPIO_TYPE_INT_LEVEL_HIGH  0x00
> +#define MPFS_GPIO_TYPE_INT_MASK                GENMASK(7, 5)
> +#define MPFS_IRQ_REG                   0x80
> +#define MPFS_INP_REG                   0x84
> +#define MPFS_OUTP_REG                  0x88
> +
> +struct mpfs_gpio_chip {
> +       struct clk *clk;
> +       struct regmap *regs;
> +       struct gpio_chip gc;
> +};
> +
> +static const struct regmap_config mpfs_gpio_regmap_config = {
> +       .reg_bits = 32,
> +       .reg_stride = 4,
> +       .val_bits = 32,
> +};
> +
> +static int mpfs_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio_index)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +
> +       regmap_update_bits(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index),
> +                          MPFS_GPIO_DIR_MASK, MPFS_GPIO_EN_IN);
> +
> +       return 0;
> +}
> +
> +static int mpfs_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio_index, int value)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +
> +       regmap_update_bits(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index),
> +                          MPFS_GPIO_DIR_MASK, MPFS_GPIO_EN_IN);
> +       regmap_update_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index),
> +                          value << gpio_index);
> +
> +       return 0;
> +}
> +
> +static int mpfs_gpio_get_direction(struct gpio_chip *gc,
> +                                  unsigned int gpio_index)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +       unsigned int gpio_cfg;
> +
> +       regmap_read(mpfs_gpio->regs, MPFS_GPIO_CTRL(gpio_index), &gpio_cfg);
> +       if (gpio_cfg & MPFS_GPIO_EN_IN)
> +               return GPIO_LINE_DIRECTION_IN;
> +
> +       return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int mpfs_gpio_get(struct gpio_chip *gc, unsigned int gpio_index)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +
> +       if (mpfs_gpio_get_direction(gc, gpio_index) == GPIO_LINE_DIRECTION_OUT)
> +               return regmap_test_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index));
> +       else
> +               return regmap_test_bits(mpfs_gpio->regs, MPFS_INP_REG, BIT(gpio_index));
> +}
> +
> +static void mpfs_gpio_set(struct gpio_chip *gc, unsigned int gpio_index, int value)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +
> +       mpfs_gpio_get(gc, gpio_index);
> +
> +       regmap_update_bits(mpfs_gpio->regs, MPFS_OUTP_REG, BIT(gpio_index),
> +                          value << gpio_index);
> +
> +       mpfs_gpio_get(gc, gpio_index);
> +}
> +
> +static int mpfs_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct mpfs_gpio_chip *mpfs_gpio;
> +       struct clk *clk;
> +       void __iomem *base;
> +       int ret, ngpios;
> +
> +       mpfs_gpio = devm_kzalloc(dev, sizeof(*mpfs_gpio), GFP_KERNEL);
> +       if (!mpfs_gpio)
> +               return -ENOMEM;
> +
> +       base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(base))
> +               return dev_err_probe(dev, PTR_ERR(base), "failed to ioremap memory resource\n");
> +
> +       mpfs_gpio->regs = devm_regmap_init_mmio(dev, base, &mpfs_gpio_regmap_config);
> +       if (IS_ERR(mpfs_gpio->regs))
> +               return dev_err_probe(dev, PTR_ERR(mpfs_gpio->regs),
> +                                    "failed to initialise regmap\n");
> +
> +       clk = devm_clk_get_enabled(dev, NULL);
> +       if (IS_ERR(clk))
> +               return dev_err_probe(dev, PTR_ERR(clk), "failed to get and enable clock\n");
> +
> +       mpfs_gpio->clk = clk;
> +
> +       ngpios = MAX_NUM_GPIO;
> +       device_property_read_u32(dev, "ngpios", &ngpios);
> +       if (ngpios > MAX_NUM_GPIO)
> +               ngpios = MAX_NUM_GPIO;
> +
> +       mpfs_gpio->gc.direction_input = mpfs_gpio_direction_input;
> +       mpfs_gpio->gc.direction_output = mpfs_gpio_direction_output;
> +       mpfs_gpio->gc.get_direction = mpfs_gpio_get_direction;
> +       mpfs_gpio->gc.get = mpfs_gpio_get;
> +       mpfs_gpio->gc.set = mpfs_gpio_set;
> +       mpfs_gpio->gc.base = -1;
> +       mpfs_gpio->gc.ngpio = ngpios;
> +       mpfs_gpio->gc.label = dev_name(dev);
> +       mpfs_gpio->gc.parent = dev;
> +       mpfs_gpio->gc.owner = THIS_MODULE;
> +
> +       ret = gpiochip_add_data(&mpfs_gpio->gc, mpfs_gpio);

Use devm_gpiochip_add_data() and remove remove()?

> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, mpfs_gpio);
> +
> +       return 0;
> +}
> +
> +static void mpfs_gpio_remove(struct platform_device *pdev)
> +{
> +       struct mpfs_gpio_chip *mpfs_gpio = platform_get_drvdata(pdev);
> +
> +       gpiochip_remove(&mpfs_gpio->gc);
> +}
> +
> +static const struct of_device_id mpfs_gpio_of_ids[] = {
> +       { .compatible = "microchip,mpfs-gpio", },
> +       { /* end of list */ }
> +};
> +
> +static struct platform_driver mpfs_gpio_driver = {
> +       .probe = mpfs_gpio_probe,
> +       .driver = {
> +               .name = "microchip,mpfs-gpio",
> +               .of_match_table = mpfs_gpio_of_ids,
> +       },
> +       .remove = mpfs_gpio_remove,
> +};
> +builtin_platform_driver(mpfs_gpio_driver);
> --
> 2.45.2
>

Just a couple nits, otherwise it's short and sweet.

Bart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support
  2024-10-24  9:57   ` Bartosz Golaszewski
@ 2024-10-24 10:32     ` Conor Dooley
  0 siblings, 0 replies; 4+ messages in thread
From: Conor Dooley @ 2024-10-24 10:32 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-gpio, Conor Dooley, Daire McNamara,
	valentina.fernandezalanis, Linus Walleij, Lewis Hanly

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Thu, Oct 24, 2024 at 11:57:04AM +0200, Bartosz Golaszewski wrote:
> On Thu, Oct 24, 2024 at 11:21 AM Conor Dooley <conor@kernel.org> wrote:
> > +       ret = gpiochip_add_data(&mpfs_gpio->gc, mpfs_gpio);
> 
> Use devm_gpiochip_add_data() and remove remove()?

You know, I feel like an idiot. I grepped for this, with the goal of
dropping the remove, and must have typoed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-10-24 10:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-24  9:20 [PATCH v8 0/1] Polarfire SoC GPIO support Conor Dooley
2024-10-24  9:20 ` [PATCH v8 1/1] gpio: mpfs: add polarfire soc gpio support Conor Dooley
2024-10-24  9:57   ` Bartosz Golaszewski
2024-10-24 10:32     ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).