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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux