[PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips

[Sergio Paracuellos]

The GPIO controller's registers are organized as sets of eight 32-bit
registers with each set controlling a bank of up to 32 pins. A single
interrupt is shared for all of the banks handled by the controller.
The driver implements this using three gpio chip instances every one
with its own irq chip. Every single pin can generate interrupts having
a total of 96 possible interrupts here. It looks like there is a problem
with interrupts being properly mapped to the gpio bank using this solution.
This problem report is in the following lore's link [0].

Device tree is using two cells for this, so only the interrupt pin and the
interrupt type are described there. Changing to have three cells to setup
also the bank and implement 'of_node_instance_match()' would also work but
this would be an ABI breakage and also a bit incoherent since gpios itself
are also using two cells and properly mapped in desired bank using through
its pin number on 'of_xlate()'.

That said, the solution adopted here is registering register a linear IRQ
domain of the total of 96 interrupts shared with the three gpio chip instances
so the bank and the interrupt is properly decoded and devices using gpio
IRQs properly work.

[0]: https://lore.kernel.org/linux-gpio/CAAMcf8C_A9dJ_v4QRKtb9eGNOpJ7BZNOGsFP4i2WFOZxOVBPnQ@xxxxxxxxxxxxxx/T/#u

Fixes: 4ba9c3afda41 ("gpio: mt7621: Add a driver for MT7621")
Co-developed-by: Vicente Bergas <vicencb@xxxxxxxxx>
Signed-off-by: Vicente Bergas <vicencb@xxxxxxxxx>
Tested-by: Vicente Bergas <vicencb@xxxxxxxxx>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
---
 drivers/gpio/gpio-mt7621.c | 281 ++++++++++++++++++++++++++++---------
 1 file changed, 213 insertions(+), 68 deletions(-)

diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
index 91230be51587..3ef10d6c4798 100644
--- a/drivers/gpio/gpio-mt7621.c
+++ b/drivers/gpio/gpio-mt7621.c
@@ -29,8 +29,8 @@
 #define GPIO_REG_EDGE		0xA0
 
 struct mtk_gc {
-	struct irq_chip irq_chip;
 	struct gpio_generic_chip chip;
+	struct mtk *parent_priv;
 	int bank;
 	u32 rising;
 	u32 falling;
@@ -41,20 +41,32 @@ struct mtk_gc {
 /**
  * struct mtk - state container for
  * data of the platform driver. It is 3
- * separate gpio-chip each one with its
- * own irq_chip.
- * @dev: device instance
+ * separate gpio-chip having an IRQ
+ * linear domain shared for all of them
+ * @pdev: platform device instance
  * @base: memory base address
+ * @irq_domain: IRQ linear domain shared across the three gpio chips
  * @gpio_irq: irq number from the device tree
+ * @num_gpios: total number of gpio pins on the three gpio chips
  * @gc_map: array of the gpio chips
  */
 struct mtk {
-	struct device *dev;
+	struct platform_device *pdev;
 	void __iomem *base;
+	struct irq_domain *irq_domain;
 	int gpio_irq;
+	int num_gpios;
 	struct mtk_gc gc_map[MTK_BANK_CNT];
 };
 
+static inline struct mtk *
+mt7621_gpio_gc_to_priv(struct gpio_chip *gc)
+{
+	struct mtk_gc *bank = gpiochip_get_data(gc);
+
+	return bank->parent_priv;
+}
+
 static inline struct mtk_gc *
 to_mediatek_gpio(struct gpio_chip *chip)
 {
@@ -67,7 +79,7 @@ static inline void
 mtk_gpio_w32(struct mtk_gc *rg, u32 offset, u32 val)
 {
 	struct gpio_chip *gc = &rg->chip.gc;
-	struct mtk *mtk = gpiochip_get_data(gc);
+	struct mtk *mtk = mt7621_gpio_gc_to_priv(gc);
 
 	offset = (rg->bank * GPIO_BANK_STRIDE) + offset;
 	gpio_generic_write_reg(&rg->chip, mtk->base + offset, val);
@@ -77,60 +89,79 @@ static inline u32
 mtk_gpio_r32(struct mtk_gc *rg, u32 offset)
 {
 	struct gpio_chip *gc = &rg->chip.gc;
-	struct mtk *mtk = gpiochip_get_data(gc);
+	struct mtk *mtk = mt7621_gpio_gc_to_priv(gc);
 
 	offset = (rg->bank * GPIO_BANK_STRIDE) + offset;
 	return gpio_generic_read_reg(&rg->chip, mtk->base + offset);
 }
 
-static irqreturn_t
-mediatek_gpio_irq_handler(int irq, void *data)
+static void
+mt7621_gpio_irq_bank_handler(struct mtk_gc *bank)
 {
-	struct gpio_chip *gc = data;
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	irqreturn_t ret = IRQ_NONE;
+	struct mtk *priv = bank->parent_priv;
+	struct irq_domain *domain = priv->irq_domain;
+	int hwbase = bank->chip.gc.offset;
 	unsigned long pending;
-	int bit;
+	unsigned int offset;
+
+	pending = mtk_gpio_r32(bank, GPIO_REG_STAT);
+	if (!pending)
+		return;
+
+	mtk_gpio_w32(bank, GPIO_REG_STAT, pending);
+
+	for_each_set_bit(offset, &pending, MTK_BANK_WIDTH)
+		generic_handle_domain_irq(domain, hwbase + offset);
+}
+
+static void
+mt7621_gpio_irq_handler(struct irq_desc *desc)
+{
+	struct mtk *priv = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int i;
 
-	pending = mtk_gpio_r32(rg, GPIO_REG_STAT);
+	chained_irq_enter(chip, desc);
+	for (i = 0; i < MTK_BANK_CNT; i++) {
+		struct mtk_gc *bank = &priv->gc_map[i];
 
-	for_each_set_bit(bit, &pending, MTK_BANK_WIDTH) {
-		generic_handle_domain_irq(gc->irq.domain, bit);
-		mtk_gpio_w32(rg, GPIO_REG_STAT, BIT(bit));
-		ret |= IRQ_HANDLED;
+		mt7621_gpio_irq_bank_handler(bank);
 	}
+	chained_irq_exit(chip, desc);
+}
 
-	return ret;
+static int
+mt7621_gpio_hwirq_to_offset(irq_hw_number_t hwirq, struct mtk_gc *bank)
+{
+	return hwirq - bank->chip.gc.offset;
 }
 
 static void
 mediatek_gpio_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
 	u32 rise, fall, high, low;
 
-	gpiochip_enable_irq(gc, d->hwirq);
-
 	guard(gpio_generic_lock_irqsave)(&rg->chip);
 
 	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
 	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
 	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
 	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
-	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (BIT(pin) & rg->rising));
-	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (BIT(pin) & rg->falling));
-	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (BIT(pin) & rg->hlevel));
-	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (BIT(pin) & rg->llevel));
+	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (BIT(mask) & rg->rising));
+	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (BIT(mask) & rg->falling));
+	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (BIT(mask) & rg->hlevel));
+	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (BIT(mask) & rg->llevel));
 }
 
 static void
 mediatek_gpio_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
 	u32 rise, fall, high, low;
 
 	scoped_guard(gpio_generic_lock_irqsave, &rg->chip) {
@@ -138,22 +169,19 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 		fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
 		high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
 		low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
-		mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(pin));
+		mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(mask));
 	}
