netdev - RE: [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver

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] [day] [month] [year] [list]

Message-ID: <BLUPR03MB373A7E62B7922C9C337408DF5320@BLUPR03MB373.namprd03.prod.outlook.com>
Date:	Tue, 27 Jan 2015 02:27:07 +0000
From:	"fugang.duan@...escale.com" <fugang.duan@...escale.com>
To:	Florian Fainelli <f.fainelli@...il.com>,
	"davem@...emloft.net" <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>
Subject: RE: [PATCH net-next 1/1] net: phy: unbind phy device from generic and
 specifical driver

From: Florian Fainelli <f.fainelli@...il.com> Sent: Tuesday, January 27, 2015 1:47 AM
> To: Duan Fugang-B38611; davem@...emloft.net
> Cc: netdev@...r.kernel.org; s.hauer@...gutronix.de;
> bhutchings@...arflare.com; stephen@...workplumber.org
> Subject: Re: [PATCH net-next 1/1] net: phy: unbind phy device from
> generic and specifical driver
> 
> On 26/01/15 09:14, Florian Fainelli wrote:
> > On 26/01/15 00:14, Fugang Duan wrote:
> >> The current .phy_detach() function only unbind generic phy driver,
> >> which causes specifical driver suspend/resume function still work like
> Atheros AT803X PHYs.
> >>
> >> For example:
> >> ifconfig eth0 down
> >> echo mem > /sys/power/status
> >>
> >> After eth0 interface down, driver call phy_detach to unbind phy
> >> driver, and then do suspend/resume operation,
> >> at803x_suspend()/at803x_resume() functions still get called that call
> >> mdio bus read/write function. When eth0 interface down, MAC driver may
> close all clocks and mdio bus cannot work. So the issue happens.
> >
> > I was just hitting this problem on Friday evening and was about to
> > submit a similar change. Thanks!
> >
> >>
> >> The patch can unbind generic and specifical driver.
> >>
> >> Signed-off-by: Fugang Duan <B38611@...escale.com>
> >
> > Acked-by: Florian Fainelli <f.fainelli@...il.com>
> > Tested-by: Florian Fainelli <f.fainelli@...il.com>
> 
> Humm, this breaks a sequence of ifconfig down then up, the driver is
> removed, and we never get to probe it again, does this work for you using
> the FEC driver?
> 
It works fine for FEC driver with below flow case.
echo enabled > /sys/class/tty/ttymxc0/power/wakeup
echo mem > /sys/power/state
ifconfig eth0 down
echo mem > /sys/power/state
ifconfig eth0 up
ping 10.192.242.204 //ping work fine
echo mem > /sys/power/state
...


I know the reason, the first fec driver use Atheros 8031 ethernet special phy driver, like:
fec 2188000.ethernet eth0: Freescale FEC PHY driver [Atheros 8031 ethernet] (mii_bus:phy_addr=2188000.ethernet:01, irq=-1)

After down and up, fec use generic phy driver, but it still can work fine:
fec 2188000.ethernet eth0: Freescale FEC PHY driver [Generic PHY] (mii_bus:phy_addr=2188000.ethernet:01, irq=-1)

After down and up, in here, phy driver only is initialized as generic phy driver.
int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
                      u32 flags, phy_interface_t interface)
{
	 ...
        /* Assume that if there is no driver, that it doesn't
         * exist, and we should use the genphy driver.
         */
        if (NULL == d->driver) {
                if (phydev->is_c45)
                        d->driver = &genphy_driver[GENPHY_DRV_10G].driver;
                else
                        d->driver = &genphy_driver[GENPHY_DRV_1G].driver;

                err = d->driver->probe(d);
                if (err >= 0)
                        err = device_bind_driver(d);

                if (err)
                        return err;
        }
	...
}

So, I suggest to use your patches set.

Regards,
Andy