Hello!
Good to see that somebody cares still about this driver, one more task off
my back. :-)
On 12/07/2016 07:28 PM, Niklas Söderlund wrote:
You only enable the WOL support fo the R-Car gen2 chips but never say that
explicitly, neither in the subject nor here.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
---
drivers/net/ethernet/renesas/sh_eth.c | 120 +++++++++++++++++++++++++++++++---
drivers/net/ethernet/renesas/sh_eth.h | 4 ++
2 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 05b0dc5..3974046 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -624,7 +624,7 @@ static struct sh_eth_cpu_data r8a779x_data = {
.register_type = SH_ETH_REG_FAST_RCAR,
- .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
+ .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD | ECSR_MPD,
.ecsipr_value = ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,
.eesipr_value = 0x01ff009f,
@@ -641,6 +641,7 @@ static struct sh_eth_cpu_data r8a779x_data = {
.tpauser = 1,
.hw_swap = 1,
.rmiimode = 1,
+ .magic = 1,
};
#endif /* CONFIG_OF */
So I suggest that you add the general WOL support code in the 1st patch
and enable the new feature per SoC family in the follow up patches.
@@ -1657,6 +1658,10 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev)
goto out;
if (!likely(mdp->irq_enabled)) {
Oops, I guess unlikely(!mdp->irq_enabled) was meant here...
+ /* Handle MagicPacket interrupt */
+ if (sh_eth_read(ndev, ECSR) & ECSR_MPD)
Why do it here?
+ sh_eth_modify(ndev, ECSR, 0, ECSR_MPD);
+
sh_eth_write(ndev, 0, EESIPR);
goto out;
}
[...]
@@ -3017,6 +3051,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
goto out_release;
}
+ /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
+ mdp->clk = devm_clk_get(&pdev->dev, NULL);
Luckily, we have <linux/clk.h> #include'd...
+ if (IS_ERR(mdp->clk))
+ mdp->clk = NULL;
+
ndev->base_addr = res->start;
spin_lock_init(&mdp->lock);
@@ -3111,6 +3150,10 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
if (ret)
goto out_napi_del;
+ mdp->wol_enabled = false;
No need, the '*mdp' was kzalloc'ed.
+ if (mdp->cd->magic && mdp->clk)
+ device_set_wakeup_capable(&pdev->dev, 1);
+
/* print device information */
netdev_info(ndev, "Base address at 0x%x, %pM, IRQ %d.\n",
(u32)ndev->base_addr, ndev->dev_addr, ndev->irq);
@@ -3150,15 +3193,71 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
#ifdef CONFIG_PM
#ifdef CONFIG_PM_SLEEP
+static int sh_eth_wol_setup(struct net_device *ndev)
+{
+ struct sh_eth_private *mdp = netdev_priv(ndev);
+
+ /* Only allow ECI interrupts */
+ mdp->irq_enabled = false;
Why 'false' if you enable IRQs below?
+ synchronize_irq(ndev->irq);
+ napi_disable(&mdp->napi);
+ sh_eth_write(ndev, DMAC_M_ECI, EESIPR);
+
+ /* Enable ECI MagicPacket interrupt */
+ sh_eth_write(ndev, ECSIPR_MPDIP, ECSIPR);
+
+ /* Enable MagicPacket */
+ sh_eth_modify(ndev, ECMR, 0, ECMR_PMDE);
+
+ /* Increased clock usage so device won't be suspended */
+ clk_enable(mdp->clk);
Hum, intermixiggn runtime PM with clock API doesn't look good...
+
+ return enable_irq_wake(ndev->irq);
+}
+
+static int sh_eth_wol_restore(struct net_device *ndev)
+{
+ struct sh_eth_private *mdp = netdev_priv(ndev);
+ int ret;
+
+ napi_enable(&mdp->napi);
+
+ /* Disable MagicPacket */
+ sh_eth_modify(ndev, ECMR, ECMR_PMDE, 0);
+
+ /* The device need to be reset to restore MagicPacket logic
+ * for next wakeup. If we close and open the device it will
+ * both be reset and all registers restored. This is what
+ * happens during suspend and resume without WoL enabled.
+ */
+ ret = sh_eth_close(ndev);
+ if (ret < 0)
+ return ret;
+ ret = sh_eth_open(ndev);
+ if (ret < 0)
+ return ret;
+
+ /* Restore clock usage count */
+ clk_disable(mdp->clk);
Hm... and RPM will think the clock is still enabled?
Why disable the clock here anyway if we're resuming?
+
+ return disable_irq_wake(ndev->irq);
+}
+
[...]
@@ -3166,14 +3265,19 @@ static int sh_eth_suspend(struct device *dev)
static int sh_eth_resume(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
+ struct sh_eth_private *mdp = netdev_priv(ndev);
int ret = 0;
- if (netif_running(ndev)) {
+ if (!netif_running(ndev))
+ return 0;
+
+ if (mdp->wol_enabled)
+ ret = sh_eth_wol_restore(ndev);
+ else
ret = sh_eth_open(ndev);
- if (ret < 0)
- return ret;
+
+ if (!ret)
Please keep the original error return logic, so that you can return 0 at
the end...
netif_device_attach(ndev);
- }
return ret;
}
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index d050f37..26c6620 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
unsigned shift_rd0:1; /* shift Rx descriptor word 0 right by 16 */
unsigned rmiimode:1; /* EtherC has RMIIMODE register */
unsigned rtrate:1; /* EtherC has RTRATE register */
+ unsigned magic:1; /* EtherC have PMDE in ECMR and MPDIP in ECSIPR */
OK, e.g. RZ/A1 doesn't have these bits...
[...]
MBR, Sergei
