Thread (12 messages) flat view 12 messages, 3 authors, 2016-02-23

RE: [PATCH V2 net-next 2/3] lan78xx: add ethtool set & get pause functions

From: <Woojung.Huh@microchip.com>
Date: 2016-02-19 00:03:52

quoted
Add ethtool operations of set_pauseram and get_pauseparm.
[...]
quoted
+static void lan78xx_get_pause(struct net_device *net,
+			      struct ethtool_pauseparam *pause)
+{
+	struct lan78xx_net *dev = netdev_priv(net);
+	struct phy_device *phydev = net->phydev;
+	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
+
+	phy_ethtool_gset(phydev, &ecmd);
+
+	pause->autoneg = dev->fc_autoneg;
+
+	if (dev->fc_autoneg) {
+		if (dev->fc_autoneg_control & FLOW_CTRL_TX)
+			pause->tx_pause = 1;
+
+		if (dev->fc_autoneg_control & FLOW_CTRL_RX)
+			pause->rx_pause = 1;
This is incorrect; you should always return the manual settings
(fc_request_control flags) here.  If autonegotiation is enabled then
your get_settings function will return the actual pause flags.

Ben.
Ben, thanks for comments.
How about comment in include/uapi/linux/ethtool.h?
It says 
** struct ethtool_pauseparam - Ethernet pause (flow control) parameters
...
* If @autoneg is non-zero, the MAC is configured to send and/or
* receive pause frames according to the result of autonegotiation.

Doesn't this mean get_pauseparam() returns pause settings based on 
Result of autonegotation? Not manual settings of rx_param & tx_param?

Woojung
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help