Suspend/resume broken on mx5/mx6 running 4.16

[Russell King - ARM Linux]

On Tue, Aug 05, 2014 at 02:16:59PM +0800, Shawn Guo wrote:
> On Tue, Aug 05, 2014 at 02:06:18PM +0800, Duan Fugang-B38611 wrote:
> > I use linux net tree cannot reproduce the issue like your log, but show another issue as below log:
> > (imx6dl sabresd board, Nfs mount rootfs)
> > 
> > root at freescale ~$ uname -r
> > 3.16.0-rc5-01146-gda388973d	
> 
> Fabio already bisect the issue down to commit a71e3c37960c (net: phy:
> Set the driver when registering an MDIO bus device).  This commit
> landed on mainline after v3.16-rc7, and that may be the reason you
> do not see it on net tree (3.16.0-rc5-01146-gda388973d).

Oh my... that commit looks totally bogus.

It has the effect (as can be seen from the oops) of attaching the MDIO bus
device (itself is a bus-less device) to the platform driver, which means
that if the platform driver supports power management, it will be called
to power manage the MDIO bus device.

Moreover, drivers do not expect to be called for power management
operations for devices which they haven't probed, and certainly not for
devices which aren't part of the same bus that the driver is registered
against.

The commit text says:

    net: phy: Set the driver when registering an MDIO bus device

    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.

which misses the implications of adding an unknown parent driver to that
class device - and the argument that it's just to track down the parent
driver is totally bogus.  That can already be done - it's the parent
device's driver pointer.  Let's take the example of FEC.

/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/mdio_bus/2188000.ethernet/

lrwxrwxrwx 1 root root    0 Aug  5 10:07 device -> ../../../2188000.ethernet
drwxr-xr-x 2 root root    0 Aug  5 10:07 power
lrwxrwxrwx 1 root root    0 Aug  5 10:07 subsystem -> ../../../../../../../class/mdio_bus
-rw-r--r-- 1 root root 4096 Aug  5 10:07 uevent

(note that the "%s-%x" format for this device in fec_main.c has been
truncated - that's another bug!)

Remembering that this is a class device, these devices have a "device"
symlink which point at the parent device.  Normal devices which are bound
to a driver have a "driver" symlink.

So, the driver can be reached by following the "device" pointer, and then
following the "driver" symlink:

/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/mdio_bus/2188000.ethernet/device/
...
lrwxrwxrwx 1 root root    0 Aug  5 10:08 driver -> ../../../../../bus/platform/drivers/fec
..

So, given that there are already perfectly good ways to discover the
information stated in the commit message, and that this commit causes
regression, I think this commit should be reverted.  Greg, do you
concur?

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.