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

From: Jan Kiszka <jan.kiszka@web.de>
To: Stefan Wahren <stefan.wahren@i2se.com>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: Phil Elwell <phil@raspberrypi.com>,
	devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	linux-gpio@vger.kernel.org,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org,
	Thierry Reding <treding@nvidia.com>
Subject: Re: [PATCH v2 1/2] pinctrl: bcm2835: Change init order for gpio hogs
Date: Sun, 2 Jan 2022 16:12:53 +0100	[thread overview]
Message-ID: <5a5bc9ac-6d8e-d6de-eec2-ea72970b88bf@web.de> (raw)
In-Reply-To: <ec60b52b-7a59-266d-9608-11c0da9053a6@i2se.com>

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