All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers.
@ 2013-05-17 15:19 Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 1/4] tg3: Add ethtool_eee struct and tg3_setup_eee() Nithin Nayak Sujir
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nithin Nayak Sujir @ 2013-05-17 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nithin Nayak Sujir

This series adds support for modifying EEE settings via ethtool. Since this can
impact Link Flap Avoidance, the driver pulls the current hardware settings if
LFA is enabled. This is similar to how we do the link settings to avoid a flap.

Nithin Nayak Sujir (4):
  tg3: Add ethtool_eee struct and tg3_setup_eee()
  tg3: Add tg3_eee_pull_config() function
  tg3: Simplify tg3_phy_eee_config_ok() by reusing tg3_eee_pull_config()
  tg3: Implement set/get_eee handlers

 drivers/net/ethernet/broadcom/tg3.c | 227 +++++++++++++++++++++++++++---------
 drivers/net/ethernet/broadcom/tg3.h |   1 +
 2 files changed, 174 insertions(+), 54 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/4] tg3: Add ethtool_eee struct and tg3_setup_eee()
  2013-05-17 15:19 [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers Nithin Nayak Sujir
@ 2013-05-17 15:19 ` Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function Nithin Nayak Sujir
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Nithin Nayak Sujir @ 2013-05-17 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nithin Nayak Sujir, Michael Chan

Add an eee structure and update it with eee settings. This will be used
for set/get_eee operations. Add common function tg3_setup_eee() that
will be used in the subsequent patches.

Reviewed-by: Ben Li <benli@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 91 +++++++++++++++++++++++--------------
 drivers/net/ethernet/broadcom/tg3.h |  1 +
 2 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 77f1fd3..91efe64 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4261,6 +4261,16 @@ static int tg3_phy_autoneg_cfg(struct tg3 *tp, u32 advertise, u32 flowctrl)
 		/* Advertise 1000-BaseT EEE ability */
 		if (advertise & ADVERTISED_1000baseT_Full)
 			val |= MDIO_AN_EEE_ADV_1000T;
+
+		if (!tp->eee.eee_enabled) {
+			val = 0;
+			tp->eee.advertised = 0;
+		} else {
+			tp->eee.advertised = advertise &
+					     (ADVERTISED_100baseT_Full |
+					      ADVERTISED_1000baseT_Full);
+		}
+
 		err = tg3_phy_cl45_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
 		if (err)
 			val = 0;
@@ -4625,6 +4635,42 @@ static void tg3_clear_mac_status(struct tg3 *tp)
 	udelay(40);
 }
 
+static void tg3_setup_eee(struct tg3 *tp)
+{
+	u32 val;
+
+	val = TG3_CPMU_EEE_LNKIDL_PCIE_NL0 |
+	      TG3_CPMU_EEE_LNKIDL_UART_IDL;
+	if (tg3_chip_rev_id(tp) == CHIPREV_ID_57765_A0)
+		val |= TG3_CPMU_EEE_LNKIDL_APE_TX_MT;
+
+	tw32_f(TG3_CPMU_EEE_LNKIDL_CTRL, val);
+
+	tw32_f(TG3_CPMU_EEE_CTRL,
+	       TG3_CPMU_EEE_CTRL_EXIT_20_1_US);
+
+	val = TG3_CPMU_EEEMD_ERLY_L1_XIT_DET |
+	      (tp->eee.tx_lpi_enabled ? TG3_CPMU_EEEMD_LPI_IN_TX : 0) |
+	      TG3_CPMU_EEEMD_LPI_IN_RX |
+	      TG3_CPMU_EEEMD_EEE_ENABLE;
+
+	if (tg3_asic_rev(tp) != ASIC_REV_5717)
+		val |= TG3_CPMU_EEEMD_SND_IDX_DET_EN;
+
+	if (tg3_flag(tp, ENABLE_APE))
+		val |= TG3_CPMU_EEEMD_APE_TX_DET_EN;
+
+	tw32_f(TG3_CPMU_EEE_MODE, tp->eee.eee_enabled ? val : 0);
+
+	tw32_f(TG3_CPMU_EEE_DBTMR1,
+	       TG3_CPMU_DBTMR1_PCIEXIT_2047US |
+	       (tp->eee.tx_lpi_timer & 0xffff));
+
+	tw32_f(TG3_CPMU_EEE_DBTMR2,
+	       TG3_CPMU_DBTMR2_APE_TX_2047US |
+	       TG3_CPMU_DBTMR2_TXIDXEQ_2047US);
+}
+
 static int tg3_setup_copper_phy(struct tg3 *tp, bool force_reset)
 {
 	bool current_link_up;
@@ -9477,38 +9523,8 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 		tg3_abort_hw(tp, 1);
 
 	/* Enable MAC control of LPI */
-	if (tp->phy_flags & TG3_PHYFLG_EEE_CAP) {
-		val = TG3_CPMU_EEE_LNKIDL_PCIE_NL0 |
-		      TG3_CPMU_EEE_LNKIDL_UART_IDL;
-		if (tg3_chip_rev_id(tp) == CHIPREV_ID_57765_A0)
-			val |= TG3_CPMU_EEE_LNKIDL_APE_TX_MT;
-
-		tw32_f(TG3_CPMU_EEE_LNKIDL_CTRL, val);
-
-		tw32_f(TG3_CPMU_EEE_CTRL,
-		       TG3_CPMU_EEE_CTRL_EXIT_20_1_US);
-
-		val = TG3_CPMU_EEEMD_ERLY_L1_XIT_DET |
-		      TG3_CPMU_EEEMD_LPI_IN_TX |
-		      TG3_CPMU_EEEMD_LPI_IN_RX |
-		      TG3_CPMU_EEEMD_EEE_ENABLE;
-
-		if (tg3_asic_rev(tp) != ASIC_REV_5717)
-			val |= TG3_CPMU_EEEMD_SND_IDX_DET_EN;
-
-		if (tg3_flag(tp, ENABLE_APE))
-			val |= TG3_CPMU_EEEMD_APE_TX_DET_EN;
-
-		tw32_f(TG3_CPMU_EEE_MODE, val);
-
-		tw32_f(TG3_CPMU_EEE_DBTMR1,
-		       TG3_CPMU_DBTMR1_PCIEXIT_2047US |
-		       TG3_CPMU_DBTMR1_LNKIDLE_2047US);
-
-		tw32_f(TG3_CPMU_EEE_DBTMR2,
-		       TG3_CPMU_DBTMR2_APE_TX_2047US |
-		       TG3_CPMU_DBTMR2_TXIDXEQ_2047US);
-	}
+	if (tp->phy_flags & TG3_PHYFLG_EEE_CAP)
+		tg3_setup_eee(tp);
 
 	if ((tp->phy_flags & TG3_PHYFLG_KEEP_LINK_ON_PWRDN) &&
 	    !(tp->phy_flags & TG3_PHYFLG_USER_CONFIGURED)) {
@@ -14975,9 +14991,18 @@ static int tg3_phy_probe(struct tg3 *tp)
 	     (tg3_asic_rev(tp) == ASIC_REV_5717 &&
 	      tg3_chip_rev_id(tp) != CHIPREV_ID_5717_A0) ||
 	     (tg3_asic_rev(tp) == ASIC_REV_57765 &&
-	      tg3_chip_rev_id(tp) != CHIPREV_ID_57765_A0)))
+	      tg3_chip_rev_id(tp) != CHIPREV_ID_57765_A0))) {
 		tp->phy_flags |= TG3_PHYFLG_EEE_CAP;
 
+		tp->eee.supported = SUPPORTED_100baseT_Full |
+				    SUPPORTED_1000baseT_Full;
+		tp->eee.advertised = ADVERTISED_100baseT_Full |
+				     ADVERTISED_1000baseT_Full;
+		tp->eee.eee_enabled = 1;
+		tp->eee.tx_lpi_enabled = 1;
+		tp->eee.tx_lpi_timer = TG3_CPMU_DBTMR1_LNKIDLE_2047US;
+	}
+
 	tg3_phy_init_link_config(tp);
 
 	if (!(tp->phy_flags & TG3_PHYFLG_KEEP_LINK_ON_PWRDN) &&
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 9b2d3ac..057532c 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3371,6 +3371,7 @@ struct tg3 {
 	unsigned int			irq_cnt;
 
 	struct ethtool_coalesce		coal;
+	struct ethtool_eee		eee;
 
 	/* firmware info */
 	const char			*fw_needed;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function
  2013-05-17 15:19 [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 1/4] tg3: Add ethtool_eee struct and tg3_setup_eee() Nithin Nayak Sujir
@ 2013-05-17 15:19 ` Nithin Nayak Sujir
  2013-05-18  1:05   ` Ben Hutchings
  2013-05-17 15:19 ` [PATCH net-next 3/4] tg3: Simplify tg3_phy_eee_config_ok() by reusing tg3_eee_pull_config() Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 4/4] tg3: Implement set/get_eee handlers Nithin Nayak Sujir
  3 siblings, 1 reply; 7+ messages in thread
