| Message ID | 20231016141256.2011861-6-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: provide Wake on LAN support | expand |
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 <o.rempel@pengutronix.de> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> @@ -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
> - 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
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)
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 <o.rempel@pengutronix.de> --- 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(-)