LinuxLists.cc - [PATCH net-next v4 0/9] net: dsa: microchip: provide Wake on LAN support

[permalink] [raw]

Subject: [PATCH net-next v4 9/9] net: dsa: microchip: do not reset the switch on shutdown if WoL is active

For Wake on Lan we should not reconfigure, reset or power down the
switch on shut down sequence.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 29 +++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 85513318d165..abfd9dcab450 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3565,6 +3565,33 @@ static int ksz_set_wol(struct dsa_switch *ds, int port,
return -EOPNOTSUPP;
}

+/**
+ * ksz_wol_is_active - Check if Wake-on-LAN is active on any port.
+ * @dev: The device structure.
+ *
+ * This function iterates through each user port on the switch, checking if
+ * Wake-on-LAN (WoL) is active on any of them.
+ *
+ * Return: true if WoL is active on any port, false otherwise.
+ */
+static bool ksz_wol_is_active(struct ksz_device *dev)
+{
+ struct dsa_port *dp;
+
+ if (!dev->wakeup_source)
+ return false;
+
+ dsa_switch_for_each_user_port(dp, dev->ds) {
+ struct ethtool_wolinfo wol;
+
+ ksz_get_wol(dev->ds, dp->index, &wol);
+ if (wol.wolopts)
+ return true;
+ }
+
+ return false;
+}
+
static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
const unsigned char *addr)
{
@@ -3822,7 +3849,7 @@ EXPORT_SYMBOL(ksz_switch_alloc);
*/
void ksz_switch_shutdown(struct ksz_device *dev)
{
- if (dev->dev_ops->reset)
+ if (dev->dev_ops->reset && !ksz_wol_is_active(dev))
dev->dev_ops->reset(dev);

dsa_switch_shutdown(dev->ds);
--
2.39.2

2023-10-16 14:13:48

[permalink] [raw]

Subject: [PATCH net-next v4 7/9] net: dsa: microchip: Add error handling for ksz_switch_macaddr_get()

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]>
---
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 d03dddbda58c..955434055f06 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

2023-10-16 14:13:56

[permalink] [raw]

Subject: [PATCH net-next v4 5/9] net: dsa: microchip: ksz9477: Add Wake on Magic Packet support

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 | 17 ++++++++++++++---
drivers/net/dsa/microchip/ksz_common.c | 13 +++++++++++--
drivers/net/dsa/microchip/ksz_common.h | 2 ++
3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 8b512b55bfe5..1f8b37b510b0 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\"" : "");

@@ -111,12 +112,14 @@ void ksz9477_get_wol(struct ksz_device *dev, int port,
if (!(pme_conf & PME_ENABLE))
return;

- wol->supported = WAKE_PHY;
+ wol->supported = WAKE_PHY | WAKE_MAGIC;

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;
}
@@ -141,7 +144,7 @@ int ksz9477_set_wol(struct ksz_device *dev, int port,
u8 pme_conf, pme_ctrl = 0;
int ret;

- if (wol->wolopts & ~WAKE_PHY)
+ if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
return -EINVAL;

ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf);
@@ -155,6 +158,14 @@ int ksz9477_set_wol(struct ksz_device *dev, int port,
if (ret)
return ret;

+ if (wol->wolopts & WAKE_MAGIC) {
+ ret = ksz_switch_macaddr_get(dev->ds, port, NULL);
+ if (ret)
+ return ret;
+
+ pme_ctrl |= PME_WOL_MAGICPKT;
+ }
+
if (wol->wolopts & WAKE_PHY)
pme_ctrl |= PME_WOL_LINKUP | PME_WOL_ENERGY;

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3f7c86e545a7..4601aaca5179 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;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a7394175fcf6..2ad49c5f1df4 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -396,6 +396,8 @@ 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);

/* Common register access functions */
static inline struct regmap *ksz_regmap_8(struct ksz_device *dev)
--
2.39.2

2023-10-16 14:13:57

[permalink] [raw]

Subject: [PATCH net-next v4 3/9] net: dsa: microchip: use wakeup-source DT property to enable PME output

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]>
---
drivers/net/dsa/microchip/ksz9477.c | 3 +++
drivers/net/dsa/microchip/ksz_common.c | 3 +++
drivers/net/dsa/microchip/ksz_common.h | 1 +
3 files changed, 7 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index cde8ef33d029..3eacf5abb0f0 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1128,6 +1128,9 @@ 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);

+ if (dev->wakeup_source)
+ ksz_write8(dev, REG_SW_PME_CTRL, PME_ENABLE);
+
return 0;
}

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

2023-10-16 14:14:06

[permalink] [raw]

