2023-02-08 10:32:54

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v6 0/9] net: add EEE support for KSZ9477 and AR8035 with i.MX6

changes v6:
- split patch set and send only first 9 patches
- Add Reviewed-by: Andrew Lunn <[email protected]>
- use 0xffff instead of GENMASK
- Document @supported_eee
- use "()" with function name in comments

changes v5:
- spell fixes
- move part of genphy_c45_read_eee_abilities() to
genphy_c45_read_eee_cap1()
- validate MDIO_PCS_EEE_ABLE register against 0xffff val.
- rename *eee_100_10000* to *eee_cap1*
- use linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)
instead of !linkmode_empty()
- add documentation to linkmode/register helpers

changes v4:
- remove following helpers:
mmd_eee_cap_to_ethtool_sup_t
mmd_eee_adv_to_ethtool_adv_t
ethtool_adv_to_mmd_eee_adv_t
and port drivers from this helpers to linkmode helpers.
- rebase against latest net-next
- port phy_init_eee() to genphy_c45_eee_is_active()

changes v3:
- rework some parts of EEE infrastructure and move it to c45 code.
- add supported_eee storage and start using it in EEE code and by the
micrel driver.
- add EEE support for ar8035 PHY
- add SmartEEE support to FEC i.MX series.

changes v2:
- use phydev->supported instead of reading MII_BMSR regiaster
- fix @get_eee > @set_eee

With this patch series we provide EEE control for KSZ9477 family of switches and
AR8035 with i.MX6 configuration.
According to my tests, on a system with KSZ8563 switch and 100Mbit idle link,
we consume 0,192W less power per port if EEE is enabled.

Oleksij Rempel (9):
net: dsa: microchip: enable EEE support
net: phy: add genphy_c45_read_eee_abilities() function
net: phy: micrel: add ksz9477_get_features()
net: phy: export phy_check_valid() function
net: phy: add genphy_c45_ethtool_get/set_eee() support
net: phy: c22: migrate to genphy_c45_write_eee_adv()
net: phy: c45: migrate to genphy_c45_write_eee_adv()
net: phy: migrate phy_init_eee() to genphy_c45_eee_is_active()
net: phy: start using genphy_c45_ethtool_get/set_eee()

drivers/net/dsa/microchip/ksz_common.c | 65 +++++
drivers/net/phy/micrel.c | 21 ++
drivers/net/phy/phy-c45.c | 316 ++++++++++++++++++++++++-
drivers/net/phy/phy.c | 153 ++----------
drivers/net/phy/phy_device.c | 26 +-
include/linux/mdio.h | 84 +++++++
include/linux/phy.h | 14 ++
include/uapi/linux/mdio.h | 8 +
8 files changed, 550 insertions(+), 137 deletions(-)

--
2.30.2



2023-02-08 10:32:58

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v6 2/9] net: phy: add genphy_c45_read_eee_abilities() function

Add generic function for EEE abilities defined by IEEE 802.3
specification. For now following registers are supported:
- IEEE 802.3-2018 45.2.3.10 EEE control and capability 1 (Register 3.20)
- IEEE 802.3cg-2019 45.2.1.186b 10BASE-T1L PMA status register
(Register 1.2295)

Since I was not able to find any flag signaling support of these
registers, we should detect link mode abilities first and then based on
these abilities doing EEE link modes detection.

Results of EEE ability detection will be stored into new variable
phydev->supported_eee.

Signed-off-by: Oleksij Rempel <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
drivers/net/phy/phy-c45.c | 70 ++++++++++++++++++++++++++++++++++++
drivers/net/phy/phy_device.c | 16 +++++++++
include/linux/mdio.h | 26 ++++++++++++++
include/linux/phy.h | 6 ++++
4 files changed, 118 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 9f9565a4819d..2d10d22e7684 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,76 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);

