netdev - Re: [PATCH v2 1/2] net: phy: Set the driver when registering an MDIO bus device

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, 5 Aug 2014 12:41:39 +0800
From:	Shawn Guo <shawn.guo@...escale.com>
To:	Florian Fainelli <f.fainelli@...il.com>
CC:	Fabio Estevam <festevam@...il.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Marek Vašut <marex@...x.de>,
	Sascha Hauer <kernel@...gutronix.de>,
	Russell King <linux@....linux.org.uk>
Subject: Re: [PATCH v2 1/2] net: phy: Set the driver when registering an MDIO
 bus device

On Mon, Aug 04, 2014 at 09:01:06PM -0700, Florian Fainelli wrote:
> On 08/04/14 20:22, Shawn Guo wrote:
> >On Mon, Aug 04, 2014 at 11:55:22PM -0300, Fabio Estevam wrote:
> >>Hi Ezequiel,
> >>
> >>On Wed, Jul 23, 2014 at 4:47 PM, Ezequiel Garcia
> >><ezequiel.garcia@...e-electrons.com> wrote:
> >>>mdiobus_register() registers a device which is already bound to a driver.
> >>>Hence, the driver pointer should be set properly in order to track down
> >>>the driver associated to the MDIO bus.
> >>>
> >>>This will be used to allow ethernet driver to pin down a MDIO bus driver,
> >>>preventing it from being unloaded while the PHY device is running.
> >>>
> >>>Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
> >>>Tested-by: Florian Fainelli <f.fainelli@...il.com>
> >>>Signed-off-by: Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
> >>>---
> >>>  drivers/net/phy/mdio_bus.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>>diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> >>>index 4eaadcf..203651e 100644
> >>>--- a/drivers/net/phy/mdio_bus.c
> >>>+++ b/drivers/net/phy/mdio_bus.c
> >>>@@ -255,6 +255,7 @@ int mdiobus_register(struct mii_bus *bus)
> >>>
> >>>         bus->dev.parent = bus->parent;
> >>>         bus->dev.class = &mdio_bus_class;
> >>>+       bus->dev.driver = bus->parent->driver;
> >>>         bus->dev.groups = NULL;
> >>>         dev_set_name(&bus->dev, "%s", bus->id);
> >>
> >>This patches causes the following regression in 3.16 (tested on mx5/mx6):
> >
> >The change will trigger a device_suspend() call on mii_bus device with
> >the pm suspend/resume callbacks being fec driver ones, since
> >bus->parent->driver points to fec driver.
> >
> >So net result is fec_suspend() will be called twice during suspend, once
> >in mii_bus device context and the other in fec device context.  In fec
> >context, it works just fine.  And the issue we're seeing is from mii_bus
> >device context, where the driver_data of the device is invalid.
> >
> >I do not think fec_suspend() should be called in mii_bus device context.
> 
> Right, that does not sound like a good thing to do. Does it work if
> you call pm_suspend_ignore_children() either in the FEC driver or in
> mdiobus_register()?

No, it doesn't stop fec_suspend() being called from mii_bus device
context.

> 
> I am not exactly sure what the best solution looks like at this point

Let's see if Ezequiel has any idea, but otherwise we should probably
revert the patch for 3.16 stable.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html