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

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

On 8/14/2025 5:30 AM, Jakub Kicinski wrote:
> On Tue, 12 Aug 2025 10:20:35 +0530 Vishal Badole wrote:
>> Ethtool has advanced with additional configurable options, but the
>> current driver does not support tx-usecs configuration.
> 
> Not sure what you mean by this, perhaps:
> 
>    current driver does not even support tx-usecs configuration.
> 
> ? tx-usecs is a very old tunable.
> 
>> Add support to configure and retrieve 'tx-usecs' using ethtool, which
>> specifies the wait time before servicing an interrupt for Tx coalescing.
>>
Thanks for pointing that out. My intent was to highlight that while 
ethtool has gained more advanced and configurable options over time, the 
driver in its current form does not have support for tx-usecs 
configuration using Ethool.
I’ll reword the sentence to make this clearer in next patch version.>
>> +	/* Check if both tx_usecs and tx_frames are set to 0 simultaneously */
>> +	if (!tx_usecs && !tx_frames) {
>> +		NL_SET_ERR_MSG_FMT_MOD(extack,
>> +				       "tx_usecs and tx_frames must not be 0 together");
>> +		return -EINVAL;
>> +	}
>> +
>>   	/* Check the bounds of values for Tx */
>> +	if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
>> +		NL_SET_ERR_MSG_FMT_MOD(extack, "tx-usecs is limited to %d usec",
>> +				       XGMAC_MAX_COAL_TX_TICK);
>> +		return -EINVAL;
>> +	}
> 
> Normal configuration granularity for this parameter is in 10s of usecs.
> You seem to be using a timer, so I think you should either round the
> value up / down to what the jiffy resolution will give you or
> reject configuration that's not expressible in jiffies (not a multiple
> of jiffies_to_usecs(1)). Otherwise users may waste time turning this
> knob by 100usec which will have zero effect.

Good point — the hardware uses a timer, so sub-jiffy resolution won’t 
have any real effect. I’ll update the code in next patch version to 
round the tx-usecs value to the nearest multiple.