[PATCH net v2 0/7] There are some bugfix for hibmcge driver

[PATCH net v2 0/7] There are some bugfix for hibmcge driver

[Jijie Shao]

There are some bugfix for hibmcge driver

---
ChangeLog:
v1 -> v2:
  - Add more details in commit log for patch1, suggested by Simon Horman.
  v1: https://lore.kernel.org/all/20250402133905.895421-1-shaojijie@huawei.com/
---

Jijie Shao (7):
  net: hibmcge: fix incorrect pause frame statistics issue
  net: hibmcge: fix incorrect multicast filtering issue
  net: hibmcge: fix the share of irq statistics among different network
    ports issue
  net: hibmcge: fix wrong mtu log issue
  net: hibmcge: fix the incorrect np_link fail state issue.
  net: hibmcge: fix not restore rx pause mac addr after reset issue
  net: hibmcge: fix multiple phy_stop() issue

 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  8 ++++---
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  | 11 +++++----
 .../ethernet/hisilicon/hibmcge/hbg_diagnose.c |  2 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_err.c  |  3 +++
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   |  7 ++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_irq.c  | 24 ++++++++++++-------
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  8 +++----
 .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 11 ++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  3 +++
 9 files changed, 55 insertions(+), 22 deletions(-)

-- 
2.33.0

[PATCH net v2 1/7] net: hibmcge: fix incorrect pause frame statistics issue

[Jijie Shao]

The driver supports pause frames,
but does not pass pause frames based on rx pause enable configuration,
resulting in incorrect pause frame statistics.

like this:
mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
	-p 64 -c 100

ethtool -S enp132s0f2 | grep -v ": 0"
NIC statistics:
     rx_octets_total_filt_cnt: 6800
     rx_filt_pkt_cnt: 100

The rx pause frames are filtered by the MAC hardware.

This patch configures pass pause frames based on the
rx puase enable status to ensure that
rx pause frames are not filtered.

mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
        -p 64 -c 100

ethtool --include-statistics -a enp132s0f2
Pause parameters for enp132s0f2:
Autonegotiate:	on
RX:		on
TX:		on
RX negotiated: on
TX negotiated: on
Statistics:
  tx_pause_frames: 0
  rx_pause_frames: 100

Fixes: 3a03763f3876 ("net: hibmcge: Add pauseparam supported in this module")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
ChangeLog:
v1 -> v2:
  - Add more details in commit log, suggested by Simon Horman.
  v1: https://lore.kernel.org/all/20250402133905.895421-1-shaojijie@huawei.com/
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c  | 3 +++
 drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 74a18033b444..7d3bbd3e2adc 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -242,6 +242,9 @@ void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en)
 			    HBG_REG_PAUSE_ENABLE_TX_B, tx_en);
 	hbg_reg_write_field(priv, HBG_REG_PAUSE_ENABLE_ADDR,
 			    HBG_REG_PAUSE_ENABLE_RX_B, rx_en);
+
+	hbg_reg_write_field(priv, HBG_REG_REC_FILT_CTRL_ADDR,
+			    HBG_REG_REC_FILT_CTRL_PAUSE_FRM_PASS_B, rx_en);
 }
 
 void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index cc2cc612770d..fd623cfd13de 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -68,6 +68,7 @@
 #define HBG_REG_TRANSMIT_CTRL_AN_EN_B		BIT(5)
 #define HBG_REG_REC_FILT_CTRL_ADDR		(HBG_REG_SGMII_BASE + 0x0064)
 #define HBG_REG_REC_FILT_CTRL_UC_MATCH_EN_B	BIT(0)
+#define HBG_REG_REC_FILT_CTRL_PAUSE_FRM_PASS_B	BIT(4)
 #define HBG_REG_RX_OCTETS_TOTAL_OK_ADDR		(HBG_REG_SGMII_BASE + 0x0080)
 #define HBG_REG_RX_OCTETS_BAD_ADDR		(HBG_REG_SGMII_BASE + 0x0084)
 #define HBG_REG_RX_UC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x0088)
-- 
2.33.0

