This patch adds the required connection between netlink ethtool and
phylib to resolve PLCA get/set config and get status messages.
Additionally, it adds the link modes for the IEEE 802.3cg Clause 147
10BASE-T1S Ethernet PHY.
Signed-off-by: Piergiorgio Beruto <[email protected]>
---
drivers/net/phy/phy-core.c | 2 +-
drivers/net/phy/phy.c | 163 +++++++++++++++++++++++++++++++++--
drivers/net/phy/phylink.c | 6 +-
include/linux/phy.h | 17 +++-
include/uapi/linux/ethtool.h | 3 +
net/ethtool/common.c | 8 ++
6 files changed, 185 insertions(+), 14 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 5d08c627a516..5d8085fffffc 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -13,7 +13,7 @@
*/
const char *phy_speed_to_str(int speed)
{
- BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 99,
+ BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 102,
"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");
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 99e3497b6aa1..6bea928e405b 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -544,36 +544,181 @@ int phy_ethtool_get_stats(struct phy_device *phydev,
EXPORT_SYMBOL(phy_ethtool_get_stats);
/**
+ * phy_ethtool_get_plca_cfg - Get PLCA RS configuration
*
+ * @phydev: the phy_device struct
+ * @plca_cfg: where to store the retrieved configuration
*/
-int phy_ethtool_get_plca_cfg(struct phy_device *dev,
+int phy_ethtool_get_plca_cfg(struct phy_device *phydev,
struct phy_plca_cfg *plca_cfg)
{
- // TODO
- return 0;
+ int ret;
+
+ if (!phydev->drv) {
+ ret = -EIO;
+ goto out;
+ }
+
+ if (!phydev->drv->get_plca_cfg) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+
+ memset(plca_cfg, 0xFF, sizeof(*plca_cfg));
+
+ mutex_lock(&phydev->lock);
+ ret = phydev->drv->get_plca_cfg(phydev, plca_cfg);
+
+ if (ret)
+ goto out_drv;
+
+out_drv:
+ mutex_unlock(&phydev->lock);
+out:
+ return ret;
}
EXPORT_SYMBOL(phy_ethtool_get_plca_cfg);
/**
+ * phy_ethtool_set_plca_cfg - Set PLCA RS configuration
*
+ * @phydev: the phy_device struct
+ * @extack: extack for reporting useful error messages
+ * @plca_cfg: new PLCA configuration to apply
*/
-int phy_ethtool_set_plca_cfg(struct phy_device *dev,
+int phy_ethtool_set_plca_cfg(struct phy_device *phydev,
struct netlink_ext_ack *extack,
const struct phy_plca_cfg *plca_cfg)
{
- // TODO
- return 0;
+ int ret;
+ struct phy_plca_cfg *curr_plca_cfg = 0;
+
+ if (!phydev->drv) {
+ ret = -EIO;
+ goto out;
+ }
+
+ if (!phydev->drv->set_plca_cfg ||
+ !phydev->drv->get_plca_cfg) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+
+ curr_plca_cfg = kmalloc(sizeof(*curr_plca_cfg), GFP_KERNEL);
+ memset(curr_plca_cfg, 0xFF, sizeof(*curr_plca_cfg));
+
+ mutex_lock(&phydev->lock);
+
+ ret = phydev->drv->get_plca_cfg(phydev, curr_plca_cfg);
+ if (ret)
+ goto out_drv;
+
+ if (curr_plca_cfg->enabled < 0 && plca_cfg->enabled >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'enable' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ if (curr_plca_cfg->node_id < 0 && plca_cfg->node_id >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'local node ID' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ if (curr_plca_cfg->node_cnt < 0 && plca_cfg->node_cnt >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'node count' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ if (curr_plca_cfg->to_tmr < 0 && plca_cfg->to_tmr >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'TO timer' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ if (curr_plca_cfg->burst_cnt < 0 && plca_cfg->burst_cnt >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'burst count' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ if (curr_plca_cfg->burst_tmr < 0 && plca_cfg->burst_tmr >= 0) {
+ NL_SET_ERR_MSG(extack,
+ "PHY does not support changing the PLCA 'burst timer' attribute");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+
+ // if enabling PLCA, perform additional sanity checks
+ if (plca_cfg->enabled > 0) {
+ if (!linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT,
+ phydev->advertising)) {
+ ret = -EOPNOTSUPP;
+ NL_SET_ERR_MSG(extack,
+ "Point to Multi-Point mode is not enabled");
+ }
+
+ // allow setting node_id concurrently with enabled
+ if (plca_cfg->node_id >= 0)
+ curr_plca_cfg->node_id = plca_cfg->node_id;
+
+ if (curr_plca_cfg->node_id >= 255) {
+ NL_SET_ERR_MSG(extack, "PLCA node ID is not set");
+ ret = -EINVAL;
+ goto out_drv;
+ }
+ }
+
+ ret = phydev->drv->set_plca_cfg(phydev, plca_cfg);
+ if (ret)
+ goto out_drv;
+
+out_drv:
+ kfree(curr_plca_cfg);
+ mutex_unlock(&phydev->lock);
+out:
+ return ret;
+
}
EXPORT_SYMBOL(phy_ethtool_set_plca_cfg);
/**
+ * phy_ethtool_get_plca_status - Get PLCA RS status information
*
+ * @phydev: the phy_device struct
+ * @plca_st: where to store the retrieved status information
*/
-int phy_ethtool_get_plca_status(struct phy_device *dev,
+int phy_ethtool_get_plca_status(struct phy_device *phydev,
struct phy_plca_status *plca_st)
{
- // TODO
- return 0;
+ int ret;
+
+ if (!phydev->drv) {
+ ret = -EIO;
+ goto out;
+ }
+
+ if (!phydev->drv->get_plca_status) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+
+ mutex_lock(&phydev->lock);
+ ret = phydev->drv->get_plca_status(phydev, plca_st);
+
+ if (ret)
+ goto out_drv;
+
+out_drv:
+ mutex_unlock(&phydev->lock);
+out:
+ return ret;
}
EXPORT_SYMBOL(phy_ethtool_get_plca_status);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 09cc65c0da93..319790221d7f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -241,12 +241,16 @@ void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps)
if (caps & MAC_ASYM_PAUSE)
__set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, linkmodes);
- if (caps & MAC_10HD)
+ if (caps & MAC_10HD) {
__set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, linkmodes);
+ __set_bit(ETHTOOL_LINK_MODE_10baseT1S_Half_BIT, linkmodes);
+ __set_bit(ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT, linkmodes);
+ }
if (caps & MAC_10FD) {
__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, linkmodes);
__set_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, linkmodes);
+ __set_bit(ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, linkmodes);
}
if (caps & MAC_100HD) {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 55160db311fa..2dfb85c6e596 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1036,6 +1036,17 @@ struct phy_driver {
int (*get_sqi)(struct phy_device *dev);
/** @get_sqi_max: Get the maximum signal quality indication */
int (*get_sqi_max)(struct phy_device *dev);
+
+ /* PLCA RS interface */
+ /** @get_plca_cfg: Return the current PLCA configuration */
+ int (*get_plca_cfg)(struct phy_device *dev,
+ struct phy_plca_cfg *plca_cfg);
+ /** @set_plca_cfg: Set the PLCA configuration */
+ int (*set_plca_cfg)(struct phy_device *dev,
+ const struct phy_plca_cfg *plca_cfg);
+ /** @get_plca_status: Return the current PLCA status info */
+ int (*get_plca_status)(struct phy_device *dev,
+ struct phy_plca_status *plca_st);
};
#define to_phy_driver(d) container_of(to_mdio_common_driver(d), \
struct phy_driver, mdiodrv)
@@ -1832,12 +1843,12 @@ int phy_ethtool_get_strings(struct phy_device *phydev, u8 *data);
int phy_ethtool_get_sset_count(struct phy_device *phydev);
int phy_ethtool_get_stats(struct phy_device *phydev,
struct ethtool_stats *stats, u64 *data);
-int phy_ethtool_get_plca_cfg(struct phy_device *dev,
+int phy_ethtool_get_plca_cfg(struct phy_device *phydev,
struct phy_plca_cfg *plca_cfg);
-int phy_ethtool_set_plca_cfg(struct phy_device *dev,
+int phy_ethtool_set_plca_cfg(struct phy_device *phydev,
struct netlink_ext_ack *extack,
const struct phy_plca_cfg *plca_cfg);
-int phy_ethtool_get_plca_status(struct phy_device *dev,
+int phy_ethtool_get_plca_status(struct phy_device *phydev,
struct phy_plca_status *plca_st);
static inline int phy_package_read(struct phy_device *phydev, u32 regnum)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 58e587ba0450..5f414deacf23 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1741,6 +1741,9 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_800000baseDR8_2_Full_BIT = 96,
ETHTOOL_LINK_MODE_800000baseSR8_Full_BIT = 97,
ETHTOOL_LINK_MODE_800000baseVR8_Full_BIT = 98,
+ ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 99,
+ ETHTOOL_LINK_MODE_10baseT1S_Half_BIT = 100,
+ ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT = 101,
/* must be last entry */
__ETHTOOL_LINK_MODE_MASK_NBITS
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 21cfe8557205..c586db0c5e68 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -208,6 +208,9 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
__DEFINE_LINK_MODE_NAME(800000, DR8_2, Full),
__DEFINE_LINK_MODE_NAME(800000, SR8, Full),
__DEFINE_LINK_MODE_NAME(800000, VR8, Full),
+ __DEFINE_LINK_MODE_NAME(10, T1S, Full),
+ __DEFINE_LINK_MODE_NAME(10, T1S, Half),
+ __DEFINE_LINK_MODE_NAME(10, T1S_P2MP, Half),
};
static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
@@ -244,6 +247,8 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
#define __LINK_MODE_LANES_X 1
#define __LINK_MODE_LANES_FX 1
#define __LINK_MODE_LANES_T1L 1
+#define __LINK_MODE_LANES_T1S 1
+#define __LINK_MODE_LANES_T1S_P2MP 1
#define __LINK_MODE_LANES_VR8 8
#define __LINK_MODE_LANES_DR8_2 8
@@ -366,6 +371,9 @@ const struct link_mode_info link_mode_params[] = {
__DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full),
__DEFINE_LINK_MODE_PARAMS(800000, SR8, Full),
__DEFINE_LINK_MODE_PARAMS(800000, VR8, Full),
+ __DEFINE_LINK_MODE_PARAMS(10, T1S, Full),
+ __DEFINE_LINK_MODE_PARAMS(10, T1S, Half),
+ __DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half),
};
static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
--
2.35.1
Hi,
On Sun, Dec 04, 2022 at 03:30:52AM +0100, Piergiorgio Beruto wrote:
> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
> index 5d08c627a516..5d8085fffffc 100644
> --- a/drivers/net/phy/phy-core.c
> +++ b/drivers/net/phy/phy-core.c
> @@ -13,7 +13,7 @@
> */
> const char *phy_speed_to_str(int speed)
> {
> - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 99,
> + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 102,
> "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");
I think you need to update settings[] in this file as well.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Sun, Dec 04, 2022 at 04:45:22PM +0000, Russell King (Oracle) wrote:
> Hi,
>
> On Sun, Dec 04, 2022 at 03:30:52AM +0100, Piergiorgio Beruto wrote:
> > diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
> > index 5d08c627a516..5d8085fffffc 100644
> > --- a/drivers/net/phy/phy-core.c
> > +++ b/drivers/net/phy/phy-core.c
> > @@ -13,7 +13,7 @@
> > */
> > const char *phy_speed_to_str(int speed)
> > {
> > - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 99,
> > + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 102,
> > "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");
>
> I think you need to update settings[] in this file as well.
Oh, sure. I've just added the following:
PHY_SETTING( 10, FULL, 10baseT1S_Full ),
PHY_SETTING( 10, HALF, 10baseT1S_Half ),
PHY_SETTING( 10, HALF, 10baseT1S_P2MP_Half ),
I will amend the patch after I reviewed all the feedback.
Thanks,
Piergiorgio
On Sun, Dec 04, 2022 at 03:30:52AM +0100, Piergiorgio Beruto wrote:
> This patch adds the required connection between netlink ethtool and
> phylib to resolve PLCA get/set config and get status messages.
> Additionally, it adds the link modes for the IEEE 802.3cg Clause 147
> 10BASE-T1S Ethernet PHY.
Please break this patch up.
> const char *phy_speed_to_str(int speed)
> {
> - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 99,
> + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 102,
> "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");
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1741,6 +1741,9 @@ enum ethtool_link_mode_bit_indices {
> ETHTOOL_LINK_MODE_800000baseDR8_2_Full_BIT = 96,
> ETHTOOL_LINK_MODE_800000baseSR8_Full_BIT = 97,
> ETHTOOL_LINK_MODE_800000baseVR8_Full_BIT = 98,
> + ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 99,
> + ETHTOOL_LINK_MODE_10baseT1S_Half_BIT = 100,
> + ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT = 101,
>
> /* must be last entry */
> __ETHTOOL_LINK_MODE_MASK_NBITS
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index 21cfe8557205..c586db0c5e68 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -208,6 +208,9 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
> __DEFINE_LINK_MODE_NAME(800000, DR8_2, Full),
> __DEFINE_LINK_MODE_NAME(800000, SR8, Full),
> __DEFINE_LINK_MODE_NAME(800000, VR8, Full),
> + __DEFINE_LINK_MODE_NAME(10, T1S, Full),
> + __DEFINE_LINK_MODE_NAME(10, T1S, Half),
> + __DEFINE_LINK_MODE_NAME(10, T1S_P2MP, Half),
> };
> static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
>
> @@ -366,6 +371,9 @@ const struct link_mode_info link_mode_params[] = {
> __DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full),
> __DEFINE_LINK_MODE_PARAMS(800000, SR8, Full),
> __DEFINE_LINK_MODE_PARAMS(800000, VR8, Full),
> + __DEFINE_LINK_MODE_PARAMS(10, T1S, Full),
> + __DEFINE_LINK_MODE_PARAMS(10, T1S, Half),
> + __DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half),
> };
> static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
This is one logical change, so makes one patch, for example.
You are aiming for lots of simple, easy to review, well described,
obviously correct patches.
Andrew
On Sun, Dec 04, 2022 at 07:12:03PM +0100, Andrew Lunn wrote:
> On Sun, Dec 04, 2022 at 03:30:52AM +0100, Piergiorgio Beruto wrote:
> > This patch adds the required connection between netlink ethtool and
> > phylib to resolve PLCA get/set config and get status messages.
> > Additionally, it adds the link modes for the IEEE 802.3cg Clause 147
> > 10BASE-T1S Ethernet PHY.
>
> Please break this patch up.
>
> > const char *phy_speed_to_str(int speed)
> > {
> > - BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 99,
> > + BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 102,
> > "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");
>
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -1741,6 +1741,9 @@ enum ethtool_link_mode_bit_indices {
> > ETHTOOL_LINK_MODE_800000baseDR8_2_Full_BIT = 96,
> > ETHTOOL_LINK_MODE_800000baseSR8_Full_BIT = 97,
> > ETHTOOL_LINK_MODE_800000baseVR8_Full_BIT = 98,
> > + ETHTOOL_LINK_MODE_10baseT1S_Full_BIT = 99,
> > + ETHTOOL_LINK_MODE_10baseT1S_Half_BIT = 100,
> > + ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT = 101,
> >
> > /* must be last entry */
> > __ETHTOOL_LINK_MODE_MASK_NBITS
> > diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> > index 21cfe8557205..c586db0c5e68 100644
> > --- a/net/ethtool/common.c
> > +++ b/net/ethtool/common.c
> > @@ -208,6 +208,9 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
> > __DEFINE_LINK_MODE_NAME(800000, DR8_2, Full),
> > __DEFINE_LINK_MODE_NAME(800000, SR8, Full),
> > __DEFINE_LINK_MODE_NAME(800000, VR8, Full),
> > + __DEFINE_LINK_MODE_NAME(10, T1S, Full),
> > + __DEFINE_LINK_MODE_NAME(10, T1S, Half),
> > + __DEFINE_LINK_MODE_NAME(10, T1S_P2MP, Half),
> > };
> > static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
> >
> > @@ -366,6 +371,9 @@ const struct link_mode_info link_mode_params[] = {
> > __DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full),
> > __DEFINE_LINK_MODE_PARAMS(800000, SR8, Full),
> > __DEFINE_LINK_MODE_PARAMS(800000, VR8, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10, T1S, Full),
> > + __DEFINE_LINK_MODE_PARAMS(10, T1S, Half),
> > + __DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half),
> > };
> > static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS);
>
> This is one logical change, so makes one patch, for example.
>
> You are aiming for lots of simple, easy to review, well described,
> obviously correct patches.
Alright, will do.
Thanks,
Piergiorgio