Messages in this thread | | | Date | Sat, 20 May 2023 02:28:02 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v4 1/2] net: dsa: microchip: ksz8: Make flow control, speed, and duplex on CPU port configurable |
| |
On Fri, May 19, 2023 at 02:46:59PM +0200, Oleksij Rempel wrote: > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c > index f56fca1b1a22..9eedccbf5b7c 100644 > --- a/drivers/net/dsa/microchip/ksz8795.c > +++ b/drivers/net/dsa/microchip/ksz8795.c > @@ -1371,6 +1371,57 @@ void ksz8_config_cpu_port(struct dsa_switch *ds) > +/** > + * ksz8_cpu_port_link_up - Configures the CPU port of the switch. > + * @dev: The KSZ device instance. > + * @speed: The desired link speed. > + * @duplex: The desired duplex mode. > + * @tx_pause: If true, enables transmit pause. > + * @rx_pause: If true, enables receive pause. > + * > + * Description: > + * The function configures flow control and speed settings for the CPU > + * port of the switch based on the desired settings, current duplex mode, and > + * speed. > + */ > +static void ksz8_cpu_port_link_up(struct ksz_device *dev, int speed, int duplex, > + bool tx_pause, bool rx_pause) > +{ > + u8 ctrl = 0; > + > + /* SW_FLOW_CTRL, SW_HALF_DUPLEX, and SW_10_MBIT bits are bootstrappable > + * at least on KSZ8873. They can have different values depending on your > + * board setup. > + */ > + if (duplex) { > + if (tx_pause || rx_pause) > + ctrl |= SW_FLOW_CTRL; > + } else { > + ctrl |= SW_HALF_DUPLEX; > + } > + > + /* This hardware only supports SPEED_10 and SPEED_100. For SPEED_10 > + * we need to set the SW_10_MBIT bit. Otherwise, we can leave it 0. > + */ > + if (speed == SPEED_10) > + ctrl |= SW_10_MBIT; > + > + ksz_rmw8(dev, REG_SW_CTRL_4, SW_HALF_DUPLEX | SW_FLOW_CTRL | > + SW_10_MBIT, ctrl);
REG_SW_CTRL_4 ... S_REPLACE_VID_CTRL ... dev->info->regs[P_XMII_CTRL_1] ... at some point we will need one more consolidation effort here, since we have at least 3 ways of reaching the same register.
> +} > + > +void ksz8_phylink_mac_link_up(struct ksz_device *dev, int port, > + unsigned int mode, phy_interface_t interface, > + struct phy_device *phydev, int speed, int duplex, > + bool tx_pause, bool rx_pause) > +{ > + /* If the port is the CPU port, apply special handling. Only the CPU > + * port is configured via global registers. > + */ > + if (dev->cpu_port == port) > + ksz8_cpu_port_link_up(dev, speed, duplex, tx_pause, rx_pause); > +} > + > static int ksz8_handle_global_errata(struct dsa_switch *ds) > { > struct ksz_device *dev = ds->priv; > @@ -1419,8 +1470,6 @@ int ksz8_setup(struct dsa_switch *ds) > */ > ds->vlan_filtering_is_global = true; > > - ksz_cfg(dev, S_REPLACE_VID_CTRL, SW_FLOW_CTRL, true); > - > /* Enable automatic fast aging when link changed detected. */ > ksz_cfg(dev, S_LINK_AGING_CTRL, SW_LINK_AUTO_AGING, true); > > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c > index a4428be5f483..6e19ad70c671 100644 > --- a/drivers/net/dsa/microchip/ksz_common.c > +++ b/drivers/net/dsa/microchip/ksz_common.c > @@ -210,6 +210,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = { > .mirror_add = ksz8_port_mirror_add, > .mirror_del = ksz8_port_mirror_del, > .get_caps = ksz8_get_caps, > + .phylink_mac_link_up = ksz8_phylink_mac_link_up,
Another future consolidation to consider: since all ksz_dev_ops now provide .phylink_mac_link_up(), the "if" condition here is no longer necessary:
static void ksz_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode, phy_interface_t interface, struct phy_device *phydev, int speed, int duplex, bool tx_pause, bool rx_pause) { struct ksz_device *dev = ds->priv;
if (dev->dev_ops->phylink_mac_link_up) dev->dev_ops->phylink_mac_link_up(dev, port, mode, interface, phydev, speed, duplex, tx_pause, rx_pause); }
which reminds me of the fact that I also had a patch to remove dev->dev_ops->phylink_mac_config(): https://patchwork.kernel.org/project/netdevbpf/patch/20230316161250.3286055-5-vladimir.oltean@nxp.com/
I give up with that patch set now, since there's zero reviewer interest. If you want and you think it's useful, you might want to adapt it for KSZ8873.
> .config_cpu_port = ksz8_config_cpu_port, > .enable_stp_addr = ksz8_enable_stp_addr, > .reset = ksz8_reset_switch, > -- > 2.39.2 >
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
| |