Re: [PATCH] gpio: mpfs: Make the irqchip immutable

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Conor,

CC Lewis

On Tue, Nov 29, 2022 at 5:31 PM Conor Dooley <conor@xxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxx

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