Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Linux>
  3. Kernel
[PATCH net-next v2 2/4] net: macb: Add ARP support to WOL
vineeth.karumanchi at amd
Feb 22, 2024, 7:38 AM
Post #1 of 4 (19 views)
Permalink
-Add wake-on LAN support using ARP with the provision to select
through ethtool. Advertise wakeup capability in the probe and
get the supported modes from OS policy (MACB_CAPS_WOL).

-Re-order MACB_WOL_<> macros for ease of extension.
-Add ARP support configurable through ethtool, "wolopts" variable in
struct macb contains the current WOL options configured through ethtool.

-For WOL via ARP, ensure the IP address is assigned and
report an error otherwise.

Co-developed-by: Harini Katakam <harini.katakam@amd.com>
Signed-off-by: Harini Katakam <harini.katakam@amd.com>
Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
---
drivers/net/ethernet/cadence/macb.h | 2 +
drivers/net/ethernet/cadence/macb_main.c | 52 +++++++++++++++++-------
2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 50cd35ef21ad..c9ca61959f3c 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -738,6 +738,7 @@
#define MACB_CAPS_MIIONRGMII 0x00000200
#define MACB_CAPS_NEED_TSUCLK 0x00000400
#define MACB_CAPS_QUEUE_DISABLE 0x00000800
+#define MACB_CAPS_WOL 0x00001000
#define MACB_CAPS_PCS 0x01000000
#define MACB_CAPS_HIGH_SPEED 0x02000000
#define MACB_CAPS_CLK_HW_CHG 0x04000000
@@ -1306,6 +1307,7 @@ struct macb {
unsigned int jumbo_max_len;

u32 wol;
+ u32 wolopts;

/* holds value of rx watermark value for pbuf_rxcutthru register */
u32 rx_watermark;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index f34933ef03b0..62d796ef4035 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -38,6 +38,7 @@
#include <linux/ptp_classify.h>
#include <linux/reset.h>
#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/inetdevice.h>
#include "macb.h"

/* This structure is only used for MACB on SiFive FU540 devices */
@@ -84,8 +85,9 @@ struct sifive_fu540_macb_mgmt {
#define GEM_MTU_MIN_SIZE ETH_MIN_MTU
#define MACB_NETIF_LSO NETIF_F_TSO

-#define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0)
-#define MACB_WOL_ENABLED (0x1 << 1)
+#define MACB_WOL_ENABLED (0x1 << 0)
+#define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 1)
+#define MACB_WOL_HAS_ARP_PACKET (0x1 << 2)

#define HS_SPEED_10000M 4
#define MACB_SERDES_RATE_10G 1
@@ -3278,18 +3280,18 @@ static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct macb *bp = netdev_priv(netdev);

- if (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) {
+ if (bp->wol & (MACB_WOL_HAS_MAGIC_PACKET | MACB_WOL_HAS_ARP_PACKET))
phylink_ethtool_get_wol(bp->phylink, wol);
- wol->supported |= WAKE_MAGIC;
-
- if (bp->wol & MACB_WOL_ENABLED)
- wol->wolopts |= WAKE_MAGIC;
- }
+ wol->supported |= (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ? WAKE_MAGIC : 0;
+ wol->supported |= (bp->wol & MACB_WOL_HAS_ARP_PACKET) ? WAKE_ARP : 0;
+ /* Pass wolopts to ethtool */
+ wol->wolopts = bp->wolopts;
}

static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct macb *bp = netdev_priv(netdev);
+ bp->wolopts = 0;
int ret;

/* Pass the order to phylink layer */
@@ -3300,11 +3302,14 @@ static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
if (!ret || ret != -EOPNOTSUPP)
return ret;

- if (!(bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ||
- (wol->wolopts & ~WAKE_MAGIC))
+ if (!(bp->wol & (MACB_WOL_HAS_MAGIC_PACKET | MACB_WOL_HAS_ARP_PACKET)) ||
+ (wol->wolopts & ~(WAKE_MAGIC | WAKE_ARP)))
return -EOPNOTSUPP;

- if (wol->wolopts & WAKE_MAGIC)
+ bp->wolopts |= (wol->wolopts & WAKE_MAGIC) ? WAKE_MAGIC : 0;
+ bp->wolopts |= (wol->wolopts & WAKE_ARP) ? WAKE_ARP : 0;
+
+ if (bp->wolopts)
bp->wol |= MACB_WOL_ENABLED;
else
bp->wol &= ~MACB_WOL_ENABLED;
@@ -5087,7 +5092,6 @@ static int macb_probe(struct platform_device *pdev)
bp->wol = 0;
if (of_property_read_bool(np, "magic-packet"))
bp->wol |= MACB_WOL_HAS_MAGIC_PACKET;
- device_set_wakeup_capable(&pdev->dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET);

bp->usrio = macb_config->usrio;

@@ -5115,6 +5119,11 @@ static int macb_probe(struct platform_device *pdev)
/* setup capabilities */
macb_configure_caps(bp, macb_config);

+ if (bp->caps & MACB_CAPS_WOL)
+ bp->wol |= (MACB_WOL_HAS_ARP_PACKET | MACB_WOL_HAS_MAGIC_PACKET);
+
+ device_set_wakeup_capable(&pdev->dev, (bp->wol) ? true : false);
+
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
if (GEM_BFEXT(DAW64, gem_readl(bp, DCFG6))) {
dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(44));
@@ -5244,6 +5253,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
struct macb_queue *queue;
+ struct in_ifaddr *ifa;
unsigned long flags;
unsigned int q;
int err;
@@ -5256,6 +5266,12 @@ static int __maybe_unused macb_suspend(struct device *dev)
return 0;

if (bp->wol & MACB_WOL_ENABLED) {
+ /* Check for IP address in WOL ARP mode */
+ ifa = rcu_dereference(__in_dev_get_rcu(bp->dev)->ifa_list);
+ if ((bp->wolopts & WAKE_ARP) && !ifa) {
+ netdev_err(netdev, "IP address not assigned\n");
+ return -EOPNOTSUPP;
+ }
spin_lock_irqsave(&bp->lock, flags);

/* Disable Tx and Rx engines before disabling the queues,
@@ -5289,6 +5305,14 @@ static int __maybe_unused macb_suspend(struct device *dev)
macb_writel(bp, TSR, -1);
macb_writel(bp, RSR, -1);

+ tmp = (bp->wolopts & WAKE_MAGIC) ? MACB_BIT(MAG) : 0;
+ if (bp->wolopts & WAKE_ARP) {
+ tmp |= MACB_BIT(ARP);
+ /* write IP address into register */
+ tmp |= MACB_BFEXT(IP,
+ (__force u32)(cpu_to_be32p((uint32_t *)&ifa->ifa_local)));
+ }
+
/* Change interrupt handler and
* Enable WoL IRQ on queue 0
*/
@@ -5304,7 +5328,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
return err;
}
queue_writel(bp->queues, IER, GEM_BIT(WOL));
- gem_writel(bp, WOL, MACB_BIT(MAG));
+ gem_writel(bp, WOL, tmp);
} else {
err = devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt,
IRQF_SHARED, netdev->name, bp->queues);
@@ -5316,7 +5340,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
return err;
}
queue_writel(bp->queues, IER, MACB_BIT(WOL));
- macb_writel(bp, WOL, MACB_BIT(MAG));
+ macb_writel(bp, WOL, tmp);
}
spin_unlock_irqrestore(&bp->lock, flags);

