netdev - Re: [PATCH 8/8] b44: add dummy PHY device if we do not find any

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3257000.PkooeomauC@lenovo>
Date:	Sun, 15 Dec 2013 19:42:24 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Hauke Mehrtens <hauke@...ke-m.de>
Cc:	davem@...emloft.net, zambrano@...adcom.com, netdev@...r.kernel.org
Subject: Re: [PATCH 8/8] b44: add dummy PHY device if we do not find any

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@...ke-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.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html