+/**
+ * genphy_c45_read_eee_cap1 - read supported EEE link modes from register 3.20
+ * @phydev: target phy_device struct
+ */
+static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
+{
+ int val;
+
+ /* IEEE 802.3-2018 45.2.3.10 EEE control and capability 1
+ * (Register 3.20)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
+ if (val < 0)
+ return val;
+
+ /* The 802.3 2018 standard says the top 2 bits are reserved and should
+ * read as 0. Also, it seems unlikely anybody will build a PHY which
+ * supports 100GBASE-R deep sleep all the way down to 100BASE-TX EEE.
+ * If MDIO_PCS_EEE_ABLE is 0xffff assume EEE is not supported.
+ */
+ if (val == 0xffff)
+ return 0;
+
+ mii_eee_cap1_mod_linkmode_t(phydev->supported_eee, val);
+
+ /* Some buggy devices indicate EEE link modes in MDIO_PCS_EEE_ABLE
+ * which they don't support as indicated by BMSR, ESTATUS etc.
+ */
+ linkmode_and(phydev->supported_eee, phydev->supported_eee,
+ phydev->supported);
+
+ return 0;
+}
+
+/**
+ * genphy_c45_read_eee_abilities - read supported EEE link modes
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_read_eee_abilities(struct phy_device *phydev)
+{
+ int val;
+
+ /* There is not indicator whether optional register
+ * "EEE control and capability 1" (3.20) is supported. Read it only
+ * on devices with appropriate linkmodes.
+ */
+ if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+ val = genphy_c45_read_eee_cap1(phydev);
+ if (val)
+ return val;
+ }
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ phydev->supported)) {
+ /* IEEE 802.3cg-2019 45.2.1.186b 10BASE-T1L PMA status register
+ * (Register 1.2295)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10T1L_STAT);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ phydev->supported_eee,
+ val & MDIO_PMA_10T1L_STAT_EEE);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_eee_abilities);
+
/**
* genphy_c45_pma_read_abilities - read supported link modes from PMA
* @phydev: target phy_device struct
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a3917c7acbd3..66a4e62009bb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -132,6 +132,18 @@ static const int phy_10gbit_full_features_array[] = {
ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
};

+static const int phy_eee_cap1_features_array[] = {
+ ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+};
+
+__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_eee_cap1_features) __ro_after_init;
+EXPORT_SYMBOL_GPL(phy_eee_cap1_features);
+
static void features_init(void)
{
/* 10/100 half/full*/
@@ -213,6 +225,10 @@ static void features_init(void)
linkmode_set_bit_array(phy_10gbit_fec_features_array,
ARRAY_SIZE(phy_10gbit_fec_features_array),
phy_10gbit_fec_features);
+ linkmode_set_bit_array(phy_eee_cap1_features_array,
+ ARRAY_SIZE(phy_eee_cap1_features_array),
+ phy_eee_cap1_features);
+
}

void phy_device_free(struct phy_device *phydev)
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index c0da30d63b1d..e75583f5d967 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -402,6 +402,32 @@ static inline u32 linkmode_adv_to_mii_t1_adv_m_t(unsigned long *advertising)
return result;
}

+/**
+ * mii_eee_cap1_mod_linkmode_t()
+ * @adv: target the linkmode advertisement settings
+ * @val: register value
+ *
+ * A function that translates value of following registers to the linkmode:
+ * IEEE 802.3-2018 45.2.3.10 "EEE control and capability 1" register (3.20)
+ * IEEE 802.3-2018 45.2.7.13 "EEE advertisement 1" register (7.60)
+ * IEEE 802.3-2018 45.2.7.14 "EEE "link partner ability 1 register (7.61)
+ */
+static inline void mii_eee_cap1_mod_linkmode_t(unsigned long *adv, u32 val)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ adv, val & MDIO_EEE_100TX);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ adv, val & MDIO_EEE_1000T);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ adv, val & MDIO_EEE_10GT);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ adv, val & MDIO_EEE_1000KX);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ adv, val & MDIO_EEE_10GKX4);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ adv, val & MDIO_EEE_10GKR);
+}
+
int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fbeba4fee8d4..c183a8a27986 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -52,6 +52,7 @@ extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_all_ports_features) __ro_after_
extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_features) __ro_after_init;
extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_fec_features) __ro_after_init;
extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_init;
+extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_eee_cap1_features) __ro_after_init;

