This patch series introduces extensive Wake on LAN (WoL) support for the
Microchip KSZ9477 family of switches, coupled with some code refactoring
and error handling enhancements. The principal aim is to enable and
manage Wake on Magic Packet and other PHY event triggers for waking up
the system, whilst ensuring that the switch isn't reset during a
shutdown if WoL is active.
The Wake on LAN functionality is optional and is particularly beneficial
if the PME pins are connected to the SoC as a wake source or to a PMIC
that can enable or wake the SoC.
changes v5:
- rework Wake on Magic Packet support.
- Make sure we show more or less realistic information on get_wol by
comparing refcounted mac address against the ports address
- fix mac address refcounting on set_wol()
- rework shutdown sequence by to handle PMIC related issues. Make sure
PME pin is net frequently toggled.
- use wakeup_source variable instead of reading PME pin register.
changes v4:
- add ksz_switch_shutdown() and do not skip dsa_switch_shutdown() and
etc.
- try to configure MAC address on WAKE_MAGIC. If not possible, prevent
WAKE_MAGIC configuration
- use ksz_switch_macaddr_get() for WAKE_MAGIC.
- prevent ksz_port_set_mac_address if WAKE_MAGIC is active
- do some more refactoring and patch reordering
changes v3:
- use ethernet address of DSA master instead from devicetree
- use dev_ops->wol* instead of list of supported switch
- don't shutdown the switch if WoL is enabled
- rework on top of latest HSR changes
changes v2:
- rebase against latest next
Oleksij Rempel (9):
net: dsa: microchip: Add missing MAC address register offset for
ksz8863
dt-bindings: net: dsa: microchip: add wakeup-source property
net: dsa: microchip: use wakeup-source DT property to enable PME
output
net: dsa: microchip: ksz9477: add Wake on LAN support
net: dsa: microchip: ksz9477: Add Wake on Magic Packet support
net: dsa: microchip: Refactor comment for ksz_switch_macaddr_get()
function
net: dsa: microchip: Add error handling for ksz_switch_macaddr_get()
net: dsa: microchip: Refactor switch shutdown routine for WoL
preparation
net: dsa: microchip: Ensure Stable PME Pin State for Wake-on-LAN
.../bindings/net/dsa/microchip,ksz.yaml | 2 +
drivers/net/dsa/microchip/ksz9477.c | 194 ++++++++++++++++++
drivers/net/dsa/microchip/ksz9477.h | 5 +
drivers/net/dsa/microchip/ksz9477_i2c.c | 5 +-
drivers/net/dsa/microchip/ksz_common.c | 96 +++++++--
drivers/net/dsa/microchip/ksz_common.h | 10 +
drivers/net/dsa/microchip/ksz_spi.c | 5 +-
7 files changed, 297 insertions(+), 20 deletions(-)
--
2.39.2
Enhance the ksz_switch_macaddr_get() function to handle errors that may
occur during the call to ksz_write8(). Specifically, this update checks
the return value of ksz_write8(), which may fail if regmap ranges
validation is not passed and returns the error code.
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 7b05de6fe987..79052a54880c 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3612,7 +3612,7 @@ int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
struct ksz_switch_macaddr *switch_macaddr;
struct ksz_device *dev = ds->priv;
const u16 *regs = dev->info->regs;
- int i;
+ int i, ret;
/* Make sure concurrent MAC address changes are blocked */
ASSERT_RTNL();
@@ -3639,8 +3639,11 @@ int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
dev->switch_macaddr = switch_macaddr;
/* Program the switch MAC address to hardware */
- for (i = 0; i < ETH_ALEN; i++)
- ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, addr[i]);
+ for (i = 0; i < ETH_ALEN; i++) {
+ ret = ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, addr[i]);
+ if (ret)
+ return ret;
+ }
return 0;
}
--
2.39.2
KSZ switches with WoL support signals wake event over PME pin. If this
pin is attached to some external PMIC or System Controller can't be
described as GPIO, the only way to describe it in the devicetree is to
use wakeup-source property. So, add support for this property and enable
PME switch output if this property is present.
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 3 +++
drivers/net/dsa/microchip/ksz_common.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 02fab1adb27f..11adae8a2037 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -4159,6 +4159,9 @@ int ksz_switch_register(struct ksz_device *dev)
dev_err(dev->dev, "inconsistent synclko settings\n");
return -EINVAL;
}
+
+ dev->wakeup_source = of_property_read_bool(dev->dev->of_node,
+ "wakeup-source");
}
ret = dsa_register_switch(dev->ds);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 8842efca0871..f7c471bc040f 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -163,6 +163,7 @@ struct ksz_device {
phy_interface_t compat_interface;
bool synclko_125;
bool synclko_disable;
+ bool wakeup_source;
struct vlan_table *vlan_cache;
--
2.39.2
Update the comment to follow kernel-doc format.
Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 377998966b13..7b05de6fe987 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3589,12 +3589,20 @@ static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
return 0;
}
-/* Program the switch's MAC address register with the MAC address of the
- * requesting user port. This single address is used by the switch for multiple
- * features, like HSR self-address filtering and WoL. Other user ports are
- * allowed to share ownership of this address as long as their MAC address is
- * the same. The user ports' MAC addresses must not change while they have
- * ownership of the switch MAC address.
+/**
+ * ksz_switch_macaddr_get - Program the switch's MAC address register.
+ * @ds: DSA switch instance.
+ * @port: Port number.
+ * @extack: Netlink extended acknowledgment.
+ *
+ * This function programs the switch's MAC address register with the MAC address
+ * of the requesting user port. This single address is used by the switch for
+ * multiple features like HSR self-address filtering and WoL. Other user ports
+ * can share ownership of this address as long as their MAC address is the same.
+ * The MAC addresses of user ports must not change while they have ownership of
+ * the switch MAC address.
+ *
+ * Return: 0 on success, or other error codes on failure.
*/
int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
struct netlink_ext_ack *extack)
--
2.39.2
Add wakeup-source property to enable Wake on Lan functionality in the
switch.
Since PME wake pin is not always attached to the SoC, use wakeup-source
instead of wakeup-gpios
Signed-off-by: Oleksij Rempel <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
index 41014f5c01c4..5751a729af33 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
@@ -72,6 +72,8 @@ properties:
interrupts:
maxItems: 1
+ wakeup-source: true
+
required:
- compatible
- reg
--
2.39.2
Ensures a stable PME (Power Management Event) pin state by disabling PME
on system start and enabling it on shutdown only if WoL (Wake-on-LAN) is
configured. This is needed to avoid issues with some PMICs (Power
Management ICs).
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 46 ++++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz9477.h | 1 +
drivers/net/dsa/microchip/ksz_common.c | 8 ++++-
drivers/net/dsa/microchip/ksz_common.h | 1 +
4 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index acc5d845363f..0e2ace0f8001 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -196,6 +196,46 @@ int ksz9477_set_wol(struct ksz_device *dev, int port,
return 0;
}
+/**
+ * ksz9477_wol_pre_shutdown - Prepares the switch device for shutdown while
+ * considering Wake-on-LAN (WoL) settings.
+ * @dev: The switch device structure.
+ * @wol_enabled: Pointer to a boolean which will be set to true if WoL is
+ * enabled on any port.
+ *
+ * This function prepares the switch device for a safe shutdown while taking
+ * into account the Wake-on-LAN (WoL) settings on the user ports. It updates
+ * the wol_enabled flag accordingly to reflect whether WoL is active on any
+ * port.
+ */
+void ksz9477_wol_pre_shutdown(struct ksz_device *dev, bool *wol_enabled)
+{
+ struct dsa_port *dp;
+ int ret;
+
+ *wol_enabled = false;
+
+ if (!dev->wakeup_source)
+ return;
+
+ dsa_switch_for_each_user_port(dp, dev->ds) {
+ u8 pme_ctrl = 0;
+
+ ret = ksz_pread8(dev, dp->index, REG_PORT_PME_CTRL, &pme_ctrl);
+ if (!ret && pme_ctrl)
+ *wol_enabled = true;
+
+ /* make sure there are no pending wake events which would
+ * prevent the device from going to sleep/shutdown.
+ */
+ ksz9477_handle_wake_reason(dev, dp->index);
+ }
+
+ /* Now we are save to enable PME pin. */
+ if (*wol_enabled)
+ ksz_write8(dev, REG_SW_PME_CTRL, PME_ENABLE);
+}
+
static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev)
{
unsigned int val;
@@ -1276,6 +1316,12 @@ int ksz9477_setup(struct dsa_switch *ds)
/* enable global MIB counter freeze function */
ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true);
+ /* Make sure PME (WoL) is not enabled. If requested, it will be
+ * enabled by ksz9477_wol_pre_shutdown(). Otherwise, some PMICs do not
+ * like PME events changes before shutdown.
+ */
+ ksz_write8(dev, REG_SW_PME_CTRL, 0);
+
return 0;
}
diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h
index fa8d0318b437..9e6f1e4b57b7 100644
--- a/drivers/net/dsa/microchip/ksz9477.h
+++ b/drivers/net/dsa/microchip/ksz9477.h
@@ -62,6 +62,7 @@ void ksz9477_get_wol(struct ksz_device *dev, int port,
struct ethtool_wolinfo *wol);
int ksz9477_set_wol(struct ksz_device *dev, int port,
struct ethtool_wolinfo *wol);
+void ksz9477_wol_pre_shutdown(struct ksz_device *dev, bool *wol_is_on);
int ksz9477_port_acl_init(struct ksz_device *dev, int port);
void ksz9477_port_acl_free(struct ksz_device *dev, int port);
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index fe2cf1de5a5c..17eca902d48c 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -321,6 +321,7 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.phylink_mac_link_up = ksz9477_phylink_mac_link_up,
.get_wol = ksz9477_get_wol,
.set_wol = ksz9477_set_wol,
+ .wol_pre_shutdown = ksz9477_wol_pre_shutdown,
.config_cpu_port = ksz9477_config_cpu_port,
.tc_cbs_set_cinc = ksz9477_tc_cbs_set_cinc,
.enable_stp_addr = ksz9477_enable_stp_addr,
@@ -3822,7 +3823,12 @@ EXPORT_SYMBOL(ksz_switch_alloc);
*/
void ksz_switch_shutdown(struct ksz_device *dev)
{
- if (dev->dev_ops->reset)
+ bool wol_enabled = false;
+
+ if (dev->dev_ops->wol_pre_shutdown)
+ dev->dev_ops->wol_pre_shutdown(dev, &wol_enabled);
+
+ if (dev->dev_ops->reset && !wol_enabled)
dev->dev_ops->reset(dev);
dsa_switch_shutdown(dev->ds);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 34a8e9784cca..41917de15ba3 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -378,6 +378,7 @@ struct ksz_dev_ops {
struct ethtool_wolinfo *wol);
int (*set_wol)(struct ksz_device *dev, int port,
struct ethtool_wolinfo *wol);
+ void (*wol_pre_shutdown)(struct ksz_device *dev, bool *wol_enabled);
void (*config_cpu_port)(struct dsa_switch *ds);
int (*enable_stp_addr)(struct ksz_device *dev);
int (*reset)(struct ksz_device *dev);
--
2.39.2
Introduce Wake on Magic Packet (WoL) functionality to the ksz9477
driver.
Major changes include:
1. Extending the `ksz9477_handle_wake_reason` function to identify Magic
Packet wake events alongside existing wake reasons.
2. Updating the `ksz9477_get_wol` and `ksz9477_set_wol` functions to
handle WAKE_MAGIC alongside the existing WAKE_PHY option, and to
program the switch's MAC address register accordingly when Magic
Packet wake-up is enabled. This change will prevent WAKE_MAGIC
activation if the related port has a different MAC address compared
to a MAC address already used by HSR or an already active WAKE_MAGIC
on another port.
3. Adding a restriction in `ksz_port_set_mac_address` to prevent MAC
address changes on ports with active Wake on Magic Packet, as the
switch's MAC address register is utilized for this feature.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 56 ++++++++++++++++++++++++--
drivers/net/dsa/microchip/ksz_common.c | 15 +++++--
drivers/net/dsa/microchip/ksz_common.h | 3 ++
3 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index b9419d4b5e7b..acc5d845363f 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -81,7 +81,8 @@ static int ksz9477_handle_wake_reason(struct ksz_device *dev, int port)
if (!pme_status)
return 0;
- dev_dbg(dev->dev, "Wake event on port %d due to: %s %s\n", port,
+ dev_dbg(dev->dev, "Wake event on port %d due to: %s %s %s\n", port,
+ pme_status & PME_WOL_MAGICPKT ? "\"Magic Packet\"" : "",
pme_status & PME_WOL_LINKUP ? "\"Link Up\"" : "",
pme_status & PME_WOL_ENERGY ? "\"Enery detect\"" : "");
@@ -109,10 +110,22 @@ void ksz9477_get_wol(struct ksz_device *dev, int port,
wol->supported = WAKE_PHY;
+ /* Check if at this moment we would be able to get the MAC address
+ * and use it for WAKE_MAGIC support. This result may change dynamically
+ * depending on configuration of other ports.
+ */
+ ret = ksz_switch_macaddr_get(dev->ds, port, NULL);
+ if (!ret) {
+ wol->supported |= WAKE_MAGIC;
+ ksz_switch_macaddr_put(dev->ds);
+ }
+
ret = ksz_pread8(dev, port, REG_PORT_PME_CTRL, &pme_ctrl);
if (ret)
return;
+ if (pme_ctrl & PME_WOL_MAGICPKT)
+ wol->wolopts |= WAKE_MAGIC;
if (pme_ctrl & (PME_WOL_LINKUP | PME_WOL_ENERGY))
wol->wolopts |= WAKE_PHY;
}
@@ -134,10 +147,10 @@ void ksz9477_get_wol(struct ksz_device *dev, int port,
int ksz9477_set_wol(struct ksz_device *dev, int port,
struct ethtool_wolinfo *wol)
{
- u8 pme_ctrl = 0;
+ u8 pme_ctrl = 0, pme_ctrl_old = 0;
int ret;
- if (wol->wolopts & ~WAKE_PHY)
+ if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
return -EINVAL;
if (!dev->wakeup_source)
@@ -147,10 +160,40 @@ int ksz9477_set_wol(struct ksz_device *dev, int port,
if (ret)
return ret;
+ if (wol->wolopts & WAKE_MAGIC)
+ pme_ctrl |= PME_WOL_MAGICPKT;
if (wol->wolopts & WAKE_PHY)
pme_ctrl |= PME_WOL_LINKUP | PME_WOL_ENERGY;
- return ksz_pwrite8(dev, port, REG_PORT_PME_CTRL, pme_ctrl);
+ ret = ksz_pread8(dev, port, REG_PORT_PME_CTRL, &pme_ctrl_old);
+ if (ret)
+ return ret;
+
+ if (pme_ctrl_old == pme_ctrl)
+ return 0;
+
+ /* To keep reference count of MAC address, we should do this
+ * operation only on change of WOL settings.
+ */
+ if (!(pme_ctrl_old & PME_WOL_MAGICPKT) &&
+ (pme_ctrl & PME_WOL_MAGICPKT)) {
+ ret = ksz_switch_macaddr_get(dev->ds, port, NULL);
+ if (ret)
+ return ret;
+ } else if ((pme_ctrl_old & PME_WOL_MAGICPKT) &&
+ !(pme_ctrl & PME_WOL_MAGICPKT)) {
+ ksz_switch_macaddr_put(dev->ds);
+ }
+
+ ret = ksz_pwrite8(dev, port, REG_PORT_PME_CTRL, pme_ctrl);
+ if (ret) {
+ if (!(pme_ctrl_old & PME_WOL_MAGICPKT) &&
+ (pme_ctrl & PME_WOL_MAGICPKT))
+ ksz_switch_macaddr_put(dev->ds);
+ return ret;
+ }
+
+ return 0;
}
static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev)
@@ -1106,6 +1149,11 @@ void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
/* clear pending wake flags */
ksz9477_handle_wake_reason(dev, port);
+
+ /* Disable all WoL options by default. Otherwise
+ * ksz_switch_macaddr_get/put logic will not work properly.
+ */
+ ksz_pwrite8(dev, port, REG_PORT_PME_CTRL, 0);
}
void ksz9477_config_cpu_port(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3f7c86e545a7..377998966b13 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3569,6 +3569,7 @@ static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
const unsigned char *addr)
{
struct dsa_port *dp = dsa_to_port(ds, port);
+ struct ethtool_wolinfo wol;
if (dp->hsr_dev) {
dev_err(ds->dev,
@@ -3577,6 +3578,14 @@ static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
return -EBUSY;
}
+ ksz_get_wol(ds, dp->index, &wol);
+ if (wol.wolopts & WAKE_MAGIC) {
+ dev_err(ds->dev,
+ "Cannot change MAC address on port %d with active Wake on Magic Packet\n",
+ port);
+ return -EBUSY;
+ }
+
return 0;
}
@@ -3587,8 +3596,8 @@ static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
* the same. The user ports' MAC addresses must not change while they have
* ownership of the switch MAC address.
*/
-static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
- struct netlink_ext_ack *extack)
+int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
+ struct netlink_ext_ack *extack)
{
struct net_device *slave = dsa_to_port(ds, port)->slave;
const unsigned char *addr = slave->dev_addr;
@@ -3628,7 +3637,7 @@ static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
return 0;
}
-static void ksz_switch_macaddr_put(struct dsa_switch *ds)
+void ksz_switch_macaddr_put(struct dsa_switch *ds)
{
struct ksz_switch_macaddr *switch_macaddr;
struct ksz_device *dev = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a7394175fcf6..8fc3210d7a3d 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -396,6 +396,9 @@ void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8 state);
bool ksz_get_gbit(struct ksz_device *dev, int port);
phy_interface_t ksz_get_xmii(struct ksz_device *dev, int port, bool gbit);
extern const struct ksz_chip_data ksz_switch_chips[];
+int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
+ struct netlink_ext_ack *extack);
+void ksz_switch_macaddr_put(struct dsa_switch *ds);
/* Common register access functions */
static inline struct regmap *ksz_regmap_8(struct ksz_device *dev)
--
2.39.2
On 10/18/23 04:39, Oleksij Rempel wrote:
> Introduce Wake on Magic Packet (WoL) functionality to the ksz9477
> driver.
>
> Major changes include:
>
> 1. Extending the `ksz9477_handle_wake_reason` function to identify Magic
> Packet wake events alongside existing wake reasons.
>
> 2. Updating the `ksz9477_get_wol` and `ksz9477_set_wol` functions to
> handle WAKE_MAGIC alongside the existing WAKE_PHY option, and to
> program the switch's MAC address register accordingly when Magic
> Packet wake-up is enabled. This change will prevent WAKE_MAGIC
> activation if the related port has a different MAC address compared
> to a MAC address already used by HSR or an already active WAKE_MAGIC
> on another port.
>
> 3. Adding a restriction in `ksz_port_set_mac_address` to prevent MAC
> address changes on ports with active Wake on Magic Packet, as the
> switch's MAC address register is utilized for this feature.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
This looks good to me, just one suggestion below
[snip]
> + if (pme_ctrl_old == pme_ctrl)
> + return 0;
> +
> + /* To keep reference count of MAC address, we should do this
> + * operation only on change of WOL settings.
> + */
> + if (!(pme_ctrl_old & PME_WOL_MAGICPKT) &&
> + (pme_ctrl & PME_WOL_MAGICPKT)) {
Maybe use a temporary variable for that condition since you re-use it
below in case you failed to perform the write of the pme_ctrl value. It
would be more readable IMHO, something like:
bool magicpkt_was_disabled = !(pme_ctrl_old & PME_WOL_MAGICPKT) &&
(pme_ctrl & PME_WOL_MAGICPKT));
--
Florian
On Wed, Oct 18, 2023 at 11:20:50AM -0700, Florian Fainelli wrote:
> On 10/18/23 04:39, Oleksij Rempel wrote:
> > Introduce Wake on Magic Packet (WoL) functionality to the ksz9477
> > driver.
> >
> > Major changes include:
> >
> > 1. Extending the `ksz9477_handle_wake_reason` function to identify Magic
> > Packet wake events alongside existing wake reasons.
> >
> > 2. Updating the `ksz9477_get_wol` and `ksz9477_set_wol` functions to
> > handle WAKE_MAGIC alongside the existing WAKE_PHY option, and to
> > program the switch's MAC address register accordingly when Magic
> > Packet wake-up is enabled. This change will prevent WAKE_MAGIC
> > activation if the related port has a different MAC address compared
> > to a MAC address already used by HSR or an already active WAKE_MAGIC
> > on another port.
> >
> > 3. Adding a restriction in `ksz_port_set_mac_address` to prevent MAC
> > address changes on ports with active Wake on Magic Packet, as the
> > switch's MAC address register is utilized for this feature.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
>
> This looks good to me, just one suggestion below
>
> [snip]
>
> > + if (pme_ctrl_old == pme_ctrl)
> > + return 0;
> > +
> > + /* To keep reference count of MAC address, we should do this
> > + * operation only on change of WOL settings.
> > + */
> > + if (!(pme_ctrl_old & PME_WOL_MAGICPKT) &&
> > + (pme_ctrl & PME_WOL_MAGICPKT)) {
>
> Maybe use a temporary variable for that condition since you re-use it below
> in case you failed to perform the write of the pme_ctrl value. It would be
> more readable IMHO, something like:
>
> bool magicpkt_was_disabled = !(pme_ctrl_old & PME_WOL_MAGICPKT) &&
> (pme_ctrl & PME_WOL_MAGICPKT));
Sounds good. I'll update it.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |