lkml.org 
[lkml]   [2021]   [Dec]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 5.10 29/76] pinctrl: bcm2835: Change init order for gpio hogs
Hi!

> From: Phil Elwell <phil@raspberrypi.com>
>
> [ Upstream commit 266423e60ea1b953fcc0cd97f3dad85857e434d1 ]
>
> ...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 does not get error handling right.

> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 1d21129f7751c..40ce18a0d0190 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);
> + }

You do gpiochip_remove(), even when it was not added.

> @@ -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;
> + }

And yes, adding the removes here makes sense, but it looks like some
are missing (in 5.10).

Something like this?

Best regards,
Pavel
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 40ce18a0d019..d605548de7df 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1247,7 +1247,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
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);
}

@@ -1264,16 +1263,18 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
sizeof(*girq->parents),
GFP_KERNEL);
if (!girq->parents) {
- pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto remove_gpio;
}

if (is_7211) {
pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
sizeof(*pc->wake_irq),
GFP_KERNEL);
- if (!pc->wake_irq)
- return -ENOMEM;
+ if (!pc->wake_irq) {
+ err = -ENOMEM;
+ goto remove_gpio;
+ }
}

/*
@@ -1297,8 +1298,10 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)

len = strlen(dev_name(pc->dev)) + 16;
name = devm_kzalloc(pc->dev, len, GFP_KERNEL);
- if (!name)
- return -ENOMEM;
+ if (!name) {
+ err = -ENOMEM;
+ goto remove_gpio;
+ }

snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);

@@ -1317,11 +1320,14 @@ 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;
+ goto remove_gpio;
}

return 0;
+
+remove_gpio:
+ pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
+ return err;
}

static struct platform_driver bcm2835_pinctrl_driver = {

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-12-31 10:53    [W:0.530 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site