netdev - Re: [PATCH 1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs

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: <20180820135536.GD6583@lunn.ch>
Date:   Mon, 20 Aug 2018 15:55:36 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ahmad Fatoum <a.fatoum@...gutronix.de>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        kernel@...gutronix.de, netdev@...r.kernel.org, mdf@...nel.org,
        Brad Mouring <brad.mouring@...com>,
        Florian Fainelli <f.fainelli@...il.com>, stable@...r.kernel.org
Subject: Re: [PATCH 1/4] net: macb: Fix regression breaking non-MDIO
 fixed-link PHYs

On Mon, Aug 20, 2018 at 02:12:35PM +0200, Ahmad Fatoum wrote:
> The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
> There, of_mdiobus_register was called even for the fixed-link representing
> the SPI-connected switch PHY, with the result that the driver attempts to
> enumerate PHYs on a non-existent MDIO bus:
> 
> 	libphy: MACB_mii_bus: probed

So there are two different things here:

> 	mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
>         [snip]
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31

These are the result of the fixed-link being considered a PHY in
of_mdiobus_register(). Patch 2 fixes that, turns it into a single
warning.

> 	macb f0028000.ethernet: broken fixed-link specification

Why is of_phy_register_fixed_link(np) failing?

> 
> Cc: <stable@...r.kernel.org>
> Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
> Signed-off-by: Ahmad Fatoum <a.fatoum@...gutronix.de>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++---------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> Fixes since v1:
> 	Added one more error path for failing to register fixed-link
> 	Fixed a whitespace issue
> 
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index dc09f9a8a49b..ef6ce8691443 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -482,11 +482,6 @@ static int macb_mii_probe(struct net_device *dev)
>  
>  	if (np) {
>  		if (of_phy_is_fixed_link(np)) {
> -			if (of_phy_register_fixed_link(np) < 0) {
> -				dev_err(&bp->pdev->dev,
> -					"broken fixed-link specification\n");
> -				return -ENODEV;
> -			}

As a separate patch, please can you use the error code which
of_phy_register_fixed_link() returns, not -ENODEV. It is possible it
is returning EPROBE_DEFER.

   Andrew