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

[Hauke Mehrtens <hauke@hauke-m.de>]

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