[PATCH v2 0/4] gpio: davinci: Add support for level-triggered interrupts

[PATCH v2 0/4] gpio: davinci: Add support for level-triggered interrupts

[Aniket Limaye]

Add support for level-triggered interrupts (IRQ_TYPE_LEVEL_HIGH and
IRQ_TYPE_LEVEL_LOW) for TDA54 GPIO controller via extended register sets.
Update the dt-bindings to add support for ti,tda54-gpio.

Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
Changes in v2:
- picked up Acked-by for dt-bindings
- Split the driver changes into 3 patches: Add proper match_data struct
  in driver, Add ext_reg match_data field and level-triggered interrupt
  support, Add new compatible with level-triggered support
- davinci_gpio_irq_setup() now only sets IRQ_TYPE_EDGE_BOTH by default
- For level-triggered interrupts use handle_level_irq()
- Link to v1: https://lore.kernel.org/r/20260923-upstream-tda54-gpio-v1-0-d1a0a199c836@ti.com

---
Aniket Limaye (4):
      dt-bindings: gpio: gpio-davinci: Add the tda54 support
      gpio: davinci: Add match data struct for get_irq_chip callback
      gpio: davinci: Add support for level-triggered interrupts
      gpio: davinci: Add ti,tda54-gpio compatible

 .../devicetree/bindings/gpio/gpio-davinci.yaml     |   1 +
 drivers/gpio/gpio-davinci.c                        | 146 +++++++++++++++++++--
 2 files changed, 137 insertions(+), 10 deletions(-)
---
base-commit: b810335a0699d1810683796b0e3ffe8827ddfe6c
change-id: 20260923-upstream-tda54-gpio-da6747dfd34b

Best regards,
-- 
Aniket Limaye <a-limaye@ti.com>

[PATCH v2 1/4] dt-bindings: gpio: gpio-davinci: Add the tda54 support

[Aniket Limaye]

New version of GPIO IP used in TDA54 SoCs adds support for level-triggered
interrupts via extended registers.
Add the device binding for ti,tda54-gpio.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
 Documentation/devicetree/bindings/gpio/gpio-davinci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml b/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
index 1434d08f8b741..02b6b9a436a68 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
@@ -18,6 +18,7 @@ properties:
               - ti,am654-gpio
               - ti,j721e-gpio
               - ti,am64-gpio
+              - ti,tda54-gpio
           - const: ti,keystone-gpio
 
       - items:

-- 
2.53.0

Re: [PATCH v2 1/4] dt-bindings: gpio: gpio-davinci: Add the tda54 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: missing interrupt-controller requirements for ti,tda54-gpio
--

commit 4e896075feb00707123d20bb29a2b6cd491b1238
Author: Aniket Limaye <a-limaye@ti.com>

dt-bindings: gpio: gpio-davinci: Add the tda54 support

This commit adds device tree binding support for the new TDA54 GPIO IP,
which supports level-triggered interrupts. It adds the "ti,tda54-gpio"
compatible string to the schema.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml b/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
> index 1434d08f8b741..02b6b9a436a68 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.yaml
> @@ -18,6 +18,7 @@ properties:
>                - ti,am654-gpio
>                - ti,j721e-gpio
>                - ti,am64-gpio
> +              - ti,tda54-gpio

[Severity: Medium]
Since the commit message indicates this hardware variant adds support for
level-triggered interrupts, implying the GPIO block acts as an interrupt
provider, should this binding enforce the presence of "interrupt-controller"
and "#interrupt-cells"?

Without an allOf/if-then conditional block requiring these properties
specifically for "ti,tda54-gpio", incomplete device tree nodes missing them
will silently pass schema validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-upstream-tda54-gpio-v2-0-5c4f8b10cdbc@ti.com?part=1

[PATCH v2 2/4] gpio: davinci: Add match data struct for get_irq_chip callback

[Aniket Limaye]

Introduce struct davinci_gpio_match_data and move the get_irq_chip()
callback into it, instead of passing the callback directly through the
.data field of struct of_device_id. This allows describing further
per-compatible properties in the match data.

