| Message ID | 20221202191749.27437-3-jerry.ray@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | dsa: lan9303: Move to PHYLINK | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 91 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Hi Jerry, On Fri, Dec 02, 2022 at 01:17:49PM -0600, Jerry Ray wrote: > -static void lan9303_adjust_link(struct dsa_switch *ds, int port, > - struct phy_device *phydev) > -{ > - struct lan9303 *chip = ds->priv; > - int ctl; > - > - if (!phy_is_pseudo_fixed_link(phydev)) > - return; > - > - ctl = lan9303_phy_read(ds, port, MII_BMCR); > - > - ctl &= ~BMCR_ANENABLE; > - > - if (phydev->speed == SPEED_100) > - ctl |= BMCR_SPEED100; > - else if (phydev->speed == SPEED_10) > - ctl &= ~BMCR_SPEED100; > - else > - dev_err(ds->dev, "unsupported speed: %d\n", phydev->speed); > - > - if (phydev->duplex == DUPLEX_FULL) > - ctl |= BMCR_FULLDPLX; > - else > - ctl &= ~BMCR_FULLDPLX; > - > - lan9303_phy_write(ds, port, MII_BMCR, ctl); > - > - if (port == chip->phy_addr_base) { > - /* Virtual Phy: Remove Turbo 200Mbit mode */ > - lan9303_read(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, &ctl); > - > - ctl &= ~LAN9303_VIRT_SPECIAL_TURBO; > - regmap_write(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, ctl); > - } > -} Is this functionality no longer necessary? For example, I don't see anywhere else in the driver that this turbo mode is disabled. I'm guessing the above code writing MII_BMCR is to force the configuration of integrated PHYs to be the fixed-link settings? How is that dealt with after the removal of the above code? > - > static int lan9303_port_enable(struct dsa_switch *ds, int port, > struct phy_device *phy) > { > @@ -1279,6 +1243,41 @@ static int lan9303_port_mdb_del(struct dsa_switch *ds, int port, > return 0; > } > > +static void lan9303_phylink_get_caps(struct dsa_switch *ds, int port, > + struct phylink_config *config) > +{ > + struct lan9303 *chip = ds->priv; > + > + dev_dbg(chip->dev, "%s(%d) entered.", __func__, port); > + > + config->mac_capabilities = MAC_10 | MAC_100 | MAC_ASYM_PAUSE | > + MAC_SYM_PAUSE; > + > + if (dsa_port_is_cpu(dsa_to_port(ds, port))) { > + /* cpu port */ > + phy_interface_empty(config->supported_interfaces); This should not be necessary - the supported_interfaces member should already be zero. Thanks.
>> -static void lan9303_adjust_link(struct dsa_switch *ds, int port, >> - struct phy_device *phydev) >> -{ >> - struct lan9303 *chip = ds->priv; >> - int ctl; >> - >> - if (!phy_is_pseudo_fixed_link(phydev)) >> - return; >> - >> - ctl = lan9303_phy_read(ds, port, MII_BMCR); >> - >> - ctl &= ~BMCR_ANENABLE; >> - >> - if (phydev->speed == SPEED_100) >> - ctl |= BMCR_SPEED100; >> - else if (phydev->speed == SPEED_10) >> - ctl &= ~BMCR_SPEED100; >> - else >> - dev_err(ds->dev, "unsupported speed: %d\n", phydev->speed); >> - >> - if (phydev->duplex == DUPLEX_FULL) >> - ctl |= BMCR_FULLDPLX; >> - else >> - ctl &= ~BMCR_FULLDPLX; >> - >> - lan9303_phy_write(ds, port, MII_BMCR, ctl); >> - >> - if (port == chip->phy_addr_base) { >> - /* Virtual Phy: Remove Turbo 200Mbit mode */ >> - lan9303_read(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, &ctl); >> - >> - ctl &= ~LAN9303_VIRT_SPECIAL_TURBO; >> - regmap_write(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, ctl); >> - } >> -} > >Is this functionality no longer necessary? For example, I don't see >anywhere else in the driver that this turbo mode is disabled. > >I'm guessing the above code writing MII_BMCR is to force the >configuration of integrated PHYs to be the fixed-link settings? >How is that dealt with after the removal of the above code? > While it should be disabled by the HW config strap settings, I'll add disabling Turbo Mode into the initialization sequence to keep the driver operation consistent. >> - >> static int lan9303_port_enable(struct dsa_switch *ds, int port, >> struct phy_device *phy) >> { >> @@ -1279,6 +1243,41 @@ static int lan9303_port_mdb_del(struct dsa_switch *ds, int port, >> return 0; >> } >> >> +static void lan9303_phylink_get_caps(struct dsa_switch *ds, int port, >> + struct phylink_config *config) >> +{ >> + struct lan9303 *chip = ds->priv; >> + >> + dev_dbg(chip->dev, "%s(%d) entered.", __func__, port); >> + >> + config->mac_capabilities = MAC_10 | MAC_100 | MAC_ASYM_PAUSE | >> + MAC_SYM_PAUSE; >> + >> + if (dsa_port_is_cpu(dsa_to_port(ds, port))) { >> + /* cpu port */ >> + phy_interface_empty(config->supported_interfaces); > >This should not be necessary - the supported_interfaces member should >already be zero. > >Thanks. > Yes, I agree. I'll remove it. Regards, J.
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index baa336bb9d15..2baf6ae7e0a0 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -1047,42 +1047,6 @@ static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum, return chip->ops->phy_write(chip, phy, regnum, val); } -static void lan9303_adjust_link(struct dsa_switch *ds, int port, - struct phy_device *phydev) -{ - struct lan9303 *chip = ds->priv; - int ctl; - - if (!phy_is_pseudo_fixed_link(phydev)) - return; - - ctl = lan9303_phy_read(ds, port, MII_BMCR); - - ctl &= ~BMCR_ANENABLE; - - if (phydev->speed == SPEED_100) - ctl |= BMCR_SPEED100; - else if (phydev->speed == SPEED_10) - ctl &= ~BMCR_SPEED100; - else - dev_err(ds->dev, "unsupported speed: %d\n", phydev->speed); - - if (phydev->duplex == DUPLEX_FULL) - ctl |= BMCR_FULLDPLX; - else - ctl &= ~BMCR_FULLDPLX; - - lan9303_phy_write(ds, port, MII_BMCR, ctl); - - if (port == chip->phy_addr_base) { - /* Virtual Phy: Remove Turbo 200Mbit mode */ - lan9303_read(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, &ctl); - - ctl &= ~LAN9303_VIRT_SPECIAL_TURBO; - regmap_write(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, ctl); - } -} - static int lan9303_port_enable(struct dsa_switch *ds, int port, struct phy_device *phy) { @@ -1279,6 +1243,41 @@ static int lan9303_port_mdb_del(struct dsa_switch *ds, int port, return 0; } +static void lan9303_phylink_get_caps(struct dsa_switch *ds, int port, + struct phylink_config *config) +{ + struct lan9303 *chip = ds->priv; + + dev_dbg(chip->dev, "%s(%d) entered.", __func__, port); + + config->mac_capabilities = MAC_10 | MAC_100 | MAC_ASYM_PAUSE | + MAC_SYM_PAUSE; + + if (dsa_port_is_cpu(dsa_to_port(ds, port))) { + /* cpu port */ + phy_interface_empty(config->supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_RMII, + config->supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_MII, + config->supported_interfaces); + } else { + /* internal ports */ + __set_bit(PHY_INTERFACE_MODE_INTERNAL, + config->supported_interfaces); + /* Compatibility for phylib's default interface type when the + * phy-mode property is absent + */ + __set_bit(PHY_INTERFACE_MODE_GMII, + config->supported_interfaces); + } + + /* This driver does not make use of the speed, duplex, pause or the + * advertisement in its mac_config, so it is safe to mark this driver + * as non-legacy. + */ + config->legacy_pre_march2020 = false; +} + /* For non-cpu ports, the max frame size is 1518. * The CPU port supports a max frame size of 1522. * There is a JUMBO flag to make the max size 2048, but this driver @@ -1304,7 +1303,7 @@ static const struct dsa_switch_ops lan9303_switch_ops = { .get_strings = lan9303_get_strings, .phy_read = lan9303_phy_read, .phy_write = lan9303_phy_write, - .adjust_link = lan9303_adjust_link, + .phylink_get_caps = lan9303_phylink_get_caps, .get_ethtool_stats = lan9303_get_ethtool_stats, .get_sset_count = lan9303_get_sset_count, .port_enable = lan9303_port_enable,
This patch replaces the .adjust_link api with the .phylink_get_caps api. Signed-off-by: Jerry Ray <jerry.ray@microchip.com> --- drivers/net/dsa/lan9303-core.c | 73 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 37 deletions(-)