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

[Phil Elwell <phil@raspberrypi.com>]

On 01/12/2021 15:38, Andy Shevchenko wrote:
> On Wed, Dec 1, 2021 at 5:18 PM Phil Elwell <phil@raspberrypi.com> wrote:
>> On 01/12/2021 12:06, Andy Shevchenko wrote:
>>> On Monday, November 29, 2021, Phil Elwell <phil@raspberrypi.com
>>> <mailto: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.
>>>
>>>
>>> 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?

I'm no expert in the field, isn't it preferable to set the gpio-ranges 
pinctrl<->GPIO correspondence with a single line of DT rather than several lines 
of code? A quick grep shows over 700 instances of gpio-ranges in DT, at least 
some of which are reflexive, and only 7 drivers with add_pin_ranges methods.

>>>      Fixes: 73345a18d464b ("pinctrl: bcm2835: Pass irqchip when adding
>>>                              gpiochip")
>>>      Signed-off-by: Phil Elwell <phil@raspberrypi.com <mailto:phil@raspberrypi.com>>
>>>
>>> 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).

checkpatch disagrees:

scripts/checkpatch.pl 0001-ARM-dts-gpio-ranges-property-is-now-required.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10:
[1] commit 2ab73c6d8323 ("gpio: Support GPIO controllers without pin-ranges")

Phil