[net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters

Xiaolei Wang posted 1 patch 1 year, 8 months ago
There is a newer version of this series
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
[net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters
Posted by Xiaolei Wang 1 year, 8 months ago
Since the given offload->sendslope only applies to the
current link speed, and userspace may reprogram it when
the link speed changes, don't even bother tracking the
port's link speed, and deduce the port transmit rate
from idleslope - sentslope instead.

Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 222540b55480..48500864017b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -348,6 +348,7 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 	u32 mode_to_use;
 	u64 value;
 	int ret;
+	s64 port_transmit_rate_kbps;
 
 	/* Queue 0 is not AVB capable */
 	if (queue <= 0 || queue >= tx_queues_count)
@@ -355,27 +356,24 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 	if (!priv->dma_cap.av)
 		return -EOPNOTSUPP;
 
+	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
+
 	/* Port Transmit Rate and Speed Divider */
-	switch (priv->speed) {
+	switch (div_s64(port_transmit_rate_kbps, 1000)) {
 	case SPEED_10000:
 		ptr = 32;
-		speed_div = 10000000;
 		break;
 	case SPEED_5000:
 		ptr = 32;
-		speed_div = 5000000;
 		break;
 	case SPEED_2500:
 		ptr = 8;
-		speed_div = 2500000;
 		break;
 	case SPEED_1000:
 		ptr = 8;
-		speed_div = 1000000;
 		break;
 	case SPEED_100:
 		ptr = 4;
-		speed_div = 100000;
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -397,11 +395,13 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 		priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
 	}
 
+	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
+
 	/* Final adjustments for HW */
-	value = div_s64(qopt->idleslope * 1024ll * ptr, speed_div);
+	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, speed_div);
+	value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps);
 	priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
 
 	value = qopt->hicredit * 1024ll * 8;
-- 
2.25.1
Re: [net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters
Posted by kernel test robot 1 year, 8 months ago
Hi Xiaolei,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-replace-priv-speed-with-the-portTransmitRate-from-the-tc-cbs-parameters/20240607-183700
base:   net/main
patch link:    https://lore.kernel.org/r/20240607103327.438455-1-xiaolei.wang%40windriver.com
patch subject: [net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240607/202406072254.05ysEMg1-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406072254.05ysEMg1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406072254.05ysEMg1-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:7:
   In file included from include/net/pkt_cls.h:7:
   In file included from include/net/sch_generic.h:5:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/riscv/include/asm/cacheflush.h:9:
   In file included from include/linux/mm.h:2253:
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:347:11: warning: unused variable 'speed_div' [-Wunused-variable]
     347 |         u32 ptr, speed_div;
         |                  ^~~~~~~~~
   2 warnings generated.


vim +/speed_div +347 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

4dbbe8dde8485b Jose Abreu                 2018-05-04  341  
1f705bc61aee5f Jose Abreu                 2018-06-27  342  static int tc_setup_cbs(struct stmmac_priv *priv,
1f705bc61aee5f Jose Abreu                 2018-06-27  343  			struct tc_cbs_qopt_offload *qopt)
1f705bc61aee5f Jose Abreu                 2018-06-27  344  {
1f705bc61aee5f Jose Abreu                 2018-06-27  345  	u32 tx_queues_count = priv->plat->tx_queues_to_use;
1f705bc61aee5f Jose Abreu                 2018-06-27  346  	u32 queue = qopt->queue;
1f705bc61aee5f Jose Abreu                 2018-06-27 @347  	u32 ptr, speed_div;
1f705bc61aee5f Jose Abreu                 2018-06-27  348  	u32 mode_to_use;
1f705bc61aee5f Jose Abreu                 2018-06-27  349  	u64 value;
1f705bc61aee5f Jose Abreu                 2018-06-27  350  	int ret;
09685c7b815a3c Xiaolei Wang               2024-06-07  351  	s64 port_transmit_rate_kbps;
1f705bc61aee5f Jose Abreu                 2018-06-27  352  
1f705bc61aee5f Jose Abreu                 2018-06-27  353  	/* Queue 0 is not AVB capable */
1f705bc61aee5f Jose Abreu                 2018-06-27  354  	if (queue <= 0 || queue >= tx_queues_count)
1f705bc61aee5f Jose Abreu                 2018-06-27  355  		return -EINVAL;
0650d4017f4d2e Jose Abreu                 2019-01-09  356  	if (!priv->dma_cap.av)
0650d4017f4d2e Jose Abreu                 2019-01-09  357  		return -EOPNOTSUPP;
1f705bc61aee5f Jose Abreu                 2018-06-27  358  
09685c7b815a3c Xiaolei Wang               2024-06-07  359  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
09685c7b815a3c Xiaolei Wang               2024-06-07  360  
24877687b375f2 Song, Yoong Siang          2021-02-18  361  	/* Port Transmit Rate and Speed Divider */
09685c7b815a3c Xiaolei Wang               2024-06-07  362  	switch (div_s64(port_transmit_rate_kbps, 1000)) {
24877687b375f2 Song, Yoong Siang          2021-02-18  363  	case SPEED_10000:
24877687b375f2 Song, Yoong Siang          2021-02-18  364  		ptr = 32;
24877687b375f2 Song, Yoong Siang          2021-02-18  365  		break;
24877687b375f2 Song, Yoong Siang          2021-02-18  366  	case SPEED_5000:
24877687b375f2 Song, Yoong Siang          2021-02-18  367  		ptr = 32;
24877687b375f2 Song, Yoong Siang          2021-02-18  368  		break;
24877687b375f2 Song, Yoong Siang          2021-02-18  369  	case SPEED_2500:
24877687b375f2 Song, Yoong Siang          2021-02-18  370  		ptr = 8;
24877687b375f2 Song, Yoong Siang          2021-02-18  371  		break;
24877687b375f2 Song, Yoong Siang          2021-02-18  372  	case SPEED_1000:
24877687b375f2 Song, Yoong Siang          2021-02-18  373  		ptr = 8;
24877687b375f2 Song, Yoong Siang          2021-02-18  374  		break;
24877687b375f2 Song, Yoong Siang          2021-02-18  375  	case SPEED_100:
24877687b375f2 Song, Yoong Siang          2021-02-18  376  		ptr = 4;
24877687b375f2 Song, Yoong Siang          2021-02-18  377  		break;
24877687b375f2 Song, Yoong Siang          2021-02-18  378  	default:
24877687b375f2 Song, Yoong Siang          2021-02-18  379  		return -EOPNOTSUPP;
24877687b375f2 Song, Yoong Siang          2021-02-18  380  	}
24877687b375f2 Song, Yoong Siang          2021-02-18  381  
1f705bc61aee5f Jose Abreu                 2018-06-27  382  	mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
1f705bc61aee5f Jose Abreu                 2018-06-27  383  	if (mode_to_use == MTL_QUEUE_DCB && qopt->enable) {
1f705bc61aee5f Jose Abreu                 2018-06-27  384  		ret = stmmac_dma_qmode(priv, priv->ioaddr, queue, MTL_QUEUE_AVB);
1f705bc61aee5f Jose Abreu                 2018-06-27  385  		if (ret)
1f705bc61aee5f Jose Abreu                 2018-06-27  386  			return ret;
1f705bc61aee5f Jose Abreu                 2018-06-27  387  
1f705bc61aee5f Jose Abreu                 2018-06-27  388  		priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
1f705bc61aee5f Jose Abreu                 2018-06-27  389  	} else if (!qopt->enable) {
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  390  		ret = stmmac_dma_qmode(priv, priv->ioaddr, queue,
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  391  				       MTL_QUEUE_DCB);
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  392  		if (ret)
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  393  			return ret;
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  394  
f317e2ea8c8873 Mohammad Athari Bin Ismail 2021-02-04  395  		priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
1f705bc61aee5f Jose Abreu                 2018-06-27  396  	}
1f705bc61aee5f Jose Abreu                 2018-06-27  397  
09685c7b815a3c Xiaolei Wang               2024-06-07  398  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
09685c7b815a3c Xiaolei Wang               2024-06-07  399  
1f705bc61aee5f Jose Abreu                 2018-06-27  400  	/* Final adjustments for HW */
09685c7b815a3c Xiaolei Wang               2024-06-07  401  	value = div_s64(qopt->idleslope * 1024ll * ptr, port_transmit_rate_kbps);
1f705bc61aee5f Jose Abreu                 2018-06-27  402  	priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
1f705bc61aee5f Jose Abreu                 2018-06-27  403  
09685c7b815a3c Xiaolei Wang               2024-06-07  404  	value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps);
1f705bc61aee5f Jose Abreu                 2018-06-27  405  	priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
1f705bc61aee5f Jose Abreu                 2018-06-27  406  
8f704ef666406f Arnd Bergmann              2018-07-06  407  	value = qopt->hicredit * 1024ll * 8;
1f705bc61aee5f Jose Abreu                 2018-06-27  408  	priv->plat->tx_queues_cfg[queue].high_credit = value & GENMASK(31, 0);
1f705bc61aee5f Jose Abreu                 2018-06-27  409  
8f704ef666406f Arnd Bergmann              2018-07-06  410  	value = qopt->locredit * 1024ll * 8;
1f705bc61aee5f Jose Abreu                 2018-06-27  411  	priv->plat->tx_queues_cfg[queue].low_credit = value & GENMASK(31, 0);
1f705bc61aee5f Jose Abreu                 2018-06-27  412  
1f705bc61aee5f Jose Abreu                 2018-06-27  413  	ret = stmmac_config_cbs(priv, priv->hw,
1f705bc61aee5f Jose Abreu                 2018-06-27  414  				priv->plat->tx_queues_cfg[queue].send_slope,
1f705bc61aee5f Jose Abreu                 2018-06-27  415  				priv->plat->tx_queues_cfg[queue].idle_slope,
1f705bc61aee5f Jose Abreu                 2018-06-27  416  				priv->plat->tx_queues_cfg[queue].high_credit,
1f705bc61aee5f Jose Abreu                 2018-06-27  417  				priv->plat->tx_queues_cfg[queue].low_credit,
1f705bc61aee5f Jose Abreu                 2018-06-27  418  				queue);
1f705bc61aee5f Jose Abreu                 2018-06-27  419  	if (ret)
1f705bc61aee5f Jose Abreu                 2018-06-27  420  		return ret;
1f705bc61aee5f Jose Abreu                 2018-06-27  421  
1f705bc61aee5f Jose Abreu                 2018-06-27  422  	dev_info(priv->device, "CBS queue %d: send %d, idle %d, hi %d, lo %d\n",
1f705bc61aee5f Jose Abreu                 2018-06-27  423  			queue, qopt->sendslope, qopt->idleslope,
1f705bc61aee5f Jose Abreu                 2018-06-27  424  			qopt->hicredit, qopt->locredit);
1f705bc61aee5f Jose Abreu                 2018-06-27  425  	return 0;
1f705bc61aee5f Jose Abreu                 2018-06-27  426  }
1f705bc61aee5f Jose Abreu                 2018-06-27  427  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters
Posted by Vladimir Oltean 1 year, 8 months ago
On Fri, Jun 07, 2024 at 06:33:27PM +0800, Xiaolei Wang wrote:
> Since the given offload->sendslope only applies to the
> current link speed, and userspace may reprogram it when
> the link speed changes, don't even bother tracking the
> port's link speed, and deduce the port transmit rate
> from idleslope - sentslope instead.
> 
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>

Patches to the "net" tree usually need a Fixes: tag pointing to the
first commit introducing an issue. They also need an explanation of the
problem being addressed and how it can negatively affect an user.

Still on the process topic, please increment the patch version from the
previous submissions, and post a change log under the "---" sign below,
as well as links on patchwork or lore to previous versions.

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 222540b55480..48500864017b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -348,6 +348,7 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  	u32 mode_to_use;
>  	u64 value;
>  	int ret;
> +	s64 port_transmit_rate_kbps;
>  
>  	/* Queue 0 is not AVB capable */
>  	if (queue <= 0 || queue >= tx_queues_count)
> @@ -355,27 +356,24 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  	if (!priv->dma_cap.av)
>  		return -EOPNOTSUPP;
>  
> +	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> +
>  	/* Port Transmit Rate and Speed Divider */
> -	switch (priv->speed) {
> +	switch (div_s64(port_transmit_rate_kbps, 1000)) {
>  	case SPEED_10000:
>  		ptr = 32;
> -		speed_div = 10000000;
>  		break;
>  	case SPEED_5000:
>  		ptr = 32;
> -		speed_div = 5000000;
>  		break;
>  	case SPEED_2500:
>  		ptr = 8;
> -		speed_div = 2500000;
>  		break;
>  	case SPEED_1000:
>  		ptr = 8;
> -		speed_div = 1000000;
>  		break;
>  	case SPEED_100:
>  		ptr = 4;
> -		speed_div = 100000;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;

This can be further compressed after the elimination of speed_div:

	switch (div_s64(port_transmit_rate_kbps, 1000)) {
	case SPEED_10000:
	case SPEED_5000:
		ptr = 32;
	case SPEED_2500:
	case SPEED_1000:
		ptr = 8;
		break;
	case SPEED_100:
		ptr = 4;
		break;
	default:
		return -EOPNOTSUPP;
	}

> @@ -397,11 +395,13 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  		priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>  	}
>  
> +	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> +

You don't need to calculate it twice in the same function.

>  	/* Final adjustments for HW */
> -	value = div_s64(qopt->idleslope * 1024ll * ptr, speed_div);
> +	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, speed_div);
> +	value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps);
>  	priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
>  
>  	value = qopt->hicredit * 1024ll * 8;
> -- 
> 2.25.1
>
Re: [net PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters
Posted by Wojciech Drewek 1 year, 8 months ago

On 07.06.2024 12:33, Xiaolei Wang wrote:
> Since the given offload->sendslope only applies to the
> current link speed, and userspace may reprogram it when
> the link speed changes, don't even bother tracking the
> port's link speed, and deduce the port transmit rate
> from idleslope - sentslope instead.
> 
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---

One nit, other than that:
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 222540b55480..48500864017b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -348,6 +348,7 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  	u32 mode_to_use;
>  	u64 value;
>  	int ret;
> +	s64 port_transmit_rate_kbps;

RCT

>  
>  	/* Queue 0 is not AVB capable */
>  	if (queue <= 0 || queue >= tx_queues_count)
> @@ -355,27 +356,24 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  	if (!priv->dma_cap.av)
>  		return -EOPNOTSUPP;
>  
> +	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> +
>  	/* Port Transmit Rate and Speed Divider */
> -	switch (priv->speed) {
> +	switch (div_s64(port_transmit_rate_kbps, 1000)) {
>  	case SPEED_10000:
>  		ptr = 32;
> -		speed_div = 10000000;
>  		break;
>  	case SPEED_5000:
>  		ptr = 32;
> -		speed_div = 5000000;
>  		break;
>  	case SPEED_2500:
>  		ptr = 8;
> -		speed_div = 2500000;
>  		break;
>  	case SPEED_1000:
>  		ptr = 8;
> -		speed_div = 1000000;
>  		break;
>  	case SPEED_100:
>  		ptr = 4;
> -		speed_div = 100000;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;
> @@ -397,11 +395,13 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  		priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
>  	}
>  
> +	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> +
>  	/* Final adjustments for HW */
> -	value = div_s64(qopt->idleslope * 1024ll * ptr, speed_div);
> +	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, speed_div);
> +	value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps);
>  	priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
>  
>  	value = qopt->hicredit * 1024ll * 8;