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

[Florian Fainelli <f.fainelli@gmail.com>]

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 {

-- 
Florian