#define PHY_BASIC_FEATURES ((unsigned long *)&phy_basic_features)
#define PHY_BASIC_T1_FEATURES ((unsigned long *)&phy_basic_t1_features)
@@ -62,6 +63,7 @@ extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_ini
#define PHY_10GBIT_FEATURES ((unsigned long *)&phy_10gbit_features)
#define PHY_10GBIT_FEC_FEATURES ((unsigned long *)&phy_10gbit_fec_features)
#define PHY_10GBIT_FULL_FEATURES ((unsigned long *)&phy_10gbit_full_features)
+#define PHY_EEE_CAP1_FEATURES ((unsigned long *)&phy_eee_cap1_features)

extern const int phy_basic_ports_array[3];
extern const int phy_fibre_port_array[1];
@@ -572,6 +574,7 @@ struct macsec_ops;
* @supported: Combined MAC/PHY supported linkmodes
* @advertising: Currently advertised linkmodes
* @adv_old: Saved advertised while power saving for WoL
+ * @supported_eee: supported PHY EEE linkmodes
* @lp_advertising: Current link partner advertised linkmodes
* @host_interfaces: PHY interface modes supported by host
* @eee_broken_modes: Energy efficient ethernet modes which should be prohibited
@@ -676,6 +679,8 @@ struct phy_device {
__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
/* used with phy_speed_down */
__ETHTOOL_DECLARE_LINK_MODE_MASK(adv_old);
+ /* used for eee validation */
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported_eee);

/* Host supported PHY interface types. Should be ignored if empty. */
DECLARE_PHY_INTERFACE_MASK(host_interfaces);
@@ -1737,6 +1742,7 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev);
int genphy_c45_an_disable_aneg(struct phy_device *phydev);
int genphy_c45_read_mdix(struct phy_device *phydev);
int genphy_c45_pma_read_abilities(struct phy_device *phydev);
+int genphy_c45_read_eee_abilities(struct phy_device *phydev);
int genphy_c45_pma_baset1_read_master_slave(struct phy_device *phydev);
int genphy_c45_read_status(struct phy_device *phydev);
int genphy_c45_baset1_read_status(struct phy_device *phydev);
--
2.30.2


2023-02-08 10:33:02

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v6 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

Add replacement for phy_ethtool_get/set_eee() functions.

Current phy_ethtool_get/set_eee() implementation is great and it is
possible to make it even better:
- this functionality is for devices implementing parts of IEEE 802.3
specification beyond Clause 22. The better place for this code is
phy-c45.c
- currently it is able to do read/write operations on PHYs with
different abilities to not existing registers. It is better to
use stored supported_eee abilities to avoid false read/write
operations.
- the eee_active detection will provide wrong results on not supported
link modes. It is better to validate speed/duplex properties against
supported EEE link modes.
- it is able to support only limited amount of link modes. We have more
EEE link modes...

By refactoring this code I address most of this point except of the last
one. Adding additional EEE link modes will need more work.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/phy-c45.c | 235 ++++++++++++++++++++++++++++++++++++++
include/linux/mdio.h | 58 ++++++++++
include/linux/phy.h | 7 ++
include/uapi/linux/mdio.h | 8 ++
4 files changed, 308 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 2d10d22e7684..cc6c6094e87d 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,129 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);

