| Message ID | 20190812062729.1892-1-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | 73345a18d464b1b945b29f54f630ace6873344e2 |
| Headers | show |
| Series | pinctrl: bcm2835: Pass irqchip when adding gpiochip | expand |
On 12/08/2019 07:27, Linus Walleij wrote: > We need to convert all old gpio irqchips to pass the irqchip > setup along when adding the gpio_chip. For more info see > drivers/gpio/TODO. > > For chained irqchips this is a pretty straight-forward > conversion. The BCM2835 has multiple parents so let's > exploit the new facility in the GPIO_IRQCHIP to actually > deal with multiple parents. > > Cc: Simon Arlott <simon@arlott.org> Please stop putting that email address in commit messages. You do not have my consent to use it. -- Simon Arlott
On Mon, Aug 12, 2019 at 8:29 AM Linus Walleij <linus.walleij@linaro.org> wrote: > We need to convert all old gpio irqchips to pass the irqchip > setup along when adding the gpio_chip. For more info see > drivers/gpio/TODO. > > For chained irqchips this is a pretty straight-forward > conversion. The BCM2835 has multiple parents so let's > exploit the new facility in the GPIO_IRQCHIP to actually > deal with multiple parents. > > Cc: Simon Arlott <simon@arlott.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Stefan Wahren <stefan.wahren@i2se.com> > Cc: Thierry Reding <treding@nvidia.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> This patch is a bit scary because I haven't tried converting multiple parents before. Any chance one of you RPi people could give it a test run, so you don't have to experience testing it in linux-next? Yours, Linus Walleij
Hi Linus, [removed Simon from CC to stop annoy him] On 23.08.19 09:46, Linus Walleij wrote: > On Mon, Aug 12, 2019 at 8:29 AM Linus Walleij <linus.walleij@linaro.org> wrote: > >> We need to convert all old gpio irqchips to pass the irqchip >> setup along when adding the gpio_chip. For more info see >> drivers/gpio/TODO. >> >> For chained irqchips this is a pretty straight-forward >> conversion. The BCM2835 has multiple parents so let's >> exploit the new facility in the GPIO_IRQCHIP to actually >> deal with multiple parents. >> >> Cc: Simon Arlott <simon@arlott.org> >> Cc: Eric Anholt <eric@anholt.net> >> Cc: Stefan Wahren <stefan.wahren@i2se.com> >> Cc: Thierry Reding <treding@nvidia.com> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > This patch is a bit scary because I haven't tried converting multiple > parents before. Any chance one of you RPi people could give it > a test run, so you don't have to experience testing it in linux-next? it's on my TODO list, but i didn't had the time to test it yet. Can you give me some test ideas? > > Yours, > Linus Walleij
On Fri, Aug 23, 2019 at 10:07 AM Stefan Wahren <stefan.wahren@i2se.com> wrote: > On 23.08.19 09:46, Linus Walleij wrote: > > On Mon, Aug 12, 2019 at 8:29 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > > >> We need to convert all old gpio irqchips to pass the irqchip > >> setup along when adding the gpio_chip. For more info see > >> drivers/gpio/TODO. > >> > >> For chained irqchips this is a pretty straight-forward > >> conversion. The BCM2835 has multiple parents so let's > >> exploit the new facility in the GPIO_IRQCHIP to actually > >> deal with multiple parents. > >> > >> Cc: Simon Arlott <simon@arlott.org> > >> Cc: Eric Anholt <eric@anholt.net> > >> Cc: Stefan Wahren <stefan.wahren@i2se.com> > >> Cc: Thierry Reding <treding@nvidia.com> > >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > > > This patch is a bit scary because I haven't tried converting multiple > > parents before. Any chance one of you RPi people could give it > > a test run, so you don't have to experience testing it in linux-next? > > it's on my TODO list, but i didn't had the time to test it yet. OK sorry for stressing :( > Can you give me some test ideas? Anything that fires IRQ on the pin controller is a good test, I sometimes just use tools/gpio/gpio-event-mon.c (the more elaborate version is inside libgpiod) and have some pushbutton or wire to ground the GPIO line under test. Yours, Linus Walleij
Hi Linus, Am 23.08.19 um 10:07 schrieb Stefan Wahren: > Hi Linus, > > [removed Simon from CC to stop annoy him] > > On 23.08.19 09:46, Linus Walleij wrote: >> On Mon, Aug 12, 2019 at 8:29 AM Linus Walleij <linus.walleij@linaro.org> wrote: >> >>> We need to convert all old gpio irqchips to pass the irqchip >>> setup along when adding the gpio_chip. For more info see >>> drivers/gpio/TODO. >>> >>> For chained irqchips this is a pretty straight-forward >>> conversion. The BCM2835 has multiple parents so let's >>> exploit the new facility in the GPIO_IRQCHIP to actually >>> deal with multiple parents. >>> >>> Cc: Simon Arlott <simon@arlott.org> please remove Simon's email address before applying >>> Cc: Eric Anholt <eric@anholt.net> >>> Cc: Stefan Wahren <stefan.wahren@i2se.com> and use my new address in the future. >>> Cc: Thierry Reding <treding@nvidia.com> >>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> >> This patch is a bit scary because I haven't tried converting multiple >> parents before. Any chance one of you RPi people could give it >> a test run, so you don't have to experience testing it in linux-next? > it's on my TODO list, but i didn't had the time to test it yet. > > Can you give me some test ideas? I configured a GPIO as input with pull-up, setup the interrupt via sysfs and wired the GPIO to GND. After that i checked that /proc/interrupts for the pin has increased. Everything worked as expected. Is it sufficient? Tested-by: Stefan Wahren <stefan.wahren@i2se.com> > >> Yours, >> Linus Walleij
On Fri, Aug 23, 2019 at 9:00 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Can you give me some test ideas? > > I configured a GPIO as input with pull-up, setup the interrupt via sysfs > and wired the GPIO to GND. > After that i checked that /proc/interrupts for the pin has increased. > Everything worked as expected. > > Is it sufficient? > > Tested-by: Stefan Wahren <stefan.wahren@i2se.com> Works for me, at least it is not crashing or going numb, elaborate testing we do in -next, thanks! Applied with your test-tag. Yours, Linus Walleij
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index 183d1ffe6a75..b729997cd887 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -69,7 +69,6 @@ struct bcm2835_pinctrl { struct device *dev; void __iomem *base; - int irq[BCM2835_NUM_IRQS]; /* note: locking assumes each bank will have its own unsigned long */ unsigned long enabled_irq_map[BCM2835_NUM_BANKS]; @@ -373,14 +372,14 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc) int group; int i; - for (i = 0; i < ARRAY_SIZE(pc->irq); i++) { - if (pc->irq[i] == irq) { + for (i = 0; i < BCM2835_NUM_IRQS; i++) { + if (chip->irq.parents[i] == irq) { group = i; break; } } /* This should not happen, every IRQ has a bank */ - if (i == ARRAY_SIZE(pc->irq)) + if (i == BCM2835_NUM_IRQS) BUG(); chained_irq_enter(host_chip, desc); @@ -995,6 +994,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; struct bcm2835_pinctrl *pc; + struct gpio_irq_chip *girq; struct resource iomem; int err, i; BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2835_NUM_GPIOS); @@ -1041,38 +1041,34 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) raw_spin_lock_init(&pc->irq_lock[i]); } + girq = &pc->gpio_chip.irq; + girq->chip = &bcm2835_gpio_irq_chip; + girq->parent_handler = bcm2835_gpio_irq_handler; + girq->num_parents = BCM2835_NUM_IRQS; + girq->parents = devm_kcalloc(&pdev->dev, BCM2835_NUM_IRQS, + sizeof(*girq->parents), + GFP_KERNEL); + if (!girq->parents) + return -ENOMEM; + /* + * Use the same handler for all groups: this is necessary + * since we use one gpiochip to cover all lines - the + * irq handler then needs to figure out which group and + * bank that was firing the IRQ and look up the per-group + * and bank data. + */ + for (i = 0; i < BCM2835_NUM_IRQS; i++) + girq->parents[i] = irq_of_parse_and_map(np, i); + girq->default_type = IRQ_TYPE_NONE; + girq->handler = handle_level_irq; + + err = gpiochip_add_data(&pc->gpio_chip, pc); if (err) { dev_err(dev, "could not add GPIO chip\n"); return err; } - err = gpiochip_irqchip_add(&pc->gpio_chip, &bcm2835_gpio_irq_chip, - 0, handle_level_irq, IRQ_TYPE_NONE); - if (err) { - dev_info(dev, "could not add irqchip\n"); - return err; - } - - for (i = 0; i < BCM2835_NUM_IRQS; i++) { - pc->irq[i] = irq_of_parse_and_map(np, i); - - if (pc->irq[i] == 0) - continue; - - /* - * Use the same handler for all groups: this is necessary - * since we use one gpiochip to cover all lines - the - * irq handler then needs to figure out which group and - * bank that was firing the IRQ and look up the per-group - * and bank data. - */ - gpiochip_set_chained_irqchip(&pc->gpio_chip, - &bcm2835_gpio_irq_chip, - pc->irq[i], - bcm2835_gpio_irq_handler); - } - pc->pctl_dev = devm_pinctrl_register(dev, &bcm2835_pinctrl_desc, pc); if (IS_ERR(pc->pctl_dev)) { gpiochip_remove(&pc->gpio_chip);
We need to convert all old gpio irqchips to pass the irqchip setup along when adding the gpio_chip. For more info see drivers/gpio/TODO. For chained irqchips this is a pretty straight-forward conversion. The BCM2835 has multiple parents so let's exploit the new facility in the GPIO_IRQCHIP to actually deal with multiple parents. Cc: Simon Arlott <simon@arlott.org> Cc: Eric Anholt <eric@anholt.net> Cc: Stefan Wahren <stefan.wahren@i2se.com> Cc: Thierry Reding <treding@nvidia.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 56 +++++++++++++-------------- 1 file changed, 26 insertions(+), 30 deletions(-) -- 2.21.0