| Message ID | 1387132925-18651-9-git-send-email-hauke@hauke-m.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Hauke, Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit : > The ADM6996L switches used on some routers do not return a valid value > when reading the PHY id register and Linux thinks there is not PHY at > all, but that is wrong. This created a dummy PHY and uses that instead. As far as I read it from the ADM6996L datasheet; the management interface on these switches is via a serial EEPROM which was usually wired up to the BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If MII_PHYSID1 and MII_PHYSID2 present bad values, I would assume that MII_BMSR would also return bad values too and that if you are given a valid link speed/duplex this might just be because all bits are set? In any case this is a case where you should register a fixed PHY device instead of creating such a dummy device which will still make the mdiobus driver/PHY state machine issue real reads/writes on the MDIO bus to a non-existent PHY. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > drivers/net/ethernet/broadcom/b44.c | 23 +++++++++++++++++++---- > drivers/net/ethernet/broadcom/b44.h | 3 +++ > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/b44.c > b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644 > --- a/drivers/net/ethernet/broadcom/b44.c > +++ b/drivers/net/ethernet/broadcom/b44.c > @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp) > struct ssb_device *sdev = bp->sdev; > struct phy_device *phydev; > int err; > + struct phy_c45_device_ids c45_ids = {0}; > + struct ssb_sprom *sprom = &sdev->bus->sprom; > > mii_bus = mdiobus_alloc(); > if (!mii_bus) { > @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp) > } > > phydev = bp->mii_bus->phy_map[bp->phy_addr]; > - if (!phydev) { > - dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr); > - err = -ENODEV; > - goto err_out_mdiobus_unregister; > + if (!phydev && > + (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) { > + dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n", > + bp->phy_addr); Your commit subject says ADM6996L but here you are also treating Broadcom switches that way, this might deserve a comment to explain that some BCM53xx switches might be SPI/GPIO connected. > + > + phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0, > + false, &c45_ids); > + if (IS_ERR(phydev)) { > + err = PTR_ERR(phydev); > + dev_err(sdev->dev, "Can not create dummy PHY\n"); > + goto err_out_mdiobus_unregister; > + } > + err = phy_device_register(phydev); > + if (err) { > + dev_err(sdev->dev, "failed to register MII bus\n"); > + goto err_out_mdiobus_unregister; > + } > } > > err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link, > diff --git a/drivers/net/ethernet/broadcom/b44.h > b/drivers/net/ethernet/broadcom/b44.h index c5ec9b4..e6780fe 100644 > --- a/drivers/net/ethernet/broadcom/b44.h > +++ b/drivers/net/ethernet/broadcom/b44.h > @@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE > struct u64_stats_sync syncp; > }; > > +#define B44_BOARDFLAG_ROBO 0x0010 /* Board has robo switch */ > +#define B44_BOARDFLAG_ADM 0x0080 /* Board has ADMtek switch */ > + > struct ssb_device; > > struct b44 {
On 12/15/2013 10:26 PM, Florian Fainelli wrote: > Hi Hauke, > > Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit : >> The ADM6996L switches used on some routers do not return a valid value >> when reading the PHY id register and Linux thinks there is not PHY at >> all, but that is wrong. This created a dummy PHY and uses that instead. > > As far as I read it from the ADM6996L datasheet; the management interface on > these switches is via a serial EEPROM which was usually wired up to the > BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If MII_PHYSID1 and > MII_PHYSID2 present bad values, I would assume that MII_BMSR would also return > bad values too and that if you are given a valid link speed/duplex this might > just be because all bits are set? Yes these registers are returning 0xffff. > In any case this is a case where you should register a fixed PHY device instead > of creating such a dummy device which will still make the mdiobus driver/PHY > state machine issue real reads/writes on the MDIO bus to a non-existent PHY. Yes I will try that. >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> drivers/net/ethernet/broadcom/b44.c | 23 +++++++++++++++++++---- >> drivers/net/ethernet/broadcom/b44.h | 3 +++ >> 2 files changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/broadcom/b44.c >> b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644 >> --- a/drivers/net/ethernet/broadcom/b44.c >> +++ b/drivers/net/ethernet/broadcom/b44.c >> @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp) >> struct ssb_device *sdev = bp->sdev; >> struct phy_device *phydev; >> int err; >> + struct phy_c45_device_ids c45_ids = {0}; >> + struct ssb_sprom *sprom = &sdev->bus->sprom; >> >> mii_bus = mdiobus_alloc(); >> if (!mii_bus) { >> @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp) >> } >> >> phydev = bp->mii_bus->phy_map[bp->phy_addr]; >> - if (!phydev) { >> - dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr); >> - err = -ENODEV; >> - goto err_out_mdiobus_unregister; >> + if (!phydev && >> + (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) { >> + dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n", >> + bp->phy_addr); > > Your commit subject says ADM6996L but here you are also treating Broadcom > switches that way, this might deserve a comment to explain that some BCM53xx > switches might be SPI/GPIO connected. This is also needed for some Broadcom switches. The Broadcom BCM5325F switch has two MII interfaces and the BCM4704 has two MACs which are both connected to one of the MII interfaces of the switch. On such devices we do get a PHY ID for the first MII interface used for the LAN ports, but when trying to read the PHY ID of the second MII interface used for the WAN port it just returns 0xffff on all addresses. When registering this dummy phy we are able to talk to the interface. This WAN port seams to be hard wired to the second MII interface in the switch. >> + >> + phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0, >> + false, &c45_ids); >> + if (IS_ERR(phydev)) { >> + err = PTR_ERR(phydev); >> + dev_err(sdev->dev, "Can not create dummy PHY\n"); >> + goto err_out_mdiobus_unregister; >> + } >> + err = phy_device_register(phydev); >> + if (err) { >> + dev_err(sdev->dev, "failed to register MII bus\n"); >> + goto err_out_mdiobus_unregister; >> + } >> } >> >> err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link, >> diff --git a/drivers/net/ethernet/broadcom/b44.h >> b/drivers/net/ethernet/broadcom/b44.h index c5ec9b4..e6780fe 100644 >> --- a/drivers/net/ethernet/broadcom/b44.h >> +++ b/drivers/net/ethernet/broadcom/b44.h >> @@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE >> struct u64_stats_sync syncp; >> }; >> >> +#define B44_BOARDFLAG_ROBO 0x0010 /* Board has robo switch */ >> +#define B44_BOARDFLAG_ADM 0x0080 /* Board has ADMtek switch */ >> + >> struct ssb_device; >> >> struct b44 { > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le lundi 16 décembre 2013, 00:31:34 Hauke Mehrtens a écrit : > On 12/15/2013 10:26 PM, Florian Fainelli wrote: > > Hi Hauke, > > > > Le dimanche 15 décembre 2013, 19:42:05 Hauke Mehrtens a écrit : > >> The ADM6996L switches used on some routers do not return a valid value > >> when reading the PHY id register and Linux thinks there is not PHY at > >> all, but that is wrong. This created a dummy PHY and uses that instead. > > > > As far as I read it from the ADM6996L datasheet; the management interface > > on these switches is via a serial EEPROM which was usually wired up to > > the BCM47xx SoC to some GPIOs (kmod-switch driver in OpenWrt). If > > MII_PHYSID1 and MII_PHYSID2 present bad values, I would assume that > > MII_BMSR would also return bad values too and that if you are given a > > valid link speed/duplex this might just be because all bits are set? > > Yes these registers are returning 0xffff. Ok, yes that's pretty typical of the absence of a physical connection between the MAC and PHY/switch. > > > In any case this is a case where you should register a fixed PHY device > > instead of creating such a dummy device which will still make the mdiobus > > driver/PHY state machine issue real reads/writes on the MDIO bus to a > > non-existent PHY. > Yes I will try that. Thanks! Note that a fixed PHY must be registered before the fixed MDIO bus is probe, you can take a look at how AR7 does it for instance (arch/mips/ar7/platform.c and drivers/net/ethernet/ti/cpmac.c). We might want to be able to change that in the future and re-scan the fixed MDIO bus though. > > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > >> --- > >> > >> drivers/net/ethernet/broadcom/b44.c | 23 +++++++++++++++++++---- > >> drivers/net/ethernet/broadcom/b44.h | 3 +++ > >> 2 files changed, 22 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/broadcom/b44.c > >> b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644 > >> --- a/drivers/net/ethernet/broadcom/b44.c > >> +++ b/drivers/net/ethernet/broadcom/b44.c > >> @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp) > >> > >> struct ssb_device *sdev = bp->sdev; > >> struct phy_device *phydev; > >> int err; > >> > >> + struct phy_c45_device_ids c45_ids = {0}; > >> + struct ssb_sprom *sprom = &sdev->bus->sprom; > >> > >> mii_bus = mdiobus_alloc(); > >> if (!mii_bus) { > >> > >> @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp) > >> > >> } > >> > >> phydev = bp->mii_bus->phy_map[bp->phy_addr]; > >> > >> - if (!phydev) { > >> - dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr); > >> - err = -ENODEV; > >> - goto err_out_mdiobus_unregister; > >> + if (!phydev && > >> + (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) > >> { > >> + dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n", > >> + bp->phy_addr); > > > > Your commit subject says ADM6996L but here you are also treating Broadcom > > switches that way, this might deserve a comment to explain that some > > BCM53xx switches might be SPI/GPIO connected. > > This is also needed for some Broadcom switches. The Broadcom BCM5325F > switch has two MII interfaces and the BCM4704 has two MACs which are > both connected to one of the MII interfaces of the switch. On such > devices we do get a PHY ID for the first MII interface used for the LAN > ports, but when trying to read the PHY ID of the second MII interface > used for the WAN port it just returns 0xffff on all addresses. When > registering this dummy phy we are able to talk to the interface. Most likely you get 0xffff because the switch is only connected to the first Ethernet MAC MDIO bus. In that specific case it might be better to make the second Ethernet MAC probe for PHYs on the first Ethernet MAC mdio bus or simply register a fixed PHY device for the WAN port since you only want one Ethernet MAC to be in control of the switch at a time. > > This WAN port seams to be hard wired to the second MII interface in the > switch. This is quite likely, most Broadcom switches support one or two IMP (CPU port) configurations.
diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c index b65a463..07e58c2 100644 --- a/drivers/net/ethernet/broadcom/b44.c +++ b/drivers/net/ethernet/broadcom/b44.c @@ -2233,6 +2233,8 @@ static int b44_register_phy_one(struct b44 *bp) struct ssb_device *sdev = bp->sdev; struct phy_device *phydev; int err; + struct phy_c45_device_ids c45_ids = {0}; + struct ssb_sprom *sprom = &sdev->bus->sprom; mii_bus = mdiobus_alloc(); if (!mii_bus) { @@ -2266,10 +2268,23 @@ static int b44_register_phy_one(struct b44 *bp) } phydev = bp->mii_bus->phy_map[bp->phy_addr]; - if (!phydev) { - dev_err(sdev->dev, "could not find PHY at %i\n", bp->phy_addr); - err = -ENODEV; - goto err_out_mdiobus_unregister; + if (!phydev && + (sprom->boardflags_lo & (B44_BOARDFLAG_ROBO | B44_BOARDFLAG_ADM))) { + dev_info(sdev->dev, "could not find PHY at %i, create dummy one\n", + bp->phy_addr); + + phydev = phy_device_create(bp->mii_bus, bp->phy_addr, 0x0, + false, &c45_ids); + if (IS_ERR(phydev)) { + err = PTR_ERR(phydev); + dev_err(sdev->dev, "Can not create dummy PHY\n"); + goto err_out_mdiobus_unregister; + } + err = phy_device_register(phydev); + if (err) { + dev_err(sdev->dev, "failed to register MII bus\n"); + goto err_out_mdiobus_unregister; + } } err = phy_connect_direct(bp->dev, phydev, &b44_adjust_link, diff --git a/drivers/net/ethernet/broadcom/b44.h b/drivers/net/ethernet/broadcom/b44.h index c5ec9b4..e6780fe 100644 --- a/drivers/net/ethernet/broadcom/b44.h +++ b/drivers/net/ethernet/broadcom/b44.h @@ -345,6 +345,9 @@ B44_STAT_REG_DECLARE struct u64_stats_sync syncp; }; +#define B44_BOARDFLAG_ROBO 0x0010 /* Board has robo switch */ +#define B44_BOARDFLAG_ADM 0x0080 /* Board has ADMtek switch */ + struct ssb_device; struct b44 {
The ADM6996L switches used on some routers do not return a valid value when reading the PHY id register and Linux thinks there is not PHY at all, but that is wrong. This created a dummy PHY and uses that instead. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- drivers/net/ethernet/broadcom/b44.c | 23 +++++++++++++++++++---- drivers/net/ethernet/broadcom/b44.h | 3 +++ 2 files changed, 22 insertions(+), 4 deletions(-)