Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable
@ 2024-05-03  6:27 Stefan Wahren
  2024-05-06  6:14 ` Linus Walleij
  2024-05-06 20:31 ` Andy Shevchenko
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Wahren @ 2024-05-03  6:27 UTC (permalink / raw)
  To: Linus Walleij, Florian Fainelli
  Cc: bcm-kernel-feedback-list, Ray Jui, Scott Branden, linux-gpio,
	linux-arm-kernel, Bartosz Golaszewski, Phil Elwell, Kent Gibson,
	Stefan Wahren

Until now after a bcm2835 pin was freed its pinmux was set to GPIO_IN.
So in case it was configured as GPIO_OUT before the configured output
level also get lost. As long as GPIO sysfs was used this wasn't
actually a problem because the pins and their possible output level
were kept by sysfs.

Since more and more Raspberry Pi users start using libgpiod they are
confused about this behavior. So make the pin freeing behavior of
GPIO_OUT configurable via module parameter. In case
pinctrl-bcm2835.persist_gpio_outputs is set to 1, the output level is
kept.

This patch based on the downstream work of Phil Elwell.

Link: https://github.com/raspberrypi/linux/pull/6117
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---

Changes in V2:
- improve parameter name as suggested by Kent Gibson

 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index f5a9372d43bd..b4293a827a89 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -244,6 +244,10 @@ static const char * const irq_type_names[] = {
 	[IRQ_TYPE_LEVEL_LOW] = "level-low",
 };

+static bool persist_gpio_outputs;
+module_param(persist_gpio_outputs, bool, 0644);
+MODULE_PARM_DESC(persist_gpio_outputs, "Enable GPIO_OUT persistence when pin is freed");
+
 static inline u32 bcm2835_gpio_rd(struct bcm2835_pinctrl *pc, unsigned reg)
 {
 	return readl(pc->base + reg);
@@ -926,6 +930,13 @@ static int bcm2835_pmx_free(struct pinctrl_dev *pctldev,
 		unsigned offset)
 {
 	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum bcm2835_fsel fsel = bcm2835_pinctrl_fsel_get(pc, offset);
+
+	if (fsel == BCM2835_FSEL_GPIO_IN)
+		return 0;
+
+	if (persist_gpio_outputs && fsel == BCM2835_FSEL_GPIO_OUT)
+		return 0;

 	/* disable by setting to GPIO_IN */
 	bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN);
@@ -970,10 +981,7 @@ static void bcm2835_pmx_gpio_disable_free(struct pinctrl_dev *pctldev,
 		struct pinctrl_gpio_range *range,
 		unsigned offset)
 {
-	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
-
-	/* disable by setting to GPIO_IN */
-	bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN);
+	bcm2835_pmx_free(pctldev, offset);
 }

 static int bcm2835_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
@@ -1419,6 +1427,9 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		goto out_remove;
 	}

+	dev_info(dev, "GPIO_OUT persistence: %s\n",
+		 persist_gpio_outputs ? "yes" : "no");
+
 	return 0;

 out_remove:
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-03  6:27 [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable Stefan Wahren
@ 2024-05-06  6:14 ` Linus Walleij
  2024-05-06 20:31 ` Andy Shevchenko
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2024-05-06  6:14 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Florian Fainelli, bcm-kernel-feedback-list, Ray Jui,
	Scott Branden, linux-gpio, linux-arm-kernel, Bartosz Golaszewski,
	Phil Elwell, Kent Gibson

On Fri, May 3, 2024 at 8:28 AM Stefan Wahren <wahrenst@gmx.net> wrote:

> Until now after a bcm2835 pin was freed its pinmux was set to GPIO_IN.
> So in case it was configured as GPIO_OUT before the configured output
> level also get lost. As long as GPIO sysfs was used this wasn't
> actually a problem because the pins and their possible output level
> were kept by sysfs.
>
> Since more and more Raspberry Pi users start using libgpiod they are
> confused about this behavior. So make the pin freeing behavior of
> GPIO_OUT configurable via module parameter. In case
> pinctrl-bcm2835.persist_gpio_outputs is set to 1, the output level is
> kept.
>
> This patch based on the downstream work of Phil Elwell.
>
> Link: https://github.com/raspberrypi/linux/pull/6117
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Patch applied!

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-03  6:27 [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable Stefan Wahren
  2024-05-06  6:14 ` Linus Walleij
@ 2024-05-06 20:31 ` Andy Shevchenko
  1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2024-05-06 20:31 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Linus Walleij, Florian Fainelli, bcm-kernel-feedback-list,
	Ray Jui, Scott Branden, linux-gpio, linux-arm-kernel,
	Bartosz Golaszewski, Phil Elwell, Kent Gibson

Fri, May 03, 2024 at 08:27:45AM +0200, Stefan Wahren kirjoitti:
> Until now after a bcm2835 pin was freed its pinmux was set to GPIO_IN.
> So in case it was configured as GPIO_OUT before the configured output
> level also get lost. As long as GPIO sysfs was used this wasn't
> actually a problem because the pins and their possible output level
> were kept by sysfs.
> 
> Since more and more Raspberry Pi users start using libgpiod they are
> confused about this behavior. So make the pin freeing behavior of
> GPIO_OUT configurable via module parameter. In case
> pinctrl-bcm2835.persist_gpio_outputs is set to 1, the output level is
> kept.
> 
> This patch based on the downstream work of Phil Elwell.

...

> +static bool persist_gpio_outputs;
> +module_param(persist_gpio_outputs, bool, 0644);

Do you need 'w' attribute?

...

> +	dev_info(dev, "GPIO_OUT persistence: %s\n",
> +		 persist_gpio_outputs ? "yes" : "no");

str_yes_no() and with it can be put on a single line.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2024-05-06 20:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-03  6:27 [PATCH V2] pinctrl: bcm2835: Make pin freeing behavior configurable Stefan Wahren
2024-05-06  6:14 ` Linus Walleij
2024-05-06 20:31 ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox