Patchwork [U-Boot] net, phy, cpsw: fix gigabit register access

login
register
mail settings
Submitter Heiko Schocher
Date July 23, 2013, 1:32 p.m.
Message ID <1374586356-16412-1-git-send-email-hs@denx.de>
Download mbox | patch
Permalink /patch/261087/
State Awaiting Upstream
Delegated to: Tom Rini
Headers show

Comments

Heiko Schocher - July 23, 2013, 1:32 p.m.
accessing a lan9303 switch with the cpsw driver results in wrong
speed detection, as the switch sets the BMSR_ERCAP in BMSR
register, and follow read of the MII_STAT1000 register fails, as
the switch does not support it. Current code did not check,
if a phy_read() fails ... fix this.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
---
 drivers/net/cpsw.c    | 2 +-
 drivers/net/phy/phy.c | 6 +++++-
 2 Dateien geändert, 6 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
Joe Hershberger - July 23, 2013, 10:17 p.m.
On Tue, Jul 23, 2013 at 8:32 AM, Heiko Schocher <hs@denx.de> wrote:
> accessing a lan9303 switch with the cpsw driver results in wrong
> speed detection, as the switch sets the BMSR_ERCAP in BMSR
> register, and follow read of the MII_STAT1000 register fails, as
> the switch does not support it. Current code did not check,
> if a phy_read() fails ... fix this.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> ---
>  drivers/net/cpsw.c    | 2 +-
>  drivers/net/phy/phy.c | 6 +++++-
>  2 Dateien geändert, 6 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index 379b679..52c08ed 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -489,7 +489,7 @@ static inline void wait_for_idle(void)
>  static int cpsw_mdio_read(struct mii_dev *bus, int phy_id,
>                                 int dev_addr, int phy_reg)
>  {
> -       unsigned short data;
> +       int data;

How is this change related to the substance of the patch?

>         u32 reg;
>
>         if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)

Seems OK otherwise.

-Joe
Heiko Schocher - July 24, 2013, 4:02 a.m.
Hello Joe,

Am 24.07.2013 00:17, schrieb Joe Hershberger:
> On Tue, Jul 23, 2013 at 8:32 AM, Heiko Schocher<hs@denx.de>  wrote:
>> accessing a lan9303 switch with the cpsw driver results in wrong
>> speed detection, as the switch sets the BMSR_ERCAP in BMSR
>> register, and follow read of the MII_STAT1000 register fails, as
>> the switch does not support it. Current code did not check,
>> if a phy_read() fails ... fix this.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Joe Hershberger<joe.hershberger@gmail.com>
>> ---
>>   drivers/net/cpsw.c    | 2 +-
>>   drivers/net/phy/phy.c | 6 +++++-
>>   2 Dateien geändert, 6 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>>
>> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
>> index 379b679..52c08ed 100644
>> --- a/drivers/net/cpsw.c
>> +++ b/drivers/net/cpsw.c
>> @@ -489,7 +489,7 @@ static inline void wait_for_idle(void)
>>   static int cpsw_mdio_read(struct mii_dev *bus, int phy_id,
>>                                  int dev_addr, int phy_reg)
>>   {
>> -       unsigned short data;
>> +       int data;
>
> How is this change related to the substance of the patch?

data is returned, and the code do:

         data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -1;
         return data;

So if data is short only, the return value is in case, the phy
read fails, 0xffff instead 0xffffffff ...

>>          u32 reg;
>>
>>          if (phy_reg&  ~PHY_REG_MASK || phy_id&  ~PHY_ID_MASK)
>
> Seems OK otherwise.

Thanks!

bye,
Heiko
Mugunthan V N - July 24, 2013, 7:38 a.m.
On 7/23/2013 7:02 PM, Heiko Schocher wrote:
> accessing a lan9303 switch with the cpsw driver results in wrong
> speed detection, as the switch sets the BMSR_ERCAP in BMSR
> register, and follow read of the MII_STAT1000 register fails, as
> the switch does not support it. Current code did not check,
> if a phy_read() fails ... fix this.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
Looks correct
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N
Tom Rini - July 30, 2013, 1:26 p.m.
On Tue, Jul 23, 2013 at 03:32:36PM +0200, Heiko Schocher wrote:

> accessing a lan9303 switch with the cpsw driver results in wrong
> speed detection, as the switch sets the BMSR_ERCAP in BMSR
> register, and follow read of the MII_STAT1000 register fails, as
> the switch does not support it. Current code did not check,
> if a phy_read() fails ... fix this.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Applied to u-boot-ti/master, thanks!

Patch

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 379b679..52c08ed 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -489,7 +489,7 @@  static inline void wait_for_idle(void)
 static int cpsw_mdio_read(struct mii_dev *bus, int phy_id,
 				int dev_addr, int phy_reg)
 {
-	unsigned short data;
+	int data;
 	u32 reg;
 
 	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index effe3e3..2fdccb8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -291,7 +291,7 @@  int genphy_parse_link(struct phy_device *phydev)
 	/* We're using autonegotiation */
 	if (mii_reg & BMSR_ANEGCAPABLE) {
 		u32 lpa = 0;
-		u32 gblpa = 0;
+		int gblpa = 0;
 		u32 estatus = 0;
 
 		/* Check for gigabit capability */
@@ -300,6 +300,10 @@  int genphy_parse_link(struct phy_device *phydev)
 			 * both PHYs in the link
 			 */
 			gblpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_STAT1000);
+			if (gblpa < 0) {
+				debug("Could not read MII_STAT1000. Ignoring gigabit capability\n");
+				gblpa = 0;
+			}
 			gblpa &= phy_read(phydev,
 					MDIO_DEVAD_NONE, MII_CTRL1000) << 2;
 		}