Subject: [PATCH net-next v4 4/9] net: dsa: microchip: ksz9477: add Wake on LAN support

Add WoL support for KSZ9477 family of switches. This code was tested on
KSZ8563 chip.

KSZ9477 family of switches supports multiple PHY events:
- wake on Link Up
- wake on Energy Detect.
Since current UAPI can't differentiate between this PHY events, map all of them
to WAKE_PHY.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 108 +++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz9477.h | 4 +
drivers/net/dsa/microchip/ksz_common.c | 24 ++++++
drivers/net/dsa/microchip/ksz_common.h | 4 +
4 files changed, 140 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 3eacf5abb0f0..8b512b55bfe5 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -56,6 +56,111 @@ int ksz9477_change_mtu(struct ksz_device *dev, int port, int mtu)
REG_SW_MTU_MASK, frame_size);
}

+/**
+ * ksz9477_handle_wake_reason - Handle wake reason on a specified port.
+ * @dev: The device structure.
+ * @port: The port number.
+ *
+ * This function reads the PME (Power Management Event) status register of a
+ * specified port to determine the wake reason. If there is no wake event, it
+ * returns early. Otherwise, it logs the wake reason which could be due to a
+ * "Magic Packet", "Link Up", or "Energy Detect" event. The PME status register
+ * is then cleared to acknowledge the handling of the wake event.
+ *
+ * Return: 0 on success, or an error code on failure.
+ */
+static int ksz9477_handle_wake_reason(struct ksz_device *dev, int port)
+{
+ u8 pme_status;
+ int ret;
+
+ ret = ksz_pread8(dev, port, REG_PORT_PME_STATUS, &pme_status);
+ if (ret)
+ return ret;
+
+ if (!pme_status)
+ return 0;
+
+ dev_dbg(dev->dev, "Wake event on port %d due to: %s %s\n", port,
+ pme_status & PME_WOL_LINKUP ? "\"Link Up\"" : "",
+ pme_status & PME_WOL_ENERGY ? "\"Enery detect\"" : "");
+
+ return ksz_pwrite8(dev, port, REG_PORT_PME_STATUS, pme_status);
+}
+
+/**
+ * ksz9477_get_wol - Get Wake-on-LAN settings for a specified port.
+ * @dev: The device structure.
+ * @port: The port number.
+ * @wol: Pointer to ethtool Wake-on-LAN settings structure.
+ *
+ * This function checks the PME Pin Control Register to see if PME Pin Output
+ * Enable is set, indicating PME is enabled. If enabled, it sets the supported
+ * and active WoL flags.
+ */
+void ksz9477_get_wol(struct ksz_device *dev, int port,
+ struct ethtool_wolinfo *wol)
+{
+ u8 pme_ctrl, pme_conf;
+ int ret;
+
+ ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf);
+ if (ret)
+ return;
+
+ if (!(pme_conf & PME_ENABLE))
+ return;
+
+ wol->supported = WAKE_PHY;
+
+ ret = ksz_pread8(dev, port, REG_PORT_PME_CTRL, &pme_ctrl);
+ if (ret)
+ return;
+
+ if (pme_ctrl & (PME_WOL_LINKUP | PME_WOL_ENERGY))
+ wol->wolopts |= WAKE_PHY;
+}
+
+/**
+ * ksz9477_set_wol - Set Wake-on-LAN settings for a specified port.
+ * @dev: The device structure.
+ * @port: The port number.
+ * @wol: Pointer to ethtool Wake-on-LAN settings structure.
+ *
+ * This function configures Wake-on-LAN (WoL) settings for a specified port.
+ * It validates the provided WoL options, checks if PME is enabled via the
+ * switch's PME Pin Control Register, clears any previous wake reasons,
+ * and sets the Magic Packet flag in the port's PME control register if
+ * specified.
+ *
+ * Return: 0 on success, or other error codes on failure.
+ */
+int ksz9477_set_wol(struct ksz_device *dev, int port,
+ struct ethtool_wolinfo *wol)
+{
+ u8 pme_conf, pme_ctrl = 0;
+ int ret;
+
+ if (wol->wolopts & ~WAKE_PHY)
+ return -EINVAL;
+
+ ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf);
+ if (ret)
+ return ret;
+
+ if (!(pme_conf & PME_ENABLE))
+ return -EOPNOTSUPP;
+
+ ret = ksz9477_handle_wake_reason(dev, port);
+ if (ret)
+ return ret;
+
+ if (wol->wolopts & WAKE_PHY)
+ pme_ctrl |= PME_WOL_LINKUP | PME_WOL_ENERGY;
+
+ return ksz_pwrite8(dev, port, REG_PORT_PME_CTRL, pme_ctrl);
+}
+
static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev)
{
unsigned int val;
@@ -1006,6 +1111,9 @@ void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16);

