[PATCH 1/2] genirq: allow irq_set_chip_handler_name_locked() to take a const irq_chip

[PATCH 1/2] genirq: allow irq_set_chip_handler_name_locked() to take a const irq_chip

[Michael Walle]

Similar to commit 393e1280f765 ("genirq: Allow irq_chip registration
functions to take a const irq_chip"), allow the
irq_set_chip_handler_name_locked() function to take a const irq_chip
argument.

Signed-off-by: Michael Walle <michael@walle.cc>
---
Given this is the correct approach, can this go through the pinctrl tree?
Of not, do we need an immutable tag?

 include/linux/irqdesc.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index a77584593f7d..1cd4e36890fb 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -209,14 +209,15 @@ static inline void irq_set_handler_locked(struct irq_data *data,
  * Must be called with irq_desc locked and valid parameters.
  */
 static inline void
-irq_set_chip_handler_name_locked(struct irq_data *data, struct irq_chip *chip,
+irq_set_chip_handler_name_locked(struct irq_data *data,
+				 const struct irq_chip *chip,
 				 irq_flow_handler_t handler, const char *name)
 {
 	struct irq_desc *desc = irq_data_to_desc(data);
 
 	desc->handle_irq = handler;
 	desc->name = name;
-	data->chip = chip;
+	data->chip = (struct irq_chip *)chip;
 }
 
 bool irq_check_status_bit(unsigned int irq, unsigned int bitmask);
-- 
2.30.2

[PATCH 2/2] pinctrl: ocelot: make irq_chip immutable

[Michael Walle]

Since recently, the kernel is nagging about mutable irq_chips:

[    2.593426] gpio gpiochip0: (ocelot-gpio): not an immutable chip, please consider fixing it!

Make it const, flag it as IRQCHIP_IMMUTABLE, add the new helper
functions and call the appropriate gpiolib functions.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/pinctrl/pinctrl-ocelot.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index 349e063a04fa..5902a77be5ef 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -1761,6 +1761,7 @@ static void ocelot_irq_mask(struct irq_data *data)
 
 	regmap_update_bits(info->map, REG(OCELOT_GPIO_INTR_ENA, info, gpio),
 			   BIT(gpio % 32), 0);
+	gpiochip_disable_irq(chip, gpio);
 }
 
 static void ocelot_irq_unmask(struct irq_data *data)
@@ -1769,6 +1770,7 @@ static void ocelot_irq_unmask(struct irq_data *data)
 	struct ocelot_pinctrl *info = gpiochip_get_data(chip);
 	unsigned int gpio = irqd_to_hwirq(data);
 
+	gpiochip_enable_irq(chip, gpio);
 	regmap_update_bits(info->map, REG(OCELOT_GPIO_INTR_ENA, info, gpio),
 			   BIT(gpio % 32), BIT(gpio % 32));
 }
@@ -1790,8 +1792,10 @@ static struct irq_chip ocelot_eoi_irqchip = {
 	.irq_mask	= ocelot_irq_mask,
 	.irq_eoi	= ocelot_irq_ack,
 	.irq_unmask	= ocelot_irq_unmask,
-	.flags          = IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED,
+	.flags          = IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED |
+			  IRQCHIP_IMMUTABLE,
 	.irq_set_type	= ocelot_irq_set_type,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS
 };
 
 static struct irq_chip ocelot_irqchip = {
@@ -1800,6 +1804,8 @@ static struct irq_chip ocelot_irqchip = {
 	.irq_ack	= ocelot_irq_ack,
 	.irq_unmask	= ocelot_irq_unmask,
 	.irq_set_type	= ocelot_irq_set_type,
+	.flags          = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS
 };
 
 static int ocelot_irq_set_type(struct irq_data *data, unsigned int type)
@@ -1863,7 +1869,7 @@ static int ocelot_gpiochip_register(struct platform_device *pdev,
 	irq = platform_get_irq_optional(pdev, 0);
 	if (irq > 0) {
 		girq = &gc->irq;
-		girq->chip = &ocelot_irqchip;
+		gpio_irq_chip_set_chip(girq, &ocelot_irqchip);
 		girq->parent_handler = ocelot_irq_handler;
 		girq->num_parents = 1;
 		girq->parents = devm_kcalloc(&pdev->dev, 1,
-- 
2.30.2

Re: [PATCH 1/2] genirq: allow irq_set_chip_handler_name_locked() to take a const irq_chip

[Marc Zyngier]

On Wed, 06 Jul 2022 16:15:52 +0100,
Michael Walle <michael@walle.cc> wrote:
> 
> Similar to commit 393e1280f765 ("genirq: Allow irq_chip registration
> functions to take a const irq_chip"), allow the
> irq_set_chip_handler_name_locked() function to take a const irq_chip
> argument.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> Given this is the correct approach, can this go through the pinctrl tree?
> Of not, do we need an immutable tag?

I'd prefer this sort of change went into the IRQ tree, as it
potentially has a wide ranging effect, and that the rest of these
changes went via the IRQ tree too. It makes it easy to take the branch
down if something goes wrong.

Given that I already do carry a bunch of GPIO related patches, I'm
happy to add that to the mix if Linus ack the second patch.

The change itself looks good to me.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[irqchip: irq/irqchip-next] genirq: Allow irq_set_chip_handler_name_locked() to take a const irq_chip

[irqchip-bot for Michael Walle]

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     ef6e5d61eb7a0a30f776a829274573094185d03d
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/ef6e5d61eb7a0a30f776a829274573094185d03d
Author:        Michael Walle <michael@walle.cc>
AuthorDate:    Wed, 06 Jul 2022 17:15:52 +02:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Mon, 11 Jul 2022 10:12:49 +01:00

genirq: Allow irq_set_chip_handler_name_locked() to take a const irq_chip

Similar to commit 393e1280f765 ("genirq: Allow irq_chip registration
functions to take a const irq_chip"), allow the
irq_set_chip_handler_name_locked() function to take a const irq_chip
argument.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220706151553.1580790-1-michael@walle.cc
---
 include/linux/irqdesc.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index a775845..1cd4e36 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -209,14 +209,15 @@ static inline void irq_set_handler_locked(struct irq_data *data,
  * Must be called with irq_desc locked and valid parameters.
  */
 static inline void
-irq_set_chip_handler_name_locked(struct irq_data *data, struct irq_chip *chip,
+irq_set_chip_handler_name_locked(struct irq_data *data,
+				 const struct irq_chip *chip,
 				 irq_flow_handler_t handler, const char *name)
 {
 	struct irq_desc *desc = irq_data_to_desc(data);
 
 	desc->handle_irq = handler;
 	desc->name = name;
-	data->chip = chip;
+	data->chip = (struct irq_chip *)chip;
 }
 
 bool irq_check_status_bit(unsigned int irq, unsigned int bitmask);

Re: [PATCH 2/2] pinctrl: ocelot: make irq_chip immutable

[Linus Walleij]

On Wed, Jul 6, 2022 at 5:16 PM Michael Walle <michael@walle.cc> wrote:

> Since recently, the kernel is nagging about mutable irq_chips:
>
> [    2.593426] gpio gpiochip0: (ocelot-gpio): not an immutable chip, please consider fixing it!
>
> Make it const, flag it as IRQCHIP_IMMUTABLE, add the new helper
> functions and call the appropriate gpiolib functions.
>
> Signed-off-by: Michael Walle <michael@walle.cc>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I hear that Marc want to queue this patch with 1/2 so go ahead Marc!

Yours,
Linus Walleij

Re: [PATCH 1/2] genirq: allow irq_set_chip_handler_name_locked() to take a const irq_chip

[Linus Walleij]

On Mon, Jul 11, 2022 at 11:11 AM Marc Zyngier <maz@kernel.org> wrote:
> On Wed, 06 Jul 2022 16:15:52 +0100,
> Michael Walle <michael@walle.cc> wrote:
> >
> > Similar to commit 393e1280f765 ("genirq: Allow irq_chip registration
> > functions to take a const irq_chip"), allow the
> > irq_set_chip_handler_name_locked() function to take a const irq_chip
> > argument.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > Given this is the correct approach, can this go through the pinctrl tree?
> > Of not, do we need an immutable tag?
>
> I'd prefer this sort of change went into the IRQ tree, as it
> potentially has a wide ranging effect, and that the rest of these
> changes went via the IRQ tree too. It makes it easy to take the branch
> down if something goes wrong.
>
> Given that I already do carry a bunch of GPIO related patches, I'm
> happy to add that to the mix if Linus ack the second patch.

I ACKed it! Go ahead.

Yours,
Linus Walleij

[irqchip: irq/irqchip-next] pinctrl: ocelot: Make irq_chip immutable

[irqchip-bot for Michael Walle]

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     51ff93923e21ed2862e83f208706e3ca31d6f409
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/51ff93923e21ed2862e83f208706e3ca31d6f409
Author:        Michael Walle <michael@walle.cc>
AuthorDate:    Wed, 06 Jul 2022 17:15:53 +02:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Sat, 16 Jul 2022 11:47:45 +01:00

pinctrl: ocelot: Make irq_chip immutable

Since recently, the kernel is nagging about mutable irq_chips:

[    2.593426] gpio gpiochip0: (ocelot-gpio): not an immutable chip, please consider fixing it!

Make it const, flag it as IRQCHIP_IMMUTABLE, add the new helper
functions and call the appropriate gpiolib functions.

Signed-off-by: Michael Walle <michael@walle.cc>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220706151553.1580790-2-michael@walle.cc
---
 drivers/pinctrl/pinctrl-ocelot.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index 5f4a8c5..425c1a9 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -1761,6 +1761,7 @@ static void ocelot_irq_mask(struct irq_data *data)
 
 	regmap_update_bits(info->map, REG(OCELOT_GPIO_INTR_ENA, info, gpio),
 			   BIT(gpio % 32), 0);
+	gpiochip_disable_irq(chip, gpio);
 }
 
 static void ocelot_irq_unmask(struct irq_data *data)
@@ -1769,6 +1770,7 @@ static void ocelot_irq_unmask(struct irq_data *data)
 	struct ocelot_pinctrl *info = gpiochip_get_data(chip);
 	unsigned int gpio = irqd_to_hwirq(data);
 
+	gpiochip_enable_irq(chip, gpio);
 	regmap_update_bits(info->map, REG(OCELOT_GPIO_INTR_ENA, info, gpio),
 			   BIT(gpio % 32), BIT(gpio % 32));
 }
@@ -1790,8 +1792,10 @@ static struct irq_chip ocelot_eoi_irqchip = {
 	.irq_mask	= ocelot_irq_mask,
 	.irq_eoi	= ocelot_irq_ack,
 	.irq_unmask	= ocelot_irq_unmask,
-	.flags          = IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED,
+	.flags          = IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED |
+			  IRQCHIP_IMMUTABLE,
 	.irq_set_type	= ocelot_irq_set_type,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS
 };
 
 static struct irq_chip ocelot_irqchip = {
@@ -1800,6 +1804,8 @@ static struct irq_chip ocelot_irqchip = {
 	.irq_ack	= ocelot_irq_ack,
 	.irq_unmask	= ocelot_irq_unmask,
 	.irq_set_type	= ocelot_irq_set_type,
+	.flags          = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS
 };
 
 static int ocelot_irq_set_type(struct irq_data *data, unsigned int type)
@@ -1863,7 +1869,7 @@ static int ocelot_gpiochip_register(struct platform_device *pdev,
 	irq = platform_get_irq_optional(pdev, 0);
 	if (irq > 0) {
 		girq = &gc->irq;
-		girq->chip = &ocelot_irqchip;
+		gpio_irq_chip_set_chip(girq, &ocelot_irqchip);
 		girq->parent_handler = ocelot_irq_handler;
 		girq->num_parents = 1;
 		girq->parents = devm_kcalloc(&pdev->dev, 1,