From: Vincent Palatin <hidden> Date: 2016-06-03 17:29:27
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv)structrk_priv_data*bsp_priv=priv;intret;+/* Keep the PHY up if we use Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return0;+ret=phy_power_on(bsp_priv,true);if(ret)returnret;
@@ -549,6 +553,10 @@ static void rk_gmac_exit(struct platform_device *pdev, void *priv){structrk_priv_data*gmac=priv;+/* The PHY was up for Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return;+phy_power_on(gmac,false);gmac_clk_enable(gmac,false);}
Hi,
Am Freitag, 3. Juni 2016, 10:29:20 schrieb Vincent Palatin:
quoted hunk
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev,
void *priv) struct rk_priv_data *bsp_priv = priv;
int ret;
+ /* Keep the PHY up if we use Wake-on-Lan. */
+ if (device_may_wakeup(&pdev->dev))
+ return 0;
+
Hmm, this looks like it would also block the initial setup of clocks and phy?
platform_device + device struct are created before probe gets called, so
something could set the wakeup flag before the driver initially probes?
Confused,
Heiko
From: Vincent Palatin <hidden> Date: 2016-06-06 21:01:21
On Mon, Jun 6, 2016 at 1:45 PM, Heiko Stübner [off-list ref] wrote:
Hi,
Am Freitag, 3. Juni 2016, 10:29:20 schrieb Vincent Palatin:
quoted
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev,
void *priv) struct rk_priv_data *bsp_priv = priv;
int ret;
+ /* Keep the PHY up if we use Wake-on-Lan. */
+ if (device_may_wakeup(&pdev->dev))
+ return 0;
+
Hmm, this looks like it would also block the initial setup of clocks and phy?
Yes, that's bad. Doug told me so but I forget to CC him on the
previous submission.
I will do another version.
platform_device + device struct are created before probe gets called, so
something could set the wakeup flag before the driver initially probes?
The device tree 'wakeup' attribute likely does it.
--
Vincent
From: Giuseppe CAVALLARO <hidden> Date: 2016-06-07 07:23:17
Hello
On 6/3/2016 7:29 PM, Vincent Palatin wrote:
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
I do not understand why you need that.
This is done inside the PHY layer and it is tested on our platforms
he idea is: If the parent wants to Wake the system then the PHY should
not power-down.
Peppe
@@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv)structrk_priv_data*bsp_priv=priv;intret;+/* Keep the PHY up if we use Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return0;+ret=phy_power_on(bsp_priv,true);if(ret)returnret;
@@ -549,6 +553,10 @@ static void rk_gmac_exit(struct platform_device *pdev, void *priv){structrk_priv_data*gmac=priv;+/* The PHY was up for Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return;+phy_power_on(gmac,false);gmac_clk_enable(gmac,false);}
From: Vincent Palatin <hidden> Date: 2016-06-08 22:26:03
On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
Hello
On 6/3/2016 7:29 PM, Vincent Palatin wrote:
quoted
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
I do not understand why you need that.
This is done inside the PHY layer and it is tested on our platforms
he idea is: If the parent wants to Wake the system then the PHY should
not power-down.
I'm not sure I understand :
you mean that this path is not called if WoL is enabled ?
[ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which
is the rk_gmac_exit() code I'm modifying ]
or the RK driver code should not power down the phy in its exit() callback ?
@@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev,
void *priv)
struct rk_priv_data *bsp_priv = priv;
int ret;
+ /* Keep the PHY up if we use Wake-on-Lan. */
+ if (device_may_wakeup(&pdev->dev))
+ return 0;
+
ret = phy_power_on(bsp_priv, true);
if (ret)
return ret;
From: Andrew Lunn <andrew@lunn.ch> Date: 2016-06-09 00:17:46
On Wed, Jun 08, 2016 at 03:25:38PM -0700, Vincent Palatin wrote:
On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
Hello
On 6/3/2016 7:29 PM, Vincent Palatin wrote:
quoted
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
I do not understand why you need that.
This is done inside the PHY layer and it is tested on our platforms
he idea is: If the parent wants to Wake the system then the PHY should
not power-down.
I'm not sure I understand :
you mean that this path is not called if WoL is enabled ?
[ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which
is the rk_gmac_exit() code I'm modifying ]
or the RK driver code should not power down the phy in its exit() callback ?
From: Vincent Palatin <hidden> Date: 2016-06-09 23:00:33
On Wed, Jun 8, 2016 at 5:17 PM, Andrew Lunn [off-list ref] wrote:
On Wed, Jun 08, 2016 at 03:25:38PM -0700, Vincent Palatin wrote:
quoted
On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
Hello
On 6/3/2016 7:29 PM, Vincent Palatin wrote:
quoted
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
I do not understand why you need that.
This is done inside the PHY layer and it is tested on our platforms
he idea is: If the parent wants to Wake the system then the PHY should
not power-down.
I'm not sure I understand :
you mean that this path is not called if WoL is enabled ?
[ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which
is the rk_gmac_exit() code I'm modifying ]
or the RK driver code should not power down the phy in its exit() callback ?
Take a look at phy_suspend().
phy_suspend() sends (or not) the PowerDown command to the PHY through
the MDIO bus, depending if WoL is disabled,
but most of my question still stands as far as I can tell :
I was trying to get a proper WoL support on the following setup :
dwmac (inside a RK3288 SoC) connected to RTL8211 PHY
The current upstream code for this case will call rk_gmac_exit() when
the MAC suspends (after the PHY has already suspended). Effectively
doing a phy_power_on(, false) which is calling regulator_disable() on
the LDO defined by the 'phy-supply' attribute.
So my reading is that the RK specific MAC code is turning off
unconditionally the PHY power regulator. Unless I'm mistaken, either
this code is incorrect for the WoL case or the naming 'phy-supply' is
misleading and should be the MAC supply.
--
Vincent
From: Giuseppe CAVALLARO <hidden> Date: 2016-06-10 12:29:54
Hello Vincent
On 6/10/2016 1:00 AM, Vincent Palatin wrote:
On Wed, Jun 8, 2016 at 5:17 PM, Andrew Lunn [off-list ref] wrote:
quoted
On Wed, Jun 08, 2016 at 03:25:38PM -0700, Vincent Palatin wrote:
quoted
On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
Hello
On 6/3/2016 7:29 PM, Vincent Palatin wrote:
quoted
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
us up.
I do not understand why you need that.
This is done inside the PHY layer and it is tested on our platforms
he idea is: If the parent wants to Wake the system then the PHY should
not power-down.
I'm not sure I understand :
you mean that this path is not called if WoL is enabled ?
[ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which
is the rk_gmac_exit() code I'm modifying ]
or the RK driver code should not power down the phy in its exit() callback ?
Take a look at phy_suspend().
phy_suspend() sends (or not) the PowerDown command to the PHY through
the MDIO bus, depending if WoL is disabled,
but most of my question still stands as far as I can tell :
I was trying to get a proper WoL support on the following setup :
dwmac (inside a RK3288 SoC) connected to RTL8211 PHY
The current upstream code for this case will call rk_gmac_exit() when
the MAC suspends (after the PHY has already suspended). Effectively
doing a phy_power_on(, false) which is calling regulator_disable() on
the LDO defined by the 'phy-supply' attribute.
So my reading is that the RK specific MAC code is turning off
unconditionally the PHY power regulator. Unless I'm mistaken, either
this code is incorrect for the WoL case or the naming 'phy-supply' is
misleading and should be the MAC supply.
ok now clear. And you are right. I can conclude that the patch is ok
for me. I just ask you to resend it elaborating a bit the subject and
surrounding the code with a comment.
I do not know your SoC but indeed, when doing WoL, some parts of the
MAC + PHY must be powered so IMO it is legal that you do not cut
the power by invoking regulator.
Peppe
From: Vincent Palatin <hidden> Date: 2016-06-11 01:01:03
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather than
re-using exit/init callbacks. So the dwmac platform driver can behave differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we can
wake up the core from it when the PHY has received the magic packet.
From: Vincent Palatin <hidden> Date: 2016-06-11 01:01:13
Let the stmmac platform drivers provide dedicated suspend and resume
callbacks rather than always re-using the init and exits callbacks.
If the driver does not provide the suspend or resume callback, we fall
back to the old behavior trying to use exit or init.
This allows a specific platform to perform only a partial power-down on
suspend if Wake-on-Lan is enabled but always perform the full shutdown
sequence if the module is unloaded.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 ++++++--
include/linux/stmmac.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
From: Vincent Palatin <hidden> Date: 2016-06-11 01:01:37
In order to use Wake-on-Lan on RK3288 integrated MAC, we need to wake-up
the CPU on the PMT interrupt when the MAC and the PHY are in low power mode.
Adding the interrupt declaration.
Signed-off-by: Vincent Palatin <redacted>
---
arch/arm/boot/dts/rk3288.dtsi | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Vincent Palatin <hidden> Date: 2016-06-11 01:01:52
When suspending the machine, do not shutdown the external PHY by cutting
its regulator in the mac platform driver suspend code if Wake-on-Lan is enabled,
else it cannot wake us up.
In order to do this, split the suspend/resume callbacks from the
init/exit callbacks, so we can condition the power-down on the lack of
need to wake-up from the LAN but do it unconditionally when unloading the
module.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 49 +++++++++++++++++++++++---
1 file changed, 44 insertions(+), 5 deletions(-)
@@ -542,15 +542,52 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv)if(ret)returnret;+bsp_priv->powered_down=true;+return0;}-staticvoidrk_gmac_exit(structplatform_device*pdev,void*priv)+staticvoidrk_gmac_powerdown(structrk_priv_data*gmac){-structrk_priv_data*gmac=priv;-phy_power_on(gmac,false);gmac_clk_enable(gmac,false);+gmac->powered_down=true;+}++staticintrk_gmac_init(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++returnrk_gmac_powerup(bsp_priv);+}++staticvoidrk_gmac_exit(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++rk_gmac_powerdown(bsp_priv);+}++staticvoidrk_gmac_suspend(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++/* Keep the PHY up if we use Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return;++rk_gmac_powerdown(bsp_priv);+}++staticvoidrk_gmac_resume(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++/* The PHY was up for Wake-on-Lan. */+if(!bsp_priv->powered_down)+return;++rk_gmac_powerup(bsp_priv);}staticvoidrk_fix_speed(void*priv,unsignedintspeed)
@@ -591,6 +628,8 @@ static int rk_gmac_probe(struct platform_device *pdev)plat_dat->init=rk_gmac_init;plat_dat->exit=rk_gmac_exit;plat_dat->fix_mac_speed=rk_fix_speed;+plat_dat->suspend=rk_gmac_suspend;+plat_dat->resume=rk_gmac_resume;plat_dat->bsp_priv=rk_gmac_setup(pdev,data);if(IS_ERR(plat_dat->bsp_priv))
From: David Miller <davem@davemloft.net> Date: 2016-06-11 01:16:17
All proper patch serieses must start with an introductory postings
ala "Subject: [PATCH 0/3] ..." which explains what the patch series
is doing at a high level, why, and how.
Thanks.
From: David Miller <davem@davemloft.net> Date: 2016-06-11 01:16:49
From: Vincent Palatin <redacted>
Date: Fri, 10 Jun 2016 18:00:36 -0700
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather than
re-using exit/init callbacks. So the dwmac platform driver can behave differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we can
wake up the core from it when the PHY has received the magic packet.
Ignore my previous email, but in the future please tag these postings
properly with "[PATCH 0/N] " at the beginning of the subject line.
Thanks.
Am Freitag, 10. Juni 2016, 18:00:38 schrieb Vincent Palatin:
quoted hunk
When suspending the machine, do not shutdown the external PHY by cutting
its regulator in the mac platform driver suspend code if Wake-on-Lan is
enabled, else it cannot wake us up.
In order to do this, split the suspend/resume callbacks from the
init/exit callbacks, so we can condition the power-down on the lack of
need to wake-up from the LAN but do it unconditionally when unloading the
module.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 49
+++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5
deletions(-)
naming it gmac->suspended and doing all accesses in the suspend/resume
callback might provide a nicer way? Now the check is in resume while the
powerdown callback is setting it.
+}
+
+static void rk_gmac_resume(struct platform_device *pdev, void *priv)
+{
+ struct rk_priv_data *bsp_priv = priv;
+
+ /* The PHY was up for Wake-on-Lan. */
+ if (!bsp_priv->powered_down)
+ return;
+
+ rk_gmac_powerup(bsp_priv);
missing something like
bsp_priv->suspended = false;
Right now it looks like your bsp_priv->powered_down will always be true
after the first suspend with powerdown.
quoted hunk
}
static void rk_fix_speed(void *priv, unsigned int speed)
@@ -591,6 +628,8 @@ static int rk_gmac_probe(struct platform_device *pdev)
From: Giuseppe CAVALLARO <hidden> Date: 2016-06-13 06:46:26
On 6/11/2016 3:00 AM, Vincent Palatin wrote:
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather than
re-using exit/init callbacks. So the dwmac platform driver can behave differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we can
wake up the core from it when the PHY has received the magic packet.
IMO these could be sent for net-next and also other glue logic
files should be reworked in order to use the new API for coherence.
Peppe
From: Vincent Palatin <hidden> Date: 2016-06-15 16:03:46
On Fri, Jun 10, 2016 at 6:57 PM, Heiko Stuebner [off-list ref] wrote:
Am Freitag, 10. Juni 2016, 18:00:38 schrieb Vincent Palatin:
quoted
When suspending the machine, do not shutdown the external PHY by cutting
its regulator in the mac platform driver suspend code if Wake-on-Lan is
enabled, else it cannot wake us up.
In order to do this, split the suspend/resume callbacks from the
init/exit callbacks, so we can condition the power-down on the lack of
need to wake-up from the LAN but do it unconditionally when unloading the
module.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 49
+++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5
deletions(-)
naming it gmac->suspended and doing all accesses in the suspend/resume
callback might provide a nicer way? Now the check is in resume while the
powerdown callback is setting it.
+}
+
+static void rk_gmac_resume(struct platform_device *pdev, void *priv)
+{
+ struct rk_priv_data *bsp_priv = priv;
+
+ /* The PHY was up for Wake-on-Lan. */
+ if (!bsp_priv->powered_down)
+ return;
+
+ rk_gmac_powerup(bsp_priv);
missing something like
bsp_priv->suspended = false;
Right now it looks like your bsp_priv->powered_down will always be true
after the first suspend with powerdown.
Yes I screw up badly, that's a good reason to use a more sensible name
for the variable.
quoted
}
static void rk_fix_speed(void *priv, unsigned int speed)
@@ -591,6 +628,8 @@ static int rk_gmac_probe(struct platform_device *pdev)
From: Vincent Palatin <hidden> Date: 2016-06-15 17:04:38
On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO
[off-list ref] wrote:
On 6/11/2016 3:00 AM, Vincent Palatin wrote:
quoted
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in
the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather
than
re-using exit/init callbacks. So the dwmac platform driver can behave
differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we
are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we
can
wake up the core from it when the PHY has received the magic packet.
IMO these could be sent for net-next and also other glue logic
files should be reworked in order to use the new API for coherence.
Given they will have the same set of functions for exit/init and
suspend/resume, you mean duplicating the callbacks like this :
From: Vincent Palatin <hidden> Date: 2016-06-15 18:32:47
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather than
re-using exit/init callbacks. So the dwmac platform driver can behave differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we can
wake up the core from it when the PHY has received the magic packet.
Changes since v1:
* rename 'powered_down' variable into 'suspended'.
* fix the logic recording the PHY suspended state according to Heiko comments.
Vincent Palatin (3):
net: stmmac: allow to split suspend/resume from init/exit callbacks
net: stmmac: dwmac-rk: keep the PHY up for WoL
ARM: dts: rockchip: add interrupt for Wake-on-Lan on RK3288
arch/arm/boot/dts/rk3288.dtsi | 5 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 48 +++++++++++++++++++---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 +++-
include/linux/stmmac.h | 2 +
4 files changed, 54 insertions(+), 9 deletions(-)
--
2.8.0.rc3.226.g39d4020
From: Vincent Palatin <hidden> Date: 2016-06-15 18:32:57
Let the stmmac platform drivers provide dedicated suspend and resume
callbacks rather than always re-using the init and exits callbacks.
If the driver does not provide the suspend or resume callback, we fall
back to the old behavior trying to use exit or init.
This allows a specific platform to perform only a partial power-down on
suspend if Wake-on-Lan is enabled but always perform the full shutdown
sequence if the module is unloaded.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 ++++++--
include/linux/stmmac.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
From: Vincent Palatin <hidden> Date: 2016-06-15 18:33:16
In order to use Wake-on-Lan on RK3288 integrated MAC, we need to wake-up
the CPU on the PMT interrupt when the MAC and the PHY are in low power mode.
Adding the interrupt declaration.
Signed-off-by: Vincent Palatin <redacted>
---
arch/arm/boot/dts/rk3288.dtsi | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Vincent Palatin <hidden> Date: 2016-06-15 18:33:42
When suspending the machine, do not shutdown the external PHY by cutting
its regulator in the mac platform driver suspend code if Wake-on-Lan is enabled,
else it cannot wake us up.
In order to do this, split the suspend/resume callbacks from the
init/exit callbacks, so we can condition the power-down on the lack of
need to wake-up from the LAN but do it unconditionally when unloading the
module.
Signed-off-by: Vincent Palatin <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 48 +++++++++++++++++++++++---
1 file changed, 43 insertions(+), 5 deletions(-)
@@ -545,14 +545,50 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv)return0;}-staticvoidrk_gmac_exit(structplatform_device*pdev,void*priv)+staticvoidrk_gmac_powerdown(structrk_priv_data*gmac){-structrk_priv_data*gmac=priv;-phy_power_on(gmac,false);gmac_clk_enable(gmac,false);}+staticintrk_gmac_init(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++returnrk_gmac_powerup(bsp_priv);+}++staticvoidrk_gmac_exit(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++rk_gmac_powerdown(bsp_priv);+}++staticvoidrk_gmac_suspend(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++/* Keep the PHY up if we use Wake-on-Lan. */+if(device_may_wakeup(&pdev->dev))+return;++rk_gmac_powerdown(bsp_priv);+bsp_priv->suspended=true;+}++staticvoidrk_gmac_resume(structplatform_device*pdev,void*priv)+{+structrk_priv_data*bsp_priv=priv;++/* The PHY was up for Wake-on-Lan. */+if(!bsp_priv->suspended)+return;++rk_gmac_powerup(bsp_priv);+bsp_priv->suspended=false;+}+staticvoidrk_fix_speed(void*priv,unsignedintspeed){structrk_priv_data*bsp_priv=priv;
@@ -591,6 +627,8 @@ static int rk_gmac_probe(struct platform_device *pdev)plat_dat->init=rk_gmac_init;plat_dat->exit=rk_gmac_exit;plat_dat->fix_mac_speed=rk_fix_speed;+plat_dat->suspend=rk_gmac_suspend;+plat_dat->resume=rk_gmac_resume;plat_dat->bsp_priv=rk_gmac_setup(pdev,data);if(IS_ERR(plat_dat->bsp_priv))
From: Giuseppe CAVALLARO <hidden> Date: 2016-06-16 13:37:51
Hi Vincent
On 6/15/2016 7:04 PM, Vincent Palatin wrote:
quoted hunk
On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
On 6/11/2016 3:00 AM, Vincent Palatin wrote:
quoted
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in
the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather
than
re-using exit/init callbacks. So the dwmac platform driver can behave
differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we
are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we
can
wake up the core from it when the PHY has received the magic packet.
IMO these could be sent for net-next and also other glue logic
files should be reworked in order to use the new API for coherence.
Given they will have the same set of functions for exit/init and
suspend/resume, you mean duplicating the callbacks like this :
@@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev)plat_dat->bsp_priv=dwmac;plat_dat->init=sti_dwmac_init;plat_dat->exit=sti_dwmac_exit;+plat_dat->suspend=sti_dwmac_exit;+plat_dat->resume=sti_dwmac_init;plat_dat->fix_mac_speed=data->fix_retime_src;ret=sti_dwmac_init(pdev,plat_dat->bsp_priv);
Is this anyhow useful ?
I think this is mandatory otherwise you are not guaranteeing the PM
stuff working on the rest of the glue-logics (not only sti); because
init/exit calls won't be called anymore. So I kindly ask you to
propagate the fix and send the V3. The implementation above is ok for
me.
peppe
From: Vincent Palatin <hidden> Date: 2016-06-16 14:51:24
Hi Giuseppe,
On Thu, Jun 16, 2016 at 6:37 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
Hi Vincent
On 6/15/2016 7:04 PM, Vincent Palatin wrote:
quoted
On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
On 6/11/2016 3:00 AM, Vincent Palatin wrote:
quoted
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in
the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather
than
re-using exit/init callbacks. So the dwmac platform driver can behave
differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we
are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we
can
wake up the core from it when the PHY has received the magic packet.
IMO these could be sent for net-next and also other glue logic
files should be reworked in order to use the new API for coherence.
Given they will have the same set of functions for exit/init and
suspend/resume, you mean duplicating the callbacks like this :
@@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev)plat_dat->bsp_priv=dwmac;plat_dat->init=sti_dwmac_init;plat_dat->exit=sti_dwmac_exit;+plat_dat->suspend=sti_dwmac_exit;+plat_dat->resume=sti_dwmac_init;plat_dat->fix_mac_speed=data->fix_retime_src;ret=sti_dwmac_init(pdev,plat_dat->bsp_priv);
Is this anyhow useful ?
I think this is mandatory otherwise you are not guaranteeing the PM
stuff working on the rest of the glue-logics (not only sti); because
init/exit calls won't be called anymore.
As mentioned in the PATCH 1/3 description: "If the driver does not
provide the suspend or resume callback, we fall
back to the old behavior trying to use exit or init. [...]"
ie.
@@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev)structplatform_device*pdev=to_platform_device(dev);ret=stmmac_suspend(dev);-if(priv->plat->exit)+if(priv->plat->suspend)+priv->plat->suspend(pdev,priv->plat->bsp_priv);+elseif(priv->plat->exit)priv->plat->exit(pdev,priv->plat->bsp_priv);returnret;
@@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev)structstmmac_priv*priv=netdev_priv(ndev);structplatform_device*pdev=to_platform_device(dev);-if(priv->plat->init)+if(priv->plat->resume)+priv->plat->resume(pdev,priv->plat->bsp_priv);+elseif(priv->plat->init)priv->plat->init(pdev,priv->plat->bsp_priv);returnstmmac_resume(dev);
So I was under the impression that everything should continue working
as before for drivers only providing init/exit,
by falling back on calling ->exit() if there is no suspend() callback
initialized for the PM calls.
You think that won't work ?
So I kindly ask you to
propagate the fix and send the V3. The implementation above is ok for
me.
peppe
From: David Miller <davem@davemloft.net> Date: 2016-06-16 21:15:22
From: Vincent Palatin <redacted>
Date: Wed, 15 Jun 2016 11:32:20 -0700
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather than
re-using exit/init callbacks. So the dwmac platform driver can behave differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we can
wake up the core from it when the PHY has received the magic packet.
Changes since v1:
* rename 'powered_down' variable into 'suspended'.
* fix the logic recording the PHY suspended state according to Heiko comments.
From: Giuseppe CAVALLARO <hidden> Date: 2016-06-17 05:38:28
On 6/16/2016 4:51 PM, Vincent Palatin wrote:
quoted hunk
Hi Giuseppe,
On Thu, Jun 16, 2016 at 6:37 AM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
Hi Vincent
On 6/15/2016 7:04 PM, Vincent Palatin wrote:
quoted
On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO
[off-list ref] wrote:
quoted
On 6/11/2016 3:00 AM, Vincent Palatin wrote:
quoted
In order to support Wake-On-Lan when using the RK3288 integrated MAC
(with an external RGMII PHY), we need to avoid shutting down the regulator
of the external PHY when the MAC is suspended as it's currently done in
the MAC
platform code.
As a first step, create independant callbacks for suspend/resume rather
than
re-using exit/init callbacks. So the dwmac platform driver can behave
differently
on suspend where it might skip shutting the PHY and at module unloading.
Then update the dwmac-rk driver to switch off the PHY regulator only if we
are
not planning to wake up from the LAN.
Finally add the PMT interrupt to the MAC device tree configuration, so we
can
wake up the core from it when the PHY has received the magic packet.
IMO these could be sent for net-next and also other glue logic
files should be reworked in order to use the new API for coherence.
Given they will have the same set of functions for exit/init and
suspend/resume, you mean duplicating the callbacks like this :
@@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev)plat_dat->bsp_priv=dwmac;plat_dat->init=sti_dwmac_init;plat_dat->exit=sti_dwmac_exit;+plat_dat->suspend=sti_dwmac_exit;+plat_dat->resume=sti_dwmac_init;plat_dat->fix_mac_speed=data->fix_retime_src;ret=sti_dwmac_init(pdev,plat_dat->bsp_priv);
Is this anyhow useful ?
I think this is mandatory otherwise you are not guaranteeing the PM
stuff working on the rest of the glue-logics (not only sti); because
init/exit calls won't be called anymore.
As mentioned in the PATCH 1/3 description: "If the driver does not
provide the suspend or resume callback, we fall
back to the old behavior trying to use exit or init. [...]"
ie.
@@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev)structplatform_device*pdev=to_platform_device(dev);ret=stmmac_suspend(dev);-if(priv->plat->exit)+if(priv->plat->suspend)+priv->plat->suspend(pdev,priv->plat->bsp_priv);+elseif(priv->plat->exit)priv->plat->exit(pdev,priv->plat->bsp_priv);returnret;
@@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev)structstmmac_priv*priv=netdev_priv(ndev);structplatform_device*pdev=to_platform_device(dev);-if(priv->plat->init)+if(priv->plat->resume)+priv->plat->resume(pdev,priv->plat->bsp_priv);+elseif(priv->plat->init)priv->plat->init(pdev,priv->plat->bsp_priv);returnstmmac_resume(dev);
So I was under the impression that everything should continue working
as before for drivers only providing init/exit,
by falling back on calling ->exit() if there is no suspend() callback
initialized for the PM calls.
You think that won't work ?
it's ok for me.
Peppe
quoted
So I kindly ask you to
propagate the fix and send the V3. The implementation above is ok for
me.
peppe