On Mon, Oct 16, 2017 at 01:50:08PM +0200, Linus Walleij wrote:
> On Mon, Oct 16, 2017 at 10:08 AM, Thierry Reding
> <thierry.reding@xxxxxxxxx> wrote:
>
> > Nevermind, I see there is now yet another conflict that would require
> > yet another rebase of that series.
>
> Sorry about this :(
>
> It sucks with moving targets, there was some fallout from Grygorii's patch
> to map IRQs dynamically and remove irq_base so we need to clean that
> up first.
>
> This particular bug from the autobuilder seems to be constrained to the
> armada driver though.
Well, the problem here is that Grygorii's patch removes the field but
with there still being one user left. So we'd have to make sure that the
real last user goes away before that patch is applied.
Looking at the code it seems a little phony. d->hwirq - chip->irq_base
seems like a very risky thing to do because you never really know what
irq_base will end up being (in the general case where it's dynamically
allocated). But the driver passes 0 to gpiochip_irqchip_add() as the
first_irq parameter, therefore the operation becomes a noop and so it
should be completely safe to remove that line.
Something like the below should do. Cc += Gregory.
Thierry
--- >8 ---
From fcd87329cf4edf6a6f5d491a1893af401be7dedf Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@xxxxxxxxxx>
Date: Mon, 16 Oct 2017 14:40:23 +0200
Subject: [PATCH] pinctrl: armada-37xx: Stop using struct gpio_chip.irq_base
The Armada 37xx driver always initializes the IRQ base to 0, hence the
subtraction is a no-op. Remove the subtraction and thereby the last user
of struct gpio_chip's .irq_base field.
Note that this was also actually a bug and only worked because of the
above assumption. If the IRQ base had been dynamically allocated, the
subtraction would've caused the wrong mask to be generated since the
struct irq_data.hwirq field is an index local to the IRQ domain. As a
result, it should now be safe to also allocate this chip's IRQ base
dynamically, unless there are consumers left that refer to the IRQs by
their global number.
Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 71b944748304..ac299a6cdfd6 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -627,14 +627,14 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
static unsigned int armada_37xx_irq_startup(struct irq_data *d)
{
struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
- int irq = d->hwirq - chip->irq_base;
+
/*
* The mask field is a "precomputed bitmask for accessing the
* chip registers" which was introduced for the generic
* irqchip framework. As we don't use this framework, we can
* reuse this field for our own usage.
*/
- d->mask = BIT(irq % GPIO_PER_REG);
+ d->mask = BIT(d->hwirq % GPIO_PER_REG);
armada_37xx_irq_unmask(d);
--
2.14.1
Attachment:
signature.asc
Description: PGP signature
