Re: [PATCH net-next v3 1/6] net: ethernet: ti: am65-cpsw: Introduce multi queue Rx
From: Roger Quadros <rogerq@kernel.org>
Date: 2024-07-08 19:42:14
Also in:
linux-omap, lkml
On 06/07/2024 04:15, Jakub Kicinski wrote:
On Wed, 03 Jul 2024 16:51:32 +0300 Roger Quadros wrote:quoted
- if (queue >= AM65_CPSW_MAX_TX_QUEUES) + if (queue >= AM65_CPSW_MAX_TX_QUEUES && + queue >= AM65_CPSW_MAX_RX_QUEUES) return -EINVAL;both MAXes are 8, the else conditions below are dead code Same for set
yes. Maybe I should just use one define for both? e.g. AM65_CPSW_MAX_QUEUES.
quoted
- tx_chn = &common->tx_chns[queue]; + if (queue < AM65_CPSW_MAX_TX_QUEUES) { + tx_chn = &common->tx_chns[queue]; + coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000; + } else { + coal->tx_coalesce_usecs = ~0; + } - coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000; + if (queue < AM65_CPSW_MAX_RX_QUEUES) { + rx_flow = &common->rx_chns.flows[queue]; + coal->rx_coalesce_usecs = rx_flow->rx_pace_timeout / 1000; + } else { + coal->rx_coalesce_usecs = ~0; + }+ for (flow_idx = 0; flow_idx < common->rx_ch_num_flows; flow_idx++) { + flow = &rx_chn->flows[flow_idx]; + for (i = 0; i < AM65_CPSW_MAX_RX_DESC; i++) { + page = page_pool_dev_alloc_pages(flow->page_pool); + if (!page) { + dev_err(common->dev, "cannot allocate page in flow %d\n", + flow_idx); + ret = -ENOMEM; + if (i) + goto fail_rx; - return ret; - } - rx_chn->pages[i] = page; + return ret; the direct returns now that it's a double-nested loop seem questionable, don't you have to goto fail_rx?
Good catch. I should just drop the "if (i)" and goto fail_rx regardless. -- cheers, -roger