[PATCH v2 0/2] pinctrl: bcm2835: Fix gpio hogs and pin reinitialisation

[PATCH v2 0/2] pinctrl: bcm2835: Fix gpio hogs and pin reinitialisation

[Phil Elwell]

Tackle two problems with the pinctrl-bcm2835 driver and its Device Tree
configuration:

1. The pinctrl-bcm2835 driver is a combined pinctrl/gpio driver.
Currently the gpio side is registered first, but this breaks gpio hogs
(which are configured during gpiochip_add_data).

2. Since [1], a "gpio-ranges" property is required in order for pins
to be returned to inputs when freed. Note that without patch 1, the
device never gets out of EPROBE_DEFER.

Note that the Fixes: tags are little more than hooks to hang the back-ports
on - no blame is intended.

[1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
    pin-ranges")

Changes in v2:
* Removed wrapping of Fixes: tags.
* Added Reviewed-by from Linus, who asks that Patch 2 be merged through the
  Broadcom/SoC tree.
* Corrected Nicolas's email address.

Phil Elwell (2):
  pinctrl: bcm2835: Change init order for gpio hogs
  ARM: dts: gpio-ranges property is now required

 arch/arm/boot/dts/bcm2711.dtsi        |  2 ++
 arch/arm/boot/dts/bcm283x.dtsi        |  2 ++
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 29 +++++++++++++++------------
 3 files changed, 20 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Phil Elwell]

...and gpio-ranges

pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
side is registered first, but this breaks gpio hogs (which are
configured during gpiochip_add_data). Part of the hog initialisation
is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
yet been registered this results in an -EPROBE_DEFER from which it can
never recover.

Change the initialisation sequence to register the pinctrl driver
first.

This also solves a similar problem with the gpio-ranges property, which
is required in order for released pins to be returned to inputs.

Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
Signed-off-by: Phil Elwell <phil@raspberrypi.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 29 +++++++++++++++------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 2abcc6ce4eba3..b607d10e4cbd8 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1244,6 +1244,18 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		raw_spin_lock_init(&pc->irq_lock[i]);
 	}
 
+	pc->pctl_desc = *pdata->pctl_desc;
+	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+	if (IS_ERR(pc->pctl_dev)) {
+		gpiochip_remove(&pc->gpio_chip);
+		return PTR_ERR(pc->pctl_dev);
+	}
+
+	pc->gpio_range = *pdata->gpio_range;
+	pc->gpio_range.base = pc->gpio_chip.base;
+	pc->gpio_range.gc = &pc->gpio_chip;
+	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
+
 	girq = &pc->gpio_chip.irq;
 	girq->chip = &bcm2835_gpio_irq_chip;
 	girq->parent_handler = bcm2835_gpio_irq_handler;
@@ -1251,8 +1263,10 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	girq->parents = devm_kcalloc(dev, BCM2835_NUM_IRQS,
 				     sizeof(*girq->parents),
 				     GFP_KERNEL);
-	if (!girq->parents)
+	if (!girq->parents) {
+		pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
 		return -ENOMEM;
+	}
 
 	if (is_7211) {
 		pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
@@ -1307,21 +1321,10 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	err = gpiochip_add_data(&pc->gpio_chip, pc);
 	if (err) {
 		dev_err(dev, "could not add GPIO chip\n");
+		pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
 		return err;
 	}
 
-	pc->pctl_desc = *pdata->pctl_desc;
-	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
-	if (IS_ERR(pc->pctl_dev)) {
-		gpiochip_remove(&pc->gpio_chip);
-		return PTR_ERR(pc->pctl_dev);
-	}
-
-	pc->gpio_range = *pdata->gpio_range;
-	pc->gpio_range.base = pc->gpio_chip.base;
-	pc->gpio_range.gc = &pc->gpio_chip;
-	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
-
 	return 0;
 }
 
-- 
2.25.1

[PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Phil Elwell]

Since [1], added in 5.7, the absence of a gpio-ranges property has
prevented GPIOs from being restored to inputs when released.
Add those properties for BCM283x and BCM2711 devices.

