From: Andy Shevchenko <andy.shevchenko@gmail.com> To: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Hoan Tran <hoan@os.amperecomputing.com>, Serge Semin <fancer.lancer@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <brgl@bgdev.pl>, Damien Le Moal <damien.lemoal@wdc.com>, Marc Zyngier <maz@kernel.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org> Subject: Re: [PATCH v2] gpio: dwapb: Make the irqchip immutable Date: Sat, 21 May 2022 19:29:35 +0200 [thread overview] Message-ID: <CAHp75Vc-z=K5GPBeXd8JvG=JgjG4Y2U9pBz0hFwjmJ-zP39kZw@mail.gmail.com> (raw) In-Reply-To: <01f7a990654697ca7ec8b2d0025f41403462c8d9.1653042121.git.geert+renesas@glider.be> On Sat, May 21, 2022 at 1:24 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 dwapb driver: > > gpio gpiochip0: (50200000.gpio): not an immutable chip, please consider fixing it! > > Fix this by making the irqchip in the dwapb driver immutable. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Against gpio/for-next. > Boot-tested on SiPEED MAiXBiT (Canaan K210). > > v2: > - Factor out hwirq using preferred helper. > --- > drivers/gpio/gpio-dwapb.c | 38 ++++++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 7130195da48d75dd..04afe728e18748df 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -95,7 +95,6 @@ struct dwapb_context { > #endif > > struct dwapb_gpio_port_irqchip { > - struct irq_chip irqchip; > unsigned int nr_irqs; > unsigned int irq[DWAPB_MAX_GPIOS]; > }; > @@ -252,24 +251,30 @@ static void dwapb_irq_mask(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > struct dwapb_gpio *gpio = to_dwapb_gpio(gc); > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > unsigned long flags; > u32 val; > > raw_spin_lock_irqsave(&gc->bgpio_lock, flags); > - val = dwapb_read(gpio, GPIO_INTMASK) | BIT(irqd_to_hwirq(d)); > + val = dwapb_read(gpio, GPIO_INTMASK) | BIT(hwirq); > dwapb_write(gpio, GPIO_INTMASK, val); > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); > + > + gpiochip_disable_irq(gc, hwirq); > } > > static void dwapb_irq_unmask(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > struct dwapb_gpio *gpio = to_dwapb_gpio(gc); > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > unsigned long flags; > u32 val; > > + gpiochip_enable_irq(gc, hwirq); > + > raw_spin_lock_irqsave(&gc->bgpio_lock, flags); > - val = dwapb_read(gpio, GPIO_INTMASK) & ~BIT(irqd_to_hwirq(d)); > + val = dwapb_read(gpio, GPIO_INTMASK) & ~BIT(hwirq); > dwapb_write(gpio, GPIO_INTMASK, val); > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); > } > @@ -364,8 +369,23 @@ static int dwapb_irq_set_wake(struct irq_data *d, unsigned int enable) > > return 0; > } > +#else > +#define dwapb_irq_set_wake NULL > #endif > > +static const struct irq_chip dwapb_irq_chip = { > + .name = DWAPB_DRIVER_NAME, > + .irq_ack = dwapb_irq_ack, > + .irq_mask = dwapb_irq_mask, > + .irq_unmask = dwapb_irq_unmask, > + .irq_set_type = dwapb_irq_set_type, > + .irq_enable = dwapb_irq_enable, > + .irq_disable = dwapb_irq_disable, > + .irq_set_wake = dwapb_irq_set_wake, > + .flags = IRQCHIP_IMMUTABLE, > + GPIOCHIP_IRQ_RESOURCE_HELPERS, > +}; > + > static int dwapb_gpio_set_debounce(struct gpio_chip *gc, > unsigned offset, unsigned debounce) > { > @@ -439,16 +459,6 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > girq->default_type = IRQ_TYPE_NONE; > > port->pirq = pirq; > - pirq->irqchip.name = DWAPB_DRIVER_NAME; > - pirq->irqchip.irq_ack = dwapb_irq_ack; > - pirq->irqchip.irq_mask = dwapb_irq_mask; > - pirq->irqchip.irq_unmask = dwapb_irq_unmask; > - pirq->irqchip.irq_set_type = dwapb_irq_set_type; > - pirq->irqchip.irq_enable = dwapb_irq_enable; > - pirq->irqchip.irq_disable = dwapb_irq_disable; > -#ifdef CONFIG_PM_SLEEP > - pirq->irqchip.irq_set_wake = dwapb_irq_set_wake; > -#endif > > /* > * Intel ACPI-based platforms mostly have the DesignWare APB GPIO > @@ -475,7 +485,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > girq->parent_handler = dwapb_irq_handler; > } > > - girq->chip = &pirq->irqchip; > + gpio_irq_chip_set_chip(girq, &dwapb_irq_chip); > > return; > > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko
WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andy.shevchenko@gmail.com> To: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Hoan Tran <hoan@os.amperecomputing.com>, Serge Semin <fancer.lancer@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <brgl@bgdev.pl>, Damien Le Moal <damien.lemoal@wdc.com>, Marc Zyngier <maz@kernel.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org> Subject: Re: [PATCH v2] gpio: dwapb: Make the irqchip immutable Date: Sat, 21 May 2022 19:29:35 +0200 [thread overview] Message-ID: <CAHp75Vc-z=K5GPBeXd8JvG=JgjG4Y2U9pBz0hFwjmJ-zP39kZw@mail.gmail.com> (raw) In-Reply-To: <01f7a990654697ca7ec8b2d0025f41403462c8d9.1653042121.git.geert+renesas@glider.be> On Sat, May 21, 2022 at 1:24 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 dwapb driver: > > gpio gpiochip0: (50200000.gpio): not an immutable chip, please consider fixing it! > > Fix this by making the irqchip in the dwapb driver immutable. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Against gpio/for-next. > Boot-tested on SiPEED MAiXBiT (Canaan K210). > > v2: > - Factor out hwirq using preferred helper. > --- > drivers/gpio/gpio-dwapb.c | 38 ++++++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c > index 7130195da48d75dd..04afe728e18748df 100644 > --- a/drivers/gpio/gpio-dwapb.c > +++ b/drivers/gpio/gpio-dwapb.c > @@ -95,7 +95,6 @@ struct dwapb_context { > #endif > > struct dwapb_gpio_port_irqchip { > - struct irq_chip irqchip; > unsigned int nr_irqs; > unsigned int irq[DWAPB_MAX_GPIOS]; > }; > @@ -252,24 +251,30 @@ static void dwapb_irq_mask(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > struct dwapb_gpio *gpio = to_dwapb_gpio(gc); > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > unsigned long flags; > u32 val; > > raw_spin_lock_irqsave(&gc->bgpio_lock, flags); > - val = dwapb_read(gpio, GPIO_INTMASK) | BIT(irqd_to_hwirq(d)); > + val = dwapb_read(gpio, GPIO_INTMASK) | BIT(hwirq); > dwapb_write(gpio, GPIO_INTMASK, val); > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); > + > + gpiochip_disable_irq(gc, hwirq); > } > > static void dwapb_irq_unmask(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > struct dwapb_gpio *gpio = to_dwapb_gpio(gc); > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > unsigned long flags; > u32 val; > > + gpiochip_enable_irq(gc, hwirq); > + > raw_spin_lock_irqsave(&gc->bgpio_lock, flags); > - val = dwapb_read(gpio, GPIO_INTMASK) & ~BIT(irqd_to_hwirq(d)); > + val = dwapb_read(gpio, GPIO_INTMASK) & ~BIT(hwirq); > dwapb_write(gpio, GPIO_INTMASK, val); > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); > } > @@ -364,8 +369,23 @@ static int dwapb_irq_set_wake(struct irq_data *d, unsigned int enable) > > return 0; > } > +#else > +#define dwapb_irq_set_wake NULL > #endif > > +static const struct irq_chip dwapb_irq_chip = { > + .name = DWAPB_DRIVER_NAME, > + .irq_ack = dwapb_irq_ack, > + .irq_mask = dwapb_irq_mask, > + .irq_unmask = dwapb_irq_unmask, > + .irq_set_type = dwapb_irq_set_type, > + .irq_enable = dwapb_irq_enable, > + .irq_disable = dwapb_irq_disable, > + .irq_set_wake = dwapb_irq_set_wake, > + .flags = IRQCHIP_IMMUTABLE, > + GPIOCHIP_IRQ_RESOURCE_HELPERS, > +}; > + > static int dwapb_gpio_set_debounce(struct gpio_chip *gc, > unsigned offset, unsigned debounce) > { > @@ -439,16 +459,6 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > girq->default_type = IRQ_TYPE_NONE; > > port->pirq = pirq; > - pirq->irqchip.name = DWAPB_DRIVER_NAME; > - pirq->irqchip.irq_ack = dwapb_irq_ack; > - pirq->irqchip.irq_mask = dwapb_irq_mask; > - pirq->irqchip.irq_unmask = dwapb_irq_unmask; > - pirq->irqchip.irq_set_type = dwapb_irq_set_type; > - pirq->irqchip.irq_enable = dwapb_irq_enable; > - pirq->irqchip.irq_disable = dwapb_irq_disable; > -#ifdef CONFIG_PM_SLEEP > - pirq->irqchip.irq_set_wake = dwapb_irq_set_wake; > -#endif > > /* > * Intel ACPI-based platforms mostly have the DesignWare APB GPIO > @@ -475,7 +485,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio, > girq->parent_handler = dwapb_irq_handler; > } > > - girq->chip = &pirq->irqchip; > + gpio_irq_chip_set_chip(girq, &dwapb_irq_chip); > > return; > > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-05-21 17:30 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-20 10:23 [PATCH v2] gpio: dwapb: Make the irqchip immutable Geert Uytterhoeven 2022-05-20 10:23 ` Geert Uytterhoeven 2022-05-20 23:40 ` Damien Le Moal 2022-05-20 23:40 ` Damien Le Moal 2022-05-21 17:29 ` Andy Shevchenko 2022-05-21 17:29 ` Andy Shevchenko 2022-05-21 17:29 ` Andy Shevchenko [this message] 2022-05-21 17:29 ` Andy Shevchenko 2022-05-22 20:13 ` Bartosz Golaszewski 2022-05-22 20:13 ` Bartosz Golaszewski 2022-05-23 7:25 ` Geert Uytterhoeven 2022-05-23 7:25 ` Geert Uytterhoeven 2022-05-23 18:56 ` Bartosz Golaszewski 2022-05-23 18:56 ` Bartosz Golaszewski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHp75Vc-z=K5GPBeXd8JvG=JgjG4Y2U9pBz0hFwjmJ-zP39kZw@mail.gmail.com' \ --to=andy.shevchenko@gmail.com \ --cc=brgl@bgdev.pl \ --cc=damien.lemoal@wdc.com \ --cc=fancer.lancer@gmail.com \ --cc=geert+renesas@glider.be \ --cc=hoan@os.amperecomputing.com \ --cc=linus.walleij@linaro.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=maz@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.