-
-	gpiochip_disable_irq(gc, d->hwirq);
 }
 
 static int
 mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
-	u32 mask = BIT(pin);
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = BIT(mt7621_gpio_hwirq_to_offset(d->hwirq, rg));
 
 	if (type == IRQ_TYPE_PROBE) {
 		if ((rg->rising | rg->falling |
@@ -190,6 +218,26 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
+static int
+mt7621_gpio_irq_reqres(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	unsigned int irq = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
+
+	return gpiochip_reqres_irq(gc, irq);
+}
+
+static void
+mt7621_gpio_irq_relres(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	unsigned int irq = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
+
+	gpiochip_relres_irq(gc, irq);
+}
+
 static int
 mediatek_gpio_xlate(struct gpio_chip *chip,
 		    const struct of_phandle_args *spec, u32 *flags)
@@ -208,14 +256,123 @@ mediatek_gpio_xlate(struct gpio_chip *chip,
 
 static const struct irq_chip mt7621_irq_chip = {
 	.name		= "mt7621-gpio",
+	.irq_request_resources = mt7621_gpio_irq_reqres,
+	.irq_release_resources = mt7621_gpio_irq_relres,
 	.irq_mask_ack	= mediatek_gpio_irq_mask,
 	.irq_mask	= mediatek_gpio_irq_mask,
 	.irq_unmask	= mediatek_gpio_irq_unmask,
 	.irq_set_type	= mediatek_gpio_irq_type,
 	.flags		= IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
+static void
+mt7621_gpio_remove(struct platform_device *pdev)
+{
+	struct mtk *priv = platform_get_drvdata(pdev);
+	int offset, virq;
+
+	if (priv->gpio_irq > 0)
+		irq_set_chained_handler_and_data(priv->gpio_irq, NULL, NULL);
+
+	/* Remove all IRQ mappings and delete the domain */
+	if (priv->irq_domain) {
+		for (offset = 0; offset < priv->num_gpios; offset++) {
+			virq = irq_find_mapping(priv->irq_domain, offset);
+			irq_dispose_mapping(virq);
+		}
+		irq_domain_remove(priv->irq_domain);
+	}
+}
+
+static struct mtk_gc *
+mt7621_gpio_hwirq_to_bank(struct mtk *priv, irq_hw_number_t hwirq)
+{
+	int i;
+
+	for (i = 0; i < MTK_BANK_CNT; i++) {
+		struct mtk_gc *bank = &priv->gc_map[i];
+
+		if (hwirq >= bank->chip.gc.offset &&
+		    hwirq < (bank->chip.gc.offset + bank->chip.gc.ngpio))
+			return bank;
+	}
+
+	return NULL;
+}
+
+static int
+mt7621_gpio_irq_map(struct irq_domain *d, unsigned int irq,
+		    irq_hw_number_t hwirq)
+{
+	struct mtk *priv = d->host_data;
+	struct mtk_gc *bank = mt7621_gpio_hwirq_to_bank(priv, hwirq);
+	struct platform_device *pdev = priv->pdev;
+	int ret;
+
+	if (!bank)
+		return -EINVAL;
+
+	dev_dbg(&pdev->dev, "Mapping irq %d for gpio line %d (bank %d)\n",
+		irq, (int)hwirq, bank->bank);
+
+	ret = irq_set_chip_data(irq, &bank->chip.gc);
+	if (ret < 0)
+		return ret;
+
+	irq_set_chip_and_handler(irq, &mt7621_irq_chip, handle_simple_irq);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static void
+mt7621_gpio_irq_unmap(struct irq_domain *d, unsigned int irq)
+{
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+}
+
+static const struct irq_domain_ops mt7621_gpio_irq_domain_ops = {
+	.map = mt7621_gpio_irq_map,
+	.unmap = mt7621_gpio_irq_unmap,
+	.xlate = irq_domain_xlate_twocell,
+};
+
+static int
+mt7621_gpio_irq_setup(struct platform_device *pdev,
+		      struct mtk *priv)
+{
+	struct device *dev = &pdev->dev;
+
+	priv->irq_domain = irq_domain_create_linear(dev_fwnode(dev),
+						    priv->num_gpios,
+						    &mt7621_gpio_irq_domain_ops,
+						    priv);
+	if (!priv->irq_domain) {
+		dev_err(dev, "Couldn't allocate IRQ domain\n");
+		return -ENXIO;
+	}
+
+	irq_set_chained_handler_and_data(priv->gpio_irq,
+					 mt7621_gpio_irq_handler, priv);
+	irq_set_status_flags(priv->gpio_irq, IRQ_DISABLE_UNLAZY);
+
+	return 0;
+}
+
+static int
+mt7621_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+	struct mtk *priv = mt7621_gpio_gc_to_priv(gc);
+	/* gc_offset is relative to this gpio_chip; want real offset */
+	int hwirq = offset + gc->offset;
+
+	if (hwirq >= priv->num_gpios)
+		return -ENXIO;
+
+	return irq_create_mapping(priv->irq_domain, hwirq);
+}
+
 static int
 mediatek_gpio_bank_probe(struct device *dev, int bank)
 {
@@ -228,6 +385,7 @@ mediatek_gpio_bank_probe(struct device *dev, int bank)
 	rg = &mtk->gc_map[bank];
 	memset(rg, 0, sizeof(*rg));
 
+	rg->parent_priv = mtk;
 	rg->bank = bank;
 
 	dat = mtk->base + GPIO_REG_DATA + (rg->bank * GPIO_BANK_STRIDE);
@@ -253,41 +411,17 @@ mediatek_gpio_bank_probe(struct device *dev, int bank)
 
 	rg->chip.gc.of_gpio_n_cells = 2;
 	rg->chip.gc.of_xlate = mediatek_gpio_xlate;
+	rg->chip.gc.ngpio = MTK_BANK_WIDTH;
 	rg->chip.gc.label = devm_kasprintf(dev, GFP_KERNEL, "%s-bank%d",
 					dev_name(dev), bank);
 	if (!rg->chip.gc.label)
 		return -ENOMEM;
 
 	rg->chip.gc.offset = bank * MTK_BANK_WIDTH;
+	if (mtk->gpio_irq > 0)
+		rg->chip.gc.to_irq = mt7621_gpio_to_irq;
 
-	if (mtk->gpio_irq) {
-		struct gpio_irq_chip *girq;
-
-		/*
-		 * Directly request the irq here instead of passing
-		 * a flow-handler because the irq is shared.
-		 */
-		ret = devm_request_irq(dev, mtk->gpio_irq,
-				       mediatek_gpio_irq_handler, IRQF_SHARED,
-				       rg->chip.gc.label, &rg->chip.gc);
-
-		if (ret) {
-			dev_err(dev, "Error requesting IRQ %d: %d\n",
-				mtk->gpio_irq, ret);
-			return ret;
-		}
-
-		girq = &rg->chip.gc.irq;
-		gpio_irq_chip_set_chip(girq, &mt7621_irq_chip);
-		/* This will let us handle the parent IRQ in the driver */
-		girq->parent_handler = NULL;
-		girq->num_parents = 0;
-		girq->parents = NULL;
-		girq->default_type = IRQ_TYPE_NONE;
-		girq->handler = handle_simple_irq;
-	}
-
-	ret = devm_gpiochip_add_data(dev, &rg->chip.gc, mtk);
+	ret = devm_gpiochip_add_data(dev, &rg->chip.gc, rg);
 	if (ret < 0) {
 		dev_err(dev, "Could not register gpio %d, ret=%d\n",
 			rg->chip.gc.ngpio, ret);
@@ -322,7 +456,8 @@ mediatek_gpio_probe(struct platform_device *pdev)
 	if (mtk->gpio_irq < 0)
 		return mtk->gpio_irq;
 
-	mtk->dev = dev;
+	mtk->pdev = pdev;
+	mtk->num_gpios = MTK_BANK_WIDTH * MTK_BANK_CNT;
 	platform_set_drvdata(pdev, mtk);
 
 	for (i = 0; i < MTK_BANK_CNT; i++) {
@@ -331,7 +466,17 @@ mediatek_gpio_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (mtk->gpio_irq > 0) {
+		ret = mt7621_gpio_irq_setup(pdev, mtk);
+		if (ret)
+			goto fail;
+	}
+
 	return 0;
+
+fail:
+	mt7621_gpio_remove(pdev);
+	return ret;
 }
 
 static const struct of_device_id mediatek_gpio_match[] = {
-- 
2.43.0