From 3c190792955f52ad0ad3a5682090b75dff019d0c Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 7 May 2024 11:54:04 +0100 Subject: [PATCH 1/6] Revert "pinctrl: bcm2835: Add strict_gpiod module parameter" This reverts commit c6ee9316ac1ed750c61d068cecc155d2c744fa1b. --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index 675cf96d7465bd..8b178b8d72420b 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -244,10 +244,6 @@ static const char * const irq_type_names[] = { [IRQ_TYPE_LEVEL_LOW] = "level-low", }; -static bool strict_gpiod; -module_param(strict_gpiod, bool, 0644); -MODULE_PARM_DESC(strict_gpiod, "unless true, outputs remain outputs when freed"); - static inline u32 bcm2835_gpio_rd(struct bcm2835_pinctrl *pc, unsigned reg) { return readl(pc->base + reg); @@ -945,8 +941,8 @@ static int bcm2835_pmx_free(struct pinctrl_dev *pctldev, struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev); enum bcm2835_fsel fsel = bcm2835_pinctrl_fsel_get(pc, offset); - /* Return non-GPIOs to GPIO_IN, unless strict_gpiod is set */ - if (strict_gpiod || (fsel != BCM2835_FSEL_GPIO_IN && fsel != BCM2835_FSEL_GPIO_OUT)) + /* Return non-GPIOs to GPIO_IN */ + if (fsel != BCM2835_FSEL_GPIO_IN && fsel != BCM2835_FSEL_GPIO_OUT) bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN); return 0; From 20ea588962e49328a4007914fed22aa5705929fe Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 7 May 2024 12:01:50 +0100 Subject: [PATCH 2/6] Revert "pinctrl: bcm2835: Only return non-GPIOs to inputs" This reverts commit 2f8da7f53ea26023dbd921edfbfa398d8f3b19d5. --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index 8b178b8d72420b..c53f469b11f60d 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -939,12 +939,9 @@ 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); - - /* Return non-GPIOs to GPIO_IN */ - if (fsel != BCM2835_FSEL_GPIO_IN && fsel != BCM2835_FSEL_GPIO_OUT) - bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN); + /* disable by setting to GPIO_IN */ + bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN); return 0; } @@ -986,7 +983,10 @@ static void bcm2835_pmx_gpio_disable_free(struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned offset) { - (void)bcm2835_pmx_free(pctldev, 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); } static int bcm2835_pmx_gpio_set_direction(struct pinctrl_dev *pctldev, From fb0f5617f6ef224ea8dea061aae693419a4a7c86 Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Fri, 3 May 2024 08:27:45 +0200 Subject: [PATCH 3/6] pinctrl: bcm2835: Make pin freeing behavior configurable commit 8ff05989b44e1a8f7d2bbe67320990ebc2fbb5e5 upstream. 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 Message-ID: <20240503062745.11298-1-wahrenst@gmx.net> Signed-off-by: Linus Walleij --- 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 c53f469b11f60d..a36df62108c38e 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); @@ -939,6 +943,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); @@ -983,10 +994,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, @@ -1374,6 +1382,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: From ee914349b253efc96980d53a49a96d044da53d5e Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 7 May 2024 12:14:12 +0100 Subject: [PATCH 4/6] pinctrl: bcm2835: Persist outputs by default Having accepted the upstream change to add the persist_gpio_outputs parameter, make it true by default. See: https://github.com/raspberrypi/linux/pull/6117 Signed-off-by: Phil Elwell --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index a36df62108c38e..cfb51cf2f7662b 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -244,7 +244,7 @@ static const char * const irq_type_names[] = { [IRQ_TYPE_LEVEL_LOW] = "level-low", }; -static bool persist_gpio_outputs; +static bool persist_gpio_outputs = true; module_param(persist_gpio_outputs, bool, 0644); MODULE_PARM_DESC(persist_gpio_outputs, "Enable GPIO_OUT persistence when pin is freed"); From 9d0976209f9ddff29197fd68f204f1a38a4de80e Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 7 May 2024 13:33:48 +0100 Subject: [PATCH 5/6] pinctrl: rp1: Use persist_gpio_outputs Following 8ff05989b44e1a8f7d2bbe67320990ebc2fbb5e5, adopt the same parameter name but with the opposite default. Signed-off-by: Phil Elwell --- drivers/pinctrl/pinctrl-rp1.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c index 292051168b61f7..6684b171e98b5d 100644 --- a/drivers/pinctrl/pinctrl-rp1.c +++ b/drivers/pinctrl/pinctrl-rp1.c @@ -573,9 +573,9 @@ static const char * const irq_type_names[] = { [IRQ_TYPE_LEVEL_LOW] = "level-low", }; -static bool strict_gpiod; -module_param(strict_gpiod, bool, 0644); -MODULE_PARM_DESC(strict_gpiod, "unless true, outputs remain outputs when freed"); +static bool persist_gpio_outputs = true; +module_param(persist_gpio_outputs, bool, 0644); +MODULE_PARM_DESC(persist_gpio_outputs, "Enable GPIO_OUT persistence when pin is freed"); static int rp1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int offset, unsigned long *configs, @@ -1205,11 +1205,12 @@ static int rp1_pmx_free(struct pinctrl_dev *pctldev, unsigned offset) struct rp1_pin_info *pin = rp1_get_pin_pctl(pctldev, offset); u32 fsel = rp1_get_fsel(pin); - /* Return non-GPIOs to GPIO_IN, unless strict_gpiod is set */ - if (strict_gpiod || fsel != RP1_FSEL_GPIO) { - rp1_set_dir(pin, RP1_DIR_INPUT); - rp1_set_fsel(pin, RP1_FSEL_GPIO); - } + /* Return all pins to GPIO_IN, unless persist_gpio_outputs is set */ + if (persist_gpio_outputs && fsel == RP1_FSEL_GPIO) + return 0; + + rp1_set_dir(pin, RP1_DIR_INPUT); + rp1_set_fsel(pin, RP1_FSEL_GPIO); return 0; } From 8c4909d621739b3fa73936219b6f23fe5aaea518 Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 7 May 2024 13:38:21 +0100 Subject: [PATCH 6/6] ARM: dts: Update strict_gpiod dtparams Following the adoption upstream of a similar parameter but with another name, update the dtparam definitions to use the new name and sense. Signed-off-by: Phil Elwell --- arch/arm/boot/dts/broadcom/bcm270x-rpi.dtsi | 2 +- arch/arm/boot/dts/broadcom/bcm2712-rpi.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/broadcom/bcm270x-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm270x-rpi.dtsi index f997d908e4b4ce..bc533f32964067 100644 --- a/arch/arm/boot/dts/broadcom/bcm270x-rpi.dtsi +++ b/arch/arm/boot/dts/broadcom/bcm270x-rpi.dtsi @@ -111,7 +111,7 @@ <&csi0>, "sync-gpios:4", <&csi0>, "sync-gpios:8=0", ; - strict_gpiod = <&chosen>, "bootargs=pinctrl_bcm2835.strict_gpiod=y"; + strict_gpiod = <&chosen>, "bootargs=pinctrl_bcm2835.persist_gpio_outputs=n"; }; }; diff --git a/arch/arm/boot/dts/broadcom/bcm2712-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm2712-rpi.dtsi index 89d12634f20b81..f88de824e9d7d7 100644 --- a/arch/arm/boot/dts/broadcom/bcm2712-rpi.dtsi +++ b/arch/arm/boot/dts/broadcom/bcm2712-rpi.dtsi @@ -99,7 +99,7 @@ nvmem_cust_rw = <&nvmem_cust>,"rw?"; nvmem_priv_rw = <&nvmem_priv>,"rw?"; nvmem_mac_rw = <&nvmem_mac>,"rw?"; - strict_gpiod = <&chosen>, "bootargs=pinctrl_rp1.strict_gpiod=y"; + strict_gpiod = <&chosen>, "bootargs=pinctrl_rp1.persist_gpio_outputs=n"; }; };