RE: [PATCH net-next 1/1] net: stmmac: add per-queue TX & RX coalesce ethtool support

[Ong, Boon Leong]

>> +	if (queue < tx_cnt) {
>> +		ec->tx_coalesce_usecs = priv->tx_coal_timer[queue];
>> +		ec->tx_max_coalesced_frames = priv->tx_coal_frames[queue];
>> +	} else {
>> +		ec->tx_coalesce_usecs = -1;
>> +		ec->tx_max_coalesced_frames = -1;
>> +	}
>> +
>> +	if (priv->use_riwt && queue < rx_cnt) {
>> +		ec->rx_max_coalesced_frames = priv->rx_coal_frames[queue];
>> +		ec->rx_coalesce_usecs = stmmac_riwt2usec(priv-
>>rx_riwt[queue],
>> +							 priv);
>> +	} else {
>> +		ec->rx_max_coalesced_frames = -1;
>> +		ec->rx_coalesce_usecs = -1;
>
>Why the use of negative values? why not leave them as 0?
The initial logic was to return negative value to unsupported TXQ & RXQ
since they are invalid. No preference here. So, we can leave it as all zeros.

>
>>  	}
>>
>>  	return 0;
>>  }
>>
>> -static int stmmac_set_coalesce(struct net_device *dev,
>> +static int stmmac_get_coalesce(struct net_device *dev,
>>  			       struct ethtool_coalesce *ec)
>> +{
>> +	return __stmmac_get_coalesce(dev, ec, -1);
>> +}
>> +
>> +static int stmmac_get_per_queue_coalesce(struct net_device *dev, u32
>queue,
>> +					 struct ethtool_coalesce *ec)
>> +{
>> +	return __stmmac_get_coalesce(dev, ec, queue);
>> +}
>> +
>> +static int __stmmac_set_coalesce(struct net_device *dev,
>> +				 struct ethtool_coalesce *ec,
>> +				 int queue)
>>  {
>>  	struct stmmac_priv *priv = netdev_priv(dev);
>> -	u32 rx_cnt = priv->plat->rx_queues_to_use;
>> +	bool all_queues = false;
>>  	unsigned int rx_riwt;
>> +	u32 max_cnt;
>> +	u32 rx_cnt;
>> +	u32 tx_cnt;
>> +
>> +	rx_cnt = priv->plat->rx_queues_to_use;
>> +	tx_cnt = priv->plat->tx_queues_to_use;
>> +	max_cnt = max(rx_cnt, tx_cnt);
>> +
>> +	if (queue < 0)
>> +		all_queues = true;
>> +	else if (queue >= max_cnt)
>> +		return -EINVAL;
>> +
>> +	/* Check not supported parameters  */
>> +	if (ec->rx_coalesce_usecs_irq ||
>> +	    ec->rx_max_coalesced_frames_irq || ec->tx_coalesce_usecs_irq ||
>> +	    ec->use_adaptive_rx_coalesce || ec->use_adaptive_tx_coalesce ||
>> +	    ec->pkt_rate_low || ec->rx_coalesce_usecs_low ||
>> +	    ec->rx_max_coalesced_frames_low || ec->tx_coalesce_usecs_high ||
>> +	    ec->tx_max_coalesced_frames_low || ec->pkt_rate_high ||
>> +	    ec->tx_coalesce_usecs_low || ec->rx_coalesce_usecs_high ||
>> +	    ec->rx_max_coalesced_frames_high ||
>> +	    ec->tx_max_coalesced_frames_irq ||
>> +	    ec->stats_block_coalesce_usecs ||
>> +	    ec->tx_max_coalesced_frames_high || ec->rate_sample_interval)
>> +		return -EOPNOTSUPP;
>
>This shouldn't be needed now that supporter types are expressed in
>dev->ethtool_ops->supported_coalesce_params, no?
Wil fix this. Thanks for pointing out.