--
2.34.1
Re: [PATCH net-next v2 2/4] net: macb: Add ARP support to WOL [ In reply to ]
andrew at lunn
Feb 22, 2024, 11:32 AM
Post #2 of 4 (19 views)
Permalink
> u32 wol;
> + u32 wolopts;

> + wol->supported |= (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ? WAKE_MAGIC : 0;
> + wol->supported |= (bp->wol & MACB_WOL_HAS_ARP_PACKET) ? WAKE_ARP : 0;

> + if (bp->caps & MACB_CAPS_WOL)
> + bp->wol |= (MACB_WOL_HAS_ARP_PACKET | MACB_WOL_HAS_MAGIC_PACKET);

So bp->wol is the capabilities of the hardware?

And bp->wolopts is what has been enabled via ethtool?

I just wounder if it would be easier to understand is bp->wol was
renamed wol_caps, similar to bp->caps?

Andrew
Re: [PATCH net-next v2 2/4] net: macb: Add ARP support to WOL [ In reply to ]
vineeth.karumanchi at amd
Feb 22, 2024, 8:46 PM
Post #3 of 4 (19 views)
Permalink
Hi Andrew,

On 23/02/24 1:02 am, Andrew Lunn wrote:
>> u32 wol;
>> + u32 wolopts;
>
>> + wol->supported |= (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ? WAKE_MAGIC : 0;
>> + wol->supported |= (bp->wol & MACB_WOL_HAS_ARP_PACKET) ? WAKE_ARP : 0;
>
>> + if (bp->caps & MACB_CAPS_WOL)
>> + bp->wol |= (MACB_WOL_HAS_ARP_PACKET | MACB_WOL_HAS_MAGIC_PACKET);
>
> So bp->wol is the capabilities of the hardware?
>

Yes, it holds the supported capabilities.

> And bp->wolopts is what has been enabled via ethtool?
>

Yes, it holds the selected options through ethtool.

> I just wounder if it would be easier to understand is bp->wol was
> renamed wol_caps, similar to bp->caps?
>
> Andrew

Sure, I will make the change.

???? vineeth
Re: [PATCH net-next v2 2/4] net: macb: Add ARP support to WOL [ In reply to ]
claudiu.beznea at tuxon
Mar 28, 2024, 2:05 AM
Post #4 of 4 (19 views)
Permalink
On 22.02.2024 17:38, Vineeth Karumanchi wrote:
> -Add wake-on LAN support using ARP with the provision to select
> through ethtool. Advertise wakeup capability in the probe and
> get the supported modes from OS policy (MACB_CAPS_WOL).
>
> -Re-order MACB_WOL_<> macros for ease of extension.
> -Add ARP support configurable through ethtool, "wolopts" variable in
> struct macb contains the current WOL options configured through ethtool.
>
> -For WOL via ARP, ensure the IP address is assigned and
> report an error otherwise.

Having '-' for each thing that you did makes the 1st time reader of this
commit message think that you did multiple things in this patch, which
should be avoided.

Also, please compose the commit message such that it responds to the
questions "what the patch does?" and "why it's necessary?"

>
> Co-developed-by: Harini Katakam <harini.katakam@amd.com>
> Signed-off-by: Harini Katakam <harini.katakam@amd.com>
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 2 +
> drivers/net/ethernet/cadence/macb_main.c | 52 +++++++++++++++++-------
> 2 files changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 50cd35ef21ad..c9ca61959f3c 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -738,6 +738,7 @@
> #define MACB_CAPS_MIIONRGMII 0x00000200
> #define MACB_CAPS_NEED_TSUCLK 0x00000400
> #define MACB_CAPS_QUEUE_DISABLE 0x00000800
> +#define MACB_CAPS_WOL 0x00001000
> #define MACB_CAPS_PCS 0x01000000
> #define MACB_CAPS_HIGH_SPEED 0x02000000
> #define MACB_CAPS_CLK_HW_CHG 0x04000000
> @@ -1306,6 +1307,7 @@ struct macb {
> unsigned int jumbo_max_len;
>
> u32 wol;
> + u32 wolopts;
>
> /* holds value of rx watermark value for pbuf_rxcutthru register */
> u32 rx_watermark;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index f34933ef03b0..62d796ef4035 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -38,6 +38,7 @@
> #include <linux/ptp_classify.h>
> #include <linux/reset.h>
> #include <linux/firmware/xlnx-zynqmp.h>
> +#include <linux/inetdevice.h>
> #include "macb.h"
>
> /* This structure is only used for MACB on SiFive FU540 devices */
> @@ -84,8 +85,9 @@ struct sifive_fu540_macb_mgmt {
> #define GEM_MTU_MIN_SIZE ETH_MIN_MTU
> #define MACB_NETIF_LSO NETIF_F_TSO
>
> -#define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0)
> -#define MACB_WOL_ENABLED (0x1 << 1)

> +#define MACB_WOL_ENABLED (0x1 << 0)> +#define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 1)

Is there a reason you changed the values of these 2 macros?

> +#define MACB_WOL_HAS_ARP_PACKET (0x1 << 2)
>
> #define HS_SPEED_10000M 4
> #define MACB_SERDES_RATE_10G 1
> @@ -3278,18 +3280,18 @@ static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> {
> struct macb *bp = netdev_priv(netdev);
>
> - if (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) {
> + if (bp->wol & (MACB_WOL_HAS_MAGIC_PACKET | MACB_WOL_HAS_ARP_PACKET))
> phylink_ethtool_get_wol(bp->phylink, wol);
> - wol->supported |= WAKE_MAGIC;
> -
> - if (bp->wol & MACB_WOL_ENABLED)
> - wol->wolopts |= WAKE_MAGIC;
> - }
> + wol->supported |= (bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ? WAKE_MAGIC : 0;
> + wol->supported |= (bp->wol & MACB_WOL_HAS_ARP_PACKET) ? WAKE_ARP : 0;
> + /* Pass wolopts to ethtool */
> + wol->wolopts = bp->wolopts;
> }
>
> static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> {
> struct macb *bp = netdev_priv(netdev);
> + bp->wolopts = 0;
> int ret;
>
> /* Pass the order to phylink layer */
> @@ -3300,11 +3302,14 @@ static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> if (!ret || ret != -EOPNOTSUPP)
> return ret;
>
> - if (!(bp->wol & MACB_WOL_HAS_MAGIC_PACKET) ||
> - (wol->wolopts & ~WAKE_MAGIC))
> + if (!(bp->wol & (MACB_WOL_HAS_MAGIC_PACKET | MACB_WOL_HAS_ARP_PACKET)) ||
> + (wol->wolopts & ~(WAKE_MAGIC | WAKE_ARP)))
> return -EOPNOTSUPP;
>
> - if (wol->wolopts & WAKE_MAGIC)
> + bp->wolopts |= (wol->wolopts & WAKE_MAGIC) ? WAKE_MAGIC : 0;
> + bp->wolopts |= (wol->wolopts & WAKE_ARP) ? WAKE_ARP : 0;
> +
> + if (bp->wolopts)
> bp->wol |= MACB_WOL_ENABLED;
> else
> bp->wol &= ~MACB_WOL_ENABLED;
> @@ -5087,7 +5092,6 @@ static int macb_probe(struct platform_device *pdev)
> bp->wol = 0;
> if (of_property_read_bool(np, "magic-packet"))
> bp->wol |= MACB_WOL_HAS_MAGIC_PACKET;
> - device_set_wakeup_capable(&pdev->dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET);
>
> bp->usrio = macb_config->usrio;
>
> @@ -5115,6 +5119,11 @@ static int macb_probe(struct platform_device *pdev)
> /* setup capabilities */
> macb_configure_caps(bp, macb_config);
>
> + if (bp->caps & MACB_CAPS_WOL)
> + bp->wol |= (MACB_WOL_HAS_ARP_PACKET | MACB_WOL_HAS_MAGIC_PACKET);
> +
> + device_set_wakeup_capable(&pdev->dev, (bp->wol) ? true : false);

It can be simplified with:

device_set_wakeup_capable(&pdev->dev, !!bp->wol);

> +
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> if (GEM_BFEXT(DAW64, gem_readl(bp, DCFG6))) {
> dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(44));
> @@ -5244,6 +5253,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
> struct net_device *netdev = dev_get_drvdata(dev);
> struct macb *bp = netdev_priv(netdev);
> struct macb_queue *queue;
> + struct in_ifaddr *ifa;
> unsigned long flags;
> unsigned int q;
> int err;
> @@ -5256,6 +5266,12 @@ static int __maybe_unused macb_suspend(struct device *dev)
> return 0;
>
> if (bp->wol & MACB_WOL_ENABLED) {
> + /* Check for IP address in WOL ARP mode */
> + ifa = rcu_dereference(__in_dev_get_rcu(bp->dev)->ifa_list);
> + if ((bp->wolopts & WAKE_ARP) && !ifa) {
> + netdev_err(netdev, "IP address not assigned\n");
> + return -EOPNOTSUPP;
> + }
> spin_lock_irqsave(&bp->lock, flags);
>
> /* Disable Tx and Rx engines before disabling the queues,
> @@ -5289,6 +5305,14 @@ static int __maybe_unused macb_suspend(struct device *dev)
> macb_writel(bp, TSR, -1);
> macb_writel(bp, RSR, -1);
>
> + tmp = (bp->wolopts & WAKE_MAGIC) ? MACB_BIT(MAG) : 0;
> + if (bp->wolopts & WAKE_ARP) {
> + tmp |= MACB_BIT(ARP);
> + /* write IP address into register */
> + tmp |= MACB_BFEXT(IP,
> + (__force u32)(cpu_to_be32p((uint32_t *)&ifa->ifa_local)));
> + }
> +
> /* Change interrupt handler and
> * Enable WoL IRQ on queue 0
> */
> @@ -5304,7 +5328,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
> return err;
> }
> queue_writel(bp->queues, IER, GEM_BIT(WOL));
> - gem_writel(bp, WOL, MACB_BIT(MAG));
> + gem_writel(bp, WOL, tmp);
> } else {
> err = devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt,
> IRQF_SHARED, netdev->name, bp->queues);
> @@ -5316,7 +5340,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
> return err;
> }
> queue_writel(bp->queues, IER, MACB_BIT(WOL));
> - macb_writel(bp, WOL, MACB_BIT(MAG));
> + macb_writel(bp, WOL, tmp);
> }
> spin_unlock_irqrestore(&bp->lock, flags);
>

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal