netdev - Re: [PATCH] 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: <20180814155812.cjuacvkhqeuctcry@pengutronix.de>
Date:   Tue, 14 Aug 2018 17:58:12 +0200
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Ahmad Fatoum <a.fatoum@...gutronix.de>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        netdev@...r.kernel.org, mdf@...nel.org, stable@...r.kernel.org,
        kernel@...gutronix.de, Brad Mouring <brad.mouring@...com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link
 PHYs

Hello Ahmad,


On Tue, Aug 14, 2018 at 04:12:40PM +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
> 	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
> 	macb f0028000.ethernet: broken fixed-link specification
> 
> Cc: <stable@...r.kernel.org>
> Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")

I added the people involved in 739de9a1563a to Cc. Maybe they want to
comment. So not stripping the remaining part of the original mail.

Best regards
Uwe

> Signed-off-by: Ahmad Fatoum <a.fatoum@...gutronix.de>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++++---------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index a6c911bb5ce2..d202a03c42ed 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -481,11 +481,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;
> -			}
>  			bp->phy_node = of_node_get(np);
>  		} else {
>  			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
> @@ -568,7 +563,7 @@ static int macb_mii_init(struct macb *bp)
>  {
>  	struct macb_platform_data *pdata;
>  	struct device_node *np;
> -	int err;
> +	int err = -ENXIO;
>  
>  	/* Enable management port */
>  	macb_writel(bp, NCR, MACB_BIT(MPE));
> @@ -591,10 +586,21 @@ static int macb_mii_init(struct macb *bp)
>  	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
>  
>  	np = bp->pdev->dev.of_node;
> -	if (pdata)
> -		bp->mii_bus->phy_mask = pdata->phy_mask;
> +	if (np && of_phy_is_fixed_link(np)) {
> +		if (of_phy_register_fixed_link(np) < 0) {
> +			dev_err(&bp->pdev->dev,
> +					"broken fixed-link specification\n");
> +			goto err_out_free_mdiobus;
> +		}
> +
> +		err = mdiobus_register(bp->mii_bus);
> +	} else {
> +		if (pdata)
> +			bp->mii_bus->phy_mask = pdata->phy_mask;
> +
> +		err = of_mdiobus_register(bp->mii_bus, np);
> +	}
>  
> -	err = of_mdiobus_register(bp->mii_bus, np);
>  	if (err)
>  		goto err_out_free_mdiobus;
>  
> @@ -606,9 +612,9 @@ static int macb_mii_init(struct macb *bp)
>  
>  err_out_unregister_bus:
>  	mdiobus_unregister(bp->mii_bus);
> +err_out_free_mdiobus:
>  	if (np && of_phy_is_fixed_link(np))
>  		of_phy_deregister_fixed_link(np);
> -err_out_free_mdiobus:
>  	of_node_put(bp->phy_node);
>  	mdiobus_free(bp->mii_bus);
>  err_out:
> -- 
> 2.18.0
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |