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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Dec 1, 2021 at 5:18 PM Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote:
> On 01/12/2021 12:06, Andy Shevchenko wrote:
> > On Monday, November 29, 2021, Phil Elwell <phil@xxxxxxxxxxxxxxx
> > <mailto: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.
> >
> >
> > We have a callback in GPIO chip to register pin ranges, why driver does it
> > separately?
>
> A few experiments (this is not my driver) appear to show that the call to
> pinctrl_add_gpio_range can be removed, but only once the gpio-ranges DT property
> has been added if we want to remain functionality throughout a bisect. That tidy
> up might be better done with a followup commit once the DT patch has also
> been accepted, unless it's possible to guarantee the sequencing between
> the pinctrl/gpio tree and the DT tree.

What I meant is why these calls are done in the probe and not in
->add_pin_ranges() callback?
Shouldn't it fix the issue you have observed?

...

> >     Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding
> >                             gpiochip")
> >     Signed-off-by: Phil Elwell <phil@xxxxxxxxxxxxxxx <mailto:phil@xxxxxxxxxxxxxxx>>
> >
> > Is it originally so strange indentation or is it only on my side?
>
> The "g" is below the "p" in the patch.

Which is wrong. Tags mustn't be multilines (i.e. split over a single line).

-- 
With Best Regards,
Andy Shevchenko