ksz9477_port_acl_init(dev, port);
+
+ /* clear pending wake flags */
+ ksz9477_handle_wake_reason(dev, port);
}

void ksz9477_config_cpu_port(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h
index f90e2e8ebe80..fa8d0318b437 100644
--- a/drivers/net/dsa/microchip/ksz9477.h
+++ b/drivers/net/dsa/microchip/ksz9477.h
@@ -58,6 +58,10 @@ void ksz9477_switch_exit(struct ksz_device *dev);
void ksz9477_port_queue_split(struct ksz_device *dev, int port);
void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr);
void ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr);
+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);

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 11adae8a2037..3f7c86e545a7 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -319,6 +319,8 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.mdb_del = ksz9477_mdb_del,
.change_mtu = ksz9477_change_mtu,
.phylink_mac_link_up = ksz9477_phylink_mac_link_up,
+ .get_wol = ksz9477_get_wol,
+ .set_wol = ksz9477_set_wol,
.config_cpu_port = ksz9477_config_cpu_port,
.tc_cbs_set_cinc = ksz9477_tc_cbs_set_cinc,
.enable_stp_addr = ksz9477_enable_stp_addr,
@@ -3543,6 +3545,26 @@ static int ksz_setup_tc(struct dsa_switch *ds, int port,
}
}

+static void ksz_get_wol(struct dsa_switch *ds, int port,
+ struct ethtool_wolinfo *wol)
+{
+ struct ksz_device *dev = ds->priv;
+
+ if (dev->dev_ops->get_wol)
+ dev->dev_ops->get_wol(dev, port, wol);
+}
+
+static int ksz_set_wol(struct dsa_switch *ds, int port,
+ struct ethtool_wolinfo *wol)
+{
+ struct ksz_device *dev = ds->priv;
+
+ if (dev->dev_ops->set_wol)
+ return dev->dev_ops->set_wol(dev, port, wol);
+
+ return -EOPNOTSUPP;
+}
+
static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
const unsigned char *addr)
{
@@ -3727,6 +3749,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.get_pause_stats = ksz_get_pause_stats,
.port_change_mtu = ksz_change_mtu,
.port_max_mtu = ksz_max_mtu,
+ .get_wol = ksz_get_wol,
+ .set_wol = ksz_set_wol,
.get_ts_info = ksz_get_ts_info,
.port_hwtstamp_get = ksz_hwtstamp_get,
.port_hwtstamp_set = ksz_hwtstamp_set,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index f7c471bc040f..a7394175fcf6 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -374,6 +374,10 @@ struct ksz_dev_ops {
int duplex, bool tx_pause, bool rx_pause);
void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
+ void (*get_wol)(struct ksz_device *dev, int port,
+ struct ethtool_wolinfo *wol);
+ int (*set_wol)(struct ksz_device *dev, int port,
+ struct ethtool_wolinfo *wol);
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

2023-10-16 14:14:12

[permalink] [raw]

Subject: [PATCH net-next v4 2/9] dt-bindings: net: dsa: microchip: add wakeup-source property

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]>
---
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

2023-10-16 14:14:14

[permalink] [raw]

Subject: [PATCH net-next v4 6/9] net: dsa: microchip: Refactor comment for ksz_switch_macaddr_get() function

Update the comment to follow kernel-doc format.

Signed-off-by: Oleksij Rempel <[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 4601aaca5179..d03dddbda58c 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

2023-10-16 14:14:24

[permalink] [raw]

Subject: [PATCH net-next v4 8/9] net: dsa: microchip: Refactor switch shutdown routine for WoL preparation

Centralize the switch shutdown routine in a dedicated function,
ksz_switch_shutdown(), to enhance code maintainability and reduce
redundancy. This change abstracts the common shutdown operations
previously duplicated in ksz9477_i2c_shutdown() and ksz_spi_shutdown().

This refactoring is a preparatory step for an upcoming patch to avoid
reset on shutdown if Wake-on-LAN (WoL) is enabled.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477_i2c.c | 5 +----
drivers/net/dsa/microchip/ksz_common.c | 18 ++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.h | 1 +
drivers/net/dsa/microchip/ksz_spi.c | 5 +----
4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
index 2710afad4f3a..cac4a607e54a 100644
--- a/drivers/net/dsa/microchip/ksz9477_i2c.c
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -66,10 +66,7 @@ static void ksz9477_i2c_shutdown(struct i2c_client *i2c)
if (!dev)
return;

- if (dev->dev_ops->reset)
- dev->dev_ops->reset(dev);
-
- dsa_switch_shutdown(dev->ds);
+ ksz_switch_shutdown(dev);

i2c_set_clientdata(i2c, NULL);
}
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 955434055f06..85513318d165 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3810,6 +3810,24 @@ struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
}
EXPORT_SYMBOL(ksz_switch_alloc);