From: Nithin Nayak Sujir @ 2013-05-17 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nithin Nayak Sujir, Michael Chan

Add tg3_eee_pull_config() to pull the settings from the hardware and
populate the eee structure.

If Link Flap Avoidance is enabled, we pull the eee settings from the hw
so as not to cause a phy reset on eee config mismatch later. This
requires moving down tg3_setup_eee() below the tg3_pull_config() to not
trample existing settings.

Reviewed-by: Ben Li <benli@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 62 +++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 91efe64..75b38c4 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -2320,6 +2320,52 @@ static void tg3_phy_apply_otp(struct tg3 *tp)
 	tg3_phy_toggle_auxctl_smdsp(tp, false);
 }
 
+static void tg3_eee_pull_config(struct tg3 *tp, struct ethtool_eee *eee)
+{
+	u32 val;
+	struct ethtool_eee *dest = &tp->eee;
+
+	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP))
+		return;
+
+	if (eee)
+		dest = eee;
+
+	if (tg3_phy_cl45_read(tp, MDIO_MMD_AN, TG3_CL45_D7_EEERES_STAT, &val))
+		return;
+
+	/* Pull eee_active and lp_advertised. */
+	if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T ||
+	    val == TG3_CL45_D7_EEERES_STAT_LP_100TX) {
+		dest->eee_active = 1;
+	} else
+		dest->eee_active = 0;
+
+	if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T)
+		dest->lp_advertised = ADVERTISED_1000baseT_Full;
+	else
+		dest->lp_advertised = ADVERTISED_100baseT_Full;
+
+	/* Pull advertised and eee_enabled settings */
+	if (tg3_phy_cl45_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, &val))
+		return;
+
+	dest->eee_enabled = !!val;
+
+	dest->advertised = 0;
+	if (val & MDIO_AN_EEE_ADV_100TX)
+		dest->advertised = ADVERTISED_100baseT_Full;
+	if (val & MDIO_AN_EEE_ADV_1000T)
+		dest->advertised |= ADVERTISED_1000baseT_Full;
+
+	/* Pull tx_lpi_enabled */
+	val = tr32(TG3_CPMU_EEE_MODE);
+	dest->tx_lpi_enabled = !!(val & TG3_CPMU_EEEMD_LPI_IN_TX);
+
+	/* Pull lpi timer value */
+	dest->tx_lpi_timer = tr32(TG3_CPMU_EEE_DBTMR1) & 0xffff;
+}
+
 static void tg3_phy_eee_adjust(struct tg3 *tp, bool current_link_up)
 {
 	u32 val;
@@ -2343,11 +2389,8 @@ static void tg3_phy_eee_adjust(struct tg3 *tp, bool current_link_up)
 
 		tw32(TG3_CPMU_EEE_CTRL, eeectl);
 
-		tg3_phy_cl45_read(tp, MDIO_MMD_AN,
-				  TG3_CL45_D7_EEERES_STAT, &val);
-
-		if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T ||
-		    val == TG3_CL45_D7_EEERES_STAT_LP_100TX)
+		tg3_eee_pull_config(tp, NULL);
+		if (tp->eee.eee_active)
 			tp->setlpicnt = 2;
 	}
 
