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: <1931903.b5YbP2FvL1@lenovo>
Date:	Sun, 15 Dec 2013 21:26:07 +0000
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

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@...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.

> +
> +		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 {

-- 
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