+/**
+ * ksz_switch_shutdown - Shutdown routine for the switch device.
+ * @dev: The switch device structure.
+ *
+ * This function is responsible for initiating a shutdown sequence for the
+ * switch device. It invokes the reset operation defined in the device
+ * operations, if available, to reset the switch. Subsequently, it calls the
+ * DSA framework's shutdown function to ensure a proper shutdown of the DSA
+ * switch.
+ */
+void ksz_switch_shutdown(struct ksz_device *dev)
+{
+ if (dev->dev_ops->reset)
+ dev->dev_ops->reset(dev);
+
+ dsa_switch_shutdown(dev->ds);
+}
+
static void ksz_parse_rgmii_delay(struct ksz_device *dev, int port_num,
struct device_node *port_dn)
{
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 2ad49c5f1df4..81d2973ba7d6 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -398,6 +398,7 @@ 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_shutdown(struct ksz_device *dev);

/* Common register access functions */
static inline struct regmap *ksz_regmap_8(struct ksz_device *dev)
diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index 279338451621..6f6d878e742c 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -114,10 +114,7 @@ static void ksz_spi_shutdown(struct spi_device *spi)
if (!dev)
return;

- if (dev->dev_ops->reset)
- dev->dev_ops->reset(dev);
-
- dsa_switch_shutdown(dev->ds);
+ ksz_switch_shutdown(dev);

spi_set_drvdata(spi, NULL);
}
--
2.39.2

2023-10-16 14:34:07

[permalink] [raw]

Subject: Re: [PATCH net-next v4 2/9] dt-bindings: net: dsa: microchip: add wakeup-source property

On Mon, Oct 16, 2023 at 04:12:49PM +0200, Oleksij Rempel wrote:
> 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]>

Andrew

2023-10-16 14:34:55

[permalink] [raw]

Subject: Re: [PATCH net-next v4 3/9] net: dsa: microchip: use wakeup-source DT property to enable PME output

On Mon, Oct 16, 2023 at 04:12:50PM +0200, Oleksij Rempel wrote:
> 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]>

Andrew

2023-10-16 14:59:42

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: dsa: microchip: ksz9477: add Wake on LAN support

> +int ksz9477_set_wol(struct ksz_device *dev, int port,
> + struct ethtool_wolinfo *wol)
> +{
> + u8 pme_conf, pme_ctrl = 0;
> + int ret;
> +
> + if (wol->wolopts & ~WAKE_PHY)
> + return -EINVAL;

EOPNOTSUPP might be better here. I'm assuming there is no other way
WoL can be supported, since this is a combined MAC/PHY device.

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2023-10-16 15:35:49

[permalink] [raw]

Subject: Re: [PATCH net-next v4 3/9] net: dsa: microchip: use wakeup-source DT property to enable PME output

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> 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: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:35:50

[permalink] [raw]

Subject: Re: [PATCH net-next v4 2/9] dt-bindings: net: dsa: microchip: add wakeup-source property

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> 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: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:38:17

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: dsa: microchip: ksz9477: add Wake on LAN support

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> Add WoL support for KSZ9477 family of switches. This code was tested on
> KSZ8563 chip.
>
> KSZ9477 family of switches supports multiple PHY events:
> - wake on Link Up
> - wake on Energy Detect.
> Since current UAPI can't differentiate between this PHY events, map all of them
> to WAKE_PHY.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
[snip]
> +void ksz9477_get_wol(struct ksz_device *dev, int port,
> + struct ethtool_wolinfo *wol)
> +{
> + u8 pme_ctrl, pme_conf;
> + int ret;
> +
> + ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf);
> + if (ret)
> + return;
> +
> + if (!(pme_conf & PME_ENABLE))
> + return;

I suppose this works beause you have separate enable bits for
WOL_LINKUP, WOL_ENERGY and WOL_MAGICPKT, you could have also left the
setting of the PME_ENABLE bit to the set_wol() routine provided that
wol->wolopts is non-zero.

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:38:40

[permalink] [raw]

Subject: Re: [PATCH net-next v4 6/9] net: dsa: microchip: Refactor comment for ksz_switch_macaddr_get() function

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> Update the comment to follow kernel-doc format.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:39:24

[permalink] [raw]

Subject: Re: [PATCH net-next v4 5/9] net: dsa: microchip: ksz9477: Add Wake on Magic Packet support

On 10/16/2023 7:12 AM, 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]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:39:36