@@ -9522,16 +9565,17 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
 	if (tg3_flag(tp, INIT_COMPLETE))
 		tg3_abort_hw(tp, 1);
 
-	/* Enable MAC control of LPI */
-	if (tp->phy_flags & TG3_PHYFLG_EEE_CAP)
-		tg3_setup_eee(tp);
-
 	if ((tp->phy_flags & TG3_PHYFLG_KEEP_LINK_ON_PWRDN) &&
 	    !(tp->phy_flags & TG3_PHYFLG_USER_CONFIGURED)) {
 		tg3_phy_pull_config(tp);
+		tg3_eee_pull_config(tp, NULL);
 		tp->phy_flags |= TG3_PHYFLG_USER_CONFIGURED;
 	}
 
+	/* Enable MAC control of LPI */
+	if (tp->phy_flags & TG3_PHYFLG_EEE_CAP)
+		tg3_setup_eee(tp);
+
 	if (reset_phy)
 		tg3_phy_reset(tp);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 3/4] tg3: Simplify tg3_phy_eee_config_ok() by reusing tg3_eee_pull_config()
  2013-05-17 15:19 [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 1/4] tg3: Add ethtool_eee struct and tg3_setup_eee() Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function Nithin Nayak Sujir
@ 2013-05-17 15:19 ` Nithin Nayak Sujir
  2013-05-17 15:19 ` [PATCH net-next 4/4] tg3: Implement set/get_eee handlers Nithin Nayak Sujir
  3 siblings, 0 replies; 7+ messages in thread
