[PATCH 1/2][net-next] gianfar: Fix pause frame handling for half duplex links

[PATCH 1/2][net-next] gianfar: Fix pause frame handling for half duplex links

[Claudiu Manoil]

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(&regs->maccfg1, tempval);
+	/* the soft reset bit is not self-resetting, so we need to
+	 * clear it before resuming normal operation
+	 */
+	gfar_write(&regs->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(&regs->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(&regs->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

[PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Claudiu Manoil]

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

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Ben Hutchings]

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.

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Claudiu Manoil]

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);
> [...]
>

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Ben Hutchings]

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.

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Lutz Jaenicke]

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.

[PATCH] gianfar: implement flow control handling

[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(&regs->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(&regs->maccfg1);
 		u32 tempval = gfar_read(&regs->maccfg2);
 		u32 ecntrl = gfar_read(&regs->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(&regs->maccfg1, tempval1);
 		gfar_write(&regs->maccfg2, tempval);
 		gfar_write(&regs->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(&regs->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(&regs->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

[PATCH] gianfar: add support for LFC (Lossless Flow Control)

[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 = &regs->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 = &regs->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(&regs->rctrl, rctrl);
+		/* Init flow control threshold values */
+		gfar_init_rqprm(priv);
+		gfar_write(&regs->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

Re: [PATCH] gianfar: add support for LFC (Lossless Flow Control)

[Joe Perches]

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/

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Claudiu Manoil]

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

Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame

[Lutz Jaenicke]

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

[PATCH][net-next v1] gianfar: Add flow control support

[Claudiu Manoil]

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(&regs->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(&regs->maccfg1);
 		u32 tempval = gfar_read(&regs->maccfg2);
 		u32 ecntrl = gfar_read(&regs->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(&regs->maccfg1, tempval1);
 		gfar_write(&regs->maccfg2, tempval);
 		gfar_write(&regs->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(&regs->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(&regs->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

Re: [PATCH][net-next v1] gianfar: Add flow control support

[Fabio Estevam]

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.

Re: [PATCH][net-next v1] gianfar: Add flow control support

[Joe Perches]

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(&regs->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(&regs->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;
}

Re: [PATCH][net-next v1] gianfar: Add flow control support

[Claudiu Manoil]

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(&regs->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(&regs->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;
> }
>

Re: [PATCH][net-next v1] gianfar: Add flow control support

[Claudiu Manoil]

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

[PATCH][net-next v2] gianfar: Add flow control support

[Claudiu Manoil]

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(&regs->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(&regs->maccfg1);
 		u32 tempval = gfar_read(&regs->maccfg2);
 		u32 ecntrl = gfar_read(&regs->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(&regs->maccfg1, tempval1);
 		gfar_write(&regs->maccfg2, tempval);
 		gfar_write(&regs->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(&regs->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(&regs->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

Re: [PATCH][net-next v2] gianfar: Add flow control support

[David Miller]

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.