2016-04-28 14:20:42

by Nathan Sullivan

[permalink] [raw]
Subject: [PATCH] net: macb: do not scan PHYs manually

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 <[email protected]>
---
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


2016-04-28 14:39:21

by kernel test robot

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

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 <[email protected]>
:::::: CC: Jeff Garzik <[email protected]>

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


Attachments:
(No filename) (3.70 kB)
.config.gz (47.52 kB)
Download all attachments

2016-04-28 15:07:09

by Josh Cartwright

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

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


Attachments:
(No filename) (1.46 kB)
signature.asc (473.00 B)
Download all attachments