On Mon, Jun 17, 2024 at 12:34:12PM +0530, Vineeth Karumanchi wrote:
Extend wake-on LAN support with an ARP packet.
...
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
...
@@ -84,8 +85,7 @@ 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)
nit: BIT() could be used here
#define HS_SPEED_10000M 4
#define MACB_SERDES_RATE_10G 1
...
@@ -5290,6 +5289,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)));
Hi Vineeth and Harini,
I guess I must be reading this wrong, beause I am confused
by the intent of the endeness handling above.
* ifa->ifa_local is a 32-bit big-endian value
* It's address is cast to a 32-bit host-endian pointer
nit: I think u32 would be preferable to uint32_t; this is kernel code.
* The value at this address is then converted to a host byte order value.
nit: Why is cpu_to_be32p() used here instead of the more commonly used
cpu_to_be32() ?
More importantly, why is a host byte order value being converted from
big-endian to host byte order?
* The value returned by cpu_to_be32p, which is big-endian, because
that is what that function does, is then cast to host-byte order.
So overall we have:
1. Cast from big endian to host byte order
2. Conversion from host byte order to big endian
(a bytes-swap on litte endian hosts; no-op on big endian hosts)
3. Cast from big endian to host byte oder
All three of these steps seem to warrant explanation.
And the combination is confusing to say the least.