Re: [PATCH v2 net-next 1/4] net/ethtool: add netlink interface for the PLCA RS
From: Andrew Lunn
Date: Mon Dec 05 2022 - 08:09:15 EST
On Mon, Dec 05, 2022 at 07:00:57AM +0100, Oleksij Rempel wrote:
> Hi Beruto,
> On Mon, Dec 05, 2022 at 02:41:35AM +0100, Piergiorgio Beruto wrote:
> > Add support for configuring the PLCA Reconciliation Sublayer on
> > multi-drop PHYs that support IEEE802.3cg-2019 Clause 148 (e.g.,
> > 10BASE-T1S). This patch adds the appropriate netlink interface
> > to ethtool.
> >
> > Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@xxxxxxxxx>
> > ---
> > MAINTAINERS | 6 +
> > drivers/net/phy/phy.c | 34 ++++
> > drivers/net/phy/phy_device.c | 3 +
> > include/linux/ethtool.h | 11 +
> > include/linux/phy.h | 64 ++++++
> > include/uapi/linux/ethtool_netlink.h | 25 +++
> > net/ethtool/Makefile | 2 +-
> > net/ethtool/netlink.c | 30 +++
> > net/ethtool/netlink.h | 6 +
> > net/ethtool/plca.c | 290 +++++++++++++++++++++++++++
> > 10 files changed, 470 insertions(+), 1 deletion(-)
> > create mode 100644 net/ethtool/plca.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 955c1be1efb2..7952243e4b43 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16396,6 +16396,12 @@ S: Maintained
> > F: Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
> > F: drivers/iio/chemical/pms7003.c
> >
> > +PLCA RECONCILIATION SUBLAYER (IEEE802.3 Clause 148)
> > +M: Piergiorgio Beruto <piergiorgio.beruto@xxxxxxxxx>
> > +L: netdev@xxxxxxxxxxxxxxx
> > +S: Maintained
> > +F: net/ethtool/plca.c
> > +
> > PLDMFW LIBRARY
> > M: Jacob Keller <jacob.e.keller@xxxxxxxxx>
> > S: Maintained
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index e5b6cb1a77f9..99e3497b6aa1 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -543,6 +543,40 @@ int phy_ethtool_get_stats(struct phy_device *phydev,
> > }
> > EXPORT_SYMBOL(phy_ethtool_get_stats);
> >
> > +/**
> > + *
> > + */
> > +int phy_ethtool_get_plca_cfg(struct phy_device *dev,
> > + struct phy_plca_cfg *plca_cfg)
> > +{
> > + // TODO
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_get_plca_cfg);
> > +
> > +/**
> > + *
> > + */
> > +int phy_ethtool_set_plca_cfg(struct phy_device *dev,
> > + struct netlink_ext_ack *extack,
> > + const struct phy_plca_cfg *plca_cfg)
> > +{
> > + // TODO
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_set_plca_cfg);
> > +
> > +/**
> > + *
> > + */
> > +int phy_ethtool_get_plca_status(struct phy_device *dev,
> > + struct phy_plca_status *plca_st)
> > +{
> > + // TODO
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_get_plca_status);
> > +
> > /**
> > * phy_start_cable_test - Start a cable test
> > *
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 716870a4499c..f248010c403d 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -3262,6 +3262,9 @@ static const struct ethtool_phy_ops phy_ethtool_phy_ops = {
> > .get_sset_count = phy_ethtool_get_sset_count,
> > .get_strings = phy_ethtool_get_strings,
> > .get_stats = phy_ethtool_get_stats,
> > + .get_plca_cfg = phy_ethtool_get_plca_cfg,
> > + .set_plca_cfg = phy_ethtool_set_plca_cfg,
> > + .get_plca_status = phy_ethtool_get_plca_status,
> > .start_cable_test = phy_start_cable_test,
> > .start_cable_test_tdr = phy_start_cable_test_tdr,
> > };
> > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> > index 9e0a76fc7de9..4bfe95ec1f0a 100644
> > --- a/include/linux/ethtool.h
> > +++ b/include/linux/ethtool.h
> > @@ -802,12 +802,16 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
> >
> > struct phy_device;
> > struct phy_tdr_config;
> > +struct phy_plca_cfg;
> > +struct phy_plca_status;
> >
> > /**
> > * struct ethtool_phy_ops - Optional PHY device options
> > * @get_sset_count: Get number of strings that @get_strings will write.
> > * @get_strings: Return a set of strings that describe the requested objects
> > * @get_stats: Return extended statistics about the PHY device.
> > + * @get_plca_cfg: Return PLCA configuration.
> > + * @set_plca_cfg: Set PLCA configuration.
> > * @start_cable_test: Start a cable test
> > * @start_cable_test_tdr: Start a Time Domain Reflectometry cable test
> > *
> > @@ -819,6 +823,13 @@ struct ethtool_phy_ops {
> > int (*get_strings)(struct phy_device *dev, u8 *data);
> > int (*get_stats)(struct phy_device *dev,
> > struct ethtool_stats *stats, u64 *data);
> > + int (*get_plca_cfg)(struct phy_device *dev,
> > + struct phy_plca_cfg *plca_cfg);
> > + int (*set_plca_cfg)(struct phy_device *dev,
> > + struct netlink_ext_ack *extack,
> > + const struct phy_plca_cfg *plca_cfg);
> > + int (*get_plca_status)(struct phy_device *dev,
> > + struct phy_plca_status *plca_st);
> > int (*start_cable_test)(struct phy_device *phydev,
> > struct netlink_ext_ack *extack);
> > int (*start_cable_test_tdr)(struct phy_device *phydev,
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 71eeb4e3b1fd..ab2c134d0a05 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -765,6 +765,63 @@ struct phy_tdr_config {
> > };
> > #define PHY_PAIR_ALL -1
> >
> > +/**
> > + * struct phy_plca_cfg - Configuration of the PLCA (Physical Layer Collision
> > + * Avoidance) Reconciliation Sublayer.
> > + *
> > + * @version: read-only PLCA register map version. 0 = not available. Ignored
> > + * when setting the configuration. Format is the same as reported by the PLCA
> > + * IDVER register (31.CA00). -1 = not available.
> > + * @enabled: PLCA configured mode (enabled/disabled). -1 = not available / don't
> > + * set. 0 = disabled, anything else = enabled.
> > + * @node_id: the PLCA local node identifier. -1 = not available / don't set.
> > + * Allowed values [0 .. 254]. 255 = node disabled.
> > + * @node_cnt: the PLCA node count (maximum number of nodes having a TO). Only
> > + * meaningful for the coordinator (node_id = 0). -1 = not available / don't
> > + * set. Allowed values [0 .. 255].
> > + * @to_tmr: The value of the PLCA to_timer in bit-times, which determines the
> > + * PLCA transmit opportunity window opening. See IEEE802.3 Clause 148 for
> > + * more details. The to_timer shall be set equal over all nodes.
> > + * -1 = not available / don't set. Allowed values [0 .. 255].
> > + * @burst_cnt: controls how many additional frames a node is allowed to send in
> > + * single transmit opportunity (TO). The default value of 0 means that the
> > + * node is allowed exactly one frame per TO. A value of 1 allows two frames
> > + * per TO, and so on. -1 = not available / don't set.
> > + * Allowed values [0 .. 255].
> > + * @burst_tmr: controls how many bit times to wait for the MAC to send a new
> > + * frame before interrupting the burst. This value should be set to a value
> > + * greater than the MAC inter-packet gap (which is typically 96 bits).
> > + * -1 = not available / don't set. Allowed values [0 .. 255].
> > + *
> > + * A structure containing configuration parameters for setting/getting the PLCA
> > + * RS configuration. The driver does not need to implement all the parameters,
> > + * but should report what is actually used.
> > + */
> > +struct phy_plca_cfg {
> > + s32 version;
> > + s16 enabled;
> > + s16 node_id;
> > + s16 node_cnt;
> > + s16 to_tmr;
> > + s16 burst_cnt;
> > + s16 burst_tmr;
> > +};
> > +
> > +/**
> > + * struct phy_plca_status - Status of the PLCA (Physical Layer Collision
> > + * Avoidance) Reconciliation Sublayer.
> > + *
> > + * @pst: The PLCA status as reported by the PST bit in the PLCA STATUS
> > + * register(31.CA03), indicating BEACON activity.
> > + *
> > + * A structure containing status information of the PLCA RS configuration.
> > + * The driver does not need to implement all the parameters, but should report
> > + * what is actually used.
> > + */
> > +struct phy_plca_status {
> > + bool pst;
> > +};
> > +
> > /**
> > * struct phy_driver - Driver structure for a particular PHY type
> > *
> > @@ -1775,6 +1832,13 @@ 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,
> > + struct phy_plca_cfg *plca_cfg);
> > +int phy_ethtool_set_plca_cfg(struct phy_device *dev,
> > + struct netlink_ext_ack *extack,
> > + const struct phy_plca_cfg *plca_cfg);
> > +int phy_ethtool_get_plca_status(struct phy_device *dev,
> > + 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_netlink.h b/include/uapi/linux/ethtool_netlink.h
> > index aaf7c6963d61..81e3d7b42d0f 100644
> > --- a/include/uapi/linux/ethtool_netlink.h
> > +++ b/include/uapi/linux/ethtool_netlink.h
> > @@ -51,6 +51,9 @@ enum {
> > ETHTOOL_MSG_MODULE_SET,
> > ETHTOOL_MSG_PSE_GET,
> > ETHTOOL_MSG_PSE_SET,
> > + ETHTOOL_MSG_PLCA_GET_CFG,
> > + ETHTOOL_MSG_PLCA_SET_CFG,
> > + ETHTOOL_MSG_PLCA_GET_STATUS,
> >
> > /* add new constants above here */
> > __ETHTOOL_MSG_USER_CNT,
> > @@ -97,6 +100,9 @@ enum {
> > ETHTOOL_MSG_MODULE_GET_REPLY,
> > ETHTOOL_MSG_MODULE_NTF,
> > ETHTOOL_MSG_PSE_GET_REPLY,
> > + ETHTOOL_MSG_PLCA_GET_CFG_REPLY,
> > + ETHTOOL_MSG_PLCA_GET_STATUS_REPLY,
> > + ETHTOOL_MSG_PLCA_NTF,
> >
> > /* add new constants above here */
> > __ETHTOOL_MSG_KERNEL_CNT,
> > @@ -880,6 +886,25 @@ enum {
> > ETHTOOL_A_PSE_MAX = (__ETHTOOL_A_PSE_CNT - 1)
> > };
> >
> > +/* PLCA */
> > +
Hi Oleksij
Please trim the text when replying. It is possible to miss comments
when having to page down, page down, page down all the time.
Thanks
Andrew