* [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
@ 2018-11-28 9:02 Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 1/2] net: phy: Fix not to call phy_resume() if PHY is not attached Yoshihiro Shimoda
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yoshihiro Shimoda @ 2018-11-28 9:02 UTC (permalink / raw)
To: andrew, f.fainelli, davem
Cc: netdev, linux-renesas-soc, devicetree, Yoshihiro Shimoda
This patch set is for R-Car Gen3 Salvator-XS boards. If we do
the following method, the phy cannot link up correctly.
1) Kernel boots by using initramfs.
--> No open the nic, so phy_device_register() and phy_probe()
deasserts the reset.
2) Kernel enters the suspend.
--> So, keep the reset signal as deassert.
--> On R-Car Salvator-XS board, unfortunately, the board power is
turned off.
3) Kernel returns from suspend.
4) ifconfig eth0 up
--> Then, since edge signal of the reset doesn't happen,
it cannot link up.
5) ifconfig eth0 down
6) ifconfig eth0 up
--> In this case, it can link up.
When resolving this issue after I got feedback from Andrew and Heiner,
I found an issue that the phy_device.c didn't call phy_resume()
if the PHY was not attached. So, patch 1 fixes it and add toggling
the phy reset to the micrel phy driver.
Changes from v1 (as RFC):
- No remove the current code of phy_device.c to avoid any side effects.
- Fix the mdio_bus_phy_resume() in phy_device.c.
- Add toggling the phy reset in micrel.c if the PHY is not attached.
Yoshihiro Shimoda (2):
net: phy: Fix not to call phy_resume() if PHY is not attached
net: phy: micrel: add toggling phy reset if PHY is not attached
drivers/net/phy/micrel.c | 8 ++++++++
drivers/net/phy/phy_device.c | 11 ++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] net: phy: Fix not to call phy_resume() if PHY is not attached
2018-11-28 9:02 [PATCH v2 0/2] net: phy: micrel: add tobling phy reset Yoshihiro Shimoda
@ 2018-11-28 9:02 ` Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 2/2] net: phy: micrel: add toggling phy reset " Yoshihiro Shimoda
2018-12-03 23:20 ` [PATCH v2 0/2] net: phy: micrel: add tobling phy reset David Miller
2 siblings, 0 replies; 9+ messages in thread
From: Yoshihiro Shimoda @ 2018-11-28 9:02 UTC (permalink / raw)
To: andrew, f.fainelli, davem
Cc: netdev, linux-renesas-soc, devicetree, Yoshihiro Shimoda
This patch fixes an issue that mdio_bus_phy_resume() doesn't call
phy_resume() if the PHY is not attached.
Fixes: 803dd9c77ac3 ("net: phy: avoid suspending twice a PHY")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/net/phy/phy_device.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e06613f..ecf5e3a9 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -227,7 +227,7 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
static DEFINE_MUTEX(phy_fixup_lock);
#ifdef CONFIG_PM
-static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
+static bool mdio_bus_phy_may_suspend(struct phy_device *phydev, bool suspend)
{
struct device_driver *drv = phydev->mdio.dev.driver;
struct phy_driver *phydrv = to_phy_driver(drv);
@@ -239,10 +239,11 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
/* PHY not attached? May suspend if the PHY has not already been
* suspended as part of a prior call to phy_disconnect() ->
* phy_detach() -> phy_suspend() because the parent netdev might be the
- * MDIO bus driver and clock gated at this point.
+ * MDIO bus driver and clock gated at this point. Also may resume if
+ * PHY is not attached.
*/
if (!netdev)
- return !phydev->suspended;
+ return suspend ? !phydev->suspended : phydev->suspended;
if (netdev->wol_enabled)
return false;
@@ -277,7 +278,7 @@ static int mdio_bus_phy_suspend(struct device *dev)
if (phydev->attached_dev && phydev->adjust_link)
phy_stop_machine(phydev);
- if (!mdio_bus_phy_may_suspend(phydev))
+ if (!mdio_bus_phy_may_suspend(phydev, true))
return 0;
return phy_suspend(phydev);
@@ -288,7 +289,7 @@ static int mdio_bus_phy_resume(struct device *dev)
struct phy_device *phydev = to_phy_device(dev);
int ret;
- if (!mdio_bus_phy_may_suspend(phydev))
+ if (!mdio_bus_phy_may_suspend(phydev, false))
goto no_resume;
ret = phy_resume(phydev);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] net: phy: micrel: add toggling phy reset if PHY is not attached
2018-11-28 9:02 [PATCH v2 0/2] net: phy: micrel: add tobling phy reset Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 1/2] net: phy: Fix not to call phy_resume() if PHY is not attached Yoshihiro Shimoda
@ 2018-11-28 9:02 ` Yoshihiro Shimoda
2018-12-03 23:20 ` [PATCH v2 0/2] net: phy: micrel: add tobling phy reset David Miller
2 siblings, 0 replies; 9+ messages in thread
From: Yoshihiro Shimoda @ 2018-11-28 9:02 UTC (permalink / raw)
To: andrew, f.fainelli, davem
Cc: netdev, linux-renesas-soc, devicetree, Yoshihiro Shimoda
This patch adds toggling phy reset if PHY is not attached. Otherwise,
some boards (e.g. R-Car H3 Salvator-XS) cannot link up correctly if
we do the following method:
1) Kernel boots by using initramfs.
--> No open the nic, so phy_device_register() and phy_probe()
deasserts the reset.
2) Kernel enters the suspend.
--> So, keep the reset signal as deassert.
--> On R-Car Salvator-XS board, unfortunately, the board power is
turned off.
3) Kernel returns from suspend.
4) ifconfig eth0 up
--> Then, since edge signal of the reset doesn't happen,
it cannot link up.
5) ifconfig eth0 down
6) ifconfig eth0 up
--> In this case, it can link up.
Reported-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/net/phy/micrel.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index c333847..fcb561b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -23,6 +23,7 @@
* ksz9477
*/
+#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/phy.h>
@@ -840,6 +841,13 @@ static int kszphy_resume(struct phy_device *phydev)
{
int ret;
+ if (!phydev->attached_dev) {
+ /* If the PHY is not attached, toggle the reset */
+ phy_device_reset(phydev, 1);
+ udelay(1);
+ phy_device_reset(phydev, 0);
+ }
+
genphy_resume(phydev);
ret = kszphy_config_reset(phydev);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-11-28 9:02 [PATCH v2 0/2] net: phy: micrel: add tobling phy reset Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 1/2] net: phy: Fix not to call phy_resume() if PHY is not attached Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 2/2] net: phy: micrel: add toggling phy reset " Yoshihiro Shimoda
@ 2018-12-03 23:20 ` David Miller
2018-12-03 23:24 ` Florian Fainelli
2 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2018-12-03 23:20 UTC (permalink / raw)
To: yoshihiro.shimoda.uh
Cc: andrew, f.fainelli, netdev, linux-renesas-soc, devicetree
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Wed, 28 Nov 2018 09:02:41 +0000
> This patch set is for R-Car Gen3 Salvator-XS boards. If we do
> the following method, the phy cannot link up correctly.
>
> 1) Kernel boots by using initramfs.
> --> No open the nic, so phy_device_register() and phy_probe()
> deasserts the reset.
> 2) Kernel enters the suspend.
> --> So, keep the reset signal as deassert.
> --> On R-Car Salvator-XS board, unfortunately, the board power is
> turned off.
> 3) Kernel returns from suspend.
> 4) ifconfig eth0 up
> --> Then, since edge signal of the reset doesn't happen,
> it cannot link up.
> 5) ifconfig eth0 down
> 6) ifconfig eth0 up
> --> In this case, it can link up.
>
> When resolving this issue after I got feedback from Andrew and Heiner,
> I found an issue that the phy_device.c didn't call phy_resume()
> if the PHY was not attached. So, patch 1 fixes it and add toggling
> the phy reset to the micrel phy driver.
>
> Changes from v1 (as RFC):
> - No remove the current code of phy_device.c to avoid any side effects.
> - Fix the mdio_bus_phy_resume() in phy_device.c.
> - Add toggling the phy reset in micrel.c if the PHY is not attached.
Series applied, thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-12-03 23:20 ` [PATCH v2 0/2] net: phy: micrel: add tobling phy reset David Miller
@ 2018-12-03 23:24 ` Florian Fainelli
2018-12-03 23:37 ` David Miller
2018-12-04 10:43 ` Andrew Lunn
0 siblings, 2 replies; 9+ messages in thread
From: Florian Fainelli @ 2018-12-03 23:24 UTC (permalink / raw)
To: David Miller, yoshihiro.shimoda.uh
Cc: andrew, netdev, linux-renesas-soc, devicetree
On 12/3/18 3:20 PM, David Miller wrote:
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Date: Wed, 28 Nov 2018 09:02:41 +0000
>
>> This patch set is for R-Car Gen3 Salvator-XS boards. If we do
>> the following method, the phy cannot link up correctly.
>>
>> 1) Kernel boots by using initramfs.
>> --> No open the nic, so phy_device_register() and phy_probe()
>> deasserts the reset.
>> 2) Kernel enters the suspend.
>> --> So, keep the reset signal as deassert.
>> --> On R-Car Salvator-XS board, unfortunately, the board power is
>> turned off.
>> 3) Kernel returns from suspend.
>> 4) ifconfig eth0 up
>> --> Then, since edge signal of the reset doesn't happen,
>> it cannot link up.
>> 5) ifconfig eth0 down
>> 6) ifconfig eth0 up
>> --> In this case, it can link up.
>>
>> When resolving this issue after I got feedback from Andrew and Heiner,
>> I found an issue that the phy_device.c didn't call phy_resume()
>> if the PHY was not attached. So, patch 1 fixes it and add toggling
>> the phy reset to the micrel phy driver.
>>
>> Changes from v1 (as RFC):
>> - No remove the current code of phy_device.c to avoid any side effects.
>> - Fix the mdio_bus_phy_resume() in phy_device.c.
>> - Add toggling the phy reset in micrel.c if the PHY is not attached.
>
> Series applied, thank you.
Meh! I guess we should be faster at reviewing stuff :/
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-12-03 23:24 ` Florian Fainelli
@ 2018-12-03 23:37 ` David Miller
2018-12-04 10:43 ` Andrew Lunn
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2018-12-03 23:37 UTC (permalink / raw)
To: f.fainelli
Cc: yoshihiro.shimoda.uh, andrew, netdev, linux-renesas-soc, devicetree
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 3 Dec 2018 15:24:43 -0800
> Meh! I guess we should be faster at reviewing stuff :/
Sorry, it was posted last Wednesday!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-12-03 23:24 ` Florian Fainelli
2018-12-03 23:37 ` David Miller
@ 2018-12-04 10:43 ` Andrew Lunn
2018-12-04 16:49 ` David Miller
2018-12-17 10:46 ` Yoshihiro Shimoda
1 sibling, 2 replies; 9+ messages in thread
From: Andrew Lunn @ 2018-12-04 10:43 UTC (permalink / raw)
To: Florian Fainelli
Cc: David Miller, yoshihiro.shimoda.uh, netdev, linux-renesas-soc,
devicetree
> > Series applied, thank you.
>
> Meh! I guess we should be faster at reviewing stuff :/
And there have been things going on behind the scenes to try to get a
real fix. Sorry, i should of mentioned that.
David, could you revert this please?
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-12-04 10:43 ` Andrew Lunn
@ 2018-12-04 16:49 ` David Miller
2018-12-17 10:46 ` Yoshihiro Shimoda
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2018-12-04 16:49 UTC (permalink / raw)
To: andrew
Cc: f.fainelli, yoshihiro.shimoda.uh, netdev, linux-renesas-soc, devicetree
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 4 Dec 2018 11:43:33 +0100
>> > Series applied, thank you.
>>
>> Meh! I guess we should be faster at reviewing stuff :/
>
> And there have been things going on behind the scenes to try to get a
> real fix. Sorry, i should of mentioned that.
>
> David, could you revert this please?
Sure, done.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 0/2] net: phy: micrel: add tobling phy reset
2018-12-04 10:43 ` Andrew Lunn
2018-12-04 16:49 ` David Miller
@ 2018-12-17 10:46 ` Yoshihiro Shimoda
1 sibling, 0 replies; 9+ messages in thread
From: Yoshihiro Shimoda @ 2018-12-17 10:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: David Miller, netdev, linux-renesas-soc, devicetree
Hello Andrew, Florian,
> From: Andrew Lunn, Sent: Tuesday, December 4, 2018 7:44 PM
>
> > > Series applied, thank you.
> >
> > Meh! I guess we should be faster at reviewing stuff :/
>
> And there have been things going on behind the scenes to try to get a
> real fix. Sorry, i should of mentioned that.
Sorry for the delayed response. How to try to get a real fix?
I have two ideas:
Idea 1: Assert/deassert the reset in phy_{suspend,resume}()
- call phy_device_reset(phydev, 0); if !phydev->attached_dev in phy_suspend()
- call phy_device_reset(phydev, 1); if !phydev->attached_dev in phy_resume()
Idea 2: Toggle the reset in phy_resume().
- call phy_device_reset(phydev, 0); udelay(1); phy_device_reset(phydev, 1);
if !phydev->attached_dev in phy_resume ()
I think the idea 1 is better. But, what do you think?
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-17 10:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 9:02 [PATCH v2 0/2] net: phy: micrel: add tobling phy reset Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 1/2] net: phy: Fix not to call phy_resume() if PHY is not attached Yoshihiro Shimoda
2018-11-28 9:02 ` [PATCH v2 2/2] net: phy: micrel: add toggling phy reset " Yoshihiro Shimoda
2018-12-03 23:20 ` [PATCH v2 0/2] net: phy: micrel: add tobling phy reset David Miller
2018-12-03 23:24 ` Florian Fainelli
2018-12-03 23:37 ` David Miller
2018-12-04 10:43 ` Andrew Lunn
2018-12-04 16:49 ` David Miller
2018-12-17 10:46 ` Yoshihiro Shimoda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).