Re: [PATCH net v2 1/7] net: hibmcge: fix incorrect pause frame statistics issue

[Michal Swiatkowski]

On Thu, Apr 03, 2025 at 09:53:05PM +0800, Jijie Shao wrote:
> The driver supports pause frames,
> but does not pass pause frames based on rx pause enable configuration,
> resulting in incorrect pause frame statistics.
> 
> like this:
> mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
> 	-p 64 -c 100
> 
> ethtool -S enp132s0f2 | grep -v ": 0"
> NIC statistics:
>      rx_octets_total_filt_cnt: 6800
>      rx_filt_pkt_cnt: 100
> 
> The rx pause frames are filtered by the MAC hardware.
> 
> This patch configures pass pause frames based on the
> rx puase enable status to ensure that
> rx pause frames are not filtered.
> 
> mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
>         -p 64 -c 100
> 
> ethtool --include-statistics -a enp132s0f2
> Pause parameters for enp132s0f2:
> Autonegotiate:	on
> RX:		on
> TX:		on
> RX negotiated: on
> TX negotiated: on
> Statistics:
>   tx_pause_frames: 0
>   rx_pause_frames: 100
> 
> Fixes: 3a03763f3876 ("net: hibmcge: Add pauseparam supported in this module")
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> ChangeLog:
> v1 -> v2:
>   - Add more details in commit log, suggested by Simon Horman.
>   v1: https://lore.kernel.org/all/20250402133905.895421-1-shaojijie@huawei.com/
> ---
>  drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c  | 3 +++
>  drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> index 74a18033b444..7d3bbd3e2adc 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> @@ -242,6 +242,9 @@ void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en)
>  			    HBG_REG_PAUSE_ENABLE_TX_B, tx_en);
>  	hbg_reg_write_field(priv, HBG_REG_PAUSE_ENABLE_ADDR,
>  			    HBG_REG_PAUSE_ENABLE_RX_B, rx_en);
> +
> +	hbg_reg_write_field(priv, HBG_REG_REC_FILT_CTRL_ADDR,
> +			    HBG_REG_REC_FILT_CTRL_PAUSE_FRM_PASS_B, rx_en);
>  }
>  
>  void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en)
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
> index cc2cc612770d..fd623cfd13de 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
> @@ -68,6 +68,7 @@
>  #define HBG_REG_TRANSMIT_CTRL_AN_EN_B		BIT(5)
>  #define HBG_REG_REC_FILT_CTRL_ADDR		(HBG_REG_SGMII_BASE + 0x0064)
>  #define HBG_REG_REC_FILT_CTRL_UC_MATCH_EN_B	BIT(0)
> +#define HBG_REG_REC_FILT_CTRL_PAUSE_FRM_PASS_B	BIT(4)
>  #define HBG_REG_RX_OCTETS_TOTAL_OK_ADDR		(HBG_REG_SGMII_BASE + 0x0080)
>  #define HBG_REG_RX_OCTETS_BAD_ADDR		(HBG_REG_SGMII_BASE + 0x0084)
>  #define HBG_REG_RX_UC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x0088)
> -- 

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> 2.33.0

Re: [PATCH net v2 1/7] net: hibmcge: fix incorrect pause frame statistics issue

[Simon Horman]

On Thu, Apr 03, 2025 at 09:53:05PM +0800, Jijie Shao wrote:
> The driver supports pause frames,
> but does not pass pause frames based on rx pause enable configuration,
> resulting in incorrect pause frame statistics.
> 
> like this:
> mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
> 	-p 64 -c 100
> 
> ethtool -S enp132s0f2 | grep -v ": 0"
> NIC statistics:
>      rx_octets_total_filt_cnt: 6800
>      rx_filt_pkt_cnt: 100
> 
> The rx pause frames are filtered by the MAC hardware.
> 
> This patch configures pass pause frames based on the
> rx puase enable status to ensure that
> rx pause frames are not filtered.
> 
> mz eno3 '01 80 c2 00 00 01 00 18 2d 04 00 9c 88 08 00 01 ff ff' \
>         -p 64 -c 100
> 
> ethtool --include-statistics -a enp132s0f2
> Pause parameters for enp132s0f2:
> Autonegotiate:	on
> RX:		on
> TX:		on
> RX negotiated: on
> TX negotiated: on
> Statistics:
>   tx_pause_frames: 0
>   rx_pause_frames: 100
> 
> Fixes: 3a03763f3876 ("net: hibmcge: Add pauseparam supported in this module")
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> ChangeLog:
> v1 -> v2:
>   - Add more details in commit log, suggested by Simon Horman.
>   v1: https://lore.kernel.org/all/20250402133905.895421-1-shaojijie@huawei.com/

