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

quoted

+	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.

quoted

 	}

 	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,

quoted

+					 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. 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel