netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000
@ 2023-01-06 18:43 Piergiorgio Beruto
  2023-01-06 18:44 ` [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS Piergiorgio Beruto
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:43 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

This patchset adds support for getting/setting the Physical Layer 
Collision Avoidace (PLCA) Reconciliation Sublayer (RS) configuration and
status on Ethernet PHYs that supports it.

PLCA is a feature that provides improved media-access performance in terms
of throughput, latency and fairness for multi-drop (P2MP) half-duplex PHYs.
PLCA is defined in Clause 148 of the IEEE802.3 specifications as amended
by 802.3cg-2019. Currently, PLCA is supported by the 10BASE-T1S single-pair
Ethernet PHY defined in the same standard and related amendments. The OPEN
Alliance SIG TC14 defines additional specifications for the 10BASE-T1S PHY,
including a standard register map for PHYs that embeds the PLCA RS (see
PLCA management registers at https://www.opensig.org/about/specifications/).

The changes proposed herein add the appropriate ethtool netlink interface
for configuring the PLCA RS on PHYs that supports it. A separate patchset
further modifies the ethtool userspace program to show and modify the
configuration/status of the PLCA RS.

Additionally, this patchset adds support for the onsemi NCN26000
Industrial Ethernet 10BASE-T1S PHY that uses the newly added PLCA
infrastructure.

Piergiorgio Beruto (5):
  net/ethtool: add netlink interface for the PLCA RS
  drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY
  drivers/net/phy: add connection between ethtool and phylib for PLCA
  drivers/net/phy: add helpers to get/set PLCA configuration
  drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY

 Documentation/networking/ethtool-netlink.rst | 138 +++++++++
 MAINTAINERS                                  |  14 +
 drivers/net/phy/Kconfig                      |   7 +
 drivers/net/phy/Makefile                     |   1 +
 drivers/net/phy/mdio-open-alliance.h         |  46 +++
 drivers/net/phy/ncn26000.c                   | 173 ++++++++++++
 drivers/net/phy/phy-c45.c                    | 183 ++++++++++++
 drivers/net/phy/phy-core.c                   |   5 +-
 drivers/net/phy/phy.c                        | 174 ++++++++++++
 drivers/net/phy/phy_device.c                 |  17 ++
 drivers/net/phy/phylink.c                    |   6 +-
 include/linux/ethtool.h                      |  12 +
 include/linux/phy.h                          |  84 ++++++
 include/uapi/linux/ethtool.h                 |   3 +
 include/uapi/linux/ethtool_netlink.h         |  25 ++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/common.c                         |   8 +
 net/ethtool/netlink.c                        |  29 ++
 net/ethtool/netlink.h                        |   6 +
 net/ethtool/plca.c                           | 277 +++++++++++++++++++
 20 files changed, 1207 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/phy/mdio-open-alliance.h
 create mode 100644 drivers/net/phy/ncn26000.c
 create mode 100644 net/ethtool/plca.c

-- 
2.37.4


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS
  2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
@ 2023-01-06 18:44 ` Piergiorgio Beruto
  2023-01-07 17:29   ` Andrew Lunn
  2023-01-06 18:44 ` [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY Piergiorgio Beruto
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:44 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

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@gmail.com>
---
 Documentation/networking/ethtool-netlink.rst | 138 +++++++++
 MAINTAINERS                                  |   6 +
 include/linux/ethtool.h                      |  12 +
 include/linux/phy.h                          |  57 ++++
 include/uapi/linux/ethtool_netlink.h         |  25 ++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/netlink.c                        |  29 ++
 net/ethtool/netlink.h                        |   6 +
 net/ethtool/plca.c                           | 277 +++++++++++++++++++
 9 files changed, 551 insertions(+), 1 deletion(-)
 create mode 100644 net/ethtool/plca.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index f10f8eb44255..c59b542eb693 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1716,6 +1716,141 @@ being used. Current supported options are toeplitz, xor or crc32.
 ETHTOOL_A_RSS_INDIR attribute returns RSS indrection table where each byte
 indicates queue number.
 
+PLCA_GET_CFG
+============
+
+Gets the IEEE 802.3cg-2019 Clause 148 Physical Layer Collision Avoidance
+(PLCA) Reconciliation Sublayer (RS) attributes.
+
+Request contents:
+
+  =====================================  ======  ==========================
+  ``ETHTOOL_A_PLCA_HEADER``              nested  request header
+  =====================================  ======  ==========================
+
+Kernel response contents:
+
+  ======================================  ======  =============================
+  ``ETHTOOL_A_PLCA_HEADER``               nested  reply header
+  ``ETHTOOL_A_PLCA_VERSION``              u16     Supported PLCA management
+                                                  interface standard/version
+  ``ETHTOOL_A_PLCA_ENABLED``              u8      PLCA Admin State
+  ``ETHTOOL_A_PLCA_NODE_ID``              u32     PLCA unique local node ID
+  ``ETHTOOL_A_PLCA_NODE_CNT``             u32     Number of PLCA nodes on the
+                                                  network, including the
+                                                  coordinator
+  ``ETHTOOL_A_PLCA_TO_TMR``               u32     Transmit Opportunity Timer
+                                                  value in bit-times (BT)
+  ``ETHTOOL_A_PLCA_BURST_CNT``            u32     Number of additional packets
+                                                  the node is allowed to send
+                                                  within a single TO
+  ``ETHTOOL_A_PLCA_BURST_TMR``            u32     Time to wait for the MAC to
+                                                  transmit a new frame before
+                                                  terminating the burst
+  ======================================  ======  =============================
+
+When set, the optional ``ETHTOOL_A_PLCA_VERSION`` attribute indicates which
+standard and version the PLCA management interface complies to. When not set,
+the interface is vendor-specific and (possibly) supplied by the driver.
+The OPEN Alliance SIG specifies a standard register map for 10BASE-T1S PHYs
+embedding the PLCA Reconcialiation Sublayer. See "10BASE-T1S PLCA Management
+Registers" at https://www.opensig.org/about/specifications/.
+
+When set, the optional ``ETHTOOL_A_PLCA_ENABLED`` attribute indicates the
+administrative state of the PLCA RS. When not set, the node operates in "plain"
+CSMA/CD mode. This option is corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.1
+aPLCAAdminState / 30.16.1.2.1 acPLCAAdminControl.
+
+When set, the optional ``ETHTOOL_A_PLCA_NODE_ID`` attribute indicates the
+configured local node ID of the PHY. This ID determines which transmit
+opportunity (TO) is reserved for the node to transmit into. This option is
+corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.4 aPLCALocalNodeID. The valid
+range for this attribute is [0 .. 255] where 255 means "not configured".
+
+When set, the optional ``ETHTOOL_A_PLCA_NODE_CNT`` attribute indicates the
+configured maximum number of PLCA nodes on the mixing-segment. This number
+determines the total number of transmit opportunities generated during a
+PLCA cycle. This attribute is relevant only for the PLCA coordinator, which is
+the node with aPLCALocalNodeID set to 0. Follower nodes ignore this setting.
+This option is corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.3
+aPLCANodeCount. The valid range for this attribute is [1 .. 255].
+
+When set, the optional ``ETHTOOL_A_PLCA_TO_TMR`` attribute indicates the
+configured value of the transmit opportunity timer in bit-times. This value
+must be set equal across all nodes sharing the medium for PLCA to work
+correctly. This option is corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.5
+aPLCATransmitOpportunityTimer. The valid range for this attribute is
+[0 .. 255].
+
+When set, the optional ``ETHTOOL_A_PLCA_BURST_CNT`` attribute indicates the
+configured number of extra packets that the node is allowed to send during a
+single transmit opportunity. By default, this attribute is 0, meaning that
+the node can only send a sigle frame per TO. When greater than 0, the PLCA RS
+keeps the TO after any transmission, waiting for the MAC to send a new frame
+for up to aPLCABurstTimer BTs. This can only happen a number of times per PLCA
+cycle up to the value of this parameter. After that, the burst is over and the
+normal counting of TOs resumes. This option is corresponding to
+``IEEE 802.3cg-2019`` 30.16.1.1.6 aPLCAMaxBurstCount. The valid range for this
+attribute is [0 .. 255].
+
+When set, the optional ``ETHTOOL_A_PLCA_BURST_TMR`` attribute indicates how
+many bit-times the PLCA RS waits for the MAC to initiate a new transmission
+when aPLCAMaxBurstCount is greater than 0. If the MAC fails to send a new
+frame within this time, the burst ends and the counting of TOs resumes.
+Otherwise, the new frame is sent as part of the current burst. This option
+is corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.7 aPLCABurstTimer. The
+valid range for this attribute is [0 .. 255]. Although, the value should be
+set greater than the Inter-Frame-Gap (IFG) time of the MAC (plus some margin)
+for PLCA burst mode to work as intended.
+
+PLCA_SET_CFG
+============
+
+Sets PLCA RS parameters.
+
+Request contents:
+
+  ======================================  ======  =============================
+  ``ETHTOOL_A_PLCA_HEADER``               nested  request header
+  ``ETHTOOL_A_PLCA_ENABLED``              u8      PLCA Admin State
+  ``ETHTOOL_A_PLCA_NODE_ID``              u8      PLCA unique local node ID
+  ``ETHTOOL_A_PLCA_NODE_CNT``             u8      Number of PLCA nodes on the
+                                                  netkork, including the
+                                                  coordinator
+  ``ETHTOOL_A_PLCA_TO_TMR``               u8      Transmit Opportunity Timer
+                                                  value in bit-times (BT)
+  ``ETHTOOL_A_PLCA_BURST_CNT``            u8      Number of additional packets
+                                                  the node is allowed to send
+                                                  within a single TO
+  ``ETHTOOL_A_PLCA_BURST_TMR``            u8      Time to wait for the MAC to
+                                                  transmit a new frame before
+                                                  terminating the burst
+  ======================================  ======  =============================
+
+For a description of each attribute, see ``PLCA_GET_CFG``.
+
+PLCA_GET_STATUS
+===============
+
+Gets PLCA RS status information.
+
+Request contents:
+
+  =====================================  ======  ==========================
+  ``ETHTOOL_A_PLCA_HEADER``              nested  request header
+  =====================================  ======  ==========================
+
+Kernel response contents:
+
+  ======================================  ======  =============================
+  ``ETHTOOL_A_PLCA_HEADER``               nested  reply header
+  ``ETHTOOL_A_PLCA_STATUS``               u8      PLCA RS operational status
+  ======================================  ======  =============================
+
+When set, the ``ETHTOOL_A_PLCA_STATUS`` attribute indicates whether the node is
+detecting the presence of the BEACON on the network. This flag is
+corresponding to ``IEEE 802.3cg-2019`` 30.16.1.1.2 aPLCAStatus.
+
 Request translation
 ===================
 
@@ -1817,4 +1952,7 @@ are netlink only.
   n/a                                 ``ETHTOOL_MSG_PHC_VCLOCKS_GET``
   n/a                                 ``ETHTOOL_MSG_MODULE_GET``
   n/a                                 ``ETHTOOL_MSG_MODULE_SET``
+  n/a                                 ``ETHTOOL_MSG_PLCA_GET_CFG``
+  n/a                                 ``ETHTOOL_MSG_PLCA_SET_CFG``
+  n/a                                 ``ETHTOOL_MSG_PLCA_GET_STATUS``
   =================================== =====================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 7f0b7181e60a..8faa15c360e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16613,6 +16613,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@gmail.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	net/ethtool/plca.c
+
 PLDMFW LIBRARY
 M:	Jacob Keller <jacob.e.keller@intel.com>
 S:	Maintained
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9e0a76fc7de9..d0da303f6634 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -802,12 +802,17 @@ 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.
+ * @get_plca_status: Get PLCA configuration.
  * @start_cable_test: Start a cable test
  * @start_cable_test_tdr: Start a Time Domain Reflectometry cable test
  *
@@ -819,6 +824,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,
+			    const struct phy_plca_cfg *plca_cfg,
+			    struct netlink_ext_ack *extack);
+	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..c2c04b989cc2 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. -1 = 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 [1 .. 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 {
+	int version;
+	int enabled;
+	int node_id;
+	int node_cnt;
+	int to_tmr;
+	int burst_cnt;
+	int 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
  *
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 5799a9db034e..75b3d6d476ff 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -52,6 +52,9 @@ enum {
 	ETHTOOL_MSG_PSE_GET,
 	ETHTOOL_MSG_PSE_SET,
 	ETHTOOL_MSG_RSS_GET,
+	ETHTOOL_MSG_PLCA_GET_CFG,
+	ETHTOOL_MSG_PLCA_SET_CFG,
+	ETHTOOL_MSG_PLCA_GET_STATUS,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -99,6 +102,9 @@ enum {
 	ETHTOOL_MSG_MODULE_NTF,
 	ETHTOOL_MSG_PSE_GET_REPLY,
 	ETHTOOL_MSG_RSS_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,
@@ -894,6 +900,25 @@ enum {
 	ETHTOOL_A_RSS_MAX = (__ETHTOOL_A_RSS_CNT - 1),
 };
 
+/* PLCA */
+
+enum {
+	ETHTOOL_A_PLCA_UNSPEC,
+	ETHTOOL_A_PLCA_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_PLCA_VERSION,			/* u16 */
+	ETHTOOL_A_PLCA_ENABLED,			/* u8  */
+	ETHTOOL_A_PLCA_STATUS,			/* u8  */
+	ETHTOOL_A_PLCA_NODE_CNT,		/* u32 */
+	ETHTOOL_A_PLCA_NODE_ID,			/* u32 */
+	ETHTOOL_A_PLCA_TO_TMR,			/* u32 */
+	ETHTOOL_A_PLCA_BURST_CNT,		/* u32 */
+	ETHTOOL_A_PLCA_BURST_TMR,		/* u32 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_PLCA_CNT,
+	ETHTOOL_A_PLCA_MAX = (__ETHTOOL_A_PLCA_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 228f13df2e18..563864c1bf5a 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -8,4 +8,4 @@ ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
 		   linkstate.o debug.o wol.o features.o privflags.o rings.o \
 		   channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
 		   tunnels.o fec.o eeprom.o stats.o phc_vclocks.o module.o \
-		   pse-pd.o
+		   pse-pd.o plca.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index aee98be6237f..9f924875bba9 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -288,6 +288,8 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_MODULE_GET]	= &ethnl_module_request_ops,
 	[ETHTOOL_MSG_PSE_GET]		= &ethnl_pse_request_ops,
 	[ETHTOOL_MSG_RSS_GET]		= &ethnl_rss_request_ops,
+	[ETHTOOL_MSG_PLCA_GET_CFG]	= &ethnl_plca_cfg_request_ops,
+	[ETHTOOL_MSG_PLCA_GET_STATUS]	= &ethnl_plca_status_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -603,6 +605,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_EEE_NTF]		= &ethnl_eee_request_ops,
 	[ETHTOOL_MSG_FEC_NTF]		= &ethnl_fec_request_ops,
 	[ETHTOOL_MSG_MODULE_NTF]	= &ethnl_module_request_ops,
+	[ETHTOOL_MSG_PLCA_NTF]		= &ethnl_plca_cfg_request_ops,
 };
 
 /* default notification handler */
@@ -696,6 +699,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
 	[ETHTOOL_MSG_EEE_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_FEC_NTF]		= ethnl_default_notify,
 	[ETHTOOL_MSG_MODULE_NTF]	= ethnl_default_notify,
+	[ETHTOOL_MSG_PLCA_NTF]		= ethnl_default_notify,
 };
 
 void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
@@ -1047,6 +1051,31 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_rss_get_policy,
 		.maxattr = ARRAY_SIZE(ethnl_rss_get_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_PLCA_GET_CFG,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_plca_get_cfg_policy,
+		.maxattr = ARRAY_SIZE(ethnl_plca_get_cfg_policy) - 1,
+	},
+	{
+		.cmd	= ETHTOOL_MSG_PLCA_SET_CFG,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_set_plca_cfg,
+		.policy = ethnl_plca_set_cfg_policy,
+		.maxattr = ARRAY_SIZE(ethnl_plca_set_cfg_policy) - 1,
+	},
+	{
+		.cmd	= ETHTOOL_MSG_PLCA_GET_STATUS,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+		.policy = ethnl_plca_get_status_policy,
+		.maxattr = ARRAY_SIZE(ethnl_plca_get_status_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 3753787ba233..f271266f6e28 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -347,6 +347,8 @@ extern const struct ethnl_request_ops ethnl_phc_vclocks_request_ops;
 extern const struct ethnl_request_ops ethnl_module_request_ops;
 extern const struct ethnl_request_ops ethnl_pse_request_ops;
 extern const struct ethnl_request_ops ethnl_rss_request_ops;
+extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
+extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
 
 extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
 extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
@@ -388,6 +390,9 @@ extern const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MO
 extern const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1];
 extern const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1];
 extern const struct nla_policy ethnl_rss_get_policy[ETHTOOL_A_RSS_CONTEXT + 1];
+extern const struct nla_policy ethnl_plca_get_cfg_policy[ETHTOOL_A_PLCA_HEADER + 1];
+extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1];
+extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
 
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
@@ -408,6 +413,7 @@ int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_module(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_pse(struct sk_buff *skb, struct genl_info *info);
+int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info);
 
 extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
 extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
new file mode 100644
index 000000000000..d9bb13ffc654
--- /dev/null
+++ b/net/ethtool/plca.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/phy.h>
+#include <linux/ethtool_netlink.h>
+
+#include "netlink.h"
+#include "common.h"
+
+struct plca_req_info {
+	struct ethnl_req_info		base;
+};
+
+struct plca_reply_data {
+	struct ethnl_reply_data		base;
+	struct phy_plca_cfg		plca_cfg;
+	struct phy_plca_status		plca_st;
+};
+
+// Helpers ------------------------------------------------------------------ //
+
+#define PLCA_REPDATA(__reply_base) \
+	container_of(__reply_base, struct plca_reply_data, base)
+
+static void plca_update_sint(int *dst, const struct nlattr *attr,
+			     bool *mod)
+{
+	if (!attr)
+		return;
+
+	*dst = nla_get_u32(attr);
+	*mod = true;
+}
+
+// PLCA get configuration message ------------------------------------------- //
+
+const struct nla_policy ethnl_plca_get_cfg_policy[] = {
+	[ETHTOOL_A_PLCA_HEADER]		=
+		NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+static int plca_get_cfg_prepare_data(const struct ethnl_req_info *req_base,
+				     struct ethnl_reply_data *reply_base,
+				     struct genl_info *info)
+{
+	struct plca_reply_data *data = PLCA_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	const struct ethtool_phy_ops *ops;
+	int ret;
+
+	// check that the PHY device is available and connected
+	if (!dev->phydev) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	// note: rtnl_lock is held already by ethnl_default_doit
+	ops = ethtool_phy_ops;
+	if (!ops || !ops->get_plca_cfg) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = ethnl_ops_begin(dev);
+	if (!ret)
+		goto out;
+
+	memset(&data->plca_cfg, 0xff,
+	       sizeof_field(struct plca_reply_data, plca_cfg));
+
+	ret = ops->get_plca_cfg(dev->phydev, &data->plca_cfg);
+	ethnl_ops_complete(dev);
+
+out:
+	return ret;
+}
+
+static int plca_get_cfg_reply_size(const struct ethnl_req_info *req_base,
+				   const struct ethnl_reply_data *reply_base)
+{
+	return nla_total_size(sizeof(u16)) +	/* _VERSION */
+	       nla_total_size(sizeof(u8)) +	/* _ENABLED */
+	       nla_total_size(sizeof(u32)) +	/* _NODE_CNT */
+	       nla_total_size(sizeof(u32)) +	/* _NODE_ID */
+	       nla_total_size(sizeof(u32)) +	/* _TO_TIMER */
+	       nla_total_size(sizeof(u32)) +	/* _BURST_COUNT */
+	       nla_total_size(sizeof(u32));	/* _BURST_TIMER */
+}
+
+static int plca_get_cfg_fill_reply(struct sk_buff *skb,
+				   const struct ethnl_req_info *req_base,
+				   const struct ethnl_reply_data *reply_base)
+{
+	const struct plca_reply_data *data = PLCA_REPDATA(reply_base);
+	const struct phy_plca_cfg *plca = &data->plca_cfg;
+
+	if ((plca->version >= 0 &&
+	     nla_put_u16(skb, ETHTOOL_A_PLCA_VERSION, plca->version)) ||
+	    (plca->enabled >= 0 &&
+	     nla_put_u8(skb, ETHTOOL_A_PLCA_ENABLED, !!plca->enabled)) ||
+	    (plca->node_id >= 0 &&
+	     nla_put_u32(skb, ETHTOOL_A_PLCA_NODE_ID, plca->node_id)) ||
+	    (plca->node_cnt >= 0 &&
+	     nla_put_u32(skb, ETHTOOL_A_PLCA_NODE_CNT, plca->node_cnt)) ||
+	    (plca->to_tmr >= 0 &&
+	     nla_put_u32(skb, ETHTOOL_A_PLCA_TO_TMR, plca->to_tmr)) ||
+	    (plca->burst_cnt >= 0 &&
+	     nla_put_u32(skb, ETHTOOL_A_PLCA_BURST_CNT, plca->burst_cnt)) ||
+	    (plca->burst_tmr >= 0 &&
+	     nla_put_u32(skb, ETHTOOL_A_PLCA_BURST_TMR, plca->burst_tmr)))
+		return -EMSGSIZE;
+
+	return 0;
+};
+
+const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PLCA_GET_CFG,
+	.reply_cmd		= ETHTOOL_MSG_PLCA_GET_CFG_REPLY,
+	.hdr_attr		= ETHTOOL_A_PLCA_HEADER,
+	.req_info_size		= sizeof(struct plca_req_info),
+	.reply_data_size	= sizeof(struct plca_reply_data),
+
+	.prepare_data		= plca_get_cfg_prepare_data,
+	.reply_size		= plca_get_cfg_reply_size,
+	.fill_reply		= plca_get_cfg_fill_reply,
+};
+
+// PLCA set configuration message ------------------------------------------- //
+
+const struct nla_policy ethnl_plca_set_cfg_policy[] = {
+	[ETHTOOL_A_PLCA_HEADER]		=
+		NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_PLCA_ENABLED]	= NLA_POLICY_MAX(NLA_U8, 1),
+	[ETHTOOL_A_PLCA_NODE_ID]	= NLA_POLICY_MAX(NLA_U32, 255),
+	[ETHTOOL_A_PLCA_NODE_CNT]	= NLA_POLICY_RANGE(NLA_U32, 1, 255),
+	[ETHTOOL_A_PLCA_TO_TMR]		= NLA_POLICY_MAX(NLA_U32, 255),
+	[ETHTOOL_A_PLCA_BURST_CNT]	= NLA_POLICY_MAX(NLA_U32, 255),
+	[ETHTOOL_A_PLCA_BURST_TMR]	= NLA_POLICY_MAX(NLA_U32, 255),
+};
+
+int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
+{
+	struct ethnl_req_info req_info = {};
+	struct nlattr **tb = info->attrs;
+	const struct ethtool_phy_ops *ops;
+	struct phy_plca_cfg plca_cfg;
+	struct net_device *dev;
+	bool mod = false;
+	int ret;
+
+	ret = ethnl_parse_header_dev_get(&req_info,
+					 tb[ETHTOOL_A_PLCA_HEADER],
+					 genl_info_net(info), info->extack,
+					 true);
+	if (!ret)
+		return ret;
+
+	dev = req_info.dev;
+
+	rtnl_lock();
+
+	// check that the PHY device is available and connected
+	if (!dev->phydev) {
+		ret = -EOPNOTSUPP;
+		goto out_rtnl;
+	}
+
+	ops = ethtool_phy_ops;
+	if (!ops || !ops->set_plca_cfg) {
+		ret = -EOPNOTSUPP;
+		goto out_rtnl;
+	}
+
+	ret = ethnl_ops_begin(dev);
+	if (!ret)
+		goto out_rtnl;
+
+	memset(&plca_cfg, 0xff, sizeof(plca_cfg));
+	plca_update_sint(&plca_cfg.enabled, tb[ETHTOOL_A_PLCA_ENABLED], &mod);
+	plca_update_sint(&plca_cfg.node_id, tb[ETHTOOL_A_PLCA_NODE_ID], &mod);
+	plca_update_sint(&plca_cfg.node_cnt, tb[ETHTOOL_A_PLCA_NODE_CNT], &mod);
+	plca_update_sint(&plca_cfg.to_tmr, tb[ETHTOOL_A_PLCA_TO_TMR], &mod);
+	plca_update_sint(&plca_cfg.burst_cnt, tb[ETHTOOL_A_PLCA_BURST_CNT],
+			 &mod);
+	plca_update_sint(&plca_cfg.burst_tmr, tb[ETHTOOL_A_PLCA_BURST_TMR],
+			 &mod);
+
+	ret = 0;
+	if (!mod)
+		goto out_ops;
+
+	ret = ops->set_plca_cfg(dev->phydev, &plca_cfg, info->extack);
+	if (!ret)
+		goto out_ops;
+
+	ethtool_notify(dev, ETHTOOL_MSG_PLCA_NTF, NULL);
+
+out_ops:
+	ethnl_ops_complete(dev);
+out_rtnl:
+	rtnl_unlock();
+	ethnl_parse_header_dev_put(&req_info);
+
+	return ret;
+}
+
+// PLCA get status message -------------------------------------------------- //
+
+const struct nla_policy ethnl_plca_get_status_policy[] = {
+	[ETHTOOL_A_PLCA_HEADER]		=
+		NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+static int plca_get_status_prepare_data(const struct ethnl_req_info *req_base,
+					struct ethnl_reply_data *reply_base,
+					struct genl_info *info)
+{
+	struct plca_reply_data *data = PLCA_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	const struct ethtool_phy_ops *ops;
+	int ret;
+
+	// check that the PHY device is available and connected
+	if (!dev->phydev) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	// note: rtnl_lock is held already by ethnl_default_doit
+	ops = ethtool_phy_ops;
+	if (!ops || !ops->get_plca_status) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = ethnl_ops_begin(dev);
+	if (!ret)
+		goto out;
+
+	memset(&data->plca_st, 0xff,
+	       sizeof_field(struct plca_reply_data, plca_st));
+
+	ret = ops->get_plca_status(dev->phydev, &data->plca_st);
+	ethnl_ops_complete(dev);
+out:
+	return ret;
+}
+
+static int plca_get_status_reply_size(const struct ethnl_req_info *req_base,
+				      const struct ethnl_reply_data *reply_base)
+{
+	return nla_total_size(sizeof(u8));	/* _STATUS */
+}
+
+static int plca_get_status_fill_reply(struct sk_buff *skb,
+				      const struct ethnl_req_info *req_base,
+				      const struct ethnl_reply_data *reply_base)
+{
+	const struct plca_reply_data *data = PLCA_REPDATA(reply_base);
+	const u8 status = data->plca_st.pst;
+
+	if (nla_put_u8(skb, ETHTOOL_A_PLCA_STATUS, !!status))
+		return -EMSGSIZE;
+
+	return 0;
+};
+
+const struct ethnl_request_ops ethnl_plca_status_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PLCA_GET_STATUS,
+	.reply_cmd		= ETHTOOL_MSG_PLCA_GET_STATUS_REPLY,
+	.hdr_attr		= ETHTOOL_A_PLCA_HEADER,
+	.req_info_size		= sizeof(struct plca_req_info),
+	.reply_data_size	= sizeof(struct plca_reply_data),
+
+	.prepare_data		= plca_get_status_prepare_data,
+	.reply_size		= plca_get_status_reply_size,
+	.fill_reply		= plca_get_status_fill_reply,
+};
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY
  2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
  2023-01-06 18:44 ` [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS Piergiorgio Beruto
@ 2023-01-06 18:44 ` Piergiorgio Beruto
  2023-01-07 17:31   ` Andrew Lunn
  2023-01-06 18:45 ` [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA Piergiorgio Beruto
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:44 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

This patch adds the link modes for the IEEE 802.3cg Clause 147 10BASE-T1S
Ethernet PHY. According to the specifications, the 10BASE-T1S supports
Point-To-Point Full-Duplex, Point-To-Point Half-Duplex and/or
Point-To-Multipoint (AKA Multi-Drop) Half-Duplex operations.

Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
---
 drivers/net/phy/phy-core.c   |  5 ++++-
 drivers/net/phy/phy_device.c | 14 ++++++++++++++
 drivers/net/phy/phylink.c    |  6 +++++-
 include/linux/phy.h          | 14 ++++++++++++++
 include/uapi/linux/ethtool.h |  3 +++
 net/ethtool/common.c         |  8 ++++++++
 6 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 5d08c627a516..a64186dc53f8 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");
@@ -260,6 +260,9 @@ static const struct phy_setting settings[] = {
 	PHY_SETTING(     10, FULL,     10baseT_Full		),
 	PHY_SETTING(     10, HALF,     10baseT_Half		),
 	PHY_SETTING(     10, FULL,     10baseT1L_Full		),
+	PHY_SETTING(     10, FULL,     10baseT1S_Full		),
+	PHY_SETTING(     10, HALF,     10baseT1S_Half		),
+	PHY_SETTING(     10, HALF,     10baseT1S_P2MP_Half	),
 };
 #undef PHY_SETTING
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 716870a4499c..8e48b3cec5e7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(phy_basic_features);
 __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_t1_features) __ro_after_init;
 EXPORT_SYMBOL_GPL(phy_basic_t1_features);
 
+__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_t1s_p2mp_features) __ro_after_init;
+EXPORT_SYMBOL_GPL(phy_basic_t1s_p2mp_features);
+
 __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_features) __ro_after_init;
 EXPORT_SYMBOL_GPL(phy_gbit_features);
 
@@ -98,6 +101,12 @@ const int phy_basic_t1_features_array[3] = {
 };
 EXPORT_SYMBOL_GPL(phy_basic_t1_features_array);
 
+const int phy_basic_t1s_p2mp_features_array[2] = {
+	ETHTOOL_LINK_MODE_TP_BIT,
+	ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT,
+};
+EXPORT_SYMBOL_GPL(phy_basic_t1s_p2mp_features_array);
+
 const int phy_gbit_features_array[2] = {
 	ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
 	ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
@@ -138,6 +147,11 @@ static void features_init(void)
 			       ARRAY_SIZE(phy_basic_t1_features_array),
 			       phy_basic_t1_features);
 
+	/* 10 half, P2MP, TP */
+	linkmode_set_bit_array(phy_basic_t1s_p2mp_features_array,
+			       ARRAY_SIZE(phy_basic_t1s_p2mp_features_array),
+			       phy_basic_t1s_p2mp_features);
+
 	/* 10/100 half/full + 1000 half/full */
 	linkmode_set_bit_array(phy_basic_ports_array,
 			       ARRAY_SIZE(phy_basic_ports_array),
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 c2c04b989cc2..1e87d3f05209 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -45,6 +45,7 @@
 
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_t1_features) __ro_after_init;
+extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_t1s_p2mp_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_fibre_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_all_ports_features) __ro_after_init;
@@ -54,6 +55,7 @@ extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_ini
 
 #define PHY_BASIC_FEATURES ((unsigned long *)&phy_basic_features)
 #define PHY_BASIC_T1_FEATURES ((unsigned long *)&phy_basic_t1_features)
+#define PHY_BASIC_T1S_P2MP_FEATURES ((unsigned long *)&phy_basic_t1s_p2mp_features)
 #define PHY_GBIT_FEATURES ((unsigned long *)&phy_gbit_features)
 #define PHY_GBIT_FIBRE_FEATURES ((unsigned long *)&phy_gbit_fibre_features)
 #define PHY_GBIT_ALL_PORTS_FEATURES ((unsigned long *)&phy_gbit_all_ports_features)
@@ -66,6 +68,7 @@ extern const int phy_fibre_port_array[1];
 extern const int phy_all_ports_features_array[7];
 extern const int phy_10_100_features_array[4];
 extern const int phy_basic_t1_features_array[3];
+extern const int phy_basic_t1s_p2mp_features_array[2];
 extern const int phy_gbit_features_array[2];
 extern const int phy_10gbit_features_array[1];
 
@@ -1036,6 +1039,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)
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 6f399afc2ff2..5fb19050991e 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.37.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA
  2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
  2023-01-06 18:44 ` [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS Piergiorgio Beruto
  2023-01-06 18:44 ` [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY Piergiorgio Beruto
@ 2023-01-06 18:45 ` Piergiorgio Beruto
  2023-01-07 17:37   ` Andrew Lunn
  2023-01-06 18:45 ` [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration Piergiorgio Beruto
  2023-01-06 18:45 ` [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY Piergiorgio Beruto
  4 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:45 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

This patch adds the required connection between netlink ethtool and
phylib to resolve PLCA get/set config and get status messages.

Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
---
 drivers/net/phy/phy.c        | 174 +++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |   3 +
 include/linux/phy.h          |   7 ++
 3 files changed, 184 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e5b6cb1a77f9..4d67ca234816 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -543,6 +543,180 @@ 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
+ *
+ * Retrieve the PLCA configuration from the PHY. Return 0 on success or a
+ * negative value if an error occurred.
+ */
+int phy_ethtool_get_plca_cfg(struct phy_device *phydev,
+			     struct phy_plca_cfg *plca_cfg)
+{
+	int ret;
+
+	if (!phydev->drv) {
+		ret = -EIO;
+		goto out;
+	}
+
+	if (!phydev->drv->get_plca_cfg) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	mutex_lock(&phydev->lock);
+	ret = phydev->drv->get_plca_cfg(phydev, plca_cfg);
+
+	mutex_unlock(&phydev->lock);
+out:
+	return ret;
+}
+
+/**
+ * phy_ethtool_set_plca_cfg - Set PLCA RS configuration
+ * @phydev: the phy_device struct
+ * @plca_cfg: new PLCA configuration to apply
+ * @extack: extack for reporting useful error messages
+ *
+ * Sets the PLCA configuration in the PHY. Return 0 on success or a
+ * negative value if an error occurred.
+ */
+int phy_ethtool_set_plca_cfg(struct phy_device *phydev,
+			     const struct phy_plca_cfg *plca_cfg,
+			     struct netlink_ext_ack *extack)
+{
+	struct phy_plca_cfg *curr_plca_cfg;
+	int ret;
+
+	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);
+	if (!curr_plca_cfg) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	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 not enabling PLCA, skip a few sanity checks
+	if (plca_cfg->enabled <= 0)
+		goto apply_cfg;
+
+	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;
+	}
+
+apply_cfg:
+	ret = phydev->drv->set_plca_cfg(phydev, plca_cfg);
+
+out_drv:
+	kfree(curr_plca_cfg);
+	mutex_unlock(&phydev->lock);
+out:
+	return ret;
+}
+
+/**
+ * phy_ethtool_get_plca_status - Get PLCA RS status information
+ * @phydev: the phy_device struct
+ * @plca_st: where to store the retrieved status information
+ *
+ * Retrieve the PLCA status information from the PHY. Return 0 on success or a
+ * negative value if an error occurred.
+ */
+int phy_ethtool_get_plca_status(struct phy_device *phydev,
+				struct phy_plca_status *plca_st)
+{
+	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);
+
+	mutex_unlock(&phydev->lock);
+out:
+	return ret;
+}
+
 /**
  * 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 8e48b3cec5e7..44bd06be9691 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3276,6 +3276,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/phy.h b/include/linux/phy.h
index 1e87d3f05209..bcaf1dfd0687 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1846,6 +1846,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 *phydev,
+			     struct phy_plca_cfg *plca_cfg);
+int phy_ethtool_set_plca_cfg(struct phy_device *phydev,
+			     const struct phy_plca_cfg *plca_cfg,
+			     struct netlink_ext_ack *extack);
+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)
 {
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration
  2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
                   ` (2 preceding siblings ...)
  2023-01-06 18:45 ` [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA Piergiorgio Beruto
@ 2023-01-06 18:45 ` Piergiorgio Beruto
  2023-01-07 18:07   ` Andrew Lunn
  2023-01-06 18:45 ` [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY Piergiorgio Beruto
  4 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:45 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

This patch adds support in phylib to read/write PLCA configuration for
Ethernet PHYs that support the OPEN Alliance "10BASE-T1S PLCA
Management Registers" specifications. These can be found at
https://www.opensig.org/about/specifications/

Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
---
 MAINTAINERS                          |   1 +
 drivers/net/phy/mdio-open-alliance.h |  46 +++++++
 drivers/net/phy/phy-c45.c            | 183 +++++++++++++++++++++++++++
 include/linux/phy.h                  |   6 +
 4 files changed, 236 insertions(+)
 create mode 100644 drivers/net/phy/mdio-open-alliance.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8faa15c360e3..4356382ad57c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16617,6 +16617,7 @@ PLCA RECONCILIATION SUBLAYER (IEEE802.3 Clause 148)
 M:	Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	drivers/net/phy/mdio-open-alliance.h
 F:	net/ethtool/plca.c
 
 PLDMFW LIBRARY
diff --git a/drivers/net/phy/mdio-open-alliance.h b/drivers/net/phy/mdio-open-alliance.h
new file mode 100644
index 000000000000..931e14660d75
--- /dev/null
+++ b/drivers/net/phy/mdio-open-alliance.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * mdio-open-alliance.h - definition of OPEN Alliance SIG standard registers
+ */
+
+#ifndef __MDIO_OPEN_ALLIANCE__
+#define __MDIO_OPEN_ALLIANCE__
+
+#include <linux/mdio.h>
+
+/* NOTE: all OATC14 registers are located in MDIO_MMD_VEND2 */
+
+/* Open Alliance TC14 (10BASE-T1S) registers */
+#define MDIO_OATC14_PLCA_IDVER	0xca00  /* PLCA ID and version */
+#define MDIO_OATC14_PLCA_CTRL0	0xca01	/* PLCA Control register 0 */
+#define MDIO_OATC14_PLCA_CTRL1	0xca02	/* PLCA Control register 1 */
+#define MDIO_OATC14_PLCA_STATUS	0xca03	/* PLCA Status register */
+#define MDIO_OATC14_PLCA_TOTMR	0xca04	/* PLCA TO Timer register */
+#define MDIO_OATC14_PLCA_BURST	0xca05	/* PLCA BURST mode register */
+
+/* Open Alliance TC14 PLCA IDVER register */
+#define MDIO_OATC14_PLCA_IDM	0xff00	/* PLCA MAP ID */
+#define MDIO_OATC14_PLCA_VER	0x00ff	/* PLCA MAP version */
+
+/* Open Alliance TC14 PLCA CTRL0 register */
+#define MDIO_OATC14_PLCA_EN	BIT(15) /* PLCA enable */
+#define MDIO_OATC14_PLCA_RST	BIT(14) /* PLCA reset */
+
+/* Open Alliance TC14 PLCA CTRL1 register */
+#define MDIO_OATC14_PLCA_NCNT	0xff00	/* PLCA node count */
+#define MDIO_OATC14_PLCA_ID	0x00ff	/* PLCA local node ID */
+
+/* Open Alliance TC14 PLCA STATUS register */
+#define MDIO_OATC14_PLCA_PST	BIT(15)	/* PLCA status indication */
+
+/* Open Alliance TC14 PLCA TOTMR register */
+#define MDIO_OATC14_PLCA_TOT	0x00ff
+
+/* Open Alliance TC14 PLCA BURST register */
+#define MDIO_OATC14_PLCA_MAXBC	0xff00
+#define MDIO_OATC14_PLCA_BTMR	0x00ff
+
+/* Version Identifiers */
+#define OATC14_IDM		0x0a00
+
+#endif /* __MDIO_OPEN_ALLIANCE__ */
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index a87a4b3ffce4..508edd1f17d7 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -8,6 +8,8 @@
 #include <linux/mii.h>
 #include <linux/phy.h>
 
+#include "mdio-open-alliance.h"
+
 /**
  * genphy_c45_baset1_able - checks if the PMA has BASE-T1 extended abilities
  * @phydev: target phy_device struct
@@ -931,6 +933,187 @@ int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_fast_retrain);
 
+/**
+ * genphy_c45_plca_get_cfg - get PLCA configuration from standard registers
+ * @phydev: target phy_device struct
+ * @plca_cfg: output structure to store the PLCA configuration
+ *
+ * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
+ *   Management Registers specifications, this function can be used to retrieve
+ *   the current PLCA configuration from the standard registers in MMD 31.
+ */
+int genphy_c45_plca_get_cfg(struct phy_device *phydev,
+			    struct phy_plca_cfg *plca_cfg)
+{
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_IDVER);
+	if (ret < 0)
+		return ret;
+
+	if ((ret & MDIO_OATC14_PLCA_IDM) != OATC14_IDM)
+		return -ENODEV;
+
+	plca_cfg->version = ret & ~MDIO_OATC14_PLCA_IDM;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_CTRL0);
+	if (ret < 0)
+		return ret;
+
+	plca_cfg->enabled = !!(ret & MDIO_OATC14_PLCA_EN);
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_CTRL1);
+	if (ret < 0)
+		return ret;
+
+	plca_cfg->node_cnt = (ret & MDIO_OATC14_PLCA_NCNT) >> 8;
+	plca_cfg->node_id = (ret & MDIO_OATC14_PLCA_ID);
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_TOTMR);
+	if (ret < 0)
+		return ret;
+
+	plca_cfg->to_tmr = ret & MDIO_OATC14_PLCA_TOT;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_BURST);
+	if (ret < 0)
+		return ret;
+
+	plca_cfg->burst_cnt = (ret & MDIO_OATC14_PLCA_MAXBC) >> 8;
+	plca_cfg->burst_tmr = (ret & MDIO_OATC14_PLCA_BTMR);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_plca_get_cfg);
+
+/**
+ * genphy_c45_plca_set_cfg - set PLCA configuration using standard registers
+ * @phydev: target phy_device struct
+ * @plca_cfg: structure containing the PLCA configuration. Fields set to -1 are
+ * not to be changed.
+ *
+ * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
+ *   Management Registers specifications, this function can be used to modify
+ *   the PLCA configuration using the standard registers in MMD 31.
+ */
+int genphy_c45_plca_set_cfg(struct phy_device *phydev,
+			    const struct phy_plca_cfg *plca_cfg)
+{
+	int ret;
+	u16 val;
+
+	// PLCA IDVER is read-only
+	if (plca_cfg->version >= 0)
+		return -EINVAL;
+
+	// first of all, disable PLCA if required
+	if (plca_cfg->enabled == 0) {
+		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
+					 MDIO_OATC14_PLCA_CTRL0,
+					 MDIO_OATC14_PLCA_EN);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	if (plca_cfg->node_cnt >= 0 || plca_cfg->node_id >= 0) {
+		if (plca_cfg->node_cnt < 0 || plca_cfg->node_id < 0) {
+			ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+					   MDIO_OATC14_PLCA_CTRL1);
+
+			if (ret < 0)
+				return ret;
+
+			val = ret;
+		}
+
+		if (plca_cfg->node_cnt >= 0)
+			val = (val & ~MDIO_OATC14_PLCA_NCNT) |
+			      (plca_cfg->node_cnt << 8);
+
+		if (plca_cfg->node_id >= 0)
+			val = (val & ~MDIO_OATC14_PLCA_ID) |
+			      (plca_cfg->node_id);
+
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+				    MDIO_OATC14_PLCA_CTRL1, val);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	if (plca_cfg->to_tmr >= 0) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+				    MDIO_OATC14_PLCA_TOTMR,
+				    plca_cfg->to_tmr);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	if (plca_cfg->burst_cnt >= 0 || plca_cfg->burst_tmr >= 0) {
+		if (plca_cfg->burst_cnt < 0 || plca_cfg->burst_tmr < 0) {
+			ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+					   MDIO_OATC14_PLCA_BURST);
+
+			if (ret < 0)
+				return ret;
+
+			val = ret;
+		}
+
+		if (plca_cfg->burst_cnt >= 0)
+			val = (val & ~MDIO_OATC14_PLCA_MAXBC) |
+			      (plca_cfg->burst_cnt << 8);
+
+		if (plca_cfg->burst_tmr >= 0)
+			val = (val & ~MDIO_OATC14_PLCA_BTMR) |
+			      (plca_cfg->burst_tmr);
+
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+				    MDIO_OATC14_PLCA_BURST, val);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	// if we need to enable PLCA, do it at the end
+	if (plca_cfg->enabled > 0) {
+		ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+				       MDIO_OATC14_PLCA_CTRL0,
+				       MDIO_OATC14_PLCA_EN);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_plca_set_cfg);
+
+/**
+ * genphy_c45_plca_get_status - get PLCA status from standard registers
+ * @phydev: target phy_device struct
+ * @plca_st: output structure to store the PLCA status
+ *
+ * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
+ *   Management Registers specifications, this function can be used to retrieve
+ *   the current PLCA status information from the standard registers in MMD 31.
+ */
+int genphy_c45_plca_get_status(struct phy_device *phydev,
+			       struct phy_plca_status *plca_st)
+{
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_STATUS);
+	if (ret < 0)
+		return ret;
+
+	plca_st->pst = !!(ret & MDIO_OATC14_PLCA_PST);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);
+
 struct phy_driver genphy_c45_driver = {
 	.phy_id         = 0xffffffff,
 	.phy_id_mask    = 0xffffffff,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bcaf1dfd0687..23aa23cde940 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1748,6 +1748,12 @@ int genphy_c45_loopback(struct phy_device *phydev, bool enable);
 int genphy_c45_pma_resume(struct phy_device *phydev);
 int genphy_c45_pma_suspend(struct phy_device *phydev);
 int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable);
