[PATCH] net: macb: do not scan PHYs manually

[PATCH] net: macb: do not scan PHYs manually

[Nathan Sullivan]

Since of_mdiobus_register and mdiobus_register will scan automatically,
do not manually scan for PHY devices in the macb ethernet driver. Doing
so will result in many nonexistent PHYs on the MDIO bus if the MDIO
lines are floating or grounded, such as when they are not used.

Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
---
 drivers/net/ethernet/cadence/macb.c |   17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 48a7d7d..e80e487 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
 	if (np) {
 		/* try dt phy registration */
 		err = of_mdiobus_register(bp->mii_bus, np);
-
-		/* fallback to standard phy registration if no phy were
-		   found during dt phy registration */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev)) {
-					err = PTR_ERR(phydev);
-					break;
-				}
-			}
-
-			if (err)
-				goto err_out_unregister_bus;
-		}
 	} else {
 		if (pdata)
 			bp->mii_bus->phy_mask = pdata->phy_mask;
-- 
1.7.10.4

Re: [PATCH] net: macb: do not scan PHYs manually

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3784 bytes --]

Hi,

[auto build test WARNING on v4.6-rc5]
[cannot apply to net-next/master next-20160428]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Nathan-Sullivan/net-macb-do-not-scan-PHYs-manually/20160428-222826
config: powerpc-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/cadence/macb.c: In function 'macb_mii_init':
>> drivers/net/ethernet/cadence/macb.c:427:20: warning: unused variable 'i' [-Wunused-variable]
     int err = -ENXIO, i;
                       ^

vim +/i +427 drivers/net/ethernet/cadence/macb.c

222ca8e0 drivers/net/ethernet/cadence/macb.c Nathan Sullivan   2015-05-22  411  		phydev->supported &= ~SUPPORTED_1000baseT_Half;
222ca8e0 drivers/net/ethernet/cadence/macb.c Nathan Sullivan   2015-05-22  412  
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  413  	phydev->advertising = phydev->supported;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  414  
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  415  	bp->link = 0;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  416  	bp->speed = 0;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  417  	bp->duplex = -1;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  418  	bp->phy_dev = phydev;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  419  
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  420  	return 0;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  421  }
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  422  
421d9df0 drivers/net/ethernet/cadence/macb.c Cyrille Pitchen   2015-03-07  423  static int macb_mii_init(struct macb *bp)
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  424  {
84e0cdb0 drivers/net/ethernet/cadence/macb.c Jamie Iles        2011-03-08  425  	struct macb_platform_data *pdata;
148cbb53 drivers/net/ethernet/cadence/macb.c Boris BREZILLON   2013-08-22  426  	struct device_node *np;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12 @427  	int err = -ENXIO, i;
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  428  
3dbda77e drivers/net/macb.c                  Uwe Kleine-Koenig 2009-07-23  429  	/* Enable management port */
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  430  	macb_writel(bp, NCR, MACB_BIT(MPE));
6c36a707 drivers/net/macb.c                  frederic RODO     2007-07-12  431  
298cf9be drivers/net/macb.c                  Lennert Buytenhek 2008-10-08  432  	bp->mii_bus = mdiobus_alloc();
298cf9be drivers/net/macb.c                  Lennert Buytenhek 2008-10-08  433  	if (bp->mii_bus == NULL) {
298cf9be drivers/net/macb.c                  Lennert Buytenhek 2008-10-08  434  		err = -ENOMEM;
298cf9be drivers/net/macb.c                  Lennert Buytenhek 2008-10-08  435  		goto err_out;

:::::: The code at line 427 was first introduced by commit
:::::: 6c36a7074436e181fb3df41f66bbdaf53980951e macb: Use generic PHY layer

:::::: TO: frederic RODO <f.rodo@til-technologies.fr>
:::::: CC: Jeff Garzik <jeff@garzik.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 48660 bytes --]

Re: [PATCH] net: macb: do not scan PHYs manually

[Josh Cartwright]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Thu, Apr 28, 2016 at 09:19:47AM -0500, Nathan Sullivan wrote:
> Since of_mdiobus_register and mdiobus_register will scan automatically,

This is only partially true.  of_mdiobus_register() only scans for PHYs
with device tree presence (starting with nodes which specify an address,
then continuing for nodes without an address specifier).

So, this patch will regress any platform currently relying on a PHY
being found/probed even though it has no representation as a device tree
node.  I don't have enough history with this driver to make a judgement
as to whether or not there are any users of this functionality.

> do not manually scan for PHY devices in the macb ethernet driver. Doing
> so will result in many nonexistent PHYs on the MDIO bus if the MDIO
> lines are floating or grounded, such as when they are not used.

Just a restatement/elaboration of the problem in an attempt to provide
more context:

The assumption being made is that the phy associated with a given macb
instance exists on that instance's controlled mdio bus.  This assumption
doesn't hold true on some dual-ethernet Zynq 7000-series based designs
where both phys exist on the same MDIO bus (for pinning reasons, or
whatever).

             MDIO
    macb0 ----+----- phy0, addrX
              |
              +----- phy1, addrY

    macb1 ----- {MDIO lines NC, not pinmuxed}

(In this case, the phy associated with macb1 is phy1, which exists on
the MDIO bus controlled by the macb0 instance).

HTH,

  Josh

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]