* [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines
2020-04-16 17:44 [PATCH 0/5] net: macb: Wake-on-Lan magic packet fixes and GEM handling nicolas.ferre
@ 2020-04-16 17:44 ` nicolas.ferre
2020-04-16 18:26 ` Harini Katakam
2020-04-16 19:21 ` Florian Fainelli
2020-04-16 17:44 ` [PATCH 2/5] net: macb: mark device wake capable when "magic-packet" property present nicolas.ferre
` (3 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: nicolas.ferre @ 2020-04-16 17:44 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
michal.simek, Nicolas Ferre, Rafal Ozieblo
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Use the proper struct device pointer to check if the wakeup flag
and wakeup source are positioned.
Use the one passed by function call which is equivalent to
&bp->dev->dev.parent.
It's preventing the trigger of a spurious interrupt in case the
Wake-on-Lan feature is used.
Fixes: bc1109d04c39 ("net: macb: Add pm runtime support")
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Harini Katakam <harini.katakam@xilinx.com>
Cc: Rafal Ozieblo <rafalo@cadence.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a0e8c5bbabc0..d1b4d6b6d7c8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4616,7 +4616,7 @@ static int __maybe_unused macb_runtime_suspend(struct device *dev)
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
- if (!(device_may_wakeup(&bp->dev->dev))) {
+ if (!(device_may_wakeup(dev))) {
clk_disable_unprepare(bp->tx_clk);
clk_disable_unprepare(bp->hclk);
clk_disable_unprepare(bp->pclk);
@@ -4632,7 +4632,7 @@ static int __maybe_unused macb_runtime_resume(struct device *dev)
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
- if (!(device_may_wakeup(&bp->dev->dev))) {
+ if (!(device_may_wakeup(dev))) {
clk_prepare_enable(bp->pclk);
clk_prepare_enable(bp->hclk);
clk_prepare_enable(bp->tx_clk);
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines
2020-04-16 17:44 ` [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines nicolas.ferre
@ 2020-04-16 18:26 ` Harini Katakam
2020-04-17 12:33 ` Nicolas Ferre
2020-04-16 19:21 ` Florian Fainelli
1 sibling, 1 reply; 17+ messages in thread
From: Harini Katakam @ 2020-04-16 18:26 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
Michal Simek, Rafal Ozieblo
Hi Nicolas,
> -----Original Message-----
> From: nicolas.ferre@microchip.com [mailto:nicolas.ferre@microchip.com]
> Sent: Thursday, April 16, 2020 11:14 PM
> To: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; Claudiu
> Beznea <claudiu.beznea@microchip.com>; Harini Katakam
> <harinik@xilinx.com>
> Cc: linux-kernel@vger.kernel.org; David S. Miller <davem@davemloft.net>;
> Alexandre Belloni <alexandre.belloni@bootlin.com>; pthombar@cadence.com;
> sergio.prado@e-labworks.com; antoine.tenart@bootlin.com;
> f.fainelli@gmail.com; linux@armlinux.org.uk; andrew@lunn.ch; Michal Simek
> <michals@xilinx.com>; Nicolas Ferre <nicolas.ferre@microchip.com>; Rafal
> Ozieblo <rafalo@cadence.com>
> Subject: [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume
> routines
>
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Use the proper struct device pointer to check if the wakeup flag and wakeup
> source are positioned.
> Use the one passed by function call which is equivalent to &bp->dev-
> >dev.parent.
>
> It's preventing the trigger of a spurious interrupt in case the Wake-on-Lan
> feature is used.
Sorry I have some mail issues; meant to reply earlier.
Tested patches 1, 2, 3 in this set and they work for me.
I'll try patch 4; it looks similar to what I'm using locally but I'll add whatever
tie-off queue handling is required on top of your series, thanks.
Regards,
Harini
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines
2020-04-16 18:26 ` Harini Katakam
@ 2020-04-17 12:33 ` Nicolas Ferre
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Ferre @ 2020-04-17 12:33 UTC (permalink / raw)
To: Harini Katakam, linux-arm-kernel, netdev, Claudiu Beznea
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
Michal Simek, Rafal Ozieblo
On 16/04/2020 at 20:26, Harini Katakam wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Nicolas,
>
>> -----Original Message-----
>> From: nicolas.ferre@microchip.com [mailto:nicolas.ferre@microchip.com]
>> Sent: Thursday, April 16, 2020 11:14 PM
>> To: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; Claudiu
>> Beznea <claudiu.beznea@microchip.com>; Harini Katakam
>> <harinik@xilinx.com>
>> Cc: linux-kernel@vger.kernel.org; David S. Miller <davem@davemloft.net>;
>> Alexandre Belloni <alexandre.belloni@bootlin.com>; pthombar@cadence.com;
>> sergio.prado@e-labworks.com; antoine.tenart@bootlin.com;
>> f.fainelli@gmail.com; linux@armlinux.org.uk; andrew@lunn.ch; Michal Simek
>> <michals@xilinx.com>; Nicolas Ferre <nicolas.ferre@microchip.com>; Rafal
>> Ozieblo <rafalo@cadence.com>
>> Subject: [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume
>> routines
>>
>> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>>
>> Use the proper struct device pointer to check if the wakeup flag and wakeup
>> source are positioned.
>> Use the one passed by function call which is equivalent to &bp->dev-
>>> dev.parent.
>>
>> It's preventing the trigger of a spurious interrupt in case the Wake-on-Lan
>> feature is used.
>
> Sorry I have some mail issues; meant to reply earlier.
> Tested patches 1, 2, 3 in this set and they work for me.
Brilliant! Thanks for the feedback.
> I'll try patch 4; it looks similar to what I'm using locally but I'll add whatever
> tie-off queue handling is required on top of your series, thanks.
Alright, I'll hold my v2 for a few days then. Thanks. Best regards,
Nicolas
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines
2020-04-16 17:44 ` [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines nicolas.ferre
2020-04-16 18:26 ` Harini Katakam
@ 2020-04-16 19:21 ` Florian Fainelli
1 sibling, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2020-04-16 19:21 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek,
Rafal Ozieblo
On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Use the proper struct device pointer to check if the wakeup flag
> and wakeup source are positioned.
> Use the one passed by function call which is equivalent to
> &bp->dev->dev.parent.
>
> It's preventing the trigger of a spurious interrupt in case the
> Wake-on-Lan feature is used.
>
> Fixes: bc1109d04c39 ("net: macb: Add pm runtime support")
> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> Cc: Harini Katakam <harini.katakam@xilinx.com>
> Cc: Rafal Ozieblo <rafalo@cadence.com>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] net: macb: mark device wake capable when "magic-packet" property present
2020-04-16 17:44 [PATCH 0/5] net: macb: Wake-on-Lan magic packet fixes and GEM handling nicolas.ferre
2020-04-16 17:44 ` [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines nicolas.ferre
@ 2020-04-16 17:44 ` nicolas.ferre
2020-04-16 19:21 ` Florian Fainelli
2020-04-16 17:44 ` [PATCH 3/5] net: macb: fix macb_get/set_wol() when moving to phylink nicolas.ferre
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: nicolas.ferre @ 2020-04-16 17:44 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
michal.simek, Nicolas Ferre, Rafal Ozieblo
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Change the way the "magic-packet" DT property is handled in the
macb_probe() function, matching DT binding documentation.
Now we mark the device as "wakeup capable" instead of calling the
device_init_wakeup() function that would enable the wakeup source.
For Ethernet WoL, enabling the wakeup_source is done by
using ethtool and associated macb_set_wol() function that
already calls device_set_wakeup_enable() for this purpose.
That would reduce power consumption by cutting more clocks if
"magic-packet" property is set but WoL is not configured by ethtool.
Fixes: 3e2a5e153906 ("net: macb: add wake-on-lan support via magic packet")
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Harini Katakam <harini.katakam@xilinx.com>
Cc: Rafal Ozieblo <rafalo@cadence.com>
Cc: Sergio Prado <sergio.prado@e-labworks.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d1b4d6b6d7c8..629660d9f17e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4384,7 +4384,7 @@ static int macb_probe(struct platform_device *pdev)
bp->wol = 0;
if (of_get_property(np, "magic-packet", NULL))
bp->wol |= MACB_WOL_HAS_MAGIC_PACKET;
- device_init_wakeup(&pdev->dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET);
+ device_set_wakeup_capable(&pdev->dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET);
spin_lock_init(&bp->lock);
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] net: macb: mark device wake capable when "magic-packet" property present
2020-04-16 17:44 ` [PATCH 2/5] net: macb: mark device wake capable when "magic-packet" property present nicolas.ferre
@ 2020-04-16 19:21 ` Florian Fainelli
0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2020-04-16 19:21 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek,
Rafal Ozieblo
On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Change the way the "magic-packet" DT property is handled in the
> macb_probe() function, matching DT binding documentation.
> Now we mark the device as "wakeup capable" instead of calling the
> device_init_wakeup() function that would enable the wakeup source.
>
> For Ethernet WoL, enabling the wakeup_source is done by
> using ethtool and associated macb_set_wol() function that
> already calls device_set_wakeup_enable() for this purpose.
>
> That would reduce power consumption by cutting more clocks if
> "magic-packet" property is set but WoL is not configured by ethtool.
>
> Fixes: 3e2a5e153906 ("net: macb: add wake-on-lan support via magic packet")
> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> Cc: Harini Katakam <harini.katakam@xilinx.com>
> Cc: Rafal Ozieblo <rafalo@cadence.com>
> Cc: Sergio Prado <sergio.prado@e-labworks.com>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5] net: macb: fix macb_get/set_wol() when moving to phylink
2020-04-16 17:44 [PATCH 0/5] net: macb: Wake-on-Lan magic packet fixes and GEM handling nicolas.ferre
2020-04-16 17:44 ` [PATCH 1/5] net: macb: fix wakeup test in runtime suspend/resume routines nicolas.ferre
2020-04-16 17:44 ` [PATCH 2/5] net: macb: mark device wake capable when "magic-packet" property present nicolas.ferre
@ 2020-04-16 17:44 ` nicolas.ferre
2020-04-16 19:22 ` Florian Fainelli
2020-04-16 17:44 ` [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller nicolas.ferre
2020-04-16 17:44 ` [PATCH 5/5] net: macb: Add WoL interrupt support for MACB " nicolas.ferre
4 siblings, 1 reply; 17+ messages in thread
From: nicolas.ferre @ 2020-04-16 17:44 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
michal.simek, Nicolas Ferre, Rafal Ozieblo
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Keep previous function goals and integrate phylink actions to them.
phylink_ethtool_get_wol() is not enough to figure out if Ethernet driver
supports Wake-on-Lan.
Initialization of "supported" and "wolopts" members is done in phylink
function, no need to keep them in calling function.
phylink_ethtool_set_wol() return value is not enough to determine
if WoL is enabled for the calling Ethernet driver. Call if first
but don't rely on its return value as most of simple PHY drivers
don't implement a set_wol() function.
Fixes: 7897b071ac3b ("net: macb: convert to phylink")
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Harini Katakam <harini.katakam@xilinx.com>
Cc: Rafal Ozieblo <rafalo@cadence.com>
Cc: Antoine Tenart <antoine.tenart@bootlin.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 629660d9f17e..b17a33c60cd4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2813,21 +2813,23 @@ static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct macb *bp = netdev_priv(netdev);
- wol->supported = 0;
- wol->wolopts = 0;
-
- if (bp->wol & MACB_WOL_HAS_MAGIC_PACKET)
+ if (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) {
phylink_ethtool_get_wol(bp->phylink, wol);
+ wol->supported |= WAKE_MAGIC;
+
+ if (bp->wol & MACB_WOL_ENABLED)
+ wol->wolopts |= WAKE_MAGIC;
+ }
}
static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct macb *bp = netdev_priv(netdev);
- int ret;
- ret = phylink_ethtool_set_wol(bp->phylink, wol);
- if (!ret)
- return 0;
+ /* Pass the order to phylink layer.
+ * Don't test return value as set_wol() is often not supported.
+ */
+ phylink_ethtool_set_wol(bp->phylink, wol);
if (!(bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ||
(wol->wolopts & ~WAKE_MAGIC))
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] net: macb: fix macb_get/set_wol() when moving to phylink
2020-04-16 17:44 ` [PATCH 3/5] net: macb: fix macb_get/set_wol() when moving to phylink nicolas.ferre
@ 2020-04-16 19:22 ` Florian Fainelli
0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2020-04-16 19:22 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek,
Rafal Ozieblo
On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Keep previous function goals and integrate phylink actions to them.
>
> phylink_ethtool_get_wol() is not enough to figure out if Ethernet driver
> supports Wake-on-Lan.
> Initialization of "supported" and "wolopts" members is done in phylink
> function, no need to keep them in calling function.
>
> phylink_ethtool_set_wol() return value is not enough to determine
> if WoL is enabled for the calling Ethernet driver. Call if first
> but don't rely on its return value as most of simple PHY drivers
> don't implement a set_wol() function.
>
> Fixes: 7897b071ac3b ("net: macb: convert to phylink")
> Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> Cc: Harini Katakam <harini.katakam@xilinx.com>
> Cc: Rafal Ozieblo <rafalo@cadence.com>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-16 17:44 [PATCH 0/5] net: macb: Wake-on-Lan magic packet fixes and GEM handling nicolas.ferre
` (2 preceding siblings ...)
2020-04-16 17:44 ` [PATCH 3/5] net: macb: fix macb_get/set_wol() when moving to phylink nicolas.ferre
@ 2020-04-16 17:44 ` nicolas.ferre
2020-04-16 19:25 ` Florian Fainelli
2020-04-17 17:14 ` Nicolas Ferre
2020-04-16 17:44 ` [PATCH 5/5] net: macb: Add WoL interrupt support for MACB " nicolas.ferre
4 siblings, 2 replies; 17+ messages in thread
From: nicolas.ferre @ 2020-04-16 17:44 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
michal.simek, Nicolas Ferre
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Adapt the Wake-on-Lan feature to the Cadence GEM Ethernet controller.
This controller has different register layout and cannot be handled by
previous code.
We disable completely interrupts on all the queues but the queue 0.
Handling of WoL interrupt is done in another interrupt handler
positioned depending on the controller version used, just between
suspend() and resume() calls.
It allows to lower pressure on the generic interrupt hot path by
removing the need to handle 2 tests for each IRQ: the first figuring out
the controller revision, the second for actually knowing if the WoL bit
is set.
Queue management in suspend()/resume() functions inspired from RFC patch
by Harini Katakam <harinik@xilinx.com>, thanks!
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/net/ethernet/cadence/macb.h | 3 +
drivers/net/ethernet/cadence/macb_main.c | 121 ++++++++++++++++++++---
2 files changed, 109 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index ab827fb4b6b9..4f1b41569260 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -90,6 +90,7 @@
#define GEM_SA3T 0x009C /* Specific3 Top */
#define GEM_SA4B 0x00A0 /* Specific4 Bottom */
#define GEM_SA4T 0x00A4 /* Specific4 Top */
+#define GEM_WOL 0x00b8 /* Wake on LAN */
#define GEM_EFTSH 0x00e8 /* PTP Event Frame Transmitted Seconds Register 47:32 */
#define GEM_EFRSH 0x00ec /* PTP Event Frame Received Seconds Register 47:32 */
#define GEM_PEFTSH 0x00f0 /* PTP Peer Event Frame Transmitted Seconds Register 47:32 */
@@ -396,6 +397,8 @@
#define MACB_PDRSFT_SIZE 1
#define MACB_SRI_OFFSET 26 /* TSU Seconds Register Increment */
#define MACB_SRI_SIZE 1
+#define GEM_WOL_OFFSET 28 /* Enable wake-on-lan interrupt */
+#define GEM_WOL_SIZE 1
/* Timer increment fields */
#define MACB_TI_CNS_OFFSET 0
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b17a33c60cd4..71e6afbdfb47 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1513,6 +1513,34 @@ static void macb_tx_restart(struct macb_queue *queue)
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
}
+static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
+{
+ struct macb_queue *queue = dev_id;
+ struct macb *bp = queue->bp;
+ u32 status;
+
+ status = queue_readl(queue, ISR);
+
+ if (unlikely(!status))
+ return IRQ_NONE;
+
+ spin_lock(&bp->lock);
+
+ if (status & GEM_BIT(WOL)) {
+ queue_writel(queue, IDR, GEM_BIT(WOL));
+ gem_writel(bp, WOL, 0);
+ netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
+ (unsigned int)(queue - bp->queues),
+ (unsigned long)status);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, GEM_BIT(WOL));
+ }
+
+ spin_unlock(&bp->lock);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t macb_interrupt(int irq, void *dev_id)
{
struct macb_queue *queue = dev_id;
@@ -3306,6 +3334,8 @@ static const struct ethtool_ops macb_ethtool_ops = {
static const struct ethtool_ops gem_ethtool_ops = {
.get_regs_len = macb_get_regs_len,
.get_regs = macb_get_regs,
+ .get_wol = macb_get_wol,
+ .set_wol = macb_set_wol,
.get_link = ethtool_op_get_link,
.get_ts_info = macb_get_ts_info,
.get_ethtool_stats = gem_get_ethtool_stats,
@@ -4534,23 +4564,56 @@ static int __maybe_unused macb_suspend(struct device *dev)
struct macb_queue *queue = bp->queues;
unsigned long flags;
unsigned int q;
+ int err;
if (!netif_running(netdev))
return 0;
if (bp->wol & MACB_WOL_ENABLED) {
- macb_writel(bp, IER, MACB_BIT(WOL));
- macb_writel(bp, WOL, MACB_BIT(MAG));
- enable_irq_wake(bp->queues[0].irq);
- netif_device_detach(netdev);
- } else {
- netif_device_detach(netdev);
+ spin_lock_irqsave(&bp->lock, flags);
+ /* Flush all status bits */
+ macb_writel(bp, TSR, -1);
+ macb_writel(bp, RSR, -1);
for (q = 0, queue = bp->queues; q < bp->num_queues;
- ++q, ++queue)
- napi_disable(&queue->napi);
- rtnl_lock();
- phylink_stop(bp->phylink);
- rtnl_unlock();
+ ++q, ++queue) {
+ /* Disable all interrupts */
+ queue_writel(queue, IDR, -1);
+ queue_readl(queue, ISR);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, -1);
+ }
+ /* Change interrupt handler and
+ * Enable WoL IRQ on queue 0
+ */
+ if (macb_is_gem(bp)) {
+ devm_free_irq(dev, bp->queues[0].irq, bp->queues);
+ err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
+ IRQF_SHARED, netdev->name, bp->queues);
+ if (err) {
+ dev_err(dev,
+ "Unable to request IRQ %d (error %d)\n",
+ bp->queues[0].irq, err);
+ return err;
+ }
+ queue_writel(bp->queues, IER, GEM_BIT(WOL));
+ gem_writel(bp, WOL, MACB_BIT(MAG));
+ } else {
+ queue_writel(bp->queues, IER, MACB_BIT(WOL));
+ macb_writel(bp, WOL, MACB_BIT(MAG));
+ }
+ spin_unlock_irqrestore(&bp->lock, flags);
+
+ enable_irq_wake(bp->queues[0].irq);
+ }
+
+ netif_device_detach(netdev);
+ for (q = 0, queue = bp->queues; q < bp->num_queues;
+ ++q, ++queue)
+ napi_disable(&queue->napi);
+
+ if (!(bp->wol & MACB_WOL_ENABLED)) {
+ phy_stop(netdev->phydev);
+ phy_suspend(netdev->phydev);
spin_lock_irqsave(&bp->lock, flags);
macb_reset_hw(bp);
spin_unlock_irqrestore(&bp->lock, flags);
@@ -4575,20 +4638,48 @@ static int __maybe_unused macb_resume(struct device *dev)
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
struct macb_queue *queue = bp->queues;
+ unsigned long flags;
unsigned int q;
+ int err;
if (!netif_running(netdev))
return 0;
pm_runtime_force_resume(dev);
+ macb_writel(bp, NCR, MACB_BIT(MPE));
+
if (bp->wol & MACB_WOL_ENABLED) {
- macb_writel(bp, IDR, MACB_BIT(WOL));
- macb_writel(bp, WOL, 0);
+ spin_lock_irqsave(&bp->lock, flags);
+ /* Disable WoL */
+ if (macb_is_gem(bp)) {
+ queue_writel(bp->queues, IDR, GEM_BIT(WOL));
+ gem_writel(bp, WOL, 0);
+ } else {
+ queue_writel(bp->queues, IDR, MACB_BIT(WOL));
+ macb_writel(bp, WOL, 0);
+ }
+ /* Clear ISR on queue 0 */
+ queue_readl(bp->queues, ISR);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(bp->queues, ISR, -1);
+ /* Replace interrupt handler on queue 0 */
+ devm_free_irq(dev, bp->queues[0].irq, bp->queues);
+ err = devm_request_irq(dev, bp->queues[0].irq, macb_interrupt,
+ IRQF_SHARED, netdev->name, bp->queues);
+ if (err) {
+ dev_err(dev,
+ "Unable to request IRQ %d (error %d)\n",
+ bp->queues[0].irq, err);
+ return err;
+ }
+ spin_unlock_irqrestore(&bp->lock, flags);
+
disable_irq_wake(bp->queues[0].irq);
+ for (q = 0, queue = bp->queues; q < bp->num_queues;
+ ++q, ++queue)
+ napi_enable(&queue->napi);
} else {
- macb_writel(bp, NCR, MACB_BIT(MPE));
-
if (netdev->hw_features & NETIF_F_NTUPLE)
gem_writel_n(bp, ETHT, SCRT2_ETHT, bp->pm_data.scrt2);
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-16 17:44 ` [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller nicolas.ferre
@ 2020-04-16 19:25 ` Florian Fainelli
2020-04-17 12:57 ` Nicolas Ferre
2020-04-17 17:14 ` Nicolas Ferre
1 sibling, 1 reply; 17+ messages in thread
From: Florian Fainelli @ 2020-04-16 19:25 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek
On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Adapt the Wake-on-Lan feature to the Cadence GEM Ethernet controller.
> This controller has different register layout and cannot be handled by
> previous code.
> We disable completely interrupts on all the queues but the queue 0.
> Handling of WoL interrupt is done in another interrupt handler
> positioned depending on the controller version used, just between
> suspend() and resume() calls.
> It allows to lower pressure on the generic interrupt hot path by
> removing the need to handle 2 tests for each IRQ: the first figuring out
> the controller revision, the second for actually knowing if the WoL bit
> is set.
>
> Queue management in suspend()/resume() functions inspired from RFC patch
> by Harini Katakam <harinik@xilinx.com>, thanks!
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
[snip]
>
> +static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
> +{
> + struct macb_queue *queue = dev_id;
> + struct macb *bp = queue->bp;
> + u32 status;
> +
> + status = queue_readl(queue, ISR);
> +
> + if (unlikely(!status))
> + return IRQ_NONE;
> +
> + spin_lock(&bp->lock);
> +
> + if (status & GEM_BIT(WOL)) {
> + queue_writel(queue, IDR, GEM_BIT(WOL));
> + gem_writel(bp, WOL, 0);
> + netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
> + (unsigned int)(queue - bp->queues),
> + (unsigned long)status);
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, GEM_BIT(WOL));
You would also need a pm_wakeup_event() call here to record that this
device did wake-up the system.
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-16 19:25 ` Florian Fainelli
@ 2020-04-17 12:57 ` Nicolas Ferre
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Ferre @ 2020-04-17 12:57 UTC (permalink / raw)
To: Florian Fainelli, linux-arm-kernel, netdev, Claudiu Beznea,
harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek
Florian,
Thank you for your review of the series!
On 16/04/2020 at 21:25, Florian Fainelli wrote:
> On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
>> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>>
>> Adapt the Wake-on-Lan feature to the Cadence GEM Ethernet controller.
>> This controller has different register layout and cannot be handled by
>> previous code.
>> We disable completely interrupts on all the queues but the queue 0.
>> Handling of WoL interrupt is done in another interrupt handler
>> positioned depending on the controller version used, just between
>> suspend() and resume() calls.
>> It allows to lower pressure on the generic interrupt hot path by
>> removing the need to handle 2 tests for each IRQ: the first figuring out
>> the controller revision, the second for actually knowing if the WoL bit
>> is set.
>>
>> Queue management in suspend()/resume() functions inspired from RFC patch
>> by Harini Katakam <harinik@xilinx.com>, thanks!
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>> ---
>
> [snip]
>
>>
>> +static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
>> +{
>> + struct macb_queue *queue = dev_id;
>> + struct macb *bp = queue->bp;
>> + u32 status;
>> +
>> + status = queue_readl(queue, ISR);
>> +
>> + if (unlikely(!status))
>> + return IRQ_NONE;
>> +
>> + spin_lock(&bp->lock);
>> +
>> + if (status & GEM_BIT(WOL)) {
>> + queue_writel(queue, IDR, GEM_BIT(WOL));
>> + gem_writel(bp, WOL, 0);
>> + netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
>> + (unsigned int)(queue - bp->queues),
>> + (unsigned long)status);
>> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> + queue_writel(queue, ISR, GEM_BIT(WOL));
>
> You would also need a pm_wakeup_event() call here to record that this
> device did wake-up the system.
Oh yes, indeed that's missing. I'll add it to my v2.
Thanks. Best regards,
Nicolas
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-16 17:44 ` [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller nicolas.ferre
2020-04-16 19:25 ` Florian Fainelli
@ 2020-04-17 17:14 ` Nicolas Ferre
2020-04-21 8:21 ` Nicolas.Ferre
1 sibling, 1 reply; 17+ messages in thread
From: Nicolas Ferre @ 2020-04-17 17:14 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam,
f.fainelli, linux
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, andrew, michal.simek
On 16/04/2020 at 19:44, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Adapt the Wake-on-Lan feature to the Cadence GEM Ethernet controller.
> This controller has different register layout and cannot be handled by
> previous code.
> We disable completely interrupts on all the queues but the queue 0.
> Handling of WoL interrupt is done in another interrupt handler
> positioned depending on the controller version used, just between
> suspend() and resume() calls.
> It allows to lower pressure on the generic interrupt hot path by
> removing the need to handle 2 tests for each IRQ: the first figuring out
> the controller revision, the second for actually knowing if the WoL bit
> is set.
>
> Queue management in suspend()/resume() functions inspired from RFC patch
> by Harini Katakam <harinik@xilinx.com>, thanks!
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 3 +
> drivers/net/ethernet/cadence/macb_main.c | 121 ++++++++++++++++++++---
> 2 files changed, 109 insertions(+), 15 deletions(-)
[..]
> @@ -4534,23 +4564,56 @@ static int __maybe_unused macb_suspend(struct device *dev)
> struct macb_queue *queue = bp->queues;
> unsigned long flags;
> unsigned int q;
> + int err;
>
> if (!netif_running(netdev))
> return 0;
>
> if (bp->wol & MACB_WOL_ENABLED) {
> - macb_writel(bp, IER, MACB_BIT(WOL));
> - macb_writel(bp, WOL, MACB_BIT(MAG));
> - enable_irq_wake(bp->queues[0].irq);
> - netif_device_detach(netdev);
> - } else {
> - netif_device_detach(netdev);
> + spin_lock_irqsave(&bp->lock, flags);
> + /* Flush all status bits */
> + macb_writel(bp, TSR, -1);
> + macb_writel(bp, RSR, -1);
> for (q = 0, queue = bp->queues; q < bp->num_queues;
> - ++q, ++queue)
> - napi_disable(&queue->napi);
> - rtnl_lock();
> - phylink_stop(bp->phylink);
> - rtnl_unlock();
> + ++q, ++queue) {
> + /* Disable all interrupts */
> + queue_writel(queue, IDR, -1);
> + queue_readl(queue, ISR);
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, -1);
> + }
> + /* Change interrupt handler and
> + * Enable WoL IRQ on queue 0
> + */
> + if (macb_is_gem(bp)) {
> + devm_free_irq(dev, bp->queues[0].irq, bp->queues);
> + err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
> + IRQF_SHARED, netdev->name, bp->queues);
> + if (err) {
> + dev_err(dev,
> + "Unable to request IRQ %d (error %d)\n",
> + bp->queues[0].irq, err);
> + return err;
> + }
> + queue_writel(bp->queues, IER, GEM_BIT(WOL));
> + gem_writel(bp, WOL, MACB_BIT(MAG));
> + } else {
> + queue_writel(bp->queues, IER, MACB_BIT(WOL));
> + macb_writel(bp, WOL, MACB_BIT(MAG));
> + }
> + spin_unlock_irqrestore(&bp->lock, flags);
> +
> + enable_irq_wake(bp->queues[0].irq);
> + }
> +
> + netif_device_detach(netdev);
> + for (q = 0, queue = bp->queues; q < bp->num_queues;
> + ++q, ++queue)
> + napi_disable(&queue->napi);
> +
> + if (!(bp->wol & MACB_WOL_ENABLED)) {
> + phy_stop(netdev->phydev);
> + phy_suspend(netdev->phydev);
Bug here: you must read:
rtnl_lock();
phylink_stop(bp->phylink);
rtnl_unlock();
Instead of the 2 previous lines. I'll correct in v2.
Sorry for the regression.
> spin_lock_irqsave(&bp->lock, flags);
> macb_reset_hw(bp);
> spin_unlock_irqrestore(&bp->lock, flags);
> @@ -4575,20 +4638,48 @@ static int __maybe_unused macb_resume(struct device *dev)
[..]
BTW: I have issue having a real resume event from the phy with this
series. I'm investigating that but didn't find anything for now.
Observation #1: when the WoL is not enabled, I don't have link issue.
But the path in suspend/resume is far more intrusive in phy state.
Observation #2: when WoL is enabled, I need to do a full ifdown/ifup
sequence for gain access again to the link:
ip link show eth0
2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
state DOWN mode DEFAULT group default qlen 1000
link/ether 54:10:ec:be:50:b0 brd ff:ff:ff:ff:ff:ff
ifdown eth0 && ifup eth0
ip link show eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 1000
link/ether 54:10:ec:be:50:b0 brd ff:ff:ff:ff:ff:ff
Observation #3: I didn't experience this behavior while playing with the
WoL on my 4.19 kernel before porting to Mainline.
Best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-17 17:14 ` Nicolas Ferre
@ 2020-04-21 8:21 ` Nicolas.Ferre
2020-04-21 8:37 ` Harini Katakam
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas.Ferre @ 2020-04-21 8:21 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu.Beznea, harini.katakam,
f.fainelli, linux
Cc: andrew, alexandre.belloni, sergio.prado, pthombar,
antoine.tenart, linux-kernel, michal.simek, davem
On 17/04/2020 at 19:14, Nicolas Ferre wrote:
> On 16/04/2020 at 19:44, nicolas.ferre@microchip.com wrote:
>> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>>
>> Adapt the Wake-on-Lan feature to the Cadence GEM Ethernet controller.
>> This controller has different register layout and cannot be handled by
>> previous code.
>> We disable completely interrupts on all the queues but the queue 0.
>> Handling of WoL interrupt is done in another interrupt handler
>> positioned depending on the controller version used, just between
>> suspend() and resume() calls.
>> It allows to lower pressure on the generic interrupt hot path by
>> removing the need to handle 2 tests for each IRQ: the first figuring out
>> the controller revision, the second for actually knowing if the WoL bit
>> is set.
>>
>> Queue management in suspend()/resume() functions inspired from RFC patch
>> by Harini Katakam <harinik@xilinx.com>, thanks!
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>> ---
>> drivers/net/ethernet/cadence/macb.h | 3 +
>> drivers/net/ethernet/cadence/macb_main.c | 121 ++++++++++++++++++++---
>> 2 files changed, 109 insertions(+), 15 deletions(-)
>
> [..]
>
>> @@ -4534,23 +4564,56 @@ static int __maybe_unused macb_suspend(struct device *dev)
>> struct macb_queue *queue = bp->queues;
>> unsigned long flags;
>> unsigned int q;
>> + int err;
>>
>> if (!netif_running(netdev))
>> return 0;
>>
>> if (bp->wol & MACB_WOL_ENABLED) {
>> - macb_writel(bp, IER, MACB_BIT(WOL));
>> - macb_writel(bp, WOL, MACB_BIT(MAG));
>> - enable_irq_wake(bp->queues[0].irq);
>> - netif_device_detach(netdev);
>> - } else {
>> - netif_device_detach(netdev);
>> + spin_lock_irqsave(&bp->lock, flags);
>> + /* Flush all status bits */
>> + macb_writel(bp, TSR, -1);
>> + macb_writel(bp, RSR, -1);
>> for (q = 0, queue = bp->queues; q < bp->num_queues;
>> - ++q, ++queue)
>> - napi_disable(&queue->napi);
>> - rtnl_lock();
>> - phylink_stop(bp->phylink);
>> - rtnl_unlock();
>> + ++q, ++queue) {
>> + /* Disable all interrupts */
>> + queue_writel(queue, IDR, -1);
>> + queue_readl(queue, ISR);
>> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> + queue_writel(queue, ISR, -1);
>> + }
>> + /* Change interrupt handler and
>> + * Enable WoL IRQ on queue 0
>> + */
>> + if (macb_is_gem(bp)) {
>> + devm_free_irq(dev, bp->queues[0].irq, bp->queues);
>> + err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
>> + IRQF_SHARED, netdev->name, bp->queues);
>> + if (err) {
>> + dev_err(dev,
>> + "Unable to request IRQ %d (error %d)\n",
>> + bp->queues[0].irq, err);
>> + return err;
>> + }
>> + queue_writel(bp->queues, IER, GEM_BIT(WOL));
>> + gem_writel(bp, WOL, MACB_BIT(MAG));
>> + } else {
>> + queue_writel(bp->queues, IER, MACB_BIT(WOL));
>> + macb_writel(bp, WOL, MACB_BIT(MAG));
>> + }
>> + spin_unlock_irqrestore(&bp->lock, flags);
>> +
>> + enable_irq_wake(bp->queues[0].irq);
>> + }
>> +
>> + netif_device_detach(netdev);
>> + for (q = 0, queue = bp->queues; q < bp->num_queues;
>> + ++q, ++queue)
>> + napi_disable(&queue->napi);
>> +
>> + if (!(bp->wol & MACB_WOL_ENABLED)) {
>> + phy_stop(netdev->phydev);
>> + phy_suspend(netdev->phydev);
>
> Bug here: you must read:
>
> rtnl_lock();
> phylink_stop(bp->phylink);
> rtnl_unlock();
>
> Instead of the 2 previous lines. I'll correct in v2.
>
> Sorry for the regression.
>
>
>> spin_lock_irqsave(&bp->lock, flags);
>> macb_reset_hw(bp);
>> spin_unlock_irqrestore(&bp->lock, flags);
>> @@ -4575,20 +4638,48 @@ static int __maybe_unused macb_resume(struct device *dev)
>
> [..]
> BTW: I have issue having a real resume event from the phy with this
> series. I'm investigating that but didn't find anything for now.
>
> Observation #1: when the WoL is not enabled, I don't have link issue.
> But the path in suspend/resume is far more intrusive in phy state.
>
> Observation #2: when WoL is enabled, I need to do a full ifdown/ifup
> sequence for gain access again to the link:
>
> ip link show eth0
> 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
> state DOWN mode DEFAULT group default qlen 1000
> link/ether 54:10:ec:be:50:b0 brd ff:ff:ff:ff:ff:ff
>
> ifdown eth0 && ifup eth0
>
> ip link show eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
> state UP mode DEFAULT group default qlen 1000
> link/ether 54:10:ec:be:50:b0 brd ff:ff:ff:ff:ff:ff
>
> Observation #3: I didn't experience this behavior while playing with the
> WoL on my 4.19 kernel before porting to Mainline.
I've reviewed this series to fix this last issues. It's was a
combination of runtime_pm not handled properly and a mix of
netif_carrier_* call with phylink calls not well positioned nor balanced
between suspend and resume.
I have a v2 series that I'm preparing for today: Harini, I prefer to
post it now so it could avoid that you hit the same issues as me while
testing on your platform.
Best regards,
Nicolas
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller
2020-04-21 8:21 ` Nicolas.Ferre
@ 2020-04-21 8:37 ` Harini Katakam
0 siblings, 0 replies; 17+ messages in thread
From: Harini Katakam @ 2020-04-21 8:37 UTC (permalink / raw)
To: Nicolas Ferre
Cc: linux-arm-kernel, netdev, Claudiu Beznea, Harini Katakam,
Florian Fainelli, linux, Andrew Lunn, Alexandre Belloni,
Sergio Prado, Parshuram Raju Thombare, antoine.tenart,
linux-kernel, Michal Simek, David Miller
On Tue, Apr 21, 2020 at 1:55 PM <Nicolas.Ferre@microchip.com> wrote:
<snip>
> I've reviewed this series to fix this last issues. It's was a
> combination of runtime_pm not handled properly and a mix of
> netif_carrier_* call with phylink calls not well positioned nor balanced
> between suspend and resume.
>
> I have a v2 series that I'm preparing for today: Harini, I prefer to
> post it now so it could avoid that you hit the same issues as me while
> testing on your platform.
OK thanks Nicolas.
Regards,
Harini
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] net: macb: Add WoL interrupt support for MACB type of Ethernet controller
2020-04-16 17:44 [PATCH 0/5] net: macb: Wake-on-Lan magic packet fixes and GEM handling nicolas.ferre
` (3 preceding siblings ...)
2020-04-16 17:44 ` [PATCH 4/5] net: macb: WoL support for GEM type of Ethernet controller nicolas.ferre
@ 2020-04-16 17:44 ` nicolas.ferre
2020-04-16 19:26 ` Florian Fainelli
4 siblings, 1 reply; 17+ messages in thread
From: nicolas.ferre @ 2020-04-16 17:44 UTC (permalink / raw)
To: linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, f.fainelli, linux, andrew,
michal.simek, Nicolas Ferre
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Handle the Wake-on-Lan interrupt for the Cadence MACB Ethernet
controller.
As we do for the GEM version, we handle of WoL interrupt in a
specialized interrupt handler for MACB version that is positionned
just between suspend() and resume() calls.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 38 +++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 71e6afbdfb47..6d535e3e803c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1513,6 +1513,34 @@ static void macb_tx_restart(struct macb_queue *queue)
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
}
+static irqreturn_t macb_wol_interrupt(int irq, void *dev_id)
+{
+ struct macb_queue *queue = dev_id;
+ struct macb *bp = queue->bp;
+ u32 status;
+
+ status = queue_readl(queue, ISR);
+
+ if (unlikely(!status))
+ return IRQ_NONE;
+
+ spin_lock(&bp->lock);
+
+ if (status & MACB_BIT(WOL)) {
+ queue_writel(queue, IDR, MACB_BIT(WOL));
+ macb_writel(bp, WOL, 0);
+ netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
+ (unsigned int)(queue - bp->queues),
+ (unsigned long)status);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, MACB_BIT(WOL));
+ }
+
+ spin_unlock(&bp->lock);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
{
struct macb_queue *queue = dev_id;
@@ -4585,8 +4613,8 @@ static int __maybe_unused macb_suspend(struct device *dev)
/* Change interrupt handler and
* Enable WoL IRQ on queue 0
*/
+ devm_free_irq(dev, bp->queues[0].irq, bp->queues);
if (macb_is_gem(bp)) {
- devm_free_irq(dev, bp->queues[0].irq, bp->queues);
err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
IRQF_SHARED, netdev->name, bp->queues);
if (err) {
@@ -4598,6 +4626,14 @@ static int __maybe_unused macb_suspend(struct device *dev)
queue_writel(bp->queues, IER, GEM_BIT(WOL));
gem_writel(bp, WOL, MACB_BIT(MAG));
} else {
+ err = devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt,
+ IRQF_SHARED, netdev->name, bp->queues);
+ if (err) {
+ dev_err(dev,
+ "Unable to request IRQ %d (error %d)\n",
+ bp->queues[0].irq, err);
+ return err;
+ }
queue_writel(bp->queues, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] net: macb: Add WoL interrupt support for MACB type of Ethernet controller
2020-04-16 17:44 ` [PATCH 5/5] net: macb: Add WoL interrupt support for MACB " nicolas.ferre
@ 2020-04-16 19:26 ` Florian Fainelli
0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2020-04-16 19:26 UTC (permalink / raw)
To: nicolas.ferre, linux-arm-kernel, netdev, Claudiu Beznea, harini.katakam
Cc: linux-kernel, David S. Miller, Alexandre Belloni, pthombar,
sergio.prado, antoine.tenart, linux, andrew, michal.simek
On 4/16/2020 10:44 AM, nicolas.ferre@microchip.com wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Handle the Wake-on-Lan interrupt for the Cadence MACB Ethernet
> controller.
> As we do for the GEM version, we handle of WoL interrupt in a
> specialized interrupt handler for MACB version that is positionned
> just between suspend() and resume() calls.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 38 +++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 71e6afbdfb47..6d535e3e803c 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1513,6 +1513,34 @@ static void macb_tx_restart(struct macb_queue *queue)
> macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
> }
>
> +static irqreturn_t macb_wol_interrupt(int irq, void *dev_id)
> +{
> + struct macb_queue *queue = dev_id;
> + struct macb *bp = queue->bp;
> + u32 status;
> +
> + status = queue_readl(queue, ISR);
> +
> + if (unlikely(!status))
> + return IRQ_NONE;
> +
> + spin_lock(&bp->lock);
> +
> + if (status & MACB_BIT(WOL)) {
> + queue_writel(queue, IDR, MACB_BIT(WOL));
> + macb_writel(bp, WOL, 0);
> + netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
> + (unsigned int)(queue - bp->queues),
> + (unsigned long)status);
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(WOL));
> + }
Likewise, this would need a call to pm_wakeup_event() to record the
wake-up event associated with this device.
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread