From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80E0BC4167B for ; Tue, 14 Nov 2023 19:53:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232285AbjKNTxt (ORCPT ); Tue, 14 Nov 2023 14:53:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35962 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231894AbjKNTxr (ORCPT ); Tue, 14 Nov 2023 14:53:47 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C7320EB for ; Tue, 14 Nov 2023 11:53:44 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D57ADC433C8; Tue, 14 Nov 2023 19:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1699991624; bh=b44Cg1Nm3xRweiZaKLR7rcsJE+tAGmLA0GlDnpXQrKM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=tcjngx+s61YLRVW7hOdBOnRXDymSRjH1ZyDHlGLBT6VwZ/G9WKzWYBJGu+2UsCfuT e4X9prUO6sNFrC2Rm8EwCeg0dXmqpoPkNXlMeDSe7/L4HQe5T3XqDAOwzc/g1lJvgT MqunrdaHvQGP/Q8YgcTAiCqSZMcSuWcQn/0YeTRooBLXZqD/dEdKcXnP0pk97Bsfua Ri3D+GXDykN5TgftzB55VrtkpcEusiPwzH2rwn5JxfSOZeRI6WGCSMuJaNwkFbsK8j AETeR1MvzJ2WCFvbgu9xxIfitY8Sy1g36aHNjoWUzIuvvY7moSNI7noDtL9qB7EPwY WiDqKhLmA89BQ== Message-ID: <7146a57c-fcf3-4330-a760-5c163fc8796c@kernel.org> Date: Tue, 14 Nov 2023 21:53:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next 3/3] net: ethernet: am65-cpsw: Error out if Enable TX/RX channel fails To: Vladimir Oltean Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, s-vadapalli@ti.com, r-gunasekaran@ti.com, srk@ti.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20231113110708.137379-1-rogerq@kernel.org> <20231113110708.137379-4-rogerq@kernel.org> <20231114120758.kz5n3x4fawhcegqi@skbuf> Content-Language: en-US From: Roger Quadros In-Reply-To: <20231114120758.kz5n3x4fawhcegqi@skbuf> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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