[PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Andy Shevchenko]

Since recently, the kernel is nagging about mutable irq_chips:

   "not an immutable chip, please consider fixing it!"

Drop the unneeded copy, flag it as IRQCHIP_IMMUTABLE, add the new
helper functions and call the appropriate gpiolib functions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: dropped renaming part (Mika)
 drivers/pinctrl/intel/pinctrl-intel.c | 40 +++++++++++++++++----------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 826d494f3cc6..4845d0b74df9 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1039,15 +1039,14 @@ static void intel_gpio_irq_ack(struct irq_data *d)
 	}
 }
 
-static void intel_gpio_irq_mask_unmask(struct irq_data *d, bool mask)
+static void intel_gpio_irq_mask_unmask(struct gpio_chip *gc, irq_hw_number_t hwirq, bool mask)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
 	const struct intel_community *community;
 	const struct intel_padgroup *padgrp;
 	int pin;
 
-	pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), &community, &padgrp);
+	pin = intel_gpio_to_pin(pctrl, hwirq, &community, &padgrp);
 	if (pin >= 0) {
 		unsigned int gpp, gpp_offset;
 		unsigned long flags;
@@ -1077,12 +1076,20 @@ static void intel_gpio_irq_mask_unmask(struct irq_data *d, bool mask)
 
 static void intel_gpio_irq_mask(struct irq_data *d)
 {
-	intel_gpio_irq_mask_unmask(d, true);
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+
+	intel_gpio_irq_mask_unmask(gc, hwirq, true);
+	gpiochip_disable_irq(gc, hwirq);
 }
 
 static void intel_gpio_irq_unmask(struct irq_data *d)
 {
-	intel_gpio_irq_mask_unmask(d, false);
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+
+	gpiochip_enable_irq(gc, hwirq);
+	intel_gpio_irq_mask_unmask(gc, hwirq, false);
 }
 
 static int intel_gpio_irq_type(struct irq_data *d, unsigned int type)
@@ -1157,6 +1164,17 @@ static int intel_gpio_irq_wake(struct irq_data *d, unsigned int on)
 	return 0;
 }
 
