| Message ID | 20190815153209.21529-2-christian.herber@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add BASE-T1 PHY support | expand |
On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote: > BASE-T1 is a category of Ethernet PHYs. > They use a single copper pair for transmission. > This patch add basic support for this category of PHYs. > It coveres the discovery of abilities and basic configuration. > It includes setting fixed speed and enabling auto-negotiation. > BASE-T1 devices should always Clause-45 managed. > Therefore, this patch extends phy-c45.c. > While for some functions like auto-neogtiation different registers are > used, the layout of these registers is the same for the used fields. > Thus, much of the logic of basic Clause-45 devices can be reused. > > Signed-off-by: Christian Herber <christian.herber@nxp.com> > --- > drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- > drivers/net/phy/phy-core.c | 4 +- > include/uapi/linux/ethtool.h | 2 + > include/uapi/linux/mdio.h | 21 +++++++ > 4 files changed, 129 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index b9d4145781ca..9ff0b8c785de 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -8,13 +8,23 @@ > #include <linux/mii.h> > #include <linux/phy.h> > > +#define IS_100BASET1(phy) (linkmode_test_bit( \ > + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ > + (phy)->supported)) > +#define IS_1000BASET1(phy) (linkmode_test_bit( \ > + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ > + (phy)->supported)) Hi Christian We already have the flag phydev->is_gigabit_capable. Maybe add a flag phydev->is_t1_capable > + > +static u32 get_aneg_ctrl(struct phy_device *phydev); > +static u32 get_aneg_stat(struct phy_device *phydev); No forward declarations please. Put the code in the right order so they are not needed. Thanks Andrew
On 15.08.2019 17:56, Andrew Lunn wrote: > On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote: >> BASE-T1 is a category of Ethernet PHYs. >> They use a single copper pair for transmission. >> This patch add basic support for this category of PHYs. >> It coveres the discovery of abilities and basic configuration. >> It includes setting fixed speed and enabling auto-negotiation. >> BASE-T1 devices should always Clause-45 managed. >> Therefore, this patch extends phy-c45.c. >> While for some functions like auto-neogtiation different registers are >> used, the layout of these registers is the same for the used fields. >> Thus, much of the logic of basic Clause-45 devices can be reused. >> >> Signed-off-by: Christian Herber <christian.herber@nxp.com> >> --- >> drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- >> drivers/net/phy/phy-core.c | 4 +- >> include/uapi/linux/ethtool.h | 2 + >> include/uapi/linux/mdio.h | 21 +++++++ >> 4 files changed, 129 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >> index b9d4145781ca..9ff0b8c785de 100644 >> --- a/drivers/net/phy/phy-c45.c >> +++ b/drivers/net/phy/phy-c45.c >> @@ -8,13 +8,23 @@ >> #include <linux/mii.h> >> #include <linux/phy.h> >> >> +#define IS_100BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ >> + (phy)->supported)) >> +#define IS_1000BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ >> + (phy)->supported)) > > Hi Christian > > We already have the flag phydev->is_gigabit_capable. Maybe add a flag > phydev->is_t1_capable > >> + >> +static u32 get_aneg_ctrl(struct phy_device *phydev); >> +static u32 get_aneg_stat(struct phy_device *phydev); > > No forward declarations please. Put the code in the right order so > they are not needed. > > Thanks > > Andrew > For whatever reason I don't have the original mail in my netdev inbox (yet). + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) + ctrl = MDIO_AN_BT1_CTRL; Code like this could be problematic once a PHY supports one of the T1 modes AND normal modes. Then normal modes would be unusable. I think this scenario isn't completely hypothetical. See the Aquantia AQCS109 that supports normal modes and (proprietary) 1000Base-T2. Maybe we need separate versions of the generic functions for T1. Then it would be up to the PHY driver to decide when to use which version. Heiner
On 15.08.2019 17:56, Andrew Lunn wrote: > Caution: EXT Email > > On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote: >> BASE-T1 is a category of Ethernet PHYs. >> They use a single copper pair for transmission. >> This patch add basic support for this category of PHYs. >> It coveres the discovery of abilities and basic configuration. >> It includes setting fixed speed and enabling auto-negotiation. >> BASE-T1 devices should always Clause-45 managed. >> Therefore, this patch extends phy-c45.c. >> While for some functions like auto-neogtiation different registers are >> used, the layout of these registers is the same for the used fields. >> Thus, much of the logic of basic Clause-45 devices can be reused. >> >> Signed-off-by: Christian Herber <christian.herber@nxp.com> >> --- >> drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- >> drivers/net/phy/phy-core.c | 4 +- >> include/uapi/linux/ethtool.h | 2 + >> include/uapi/linux/mdio.h | 21 +++++++ >> 4 files changed, 129 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >> index b9d4145781ca..9ff0b8c785de 100644 >> --- a/drivers/net/phy/phy-c45.c >> +++ b/drivers/net/phy/phy-c45.c >> @@ -8,13 +8,23 @@ >> #include <linux/mii.h> >> #include <linux/phy.h> >> >> +#define IS_100BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ >> + (phy)->supported)) >> +#define IS_1000BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ >> + (phy)->supported)) > > Hi Christian > > We already have the flag phydev->is_gigabit_capable. Maybe add a flag > phydev->is_t1_capable > >> + >> +static u32 get_aneg_ctrl(struct phy_device *phydev); >> +static u32 get_aneg_stat(struct phy_device *phydev); > > No forward declarations please. Put the code in the right order so > they are not needed. > > Thanks > > Andrew > Hi Andrew, thanks for feedback. The use of an additional flag is a good proposal. I was hesitant to touch the phydev structure. I will add this along with removing the forward declaration in v2. Regards, Christian
On 15.08.2019 18:34, Heiner Kallweit wrote: > Caution: EXT Email > > On 15.08.2019 17:56, Andrew Lunn wrote: >> On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote: >>> BASE-T1 is a category of Ethernet PHYs. >>> They use a single copper pair for transmission. >>> This patch add basic support for this category of PHYs. >>> It coveres the discovery of abilities and basic configuration. >>> It includes setting fixed speed and enabling auto-negotiation. >>> BASE-T1 devices should always Clause-45 managed. >>> Therefore, this patch extends phy-c45.c. >>> While for some functions like auto-neogtiation different registers are >>> used, the layout of these registers is the same for the used fields. >>> Thus, much of the logic of basic Clause-45 devices can be reused. >>> >>> Signed-off-by: Christian Herber <christian.herber@nxp.com> >>> --- >>> drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- >>> drivers/net/phy/phy-core.c | 4 +- >>> include/uapi/linux/ethtool.h | 2 + >>> include/uapi/linux/mdio.h | 21 +++++++ >>> 4 files changed, 129 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >>> index b9d4145781ca..9ff0b8c785de 100644 >>> --- a/drivers/net/phy/phy-c45.c >>> +++ b/drivers/net/phy/phy-c45.c >>> @@ -8,13 +8,23 @@ >>> #include <linux/mii.h> >>> #include <linux/phy.h> >>> >>> +#define IS_100BASET1(phy) (linkmode_test_bit( \ >>> + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ >>> + (phy)->supported)) >>> +#define IS_1000BASET1(phy) (linkmode_test_bit( \ >>> + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ >>> + (phy)->supported)) >> >> Hi Christian >> >> We already have the flag phydev->is_gigabit_capable. Maybe add a flag >> phydev->is_t1_capable >> >>> + >>> +static u32 get_aneg_ctrl(struct phy_device *phydev); >>> +static u32 get_aneg_stat(struct phy_device *phydev); >> >> No forward declarations please. Put the code in the right order so >> they are not needed. >> >> Thanks >> >> Andrew >> > > For whatever reason I don't have the original mail in my netdev inbox (yet). > > + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) > + ctrl = MDIO_AN_BT1_CTRL; > > Code like this could be problematic once a PHY supports one of the T1 modes > AND normal modes. Then normal modes would be unusable. > > I think this scenario isn't completely hypothetical. See the Aquantia > AQCS109 that supports normal modes and (proprietary) 1000Base-T2. > > Maybe we need separate versions of the generic functions for T1. > Then it would be up to the PHY driver to decide when to use which > version. > > Heiner > Integrating this with the existing driver or creating a new on is an interesting question. I came to the conclusion that it is most efficient to integrate to avoid all to much copy paste code. So far, I am not aware of any device that supports T1 and something else at the same time. From a HW perspective, I also consider this quite unlikely. In the unlikely case that such a device comes up, support from the genphy driver would be limited to the BASE-T1 modes. But i would rather create the special case for the special device and cater the current mainstream support to the mainstream devices. I think this boils down to a general strategy for the PHY framework, as this can happen for other classes of devices also like NGBASE-T1, MultiGBASE-T and future unknown devices. For now, I think the architecture is sufficiently scalable with a single c45 genphy driver Christian
On 15.08.2019 17:32, Christian Herber wrote: > BASE-T1 is a category of Ethernet PHYs. > They use a single copper pair for transmission. > This patch add basic support for this category of PHYs. > It coveres the discovery of abilities and basic configuration. > It includes setting fixed speed and enabling auto-negotiation. > BASE-T1 devices should always Clause-45 managed. > Therefore, this patch extends phy-c45.c. > While for some functions like auto-neogtiation different registers are > used, the layout of these registers is the same for the used fields. > Thus, much of the logic of basic Clause-45 devices can be reused. > > Signed-off-by: Christian Herber <christian.herber@nxp.com> > --- > drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- > drivers/net/phy/phy-core.c | 4 +- > include/uapi/linux/ethtool.h | 2 + > include/uapi/linux/mdio.h | 21 +++++++ > 4 files changed, 129 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index b9d4145781ca..9ff0b8c785de 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -8,13 +8,23 @@ > #include <linux/mii.h> > #include <linux/phy.h> > > +#define IS_100BASET1(phy) (linkmode_test_bit( \ > + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ > + (phy)->supported)) > +#define IS_1000BASET1(phy) (linkmode_test_bit( \ > + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ > + (phy)->supported)) > + Why is there no such macro for 10BaseT1? > +static u32 get_aneg_ctrl(struct phy_device *phydev); > +static u32 get_aneg_stat(struct phy_device *phydev); > + > /** > * genphy_c45_setup_forced - configures a forced speed > * @phydev: target phy_device struct > */ > int genphy_c45_pma_setup_forced(struct phy_device *phydev) > { > - int ctrl1, ctrl2, ret; > + int ctrl1, ctrl2, base_t1_ctrl = 0, ret; > > /* Half duplex is not supported */ > if (phydev->duplex != DUPLEX_FULL) > @@ -28,6 +38,16 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) > if (ctrl2 < 0) > return ctrl2; > > + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { 10BaseT1 doesn't need to be considered here? And maybe it would be better to have a flag for BaseT1 at phy_device level (based on bit MDIO_PMA_EXTABLE_BASET1?) instead of checking whether certain T1 modes are supported. Then we would be more future-proof in case of new T1 modes. > + base_t1_ctrl = phy_read_mmd(phydev, > + MDIO_MMD_PMAPMD, > + MDIO_PMA_BASET1CTRL); > + if (base_t1_ctrl < 0) > + return base_t1_ctrl; > + > + base_t1_ctrl &= ~(MDIO_PMA_BASET1CTRL_TYPE); > + } > + > ctrl1 &= ~MDIO_CTRL1_SPEEDSEL; > /* > * PMA/PMD type selection is 1.7.5:0 not 1.7.3:0. See 45.2.1.6.1 > @@ -41,12 +61,21 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) > break; > case SPEED_100: > ctrl1 |= MDIO_PMA_CTRL1_SPEED100; > - ctrl2 |= MDIO_PMA_CTRL2_100BTX; > + if (IS_100BASET1(phydev)) { > + ctrl2 |= MDIO_PMA_CTRL2_BT1; > + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_100BT1; > + } else { > + ctrl2 |= MDIO_PMA_CTRL2_100BTX; > + } > break; > case SPEED_1000: > ctrl1 |= MDIO_PMA_CTRL1_SPEED1000; > - /* Assume 1000base-T */ > - ctrl2 |= MDIO_PMA_CTRL2_1000BT; > + if (IS_1000BASET1(phydev)) { > + ctrl2 |= MDIO_PMA_CTRL2_BT1; > + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_1000BT1; > + } else { > + ctrl2 |= MDIO_PMA_CTRL2_1000BT; > + } > break; > case SPEED_2500: > ctrl1 |= MDIO_CTRL1_SPEED2_5G; > @@ -75,6 +104,14 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) > if (ret < 0) > return ret; > > + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { > + ret = phy_write_mmd(phydev, > + MDIO_MMD_PMAPMD, > + MDIO_PMA_BASET1CTRL, > + base_t1_ctrl); > + if (ret < 0) > + return ret; > + } > return genphy_c45_an_disable_aneg(phydev); > } > EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced); > @@ -135,8 +172,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_config_aneg); > */ > int genphy_c45_an_disable_aneg(struct phy_device *phydev) > { > - > - return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, > + return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), > MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); > } > EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); > @@ -151,7 +187,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); > */ > int genphy_c45_restart_aneg(struct phy_device *phydev) > { > - return phy_set_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, > + return phy_set_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), > MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); > } > EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg); > @@ -171,7 +207,7 @@ int genphy_c45_check_and_restart_aneg(struct phy_device *phydev, bool restart) > > if (!restart) { > /* Configure and restart aneg if it wasn't set before */ > - ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1); > + ret = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev)); > if (ret < 0) > return ret; > > @@ -199,7 +235,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_check_and_restart_aneg); > */ > int genphy_c45_aneg_done(struct phy_device *phydev) > { > - int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > + int val = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_stat(phydev)); > > return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0; > } > @@ -385,7 +421,9 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); > * PMA Extended Abilities (1.11) register, indicating 1000BASET an 10G related > * modes. If bit 1.11.14 is set, then the list is also extended with the modes > * in the 2.5G/5G PMA Extended register (1.21), indicating if 2.5GBASET and > - * 5GBASET are supported. > + * 5GBASET are supported. If bit 1.11.11 is set, then the list is also extended > + * with the modes in the BASE-T1 PMA Extended register (1.18), indicating if > + * 10/100/1000BASET-1 are supported. > */ > int genphy_c45_pma_read_abilities(struct phy_device *phydev) > { > @@ -470,6 +508,29 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev) > phydev->supported, > val & MDIO_PMA_NG_EXTABLE_5GBT); > } > + > + if (val & MDIO_PMA_EXTABLE_BASET1) { > + val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, > + MDIO_PMA_BASET1_EXTABLE); > + if (val < 0) > + return val; > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT, > + phydev->supported, > + val & MDIO_PMA_BASET1_EXTABLE_100BT1); > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, > + phydev->supported, > + val & MDIO_PMA_BASET1_EXTABLE_1000BT1); > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, > + phydev->supported, > + val & MDIO_PMA_BASET1_EXTABLE_10BT1L); > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, > + phydev->supported, > + val & MDIO_PMA_BASET1_EXTABLE_10BT1S); > + } > } > > return 0; > @@ -509,6 +570,38 @@ int genphy_c45_read_status(struct phy_device *phydev) > } > EXPORT_SYMBOL_GPL(genphy_c45_read_status); > > +/** > + * get_aneg_ctrl - Get the register address for auto- > + * negotiation control register > + * @phydev: target phy_device struct > + * > + */ > +static u32 get_aneg_ctrl(struct phy_device *phydev) > +{ > + u32 ctrl = MDIO_CTRL1; > + > + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) > + ctrl = MDIO_AN_BT1_CTRL; > + AFAICS 10BaseT1 has separate aneg registers (526/527). To be considered here? > + return ctrl; > +} > + > +/** > + * get_aneg_ctrl - Get the register address for auto- > + * negotiation status register > + * @phydev: target phy_device struct > + * > + */ > +static u32 get_aneg_stat(struct phy_device *phydev) > +{ > + u32 stat = MDIO_STAT1; > + > + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) > + stat = MDIO_AN_BT1_STAT; > + > + return stat; > +} > + > /* The gen10g_* functions are the old Clause 45 stub */ > > int gen10g_config_aneg(struct phy_device *phydev) > diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c > index 369903d9b6ec..b50576f7709a 100644 > --- a/drivers/net/phy/phy-core.c > +++ b/drivers/net/phy/phy-core.c > @@ -8,7 +8,7 @@ > > const char *phy_speed_to_str(int speed) > { > - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 69, > + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 71, > "Enum ethtool_link_mode_bit_indices and phylib are out of sync. " > "If a speed or mode has been added please update phy_speed_to_str " > "and the PHY settings array.\n"); > @@ -140,6 +140,8 @@ static const struct phy_setting settings[] = { > /* 10M */ > PHY_SETTING( 10, FULL, 10baseT_Full ), > PHY_SETTING( 10, HALF, 10baseT_Half ), > + PHY_SETTING( 10, FULL, 10baseT1L_Full ), > + PHY_SETTING( 10, FULL, 10baseT1S_Full ), > }; > #undef PHY_SETTING > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index dd06302aa93e..e429cc8da31a 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -1485,6 +1485,8 @@ enum ethtool_link_mode_bit_indices { > ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66, > ETHTOOL_LINK_MODE_100baseT1_Full_BIT = 67, > ETHTOOL_LINK_MODE_1000baseT1_Full_BIT = 68, > + ETHTOOL_LINK_MODE_10baseT1L_Full_BIT = 69, > + ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 70, > > /* must be last entry */ > __ETHTOOL_LINK_MODE_MASK_NBITS > diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h > index 0a552061ff1c..6fd5ff632b8e 100644 > --- a/include/uapi/linux/mdio.h > +++ b/include/uapi/linux/mdio.h > @@ -43,6 +43,7 @@ > #define MDIO_PKGID1 14 /* Package identifier */ > #define MDIO_PKGID2 15 > #define MDIO_AN_ADVERTISE 16 /* AN advertising (base page) */ > +#define MDIO_PMA_BASET1_EXTABLE 18 /* BASE-T1 PMA/PMD extended ability */ > #define MDIO_AN_LPA 19 /* AN LP abilities (base page) */ > #define MDIO_PCS_EEE_ABLE 20 /* EEE Capability register */ > #define MDIO_PMA_NG_EXTABLE 21 /* 2.5G/5G PMA/PMD extended ability */ > @@ -57,11 +58,16 @@ > #define MDIO_PMA_10GBT_SNR 133 /* 10GBASE-T SNR margin, lane A. > * Lanes B-D are numbered 134-136. */ > #define MDIO_PMA_10GBR_FECABLE 170 /* 10GBASE-R FEC ability */ > +#define MDIO_PMA_BASET1CTRL 2100 /* BASE-T1 PMA/PMD control */ > #define MDIO_PCS_10GBX_STAT1 24 /* 10GBASE-X PCS status 1 */ > #define MDIO_PCS_10GBRT_STAT1 32 /* 10GBASE-R/-T PCS status 1 */ > #define MDIO_PCS_10GBRT_STAT2 33 /* 10GBASE-R/-T PCS status 2 */ > #define MDIO_AN_10GBT_CTRL 32 /* 10GBASE-T auto-negotiation control */ > #define MDIO_AN_10GBT_STAT 33 /* 10GBASE-T auto-negotiation status */ > +#define MDIO_AN_BT1_CTRL 512 /* BASE-T1 auto-negotiation control */ > +#define MDIO_AN_BT1_STAT 513 /* BASE-T1 auto-negotiation status */ > +#define MDIO_AN_10BT1_CTRL 526 /* 10BASE-T1 auto-negotiation control */ > +#define MDIO_AN_10BT1_STAT 527 /* 10BASE-T1 auto-negotiation status */ > > /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */ > #define MDIO_PMA_LASI_RXCTRL 0x9000 /* RX_ALARM control */ > @@ -151,6 +157,7 @@ > #define MDIO_PMA_CTRL2_100BTX 0x000e /* 100BASE-TX type */ > #define MDIO_PMA_CTRL2_10BT 0x000f /* 10BASE-T type */ > #define MDIO_PMA_CTRL2_2_5GBT 0x0030 /* 2.5GBaseT type */ > +#define MDIO_PMA_CTRL2_BT1 0x003D /* BASE-T1 type */ > #define MDIO_PMA_CTRL2_5GBT 0x0031 /* 5GBaseT type */ > #define MDIO_PCS_CTRL2_TYPE 0x0003 /* PCS type selection */ > #define MDIO_PCS_CTRL2_10GBR 0x0000 /* 10GBASE-R type */ > @@ -205,8 +212,16 @@ > #define MDIO_PMA_EXTABLE_1000BKX 0x0040 /* 1000BASE-KX ability */ > #define MDIO_PMA_EXTABLE_100BTX 0x0080 /* 100BASE-TX ability */ > #define MDIO_PMA_EXTABLE_10BT 0x0100 /* 10BASE-T ability */ > +#define MDIO_PMA_EXTABLE_BASET1 0x0800 /* BASE-T1 ability */ > #define MDIO_PMA_EXTABLE_NBT 0x4000 /* 2.5/5GBASE-T ability */ > > +/* PMA BASE-T1 control register. */ > +#define MDIO_PMA_BASET1CTRL_TYPE 0x000f /* PMA/PMD BASE-T1 type sel. */ > +#define MDIO_PMA_BASET1CTRL_TYPE_100BT1 0x0000 /* 100BASE-T1 */ > +#define MDIO_PMA_BASET1CTRL_TYPE_1000BT1 0x0001 /* 1000BASE-T1 */ > +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1L 0x0002 /* 10BASE-T1L */ > +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1S 0x0003 /* 10BASE-T1S */ > + > /* PHY XGXS lane state register. */ > #define MDIO_PHYXS_LNSTAT_SYNC0 0x0001 > #define MDIO_PHYXS_LNSTAT_SYNC1 0x0002 > @@ -281,6 +296,12 @@ > #define MDIO_PMA_NG_EXTABLE_2_5GBT 0x0001 /* 2.5GBASET ability */ > #define MDIO_PMA_NG_EXTABLE_5GBT 0x0002 /* 5GBASET ability */ > > +/* BASE-T1 Extended abilities register. */ > +#define MDIO_PMA_BASET1_EXTABLE_100BT1 0x0001 /* 100BASE-T1 ability */ > +#define MDIO_PMA_BASET1_EXTABLE_1000BT1 0x0002 /* 1000BASE-T1 ability */ > +#define MDIO_PMA_BASET1_EXTABLE_10BT1L 0x0004 /* 10BASE-T1L ability */ > +#define MDIO_PMA_BASET1_EXTABLE_10BT1S 0x0008 /* 10BASE-T1S ability */ > + > /* LASI RX_ALARM control/status registers. */ > #define MDIO_PMA_LASI_RX_PHYXSLFLT 0x0001 /* PHY XS RX local fault */ > #define MDIO_PMA_LASI_RX_PCSLFLT 0x0008 /* PCS RX local fault */ >
On 16.08.2019 23:13, Heiner Kallweit wrote: > > On 15.08.2019 17:32, Christian Herber wrote: >> BASE-T1 is a category of Ethernet PHYs. >> They use a single copper pair for transmission. >> This patch add basic support for this category of PHYs. >> It coveres the discovery of abilities and basic configuration. >> It includes setting fixed speed and enabling auto-negotiation. >> BASE-T1 devices should always Clause-45 managed. >> Therefore, this patch extends phy-c45.c. >> While for some functions like auto-neogtiation different registers are >> used, the layout of these registers is the same for the used fields. >> Thus, much of the logic of basic Clause-45 devices can be reused. >> >> Signed-off-by: Christian Herber <christian.herber@nxp.com> >> --- >> drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- >> drivers/net/phy/phy-core.c | 4 +- >> include/uapi/linux/ethtool.h | 2 + >> include/uapi/linux/mdio.h | 21 +++++++ >> 4 files changed, 129 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c >> index b9d4145781ca..9ff0b8c785de 100644 >> --- a/drivers/net/phy/phy-c45.c >> +++ b/drivers/net/phy/phy-c45.c >> @@ -8,13 +8,23 @@ >> #include <linux/mii.h> >> #include <linux/phy.h> >> >> +#define IS_100BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ >> + (phy)->supported)) >> +#define IS_1000BASET1(phy) (linkmode_test_bit( \ >> + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ >> + (phy)->supported)) >> + > Why is there no such macro for 10BaseT1? > >> +static u32 get_aneg_ctrl(struct phy_device *phydev); >> +static u32 get_aneg_stat(struct phy_device *phydev); >> + >> /** >> * genphy_c45_setup_forced - configures a forced speed >> * @phydev: target phy_device struct >> */ >> int genphy_c45_pma_setup_forced(struct phy_device *phydev) >> { >> - int ctrl1, ctrl2, ret; >> + int ctrl1, ctrl2, base_t1_ctrl = 0, ret; >> >> /* Half duplex is not supported */ >> if (phydev->duplex != DUPLEX_FULL) >> @@ -28,6 +38,16 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) >> if (ctrl2 < 0) >> return ctrl2; >> >> + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { > > 10BaseT1 doesn't need to be considered here? > And maybe it would be better to have a flag for BaseT1 at phy_device level > (based on bit MDIO_PMA_EXTABLE_BASET1?) instead of checking whether certain > T1 modes are supported. Then we would be more future-proof in case of new > T1 modes. > >> + base_t1_ctrl = phy_read_mmd(phydev, >> + MDIO_MMD_PMAPMD, >> + MDIO_PMA_BASET1CTRL); >> + if (base_t1_ctrl < 0) >> + return base_t1_ctrl; >> + >> + base_t1_ctrl &= ~(MDIO_PMA_BASET1CTRL_TYPE); >> + } >> + >> ctrl1 &= ~MDIO_CTRL1_SPEEDSEL; >> /* >> * PMA/PMD type selection is 1.7.5:0 not 1.7.3:0. See 45.2.1.6.1 >> @@ -41,12 +61,21 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) >> break; >> case SPEED_100: >> ctrl1 |= MDIO_PMA_CTRL1_SPEED100; >> - ctrl2 |= MDIO_PMA_CTRL2_100BTX; >> + if (IS_100BASET1(phydev)) { >> + ctrl2 |= MDIO_PMA_CTRL2_BT1; >> + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_100BT1; >> + } else { >> + ctrl2 |= MDIO_PMA_CTRL2_100BTX; >> + } >> break; >> case SPEED_1000: >> ctrl1 |= MDIO_PMA_CTRL1_SPEED1000; >> - /* Assume 1000base-T */ >> - ctrl2 |= MDIO_PMA_CTRL2_1000BT; >> + if (IS_1000BASET1(phydev)) { >> + ctrl2 |= MDIO_PMA_CTRL2_BT1; >> + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_1000BT1; >> + } else { >> + ctrl2 |= MDIO_PMA_CTRL2_1000BT; >> + } >> break; >> case SPEED_2500: >> ctrl1 |= MDIO_CTRL1_SPEED2_5G; >> @@ -75,6 +104,14 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) >> if (ret < 0) >> return ret; >> >> + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { >> + ret = phy_write_mmd(phydev, >> + MDIO_MMD_PMAPMD, >> + MDIO_PMA_BASET1CTRL, >> + base_t1_ctrl); >> + if (ret < 0) >> + return ret; >> + } >> return genphy_c45_an_disable_aneg(phydev); >> } >> EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced); >> @@ -135,8 +172,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_config_aneg); >> */ >> int genphy_c45_an_disable_aneg(struct phy_device *phydev) >> { >> - >> - return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, >> + return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), >> MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); >> } >> EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); >> @@ -151,7 +187,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); >> */ >> int genphy_c45_restart_aneg(struct phy_device *phydev) >> { >> - return phy_set_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, >> + return phy_set_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), >> MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); >> } >> EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg); >> @@ -171,7 +207,7 @@ int genphy_c45_check_and_restart_aneg(struct phy_device *phydev, bool restart) >> >> if (!restart) { >> /* Configure and restart aneg if it wasn't set before */ >> - ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1); >> + ret = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev)); >> if (ret < 0) >> return ret; >> >> @@ -199,7 +235,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_check_and_restart_aneg); >> */ >> int genphy_c45_aneg_done(struct phy_device *phydev) >> { >> - int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); >> + int val = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_stat(phydev)); >> >> return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0; >> } >> @@ -385,7 +421,9 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); >> * PMA Extended Abilities (1.11) register, indicating 1000BASET an 10G related >> * modes. If bit 1.11.14 is set, then the list is also extended with the modes >> * in the 2.5G/5G PMA Extended register (1.21), indicating if 2.5GBASET and >> - * 5GBASET are supported. >> + * 5GBASET are supported. If bit 1.11.11 is set, then the list is also extended >> + * with the modes in the BASE-T1 PMA Extended register (1.18), indicating if >> + * 10/100/1000BASET-1 are supported. >> */ >> int genphy_c45_pma_read_abilities(struct phy_device *phydev) >> { >> @@ -470,6 +508,29 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev) >> phydev->supported, >> val & MDIO_PMA_NG_EXTABLE_5GBT); >> } >> + >> + if (val & MDIO_PMA_EXTABLE_BASET1) { >> + val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, >> + MDIO_PMA_BASET1_EXTABLE); >> + if (val < 0) >> + return val; >> + >> + linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT, >> + phydev->supported, >> + val & MDIO_PMA_BASET1_EXTABLE_100BT1); >> + >> + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, >> + phydev->supported, >> + val & MDIO_PMA_BASET1_EXTABLE_1000BT1); >> + >> + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, >> + phydev->supported, >> + val & MDIO_PMA_BASET1_EXTABLE_10BT1L); >> + >> + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, >> + phydev->supported, >> + val & MDIO_PMA_BASET1_EXTABLE_10BT1S); >> + } >> } >> >> return 0; >> @@ -509,6 +570,38 @@ int genphy_c45_read_status(struct phy_device *phydev) >> } >> EXPORT_SYMBOL_GPL(genphy_c45_read_status); >> >> +/** >> + * get_aneg_ctrl - Get the register address for auto- >> + * negotiation control register >> + * @phydev: target phy_device struct >> + * >> + */ >> +static u32 get_aneg_ctrl(struct phy_device *phydev) >> +{ >> + u32 ctrl = MDIO_CTRL1; >> + >> + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) >> + ctrl = MDIO_AN_BT1_CTRL; >> + > AFAICS 10BaseT1 has separate aneg registers (526/527). > To be considered here? > >> + return ctrl; >> +} >> + >> +/** >> + * get_aneg_ctrl - Get the register address for auto- >> + * negotiation status register >> + * @phydev: target phy_device struct >> + * >> + */ >> +static u32 get_aneg_stat(struct phy_device *phydev) >> +{ >> + u32 stat = MDIO_STAT1; >> + >> + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) >> + stat = MDIO_AN_BT1_STAT; >> + >> + return stat; >> +} >> + >> /* The gen10g_* functions are the old Clause 45 stub */ >> >> int gen10g_config_aneg(struct phy_device *phydev) >> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c >> index 369903d9b6ec..b50576f7709a 100644 >> --- a/drivers/net/phy/phy-core.c >> +++ b/drivers/net/phy/phy-core.c >> @@ -8,7 +8,7 @@ >> >> const char *phy_speed_to_str(int speed) >> { >> - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 69, >> + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 71, >> "Enum ethtool_link_mode_bit_indices and phylib are out of sync. " >> "If a speed or mode has been added please update phy_speed_to_str " >> "and the PHY settings array.\n"); >> @@ -140,6 +140,8 @@ static const struct phy_setting settings[] = { >> /* 10M */ >> PHY_SETTING( 10, FULL, 10baseT_Full ), >> PHY_SETTING( 10, HALF, 10baseT_Half ), >> + PHY_SETTING( 10, FULL, 10baseT1L_Full ), >> + PHY_SETTING( 10, FULL, 10baseT1S_Full ), >> }; >> #undef PHY_SETTING >> >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h >> index dd06302aa93e..e429cc8da31a 100644 >> --- a/include/uapi/linux/ethtool.h >> +++ b/include/uapi/linux/ethtool.h >> @@ -1485,6 +1485,8 @@ enum ethtool_link_mode_bit_indices { >> ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66, >> ETHTOOL_LINK_MODE_100baseT1_Full_BIT = 67, >> ETHTOOL_LINK_MODE_1000baseT1_Full_BIT = 68, >> + ETHTOOL_LINK_MODE_10baseT1L_Full_BIT = 69, >> + ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 70, >> >> /* must be last entry */ >> __ETHTOOL_LINK_MODE_MASK_NBITS >> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h >> index 0a552061ff1c..6fd5ff632b8e 100644 >> --- a/include/uapi/linux/mdio.h >> +++ b/include/uapi/linux/mdio.h >> @@ -43,6 +43,7 @@ >> #define MDIO_PKGID1 14 /* Package identifier */ >> #define MDIO_PKGID2 15 >> #define MDIO_AN_ADVERTISE 16 /* AN advertising (base page) */ >> +#define MDIO_PMA_BASET1_EXTABLE 18 /* BASE-T1 PMA/PMD extended ability */ >> #define MDIO_AN_LPA 19 /* AN LP abilities (base page) */ >> #define MDIO_PCS_EEE_ABLE 20 /* EEE Capability register */ >> #define MDIO_PMA_NG_EXTABLE 21 /* 2.5G/5G PMA/PMD extended ability */ >> @@ -57,11 +58,16 @@ >> #define MDIO_PMA_10GBT_SNR 133 /* 10GBASE-T SNR margin, lane A. >> * Lanes B-D are numbered 134-136. */ >> #define MDIO_PMA_10GBR_FECABLE 170 /* 10GBASE-R FEC ability */ >> +#define MDIO_PMA_BASET1CTRL 2100 /* BASE-T1 PMA/PMD control */ >> #define MDIO_PCS_10GBX_STAT1 24 /* 10GBASE-X PCS status 1 */ >> #define MDIO_PCS_10GBRT_STAT1 32 /* 10GBASE-R/-T PCS status 1 */ >> #define MDIO_PCS_10GBRT_STAT2 33 /* 10GBASE-R/-T PCS status 2 */ >> #define MDIO_AN_10GBT_CTRL 32 /* 10GBASE-T auto-negotiation control */ >> #define MDIO_AN_10GBT_STAT 33 /* 10GBASE-T auto-negotiation status */ >> +#define MDIO_AN_BT1_CTRL 512 /* BASE-T1 auto-negotiation control */ >> +#define MDIO_AN_BT1_STAT 513 /* BASE-T1 auto-negotiation status */ >> +#define MDIO_AN_10BT1_CTRL 526 /* 10BASE-T1 auto-negotiation control */ >> +#define MDIO_AN_10BT1_STAT 527 /* 10BASE-T1 auto-negotiation status */ >> >> /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */ >> #define MDIO_PMA_LASI_RXCTRL 0x9000 /* RX_ALARM control */ >> @@ -151,6 +157,7 @@ >> #define MDIO_PMA_CTRL2_100BTX 0x000e /* 100BASE-TX type */ >> #define MDIO_PMA_CTRL2_10BT 0x000f /* 10BASE-T type */ >> #define MDIO_PMA_CTRL2_2_5GBT 0x0030 /* 2.5GBaseT type */ >> +#define MDIO_PMA_CTRL2_BT1 0x003D /* BASE-T1 type */ >> #define MDIO_PMA_CTRL2_5GBT 0x0031 /* 5GBaseT type */ >> #define MDIO_PCS_CTRL2_TYPE 0x0003 /* PCS type selection */ >> #define MDIO_PCS_CTRL2_10GBR 0x0000 /* 10GBASE-R type */ >> @@ -205,8 +212,16 @@ >> #define MDIO_PMA_EXTABLE_1000BKX 0x0040 /* 1000BASE-KX ability */ >> #define MDIO_PMA_EXTABLE_100BTX 0x0080 /* 100BASE-TX ability */ >> #define MDIO_PMA_EXTABLE_10BT 0x0100 /* 10BASE-T ability */ >> +#define MDIO_PMA_EXTABLE_BASET1 0x0800 /* BASE-T1 ability */ >> #define MDIO_PMA_EXTABLE_NBT 0x4000 /* 2.5/5GBASE-T ability */ >> >> +/* PMA BASE-T1 control register. */ >> +#define MDIO_PMA_BASET1CTRL_TYPE 0x000f /* PMA/PMD BASE-T1 type sel. */ >> +#define MDIO_PMA_BASET1CTRL_TYPE_100BT1 0x0000 /* 100BASE-T1 */ >> +#define MDIO_PMA_BASET1CTRL_TYPE_1000BT1 0x0001 /* 1000BASE-T1 */ >> +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1L 0x0002 /* 10BASE-T1L */ >> +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1S 0x0003 /* 10BASE-T1S */ >> + >> /* PHY XGXS lane state register. */ >> #define MDIO_PHYXS_LNSTAT_SYNC0 0x0001 >> #define MDIO_PHYXS_LNSTAT_SYNC1 0x0002 >> @@ -281,6 +296,12 @@ >> #define MDIO_PMA_NG_EXTABLE_2_5GBT 0x0001 /* 2.5GBASET ability */ >> #define MDIO_PMA_NG_EXTABLE_5GBT 0x0002 /* 5GBASET ability */ >> >> +/* BASE-T1 Extended abilities register. */ >> +#define MDIO_PMA_BASET1_EXTABLE_100BT1 0x0001 /* 100BASE-T1 ability */ >> +#define MDIO_PMA_BASET1_EXTABLE_1000BT1 0x0002 /* 1000BASE-T1 ability */ >> +#define MDIO_PMA_BASET1_EXTABLE_10BT1L 0x0004 /* 10BASE-T1L ability */ >> +#define MDIO_PMA_BASET1_EXTABLE_10BT1S 0x0008 /* 10BASE-T1S ability */ >> + >> /* LASI RX_ALARM control/status registers. */ >> #define MDIO_PMA_LASI_RX_PHYXSLFLT 0x0001 /* PHY XS RX local fault */ >> #define MDIO_PMA_LASI_RX_PCSLFLT 0x0008 /* PCS RX local fault */ >> > > I have already prepared my next patch with a global is_baset1_capable property in the phy device. It is intentional that I did not introduce IS_10BASET1 macro. I should have probably explained in the cover letter. Will do for v2. 100 and 1000BASE-T1 are already more mature than 10BASE-T1. For 10BASE-T1, the only support added right now is the detection of the capability, i.e. reporting to ethtool. I would suggest enabling more support for 10BASE-T1 once there is silicon available on a larger scale and usage is more clear.
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index b9d4145781ca..9ff0b8c785de 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -8,13 +8,23 @@ #include <linux/mii.h> #include <linux/phy.h> +#define IS_100BASET1(phy) (linkmode_test_bit( \ + ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \ + (phy)->supported)) +#define IS_1000BASET1(phy) (linkmode_test_bit( \ + ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \ + (phy)->supported)) + +static u32 get_aneg_ctrl(struct phy_device *phydev); +static u32 get_aneg_stat(struct phy_device *phydev); + /** * genphy_c45_setup_forced - configures a forced speed * @phydev: target phy_device struct */ int genphy_c45_pma_setup_forced(struct phy_device *phydev) { - int ctrl1, ctrl2, ret; + int ctrl1, ctrl2, base_t1_ctrl = 0, ret; /* Half duplex is not supported */ if (phydev->duplex != DUPLEX_FULL) @@ -28,6 +38,16 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) if (ctrl2 < 0) return ctrl2; + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { + base_t1_ctrl = phy_read_mmd(phydev, + MDIO_MMD_PMAPMD, + MDIO_PMA_BASET1CTRL); + if (base_t1_ctrl < 0) + return base_t1_ctrl; + + base_t1_ctrl &= ~(MDIO_PMA_BASET1CTRL_TYPE); + } + ctrl1 &= ~MDIO_CTRL1_SPEEDSEL; /* * PMA/PMD type selection is 1.7.5:0 not 1.7.3:0. See 45.2.1.6.1 @@ -41,12 +61,21 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) break; case SPEED_100: ctrl1 |= MDIO_PMA_CTRL1_SPEED100; - ctrl2 |= MDIO_PMA_CTRL2_100BTX; + if (IS_100BASET1(phydev)) { + ctrl2 |= MDIO_PMA_CTRL2_BT1; + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_100BT1; + } else { + ctrl2 |= MDIO_PMA_CTRL2_100BTX; + } break; case SPEED_1000: ctrl1 |= MDIO_PMA_CTRL1_SPEED1000; - /* Assume 1000base-T */ - ctrl2 |= MDIO_PMA_CTRL2_1000BT; + if (IS_1000BASET1(phydev)) { + ctrl2 |= MDIO_PMA_CTRL2_BT1; + base_t1_ctrl |= MDIO_PMA_BASET1CTRL_TYPE_1000BT1; + } else { + ctrl2 |= MDIO_PMA_CTRL2_1000BT; + } break; case SPEED_2500: ctrl1 |= MDIO_CTRL1_SPEED2_5G; @@ -75,6 +104,14 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev) if (ret < 0) return ret; + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) { + ret = phy_write_mmd(phydev, + MDIO_MMD_PMAPMD, + MDIO_PMA_BASET1CTRL, + base_t1_ctrl); + if (ret < 0) + return ret; + } return genphy_c45_an_disable_aneg(phydev); } EXPORT_SYMBOL_GPL(genphy_c45_pma_setup_forced); @@ -135,8 +172,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_config_aneg); */ int genphy_c45_an_disable_aneg(struct phy_device *phydev) { - - return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, + return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); } EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); @@ -151,7 +187,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_an_disable_aneg); */ int genphy_c45_restart_aneg(struct phy_device *phydev) { - return phy_set_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1, + return phy_set_bits_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev), MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART); } EXPORT_SYMBOL_GPL(genphy_c45_restart_aneg); @@ -171,7 +207,7 @@ int genphy_c45_check_and_restart_aneg(struct phy_device *phydev, bool restart) if (!restart) { /* Configure and restart aneg if it wasn't set before */ - ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1); + ret = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_ctrl(phydev)); if (ret < 0) return ret; @@ -199,7 +235,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_check_and_restart_aneg); */ int genphy_c45_aneg_done(struct phy_device *phydev) { - int val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); + int val = phy_read_mmd(phydev, MDIO_MMD_AN, get_aneg_stat(phydev)); return val < 0 ? val : val & MDIO_AN_STAT1_COMPLETE ? 1 : 0; } @@ -385,7 +421,9 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); * PMA Extended Abilities (1.11) register, indicating 1000BASET an 10G related * modes. If bit 1.11.14 is set, then the list is also extended with the modes * in the 2.5G/5G PMA Extended register (1.21), indicating if 2.5GBASET and - * 5GBASET are supported. + * 5GBASET are supported. If bit 1.11.11 is set, then the list is also extended + * with the modes in the BASE-T1 PMA Extended register (1.18), indicating if + * 10/100/1000BASET-1 are supported. */ int genphy_c45_pma_read_abilities(struct phy_device *phydev) { @@ -470,6 +508,29 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev) phydev->supported, val & MDIO_PMA_NG_EXTABLE_5GBT); } + + if (val & MDIO_PMA_EXTABLE_BASET1) { + val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, + MDIO_PMA_BASET1_EXTABLE); + if (val < 0) + return val; + + linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT, + phydev->supported, + val & MDIO_PMA_BASET1_EXTABLE_100BT1); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, + phydev->supported, + val & MDIO_PMA_BASET1_EXTABLE_1000BT1); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, + phydev->supported, + val & MDIO_PMA_BASET1_EXTABLE_10BT1L); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, + phydev->supported, + val & MDIO_PMA_BASET1_EXTABLE_10BT1S); + } } return 0; @@ -509,6 +570,38 @@ int genphy_c45_read_status(struct phy_device *phydev) } EXPORT_SYMBOL_GPL(genphy_c45_read_status); +/** + * get_aneg_ctrl - Get the register address for auto- + * negotiation control register + * @phydev: target phy_device struct + * + */ +static u32 get_aneg_ctrl(struct phy_device *phydev) +{ + u32 ctrl = MDIO_CTRL1; + + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) + ctrl = MDIO_AN_BT1_CTRL; + + return ctrl; +} + +/** + * get_aneg_ctrl - Get the register address for auto- + * negotiation status register + * @phydev: target phy_device struct + * + */ +static u32 get_aneg_stat(struct phy_device *phydev) +{ + u32 stat = MDIO_STAT1; + + if (IS_100BASET1(phydev) || IS_1000BASET1(phydev)) + stat = MDIO_AN_BT1_STAT; + + return stat; +} + /* The gen10g_* functions are the old Clause 45 stub */ int gen10g_config_aneg(struct phy_device *phydev) diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index 369903d9b6ec..b50576f7709a 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -8,7 +8,7 @@ const char *phy_speed_to_str(int speed) { - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 69, + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 71, "Enum ethtool_link_mode_bit_indices and phylib are out of sync. " "If a speed or mode has been added please update phy_speed_to_str " "and the PHY settings array.\n"); @@ -140,6 +140,8 @@ static const struct phy_setting settings[] = { /* 10M */ PHY_SETTING( 10, FULL, 10baseT_Full ), PHY_SETTING( 10, HALF, 10baseT_Half ), + PHY_SETTING( 10, FULL, 10baseT1L_Full ), + PHY_SETTING( 10, FULL, 10baseT1S_Full ), }; #undef PHY_SETTING diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index dd06302aa93e..e429cc8da31a 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -1485,6 +1485,8 @@ enum ethtool_link_mode_bit_indices { ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66, ETHTOOL_LINK_MODE_100baseT1_Full_BIT = 67, ETHTOOL_LINK_MODE_1000baseT1_Full_BIT = 68, + ETHTOOL_LINK_MODE_10baseT1L_Full_BIT = 69, + ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 70, /* must be last entry */ __ETHTOOL_LINK_MODE_MASK_NBITS diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h index 0a552061ff1c..6fd5ff632b8e 100644 --- a/include/uapi/linux/mdio.h +++ b/include/uapi/linux/mdio.h @@ -43,6 +43,7 @@ #define MDIO_PKGID1 14 /* Package identifier */ #define MDIO_PKGID2 15 #define MDIO_AN_ADVERTISE 16 /* AN advertising (base page) */ +#define MDIO_PMA_BASET1_EXTABLE 18 /* BASE-T1 PMA/PMD extended ability */ #define MDIO_AN_LPA 19 /* AN LP abilities (base page) */ #define MDIO_PCS_EEE_ABLE 20 /* EEE Capability register */ #define MDIO_PMA_NG_EXTABLE 21 /* 2.5G/5G PMA/PMD extended ability */ @@ -57,11 +58,16 @@ #define MDIO_PMA_10GBT_SNR 133 /* 10GBASE-T SNR margin, lane A. * Lanes B-D are numbered 134-136. */ #define MDIO_PMA_10GBR_FECABLE 170 /* 10GBASE-R FEC ability */ +#define MDIO_PMA_BASET1CTRL 2100 /* BASE-T1 PMA/PMD control */ #define MDIO_PCS_10GBX_STAT1 24 /* 10GBASE-X PCS status 1 */ #define MDIO_PCS_10GBRT_STAT1 32 /* 10GBASE-R/-T PCS status 1 */ #define MDIO_PCS_10GBRT_STAT2 33 /* 10GBASE-R/-T PCS status 2 */ #define MDIO_AN_10GBT_CTRL 32 /* 10GBASE-T auto-negotiation control */ #define MDIO_AN_10GBT_STAT 33 /* 10GBASE-T auto-negotiation status */ +#define MDIO_AN_BT1_CTRL 512 /* BASE-T1 auto-negotiation control */ +#define MDIO_AN_BT1_STAT 513 /* BASE-T1 auto-negotiation status */ +#define MDIO_AN_10BT1_CTRL 526 /* 10BASE-T1 auto-negotiation control */ +#define MDIO_AN_10BT1_STAT 527 /* 10BASE-T1 auto-negotiation status */ /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */ #define MDIO_PMA_LASI_RXCTRL 0x9000 /* RX_ALARM control */ @@ -151,6 +157,7 @@ #define MDIO_PMA_CTRL2_100BTX 0x000e /* 100BASE-TX type */ #define MDIO_PMA_CTRL2_10BT 0x000f /* 10BASE-T type */ #define MDIO_PMA_CTRL2_2_5GBT 0x0030 /* 2.5GBaseT type */ +#define MDIO_PMA_CTRL2_BT1 0x003D /* BASE-T1 type */ #define MDIO_PMA_CTRL2_5GBT 0x0031 /* 5GBaseT type */ #define MDIO_PCS_CTRL2_TYPE 0x0003 /* PCS type selection */ #define MDIO_PCS_CTRL2_10GBR 0x0000 /* 10GBASE-R type */ @@ -205,8 +212,16 @@ #define MDIO_PMA_EXTABLE_1000BKX 0x0040 /* 1000BASE-KX ability */ #define MDIO_PMA_EXTABLE_100BTX 0x0080 /* 100BASE-TX ability */ #define MDIO_PMA_EXTABLE_10BT 0x0100 /* 10BASE-T ability */ +#define MDIO_PMA_EXTABLE_BASET1 0x0800 /* BASE-T1 ability */ #define MDIO_PMA_EXTABLE_NBT 0x4000 /* 2.5/5GBASE-T ability */ +/* PMA BASE-T1 control register. */ +#define MDIO_PMA_BASET1CTRL_TYPE 0x000f /* PMA/PMD BASE-T1 type sel. */ +#define MDIO_PMA_BASET1CTRL_TYPE_100BT1 0x0000 /* 100BASE-T1 */ +#define MDIO_PMA_BASET1CTRL_TYPE_1000BT1 0x0001 /* 1000BASE-T1 */ +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1L 0x0002 /* 10BASE-T1L */ +#define MDIO_PMA_BASET1CTRL_TYPE_10BT1S 0x0003 /* 10BASE-T1S */ + /* PHY XGXS lane state register. */ #define MDIO_PHYXS_LNSTAT_SYNC0 0x0001 #define MDIO_PHYXS_LNSTAT_SYNC1 0x0002 @@ -281,6 +296,12 @@ #define MDIO_PMA_NG_EXTABLE_2_5GBT 0x0001 /* 2.5GBASET ability */ #define MDIO_PMA_NG_EXTABLE_5GBT 0x0002 /* 5GBASET ability */ +/* BASE-T1 Extended abilities register. */ +#define MDIO_PMA_BASET1_EXTABLE_100BT1 0x0001 /* 100BASE-T1 ability */ +#define MDIO_PMA_BASET1_EXTABLE_1000BT1 0x0002 /* 1000BASE-T1 ability */ +#define MDIO_PMA_BASET1_EXTABLE_10BT1L 0x0004 /* 10BASE-T1L ability */ +#define MDIO_PMA_BASET1_EXTABLE_10BT1S 0x0008 /* 10BASE-T1S ability */ + /* LASI RX_ALARM control/status registers. */ #define MDIO_PMA_LASI_RX_PHYXSLFLT 0x0001 /* PHY XS RX local fault */ #define MDIO_PMA_LASI_RX_PCSLFLT 0x0008 /* PCS RX local fault */
BASE-T1 is a category of Ethernet PHYs. They use a single copper pair for transmission. This patch add basic support for this category of PHYs. It coveres the discovery of abilities and basic configuration. It includes setting fixed speed and enabling auto-negotiation. BASE-T1 devices should always Clause-45 managed. Therefore, this patch extends phy-c45.c. While for some functions like auto-neogtiation different registers are used, the layout of these registers is the same for the used fields. Thus, much of the logic of basic Clause-45 devices can be reused. Signed-off-by: Christian Herber <christian.herber@nxp.com> --- drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++---- drivers/net/phy/phy-core.c | 4 +- include/uapi/linux/ethtool.h | 2 + include/uapi/linux/mdio.h | 21 +++++++ 4 files changed, 129 insertions(+), 11 deletions(-)