[permalink] [raw]

Subject: Re: [PATCH net-next v4 8/9] net: dsa: microchip: Refactor switch shutdown routine for WoL preparation

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> Centralize the switch shutdown routine in a dedicated function,
> ksz_switch_shutdown(), to enhance code maintainability and reduce
> redundancy. This change abstracts the common shutdown operations
> previously duplicated in ksz9477_i2c_shutdown() and ksz_spi_shutdown().
>
> This refactoring is a preparatory step for an upcoming patch to avoid
> reset on shutdown if Wake-on-LAN (WoL) is enabled.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-10-16 15:39:49

[permalink] [raw]

Subject: Re: [PATCH net-next v4 9/9] net: dsa: microchip: do not reset the switch on shutdown if WoL is active

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> For Wake on Lan we should not reconfigure, reset or power down the
> switch on shut down sequence.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-10-17 01:36:30

[permalink] [raw]

Subject: Re: [PATCH net-next v4 5/9] net: dsa: microchip: ksz9477: Add Wake on Magic Packet support

> @@ -155,6 +158,14 @@ int ksz9477_set_wol(struct ksz_device *dev, int port,
> if (ret)
> return ret;
>
> + if (wol->wolopts & WAKE_MAGIC) {
> + ret = ksz_switch_macaddr_get(dev->ds, port, NULL);
> + if (ret)
> + return ret;
> +
> + pme_ctrl |= PME_WOL_MAGICPKT;
> + }

There is no matching ksz_switch_macaddr_put() here when the user turns
WAKE_MAGIC off. Ideally you need to keep track of the WoL state per
port, and when the user disables it, release the MAC address.

> +
> if (wol->wolopts & WAKE_PHY)
> pme_ctrl |= PME_WOL_LINKUP | PME_WOL_ENERGY;
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 3f7c86e545a7..4601aaca5179 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);

This is not really an error, as in something went wrong. Its just a
hardware restriction. So dev_warn() would be better, or nothing at all
in the logs.

Andrew

2023-10-17 01:38:34

[permalink] [raw]

Subject: Re: [PATCH net-next v4 7/9] net: dsa: microchip: Add error handling for ksz_switch_macaddr_get()

On Mon, Oct 16, 2023 at 04:12:54PM +0200, Oleksij Rempel wrote:
> 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]>

Andrew

2023-10-17 01:43:11

[permalink] [raw]

Subject: Re: [PATCH net-next v4 5/9] net: dsa: microchip: ksz9477: Add Wake on Magic Packet support

> - wol->supported = WAKE_PHY;
> + wol->supported = WAKE_PHY | WAKE_MAGIC;

It is clearly racy, but maybe try a ksz_switch_macaddr_get() and only
if it is successful add WAKE_MAGIC. Then do a
ksz_switch_macaddr_put(). Given the limitations of the hardware, it
might help the user understand what the hardware can do.

But this is not a must.

Andrew

2023-10-17 02:00:01

[permalink] [raw]

Subject: Re: [PATCH net-next v4 7/9] net: dsa: microchip: Add error handling for ksz_switch_macaddr_get()

On 10/16/2023 7:12 AM, Oleksij Rempel wrote:
> 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: Florian Fainelli <[email protected]>
--
Florian

2023-10-17 06:19:43

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: dsa: microchip: ksz9477: add Wake on LAN support

On Mon, Oct 16, 2023 at 08:37:50AM -0700, Florian Fainelli wrote:
> [snip]
> > +void ksz9477_get_wol(struct ksz_device *dev, int port,
> > + struct ethtool_wolinfo *wol)
> > +{
> > + u8 pme_ctrl, pme_conf;
> > + int ret;
> > +
> > + ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf);
> > + if (ret)
> > + return;
> > +
> > + if (!(pme_conf & PME_ENABLE))
> > + return;
>
> I suppose this works beause you have separate enable bits for WOL_LINKUP,
> WOL_ENERGY and WOL_MAGICPKT, you could have also left the setting of the
> PME_ENABLE bit to the set_wol() routine provided that wol->wolopts is
> non-zero.

Setting the PME_ENABLE bit in the set_wol() function might imply that it
should also be cleared within set_wol(), necessitating refcounting due
to its global bit nature.

As an alternative, the reading of the REG_SW_PME_CTRL register could be
replaced by checking the wakeup_source variable, which might result in
some optimization.

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 |

2023-10-17 06:26:09