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

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

[Hans de Goede]

Hi All,

Here is a patch for Cherry Trail devices to pass the irqchip when adding
the gpiochip instead of registering it separately. Similar to how
this is done for Bay Trail devices in Andy's recent series:
"[RESEND][PATCH v2 0/7] gpiolib: fix GPIO <-> pin mapping registration".

Note this series depends on that series as well as on the cherryview changes
currently queued in pinctrl/intel.git/for-next .

Regards,

Hans

[PATCH 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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 57 +++++++++++++---------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index dff2a81250b6..c5a44be1e904 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -165,6 +165,7 @@ struct chv_pinctrl {
 	struct gpio_chip chip;
 	struct irq_chip irqchip;
 	void __iomem *regs;
+	bool need_valid_mask;
 	unsigned intr_lines[16];
 	const struct chv_community *community;
 	u32 saved_intmask;
@@ -1555,13 +1556,40 @@ 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->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);
+
+	return 0;
+}
+
 static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 {
 	const struct chv_gpio_pinrange *range;
 	struct gpio_chip *chip = &pctrl->chip;
-	bool need_valid_mask = !dmi_check_system(chv_no_valid_mask);
 	const struct chv_community *community = pctrl->community;
-	int ret, i, irq_base;
+	int ret, i, irq_base = 0;
+
+	pctrl->need_valid_mask = !dmi_check_system(chv_no_valid_mask);
 
 	*chip = chv_gpio_chip;
 
@@ -1569,7 +1597,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 	chip->label = dev_name(pctrl->dev);
 	chip->parent = pctrl->dev;
 	chip->base = -1;
-	if (need_valid_mask)
+	if (pctrl->need_valid_mask)
 		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
 
 	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
@@ -1589,26 +1617,9 @@ 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) {
+	if (!pctrl->need_valid_mask) {
 		irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
 						community->npins, NUMA_NO_NODE);
 		if (irq_base < 0) {
@@ -1632,7 +1643,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
-	if (!need_valid_mask) {
+	if (!pctrl->need_valid_mask) {
 		for (i = 0; i < community->ngpio_ranges; i++) {
 			range = &community->gpio_ranges[i];
 
-- 
2.23.0

[PATCH 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.

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 c5a44be1e904..38b359db6013 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1582,6 +1582,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;
@@ -1595,6 +1616,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 (pctrl->need_valid_mask)
@@ -1606,17 +1628,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 (!pctrl->need_valid_mask) {
-- 
2.23.0

[PATCH 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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 41 +++++++++++-----------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 38b359db6013..1f9751ae0ed0 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -166,6 +166,7 @@ struct chv_pinctrl {
 	struct irq_chip irqchip;
 	void __iomem *regs;
 	bool need_valid_mask;
+	unsigned int irq;
 	unsigned intr_lines[16];
 	const struct chv_community *community;
 	u32 saved_intmask;
@@ -1619,16 +1620,25 @@ 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 (pctrl->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;
-	}
+	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;
 
-	chv_gpio_irq_init_hw(chip);
+	chip->irq.chip = &pctrl->irqchip;
+	if (pctrl->need_valid_mask)
+		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
+	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 (!pctrl->need_valid_mask) {
 		irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
@@ -1639,18 +1649,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;
 	}
 
@@ -1664,8 +1665,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 1/3] pinctrl: cherryview: Split out irq hw-init into a separate helper function

[Mika Westerberg]

On Wed, Nov 06, 2019 at 01:09:54PM +0100, Hans de Goede 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.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 57 +++++++++++++---------
>  1 file changed, 34 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index dff2a81250b6..c5a44be1e904 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -165,6 +165,7 @@ struct chv_pinctrl {
>  	struct gpio_chip chip;
>  	struct irq_chip irqchip;
>  	void __iomem *regs;
> +	bool need_valid_mask;

Please add kernel-doc about this one.

Otherwise looks good.

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

[Mika Westerberg]

On Wed, Nov 06, 2019 at 01:09:56PM +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.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 41 +++++++++++-----------
>  1 file changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index 38b359db6013..1f9751ae0ed0 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -166,6 +166,7 @@ struct chv_pinctrl {
>  	struct irq_chip irqchip;
>  	void __iomem *regs;
>  	bool need_valid_mask;
> +	unsigned int irq;

Same here.