@@ -216,13 +216,6 @@ static const struct ksz_dev_ops ksz8_dev_ops = {
.change_mtu = ksz8_change_mtu,
};
-static void ksz9477_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);
-
static const struct ksz_dev_ops ksz9477_dev_ops = {
.setup = ksz9477_setup,
.get_port_addr = ksz9477_get_port_addr,
@@ -249,7 +242,6 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
.mdb_add = ksz9477_mdb_add,
.mdb_del = ksz9477_mdb_del,
.change_mtu = ksz9477_change_mtu,
- .phylink_mac_link_up = ksz9477_phylink_mac_link_up,
.config_cpu_port = ksz9477_config_cpu_port,
.tc_cbs_set_cinc = ksz9477_tc_cbs_set_cinc,
.enable_stp_addr = ksz9477_enable_stp_addr,
@@ -286,7 +278,6 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
.mdb_add = ksz9477_mdb_add,
.mdb_del = ksz9477_mdb_del,
.change_mtu = lan937x_change_mtu,
- .phylink_mac_link_up = ksz9477_phylink_mac_link_up,
.config_cpu_port = lan937x_config_cpu_port,
.tc_cbs_set_cinc = lan937x_tc_cbs_set_cinc,
.enable_stp_addr = ksz9477_enable_stp_addr,
@@ -2983,15 +2974,18 @@ static void ksz_duplex_flowctrl(struct ksz_device *dev, int port, int duplex,
ksz_regfields_write(dev, port, RF_MII_RX_FLOW_CTRL, !!rx_pause);
}
-static void ksz9477_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)
+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;
struct ksz_port *p;
+ if (ksz_is_ksz88x3(dev))
+ return;
+
p = &dev->ports[port];
/* Internal PHYs */
@@ -3005,20 +2999,6 @@ static void ksz9477_phylink_mac_link_up(struct ksz_device *dev, int port,
ksz_duplex_flowctrl(dev, port, duplex, tx_pause, rx_pause);
}
-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);
-}
-
static int ksz_switch_detect(struct ksz_device *dev)
{
u8 id1, id2, id4;
@@ -381,11 +381,6 @@ struct ksz_dev_ops {
void (*phylink_mac_config)(struct ksz_device *dev, int port,
unsigned int mode,
const struct phylink_link_state *state);
- void (*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);
void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
void (*config_cpu_port)(struct dsa_switch *ds);
Prior to commit c476bede4b0f ("net: dsa: microchip: ksz8795: use common xmii function"), the logic from ksz8795_cpu_interface_select() used to modify the following fields in REG_PORT_5_CTRL_6, based on the phy-mode of the CPU port: - PORT_INTERFACE_TYPE (now P_MII_SEL_M) - PORT_GMII_1GPS_MODE (now P_GMII_1GBIT_M) - PORT_RGMII_ID_IN_ENABLE (now P_RGMII_ID_IG_ENABLE) - PORT_RGMII_ID_OUT_ENABLE (now P_RGMII_ID_EG_ENABLE) That logic was replaced to a call with ksz_set_xmii(), which only touches 3 of those 4 fields. It does not touch PORT_GMII_1GPS_MODE (now P_GMII_1GBIT_M, or RF_GMII_1GBIT, in its reg_fields form). That is handled by ksz_set_gbit(), which should have been called as well, for code-wise identical behavior. The driver has always written to PORT_GMII_1GPS_MODE to enable RGMII ports at their maximum speed, since the initial commit e66f840c08a2 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver"). Searching in the KSZ8975 documentation, I see that the Is_1Gbps field (what the driver calls P_GMII_1GBIT_M) is set to 1 by default, unless pin strapping via LED1_0 sets it to zero, case in which the RGMII speed is either 10 or 100 Mbps - controlled via P_MII_100MBIT. I can only imagine this being a problem if there are boards where the pin strapping is wrong (asking for 100Mbps when the link was capable of gigabit), and the initial version of the driver was overwriting that. That may or may not be plausible. For example, this commit does indicate a (different, but still) incorrect pin strapping setting which does need to be fixed up by the Linux driver: https://patchwork.kernel.org/project/netdevbpf/patch/20230315231916.2998480-1-vladimir.oltean@nxp.com/ In any case, it makes sense for Linux to configure the switch to match the device tree description, and that means calling the missing ksz_port_set_xmii_speed(). The current position of the ksz_port_set_xmii_speed() call is from the ksz9477_phylink_mac_link_up() handler, but this is called for all dev_ops except for ksz8_dev_ops. Studyint ksz9477_phylink_mac_link_up() a bit, its contents seems to be more than useful, since it also calls ksz_duplex_flowctrl() and that is also something that KSZ8765 supports but doesn't call. It seems to be desirable to move the entirety of ksz9477_phylink_mac_link_up() into the common ksz_phylink_mac_link_up(), and remove the family-specific handling. The KSZ8830 switch is a bit special. It uses ksz8_dev_ops() (so it has skipped phylink_mac_link_up() so far), but it uses the ksz8863_regs[], which don't have P_GMII_1GBIT_M defined. This makes sense, since the KSZ8830 switch comes either in RMII or MII variants, both of which are fixed speeds (100Mbps). So we still need to skip phylink_mac_link_up() completely for these, and the test for ksz_is_ksz88x3(), which is also present in ksz_phylink_mac_config(), achieves that. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/dsa/microchip/ksz_common.c | 38 ++++++-------------------- drivers/net/dsa/microchip/ksz_common.h | 5 ---- 2 files changed, 9 insertions(+), 34 deletions(-)