Hi Simon,
On 6/18/2024 4:26 PM, Simon Horman wrote:
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
OK.
#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.
tmp |= MACB_BFEXT(IP, be32_to_cpu(ifa->ifa_local));
The above snippet will address above points.
Consider the ip address is : 11.11.70.78
1. ifa->ifa_local : returns be32 -> 0x4E460b0b
2. be32_to_cpu(ifa->ifa_local) : converts be32 to host byte order u32:
0x0b0b464e
There are no sparse errors as well.
I will make the change, please let me know your suggestions/thoughts.
Thanks
🙏 vineeth
...