+int genphy_c45_plca_get_cfg(struct phy_device *phydev,
+			    struct phy_plca_cfg *plca_cfg);
+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);
 
 /* Generic C45 PHY driver */
 extern struct phy_driver genphy_c45_driver;
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
                   ` (3 preceding siblings ...)
  2023-01-06 18:45 ` [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration Piergiorgio Beruto
@ 2023-01-06 18:45 ` Piergiorgio Beruto
  2023-01-07 18:23   ` Andrew Lunn
  4 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-06 18:45 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-kernel, netdev, Oleksij Rempel, mailhol.vincent,
	sudheer.mogilappagari, sbhatta, linux-doc, wangjie125, corbet,
	lkp, gal, gustavoars

This patch adds support for the onsemi NCN26000 10BASE-T1S industrial
Ethernet PHY. The driver supports Point-to-Multipoint operation without
auto-negotiation and with link control handling. The PHY also features
PLCA for improving performance in P2MP mode.

Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
---
 MAINTAINERS                |   7 ++
 drivers/net/phy/Kconfig    |   7 ++
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/ncn26000.c | 173 +++++++++++++++++++++++++++++++++++++
 4 files changed, 188 insertions(+)
 create mode 100644 drivers/net/phy/ncn26000.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4356382ad57c..c1dadb34009d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15581,6 +15581,13 @@ L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	arch/mips/boot/dts/ralink/omega2p.dts
 
+ONSEMI ETHERNET PHY DRIVERS
+M:	Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
+L:	netdev@vger.kernel.org
+S:	Supported
+W:	http://www.onsemi.com
+F:	drivers/net/phy/ncn*
+
 OP-TEE DRIVER
 M:	Jens Wiklander <jens.wiklander@linaro.org>
 L:	op-tee@lists.trustedfirmware.org
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1327290decab..08706efa4b43 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -264,6 +264,13 @@ config NATIONAL_PHY
 	help
 	  Currently supports the DP83865 PHY.
 
+config NCN26000_PHY
+	tristate "onsemi 10BASE-T1S Ethernet PHY"
+	help
+	  Adds support for the onsemi 10BASE-T1S Ethernet PHY.
+	  Currently supports the NCN26000 10BASE-T1S Industrial PHY
+	  with MII interface.
+
 config NXP_C45_TJA11XX_PHY
 	tristate "NXP C45 TJA11XX PHYs"
 	depends on PTP_1588_CLOCK_OPTIONAL
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index f7138d3c896b..b5138066ba04 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_MICROCHIP_T1_PHY)	+= microchip_t1.o
 obj-$(CONFIG_MICROSEMI_PHY)	+= mscc/
 obj-$(CONFIG_MOTORCOMM_PHY)	+= motorcomm.o
 obj-$(CONFIG_NATIONAL_PHY)	+= national.o