+static const struct irq_chip intel_gpio_irq_chip = {
+	.name		= "intel-gpio",
+	.irq_ack	= intel_gpio_irq_ack,
+	.irq_mask	= intel_gpio_irq_mask,
+	.irq_unmask	= intel_gpio_irq_unmask,
+	.irq_set_type	= intel_gpio_irq_type,
+	.irq_set_wake	= intel_gpio_irq_wake,
+	.flags		= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
 static int intel_gpio_community_irq_handler(struct intel_pinctrl *pctrl,
 					    const struct intel_community *community)
 {
@@ -1319,15 +1337,6 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 	pctrl->chip.add_pin_ranges = intel_gpio_add_pin_ranges;
 	pctrl->irq = irq;
 
-	/* Setup IRQ chip */
-	pctrl->irqchip.name = dev_name(pctrl->dev);
-	pctrl->irqchip.irq_ack = intel_gpio_irq_ack;
-	pctrl->irqchip.irq_mask = intel_gpio_irq_mask;
-	pctrl->irqchip.irq_unmask = intel_gpio_irq_unmask;
-	pctrl->irqchip.irq_set_type = intel_gpio_irq_type;
-	pctrl->irqchip.irq_set_wake = intel_gpio_irq_wake;
-	pctrl->irqchip.flags = IRQCHIP_MASK_ON_SUSPEND;
-
 	/*
 	 * On some platforms several GPIO controllers share the same interrupt
 	 * line.
@@ -1340,8 +1349,9 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
+	/* Setup IRQ chip */
 	girq = &pctrl->chip.irq;
-	girq->chip = &pctrl->irqchip;
+	gpio_irq_chip_set_chip(girq, &intel_gpio_irq_chip);
 	/* This will let us handle the IRQ in the driver */
 	girq->parent_handler = NULL;
 	girq->num_parents = 0;
-- 
2.35.1

[PATCH v2 2/2] pinctrl: intel: Drop unsued irqchip member in struct intel_pinctrl

[Andy Shevchenko]

There is no users of irqchip member in struct intel_pinctrl. Drop it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
 drivers/pinctrl/intel/pinctrl-intel.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index c4fef03b663f..710341bb67cc 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -223,7 +223,6 @@ struct intel_pinctrl_context {
  * @pctldesc: Pin controller description
  * @pctldev: Pointer to the pin controller device
  * @chip: GPIO chip in this pin controller
- * @irqchip: IRQ chip in this pin controller
  * @soc: SoC/PCH specific pin configuration data
  * @communities: All communities in this pin controller
  * @ncommunities: Number of communities in this pin controller
@@ -236,7 +235,6 @@ struct intel_pinctrl {
 	struct pinctrl_desc pctldesc;
 	struct pinctrl_dev *pctldev;
 	struct gpio_chip chip;
-	struct irq_chip irqchip;
 	const struct intel_pinctrl_soc_data *soc;
 	struct intel_community *communities;
 	size_t ncommunities;
-- 
2.35.1

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Mika Westerberg]

On Tue, May 17, 2022 at 07:38:19PM +0300, Andy Shevchenko wrote:
> +static const struct irq_chip intel_gpio_irq_chip = {
> +	.name		= "intel-gpio",
> +	.irq_ack	= intel_gpio_irq_ack,
> +	.irq_mask	= intel_gpio_irq_mask,
> +	.irq_unmask	= intel_gpio_irq_unmask,
> +	.irq_set_type	= intel_gpio_irq_type,
> +	.irq_set_wake	= intel_gpio_irq_wake,
> +	.flags		= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};

You still have the inconsistent alignment here.

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Andy Shevchenko]

On Wed, May 18, 2022 at 08:08:17AM +0300, Mika Westerberg wrote:
> On Tue, May 17, 2022 at 07:38:19PM +0300, Andy Shevchenko wrote:
> > +static const struct irq_chip intel_gpio_irq_chip = {
> > +	.name		= "intel-gpio",
> > +	.irq_ack	= intel_gpio_irq_ack,
> > +	.irq_mask	= intel_gpio_irq_mask,
> > +	.irq_unmask	= intel_gpio_irq_unmask,
> > +	.irq_set_type	= intel_gpio_irq_type,
> > +	.irq_set_wake	= intel_gpio_irq_wake,
> > +	.flags		= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> > +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > +};
> 
> You still have the inconsistent alignment here.

I'm not sure what problem do you see.

If you are talking about last line, it's special and that's how it's done
in other drivers which have been converted already.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Mika Westerberg]

On Wed, May 18, 2022 at 11:46:06AM +0300, Andy Shevchenko wrote:
> On Wed, May 18, 2022 at 08:08:17AM +0300, Mika Westerberg wrote:
> > On Tue, May 17, 2022 at 07:38:19PM +0300, Andy Shevchenko wrote:
> > > +static const struct irq_chip intel_gpio_irq_chip = {
> > > +	.name		= "intel-gpio",
> > > +	.irq_ack	= intel_gpio_irq_ack,
> > > +	.irq_mask	= intel_gpio_irq_mask,
> > > +	.irq_unmask	= intel_gpio_irq_unmask,
> > > +	.irq_set_type	= intel_gpio_irq_type,
> > > +	.irq_set_wake	= intel_gpio_irq_wake,
> > > +	.flags		= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> > > +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > > +};
> > 
> > You still have the inconsistent alignment here.
> 
> I'm not sure what problem do you see.

I mean the tab alignment you use:

	.name		= "intel-gpio",
	.irq_ack	= intel_gpio_irq_ack,
	.irq_mask	= intel_gpio_irq_mask,
	.irq_unmask	= intel_gpio_irq_unmask,
	.irq_set_type	= intel_gpio_irq_type,
	.irq_set_wake	= intel_gpio_irq_wake,

All the other struct initializations in the driver use this style (and I
prefer it too):

	.name = "intel-gpio",
	.irq_ack = intel_gpio_irq_ack,
	.irq_mask = intel_gpio_irq_mask,
	.irq_unmask = intel_gpio_irq_unmask,
	.irq_set_type = intel_gpio_irq_type,
	.irq_set_wake = intel_gpio_irq_wake,

Hope this explains.

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Andy Shevchenko]

