* [PATCH 0/2] gpio: davinci: Add support for level-triggered interrupts
@ 2026-09-23 12:56 Aniket Limaye
2026-09-23 12:56 ` [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support Aniket Limaye
2026-09-23 12:56 ` [PATCH 2/2] gpio: davinci: Add level support via extended registers Aniket Limaye
0 siblings, 2 replies; 7+ messages in thread
From: Aniket Limaye @ 2026-09-23 12:56 UTC (permalink / raw)
To: Keerthy, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel,
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>
---
Aniket Limaye (2):
dt-bindings: gpio: gpio-davinci: Add the tda54 support
gpio: davinci: Add level support via extended registers
.../devicetree/bindings/gpio/gpio-davinci.yaml | 1 +
drivers/gpio/gpio-davinci.c | 103 +++++++++++++++++++--
2 files changed, 98 insertions(+), 6 deletions(-)
---
base-commit: 4220e7e54566d6d35ca26dbd166aaa5c7de44acc
change-id: 20260923-upstream-tda54-gpio-da6747dfd34b
Best regards,
--
Aniket Limaye <a-limaye@ti.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support 2026-09-23 12:56 [PATCH 0/2] gpio: davinci: Add support for level-triggered interrupts Aniket Limaye @ 2026-09-23 12:56 ` Aniket Limaye 2026-09-23 13:57 ` Krzysztof Kozlowski 2026-09-28 12:28 ` Bartosz Golaszewski 2026-09-23 12:56 ` [PATCH 2/2] gpio: davinci: Add level support via extended registers Aniket Limaye 1 sibling, 2 replies; 7+ messages in thread From: Aniket Limaye @ 2026-09-23 12:56 UTC (permalink / raw) To: Keerthy, Linus Walleij, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel, 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. 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 1434d08f8b74..02b6b9a436a6 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support 2026-09-23 12:56 ` [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support Aniket Limaye @ 2026-09-23 13:57 ` Krzysztof Kozlowski 2026-09-28 12:28 ` Bartosz Golaszewski 1 sibling, 0 replies; 7+ messages in thread From: Krzysztof Kozlowski @ 2026-09-23 13:57 UTC (permalink / raw) To: Aniket Limaye, Keerthy, Linus Walleij, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel On 23/09/2026 14:56, Aniket Limaye wrote: > 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. > > Signed-off-by: Aniket Limaye <a-limaye@ti.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support 2026-09-23 12:56 ` [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support Aniket Limaye 2026-09-23 13:57 ` Krzysztof Kozlowski @ 2026-09-28 12:28 ` Bartosz Golaszewski 1 sibling, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-09-28 12:28 UTC (permalink / raw) To: Aniket Limaye Cc: Keerthy, Linus Walleij, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley, vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel On Wed, 23 Sep 2026 14:56:44 +0200, Aniket Limaye <a-limaye@ti.com> said: > 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. > Nit: Does the new feature really matter for the binding if it doesn't warrant any changes in properties? Bart > 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 1434d08f8b74..02b6b9a436a6 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] gpio: davinci: Add level support via extended registers 2026-09-23 12:56 [PATCH 0/2] gpio: davinci: Add support for level-triggered interrupts Aniket Limaye 2026-09-23 12:56 ` [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support Aniket Limaye @ 2026-09-23 12:56 ` Aniket Limaye 2026-09-23 13:08 ` sashiko-bot 2026-09-28 12:47 ` Bartosz Golaszewski 1 sibling, 2 replies; 7+ messages in thread From: Aniket Limaye @ 2026-09-23 12:56 UTC (permalink / raw) To: Keerthy, Linus Walleij, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel, 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. Signed-off-by: Aniket Limaye <a-limaye@ti.com> --- drivers/gpio/gpio-davinci.c | 103 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 6 deletions(-) diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c index cccbaea1dec2..e5a3abe6b9d3 100644 --- a/drivers/gpio/gpio-davinci.c +++ b/drivers/gpio/gpio-davinci.c @@ -39,12 +39,20 @@ 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); #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; @@ -58,9 +66,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; }; @@ -168,6 +178,7 @@ 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; + bool ext_reg = false; /* * The gpio banks conceptually expose a segmented bitmap, @@ -190,6 +201,9 @@ 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"); + if (device_is_compatible(dev, "ti,tda54-gpio")) + ext_reg = true; + if (gpio_unbanked) nirq = gpio_unbanked; else @@ -235,8 +249,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) @@ -267,10 +284,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); } @@ -280,12 +302,14 @@ 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); + unsigned int ext_status = g_ext ? (IRQ_TYPE_LEVEL_MASK) : 0; gpiochip_enable_irq(&chips->chip, hwirq); - status &= IRQ_TYPE_EDGE_BOTH; + status &= IRQ_TYPE_EDGE_BOTH | ext_status; if (!status) status = IRQ_TYPE_EDGE_BOTH; @@ -293,6 +317,18 @@ 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 int gpio_irq_type_ext(struct irq_data *d, unsigned int trigger) +{ + if (trigger & ~(IRQ_TYPE_EDGE_BOTH | IRQ_TYPE_LEVEL_MASK)) + return -EINVAL; + + return 0; } static int gpio_irq_type(struct irq_data *d, unsigned trigger) @@ -303,6 +339,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, @@ -387,10 +432,13 @@ 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; + u32 ext_trigger; 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; @@ -400,13 +448,21 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger) mask = __gpio_mask(i); - if (trigger & ~IRQ_TYPE_EDGE_BOTH) + ext_trigger = g_ext ? (IRQ_TYPE_LEVEL_MASK) : 0; + + if (trigger & ~(IRQ_TYPE_EDGE_BOTH | ext_trigger)) 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 (ext_trigger) { + 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; } @@ -418,8 +474,12 @@ davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq, struct davinci_gpio_controller *chips = (struct davinci_gpio_controller *)d->host_data; - irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq, - "davinci_gpio"); + if (chips->regs_ext[0]) + irq_set_chip_and_handler_name(irq, &gpio_irqchip_ext, handle_simple_irq, + "davinci_gpio"); + else + irq_set_chip_and_handler_name(irq, &gpio_irqchip, 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)); @@ -469,6 +529,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; @@ -495,9 +556,9 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) dev_err(dev, "Couldn't allocate IRQ numbers\n"); return irq; } - irq_domain = irq_domain_create_legacy(dev_fwnode(dev), ngpio, irq, 0, &davinci_gpio_irq_ops, chips); + if (!irq_domain) { dev_err(dev, "Couldn't register an IRQ domain\n"); return -ENODEV; @@ -534,6 +595,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->set_high); + writel_relaxed(~0, &g_ext->set_low); + } /* set the direct IRQs up to use that irqchip */ for (gpio = 0; gpio < chips->gpio_unbanked; gpio++) { @@ -558,6 +624,12 @@ 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 @@ -597,7 +669,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; @@ -611,6 +685,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 */ @@ -621,7 +701,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; @@ -641,6 +723,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); + } } } @@ -671,6 +761,7 @@ 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,tda54-gpio", keystone_gpio_get_irq_chip}, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, davinci_gpio_ids); -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio: davinci: Add level support via extended registers 2026-09-23 12:56 ` [PATCH 2/2] gpio: davinci: Add level support via extended registers Aniket Limaye @ 2026-09-23 13:08 ` sashiko-bot 2026-09-28 12:47 ` Bartosz Golaszewski 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-09-23 13:08 UTC (permalink / raw) To: Aniket Limaye; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] gpio: davinci: interrupt storm due to incorrect default level triggers - [High] gpio: davinci: infinite loop in chained handler for level IRQs -- commit 31955d22d1eb969a680fcf19a5faaeb164ffab12 Author: Aniket Limaye <a-limaye@ti.com> gpio: davinci: Add level support via extended registers This commit adds support for level-triggered interrupts for the TDA54 GPIO controller. It utilizes the extended register sets to allow handling of IRQ_TYPE_LEVEL_HIGH and IRQ_TYPE_LEVEL_LOW triggers. > diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c > index cccbaea1dec24..e5a3abe6b9d3f 100644 > --- a/drivers/gpio/gpio-davinci.c > +++ b/drivers/gpio/gpio-davinci.c [ ... ] > @@ -293,6 +317,18 @@ static void gpio_irq_unmask(struct irq_data *d) [ ... ] > +static int gpio_irq_type_ext(struct irq_data *d, unsigned int trigger) > +{ > + if (trigger & ~(IRQ_TYPE_EDGE_BOTH | IRQ_TYPE_LEVEL_MASK)) > + return -EINVAL; > + > + return 0; > +} [Severity: High] Does this code need to reconfigure the flow handler to handle_level_irq when a level trigger is requested? If a level trigger is requested, we need to assign the proper flow handler using irq_set_handler_locked(). Since this doesn't happen, handle_simple_irq will continue to be used, which does not mask the interrupt line while the action handler runs. If the external line remains asserted during a level-triggered interrupt, could the chained handler (gpio_irq_handler) continuously detect the active status in the intstat register? Since the intstat bit immediately re-latches after being cleared, wouldn't the while (1) loop inside gpio_irq_handler spin infinitely and cause a CPU lockup? [ ... ] > @@ -418,8 +474,12 @@ davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq, > struct davinci_gpio_controller *chips = > (struct davinci_gpio_controller *)d->host_data; > > - irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq, > - "davinci_gpio"); > + if (chips->regs_ext[0]) > + irq_set_chip_and_handler_name(irq, &gpio_irqchip_ext, handle_simple_irq, > + "davinci_gpio"); [Severity: High] Is it safe to hardcode handle_simple_irq as the default handler for all banked GPIO interrupts here, when level interrupts are supported? As mentioned above in gpio_irq_type_ext(), using handle_simple_irq for level interrupts can lead to an infinite loop in the chained handler because it does not mask the line. [ ... ] > @@ -534,6 +595,11 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) [ ... ] > /* default trigger: both edges */ > 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->set_high); > + writel_relaxed(~0, &g_ext->set_low); > + } [Severity: High] Will this unconditionally enable level interrupts for all unbanked GPIOs during controller setup? The code writes ~0 to both set_high and set_low registers, which enables level-triggered interrupts for both high and low states on all unbanked pins simultaneously. Since a digital line must be either high or low, wouldn't this guarantee the interrupt condition is continuously met? If an unbanked GPIO interrupt is then unmasked without explicitly overriding the trigger type (for example, relying on the default 'both edges' state), wouldn't this trigger an unrecoverable interrupt storm? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260923-upstream-tda54-gpio-v1-0-d1a0a199c836@ti.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio: davinci: Add level support via extended registers 2026-09-23 12:56 ` [PATCH 2/2] gpio: davinci: Add level support via extended registers Aniket Limaye 2026-09-23 13:08 ` sashiko-bot @ 2026-09-28 12:47 ` Bartosz Golaszewski 1 sibling, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-09-28 12:47 UTC (permalink / raw) To: Aniket Limaye Cc: Keerthy, Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley, vigneshr, u-kumar1, nm, linux-gpio, devicetree, linux-kernel On Wed, Sep 23, 2026 at 2:58 PM Aniket Limaye <a-limaye@ti.com> wrote: > > Add support for level-triggered interrupts (IRQ_TYPE_LEVEL_HIGH and > IRQ_TYPE_LEVEL_LOW) for TDA54 GPIO controller via extended register sets. > > Signed-off-by: Aniket Limaye <a-limaye@ti.com> > --- > drivers/gpio/gpio-davinci.c | 103 +++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 97 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c > index cccbaea1dec2..e5a3abe6b9d3 100644 > --- a/drivers/gpio/gpio-davinci.c > +++ b/drivers/gpio/gpio-davinci.c > @@ -39,12 +39,20 @@ 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); > > #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; > @@ -58,9 +66,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; > }; > > @@ -168,6 +178,7 @@ 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; > + bool ext_reg = false; > > /* > * The gpio banks conceptually expose a segmented bitmap, > @@ -190,6 +201,9 @@ 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"); > > + if (device_is_compatible(dev, "ti,tda54-gpio")) > + ext_reg = true; > + Don't do it this way please. Instead: split this patch into three: 1. Add a new match data struct and move the get_irqchip() callback into it instead of passing it directly via the of_device_id field. 2. Add support for level interrupts and extend the match data struct with the ext_reg flag. 3. Add the new compatible with its dedicated match data entry. Bart ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-28 12:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-23 12:56 [PATCH 0/2] gpio: davinci: Add support for level-triggered interrupts Aniket Limaye 2026-09-23 12:56 ` [PATCH 1/2] dt-bindings: gpio: gpio-davinci: Add the tda54 support Aniket Limaye 2026-09-23 13:57 ` Krzysztof Kozlowski 2026-09-28 12:28 ` Bartosz Golaszewski 2026-09-23 12:56 ` [PATCH 2/2] gpio: davinci: Add level support via extended registers Aniket Limaye 2026-09-23 13:08 ` sashiko-bot 2026-09-28 12:47 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox