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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 12053C27C4F for ; Sat, 15 Jun 2024 14:48:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0/DeHs+zBAffnlQYjo6QqlU0bYkI8NsUySUnEt8HXjo=; b=I4RNzBE4IYKxYaSCnufZOalcr0 TUeApb7ViYpH+Ot4tZXiqMVL+HjIUrw+/EjwQZsRQUL5Lw0MDrYJW/W6AXDiXrx+T2iCbM1mV+eTs ieB9Uce5FmbJXC8WOOXLftHdxEIqE8y6znXtYMSwcpSfOFtp2O5m4zyXqAFVIpJcq5m6/94QGE8FQ VR1zdnVDLyFQSVafFqDOiEumGD0xLfqxCLbN38tF8v4wmD0+KQ+SZ3Qn5bDgIaZvesVRUwOkxybCJ R9LxOQBmnlvtQmP+wl34P8f0VPh2OB6019AXYZn6AB+5H+VA3IUbaSTAP5E+6S3LSJGnFH40ZVuZG 3G22k3Aw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sIUhB-00000005Zx3-3jsL; Sat, 15 Jun 2024 14:47:57 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sIUh8-00000005Zw7-0kD3 for linux-arm-kernel@lists.infradead.org; Sat, 15 Jun 2024 14:47:56 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id E6F8C60472; Sat, 15 Jun 2024 14:47:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AFDE8C116B1; Sat, 15 Jun 2024 14:47:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1718462872; bh=fCXa3vZakqpvWMT0I15XosFZpQRBqFNMEGngNkHbA1k=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nrMaZFtrG0SE+dgMHmeC2txn+fNLcYtVY2pUD2TcZl7TRUjHU0TYb+o3yh6DkJ7pw zSQV4ur34vd0LzQsuoB/LaTBrIV7v6eHdx/6yo69bIshqm8wBRDUwqhC1fHETNTvKo szhRaETr+J3CBIc4tgFQ4xRIrhvbLv5jjsSoxwexjA8gMZrMDLkF49ahZSpE+5z2AX CXMQqe2UveF+wmfpgK6RTYsPxRJI1z1dJ416dtGJJy6tw1vJW8nrAdhPDe2i9ssgy5 w42PwjIcgUd+NvNYwC72ZCy7QcVw1L4DwlVAwo9E6BSdApFBGOmBpJUm9lGRzyAYEQ h9w7yk7/nNXHg== Date: Sat, 15 Jun 2024 15:47:47 +0100 From: Simon Horman To: Xiaolei Wang Cc: olteanv@gmail.com, linux@armlinux.org.uk, alexandre.torgue@foss.st.com, andrew@lunn.ch, joabreu@synopsys.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, wojciech.drewek@intel.com, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [net PATCH] net: stmmac: No need to calculate speed divider when offload is disabled Message-ID: <20240615144747.GE8447@kernel.org> References: <20240614081916.764761-1-xiaolei.wang@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240614081916.764761-1-xiaolei.wang@windriver.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240615_074754_733771_90C76146 X-CRM114-Status: GOOD ( 20.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jun 14, 2024 at 04:19:16PM +0800, Xiaolei Wang wrote: > commit be27b8965297 ("net: stmmac: replace priv->speed with > the portTransmitRate from the tc-cbs parameters") introduced > a problem. When deleting, it prompts "Invalid portTransmitRate > 0 (idleSlope - sendSlope)" and exits. Add judgment on cbs.enable. > Only when offload is enabled, speed divider needs to be calculated. > > Fixes: be27b8965297 ("net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters") > Signed-off-by: Xiaolei Wang > --- > .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 38 ++++++++++--------- > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > index 1562fbdd0a04..b0fd2d6e525e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > @@ -358,24 +358,26 @@ static int tc_setup_cbs(struct stmmac_priv *priv, > > port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope; > > - /* Port Transmit Rate and Speed Divider */ > - switch (div_s64(port_transmit_rate_kbps, 1000)) { > - case SPEED_10000: > - case SPEED_5000: > - ptr = 32; > - break; > - case SPEED_2500: > - case SPEED_1000: > - ptr = 8; > - break; > - case SPEED_100: > - ptr = 4; > - break; > - default: > - netdev_err(priv->dev, > - "Invalid portTransmitRate %lld (idleSlope - sendSlope)\n", > - port_transmit_rate_kbps); > - return -EINVAL; > + if (qopt->enable) { > + /* Port Transmit Rate and Speed Divider */ > + switch (div_s64(port_transmit_rate_kbps, 1000)) { > + case SPEED_10000: > + case SPEED_5000: > + ptr = 32; > + break; > + case SPEED_2500: > + case SPEED_1000: > + ptr = 8; > + break; > + case SPEED_100: > + ptr = 4; > + break; > + default: > + netdev_err(priv->dev, > + "Invalid portTransmitRate %lld (idleSlope - sendSlope)\n", > + port_transmit_rate_kbps); > + return -EINVAL; > + } > } > mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use; Hi Xiaolei Wang, The code following this function looks like this: if (mode_to_use == MTL_QUEUE_DCB && qopt->enable) { ret = stmmac_dma_qmode(priv, priv->ioaddr, queue, MTL_QUEUE_AVB); if (ret) return ret; priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB; } else if (!qopt->enable) { ret = stmmac_dma_qmode(priv, priv->ioaddr, queue, MTL_QUEUE_DCB); if (ret) return ret; priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB; } /* Final adjustments for HW */ value = div_s64(qopt->idleslope * 1024ll * ptr, port_transmit_rate_kbps); priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0); value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps); priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0); And the div_s64() lines above appear to use ptr uninitialised in the !qopt->enable case. Flagged by Smatch. -- pw-bot: changes-requested