Re: [PATCH net-next v3 4/4] net: airoha: Add the capability to allocate hfwd descriptors in SRAM

[Simon Horman <horms@kernel.org>]

On Thu, May 22, 2025 at 02:58:56PM +0200, Lorenzo Bianconi wrote:
> > On Wed, May 21, 2025 at 09:16:39AM +0200, Lorenzo Bianconi wrote:
> > > In order to improve packet processing and packet forwarding
> > > performances, EN7581 SoC supports consuming SRAM instead of DRAM for
> > > hw forwarding descriptors queue.
> > > For downlink hw accelerated traffic request to consume SRAM memory
> > > for hw forwarding descriptors queue.
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > >  drivers/net/ethernet/airoha/airoha_eth.c | 11 +----------
> > >  drivers/net/ethernet/airoha/airoha_eth.h |  9 +++++++++
> > >  drivers/net/ethernet/airoha/airoha_ppe.c |  6 ++++++
> > >  3 files changed, 16 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > > index 20e590d76735e72a1a538a42d2a1f49b882deccc..3cd56de716a5269b1530cff6d0ca3414d92ecb69 100644
> > > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > > @@ -71,15 +71,6 @@ static void airoha_qdma_irq_disable(struct airoha_irq_bank *irq_bank,
> > >  	airoha_qdma_set_irqmask(irq_bank, index, mask, 0);
> > >  }
> > >  
> > > -static bool airhoa_is_lan_gdm_port(struct airoha_gdm_port *port)
> > > -{
> > > -	/* GDM1 port on EN7581 SoC is connected to the lan dsa switch.
> > > -	 * GDM{2,3,4} can be used as wan port connected to an external
> > > -	 * phy module.
> > > -	 */
> > > -	return port->id == 1;
> > > -}
> > > -
> > >  static void airoha_set_macaddr(struct airoha_gdm_port *port, const u8 *addr)
> > >  {
> > >  	struct airoha_eth *eth = port->qdma->eth;
> > > @@ -1128,7 +1119,7 @@ static int airoha_qdma_init_hfwd_queues(struct airoha_qdma *qdma)
> > >  			LMGR_INIT_START | LMGR_SRAM_MODE_MASK |
> > >  			HW_FWD_DESC_NUM_MASK,
> > >  			FIELD_PREP(HW_FWD_DESC_NUM_MASK, HW_DSCP_NUM) |
> > > -			LMGR_INIT_START);
> > > +			LMGR_INIT_START | LMGR_SRAM_MODE_MASK);
> > 
> > Hi Lorenzo,
> 
> Hi Simon,
> 
> > 
> > I'm wondering if setting the LMGR_SRAM_MODE_MASK bit (maybe a different
> > name for the #define would be nice) is dependent on the SRAM region
> 
> I did this way because LMGR_SRAM_MODE_MASK is just a bit. Do you prefer
> to do something like:
> 
> FIELD_PREP(LMGR_SRAM_MODE_MASK, 1)?

Let's leave it as you have it in this patch :)

> 
> > being described in DT, as per code added above this line to this
> > function by the previous patch in this series.
> 
> Are you referring to qdma0_buf/qdma1_buf memory regions?
> https://patchwork.kernel.org/project/netdevbpf/patch/20250521-airopha-desc-sram-v3-1-a6e9b085b4f0@kernel.org/
> 
> If so, they are DRAM memory-regions and not SRAM ones. They are used for
> hw forwarding buffers queue. SRAM is used for hw forwarding descriptor queue.

Yes, my mistake.

With that cleared up I think this patch looks good.

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