Linux-GPIO Archive mirror
 help / color / mirror / Atom feed
* [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
@ 2024-04-19 20:40 Stefan Wahren
  2024-05-02 10:22 ` Stefan Wahren
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Wahren @ 2024-04-19 20:40 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.strict_gpiod is set to 0, 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>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index f5a9372d43bd..a5b2096c97fc 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 strict_gpiod = true;
+module_param(strict_gpiod, bool, 0644);
+MODULE_PARM_DESC(strict_gpiod, "unless true, GPIO_OUT remain when pin freed");
+
 static inline u32 bcm2835_gpio_rd(struct bcm2835_pinctrl *pc, unsigned reg)
 {
 	return readl(pc->base + reg);
@@ -926,6 +930,14 @@ 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;
+
+	/* preserve GPIO_OUT in non-strict mode */
+	if (!strict_gpiod && 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 +982,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 +1428,9 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		goto out_remove;
 	}

+	dev_info(dev, "GPIO_OUT remain when pin freed: %s\n",
+		 strict_gpiod ? "no" : "yes");
+
 	return 0;

 out_remove:
--
2.34.1


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

* Re: [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-04-19 20:40 [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable Stefan Wahren
@ 2024-05-02 10:22 ` Stefan Wahren
  2024-05-02 11:04   ` Kent Gibson
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Wahren @ 2024-05-02 10:22 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

Am 19.04.24 um 22:40 schrieb 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.strict_gpiod is set to 0, 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>
> ---
>   drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
Gentle ping ...

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

* Re: [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-02 10:22 ` Stefan Wahren
@ 2024-05-02 11:04   ` Kent Gibson
  2024-05-02 11:11     ` Stefan Wahren
  0 siblings, 1 reply; 6+ messages in thread
From: Kent Gibson @ 2024-05-02 11:04 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

On Thu, May 02, 2024 at 12:22:07PM +0200, Stefan Wahren wrote:
> Am 19.04.24 um 22:40 schrieb 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.strict_gpiod is set to 0, 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>
> > ---
> >   drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
> >   1 file changed, 16 insertions(+), 4 deletions(-)
> Gentle ping ...

I can't comment on the substance of the change as pinctrl is outside my
wheelhouse, but the "strict_gpiod" name could be better.
The point is to make GPIO outputs persist, right?
The name should better reflect that.

Cheers,
Kent.



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

* Re: [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-02 11:04   ` Kent Gibson
@ 2024-05-02 11:11     ` Stefan Wahren
  2024-05-02 11:15       ` Kent Gibson
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Wahren @ 2024-05-02 11:11 UTC (permalink / raw
  To: Kent Gibson
  Cc: Linus Walleij, Florian Fainelli, bcm-kernel-feedback-list,
	Ray Jui, Scott Branden, linux-gpio, linux-arm-kernel,
	Bartosz Golaszewski, Phil Elwell

Hi Kent,

Am 02.05.24 um 13:04 schrieb Kent Gibson:
> On Thu, May 02, 2024 at 12:22:07PM +0200, Stefan Wahren wrote:
>> Am 19.04.24 um 22:40 schrieb 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.strict_gpiod is set to 0, 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>
>>> ---
>>>    drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
>>>    1 file changed, 16 insertions(+), 4 deletions(-)
>> Gentle ping ...
> I can't comment on the substance of the change as pinctrl is outside my
> wheelhouse, but the "strict_gpiod" name could be better.
> The point is to make GPIO outputs persist, right?
Yes, correct.
> The name should better reflect that.
Finding good and short names is hard, do you have a suggestion?

Thanks
>
> Cheers,
> Kent.
>
>
>
> _______________________________________________
> 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] 6+ messages in thread

* Re: [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-02 11:11     ` Stefan Wahren
@ 2024-05-02 11:15       ` Kent Gibson
  2024-05-02 12:20         ` Stefan Wahren
  0 siblings, 1 reply; 6+ messages in thread
From: Kent Gibson @ 2024-05-02 11:15 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

On Thu, May 02, 2024 at 01:11:06PM +0200, Stefan Wahren wrote:
> Hi Kent,
>
> Am 02.05.24 um 13:04 schrieb Kent Gibson:
> > On Thu, May 02, 2024 at 12:22:07PM +0200, Stefan Wahren wrote:
> > > Am 19.04.24 um 22:40 schrieb 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.strict_gpiod is set to 0, 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>
> > > > ---
> > > >    drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
> > > >    1 file changed, 16 insertions(+), 4 deletions(-)
> > > Gentle ping ...
> > I can't comment on the substance of the change as pinctrl is outside my
> > wheelhouse, but the "strict_gpiod" name could be better.
> > The point is to make GPIO outputs persist, right?
> Yes, correct.
> > The name should better reflect that.
> Finding good and short names is hard, do you have a suggestion?
>

How about "persist_gpio_outputs"?

Cheers,
Kent.

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

* Re: [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable
  2024-05-02 11:15       ` Kent Gibson
@ 2024-05-02 12:20         ` Stefan Wahren
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Wahren @ 2024-05-02 12:20 UTC (permalink / raw
  To: Kent Gibson
  Cc: Linus Walleij, Florian Fainelli, bcm-kernel-feedback-list,
	Ray Jui, Scott Branden, linux-gpio, linux-arm-kernel,
	Bartosz Golaszewski, Phil Elwell

Am 02.05.24 um 13:15 schrieb Kent Gibson:
> On Thu, May 02, 2024 at 01:11:06PM +0200, Stefan Wahren wrote:
>> Hi Kent,
>>
>> Am 02.05.24 um 13:04 schrieb Kent Gibson:
>>> On Thu, May 02, 2024 at 12:22:07PM +0200, Stefan Wahren wrote:
>>>> Am 19.04.24 um 22:40 schrieb 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.strict_gpiod is set to 0, 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>
>>>>> ---
>>>>>     drivers/pinctrl/bcm/pinctrl-bcm2835.c | 20 ++++++++++++++++----
>>>>>     1 file changed, 16 insertions(+), 4 deletions(-)
>>>> Gentle ping ...
>>> I can't comment on the substance of the change as pinctrl is outside my
>>> wheelhouse, but the "strict_gpiod" name could be better.
>>> The point is to make GPIO outputs persist, right?
>> Yes, correct.
>>> The name should better reflect that.
>> Finding good and short names is hard, do you have a suggestion?
>>
> How about "persist_gpio_outputs"?
I'm fine with that. I'll wait until tomorrow before sending a new version.
>
> Cheers,
> Kent.


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

end of thread, other threads:[~2024-05-02 12:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 20:40 [PATCH] pinctrl: bcm2835: Make pin freeing behavior configurable Stefan Wahren
2024-05-02 10:22 ` Stefan Wahren
2024-05-02 11:04   ` Kent Gibson
2024-05-02 11:11     ` Stefan Wahren
2024-05-02 11:15       ` Kent Gibson
2024-05-02 12:20         ` Stefan Wahren

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 read-only IMAP folder(s) and NNTP newsgroup(s).