+/**
+ * genphy_c45_write_eee_adv - write advertised EEE link modes
+ * @phydev: target phy_device struct
+ * @adv: the linkmode advertisement settings
+ */
+int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
+{
+ int val, changed;
+
+ if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+ val = linkmode_to_mii_eee_cap1_t(adv);
+
+ /* In eee_broken_modes are stored MDIO_AN_EEE_ADV specific raw
+ * register values.
+ */
+ val &= ~phydev->eee_broken_modes;
+
+ /* IEEE 802.3-2018 45.2.7.13 EEE advertisement 1
+ * (Register 7.60)
+ */
+ val = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
+ MDIO_AN_EEE_ADV,
+ MDIO_EEE_100TX | MDIO_EEE_1000T |
+ MDIO_EEE_10GT | MDIO_EEE_1000KX |
+ MDIO_EEE_10GKX4 | MDIO_EEE_10GKR,
+ val);
+ if (val < 0)
+ return val;
+ if (val > 0)
+ changed = 1;
+ }
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ phydev->supported_eee)) {
+ val = linkmode_adv_to_mii_10base_t1_t(adv);
+ /* IEEE 802.3cg-2019 45.2.7.25 10BASE-T1 AN control register
+ * (Register 7.526)
+ */
+ val = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
+ MDIO_AN_10BT1_AN_CTRL,
+ MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L,
+ val);
+ if (val < 0)
+ return val;
+ if (val > 0)
+ changed = 1;
+ }
+
+ return changed;
+}
+
+/**
+ * genphy_c45_read_eee_adv - read advertised EEE link modes
+ * @phydev: target phy_device struct
+ * @adv: the linkmode advertisement status
+ */
+static int genphy_c45_read_eee_adv(struct phy_device *phydev,
+ unsigned long *adv)
+{
+ int val;
+
+ if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+ /* IEEE 802.3-2018 45.2.7.13 EEE advertisement 1
+ * (Register 7.60)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
+ if (val < 0)
+ return val;
+
+ mii_eee_cap1_mod_linkmode_t(adv, val);
+ }
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ phydev->supported_eee)) {
+ /* IEEE 802.3cg-2019 45.2.7.25 10BASE-T1 AN control register
+ * (Register 7.526)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10BT1_AN_CTRL);
+ if (val < 0)
+ return val;
+
+ mii_10base_t1_adv_mod_linkmode_t(adv, val);
+ }
+
+ return 0;
+}
+
+/**
+ * genphy_c45_read_eee_lpa - read advertised LP EEE link modes
+ * @phydev: target phy_device struct
+ * @lpa: the linkmode LP advertisement status
+ */
+static int genphy_c45_read_eee_lpa(struct phy_device *phydev,
+ unsigned long *lpa)
+{
+ int val;
+
+ if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+ /* IEEE 802.3-2018 45.2.7.14 EEE link partner ability 1
+ * (Register 7.61)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
+ if (val < 0)
+ return val;
+
+ mii_eee_cap1_mod_linkmode_t(lpa, val);
+ }
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ phydev->supported_eee)) {
+ /* IEEE 802.3cg-2019 45.2.7.26 10BASE-T1 AN status register
+ * (Register 7.527)
+ */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10BT1_AN_STAT);
+ if (val < 0)
+ return val;
+
+ mii_10base_t1_adv_mod_linkmode_t(lpa, val);
+ }
+
+ return 0;
+}
+
/**
* genphy_c45_read_eee_cap1 - read supported EEE link modes from register 3.20
* @phydev: target phy_device struct
@@ -1194,6 +1317,118 @@ int genphy_c45_plca_get_status(struct phy_device *phydev,
}
EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);

+/**
+ * genphy_c45_eee_is_active - get EEE supported and status
+ * @phydev: target phy_device struct
+ * @data: ethtool_eee data
+ *
+ * Description: it reports the possible state of EEE functionality.
+ */
+int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
+ unsigned long *lp, bool *is_enabled)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_adv) = {};
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_lp) = {};
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+ bool eee_enabled, eee_active;
+ int ret;
+
+ ret = genphy_c45_read_eee_adv(phydev, tmp_adv);
+ if (ret)
+ return ret;
+
+ ret = genphy_c45_read_eee_lpa(phydev, tmp_lp);
+ if (ret)
+ return ret;
+
+ eee_enabled = !linkmode_empty(tmp_adv);
+ linkmode_and(common, tmp_adv, tmp_lp);
+ if (eee_enabled && !linkmode_empty(common))
+ eee_active = phy_check_valid(phydev->speed, phydev->duplex,
+ common);
+ else
+ eee_active = false;
+
+ if (adv)
+ linkmode_copy(adv, tmp_adv);
+ if (lp)
+ linkmode_copy(lp, tmp_lp);
+ if (is_enabled)
+ *is_enabled = eee_enabled;
+
+ return eee_active;
+}
+EXPORT_SYMBOL(genphy_c45_eee_is_active);
+
+/**
+ * genphy_c45_ethtool_get_eee - get EEE supported and status
+ * @phydev: target phy_device struct
+ * @data: ethtool_eee data
+ *
+ * Description: it reports the Supported/Advertisement/LP Advertisement
+ * capabilities.
+ */
+int genphy_c45_ethtool_get_eee(struct phy_device *phydev,
+ struct ethtool_eee *data)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(lp) = {};
+ bool overflow = false, is_enabled;
+ int ret;
+
+ ret = genphy_c45_eee_is_active(phydev, adv, lp, &is_enabled);
+ if (ret < 0)
+ return ret;
+
+ data->eee_enabled = is_enabled;
+ data->eee_active = ret;
+
+ if (!ethtool_convert_link_mode_to_legacy_u32(&data->supported,
+ phydev->supported_eee))
+ overflow = true;
+ if (!ethtool_convert_link_mode_to_legacy_u32(&data->advertised, adv))
+ overflow = true;
+ if (!ethtool_convert_link_mode_to_legacy_u32(&data->lp_advertised, lp))
+ overflow = true;
+
+ if (overflow)
+ phydev_warn(phydev, "Not all supported or advertised EEE link modes were passed to the user space\n");
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c45_ethtool_get_eee);
+
+/**
+ * genphy_c45_ethtool_set_eee - get EEE supported and status
+ * @phydev: target phy_device struct
+ * @data: ethtool_eee data
+ *
+ * Description: it reportes the Supported/Advertisement/LP Advertisement
+ * capabilities.
+ */
+int genphy_c45_ethtool_set_eee(struct phy_device *phydev,
+ struct ethtool_eee *data)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
+ int ret;
+
+ if (data->eee_enabled) {
+ if (data->advertised)
+ adv[0] = data->advertised;
+ else
+ linkmode_copy(adv, phydev->supported_eee);
+ }
+
+ ret = genphy_c45_write_eee_adv(phydev, adv);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ return phy_restart_aneg(phydev);
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c45_ethtool_set_eee);
+
struct phy_driver genphy_c45_driver = {
.phy_id = 0xffffffff,
.phy_id_mask = 0xffffffff,
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e75583f5d967..e3568e44efd0 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -428,6 +428,64 @@ static inline void mii_eee_cap1_mod_linkmode_t(unsigned long *adv, u32 val)
adv, val & MDIO_EEE_10GKR);
}