[1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
    pin-ranges")

Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
Signed-off-by: Phil Elwell <phil@raspberrypi.com>
---
 arch/arm/boot/dts/bcm2711.dtsi | 2 ++
 arch/arm/boot/dts/bcm283x.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 9e01dbca4a011..dff18fc9a9065 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -582,6 +582,8 @@ &gpio {
 		     <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
 		     <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
 
+	gpio-ranges = <&gpio 0 0 58>;
+
 	gpclk0_gpio49: gpclk0_gpio49 {
 		pin-gpclk {
 			pins = "gpio49";
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index a3e06b6809476..c113661a6668f 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -126,6 +126,8 @@ gpio: gpio@7e200000 {
 			interrupt-controller;
 			#interrupt-cells = <2>;
 
+			gpio-ranges = <&gpio 0 0 54>;
+
 			/* Defines common pin muxing groups
 			 *
 			 * While each pin can have its mux selected
-- 
2.25.1

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Linus Walleij]

On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:

> Since [1], added in 5.7, the absence of a gpio-ranges property has
> prevented GPIOs from being restored to inputs when released.
> Add those properties for BCM283x and BCM2711 devices.
>
> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>     pin-ranges")
>
> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Please funnel this patch through the SoC tree.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Florian Fainelli]

On 12/6/21 2:33 AM, Linus Walleij wrote:
> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
> 
>> Since [1], added in 5.7, the absence of a gpio-ranges property has
>> prevented GPIOs from being restored to inputs when released.
>> Add those properties for BCM283x and BCM2711 devices.
>>
>> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>     pin-ranges")
>>
>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Please funnel this patch through the SoC tree.

This one was definitively going to go via ARM SoC in the absence of any
explicit routing, did you mean that patch #1 should also be routed via
ARM SoC?
-- 
Florian

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Linus Walleij]

On Mon, Dec 6, 2021 at 6:25 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
[Me]
> > Please funnel this patch through the SoC tree.
>
> This one was definitively going to go via ARM SoC in the absence of any
> explicit routing, did you mean that patch #1 should also be routed via
> ARM SoC?

No just over-clarifying.
I will merge patch #1 after some review slack.

Yours,
Linus Walleij

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Florian Fainelli]

On 12/6/21 1:22 AM, Phil Elwell wrote:
> ...and gpio-ranges
> 
> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
> side is registered first, but this breaks gpio hogs (which are
> configured during gpiochip_add_data). Part of the hog initialisation
> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
> yet been registered this results in an -EPROBE_DEFER from which it can
> never recover.
> 
> Change the initialisation sequence to register the pinctrl driver
> first.
> 
> This also solves a similar problem with the gpio-ranges property, which
> is required in order for released pins to be returned to inputs.
> 
> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Linus Walleij]

On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:

> ...and gpio-ranges
>
> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
> side is registered first, but this breaks gpio hogs (which are
> configured during gpiochip_add_data). Part of the hog initialisation
> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
> yet been registered this results in an -EPROBE_DEFER from which it can
> never recover.
>
> Change the initialisation sequence to register the pinctrl driver
> first.
>
> This also solves a similar problem with the gpio-ranges property, which
> is required in order for released pins to be returned to inputs.
>
> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

This patch (1/2) applied for fixes.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[nicolas saenz julienne]

On Mon, 2021-12-06 at 11:33 +0100, Linus Walleij wrote:
> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
> 
> > Since [1], added in 5.7, the absence of a gpio-ranges property has
> > prevented GPIOs from being restored to inputs when released.
> > Add those properties for BCM283x and BCM2711 devices.
> > 
> > [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
> >     pin-ranges")
> > 
> > Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
> > Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Please funnel this patch through the SoC tree.

Applied for fixes.

Thanks,
Nicolas

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Marek Szyprowski]

Hi Phil,

On 06.12.2021 10:22, Phil Elwell wrote:
> Since [1], added in 5.7, the absence of a gpio-ranges property has
> prevented GPIOs from being restored to inputs when released.
> Add those properties for BCM283x and BCM2711 devices.
>
> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>      pin-ranges")
>
> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

This patch breaks today's linux-next (next-20211214) on RPi3 and RPi4. 
Either there is something missing or wrong here. Booting stops after 
following messages (on RPi4):

[    3.186786] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
[    3.234513] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
[    3.276703] mmc0: SDHCI controller on fe340000.mmc [fe340000.mmc] 
using ADMA
[    3.287191] pinctrl-bcm2835 fe200000.gpio

> ---
>   arch/arm/boot/dts/bcm2711.dtsi | 2 ++
>   arch/arm/boot/dts/bcm283x.dtsi | 2 ++
>   2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 9e01dbca4a011..dff18fc9a9065 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -582,6 +582,8 @@ &gpio {
>   		     <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
>   		     <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
>   
> +	gpio-ranges = <&gpio 0 0 58>;
> +
>   	gpclk0_gpio49: gpclk0_gpio49 {
>   		pin-gpclk {
>   			pins = "gpio49";
> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> index a3e06b6809476..c113661a6668f 100644
> --- a/arch/arm/boot/dts/bcm283x.dtsi
> +++ b/arch/arm/boot/dts/bcm283x.dtsi
> @@ -126,6 +126,8 @@ gpio: gpio@7e200000 {
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
>   
> +			gpio-ranges = <&gpio 0 0 54>;
> +
>   			/* Defines common pin muxing groups
>   			 *
>   			 * While each pin can have its mux selected

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Phil Elwell]

Hi Marek,

On 14/12/2021 14:21, Marek Szyprowski wrote:
> Hi Phil,
> 
> On 06.12.2021 10:22, Phil Elwell wrote:
>> Since [1], added in 5.7, the absence of a gpio-ranges property has
>> prevented GPIOs from being restored to inputs when released.
>> Add those properties for BCM283x and BCM2711 devices.
>>
>> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>       pin-ranges")
>>
>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> This patch breaks today's linux-next (next-20211214) on RPi3 and RPi4.
> Either there is something missing or wrong here. Booting stops after
> following messages (on RPi4):
> 
> [    3.186786] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
> [    3.234513] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
> [    3.276703] mmc0: SDHCI controller on fe340000.mmc [fe340000.mmc]
> using ADMA
> [    3.287191] pinctrl-bcm2835 fe200000.gpio

This patch is part of a two-patch set, the cover note for which says:

     2. Since [1], a "gpio-ranges" property is required in order for pins
     to be returned to inputs when freed. Note that without patch 1, the
     device never gets out of EPROBE_DEFER.

It looks as though patch 2 has been merged without/before patch 1
("pinctrl: bcm2835: Change init order for gpio hogs").

Phil

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Florian Fainelli]

On 12/14/21 6:32 AM, Phil Elwell wrote:
> Hi Marek,
> 
> On 14/12/2021 14:21, Marek Szyprowski wrote:
>> Hi Phil,
>>
>> On 06.12.2021 10:22, Phil Elwell wrote:
>>> Since [1], added in 5.7, the absence of a gpio-ranges property has
>>> prevented GPIOs from being restored to inputs when released.
>>> Add those properties for BCM283x and BCM2711 devices.
>>>
>>> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>>       pin-ranges")
>>>
>>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>> pin-ranges")
>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>
>> This patch breaks today's linux-next (next-20211214) on RPi3 and RPi4.
>> Either there is something missing or wrong here. Booting stops after
>> following messages (on RPi4):
>>
>> [    3.186786] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
>> [    3.234513] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
>> [    3.276703] mmc0: SDHCI controller on fe340000.mmc [fe340000.mmc]
>> using ADMA
>> [    3.287191] pinctrl-bcm2835 fe200000.gpio
> 
> This patch is part of a two-patch set, the cover note for which says:
> 
>     2. Since [1], a "gpio-ranges" property is required in order for pins
>     to be returned to inputs when freed. Note that without patch 1, the
>     device never gets out of EPROBE_DEFER.
> 
> It looks as though patch 2 has been merged without/before patch 1
> ("pinctrl: bcm2835: Change init order for gpio hogs").

Yes, the hope was that there would be no such breakage, I suppose we
will have to work out a plan to address that and coordinate both changes
landing in at the same time.

I will work with Arnd to back out the Device Tree changes, sorry about that.
-- 
Florian

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[nicolas saenz julienne]

Florian,

On Tue, 2021-12-14 at 09:12 -0800, Florian Fainelli wrote:
> On 12/14/21 6:32 AM, Phil Elwell wrote:
> > Hi Marek,
> > 
> > On 14/12/2021 14:21, Marek Szyprowski wrote:
> > > Hi Phil,
> > > 
> > > On 06.12.2021 10:22, Phil Elwell wrote:
> > > > Since [1], added in 5.7, the absence of a gpio-ranges property has
> > > > prevented GPIOs from being restored to inputs when released.
> > > > Add those properties for BCM283x and BCM2711 devices.
> > > > 
> > > > [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
> > > >       pin-ranges")
> > > > 
> > > > Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without
> > > > pin-ranges")
> > > > Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> > > 
> > > This patch breaks today's linux-next (next-20211214) on RPi3 and RPi4.
> > > Either there is something missing or wrong here. Booting stops after
> > > following messages (on RPi4):
> > > 
> > > [    3.186786] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
> > > [    3.234513] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
> > > [    3.276703] mmc0: SDHCI controller on fe340000.mmc [fe340000.mmc]
> > > using ADMA
> > > [    3.287191] pinctrl-bcm2835 fe200000.gpio
> > 
> > This patch is part of a two-patch set, the cover note for which says:
> > 
> >     2. Since [1], a "gpio-ranges" property is required in order for pins
> >     to be returned to inputs when freed. Note that without patch 1, the
> >     device never gets out of EPROBE_DEFER.
> > 
> > It looks as though patch 2 has been merged without/before patch 1
> > ("pinctrl: bcm2835: Change init order for gpio hogs").
> 
> Yes, the hope was that there would be no such breakage, I suppose we
> will have to work out a plan to address that and coordinate both changes
> landing in at the same time.
> 
> I will work with Arnd to back out the Device Tree changes, sorry about that.

This is linux-next, so I can back out the DT change myself. Sorry for the
breakage.

As for channeling the path, would it make sense for linusw to take it alonside
GPIO fix?

Regards,
Nicolas

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Florian Fainelli]

On 12/15/21 1:02 AM, nicolas saenz julienne wrote:
> Florian,
> 
> On Tue, 2021-12-14 at 09:12 -0800, Florian Fainelli wrote:
>> On 12/14/21 6:32 AM, Phil Elwell wrote:
>>> Hi Marek,
>>>
>>> On 14/12/2021 14:21, Marek Szyprowski wrote:
>>>> Hi Phil,
>>>>
>>>> On 06.12.2021 10:22, Phil Elwell wrote:
>>>>> Since [1], added in 5.7, the absence of a gpio-ranges property has
>>>>> prevented GPIOs from being restored to inputs when released.
>>>>> Add those properties for BCM283x and BCM2711 devices.
>>>>>
>>>>> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>>>>       pin-ranges")
>>>>>
>>>>> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without
>>>>> pin-ranges")
>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>
>>>> This patch breaks today's linux-next (next-20211214) on RPi3 and RPi4.
>>>> Either there is something missing or wrong here. Booting stops after
>>>> following messages (on RPi4):
>>>>
>>>> [    3.186786] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
>>>> [    3.234513] pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
>>>> [    3.276703] mmc0: SDHCI controller on fe340000.mmc [fe340000.mmc]
>>>> using ADMA
>>>> [    3.287191] pinctrl-bcm2835 fe200000.gpio
>>>
>>> This patch is part of a two-patch set, the cover note for which says:
>>>
>>>     2. Since [1], a "gpio-ranges" property is required in order for pins
>>>     to be returned to inputs when freed. Note that without patch 1, the
>>>     device never gets out of EPROBE_DEFER.
>>>
>>> It looks as though patch 2 has been merged without/before patch 1
>>> ("pinctrl: bcm2835: Change init order for gpio hogs").
>>
>> Yes, the hope was that there would be no such breakage, I suppose we
>> will have to work out a plan to address that and coordinate both changes
>> landing in at the same time.
>>
>> I will work with Arnd to back out the Device Tree changes, sorry about that.
> 
> This is linux-next, so I can back out the DT change myself. Sorry for the
> breakage.
> 
> As for channeling the path, would it make sense for linusw to take it alonside
> GPIO fix?

That would definitively work, Linus, are you comfortable with doing
that? I will reply to the patch with an Acked-by if that helps.
-- 
Florian

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Florian Fainelli]

On 12/6/21 1:22 AM, Phil Elwell wrote:
> Since [1], added in 5.7, the absence of a gpio-ranges property has
> prevented GPIOs from being restored to inputs when released.
> Add those properties for BCM283x and BCM2711 devices.
> 
> [1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without
>     pin-ranges")
> 
> Fixes: 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Linus Walleij]

On Wed, Dec 15, 2021 at 6:14 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/15/21 1:02 AM, nicolas saenz julienne wrote:

> > As for channeling the path, would it make sense for linusw to take it alonside
> > GPIO fix?
>
> That would definitively work, Linus, are you comfortable with doing
> that? I will reply to the patch with an Acked-by if that helps.

Do you want me to merge this patch (2/2) into the pinctrl tree,
where patch (1/2) is already merged?

Sorry just a bit confused.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Florian Fainelli]

On 12/15/2021 7:27 PM, Linus Walleij wrote:
> On Wed, Dec 15, 2021 at 6:14 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 12/15/21 1:02 AM, nicolas saenz julienne wrote:
> 
>>> As for channeling the path, would it make sense for linusw to take it alonside
>>> GPIO fix?
>>
>> That would definitively work, Linus, are you comfortable with doing
>> that? I will reply to the patch with an Acked-by if that helps.
> 
> Do you want me to merge this patch (2/2) into the pinctrl tree,
> where patch (1/2) is already merged?

Yes please merge patch 2 into the pinctrl tree where patch 1 is already 
applied. Thanks!
-- 
Florian

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[Linus Walleij]

On Thu, Dec 16, 2021 at 4:28 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/15/2021 7:27 PM, Linus Walleij wrote:
> > On Wed, Dec 15, 2021 at 6:14 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >> On 12/15/21 1:02 AM, nicolas saenz julienne wrote:
> >
> >>> As for channeling the path, would it make sense for linusw to take it alonside
> >>> GPIO fix?
> >>
> >> That would definitively work, Linus, are you comfortable with doing
> >> that? I will reply to the patch with an Acked-by if that helps.
> >
> > Do you want me to merge this patch (2/2) into the pinctrl tree,
> > where patch (1/2) is already merged?
>
> Yes please merge patch 2 into the pinctrl tree where patch 1 is already
> applied. Thanks!

OK! Patch applied!

Yours,
Linus Walleij

Re: [PATCH v2 2/2] ARM: dts: gpio-ranges property is now required

[nicolas saenz julienne]

On Thu, 2021-12-16 at 04:31 +0100, Linus Walleij wrote:
> On Thu, Dec 16, 2021 at 4:28 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > On 12/15/2021 7:27 PM, Linus Walleij wrote:
> > > On Wed, Dec 15, 2021 at 6:14 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > On 12/15/21 1:02 AM, nicolas saenz julienne wrote:
> > > 
> > > > > As for channeling the path, would it make sense for linusw to take it alonside
> > > > > GPIO fix?
> > > > 
> > > > That would definitively work, Linus, are you comfortable with doing
> > > > that? I will reply to the patch with an Acked-by if that helps.
> > > 
> > > Do you want me to merge this patch (2/2) into the pinctrl tree,
> > > where patch (1/2) is already merged?
> > 
> > Yes please merge patch 2 into the pinctrl tree where patch 1 is already
> > applied. Thanks!
> 
> OK! Patch applied!

Thanks!

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Stefan Wahren]

Am 10.12.21 um 00:24 schrieb Linus Walleij:
> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>
>> ...and gpio-ranges
>>
>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>> side is registered first, but this breaks gpio hogs (which are
>> configured during gpiochip_add_data). Part of the hog initialisation
>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>> yet been registered this results in an -EPROBE_DEFER from which it can
>> never recover.
>>
>> Change the initialisation sequence to register the pinctrl driver
>> first.
>>
>> This also solves a similar problem with the gpio-ranges property, which
>> is required in order for released pins to be returned to inputs.
>>
>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> This patch (1/2) applied for fixes.

Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
stays in the last state instead of the configured heartbeat behavior.
Also there are no GPIO LEDs in /sys/class/leds/ directory.

After reverting this change everything is back to normal.

Best regards

>
> Yours,
> Linus Walleij
>
> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Florian Fainelli]

On 12/29/2021 11:07 AM, Stefan Wahren wrote:
> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>
>>> ...and gpio-ranges
>>>
>>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>>> side is registered first, but this breaks gpio hogs (which are
>>> configured during gpiochip_add_data). Part of the hog initialisation
>>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>>> yet been registered this results in an -EPROBE_DEFER from which it can
>>> never recover.
>>>
>>> Change the initialisation sequence to register the pinctrl driver
>>> first.
>>>
>>> This also solves a similar problem with the gpio-ranges property, which
>>> is required in order for released pins to be returned to inputs.
>>>
>>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>> This patch (1/2) applied for fixes.
> 
> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
> stays in the last state instead of the configured heartbeat behavior.
> Also there are no GPIO LEDs in /sys/class/leds/ directory.
> 
> After reverting this change everything is back to normal.

And this patch has already been applied to the stable 5.15 and 5.10 
branches as well, FWIW.
-- 
Florian

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Linus Walleij]

On Wed, Dec 29, 2021 at 8:07 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Am 10.12.21 um 00:24 schrieb Linus Walleij:
> > On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >
> >> ...and gpio-ranges
> >>
> >> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
> >> side is registered first, but this breaks gpio hogs (which are
> >> configured during gpiochip_add_data). Part of the hog initialisation
> >> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
> >> yet been registered this results in an -EPROBE_DEFER from which it can
> >> never recover.
> >>
> >> Change the initialisation sequence to register the pinctrl driver
> >> first.
> >>
> >> This also solves a similar problem with the gpio-ranges property, which
> >> is required in order for released pins to be returned to inputs.
> >>
> >> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
> >> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> > This patch (1/2) applied for fixes.
>
> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
> stays in the last state instead of the configured heartbeat behavior.
> Also there are no GPIO LEDs in /sys/class/leds/ directory.
>
> After reverting this change everything is back to normal.

Oh what a mess. OK I reverted the fix.

Yours,
Linus Walleij

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Jan Kiszka]

On 02.01.22 07:54, Linus Walleij wrote:
> On Wed, Dec 29, 2021 at 8:07 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>
>>>> ...and gpio-ranges
>>>>
>>>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>>>> side is registered first, but this breaks gpio hogs (which are
>>>> configured during gpiochip_add_data). Part of the hog initialisation
>>>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>>>> yet been registered this results in an -EPROBE_DEFER from which it can
>>>> never recover.
>>>>
>>>> Change the initialisation sequence to register the pinctrl driver
>>>> first.
>>>>
>>>> This also solves a similar problem with the gpio-ranges property, which
>>>> is required in order for released pins to be returned to inputs.
>>>>
>>>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>> This patch (1/2) applied for fixes.
>>
>> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
>> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
>> stays in the last state instead of the configured heartbeat behavior.
>> Also there are no GPIO LEDs in /sys/class/leds/ directory.
>>
>> After reverting this change everything is back to normal.
>
> Oh what a mess. OK I reverted the fix.
>

I happened to debug this regression as well: The issue of the patch
seems to be that it initializes gpio_range.base with -1, because
gpio_chip.base is not yet set at this point. Maybe that can be achieved
differently, to please all cases.

Jan

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Stefan Wahren]

Hi Jan,

Am 02.01.22 um 12:02 schrieb Jan Kiszka:
> On 02.01.22 07:54, Linus Walleij wrote:
>> On Wed, Dec 29, 2021 at 8:07 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>>>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>>
>>>>> ...and gpio-ranges
>>>>>
>>>>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>>>>> side is registered first, but this breaks gpio hogs (which are
>>>>> configured during gpiochip_add_data). Part of the hog initialisation
>>>>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>>>>> yet been registered this results in an -EPROBE_DEFER from which it can
>>>>> never recover.
>>>>>
>>>>> Change the initialisation sequence to register the pinctrl driver
>>>>> first.
>>>>>
>>>>> This also solves a similar problem with the gpio-ranges property, which
>>>>> is required in order for released pins to be returned to inputs.
>>>>>
>>>>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>> This patch (1/2) applied for fixes.
>>> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
>>> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
>>> stays in the last state instead of the configured heartbeat behavior.
>>> Also there are no GPIO LEDs in /sys/class/leds/ directory.
>>>
>>> After reverting this change everything is back to normal.
>> Oh what a mess. OK I reverted the fix.
>>
> I happened to debug this regression as well: The issue of the patch
> seems to be that it initializes gpio_range.base with -1, because
> gpio_chip.base is not yet set at this point. Maybe that can be achieved
> differently, to please all cases.

thanks for the hint.

I tried to reproduce the original issue with the gpio hog by adding one
to the RPi Zero W. Here are my test results of this series based on
Linux 5.16:

1. - Patch 1 & 2 not applied: boot hangs caused by gpio hog
2. - Patch 1 applied, Patch 2 not applied: boot hangs caused by gpio hog
3. - Patch 1 not applied, Patch 2 applied: boot hangs caused by gpio hog
4. - Patch 1 & 2 applied: boot ok, LEDs okay, hog okay

So both patches must be applied at the same time. In general this is
done via immutable branch. But this requires caution while backporting.

Best regards

>
> Jan

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Jan Kiszka]

On 02.01.22 13:33, Stefan Wahren wrote:
> Hi Jan,
>
> Am 02.01.22 um 12:02 schrieb Jan Kiszka:
>> On 02.01.22 07:54, Linus Walleij wrote:
>>> On Wed, Dec 29, 2021 at 8:07 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>>> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>>>>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>>>
>>>>>> ...and gpio-ranges
>>>>>>
>>>>>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>>>>>> side is registered first, but this breaks gpio hogs (which are
>>>>>> configured during gpiochip_add_data). Part of the hog initialisation
>>>>>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>>>>>> yet been registered this results in an -EPROBE_DEFER from which it can
>>>>>> never recover.
>>>>>>
>>>>>> Change the initialisation sequence to register the pinctrl driver
>>>>>> first.
>>>>>>
>>>>>> This also solves a similar problem with the gpio-ranges property, which
>>>>>> is required in order for released pins to be returned to inputs.
>>>>>>
>>>>>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>> This patch (1/2) applied for fixes.
>>>> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
>>>> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
>>>> stays in the last state instead of the configured heartbeat behavior.
>>>> Also there are no GPIO LEDs in /sys/class/leds/ directory.
>>>>
>>>> After reverting this change everything is back to normal.
>>> Oh what a mess. OK I reverted the fix.
>>>
>> I happened to debug this regression as well: The issue of the patch
>> seems to be that it initializes gpio_range.base with -1, because
>> gpio_chip.base is not yet set at this point. Maybe that can be achieved
>> differently, to please all cases.
>
> thanks for the hint.
>
> I tried to reproduce the original issue with the gpio hog by adding one
> to the RPi Zero W. Here are my test results of this series based on
> Linux 5.16:
>
> 1. - Patch 1 & 2 not applied: boot hangs caused by gpio hog
> 2. - Patch 1 applied, Patch 2 not applied: boot hangs caused by gpio hog
> 3. - Patch 1 not applied, Patch 2 applied: boot hangs caused by gpio hog
> 4. - Patch 1 & 2 applied: boot ok, LEDs okay, hog okay
>
> So both patches must be applied at the same time. In general this is
> done via immutable branch. But this requires caution while backporting.
>

Indeed - upstream and stable are missing patch 2, and that fixes the
issue as well. Too bad that this series was split during merge.

Jan

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Jan Kiszka]

On 02.01.22 16:12, Jan Kiszka wrote:
> On 02.01.22 13:33, Stefan Wahren wrote:
>> Hi Jan,
>>
>> Am 02.01.22 um 12:02 schrieb Jan Kiszka:
>>> On 02.01.22 07:54, Linus Walleij wrote:
>>>> On Wed, Dec 29, 2021 at 8:07 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>>>> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>>>>>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>>>>
>>>>>>> ...and gpio-ranges
>>>>>>>
>>>>>>> pinctrl-bcm2835 is a combined pinctrl/gpio driver. Currently the gpio
>>>>>>> side is registered first, but this breaks gpio hogs (which are
>>>>>>> configured during gpiochip_add_data). Part of the hog initialisation
>>>>>>> is a call to pinctrl_gpio_request, and since the pinctrl driver hasn't
>>>>>>> yet been registered this results in an -EPROBE_DEFER from which it can
>>>>>>> never recover.
>>>>>>>
>>>>>>> Change the initialisation sequence to register the pinctrl driver
>>>>>>> first.
>>>>>>>
>>>>>>> This also solves a similar problem with the gpio-ranges property, which
>>>>>>> is required in order for released pins to be returned to inputs.
>>>>>>>
>>>>>>> Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding gpiochip")
>>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>>> This patch (1/2) applied for fixes.
>>>>> Unfortunately this change breaks all GPIO LEDs at least on the Raspberry
>>>>> Pi 3 Plus (Linux 5.16-rc7, multi_v7_defconfig). The ACT LED for instance
>>>>> stays in the last state instead of the configured heartbeat behavior.
>>>>> Also there are no GPIO LEDs in /sys/class/leds/ directory.
>>>>>
>>>>> After reverting this change everything is back to normal.
>>>> Oh what a mess. OK I reverted the fix.
>>>>
>>> I happened to debug this regression as well: The issue of the patch
>>> seems to be that it initializes gpio_range.base with -1, because
>>> gpio_chip.base is not yet set at this point. Maybe that can be achieved
>>> differently, to please all cases.
>>
>> thanks for the hint.
>>
>> I tried to reproduce the original issue with the gpio hog by adding one
>> to the RPi Zero W. Here are my test results of this series based on
>> Linux 5.16:
>>
>> 1. - Patch 1 & 2 not applied: boot hangs caused by gpio hog
>> 2. - Patch 1 applied, Patch 2 not applied: boot hangs caused by gpio hog
>> 3. - Patch 1 not applied, Patch 2 applied: boot hangs caused by gpio hog
>> 4. - Patch 1 & 2 applied: boot ok, LEDs okay, hog okay
>>
>> So both patches must be applied at the same time. In general this is
>> done via immutable branch. But this requires caution while backporting.
>>
>
> Indeed - upstream and stable are missing patch 2, and that fixes the
> issue as well. Too bad that this series was split during merge.
>

But, in fact, this series was misordered then, suggesting that patch 1
was independent of patch 2, but it actually depended on patch 2 to avoid
even temporary regressions.

Jan

Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs

[Linus Walleij]

On Sun, Jan 2, 2022 at 4:16 PM Jan Kiszka <jan.kiszka@web.de> wrote:

> But, in fact, this series was misordered then, suggesting that patch 1
> was independent of patch 2, but it actually depended on patch 2 to avoid
> even temporary regressions.

I picked patch 2/2 out of my tree and sent it off to the SoC tree that applies
DTS fixes.

Let's see if they manage to get it to Torvalds before the merge window.

Yours,
Linus Walleij