Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: airoha: enable RX_DONE interrupt for RX queue 31
@ 2026-08-30 12:28 Lorenzo Bianconi
  2026-09-02 15:29 ` [net] " netdev-bot+sashiko
  2026-09-03  9:50 ` [PATCH net] " patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: Lorenzo Bianconi @ 2026-08-30 12:28 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lorenzo Bianconi
  Cc: linux-arm-kernel, linux-mediatek, netdev

RX queue 31 has always been allocated and filled by airoha_qdma_init_rx()
since RX_DONE_INT_MASK spans queues 0-31, but none of the RX_IRQ*
_BANK_PIN_MASK values covered BIT(31). As a consequence the RX_DONE
interrupt for queue 31 was never enabled, airoha_qdma_rx_process() never
ran on that queue and its buffers were never reaped.

Route RX queue 31's RX_DONE interrupt to IRQ bank 1 so that the queue
is drained and its buffers returned to the page pool.

Fixes: f252493e1835 ("net: airoha: Enable multiple IRQ lines support in airoha_eth driver.")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
 #define RX_IRQ_BANK_PIN_MASK(_n)		\

---
base-commit: 2188569e7e1b0bc3f3b557dc97ab7a02befc11c8
change-id: 20260830-airoha-rxdone-rxq31-21dc1fac2174

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [net] net: airoha: enable RX_DONE interrupt for RX queue 31
  2026-08-30 12:28 [PATCH net] net: airoha: enable RX_DONE interrupt for RX queue 31 Lorenzo Bianconi
@ 2026-09-02 15:29 ` netdev-bot+sashiko
  2026-09-02 22:19   ` Lorenzo Bianconi
  2026-09-03  9:50 ` [PATCH net] " patchwork-bot+netdevbpf
  1 sibling, 1 reply; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-02 15:29 UTC (permalink / raw)
  To: lorenzo
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-arm-kernel,
	linux-mediatek, netdev

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [net] net: airoha: enable RX_DONE interrupt for RX queue 31
  2026-09-02 15:29 ` [net] " netdev-bot+sashiko
@ 2026-09-02 22:19   ` Lorenzo Bianconi
  2026-09-03  9:00     ` Simon Horman
  0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2026-09-02 22:19 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-arm-kernel,
	linux-mediatek, netdev

[-- Attachment #1: Type: text/plain, Size: 4359 bytes --]

> 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.

Here I wanted just to align to the vendor sdk and enable rx queues 24-31 (31
was missing) in preparation for HW GRO support. The other rx queues are
out-of-scope.

Regards,
Lorenzo

> 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [net] net: airoha: enable RX_DONE interrupt for RX queue 31
  2026-09-02 22:19   ` Lorenzo Bianconi
@ 2026-09-03  9:00     ` Simon Horman
  0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2026-09-03  9:00 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev-bot+sashiko, andrew+netdev, davem, edumazet, kuba, pabeni,
	linux-arm-kernel, linux-mediatek, netdev

On Thu, Sep 03, 2026 at 12:19:25AM +0200, Lorenzo Bianconi wrote:
> > 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.
> 
> Here I wanted just to align to the vendor sdk and enable rx queues 24-31 (31
> was missing) in preparation for HW GRO support. The other rx queues are
> out-of-scope.

Thanks Lorenzo,

Seems reasonable to me.

Reviewed-by: Simon Horman <horms@kernel.org>




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net: airoha: enable RX_DONE interrupt for RX queue 31
  2026-08-30 12:28 [PATCH net] net: airoha: enable RX_DONE interrupt for RX queue 31 Lorenzo Bianconi
  2026-09-02 15:29 ` [net] " netdev-bot+sashiko
@ 2026-09-03  9:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-03  9:50 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-arm-kernel,
	linux-mediatek, netdev

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun, 30 Aug 2026 14:28:27 +0200 you wrote:
> RX queue 31 has always been allocated and filled by airoha_qdma_init_rx()
> since RX_DONE_INT_MASK spans queues 0-31, but none of the RX_IRQ*
> _BANK_PIN_MASK values covered BIT(31). As a consequence the RX_DONE
> interrupt for queue 31 was never enabled, airoha_qdma_rx_process() never
> ran on that queue and its buffers were never reaped.
> 
> Route RX queue 31's RX_DONE interrupt to IRQ bank 1 so that the queue
> is drained and its buffers returned to the page pool.
> 
> [...]

Here is the summary with links:
  - [net] net: airoha: enable RX_DONE interrupt for RX queue 31
    https://git.kernel.org/netdev/net/c/7db28abbea0f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-03  9:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-30 12:28 [PATCH net] net: airoha: enable RX_DONE interrupt for RX queue 31 Lorenzo Bianconi
2026-09-02 15:29 ` [net] " netdev-bot+sashiko
2026-09-02 22:19   ` Lorenzo Bianconi
2026-09-03  9:00     ` Simon Horman
2026-09-03  9:50 ` [PATCH net] " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox