From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Lunn <andrew@lunn.ch>
Subject: Re: [PATCH] smsc95xx: Add support for automated PHY address detection
Date: Sun, 23 Dec 2018 11:56:43 +0100
Message-ID: <20181223105643.GF32178@lunn.ch>
References: <20180911101223.4217-1-marex@denx.de>
 <a3281697-acb4-279f-c457-15e8d09758e8@denx.de>
 <20181223102329.GA32178@lunn.ch>
 <184170a4-3827-5609-ce77-66a49fd56aa1@denx.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-usb@vger.kernel.org,
        "David S . Miller" <davem@davemloft.net>,
        Nisar Sayed <Nisar.Sayed@microchip.com>,
        Woojung Huh <Woojung.Huh@microchip.com>,
        netdev <netdev@vger.kernel.org>,
        Florian Fainelli <f.fainelli@gmail.com>
To: Marek Vasut <marex@denx.de>
Return-path: <netdev-owner@vger.kernel.org>
Received: from vps0.lunn.ch ([185.16.172.187]:56133 "EHLO vps0.lunn.ch"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1727811AbeLWK4r (ORCPT <rfc822;netdev@vger.kernel.org>);
        Sun, 23 Dec 2018 05:56:47 -0500
Content-Disposition: inline
In-Reply-To: <184170a4-3827-5609-ce77-66a49fd56aa1@denx.de>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.

     Andrew