| Message ID | 20231013122405.3745475-3-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 Fri, Oct 13, 2023 at 02:24:00PM +0200, Oleksij Rempel wrote: > Set a unique global MAC address for each switch on the network at system > startup by syncing the switch's global MAC address with the Ethernet > address of the DSA master interface. This is crucial for supporting > Wake-on-LAN (WoL) functionality, as it requires a unique address for > each switch. > > Although the operation is performed only at system start and won't sync > if the master Ethernet address changes dynamically, it lays the > groundwork for WoL support by ensuring a unique MAC address for each > switch. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- Why not take the MAC address of the user port at ksz9477_set_wol() time, and use the existing ksz_switch_macaddr_get() API that was just added so that this use case could work?
On Fri, Oct 13, 2023 at 02:24:00PM +0200, Oleksij Rempel wrote: > Set a unique global MAC address for each switch on the network at system > startup by syncing the switch's global MAC address with the Ethernet > address of the DSA master interface. This is crucial for supporting > Wake-on-LAN (WoL) functionality, as it requires a unique address for > each switch. > > Although the operation is performed only at system start and won't sync > if the master Ethernet address changes dynamically, it lays the > groundwork for WoL support by ensuring a unique MAC address for each > switch. I've not been following this patchset, so sorry if i make points others have asked on earlier versions. Maybe it would be good to add that the hardware only supports one MAC address for all ports for WoL, and its this address. At least that is my assumption. > + * ksz_cmn_set_default_switch_mac_addr - Set the switch's global MAC address > + * from master port. Florian is doing a search replace to make use of the word `conduit`. > @@ -3572,8 +3633,6 @@ static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port, > const unsigned char *addr = slave->dev_addr; and this might need to change to user?
On 10/13/2023 5:32 AM, Vladimir Oltean wrote: > On Fri, Oct 13, 2023 at 02:24:00PM +0200, Oleksij Rempel wrote: >> Set a unique global MAC address for each switch on the network at system >> startup by syncing the switch's global MAC address with the Ethernet >> address of the DSA master interface. This is crucial for supporting >> Wake-on-LAN (WoL) functionality, as it requires a unique address for >> each switch. >> >> Although the operation is performed only at system start and won't sync >> if the master Ethernet address changes dynamically, it lays the >> groundwork for WoL support by ensuring a unique MAC address for each >> switch. >> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> --- > > Why not take the MAC address of the user port at ksz9477_set_wol() time, > and use the existing ksz_switch_macaddr_get() API that was just added so > that this use case could work? Agreed we do that in a number of Ethernet MAC and PHY drivers FWIW (net_device::dev_addr).
On Sun, Oct 15, 2023 at 02:18:43PM -0700, Florian Fainelli wrote: > > > On 10/13/2023 5:32 AM, Vladimir Oltean wrote: > > On Fri, Oct 13, 2023 at 02:24:00PM +0200, Oleksij Rempel wrote: > > > Set a unique global MAC address for each switch on the network at system > > > startup by syncing the switch's global MAC address with the Ethernet > > > address of the DSA master interface. This is crucial for supporting > > > Wake-on-LAN (WoL) functionality, as it requires a unique address for > > > each switch. > > > > > > Although the operation is performed only at system start and won't sync > > > if the master Ethernet address changes dynamically, it lays the > > > groundwork for WoL support by ensuring a unique MAC address for each > > > switch. > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > --- > > > > Why not take the MAC address of the user port at ksz9477_set_wol() time, > > and use the existing ksz_switch_macaddr_get() API that was just added so > > that this use case could work? > > Agreed we do that in a number of Ethernet MAC and PHY drivers FWIW > (net_device::dev_addr). > -- > Florian To be clear (to Oleksij), the request is for WoL to use the same runtime management of the global MAC address (ksz_switch_macaddr_get) as HSR, and also extend ksz_port_set_mac_address() to deny address changes to a port with WoL active. Thus, multiple user ports could have WoL enabled as long as they share the same MAC address. MAC address changes are also possible while WoL is not enabled. I guess wol->supported should only get set on those user ports which have the same MAC address as the global MAC address (if a global MAC address is configured), or on all user ports (if there is none).
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 02fab1adb27f..db0ef4ad181e 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -2173,6 +2173,57 @@ static int ksz_pirq_setup(struct ksz_device *dev, u8 p) return ksz_irq_common_setup(dev, pirq); } +/** + * ksz_cmn_write_global_mac_addr - Write global MAC address to switch. + * @dev: The device structure. + * @addr: Pointer to MAC address. + * + * This function programs the switch's MAC address register with the given + * MAC address. The global MAC address is used as the source address in MAC + * pause control frames, for HSR self-address filtering, Wake-on-LAN (WoL), + * and for self-address filtering purposes. + * + * Return: 0 on success, or an error code on failure. + */ +static int ksz_cmn_write_global_mac_addr(struct ksz_device *dev, + const unsigned char *addr) +{ + const u16 *regs = dev->info->regs; + int i, ret; + + 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; +} + +/** + * ksz_cmn_set_default_switch_mac_addr - Set the switch's global MAC address + * from master port. + * @dev: The device structure. + * + * This function retrieves the MAC address from the master network device + * associated with the CPU port and writes it to the switch's global MAC + * address register. + * + * Return: 0 on success, or an error code on failure. + */ +static int ksz_cmn_set_default_switch_mac_addr(struct ksz_device *dev) +{ + struct dsa_switch *ds = dev->ds; + struct net_device *master; + + /* use CPU port to get master net device because it is guaranteed + * to be a valid port + */ + master = dsa_port_to_master(dsa_to_port(ds, dev->cpu_port)); + + return ksz_cmn_write_global_mac_addr(dev, master->dev_addr); +} + static int ksz_setup(struct dsa_switch *ds) { struct ksz_device *dev = ds->priv; @@ -2194,6 +2245,16 @@ static int ksz_setup(struct dsa_switch *ds) return ret; } + /* Set switch MAC address from master port. + * In the current implementation, this operation is only + * performed during system start and will not sync if the master + * Ethernet address changes dynamically thereafter. The global MAC + * address still can be overwritten for HSR use cases. + */ + ret = ksz_cmn_set_default_switch_mac_addr(dev); + if (ret) + return ret; + /* set broadcast storm protection 10% rate */ regmap_update_bits(ksz_regmap_16(dev), regs[S_BROADCAST_CTRL], BROADCAST_STORM_RATE, @@ -3572,8 +3633,6 @@ static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port, const unsigned char *addr = slave->dev_addr; struct ksz_switch_macaddr *switch_macaddr; struct ksz_device *dev = ds->priv; - const u16 *regs = dev->info->regs; - int i; /* Make sure concurrent MAC address changes are blocked */ ASSERT_RTNL(); @@ -3599,11 +3658,7 @@ static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port, refcount_set(&switch_macaddr->refcount, 1); 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]); - - return 0; + return ksz_cmn_write_global_mac_addr(dev, addr); } static void ksz_switch_macaddr_put(struct dsa_switch *ds)
Set a unique global MAC address for each switch on the network at system startup by syncing the switch's global MAC address with the Ethernet address of the DSA master interface. This is crucial for supporting Wake-on-LAN (WoL) functionality, as it requires a unique address for each switch. Although the operation is performed only at system start and won't sync if the master Ethernet address changes dynamically, it lays the groundwork for WoL support by ensuring a unique MAC address for each switch. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz_common.c | 69 +++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 7 deletions(-)