On 01/12/2021 15:38, Andy Shevchenko wrote:
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?
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@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).
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
