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

["Badole, Vishal" <vishal.badole@amd.com>]

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.
> [...]