+/**
+ * mii_eee_cap1_mod_linkmode_t
+ * @adv: the linkmode advertisement settings
+ *
+ * A function that translates linkmode to value for IEEE 802.3-2018 45.2.7.13
+ * "EEE advertisement 1" register (7.60)
+ */
+static inline u32 linkmode_to_mii_eee_cap1_t(unsigned long *adv)
+{
+ u32 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, adv))
+ result |= MDIO_EEE_100TX;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, adv))
+ result |= MDIO_EEE_1000T;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, adv))
+ result |= MDIO_EEE_10GT;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, adv))
+ result |= MDIO_EEE_1000KX;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, adv))
+ result |= MDIO_EEE_10GKX4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, adv))
+ result |= MDIO_EEE_10GKR;
+
+ return result;
+}
+
+/**
+ * mii_10base_t1_adv_mod_linkmode_t
+ * @adv: linkmode advertisement settings
+ * @val: register value
+ *
+ * A function that translates IEEE 802.3cg-2019 45.2.7.26 "10BASE-T1 AN status"
+ * register (7.527) value to the linkmode.
+ */
+static inline void mii_10base_t1_adv_mod_linkmode_t(unsigned long *adv, u16 val)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+ adv, val & MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L);
+}
+
+/**
+ * linkmode_adv_to_mii_10base_t1_t
+ * @adv: linkmode advertisement settings
+ *
+ * A function that translates the linkmode to IEEE 802.3cg-2019 45.2.7.25
+ * "10BASE-T1 AN control" register (7.526) value.
+ */
+static inline u32 linkmode_adv_to_mii_10base_t1_t(unsigned long *adv)
+{
+ u32 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, adv))
+ result |= MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L;
+
+ return result;
+}
+
int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7a8e541de3f3..727bff531a14 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1758,6 +1758,13 @@ int genphy_c45_plca_set_cfg(struct phy_device *phydev,
const struct phy_plca_cfg *plca_cfg);
int genphy_c45_plca_get_status(struct phy_device *phydev,
struct phy_plca_status *plca_st);
+int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
+ unsigned long *lp, bool *is_enabled);
+int genphy_c45_ethtool_get_eee(struct phy_device *phydev,
+ struct ethtool_eee *data);
+int genphy_c45_ethtool_set_eee(struct phy_device *phydev,
+ struct ethtool_eee *data);
+int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv);

