diff mbox series

[RFC,v2,net-next,01/15] phy: introduce phy_get_status() and use it to report CDR lock

Message ID 20230923134904.3627402-2-vladimir.oltean@nxp.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Add C72/C73 copper backplane support for LX2160 | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1445 this patch: 1445
netdev/cc_maintainers success CCed 3 of 3 maintainers
netdev/build_clang success Errors and warnings before: 1376 this patch: 1376
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1483 this patch: 1483
netdev/checkpatch warning WARNING: ENOSYS means 'invalid syscall nr' and nothing else
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean Sept. 23, 2023, 1:48 p.m. UTC
Some modules, like the MTIP AN/LT block used as a copper backplane PHY
driver, need this extra information from the SerDes PHY as another
source of "link up" information.

Namely, the 25GBase-R PCS does not have a MDIO_CTRL1_LPOWER bit
implemented in its MDIO_MMD_PCS:MDIO_CTRL1 register. That bit is
typically set from phy_suspend() or phylink_pcs_disable() implementations,
and that is supposed to cause a link drop event on the link partner.
But here it does not happen.

By implementing the networking phylink_pcs_disable() as phy_power_off(),
we are able to actually power down the lane in a way that is visible to
the remote end. Where it is visible is the CDR lock, so we introduce
PHY_STATUS_TYPE_CDR_LOCK as an extra link indication, we are able to
detect that condition and signal it to upper layers of the network
stack.

A more high-level and generic phy_get_status() operation was chosen
instead of the more specific phy_get_cdr_lock() alternative, because I
saw this as being more in the spirit of the generic PHY API.
Also, phy_get_status() is more extensible and reusable for other
purposes as well.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: reimplement phy_check_cdr_lock() as something more generic

 drivers/phy/phy-core.c  | 31 ++++++++++++++++++++++++++
 include/linux/phy/phy.h | 49 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

Comments

Florian Fainelli Oct. 2, 2023, 7:16 p.m. UTC | #1
On 9/23/23 06:48, Vladimir Oltean wrote:
> Some modules, like the MTIP AN/LT block used as a copper backplane PHY
> driver, need this extra information from the SerDes PHY as another
> source of "link up" information.
> 
> Namely, the 25GBase-R PCS does not have a MDIO_CTRL1_LPOWER bit
> implemented in its MDIO_MMD_PCS:MDIO_CTRL1 register. That bit is
> typically set from phy_suspend() or phylink_pcs_disable() implementations,
> and that is supposed to cause a link drop event on the link partner.
> But here it does not happen.
> 
> By implementing the networking phylink_pcs_disable() as phy_power_off(),
> we are able to actually power down the lane in a way that is visible to
> the remote end. Where it is visible is the CDR lock, so we introduce
> PHY_STATUS_TYPE_CDR_LOCK as an extra link indication, we are able to
> detect that condition and signal it to upper layers of the network
> stack.
> 
> A more high-level and generic phy_get_status() operation was chosen
> instead of the more specific phy_get_cdr_lock() alternative, because I
> saw this as being more in the spirit of the generic PHY API.
> Also, phy_get_status() is more extensible and reusable for other
> purposes as well.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
diff mbox series

Patch

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 96a0b1e111f3..3b7e04a59a00 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -553,6 +553,37 @@  int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 }
 EXPORT_SYMBOL_GPL(phy_validate);
 
+/**
+ * phy_get_status() - Query various parameters of a PHY
+ * @phy: the phy returned by phy_get()
+ * @type: type of the status being queried
+ * @opts: pointer to union of status structures, determined by type
+ *
+ * phy_init() must have been called on the phy. The status is relative to the
+ * current phy mode, that can be changed using phy_set_mode(). Not all status
+ * types may be relevant to all phy modes.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_get_status(struct phy *phy, enum phy_status_type type,
+		   union phy_status_opts *opts)
+{
+	int ret;
+
+	if (!phy)
+		return -EINVAL;
+
+	if (!phy->ops->get_status)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->get_status(phy, type, opts);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_get_status);
+
 /**
  * _of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @np: device_node for which to get the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index f6d607ef0e80..6be348f1fa0e 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -51,6 +51,29 @@  enum phy_media {
 	PHY_MEDIA_DAC,
 };
 
+enum phy_status_type {
+	/* Valid for PHY_MODE_ETHERNET */
+	PHY_STATUS_CDR_LOCK,
+};
+
+/* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit
+ * stream, it means that the stream contains valid bit transitions for the
+ * configured protocol. This indicates that a link partner is physically
+ * present and powered on.
+ */
+struct phy_status_opts_cdr {
+	bool cdr_locked;
+};
+
+/**
+ * union phy_status_opts - Opaque generic phy status
+ *
+ * @cdr:	Configuration set applicable for PHY_STATUS_CDR_LOCK.
+ */
+union phy_status_opts {
+	struct phy_status_opts_cdr		cdr;
+};
+
 /**
  * union phy_configure_opts - Opaque generic phy configuration
  *
@@ -78,6 +101,7 @@  union phy_configure_opts {
  * @set_speed: set the speed of the phy (optional)
  * @reset: resetting the phy
  * @calibrate: calibrate the phy
+ * @get_status: get the mode-specific status of the phy
  * @release: ops to be performed while the consumer relinquishes the PHY
  * @owner: the module owner containing the ops
  */
@@ -122,6 +146,20 @@  struct phy_ops {
 			    union phy_configure_opts *opts);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
+
+	/**
+	 * @get_status:
+	 *
+	 * Optional.
+	 *
+	 * Used to query the mode-specific status of the phy. Must have no side
+	 * effects.
+	 *
+	 * Returns: 0 if the operation was successful, negative error code
+	 * otherwise.
+	 */
+	int	(*get_status)(struct phy *phy, enum phy_status_type type,
+			      union phy_status_opts *opts);
 	void	(*release)(struct phy *phy);
 	struct module *owner;
 };
@@ -236,6 +274,8 @@  int phy_set_speed(struct phy *phy, int speed);
 int phy_configure(struct phy *phy, union phy_configure_opts *opts);
 int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 		 union phy_configure_opts *opts);
+int phy_get_status(struct phy *phy, enum phy_status_type type,
+		   union phy_status_opts *opts);
 
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
@@ -414,6 +454,15 @@  static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 	return -ENOSYS;
 }
 
+static inline int phy_get_status(struct phy *phy, enum phy_status_type type,
+				 union phy_status_opts *opts)
+{
+	if (!phy)
+		return 0;
+
+	return -ENOSYS;
+}
+
 static inline int phy_get_bus_width(struct phy *phy)
 {
 	return -ENOSYS;