Re: [PATCH net v3 1/2] net: ethernet: ti: am65-cpsw: Fix multi queue Rx on J7

[Simon Horman]

On Fri, Nov 01, 2024 at 12:18:50PM +0200, Roger Quadros wrote:
> On J7 platforms, setting up multiple RX flows was failing
> as the RX free descriptor ring 0 is shared among all flows
> and we did not allocate enough elements in the RX free descriptor
> ring 0 to accommodate for all RX flows.
> 
> This issue is not present on AM62 as separate pair of
> rings are used for free and completion rings for each flow.
> 
> Fix this by allocating enough elements for RX free descriptor
> ring 0.
> 
> However, we can no longer rely on desc_idx (descriptor based
> offsets) to identify the pages in the respective flows as
> free descriptor ring includes elements for all flows.
> To solve this, introduce a new swdata data structure to store
> flow_id and page. This can be used to identify which flow (page_pool)
> and page the descriptor belonged to when popped out of the
> RX rings.
> 
> Fixes: da70d184a8c3 ("net: ethernet: ti: am65-cpsw: Introduce multi queue Rx")
> Signed-off-by: Roger Quadros <rogerq@xxxxxxxxxx>

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>

> @@ -764,8 +759,8 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common)
>  
>  fail_rx:
>  	for (i = 0; i < common->rx_ch_num_flows; i++)
> -		k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, i, &rx_chn->flows[i],
> -					  am65_cpsw_nuss_rx_cleanup, 0);
> +		k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, i, rx_chn,
> +					  am65_cpsw_nuss_rx_cleanup, !!i);

Hi Roger,

I wonder if, as a follow-up, the skip_fdq (last) parameter of
k3_udma_glue_reset_rx_chn() can be dropped. It seems that all callers
follow the pattern above of passing i as the flow_num (2nd) argument,
and !!i as the skip_fdq argument. If so, k3_udma_glue_reset_rx_chn could
simply derive skip_fdq as !!flow_num.