linux-kernel - Re: [PATCH v2] net: macb: do not scan PHYs manually

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160429124906.GD4053@lunn.ch>
Date:	Fri, 29 Apr 2016 14:49:06 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Josh Cartwright <joshc@...com>
Cc:	Nathan Sullivan <nathan.sullivan@...com>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	Florian Fainelli <f.fainelli@...il.com>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
Subject: Re: [PATCH v2] net: macb: do not scan PHYs manually

> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index eec3200..d843bc9 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -419,11 +419,62 @@ static int macb_mii_probe(struct net_device *dev)
>  	return 0;
>  }
>  
> +static int macb_mii_of_init(struct macb *bp, struct device_node *np)
> +{
> +	struct device_node *mdio;
> +	int err, i;
> +
> +	mdio = of_get_child_by_name(np, "mdio");
> +	if (mdio)
> +		return of_mdiobus_register(bp->mii_bus, mdio);

We want to encourage driver writers to use an mdio subnode inside
there MAC node. So i wounder if this looking for the child and using
it should go into the core code?

Florian: What do you think?

> +
> +	dev_warn(&bp->pdev->dev,
> +		 "using deprecated PHY probing mechanism.  Please update device tree.");
> +
> +	/* try dt phy registration */
> +	err = of_mdiobus_register(bp->mii_bus, np);
> +	if (err)
> +		return err;
> +
> +	/* fallback to standard phy registration if no phy were
> +	 * found during dt phy registration
> +	 */
> +	if (!phy_find_first(bp->mii_bus)) {

I would also suggest putting a warning here, saying that PHYs should
be listed in the device tree.

> +		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);

FYI: There is a change making its way through which will mean
mdiobus_scan() will return -ENODEV where there is nothing on the bus
at that address, rather than the current NULL. You will need to adopt
this here.

     Andrew