From: Nithin Nayak Sujir @ 2013-05-17 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nithin Nayak Sujir, Michael Chan

eee_config_ok() was checking only for mismatch in advertised settings.
This patch expands the scope of eee_config_ok() to check for mismatch in
the other eee settings. On mismatch we will require a call setup_eee()
to push the configured settings to the hardware.

Reviewed-by: Ben Li <benli@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 75b38c4..3aeb98f 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4558,26 +4558,27 @@ static int tg3_init_5401phy_dsp(struct tg3 *tp)
 
 static bool tg3_phy_eee_config_ok(struct tg3 *tp)
 {
-	u32 val;
-	u32 tgtadv = 0;
-	u32 advertising = tp->link_config.advertising;
+	struct ethtool_eee eee;
 
 	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP))
 		return true;
 
-	if (tg3_phy_cl45_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, &val))
-		return false;
-
-	val &= (MDIO_AN_EEE_ADV_100TX | MDIO_AN_EEE_ADV_1000T);
+	tg3_eee_pull_config(tp, &eee);
 
+	if (tp->eee.eee_enabled) {
+		if (tp->eee.advertised != eee.advertised)
+			return false;
 
-	if (advertising & ADVERTISED_100baseT_Full)
-		tgtadv |= MDIO_AN_EEE_ADV_100TX;
-	if (advertising & ADVERTISED_1000baseT_Full)
-		tgtadv |= MDIO_AN_EEE_ADV_1000T;
+		if (tp->eee.tx_lpi_timer != eee.tx_lpi_timer)
+			return false;
 
-	if (val != tgtadv)
-		return false;
+		if (tp->eee.tx_lpi_enabled != eee.tx_lpi_enabled)
+			return false;
+	} else {
+		/* EEE is disabled but we're advertising */
+		if (eee.advertised)
+			return false;
+	}
 
 	return true;
 }
