[PATCH v4 1/3] pinctrl: cherryview: Split out irq hw-init into a separate helper function

[PATCH v4 1/3] pinctrl: cherryview: Split out irq hw-init into a separate helper function

[Hans de Goede]

Split out irq hw-init into a separate chv_gpio_irq_init_hw() function.
This is a preparation patch for passing the irqchip when adding the
gpiochip.

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Add kerneldoc for chv_pinctrl.need_valid_mask struct member

Changes in v3:
- Check for pctrl->chip.irq.init_valid_mask instead of storing the result
  of the dmi check in a new need_valid_mask pctrl struct member
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 45 +++++++++++++---------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 582fa8a75559..7a4e2af5153c 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1555,6 +1555,32 @@ static void chv_init_irq_valid_mask(struct gpio_chip *chip,
 	}
 }
 
+static int chv_gpio_irq_init_hw(struct gpio_chip *chip)
+{
+	struct chv_pinctrl *pctrl = gpiochip_get_data(chip);
+
+	/*
+	 * The same set of machines in chv_no_valid_mask[] have incorrectly
+	 * configured GPIOs that generate spurious interrupts so we use
+	 * this same list to apply another quirk for them.
+	 *
+	 * See also https://bugzilla.kernel.org/show_bug.cgi?id=197953.
+	 */
+	if (!pctrl->chip.irq.init_valid_mask) {
+		/*
+		 * Mask all interrupts the community is able to generate
+		 * but leave the ones that can only generate GPEs unmasked.
+		 */
+		chv_writel(GENMASK(31, pctrl->community->nirqs),
+			   pctrl->regs + CHV_INTMASK);
+	}
+
+	/* Clear all interrupts */
+	chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
+
+	return 0;
+}
+
 static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 {
 	const struct chv_gpio_pinrange *range;
@@ -1589,24 +1615,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		}
 	}
 
-	/*
-	 * The same set of machines in chv_no_valid_mask[] have incorrectly
-	 * configured GPIOs that generate spurious interrupts so we use
-	 * this same list to apply another quirk for them.
-	 *
-	 * See also https://bugzilla.kernel.org/show_bug.cgi?id=197953.
-	 */
-	if (!need_valid_mask) {
-		/*
-		 * Mask all interrupts the community is able to generate
-		 * but leave the ones that can only generate GPEs unmasked.
-		 */
-		chv_writel(GENMASK(31, pctrl->community->nirqs),
-			   pctrl->regs + CHV_INTMASK);
-	}
-
-	/* Clear all interrupts */
-	chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
+	chv_gpio_irq_init_hw(chip);
 
 	if (!need_valid_mask) {
 		irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
-- 
2.23.0

[PATCH v4 2/3] pinctrl: cherryview: Add GPIO <-> pin mapping ranges via callback

[Hans de Goede]

When IRQ chip is instantiated via GPIO library flow, the few functions,
in particular the ACPI event registration mechanism, on some of ACPI based
platforms expect that the pin ranges are initialized to that point.

Add GPIO <-> pin mapping ranges via callback in the GPIO library flow.

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 33 ++++++++++++++--------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 7a4e2af5153c..b3f6f7726b04 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1581,6 +1581,27 @@ static int chv_gpio_irq_init_hw(struct gpio_chip *chip)
 	return 0;
 }
 