No functional change intended.

Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
 drivers/gpio/gpio-davinci.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index cccbaea1dec24..ab1b465dfcd15 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -41,6 +41,10 @@ struct davinci_gpio_regs {
 
 typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
 
+struct davinci_gpio_match_data {
+	gpio_get_irq_chip_cb_t get_irq_chip;
+};
+
 #define BINTEN	0x8 /* GPIO Interrupt Per-Bank Enable Register */
 
 static void __iomem *gpio_base;
@@ -449,6 +453,14 @@ static struct irq_chip *keystone_gpio_get_irq_chip(unsigned int irq)
 	return &gpio_unbanked;
 };
 
+static const struct davinci_gpio_match_data davinci_gpio_data = {
+	.get_irq_chip = davinci_gpio_get_irq_chip,
+};
+
+static const struct davinci_gpio_match_data keystone_gpio_data = {
+	.get_irq_chip = keystone_gpio_get_irq_chip,
+};
+
 static const struct of_device_id davinci_gpio_ids[];
 
 /*
@@ -478,8 +490,13 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 * Use davinci_gpio_get_irq_chip by default to handle non DT cases
 	 */
 	gpio_get_irq_chip = davinci_gpio_get_irq_chip;
-	if (dev->of_node)
-		gpio_get_irq_chip = (gpio_get_irq_chip_cb_t)device_get_match_data(dev);
+	if (dev->of_node) {
+		const struct davinci_gpio_match_data *match_data;
+
+		match_data = device_get_match_data(dev);
+		if (match_data)
+			gpio_get_irq_chip = match_data->get_irq_chip;
+	}
 
 	ngpio = chips->chip.ngpio;
 