@@ -4880,8 +4881,10 @@ static int tg3_setup_copper_phy(struct tg3 *tp, bool force_reset)
 			 */
 			if (!eee_config_ok &&
 			    (tp->phy_flags & TG3_PHYFLG_KEEP_LINK_ON_PWRDN) &&
-			    !force_reset)
+			    !force_reset) {
+				tg3_setup_eee(tp);
 				tg3_phy_reset(tp);
+			}
 		} else {
 			if (!(bmcr & BMCR_ANENABLE) &&
 			    tp->link_config.speed == current_speed &&
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 4/4] tg3: Implement set/get_eee handlers
  2013-05-17 15:19 [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers Nithin Nayak Sujir
                   ` (2 preceding siblings ...)
  2013-05-17 15:19 ` [PATCH net-next 3/4] tg3: Simplify tg3_phy_eee_config_ok() by reusing tg3_eee_pull_config() Nithin Nayak Sujir
@ 2013-05-17 15:19 ` Nithin Nayak Sujir
  2013-05-18  1:08   ` Ben Hutchings
  3 siblings, 1 reply; 7+ messages in thread
From: Nithin Nayak Sujir @ 2013-05-17 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nithin Nayak Sujir, Michael Chan

Reviewed-by: Ben Li <benli@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 3aeb98f..7d4f664 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -13657,6 +13657,51 @@ static int tg3_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec)
 	return 0;
 }
 
+static int tg3_set_eee(struct net_device *dev, struct ethtool_eee *edata)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP)) {
+		netdev_warn(tp->dev,
+			    "Board does not support EEE!\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (edata->advertised != tp->eee.advertised) {
+		netdev_warn(tp->dev,
+			    "Direct manipulation of EEE advertisement is not supported\n");
+		return -EINVAL;
+	}
+
+	tp->eee = *edata;
+
+	tp->phy_flags |= TG3_PHYFLG_USER_CONFIGURED;
+	tg3_warn_mgmt_link_flap(tp);
+
+	if (netif_running(tp->dev)) {
+		tg3_full_lock(tp, 0);
+		tg3_setup_eee(tp);
+		tg3_phy_reset(tp);
+		tg3_full_unlock(tp);
+	}
+
+	return 0;
+}
+
+static int tg3_get_eee(struct net_device *dev, struct ethtool_eee *edata)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP)) {
+		netdev_warn(tp->dev,
+			    "Board does not support EEE!\n");
+		return -EOPNOTSUPP;
+	}
+
+	*edata = tp->eee;
+	return 0;
+}
+
 static const struct ethtool_ops tg3_ethtool_ops = {
 	.get_settings		= tg3_get_settings,
 	.set_settings		= tg3_set_settings,
@@ -13690,6 +13735,8 @@ static const struct ethtool_ops tg3_ethtool_ops = {
 	.get_channels		= tg3_get_channels,
 	.set_channels		= tg3_set_channels,
 	.get_ts_info		= tg3_get_ts_info,
+	.get_eee		= tg3_get_eee,
+	.set_eee		= tg3_set_eee,
 };
 
 static struct rtnl_link_stats64 *tg3_get_stats64(struct net_device *dev,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function
  2013-05-17 15:19 ` [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function Nithin Nayak Sujir
@ 2013-05-18  1:05   ` Ben Hutchings
  0 siblings, 0 replies; 7+ messages in thread
From: Ben Hutchings @ 2013-05-18  1:05 UTC (permalink / raw)
  To: Nithin Nayak Sujir; +Cc: davem, netdev, Michael Chan

On Fri, 2013-05-17 at 08:19 -0700, Nithin Nayak Sujir wrote:
> Add tg3_eee_pull_config() to pull the settings from the hardware and
> populate the eee structure.
> 
> If Link Flap Avoidance is enabled, we pull the eee settings from the hw
> so as not to cause a phy reset on eee config mismatch later. This
> requires moving down tg3_setup_eee() below the tg3_pull_config() to not
> trample existing settings.
> 
> Reviewed-by: Ben Li <benli@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 62 +++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 91efe64..75b38c4 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -2320,6 +2320,52 @@ static void tg3_phy_apply_otp(struct tg3 *tp)
>  	tg3_phy_toggle_auxctl_smdsp(tp, false);
>  }
>  
> +static void tg3_eee_pull_config(struct tg3 *tp, struct ethtool_eee *eee)
> +{
> +	u32 val;
> +	struct ethtool_eee *dest = &tp->eee;
> +
> +	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP))
> +		return;
> +
> +	if (eee)
> +		dest = eee;
> +
> +	if (tg3_phy_cl45_read(tp, MDIO_MMD_AN, TG3_CL45_D7_EEERES_STAT, &val))
> +		return;
> +
> +	/* Pull eee_active and lp_advertised. */
> +	if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T ||
> +	    val == TG3_CL45_D7_EEERES_STAT_LP_100TX) {
> +		dest->eee_active = 1;
> +	} else
> +		dest->eee_active = 0;
> +
> +	if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T)
> +		dest->lp_advertised = ADVERTISED_1000baseT_Full;
> +	else
> +		dest->lp_advertised = ADVERTISED_100baseT_Full;
[...]

