[PATCH RFC 0/4] gpio: loongson-64bit: add interrupt support for more models

[PATCH RFC 0/4] gpio: loongson-64bit: add interrupt support for more models

[Miao Wang via B4 Relay]

This patch series aims to correctly add interrupt support for GPIO
controllers found on more Loongson CPUs. The GPIO controllers in such
models do not have a complete intgrated interrupt controller, but choose
to delegate interrupt handling to their parent interrupt controller.
This sounds like a heirarchical interrupt setup, but what makes it
more complex is that multiple GPIO lines in these controllers may share
one interrupt line on the parent interrupt controller. The details is
described in the third patch.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
Miao Wang (4):
      gpio: loongson-64bit: Support for irq controller on 2k2000 and 7a2000
      gpio: loongson-64bit: Generalize interrupt controller initialization
      gpio: loongson-64bit: Add shared interrupt support
      gpio: loongson-64bit: remove gpio_to_irq() handler

 drivers/gpio/gpio-loongson-64bit.c | 708 +++++++++++++++++++++++++++++++++++--
 1 file changed, 683 insertions(+), 25 deletions(-)
---
base-commit: 481ed5dd3ed7136f627b8ec372ba39f5b2e7d27f
change-id: 20260925-loongsongpio-829824a3d562

Best regards,
-- 
Miao Wang <shankerwangmiao@gmail.com>

[PATCH RFC 1/4] gpio: loongson-64bit: Support for irq controller on 2k2000 and 7a2000

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

According to the User Manual, the GPIO controllers on 2K2000 and 7A2000
include irq controllers, which are similar to those on 2K0300, except
for the support for edge-triggered interrupts on both edges.

This patch makes the requirement for intdual register optional and
links the irq controller to the GPIO controller for 2K2000 and 7A2000.

