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

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

 



Hi Andy,

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.

    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.

Thanks,

Phil

    ---
      drivers/pinctrl/bcm/pinctrl-bcm2835.c | 29 +++++++++++++++------------
      1 file changed, 16 insertions(+), 13 deletions(-)

    diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
    b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
    index 2abcc6ce4eba..b607d10e4cbd 100644
    --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
    +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
    @@ -1244,6 +1244,18 @@ static int bcm2835_pinctrl_probe(struct
    platform_device *pdev)
                     raw_spin_lock_init(&pc->irq_lock[i]);
             }

    +       pc->pctl_desc = *pdata->pctl_desc;
    +       pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
    +       if (IS_ERR(pc->pctl_dev)) {
    +               gpiochip_remove(&pc->gpio_chip);
    +               return PTR_ERR(pc->pctl_dev);
    +       }
    +
    +       pc->gpio_range = *pdata->gpio_range;
    +       pc->gpio_range.base = pc->gpio_chip.base;
    +       pc->gpio_range.gc = &pc->gpio_chip;
    +       pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
    +
             girq = &pc->gpio_chip.irq;
             girq->chip = &bcm2835_gpio_irq_chip;
             girq->parent_handler = bcm2835_gpio_irq_handler;
    @@ -1251,8 +1263,10 @@ static int bcm2835_pinctrl_probe(struct
    platform_device *pdev)
             girq->parents = devm_kcalloc(dev, BCM2835_NUM_IRQS,
                                          sizeof(*girq->parents),
                                          GFP_KERNEL);
    -       if (!girq->parents)
    +       if (!girq->parents) {
    +               pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
                     return -ENOMEM;
    +       }

             if (is_7211) {
                     pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
    @@ -1307,21 +1321,10 @@ static int bcm2835_pinctrl_probe(struct
    platform_device *pdev)
             err = gpiochip_add_data(&pc->gpio_chip, pc);
             if (err) {
                     dev_err(dev, "could not add GPIO chip\n");
    +               pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
                     return err;
             }

    -       pc->pctl_desc = *pdata->pctl_desc;
    -       pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
    -       if (IS_ERR(pc->pctl_dev)) {
    -               gpiochip_remove(&pc->gpio_chip);
    -               return PTR_ERR(pc->pctl_dev);
    -       }
    -
    -       pc->gpio_range = *pdata->gpio_range;
    -       pc->gpio_range.base = pc->gpio_chip.base;
    -       pc->gpio_range.gc = &pc->gpio_chip;
    -       pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
    -
             return 0;
      }

-- 2.25.1



--
With Best Regards,
Andy Shevchenko





[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