+obj-$(CONFIG_NCN26000_PHY)	+= ncn26000.o
 obj-$(CONFIG_NXP_C45_TJA11XX_PHY)	+= nxp-c45-tja11xx.o
 obj-$(CONFIG_NXP_TJA11XX_PHY)	+= nxp-tja11xx.o
 obj-$(CONFIG_QSEMI_PHY)		+= qsemi.o
diff --git a/drivers/net/phy/ncn26000.c b/drivers/net/phy/ncn26000.c
new file mode 100644
index 000000000000..2b2ce49c32fc
--- /dev/null
+++ b/drivers/net/phy/ncn26000.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/*
+ *  Driver for the onsemi 10BASE-T1S NCN26000 PHYs family.
+ *
+ * Copyright 2022 onsemi
+ */
+#include <linux/kernel.h>
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+
+#include "mdio-open-alliance.h"
+
+#define PHY_ID_NCN26000			0x180FF5A1
+
+#define NCN26000_REG_IRQ_CTL            16
+#define NCN26000_REG_IRQ_STATUS         17
+
+// the NCN26000 maps link_ctrl to BMCR_ANENABLE
+#define NCN26000_BCMR_LINK_CTRL_BIT	BMCR_ANENABLE
+
+// the NCN26000 maps link_status to BMSR_ANEGCOMPLETE
+#define NCN26000_BMSR_LINK_STATUS_BIT	BMSR_ANEGCOMPLETE
+
+#define NCN26000_IRQ_LINKST_BIT		BIT(0)
+#define NCN26000_IRQ_PLCAST_BIT		BIT(1)
+#define NCN26000_IRQ_LJABBER_BIT	BIT(2)
+#define NCN26000_IRQ_RJABBER_BIT	BIT(3)
+#define NCN26000_IRQ_PLCAREC_BIT	BIT(4)
+#define NCN26000_IRQ_PHYSCOL_BIT	BIT(5)
+#define NCN26000_IRQ_RESET_BIT		BIT(15)
+
+#define TO_TMR_DEFAULT			32
+
+// driver callbacks --------------------------------------------------------- //
+
+static int ncn26000_config_init(struct phy_device *phydev)
+{
+	/* HW bug workaround: the default value of the PLCA TO_TIMER should be
+	 * 32, where the current version of NCN26000 reports 24. This will be
+	 * fixed in future PHY versions. For the time being, we force the
+	 * correct default here.
+	 */
+	return phy_write_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_TOTMR,
+			     TO_TMR_DEFAULT);
+}
+
+static int ncn26000_config_aneg(struct phy_device *phydev)
+{
+	/* Note: the NCN26000 supports only P2MP link mode. Therefore, AN is not
+	 * supported. However, this function is invoked by phylib to enable the
+	 * PHY, regardless of the AN support.
+	 */
+	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+	phydev->mdix = ETH_TP_MDI;
+
+	// bring up the link
+	return phy_write(phydev, MII_BMCR, NCN26000_BCMR_LINK_CTRL_BIT);
+}
+
+static int ncn26000_read_status(struct phy_device *phydev)
+{
+	/* The NCN26000 reports NCN26000_LINK_STATUS_BIT if the link status of
+	 * the PHY is up. It further reports the logical AND of the link status
+	 * and the PLCA status in the BMSR_LSTATUS bit.
+	 */
+	int ret;
+
+	/* The link state is latched low so that momentary link
+	 * drops can be detected. Do not double-read the status
+	 * in polling mode to detect such short link drops except
+	 * the link was already down.
+	 */
+	if (!phy_polling_mode(phydev) || !phydev->link) {
+		ret = phy_read(phydev, MII_BMSR);
+		if (ret < 0)
+			return ret;
+		else if (ret & NCN26000_BMSR_LINK_STATUS_BIT)
+			goto upd_link;
+	}
+
+	ret = phy_read(phydev, MII_BMSR);
+	if (unlikely(ret < 0))
+		return ret;
+
+upd_link:
+	// update link status
+	if (ret & NCN26000_BMSR_LINK_STATUS_BIT) {
+		phydev->link = 1;
+		phydev->pause = 0;
+		phydev->duplex = DUPLEX_HALF;
+		phydev->speed = SPEED_10;
+	} else {
+		phydev->link = 0;
+		phydev->duplex = DUPLEX_UNKNOWN;
+		phydev->speed = SPEED_UNKNOWN;
+	}
+
+	return 0;
+}
+
+static irqreturn_t ncn26000_handle_interrupt(struct phy_device *phydev)
+{
+	int ret;
+
+	// read and aknowledge the IRQ status register
+	ret = phy_read(phydev, NCN26000_REG_IRQ_STATUS);
+
+	// check only link status changes
+	if (unlikely(ret < 0) || (ret & NCN26000_REG_IRQ_STATUS) == 0)
+		return IRQ_NONE;
+
+	phy_trigger_machine(phydev);
+	return IRQ_HANDLED;
+}
+
+static int ncn26000_config_intr(struct phy_device *phydev)
+{
+	int ret;
+	u16 irqe;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		// acknowledge IRQs
+		ret = phy_read(phydev, NCN26000_REG_IRQ_STATUS);
+		if (ret < 0)
+			return ret;
+
+		// get link status notifications
+		irqe = NCN26000_IRQ_LINKST_BIT;
+	} else {
+		// disable all IRQs
+		irqe = 0;
+	}
+
+	ret = phy_write(phydev, NCN26000_REG_IRQ_CTL, irqe);
+	if (ret != 0)
+		return ret;
+
+	return 0;
+}
+
+static struct phy_driver ncn26000_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(PHY_ID_NCN26000),
+		.name			= "NCN26000",
+		.features		= PHY_BASIC_T1S_P2MP_FEATURES,
+		.config_init            = ncn26000_config_init,
+		.config_intr            = ncn26000_config_intr,
+		.config_aneg		= ncn26000_config_aneg,
+		.read_status		= ncn26000_read_status,
+		.handle_interrupt       = ncn26000_handle_interrupt,
+		.get_plca_cfg		= genphy_c45_plca_get_cfg,
+		.set_plca_cfg		= genphy_c45_plca_set_cfg,
+		.get_plca_status	= genphy_c45_plca_get_status,
+		.soft_reset             = genphy_soft_reset,
+	},
+};
+
+module_phy_driver(ncn26000_driver);
+
+static struct mdio_device_id __maybe_unused ncn26000_tbl[] = {
+	{ PHY_ID_MATCH_MODEL(PHY_ID_NCN26000) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, ncn26000_tbl);
+
+MODULE_AUTHOR("Piergiorgio Beruto");
+MODULE_DESCRIPTION("onsemi 10BASE-T1S PHY driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS
  2023-01-06 18:44 ` [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS Piergiorgio Beruto
@ 2023-01-07 17:29   ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2023-01-07 17:29 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Fri, Jan 06, 2023 at 07:44:23PM +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@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY
  2023-01-06 18:44 ` [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY Piergiorgio Beruto
@ 2023-01-07 17:31   ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2023-01-07 17:31 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Fri, Jan 06, 2023 at 07:44:44PM +0100, Piergiorgio Beruto wrote:
> This patch adds the link modes for the IEEE 802.3cg Clause 147 10BASE-T1S
> Ethernet PHY. According to the specifications, the 10BASE-T1S supports
> Point-To-Point Full-Duplex, Point-To-Point Half-Duplex and/or
> Point-To-Multipoint (AKA Multi-Drop) Half-Duplex operations.
> 
> Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA
  2023-01-06 18:45 ` [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA Piergiorgio Beruto
@ 2023-01-07 17:37   ` Andrew Lunn
  2023-01-08 23:48     ` Piergiorgio Beruto
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2023-01-07 17:37 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

> +	// if not enabling PLCA, skip a few sanity checks
> +	if (plca_cfg->enabled <= 0)
> +		goto apply_cfg;
> +
> +	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;
> +	}
> +
> +apply_cfg:
> +	ret = phydev->drv->set_plca_cfg(phydev, plca_cfg);

Goto which don't jump to the end of the function is generally frowned
upon. I suggest you put these sanity checks into a little helper, so
you can avoid the goto.

With that change make, feel free to add my reviewed-by.

     Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration
  2023-01-06 18:45 ` [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration Piergiorgio Beruto
@ 2023-01-07 18:07   ` Andrew Lunn
  2023-01-08 23:50     ` Piergiorgio Beruto
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2023-01-07 18:07 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

> +/**
> + * genphy_c45_plca_set_cfg - set PLCA configuration using standard registers
> + * @phydev: target phy_device struct
> + * @plca_cfg: structure containing the PLCA configuration. Fields set to -1 are
> + * not to be changed.
> + *
> + * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
> + *   Management Registers specifications, this function can be used to modify
> + *   the PLCA configuration using the standard registers in MMD 31.
> + */
> +int genphy_c45_plca_set_cfg(struct phy_device *phydev,
> +			    const struct phy_plca_cfg *plca_cfg)
> +{
> +	int ret;
> +	u16 val;
> +
> +	// PLCA IDVER is read-only
> +	if (plca_cfg->version >= 0)
> +		return -EINVAL;
> +
> +	// first of all, disable PLCA if required
> +	if (plca_cfg->enabled == 0) {
> +		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> +					 MDIO_OATC14_PLCA_CTRL0,
> +					 MDIO_OATC14_PLCA_EN);
> +
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (plca_cfg->node_cnt >= 0 || plca_cfg->node_id >= 0) {
> +		if (plca_cfg->node_cnt < 0 || plca_cfg->node_id < 0) {

I think it would be good to add some comments since this code is not
immediately obvious to me. I had to think about it for a while.

> +	if (plca_cfg->burst_cnt >= 0 || plca_cfg->burst_tmr >= 0) {
> +		if (plca_cfg->burst_cnt < 0 || plca_cfg->burst_tmr < 0) {
> +			ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> +					   MDIO_OATC14_PLCA_BURST);
> +

This follows the same patterns, so maybe comments here as well?

With that, you can add my Reviewed-by.

     Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-06 18:45 ` [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY Piergiorgio Beruto
@ 2023-01-07 18:23   ` Andrew Lunn
  2023-01-08 23:57     ` Piergiorgio Beruto
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2023-01-07 18:23 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

> +++ b/drivers/net/phy/Kconfig
> @@ -264,6 +264,13 @@ config NATIONAL_PHY
>  	help
>  	  Currently supports the DP83865 PHY.
>  
> +config NCN26000_PHY
> +	tristate "onsemi 10BASE-T1S Ethernet PHY"
> +	help
> +	  Adds support for the onsemi 10BASE-T1S Ethernet PHY.
> +	  Currently supports the NCN26000 10BASE-T1S Industrial PHY
> +	  with MII interface.
> +
>  config NXP_C45_TJA11XX_PHY
>  	tristate "NXP C45 TJA11XX PHYs"

These are actually sorted by the tristate string, which is what you
see when you use

make menuconfig

So 'onsemi' should be after 'NXP TJA11xx PHYs support'. Also, all the
other entries capitalise the first word.

>  	depends on PTP_1588_CLOCK_OPTIONAL
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index f7138d3c896b..b5138066ba04 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -77,6 +77,7 @@ obj-$(CONFIG_MICROCHIP_T1_PHY)	+= microchip_t1.o
>  obj-$(CONFIG_MICROSEMI_PHY)	+= mscc/
>  obj-$(CONFIG_MOTORCOMM_PHY)	+= motorcomm.o
>  obj-$(CONFIG_NATIONAL_PHY)	+= national.o
> +obj-$(CONFIG_NCN26000_PHY)	+= ncn26000.o
>  obj-$(CONFIG_NXP_C45_TJA11XX_PHY)	+= nxp-c45-tja11xx.o
>  obj-$(CONFIG_NXP_TJA11XX_PHY)	+= nxp-tja11xx.o
>  obj-$(CONFIG_QSEMI_PHY)		+= qsemi.o

This is sorted by CONFIG_ symbol, so is correct.

> +
> +// driver callbacks --------------------------------------------------------- //

Comments like this don't really add any value.

> +static irqreturn_t ncn26000_handle_interrupt(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	// read and aknowledge the IRQ status register
> +	ret = phy_read(phydev, NCN26000_REG_IRQ_STATUS);
> +
> +	// check only link status changes
> +	if (unlikely(ret < 0) || (ret & NCN26000_REG_IRQ_STATUS) == 0)
> +		return IRQ_NONE;

More usage of unlikely(). If this was on the hot path, handling 10M
frames a second, then maybe unlikley() could be justified. But how
often do you get PHY interrupts? Once a day?

      Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA
  2023-01-07 17:37   ` Andrew Lunn
@ 2023-01-08 23:48     ` Piergiorgio Beruto
  0 siblings, 0 replies; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-08 23:48 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

Hello Andrew,
thank you very much for your review.
I have fixed this issue.

Piergiorgio

On Sat, Jan 07, 2023 at 06:37:52PM +0100, Andrew Lunn wrote:
> > +	// if not enabling PLCA, skip a few sanity checks
> > +	if (plca_cfg->enabled <= 0)
> > +		goto apply_cfg;
> > +
> > +	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;
> > +	}
> > +
> > +apply_cfg:
> > +	ret = phydev->drv->set_plca_cfg(phydev, plca_cfg);
> 
> Goto which don't jump to the end of the function is generally frowned
> upon. I suggest you put these sanity checks into a little helper, so
> you can avoid the goto.
> 
> With that change make, feel free to add my reviewed-by.
> 
>      Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration
  2023-01-07 18:07   ` Andrew Lunn
@ 2023-01-08 23:50     ` Piergiorgio Beruto
  0 siblings, 0 replies; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-08 23:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Sat, Jan 07, 2023 at 07:07:52PM +0100, Andrew Lunn wrote:
> > +/**
> > + * genphy_c45_plca_set_cfg - set PLCA configuration using standard registers
> > + * @phydev: target phy_device struct
> > + * @plca_cfg: structure containing the PLCA configuration. Fields set to -1 are
> > + * not to be changed.
> > + *
> > + * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
> > + *   Management Registers specifications, this function can be used to modify
> > + *   the PLCA configuration using the standard registers in MMD 31.
> > + */
> > +int genphy_c45_plca_set_cfg(struct phy_device *phydev,
> > +			    const struct phy_plca_cfg *plca_cfg)
> > +{
> > +	int ret;
> > +	u16 val;
> > +
> > +	// PLCA IDVER is read-only
> > +	if (plca_cfg->version >= 0)
> > +		return -EINVAL;
> > +
> > +	// first of all, disable PLCA if required
> > +	if (plca_cfg->enabled == 0) {
> > +		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
> > +					 MDIO_OATC14_PLCA_CTRL0,
> > +					 MDIO_OATC14_PLCA_EN);
> > +
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> > +	if (plca_cfg->node_cnt >= 0 || plca_cfg->node_id >= 0) {
> > +		if (plca_cfg->node_cnt < 0 || plca_cfg->node_id < 0) {
> 
> I think it would be good to add some comments since this code is not
> immediately obvious to me. I had to think about it for a while.
Yes, I realize it is not very intuitive at furst glance :-)
I have added a comment explaining that we need to read back the register
to perform merge/pure of the configuration IFF we need to set one among
node ID / node count but not both.

> 
> > +	if (plca_cfg->burst_cnt >= 0 || plca_cfg->burst_tmr >= 0) {
> > +		if (plca_cfg->burst_cnt < 0 || plca_cfg->burst_tmr < 0) {
> > +			ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> > +					   MDIO_OATC14_PLCA_BURST);
> > +
> 
> This follows the same patterns, so maybe comments here as well?
Yup, added the same comment.
> 
> With that, you can add my Reviewed-by.
Thanks!
> 
>      Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-07 18:23   ` Andrew Lunn
@ 2023-01-08 23:57     ` Piergiorgio Beruto
  2023-01-09 13:31       ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-08 23:57 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Sat, Jan 07, 2023 at 07:23:59PM +0100, Andrew Lunn wrote:
> > +++ b/drivers/net/phy/Kconfig
> > @@ -264,6 +264,13 @@ config NATIONAL_PHY
> >  	help
> >  	  Currently supports the DP83865 PHY.
> >  
> > +config NCN26000_PHY
> > +	tristate "onsemi 10BASE-T1S Ethernet PHY"
> > +	help
> > +	  Adds support for the onsemi 10BASE-T1S Ethernet PHY.
> > +	  Currently supports the NCN26000 10BASE-T1S Industrial PHY
> > +	  with MII interface.
> > +
> >  config NXP_C45_TJA11XX_PHY
> >  	tristate "NXP C45 TJA11XX PHYs"
> 
> These are actually sorted by the tristate string, which is what you
> see when you use
> 
> make menuconfig
> 
> So 'onsemi' should be after 'NXP TJA11xx PHYs support'. Also, all the
> other entries capitalise the first word.
As for the order I fixed it. Thanks for noticing.

Regarding the capitalization, I have a little problem here. 'onsemi' is a
brand and according to company rules it MUST be written all in
lowercase. I know we're not obliged to follow any company directive here, but 
as wierd as it might sound, I'd rather keep it lowercase just not to get 
comments later on trying to fix this, if you agree...

> 
> >  	depends on PTP_1588_CLOCK_OPTIONAL
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index f7138d3c896b..b5138066ba04 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -77,6 +77,7 @@ obj-$(CONFIG_MICROCHIP_T1_PHY)	+= microchip_t1.o
> >  obj-$(CONFIG_MICROSEMI_PHY)	+= mscc/
> >  obj-$(CONFIG_MOTORCOMM_PHY)	+= motorcomm.o
> >  obj-$(CONFIG_NATIONAL_PHY)	+= national.o
> > +obj-$(CONFIG_NCN26000_PHY)	+= ncn26000.o
> >  obj-$(CONFIG_NXP_C45_TJA11XX_PHY)	+= nxp-c45-tja11xx.o
> >  obj-$(CONFIG_NXP_TJA11XX_PHY)	+= nxp-tja11xx.o
> >  obj-$(CONFIG_QSEMI_PHY)		+= qsemi.o
> 
> This is sorted by CONFIG_ symbol, so is correct.
> 
> > +
> > +// driver callbacks --------------------------------------------------------- //
> 
> Comments like this don't really add any value.
Sure, I can remove it.
> 
> > +static irqreturn_t ncn26000_handle_interrupt(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	// read and aknowledge the IRQ status register
> > +	ret = phy_read(phydev, NCN26000_REG_IRQ_STATUS);
> > +
> > +	// check only link status changes
> > +	if (unlikely(ret < 0) || (ret & NCN26000_REG_IRQ_STATUS) == 0)
> > +		return IRQ_NONE;
> 
> More usage of unlikely(). If this was on the hot path, handling 10M
> frames a second, then maybe unlikley() could be justified. But how
> often do you get PHY interrupts? Once a day?
Right, it is my instinct to use unlikely on any sanity check which is
effectively unlikely to happen. But I understand it is not needed here.

> 
>       Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-08 23:57     ` Piergiorgio Beruto
@ 2023-01-09 13:31       ` Andrew Lunn
  2023-01-09 14:21         ` Piergiorgio Beruto
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2023-01-09 13:31 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Mon, Jan 09, 2023 at 12:57:03AM +0100, Piergiorgio Beruto wrote:
> On Sat, Jan 07, 2023 at 07:23:59PM +0100, Andrew Lunn wrote:
> > > +++ b/drivers/net/phy/Kconfig
> > > @@ -264,6 +264,13 @@ config NATIONAL_PHY
> > >  	help
> > >  	  Currently supports the DP83865 PHY.
> > >  
> > > +config NCN26000_PHY
> > > +	tristate "onsemi 10BASE-T1S Ethernet PHY"
> > > +	help
> > > +	  Adds support for the onsemi 10BASE-T1S Ethernet PHY.
> > > +	  Currently supports the NCN26000 10BASE-T1S Industrial PHY
> > > +	  with MII interface.
> > > +
> > >  config NXP_C45_TJA11XX_PHY
> > >  	tristate "NXP C45 TJA11XX PHYs"
> > 
> > These are actually sorted by the tristate string, which is what you
> > see when you use
> > 
> > make menuconfig
> > 
> > So 'onsemi' should be after 'NXP TJA11xx PHYs support'. Also, all the
> > other entries capitalise the first word.
> As for the order I fixed it. Thanks for noticing.
> 
> Regarding the capitalization, I have a little problem here. 'onsemi' is a
> brand and according to company rules it MUST be written all in
> lowercase. I know we're not obliged to follow any company directive here, but 
> as wierd as it might sound, I'd rather keep it lowercase just not to get 
> comments later on trying to fix this, if you agree...

Linux tends to ignore Marketing, because Marketing tends to change its
mind every 6 months. Also, Linux ignores companies being bought and
sold, changing their name. So this PHY will forever be called whatever
name you give it here. The vitesse PHY driver is an example of
this. They got bought by Microsemi, and then Microchip bought
Microsemi. The PHY driver is still called vitesse.c.

How about using the legal name, 'ON Semiconductor
Corporation'

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-09 13:31       ` Andrew Lunn
@ 2023-01-09 14:21         ` Piergiorgio Beruto
  2023-01-09 14:24           ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-09 14:21 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Mon, Jan 09, 2023 at 02:31:39PM +0100, Andrew Lunn wrote:
> Linux tends to ignore Marketing, because Marketing tends to change its
> mind every 6 months. Also, Linux ignores companies being bought and
> sold, changing their name. So this PHY will forever be called whatever
> name you give it here. The vitesse PHY driver is an example of
> this. They got bought by Microsemi, and then Microchip bought
> Microsemi. The PHY driver is still called vitesse.c.
> 
> How about using the legal name, 'ON Semiconductor
> Corporation'
That's perfectly clear Andrew, I can certainly see why Linux should
ignore marketing.

Sill, 'ON Semiconductor' is the old company name, the product datasheet can be
found at www.onsemi.com. I would honestly feel more comfortable using
the current company name. If you really want the first 'o' to be
capitalized, then so be it. Hopefully people will not notice :-)

Thanks,
Piergiorgio

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-09 14:21         ` Piergiorgio Beruto
@ 2023-01-09 14:24           ` Andrew Lunn
  2023-01-09 17:12             ` Piergiorgio Beruto
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2023-01-09 14:24 UTC (permalink / raw)
  To: Piergiorgio Beruto
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Mon, Jan 09, 2023 at 03:21:18PM +0100, Piergiorgio Beruto wrote:
> On Mon, Jan 09, 2023 at 02:31:39PM +0100, Andrew Lunn wrote:
> > Linux tends to ignore Marketing, because Marketing tends to change its
> > mind every 6 months. Also, Linux ignores companies being bought and
> > sold, changing their name. So this PHY will forever be called whatever
> > name you give it here. The vitesse PHY driver is an example of
> > this. They got bought by Microsemi, and then Microchip bought
> > Microsemi. The PHY driver is still called vitesse.c.
> > 
> > How about using the legal name, 'ON Semiconductor
> > Corporation'
> That's perfectly clear Andrew, I can certainly see why Linux should
> ignore marketing.
> 
> Sill, 'ON Semiconductor' is the old company name, the product datasheet can be
> found at www.onsemi.com. I would honestly feel more comfortable using
> the current company name. If you really want the first 'o' to be
> capitalized, then so be it. Hopefully people will not notice :-)

I would prefer it was capitalised, to make it uniform with all the
other entries.

      Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY
  2023-01-09 14:24           ` Andrew Lunn
@ 2023-01-09 17:12             ` Piergiorgio Beruto
  0 siblings, 0 replies; 18+ messages in thread
From: Piergiorgio Beruto @ 2023-01-09 17:12 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel, netdev,
	Oleksij Rempel, mailhol.vincent, sudheer.mogilappagari, sbhatta,
	linux-doc, wangjie125, corbet, lkp, gal, gustavoars

On Mon, Jan 09, 2023 at 03:24:26PM +0100, Andrew Lunn wrote:
> On Mon, Jan 09, 2023 at 03:21:18PM +0100, Piergiorgio Beruto wrote:
> > On Mon, Jan 09, 2023 at 02:31:39PM +0100, Andrew Lunn wrote:
> > > Linux tends to ignore Marketing, because Marketing tends to change its
> > > mind every 6 months. Also, Linux ignores companies being bought and
> > > sold, changing their name. So this PHY will forever be called whatever
> > > name you give it here. The vitesse PHY driver is an example of
> > > this. They got bought by Microsemi, and then Microchip bought
> > > Microsemi. The PHY driver is still called vitesse.c.
> > > 
> > > How about using the legal name, 'ON Semiconductor
> > > Corporation'
> > That's perfectly clear Andrew, I can certainly see why Linux should
> > ignore marketing.
> > 
> > Sill, 'ON Semiconductor' is the old company name, the product datasheet can be
> > found at www.onsemi.com. I would honestly feel more comfortable using
> > the current company name. If you really want the first 'o' to be
> > capitalized, then so be it. Hopefully people will not notice :-)
> 
> I would prefer it was capitalised, to make it uniform with all the
> other entries.
I've just re-submitted the patches, fixing also a warning on a
multi-line comment w/o newline.

Thanks!
Piergiorgio

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-01-09 17:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-06 18:43 [PATCH v2 net-next 0/5] add PLCA RS support and onsemi NCN26000 Piergiorgio Beruto
2023-01-06 18:44 ` [PATCH v2 net-next 1/5] net/ethtool: add netlink interface for the PLCA RS Piergiorgio Beruto
2023-01-07 17:29   ` Andrew Lunn
2023-01-06 18:44 ` [PATCH v2 net-next 2/5] drivers/net/phy: add the link modes for the 10BASE-T1S Ethernet PHY Piergiorgio Beruto
2023-01-07 17:31   ` Andrew Lunn
2023-01-06 18:45 ` [PATCH v2 net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA Piergiorgio Beruto
2023-01-07 17:37   ` Andrew Lunn
2023-01-08 23:48     ` Piergiorgio Beruto
2023-01-06 18:45 ` [PATCH v2 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration Piergiorgio Beruto
2023-01-07 18:07   ` Andrew Lunn
2023-01-08 23:50     ` Piergiorgio Beruto
2023-01-06 18:45 ` [PATCH v2 net-next 5/5] drivers/net/phy: add driver for the onsemi NCN26000 10BASE-T1S PHY Piergiorgio Beruto
2023-01-07 18:23   ` Andrew Lunn
2023-01-08 23:57     ` Piergiorgio Beruto
2023-01-09 13:31       ` Andrew Lunn
2023-01-09 14:21         ` Piergiorgio Beruto
2023-01-09 14:24           ` Andrew Lunn
2023-01-09 17:12             ` Piergiorgio Beruto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).