[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ec41f61f-d500-4dda-8a79-37a68ddafced@windriver.com>
Date: Sun, 16 Jun 2024 09:15:05 +0800
From: xiaolei wang <xiaolei.wang@...driver.com>
To: Simon Horman <horms@...nel.org>
Cc: olteanv@...il.com, linux@...linux.org.uk, alexandre.torgue@...s.st.com,
andrew@...n.ch, joabreu@...opsys.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
mcoquelin.stm32@...il.com, wojciech.drewek@...el.com,
netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [net PATCH] net: stmmac: No need to calculate speed divider when
offload is disabled
On 6/15/24 22:47, Simon Horman wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> 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 <xiaolei.wang@...driver.com>
>> ---
>> .../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.
Oh, when deleting the configuration, idleslope and sendslope are both 0,
do you mean we also need to set ptr to 0?
thanks
xiaolei
>
> Flagged by Smatch.
>
> --
> pw-bot: changes-requested
Powered by blists - more mailing lists