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
