| Message ID | 4ee1a396acc34871dbae73a5b032915f745795ec.1669738949.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | gpio: mpfs: Make the irqchip immutable | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Guessing tree name failed |
Hey Geert, On Tue, Nov 29, 2022 at 05:23:22PM +0100, Geert Uytterhoeven wrote: > Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as > immutable") added a warning to indicate if the gpiolib is altering the > internals of irqchips. Following this change the following warning is > now observed for the gpio-mpfs driver: > > gpio gpiochip0: (20122000.gpio): not an immutable chip, please consider fixing it! > > Fix this by making the irqchip in the gpio-mpfs driver immutable. > > While at it, drop of the unneeded masking of the hwirq number, as it is > always smaller than the number of GPIOs/interrupts handled by the > controller. Huh, I didn't think this was upstream yet? Lewis should be working on an updated version of it at the moment, last I heard he was looking into using regmap stuff as pointed out in a review. > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Not tested with interrupts, as there are no inputs described in the > Icicle DTS yet. > --- > drivers/gpio/gpio-mpfs.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c > index 168be0b90cf8c656..fd294b581ae77369 100644 > --- a/drivers/gpio/gpio-mpfs.c > +++ b/drivers/gpio/gpio-mpfs.c > @@ -168,8 +168,9 @@ static void mpfs_gpio_irq_unmask(struct irq_data *data) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); > - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; > + int gpio_index = irqd_to_hwirq(data); > > + gpiochip_enable_irq(gc, gpio_index); > mpfs_gpio_direction_input(gc, gpio_index); > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), > @@ -180,11 +181,12 @@ static void mpfs_gpio_irq_mask(struct irq_data *data) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); > - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; > + int gpio_index = irqd_to_hwirq(data); > > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), > MPFS_GPIO_EN_INT, 0); > + gpiochip_disable_irq(gc, gpio_index); > } > > static const struct irq_chip mpfs_gpio_irqchip = { > @@ -192,7 +194,8 @@ static const struct irq_chip mpfs_gpio_irqchip = { > .irq_set_type = mpfs_gpio_irq_set_type, > .irq_mask = mpfs_gpio_irq_mask, > .irq_unmask = mpfs_gpio_irq_unmask, > - .flags = IRQCHIP_MASK_ON_SUSPEND, > + .flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND, > + GPIOCHIP_IRQ_RESOURCE_HELPERS, > }; > > static void mpfs_gpio_irq_handler(struct irq_desc *desc) > -- > 2.25.1 >
Hi Conor, CC Lewis On Tue, Nov 29, 2022 at 5:31 PM Conor Dooley <conor@kernel.org> wrote: > On Tue, Nov 29, 2022 at 05:23:22PM +0100, Geert Uytterhoeven wrote: > > Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as > > immutable") added a warning to indicate if the gpiolib is altering the > > internals of irqchips. Following this change the following warning is > > now observed for the gpio-mpfs driver: > > > > gpio gpiochip0: (20122000.gpio): not an immutable chip, please consider fixing it! > > > > Fix this by making the irqchip in the gpio-mpfs driver immutable. > > > > While at it, drop of the unneeded masking of the hwirq number, as it is > > always smaller than the number of GPIOs/interrupts handled by the > > controller. > > Huh, I didn't think this was upstream yet? > Lewis should be working on an updated version of it at the moment, last > I heard he was looking into using regmap stuff as pointed out in a > review. Oh right, this is still a local patch. Sorry, I thought it was already upstream... > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > Not tested with interrupts, as there are no inputs described in the > > Icicle DTS yet. > > --- > > drivers/gpio/gpio-mpfs.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c > > index 168be0b90cf8c656..fd294b581ae77369 100644 > > --- a/drivers/gpio/gpio-mpfs.c > > +++ b/drivers/gpio/gpio-mpfs.c > > @@ -168,8 +168,9 @@ static void mpfs_gpio_irq_unmask(struct irq_data *data) > > { > > struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > > struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); > > - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; > > + int gpio_index = irqd_to_hwirq(data); > > > > + gpiochip_enable_irq(gc, gpio_index); > > mpfs_gpio_direction_input(gc, gpio_index); > > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); > > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), > > @@ -180,11 +181,12 @@ static void mpfs_gpio_irq_mask(struct irq_data *data) > > { > > struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > > struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); > > - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; > > + int gpio_index = irqd_to_hwirq(data); > > > > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); > > mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), > > MPFS_GPIO_EN_INT, 0); > > + gpiochip_disable_irq(gc, gpio_index); > > } > > > > static const struct irq_chip mpfs_gpio_irqchip = { > > @@ -192,7 +194,8 @@ static const struct irq_chip mpfs_gpio_irqchip = { > > .irq_set_type = mpfs_gpio_irq_set_type, > > .irq_mask = mpfs_gpio_irq_mask, > > .irq_unmask = mpfs_gpio_irq_unmask, > > - .flags = IRQCHIP_MASK_ON_SUSPEND, > > + .flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND, > > + GPIOCHIP_IRQ_RESOURCE_HELPERS, > > }; > > > > static void mpfs_gpio_irq_handler(struct irq_desc *desc) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Nov 29, 2022 at 5:23 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as > immutable") added a warning to indicate if the gpiolib is altering the > internals of irqchips. Following this change the following warning is > now observed for the gpio-mpfs driver: > > gpio gpiochip0: (20122000.gpio): not an immutable chip, please consider fixing it! > > Fix this by making the irqchip in the gpio-mpfs driver immutable. > > While at it, drop of the unneeded masking of the hwirq number, as it is > always smaller than the number of GPIOs/interrupts handled by the > controller. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c index 168be0b90cf8c656..fd294b581ae77369 100644 --- a/drivers/gpio/gpio-mpfs.c +++ b/drivers/gpio/gpio-mpfs.c @@ -168,8 +168,9 @@ static void mpfs_gpio_irq_unmask(struct irq_data *data) { struct gpio_chip *gc = irq_data_get_irq_chip_data(data); struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; + int gpio_index = irqd_to_hwirq(data); + gpiochip_enable_irq(gc, gpio_index); mpfs_gpio_direction_input(gc, gpio_index); mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), @@ -180,11 +181,12 @@ static void mpfs_gpio_irq_mask(struct irq_data *data) { struct gpio_chip *gc = irq_data_get_irq_chip_data(data); struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc); - int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO; + int gpio_index = irqd_to_hwirq(data); mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1); mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index), MPFS_GPIO_EN_INT, 0); + gpiochip_disable_irq(gc, gpio_index); } static const struct irq_chip mpfs_gpio_irqchip = { @@ -192,7 +194,8 @@ static const struct irq_chip mpfs_gpio_irqchip = { .irq_set_type = mpfs_gpio_irq_set_type, .irq_mask = mpfs_gpio_irq_mask, .irq_unmask = mpfs_gpio_irq_unmask, - .flags = IRQCHIP_MASK_ON_SUSPEND, + .flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND, + GPIOCHIP_IRQ_RESOURCE_HELPERS, }; static void mpfs_gpio_irq_handler(struct irq_desc *desc)
Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as immutable") added a warning to indicate if the gpiolib is altering the internals of irqchips. Following this change the following warning is now observed for the gpio-mpfs driver: gpio gpiochip0: (20122000.gpio): not an immutable chip, please consider fixing it! Fix this by making the irqchip in the gpio-mpfs driver immutable. While at it, drop of the unneeded masking of the hwirq number, as it is always smaller than the number of GPIOs/interrupts handled by the controller. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Not tested with interrupts, as there are no inputs described in the Icicle DTS yet. --- drivers/gpio/gpio-mpfs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)