From 973fbe68df39a315bf1233ea7f50e87f8a649947 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 14 Nov 2022 19:07:27 +0200 Subject: [PATCH 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs These are Gen3 Aquantia N-BASET PHYs which support 5GBASE-T, 2.5GBASE-T, 1000BASE-T and 100BASE-TX (not 10G); also EEE, Sync-E, PTP, PoE. The 112 is a single PHY package, the 412 is a quad PHY package. The system-side SERDES interface of these PHYs selects its protocol depending on the negotiated media side link speed. That protocol can be 1000BASE-X, 2500BASE-X, 10GBASE-R, SGMII, USXGMII. The configuration of which SERDES protocol to use for which link speed is made by firmware; even though it could be overwritten over MDIO by Linux, we assume that the firmware provisioning is ok for the board on which the driver probes. For cases when the system side runs at a fixed rate, we want phylib/phylink to detect the PAUSE rate matching ability of these PHYs, so we need to use the Aquantia rather than the generic C45 driver. This needs aqr107_read_status() -> aqr107_read_rate() to set phydev->rate_matching, as well as the aqr107_get_rate_matching() method. I am a bit unsure about the naming convention in the driver. Since AQR107 is a Gen2 PHY, I assume all functions prefixed with "aqr107_" rather than "aqr_" mean Gen2+ features. So I've reused this naming convention. I've tested PHY "SGMII" statistics as well as the .link_change_notify method, which prints: Aquantia AQR412 mdio_mux-0.4:00: Link partner is Aquantia PHY, FW 4.3, fast-retrain downshift advertised, fast reframe advertised Tested SERDES protocols are usxgmii and 2500base-x (the latter with PAUSE rate matching). Tested link modes are 100/1000/2500 Base-T (with Aquantia link partner and with other link partners). No notable events observed. The placement of these PHY IDs in the driver is right before AQR113C, a Gen4 PHY. Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- drivers/net/phy/aquantia_main.c | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index 47a76df36b747..334a6904ca5a3 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -22,6 +22,8 @@ #define PHY_ID_AQR107 0x03a1b4e0 #define PHY_ID_AQCS109 0x03a1b5c2 #define PHY_ID_AQR405 0x03a1b4b0 +#define PHY_ID_AQR112 0x03a1b662 +#define PHY_ID_AQR412 0x03a1b712 #define PHY_ID_AQR113C 0x31c31c12 #define MDIO_PHYXS_VEND_IF_STATUS 0xe812 @@ -800,6 +802,42 @@ static struct phy_driver aqr_driver[] = { .handle_interrupt = aqr_handle_interrupt, .read_status = aqr_read_status, }, +{ + PHY_ID_MATCH_MODEL(PHY_ID_AQR112), + .name = "Aquantia AQR112", + .probe = aqr107_probe, + .config_aneg = aqr_config_aneg, + .config_intr = aqr_config_intr, + .handle_interrupt = aqr_handle_interrupt, + .get_tunable = aqr107_get_tunable, + .set_tunable = aqr107_set_tunable, + .suspend = aqr107_suspend, + .resume = aqr107_resume, + .read_status = aqr107_read_status, + .get_rate_matching = aqr107_get_rate_matching, + .get_sset_count = aqr107_get_sset_count, + .get_strings = aqr107_get_strings, + .get_stats = aqr107_get_stats, + .link_change_notify = aqr107_link_change_notify, +}, +{ + PHY_ID_MATCH_MODEL(PHY_ID_AQR412), + .name = "Aquantia AQR412", + .probe = aqr107_probe, + .config_aneg = aqr_config_aneg, + .config_intr = aqr_config_intr, + .handle_interrupt = aqr_handle_interrupt, + .get_tunable = aqr107_get_tunable, + .set_tunable = aqr107_set_tunable, + .suspend = aqr107_suspend, + .resume = aqr107_resume, + .read_status = aqr107_read_status, + .get_rate_matching = aqr107_get_rate_matching, + .get_sset_count = aqr107_get_sset_count, + .get_strings = aqr107_get_strings, + .get_stats = aqr107_get_stats, + .link_change_notify = aqr107_link_change_notify, +}, { PHY_ID_MATCH_MODEL(PHY_ID_AQR113C), .name = "Aquantia AQR113C", @@ -831,6 +869,8 @@ static struct mdio_device_id __maybe_unused aqr_tbl[] = { { PHY_ID_MATCH_MODEL(PHY_ID_AQR107) }, { PHY_ID_MATCH_MODEL(PHY_ID_AQCS109) }, { PHY_ID_MATCH_MODEL(PHY_ID_AQR405) }, + { PHY_ID_MATCH_MODEL(PHY_ID_AQR112) }, + { PHY_ID_MATCH_MODEL(PHY_ID_AQR412) }, { PHY_ID_MATCH_MODEL(PHY_ID_AQR113C) }, { } }; From 3e7e783291b42f95756dcf54cd8c67973e6ddefd Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 14 Nov 2022 19:07:28 +0200 Subject: [PATCH 2/4] net: dsa: felix: use phylink_generic_validate() Drop the custom implementation of phylink_validate() in favor of the generic one, which requires config->mac_capabilities to be set. This was used up until now because of the possibility of being paired with Aquantia PHYs with support for rate matching. The phylink framework gained generic support for these, and knows to advertise all 10/100/1000 lower speed link modes when our SERDES protocol is 2500base-x (fixed speed). Signed-off-by: Vladimir Oltean Reviewed-by: Russell King (Oracle) Signed-off-by: Jakub Kicinski --- drivers/net/dsa/ocelot/felix.c | 16 ++++--------- drivers/net/dsa/ocelot/felix.h | 3 --- drivers/net/dsa/ocelot/felix_vsc9959.c | 30 ------------------------ drivers/net/dsa/ocelot/seville_vsc9953.c | 27 --------------------- 4 files changed, 4 insertions(+), 72 deletions(-) diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index dd3a18cc89dd2..44e160f320678 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1048,21 +1048,14 @@ static void felix_phylink_get_caps(struct dsa_switch *ds, int port, */ config->legacy_pre_march2020 = false; + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | + MAC_10 | MAC_100 | MAC_1000FD | + MAC_2500FD; + __set_bit(ocelot->ports[port]->phy_mode, config->supported_interfaces); } -static void felix_phylink_validate(struct dsa_switch *ds, int port, - unsigned long *supported, - struct phylink_link_state *state) -{ - struct ocelot *ocelot = ds->priv; - struct felix *felix = ocelot_to_felix(ocelot); - - if (felix->info->phylink_validate) - felix->info->phylink_validate(ocelot, port, supported, state); -} - static struct phylink_pcs *felix_phylink_mac_select_pcs(struct dsa_switch *ds, int port, phy_interface_t iface) @@ -2050,7 +2043,6 @@ const struct dsa_switch_ops felix_switch_ops = { .get_sset_count = felix_get_sset_count, .get_ts_info = felix_get_ts_info, .phylink_get_caps = felix_phylink_get_caps, - .phylink_validate = felix_phylink_validate, .phylink_mac_select_pcs = felix_phylink_mac_select_pcs, .phylink_mac_link_down = felix_phylink_mac_link_down, .phylink_mac_link_up = felix_phylink_mac_link_up, diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h index c9c29999c3363..42338116eed01 100644 --- a/drivers/net/dsa/ocelot/felix.h +++ b/drivers/net/dsa/ocelot/felix.h @@ -52,9 +52,6 @@ struct felix_info { int (*mdio_bus_alloc)(struct ocelot *ocelot); void (*mdio_bus_free)(struct ocelot *ocelot); - void (*phylink_validate)(struct ocelot *ocelot, int port, - unsigned long *supported, - struct phylink_link_state *state); int (*port_setup_tc)(struct dsa_switch *ds, int port, enum tc_setup_type type, void *type_data); void (*tas_guard_bands_update)(struct ocelot *ocelot, int port); diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index 26a35ae322d10..b0ae8d6156f6b 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -885,35 +885,6 @@ static int vsc9959_reset(struct ocelot *ocelot) return 0; } -static void vsc9959_phylink_validate(struct ocelot *ocelot, int port, - unsigned long *supported, - struct phylink_link_state *state) -{ - __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; - - phylink_set_port_modes(mask); - phylink_set(mask, Autoneg); - phylink_set(mask, Pause); - phylink_set(mask, Asym_Pause); - phylink_set(mask, 10baseT_Half); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 100baseT_Half); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 1000baseT_Half); - phylink_set(mask, 1000baseT_Full); - phylink_set(mask, 1000baseX_Full); - - if (state->interface == PHY_INTERFACE_MODE_INTERNAL || - state->interface == PHY_INTERFACE_MODE_2500BASEX || - state->interface == PHY_INTERFACE_MODE_USXGMII) { - phylink_set(mask, 2500baseT_Full); - phylink_set(mask, 2500baseX_Full); - } - - linkmode_and(supported, supported, mask); - linkmode_and(state->advertising, state->advertising, mask); -} - /* Watermark encode * Bit 8: Unit; 0:1, 1:16 * Bit 7-0: Value to be multiplied with unit @@ -2588,7 +2559,6 @@ static const struct felix_info felix_info_vsc9959 = { .ptp_caps = &vsc9959_ptp_caps, .mdio_bus_alloc = vsc9959_mdio_bus_alloc, .mdio_bus_free = vsc9959_mdio_bus_free, - .phylink_validate = vsc9959_phylink_validate, .port_modes = vsc9959_port_modes, .port_setup_tc = vsc9959_port_setup_tc, .port_sched_speed_set = vsc9959_sched_speed_set, diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c index 7af33b2c685da..6500c1697dd61 100644 --- a/drivers/net/dsa/ocelot/seville_vsc9953.c +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c @@ -840,32 +840,6 @@ static int vsc9953_reset(struct ocelot *ocelot) return 0; } -static void vsc9953_phylink_validate(struct ocelot *ocelot, int port, - unsigned long *supported, - struct phylink_link_state *state) -{ - __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; - - phylink_set_port_modes(mask); - phylink_set(mask, Autoneg); - phylink_set(mask, Pause); - phylink_set(mask, Asym_Pause); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 10baseT_Half); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 100baseT_Half); - phylink_set(mask, 1000baseT_Full); - phylink_set(mask, 1000baseX_Full); - - if (state->interface == PHY_INTERFACE_MODE_INTERNAL) { - phylink_set(mask, 2500baseT_Full); - phylink_set(mask, 2500baseX_Full); - } - - linkmode_and(supported, supported, mask); - linkmode_and(state->advertising, state->advertising, mask); -} - /* Watermark encode * Bit 9: Unit; 0:1, 1:16 * Bit 8-0: Value to be multiplied with unit @@ -1007,7 +981,6 @@ static const struct felix_info seville_info_vsc9953 = { .num_tx_queues = OCELOT_NUM_TC, .mdio_bus_alloc = vsc9953_mdio_bus_alloc, .mdio_bus_free = vsc9953_mdio_bus_free, - .phylink_validate = vsc9953_phylink_validate, .port_modes = vsc9953_port_modes, }; From de8586ed4311286a7fb17c3b0166076c1a548518 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 14 Nov 2022 19:07:29 +0200 Subject: [PATCH 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control As phylink gained generic support for PHYs with rate matching via PAUSE frames, the phylink_mac_link_up() method will be called with the maximum speed and with rx_pause=true if rate matching is in use. This means that setups with 2500base-x as the SERDES protocol between the MAC/PCS and the PHY now work with no need for the driver to do anything special. Tested with fsl-ls1028a-qds-7777.dts. Signed-off-by: Vladimir Oltean Reviewed-by: Russell King (Oracle) Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mscc/ocelot.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 13b14110a0603..da56f9bfeaf0d 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -872,10 +872,8 @@ void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port, return; } - /* Handle RX pause in all cases, with 2500base-X this is used for rate - * adaptation. - */ - mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA; + if (rx_pause) + mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA; if (tx_pause) mac_fc_cfg |= SYS_MAC_FC_CFG_TX_FC_ENA | From 53d04b9811107633f25be02a5d981a6070d09e6e Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 14 Nov 2022 19:07:30 +0200 Subject: [PATCH 4/4] net: dsa: remove phylink_validate() method As of now, no DSA driver uses a custom link mode validation procedure anymore. So remove this DSA operation and let phylink determine what is supported based on config->mac_capabilities (if provided by the driver). Leave a comment why we left the code that we did, and that there is more work to do. Signed-off-by: Vladimir Oltean Reviewed-by: Russell King (Oracle) Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 3 --- net/dsa/port.c | 18 ++++++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index ee369670e20e4..dde364688739a 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -880,9 +880,6 @@ struct dsa_switch_ops { */ void (*phylink_get_caps)(struct dsa_switch *ds, int port, struct phylink_config *config); - void (*phylink_validate)(struct dsa_switch *ds, int port, - unsigned long *supported, - struct phylink_link_state *state); struct phylink_pcs *(*phylink_mac_select_pcs)(struct dsa_switch *ds, int port, phy_interface_t iface); diff --git a/net/dsa/port.c b/net/dsa/port.c index 208168276995a..48c9eaa74aee0 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1536,16 +1536,14 @@ static void dsa_port_phylink_validate(struct phylink_config *config, unsigned long *supported, struct phylink_link_state *state) { - struct dsa_port *dp = container_of(config, struct dsa_port, pl_config); - struct dsa_switch *ds = dp->ds; - - if (!ds->ops->phylink_validate) { - if (config->mac_capabilities) - phylink_generic_validate(config, supported, state); - return; - } - - ds->ops->phylink_validate(ds, dp->index, supported, state); + /* Skip call for drivers which don't yet set mac_capabilities, + * since validating in that case would mean their PHY will advertise + * nothing. In turn, skipping validation makes them advertise + * everything that the PHY supports, so those drivers should be + * converted ASAP. + */ + if (config->mac_capabilities) + phylink_generic_validate(config, supported, state); } static void dsa_port_phylink_mac_pcs_get_state(struct phylink_config *config,