[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 8 Aug 2013 20:10:00 +0300
From: Claudiu Manoil <claudiu.manoil@...escale.com>
To: Ben Hutchings <bhutchings@...arflare.com>
CC: <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause
frame
Hi Ben,
On 8/7/2013 10:12 PM, Ben Hutchings wrote:
> On Wed, 2013-08-07 at 13:24 +0300, Claudiu Manoil wrote:
>> Allow Rx/Tx pause frame configuration via ethtool.
>> The gfar devices feature link autonegotioation by default.
>
> So the MAC configuration bits are actually copied to the PHY autoneg
> basic page, and then the PHY autoneg result is automatically used by the
> MAC?
>
> This is of course possible to do in hardware, but... since this MAC is
> not smart enough to ignore pause settings when running in half-duplex
> mode, I seriously doubt it is doing all this by itself.
>
I just wanted to say actually that the pause->autoneg parameter is not
needed by the gianfar driver, but I didn't know what to do with it in
get_pauseparam(), apparently pause->autoneg needs a value (or can
simply ignore this param?).
I don't see what autonegotiation has to do with enabling/disabling
pause frame generation in this case. My understanding is that link
autonegotiation is taken care somewhere else, by the phy state machine.
Each time this happens, the gianfar driver gets notified via the
adjust_link() hook that it implements and makes the necessary configs
in the mac registers.
Besides, autoneg info is already being displayed by ethtool (see
print below).
So I don't understand the use of pause->autoneg. What should I do with
it?
Thanks,
Claudiu
____
root@...20rdb-pd:~# ethtool eth2
Settings for eth2:
Supported ports: [ MII ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Advertised pause frame use: No
Advertised auto-negotiation: Yes
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 1
Transceiver: external
Auto-negotiation: on
Supports Wake-on: g
Wake-on: d
Current message level: 0x0000003f (63)
drv probe link timer ifdown ifup
Link detected: yes
>> The device is being configured with the new pause frame
>> parameters if the link is up, depending on link duplex (no
>> pause frames for half-duplex links), or during link autoneg
>> (see adjust_link()).
> [...]
>> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
>> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
>> @@ -535,6 +535,34 @@ static int gfar_sringparam(struct net_device *dev,
>> return err;
>> }
>>
>> +static void gfar_gpauseparam(struct net_device *dev,
>> + struct ethtool_pauseparam *pause)
>> +{
>> + struct gfar_private *priv = netdev_priv(dev);
>> +
>> + pause->autoneg = AUTONEG_ENABLE;
>> + if (priv->rx_pause)
>> + pause->rx_pause = 1;
>> + if (priv->tx_pause)
>> + pause->tx_pause = 1;
>> +}
>> +
>> +static int gfar_spauseparam(struct net_device *dev,
>> + struct ethtool_pauseparam *pause)
>> +{
>> + struct gfar_private *priv = netdev_priv(dev);
>> + struct phy_device *phydev = priv->phydev;
>
> You need to reject an unsupported setting of pause->autoneg here.
>
> Ben.
>
>> + priv->rx_pause = !!pause->rx_pause;
>> + priv->tx_pause = !!pause->tx_pause;
>> +
>> + /* update h/w settings, if link is up */
>> + if (phydev && phydev->link)
>> + gfar_configure_pause(priv, !!phydev->duplex);
>> +
>> + return 0;
>> +}
>> +
>> int gfar_set_features(struct net_device *dev, netdev_features_t features)
>> {
>> struct gfar_private *priv = netdev_priv(dev);
> [...]
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists