Re: [net] net: airoha: enable RX_DONE interrupt for RX queue 31

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix: the patch only adds BIT(31) to
  RX_IRQ1_BANK_PIN_MASK, so the exact defect the commit message…

--

net: airoha: enable RX_DONE interrupt for RX queue 31

This adds BIT(31) to RX_IRQ1_BANK_PIN_MASK so that RX queue 31's RX_DONE
interrupt is routed to IRQ bank 1.  The stated goal is that the queue gets
drained and its buffers returned to the page pool.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index fa9a8edce22f..8277c1c87bb3 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -538,7 +538,7 @@ struct airoha_wdma_info {
>  
>  /* RX queue to IRQ mapping: BIT(q) in IRQ(n) */
>  #define RX_IRQ0_BANK_PIN_MASK			0x839f
> -#define RX_IRQ1_BANK_PIN_MASK			0x7fe00000
> +#define RX_IRQ1_BANK_PIN_MASK			0xffe00000
>  #define RX_IRQ2_BANK_PIN_MASK			0x20
>  #define RX_IRQ3_BANK_PIN_MASK			0x40

[Severity: High]
Does this leave the same problem in place for the other RX rings?  After the
change the union of the four bank masks is:

  0x839f | 0xffe00000 | 0x20 | 0x40 = 0xffe083ff

so rings 0-9, 15 and 21-31 are covered, but bits 10-14 and 16-20 are still
clear in every bank mask.

Those rings are still allocated and filled, because airoha_qdma_init_rx() in
drivers/net/ethernet/airoha/airoha_eth.c gates only on RX_DONE_INT_MASK:

	if (!(RX_DONE_INT_MASK & BIT(i))) {
		/* rx-queue not binded to irq */
		continue;
	}

	err = airoha_qdma_init_rx_queue(&qdma->q_rx[i], qdma,
					RX_DSCP_NUM(i));

and in drivers/net/ethernet/airoha/airoha_regs.h RX_DONE_INT_MASK expands to
0xffffffff:

	#define RX_DONE_HIGH_OFFSET	fls(RX_DONE_HIGH_INT_MASK)
	#define RX_DONE_INT_MASK	\
		((RX_DONE_HIGH_INT_MASK << RX_DONE_HIGH_OFFSET) | RX_DONE_LOW_INT_MASK)

with RX_DONE_HIGH_INT_MASK and RX_DONE_LOW_INT_MASK both 0xffff.  So the
"rx-queue not binded to irq" comment filters nothing and all 32 rings get
programmed into hardware and filled from the page pool.

Meanwhile airoha_qdma_hw_init() only unmasks RX_DONE according to the pin
masks:

	airoha_qdma_irq_enable(&qdma->irq_banks[i], QDMA_INT_REG_IDX1,
			       INT_RX1_MASK(RX_IRQ_BANK_PIN_MASK(i)));
	airoha_qdma_irq_enable(&qdma->irq_banks[i], QDMA_INT_REG_IDX2,
			       INT_RX2_MASK(RX_IRQ_BANK_PIN_MASK(i)));

and airoha_qdma_rx_napi_poll() re-arms through the same mask:

	for (i = 0; i < ARRAY_SIZE(qdma->irq_banks); i++) {
		if (!(BIT(qid) & RX_IRQ_BANK_PIN_MASK(i)))
			continue;

Since rx_intr_mask in airoha_irq_handler() comes from the per-bank irqmask,
rings 10-14 and 16-20 are never passed to napi_schedule(), so
airoha_qdma_rx_process() never runs for them and their buffers stay pinned
from probe until airoha_qdma_cleanup_rx_queue().  Is that not exactly the
behaviour the commit message describes as a bug for ring 31?

Ring 11 stands out here, since RX_DSCP_NUM() gives it 128 descriptors, the
same as rings 2 and 15 which are mapped to bank 0:

	#define RX_DSCP_NUM(_n)			\
		((_n) ==  2 ? 128 :		\
		 (_n) == 11 ? 128 :		\
		 (_n) == 15 ? 128 :		\
		 (_n) ==  0 ? 1024 : 16)

while BIT(11) is absent from all pin masks, and the ring this patch enables
(31) uses the default 16 descriptors.

Would it make sense to give airoha_qdma_init_rx() and the pin masks a single
source of truth, either by having the masks cover every ring init_rx arms, or
by having init_rx key off RX_IRQ_BANK_PIN_MASK()?

Alternatively, if the sparse mapping is deliberate and those rings are simply
unused by the hardware configuration, does the justification given for
BIT(31) still hold?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260830-airoha-rxdone-rxq31-v1-1-830a91503f2f%40kernel.org