Thanks for the update.

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net v2 2/7] net: hibmcge: fix incorrect multicast filtering issue

[Jijie Shao]

The driver does not support multicast filtering,
the mask must be set to 0xFFFFFFFF. Otherwise,
incorrect filtering occurs.

This patch fixes this problem.

Fixes: 37b367d60d0f ("net: hibmcge: Add unicast frame filter supported in this module")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c  | 4 ++++
 drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 7d3bbd3e2adc..9b65eef62b3f 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -234,6 +234,10 @@ void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable)
 {
 	hbg_reg_write_field(priv, HBG_REG_REC_FILT_CTRL_ADDR,
 			    HBG_REG_REC_FILT_CTRL_UC_MATCH_EN_B, enable);
+
+	/* only uc filter is supported, so set all bits of mc mask reg to 1 */
+	hbg_reg_write64(priv, HBG_REG_STATION_ADDR_LOW_MSK_0, U64_MAX);
+	hbg_reg_write64(priv, HBG_REG_STATION_ADDR_LOW_MSK_1, U64_MAX);
 }
 
 void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index fd623cfd13de..a6e7f5e62b48 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -135,6 +135,8 @@
 #define HBG_REG_STATION_ADDR_HIGH_4_ADDR	(HBG_REG_SGMII_BASE + 0x0224)
 #define HBG_REG_STATION_ADDR_LOW_5_ADDR		(HBG_REG_SGMII_BASE + 0x0228)
 #define HBG_REG_STATION_ADDR_HIGH_5_ADDR	(HBG_REG_SGMII_BASE + 0x022C)
+#define HBG_REG_STATION_ADDR_LOW_MSK_0		(HBG_REG_SGMII_BASE + 0x0230)
+#define HBG_REG_STATION_ADDR_LOW_MSK_1		(HBG_REG_SGMII_BASE + 0x0238)
 
 /* PCU */
 #define HBG_REG_TX_FIFO_THRSLD_ADDR		(HBG_REG_SGMII_BASE + 0x0420)
-- 
2.33.0

[PATCH net v2 3/7] net: hibmcge: fix the share of irq statistics among different network ports issue

[Jijie Shao]

hbg_irqs is a global array which contains irq statistics.
However, the irq statistics of different network ports
point to the same global array. As a result, the statistics are incorrect.

This patch allocates a statistics array for each network port
to prevent the statistics of different network ports
from affecting each other.

irq statistics are removed from hbg_irq_info. Therefore,
all data in hbg_irq_info remains unchanged. Therefore,
the input parameter of some functions is changed to const.

Fixes: 4d089035fa19 ("net: hibmcge: Add interrupt supported in this module")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  8 ++++---
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  |  4 ++--
 .../ethernet/hisilicon/hibmcge/hbg_diagnose.c |  2 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_irq.c  | 24 ++++++++++++-------
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  2 +-
 5 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index f8cdab62bf85..7725cb0c5c8a 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -108,14 +108,16 @@ struct hbg_irq_info {
 	bool re_enable;
 	bool need_print;
 	bool need_reset;
-	u64 count;
 
-	void (*irq_handle)(struct hbg_priv *priv, struct hbg_irq_info *info);
+	void (*irq_handle)(struct hbg_priv *priv,
+			   const struct hbg_irq_info *info);
 };
 
 struct hbg_vector {
 	char name[HBG_VECTOR_NUM][32];
-	struct hbg_irq_info *info_array;
+
+	u64 *stats_array;
+	const struct hbg_irq_info *info_array;
 	u32 info_array_len;
 };
 
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
index 5e0ba4d5b08d..9c09e4835990 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -61,7 +61,7 @@ static int hbg_dbg_irq_info(struct seq_file *s, void *unused)
 {
 	struct net_device *netdev = dev_get_drvdata(s->private);
 	struct hbg_priv *priv = netdev_priv(netdev);
-	struct hbg_irq_info *info;
+	const struct hbg_irq_info *info;
 	u32 i;
 
 	for (i = 0; i < priv->vectors.info_array_len; i++) {
@@ -73,7 +73,7 @@ static int hbg_dbg_irq_info(struct seq_file *s, void *unused)
 								info->mask)),
 			   str_true_false(info->need_reset),
 			   str_true_false(info->need_print),
-			   info->count);
+			   priv->vectors.stats_array[i]);
 	}
 
 	return 0;
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_diagnose.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_diagnose.c
index d61c03f34ff0..f23fb5920c3c 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_diagnose.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_diagnose.c
@@ -234,7 +234,7 @@ static u64 hbg_get_irq_stats(struct hbg_vector *vectors, u32 mask)
 
 	for (i = 0; i < vectors->info_array_len; i++)
 		if (vectors->info_array[i].mask == mask)
-			return vectors->info_array[i].count;
+			return vectors->stats_array[i];
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
index e79e9ab3e530..8af0bc4cca21 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
@@ -6,7 +6,7 @@
 #include "hbg_hw.h"
 
 static void hbg_irq_handle_err(struct hbg_priv *priv,
-			       struct hbg_irq_info *irq_info)
+			       const struct hbg_irq_info *irq_info)
 {
 	if (irq_info->need_print)
 		dev_err(&priv->pdev->dev,
@@ -17,30 +17,30 @@ static void hbg_irq_handle_err(struct hbg_priv *priv,
 }
 
 static void hbg_irq_handle_tx(struct hbg_priv *priv,
-			      struct hbg_irq_info *irq_info)
+			      const struct hbg_irq_info *irq_info)
 {
 	napi_schedule(&priv->tx_ring.napi);
 }
 
 static void hbg_irq_handle_rx(struct hbg_priv *priv,
-			      struct hbg_irq_info *irq_info)
+			      const struct hbg_irq_info *irq_info)
 {
 	napi_schedule(&priv->rx_ring.napi);
 }
 
 static void hbg_irq_handle_rx_buf_val(struct hbg_priv *priv,
-				      struct hbg_irq_info *irq_info)
+				      const struct hbg_irq_info *irq_info)
 {
 	priv->stats.rx_fifo_less_empty_thrsld_cnt++;
 }
 
 #define HBG_IRQ_I(name, handle) \
-	{#name, HBG_INT_MSK_##name##_B, false, false, false, 0, handle}
+	{#name, HBG_INT_MSK_##name##_B, false, false, false, handle}
 #define HBG_ERR_IRQ_I(name, need_print, ndde_reset) \
 	{#name, HBG_INT_MSK_##name##_B, true, need_print, \
-	ndde_reset, 0, hbg_irq_handle_err}
+	ndde_reset, hbg_irq_handle_err}
 
-static struct hbg_irq_info hbg_irqs[] = {
+static const struct hbg_irq_info hbg_irqs[] = {
 	HBG_IRQ_I(RX, hbg_irq_handle_rx),
 	HBG_IRQ_I(TX, hbg_irq_handle_tx),
 	HBG_ERR_IRQ_I(TX_PKT_CPL, true, true),
@@ -64,7 +64,7 @@ static struct hbg_irq_info hbg_irqs[] = {
 
 static irqreturn_t hbg_irq_handle(int irq_num, void *p)
 {
-	struct hbg_irq_info *info;
+	const struct hbg_irq_info *info;
 	struct hbg_priv *priv = p;
 	u32 status;
 	u32 i;
@@ -79,7 +79,7 @@ static irqreturn_t hbg_irq_handle(int irq_num, void *p)
 			hbg_hw_irq_enable(priv, info->mask, false);
 			hbg_hw_irq_clear(priv, info->mask);
 
-			info->count++;
+			priv->vectors.stats_array[i]++;
 			if (info->irq_handle)
 				info->irq_handle(priv, info);
 
@@ -132,6 +132,12 @@ int hbg_irq_init(struct hbg_priv *priv)
 					     irq_names_map[i]);
 	}
 
+	vectors->stats_array = devm_kcalloc(&priv->pdev->dev,
+					    ARRAY_SIZE(hbg_irqs),
+					    sizeof(u64), GFP_KERNEL);
+	if (!vectors->stats_array)
+		return -ENOMEM;
+
 	vectors->info_array = hbg_irqs;
 	vectors->info_array_len = ARRAY_SIZE(hbg_irqs);
 	return 0;
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 2ac5454338e4..e5c961ad4b9b 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -21,7 +21,7 @@
 
 static void hbg_all_irq_enable(struct hbg_priv *priv, bool enabled)
 {
-	struct hbg_irq_info *info;
+	const struct hbg_irq_info *info;
 	u32 i;
 
 	for (i = 0; i < priv->vectors.info_array_len; i++) {
-- 
2.33.0

[PATCH net v2 4/7] net: hibmcge: fix wrong mtu log issue

[Jijie Shao]

A dbg log is generated when the driver modifies the MTU,
which is expected to trace the change of the MTU.

However, the log is recorded after WRITE_ONCE().
At this time, netdev->mtu has been changed to the new value.
As a result, netdev->mtu is the same as new_mtu.

This patch modifies the log location and records logs before WRITE_ONCE().

Fixes: ff4edac6e9bd ("net: hibmcge: Implement some .ndo functions")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index e5c961ad4b9b..2e64dc1ab355 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -203,12 +203,12 @@ static int hbg_net_change_mtu(struct net_device *netdev, int new_mtu)
 	if (netif_running(netdev))
 		return -EBUSY;
 
-	hbg_hw_set_mtu(priv, new_mtu);
-	WRITE_ONCE(netdev->mtu, new_mtu);
-
 	dev_dbg(&priv->pdev->dev,
 		"change mtu from %u to %u\n", netdev->mtu, new_mtu);
 
+	hbg_hw_set_mtu(priv, new_mtu);
+	WRITE_ONCE(netdev->mtu, new_mtu);
+
 	return 0;
 }
 
-- 
2.33.0

[PATCH net v2 5/7] net: hibmcge: fix the incorrect np_link fail state issue.

[Jijie Shao]

In the debugfs file, the driver displays the np_link fail state
based on the HBG_NIC_STATE_NP_LINK_FAIL.

However, HBG_NIC_STATE_NP_LINK_FAIL is cleared in hbg_service_task()
So, this value of np_link fail is always false.

This patch directly reads the related register to display the real state.

Fixes: e0306637e85d ("net: hibmcge: Add support for mac link exception handling feature")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
index 9c09e4835990..01ad82d2f5cc 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -106,6 +106,7 @@ static int hbg_dbg_nic_state(struct seq_file *s, void *unused)
 {
 	struct net_device *netdev = dev_get_drvdata(s->private);
 	struct hbg_priv *priv = netdev_priv(netdev);
+	bool np_link_fail;
 
 	seq_printf(s, "event handling state: %s\n",
 		   state_str_true_false(priv, HBG_NIC_STATE_EVENT_HANDLING));
@@ -117,8 +118,10 @@ 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));
+
+	np_link_fail = !hbg_reg_read_field(priv, HBG_REG_AN_NEG_STATE_ADDR,
+					   HBG_REG_AN_NEG_STATE_NP_LINK_OK_B);
+	seq_printf(s, "np_link fail state: %s\n", str_true_false(np_link_fail));
 
 	return 0;
 }
-- 
2.33.0

[PATCH net v2 6/7] net: hibmcge: fix not restore rx pause mac addr after reset issue

[Jijie Shao]

In normal cases, the driver must ensure that the value
of rx pause mac addr is the same as the MAC address of
the network port. This ensures that the driver can
receive pause frames whose destination address is
the MAC address of the network port.

Currently, the rx pause addr does not restored after reset.

The index of the MAC address of the host is always 0.
Therefore, this patch sets rx pause addr to
the MAC address with index 0.

Fixes: 3f5a61f6d504 ("net: hibmcge: Add reset supported in this module")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
index 4e8cb66f601c..a0bcfb5a713d 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
@@ -26,12 +26,15 @@ static void hbg_restore_mac_table(struct hbg_priv *priv)
 
 static void hbg_restore_user_def_settings(struct hbg_priv *priv)
 {
+	/* The index of host mac is always 0. */
+	u64 rx_pause_addr = ether_addr_to_u64(priv->filter.mac_table[0].addr);
 	struct ethtool_pauseparam *pause_param = &priv->user_def.pause_param;
 
 	hbg_restore_mac_table(priv);
 	hbg_hw_set_mtu(priv, priv->netdev->mtu);
 	hbg_hw_set_pause_enable(priv, pause_param->tx_pause,
 				pause_param->rx_pause);
+	hbg_hw_set_rx_pause_mac_addr(priv, rx_pause_addr);
 }
 
 int hbg_rebuild(struct hbg_priv *priv)
-- 
2.33.0

Re: [PATCH net v2 6/7] net: hibmcge: fix not restore rx pause mac addr after reset issue

[Jakub Kicinski]

On Thu, 3 Apr 2025 21:53:10 +0800 Jijie Shao wrote:
> In normal cases, the driver must ensure that the value
> of rx pause mac addr is the same as the MAC address of
> the network port. This ensures that the driver can
> receive pause frames whose destination address is
> the MAC address of the network port.

I thought "in normal cases" pause frames use 01:80:C2:00:00:01
as the destination address!?

Are you sure this patch is not a misunderstanding, and the issue
is already fixed by patch 2?

> Currently, the rx pause addr does not restored after reset.
> 
> The index of the MAC address of the host is always 0.
> Therefore, this patch sets rx pause addr to
> the MAC address with index 0.
-- 
pw-bot: cr

Re: [PATCH net v2 6/7] net: hibmcge: fix not restore rx pause mac addr after reset issue

[Jijie Shao]

on 2025/4/4 22:58, Jakub Kicinski wrote:
> On Thu, 3 Apr 2025 21:53:10 +0800 Jijie Shao wrote:
>> In normal cases, the driver must ensure that the value
>> of rx pause mac addr is the same as the MAC address of
>> the network port. This ensures that the driver can
>> receive pause frames whose destination address is
>> the MAC address of the network port.
> I thought "in normal cases" pause frames use 01:80:C2:00:00:01
> as the destination address!?

No, the address set in .ndo_set_mac_address() is used.
01:80:C2:00:00:01 is supported by default. No additional configuration is required.

>
> Are you sure this patch is not a misunderstanding, and the issue
> is already fixed by patch 2?

It's a different issue. This patch is used to solve the problem that
the configuration is lost after reset.

>
>> Currently, the rx pause addr does not restored after reset.

Here.

>>
>> The index of the MAC address of the host is always 0.
>> Therefore, this patch sets rx pause addr to
>> the MAC address with index 0.

Re: [PATCH net v2 6/7] net: hibmcge: fix not restore rx pause mac addr after reset issue

[Jakub Kicinski]

On Mon, 7 Apr 2025 09:06:42 +0800 Jijie Shao wrote:
> on 2025/4/4 22:58, Jakub Kicinski wrote:
> > On Thu, 3 Apr 2025 21:53:10 +0800 Jijie Shao wrote:  
> >> In normal cases, the driver must ensure that the value
> >> of rx pause mac addr is the same as the MAC address of
> >> the network port. This ensures that the driver can
> >> receive pause frames whose destination address is
> >> the MAC address of the network port.  
> > I thought "in normal cases" pause frames use 01:80:C2:00:00:01
> > as the destination address!?  
> 
> No, the address set in .ndo_set_mac_address() is used.
> 01:80:C2:00:00:01 is supported by default. No additional configuration is required.

Are you talking about source or destination?
How does the sender learn the receiver's address? Via LLDP?
You need to explain all this much better in the commit message.
It is not "normal" for a switched Ethernet network.

Re: [PATCH net v2 6/7] net: hibmcge: fix not restore rx pause mac addr after reset issue

[Jijie Shao]

on 2025/4/8 1:11, Jakub Kicinski wrote:
> On Mon, 7 Apr 2025 09:06:42 +0800 Jijie Shao wrote:
>> on 2025/4/4 22:58, Jakub Kicinski wrote:
>>> On Thu, 3 Apr 2025 21:53:10 +0800 Jijie Shao wrote:
>>>> In normal cases, the driver must ensure that the value
>>>> of rx pause mac addr is the same as the MAC address of
>>>> the network port. This ensures that the driver can
>>>> receive pause frames whose destination address is
>>>> the MAC address of the network port.
>>> I thought "in normal cases" pause frames use 01:80:C2:00:00:01
>>> as the destination address!?
>> No, the address set in .ndo_set_mac_address() is used.
>> 01:80:C2:00:00:01 is supported by default. No additional configuration is required.
> Are you talking about source or destination?
> How does the sender learn the receiver's address? Via LLDP?
> You need to explain all this much better in the commit message.
> It is not "normal" for a switched Ethernet network.


ok,

Thanks
Jijie Shao

[PATCH net v2 7/7] net: hibmcge: fix multiple phy_stop() issue

[Jijie Shao]

After detecting the np_link_fail exception,
the driver attempts to fix the exception by
using phy_stop() and phy_start() in the scheduled task.

However, hbg_fix_np_link_fail() and .ndo_stop()
may be concurrently executed. As a result,
phy_stop() is executed twice, and the following Calltrace occurs:

 hibmcge 0000:84:00.2 enp132s0f2: Link is Down
 hibmcge 0000:84:00.2: failed to link between MAC and PHY, try to fix...
 ------------[ cut here ]------------
 called from state HALTED
 WARNING: CPU: 71 PID: 23391 at drivers/net/phy/phy.c:1503 phy_stop...
 ...
 pc : phy_stop+0x138/0x180
 lr : phy_stop+0x138/0x180
 sp : ffff8000c76bbd40
 x29: ffff8000c76bbd40 x28: 0000000000000000 x27: 0000000000000000
 x26: ffff2020047358c0 x25: ffff202004735940 x24: ffff20200000e405
 x23: ffff2020060e5178 x22: ffff2020060e4000 x21: ffff2020060e49c0
 x20: ffff2020060e5170 x19: ffff20202538e000 x18: 0000000000000020
 x17: 0000000000000000 x16: ffffcede02e28f40 x15: ffffffffffffffff
 x14: 0000000000000000 x13: 205d313933333254 x12: 5b5d393430303233
 x11: ffffcede04555958 x10: ffffcede04495918 x9 : ffffcede0274fee0
 x8 : 00000000000bffe8 x7 : c0000000ffff7fff x6 : 0000000000000001
 x5 : 00000000002bffa8 x4 : 0000000000000000 x3 : 0000000000000000
 x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff20202e429480
 Call trace:
  phy_stop+0x138/0x180
  hbg_fix_np_link_fail+0x4c/0x90 [hibmcge]
  hbg_service_task+0xfc/0x148 [hibmcge]
  process_one_work+0x180/0x398
  worker_thread+0x210/0x328
  kthread+0xe0/0xf0
  ret_from_fork+0x10/0x20
 ---[ end trace 0000000000000000 ]---

This patch adds the rtnl_lock to hbg_fix_np_link_fail()
to ensure that other operations are not performed concurrently.
In addition, np_link_fail exception can be fixed
only when the PHY is link.

Fixes: e0306637e85d ("net: hibmcge: Add support for mac link exception handling feature")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
index f29a937ad087..42b0083c9193 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2024 Hisilicon Limited.
 
 #include <linux/phy.h>
+#include <linux/rtnetlink.h>
 #include "hbg_common.h"
 #include "hbg_hw.h"
 #include "hbg_mdio.h"
@@ -133,12 +134,17 @@ void hbg_fix_np_link_fail(struct hbg_priv *priv)
 {
 	struct device *dev = &priv->pdev->dev;
 
+	rtnl_lock();
+
 	if (priv->stats.np_link_fail_cnt >= HBG_NP_LINK_FAIL_RETRY_TIMES) {
 		dev_err(dev, "failed to fix the MAC link status\n");
 		priv->stats.np_link_fail_cnt = 0;
-		return;
+		goto unlock;
 	}
 
+	if (!priv->mac.phydev->link)
+		goto unlock;
+
 	priv->stats.np_link_fail_cnt++;
 	dev_err(dev, "failed to link between MAC and PHY, try to fix...\n");
 
@@ -147,6 +153,9 @@ void hbg_fix_np_link_fail(struct hbg_priv *priv)
 	 */
 	hbg_phy_stop(priv);
 	hbg_phy_start(priv);
+
+unlock:
+	rtnl_unlock();
 }
 
 static void hbg_phy_adjust_link(struct net_device *netdev)
-- 
2.33.0

Re: [PATCH net v2 7/7] net: hibmcge: fix multiple phy_stop() issue

[Simon Horman]

On Thu, Apr 03, 2025 at 09:53:11PM +0800, Jijie Shao wrote:
> After detecting the np_link_fail exception,
> the driver attempts to fix the exception by
> using phy_stop() and phy_start() in the scheduled task.
> 
> However, hbg_fix_np_link_fail() and .ndo_stop()
> may be concurrently executed. As a result,
> phy_stop() is executed twice, and the following Calltrace occurs:
> 
>  hibmcge 0000:84:00.2 enp132s0f2: Link is Down
>  hibmcge 0000:84:00.2: failed to link between MAC and PHY, try to fix...
>  ------------[ cut here ]------------
>  called from state HALTED
>  WARNING: CPU: 71 PID: 23391 at drivers/net/phy/phy.c:1503 phy_stop...
>  ...
>  pc : phy_stop+0x138/0x180
>  lr : phy_stop+0x138/0x180
>  sp : ffff8000c76bbd40
>  x29: ffff8000c76bbd40 x28: 0000000000000000 x27: 0000000000000000
>  x26: ffff2020047358c0 x25: ffff202004735940 x24: ffff20200000e405
>  x23: ffff2020060e5178 x22: ffff2020060e4000 x21: ffff2020060e49c0
>  x20: ffff2020060e5170 x19: ffff20202538e000 x18: 0000000000000020
>  x17: 0000000000000000 x16: ffffcede02e28f40 x15: ffffffffffffffff
>  x14: 0000000000000000 x13: 205d313933333254 x12: 5b5d393430303233
>  x11: ffffcede04555958 x10: ffffcede04495918 x9 : ffffcede0274fee0
>  x8 : 00000000000bffe8 x7 : c0000000ffff7fff x6 : 0000000000000001
>  x5 : 00000000002bffa8 x4 : 0000000000000000 x3 : 0000000000000000
>  x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff20202e429480
>  Call trace:
>   phy_stop+0x138/0x180
>   hbg_fix_np_link_fail+0x4c/0x90 [hibmcge]
>   hbg_service_task+0xfc/0x148 [hibmcge]
>   process_one_work+0x180/0x398
>   worker_thread+0x210/0x328
>   kthread+0xe0/0xf0
>   ret_from_fork+0x10/0x20
>  ---[ end trace 0000000000000000 ]---
> 
> This patch adds the rtnl_lock to hbg_fix_np_link_fail()
> to ensure that other operations are not performed concurrently.
> In addition, np_link_fail exception can be fixed
> only when the PHY is link.
> 
> Fixes: e0306637e85d ("net: hibmcge: Add support for mac link exception handling feature")
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

Reviewed-by: Simon Horman <horms@kernel.org>