This is wrong; you're looking at the autoneg result, not the link
partner advertising mask (7.61, MDIO_AN_EEE_LPABLE).  The link partner
might be advertising multiple modes and you should report them all
(including modes the local PHY doesn't support).  This possibly belongs
in a generic function in the mdio module.

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] 7+ messages in thread

* Re: [PATCH net-next 4/4] tg3: Implement set/get_eee handlers
  2013-05-17 15:19 ` [PATCH net-next 4/4] tg3: Implement set/get_eee handlers Nithin Nayak Sujir
@ 2013-05-18  1:08   ` Ben Hutchings
  0 siblings, 0 replies; 7+ messages in thread
From: Ben Hutchings @ 2013-05-18  1:08 UTC (permalink / raw)
  To: Nithin Nayak Sujir; +Cc: davem, netdev, Michael Chan

On Fri, 2013-05-17 at 08:19 -0700, Nithin Nayak Sujir wrote:
> Reviewed-by: Ben Li <benli@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 47 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 3aeb98f..7d4f664 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -13657,6 +13657,51 @@ static int tg3_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec)
>  	return 0;
>  }
>  
> +static int tg3_set_eee(struct net_device *dev, struct ethtool_eee *edata)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +
> +	if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP)) {
> +		netdev_warn(tp->dev,
> +			    "Board does not support EEE!\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (edata->advertised != tp->eee.advertised) {
> +		netdev_warn(tp->dev,
> +			    "Direct manipulation of EEE advertisement is not supported\n");
> +		return -EINVAL;
> +	}
[...]

This should also check that edata->tx_lpi_timer is within the valid
range (16-bit, if I read your first patch right).

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] 7+ messages in thread

end of thread, other threads:[~2013-05-18  1:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-17 15:19 [PATCH net-next 0/4] tg3: Add set_eee and get_eee handlers Nithin Nayak Sujir
2013-05-17 15:19 ` [PATCH net-next 1/4] tg3: Add ethtool_eee struct and tg3_setup_eee() Nithin Nayak Sujir
2013-05-17 15:19 ` [PATCH net-next 2/4] tg3: Add tg3_eee_pull_config() function Nithin Nayak Sujir
2013-05-18  1:05   ` Ben Hutchings
2013-05-17 15:19 ` [PATCH net-next 3/4] tg3: Simplify tg3_phy_eee_config_ok() by reusing tg3_eee_pull_config() Nithin Nayak Sujir
2013-05-17 15:19 ` [PATCH net-next 4/4] tg3: Implement set/get_eee handlers Nithin Nayak Sujir
2013-05-18  1:08   ` Ben Hutchings

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.