@@ -668,9 +685,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(davinci_gpio_dev_pm_ops, davinci_gpio_suspend,
 			 davinci_gpio_resume);
 
 static const struct of_device_id davinci_gpio_ids[] = {
-	{ .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
-	{ .compatible = "ti,am654-gpio", keystone_gpio_get_irq_chip},
-	{ .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
+	{ .compatible = "ti,keystone-gpio", .data = &keystone_gpio_data },
+	{ .compatible = "ti,am654-gpio", .data = &keystone_gpio_data },
+	{ .compatible = "ti,dm6441-gpio", .data = &davinci_gpio_data },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, davinci_gpio_ids);

-- 
2.53.0

[PATCH v2 3/4] gpio: davinci: Add support for level-triggered interrupts

[Aniket Limaye]

Add support for IRQ_TYPE_LEVEL_HIGH and IRQ_TYPE_LEVEL_LOW through the
extended registers: set_high/clr_high/set_low/clr_low. Extend struct
davinci_gpio_match_data with an ext_reg flag to describe such
controllers.

The level enables are cleared on setup and are only set when a level
trigger is requested, so the default trigger of both edges is
preserved.

No existing compatible sets ext_reg, so there is no functional change
for current users.

Signed-off-by: Aniket Limaye <a-limaye@ti.com>

---

Changes in v2:
- davinci_gpio_irq_setup() now only sets IRQ_TYPE_EDGE_BOTH by default
- For level-triggered interrupts use handle_level_irq()
- Link to v1: https://lore.kernel.org/r/20260923-upstream-tda54-gpio-v1-2-d1a0a199c836@ti.com
---
 drivers/gpio/gpio-davinci.c | 113 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index ab1b465dfcd15..8d0ebf4afd169 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -39,16 +39,25 @@ struct davinci_gpio_regs {
 	u32	intstat;
 };
 
+struct davinci_gpio_regs_ext {
+	u32	set_high;
+	u32	clr_high;
+	u32	set_low;
+	u32	clr_low;
+};
+
 typedef struct irq_chip *(*gpio_get_irq_chip_cb_t)(unsigned int irq);
 
 struct davinci_gpio_match_data {
 	gpio_get_irq_chip_cb_t get_irq_chip;
+	bool ext_reg;
 };
 
 #define BINTEN	0x8 /* GPIO Interrupt Per-Bank Enable Register */
 
 static void __iomem *gpio_base;
 static unsigned int offset_array[5] = {0x10, 0x38, 0x60, 0x88, 0xb0};
+static unsigned int offset_array_ext[5] = {0xd8, 0xe8, 0xf8, 0x108, 0x118};
 
 struct davinci_gpio_irq_data {
 	void __iomem			*regs;
@@ -62,9 +71,11 @@ struct davinci_gpio_controller {
 	/* Serialize access to GPIO registers */
 	spinlock_t		lock;
 	void __iomem		*regs[MAX_REGS_BANKS];
+	void __iomem		*regs_ext[MAX_REGS_BANKS];
 	int			gpio_unbanked;
 	int			irqs[MAX_INT_PER_BANK];
 	struct davinci_gpio_regs context[MAX_REGS_BANKS];
+	struct davinci_gpio_regs_ext	context_ext[MAX_REGS_BANKS];
 	u32			binten_context;
 };
 
@@ -172,6 +183,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	unsigned int ngpio, nbank, nirq, gpio_unbanked;
 	struct davinci_gpio_controller *chips;
 	struct device *dev = &pdev->dev;
+	const struct davinci_gpio_match_data *match_data;
+	bool ext_reg = false;
 
 	/*
 	 * The gpio banks conceptually expose a segmented bitmap,
@@ -194,6 +207,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to get the unbanked GPIOs property\n");
 
+	match_data = device_get_match_data(dev);
+	if (match_data)
+		ext_reg = match_data->ext_reg;
+
 	if (gpio_unbanked)
 		nirq = gpio_unbanked;
 	else
@@ -239,8 +256,11 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	chips->gpio_unbanked = gpio_unbanked;
 
 	nbank = DIV_ROUND_UP(ngpio, 32);
-	for (bank = 0; bank < nbank; bank++)
+	for (bank = 0; bank < nbank; bank++) {
 		chips->regs[bank] = gpio_base + offset_array[bank];
+		if (ext_reg)
+			chips->regs_ext[bank] = gpio_base + offset_array_ext[bank];
+	}
 
 	ret = devm_gpiochip_add_data(dev, &chips->chip, chips);
 	if (ret)
@@ -271,10 +291,15 @@ static void gpio_irq_mask(struct irq_data *d)
 	struct davinci_gpio_controller *chips = irq_data_get_irq_chip_data(d);
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	struct davinci_gpio_regs __iomem *g = chips->regs[hwirq / 32];
+	struct davinci_gpio_regs_ext __iomem *g_ext = chips->regs_ext[hwirq / 32];
 	uintptr_t mask = (uintptr_t)irq_data_get_irq_handler_data(d);
 
 	writel_relaxed(mask, &g->clr_falling);
 	writel_relaxed(mask, &g->clr_rising);
+	if (g_ext) {
+		writel_relaxed(mask, &g_ext->clr_high);
+		writel_relaxed(mask, &g_ext->clr_low);
+	}
 
 	gpiochip_disable_irq(&chips->chip, hwirq);
 }
@@ -284,12 +309,16 @@ static void gpio_irq_unmask(struct irq_data *d)
 	struct davinci_gpio_controller *chips = irq_data_get_irq_chip_data(d);
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	struct davinci_gpio_regs __iomem *g = chips->regs[hwirq / 32];
+	struct davinci_gpio_regs_ext __iomem *g_ext = chips->regs_ext[hwirq / 32];
 	uintptr_t mask = (uintptr_t)irq_data_get_irq_handler_data(d);
 	unsigned status = irqd_get_trigger_type(d);
 
 	gpiochip_enable_irq(&chips->chip, hwirq);
 
-	status &= IRQ_TYPE_EDGE_BOTH;
+	if (g_ext)
+		status &= IRQ_TYPE_SENSE_MASK;
+	else
+		status &= IRQ_TYPE_EDGE_BOTH;
 	if (!status)
 		status = IRQ_TYPE_EDGE_BOTH;
 
@@ -297,6 +326,31 @@ static void gpio_irq_unmask(struct irq_data *d)
 		writel_relaxed(mask, &g->set_falling);
 	if (status & IRQ_TYPE_EDGE_RISING)
 		writel_relaxed(mask, &g->set_rising);
+	if (status & IRQ_TYPE_LEVEL_HIGH)
+		writel_relaxed(mask, &g_ext->set_high);
+	if (status & IRQ_TYPE_LEVEL_LOW)
+		writel_relaxed(mask, &g_ext->set_low);
+}
+
+static bool gpio_irq_trigger_valid_ext(unsigned int trigger)
+{
+	if (trigger & ~IRQ_TYPE_SENSE_MASK)
+		return false;
+
+	return (trigger & IRQ_TYPE_LEVEL_MASK) != IRQ_TYPE_LEVEL_MASK;
+}
+
+static int gpio_irq_type_ext(struct irq_data *d, unsigned int trigger)
+{
+	if (!gpio_irq_trigger_valid_ext(trigger))
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else
+		irq_set_handler_locked(d, handle_simple_irq);
+
+	return 0;
 }
 
 static int gpio_irq_type(struct irq_data *d, unsigned trigger)
@@ -307,6 +361,15 @@ static int gpio_irq_type(struct irq_data *d, unsigned trigger)
 	return 0;
 }
 
+static const struct irq_chip gpio_irqchip_ext = {
+	.name		= "GPIO",
+	.irq_unmask	= gpio_irq_unmask,
+	.irq_mask	= gpio_irq_mask,
+	.irq_set_type	= gpio_irq_type_ext,
+	.flags		= IRQCHIP_IMMUTABLE | IRQCHIP_SET_TYPE_MASKED | IRQCHIP_SKIP_SET_WAKE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
 static const struct irq_chip gpio_irqchip = {
 	.name		= "GPIO",
 	.irq_unmask	= gpio_irq_unmask,
@@ -391,10 +454,12 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
 {
 	struct davinci_gpio_controller *d;
 	struct davinci_gpio_regs __iomem *g;
+	struct davinci_gpio_regs_ext __iomem *g_ext;
 	u32 mask, i;
 
 	d = (struct davinci_gpio_controller *)irq_data_get_irq_handler_data(data);
 	g = (struct davinci_gpio_regs __iomem *)d->regs[0];
+	g_ext = (struct davinci_gpio_regs_ext __iomem *)d->regs_ext[0];
 	for (i = 0; i < MAX_INT_PER_BANK; i++)
 		if (data->irq == d->irqs[i])
 			break;
@@ -404,13 +469,20 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
 
 	mask = __gpio_mask(i);
 
-	if (trigger & ~IRQ_TYPE_EDGE_BOTH)
+	if (g_ext ? !gpio_irq_trigger_valid_ext(trigger) :
+		    (trigger & ~IRQ_TYPE_EDGE_BOTH))
 		return -EINVAL;
 
 	writel_relaxed(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
 		     ? &g->set_falling : &g->clr_falling);
 	writel_relaxed(mask, (trigger & IRQ_TYPE_EDGE_RISING)
 		     ? &g->set_rising : &g->clr_rising);
+	if (g_ext) {
+		writel_relaxed(mask, (trigger & IRQ_TYPE_LEVEL_HIGH)
+			       ? &g_ext->set_high : &g_ext->clr_high);
+		writel_relaxed(mask, (trigger & IRQ_TYPE_LEVEL_LOW)
+			       ? &g_ext->set_low : &g_ext->clr_low);
+	}
 
 	return 0;
 }
@@ -421,9 +493,11 @@ davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 {
 	struct davinci_gpio_controller *chips =
 				(struct davinci_gpio_controller *)d->host_data;
+	const struct irq_chip *chip = chips->regs_ext[0] ? &gpio_irqchip_ext :
+							    &gpio_irqchip;
 
-	irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
-				"davinci_gpio");
+	irq_set_chip_and_handler_name(irq, chip, handle_simple_irq,
+				      "davinci_gpio");
 	irq_set_irq_type(irq, IRQ_TYPE_NONE);
 	irq_set_chip_data(irq, (__force void *)chips);
 	irq_set_handler_data(irq, (void *)(uintptr_t)__gpio_mask(hw));
@@ -481,6 +555,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
 	struct davinci_gpio_regs __iomem *g;
+	struct davinci_gpio_regs_ext __iomem *g_ext;
 	struct irq_domain	*irq_domain = NULL;
 	struct irq_chip *irq_chip;
 	struct davinci_gpio_irq_data *irqdata;
@@ -551,6 +626,11 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		g = chips->regs[0];
 		writel_relaxed(~0, &g->set_falling);
 		writel_relaxed(~0, &g->set_rising);
+		g_ext = chips->regs_ext[0];
+		if (g_ext) {
+			writel_relaxed(~0, &g_ext->clr_high);
+			writel_relaxed(~0, &g_ext->clr_low);
+		}
 
 		/* set the direct IRQs up to use that irqchip */
 		for (gpio = 0; gpio < chips->gpio_unbanked; gpio++) {
@@ -575,6 +655,11 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		g = chips->regs[bank / 2];
 		writel_relaxed(~0, &g->clr_falling);
 		writel_relaxed(~0, &g->clr_rising);
+		g_ext = chips->regs_ext[bank / 2];
+		if (g_ext) {
+			writel_relaxed(~0, &g_ext->clr_high);
+			writel_relaxed(~0, &g_ext->clr_low);
+		}
 
 		/*
 		 * Each chip handles 32 gpios, and each irq bank consists of 16
@@ -614,7 +699,9 @@ static void davinci_gpio_save_context(struct davinci_gpio_controller *chips,
 				      u32 nbank)
 {
 	struct davinci_gpio_regs __iomem *g;
+	struct davinci_gpio_regs_ext __iomem *g_ext;
 	struct davinci_gpio_regs *context;
+	struct davinci_gpio_regs_ext *context_ext;
 	u32 bank;
 	void __iomem *base;
 
@@ -628,6 +715,12 @@ static void davinci_gpio_save_context(struct davinci_gpio_controller *chips,
 		context->set_data = readl_relaxed(&g->set_data);
 		context->set_rising = readl_relaxed(&g->set_rising);
 		context->set_falling = readl_relaxed(&g->set_falling);
+		g_ext = chips->regs_ext[bank];
+		if (g_ext) {
+			context_ext = &chips->context_ext[bank];
+			context_ext->set_high = readl_relaxed(&g_ext->set_high);
+			context_ext->set_low = readl_relaxed(&g_ext->set_low);
+		}
 	}
 
 	/* Clear all interrupt status registers */
@@ -638,7 +731,9 @@ static void davinci_gpio_restore_context(struct davinci_gpio_controller *chips,
 					 u32 nbank)
 {
 	struct davinci_gpio_regs __iomem *g;
+	struct davinci_gpio_regs_ext __iomem *g_ext;
 	struct davinci_gpio_regs *context;
+	struct davinci_gpio_regs_ext *context_ext;
 	u32 bank;
 	void __iomem *base;
 
@@ -658,6 +753,14 @@ static void davinci_gpio_restore_context(struct davinci_gpio_controller *chips,
 			writel_relaxed(context->set_rising, &g->set_rising);
 		if (readl_relaxed(&g->set_falling) != context->set_falling)
 			writel_relaxed(context->set_falling, &g->set_falling);
+		g_ext = chips->regs_ext[bank];
+		if (g_ext) {
+			context_ext = &chips->context_ext[bank];
+			if (readl_relaxed(&g_ext->set_high) != context_ext->set_high)
+				writel_relaxed(context_ext->set_high, &g_ext->set_high);
+			if (readl_relaxed(&g_ext->set_low) != context_ext->set_low)
+				writel_relaxed(context_ext->set_low, &g_ext->set_low);
+		}
 	}
 }
 

-- 
2.53.0

Re: [PATCH v2 3/4] gpio: davinci: Add support for level-triggered interrupts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] irq: unbanked level interrupts fail to update flow handler and overwrite parent irq_set_type
- [High] gpio: davinci: missing irq_ack causes spurious level interrupts
- [High] gpio: davinci: out-of-bounds array access due to unvalidated ti,ngpio
- [Medium] gpio: TOCTOU race and massive overhead due to gpiochip_[dis|en]able_irq in mask/unmask callbacks
- [Medium] gpio: davinci: W1S registers not cleared on resume
--

commit f6da5fa1d3ffb73b515603cd9539eceb04b2addc
Author: Aniket Limaye <a-limaye@ti.com>
Subject: gpio: davinci: Add support for level-triggered interrupts

This patch adds support for IRQ_TYPE_LEVEL_HIGH and IRQ_TYPE_LEVEL_LOW triggers to the DaVinci GPIO driver by utilizing extended registers. It clears level enables on setup and only sets them when a level trigger is explicitly requested to preserve the default both-edges trigger.

Link: https://lore.kernel.org/r/20260923-upstream-tda54-gpio-v1-2-d1a0a199c836@ti.com

[Severity: High]
In drivers/gpio/gpio-davinci.c, around line 623 in davinci_gpio_irq_setup, the code overwrites the parent controller's irq_set_type callback with gpio_irq_type_unbanked.

Then, around line 472 in gpio_irq_type_unbanked, it configures the new level registers but returns 0 without calling irq_set_handler_locked to set handle_level_irq, and without chaining to the parent's original irq_set_type callback.

Because unbanked GPIO interrupts bypass the local IRQ domain and use the parent controller directly, won't this leave the parent controller unaware of the level trigger configuration? Does this mean the flow handler remains handle_edge_irq, meaning we miss the required mask/ack sequence and risk an immediate interrupt storm when the line is held active?

[Severity: Medium]
In drivers/gpio/gpio-davinci.c, handle_level_irq is used for the new level-triggered interrupts, which means the irq_mask and irq_unmask callbacks are invoked on every single interrupt.

Around line 302 in gpio_irq_mask and line 315 in gpio_irq_unmask, the driver calls gpiochip_disable_irq and gpiochip_enable_irq. Since these gpiolib functions take spinlocks, won't this add massive overhead on every level interrupt?

Furthermore, clearing the GPIOD_FLAG_IRQ_IS_ENABLED flag during the interrupt allows a concurrent userspace thread to successfully change the GPIO direction to output. When the interrupt finishes and gpiochip_enable_irq is called, could this trigger a WARN_ON because the pin is now an output? Should these gpiolib calls be moved to irq_enable and irq_disable instead?

[Severity: High]
In drivers/gpio/gpio-davinci.c around line 364, the newly added gpio_irqchip_ext irq_chip lacks an irq_ack or irq_mask_ack callback.

In the chained handler gpio_irq_handler around line 404, it reads and clears intstat before calling generic_handle_domain_irq. For level-triggered interrupts, the hardware will immediately re-latch the intstat bit since the external line is still active.

When handle_level_irq masks the interrupt, it cannot clear this re-latched intstat due to the missing ack callback. Even after the device ISR runs and de-asserts the line, intstat remains latched. Could this cause the while loop in gpio_irq_handler to see a stale intstat bit and fire a spurious interrupt upon return, eventually causing the spurious interrupt detector to permanently disable the IRQ line?

[Severity: Medium]
In drivers/gpio/gpio-davinci.c around line 757, davinci_gpio_restore_context attempts to restore the extension registers set_high and set_low upon system resume.

Since these appear to be Write-1-to-Set registers in hardware, writing a 0 to a bit position has no effect. If firmware enabled a bit during suspend and it is 0 in the saved context, writing the 0 fails to clear it.

Could this leave level-triggered interrupts improperly enabled after resume? Would it be better to write the negated context to the corresponding clr_high and clr_low registers to properly clear bits?

[Severity: High]
In drivers/gpio/gpio-davinci.c around line 262 in davinci_gpio_probe, the initialization loop iterates nbank times to initialize chips->regs_ext and reads from offset_array_ext.

If a malformed Device Tree provides a ti,ngpio value greater than 160, nbank will exceed MAX_REGS_BANKS which is 5.

Could this cause an out-of-bounds read past the static offset_array_ext and a heap buffer overflow when writing past the end of the chips->regs_ext array? Since the only sanity check on nirq uses gpio_unbanked rather than ngpio, should we validate ngpio against MAX_REGS_BANKS before iterating?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-upstream-tda54-gpio-v2-0-5c4f8b10cdbc@ti.com?part=3

[PATCH v2 4/4] gpio: davinci: Add ti,tda54-gpio compatible

[Aniket Limaye]

Add the ti,tda54-gpio compatible with extended registers for
level-triggered interrupt support.

Signed-off-by: Aniket Limaye <a-limaye@ti.com>
---
 drivers/gpio/gpio-davinci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 8d0ebf4afd169..db1f722693e0e 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -535,6 +535,11 @@ static const struct davinci_gpio_match_data keystone_gpio_data = {
 	.get_irq_chip = keystone_gpio_get_irq_chip,
 };
 
+static const struct davinci_gpio_match_data tda54_gpio_data = {
+	.get_irq_chip = keystone_gpio_get_irq_chip,
+	.ext_reg = true,
+};
+
 static const struct of_device_id davinci_gpio_ids[];
 
 /*
@@ -791,6 +796,7 @@ static const struct of_device_id davinci_gpio_ids[] = {
 	{ .compatible = "ti,keystone-gpio", .data = &keystone_gpio_data },
 	{ .compatible = "ti,am654-gpio", .data = &keystone_gpio_data },
 	{ .compatible = "ti,dm6441-gpio", .data = &davinci_gpio_data },
+	{ .compatible = "ti,tda54-gpio", .data = &tda54_gpio_data },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, davinci_gpio_ids);

-- 
2.53.0

Re: [PATCH v2 4/4] gpio: davinci: Add ti,tda54-gpio compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: davinci: missing IRQ flow handler update for level-triggered unbanked IRQs
--

commit 8228d1919a6fb52d7ab9246eaa1447341a319a9f
Author: Aniket Limaye <a-limaye@ti.com>

gpio: davinci: Add ti,tda54-gpio compatible
    
Add the ti,tda54-gpio compatible with extended registers for
level-triggered interrupt support.

> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 8d0ebf4afd169..db1f722693e0e 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -535,6 +535,11 @@ static const struct davinci_gpio_match_data keystone_gpio_data = {
>  	.get_irq_chip = keystone_gpio_get_irq_chip,
>  };
>  
> +static const struct davinci_gpio_match_data tda54_gpio_data = {
> +	.get_irq_chip = keystone_gpio_get_irq_chip,
> +	.ext_reg = true,

[Severity: High]
Does enabling extended registers for level-triggered interrupts here result
in missing flow handler updates for unbanked interrupts?

For unbanked interrupts, the driver overrides the parent irqchip's
.irq_set_type with gpio_irq_type_unbanked(). While this function configures
the local GPIO hardware for level triggers when g_ext is available:

drivers/gpio/gpio-davinci.c:gpio_irq_type_unbanked() {
	...
	if (g_ext) {
		writel_relaxed(mask, (trigger & IRQ_TYPE_LEVEL_HIGH)
			       ? &g_ext->set_high : &g_ext->clr_high);
		writel_relaxed(mask, (trigger & IRQ_TYPE_LEVEL_LOW)
			       ? &g_ext->set_low : &g_ext->clr_low);
	}

	return 0;
}

It returns 0 without calling the parent's .irq_set_type callback or
dynamically updating the flow handler (e.g., via irq_set_handler_locked()).

Will this cause the parent interrupt controller and the logical IRQ flow
handler to remain configured for default edge triggers, potentially leading
to mishandled level interrupts?

> +};
> +
>  static const struct of_device_id davinci_gpio_ids[];

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-upstream-tda54-gpio-v2-0-5c4f8b10cdbc@ti.com?part=4