Messages in this thread | | | Date | Tue, 14 Nov 2023 21:53:38 +0200 | Subject | Re: [PATCH net-next 3/3] net: ethernet: am65-cpsw: Error out if Enable TX/RX channel fails | From | Roger Quadros <> |
| |
On 14/11/2023 14:07, Vladimir Oltean wrote: > On Mon, Nov 13, 2023 at 01:07:08PM +0200, Roger Quadros wrote: >> k3_udma_glue_enable_rx/tx_chn returns error code on failure. >> Bail out on error while enabling TX/RX channel. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 33 +++++++++++++++++++----- >> 1 file changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> index 7c440899c93c..340f25bf33b1 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> @@ -372,7 +372,7 @@ static void am65_cpsw_init_port_emac_ale(struct am65_cpsw_port *port); >> static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common) >> { >> struct am65_cpsw_host *host_p = am65_common_get_host(common); >> - int port_idx, i, ret; >> + int port_idx, i, ret, tx; >> struct sk_buff *skb; >> u32 val, port_mask; >> >> @@ -453,13 +453,22 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common) >> } >> kmemleak_not_leak(skb); >> } >> - k3_udma_glue_enable_rx_chn(common->rx_chns.rx_chn); >> >> - for (i = 0; i < common->tx_ch_num; i++) { >> - ret = k3_udma_glue_enable_tx_chn(common->tx_chns[i].tx_chn); >> - if (ret) >> - return ret; > > Can you comment on the kmemleak_not_leak(skb) call above, and its > relationship to the pre-existing error handling path in am65_cpsw_nuss_common_open()?
I am not aware why it was added. It sure looks odd and I'll get rid of it and add the necessary error handling.
> I see that the dev_kfree_skb_any() call is being made from am65_cpsw_nuss_rx_cleanup(), > which is only called from am65_cpsw_nuss_common_stop(). > > So if there are errors during am65_cpsw_nuss_common_open() and > descriptors have already been added to the RX DMA channel, they will not > be removed either from hardware or from software. How does that work?
I believe this is a gap and I will address it in the next revision. Thanks!
-- cheers, -roger
| |