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]

Date:   Tue, 14 Aug 2018 21:03:52 -0500
From:   Brad Mouring <brad.mouring@...com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
CC:     Ahmad Fatoum <a.fatoum@...gutronix.de>,
        "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>,
        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 Ahmed, Uwe,

On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
> 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

You should probably prod Andrew Lunn, he suggested that I move the fixed
link code from macb_mii_init() to _probe(). Here, you're at least partially
directly undoing that.

(ref: https://www.mail-archive.com/netdev@vger.kernel.org/msg221018.html)

> > 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                 | https://urldefense.proofpoint.com/v2/url?u=http-3A__www.pengutronix.de_&d=DwIDAw&c=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=8iZb7YNOSMVIG_mTIHDL03ZcObgQI_gGlWrSewdGETA&m=IQAK1YsKs7Z2bvZxuajiSXw3asFiEztQKYkvy-LpBn8&s=XKrOhFxQshoEcDxMZVSATnJW2cbaD16mQofKdEbJVW0&e=  |

-- 
Brad Mouring
Senior Software Engineer
National Instruments
512-683-6610 / bmouring@...com