[v1] amd-xgbe: Configure and retrieve 'tx-usecs' for Tx coalescing

[PATCH net-next] amd-xgbe: Configure and retrieve 'tx-usecs' for Tx coalescing

Posted by Vishal Badole 3 months, 3 weeks ago

Ethtool has advanced with additional configurable options, but the
current driver does not support tx-usecs configuration.

Add support to configure and retrieve 'tx-usecs' using ethtool, which
specifies the wait time before servicing an interrupt for Tx coalescing.

Signed-off-by: Vishal Badole <Vishal.Badole@amd.com>
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 19 +++++++++++++++++--
 drivers/net/ethernet/amd/xgbe/xgbe.h         |  1 +
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index 12395428ffe1..362f8623433a 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -450,6 +450,7 @@ static int xgbe_get_coalesce(struct net_device *netdev,
 	ec->rx_coalesce_usecs = pdata->rx_usecs;
 	ec->rx_max_coalesced_frames = pdata->rx_frames;
 
+	ec->tx_coalesce_usecs = pdata->tx_usecs;
 	ec->tx_max_coalesced_frames = pdata->tx_frames;
 
 	return 0;
@@ -463,7 +464,7 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	struct xgbe_prv_data *pdata = netdev_priv(netdev);
 	struct xgbe_hw_if *hw_if = &pdata->hw_if;
 	unsigned int rx_frames, rx_riwt, rx_usecs;
-	unsigned int tx_frames;
+	unsigned int tx_frames, tx_usecs;
 
 	rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
 	rx_usecs = ec->rx_coalesce_usecs;
@@ -485,9 +486,22 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 		return -EINVAL;
 	}
 
+	tx_usecs = ec->tx_coalesce_usecs;
 	tx_frames = ec->tx_max_coalesced_frames;
 
+	/* Check if both tx_usecs and tx_frames are set to 0 simultaneously */
+	if (!tx_usecs && !tx_frames) {
+		netdev_err(netdev,
+			   "tx_usecs and tx_frames must not be 0 together\n");
+		return -EINVAL;
+	}
+
 	/* Check the bounds of values for Tx */
+	if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
+		netdev_err(netdev, "tx-usecs is limited to %d usec\n",
+			   XGMAC_MAX_COAL_TX_TICK);
+		return -EINVAL;
+	}
 	if (tx_frames > pdata->tx_desc_count) {
 		netdev_err(netdev, "tx-frames is limited to %d frames\n",
 			   pdata->tx_desc_count);
@@ -499,6 +513,7 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	pdata->rx_frames = rx_frames;
 	hw_if->config_rx_coalesce(pdata);
 
+	pdata->tx_usecs = tx_usecs;
 	pdata->tx_frames = tx_frames;
 	hw_if->config_tx_coalesce(pdata);
 
@@ -830,7 +845,7 @@ static int xgbe_set_channels(struct net_device *netdev,
 }
 
 static const struct ethtool_ops xgbe_ethtool_ops = {
-	.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
+	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
 	.get_drvinfo = xgbe_get_drvinfo,
 	.get_msglevel = xgbe_get_msglevel,
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 42fa4f84ff01..e330ae9ea685 100755
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -272,6 +272,7 @@
 /* Default coalescing parameters */
 #define XGMAC_INIT_DMA_TX_USECS		1000
 #define XGMAC_INIT_DMA_TX_FRAMES	25
+#define XGMAC_MAX_COAL_TX_TICK		100000
 
 #define XGMAC_MAX_DMA_RIWT		0xff
 #define XGMAC_INIT_DMA_RX_USECS		30
-- 
2.34.1

Re: [PATCH net-next] amd-xgbe: Configure and retrieve 'tx-usecs' for Tx coalescing

Posted by Vadim Fedorenko 3 months, 3 weeks ago

On 16/06/2025 11:42, Vishal Badole wrote:
> Ethtool has advanced with additional configurable options, but the
> current driver does not support tx-usecs configuration.
> 
> Add support to configure and retrieve 'tx-usecs' using ethtool, which
> specifies the wait time before servicing an interrupt for Tx coalescing.
> 
> Signed-off-by: Vishal Badole <Vishal.Badole@amd.com>
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 19 +++++++++++++++++--
>   drivers/net/ethernet/amd/xgbe/xgbe.h         |  1 +
>   2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> index 12395428ffe1..362f8623433a 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> @@ -450,6 +450,7 @@ static int xgbe_get_coalesce(struct net_device *netdev,
>   	ec->rx_coalesce_usecs = pdata->rx_usecs;
>   	ec->rx_max_coalesced_frames = pdata->rx_frames;
>   
> +	ec->tx_coalesce_usecs = pdata->tx_usecs;
>   	ec->tx_max_coalesced_frames = pdata->tx_frames;
>   
>   	return 0;
> @@ -463,7 +464,7 @@ static int xgbe_set_coalesce(struct net_device *netdev,
>   	struct xgbe_prv_data *pdata = netdev_priv(netdev);
>   	struct xgbe_hw_if *hw_if = &pdata->hw_if;
>   	unsigned int rx_frames, rx_riwt, rx_usecs;
> -	unsigned int tx_frames;
> +	unsigned int tx_frames, tx_usecs;
>   
>   	rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
>   	rx_usecs = ec->rx_coalesce_usecs;
> @@ -485,9 +486,22 @@ static int xgbe_set_coalesce(struct net_device *netdev,
>   		return -EINVAL;
>   	}
>   
> +	tx_usecs = ec->tx_coalesce_usecs;
>   	tx_frames = ec->tx_max_coalesced_frames;
>   
> +	/* Check if both tx_usecs and tx_frames are set to 0 simultaneously */
> +	if (!tx_usecs && !tx_frames) {
> +		netdev_err(netdev,
> +			   "tx_usecs and tx_frames must not be 0 together\n");
> +		return -EINVAL;
> +	}
> +
>   	/* Check the bounds of values for Tx */
> +	if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
> +		netdev_err(netdev, "tx-usecs is limited to %d usec\n",
> +			   XGMAC_MAX_COAL_TX_TICK);
> +		return -EINVAL;
> +	}

ethtool uses netlink interface now and coalesce callbacks have extack
parameters to return error information back to user-space. It would be
great to switch to use it instead of adding more netdev_err messages.

[...]

Re: [PATCH net-next] amd-xgbe: Configure and retrieve 'tx-usecs' for Tx coalescing

Posted by Badole, Vishal 3 months, 3 weeks ago


On 6/16/2025 4:59 PM, Vadim Fedorenko wrote:
> On 16/06/2025 11:42, Vishal Badole wrote:
>> Ethtool has advanced with additional configurable options, but the
>> current driver does not support tx-usecs configuration.
>>
>> Add support to configure and retrieve 'tx-usecs' using ethtool, which
>> specifies the wait time before servicing an interrupt for Tx coalescing.
>>
>> Signed-off-by: Vishal Badole <Vishal.Badole@amd.com>
>> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>   drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 19 +++++++++++++++++--
>>   drivers/net/ethernet/amd/xgbe/xgbe.h         |  1 +
>>   2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/ 
>> net/ethernet/amd/xgbe/xgbe-ethtool.c
>> index 12395428ffe1..362f8623433a 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
>> @@ -450,6 +450,7 @@ static int xgbe_get_coalesce(struct net_device 
>> *netdev,
>>       ec->rx_coalesce_usecs = pdata->rx_usecs;
>>       ec->rx_max_coalesced_frames = pdata->rx_frames;
>> +    ec->tx_coalesce_usecs = pdata->tx_usecs;
>>       ec->tx_max_coalesced_frames = pdata->tx_frames;
>>       return 0;
>> @@ -463,7 +464,7 @@ static int xgbe_set_coalesce(struct net_device 
>> *netdev,
>>       struct xgbe_prv_data *pdata = netdev_priv(netdev);
>>       struct xgbe_hw_if *hw_if = &pdata->hw_if;
>>       unsigned int rx_frames, rx_riwt, rx_usecs;
>> -    unsigned int tx_frames;
>> +    unsigned int tx_frames, tx_usecs;
>>       rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
>>       rx_usecs = ec->rx_coalesce_usecs;
>> @@ -485,9 +486,22 @@ static int xgbe_set_coalesce(struct net_device 
>> *netdev,
>>           return -EINVAL;
>>       }
>> +    tx_usecs = ec->tx_coalesce_usecs;
>>       tx_frames = ec->tx_max_coalesced_frames;
>> +    /* Check if both tx_usecs and tx_frames are set to 0 
>> simultaneously */
>> +    if (!tx_usecs && !tx_frames) {
>> +        netdev_err(netdev,
>> +               "tx_usecs and tx_frames must not be 0 together\n");
>> +        return -EINVAL;
>> +    }
>> +
>>       /* Check the bounds of values for Tx */
>> +    if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
>> +        netdev_err(netdev, "tx-usecs is limited to %d usec\n",
>> +               XGMAC_MAX_COAL_TX_TICK);
>> +        return -EINVAL;
>> +    }
> 
> ethtool uses netlink interface now and coalesce callbacks have extack
> parameters to return error information back to user-space. It would be
> great to switch to use it instead of adding more netdev_err messages.
> 
Hi Vadim.
Thank you for your observations. Since this driver is quite old, we have 
used netdev_err() to report errors to maintain consistency. In the 
future, we plan to upgrade the driver to use netlink interfaces with 
extack parameters for returning error information to user-space.
> [...]