/* Generic C45 PHY driver */
extern struct phy_driver genphy_c45_driver;
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 75b7257a51e1..256b463e47a6 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -79,6 +79,8 @@
#define MDIO_AN_T1_LP_L 517 /* BASE-T1 AN LP Base Page ability register [15:0] */
#define MDIO_AN_T1_LP_M 518 /* BASE-T1 AN LP Base Page ability register [31:16] */
#define MDIO_AN_T1_LP_H 519 /* BASE-T1 AN LP Base Page ability register [47:32] */
+#define MDIO_AN_10BT1_AN_CTRL 526 /* 10BASE-T1 AN control register */
+#define MDIO_AN_10BT1_AN_STAT 527 /* 10BASE-T1 AN status register */
#define MDIO_PMA_PMD_BT1_CTRL 2100 /* BASE-T1 PMA/PMD control register */

/* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */
@@ -340,6 +342,12 @@
#define MDIO_AN_T1_LP_H_10L_TX_HI_REQ 0x1000 /* 10BASE-T1L High Level LP Transmit Request */
#define MDIO_AN_T1_LP_H_10L_TX_HI 0x2000 /* 10BASE-T1L High Level LP Transmit Ability */

+/* 10BASE-T1 AN control register */
+#define MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L 0x4000 /* 10BASE-T1L EEE ability advertisement */
+
+/* 10BASE-T1 AN status register */
+#define MDIO_AN_10BT1_AN_STAT_LPA_EEE_T1L 0x4000 /* 10BASE-T1L LP EEE ability advertisement */
+
/* BASE-T1 PMA/PMD control register */
#define MDIO_PMA_PMD_BT1_CTRL_CFG_MST 0x4000 /* MASTER-SLAVE config value */

--
2.30.2


2023-02-08 13:08:54

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v6 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

On Wed, Feb 08, 2023 at 11:32:07AM +0100, Oleksij Rempel wrote:
> Add replacement for phy_ethtool_get/set_eee() functions.
>
> Current phy_ethtool_get/set_eee() implementation is great and it is
> possible to make it even better:
> - this functionality is for devices implementing parts of IEEE 802.3
> specification beyond Clause 22. The better place for this code is
> phy-c45.c
> - currently it is able to do read/write operations on PHYs with
> different abilities to not existing registers. It is better to
> use stored supported_eee abilities to avoid false read/write
> operations.
> - the eee_active detection will provide wrong results on not supported
> link modes. It is better to validate speed/duplex properties against
> supported EEE link modes.
> - it is able to support only limited amount of link modes. We have more
> EEE link modes...
>
> By refactoring this code I address most of this point except of the last
> one. Adding additional EEE link modes will need more work.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

> +/**
> + * genphy_c45_eee_is_active - get EEE supported and status
> + * @phydev: target phy_device struct
> + * @data: ethtool_eee data

I forgot to update this comment.

> + *
> + * Description: it reports the possible state of EEE functionality.
> + */
> +int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
> + unsigned long *lp, bool *is_enabled)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_adv) = {};
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_lp) = {};
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> + bool eee_enabled, eee_active;
> + int ret;
> +
> + ret = genphy_c45_read_eee_adv(phydev, tmp_adv);
> + if (ret)
> + return ret;
> +
> + ret = genphy_c45_read_eee_lpa(phydev, tmp_lp);
> + if (ret)
> + return ret;
> +
> + eee_enabled = !linkmode_empty(tmp_adv);
> + linkmode_and(common, tmp_adv, tmp_lp);
> + if (eee_enabled && !linkmode_empty(common))i

and linkmode_empty() can be replaced with linkmode_intersects()

I'll wait if more comments will come and make a new version tomorrow.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |