[U-Boot] [PATCH] phylib: Detect link on 10G devices correctly

[U-Boot] [PATCH] phylib: Detect link on 10G devices correctly

[Andy Fleming]

gen10g_startup() had 2 bugs:

1) It had a boolean logic error in checking the MMD mask, and
always checked all of them.

2) It checked devices which don't actually report link state, which
meant that it would never believe the link was fully up.

Fix the boolean logic, and then mask the MMD mask so only link-reporting
devices are checked.

Signed-off-by: Andy Fleming <afleming@freescale.com>
Reported-by: Ed Swarthout <Ed.Swarthout@freescale.com>
---

 drivers/net/phy/generic_10g.c |    8 ++++++--
 include/linux/mdio.h          |    8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/generic_10g.c b/drivers/net/phy/generic_10g.c
index 315c508..1c3e69e 100644
--- a/drivers/net/phy/generic_10g.c
+++ b/drivers/net/phy/generic_10g.c
@@ -36,7 +36,7 @@ int gen10g_shutdown(struct phy_device *phydev)
 int gen10g_startup(struct phy_device *phydev)
 {
 	int devad, reg;
-	u32 mmd_mask = phydev->mmds;
+	u32 mmd_mask = phydev->mmds & MDIO_DEVS_LINK;
 
 	phydev->link = 1;
 
@@ -44,8 +44,12 @@ int gen10g_startup(struct phy_device *phydev)
 	phydev->speed = SPEED_10000;
 	phydev->duplex = DUPLEX_FULL;
 
+	/*
+	 * Go through all the link-reporting devices, and make sure
+	 * they're all up and happy
+	 */
 	for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
-		if (!mmd_mask & 1)
+		if (!(mmd_mask & 1))
 			continue;
 
 		/* Read twice because link state is latched and a
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 022d772..f2080dd 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -120,6 +120,14 @@
 #define MDIO_DEVS_VEND1			MDIO_DEVS_PRESENT(MDIO_MMD_VEND1)
 #define MDIO_DEVS_VEND2			MDIO_DEVS_PRESENT(MDIO_MMD_VEND2)
 
+#define MDIO_DEVS_LINK			(MDIO_DEVS_PMAPMD | \
+					MDIO_DEVS_WIS | \
+					MDIO_DEVS_PCS | \
+					MDIO_DEVS_PHYXS | \
+					MDIO_DEVS_DTEXS | \
+					MDIO_DEVS_AN)
+					
+
 
 /* Control register 2. */
 #define MDIO_PMA_CTRL2_TYPE		0x000f	/* PMA/PMD type selection */
-- 
1.6.5.2.g6ff9a

[U-Boot] [PATCH] phylib: Detect link on 10G devices correctly

[Detlev Zundel]

Hi Andy,

> gen10g_startup() had 2 bugs:
>
> 1) It had a boolean logic error in checking the MMD mask, and
> always checked all of them.

Good catch.

> 2) It checked devices which don't actually report link state, which
> meant that it would never believe the link was fully up.
>
> Fix the boolean logic, and then mask the MMD mask so only link-reporting
> devices are checked.
>
> Signed-off-by: Andy Fleming <afleming@freescale.com>
> Reported-by: Ed Swarthout <Ed.Swarthout@freescale.com>
> ---
>
>  drivers/net/phy/generic_10g.c |    8 ++++++--
>  include/linux/mdio.h          |    8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/generic_10g.c b/drivers/net/phy/generic_10g.c
> index 315c508..1c3e69e 100644
> --- a/drivers/net/phy/generic_10g.c
> +++ b/drivers/net/phy/generic_10g.c
> @@ -36,7 +36,7 @@ int gen10g_shutdown(struct phy_device *phydev)
>  int gen10g_startup(struct phy_device *phydev)
>  {
>  	int devad, reg;
> -	u32 mmd_mask = phydev->mmds;
> +	u32 mmd_mask = phydev->mmds & MDIO_DEVS_LINK;

So we are introducing this new macro, right?  I don't see this in Linux
- how is this handled there?  If we do have to create a new name, can't
we use something more self-documenting, e.g. MDIO_DEVS_REPORT_LINK or
such?

Cheers
  Detlev

-- 
Quantum physicists can either know how fast they do it, or where they
do it, but not both.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] phylib: Detect link on 10G devices correctly

[Wolfgang Denk]

Dear Andy Fleming,

In message <1305762259-5639-1-git-send-email-afleming@freescale.com> you wrote:
> gen10g_startup() had 2 bugs:
> 
> 1) It had a boolean logic error in checking the MMD mask, and
> always checked all of them.
> 
> 2) It checked devices which don't actually report link state, which
> meant that it would never believe the link was fully up.
> 
> Fix the boolean logic, and then mask the MMD mask so only link-reporting
> devices are checked.
> 
> Signed-off-by: Andy Fleming <afleming@freescale.com>
> Reported-by: Ed Swarthout <Ed.Swarthout@freescale.com>
> ---
> 
>  drivers/net/phy/generic_10g.c |    8 ++++++--
>  include/linux/mdio.h          |    8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
WARNING:  This Product Attracts Every Other Piece  of  Matter in  the
Universe, Including the Products of Other Manufacturers, with a Force
Proportional  to the Product of the Masses and Inversely Proportional
to the Distance Between Them.