* [PATCH 1/2][net-next] gianfar: Fix pause frame handling for half duplex links @ 2013-08-07 10:24 Claudiu Manoil 2013-08-07 10:24 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil 0 siblings, 1 reply; 18+ messages in thread From: Claudiu Manoil @ 2013-08-07 10:24 UTC (permalink / raw) To: netdev; +Cc: David S. Miller MACCFG1 register bits to enable PAUSE frame handling are set by the driver unconditionally. Per 802.3 standard, PAUSE frame handling is a full duplex feature, and neither meaningful nor correct in half duplex. Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com> --- drivers/net/ethernet/freescale/gianfar.c | 31 +++++++++++++++++++++++++++++-- drivers/net/ethernet/freescale/gianfar.h | 2 ++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index edf06f1..54cf9be 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -764,6 +764,9 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) FSL_GIANFAR_DEV_HAS_EXTENDED_HASH | FSL_GIANFAR_DEV_HAS_TIMER; + /* default pause frame settings */ + priv->rx_pause = priv->tx_pause = true; + ctype = of_get_property(np, "phy-connection-type", NULL); /* We only care about rgmii-id. The rest are autodetected */ @@ -1016,8 +1019,10 @@ static int gfar_probe(struct platform_device *ofdev) /* We need to delay at least 3 TX clocks */ udelay(2); - tempval = (MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); - gfar_write(®s->maccfg1, tempval); + /* the soft reset bit is not self-resetting, so we need to + * clear it before resuming normal operation + */ + gfar_write(®s->maccfg1, 0); /* Initialize MACCFG2. */ tempval = MACCFG2_INIT_SETTINGS; @@ -3025,6 +3030,25 @@ static irqreturn_t gfar_interrupt(int irq, void *grp_id) return IRQ_HANDLED; } +/* toggle pause frame settings */ +void gfar_configure_pause(struct gfar_private *priv, bool en) +{ + struct gfar __iomem *regs = priv->gfargrp[0].regs; + u32 tempval = gfar_read(®s->maccfg1); + + if (en && priv->rx_pause) + tempval |= MACCFG1_RX_FLOW; + else + tempval &= ~MACCFG1_RX_FLOW; + + if (en && priv->tx_pause) + tempval |= MACCFG1_TX_FLOW; + else + tempval &= ~MACCFG1_TX_FLOW; + + gfar_write(®s->maccfg1, tempval); +} + /* Called every time the controller might need to be made * aware of new link state. The PHY code conveys this * information through variables in the phydev structure, and this @@ -3056,6 +3080,9 @@ static void adjust_link(struct net_device *dev) else tempval |= MACCFG2_FULL_DUPLEX; + /* update pause frame settings */ + gfar_configure_pause(priv, !!phydev->duplex); + priv->oldduplex = phydev->duplex; } diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index ee19f2c..e6a03f4 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -1084,6 +1084,8 @@ struct gfar_private { int oldspeed; int oldduplex; int oldlink; + bool rx_pause; + bool tx_pause; /* Bitfield update lock */ spinlock_t bflock; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-07 10:24 [PATCH 1/2][net-next] gianfar: Fix pause frame handling for half duplex links Claudiu Manoil @ 2013-08-07 10:24 ` Claudiu Manoil 2013-08-07 19:12 ` Ben Hutchings 0 siblings, 1 reply; 18+ messages in thread From: Claudiu Manoil @ 2013-08-07 10:24 UTC (permalink / raw) To: netdev; +Cc: David S. Miller Allow Rx/Tx pause frame configuration via ethtool. The gfar devices feature link autonegotioation by default. 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()). Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com> --- drivers/net/ethernet/freescale/gianfar.h | 1 + drivers/net/ethernet/freescale/gianfar_ethtool.c | 30 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index e6a03f4..aaac843 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -1186,6 +1186,7 @@ void gfar_init_sysfs(struct net_device *dev); int gfar_set_features(struct net_device *dev, netdev_features_t features); extern void gfar_check_rx_parser_mode(struct gfar_private *priv); extern void gfar_vlan_mode(struct net_device *dev, netdev_features_t features); +void gfar_configure_pause(struct gfar_private *priv, bool en); extern const struct ethtool_ops gfar_ethtool_ops; diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c index 21cd881..6cf89c1 100644 --- 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; + + 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); @@ -1806,6 +1834,8 @@ const struct ethtool_ops gfar_ethtool_ops = { .set_coalesce = gfar_scoalesce, .get_ringparam = gfar_gringparam, .set_ringparam = gfar_sringparam, + .get_pauseparam = gfar_gpauseparam, + .set_pauseparam = gfar_spauseparam, .get_strings = gfar_gstrings, .get_sset_count = gfar_sset_count, .get_ethtool_stats = gfar_fill_stats, -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-07 10:24 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil @ 2013-08-07 19:12 ` Ben Hutchings 2013-08-08 17:10 ` Claudiu Manoil 0 siblings, 1 reply; 18+ messages in thread From: Ben Hutchings @ 2013-08-07 19:12 UTC (permalink / raw) To: Claudiu Manoil; +Cc: netdev, David S. Miller 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. > 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); [...] -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-07 19:12 ` Ben Hutchings @ 2013-08-08 17:10 ` Claudiu Manoil 2013-08-08 18:45 ` Ben Hutchings 0 siblings, 1 reply; 18+ messages in thread From: Claudiu Manoil @ 2013-08-08 17:10 UTC (permalink / raw) To: Ben Hutchings; +Cc: netdev, David S. Miller 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@p1020rdb-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); > [...] > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-08 17:10 ` Claudiu Manoil @ 2013-08-08 18:45 ` Ben Hutchings 2013-08-09 8:26 ` Lutz Jaenicke 0 siblings, 1 reply; 18+ messages in thread From: Ben Hutchings @ 2013-08-08 18:45 UTC (permalink / raw) To: Claudiu Manoil; +Cc: netdev, David S. Miller On Thu, 2013-08-08 at 20:10 +0300, Claudiu Manoil wrote: > 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. That's what it *should* be doing. But it's not; it's using priv->rx_pause and priv->tx_pause, not phydev->pause and phydev->asym_pause. I.e. pause autoneg is really always *disabled*. > 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? [...] Pause frame generation may be forced even though autonegotiation is enabled. I don't think this is a good idea but many drivers do support it. And gianfar does appear to allow autonegotiation to be disabled, anyway, so in any case you have to support both forced and autonegotiated cases. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-08 18:45 ` Ben Hutchings @ 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 8:26 ` [PATCH] gianfar: implement flow control handling Lutz Jaenicke ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Lutz Jaenicke @ 2013-08-09 8:26 UTC (permalink / raw) To: netdev; +Cc: Claudiu Manoil, Ben Hutchings, David S. Miller I have been working on the same issue and came up with a similar modification (that additionally implements LFC if supported) I have attached the two patches created: * gianfar: implement flow control handling which looks quite similar to Claudiu's patches * gianfar: add support for LFC (Lossless Flow Control) which is more or less just extracted from some patch I found on the internet linux-2.6.29.6-mpc8308e_rdb.patch The patches I have taken from a custom 3.4 based kernel so they may not apply cleanly but maybe bits of it can be used to generate fully supported solution. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] gianfar: implement flow control handling 2013-08-09 8:26 ` Lutz Jaenicke @ 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 17:19 ` [PATCH][net-next v1] gianfar: Add flow control support Claudiu Manoil 2013-08-09 8:26 ` [PATCH] gianfar: add support for LFC (Lossless Flow Control) Lutz Jaenicke 2013-08-09 10:12 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil 2 siblings, 1 reply; 18+ messages in thread From: Lutz Jaenicke @ 2013-08-09 8:26 UTC (permalink / raw) To: netdev; +Cc: Claudiu Manoil, Ben Hutchings, David S. Miller, Lutz Jaenicke This implementation is inspired by the tg3 driver! --- drivers/net/ethernet/freescale/gianfar.c | 31 +++++++++- drivers/net/ethernet/freescale/gianfar.h | 7 ++- drivers/net/ethernet/freescale/gianfar_ethtool.c | 71 ++++++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 91bd6da..0da003c 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -1065,7 +1065,11 @@ static int gfar_probe(struct platform_device *ofdev) /* We need to delay at least 3 TX clocks */ udelay(2); - tempval = (MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + tempval = 0; + if (!priv->pause_aneg_en && priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (!priv->pause_aneg_en && priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; gfar_write(®s->maccfg1, tempval); /* Initialize MACCFG2. */ @@ -3042,6 +3046,7 @@ static void adjust_link(struct net_device *dev) lock_tx_qs(priv); if (phydev->link) { + u32 tempval1 = gfar_read(®s->maccfg1); u32 tempval = gfar_read(®s->maccfg2); u32 ecntrl = gfar_read(®s->ecntrl); @@ -3088,6 +3093,30 @@ static void adjust_link(struct net_device *dev) priv->oldspeed = phydev->speed; } + tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + if (phydev->duplex) { + if (!priv->pause_aneg_en) { + if (priv->tx_pause_en) + tempval1 |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + tempval1 |= MACCFG1_RX_FLOW; + } else { + u8 flowctrl; + u32 lcl_adv = mii_advertise_flowctrl(phydev->advertising); + u32 rmt_adv = 0; + if (phydev->pause) + rmt_adv = LPA_PAUSE_CAP; + if (phydev->asym_pause) + rmt_adv |= LPA_PAUSE_ASYM; + flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv); + if (flowctrl & FLOW_CTRL_TX) + tempval1 |= MACCFG1_TX_FLOW; + if (flowctrl & FLOW_CTRL_RX) + tempval1 |= MACCFG1_RX_FLOW; + } + } + + gfar_write(®s->maccfg1, tempval1); gfar_write(®s->maccfg2, tempval); gfar_write(®s->ecntrl, ecntrl); diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index aedfcc4..41740e2 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -148,6 +148,8 @@ extern const char gfar_driver_version[]; | SUPPORTED_100baseT_Half \ | SUPPORTED_100baseT_Full \ | SUPPORTED_Autoneg \ + | SUPPORTED_Pause \ + | SUPPORTED_Asym_Pause \ | SUPPORTED_MII) /* TBI register addresses */ @@ -1105,7 +1107,10 @@ struct gfar_private { extended_hash:1, bd_stash_en:1, rx_filer_enable:1, - wol_en:1; /* Wake-on-LAN enabled */ + wol_en:1, /* Wake-on-LAN enabled */ + pause_aneg_en:1, + tx_pause_en:1, + rx_pause_en:1; unsigned short padding; /* PHY stuff */ diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c index 01748d1..0fc1c65d 100644 --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c @@ -1836,6 +1836,75 @@ static int gfar_nway_reset(struct net_device *dev) return phy_start_aneg(priv->phydev); } +static void gfar_get_pauseparam(struct net_device *dev, struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + + epause->autoneg = !!priv->pause_aneg_en; + epause->rx_pause = !!priv->rx_pause_en; + epause->tx_pause = !!priv->tx_pause_en; + +} + + +static int gfar_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + struct phy_device *phydev = priv->phydev; + struct gfar __iomem *regs = priv->gfargrp[0].regs; + u32 oldadv, newadv; + int err = 0; + + if (!(phydev->supported & SUPPORTED_Pause) || + (!(phydev->supported & SUPPORTED_Asym_Pause) && + (epause->rx_pause != epause->tx_pause))) + return -EINVAL; + + priv->rx_pause_en = priv->tx_pause_en = 0; + if (epause->rx_pause) { + priv->rx_pause_en = 1; + + if (epause->tx_pause) { + priv->tx_pause_en = 1; + newadv = ADVERTISED_Pause; + } else + newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause; + } else if (epause->tx_pause) { + priv->tx_pause_en = 1; + newadv = ADVERTISED_Asym_Pause; + } else + newadv = 0; + + if (epause->autoneg) + priv->pause_aneg_en = 1; + else + priv->pause_aneg_en = 0; + + oldadv = phydev->advertising & + (ADVERTISED_Pause | ADVERTISED_Asym_Pause); + if (oldadv != newadv) { + phydev->advertising &= + ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + phydev->advertising |= newadv; + if (phydev->autoneg) { + return phy_start_aneg(phydev); + } + + if (!epause->autoneg) { + u32 tempval; + tempval = gfar_read(®s->maccfg1); + tempval &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + if (priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; + gfar_write(®s->maccfg1, tempval); + } + } + + return err; +} + const struct ethtool_ops gfar_ethtool_ops = { .get_settings = gfar_gsettings, .set_settings = gfar_ssettings, @@ -1861,4 +1930,6 @@ const struct ethtool_ops gfar_ethtool_ops = { .get_priv_flags = gfar_get_priv_flags, .set_priv_flags = gfar_set_priv_flags, .nway_reset = gfar_nway_reset, + .get_pauseparam = gfar_get_pauseparam, + .set_pauseparam = gfar_set_pauseparam, }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH][net-next v1] gianfar: Add flow control support 2013-08-09 8:26 ` [PATCH] gianfar: implement flow control handling Lutz Jaenicke @ 2013-08-09 17:19 ` Claudiu Manoil 2013-08-09 17:36 ` Fabio Estevam ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Claudiu Manoil @ 2013-08-09 17:19 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Ben Hutchings, Lutz Jaenicke eTSEC has Rx and Tx flow control capabilities that may be enabled through MACCFG1[Rx_Flow, Tx_Flow] bits. These bits must not be set however when eTSEC is operated in Half-Duplex mode. Unfortunately, the driver currently sets these bits unconditionally. This patch adds the proper handling of the PAUSE frame capability register bits by implementing the ethtool -A interface. When pause autoneg is enabled, the controller uses the phy's capability to negotiate PAUSE frame settings with the link partner and reconfigures its Rx_Flow and Tx_Flow settings to match the capabilities of the link partner. If pause autoneg is off, the PAUSE frame generation may be forced manually (ethtool -A). Flow control is disabled by default now. This implementation is inspired by the tg3 driver. Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com> --- v1 - Cleaned-up version of: http://patchwork.ozlabs.org/patch/265940/ drivers/net/ethernet/freescale/gianfar.c | 38 +++++++++++- drivers/net/ethernet/freescale/gianfar.h | 10 +++- drivers/net/ethernet/freescale/gianfar_ethtool.c | 75 ++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index edf06f1..56d729e 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -1016,7 +1016,14 @@ static int gfar_probe(struct platform_device *ofdev) /* We need to delay at least 3 TX clocks */ udelay(2); - tempval = (MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + tempval = 0; + if (!priv->pause_aneg_en && priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (!priv->pause_aneg_en && priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; + /* the soft reset bit is not self-resetting, so we need to + * clear it before resuming normal operation + */ gfar_write(®s->maccfg1, tempval); /* Initialize MACCFG2. */ @@ -1462,7 +1469,7 @@ static int init_phy(struct net_device *dev) struct gfar_private *priv = netdev_priv(dev); uint gigabit_support = priv->device_flags & FSL_GIANFAR_DEV_HAS_GIGABIT ? - SUPPORTED_1000baseT_Full : 0; + GFAR_SUPPORTED_GBIT : 0; phy_interface_t interface; priv->oldlink = 0; @@ -3043,6 +3050,7 @@ static void adjust_link(struct net_device *dev) lock_tx_qs(priv); if (phydev->link) { + u32 tempval1 = gfar_read(®s->maccfg1); u32 tempval = gfar_read(®s->maccfg2); u32 ecntrl = gfar_read(®s->ecntrl); @@ -3091,6 +3099,32 @@ static void adjust_link(struct net_device *dev) priv->oldspeed = phydev->speed; } + tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + if (phydev->duplex) { + if (!priv->pause_aneg_en) { + if (priv->tx_pause_en) + tempval1 |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + tempval1 |= MACCFG1_RX_FLOW; + } else { + u8 flowctrl; + u16 lcl_adv = mii_advertise_flowctrl( + phydev->advertising); + u16 rmt_adv = 0; + if (phydev->pause) + rmt_adv = LPA_PAUSE_CAP; + if (phydev->asym_pause) + rmt_adv |= LPA_PAUSE_ASYM; + flowctrl = mii_resolve_flowctrl_fdx( + lcl_adv, rmt_adv); + if (flowctrl & FLOW_CTRL_TX) + tempval1 |= MACCFG1_TX_FLOW; + if (flowctrl & FLOW_CTRL_RX) + tempval1 |= MACCFG1_RX_FLOW; + } + } + + gfar_write(®s->maccfg1, tempval1); gfar_write(®s->maccfg2, tempval); gfar_write(®s->ecntrl, ecntrl); diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index ee19f2c..46f56f3 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -146,6 +146,10 @@ extern const char gfar_driver_version[]; | SUPPORTED_Autoneg \ | SUPPORTED_MII) +#define GFAR_SUPPORTED_GBIT (SUPPORTED_1000baseT_Full \ + | SUPPORTED_Pause \ + | SUPPORTED_Asym_Pause) + /* TBI register addresses */ #define MII_TBICON 0x11 @@ -1100,7 +1104,11 @@ struct gfar_private { /* Wake-on-LAN enabled */ wol_en:1, /* Enable priorty based Tx scheduling in Hw */ - prio_sched_en:1; + prio_sched_en:1, + /* Flow control flags */ + pause_aneg_en:1, + tx_pause_en:1, + rx_pause_en:1; /* The total tx and rx ring size for the enabled queues */ unsigned int total_tx_ring_size; diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c index 21cd881..b245068 100644 --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c @@ -535,6 +535,79 @@ static int gfar_sringparam(struct net_device *dev, return err; } +static void gfar_gpauseparam(struct net_device *dev, + struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + + epause->autoneg = !!priv->pause_aneg_en; + epause->rx_pause = !!priv->rx_pause_en; + epause->tx_pause = !!priv->tx_pause_en; + +} + +static int gfar_spauseparam(struct net_device *dev, + struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + struct phy_device *phydev = priv->phydev; + struct gfar __iomem *regs = priv->gfargrp[0].regs; + u32 oldadv, newadv; + int err = 0; + + if (!(phydev->supported & SUPPORTED_Pause) || + (!(phydev->supported & SUPPORTED_Asym_Pause) && + (epause->rx_pause != epause->tx_pause))) + return -EINVAL; + + priv->rx_pause_en = priv->tx_pause_en = 0; + if (epause->rx_pause) { + priv->rx_pause_en = 1; + + if (epause->tx_pause) { + priv->tx_pause_en = 1; + /* FLOW_CTRL_RX & TX */ + newadv = ADVERTISED_Pause; + } else /* FLOW_CTLR_RX */ + newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause; + } else if (epause->tx_pause) { + priv->tx_pause_en = 1; + newadv = ADVERTISED_Asym_Pause; /* FLOW_CTLR_TX */ + } else + newadv = 0; + + if (epause->autoneg) + priv->pause_aneg_en = 1; + else + priv->pause_aneg_en = 0; + + oldadv = phydev->advertising & + (ADVERTISED_Pause | ADVERTISED_Asym_Pause); + if (oldadv != newadv) { + phydev->advertising &= + ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + phydev->advertising |= newadv; + if (phydev->autoneg) + /* inform link partner of our + * new flow ctrl settings + */ + return phy_start_aneg(phydev); + + if (!epause->autoneg) { + u32 tempval; + tempval = gfar_read(®s->maccfg1); + tempval &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + if (priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; + gfar_write(®s->maccfg1, tempval); + } + } + + return err; +} + int gfar_set_features(struct net_device *dev, netdev_features_t features) { struct gfar_private *priv = netdev_priv(dev); @@ -1806,6 +1879,8 @@ const struct ethtool_ops gfar_ethtool_ops = { .set_coalesce = gfar_scoalesce, .get_ringparam = gfar_gringparam, .set_ringparam = gfar_sringparam, + .get_pauseparam = gfar_gpauseparam, + .set_pauseparam = gfar_spauseparam, .get_strings = gfar_gstrings, .get_sset_count = gfar_sset_count, .get_ethtool_stats = gfar_fill_stats, -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH][net-next v1] gianfar: Add flow control support 2013-08-09 17:19 ` [PATCH][net-next v1] gianfar: Add flow control support Claudiu Manoil @ 2013-08-09 17:36 ` Fabio Estevam 2013-08-12 7:09 ` Claudiu Manoil 2013-08-09 19:09 ` Joe Perches 2013-08-12 10:53 ` [PATCH][net-next v2] " Claudiu Manoil 2 siblings, 1 reply; 18+ messages in thread From: Fabio Estevam @ 2013-08-09 17:36 UTC (permalink / raw) To: Claudiu Manoil; +Cc: netdev, David S. Miller, Ben Hutchings, Lutz Jaenicke On Fri, Aug 9, 2013 at 2:19 PM, Claudiu Manoil <claudiu.manoil@freescale.com> wrote: > +static int gfar_spauseparam(struct net_device *dev, > + struct ethtool_pauseparam *epause) > +{ > + struct gfar_private *priv = netdev_priv(dev); > + struct phy_device *phydev = priv->phydev; > + struct gfar __iomem *regs = priv->gfargrp[0].regs; > + u32 oldadv, newadv; > + int err = 0; You could drop this 'err' variable, no? ... > + > + return err; Then just 'return 0' here. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][net-next v1] gianfar: Add flow control support 2013-08-09 17:36 ` Fabio Estevam @ 2013-08-12 7:09 ` Claudiu Manoil 0 siblings, 0 replies; 18+ messages in thread From: Claudiu Manoil @ 2013-08-12 7:09 UTC (permalink / raw) To: Fabio Estevam; +Cc: netdev, David S. Miller, Ben Hutchings, Lutz Jaenicke Right, thanks. ___ Claudiu On 8/9/2013 8:36 PM, Fabio Estevam wrote: > On Fri, Aug 9, 2013 at 2:19 PM, Claudiu Manoil > <claudiu.manoil@freescale.com> wrote: > >> +static int gfar_spauseparam(struct net_device *dev, >> + struct ethtool_pauseparam *epause) >> +{ >> + struct gfar_private *priv = netdev_priv(dev); >> + struct phy_device *phydev = priv->phydev; >> + struct gfar __iomem *regs = priv->gfargrp[0].regs; >> + u32 oldadv, newadv; >> + int err = 0; > > You could drop this 'err' variable, no? > > ... > >> + >> + return err; > > Then just 'return 0' here. > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][net-next v1] gianfar: Add flow control support 2013-08-09 17:19 ` [PATCH][net-next v1] gianfar: Add flow control support Claudiu Manoil 2013-08-09 17:36 ` Fabio Estevam @ 2013-08-09 19:09 ` Joe Perches 2013-08-12 7:08 ` Claudiu Manoil 2013-08-12 10:53 ` [PATCH][net-next v2] " Claudiu Manoil 2 siblings, 1 reply; 18+ messages in thread From: Joe Perches @ 2013-08-09 19:09 UTC (permalink / raw) To: Claudiu Manoil; +Cc: netdev, David S. Miller, Ben Hutchings, Lutz Jaenicke On Fri, 2013-08-09 at 20:19 +0300, Claudiu Manoil wrote: > eTSEC has Rx and Tx flow control capabilities that may be enabled > through MACCFG1[Rx_Flow, Tx_Flow] bits. Trivial note: > diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c [] > @@ -3043,6 +3050,7 @@ static void adjust_link(struct net_device *dev) [] > + u32 tempval1 = gfar_read(®s->maccfg1); [] > @@ -3091,6 +3099,32 @@ static void adjust_link(struct net_device *dev) > priv->oldspeed = phydev->speed; > } > > + tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); > + if (phydev->duplex) { > + if (!priv->pause_aneg_en) { > + if (priv->tx_pause_en) > + tempval1 |= MACCFG1_TX_FLOW; > + if (priv->rx_pause_en) > + tempval1 |= MACCFG1_RX_FLOW; > + } else { > + u8 flowctrl; > + u16 lcl_adv = mii_advertise_flowctrl( > + phydev->advertising); > + u16 rmt_adv = 0; > + if (phydev->pause) > + rmt_adv = LPA_PAUSE_CAP; > + if (phydev->asym_pause) > + rmt_adv |= LPA_PAUSE_ASYM; > + flowctrl = mii_resolve_flowctrl_fdx( > + lcl_adv, rmt_adv); > + if (flowctrl & FLOW_CTRL_TX) > + tempval1 |= MACCFG1_TX_FLOW; > + if (flowctrl & FLOW_CTRL_RX) > + tempval1 |= MACCFG1_RX_FLOW; > + } > + } I think this bit would be nicer as a static function. Something like: (uncompiled/untested) static int gfar_get_cfg1(struct net_device *dev) { struct gfar_private *priv = netdev_priv(dev); struct gfar __iomem *regs = priv->gfargrp[0].regs; struct phy_device *phydev = priv->phydev; int val; val = gfar_read(®s->maccfg1); val &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); if (!phydev->duplex) return val; if (!priv->pause_aneg_en) { if (priv->tx_pause_en) val |= MACCFG1_TX_FLOW; if (priv->rx_pause_en) val |= MACCFG1_RX_FLOW; } else { u8 flowctrl; u16 lcl_adv = mii_advertise_flowctrl(phydev->advertising); u16 rmt_adv = 0; if (phydev->pause) rmt_adv = LPA_PAUSE_CAP; if (phydev->asym_pause) rmt_adv |= LPA_PAUSE_ASYM; flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv); if (flowctrl & FLOW_CTRL_TX) val |= MACCFG1_TX_FLOW; if (flowctrl & FLOW_CTRL_RX) val |= MACCFG1_RX_FLOW; } return val; } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH][net-next v1] gianfar: Add flow control support 2013-08-09 19:09 ` Joe Perches @ 2013-08-12 7:08 ` Claudiu Manoil 0 siblings, 0 replies; 18+ messages in thread From: Claudiu Manoil @ 2013-08-12 7:08 UTC (permalink / raw) To: Joe Perches; +Cc: netdev, David S. Miller, Ben Hutchings, Lutz Jaenicke Will provide a static function to factor out the code that configures the 2 flow control bits for maccfg1. Thanks. ___ Claudiu On 8/9/2013 10:09 PM, Joe Perches wrote: > On Fri, 2013-08-09 at 20:19 +0300, Claudiu Manoil wrote: >> eTSEC has Rx and Tx flow control capabilities that may be enabled >> through MACCFG1[Rx_Flow, Tx_Flow] bits. > > Trivial note: > >> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c > [] >> @@ -3043,6 +3050,7 @@ static void adjust_link(struct net_device *dev) > [] >> + u32 tempval1 = gfar_read(®s->maccfg1); > [] >> @@ -3091,6 +3099,32 @@ static void adjust_link(struct net_device *dev) >> priv->oldspeed = phydev->speed; >> } >> >> + tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); >> + if (phydev->duplex) { >> + if (!priv->pause_aneg_en) { >> + if (priv->tx_pause_en) >> + tempval1 |= MACCFG1_TX_FLOW; >> + if (priv->rx_pause_en) >> + tempval1 |= MACCFG1_RX_FLOW; >> + } else { >> + u8 flowctrl; >> + u16 lcl_adv = mii_advertise_flowctrl( >> + phydev->advertising); >> + u16 rmt_adv = 0; >> + if (phydev->pause) >> + rmt_adv = LPA_PAUSE_CAP; >> + if (phydev->asym_pause) >> + rmt_adv |= LPA_PAUSE_ASYM; >> + flowctrl = mii_resolve_flowctrl_fdx( >> + lcl_adv, rmt_adv); >> + if (flowctrl & FLOW_CTRL_TX) >> + tempval1 |= MACCFG1_TX_FLOW; >> + if (flowctrl & FLOW_CTRL_RX) >> + tempval1 |= MACCFG1_RX_FLOW; >> + } >> + } > > I think this bit would be nicer as a static function. > > Something like: (uncompiled/untested) > > static int gfar_get_cfg1(struct net_device *dev) > { > struct gfar_private *priv = netdev_priv(dev); > struct gfar __iomem *regs = priv->gfargrp[0].regs; > struct phy_device *phydev = priv->phydev; > int val; > > val = gfar_read(®s->maccfg1); > val &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); > > if (!phydev->duplex) > return val; > > if (!priv->pause_aneg_en) { > if (priv->tx_pause_en) > val |= MACCFG1_TX_FLOW; > if (priv->rx_pause_en) > val |= MACCFG1_RX_FLOW; > } else { > u8 flowctrl; > u16 lcl_adv = mii_advertise_flowctrl(phydev->advertising); > u16 rmt_adv = 0; > if (phydev->pause) > rmt_adv = LPA_PAUSE_CAP; > if (phydev->asym_pause) > rmt_adv |= LPA_PAUSE_ASYM; > flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv); > if (flowctrl & FLOW_CTRL_TX) > val |= MACCFG1_TX_FLOW; > if (flowctrl & FLOW_CTRL_RX) > val |= MACCFG1_RX_FLOW; > } > > return val; > } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH][net-next v2] gianfar: Add flow control support 2013-08-09 17:19 ` [PATCH][net-next v1] gianfar: Add flow control support Claudiu Manoil 2013-08-09 17:36 ` Fabio Estevam 2013-08-09 19:09 ` Joe Perches @ 2013-08-12 10:53 ` Claudiu Manoil 2013-08-13 22:29 ` David Miller 2 siblings, 1 reply; 18+ messages in thread From: Claudiu Manoil @ 2013-08-12 10:53 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Ben Hutchings, Lutz Jaenicke eTSEC has Rx and Tx flow control capabilities that may be enabled through MACCFG1[Rx_Flow, Tx_Flow] bits. These bits must not be set however when eTSEC is operated in Half-Duplex mode. Unfortunately, the driver currently sets these bits unconditionally. This patch adds the proper handling of the PAUSE frame capability register bits by implementing the ethtool -A interface. When pause autoneg is enabled, the controller uses the phy's capability to negotiate PAUSE frame settings with the link partner and reconfigures its Rx_Flow and Tx_Flow settings to match the capabilities of the link partner. If pause autoneg is off, the PAUSE frame generation may be forced manually (ethtool -A). Flow control is disabled by default now. This implementation is inspired by the tg3 driver. Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com> --- v1 - Cleaned-up version of: http://patchwork.ozlabs.org/patch/265940/ v2 - move code to gfar_get_flowctrl_cfg() - remove unused 'err' drivers/net/ethernet/freescale/gianfar.c | 51 +++++++++++++++- drivers/net/ethernet/freescale/gianfar.h | 10 +++- drivers/net/ethernet/freescale/gianfar_ethtool.c | 74 ++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index edf06f1..808b6a2 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -1016,7 +1016,14 @@ static int gfar_probe(struct platform_device *ofdev) /* We need to delay at least 3 TX clocks */ udelay(2); - tempval = (MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + tempval = 0; + if (!priv->pause_aneg_en && priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (!priv->pause_aneg_en && priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; + /* the soft reset bit is not self-resetting, so we need to + * clear it before resuming normal operation + */ gfar_write(®s->maccfg1, tempval); /* Initialize MACCFG2. */ @@ -1462,7 +1469,7 @@ static int init_phy(struct net_device *dev) struct gfar_private *priv = netdev_priv(dev); uint gigabit_support = priv->device_flags & FSL_GIANFAR_DEV_HAS_GIGABIT ? - SUPPORTED_1000baseT_Full : 0; + GFAR_SUPPORTED_GBIT : 0; phy_interface_t interface; priv->oldlink = 0; @@ -3025,6 +3032,41 @@ static irqreturn_t gfar_interrupt(int irq, void *grp_id) return IRQ_HANDLED; } +static u32 gfar_get_flowctrl_cfg(struct gfar_private *priv) +{ + struct phy_device *phydev = priv->phydev; + u32 val = 0; + + if (!phydev->duplex) + return val; + + if (!priv->pause_aneg_en) { + if (priv->tx_pause_en) + val |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + val |= MACCFG1_RX_FLOW; + } else { + u16 lcl_adv, rmt_adv; + u8 flowctrl; + /* get link partner capabilities */ + rmt_adv = 0; + if (phydev->pause) + rmt_adv = LPA_PAUSE_CAP; + if (phydev->asym_pause) + rmt_adv |= LPA_PAUSE_ASYM; + + lcl_adv = mii_advertise_flowctrl(phydev->advertising); + + flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv); + if (flowctrl & FLOW_CTRL_TX) + val |= MACCFG1_TX_FLOW; + if (flowctrl & FLOW_CTRL_RX) + val |= MACCFG1_RX_FLOW; + } + + return val; +} + /* Called every time the controller might need to be made * aware of new link state. The PHY code conveys this * information through variables in the phydev structure, and this @@ -3043,6 +3085,7 @@ static void adjust_link(struct net_device *dev) lock_tx_qs(priv); if (phydev->link) { + u32 tempval1 = gfar_read(®s->maccfg1); u32 tempval = gfar_read(®s->maccfg2); u32 ecntrl = gfar_read(®s->ecntrl); @@ -3091,6 +3134,10 @@ static void adjust_link(struct net_device *dev) priv->oldspeed = phydev->speed; } + tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + tempval1 |= gfar_get_flowctrl_cfg(priv); + + gfar_write(®s->maccfg1, tempval1); gfar_write(®s->maccfg2, tempval); gfar_write(®s->ecntrl, ecntrl); diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index ee19f2c..46f56f3 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -146,6 +146,10 @@ extern const char gfar_driver_version[]; | SUPPORTED_Autoneg \ | SUPPORTED_MII) +#define GFAR_SUPPORTED_GBIT (SUPPORTED_1000baseT_Full \ + | SUPPORTED_Pause \ + | SUPPORTED_Asym_Pause) + /* TBI register addresses */ #define MII_TBICON 0x11 @@ -1100,7 +1104,11 @@ struct gfar_private { /* Wake-on-LAN enabled */ wol_en:1, /* Enable priorty based Tx scheduling in Hw */ - prio_sched_en:1; + prio_sched_en:1, + /* Flow control flags */ + pause_aneg_en:1, + tx_pause_en:1, + rx_pause_en:1; /* The total tx and rx ring size for the enabled queues */ unsigned int total_tx_ring_size; diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c index 21cd881..d3d7ede 100644 --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c @@ -535,6 +535,78 @@ static int gfar_sringparam(struct net_device *dev, return err; } +static void gfar_gpauseparam(struct net_device *dev, + struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + + epause->autoneg = !!priv->pause_aneg_en; + epause->rx_pause = !!priv->rx_pause_en; + epause->tx_pause = !!priv->tx_pause_en; +} + +static int gfar_spauseparam(struct net_device *dev, + struct ethtool_pauseparam *epause) +{ + struct gfar_private *priv = netdev_priv(dev); + struct phy_device *phydev = priv->phydev; + struct gfar __iomem *regs = priv->gfargrp[0].regs; + u32 oldadv, newadv; + + if (!(phydev->supported & SUPPORTED_Pause) || + (!(phydev->supported & SUPPORTED_Asym_Pause) && + (epause->rx_pause != epause->tx_pause))) + return -EINVAL; + + priv->rx_pause_en = priv->tx_pause_en = 0; + if (epause->rx_pause) { + priv->rx_pause_en = 1; + + if (epause->tx_pause) { + priv->tx_pause_en = 1; + /* FLOW_CTRL_RX & TX */ + newadv = ADVERTISED_Pause; + } else /* FLOW_CTLR_RX */ + newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause; + } else if (epause->tx_pause) { + priv->tx_pause_en = 1; + /* FLOW_CTLR_TX */ + newadv = ADVERTISED_Asym_Pause; + } else + newadv = 0; + + if (epause->autoneg) + priv->pause_aneg_en = 1; + else + priv->pause_aneg_en = 0; + + oldadv = phydev->advertising & + (ADVERTISED_Pause | ADVERTISED_Asym_Pause); + if (oldadv != newadv) { + phydev->advertising &= + ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + phydev->advertising |= newadv; + if (phydev->autoneg) + /* inform link partner of our + * new flow ctrl settings + */ + return phy_start_aneg(phydev); + + if (!epause->autoneg) { + u32 tempval; + tempval = gfar_read(®s->maccfg1); + tempval &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW); + if (priv->tx_pause_en) + tempval |= MACCFG1_TX_FLOW; + if (priv->rx_pause_en) + tempval |= MACCFG1_RX_FLOW; + gfar_write(®s->maccfg1, tempval); + } + } + + return 0; +} + int gfar_set_features(struct net_device *dev, netdev_features_t features) { struct gfar_private *priv = netdev_priv(dev); @@ -1806,6 +1878,8 @@ const struct ethtool_ops gfar_ethtool_ops = { .set_coalesce = gfar_scoalesce, .get_ringparam = gfar_gringparam, .set_ringparam = gfar_sringparam, + .get_pauseparam = gfar_gpauseparam, + .set_pauseparam = gfar_spauseparam, .get_strings = gfar_gstrings, .get_sset_count = gfar_sset_count, .get_ethtool_stats = gfar_fill_stats, -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH][net-next v2] gianfar: Add flow control support 2013-08-12 10:53 ` [PATCH][net-next v2] " Claudiu Manoil @ 2013-08-13 22:29 ` David Miller 0 siblings, 0 replies; 18+ messages in thread From: David Miller @ 2013-08-13 22:29 UTC (permalink / raw) To: claudiu.manoil; +Cc: netdev, bhutchings, ljaenicke From: Claudiu Manoil <claudiu.manoil@freescale.com> Date: Mon, 12 Aug 2013 13:53:26 +0300 > eTSEC has Rx and Tx flow control capabilities that may be enabled > through MACCFG1[Rx_Flow, Tx_Flow] bits. These bits must not be set > however when eTSEC is operated in Half-Duplex mode. Unfortunately, > the driver currently sets these bits unconditionally. > This patch adds the proper handling of the PAUSE frame capability > register bits by implementing the ethtool -A interface. When pause > autoneg is enabled, the controller uses the phy's capability to > negotiate PAUSE frame settings with the link partner and reconfigures > its Rx_Flow and Tx_Flow settings to match the capabilities of the > link partner. If pause autoneg is off, the PAUSE frame generation > may be forced manually (ethtool -A). Flow control is disabled by > default now. > This implementation is inspired by the tg3 driver. > > Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com> > Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com> Applied to net-next, thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] gianfar: add support for LFC (Lossless Flow Control) 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 8:26 ` [PATCH] gianfar: implement flow control handling Lutz Jaenicke @ 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 8:39 ` Joe Perches 2013-08-09 10:12 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil 2 siblings, 1 reply; 18+ messages in thread From: Lutz Jaenicke @ 2013-08-09 8:26 UTC (permalink / raw) To: netdev; +Cc: Claudiu Manoil, Ben Hutchings, David S. Miller, Lutz Jaenicke Implement Lossless Flow Control (let the MAC automatically generate PAUSE frames in case of too much incoming traffic. Selection of this feature is performed via device tree parameter. The board (and processor) specific device tree is authoritative wrt this information. --- drivers/net/ethernet/freescale/gianfar.c | 40 +++++++++++++++++++++++++++++- drivers/net/ethernet/freescale/gianfar.h | 34 ++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 8436f00..5273492 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -169,10 +169,12 @@ static void gfar_init_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp, static int gfar_init_bds(struct net_device *ndev) { struct gfar_private *priv = netdev_priv(ndev); + struct gfar __iomem *regs = priv->gfargrp[0].regs; struct gfar_priv_tx_q *tx_queue = NULL; struct gfar_priv_rx_q *rx_queue = NULL; struct txbd8 *txbdp; struct rxbd8 *rxbdp; + u32 *rfbptr; int i, j; for (i = 0; i < priv->num_tx_queues; i++) { @@ -197,6 +199,7 @@ static int gfar_init_bds(struct net_device *ndev) txbdp->status |= TXBD_WRAP; } + rfbptr = ®s->rfbptr0; for (i = 0; i < priv->num_rx_queues; i++) { rx_queue = priv->rx_queue[i]; rx_queue->cur_rx = rx_queue->rx_bd_base; @@ -222,6 +225,11 @@ static int gfar_init_bds(struct net_device *ndev) rxbdp++; } + if (priv->device_flags & FSL_GIANFAR_DEV_HAS_LFC) + rx_queue->rfbptr = rfbptr; + else + rx_queue->rfbptr = 0; + rfbptr += 2; } @@ -340,6 +348,19 @@ static void gfar_init_tx_rx_base(struct gfar_private *priv) } } +static void gfar_init_rqprm(struct gfar_private *priv) +{ + struct gfar __iomem *regs = priv->gfargrp[0].regs; + u32 __iomem *baddr; + int i; + + baddr = ®s->rqprm0; + for(i = 0; i < priv->num_rx_queues; i++) { + gfar_write(baddr, priv->rx_queue[i]->rx_ring_size | (DEFAULT_RX_LFC_THR << FBTHR_SHIFT)); + baddr++; + } +} + static void gfar_init_mac(struct net_device *ndev) { struct gfar_private *priv = netdev_priv(ndev); @@ -348,6 +369,15 @@ static void gfar_init_mac(struct net_device *ndev) u32 tctrl = 0; u32 attrs = 0; + if (priv->device_flags & FSL_GIANFAR_DEV_HAS_LFC) { + /* Clear the LFC bit */ + gfar_write(®s->rctrl, rctrl); + /* Init flow control threshold values */ + gfar_init_rqprm(priv); + gfar_write(®s->ptv, DEFAULT_LFC_PTVVAL); + rctrl |= RCTRL_LFC; + } + /* write the tx/rx base registers */ gfar_init_tx_rx_base(priv); @@ -443,7 +473,6 @@ static struct net_device_stats *gfar_get_stats(struct net_device *dev) dev->stats.tx_bytes = tx_bytes; dev->stats.tx_packets = tx_packets; - return &dev->stats; } @@ -672,6 +701,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) const u32 *stash; const u32 *stash_len; const u32 *stash_idx; + const int *lfc_flag; unsigned int num_tx_qs, num_rx_qs; u32 *tx_queues, *rx_queues; @@ -824,6 +854,10 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) if (of_get_property(np, "fsl,magic-packet", NULL)) priv->device_flags |= FSL_GIANFAR_DEV_HAS_MAGIC_PACKET; + lfc_flag = of_get_property(np, "fsl,lossless-flow-ctrl", NULL); + if (lfc_flag && (*lfc_flag == 1)) + priv->device_flags |= FSL_GIANFAR_DEV_HAS_LFC; + priv->phy_node = of_parse_phandle(np, "phy-handle", 0); /* Find the TBI PHY. If it's not there, we don't support SGMII */ @@ -2900,6 +2934,10 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) /* Setup the new bdp */ gfar_new_rxbdp(rx_queue, bdp, newskb); + /* Update Last Free RxBD pointer for LFC */ + if (rx_queue->rfbptr) + gfar_write(rx_queue->rfbptr, (u32)bdp); + /* Update to the next pointer */ bdp = next_bd(bdp, base, rx_queue->rx_ring_size); diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index 41740e2..0a6ae93 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -104,6 +104,10 @@ extern const char gfar_driver_version[]; #define GFAR_MAX_FIFO_STARVE 511 #define GFAR_MAX_FIFO_STARVE_OFF 511 +#define FBTHR_SHIFT 24 +#define DEFAULT_RX_LFC_THR 16 +#define DEFAULT_LFC_PTVVAL 4 + #define DEFAULT_RX_BUFFER_SIZE 1536 #define TX_RING_MOD_MASK(size) (size-1) #define RX_RING_MOD_MASK(size) (size-1) @@ -278,6 +282,7 @@ extern const char gfar_driver_version[]; #define RCTRL_TS_ENABLE 0x01000000 #define RCTRL_PAL_MASK 0x001f0000 +#define RCTRL_LFC 0x00004000 #define RCTRL_VLEX 0x00002000 #define RCTRL_FILREN 0x00001000 #define RCTRL_GHTX 0x00000400 @@ -850,7 +855,32 @@ struct gfar { u8 res23c[248]; u32 attr; /* 0x.bf8 - Attributes Register */ u32 attreli; /* 0x.bfc - Attributes Extract Length and Extract Index Register */ - u8 res24[688]; + u32 rqprm0; /* 0x.c00 - Receive queue parameters register 0 */ + u32 rqprm1; /* 0x.c04 - Receive queue parameters register 1 */ + u32 rqprm2; /* 0x.c08 - Receive queue parameters register 2 */ + u32 rqprm3; /* 0x.c0c - Receive queue parameters register 3 */ + u32 rqprm4; /* 0x.c10 - Receive queue parameters register 4 */ + u32 rqprm5; /* 0x.c14 - Receive queue parameters register 5 */ + u32 rqprm6; /* 0x.c18 - Receive queue parameters register 6 */ + u32 rqprm7; /* 0x.c1c - Receive queue parameters register 7 */ + u8 res24[36]; + u32 rfbptr0; /* 0x.c44 - Last free RxBD pointer for ring 0 */ + u8 res24a[4]; + u32 rfbptr1; /* 0x.c4c - Last free RxBD pointer for ring 1 */ + u8 res24b[4]; + u32 rfbptr2; /* 0x.c54 - Last free RxBD pointer for ring 2 */ + u8 res24c[4]; + u32 rfbptr3; /* 0x.c5c - Last free RxBD pointer for ring 3 */ + u8 res24d[4]; + u32 rfbptr4; /* 0x.c64 - Last free RxBD pointer for ring 4 */ + u8 res24e[4]; + u32 rfbptr5; /* 0x.c6c - Last free RxBD pointer for ring 5 */ + u8 res24f[4]; + u32 rfbptr6; /* 0x.c74 - Last free RxBD pointer for ring 6 */ + u8 res24g[4]; + u32 rfbptr7; /* 0x.c7c - Last free RxBD pointer for ring 7 */ + u8 res24h[4]; + u8 res24x[556]; u32 isrg0; /* 0x.eb0 - Interrupt steering group 0 register */ u32 isrg1; /* 0x.eb4 - Interrupt steering group 1 register */ u32 isrg2; /* 0x.eb8 - Interrupt steering group 2 register */ @@ -889,6 +919,7 @@ struct gfar { #define FSL_GIANFAR_DEV_HAS_BD_STASHING 0x00000200 #define FSL_GIANFAR_DEV_HAS_BUF_STASHING 0x00000400 #define FSL_GIANFAR_DEV_HAS_TIMER 0x00000800 +#define FSL_GIANFAR_DEV_HAS_LFC 0x80000000 #if (MAXGROUPS == 2) #define DEFAULT_MAPPING 0xAA @@ -998,6 +1029,7 @@ struct gfar_priv_rx_q { /* RX Coalescing values */ unsigned char rxcoalescing; unsigned long rxic; + u32 *rfbptr; }; /** -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] gianfar: add support for LFC (Lossless Flow Control) 2013-08-09 8:26 ` [PATCH] gianfar: add support for LFC (Lossless Flow Control) Lutz Jaenicke @ 2013-08-09 8:39 ` Joe Perches 0 siblings, 0 replies; 18+ messages in thread From: Joe Perches @ 2013-08-09 8:39 UTC (permalink / raw) To: Lutz Jaenicke; +Cc: netdev, Claudiu Manoil, Ben Hutchings, David S. Miller On Fri, 2013-08-09 at 10:26 +0200, Lutz Jaenicke wrote: > Implement Lossless Flow Control (let the MAC automatically generate > PAUSE frames in case of too much incoming traffic. > Selection of this feature is performed via device tree parameter. > The board (and processor) specific device tree is authoritative wrt this > information. Just a trivial note: > diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c [] > @@ -169,10 +169,12 @@ static void gfar_init_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp, [] > + u32 *rfbptr; [] > @@ -222,6 +225,11 @@ static int gfar_init_bds(struct net_device *ndev) [] > + if (priv->device_flags & FSL_GIANFAR_DEV_HAS_LFC) > + rx_queue->rfbptr = rfbptr; > + else > + rx_queue->rfbptr = 0; s/0/NULL/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 8:26 ` [PATCH] gianfar: implement flow control handling Lutz Jaenicke 2013-08-09 8:26 ` [PATCH] gianfar: add support for LFC (Lossless Flow Control) Lutz Jaenicke @ 2013-08-09 10:12 ` Claudiu Manoil 2013-08-09 10:15 ` Lutz Jaenicke 2 siblings, 1 reply; 18+ messages in thread From: Claudiu Manoil @ 2013-08-09 10:12 UTC (permalink / raw) To: Lutz Jaenicke; +Cc: netdev, Ben Hutchings, David S. Miller Thanks, First patch seems to include the complete implementation of ethtool -A, fixing the pause issue for half-duplex links as well, and it also implements pause autoneg. (nice) I'm going to apply an test this code and re-spin the resulting patch(es). Are you the author of this patch? (gianfar: implement flow control handling) Claudiu On 8/9/2013 11:26 AM, Lutz Jaenicke wrote: > I have been working on the same issue and came up with a similar > modification (that additionally implements LFC if supported) > I have attached the two patches created: > * gianfar: implement flow control handling > which looks quite similar to Claudiu's patches > * gianfar: add support for LFC (Lossless Flow Control) > which is more or less just extracted from some patch I found on the internet > linux-2.6.29.6-mpc8308e_rdb.patch > The patches I have taken from a custom 3.4 based kernel so they may not > apply cleanly but maybe bits of it can be used to generate fully supported > solution. > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame 2013-08-09 10:12 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil @ 2013-08-09 10:15 ` Lutz Jaenicke 0 siblings, 0 replies; 18+ messages in thread From: Lutz Jaenicke @ 2013-08-09 10:15 UTC (permalink / raw) To: Claudiu Manoil; +Cc: netdev, Ben Hutchings, David S. Miller On Fri, Aug 09, 2013 at 01:12:15PM +0300, Claudiu Manoil wrote: > Thanks, > > First patch seems to include the complete implementation of ethtool -A, > fixing the pause issue for half-duplex links as well, and it also > implements pause autoneg. (nice) > I'm going to apply an test this code and re-spin the resulting > patch(es). > Are you the author of this patch? > (gianfar: implement flow control handling) Yes. Best regards, Lutz -- Dr.-Ing. Lutz Jänicke CTO Innominate Security Technologies AG /protecting industrial networks/ tel: +49.30.921028-200 fax: +49.30.921028-020 Rudower Chaussee 13 D-12489 Berlin, Germany www.innominate.com Register Court: AG Charlottenburg, HR B 81603 Management Board: Dirk Seewald Chairman of the Supervisory Board: Christoph Leifer ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-08-13 22:29 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-07 10:24 [PATCH 1/2][net-next] gianfar: Fix pause frame handling for half duplex links Claudiu Manoil 2013-08-07 10:24 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil 2013-08-07 19:12 ` Ben Hutchings 2013-08-08 17:10 ` Claudiu Manoil 2013-08-08 18:45 ` Ben Hutchings 2013-08-09 8:26 ` Lutz Jaenicke 2013-08-09 8:26 ` [PATCH] gianfar: implement flow control handling Lutz Jaenicke 2013-08-09 17:19 ` [PATCH][net-next v1] gianfar: Add flow control support Claudiu Manoil 2013-08-09 17:36 ` Fabio Estevam 2013-08-12 7:09 ` Claudiu Manoil 2013-08-09 19:09 ` Joe Perches 2013-08-12 7:08 ` Claudiu Manoil 2013-08-12 10:53 ` [PATCH][net-next v2] " Claudiu Manoil 2013-08-13 22:29 ` David Miller 2013-08-09 8:26 ` [PATCH] gianfar: add support for LFC (Lossless Flow Control) Lutz Jaenicke 2013-08-09 8:39 ` Joe Perches 2013-08-09 10:12 ` [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame Claudiu Manoil 2013-08-09 10:15 ` Lutz Jaenicke
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.