net, phy: wrong 1000BASE detection with a lan9303 switch
Subsystems (2)
Hello,
I have problems with a lan9303 switch on an am335x based board, which does not support 1000FD/HD modes, but as it set BMSR_ERCAP bit in BMSR and returns 0xffff for reads on the MII_STAT1000 and MII_CTRL1000 registers, u-boot code detects SPEED_1000 in drivers/net/phy/phy.c genphy_parse_link() ... which is wrong, as this switch does not support 1000 modes ...
I found in ./common/miiphyutil.c miiphy_speed() the define CONFIG_PHY_GIGE which it seems lacks in drivers/net/phy/phy.c genphy_parse_link() ? Or is there another option?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hello,
Am 10.07.2013 12:09, schrieb Heiko Schocher:
Quoted 9 lines
Hello,
I have problems with a lan9303 switch on an am335x based board, which does not support 1000FD/HD modes, but as it set
BMSR_ERCAPbit in BMSR and returns0xfffffor reads on theMII_STAT1000andMII_CTRL1000registers, u-boot code detectsSPEED_1000indrivers/net/phy/phy.cgenphy_parse_link()... which is wrong, as this switch does not support 1000 modes ...
Quoted 3 lines
I found in
./common/miiphyutil.cmiiphy_speed()the defineCONFIG_PHY_GIGEwhich it seems lacks indrivers/net/phy/phy.cgenphy_parse_link()? Or is there another option?
Just an update ... the switch does not support reads for the registers 0x09 and 0x0a, but sets the BMSR_ERCAP bit in BMSR.
A phy_read on register 0x09 or 0x0a fails ... In my case the drivers/net/cpsw.c returns -1 ... and code in drivers/net/phy/phy.c:
int genphy_parse_link(struct phy_device *phydev)
{
int mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR);
/* We're using autonegotiation */
if (mii_reg & BMSR_ANEGCAPABLE) {
u32 lpa = 0;
u32 gblpa = 0;
u32 estatus = 0;
/* Check for gigabit capability */
if (mii_reg & BMSR_ERCAP) {
/* We want a list of states supported by
* both PHYs in the link
*/
gblpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_STAT1000);
gblpa &= phy_read(phydev,
MDIO_DEVAD_NONE, MII_CTRL1000) << 2;
}[...]
does not check, if phy_read returns with an error ...
Would this be an acceptable patch for it:
drivers/net/cpsw.c +1 -1
@@ -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 )
drivers/net/phy/phy.c +5 -1
@@ -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 ) { +printf ("Could not read MII_STAT1000. Ignoring gigabit capability\n" ); + gblpa =0 ; + } gblpa &=phy_read (phydev,MDIO_DEVAD_NONE ,MII_CTRL1000 ) <<2 ; }
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Dear Heiko,
Quoted 2 lines · In message <[email protected]> you wrote:
Would this be an acceptable patch for it:
...
drivers/net/phy/phy.c int genphy_parse_link(struct phy_device *phydev) · line 308
Quoted 10 lines
drivers/net/phy/phy.c +4
-0@@ -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 ) { +printf ("Could not read MII_STAT1000. Ignoring gigabit capability\n" ); + gblpa =0 ; + } gblpa &=phy_read (phydev,MDIO_DEVAD_NONE ,MII_CTRL1000 ) <<2 ;
Well, this other phy_read() probably needs exactly the same error handling - and I doubt if we should actually try riding the reg when the first one failed already?
Best regards,
Wolfgang Denk
Signature · 5 lines
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: [email protected] Without freedom of choice there is no creativity. -- Kirk, "The return of the Archons", stardate 3157.4
Hello Wolfgang,
Am 10.07.2013 13:15, schrieb Wolfgang Denk:
Quoted 6 lines
Dear Heiko,
Quoted 2 lines · In message<[email protected]> you wrote:
Would this be an acceptable patch for it:
...
Quoted 14 lines
drivers/net/phy/phy.c int genphy_parse_link(struct phy_device *phydev) · line 308
Quoted 10 lines
drivers/net/phy/phy.c +4
-0@@ -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 ) { +printf ("Could not read MII_STAT1000. Ignoring gigabit capability\n" ); + gblpa =0 ; + } gblpa&=phy_read (phydev,MDIO_DEVAD_NONE ,MII_CTRL1000 )<<2 ;Well, this other
phy_read()probably needs exactly the same error handling - and I doubt if we should actually try riding the reg when the first one failed already?
Yes, you are right. The first question is, if such a patch is the right way for fixing it, or if there is a better way ...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany