diff mbox series

[net-next,5/7] net: hibmcge: Add mac link exception handling feature in this module

Message ID 20250213035529.2402283-6-shaojijie@huawei.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Support some enhances features for the HIBMCGE driver | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
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: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 9 this patch: 9
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: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 163 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jijie Shao Feb. 13, 2025, 3:55 a.m. UTC
If the rate changed frequently, the PHY link ok,
but the MAC link maybe fails.
As a result, the network port is unavailable.

According to the documents of the chip,
core_reset needs to do to fix the fault.

In hw_adjus_link(), the core_reset is added to try to
ensure that MAC link status is normal.
In addition, MAC link failure detection is added.
If the MAC link fails after core_reset,
the PHY will reset and re-link up to five times.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  4 ++-
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  |  2 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_err.c  | 26 +++++++++++++++--
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   | 29 ++++++++++++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.h   |  2 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 13 ++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  1 +
 7 files changed, 70 insertions(+), 7 deletions(-)

Comments

Andrew Lunn Feb. 13, 2025, 8:05 p.m. UTC | #1
> +int hbg_reset_phy(struct hbg_priv *priv)
> +{
> +	struct phy_device *phydev = priv->mac.phydev;
> +
> +	if (phydev->drv->soft_reset)
> +		return phydev->drv->soft_reset(phydev);
> +
> +	return genphy_soft_reset(phydev);
> +}

A MAC driver should not be doing a soft reset on a PHY. For some
devices, this clears out all the settings. I would suggest you use
phy_stop(), phy_start() which are functions a MAC driver is allowed to
use.

	Andrew
Jijie Shao Feb. 14, 2025, 2:39 a.m. UTC | #2
on 2025/2/14 4:05, Andrew Lunn wrote:
>> +int hbg_reset_phy(struct hbg_priv *priv)
>> +{
>> +	struct phy_device *phydev = priv->mac.phydev;
>> +
>> +	if (phydev->drv->soft_reset)
>> +		return phydev->drv->soft_reset(phydev);
>> +
>> +	return genphy_soft_reset(phydev);
>> +}
> A MAC driver should not be doing a soft reset on a PHY. For some
> devices, this clears out all the settings. I would suggest you use
> phy_stop(), phy_start() which are functions a MAC driver is allowed to
> use.
>
> 	Andrew

Well, I think your advice is helpful.

It is also a good practice to restart the PHY, which also triggers link relinking.

Thank you. I'll test this method.

Thanks,
Jijie Shao
diff mbox series

Patch

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index f45e899c62d8..e942a1e6f859 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -37,6 +37,7 @@  enum hbg_nic_state {
 	HBG_NIC_STATE_RESETTING,
 	HBG_NIC_STATE_RESET_FAIL,
 	HBG_NIC_STATE_NEED_RESET, /* trigger a reset in scheduled task */
+	HBG_NIC_STATE_NP_LINK_FAIL,
 };
 
 enum hbg_reset_type {
@@ -82,7 +83,7 @@  enum hbg_hw_event_type {
 	HBG_HW_EVENT_NONE = 0,
 	HBG_HW_EVENT_INIT, /* driver is loading */
 	HBG_HW_EVENT_RESET,
-
+	HBG_HW_EVENT_CORE_RESET,
 	HBG_HW_EVENT_SERDES_LOOPBACK_ENABLE = 4,
 	HBG_HW_EVENT_SERDES_LOOPBACK_DISABLE = 5,
 };
@@ -257,6 +258,7 @@  struct hbg_stats {
 	u64 tx_dma_err_cnt;
 
 	u64 self_test_rx_pkt_cnt;
+	u64 np_link_fail_cnt;
 };
 
 typedef void (*self_test_pkt_recv)(struct net_device *ndev,
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
index 55ce90b4319a..5e0ba4d5b08d 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -117,6 +117,8 @@  static int hbg_dbg_nic_state(struct seq_file *s, void *unused)
 		   reset_type_str[priv->reset_type]);
 	seq_printf(s, "need reset state: %s\n",
 		   state_str_true_false(priv, HBG_NIC_STATE_NEED_RESET));
+	seq_printf(s, "np_link fail state: %s\n",
+		   state_str_true_false(priv, HBG_NIC_STATE_NP_LINK_FAIL));
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
index 4e8cb66f601c..01c4b246d040 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
@@ -46,6 +46,16 @@  int hbg_rebuild(struct hbg_priv *priv)
 	return 0;
 }
 
+int hbg_reset_phy(struct hbg_priv *priv)
+{
+	struct phy_device *phydev = priv->mac.phydev;
+
+	if (phydev->drv->soft_reset)
+		return phydev->drv->soft_reset(phydev);
+
+	return genphy_soft_reset(phydev);
+}
+
 static int hbg_reset_prepare(struct hbg_priv *priv, enum hbg_reset_type type)
 {
 	int ret;
@@ -61,12 +71,22 @@  static int hbg_reset_prepare(struct hbg_priv *priv, enum hbg_reset_type type)
 	priv->reset_type = type;
 	set_bit(HBG_NIC_STATE_RESETTING, &priv->state);
 	clear_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
-	ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_RESET);
+
+	ret = hbg_reset_phy(priv);
 	if (ret) {
-		set_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
-		clear_bit(HBG_NIC_STATE_RESETTING, &priv->state);
+		dev_err(&priv->pdev->dev, "failed to reset phy\n");
+		goto reset_fail;
 	}
 
+	ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_RESET);
+	if (ret)
+		goto reset_fail;
+
+	return 0;
+
+reset_fail:
+	set_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
+	clear_bit(HBG_NIC_STATE_RESETTING, &priv->state);
 	return ret;
 }
 
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index d978535e06a0..cb145f524e15 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -17,6 +17,7 @@ 
  */
 #define HBG_ENDIAN_CTRL_LE_DATA_BE	0x0
 #define HBG_PCU_FRAME_LEN_PLUS 4
+#define HBG_LINK_FAIL_RETRY_TIMES	5
 
 static bool hbg_hw_spec_is_valid(struct hbg_priv *priv)
 {
@@ -217,12 +218,38 @@  void hbg_hw_fill_buffer(struct hbg_priv *priv, u32 buffer_dma_addr)
 	hbg_reg_write(priv, HBG_REG_RX_CFF_ADDR_ADDR, buffer_dma_addr);
 }
 
-void hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
+int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
 {
+	struct hbg_stats *stats = &priv->stats;
+	int ret;
+
+	clear_bit(HBG_NIC_STATE_NP_LINK_FAIL, &priv->state);
+	hbg_hw_mac_enable(priv, HBG_STATUS_DISABLE);
+
 	hbg_reg_write_field(priv, HBG_REG_PORT_MODE_ADDR,
 			    HBG_REG_PORT_MODE_M, speed);
 	hbg_reg_write_field(priv, HBG_REG_DUPLEX_TYPE_ADDR,
 			    HBG_REG_DUPLEX_B, duplex);
+
+	ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_CORE_RESET);
+	if (ret)
+		return ret;
+
+	hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
+
+	if (!hbg_reg_read_field(priv, HBG_REG_AN_NEG_STATE_ADDR,
+				HBG_REG_AN_NEG_STATE_NP_LINK_OK_B)) {
+		set_bit(HBG_NIC_STATE_NP_LINK_FAIL, &priv->state);
+
+		stats->np_link_fail_cnt++;
+		if (!(stats->np_link_fail_cnt % HBG_LINK_FAIL_RETRY_TIMES))
+			return -EFAULT;
+
+		return -ENOLINK;
+	}
+
+	stats->np_link_fail_cnt = 0;
+	return 0;
 }
 
 /* only support uc filter */
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index fc216fcfae06..4c9bef0348b4 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -44,7 +44,7 @@  static inline void hbg_reg_write64(struct hbg_priv *priv, u32 addr, u64 value)
 int hbg_hw_event_notify(struct hbg_priv *priv,
 			enum hbg_hw_event_type event_type);
 int hbg_hw_init(struct hbg_priv *priv);
-void hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
+int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
 u32 hbg_hw_get_irq_status(struct hbg_priv *priv);
 void hbg_hw_irq_clear(struct hbg_priv *priv, u32 mask);
 bool hbg_hw_irq_is_enabled(struct hbg_priv *priv, u32 mask);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
index db6bc4cfb971..8de6d57bd5f3 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -3,6 +3,7 @@ 
 
 #include <linux/phy.h>
 #include "hbg_common.h"
+#include "hbg_err.h"
 #include "hbg_hw.h"
 #include "hbg_mdio.h"
 #include "hbg_reg.h"
@@ -132,6 +133,7 @@  static void hbg_phy_adjust_link(struct net_device *netdev)
 	struct hbg_priv *priv = netdev_priv(netdev);
 	struct phy_device *phydev = netdev->phydev;
 	u32 speed;
+	int ret;
 
 	if (phydev->link != priv->mac.link_status) {
 		if (phydev->link) {
@@ -152,7 +154,16 @@  static void hbg_phy_adjust_link(struct net_device *netdev)
 			priv->mac.speed = speed;
 			priv->mac.duplex = phydev->duplex;
 			priv->mac.autoneg = phydev->autoneg;
-			hbg_hw_adjust_link(priv, speed, phydev->duplex);
+			ret = hbg_hw_adjust_link(priv, speed, phydev->duplex);
+			if (ret == -ENOLINK) {
+				dev_err(&priv->pdev->dev,
+					"failed to link between MAC and PHY\n");
+				hbg_reset_phy(priv);
+			} else if (ret == -EFAULT) {
+				dev_err(&priv->pdev->dev,
+					"failed to fix the MAC link status\n");
+			}
+
 			hbg_flowctrl_cfg(priv);
 		}
 
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index fe146c2c5e80..9f9346b7f1be 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -54,6 +54,7 @@ 
 #define HBG_REG_PAUSE_ENABLE_RX_B		BIT(0)
 #define HBG_REG_PAUSE_ENABLE_TX_B		BIT(1)
 #define HBG_REG_AN_NEG_STATE_ADDR		(HBG_REG_SGMII_BASE + 0x0058)
+#define HBG_REG_AN_NEG_STATE_NP_LINK_OK_B	BIT(15)
 #define HBG_REG_TRANSMIT_CTRL_ADDR		(HBG_REG_SGMII_BASE + 0x0060)
 #define HBG_REG_TRANSMIT_CTRL_PAD_EN_B		BIT(7)
 #define HBG_REG_TRANSMIT_CTRL_CRC_ADD_B		BIT(6)