Re: [PATCH 2/2] gpio: davinci: Add level support via extended registers

[Aniket Limaye]

On 28/09/26 18:17, Bartosz Golaszewski wrote:
> On Wed, Sep 23, 2026 at 2:58 PM Aniket Limaye <a-limaye@xxxxxx> 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@xxxxxx>
> > ---
> >   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

Thanks for your feedback,

Sent v2 here:
https://lore.kernel.org/all/20261016-upstream-tda54-gpio-v2-0-5c4f8b10cdbc@xxxxxx/

Regards,
Aniket