Re: [PATCH] smsc95xx: Add support for automated PHY address detection

[Marek Vasut <marex@denx.de>]

On 12/23/18 11:56 AM, Andrew Lunn wrote:
> On Sun, Dec 23, 2018 at 11:43:05AM +0100, Marek Vasut wrote:
>> On 12/23/18 11:23 AM, Andrew Lunn wrote:
>>>>> +static int smsc95xx_phy_address(struct usbnet *dev)
>>>>> +{
>>>>> +	u32 read_buf;
>>>>> +	int ret, id1, id2, phyad;
>>>>> +
>>>>> +	ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>> +	/* Check if using external PHY, if not, use internal PHY address */
>>>>> +	if (!(read_buf & HW_CFG_PSEL_))
>>>>> +		return SMSC95XX_INTERNAL_PHY_ID;
>>>>> +
>>>>> +	/*
>>>>> +	 * Detect external PHY address. Here we probe the MDIO bus from
>>>>> +	 * the highest address, since some PHYs respond also on address
>>>>> +	 * zero, which they consider MDIO broadcast address. We really
>>>>> +	 * want to get their proper address instead though, so we scan
>>>>> +	 * address zero last.
>>>>> +	 */
>>>>> +	for (phyad = 0x1f; phyad >= 0; phyad--) {
>>>>> +		id1 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID1);
>>>>> +		id2 = smsc95xx_mdio_read(dev->net, phyad, MII_PHYSID2);
>>>>> +		/* Check for valid response from the PHY */
>>>>> +		if (id1 > 0 && id2 > 0 && id1 != 0x7fff && id2 != 0xffff)
>>>>> +			return phyad;
>>>>> +	}
>>>
>>> This would be so much easier if the driver used the core mdio/phy
>>> code. Just set mdio->phy_mask to ~BIT(0) and then use
>>> phy_find_first().
>>
>> That's in the pipeline, along with PM cleanups, but low prio.
> 
> Great. Does using the broadcast address actually cause a problem?  If
> not, i would say lets drop this part of the patch until you do the
> cleanup.

Yeah, at least the TJA11xx doesn't respond on 0x0, so I wouldn't depend
on the broadcast address. In my case, the PHY just sits at 0x4.

-- 
Best regards,
Marek Vasut