Re: [PATCH v2 1/3] pinctrl: sunxi: dt: number the IRQ banks from the first one that has IRQs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: sunxi: sunxi_irq_base_reg() ignores irq_bank_map on A733
--

commit 1d99efa95c547fd409284e1974176bcf35345f69
Author: Pablo Mazzini <pmazzini@gmail.com>

pinctrl: sunxi: dt: number the IRQ banks from the first one that has IRQs

This commit updates prepare_function_table() to number logical irq banks
starting from the first bank that supports interrupts rather than bank 0. It
also updates the irq_bank_map tables for the a523 and a733 pinctrl drivers to
map logical irq banks 0 through 9 to hardware banks 1 through 10.

> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> index ca82f08a3a65b..3ebe926a1bc6b 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun60i-a733.c
> @@ -21,8 +21,11 @@ static const u8 a733_irq_bank_muxes[SUNXI_PINCTRL_MAX_BANKS] =
>  /*	  PA  PB  PC  PD  PE  PF  PG  PH  PI  PJ  PK */
>  	{  0, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14};
>  
> +static const unsigned int a733_irq_bank_map[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
> +
>  static struct sunxi_pinctrl_desc a733_pinctrl_data = {
> -	.irq_banks = 10,
> +	.irq_banks = ARRAY_SIZE(a733_irq_bank_map),
> +	.irq_bank_map = a733_irq_bank_map,

[Severity: High]
Does sunxi_irq_base_reg() actually consult irq_bank_map on the a733?

With the change in prepare_function_table(), ports b through k on the a733 are
now assigned logical irqbank values 0 through 9 instead of 1 through 10, and
a733_irq_bank_map is added here to map those logical banks back to hardware
banks 1 through 10.

However, a733_pinctrl_probe() sets SUNXI_PINCTRL_NCAT3_REG_LAYOUT, and
sunxi_irq_base_reg() in drivers/pinctrl/sunxi/pinctrl-sunxi.h handles that
layout without calling sunxi_irq_hw_bank_num():

static inline u32 sunxi_irq_base_reg(const struct sunxi_pinctrl *pctl, u16 bank)
{
	if (pctl->flags & SUNXI_PINCTRL_NCAT3_REG_LAYOUT)
		return pctl->bank_offset + bank * pctl->bank_mem_size +
		       A733_IRQ_REGS_OFFSET;

	return IRQ_REGS_OFFSET +
	       sunxi_irq_hw_bank_num(pctl->desc, bank) * IRQ_MEM_SIZE;
}

Because the SUNXI_PINCTRL_NCAT3_REG_LAYOUT branch multiplies
pctl->bank_mem_size by the raw logical bank index instead of
sunxi_irq_hw_bank_num(pctl->desc, bank), won't all gpio interrupt register
accesses via sunxi_irq_cfg_reg(), sunxi_irq_ctrl_reg(),
sunxi_irq_status_reg(), and sunxi_irq_deb_reg() on the a733 ignore
a733_irq_bank_map and access hardware banks 0 through 9 (ports a through j)
instead of banks 1 through 10 (ports b through k)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025135745.102970-1-pmazzini@gmail.com?part=1