To make the change compatiable with possible existing firmware code for
2K2000 and 7A2000, the initialization of the irq controller will not
fail when not given enough firmware IRQs. Instead, it will only
initialize the irq controller with the available firmware IRQs.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/gpio/gpio-loongson-64bit.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 0fdf15faa344d2db0a1cf52dc52c3f58aabef49c..64eb9f46ce31cc289f236af44311ff39f05de3b0 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -183,6 +183,8 @@ static int loongson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 	u8 pol = 0, edge = 0, dual = 0;
 
 	if ((type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
+		if (!lgpio->chip_data->intdual_offset)
+			return -EINVAL;
 		edge = 1;
 		dual = 1;
 		irq_set_handler_locked(data, handle_edge_irq);
@@ -210,7 +212,8 @@ static int loongson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 
 	writeb(pol, lgpio->reg_base + lgpio->chip_data->intpol_offset + hwirq);
 	writeb(edge, lgpio->reg_base + lgpio->chip_data->intedge_offset + hwirq);
-	writeb(dual, lgpio->reg_base + lgpio->chip_data->intdual_offset + hwirq);
+	if (lgpio->chip_data->intdual_offset)
+		writeb(dual, lgpio->reg_base + lgpio->chip_data->intdual_offset + hwirq);
 
 	return 0;
 }
@@ -260,7 +263,7 @@ static int loongson_gpio_init_irqchip(struct platform_device *pdev,
 	chip->irq.parent_handler_data = lgpio;
 	gpio_irq_chip_set_chip(&chip->irq, data->girqchip);
 
-	chip->irq.num_parents = data->intr_num;
+	chip->irq.num_parents = 0;
 	chip->irq.parents = devm_kcalloc(&pdev->dev, data->intr_num,
 					 sizeof(*chip->irq.parents), GFP_KERNEL);
 	if (!chip->irq.parents)
@@ -269,11 +272,13 @@ static int loongson_gpio_init_irqchip(struct platform_device *pdev,
 	for (i = 0; i < data->intr_num; i++) {
 		int ret;
 
-		ret = platform_get_irq(pdev, i);
+		ret = platform_get_irq_optional(pdev, i);
+		if (ret == -ENXIO)
+			continue;
 		if (ret < 0)
 			return dev_err_probe(&pdev->dev, ret,
 					     "failed to get IRQ %d\n", i);
-		chip->irq.parents[i] = ret;
+		chip->irq.parents[chip->irq.num_parents++] = ret;
 	}
 
 	for (i = 0; i < data->intr_num; i++) {
@@ -281,6 +286,11 @@ static int loongson_gpio_init_irqchip(struct platform_device *pdev,
 		writeb(0x1, lgpio->reg_base + data->intclr_offset + i);
 	}
 
+	if (chip->irq.num_parents == 0) {
+		dev_warn(&pdev->dev, "no parent IRQs have been found, GPIO IRQs will not be available\n");
+		gpio_irq_chip_set_chip(&chip->irq, NULL);
+	}
+
 	return 0;
 }
 
@@ -413,6 +423,13 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data1 = {
 	.in_offset = 0xa00,
 	.out_offset = 0x900,
 	.inten_offset = 0xb00,
+	.intpol_offset = 0xc00,
+	.intedge_offset = 0xd00,
+	.intclr_offset = 0xe00,
+	.intsts_offset = 0xf00,
+	.intr_num = 8,
+	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
+	.girqchip = &loongson_gpio_ls2k0300_irqchip,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data2 = {
@@ -449,6 +466,13 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls7a2000_data0 = {
 	.in_offset = 0xa00,
 	.out_offset = 0x900,
 	.inten_offset = 0xb00,
+	.intpol_offset = 0xc00,
+	.intedge_offset = 0xd00,
+	.intclr_offset = 0xe00,
+	.intsts_offset = 0xf00,
+	.intr_num = 5,
+	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
+	.girqchip = &loongson_gpio_ls2k0300_irqchip,
 };
 
 /* LS7A2000 ACPI GPIO */

-- 
2.55.0

[PATCH RFC 2/4] gpio: loongson-64bit: Generalize interrupt controller initialization

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

This patch turns the interrupt controller initialization function into
a handler in struct loongson_gpio_chip_data, which will be called by
loongson_gpio_init(), so that we can add support for different types
of interrupt controllers.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/gpio/gpio-loongson-64bit.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 64eb9f46ce31cc289f236af44311ff39f05de3b0..a1b23ba21f64162af2096131520d1dac27798983 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -39,6 +39,8 @@ struct loongson_gpio_chip_data {
 	unsigned int		intr_num;
 	irq_flow_handler_t	irq_handler;
 	const struct irq_chip	*girqchip;
+	int			(*irq_init)(struct platform_device *pdev,
+					    struct loongson_gpio_chip *lgpio);
 };
 
 struct loongson_gpio_chip {
@@ -286,11 +288,6 @@ static int loongson_gpio_init_irqchip(struct platform_device *pdev,
 		writeb(0x1, lgpio->reg_base + data->intclr_offset + i);
 	}
 
-	if (chip->irq.num_parents == 0) {
-		dev_warn(&pdev->dev, "no parent IRQs have been found, GPIO IRQs will not be available\n");
-		gpio_irq_chip_set_chip(&chip->irq, NULL);
-	}
-
 	return 0;
 }
 
@@ -329,9 +326,13 @@ static int loongson_gpio_init(struct platform_device *pdev, struct loongson_gpio
 	lgpio->chip.gc.label = lgpio->chip_data->label;
 	lgpio->chip.gc.can_sleep = false;
 	if (lgpio->chip_data->girqchip) {
-		ret = loongson_gpio_init_irqchip(pdev, lgpio);
+		ret = lgpio->chip_data->irq_init(pdev, lgpio);
 		if (ret)
 			return dev_err_probe(&pdev->dev, ret, "failed to initialize irqchip\n");
+		if (lgpio->chip.gc.irq.num_parents == 0) {
+			dev_warn(&pdev->dev, "no parent IRQs ha been found, GPIO IRQs will not be available\n");
+			gpio_irq_chip_set_chip(&lgpio->chip.gc.irq, NULL);
+		}
 	} else if (lgpio->chip_data->inten_offset) {
 		lgpio->chip.gc.to_irq = loongson_gpio_to_irq;
 	}
@@ -387,6 +388,7 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k0300_data = {
 	.intr_num = 7,
 	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
 	.girqchip = &loongson_gpio_ls2k0300_irqchip,
+	.irq_init = loongson_gpio_init_irqchip,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data0 = {
@@ -430,6 +432,7 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data1 = {
 	.intr_num = 8,
 	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
 	.girqchip = &loongson_gpio_ls2k0300_irqchip,
+	.irq_init = loongson_gpio_init_irqchip,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data2 = {
@@ -473,6 +476,7 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls7a2000_data0 = {
 	.intr_num = 5,
 	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
 	.girqchip = &loongson_gpio_ls2k0300_irqchip,
+	.irq_init = loongson_gpio_init_irqchip,
 };
 
 /* LS7A2000 ACPI GPIO */

-- 
2.55.0

[PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

The GPIO controllers found in 2K1000LA, 2K0500, 2K2000 (the first part),
3A5000, 7A1000, 3A6000 lack a dedicated interrupt controller inside it,
instead, they route the input signals via an interrupt enable gate,
controlled by the inten register, to their parent interrupt controller,
letting the latter do the actual interrupt handling. To make things
worse, interrupt signals from multiple GPIO lines are combined into
one interrupt line on the parent interrupt controller, preventing
implementing such controllers as hierarchical interrupt chips. As a
result, the GPIO lines sharing the same parent interrupt line can fall
into the following conditions:

  - Only one of the GPIO lines is in edge-triggered mode, while IRQ on
    other lines cannot be requested.
  - Only one of the GPIO lines is in level-triggered mode, while IRQ on
    other lines cannot be requested, if the register controlling the
    polarity, intpol, is not provided on the GPIO controller, in which
    case, the polarity is controlled by the parent interrupt controller.
  - One or more of the GPIO lines is in level-triggered mode, if intpol
    is provided on the GPIO controller.

Originally gpio_generic_chip.to_irq method was implemented to return the
irq number of the parent interrupt line corresponding to a given GPIO
line and the method unconditionally enable the interrupt on the GPIO
line by writing 1 to the inten register. This approach, however, has
three limitations. Firstly, there is no guarantee that gpio_to_irq will
be called first, so there would be no chance to enable the interrupt on
the GPIO line. Secondly, the sharing of the parent interrupt line among
multiple GPIO lines is not correctly handled, failing to deny requests
for conflicting requests on other GPIO lines. Thirdly, when the irq
requested is released, the interrupt on the GPIO line will never be
disabled. This was discussed in the v5 version of the patch series
introducing this driver [1].

To solve this, this patch implements a cascaded interrupt controller,
which manages the sharing of the parent interrupt line among multiple
GPIO lines, denying conflicting requests and properly enabling or
disabling the interrupt on the GPIO line as needed. To implement this,
mask/unmask/set_type/shutdown handlers should call the handlers of the
parent interrupt controller to manage and change the state of the
interrupts on the parent interrupt line as well.

What remain undecided is the firmware interface that declares platform
irq numbers for the GPIO controller. If the GPIO controller has N gpio
lines and M parent interrupt lines, it is to be decided how to list the
platform irq numbers. The first approach could be to list M unique
irq numbers corresponding to the M parent interrupt lines, while the
second approach could be to list N irq numbers corresponding to the N
gpio lines, which might have duplicate entries. Previously the second
approach was chosen and it was found that listing duplicate interrupt
numbers is not restricted by ACPI and platform devices. However,
actual ACPI tables found in the published firmware chose the first
approach. I choose to use the first approach as well temporarily in this
patch, since it would be easier to implement. Thankfully when the driver
is implemented in the first approach, it can accept the data from the
second approach as well, due to the mapping used, except for 2K0500 and
2K1000. I have no sample firmwares for these two platforms and cannot
verify which approach is used.

[1]: https://lore.kernel.org/all/CACRpkdbgP9m40t_Ky4H+SQi9TELikomT2M-JpF7+auKmzOxQdg@mail.gmail.com/
[2]: https://lore.kernel.org/all/allOoYUZXocOfk8F@ashevche-desk.local/

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/gpio/gpio-loongson-64bit.c | 650 +++++++++++++++++++++++++++++++++++++
 1 file changed, 650 insertions(+)

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index a1b23ba21f64162af2096131520d1dac27798983..6e5687dcb413c247fb0d76c596c445f94efccd01 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -7,6 +7,7 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/cleanup.h>
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
 #include <linux/module.h>
@@ -17,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/bitops.h>
 #include <linux/reset.h>
+#include <linux/interrupt.h>
 #include <asm/types.h>
 
 enum loongson_gpio_mode {
@@ -24,6 +26,8 @@ enum loongson_gpio_mode {
 	BYTE_CTRL_MODE,
 };
 
+struct loongson_gpio_chip;
+
 struct loongson_gpio_chip_data {
 	const char		*label;
 	enum loongson_gpio_mode	mode;
@@ -39,14 +43,19 @@ struct loongson_gpio_chip_data {
 	unsigned int		intr_num;
 	irq_flow_handler_t	irq_handler;
 	const struct irq_chip	*girqchip;
+	unsigned int		(*irq_mapping)(struct loongson_gpio_chip *lgpio,
+					       unsigned int pin);
 	int			(*irq_init)(struct platform_device *pdev,
 					    struct loongson_gpio_chip *lgpio);
 };
 
+struct loongson_gpio_shared_irq_data;
+
 struct loongson_gpio_chip {
 	struct gpio_generic_chip chip;
 	spinlock_t		lock;
 	void __iomem		*reg_base;
+	struct loongson_gpio_shared_irq_data *shared_irq_data;
 	const struct loongson_gpio_chip_data *chip_data;
 };
 
@@ -291,6 +300,609 @@ static int loongson_gpio_init_irqchip(struct platform_device *pdev,
 	return 0;
 }
 
+/*
+ * About shared IRQs for GPIO pins:
+ *
+ * This kind of GPIO controller lacks interrupt status register to identify
+ * edge-triggered interrupts, and the handling of such interrupts must be
+ * deligated to the parent IRQ controller.
+ * This kind of GPIO controller should typically have a inten register to enable
+ * or disable interrupts for individual GPIO pins, intpol register to configure
+ * the interrupt polarity. The signal sent to the parent interrupt controller
+ * is calculated by bit-wise comparing the GPIO line and the intpol register and
+ * then masking with inten.
+ * However, The interrput input lines of the parent IRQ controller is not enough
+ * to 1:1 map to the GPIO pins, necessitating the use of shared IRQs. A mapping
+ * is statically establised to map multiple GPIO pins to a single parent IRQ
+ * line. The level of the parent IRQ is determined by or-ing the levels of all
+ * GPIO pins mapped to it. Suppose gpio pin 0 and gpio pin 8 are mapped to the
+ * same parent IRQ, the level of the parent IRQ is determined by the following
+ * expression:
+ *     (~(gpio[0] ^ intpol[0]) & inten[0]) |
+ *     (~(gpio[8] ^ intpol[8]) & inten[8])
+ * When the intpol register is not available, its value is assumed to be 1, and
+ * the expression becomes:
+ *     (gpio[0] & inten[0]) | (gpio[8] & inten[8])
+ *
+ * According to the hardware design, IRQ can be requested on only one of the
+ * GPIO pins sharing the same parent IRQ, unless the intpol register is
+ * available and all the IRQs are level-triggered.
+ *
+ * Convention on the usage of shared IRQs for GPIO pins:
+ * - When a parent IRQ is exclusively used, the parent IRQ should be masked or
+ *   unmasked accordingly when the irq pin using it is masked or unmasked.
+ * - When a parent IRQ is shared among multiple GPIO pins, the parent IRQ
+ *   should always be unmasked and masking or unmasking individual GPIO pins
+ *   should be done through the GPIO controller's inten register.
+ */
+
+/*
+ * We assume a maximum of 128 GPIOs for the bitmap, which should be
+ * sufficient for all the actual hardware controllers defined in this
+ * driver.
+ */
+enum { MAX_GPIO_WITH_IRQ = 128 };
+
+struct loongson_gpio_shared_per_parent_irq_data {
+	unsigned int parent_index;
+	unsigned int parent_irq;
+	/*
+	 * The lock of the irq_desc of the parent irq also protects
+	 * exclusive/usedby below. It must be taken before
+	 * lgpio->shared_irq_data->lock.
+	 */
+	struct irq_data *parent_irq_data;
+	struct loongson_gpio_chip *lgpio;
+
+	/*
+	 * exclusive: exclusive usage, only one gpio pin can use this parent irq.
+	 *   It will happen in two cases:
+	 *     - The parent irq is used by a gpio pin which is edge triggered, and
+	 *       thus cannot be used by other gpio pins which are mapped to the same
+	 *       parent irq.
+	 *     - The parent irq is used by a gpio pin which is level triggered, and
+	 *       the gpio chip lacks the polarity control register, and thus the
+	 *       interrupt polarity is controlled by the parent irq controller, and
+	 *       thus cannot be used by other gpio pins which are mapped to the
+	 *       same parent irq.
+	 *   When set, the request for other gpio pins which are mapped to this
+	 *   parent irq will be rejected.
+	 */
+	unsigned int exclusive : 1;
+	/*
+	 * usedby: When exclusive is set, these bits indicate the gpio pin which
+	 *   is using this parent irq. When exclusive is cleared, these bits
+	 *   indicate the number of gpio pins which are using this parent irq.
+	 * When both exclusive and usedby are 0, it means that the parent irq is not
+	 * used by any gpio pin.
+	 */
+	unsigned int usedby : 31;
+};
+
+struct loongson_gpio_shared_irq_data {
+	/*
+	 * Protects the read/write operations on the irq related registers
+	 */
+	raw_spinlock_t lock;
+	int *irq_in_use;
+	struct loongson_gpio_shared_per_parent_irq_data per_parent_irq_data[];
+};
+
+static void loongson_gpio_write_register(struct loongson_gpio_chip *lgpio,
+					 unsigned int reg_offset,
+					 unsigned int line, u8 value)
+{
+	void __iomem *reg = lgpio->reg_base + reg_offset;
+
+	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
+		u32 bval;
+
+		reg += (line / 32) * 4;
+		bval = readl(reg);
+
+		if (value)
+			bval |= BIT(line % 32);
+		else
+			bval &= ~BIT(line % 32);
+
+		writel(bval, reg);
+	} else {
+		writeb(value, reg + line);
+	}
+}
+
+static u8 loongson_gpio_read_register(struct loongson_gpio_chip *lgpio,
+				      unsigned int reg_offset,
+				      unsigned int line)
+{
+	void __iomem *reg = lgpio->reg_base + reg_offset;
+
+	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
+		u32 bval;
+
+		reg += (line / 32) * 4;
+		bval = readl(reg);
+
+		return (bval >> (line % 32)) & 1;
+	}
+
+	return readb(reg + line) & 1;
+}
+
+static void loongson_gpio_shared_mask_locked(struct loongson_gpio_chip *lgpio,
+					  irq_hw_number_t hwirq,
+					  struct loongson_gpio_shared_per_parent_irq_data *ppid)
+{
+	struct irq_data *parent_irq_data = ppid->parent_irq_data;
+
+	if (ppid->exclusive)
+		parent_irq_data->chip->irq_mask(parent_irq_data);
+
+	scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+		loongson_gpio_write_register(lgpio, lgpio->chip_data->inten_offset,
+					     hwirq, 0);
+}
+
+static void loongson_gpio_shared_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+	unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
+	struct loongson_gpio_shared_per_parent_irq_data *ppid =
+		&lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
+
+	/*
+	 * If irq_in_use is not set, then set_type handler must has not been called
+	 * and no trigger type has been set for this gpio pin, so we should not
+	 * proceed and do nothing.
+	 */
+	if (!lgpio->shared_irq_data->irq_in_use[hwirq])
+		return;
+
+	guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
+
+	loongson_gpio_shared_mask_locked(lgpio, hwirq, ppid);
+}
+
+static void loongson_gpio_shared_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+	unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
+	struct loongson_gpio_shared_per_parent_irq_data *ppid =
+		&lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
+	struct irq_data *parent_irq_data = ppid->parent_irq_data;
+
+	if (!lgpio->shared_irq_data->irq_in_use[hwirq])
+		return;
+
+	guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
+
+	if (ppid->exclusive)
+		parent_irq_data->chip->irq_mask(parent_irq_data);
+
+	scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+		loongson_gpio_write_register(lgpio, lgpio->chip_data->inten_offset,
+					     hwirq, 1);
+
+	if (ppid->exclusive)
+		parent_irq_data->chip->irq_unmask(parent_irq_data);
+}
+
+static void loongson_gpio_shared_irq_handler(struct irq_desc *desc);
+
+static int loongson_gpio_shared_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+	unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
+	struct loongson_gpio_shared_per_parent_irq_data *ppid =
+		&lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
+	unsigned int intpol_offset = lgpio->chip_data->intpol_offset;
+	struct irq_data *parent_irq_data = ppid->parent_irq_data;
+	struct irq_chip *parent_irq_chip = parent_irq_data->chip;
+	int ret = 0;
+
+	type &= IRQ_TYPE_SENSE_MASK;
+
+	if (type != IRQ_TYPE_EDGE_FALLING && type != IRQ_TYPE_EDGE_RISING &&
+	    type != IRQ_TYPE_LEVEL_LOW && type != IRQ_TYPE_LEVEL_HIGH)
+		return -EINVAL;
+
+	guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
+
+	if (ppid->exclusive && hwirq != ppid->usedby)
+		return -EBUSY;
+
+	/*
+	 * When the irq is not exclusively used and is currently shared by
+	 * more than one gpio pin; or is currently used by single gpio pin and
+	 * the gpio pin is not the one requesting to set the irq type, then the
+	 * irq type can only be set to level triggered.
+	 */
+	if (!ppid->exclusive &&
+	    (ppid->usedby > 1 ||
+	     (ppid->usedby == 1 && !lgpio->shared_irq_data->irq_in_use[hwirq]))) {
+		if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_LEVEL_LOW)
+			return -EBUSY;
+
+		/*
+		 * When exclusive is not set and usedby at least one, the gpio chip must
+		 * have intpol register to control the interrupt polarity. We will never
+		 * reach here if the gpio chip lacks intpol register, because in that
+		 * case, exclusive will be set no matter what trigger type is requested.
+		 */
+		BUG_ON(!intpol_offset);
+		/*
+		 * Since only level triggered irqs are allowed to be shared, we can
+		 * safely set the intpol register without first disabling the pin in
+		 * inten register.
+		 */
+		scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+			loongson_gpio_write_register(lgpio, intpol_offset, hwirq,
+						     type == IRQ_TYPE_LEVEL_HIGH);
+
+		if (!lgpio->shared_irq_data->irq_in_use[hwirq])
+			ppid->usedby++;
+
+		irq_set_handler_locked(data, handle_level_irq);
+	} else {
+		parent_irq_chip->irq_mask(parent_irq_data);
+
+		if (type == IRQ_TYPE_EDGE_FALLING || type == IRQ_TYPE_EDGE_RISING) {
+			if (intpol_offset) {
+				ret = parent_irq_chip->irq_set_type(parent_irq_data,
+								    IRQ_TYPE_EDGE_RISING);
+				if (ret < 0)
+					goto out_restore_handler;
+
+				scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+					loongson_gpio_write_register(lgpio, intpol_offset,
+								     hwirq,
+								     type == IRQ_TYPE_EDGE_RISING);
+			} else {
+				ret = parent_irq_chip->irq_set_type(parent_irq_data, type);
+				if (ret < 0)
+					goto out_restore_handler;
+			}
+
+			ppid->exclusive = 1;
+			ppid->usedby = hwirq;
+
+			/*
+			 * We deliberately set the handler to level_irq even for
+			 * edge-triggered interrupts because we cannot provide .irq_ack()
+			 * handler.
+			 */
+			irq_set_handler_locked(data, handle_level_irq);
+		} else {
+			if (intpol_offset) {
+				ret = parent_irq_chip->irq_set_type(parent_irq_data,
+								    IRQ_TYPE_LEVEL_HIGH);
+				if (ret < 0)
+					goto out_restore_handler;
+
+				scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+					loongson_gpio_write_register(lgpio, intpol_offset,
+								     hwirq,
+								     type == IRQ_TYPE_LEVEL_HIGH);
+				ppid->exclusive = 0;
+				ppid->usedby = 1;
+			} else {
+				ret = parent_irq_chip->irq_set_type(parent_irq_data, type);
+				if (ret < 0)
+					goto out_restore_handler;
+
+				ppid->exclusive = 1;
+				ppid->usedby = hwirq;
+			}
+
+			irq_set_handler_locked(data, handle_level_irq);
+		}
+out_restore_handler:
+		/* ->irq_set_type() would override irq_handler, so restore here */
+		irq_set_handler_locked(parent_irq_data, loongson_gpio_shared_irq_handler);
+		if (ret < 0)
+			goto out;
+	}
+
+	lgpio->shared_irq_data->irq_in_use[hwirq] = 1;
+
+out:
+	if (ppid->exclusive) {
+		u8 inten;
+
+		scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
+			inten = loongson_gpio_read_register(lgpio, lgpio->chip_data->inten_offset,
+							    hwirq);
+		if (inten)
+			parent_irq_chip->irq_unmask(parent_irq_data);
+	} else
+		parent_irq_chip->irq_unmask(parent_irq_data);
+
+	return ret;
+}
+
+static void loongson_gpio_shared_irq_shutdown(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+	unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
+	struct loongson_gpio_shared_per_parent_irq_data *ppid =
+		&lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
+
+	if (!lgpio->shared_irq_data->irq_in_use[hwirq])
+		return;
+
+	guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
+
+	loongson_gpio_shared_mask_locked(lgpio, hwirq, ppid);
+
+	if (!ppid->exclusive) {
+		if (lgpio->shared_irq_data->irq_in_use[hwirq]) {
+			if (!WARN_ON(ppid->usedby == 0))
+				ppid->usedby--;
+		}
+	} else {
+		if (ppid->usedby == hwirq) {
+			ppid->usedby = 0;
+			ppid->exclusive = 0;
+		}
+	}
+
+	lgpio->shared_irq_data->irq_in_use[hwirq] = 0;
+}
+
+static void loongson_gpio_shared_irq_init_valid_mask(struct gpio_chip *gc,
+						     unsigned long *valid_mask,
+						     unsigned int ngpios)
+{
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(gc);
+	struct loongson_gpio_shared_irq_data *shared_irq_data = lgpio->shared_irq_data;
+	const struct loongson_gpio_chip_data *data = lgpio->chip_data;
+	unsigned int i;
+
+	for (i = 0; i < ngpios; i++) {
+		unsigned int parent_irq_index = data->irq_mapping(lgpio, i);
+
+		/*
+		 * A parent_irq of 0 means the corresponding interrupt line is
+		 * absent from the device tree or ACPI namespace.
+		 */
+		if (i >= MAX_GPIO_WITH_IRQ ||
+		    parent_irq_index >= data->intr_num ||
+		    !shared_irq_data->per_parent_irq_data[parent_irq_index].parent_irq)
+			clear_bit(i, valid_mask);
+	}
+}
+
+static int loongson_gpio_shared_irq_init_hw(struct gpio_chip *gc)
+{
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(gc);
+	const struct loongson_gpio_chip_data *data = lgpio->chip_data;
+	struct loongson_gpio_shared_irq_data *shared_irq_data = lgpio->shared_irq_data;
+	int i;
+
+	shared_irq_data->irq_in_use = devm_kcalloc(gc->parent, gc->ngpio,
+						   sizeof(*shared_irq_data->irq_in_use),
+						   GFP_KERNEL);
+
+	if (!shared_irq_data->irq_in_use)
+		return -ENOMEM;
+
+	for (i = 0; i < gc->ngpio; i++)
+		loongson_gpio_write_register(lgpio, data->inten_offset, i, 0);
+
+	return 0;
+}
+
+static int loongson_gpio_init_shared_irqchip(struct platform_device *pdev,
+					     struct loongson_gpio_chip *lgpio)
+{
+	const struct loongson_gpio_chip_data *data = lgpio->chip_data;
+	struct gpio_chip *chip = &lgpio->chip.gc;
+	struct loongson_gpio_shared_irq_data *shared_irq_data;
+	int i;
+	int available_parent_irqs;
+
+	chip->irq.default_type = IRQ_TYPE_NONE;
+	chip->irq.handler = handle_bad_irq;
+	chip->irq.parent_handler = data->irq_handler;
+	chip->irq.init_valid_mask = loongson_gpio_shared_irq_init_valid_mask;
+	chip->irq.init_hw = loongson_gpio_shared_irq_init_hw;
+
+	shared_irq_data = devm_kzalloc(&pdev->dev,
+				       struct_size(shared_irq_data,
+						   per_parent_irq_data,
+						   data->intr_num),
+				       GFP_KERNEL);
+	if (!shared_irq_data)
+		return -ENOMEM;
+
+	lgpio->shared_irq_data = shared_irq_data;
+	raw_spin_lock_init(&shared_irq_data->lock);
+	for (i = 0; i < data->intr_num; i++) {
+		shared_irq_data->per_parent_irq_data[i].parent_index = i;
+		shared_irq_data->per_parent_irq_data[i].lgpio = lgpio;
+	}
+
+	gpio_irq_chip_set_chip(&chip->irq, data->girqchip);
+
+	chip->irq.parents = devm_kcalloc(&pdev->dev, data->intr_num,
+					 sizeof(*chip->irq.parents), GFP_KERNEL);
+	if (!chip->irq.parents)
+		return -ENOMEM;
+
+	chip->irq.per_parent_data = true;
+	chip->irq.parent_handler_data_array =
+		devm_kcalloc(&pdev->dev, data->intr_num,
+			     sizeof(*chip->irq.parent_handler_data_array),
+			     GFP_KERNEL);
+	if (!chip->irq.parent_handler_data_array)
+		return -ENOMEM;
+
+	for (i = 0, available_parent_irqs = 0; i < data->intr_num; i++) {
+		int ret;
+		struct irq_data *parent_irq_data;
+		int dup = 0, j;
+
+		ret = platform_get_irq_optional(pdev, i);
+
+		if (ret == -ENXIO)
+			continue;
+
+		if (ret < 0)
+			return dev_err_probe(&pdev->dev, ret,
+					     "failed to get IRQ %d\n", i);
+
+		for (j = 0; j < available_parent_irqs; j++)
+			if (chip->irq.parents[j] == ret) {
+				dup = 1;
+				break;
+			}
+		if (dup) {
+			dev_warn(&pdev->dev,
+				 FW_BUG "IRQ for parent index %d is duplicated, skipping\n", i);
+			continue;
+		}
+
+		parent_irq_data = irq_get_irq_data(ret);
+		if (WARN_ON(!parent_irq_data))
+			continue;
+		/* Implementing ->irq_set_type() is required for a parent irqchip */
+		if (parent_irq_data->chip->irq_set_type == NULL)
+			continue;
+
+		chip->irq.parents[available_parent_irqs] = ret;
+		chip->irq.parent_handler_data_array[available_parent_irqs] =
+			&shared_irq_data->per_parent_irq_data[i];
+
+		shared_irq_data->per_parent_irq_data[i].parent_irq = ret;
+		shared_irq_data->per_parent_irq_data[i].parent_irq_data =
+			irq_get_irq_data(ret);
+
+		available_parent_irqs++;
+	}
+	chip->irq.num_parents = available_parent_irqs;
+
+	return 0;
+}
+
+/*
+ * Determine the GPIO pins which are triggering the parent irq given by
+ * ppid, by comparing each mapped pin's line level against its polarity.
+ */
+static void loongson_gpio_shared_irq_poll(struct loongson_gpio_chip *lgpio,
+					  unsigned int parent_index,
+					  unsigned long *irq_pending_map,
+					  unsigned int ngpio)
+{
+	const struct loongson_gpio_chip_data *data = lgpio->chip_data;
+	unsigned int i;
+
+	for (i = 0; i < ngpio; i++) {
+		u8 inten, intpol, level;
+
+		if (data->irq_mapping(lgpio, i) != parent_index)
+			continue;
+
+		scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock) {
+			inten = !!loongson_gpio_read_register(lgpio, data->inten_offset, i);
+			intpol = !!loongson_gpio_read_register(lgpio, data->intpol_offset, i);
+			level = !!loongson_gpio_read_register(lgpio, data->in_offset, i);
+		}
+
+		if (inten && intpol == level)
+			bitmap_set(irq_pending_map, i, 1);
+	}
+}
+
+static void loongson_gpio_shared_irq_handler(struct irq_desc *desc)
+{
+	struct loongson_gpio_shared_per_parent_irq_data *ppid =
+		irq_desc_get_handler_data(desc);
+	struct loongson_gpio_chip *lgpio = ppid->lgpio;
+	struct gpio_chip *chip = &lgpio->chip.gc;
+	struct irq_chip *girqchip = irq_desc_get_chip(desc);
+	unsigned int ngpio = chip->ngpio > MAX_GPIO_WITH_IRQ ? MAX_GPIO_WITH_IRQ : chip->ngpio;
+	unsigned int exclusive, usedby, i;
+	DECLARE_BITMAP(irq_pending_map, MAX_GPIO_WITH_IRQ);
+
+	bitmap_clear(irq_pending_map, 0, ngpio);
+
+	chained_irq_enter(girqchip, desc);
+
+	scoped_guard(raw_spinlock_irqsave, &desc->lock) {
+		exclusive = ppid->exclusive;
+		usedby = ppid->usedby;
+	}
+	if (exclusive && usedby < ngpio)
+		bitmap_set(irq_pending_map, usedby, 1);
+	else if (usedby) {
+		/*
+		 * When a parent irq is shared by more than 1 gpio pin,
+		 * these IRQs must be level-triggered, and the polarity
+		 * register must be available in the GPIO controller to
+		 * determine the triggering GPIO line.
+		 */
+		loongson_gpio_shared_irq_poll(lgpio,
+					      ppid->parent_index,
+					      irq_pending_map,
+					      ngpio);
+	}
+
+	if (!bitmap_empty(irq_pending_map, ngpio)) {
+		for (i = 0; i < ngpio; i++) {
+			if (test_bit(i, irq_pending_map))
+				generic_handle_domain_irq(chip->irq.domain, i);
+		}
+	}
+
+	chained_irq_exit(girqchip, desc);
+}
+
+static const struct irq_chip loongson_gpio_shared_irqchip = {
+	.irq_mask	= loongson_gpio_shared_irq_mask,
+	.irq_unmask	= loongson_gpio_shared_irq_unmask,
+	.irq_set_type	= loongson_gpio_shared_irq_set_type,
+	.irq_shutdown	= loongson_gpio_shared_irq_shutdown,
+	.flags		= IRQCHIP_IMMUTABLE | IRQCHIP_SKIP_SET_WAKE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static unsigned int loongson_gpio_irq_mapping_mod8(struct loongson_gpio_chip *lgpio,
+						   unsigned int pin)
+{
+	return pin % 8;
+}
+
+static unsigned int loongson_gpio_irq_mapping_cap4(struct loongson_gpio_chip *lgpio,
+						   unsigned int pin)
+{
+	return pin >= 4 ? 4 : pin;
+}
+
+static unsigned int loongson_gpio_irq_mapping_cap4_2(struct loongson_gpio_chip *lgpio,
+						    unsigned int pin)
+{
+	if (pin < 4)
+		return pin;
+	else if (pin < 32)
+		return 4;
+	else
+		return 5;
+}
+
+static unsigned int loongson_gpio_irq_mapping_div32(struct loongson_gpio_chip *lgpio,
+						    unsigned int pin)
+{
+	return pin / 32;
+}
+
 static int loongson_gpio_init(struct platform_device *pdev, struct loongson_gpio_chip *lgpio,
 			      void __iomem *reg_base)
 {
@@ -371,6 +983,11 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
 	.in_offset = 0x20,
 	.out_offset = 0x10,
 	.inten_offset = 0x30,
+	.intr_num = 6,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_cap4_2,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k0300_data = {
@@ -398,6 +1015,11 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data0 = {
 	.in_offset = 0x8,
 	.out_offset = 0x10,
 	.inten_offset = 0xb0,
+	.intr_num = 2,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_div32,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data1 = {
@@ -407,6 +1029,11 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data1 = {
 	.in_offset = 0x8,
 	.out_offset = 0x10,
 	.inten_offset = 0x98,
+	.intr_num = 2,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_div32,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data0 = {
@@ -416,6 +1043,12 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data0 = {
 	.in_offset = 0xc,
 	.out_offset = 0x8,
 	.inten_offset = 0x14,
+	.intpol_offset = 0x10,
+	.intr_num = 8,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_mod8,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data1 = {
@@ -450,6 +1083,12 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls3a5000_data = {
 	.in_offset = 0xc,
 	.out_offset = 0x8,
 	.inten_offset = 0x14,
+	.intpol_offset = 0x10,
+	.intr_num = 8,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_mod8,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls7a_data = {
@@ -459,6 +1098,11 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls7a_data = {
 	.in_offset = 0xa00,
 	.out_offset = 0x900,
 	.inten_offset = 0xb00,
+	.intr_num = 5,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_cap4,
 };
 
 /* LS7A2000 chipset GPIO */
@@ -496,6 +1140,12 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls3a6000_data = {
 	.in_offset = 0xc,
 	.out_offset = 0x8,
 	.inten_offset = 0x14,
+	.intpol_offset = 0x10,
+	.intr_num = 8,
+	.irq_init = loongson_gpio_init_shared_irqchip,
+	.irq_handler = loongson_gpio_shared_irq_handler,
+	.girqchip = &loongson_gpio_shared_irqchip,
+	.irq_mapping = loongson_gpio_irq_mapping_mod8,
 };
 
 static const struct of_device_id loongson_gpio_of_match[] = {

-- 
2.55.0

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Linus Walleij]

Hi Miao,

thanks for your patch!

On Fri, Sep 25, 2026 at 6:28 PM Miao Wang via B4 Relay
<devnull+shankerwangmiao.gmail.com@kernel.org> wrote:

> +struct loongson_gpio_shared_per_parent_irq_data {
> +       unsigned int parent_index;
> +       unsigned int parent_irq;
> +       /*
> +        * The lock of the irq_desc of the parent irq also protects
> +        * exclusive/usedby below. It must be taken before
> +        * lgpio->shared_irq_data->lock.
> +        */
> +       struct irq_data *parent_irq_data;
> +       struct loongson_gpio_chip *lgpio;
> +
> +       /*
> +        * exclusive: exclusive usage, only one gpio pin can use this parent irq.
> +        *   It will happen in two cases:
> +        *     - The parent irq is used by a gpio pin which is edge triggered, and
> +        *       thus cannot be used by other gpio pins which are mapped to the same
> +        *       parent irq.
> +        *     - The parent irq is used by a gpio pin which is level triggered, and
> +        *       the gpio chip lacks the polarity control register, and thus the
> +        *       interrupt polarity is controlled by the parent irq controller, and
> +        *       thus cannot be used by other gpio pins which are mapped to the
> +        *       same parent irq.
> +        *   When set, the request for other gpio pins which are mapped to this
> +        *   parent irq will be rejected.
> +        */
> +       unsigned int exclusive : 1;

So use bool exclusive;

> +       /*
> +        * usedby: When exclusive is set, these bits indicate the gpio pin which
> +        *   is using this parent irq. When exclusive is cleared, these bits
> +        *   indicate the number of gpio pins which are using this parent irq.
> +        * When both exclusive and usedby are 0, it means that the parent irq is not
> +        * used by any gpio pin.
> +        */
> +       unsigned int usedby : 31;

Is this a bitmap? Then use a bitmap abstraction.

> +static void loongson_gpio_shared_irq_unmask(struct irq_data *data)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
> +       struct irq_data *parent_irq_data = ppid->parent_irq_data;

Just look at this code.

Clearly this needs refactoring, we can't have function heads looking
like this.

> +
> +       if (!lgpio->shared_irq_data->irq_in_use[hwirq])
> +               return;
> +
> +       guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
> +
> +       if (ppid->exclusive)
> +               parent_irq_data->chip->irq_mask(parent_irq_data);
> +
> +       scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
> +               loongson_gpio_write_register(lgpio, lgpio->chip_data->inten_offset,
> +                                            hwirq, 1);

This locking is ... really hard to follow, what is going on?
It looks like code written by an LLM.

> +static int loongson_gpio_shared_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
> +       unsigned int intpol_offset = lgpio->chip_data->intpol_offset;
> +       struct irq_data *parent_irq_data = ppid->parent_irq_data;
> +       struct irq_chip *parent_irq_chip = parent_irq_data->chip;
> +       int ret = 0;

Again, what is this? I can't read functions that look like this,
even less maintain them.

> +       /*
> +        * When the irq is not exclusively used and is currently shared by
> +        * more than one gpio pin; or is currently used by single gpio pin and
> +        * the gpio pin is not the one requesting to set the irq type, then the
> +        * irq type can only be set to level triggered.
> +        */
> +       if (!ppid->exclusive &&
> +           (ppid->usedby > 1 ||
> +            (ppid->usedby == 1 && !lgpio->shared_irq_data->irq_in_use[hwirq]))) {
> +               if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_LEVEL_LOW)
> +                       return -EBUSY;
> +
> +               /*
> +                * When exclusive is not set and usedby at least one, the gpio chip must
> +                * have intpol register to control the interrupt polarity. We will never
> +                * reach here if the gpio chip lacks intpol register, because in that
> +                * case, exclusive will be set no matter what trigger type is requested.
> +                */
> +               BUG_ON(!intpol_offset);
> +               /*
> +                * Since only level triggered irqs are allowed to be shared, we can
> +                * safely set the intpol register without first disabling the pin in
> +                * inten register.
> +                */
> +               scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
> +                       loongson_gpio_write_register(lgpio, intpol_offset, hwirq,
> +                                                    type == IRQ_TYPE_LEVEL_HIGH);
> +
> +               if (!lgpio->shared_irq_data->irq_in_use[hwirq])
> +                       ppid->usedby++;
> +
> +               irq_set_handler_locked(data, handle_level_irq);

I like the attention to detail here though!

> +static void loongson_gpio_shared_irq_shutdown(struct irq_data *data)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];

Here again some crazy function header.

I think some helpers need to be broken out so the code becomes more
readable, see what you can do with it to make it easier to follow.

Yours,
Linus Walleij

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Miao Wang]

Hi Linus,

Thanks for your suggestions.

> 2026年10月01日 16:33，Linus Walleij <linusw@kernel.org> 写道：
> 
> Hi Miao,
> 
> thanks for your patch!
> 
> On Fri, Sep 25, 2026 at 6:28 PM Miao Wang via B4 Relay
> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
> 
>> +struct loongson_gpio_shared_per_parent_irq_data {
>> +       unsigned int parent_index;
>> +       unsigned int parent_irq;
>> +       /*
>> +        * The lock of the irq_desc of the parent irq also protects
>> +        * exclusive/usedby below. It must be taken before
>> +        * lgpio->shared_irq_data->lock.
>> +        */
>> +       struct irq_data *parent_irq_data;
>> +       struct loongson_gpio_chip *lgpio;
>> +
>> +       /*
>> +        * exclusive: exclusive usage, only one gpio pin can use this parent irq.
>> +        *   It will happen in two cases:
>> +        *     - The parent irq is used by a gpio pin which is edge triggered, and
>> +        *       thus cannot be used by other gpio pins which are mapped to the same
>> +        *       parent irq.
>> +        *     - The parent irq is used by a gpio pin which is level triggered, and
>> +        *       the gpio chip lacks the polarity control register, and thus the
>> +        *       interrupt polarity is controlled by the parent irq controller, and
>> +        *       thus cannot be used by other gpio pins which are mapped to the
>> +        *       same parent irq.
>> +        *   When set, the request for other gpio pins which are mapped to this
>> +        *   parent irq will be rejected.
>> +        */
>> +       unsigned int exclusive : 1;
> 
> So use bool exclusive;
> 
>> +       /*
>> +        * usedby: When exclusive is set, these bits indicate the gpio pin which
>> +        *   is using this parent irq. When exclusive is cleared, these bits
>> +        *   indicate the number of gpio pins which are using this parent irq.
>> +        * When both exclusive and usedby are 0, it means that the parent irq is not
>> +        * used by any gpio pin.
>> +        */
>> +       unsigned int usedby : 31;
> 
> Is this a bitmap? Then use a bitmap abstraction.

When exclusive is 1, it is a pure number while when exclusive is 0, 
it is a bitmap.

> 
>> +static void loongson_gpio_shared_irq_unmask(struct irq_data *data)
>> +{
>> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
>> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
>> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
>> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
>> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
>> +       struct irq_data *parent_irq_data = ppid->parent_irq_data;
> 
> Just look at this code.
> 
> Clearly this needs refactoring, we can't have function heads looking
> like this.
> 
>> +
>> +       if (!lgpio->shared_irq_data->irq_in_use[hwirq])
>> +               return;
>> +
>> +       guard(raw_spinlock_irqsave)(&irq_data_to_desc(ppid->parent_irq_data)->lock);
>> +
>> +       if (ppid->exclusive)
>> +               parent_irq_data->chip->irq_mask(parent_irq_data);
>> +
>> +       scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
>> +               loongson_gpio_write_register(lgpio, lgpio->chip_data->inten_offset,
>> +                                            hwirq, 1);
> 
> This locking is ... really hard to follow, what is going on?
> It looks like code written by an LLM.

No, it is written by my hand. lgpio->shared_irq_data->lock is used to protect
the irq related registers of the GPIO controller. It is necessary because writing
to a bit of a register may involve RMW, which is not atomic. As a result, every
call to loongson_gpio_{read,write}_register is protected by this lock.

> 
>> +static int loongson_gpio_shared_irq_set_type(struct irq_data *data, unsigned int type)
>> +{
>> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
>> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
>> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
>> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
>> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
>> +       unsigned int intpol_offset = lgpio->chip_data->intpol_offset;
>> +       struct irq_data *parent_irq_data = ppid->parent_irq_data;
>> +       struct irq_chip *parent_irq_chip = parent_irq_data->chip;
>> +       int ret = 0;
> 
> Again, what is this? I can't read functions that look like this,
> even less maintain them.
> 
>> +       /*
>> +        * When the irq is not exclusively used and is currently shared by
>> +        * more than one gpio pin; or is currently used by single gpio pin and
>> +        * the gpio pin is not the one requesting to set the irq type, then the
>> +        * irq type can only be set to level triggered.
>> +        */
>> +       if (!ppid->exclusive &&
>> +           (ppid->usedby > 1 ||
>> +            (ppid->usedby == 1 && !lgpio->shared_irq_data->irq_in_use[hwirq]))) {
>> +               if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_LEVEL_LOW)
>> +                       return -EBUSY;
>> +
>> +               /*
>> +                * When exclusive is not set and usedby at least one, the gpio chip must
>> +                * have intpol register to control the interrupt polarity. We will never
>> +                * reach here if the gpio chip lacks intpol register, because in that
>> +                * case, exclusive will be set no matter what trigger type is requested.
>> +                */
>> +               BUG_ON(!intpol_offset);
>> +               /*
>> +                * Since only level triggered irqs are allowed to be shared, we can
>> +                * safely set the intpol register without first disabling the pin in
>> +                * inten register.
>> +                */
>> +               scoped_guard(raw_spinlock_irqsave, &lgpio->shared_irq_data->lock)
>> +                       loongson_gpio_write_register(lgpio, intpol_offset, hwirq,
>> +                                                    type == IRQ_TYPE_LEVEL_HIGH);
>> +
>> +               if (!lgpio->shared_irq_data->irq_in_use[hwirq])
>> +                       ppid->usedby++;
>> +
>> +               irq_set_handler_locked(data, handle_level_irq);
> 
> I like the attention to detail here though!
> 
>> +static void loongson_gpio_shared_irq_shutdown(struct irq_data *data)
>> +{
>> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
>> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
>> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
>> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
>> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
>> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
> 
> Here again some crazy function header.
> 
> I think some helpers need to be broken out so the code becomes more
> readable, see what you can do with it to make it easier to follow.

I did notice that the header of these functions are duplicated and are a bit long.
However, multiple of the variables defined here will be used later. It is not
obtaining B from A, C from B, and D from C and only using D later, so that we can
introduce a helper to obtain D from A. The problem here is that C and D will both
be used later. What's your idea about the styling here?

Cheers,
Miao Wang

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Linus Walleij]

On Thu, Oct 1, 2026 at 10:59 AM Miao Wang <shankerwangmiao@gmail.com> wrote:

> > Here again some crazy function header.
> >
> > I think some helpers need to be broken out so the code becomes more
> > readable, see what you can do with it to make it easier to follow.
>
> I did notice that the header of these functions are duplicated and are a bit long.
> However, multiple of the variables defined here will be used later. It is not
> obtaining B from A, C from B, and D from C and only using D later, so that we can
> introduce a helper to obtain D from A. The problem here is that C and D will both
> be used later. What's your idea about the styling here?

Maybe struct loongson_gpio_shared_per_parent_irq_data is an uneccesary
long struct name? It obscures the view. On the other hand it is clear what
it is doing...

I usually use minimalist variable names to make thing less cloudy, it is
probably a personal preference. For example I would do:

> +static int loongson_gpio_shared_irq_set_type(struct irq_data *data, unsigned int type)
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +       unsigned int parent_irq_index = lgpio->chip_data->irq_mapping(lgpio, hwirq);
> +       struct loongson_gpio_shared_per_parent_irq_data *ppid =
> +               &lgpio->shared_irq_data->per_parent_irq_data[parent_irq_index];
> +       unsigned int intpol_offset = lgpio->chip_data->intpol_offset;
> +       struct irq_data *parent_irq_data = ppid->parent_irq_data;
> +       struct irq_chip *parent_irq_chip = parent_irq_data->chip;
> +       int ret = 0;

static int loongson_gpio_shared_irq_set_type(struct irq_data *d,
unsigned int type)
{
    struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
    struct loongson_gpio_chip *lg = to_loongson_gpio_chip(gc);
    irq_hw_number_t hwirq = irqd_to_hwirq(d);
    unsigned int parent = lgpio->chip_data->irq_mapping(lg, hwirq);
    struct loongson_gpio_shared_per_parent_irq_data *ppid =
       &lgpio->shared_irq_data->per_parent_irq_data[parent];
    unsigned int off = lgpio->chip_data->intpol_offset;
    struct irq_data *pirqd = ppid->parent_irq_data;
    struct irq_chip *pic = parent_irq_data->chip;
    int ret = 0;

Then I would have already from the beginning (yeah...) used lightweight
struct and member names and it would all be less cluttered.

Admittedly this is a matter of taste. It's not like I would NACK it.

Yours,
Linus Walleij

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Miao Wang]

Hi, 

> 2026年10月1日 16:59，Miao Wang <shankerwangmiao@gmail.com> 写道：
> 
> Hi Linus,
> 
> Thanks for your suggestions.
> 
>> 2026年10月01日 16:33，Linus Walleij <linusw@kernel.org> 写道：
>> 
>> Hi Miao,
>> 
>> thanks for your patch!
>> 
>> On Fri, Sep 25, 2026 at 6:28 PM Miao Wang via B4 Relay
>> <devnull+shankerwangmiao.gmail.com@kernel.org> wrote:
>> 
>>> +struct loongson_gpio_shared_per_parent_irq_data {
>>> +       unsigned int parent_index;
>>> +       unsigned int parent_irq;
>>> +       /*
>>> +        * The lock of the irq_desc of the parent irq also protects
>>> +        * exclusive/usedby below. It must be taken before
>>> +        * lgpio->shared_irq_data->lock.
>>> +        */
>>> +       struct irq_data *parent_irq_data;
>>> +       struct loongson_gpio_chip *lgpio;
>>> +
>>> +       /*
>>> +        * exclusive: exclusive usage, only one gpio pin can use this parent irq.
>>> +        *   It will happen in two cases:
>>> +        *     - The parent irq is used by a gpio pin which is edge triggered, and
>>> +        *       thus cannot be used by other gpio pins which are mapped to the same
>>> +        *       parent irq.
>>> +        *     - The parent irq is used by a gpio pin which is level triggered, and
>>> +        *       the gpio chip lacks the polarity control register, and thus the
>>> +        *       interrupt polarity is controlled by the parent irq controller, and
>>> +        *       thus cannot be used by other gpio pins which are mapped to the
>>> +        *       same parent irq.
>>> +        *   When set, the request for other gpio pins which are mapped to this
>>> +        *   parent irq will be rejected.
>>> +        */
>>> +       unsigned int exclusive : 1;
>> 
>> So use bool exclusive;
>> 
>>> +       /*
>>> +        * usedby: When exclusive is set, these bits indicate the gpio pin which
>>> +        *   is using this parent irq. When exclusive is cleared, these bits
>>> +        *   indicate the number of gpio pins which are using this parent irq.
>>> +        * When both exclusive and usedby are 0, it means that the parent irq is not
>>> +        * used by any gpio pin.
>>> +        */
>>> +       unsigned int usedby : 31;
>> 
>> Is this a bitmap? Then use a bitmap abstraction.
> 
> When exclusive is 1, it is a pure number while when exclusive is 0, 
> it is a bitmap.

To correct my self, usedby is not a bitmap. When exclusive is 1, it stores the using pin
id, while when exclusive is zero, it is a counter which stores the number of using pins.

Cheers,

Miao Wang

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Linus Walleij]

On Thu, Oct 1, 2026 at 7:47 PM Miao Wang <shankerwangmiao@gmail.com> wrote:

> >>> +       unsigned int usedby : 31;
> >>
> >> Is this a bitmap? Then use a bitmap abstraction.
> >
> > When exclusive is 1, it is a pure number while when exclusive is 0,
> > it is a bitmap.
>
> To correct my self, usedby is not a bitmap. When exclusive is 1, it stores the using pin
> id, while when exclusive is zero, it is a counter which stores the number of using pins.

But what is this mess with using 31 bits for the pin ID?

How many bits does it *actually* need, using 31 bits looks
like some way to try to help the compiler to optimize the struct
which is pointless because the struct isn't even suffixed with
__packed...

Yours,
Linus Walleij

Re: [PATCH RFC 3/4] gpio: loongson-64bit: Add shared interrupt support

[Miao Wang]

Hi,

> 2026年10月2日 03:43，Linus Walleij <linusw@kernel.org> 写道：
> 
> On Thu, Oct 1, 2026 at 7:47 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
> 
>>>>> +       unsigned int usedby : 31;
>>>> 
>>>> Is this a bitmap? Then use a bitmap abstraction.
>>> 
>>> When exclusive is 1, it is a pure number while when exclusive is 0,
>>> it is a bitmap.
>> 
>> To correct my self, usedby is not a bitmap. When exclusive is 1, it stores the using pin
>> id, while when exclusive is zero, it is a counter which stores the number of using pins.
> 
> But what is this mess with using 31 bits for the pin ID?
> 
> How many bits does it *actually* need, using 31 bits looks
> like some way to try to help the compiler to optimize the struct
> which is pointless because the struct isn't even suffixed with
> __packed...

Ah, you are right about it. I intended to reduce the memory that
is needed to hold the struct. However I realized that by using
bit fields, the memory cannot be saved due to the alignment. Will
change in next version.

Cheers,

Miao Wang

[PATCH RFC 4/4] gpio: loongson-64bit: remove gpio_to_irq() handler

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

As pointed out in the previous commit, gpio_to_irq() should not be used
because it should not be used to manipulate the registers to enable
interrupts.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 drivers/gpio/gpio-loongson-64bit.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 6e5687dcb413c247fb0d76c596c445f94efccd01..d24eb52ef13ce386437c422e290f1b0ea4e6672b 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -141,24 +141,6 @@ static int loongson_gpio_set(struct gpio_chip *chip, unsigned int pin, int value
 	return 0;
 }
 
-static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
-	unsigned int u;
-	struct platform_device *pdev = to_platform_device(chip->parent);
-	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
-
-	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
-		/* Get the register index from offset then multiply by bytes per register */
-		u = readl(lgpio->reg_base + lgpio->chip_data->inten_offset + (offset / 32) * 4);
-		u |= BIT(offset % 32);
-		writel(u, lgpio->reg_base + lgpio->chip_data->inten_offset + (offset / 32) * 4);
-	} else {
-		writeb(1, lgpio->reg_base + lgpio->chip_data->inten_offset + offset);
-	}
-
-	return platform_get_irq(pdev, offset);
-}
-
 static void loongson_gpio_irq_ack(struct irq_data *data)
 {
 	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
@@ -945,8 +927,6 @@ static int loongson_gpio_init(struct platform_device *pdev, struct loongson_gpio
 			dev_warn(&pdev->dev, "no parent IRQs ha been found, GPIO IRQs will not be available\n");
 			gpio_irq_chip_set_chip(&lgpio->chip.gc.irq, NULL);
 		}
-	} else if (lgpio->chip_data->inten_offset) {
-		lgpio->chip.gc.to_irq = loongson_gpio_to_irq;
 	}
 
 	return devm_gpiochip_add_data(&pdev->dev, &lgpio->chip.gc, lgpio);

-- 
2.55.0

Re: [PATCH RFC 0/4] gpio: loongson-64bit: add interrupt support for more models

[Bartosz Golaszewski]

On Fri, 25 Sep 2026 18:28:35 +0200, Miao Wang via B4 Relay
<devnull+shankerwangmiao.gmail.com@kernel.org> said:
> This patch series aims to correctly add interrupt support for GPIO
> controllers found on more Loongson CPUs. The GPIO controllers in such
> models do not have a complete intgrated interrupt controller, but choose
> to delegate interrupt handling to their parent interrupt controller.
> This sounds like a heirarchical interrupt setup, but what makes it
> more complex is that multiple GPIO lines in these controllers may share
> one interrupt line on the parent interrupt controller. The details is
> described in the third patch.
>
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
> Miao Wang (4):
>       gpio: loongson-64bit: Support for irq controller on 2k2000 and 7a2000
>       gpio: loongson-64bit: Generalize interrupt controller initialization
>       gpio: loongson-64bit: Add shared interrupt support
>       gpio: loongson-64bit: remove gpio_to_irq() handler
>
>  drivers/gpio/gpio-loongson-64bit.c | 708 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 683 insertions(+), 25 deletions(-)
> ---
> base-commit: 481ed5dd3ed7136f627b8ec372ba39f5b2e7d27f
> change-id: 20260925-loongsongpio-829824a3d562
>
> Best regards,
> --
> Miao Wang <shankerwangmiao@gmail.com>
>
>
>

Can you please resend it with linux-kernel@vger.kernel.org in Cc? Otherwise
Sashiko doesn't pick it up. I'd like it to take a look as I don't have enough
context of this driver for a thorough review myself and it's a complex change.

Bart

Re: [PATCH RFC 0/4] gpio: loongson-64bit: add interrupt support for more models

[Miao Wang]

Hi,

> 2026年09月29日 21:01，Bartosz Golaszewski <brgl@kernel.org> 写道：
> 
> Can you please resend it with linux-kernel@vger.kernel.org in Cc? Otherwise
> Sashiko doesn't pick it up. I'd like it to take a look as I don't have enough
> context of this driver for a thorough review myself and it's a complex change.

Sorry but I will not be able to resend the patches until the repair of my
laptop ends. I'll do it as soon as my laptop is back.

In the meantime, I have discussed the details and the related context of this
change in the third patch.

Cheers,

Miao Wang