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

[Jan Kiszka <jan.kiszka@xxxxxx>]

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@xxxxxxxx> wrote:
>>>>> Am 10.12.21 um 00:24 schrieb Linus Walleij:
>>>>>> On Mon, Dec 6, 2021 at 10:22 AM Phil Elwell <phil@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>>>>>> 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