+static int chv_gpio_add_pin_ranges(struct gpio_chip *chip)
+{
+	struct chv_pinctrl *pctrl = gpiochip_get_data(chip);
+	const struct chv_community *community = pctrl->community;
+	const struct chv_gpio_pinrange *range;
+	int ret, i;
+
+	for (i = 0; i < community->ngpio_ranges; i++) {
+		range = &community->gpio_ranges[i];
+		ret = gpiochip_add_pin_range(chip, dev_name(pctrl->dev),
+					     range->base, range->base,
+					     range->npins);
+		if (ret) {
+			dev_err(pctrl->dev, "failed to add GPIO pin range\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 {
 	const struct chv_gpio_pinrange *range;
@@ -1593,6 +1614,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 
 	chip->ngpio = community->pins[community->npins - 1].number + 1;
 	chip->label = dev_name(pctrl->dev);
+	chip->add_pin_ranges = chv_gpio_add_pin_ranges;
 	chip->parent = pctrl->dev;
 	chip->base = -1;
 	if (need_valid_mask)
@@ -1604,17 +1626,6 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
-	for (i = 0; i < community->ngpio_ranges; i++) {
-		range = &community->gpio_ranges[i];
-		ret = gpiochip_add_pin_range(chip, dev_name(pctrl->dev),
-					     range->base, range->base,
-					     range->npins);
-		if (ret) {
-			dev_err(pctrl->dev, "failed to add GPIO pin range\n");
-			return ret;
-		}
-	}
-
 	chv_gpio_irq_init_hw(chip);
 
 	if (!need_valid_mask) {
-- 
2.23.0

Re: [PATCH v4 2/3] pinctrl: cherryview: Add GPIO <-> pin mapping ranges via callback

[Linus Walleij]

On Thu, Nov 14, 2019 at 11:08 AM Hans de Goede <hdegoede@redhat.com> wrote:

> When IRQ chip is instantiated via GPIO library flow, the few functions,
> in particular the ACPI event registration mechanism, on some of ACPI based
> platforms expect that the pin ranges are initialized to that point.
>
> Add GPIO <-> pin mapping ranges via callback in the GPIO library flow.
>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v4 3/3] pinctrl: cherryview: Pass irqchip when adding gpiochip

[Hans de Goede]

We need to convert all old gpio irqchips to pass the irqchip
setup along when adding the gpio_chip. For more info see
drivers/gpio/TODO.

For chained irqchips this is a pretty straight-forward conversion.

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Add kerneldoc for chv_pinctrl.irq struct member

Changes in v4:
- Rebase on latest intel-pinctrl/for-next
- Fold if (need_valid_mask) ... if (!need_valid_mask) ... into an if-else
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 45 +++++++++++-----------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index b3f6f7726b04..60527b93a711 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -149,6 +149,7 @@ struct chv_pin_context {
  * @chip: GPIO chip in this pin controller
  * @irqchip: IRQ chip in this pin controller
  * @regs: MMIO registers
+ * @irq: Our parent irq
  * @intr_lines: Stores mapping between 16 HW interrupt wires and GPIO
  *		offset (in GPIO number space)
  * @community: Community this pinctrl instance represents
@@ -165,6 +166,7 @@ struct chv_pinctrl {
 	struct gpio_chip chip;
 	struct irq_chip irqchip;
 	void __iomem *regs;
+	unsigned int irq;
 	unsigned int intr_lines[16];
 	const struct chv_community *community;
 	u32 saved_intmask;
@@ -1617,18 +1619,26 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 	chip->add_pin_ranges = chv_gpio_add_pin_ranges;
 	chip->parent = pctrl->dev;
 	chip->base = -1;
-	if (need_valid_mask)
-		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
-
-	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
-	if (ret) {
-		dev_err(pctrl->dev, "Failed to register gpiochip\n");
-		return ret;
-	}
 
-	chv_gpio_irq_init_hw(chip);
+	pctrl->irq = irq;
+	pctrl->irqchip.name = "chv-gpio";
+	pctrl->irqchip.irq_startup = chv_gpio_irq_startup;
+	pctrl->irqchip.irq_ack = chv_gpio_irq_ack;
+	pctrl->irqchip.irq_mask = chv_gpio_irq_mask;
+	pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask;
+	pctrl->irqchip.irq_set_type = chv_gpio_irq_type;
+	pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE;
 
-	if (!need_valid_mask) {
+	chip->irq.chip = &pctrl->irqchip;
+	chip->irq.init_hw = chv_gpio_irq_init_hw;
+	chip->irq.parent_handler = chv_gpio_irq_handler;
+	chip->irq.num_parents = 1;
+	chip->irq.parents = &pctrl->irq;
+	chip->irq.default_type = IRQ_TYPE_NONE;
+	chip->irq.handler = handle_bad_irq;
+	if (need_valid_mask) {
+		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
+	} else {
 		irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
 						community->npins, NUMA_NO_NODE);
 		if (irq_base < 0) {
@@ -1637,18 +1647,9 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		}
 	}
 
-	pctrl->irqchip.name = "chv-gpio";
-	pctrl->irqchip.irq_startup = chv_gpio_irq_startup;
-	pctrl->irqchip.irq_ack = chv_gpio_irq_ack;
-	pctrl->irqchip.irq_mask = chv_gpio_irq_mask;
-	pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask;
-	pctrl->irqchip.irq_set_type = chv_gpio_irq_type;
-	pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE;
-
-	ret = gpiochip_irqchip_add(chip, &pctrl->irqchip, 0,
-				   handle_bad_irq, IRQ_TYPE_NONE);
+	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
 	if (ret) {
-		dev_err(pctrl->dev, "failed to add IRQ chip\n");
+		dev_err(pctrl->dev, "Failed to register gpiochip\n");
 		return ret;
 	}
 
@@ -1662,8 +1663,6 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		}
 	}
 
-	gpiochip_set_chained_irqchip(chip, &pctrl->irqchip, irq,
-				     chv_gpio_irq_handler);
 	return 0;
 }
 
-- 
2.23.0

Re: [PATCH v4 3/3] pinctrl: cherryview: Pass irqchip when adding gpiochip

[Andy Shevchenko]

On Thu, Nov 14, 2019 at 11:08:04AM +0100, Hans de Goede wrote:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
> 
> For chained irqchips this is a pretty straight-forward conversion.
> 

Pushed all three to my review and testing queue, thanks!
(For v5.6 I suppose due to dependencies)

> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Add kerneldoc for chv_pinctrl.irq struct member
> 
> Changes in v4:
> - Rebase on latest intel-pinctrl/for-next
> - Fold if (need_valid_mask) ... if (!need_valid_mask) ... into an if-else
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 45 +++++++++++-----------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index b3f6f7726b04..60527b93a711 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -149,6 +149,7 @@ struct chv_pin_context {
>   * @chip: GPIO chip in this pin controller
>   * @irqchip: IRQ chip in this pin controller
>   * @regs: MMIO registers
> + * @irq: Our parent irq
>   * @intr_lines: Stores mapping between 16 HW interrupt wires and GPIO
>   *		offset (in GPIO number space)
>   * @community: Community this pinctrl instance represents
> @@ -165,6 +166,7 @@ struct chv_pinctrl {
>  	struct gpio_chip chip;
>  	struct irq_chip irqchip;
>  	void __iomem *regs;
> +	unsigned int irq;
>  	unsigned int intr_lines[16];
>  	const struct chv_community *community;
>  	u32 saved_intmask;
> @@ -1617,18 +1619,26 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>  	chip->add_pin_ranges = chv_gpio_add_pin_ranges;
>  	chip->parent = pctrl->dev;
>  	chip->base = -1;
> -	if (need_valid_mask)
> -		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
> -
> -	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
> -	if (ret) {
> -		dev_err(pctrl->dev, "Failed to register gpiochip\n");
> -		return ret;
> -	}
>  
> -	chv_gpio_irq_init_hw(chip);
> +	pctrl->irq = irq;
> +	pctrl->irqchip.name = "chv-gpio";
> +	pctrl->irqchip.irq_startup = chv_gpio_irq_startup;
> +	pctrl->irqchip.irq_ack = chv_gpio_irq_ack;
> +	pctrl->irqchip.irq_mask = chv_gpio_irq_mask;
> +	pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask;
> +	pctrl->irqchip.irq_set_type = chv_gpio_irq_type;
> +	pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE;
>  
> -	if (!need_valid_mask) {
> +	chip->irq.chip = &pctrl->irqchip;
> +	chip->irq.init_hw = chv_gpio_irq_init_hw;
> +	chip->irq.parent_handler = chv_gpio_irq_handler;
> +	chip->irq.num_parents = 1;
> +	chip->irq.parents = &pctrl->irq;
> +	chip->irq.default_type = IRQ_TYPE_NONE;
> +	chip->irq.handler = handle_bad_irq;
> +	if (need_valid_mask) {
> +		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
> +	} else {
>  		irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
>  						community->npins, NUMA_NO_NODE);
>  		if (irq_base < 0) {
> @@ -1637,18 +1647,9 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>  		}
>  	}
>  
> -	pctrl->irqchip.name = "chv-gpio";
> -	pctrl->irqchip.irq_startup = chv_gpio_irq_startup;
> -	pctrl->irqchip.irq_ack = chv_gpio_irq_ack;
> -	pctrl->irqchip.irq_mask = chv_gpio_irq_mask;
> -	pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask;
> -	pctrl->irqchip.irq_set_type = chv_gpio_irq_type;
> -	pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE;
> -
> -	ret = gpiochip_irqchip_add(chip, &pctrl->irqchip, 0,
> -				   handle_bad_irq, IRQ_TYPE_NONE);
> +	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
>  	if (ret) {
> -		dev_err(pctrl->dev, "failed to add IRQ chip\n");
> +		dev_err(pctrl->dev, "Failed to register gpiochip\n");
>  		return ret;
>  	}
>  
> @@ -1662,8 +1663,6 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>  		}
>  	}
>  
> -	gpiochip_set_chained_irqchip(chip, &pctrl->irqchip, irq,
> -				     chv_gpio_irq_handler);
>  	return 0;
>  }
>  
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] pinctrl: cherryview: Pass irqchip when adding gpiochip

[Linus Walleij]

On Fri, Nov 15, 2019 at 11:07 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, Nov 14, 2019 at 11:08:04AM +0100, Hans de Goede wrote:
> > We need to convert all old gpio irqchips to pass the irqchip
> > setup along when adding the gpio_chip. For more info see
> > drivers/gpio/TODO.
> >
> > For chained irqchips this is a pretty straight-forward conversion.
> >
>
> Pushed all three to my review and testing queue, thanks!
> (For v5.6 I suppose due to dependencies)

Ooops I noticed this and took out the two others I started to apply.
Sorry for the confusion, I let you send these to me when you think
the time is right. (Like right after v5.5-rc1).

Yours,
Linus Walleij

Re: [PATCH v4 1/3] pinctrl: cherryview: Split out irq hw-init into a separate helper function

[Linus Walleij]

On Thu, Nov 14, 2019 at 11:08 AM Hans de Goede <hdegoede@redhat.com> wrote:

> Split out irq hw-init into a separate chv_gpio_irq_init_hw() function.
> This is a preparation patch for passing the irqchip when adding the
> gpiochip.
>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Add kerneldoc for chv_pinctrl.need_valid_mask struct member
>
> Changes in v3:
> - Check for pctrl->chip.irq.init_valid_mask instead of storing the result
>   of the dmi check in a new need_valid_mask pctrl struct member

Patch applied.

Yours,
Linus Walleij