Re: [PATCH 5/5] pinctrl: sunxi: a523: add missing IRQ bank (plus old DT workaround)

[Andre Przywara <andre.przywara@arm.com>]

Hi Chen-Yu,

many thanks for having a look!

On 3/23/26 18:41, Chen-Yu Tsai wrote:
> On Mon, Mar 23, 2026 at 7:02 PM Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> The Allwinner A532 SoC implements 10 GPIO banks, each of which is
>> interrupt capable. However the first bank (PortA) is skipped, so the
>> indicies of those banks range from 1 to 10, not 0 to 9.
>> We described the skipped bank correctly, but missed that for the IRQ
>> banks, where we rely on the IRQ bank index to be aligned with the MMIO
>> register offset, starting at 0x200.
>>
>> Correct that by increasing the number of IRQ banks to 11, to cover both
>> the first skipped one, but also the last one (PortK). This fixes a bug
>> where the interrupt numbers would be off-by-one, due to that
>> mis-enumeration.
>> The big caveat is that now old DTs break the kernel, since they only
>> provide 10 interrupts, and the driver bails out entirely due to the last
>> missing one. So add a workaround for this particular case, where we
>> detect the requirement for 11 banks, but only 10 interrupts provided,
>> and continue with 10 IRQs, albeit emitting a warning about a DT update.
>> This would still be broken in terms of interrupt assignment, but it was
>> broken the whole time before, so it's not a regression.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>   drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c |  2 +-
>>   drivers/pinctrl/sunxi/pinctrl-sunxi.c       | 22 +++++++++++++--------
>>   2 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
>> index b6f78f1f30ac..a1d157de53d2 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sun55i-a523.c
>> @@ -17,7 +17,7 @@ static const u8 a523_nr_bank_pins[SUNXI_PINCTRL_MAX_BANKS] =
>>   /*       PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
>>          {  0, 15, 17, 24, 16,  7, 15, 20, 17, 28, 24 };
>>
>> -static const unsigned int a523_irq_bank_map[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
>> +static const unsigned int a523_irq_bank_map[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
> 
> Actually you don't even need this, since this is a linear mapping.
> 
>  From sunxi_irq_hw_bank_num():
> 
>      if (!desc->irq_bank_map)
>              return bank;
>      else
>              return desc->irq_bank_map[bank];

Yeah, I was wondering about that as well, and there are other cases were 
we wouldn't need the map, espeically in the PRMC pinctrl instances. I 
might add a separate patch for that.

>>   static const u8 a523_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
>>   /*       PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index 6a86b7989b25..ffee79397590 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/irqdomain.h>
>>   #include <linux/of.h>
>>   #include <linux/of_clk.h>
>> +#include <linux/of_irq.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/regulator/consumer.h>
>>   #include <linux/slab.h>
>> @@ -1582,6 +1583,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>>          struct sunxi_pinctrl *pctl;
>>          struct pinmux_ops *pmxops;
>>          int i, ret, last_pin, pin_idx;
>> +       int num_irq_banks;
>>          struct clk *clk;
>>
>>          pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
>> @@ -1715,16 +1717,20 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>>                  goto gpiochip_error;
>>          }
>>
>> -       pctl->irq = devm_kcalloc(&pdev->dev,
>> -                                pctl->desc->irq_banks,
>> -                                sizeof(*pctl->irq),
>> -                                GFP_KERNEL);
>> +       num_irq_banks = pctl->desc->irq_banks;
>> +       /* Workaround for old A523 DT, exposing one less interrupt. */
>> +       if (num_irq_banks == 11 && of_irq_count(node) < 11) {
>> +               num_irq_banks = 10;
>> +               pr_warn("Not enough PIO interrupts, please update your DT!\n");
>> +       }
> 
> I would probably make the check universal, and also use dev_warn().
> 
>      num_irq_banks = of_irq_count(node);
>      if (num_irq_banks != pctrl->desc->irq_banks) {
>          dev_warn(&pdev->dev, "Incorrect number of PIO interrupts,
> please update your DT!\n");
>          num_irq_banks = min(num_irq_banks, pctrl->desc->irq_banks);
>      }

Ah, nice one, that's of course much better. But I see that there is 
other code using desc->irq_banks, and if the array allocation is 
different, that will not end well. I will check how we can use 
num_irq_banks there as well.

Thanks!
Andre



> 
> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
> 
> 
>> +       pctl->irq = devm_kcalloc(&pdev->dev, num_irq_banks,
>> +                                sizeof(*pctl->irq), GFP_KERNEL);
>>          if (!pctl->irq) {
>>                  ret = -ENOMEM;
>>                  goto gpiochip_error;
>>          }
>>
>> -       for (i = 0; i < pctl->desc->irq_banks; i++) {
>> +       for (i = 0; i < num_irq_banks; i++) {
>>                  pctl->irq[i] = platform_get_irq(pdev, i);
>>                  if (pctl->irq[i] < 0) {
>>                          ret = pctl->irq[i];
>> @@ -1733,7 +1739,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>>          }
>>
>>          pctl->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev),
>> -                                               pctl->desc->irq_banks * IRQ_PER_BANK,
>> +                                               num_irq_banks * IRQ_PER_BANK,
>>                                                  &sunxi_pinctrl_irq_domain_ops, pctl);
>>          if (!pctl->domain) {
>>                  dev_err(&pdev->dev, "Couldn't register IRQ domain\n");
>> @@ -1741,7 +1747,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>>                  goto gpiochip_error;
>>          }
>>
>> -       for (i = 0; i < (pctl->desc->irq_banks * IRQ_PER_BANK); i++) {
>> +       for (i = 0; i < (num_irq_banks * IRQ_PER_BANK); i++) {
>>                  int irqno = irq_create_mapping(pctl->domain, i);
>>
>>                  irq_set_lockdep_class(irqno, &sunxi_pinctrl_irq_lock_class,
>> @@ -1751,7 +1757,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>>                  irq_set_chip_data(irqno, pctl);
>>          }
>>
>> -       for (i = 0; i < pctl->desc->irq_banks; i++) {
>> +       for (i = 0; i < num_irq_banks; i++) {
>>                  /* Mask and clear all IRQs before registering a handler */
>>                  writel(0, pctl->membase +
>>                            sunxi_irq_ctrl_reg_from_bank(pctl->desc, i));
>> --
>> 2.43.0
>>
>>