On Wed, May 18, 2022 at 11:19 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, May 18, 2022 at 11:46:06AM +0300, Andy Shevchenko wrote:
> > On Wed, May 18, 2022 at 08:08:17AM +0300, Mika Westerberg wrote:
> > > On Tue, May 17, 2022 at 07:38:19PM +0300, Andy Shevchenko wrote:
> > > > +static const struct irq_chip intel_gpio_irq_chip = {
> > > > + .name           = "intel-gpio",
> > > > + .irq_ack        = intel_gpio_irq_ack,
> > > > + .irq_mask       = intel_gpio_irq_mask,
> > > > + .irq_unmask     = intel_gpio_irq_unmask,
> > > > + .irq_set_type   = intel_gpio_irq_type,
> > > > + .irq_set_wake   = intel_gpio_irq_wake,
> > > > + .flags          = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> > > > + GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > > > +};
> > >
> > > You still have the inconsistent alignment here.
> >
> > I'm not sure what problem do you see.
>
> I mean the tab alignment you use:
>
>         .name           = "intel-gpio",
>         .irq_ack        = intel_gpio_irq_ack,
>         .irq_mask       = intel_gpio_irq_mask,
>         .irq_unmask     = intel_gpio_irq_unmask,
>         .irq_set_type   = intel_gpio_irq_type,
>         .irq_set_wake   = intel_gpio_irq_wake,
>
> All the other struct initializations in the driver use this style (and I
> prefer it too):
>
>         .name = "intel-gpio",
>         .irq_ack = intel_gpio_irq_ack,
>         .irq_mask = intel_gpio_irq_mask,
>         .irq_unmask = intel_gpio_irq_unmask,
>         .irq_set_type = intel_gpio_irq_type,
>         .irq_set_wake = intel_gpio_irq_wake,
>
> Hope this explains.

Yes, thanks!

Okay, can you give your conditional Ack if there are no other
comments? I will fix it locally.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Mika Westerberg]

On Wed, May 18, 2022 at 12:05:38PM +0200, Andy Shevchenko wrote:
> On Wed, May 18, 2022 at 11:19 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Wed, May 18, 2022 at 11:46:06AM +0300, Andy Shevchenko wrote:
> > > On Wed, May 18, 2022 at 08:08:17AM +0300, Mika Westerberg wrote:
> > > > On Tue, May 17, 2022 at 07:38:19PM +0300, Andy Shevchenko wrote:
> > > > > +static const struct irq_chip intel_gpio_irq_chip = {
> > > > > + .name           = "intel-gpio",
> > > > > + .irq_ack        = intel_gpio_irq_ack,
> > > > > + .irq_mask       = intel_gpio_irq_mask,
> > > > > + .irq_unmask     = intel_gpio_irq_unmask,
> > > > > + .irq_set_type   = intel_gpio_irq_type,
> > > > > + .irq_set_wake   = intel_gpio_irq_wake,
> > > > > + .flags          = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> > > > > + GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > > > > +};
> > > >
> > > > You still have the inconsistent alignment here.
> > >
> > > I'm not sure what problem do you see.
> >
> > I mean the tab alignment you use:
> >
> >         .name           = "intel-gpio",
> >         .irq_ack        = intel_gpio_irq_ack,
> >         .irq_mask       = intel_gpio_irq_mask,
> >         .irq_unmask     = intel_gpio_irq_unmask,
> >         .irq_set_type   = intel_gpio_irq_type,
> >         .irq_set_wake   = intel_gpio_irq_wake,
> >
> > All the other struct initializations in the driver use this style (and I
> > prefer it too):
> >
> >         .name = "intel-gpio",
> >         .irq_ack = intel_gpio_irq_ack,
> >         .irq_mask = intel_gpio_irq_mask,
> >         .irq_unmask = intel_gpio_irq_unmask,
> >         .irq_set_type = intel_gpio_irq_type,
> >         .irq_set_wake = intel_gpio_irq_wake,
> >
> > Hope this explains.
> 
> Yes, thanks!
> 
> Okay, can you give your conditional Ack if there are no other
> comments? I will fix it locally.

Sure. There was typo also in the second patch $subject, please fix it
too while you apply them. For both,

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v2 1/2] pinctrl: intel: make irq_chip immutable

[Andy Shevchenko]

On Wed, May 18, 2022 at 01:11:42PM +0300, Mika Westerberg wrote:
> On Wed, May 18, 2022 at 12:05:38PM +0200, Andy Shevchenko wrote:
> > On Wed, May 18, 2022 at 11:19 AM Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > On Wed, May 18, 2022 at 11:46:06AM +0300, Andy Shevchenko wrote:

...

> > > Hope this explains.
> > 
> > Yes, thanks!
> > 
> > Okay, can you give your conditional Ack if there are no other
> > comments? I will fix it locally.
> 
> Sure. There was typo also in the second patch $subject, please fix it
> too while you apply them.

Fixed.

> For both,
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko