| Message ID | 20190221095128.28188-2-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: phy: marvell10g: Add 2.5GBaseT | expand |
On Thu, Feb 21, 2019 at 10:51:22AM +0100, Maxime Chevallier wrote: > The Alaska family of 10G PHYs has more abilities than the ones listed in > PHY_10GBIT_FULL_FEATURES, the exact list depending on the model. > > Make use of the newly introduced .get_features call to build this list, > using genphy_c45_pma_read_abilities to build the list of supported > linkmodes, and adding autoneg ability based on what's reported by the AN > MMD. > > .config_init is still used to validate the interface_mode. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > drivers/net/phy/marvell10g.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c > index 9ea27acf05ad..65ef469adf58 100644 > --- a/drivers/net/phy/marvell10g.c > +++ b/drivers/net/phy/marvell10g.c > @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev) > > static int mv3310_config_init(struct phy_device *phydev) > { > - int ret, val; > - > /* Check that the PHY interface type is compatible */ > if (phydev->interface != PHY_INTERFACE_MODE_SGMII && > phydev->interface != PHY_INTERFACE_MODE_XAUI && > @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev) > phydev->interface != PHY_INTERFACE_MODE_10GKR) > return -ENODEV; > > + return 0; > +} > + > +static int mv3310_get_features(struct phy_device *phydev) > +{ > + int ret, val; Please try to keep the formatting/style consistent in the file you are editing. A blank line here would do that. Thanks. > if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) { > val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > if (val < 0) > @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = { > .phy_id = 0x002b09aa, > .phy_id_mask = MARVELL_PHY_ID_MASK, > .name = "mv88x3310", > - .features = PHY_10GBIT_FEATURES, > + .get_features = mv3310_get_features, > .soft_reset = gen10g_no_soft_reset, > .config_init = mv3310_config_init, > .probe = mv3310_probe, > -- > 2.19.2 > >
Hello Russell, On Thu, 21 Feb 2019 10:22:15 +0000 Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: >> + return 0; >> +} >> + >> +static int mv3310_get_features(struct phy_device *phydev) >> +{ >> + int ret, val; > >Please try to keep the formatting/style consistent in the file you are >editing. A blank line here would do that. Thanks. Sorry, I missed that (and so did checkpatch apparently). I'll send a V2. Thanks, Maxime
On 21.02.2019 10:51, Maxime Chevallier wrote: > The Alaska family of 10G PHYs has more abilities than the ones listed in > PHY_10GBIT_FULL_FEATURES, the exact list depending on the model. > > Make use of the newly introduced .get_features call to build this list, > using genphy_c45_pma_read_abilities to build the list of supported > linkmodes, and adding autoneg ability based on what's reported by the AN > MMD. > > .config_init is still used to validate the interface_mode. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > drivers/net/phy/marvell10g.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c > index 9ea27acf05ad..65ef469adf58 100644 > --- a/drivers/net/phy/marvell10g.c > +++ b/drivers/net/phy/marvell10g.c > @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev) > > static int mv3310_config_init(struct phy_device *phydev) > { > - int ret, val; > - > /* Check that the PHY interface type is compatible */ > if (phydev->interface != PHY_INTERFACE_MODE_SGMII && > phydev->interface != PHY_INTERFACE_MODE_XAUI && > @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev) > phydev->interface != PHY_INTERFACE_MODE_10GKR) > return -ENODEV; > > + return 0; > +} > + > +static int mv3310_get_features(struct phy_device *phydev) > +{ After my just submitted patch to include the aneg capability checking in genphy_c45_pma_read_abilities() function mv3310_get_features() isn't needed any longer and can be replaced with the generic one. But we can make this change afterwards, then you don't have to rework your series. Also I'm not sure whether there will be a 5.0-rc8 or whether beginning of next week we'll see 5.0. In the latter case we're a little bit in a hurry because the merge window will start very soon. > + int ret, val; > if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) { > val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > if (val < 0) > @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = { > .phy_id = 0x002b09aa, > .phy_id_mask = MARVELL_PHY_ID_MASK, > .name = "mv88x3310", > - .features = PHY_10GBIT_FEATURES, > + .get_features = mv3310_get_features, > .soft_reset = gen10g_no_soft_reset, > .config_init = mv3310_config_init, > .probe = mv3310_probe, >
On Fri, 22 Feb 2019 19:42:29 +0100 Heiner Kallweit <hkallweit1@gmail.com> wrote: >After my just submitted patch to include the aneg capability checking in >genphy_c45_pma_read_abilities() function mv3310_get_features() isn't >needed any longer and can be replaced with the generic one. I'll still need it to handle the 2.5G/5G abilities that aren't correctly reported on 3310. I'll be able to use the generic function for the 2110 though, which is nice. >But we can make this change afterwards, then you don't have to >rework your series. > >Also I'm not sure whether there will be a 5.0-rc8 or whether beginning >of next week we'll see 5.0. In the latter case we're a little bit in a >hurry because the merge window will start very soon. OK, I'll re-spin the series quickly with the small cleanup needed if everything's OK to you regarding the rest of it. Thanks, Maxime
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c index 9ea27acf05ad..65ef469adf58 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev) static int mv3310_config_init(struct phy_device *phydev) { - int ret, val; - /* Check that the PHY interface type is compatible */ if (phydev->interface != PHY_INTERFACE_MODE_SGMII && phydev->interface != PHY_INTERFACE_MODE_XAUI && @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev) phydev->interface != PHY_INTERFACE_MODE_10GKR) return -ENODEV; + return 0; +} + +static int mv3310_get_features(struct phy_device *phydev) +{ + int ret, val; if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) { val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); if (val < 0) @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = { .phy_id = 0x002b09aa, .phy_id_mask = MARVELL_PHY_ID_MASK, .name = "mv88x3310", - .features = PHY_10GBIT_FEATURES, + .get_features = mv3310_get_features, .soft_reset = gen10g_no_soft_reset, .config_init = mv3310_config_init, .probe = mv3310_probe,
The Alaska family of 10G PHYs has more abilities than the ones listed in PHY_10GBIT_FULL_FEATURES, the exact list depending on the model. Make use of the newly introduced .get_features call to build this list, using genphy_c45_pma_read_abilities to build the list of supported linkmodes, and adding autoneg ability based on what's reported by the AN MMD. .config_init is still used to validate the interface_mode. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- drivers/net/phy/marvell10g.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)