netdev - Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices

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: <20200622131652.GA5822@lsv03152.swis.in-blr01.nxp.com>
Date:   Mon, 22 Jun 2020 18:46:52 +0530
From:   Calvin Johnson <calvin.johnson@....nxp.com>
To:     "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
Cc:     Jeremy Linton <jeremy.linton@....com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Jon <jon@...id-run.com>,
        Cristi Sovaiala <cristian.sovaiala@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "linux.cj@...il.com" <linux.cj@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe
 c45 devices

On Mon, Jun 22, 2020 at 09:29:17AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Calvin Johnson (OSS) <calvin.johnson@....nxp.com>
> > Sent: Monday, June 22, 2020 11:19 AM
> > To: Jeremy Linton <jeremy.linton@....com>; Russell King - ARM Linux admin
> > <linux@...linux.org.uk>; Jon <jon@...id-run.com>; Cristi Sovaiala
> > <cristian.sovaiala@....com>; Ioana Ciornei <ioana.ciornei@....com>; Andrew
> > Lunn <andrew@...n.ch>; Andy Shevchenko <andy.shevchenko@...il.com>;
> > Florian Fainelli <f.fainelli@...il.com>; Madalin Bucur (OSS)
> > <madalin.bucur@....nxp.com>
> > Cc: linux.cj@...il.com; netdev@...r.kernel.org; Calvin Johnson (OSS)
> > <calvin.johnson@....nxp.com>
> > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe
> > c45 devices
> > 
> > From: Jeremy Linton <jeremy.linton@....com>
> > 
> > The mdiobus_scan logic is currently hardcoded to only
> > work with c22 devices. This works fairly well in most
> > cases, but its possible that a c45 device doesn't respond
> > despite being a standard phy. If the parent hardware
> > is capable, it makes sense to scan for c22 devices before
> > falling back to c45.
> > 
> > As we want this to reflect the capabilities of the STA,
> > lets add a field to the mii_bus structure to represent
> > the capability. That way devices can opt into the extended
> > scanning. Existing users should continue to default to c22
> > only scanning as long as they are zero'ing the structure
> > before use.
> 
> How is this default working for existing users, the code below does not seem
> to do anything for a zeroed struct, as there is no default in the switch?

Looking further into this, I think MDIOBUS_C22 = 0, was correct. Prior to
this patch, get_phy_device() was executed for C22 in this path. I'll discuss
with Russell and Andrew on this and get back.

> 
> > 
> > Signed-off-by: Jeremy Linton <jeremy.linton@....com>
> > Signed-off-by: Calvin Johnson <calvin.johnson@....nxp.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Reserve "0" to mean that no mdiobus capabilities have been declared.
> > 
> >  drivers/net/phy/mdio_bus.c | 17 +++++++++++++++--
> >  include/linux/phy.h        |  8 ++++++++
> >  2 files changed, 23 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> > index 6ceee82b2839..e6c179b89907 100644
> > --- a/drivers/net/phy/mdio_bus.c
> > +++ b/drivers/net/phy/mdio_bus.c
> > @@ -739,10 +739,23 @@ EXPORT_SYMBOL(mdiobus_free);
> >   */
> >  struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
> >  {
> > -	struct phy_device *phydev;
> > +	struct phy_device *phydev = ERR_PTR(-ENODEV);
> >  	int err;
> > 
> > -	phydev = get_phy_device(bus, addr, false);
> > +	switch (bus->probe_capabilities) {
> > +	case MDIOBUS_C22:
> > +		phydev = get_phy_device(bus, addr, false);
> > +		break;
> > +	case MDIOBUS_C45:
> > +		phydev = get_phy_device(bus, addr, true);
> > +		break;
> > +	case MDIOBUS_C22_C45:
> > +		phydev = get_phy_device(bus, addr, false);
> > +		if (IS_ERR(phydev))
> > +			phydev = get_phy_device(bus, addr, true);
> > +		break;
> > +	}
> > +
> >  	if (IS_ERR(phydev))
> >  		return phydev;
> > 
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 9248dd2ce4ca..7860d56c6bf5 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -298,6 +298,14 @@ struct mii_bus {
> >  	/* RESET GPIO descriptor pointer */
> >  	struct gpio_desc *reset_gpiod;
> > 
> > +	/* bus capabilities, used for probing */
> > +	enum {
> > +		MDIOBUS_NO_CAP = 0,
> > +		MDIOBUS_C22,
> > +		MDIOBUS_C45,
> > +		MDIOBUS_C22_C45,
> > +	} probe_capabilities;
> > +
> >  	/* protect access to the shared element */
> >  	struct mutex shared_lock;
> > 
> > --
> > 2.17.1
>