[PATCH 0/2] pinctrl: bcm63xx: properly request gpios

[PATCH 0/2] pinctrl: bcm63xx: properly request gpios

[Álvaro Fernández Rojas]

Add gpio_chip request/free functions to gpio-regmap and use them on bcm63xx in
order to properly call gpio_request_enable when requesting a gpio.

Álvaro Fernández Rojas (2):
  gpio: regmap: add request/free gpio_chip functions
  pinctrl: bcm63xx: implement gpio_regmap request/free

 drivers/gpio/gpio-regmap.c            |  2 ++
 drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 14 ++++++++++++++
 include/linux/gpio/regmap.h           |  4 ++++
 3 files changed, 20 insertions(+)

-- 
2.39.5

[PATCH 1/2] gpio: regmap: add request/free gpio_chip functions

[Álvaro Fernández Rojas]

Allow configuring gpio_chip request/free functions when creating a gpio-regmap.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/gpio/gpio-regmap.c  | 2 ++
 include/linux/gpio/regmap.h | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 71684dee2ca5..32ec85b41653 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	chip->names = config->names;
 	chip->label = config->label ?: dev_name(config->parent);
 	chip->can_sleep = regmap_might_sleep(config->regmap);
+	chip->request = config->request;
+	chip->free = config->free;
 
 	chip->get = gpio_regmap_get;
 	if (gpio->reg_set_base && gpio->reg_clr_base)
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index a9f7b7faf57b..cd4ea9661eea 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -5,6 +5,7 @@
 
 struct device;
 struct fwnode_handle;
+struct gpio_chip;
 struct gpio_regmap;
 struct irq_domain;
 struct regmap;
@@ -82,6 +83,9 @@ struct gpio_regmap_config {
 			      unsigned int offset, unsigned int *reg,
 			      unsigned int *mask);
 
+	int (*request)(struct gpio_chip *chip, unsigned int offset);
+	void (*free)(struct gpio_chip *chip, unsigned int offset);
+
 	void *drvdata;
 };
 
-- 
2.39.5

Re: [PATCH 1/2] gpio: regmap: add request/free gpio_chip functions

[Jonas Gorski]

Hi,

On Tue, Jan 7, 2025 at 11:27 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Allow configuring gpio_chip request/free functions when creating a gpio-regmap.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/gpio/gpio-regmap.c  | 2 ++
>  include/linux/gpio/regmap.h | 4 ++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 71684dee2ca5..32ec85b41653 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
>         chip->names = config->names;
>         chip->label = config->label ?: dev_name(config->parent);
>         chip->can_sleep = regmap_might_sleep(config->regmap);
> +       chip->request = config->request;
> +       chip->free = config->free;

I wonder if you couldn't just use gpiochip_generic_request() /
gpiochip_generic_free() unconditionally here. AFAIU, these don't do
anything when there are no GPIO ranges defined (so should not
interfere with non-pinctrl linked devices), as well as nothing when
CONFIG_PINCTRL isn't enabled, so they should be NOPs if there is no
pinctrl link, and do the right thing if there is one.

Best Regards,
Jonas

Re: [PATCH 1/2] gpio: regmap: add request/free gpio_chip functions

[Álvaro Fernández Rojas]

El mar, 7 ene 2025 a las 11:43, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> Hi,
>
> On Tue, Jan 7, 2025 at 11:27 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> >
> > Allow configuring gpio_chip request/free functions when creating a gpio-regmap.
> >
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >  drivers/gpio/gpio-regmap.c  | 2 ++
> >  include/linux/gpio/regmap.h | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 71684dee2ca5..32ec85b41653 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
> >         chip->names = config->names;
> >         chip->label = config->label ?: dev_name(config->parent);
> >         chip->can_sleep = regmap_might_sleep(config->regmap);
> > +       chip->request = config->request;
> > +       chip->free = config->free;
>
> I wonder if you couldn't just use gpiochip_generic_request() /
> gpiochip_generic_free() unconditionally here. AFAIU, these don't do
> anything when there are no GPIO ranges defined (so should not
> interfere with non-pinctrl linked devices), as well as nothing when
> CONFIG_PINCTRL isn't enabled, so they should be NOPs if there is no
> pinctrl link, and do the right thing if there is one.
>
> Best Regards,
> Jonas

@Jonas Your proposal is now used in OpenWrt for realtek
https://github.com/openwrt/openwrt/blob/6ef6014887c393dc07f0349028d93e4fa82e0733/target/linux/realtek/patches-6.6/801-gpio-regmap-Use-generic-request-free-ops.patch,
so I guess that we could send that patch to linux-gpio instead.

@Sander can you send that patch to linux-gpio? We need it for bmips too:
https://github.com/openwrt/openwrt/pull/17487

Best regards,
Álvaro.

[PATCH 2/2] pinctrl: bcm63xx: implement gpio_regmap request/free

[Álvaro Fernández Rojas]

Implement pinctrl gpio request/free functions on gpio_regmap, which ensures
calling gpio_request_enable on bcm63xx pinctrl drivers when a gpio is
requested.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
index 59d2ce8462d8..4abd52613dfe 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
@@ -6,10 +6,12 @@
  * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
  */
 
+#include <linux/gpio/driver.h>
 #include <linux/gpio/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mod_devicetable.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 
 #include "pinctrl-bcm63xx.h"
@@ -32,6 +34,16 @@ static int bcm63xx_reg_mask_xlate(struct gpio_regmap *gpio,
 	return 0;
 }
 
+static int bcm63xx_request(struct gpio_chip *chip, unsigned int offset)
+{
+	return pinctrl_gpio_request(chip, offset);
+}
+
+static void bcm63xx_free(struct gpio_chip *chip, unsigned int offset)
+{
+	pinctrl_gpio_free(chip, offset);
+}
+
 static const struct of_device_id bcm63xx_gpio_of_match[] = {
 	{ .compatible = "brcm,bcm6318-gpio", },
 	{ .compatible = "brcm,bcm6328-gpio", },
@@ -57,6 +69,8 @@ static int bcm63xx_gpio_probe(struct device *dev, struct device_node *node,
 	grc.reg_dir_out_base = BCM63XX_DIROUT_REG;
 	grc.reg_set_base = BCM63XX_DATA_REG;
 	grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
+	grc.request = bcm63xx_request;
+	grc.free = bcm63xx_free;
 
 	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &grc));
 }
-- 
2.39.5

Re: [PATCH 0/2] pinctrl: bcm63xx: properly request gpios

[Linus Walleij]

On Tue, Jan 7, 2025 at 11:27 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:

> Add gpio_chip request/free functions to gpio-regmap and use them on bcm63xx in
> order to properly call gpio_request_enable when requesting a gpio.

It's a bit hacky, Sander found the silver bullet patch and Bartosz has
merged it! But thanks for looking after this issue.

Yours,
Linus Walleij