* [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch @ 2013-07-10 10:09 Heiko Schocher 2013-07-10 10:55 ` Heiko Schocher 0 siblings, 1 reply; 4+ messages in thread From: Heiko Schocher @ 2013-07-10 10:09 UTC (permalink / raw) To: u-boot 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch 2013-07-10 10:09 [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch Heiko Schocher @ 2013-07-10 10:55 ` Heiko Schocher 2013-07-10 11:15 ` Wolfgang Denk 0 siblings, 1 reply; 4+ messages in thread From: Heiko Schocher @ 2013-07-10 10:55 UTC (permalink / raw) To: u-boot Hello, Am 10.07.2013 12:09, schrieb Heiko Schocher: > 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? 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: 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 7c0eaec..4522382 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) { + printf ("Could not read MII_STAT1000. Ignoring gigabit capability\n"); + gblpa = 0; + } gblpa &= phy_read(phydev, MDIO_DEVAD_NONE, MII_CTRL1000) << 2; } bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch 2013-07-10 10:55 ` Heiko Schocher @ 2013-07-10 11:15 ` Wolfgang Denk 2013-07-10 11:33 ` Heiko Schocher 0 siblings, 1 reply; 4+ messages in thread From: Wolfgang Denk @ 2013-07-10 11:15 UTC (permalink / raw) To: u-boot Dear Heiko, In message <51DD3D92.8050701@denx.de> you wrote: > > Would this be an acceptable patch for it: ... > @@ -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 -- 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@denx.de Without freedom of choice there is no creativity. -- Kirk, "The return of the Archons", stardate 3157.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch 2013-07-10 11:15 ` Wolfgang Denk @ 2013-07-10 11:33 ` Heiko Schocher 0 siblings, 0 replies; 4+ messages in thread From: Heiko Schocher @ 2013-07-10 11:33 UTC (permalink / raw) To: u-boot Hello Wolfgang, Am 10.07.2013 13:15, schrieb Wolfgang Denk: > Dear Heiko, > > In message<51DD3D92.8050701@denx.de> you wrote: >> >> Would this be an acceptable patch for it: > ... >> @@ -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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-10 11:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-10 10:09 [U-Boot] net, phy: wrong 1000BASE detection with a lan9303 switch Heiko Schocher 2013-07-10 10:55 ` Heiko Schocher 2013-07-10 11:15 ` Wolfgang Denk 2013-07-10 11:33 ` Heiko Schocher
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox