[PATCH net-next 0/7] Support some features for the HIBMCGE driver

[PATCH net-next 0/7] Support some features for the HIBMCGE driver

[Jijie Shao]

In this patch series, The HIBMCGE driver implements some functions
such as statistics query, dump register, unicast MAC address filtering,
debugfs and reset.

Jijie Shao (7):
  net: hibmcge: Add dump statistics supported in this module
  net: hibmcge: Add debugfs supported in this module
  net: hibmcge: Add unicast frame filter supported in this module
  net: hibmcge: Add register dump supported in this module
  net: hibmcge: Add pauseparam supported in this module
  net: hibmcge: Add nway_reset supported in this module
  net: hibmcge: Add reset supported in this module

 .../net/ethernet/hisilicon/hibmcge/Makefile   |   3 +-
 .../ethernet/hisilicon/hibmcge/hbg_common.h   | 135 +++++++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  | 190 +++++++++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.h  |  12 +
 .../net/ethernet/hisilicon/hibmcge/hbg_err.c  | 140 +++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_err.h  |  13 +
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.c  | 381 ++++++++++++++++++
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.h  |   1 +
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   |  48 ++-
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.h   |   6 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 266 +++++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  | 136 +++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 171 +++++++-
 13 files changed, 1471 insertions(+), 31 deletions(-)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_err.h

-- 
2.33.0

[PATCH net-next 1/7] net: hibmcge: Add dump statistics supported in this module

[Jijie Shao]

The driver supports many hw statistics. This patch supports
dump statistics through ethtool_ops and ndo.get_stats64().

The type of statistics register is u32,
and the type of driver statistics u64.
To prevent the statistics register from overflowing,
the driver dump the statistics every 5 minutes.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_common.h   | 100 ++++++++++
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.c  | 178 ++++++++++++++++++
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.h  |   1 +
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  53 +++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  97 ++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 171 ++++++++++++++++-
 6 files changed, 596 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 96daf058d387..411afc9b916b 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -116,6 +116,104 @@ struct hbg_mac {
 	u32 link_status;
 };
 
+struct hbg_stats {
+	u64 rx_desc_drop;
+	u64 rx_desc_l2_err_cnt;
+	u64 rx_desc_pkt_len_err_cnt;
+	u64 rx_desc_l3_err_cnt;
+	u64 rx_desc_l3_wrong_head_cnt;
+	u64 rx_desc_l3_csum_err_cnt;
+	u64 rx_desc_l3_len_err_cnt;
+	u64 rx_desc_l3_zero_ttl_cnt;
+	u64 rx_desc_l3_other_cnt;
+	u64 rx_desc_l4_err_cnt;
+	u64 rx_desc_l4_wrong_head_cnt;
+	u64 rx_desc_l4_len_err_cnt;
+	u64 rx_desc_l4_csum_err_cnt;
+	u64 rx_desc_l4_zero_port_num_cnt;
+	u64 rx_desc_l4_other_cnt;
+	u64 rx_desc_frag_cnt;
+	u64 rx_desc_ip_ver_err_cnt;
+	u64 rx_desc_ipv4_pkt_cnt;
+	u64 rx_desc_ipv6_pkt_cnt;
+	u64 rx_desc_no_ip_pkt_cnt;
+	u64 rx_desc_ip_pkt_cnt;
+	u64 rx_desc_tcp_pkt_cnt;
+	u64 rx_desc_udp_pkt_cnt;
+	u64 rx_desc_vlan_pkt_cnt;
+	u64 rx_desc_icmp_pkt_cnt;
+	u64 rx_desc_arp_pkt_cnt;
+	u64 rx_desc_rarp_pkt_cnt;
+	u64 rx_desc_multicast_pkt_cnt;
+	u64 rx_desc_broadcast_pkt_cnt;
+	u64 rx_desc_ipsec_pkt_cnt;
+	u64 rx_desc_ip_opt_pkt_cnt;
+	u64 rx_desc_key_not_match_cnt;
+
+	u64 rx_octets_total_ok_cnt;
+	u64 rx_uc_pkts_cnt;
+	u64 rx_mc_pkts_cnt;
+	u64 rx_bc_pkts_cnt;
+	u64 rx_vlan_pkt_cnt;
+	u64 rx_octets_bad_cnt;
+	u64 rx_octets_total_filt_cnt;
+	u64 rx_filt_pkt_cnt;
+	u64 rx_trans_pkg_cnt;
+	u64 rx_framesize_64;
+	u64 rx_framesize_65_127;
+	u64 rx_framesize_128_255;
+	u64 rx_framesize_256_511;
+	u64 rx_framesize_512_1023;
+	u64 rx_framesize_1024_1518;
+	u64 rx_framesize_bt_1518;
+	u64 rx_fcs_error_cnt;
+	u64 rx_data_error_cnt;
+	u64 rx_align_error_cnt;
+	u64 rx_frame_long_err_cnt;
+	u64 rx_jabber_err_cnt;
+	u64 rx_pause_macctl_frame_cnt;
+	u64 rx_unknown_macctl_frame_cnt;
+	u64 rx_frame_very_long_err_cnt;
+	u64 rx_frame_runt_err_cnt;
+	u64 rx_frame_short_err_cnt;
+	u64 rx_over_flow_cnt;
+	u64 rx_addr_overflow_cnt;
+	u64 rx_bufrq_err_cnt;
+	u64 rx_we_err_cnt;
+	u64 rx_overrun_cnt;
+	u64 rx_lengthfield_err_cnt;
+	u64 rx_fail_comma_cnt;
+
+	u64 rx_dma_err_cnt;
+	u64 rx_fifo_fill_full_cnt;
+
+	u64 tx_octets_total_ok_cnt;
+	u64 tx_uc_pkts_cnt;
+	u64 tx_mc_pkts_cnt;
+	u64 tx_bc_pkts_cnt;
+	u64 tx_vlan_pkt_cnt;
+	u64 tx_octets_bad_cnt;
+	u64 tx_trans_pkg_cnt;
+	u64 tx_pause_frame_cnt;
+	u64 tx_framesize_64;
+	u64 tx_framesize_65_127;
+	u64 tx_framesize_128_255;
+	u64 tx_framesize_256_511;
+	u64 tx_framesize_512_1023;
+	u64 tx_framesize_1024_1518;
+	u64 tx_framesize_bt_1518;
+	u64 tx_underrun_err_cnt;
+	u64 tx_add_cs_fail_cnt;
+	u64 tx_we_err_cnt;
+	u64 tx_bufrl_err_cnt;
+	u64 tx_crc_err_cnt;
+	u64 tx_drop_cnt;
+	u64 tx_excessive_length_drop_cnt;
+
+	u64 tx_timeout_cnt;
+	u64 tx_dma_err_cnt;
+};
+
 struct hbg_priv {
 	struct net_device *netdev;
 	struct pci_dev *pdev;
@@ -126,6 +224,8 @@ struct hbg_priv {
 	struct hbg_vector vectors;
 	struct hbg_ring tx_ring;
 	struct hbg_ring rx_ring;
+	struct hbg_stats stats;
+	struct delayed_work service_task;
 };
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
index c3370114aef3..59f8c84d43fa 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -3,12 +3,190 @@
 
 #include <linux/ethtool.h>
 #include <linux/phy.h>
+#include "hbg_common.h"
 #include "hbg_ethtool.h"
+#include "hbg_hw.h"
+
+#define HBG_STATS_FIELD_OFF(f) (offsetof(struct hbg_stats, f))
+#define HBG_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))
+#define HBG_STATS_UPDATE(p, offset, val) (HBG_STATS_READ(p, offset) += (val))
+
+struct hbg_ethtool_stats {
+	char name[ETH_GSTRING_LEN];
+	unsigned long offset;
+	u32 reg; /* set to 0 if stats is not updated via dump reg */
+};
+
+#define HBG_STATS_I(stats) { #stats, HBG_STATS_FIELD_OFF(stats), 0}
+#define HBG_STATS_REG_I(stats, reg) { #stats, HBG_STATS_FIELD_OFF(stats), reg}
+
+static const struct hbg_ethtool_stats hbg_ethtool_stats_map[] = {
+	HBG_STATS_I(rx_desc_l2_err_cnt),
+	HBG_STATS_I(rx_desc_pkt_len_err_cnt),
+	HBG_STATS_I(rx_desc_l3_err_cnt),
+	HBG_STATS_I(rx_desc_l3_wrong_head_cnt),
+	HBG_STATS_I(rx_desc_l3_csum_err_cnt),
+	HBG_STATS_I(rx_desc_l3_len_err_cnt),
+	HBG_STATS_I(rx_desc_l3_zero_ttl_cnt),
+	HBG_STATS_I(rx_desc_l3_other_cnt),
+	HBG_STATS_I(rx_desc_l4_err_cnt),
+	HBG_STATS_I(rx_desc_l4_wrong_head_cnt),
+	HBG_STATS_I(rx_desc_l4_len_err_cnt),
+	HBG_STATS_I(rx_desc_l4_csum_err_cnt),
+	HBG_STATS_I(rx_desc_l4_zero_port_num_cnt),
+	HBG_STATS_I(rx_desc_l4_other_cnt),
+	HBG_STATS_I(rx_desc_frag_cnt),
+	HBG_STATS_I(rx_desc_ip_ver_err_cnt),
+	HBG_STATS_I(rx_desc_ipv4_pkt_cnt),
+	HBG_STATS_I(rx_desc_ipv6_pkt_cnt),
+	HBG_STATS_I(rx_desc_no_ip_pkt_cnt),
+	HBG_STATS_I(rx_desc_ip_pkt_cnt),
+	HBG_STATS_I(rx_desc_tcp_pkt_cnt),
+	HBG_STATS_I(rx_desc_udp_pkt_cnt),
+	HBG_STATS_I(rx_desc_vlan_pkt_cnt),
+	HBG_STATS_I(rx_desc_icmp_pkt_cnt),
+	HBG_STATS_I(rx_desc_arp_pkt_cnt),
+	HBG_STATS_I(rx_desc_rarp_pkt_cnt),
+	HBG_STATS_I(rx_desc_multicast_pkt_cnt),
+	HBG_STATS_I(rx_desc_broadcast_pkt_cnt),
+	HBG_STATS_I(rx_desc_ipsec_pkt_cnt),
+	HBG_STATS_I(rx_desc_ip_opt_pkt_cnt),
+	HBG_STATS_I(rx_desc_key_not_match_cnt),
+
+	HBG_STATS_REG_I(rx_octets_total_ok_cnt,
+			HBG_REG_RX_OCTETS_TOTAL_OK_ADDR),
+	HBG_STATS_REG_I(rx_octets_bad_cnt, HBG_REG_RX_OCTETS_BAD_ADDR),
+	HBG_STATS_REG_I(rx_octets_total_filt_cnt,
+			HBG_REG_RX_OCTETS_TOTAL_FILT_ADDR),
+	HBG_STATS_REG_I(rx_uc_pkts_cnt, HBG_REG_RX_UC_PKTS_ADDR),
+	HBG_STATS_REG_I(rx_mc_pkts_cnt, HBG_REG_RX_MC_PKTS_ADDR),
+	HBG_STATS_REG_I(rx_bc_pkts_cnt, HBG_REG_RX_BC_PKTS_ADDR),
+	HBG_STATS_REG_I(rx_vlan_pkt_cnt, HBG_REG_RX_TAGGED_ADDR),
+	HBG_STATS_REG_I(rx_filt_pkt_cnt, HBG_REG_RX_FILT_PKT_CNT_ADDR),
+	HBG_STATS_REG_I(rx_trans_pkg_cnt, HBG_REG_RX_TRANS_PKG_CNT_ADDR),
+	HBG_STATS_REG_I(rx_framesize_64, HBG_REG_RX_PKTS_64OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_65_127,
+			HBG_REG_RX_PKTS_65TO127OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_128_255,
+			HBG_REG_RX_PKTS_128TO255OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_256_511,
+			HBG_REG_RX_PKTS_256TO511OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_512_1023,
+			HBG_REG_RX_PKTS_512TO1023OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_1024_1518,
+			HBG_REG_RX_PKTS_1024TO1518OCTETS_ADDR),
+	HBG_STATS_REG_I(rx_framesize_bt_1518,
+			HBG_REG_RX_PKTS_1519TOMAXOCTETS_ADDR),
+	HBG_STATS_REG_I(rx_fcs_error_cnt, HBG_REG_RX_FCS_ERRORS_ADDR),
+	HBG_STATS_REG_I(rx_data_error_cnt, HBG_REG_RX_DATA_ERR_ADDR),
+	HBG_STATS_REG_I(rx_align_error_cnt, HBG_REG_RX_ALIGN_ERRORS_ADDR),
+	HBG_STATS_REG_I(rx_frame_long_err_cnt, HBG_REG_RX_LONG_ERRORS_ADDR),
+	HBG_STATS_REG_I(rx_jabber_err_cnt, HBG_REG_RX_JABBER_ERRORS_ADDR),
+	HBG_STATS_REG_I(rx_pause_macctl_frame_cnt,
+			HBG_REG_RX_PAUSE_MACCTL_FRAMCOUNTER_ADDR),
+	HBG_STATS_REG_I(rx_unknown_macctl_frame_cnt,
+			HBG_REG_RX_UNKNOWN_MACCTL_FRAMCOUNTER_ADDR),
+	HBG_STATS_REG_I(rx_frame_very_long_err_cnt,
+			HBG_REG_RX_VERY_LONG_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_frame_runt_err_cnt, HBG_REG_RX_RUNT_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_frame_short_err_cnt, HBG_REG_RX_SHORT_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_over_flow_cnt, HBG_REG_RX_OVER_FLOW_CNT_ADDR),
+	HBG_STATS_REG_I(rx_addr_overflow_cnt, HBG_REG_RX_ADDR_OVERFLOW_ADDR),
+	HBG_STATS_REG_I(rx_bufrq_err_cnt, HBG_REG_RX_BUFRQ_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_we_err_cnt, HBG_REG_RX_WE_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_overrun_cnt, HBG_REG_RX_OVERRUN_CNT_ADDR),
+	HBG_STATS_REG_I(rx_lengthfield_err_cnt,
+			HBG_REG_RX_LENGTHFIELD_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(rx_fail_comma_cnt, HBG_REG_RX_FAIL_COMMA_CNT_ADDR),
+	HBG_STATS_I(rx_dma_err_cnt),
+	HBG_STATS_I(rx_fifo_fill_full_cnt),
+
+	HBG_STATS_REG_I(tx_octets_total_ok_cnt,
+			HBG_REG_OCTETS_TRANSMITTED_OK_ADDR),
+	HBG_STATS_REG_I(tx_uc_pkts_cnt, HBG_REG_TX_UC_PKTS_ADDR),
+	HBG_STATS_REG_I(tx_mc_pkts_cnt, HBG_REG_TX_MC_PKTS_ADDR),
+	HBG_STATS_REG_I(tx_bc_pkts_cnt, HBG_REG_TX_BC_PKTS_ADDR),
+	HBG_STATS_REG_I(tx_vlan_pkt_cnt, HBG_REG_TX_TAGGED_ADDR),
+	HBG_STATS_REG_I(tx_octets_bad_cnt, HBG_REG_OCTETS_TRANSMITTED_BAD_ADDR),
+	HBG_STATS_REG_I(tx_trans_pkg_cnt, HBG_REG_TX_TRANS_PKG_CNT_ADDR),
+	HBG_STATS_REG_I(tx_pause_frame_cnt, HBG_REG_TX_PAUSE_FRAMES_ADDR),
+	HBG_STATS_REG_I(tx_framesize_64, HBG_REG_TX_PKTS_64OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_65_127,
+			HBG_REG_TX_PKTS_65TO127OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_128_255,
+			HBG_REG_TX_PKTS_128TO255OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_256_511,
+			HBG_REG_TX_PKTS_256TO511OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_512_1023,
+			HBG_REG_TX_PKTS_512TO1023OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_1024_1518,
+			HBG_REG_TX_PKTS_1024TO1518OCTETS_ADDR),
+	HBG_STATS_REG_I(tx_framesize_bt_1518,
+			HBG_REG_TX_PKTS_1519TOMAXOCTETS_ADDR),
+	HBG_STATS_REG_I(tx_underrun_err_cnt, HBG_REG_TX_UNDERRUN_ADDR),
+	HBG_STATS_REG_I(tx_add_cs_fail_cnt, HBG_REG_TX_CS_FAIL_CNT_ADDR),
+	HBG_STATS_REG_I(tx_bufrl_err_cnt, HBG_REG_TX_BUFRL_ERR_CNT_ADDR),
+	HBG_STATS_REG_I(tx_crc_err_cnt, HBG_REG_TX_CRC_ERROR_ADDR),
+	HBG_STATS_REG_I(tx_drop_cnt, HBG_REG_TX_DROP_CNT_ADDR),
+	HBG_STATS_REG_I(tx_excessive_length_drop_cnt,
+			HBG_REG_TX_EXCESSIVE_LENGTH_DROP_ADDR),
+	HBG_STATS_I(tx_dma_err_cnt),
+};
+
+static int hbg_ethtool_get_sset_count(struct net_device *netdev, int stringset)
+{
+	if (stringset != ETH_SS_STATS)
+		return -EOPNOTSUPP;
+
+	return ARRAY_SIZE(hbg_ethtool_stats_map);
+}
+
+static void hbg_ethtool_get_strings(struct net_device *netdev,
+				    u32 stringset, u8 *data)
+{
+	u32 i;
+
+	if (stringset != ETH_SS_STATS)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(hbg_ethtool_stats_map); i++)
+		ethtool_puts(&data, hbg_ethtool_stats_map[i].name);
+}
+
+void hbg_update_stats(struct hbg_priv *priv)
+{
+	const struct hbg_ethtool_stats *stats_info;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(hbg_ethtool_stats_map); i++) {
+		stats_info = &hbg_ethtool_stats_map[i];
+		if (!stats_info->reg)
+			continue;
+
+		HBG_STATS_UPDATE(&priv->stats, stats_info->offset,
+				 hbg_reg_read(priv, stats_info->reg));
+	}
+}
+
+static void hbg_ethtool_get_stats(struct net_device *netdev,
+				  struct ethtool_stats *stats, u64 *data)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+	u32 i;
+
+	hbg_update_stats(priv);
+	for (i = 0; i < ARRAY_SIZE(hbg_ethtool_stats_map); i++)
+		*data++ = HBG_STATS_READ(&priv->stats,
+					 hbg_ethtool_stats_map[i].offset);
+}
 
 static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
 	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.get_sset_count		= hbg_ethtool_get_sset_count,
+	.get_strings		= hbg_ethtool_get_strings,
+	.get_ethtool_stats	= hbg_ethtool_get_stats,
 };
 
 void hbg_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
index 628707ec2686..995ec369ff30 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
@@ -7,5 +7,6 @@
 #include <linux/netdevice.h>
 
 void hbg_ethtool_set_ops(struct net_device *netdev);
+void hbg_update_stats(struct hbg_priv *priv);
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 75505fb5cc4a..33fe92104e90 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -134,6 +134,24 @@ static void hbg_net_tx_timeout(struct net_device *netdev, unsigned int txqueue)
 	netdev_info(netdev, "%s", buf);
 }
 
+static void hbg_net_get_stats(struct net_device *netdev,
+			      struct rtnl_link_stats64 *stats)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+	struct hbg_stats *h_stats = &priv->stats;
+
+	hbg_update_stats(priv);
+	dev_get_tstats64(netdev, stats);
+	stats->tx_dropped += h_stats->tx_drop_cnt;
+	stats->tx_dropped += h_stats->tx_excessive_length_drop_cnt;
+
+	stats->tx_errors += h_stats->tx_add_cs_fail_cnt;
+	stats->tx_errors += h_stats->tx_bufrl_err_cnt;
+	stats->tx_errors += h_stats->tx_underrun_err_cnt;
+	stats->tx_errors += h_stats->tx_we_err_cnt;
+	stats->tx_errors += h_stats->tx_crc_err_cnt;
+}
+
 static const struct net_device_ops hbg_netdev_ops = {
 	.ndo_open		= hbg_net_open,
 	.ndo_stop		= hbg_net_stop,
@@ -142,8 +160,35 @@ static const struct net_device_ops hbg_netdev_ops = {
 	.ndo_set_mac_address	= hbg_net_set_mac_address,
 	.ndo_change_mtu		= hbg_net_change_mtu,
 	.ndo_tx_timeout		= hbg_net_tx_timeout,
+	.ndo_get_stats64	= hbg_net_get_stats,
 };
 
+static void hbg_service_task(struct work_struct *work)
+{
+	struct hbg_priv *priv = container_of(work, struct hbg_priv,
+					     service_task.work);
+
+	/* The type of statistics register is u32,
+	 * and the type of driver statistics is u64.
+	 * To prevent the statistics register from overflowing,
+	 * the driver dumps the statistics every 5 minutes.
+	 */
+	hbg_update_stats(priv);
+	schedule_delayed_work(&priv->service_task,
+			      msecs_to_jiffies(5 * 60 * MSEC_PER_SEC));
+}
+
+static void hbg_delaywork_init(struct hbg_priv *priv)
+{
+	INIT_DELAYED_WORK(&priv->service_task, hbg_service_task);
+	schedule_delayed_work(&priv->service_task, 0);
+}
+
+static void hbg_delaywork_uninit(void *data)
+{
+	cancel_delayed_work_sync(data);
+}
+
 static int hbg_init(struct hbg_priv *priv)
 {
 	int ret;
@@ -160,7 +205,13 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
-	return hbg_mdio_init(priv);
+	ret = hbg_mdio_init(priv);
+	if (ret)
+		return ret;
+
+	hbg_delaywork_init(priv);
+	return devm_add_action_or_reset(&priv->pdev->dev, hbg_delaywork_uninit,
+					&priv->service_task);
 }
 
 static int hbg_pci_init(struct pci_dev *pdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index 57d81c6d7633..b1dbaa98c6b3 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -47,12 +47,57 @@
 #define HBG_REG_TRANSMIT_CTRL_PAD_EN_B		BIT(7)
 #define HBG_REG_TRANSMIT_CTRL_CRC_ADD_B		BIT(6)
 #define HBG_REG_TRANSMIT_CTRL_AN_EN_B		BIT(5)
+#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)
+#define HBG_REG_RX_MC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x008C)
+#define HBG_REG_RX_BC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x0090)
+#define HBG_REG_RX_PKTS_64OCTETS_ADDR		(HBG_REG_SGMII_BASE + 0x0094)
+#define HBG_REG_RX_PKTS_65TO127OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x0098)
+#define HBG_REG_RX_PKTS_128TO255OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x009C)
+#define HBG_REG_RX_PKTS_256TO511OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x00A0)
+#define HBG_REG_RX_PKTS_512TO1023OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x00A4)
+#define HBG_REG_RX_PKTS_1024TO1518OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x00A8)
+#define HBG_REG_RX_PKTS_1519TOMAXOCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x00AC)
+#define HBG_REG_RX_FCS_ERRORS_ADDR		(HBG_REG_SGMII_BASE + 0x00B0)
+#define HBG_REG_RX_TAGGED_ADDR			(HBG_REG_SGMII_BASE + 0x00B4)
+#define HBG_REG_RX_DATA_ERR_ADDR		(HBG_REG_SGMII_BASE + 0x00B8)
+#define HBG_REG_RX_ALIGN_ERRORS_ADDR		(HBG_REG_SGMII_BASE + 0x00BC)
+#define HBG_REG_RX_LONG_ERRORS_ADDR		(HBG_REG_SGMII_BASE + 0x00C0)
+#define HBG_REG_RX_JABBER_ERRORS_ADDR		(HBG_REG_SGMII_BASE + 0x00C4)
+#define HBG_REG_RX_PAUSE_MACCTL_FRAMCOUNTER_ADDR   (HBG_REG_SGMII_BASE + 0x00C8)
+#define HBG_REG_RX_UNKNOWN_MACCTL_FRAMCOUNTER_ADDR (HBG_REG_SGMII_BASE + 0x00CC)
+#define HBG_REG_RX_VERY_LONG_ERR_CNT_ADDR	(HBG_REG_SGMII_BASE + 0x00D0)
+#define HBG_REG_RX_RUNT_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x00D4)
+#define HBG_REG_RX_SHORT_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x00D8)
+#define HBG_REG_RX_FILT_PKT_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x00E8)
+#define HBG_REG_RX_OCTETS_TOTAL_FILT_ADDR	(HBG_REG_SGMII_BASE + 0x00EC)
+#define HBG_REG_OCTETS_TRANSMITTED_OK_ADDR	(HBG_REG_SGMII_BASE + 0x0100)
+#define HBG_REG_OCTETS_TRANSMITTED_BAD_ADDR	(HBG_REG_SGMII_BASE + 0x0104)
+#define HBG_REG_TX_UC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x0108)
+#define HBG_REG_TX_MC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x010C)
+#define HBG_REG_TX_BC_PKTS_ADDR			(HBG_REG_SGMII_BASE + 0x0110)
+#define HBG_REG_TX_PKTS_64OCTETS_ADDR		(HBG_REG_SGMII_BASE + 0x0114)
+#define HBG_REG_TX_PKTS_65TO127OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x0118)
+#define HBG_REG_TX_PKTS_128TO255OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x011C)
+#define HBG_REG_TX_PKTS_256TO511OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x0120)
+#define HBG_REG_TX_PKTS_512TO1023OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x0124)
+#define HBG_REG_TX_PKTS_1024TO1518OCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x0128)
+#define HBG_REG_TX_PKTS_1519TOMAXOCTETS_ADDR	(HBG_REG_SGMII_BASE + 0x012C)
+#define HBG_REG_TX_EXCESSIVE_LENGTH_DROP_ADDR	(HBG_REG_SGMII_BASE + 0x014C)
+#define HBG_REG_TX_UNDERRUN_ADDR		(HBG_REG_SGMII_BASE + 0x0150)
+#define HBG_REG_TX_TAGGED_ADDR			(HBG_REG_SGMII_BASE + 0x0154)
+#define HBG_REG_TX_CRC_ERROR_ADDR		(HBG_REG_SGMII_BASE + 0x0158)
+#define HBG_REG_TX_PAUSE_FRAMES_ADDR		(HBG_REG_SGMII_BASE + 0x015C)
 #define HBG_REG_CF_CRC_STRIP_ADDR		(HBG_REG_SGMII_BASE + 0x01B0)
 #define HBG_REG_CF_CRC_STRIP_B			BIT(0)
 #define HBG_REG_MODE_CHANGE_EN_ADDR		(HBG_REG_SGMII_BASE + 0x01B4)
 #define HBG_REG_MODE_CHANGE_EN_B		BIT(0)
 #define HBG_REG_RECV_CTRL_ADDR			(HBG_REG_SGMII_BASE + 0x01E0)
 #define HBG_REG_RECV_CTRL_STRIP_PAD_EN_B	BIT(3)
+#define HBG_REG_RX_OVERRUN_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x01EC)
+#define HBG_REG_RX_LENGTHFIELD_ERR_CNT_ADDR	(HBG_REG_SGMII_BASE + 0x01F4)
+#define HBG_REG_RX_FAIL_COMMA_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x01F8)
 #define HBG_REG_STATION_ADDR_LOW_2_ADDR		(HBG_REG_SGMII_BASE + 0x0210)
 #define HBG_REG_STATION_ADDR_HIGH_2_ADDR	(HBG_REG_SGMII_BASE + 0x0214)
 
@@ -79,10 +124,16 @@
 #define HBG_REG_CF_INTRPT_STAT_ADDR		(HBG_REG_SGMII_BASE + 0x0434)
 #define HBG_REG_CF_INTRPT_CLR_ADDR		(HBG_REG_SGMII_BASE + 0x0438)
 #define HBG_REG_MAX_FRAME_LEN_ADDR		(HBG_REG_SGMII_BASE + 0x0444)
+#define HBG_REG_TX_DROP_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0448)
+#define HBG_REG_RX_OVER_FLOW_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x044C)
 #define HBG_REG_MAX_FRAME_LEN_M			GENMASK(15, 0)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR		(HBG_REG_SGMII_BASE + 0x045C)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR_TX_M	GENMASK(8, 0)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR_RX_M	GENMASK(24, 16)
+#define HBG_REG_TX_CS_FAIL_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0460)
+#define HBG_REG_RX_TRANS_PKG_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0464)
+#define HBG_REG_TX_TRANS_PKG_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0468)
+#define HBG_REG_RX_ADDR_OVERFLOW_ADDR		(HBG_REG_SGMII_BASE + 0x046C)
 #define HBG_REG_TX_CFF_ADDR_0_ADDR		(HBG_REG_SGMII_BASE + 0x0488)
 #define HBG_REG_TX_CFF_ADDR_1_ADDR		(HBG_REG_SGMII_BASE + 0x048C)
 #define HBG_REG_TX_CFF_ADDR_2_ADDR		(HBG_REG_SGMII_BASE + 0x0490)
@@ -101,6 +152,9 @@
 #define HBG_REG_RX_CTRL_RXBUF_1ST_SKIP_SIZE2_M	GENMASK(3, 0)
 #define HBG_REG_RX_PKT_MODE_ADDR		(HBG_REG_SGMII_BASE + 0x04F4)
 #define HBG_REG_RX_PKT_MODE_PARSE_MODE_M	GENMASK(22, 21)
+#define HBG_REG_RX_BUFRQ_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x058C)
+#define HBG_REG_TX_BUFRL_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0590)
+#define HBG_REG_RX_WE_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0594)
 #define HBG_REG_CF_IND_TXINT_MSK_ADDR		(HBG_REG_SGMII_BASE + 0x0694)
 #define HBG_REG_IND_INTR_MASK_B			BIT(0)
 #define HBG_REG_CF_IND_TXINT_STAT_ADDR		(HBG_REG_SGMII_BASE + 0x0698)
@@ -139,5 +193,48 @@ struct hbg_rx_desc {
 };
 
 #define HBG_RX_DESC_W2_PKT_LEN_M	GENMASK(31, 16)
+#define HBG_RX_DESC_W2_PORT_NUM_M	GENMASK(15, 12)
+#define HBG_RX_DESC_W4_IP_TCP_UDP_M	GENMASK(31, 30)
+#define HBG_RX_DESC_W4_IPSEC_B		BIT(29)
+#define HBG_RX_DESC_W4_IP_VERSION_B	BIT(28)
+#define HBG_RX_DESC_W4_L4_ERR_CODE_M	GENMASK(26, 23)
+#define HBG_RX_DESC_W4_FRAG_B		BIT(22)
+#define HBG_RX_DESC_W4_OPT_B		BIT(21)
+#define HBG_RX_DESC_W4_IP_VERSION_ERR_B	BIT(20)
+#define HBG_RX_DESC_W4_BRD_CST_B	BIT(19)
+#define HBG_RX_DESC_W4_MUL_CST_B	BIT(18)
+#define HBG_RX_DESC_W4_ARP_B		BIT(17)
+#define HBG_RX_DESC_W4_RARP_B		BIT(16)
+#define HBG_RX_DESC_W4_ICMP_B		BIT(15)
+#define HBG_RX_DESC_W4_VLAN_FLAG_B	BIT(14)
+#define HBG_RX_DESC_W4_DROP_B		BIT(13)
+#define HBG_RX_DESC_W4_L3_ERR_CODE_M	GENMASK(12, 9)
+#define HBG_RX_DESC_W4_L2_ERR_B		BIT(8)
+#define HBG_RX_DESC_W4_IDX_MATCH_B	BIT(7)
+
+enum hbg_l3_err_code {
+	HBG_L3_OK,
+	HBG_L3_WRONG_HEAD,
+	HBG_L3_CSUM_ERR,
+	HBG_L3_LEN_ERR,
+	HBG_L3_ZERO_TTL,
+	HBG_L3_RSVD,
+};
+
+enum hbg_l4_err_code {
+	HBG_L4_OK,
+	HBG_L4_WRONG_HEAD,
+	HBG_L4_LEN_ERR,
+	HBG_L4_CSUM_ERR,
+	HBG_L4_ZERO_PORT_NUM,
+	HBG_L4_RSVD,
+};
+
+enum hbg_pkt_type_code {
+	HBG_NO_IP_PKT = 0,
+	HBG_IP_PKT,
+	HBG_TCP_PKT,
+	HBG_UDP_PKT,
+};
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
index f4f256a0dfea..d100b2357ec2 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
@@ -38,8 +38,14 @@ static int hbg_dma_map(struct hbg_buffer *buffer)
 	buffer->skb_dma = dma_map_single(&priv->pdev->dev,
 					 buffer->skb->data, buffer->skb_len,
 					 buffer_to_dma_dir(buffer));
-	if (unlikely(dma_mapping_error(&priv->pdev->dev, buffer->skb_dma)))
+	if (unlikely(dma_mapping_error(&priv->pdev->dev, buffer->skb_dma))) {
+		if (buffer->dir == HBG_DIR_RX)
+			priv->stats.rx_dma_err_cnt++;
+		else
+			priv->stats.tx_dma_err_cnt++;
+
 		return -ENOMEM;
+	}
 
 	return 0;
 }
@@ -195,14 +201,169 @@ static int hbg_napi_tx_recycle(struct napi_struct *napi, int budget)
 	return packet_done;
 }
 
+static bool hbg_rx_check_l3l4_error(struct hbg_priv *priv,
+				    struct hbg_rx_desc *desc)
+{
+	if (likely(!FIELD_GET(HBG_RX_DESC_W4_L3_ERR_CODE_M, desc->word4) &&
+		   !FIELD_GET(HBG_RX_DESC_W4_L4_ERR_CODE_M, desc->word4)))
+		return true;
+
+	switch (FIELD_GET(HBG_RX_DESC_W4_L3_ERR_CODE_M, desc->word4)) {
+	case HBG_L3_OK:
+		break;
+	case HBG_L3_WRONG_HEAD:
+		priv->stats.rx_desc_l3_wrong_head_cnt++;
+		return false;
+	case HBG_L3_CSUM_ERR:
+		priv->stats.rx_desc_l3_csum_err_cnt++;
+		return false;
+	case HBG_L3_LEN_ERR:
+		priv->stats.rx_desc_l3_len_err_cnt++;
+		return false;
+	case HBG_L3_ZERO_TTL:
+		priv->stats.rx_desc_l3_zero_ttl_cnt++;
+		return false;
+	default:
+		priv->stats.rx_desc_l3_other_cnt++;
+		return false;
+	}
+
+	switch (FIELD_GET(HBG_RX_DESC_W4_L4_ERR_CODE_M, desc->word4)) {
+	case HBG_L4_OK:
+		break;
+	case HBG_L4_WRONG_HEAD:
+		priv->stats.rx_desc_l4_wrong_head_cnt++;
+		return false;
+	case HBG_L4_LEN_ERR:
+		priv->stats.rx_desc_l4_len_err_cnt++;
+		return false;
+	case HBG_L4_CSUM_ERR:
+		priv->stats.rx_desc_l4_csum_err_cnt++;
+		return false;
+	case HBG_L4_ZERO_PORT_NUM:
+		priv->stats.rx_desc_l4_zero_port_num_cnt++;
+		return false;
+	default:
+		priv->stats.rx_desc_l4_other_cnt++;
+		return false;
+	}
+
+	return true;
+}
+
+static void hbg_update_rx_ip_protocol_stats(struct hbg_priv *priv,
+					    struct hbg_rx_desc *desc)
+{
+	if (unlikely(!FIELD_GET(HBG_RX_DESC_W4_IP_TCP_UDP_M, desc->word4))) {
+		priv->stats.rx_desc_no_ip_pkt_cnt++;
+		return;
+	}
+
+	if (unlikely(FIELD_GET(HBG_RX_DESC_W4_IP_VERSION_ERR_B, desc->word4))) {
+		priv->stats.rx_desc_ip_ver_err_cnt++;
+		return;
+	}
+
+	/* 0:ipv4, 1:ipv6 */
+	if (FIELD_GET(HBG_RX_DESC_W4_IP_VERSION_B, desc->word4))
+		priv->stats.rx_desc_ipv6_pkt_cnt++;
+	else
+		priv->stats.rx_desc_ipv4_pkt_cnt++;
+
+	switch (FIELD_GET(HBG_RX_DESC_W4_IP_TCP_UDP_M, desc->word4)) {
+	case HBG_IP_PKT:
+		priv->stats.rx_desc_ip_pkt_cnt++;
+		if (FIELD_GET(HBG_RX_DESC_W4_OPT_B, desc->word4))
+			priv->stats.rx_desc_ip_opt_pkt_cnt++;
+		if (FIELD_GET(HBG_RX_DESC_W4_FRAG_B, desc->word4))
+			priv->stats.rx_desc_frag_cnt++;
+
+		if (FIELD_GET(HBG_RX_DESC_W4_ICMP_B, desc->word4))
+			priv->stats.rx_desc_icmp_pkt_cnt++;
+		else if (FIELD_GET(HBG_RX_DESC_W4_IPSEC_B, desc->word4))
+			priv->stats.rx_desc_ipsec_pkt_cnt++;
+		break;
+	case HBG_TCP_PKT:
+		priv->stats.rx_desc_tcp_pkt_cnt++;
+		break;
+	case HBG_UDP_PKT:
+		priv->stats.rx_desc_udp_pkt_cnt++;
+		break;
+	default:
+		priv->stats.rx_desc_no_ip_pkt_cnt++;
+		break;
+	}
+}
+
+static void hbg_update_rx_protocol_stats(struct hbg_priv *priv,
+					 struct hbg_rx_desc *desc)
+{
+	if (unlikely(!FIELD_GET(HBG_RX_DESC_W4_IDX_MATCH_B, desc->word4))) {
+		priv->stats.rx_desc_key_not_match_cnt++;
+		return;
+	}
+
+	if (FIELD_GET(HBG_RX_DESC_W4_BRD_CST_B, desc->word4))
+		priv->stats.rx_desc_broadcast_pkt_cnt++;
+	else if (FIELD_GET(HBG_RX_DESC_W4_MUL_CST_B, desc->word4))
+		priv->stats.rx_desc_multicast_pkt_cnt++;
+
+	if (FIELD_GET(HBG_RX_DESC_W4_VLAN_FLAG_B, desc->word4))
+		priv->stats.rx_desc_vlan_pkt_cnt++;
+
+	if (FIELD_GET(HBG_RX_DESC_W4_ARP_B, desc->word4)) {
+		priv->stats.rx_desc_arp_pkt_cnt++;
+		return;
+	} else if (FIELD_GET(HBG_RX_DESC_W4_RARP_B, desc->word4)) {
+		priv->stats.rx_desc_rarp_pkt_cnt++;
+		return;
+	}
+
+	hbg_update_rx_ip_protocol_stats(priv, desc);
+}
+
+static bool hbg_rx_pkt_check(struct hbg_priv *priv, struct hbg_rx_desc *desc)
+{
+	if (unlikely(FIELD_GET(HBG_RX_DESC_W2_PKT_LEN_M, desc->word2) >
+		     priv->dev_specs.max_frame_len)) {
+		priv->netdev->stats.rx_length_errors++;
+		priv->stats.rx_desc_pkt_len_err_cnt++;
+		return false;
+	}
+
+	if (unlikely(FIELD_GET(HBG_RX_DESC_W2_PORT_NUM_M, desc->word2) !=
+		     priv->dev_specs.mac_id ||
+		     FIELD_GET(HBG_RX_DESC_W4_DROP_B, desc->word4))) {
+		priv->netdev->stats.rx_dropped++;
+		priv->stats.rx_desc_drop++;
+		return false;
+	}
+
+	if (unlikely(FIELD_GET(HBG_RX_DESC_W4_L2_ERR_B, desc->word4))) {
+		priv->netdev->stats.rx_errors++;
+		priv->stats.rx_desc_l2_err_cnt++;
+		return false;
+	}
+
+	if (unlikely(!hbg_rx_check_l3l4_error(priv, desc))) {
+		priv->netdev->stats.rx_errors++;
+		return false;
+	}
+
+	hbg_update_rx_protocol_stats(priv, desc);
+	return true;
+}
+
 static int hbg_rx_fill_one_buffer(struct hbg_priv *priv)
 {
 	struct hbg_ring *ring = &priv->rx_ring;
 	struct hbg_buffer *buffer;
 	int ret;
 
-	if (hbg_queue_is_full(ring->ntc, ring->ntu, ring))
+	if (hbg_queue_is_full(ring->ntc, ring->ntu, ring)) {
+		priv->stats.rx_fifo_fill_full_cnt++;
 		return 0;
+	}
 
 	buffer = &ring->queue[ring->ntu];
 	ret = hbg_buffer_alloc_skb(buffer);
@@ -257,8 +418,12 @@ static int hbg_napi_rx_poll(struct napi_struct *napi, int budget)
 		rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
 		pkt_len = FIELD_GET(HBG_RX_DESC_W2_PKT_LEN_M, rx_desc->word2);
 
-		hbg_dma_unmap(buffer);
+		if (unlikely(!hbg_rx_pkt_check(priv, rx_desc))) {
+			hbg_buffer_free(buffer);
+			goto next_buffer;
+		}
 
+		hbg_dma_unmap(buffer);
 		skb_reserve(buffer->skb, HBG_PACKET_HEAD_SIZE + NET_IP_ALIGN);
 		skb_put(buffer->skb, pkt_len);
 		buffer->skb->protocol = eth_type_trans(buffer->skb,
-- 
2.33.0

[PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Jijie Shao]

This patch supports querying the detailed status of the port
through debugfs, including the TX/RX ring, specifications,
interrupt and port status.

This driver supports four interrupts. the abnormal interrupt has
multiple interrupt sources. To locate the exception cause in detail,
the debugfs displays the status and count of each interrupt source.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../net/ethernet/hisilicon/hibmcge/Makefile   |   3 +-
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  | 150 ++++++++++++++++++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.h  |  12 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  26 ++-
 4 files changed, 189 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/Makefile b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
index ae58ac38c206..1a0ec2fb8c24 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/Makefile
+++ b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
@@ -5,4 +5,5 @@
 
 obj-$(CONFIG_HIBMCGE) += hibmcge.o
 
-hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o
+hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o \
+		hbg_debugfs.o
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
new file mode 100644
index 000000000000..e65e1d498d2b
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/seq_file.h>
+#include "hbg_common.h"
+#include "hbg_debugfs.h"
+#include "hbg_hw.h"
+#include "hbg_irq.h"
+#include "hbg_txrx.h"
+
+static struct dentry *hbg_dbgfs_root;
+
+struct hbg_dbg_info {
+	const char *name;
+	int (*read)(struct seq_file *seq, void *data);
+};
+
+#define hbg_get_bool_str(state) ((state) ? "true" : "false")
+
+static int hbg_dbg_dev_spec(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+	struct hbg_dev_specs *specs;
+
+	specs = &priv->dev_specs;
+	seq_printf(s, "mac id: %u\n", specs->mac_id);
+	seq_printf(s, "phy addr: %u\n", specs->phy_addr);
+	seq_printf(s, "mac addr: %pM\n", specs->mac_addr.sa_data);
+	seq_printf(s, "vlan layers: %u\n", specs->vlan_layers);
+	seq_printf(s, "max frame len: %u\n", specs->max_frame_len);
+	seq_printf(s, "min mtu: %u, max mtu: %u\n",
+		   specs->min_mtu, specs->max_mtu);
+	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
+
+	return 0;
+}
+
+static void hbg_dbg_ring(struct hbg_priv *priv, struct hbg_ring *ring,
+			 struct seq_file *s)
+{
+	u32 irq_mask = ring->dir == HBG_DIR_TX ? HBG_INT_MSK_TX_B :
+						 HBG_INT_MSK_RX_B;
+
+	seq_printf(s, "ring used num: %u\n",
+		   hbg_get_queue_used_num(ring));
+	seq_printf(s, "ring max num: %u\n", ring->len);
+	seq_printf(s, "ring head: %u, tail: %u\n", ring->head, ring->tail);
+	seq_printf(s, "fifo used num: %u\n",
+		   hbg_hw_get_fifo_used_num(priv, ring->dir));
+	seq_printf(s, "fifo max num: %u\n",
+		   hbg_get_spec_fifo_max_num(priv, ring->dir));
+	seq_printf(s, "irq enabled: %s\n",
+		   hbg_get_bool_str(hbg_hw_irq_is_enabled(priv, irq_mask)));
+}
+
+static int hbg_dbg_tx_ring(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_dbg_ring(priv, &priv->tx_ring, s);
+	return 0;
+}
+
+static int hbg_dbg_rx_ring(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_dbg_ring(priv, &priv->rx_ring, s);
+	return 0;
+}
+
+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;
+	u32 i;
+
+	for (i = 0; i < priv->vectors.info_array_len; i++) {
+		info = &priv->vectors.info_array[i];
+		seq_printf(s,
+			   "%-20s: is enabled: %s, print: %s, count: %llu\n",
+			   info->name,
+			   hbg_get_bool_str(hbg_hw_irq_is_enabled(priv,
+								  info->mask)),
+			   hbg_get_bool_str(info->need_print),
+			   info->count);
+	}
+
+	return 0;
+}
+
+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);
+
+	seq_printf(s, "event handling state: %s\n",
+		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
+					     &priv->state)));
+
+	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);
+	return 0;
+}
+
+static const struct hbg_dbg_info hbg_dbg_infos[] = {
+	{ "dev_spec", hbg_dbg_dev_spec },
+	{ "tx_ring", hbg_dbg_tx_ring },
+	{ "rx_ring", hbg_dbg_rx_ring },
+	{ "irq_info", hbg_dbg_irq_info },
+	{ "nic_state", hbg_dbg_nic_state },
+};
+
+static void hbg_debugfs_uninit(void *data)
+{
+	debugfs_remove_recursive((struct dentry *)data);
+}
+
+int hbg_debugfs_init(struct hbg_priv *priv)
+{
+	const char *name = pci_name(priv->pdev);
+	struct device *dev = &priv->pdev->dev;
+	struct dentry *root;
+	u32 i;
+
+	root = debugfs_create_dir(name, hbg_dbgfs_root);
+
+	for (i = 0; i < ARRAY_SIZE(hbg_dbg_infos); i++)
+		debugfs_create_devm_seqfile(dev, hbg_dbg_infos[i].name,
+					    root, hbg_dbg_infos[i].read);
+
+	return devm_add_action_or_reset(dev, hbg_debugfs_uninit, root);
+}
+
+void hbg_debugfs_register(void)
+{
+	hbg_dbgfs_root = debugfs_create_dir("hibmcge", NULL);
+}
+
+void hbg_debugfs_unregister(void)
+{
+	debugfs_remove_recursive(hbg_dbgfs_root);
+	hbg_dbgfs_root = NULL;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h
new file mode 100644
index 000000000000..678651ec710b
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_DEBUGFS_H
+#define __HBG_DEBUGFS_H
+
+void hbg_debugfs_register(void);
+void hbg_debugfs_unregister(void);
+
+int hbg_debugfs_init(struct hbg_priv *priv);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 33fe92104e90..30576483a938 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -11,6 +11,7 @@
 #include "hbg_irq.h"
 #include "hbg_mdio.h"
 #include "hbg_txrx.h"
+#include "hbg_debugfs.h"
 
 static void hbg_change_mtu(struct hbg_priv *priv, int new_mtu);
 
@@ -209,6 +210,10 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
+	ret = hbg_debugfs_init(priv);
+	if (ret)
+		return ret;
+
 	hbg_delaywork_init(priv);
 	return devm_add_action_or_reset(&priv->pdev->dev, hbg_delaywork_uninit,
 					&priv->service_task);
@@ -296,7 +301,26 @@ static struct pci_driver hbg_driver = {
 	.id_table	= hbg_pci_tbl,
 	.probe		= hbg_probe,
 };
-module_pci_driver(hbg_driver);
+
+static int __init hbg_module_init(void)
+{
+	int ret;
+
+	hbg_debugfs_register();
+	ret = pci_register_driver(&hbg_driver);
+	if (ret)
+		hbg_debugfs_unregister();
+
+	return ret;
+}
+module_init(hbg_module_init);
+
+static void __exit hbg_module_exit(void)
+{
+	pci_unregister_driver(&hbg_driver);
+	hbg_debugfs_unregister();
+}
+module_exit(hbg_module_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
-- 
2.33.0

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Andrew Lunn]

> +static int hbg_dbg_dev_spec(struct seq_file *s, void *unused)
> +{
> +	struct net_device *netdev = dev_get_drvdata(s->private);
> +	struct hbg_priv *priv = netdev_priv(netdev);
> +	struct hbg_dev_specs *specs;
> +
> +	specs = &priv->dev_specs;
> +	seq_printf(s, "mac id: %u\n", specs->mac_id);
> +	seq_printf(s, "phy addr: %u\n", specs->phy_addr);
> +	seq_printf(s, "mac addr: %pM\n", specs->mac_addr.sa_data);
> +	seq_printf(s, "vlan layers: %u\n", specs->vlan_layers);
> +	seq_printf(s, "max frame len: %u\n", specs->max_frame_len);
> +	seq_printf(s, "min mtu: %u, max mtu: %u\n",
> +		   specs->min_mtu, specs->max_mtu);

I think these are all available via standard APIs. There is no need to
have them in debugfs as well.

> +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);

Is this interesting? Are you clocking it greater than 2.5MHz?

> +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;
> +	u32 i;
> +
> +	for (i = 0; i < priv->vectors.info_array_len; i++) {
> +		info = &priv->vectors.info_array[i];
> +		seq_printf(s,
> +			   "%-20s: is enabled: %s, print: %s, count: %llu\n",
> +			   info->name,
> +			   hbg_get_bool_str(hbg_hw_irq_is_enabled(priv,
> +								  info->mask)),
> +			   hbg_get_bool_str(info->need_print),
> +			   info->count);
> +	}

How does this differ from what is available already from the IRQ
subsystem?

> +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);
> +
> +	seq_printf(s, "event handling state: %s\n",
> +		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
> +					     &priv->state)));
> +
> +	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);

Don't you have this via ethtool -S ?

> @@ -209,6 +210,10 @@ static int hbg_init(struct hbg_priv *priv)
>  	if (ret)
>  		return ret;
>  
> +	ret = hbg_debugfs_init(priv);
> +	if (ret)
> +		return ret;
> +

There is no need to test the results from debugfs calls.

> +static int __init hbg_module_init(void)
> +{
> +	int ret;
> +
> +	hbg_debugfs_register();
> +	ret = pci_register_driver(&hbg_driver);
> +	if (ret)
> +		hbg_debugfs_unregister();

This seems odd. I would expect that each device has its own debugfs,
there is nothing global.

	Andrew

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Jijie Shao]

on 2024/10/23 22:00, Andrew Lunn wrote:
>> +static int hbg_dbg_dev_spec(struct seq_file *s, void *unused)
>> +{
>> +	struct net_device *netdev = dev_get_drvdata(s->private);
>> +	struct hbg_priv *priv = netdev_priv(netdev);
>> +	struct hbg_dev_specs *specs;
>> +
>> +	specs = &priv->dev_specs;
>> +	seq_printf(s, "mac id: %u\n", specs->mac_id);
>> +	seq_printf(s, "phy addr: %u\n", specs->phy_addr);
>> +	seq_printf(s, "mac addr: %pM\n", specs->mac_addr.sa_data);
>> +	seq_printf(s, "vlan layers: %u\n", specs->vlan_layers);
>> +	seq_printf(s, "max frame len: %u\n", specs->max_frame_len);
>> +	seq_printf(s, "min mtu: %u, max mtu: %u\n",
>> +		   specs->min_mtu, specs->max_mtu);
> I think these are all available via standard APIs. There is no need to
> have them in debugfs as well.

Yes, and these specifications are displayed by running the ethtool -d command. You can delete these specifications,
We will discuss internally, there is a high probability that this debugfs file will be deleted in v2.

>
>> +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
> Is this interesting? Are you clocking it greater than 2.5MHz?

MDIO controller supports 1MHz, 2.5MHz, 12.5MHz, and 25MHz
Of course, we chose and tested 2.5M in actual work, but this can be modified.

>
>> +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;
>> +	u32 i;
>> +
>> +	for (i = 0; i < priv->vectors.info_array_len; i++) {
>> +		info = &priv->vectors.info_array[i];
>> +		seq_printf(s,
>> +			   "%-20s: is enabled: %s, print: %s, count: %llu\n",
>> +			   info->name,
>> +			   hbg_get_bool_str(hbg_hw_irq_is_enabled(priv,
>> +								  info->mask)),
>> +			   hbg_get_bool_str(info->need_print),
>> +			   info->count);
>> +	}
> How does this differ from what is available already from the IRQ
> subsystem?

We requested three interrupts: "tx", "rx", "err"
The err interrupt is a summary interrupt. We distinguish different errors
based on the status register and mask.

With "cat /proc/interrupts | grep hibmcge",
we can't distinguish the detailed cause of the error,
so we added this file to debugfs.

the following effects are achieved:
[root@localhost sjj]# cat /sys/kernel/debug/hibmcge/0000\:83\:00.1/irq_info
RX                  : is enabled: true, print: false, count: 2
TX                  : is enabled: true, print: false, count: 0
MAC_MII_FIFO_ERR    : is enabled: false, print: true, count: 0
MAC_PCS_RX_FIFO_ERR : is enabled: false, print: true, count: 0
MAC_PCS_TX_FIFO_ERR : is enabled: false, print: true, count: 0
MAC_APP_RX_FIFO_ERR : is enabled: false, print: true, count: 0
MAC_APP_TX_FIFO_ERR : is enabled: false, print: true, count: 0
SRAM_PARITY_ERR     : is enabled: true, print: true, count: 0
TX_AHB_ERR          : is enabled: true, print: true, count: 0
RX_BUF_AVL          : is enabled: true, print: false, count: 0
REL_BUF_ERR         : is enabled: true, print: true, count: 0
TXCFG_AVL           : is enabled: true, print: false, count: 0
TX_DROP             : is enabled: true, print: false, count: 0
RX_DROP             : is enabled: true, print: false, count: 0
RX_AHB_ERR          : is enabled: true, print: true, count: 0
MAC_FIFO_ERR        : is enabled: true, print: false, count: 0
RBREQ_ERR           : is enabled: true, print: false, count: 0
WE_ERR              : is enabled: true, print: false, count: 0


The irq framework of hibmcge driver also includes tx/rx interrupts.
Therefore, these interrupts are not distinguished separately in debugfs.

>
>> +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);
>> +
>> +	seq_printf(s, "event handling state: %s\n",
>> +		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
>> +					     &priv->state)));
>> +
>> +	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);
> Don't you have this via ethtool -S ?

Although tx_timeout_cnt is a statistical item, it is not displayed in the ethtool -S.

>
>> @@ -209,6 +210,10 @@ static int hbg_init(struct hbg_priv *priv)
>>   	if (ret)
>>   		return ret;
>>   
>> +	ret = hbg_debugfs_init(priv);
>> +	if (ret)
>> +		return ret;
>> +
> There is no need to test the results from debugfs calls.

ok

>
>> +static int __init hbg_module_init(void)
>> +{
>> +	int ret;
>> +
>> +	hbg_debugfs_register();
>> +	ret = pci_register_driver(&hbg_driver);
>> +	if (ret)
>> +		hbg_debugfs_unregister();
> This seems odd. I would expect that each device has its own debugfs,
> there is nothing global.
>
> 	Andrew

Yes, that's how we designed it.
In this, We register and create the root dir of hibmcge,
And in each probe(), device create their own dir using bdf:

/sys/kernel/debug/hibmcge/0000\:83\:00.1/
/sys/kernel/debug/hibmcge/0000\:83\:00.2/

for each device:
[root@localhost sjj]# ls -n /sys/kernel/debug/hibmcge/0000\:83\:00.1/
-r--r--r--. 1 0 0 0 10月 24 09:42 dev_spec
-r--r--r--. 1 0 0 0 10月 24 09:42 irq_info
-r--r--r--. 1 0 0 0 10月 24 09:42 mac_talbe
-r--r--r--. 1 0 0 0 10月 24 09:42 nic_state
-r--r--r--. 1 0 0 0 10月 24 09:42 rx_ring
-r--r--r--. 1 0 0 0 10月 24 09:42 tx_ring


Thanks a lot!
Jijie Shao

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Andrew Lunn]

> > > +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
> > Is this interesting? Are you clocking it greater than 2.5MHz?
> 
> MDIO controller supports 1MHz, 2.5MHz, 12.5MHz, and 25MHz
> Of course, we chose and tested 2.5M in actual work, but this can be modified.

How? What API are you using it allow it to be modified? Why cannot you
get the value using the same API?

> We requested three interrupts: "tx", "rx", "err"
> The err interrupt is a summary interrupt. We distinguish different errors
> based on the status register and mask.
> 
> With "cat /proc/interrupts | grep hibmcge",
> we can't distinguish the detailed cause of the error,
> so we added this file to debugfs.
> 
> the following effects are achieved:
> [root@localhost sjj]# cat /sys/kernel/debug/hibmcge/0000\:83\:00.1/irq_info
> RX                  : is enabled: true, print: false, count: 2
> TX                  : is enabled: true, print: false, count: 0
> MAC_MII_FIFO_ERR    : is enabled: false, print: true, count: 0
> MAC_PCS_RX_FIFO_ERR : is enabled: false, print: true, count: 0
> MAC_PCS_TX_FIFO_ERR : is enabled: false, print: true, count: 0
> MAC_APP_RX_FIFO_ERR : is enabled: false, print: true, count: 0
> MAC_APP_TX_FIFO_ERR : is enabled: false, print: true, count: 0
> SRAM_PARITY_ERR     : is enabled: true, print: true, count: 0
> TX_AHB_ERR          : is enabled: true, print: true, count: 0
> RX_BUF_AVL          : is enabled: true, print: false, count: 0
> REL_BUF_ERR         : is enabled: true, print: true, count: 0
> TXCFG_AVL           : is enabled: true, print: false, count: 0
> TX_DROP             : is enabled: true, print: false, count: 0
> RX_DROP             : is enabled: true, print: false, count: 0
> RX_AHB_ERR          : is enabled: true, print: true, count: 0
> MAC_FIFO_ERR        : is enabled: true, print: false, count: 0
> RBREQ_ERR           : is enabled: true, print: false, count: 0
> WE_ERR              : is enabled: true, print: false, count: 0
> 
> 
> The irq framework of hibmcge driver also includes tx/rx interrupts.
> Therefore, these interrupts are not distinguished separately in debugfs.

Please make this a patch of its own, and include this in the commit
message.

Ideally you need to show there is no standard API for what you want to
put into debugfs, because if there is a standard API, you don't need
debugfs...

> 
> > 
> > > +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);
> > > +
> > > +	seq_printf(s, "event handling state: %s\n",
> > > +		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
> > > +					     &priv->state)));
> > > +
> > > +	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);
> > Don't you have this via ethtool -S ?
> 
> Although tx_timeout_cnt is a statistical item, it is not displayed in the ethtool -S.

Why?

	Andrew

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Jijie Shao]

on 2024/10/24 20:05, Andrew Lunn wrote:
>>>> +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
>>> Is this interesting? Are you clocking it greater than 2.5MHz?
>> MDIO controller supports 1MHz, 2.5MHz, 12.5MHz, and 25MHz
>> Of course, we chose and tested 2.5M in actual work, but this can be modified.
> How? What API are you using it allow it to be modified? Why cannot you
> get the value using the same API?

This frequency cannot be modified dynamically.
There are some specification registers that store some initialization configuration parameters
written by the BMC, such as the default MAC address and hardware FIFO size and mdio frequency.

When the device is in prob, the driver reads the related configuration information and
initializes the device based on the configuration.

>
>> We requested three interrupts: "tx", "rx", "err"
>> The err interrupt is a summary interrupt. We distinguish different errors
>> based on the status register and mask.
>>
>> With "cat /proc/interrupts | grep hibmcge",
>> we can't distinguish the detailed cause of the error,
>> so we added this file to debugfs.
>>
>> the following effects are achieved:
>> [root@localhost sjj]# cat /sys/kernel/debug/hibmcge/0000\:83\:00.1/irq_info
>> RX                  : is enabled: true, print: false, count: 2
>> TX                  : is enabled: true, print: false, count: 0
>> MAC_MII_FIFO_ERR    : is enabled: false, print: true, count: 0
>> MAC_PCS_RX_FIFO_ERR : is enabled: false, print: true, count: 0
>> MAC_PCS_TX_FIFO_ERR : is enabled: false, print: true, count: 0
>> MAC_APP_RX_FIFO_ERR : is enabled: false, print: true, count: 0
>> MAC_APP_TX_FIFO_ERR : is enabled: false, print: true, count: 0
>> SRAM_PARITY_ERR     : is enabled: true, print: true, count: 0
>> TX_AHB_ERR          : is enabled: true, print: true, count: 0
>> RX_BUF_AVL          : is enabled: true, print: false, count: 0
>> REL_BUF_ERR         : is enabled: true, print: true, count: 0
>> TXCFG_AVL           : is enabled: true, print: false, count: 0
>> TX_DROP             : is enabled: true, print: false, count: 0
>> RX_DROP             : is enabled: true, print: false, count: 0
>> RX_AHB_ERR          : is enabled: true, print: true, count: 0
>> MAC_FIFO_ERR        : is enabled: true, print: false, count: 0
>> RBREQ_ERR           : is enabled: true, print: false, count: 0
>> WE_ERR              : is enabled: true, print: false, count: 0
>>
>>
>> The irq framework of hibmcge driver also includes tx/rx interrupts.
>> Therefore, these interrupts are not distinguished separately in debugfs.
> Please make this a patch of its own, and include this in the commit
> message.
>
> Ideally you need to show there is no standard API for what you want to
> put into debugfs, because if there is a standard API, you don't need
> debugfs...

Because standard API don't meet my needs, I added detailed interrupt information to debugfs.
I'll add a detailed description to the commit message of v2.

>
>>>> +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);
>>>> +
>>>> +	seq_printf(s, "event handling state: %s\n",
>>>> +		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
>>>> +					     &priv->state)));
>>>> +
>>>> +	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);
>>> Don't you have this via ethtool -S ?
>> Although tx_timeout_cnt is a statistical item, it is not displayed in the ethtool -S.
> Why?
>
> 	Andrew

This was decided by our internal discussion before,
and we'll revisit it, and move it to ethtool -S in the next version if it's okay with us.

Thanks,
Jijie Shao

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Andrew Lunn]

On Thu, Oct 24, 2024 at 10:06:14PM +0800, Jijie Shao wrote:
> 
> on 2024/10/24 20:05, Andrew Lunn wrote:
> > > > > +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
> > > > Is this interesting? Are you clocking it greater than 2.5MHz?
> > > MDIO controller supports 1MHz, 2.5MHz, 12.5MHz, and 25MHz
> > > Of course, we chose and tested 2.5M in actual work, but this can be modified.
> > How? What API are you using it allow it to be modified? Why cannot you
> > get the value using the same API?
> 
> This frequency cannot be modified dynamically.
> There are some specification registers that store some initialization configuration parameters
> written by the BMC, such as the default MAC address and hardware FIFO size and mdio frequency.
> 
> When the device is in prob, the driver reads the related configuration information and
> initializes the device based on the configuration.

Does the BMC have an API to set these values? And show these values?

	Andrew

Re: [PATCH net-next 2/7] net: hibmcge: Add debugfs supported in this module

[Jijie Shao]

on 2024/10/24 22:21, Andrew Lunn wrote:
> On Thu, Oct 24, 2024 at 10:06:14PM +0800, Jijie Shao wrote:
>> on 2024/10/24 20:05, Andrew Lunn wrote:
>>>>>> +	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
>>>>> Is this interesting? Are you clocking it greater than 2.5MHz?
>>>> MDIO controller supports 1MHz, 2.5MHz, 12.5MHz, and 25MHz
>>>> Of course, we chose and tested 2.5M in actual work, but this can be modified.
>>> How? What API are you using it allow it to be modified? Why cannot you
>>> get the value using the same API?
>> This frequency cannot be modified dynamically.
>> There are some specification registers that store some initialization configuration parameters
>> written by the BMC, such as the default MAC address and hardware FIFO size and mdio frequency.
>>
>> When the device is in prob, the driver reads the related configuration information and
>> initializes the device based on the configuration.
> Does the BMC have an API to set these values? And show these values?
>
> 	Andrew

Currently, there are no other API except devmem.

But this is not important.
According to the discussion in patch "[PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module",
this debugfs file will be deleted. I will put these informations in register dump by ethtool -d.

Thanks

[PATCH net-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

[Jijie Shao]

MAC supports filtering unmatched unicast packets according to the
MAC address table. This patch adds the support for
unicast frame filtering.

To support automatic restoration of MAC entries
after reset, the driver saves a copy of MAC entries in the driver.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  13 ++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  |  26 ++++
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   |  17 ++-
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.h   |   3 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 141 +++++++++++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |   3 +
 6 files changed, 197 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 411afc9b916b..491192a4fc74 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -84,6 +84,7 @@ struct hbg_dev_specs {
 	u32 vlan_layers;
 	u32 max_mtu;
 	u32 min_mtu;
+	u32 uc_mac_num;
 
 	u32 max_frame_len;
 	u32 rx_buf_size;
@@ -214,6 +215,17 @@ struct hbg_stats {
 	u64 tx_dma_err_cnt;
 };
 
+struct hbg_mac_table_entry {
+	u8 addr[ETH_ALEN];
+};
+
+struct hbg_mac_filter {
+	struct hbg_mac_table_entry *mac_table;
+	u32 table_max_len;
+	bool table_overflow;
+	bool enabled;
+};
+
 struct hbg_priv {
 	struct net_device *netdev;
 	struct pci_dev *pdev;
@@ -226,6 +238,7 @@ struct hbg_priv {
 	struct hbg_ring rx_ring;
 	struct hbg_stats stats;
 	struct delayed_work service_task;
+	struct hbg_mac_filter filter;
 };
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
index e65e1d498d2b..85d5cb3cd603 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -35,6 +35,7 @@ static int hbg_dbg_dev_spec(struct seq_file *s, void *unused)
 	seq_printf(s, "min mtu: %u, max mtu: %u\n",
 		   specs->min_mtu, specs->max_mtu);
 	seq_printf(s, "mdio frequency: %u\n", specs->mdio_frequency);
+	seq_printf(s, "uc mac max num: %u\n", specs->uc_mac_num);
 
 	return 0;
 }
@@ -109,12 +110,37 @@ static int hbg_dbg_nic_state(struct seq_file *s, void *unused)
 	return 0;
 }
 
+static int hbg_dbg_mac_table(struct seq_file *s, void *unused)
+{
+	struct net_device *netdev = dev_get_drvdata(s->private);
+	struct hbg_priv *priv = netdev_priv(netdev);
+	struct hbg_mac_filter *filter;
+	u32 i;
+
+	filter = &priv->filter;
+	seq_printf(s, "mac addr max count: %u\n", filter->table_max_len);
+	seq_printf(s, "filter enabled: %s\n",
+		   hbg_get_bool_str(filter->enabled));
+	seq_printf(s, "table overflow: %s\n",
+		   hbg_get_bool_str(filter->table_overflow));
+
+	for (i = 0; i < filter->table_max_len; i++) {
+		if (is_zero_ether_addr(filter->mac_table[i].addr))
+			continue;
+
+		seq_printf(s, "[%u] %pM\n", i, filter->mac_table[i].addr);
+	}
+
+	return 0;
+}
+
 static const struct hbg_dbg_info hbg_dbg_infos[] = {
 	{ "dev_spec", hbg_dbg_dev_spec },
 	{ "tx_ring", hbg_dbg_tx_ring },
 	{ "rx_ring", hbg_dbg_rx_ring },
 	{ "irq_info", hbg_dbg_irq_info },
 	{ "nic_state", hbg_dbg_nic_state },
+	{ "mac_talbe", hbg_dbg_mac_table },
 };
 
 static void hbg_debugfs_uninit(void *data)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 05295c2ad439..29d66a0ea0a6 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -67,6 +67,8 @@ static int hbg_hw_dev_specs_init(struct hbg_priv *priv)
 	specs->vlan_layers = hbg_reg_read(priv, HBG_REG_VLAN_LAYERS_ADDR);
 	specs->rx_fifo_num = hbg_reg_read(priv, HBG_REG_RX_FIFO_NUM_ADDR);
 	specs->tx_fifo_num = hbg_reg_read(priv, HBG_REG_TX_FIFO_NUM_ADDR);
+	specs->uc_mac_num = hbg_reg_read(priv, HBG_REG_UC_MAC_NUM_ADDR);
+
 	mac_addr = hbg_reg_read64(priv, HBG_REG_MAC_ADDR_ADDR);
 	u64_to_ether_addr(mac_addr, (u8 *)specs->mac_addr.sa_data);
 
@@ -135,9 +137,13 @@ void hbg_hw_irq_enable(struct hbg_priv *priv, u32 mask, bool enable)
 	hbg_reg_write(priv, HBG_REG_CF_INTRPT_MSK_ADDR, value);
 }
 
-void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr)
+void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr, u32 index)
 {
-	hbg_reg_write64(priv, HBG_REG_STATION_ADDR_LOW_2_ADDR, mac_addr);
+	u32 addr;
+
+	/* mac addr is u64, so the addr offset is 0x8 */
+	addr = HBG_REG_STATION_ADDR_LOW_2_ADDR + (index * 0x8);
+	hbg_reg_write64(priv, addr, mac_addr);
 }
 
 static void hbg_hw_set_pcu_max_frame_len(struct hbg_priv *priv,
@@ -207,6 +213,13 @@ void hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
 			    HBG_REG_DUPLEX_B, duplex);
 }
 
+/* only support uc filter */
+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);
+}
+
 static void hbg_hw_init_transmit_ctrl(struct hbg_priv *priv)
 {
 	u32 ctrl = 0;
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 14fb39241c93..6eb4b7d2cba8 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -51,9 +51,10 @@ bool hbg_hw_irq_is_enabled(struct hbg_priv *priv, u32 mask);
 void hbg_hw_irq_enable(struct hbg_priv *priv, u32 mask, bool enable);
 void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu);
 void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable);
-void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr);
+void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr, u32 index);
 u32 hbg_hw_get_fifo_used_num(struct hbg_priv *priv, enum hbg_dir dir);
 void hbg_hw_set_tx_desc(struct hbg_priv *priv, struct hbg_tx_desc *tx_desc);
 void hbg_hw_fill_buffer(struct hbg_priv *priv, u32 buffer_dma_addr);
+void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable);
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 30576483a938..0b7cfbd166ec 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -57,7 +57,7 @@ static int hbg_hw_txrx_clear(struct hbg_priv *priv)
 
 	/* After reset, regs need to be reconfigured */
 	hbg_hw_init(priv);
-	hbg_hw_set_uc_addr(priv, ether_addr_to_u64(priv->netdev->dev_addr));
+	hbg_hw_set_uc_addr(priv, ether_addr_to_u64(priv->netdev->dev_addr), 0);
 	hbg_change_mtu(priv, priv->netdev->mtu);
 
 	return 0;
@@ -75,19 +75,128 @@ static int hbg_net_stop(struct net_device *netdev)
 	return hbg_hw_txrx_clear(priv);
 }
 
+static void hbg_update_promisc_mode(struct net_device *netdev)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	priv->filter.enabled = !(priv->filter.table_overflow ||
+				 (netdev->flags & IFF_PROMISC));
+	hbg_hw_set_mac_filter_enable(priv, priv->filter.enabled);
+}
+
+static void hbg_set_mac_to_mac_table(struct hbg_priv *priv,
+				     u32 index, const u8 *addr)
+{
+	if (addr) {
+		ether_addr_copy(priv->filter.mac_table[index].addr, addr);
+		hbg_hw_set_uc_addr(priv, ether_addr_to_u64(addr), index);
+	} else {
+		eth_zero_addr(priv->filter.mac_table[index].addr);
+		hbg_hw_set_uc_addr(priv, 0, index);
+	}
+}
+
+static int hbg_get_index_from_mac_table(struct hbg_priv *priv,
+					const u8 *addr, u32 *index)
+{
+	u32 i;
+
+	for (i = 0; i < priv->filter.table_max_len; i++)
+		if (ether_addr_equal(priv->filter.mac_table[i].addr, addr)) {
+			*index = i;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static int hbg_add_mac_to_filter(struct hbg_priv *priv, const u8 *addr)
+{
+	u32 index;
+
+	/* already exists */
+	if (!hbg_get_index_from_mac_table(priv, addr, &index))
+		return 0;
+
+	for (index = 0; index < priv->filter.table_max_len; index++)
+		if (is_zero_ether_addr(priv->filter.mac_table[index].addr)) {
+			hbg_set_mac_to_mac_table(priv, index, addr);
+			return 0;
+		}
+
+	if (!priv->filter.table_overflow) {
+		priv->filter.table_overflow = true;
+		hbg_update_promisc_mode(priv->netdev);
+		dev_info(&priv->pdev->dev, "mac table is overflow\n");
+	}
+
+	return -ENOSPC;
+}
+
+static void hbg_del_mac_from_filter(struct hbg_priv *priv, const u8 *addr)
+{
+	u32 index;
+
+	/* not exists */
+	if (hbg_get_index_from_mac_table(priv, addr, &index))
+		return;
+
+	hbg_set_mac_to_mac_table(priv, index, NULL);
+
+	if (priv->filter.table_overflow) {
+		priv->filter.table_overflow = false;
+		hbg_update_promisc_mode(priv->netdev);
+		dev_info(&priv->pdev->dev, "mac table is not full\n");
+	}
+}
+
+static int hbg_uc_sync(struct net_device *netdev, const unsigned char *addr)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	return hbg_add_mac_to_filter(priv, addr);
+}
+
+static int hbg_uc_unsync(struct net_device *netdev, const unsigned char *addr)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	if (ether_addr_equal(netdev->dev_addr, (u8 *)addr))
+		return 0;
+
+	hbg_del_mac_from_filter(priv, addr);
+	return 0;
+}
+
+static void hbg_net_set_rx_mode(struct net_device *netdev)
+{
+	hbg_update_promisc_mode(netdev);
+	__dev_uc_sync(netdev, hbg_uc_sync, hbg_uc_unsync);
+}
+
 static int hbg_net_set_mac_address(struct net_device *netdev, void *addr)
 {
 	struct hbg_priv *priv = netdev_priv(netdev);
 	u8 *mac_addr;
+	bool is_exists;
+	u32 index;
 
 	mac_addr = ((struct sockaddr *)addr)->sa_data;
 
 	if (!is_valid_ether_addr(mac_addr))
 		return -EADDRNOTAVAIL;
 
-	hbg_hw_set_uc_addr(priv, ether_addr_to_u64(mac_addr));
-	dev_addr_set(netdev, mac_addr);
+	/* The index of host mac is always 0.
+	 * If new mac address already exists,
+	 * delete the existing mac address and
+	 * add it to the position with index 0.
+	 */
+	is_exists = !hbg_get_index_from_mac_table(priv, mac_addr, &index);
+	hbg_set_mac_to_mac_table(priv, 0, mac_addr);
+	if (is_exists)
+		hbg_set_mac_to_mac_table(priv, index, NULL);
 
+	dev_addr_set(netdev, mac_addr);
 	return 0;
 }
 
@@ -162,6 +271,7 @@ static const struct net_device_ops hbg_netdev_ops = {
 	.ndo_change_mtu		= hbg_net_change_mtu,
 	.ndo_tx_timeout		= hbg_net_tx_timeout,
 	.ndo_get_stats64	= hbg_net_get_stats,
+	.ndo_set_rx_mode	= hbg_net_set_rx_mode,
 };
 
 static void hbg_service_task(struct work_struct *work)
@@ -190,6 +300,25 @@ static void hbg_delaywork_uninit(void *data)
 	cancel_delayed_work_sync(data);
 }
 
+static int hbg_mac_filter_init(struct hbg_priv *priv)
+{
+	struct hbg_dev_specs *dev_specs = &priv->dev_specs;
+	struct hbg_mac_filter *filter = &priv->filter;
+	struct hbg_mac_table_entry *tmp_table;
+
+	tmp_table = devm_kcalloc(&priv->pdev->dev, dev_specs->uc_mac_num,
+				 sizeof(*tmp_table), GFP_KERNEL);
+	if (!tmp_table)
+		return -ENOMEM;
+
+	filter->mac_table = tmp_table;
+	filter->table_max_len = dev_specs->uc_mac_num;
+	filter->enabled = true;
+
+	hbg_hw_set_mac_filter_enable(priv, filter->enabled);
+	return 0;
+}
+
 static int hbg_init(struct hbg_priv *priv)
 {
 	int ret;
@@ -214,6 +343,10 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
+	ret = hbg_mac_filter_init(priv);
+	if (ret)
+		return ret;
+
 	hbg_delaywork_init(priv);
 	return devm_add_action_or_reset(&priv->pdev->dev, hbg_delaywork_uninit,
 					&priv->service_task);
@@ -272,6 +405,8 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		return ret;
 
+	netdev->priv_flags |= IFF_UNICAST_FLT;
+
 	netdev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
 	netdev->max_mtu = priv->dev_specs.max_mtu;
 	netdev->min_mtu = priv->dev_specs.min_mtu;
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index b1dbaa98c6b3..59bda7a8ce5f 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -10,6 +10,7 @@
 #define HBG_REG_MAC_ID_ADDR			0x0008
 #define HBG_REG_PHY_ID_ADDR			0x000C
 #define HBG_REG_MAC_ADDR_ADDR			0x0010
+#define HBG_REG_UC_MAC_NUM_ADDR			0x0018
 #define HBG_REG_MDIO_FREQ_ADDR			0x0024
 #define HBG_REG_MAX_MTU_ADDR			0x0028
 #define HBG_REG_MIN_MTU_ADDR			0x002C
@@ -47,6 +48,8 @@
 #define HBG_REG_TRANSMIT_CTRL_PAD_EN_B		BIT(7)
 #define HBG_REG_TRANSMIT_CTRL_CRC_ADD_B		BIT(6)
 #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_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-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

[Andrew Lunn]

> +static int hbg_add_mac_to_filter(struct hbg_priv *priv, const u8 *addr)
> +{
> +	u32 index;
> +
> +	/* already exists */
> +	if (!hbg_get_index_from_mac_table(priv, addr, &index))
> +		return 0;
> +
> +	for (index = 0; index < priv->filter.table_max_len; index++)
> +		if (is_zero_ether_addr(priv->filter.mac_table[index].addr)) {
> +			hbg_set_mac_to_mac_table(priv, index, addr);
> +			return 0;
> +		}
> +
> +	if (!priv->filter.table_overflow) {
> +		priv->filter.table_overflow = true;
> +		hbg_update_promisc_mode(priv->netdev);
> +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
> +	}
> +
> +	return -ENOSPC;

I _think_ this is wrong. If you run out of hardware resources, you
should change the interface to promiscuous mode and let the stack do
the filtering. Offloading it to hardware is just an acceleration,
nothing more.

	Andrew

Re: [PATCH net-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

[Jijie Shao]

on 2024/10/23 22:05, Andrew Lunn wrote:
>> +static int hbg_add_mac_to_filter(struct hbg_priv *priv, const u8 *addr)
>> +{
>> +	u32 index;
>> +
>> +	/* already exists */
>> +	if (!hbg_get_index_from_mac_table(priv, addr, &index))
>> +		return 0;
>> +
>> +	for (index = 0; index < priv->filter.table_max_len; index++)
>> +		if (is_zero_ether_addr(priv->filter.mac_table[index].addr)) {
>> +			hbg_set_mac_to_mac_table(priv, index, addr);
>> +			return 0;
>> +		}
>> +
>> +	if (!priv->filter.table_overflow) {
>> +		priv->filter.table_overflow = true;
>> +		hbg_update_promisc_mode(priv->netdev);
>> +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
>> +	}
>> +
>> +	return -ENOSPC;
> I _think_ this is wrong. If you run out of hardware resources, you
> should change the interface to promiscuous mode and let the stack do
> the filtering. Offloading it to hardware is just an acceleration,
> nothing more.
>
> 	Andrew

In hbg_update_promisc_mode():
priv->filter.enabled = !(priv->filter.table_overflow || (netdev->flags & IFF_PROMISC));
hbg_hw_set_mac_filter_enable(priv, priv->filter.enabled);

if table_overflow， and netdev->flags not set IFF_PROMISC，
the priv->filter.enabled will set to false， Then， The MAC filter will be closed.
I think it's probably the same thing you said

In this:
+	if (!priv->filter.table_overflow) {
+		priv->filter.table_overflow = true;
+		hbg_update_promisc_mode(priv->netdev);
+		dev_info(&priv->pdev->dev, "mac table is overflow\n");
+	}
+
+	return -ENOSPC;

When the first overflow occurs, a log is printed, the MAC filter will be disabled, and -ENOSPC is returned.
If continue to add MAC addresses, -ENOSPC is returned only.

Thanks，
Jijie Shao

Re: [PATCH net-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

[Andrew Lunn]

On Thu, Oct 24, 2024 at 11:09:22AM +0800, Jijie Shao wrote:
> 
> on 2024/10/23 22:05, Andrew Lunn wrote:
> > > +static int hbg_add_mac_to_filter(struct hbg_priv *priv, const u8 *addr)
> > > +{
> > > +	u32 index;
> > > +
> > > +	/* already exists */
> > > +	if (!hbg_get_index_from_mac_table(priv, addr, &index))
> > > +		return 0;
> > > +
> > > +	for (index = 0; index < priv->filter.table_max_len; index++)
> > > +		if (is_zero_ether_addr(priv->filter.mac_table[index].addr)) {
> > > +			hbg_set_mac_to_mac_table(priv, index, addr);
> > > +			return 0;
> > > +		}
> > > +
> > > +	if (!priv->filter.table_overflow) {
> > > +		priv->filter.table_overflow = true;
> > > +		hbg_update_promisc_mode(priv->netdev);
> > > +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
> > > +	}
> > > +
> > > +	return -ENOSPC;
> > I _think_ this is wrong. If you run out of hardware resources, you
> > should change the interface to promiscuous mode and let the stack do
> > the filtering. Offloading it to hardware is just an acceleration,
> > nothing more.
> > 
> > 	Andrew
> 
> In hbg_update_promisc_mode():
> priv->filter.enabled = !(priv->filter.table_overflow || (netdev->flags & IFF_PROMISC));
> hbg_hw_set_mac_filter_enable(priv, priv->filter.enabled);
> 
> if table_overflow， and netdev->flags not set IFF_PROMISC，
> the priv->filter.enabled will set to false， Then， The MAC filter will be closed.
> I think it's probably the same thing you said
> 
> In this:
> +	if (!priv->filter.table_overflow) {
> +		priv->filter.table_overflow = true;
> +		hbg_update_promisc_mode(priv->netdev);
> +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
> +	}
> +
> +	return -ENOSPC;
> 
> When the first overflow occurs, a log is printed, the MAC filter will be disabled, and -ENOSPC is returned.
> If continue to add MAC addresses, -ENOSPC is returned only.

This is not obvious from a quick look at the code. Maybe a comment
would be good.

	Andrew

Re: [PATCH net-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

[Jijie Shao]

on 2024/10/24 20:07, Andrew Lunn wrote:
> On Thu, Oct 24, 2024 at 11:09:22AM +0800, Jijie Shao wrote:
>> on 2024/10/23 22:05, Andrew Lunn wrote:
>>>> +static int hbg_add_mac_to_filter(struct hbg_priv *priv, const u8 *addr)
>>>> +{
>>>> +	u32 index;
>>>> +
>>>> +	/* already exists */
>>>> +	if (!hbg_get_index_from_mac_table(priv, addr, &index))
>>>> +		return 0;
>>>> +
>>>> +	for (index = 0; index < priv->filter.table_max_len; index++)
>>>> +		if (is_zero_ether_addr(priv->filter.mac_table[index].addr)) {
>>>> +			hbg_set_mac_to_mac_table(priv, index, addr);
>>>> +			return 0;
>>>> +		}
>>>> +
>>>> +	if (!priv->filter.table_overflow) {
>>>> +		priv->filter.table_overflow = true;
>>>> +		hbg_update_promisc_mode(priv->netdev);
>>>> +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
>>>> +	}
>>>> +
>>>> +	return -ENOSPC;
>>> I _think_ this is wrong. If you run out of hardware resources, you
>>> should change the interface to promiscuous mode and let the stack do
>>> the filtering. Offloading it to hardware is just an acceleration,
>>> nothing more.
>>>
>>> 	Andrew
>> In hbg_update_promisc_mode():
>> priv->filter.enabled = !(priv->filter.table_overflow || (netdev->flags & IFF_PROMISC));
>> hbg_hw_set_mac_filter_enable(priv, priv->filter.enabled);
>>
>> if table_overflow， and netdev->flags not set IFF_PROMISC，
>> the priv->filter.enabled will set to false， Then， The MAC filter will be closed.
>> I think it's probably the same thing you said
>>
>> In this:
>> +	if (!priv->filter.table_overflow) {
>> +		priv->filter.table_overflow = true;
>> +		hbg_update_promisc_mode(priv->netdev);
>> +		dev_info(&priv->pdev->dev, "mac table is overflow\n");
>> +	}
>> +
>> +	return -ENOSPC;
>>
>> When the first overflow occurs, a log is printed, the MAC filter will be disabled, and -ENOSPC is returned.
>> If continue to add MAC addresses, -ENOSPC is returned only.
> This is not obvious from a quick look at the code. Maybe a comment
> would be good.
>
> 	Andrew

ok, Thanks!

[PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Jijie Shao]

With the ethtool of a specific version,
the following effects are achieved:

[root@localhost sjj]# ./ethtool -d enp131s0f1
[SPEC] VALID                    [0x0000]: 0x00000001
[SPEC] EVENT_REQ                [0x0004]: 0x00000000
[SPEC] MAC_ID                   [0x0008]: 0x00000002
[SPEC] PHY_ADDR                 [0x000c]: 0x00000002
[SPEC] MAC_ADDR_L               [0x0010]: 0x00000808
[SPEC] MAC_ADDR_H               [0x0014]: 0x08080802
[SPEC] UC_MAX_NUM               [0x0018]: 0x00000004
[SPEC] MAX_MTU                  [0x0028]: 0x00000fc2
[SPEC] MIN_MTU                  [0x002c]: 0x00000100
[SPEC] TX_FIFO_NUM              [0x0030]: 0x00000040
[SPEC] RX_FIFO_NUM              [0x0034]: 0x0000007f
[SPEC] VLAN_LAYERS              [0x0038]: 0x00000002
[MDIO] COMMAND_REG              [0x0000]: 0x0000185f
[MDIO] ADDR_REG                 [0x0004]: 0x00000000
[MDIO] WDATA_REG                [0x0008]: 0x0000a000
[MDIO] RDATA_REG                [0x000c]: 0x00000000
[MDIO] STA_REG                  [0x0010]: 0x00000000
[GMAC] DUPLEX_TYPE              [0x0008]: 0x00000001
[GMAC] FD_FC_TYPE               [0x000c]: 0x00008808
[GMAC] FC_TX_TIMER              [0x001c]: 0x000000ff
[GMAC] FD_FC_ADDR_LOW           [0x0020]: 0xc2000001
[GMAC] FD_FC_ADDR_HIGH          [0x0024]: 0x00000180
[GMAC] MAX_FRM_SIZE             [0x003c]: 0x000005f6
[GMAC] PORT_MODE                [0x0040]: 0x00000002
[GMAC] PORT_EN                  [0x0044]: 0x00000006
...

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.c  | 163 ++++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  34 ++++
 2 files changed, 197 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
index 59f8c84d43fa..a630c7d8ef5c 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -133,6 +133,126 @@ static const struct hbg_ethtool_stats hbg_ethtool_stats_map[] = {
 	HBG_STATS_I(tx_dma_err_cnt),
 };
 
+#define HBG_REG_NAEM_MAX_LEN	24
+#define HBG_REG_TYPE_MAX_LEN	8
+
+struct hbg_reg_offset_name_map {
+	u32 reg_offset;
+	char name[HBG_REG_NAEM_MAX_LEN];
+};
+
+struct hbg_reg_type_info {
+	char name[HBG_REG_TYPE_MAX_LEN];
+	u32 offset_base;
+	const struct hbg_reg_offset_name_map *reg_maps;
+	u32 reg_num;
+};
+
+struct hbg_reg_info {
+	char name[HBG_REG_NAEM_MAX_LEN + HBG_REG_TYPE_MAX_LEN];
+	u32 offset;
+	u32 val;
+};
+
+const struct hbg_reg_offset_name_map hbg_dev_spec_reg_map[] = {
+	{HBG_REG_SPEC_VALID_ADDR, "VALID"},
+	{HBG_REG_EVENT_REQ_ADDR, "EVENT_REQ"},
+	{HBG_REG_MAC_ID_ADDR, "MAC_ID"},
+	{HBG_REG_PHY_ID_ADDR, "PHY_ADDR"},
+	{HBG_REG_MAC_ADDR_ADDR, "MAC_ADDR_L"},
+	{HBG_REG_MAC_ADDR_HIGH_ADDR, "MAC_ADDR_H"},
+	{HBG_REG_UC_MAC_NUM_ADDR, "UC_MAX_NUM"},
+	{HBG_REG_MAX_MTU_ADDR, "MAX_MTU"},
+	{HBG_REG_MIN_MTU_ADDR, "MIN_MTU"},
+	{HBG_REG_TX_FIFO_NUM_ADDR, "TX_FIFO_NUM"},
+	{HBG_REG_RX_FIFO_NUM_ADDR, "RX_FIFO_NUM"},
+	{HBG_REG_VLAN_LAYERS_ADDR, "VLAN_LAYERS"},
+};
+
+const struct hbg_reg_offset_name_map hbg_mdio_reg_map[] = {
+	{HBG_REG_MDIO_COMMAND_ADDR, "COMMAND_REG"},
+	{HBG_REG_MDIO_ADDR_ADDR, "ADDR_REG"},
+	{HBG_REG_MDIO_WDATA_ADDR, "WDATA_REG"},
+	{HBG_REG_MDIO_RDATA_ADDR, "RDATA_REG"},
+	{HBG_REG_MDIO_STA_ADDR, "STA_REG"},
+};
+
+const struct hbg_reg_offset_name_map hbg_gmac_reg_map[] = {
+	{HBG_REG_DUPLEX_TYPE_ADDR, "DUPLEX_TYPE"},
+	{HBG_REG_FD_FC_TYPE_ADDR, "FD_FC_TYPE"},
+	{HBG_REG_FC_TX_TIMER_ADDR, "FC_TX_TIMER"},
+	{HBG_REG_FD_FC_ADDR_LOW_ADDR, "FD_FC_ADDR_LOW"},
+	{HBG_REG_FD_FC_ADDR_HIGH_ADDR, "FD_FC_ADDR_HIGH"},
+	{HBG_REG_MAX_FRAME_SIZE_ADDR, "MAX_FRM_SIZE"},
+	{HBG_REG_PORT_MODE_ADDR, "PORT_MODE"},
+	{HBG_REG_PORT_ENABLE_ADDR, "PORT_EN"},
+	{HBG_REG_PAUSE_ENABLE_ADDR, "PAUSE_EN"},
+	{HBG_REG_AN_NEG_STATE_ADDR, "AN_NEG_STATE"},
+	{HBG_REG_LINE_LOOP_BACK_ADDR, "LINE_LOOP_BACK"},
+	{HBG_REG_CF_CRC_STRIP_ADDR, "CF_CRC_STRIP"},
+	{HBG_REG_MODE_CHANGE_EN_ADDR, "MODE_CHANGE_EN"},
+	{HBG_REG_LOOP_REG_ADDR, "LOOP_REG"},
+	{HBG_REG_RECV_CTRL_ADDR, "RECV_CONTROL"},
+	{HBG_REG_VLAN_CODE_ADDR, "VLAN_CODE"},
+	{HBG_REG_STATION_ADDR_LOW_0_ADDR, "STATION_ADDR_LOW_0"},
+	{HBG_REG_STATION_ADDR_HIGH_0_ADDR, "STATION_ADDR_HIGH_0"},
+	{HBG_REG_STATION_ADDR_LOW_1_ADDR, "STATION_ADDR_LOW_1"},
+	{HBG_REG_STATION_ADDR_HIGH_1_ADDR, "STATION_ADDR_HIGH_1"},
+	{HBG_REG_STATION_ADDR_LOW_2_ADDR, "STATION_ADDR_LOW_2"},
+	{HBG_REG_STATION_ADDR_HIGH_2_ADDR, "STATION_ADDR_HIGH_2"},
+	{HBG_REG_STATION_ADDR_LOW_3_ADDR, "STATION_ADDR_LOW_3"},
+	{HBG_REG_STATION_ADDR_HIGH_3_ADDR, "STATION_ADDR_HIGH_3"},
+	{HBG_REG_STATION_ADDR_LOW_4_ADDR, "STATION_ADDR_LOW_4"},
+	{HBG_REG_STATION_ADDR_HIGH_4_ADDR, "STATION_ADDR_HIGH_4"},
+	{HBG_REG_STATION_ADDR_LOW_5_ADDR, "STATION_ADDR_LOW_5"},
+	{HBG_REG_STATION_ADDR_HIGH_5_ADDR, "STATION_ADDR_HIGH_5"},
+};
+
+const struct hbg_reg_offset_name_map hbg_pcu_reg_map[] = {
+	{HBG_REG_TX_FIFO_THRSLD_ADDR, "CF_TX_FIFO_THRSLD"},
+	{HBG_REG_RX_FIFO_THRSLD_ADDR, "CF_RX_FIFO_THRSLD"},
+	{HBG_REG_CFG_FIFO_THRSLD_ADDR, "CF_CFG_FIFO_THRSLD"},
+	{HBG_REG_CF_INTRPT_MSK_ADDR, "CF_INTRPT_MSK"},
+	{HBG_REG_CF_INTRPT_STAT_ADDR, "CF_INTRPT_STAT"},
+	{HBG_REG_CF_INTRPT_CLR_ADDR, "CF_INTRPT_CLR"},
+	{HBG_REG_TX_BUS_ERR_ADDR_ADDR, "TX_BUS_ERR_ADDR"},
+	{HBG_REG_RX_BUS_ERR_ADDR_ADDR, "RX_BUS_ERR_ADDR"},
+	{HBG_REG_MAX_FRAME_LEN_ADDR, "MAX_FRAME_LEN"},
+	{HBG_REG_DEBUG_ST_MCH_ADDR, "DEBUG_ST_MCH"},
+	{HBG_REG_FIFO_CURR_STATUS_ADDR, "FIFO_CURR_STATUS"},
+	{HBG_REG_FIFO_HIST_STATUS_ADDR, "FIFO_HIS_STATUS"},
+	{HBG_REG_CF_CFF_DATA_NUM_ADDR, "CF_CFF_DATA_NUM"},
+	{HBG_REG_CF_TX_PAUSE_ADDR, "CF_TX_PAUSE"},
+	{HBG_REG_TX_CFF_ADDR_0_ADDR, "TX_CFF_ADDR_0"},
+	{HBG_REG_TX_CFF_ADDR_1_ADDR, "TX_CFF_ADDR_1"},
+	{HBG_REG_TX_CFF_ADDR_2_ADDR, "TX_CFF_ADDR_2"},
+	{HBG_REG_TX_CFF_ADDR_3_ADDR, "TX_CFF_ADDR_3"},
+	{HBG_REG_RX_CFF_ADDR_ADDR, "RX_CFF_ADDR"},
+	{HBG_REG_RX_BUF_SIZE_ADDR, "RX_BUF_SIZE"},
+	{HBG_REG_BUS_CTRL_ADDR, "BUS_CTRL"},
+	{HBG_REG_RX_CTRL_ADDR, "RX_CTRL"},
+	{HBG_REG_RX_PKT_MODE_ADDR, "RX_PKT_MODE"},
+	{HBG_REG_DBG_ST0_ADDR, "DBG_ST0"},
+	{HBG_REG_DBG_ST1_ADDR, "DBG_ST1"},
+	{HBG_REG_DBG_ST2_ADDR, "DBG_ST2"},
+	{HBG_REG_BUS_RST_EN_ADDR, "BUS_RST_EN"},
+	{HBG_REG_CF_IND_TXINT_MSK_ADDR, "CF_IND_TXINT_MSK"},
+	{HBG_REG_CF_IND_TXINT_STAT_ADDR, "CF_IND_TXINT_STAT"},
+	{HBG_REG_CF_IND_TXINT_CLR_ADDR, "CF_IND_TXINT_CLR"},
+	{HBG_REG_CF_IND_RXINT_MSK_ADDR, "CF_IND_RXINT_MSK"},
+	{HBG_REG_CF_IND_RXINT_STAT_ADDR, "CF_IND_RXINT_STAT"},
+	{HBG_REG_CF_IND_RXINT_CLR_ADDR, "CF_IND_RXINT_CLR"},
+};
+
+#define HBG_REG_TYPE_INFO_I(name, base, map) {name, base, map, ARRAY_SIZE(map)}
+
+const struct hbg_reg_type_info hbg_type_infos[] = {
+	HBG_REG_TYPE_INFO_I("SPEC", 0, hbg_dev_spec_reg_map),
+	HBG_REG_TYPE_INFO_I("MDIO", HBG_REG_MDIO_BASE, hbg_mdio_reg_map),
+	HBG_REG_TYPE_INFO_I("GMAC", HBG_REG_SGMII_BASE, hbg_gmac_reg_map),
+	HBG_REG_TYPE_INFO_I("PCU", HBG_REG_SGMII_BASE, hbg_pcu_reg_map),
+};
+
 static int hbg_ethtool_get_sset_count(struct net_device *netdev, int stringset)
 {
 	if (stringset != ETH_SS_STATS)
@@ -180,6 +300,47 @@ static void hbg_ethtool_get_stats(struct net_device *netdev,
 					 hbg_ethtool_stats_map[i].offset);
 }
 
+static int hbg_ethtool_get_regs_len(struct net_device *netdev)
+{
+	u32 len = 0;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(hbg_type_infos); i++)
+		len += hbg_type_infos[i].reg_num * sizeof(struct hbg_reg_info);
+
+	return len;
+}
+
+static u32 hbg_get_reg_info(struct hbg_priv *priv,
+			    const struct hbg_reg_type_info *type_info,
+			    const struct hbg_reg_offset_name_map *reg_map,
+			    struct hbg_reg_info *info)
+{
+	info->val = hbg_reg_read(priv, reg_map->reg_offset);
+	info->offset = reg_map->reg_offset - type_info->offset_base;
+	snprintf(info->name, sizeof(info->name),
+		 "[%s] %s", type_info->name, reg_map->name);
+
+	return sizeof(*info);
+}
+
+static void hbg_ethtool_get_regs(struct net_device *netdev,
+				 struct ethtool_regs *regs, void *data)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+	const struct hbg_reg_type_info *info;
+	u32 i, j, offset = 0;
+
+	regs->version = 0;
+	for (i = 0; i < ARRAY_SIZE(hbg_type_infos); i++) {
+		info = &hbg_type_infos[i];
+		for (j = 0; j < info->reg_num; j++)
+			offset += hbg_get_reg_info(priv, info,
+						   &info->reg_maps[j],
+						   data + offset);
+	}
+}
+
 static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
@@ -187,6 +348,8 @@ static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_sset_count		= hbg_ethtool_get_sset_count,
 	.get_strings		= hbg_ethtool_get_strings,
 	.get_ethtool_stats	= hbg_ethtool_get_stats,
+	.get_regs_len		= hbg_ethtool_get_regs_len,
+	.get_regs		= hbg_ethtool_get_regs,
 };
 
 void hbg_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index 59bda7a8ce5f..bbfefe9c1e61 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -10,6 +10,7 @@
 #define HBG_REG_MAC_ID_ADDR			0x0008
 #define HBG_REG_PHY_ID_ADDR			0x000C
 #define HBG_REG_MAC_ADDR_ADDR			0x0010
+#define HBG_REG_MAC_ADDR_HIGH_ADDR		0x0014
 #define HBG_REG_UC_MAC_NUM_ADDR			0x0018
 #define HBG_REG_MDIO_FREQ_ADDR			0x0024
 #define HBG_REG_MAX_MTU_ADDR			0x0028
@@ -29,6 +30,7 @@
 #define HBG_REG_MDIO_COMMAND_OP_M		GENMASK(11, 10)
 #define HBG_REG_MDIO_COMMAND_PRTAD_M		GENMASK(9, 5)
 #define HBG_REG_MDIO_COMMAND_DEVAD_M		GENMASK(4, 0)
+#define HBG_REG_MDIO_ADDR_ADDR			(HBG_REG_MDIO_BASE + 0x0004)
 #define HBG_REG_MDIO_WDATA_ADDR			(HBG_REG_MDIO_BASE + 0x0008)
 #define HBG_REG_MDIO_WDATA_M			GENMASK(15, 0)
 #define HBG_REG_MDIO_RDATA_ADDR			(HBG_REG_MDIO_BASE + 0x000C)
@@ -37,6 +39,10 @@
 /* GMAC */
 #define HBG_REG_SGMII_BASE			0x10000
 #define HBG_REG_DUPLEX_TYPE_ADDR		(HBG_REG_SGMII_BASE + 0x0008)
+#define HBG_REG_FD_FC_TYPE_ADDR			(HBG_REG_SGMII_BASE + 0x000C)
+#define HBG_REG_FC_TX_TIMER_ADDR		(HBG_REG_SGMII_BASE + 0x001C)
+#define HBG_REG_FD_FC_ADDR_LOW_ADDR		(HBG_REG_SGMII_BASE + 0x0020)
+#define HBG_REG_FD_FC_ADDR_HIGH_ADDR		(HBG_REG_SGMII_BASE + 0x0024)
 #define HBG_REG_DUPLEX_B			BIT(0)
 #define HBG_REG_MAX_FRAME_SIZE_ADDR		(HBG_REG_SGMII_BASE + 0x003C)
 #define HBG_REG_PORT_MODE_ADDR			(HBG_REG_SGMII_BASE + 0x0040)
@@ -44,6 +50,8 @@
 #define HBG_REG_PORT_ENABLE_ADDR		(HBG_REG_SGMII_BASE + 0x0044)
 #define HBG_REG_PORT_ENABLE_RX_B		BIT(1)
 #define HBG_REG_PORT_ENABLE_TX_B		BIT(2)
+#define HBG_REG_PAUSE_ENABLE_ADDR		(HBG_REG_SGMII_BASE + 0x0048)
+#define HBG_REG_AN_NEG_STATE_ADDR		(HBG_REG_SGMII_BASE + 0x0058)
 #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)
@@ -92,19 +100,35 @@
 #define HBG_REG_TX_TAGGED_ADDR			(HBG_REG_SGMII_BASE + 0x0154)
 #define HBG_REG_TX_CRC_ERROR_ADDR		(HBG_REG_SGMII_BASE + 0x0158)
 #define HBG_REG_TX_PAUSE_FRAMES_ADDR		(HBG_REG_SGMII_BASE + 0x015C)
+#define HBG_REG_LINE_LOOP_BACK_ADDR		(HBG_REG_SGMII_BASE + 0x01A8)
 #define HBG_REG_CF_CRC_STRIP_ADDR		(HBG_REG_SGMII_BASE + 0x01B0)
 #define HBG_REG_CF_CRC_STRIP_B			BIT(0)
 #define HBG_REG_MODE_CHANGE_EN_ADDR		(HBG_REG_SGMII_BASE + 0x01B4)
 #define HBG_REG_MODE_CHANGE_EN_B		BIT(0)
+#define HBG_REG_LOOP_REG_ADDR			(HBG_REG_SGMII_BASE + 0x01DC)
 #define HBG_REG_RECV_CTRL_ADDR			(HBG_REG_SGMII_BASE + 0x01E0)
+#define HBG_REG_VLAN_CODE_ADDR			(HBG_REG_SGMII_BASE + 0x01E8)
 #define HBG_REG_RECV_CTRL_STRIP_PAD_EN_B	BIT(3)
 #define HBG_REG_RX_OVERRUN_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x01EC)
 #define HBG_REG_RX_LENGTHFIELD_ERR_CNT_ADDR	(HBG_REG_SGMII_BASE + 0x01F4)
 #define HBG_REG_RX_FAIL_COMMA_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x01F8)
+#define HBG_REG_STATION_ADDR_LOW_0_ADDR		(HBG_REG_SGMII_BASE + 0x0200)
+#define HBG_REG_STATION_ADDR_HIGH_0_ADDR	(HBG_REG_SGMII_BASE + 0x0204)
+#define HBG_REG_STATION_ADDR_LOW_1_ADDR		(HBG_REG_SGMII_BASE + 0x0208)
+#define HBG_REG_STATION_ADDR_HIGH_1_ADDR	(HBG_REG_SGMII_BASE + 0x020C)
 #define HBG_REG_STATION_ADDR_LOW_2_ADDR		(HBG_REG_SGMII_BASE + 0x0210)
 #define HBG_REG_STATION_ADDR_HIGH_2_ADDR	(HBG_REG_SGMII_BASE + 0x0214)
+#define HBG_REG_STATION_ADDR_LOW_3_ADDR		(HBG_REG_SGMII_BASE + 0x0218)
+#define HBG_REG_STATION_ADDR_HIGH_3_ADDR	(HBG_REG_SGMII_BASE + 0x021C)
+#define HBG_REG_STATION_ADDR_LOW_4_ADDR		(HBG_REG_SGMII_BASE + 0x0220)
+#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)
 
 /* PCU */
+#define HBG_REG_TX_FIFO_THRSLD_ADDR		(HBG_REG_SGMII_BASE + 0x0420)
+#define HBG_REG_RX_FIFO_THRSLD_ADDR		(HBG_REG_SGMII_BASE + 0x0424)
+#define HBG_REG_CFG_FIFO_THRSLD_ADDR		(HBG_REG_SGMII_BASE + 0x0428)
 #define HBG_REG_CF_INTRPT_MSK_ADDR		(HBG_REG_SGMII_BASE + 0x042C)
 #define HBG_INT_MSK_WE_ERR_B			BIT(31)
 #define HBG_INT_MSK_RBREQ_ERR_B			BIT(30)
@@ -126,10 +150,15 @@
 #define HBG_INT_MSK_RX_B			BIT(0) /* just used in driver */
 #define HBG_REG_CF_INTRPT_STAT_ADDR		(HBG_REG_SGMII_BASE + 0x0434)
 #define HBG_REG_CF_INTRPT_CLR_ADDR		(HBG_REG_SGMII_BASE + 0x0438)
+#define HBG_REG_TX_BUS_ERR_ADDR_ADDR		(HBG_REG_SGMII_BASE + 0x043C)
+#define HBG_REG_RX_BUS_ERR_ADDR_ADDR		(HBG_REG_SGMII_BASE + 0x0440)
 #define HBG_REG_MAX_FRAME_LEN_ADDR		(HBG_REG_SGMII_BASE + 0x0444)
 #define HBG_REG_TX_DROP_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0448)
 #define HBG_REG_RX_OVER_FLOW_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x044C)
 #define HBG_REG_MAX_FRAME_LEN_M			GENMASK(15, 0)
+#define HBG_REG_DEBUG_ST_MCH_ADDR		(HBG_REG_SGMII_BASE + 0x0450)
+#define HBG_REG_FIFO_CURR_STATUS_ADDR		(HBG_REG_SGMII_BASE + 0x0454)
+#define HBG_REG_FIFO_HIST_STATUS_ADDR		(HBG_REG_SGMII_BASE + 0x0458)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR		(HBG_REG_SGMII_BASE + 0x045C)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR_TX_M	GENMASK(8, 0)
 #define HBG_REG_CF_CFF_DATA_NUM_ADDR_RX_M	GENMASK(24, 16)
@@ -137,6 +166,7 @@
 #define HBG_REG_RX_TRANS_PKG_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0464)
 #define HBG_REG_TX_TRANS_PKG_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0468)
 #define HBG_REG_RX_ADDR_OVERFLOW_ADDR		(HBG_REG_SGMII_BASE + 0x046C)
+#define HBG_REG_CF_TX_PAUSE_ADDR		(HBG_REG_SGMII_BASE + 0x0470)
 #define HBG_REG_TX_CFF_ADDR_0_ADDR		(HBG_REG_SGMII_BASE + 0x0488)
 #define HBG_REG_TX_CFF_ADDR_1_ADDR		(HBG_REG_SGMII_BASE + 0x048C)
 #define HBG_REG_TX_CFF_ADDR_2_ADDR		(HBG_REG_SGMII_BASE + 0x0490)
@@ -158,6 +188,10 @@
 #define HBG_REG_RX_BUFRQ_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x058C)
 #define HBG_REG_TX_BUFRL_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0590)
 #define HBG_REG_RX_WE_ERR_CNT_ADDR		(HBG_REG_SGMII_BASE + 0x0594)
+#define HBG_REG_DBG_ST0_ADDR			(HBG_REG_SGMII_BASE + 0x05E4)
+#define HBG_REG_DBG_ST1_ADDR			(HBG_REG_SGMII_BASE + 0x05E8)
+#define HBG_REG_DBG_ST2_ADDR			(HBG_REG_SGMII_BASE + 0x05EC)
+#define HBG_REG_BUS_RST_EN_ADDR			(HBG_REG_SGMII_BASE + 0x0688)
 #define HBG_REG_CF_IND_TXINT_MSK_ADDR		(HBG_REG_SGMII_BASE + 0x0694)
 #define HBG_REG_IND_INTR_MASK_B			BIT(0)
 #define HBG_REG_CF_IND_TXINT_STAT_ADDR		(HBG_REG_SGMII_BASE + 0x0698)
-- 
2.33.0

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Andrew Lunn]

On Wed, Oct 23, 2024 at 09:42:10PM +0800, Jijie Shao wrote:
> With the ethtool of a specific version,
> the following effects are achieved:
> 
> [root@localhost sjj]# ./ethtool -d enp131s0f1
> [SPEC] VALID                    [0x0000]: 0x00000001
> [SPEC] EVENT_REQ                [0x0004]: 0x00000000
> [SPEC] MAC_ID                   [0x0008]: 0x00000002
> [SPEC] PHY_ADDR                 [0x000c]: 0x00000002
> [SPEC] MAC_ADDR_L               [0x0010]: 0x00000808
> [SPEC] MAC_ADDR_H               [0x0014]: 0x08080802
> [SPEC] UC_MAX_NUM               [0x0018]: 0x00000004
> [SPEC] MAX_MTU                  [0x0028]: 0x00000fc2
> [SPEC] MIN_MTU                  [0x002c]: 0x00000100

Seems like this makes your debugfs patches redundant?

> +static u32 hbg_get_reg_info(struct hbg_priv *priv,
> +			    const struct hbg_reg_type_info *type_info,
> +			    const struct hbg_reg_offset_name_map *reg_map,
> +			    struct hbg_reg_info *info)
> +{
> +	info->val = hbg_reg_read(priv, reg_map->reg_offset);
> +	info->offset = reg_map->reg_offset - type_info->offset_base;
> +	snprintf(info->name, sizeof(info->name),
> +		 "[%s] %s", type_info->name, reg_map->name);
> +
> +	return sizeof(*info);
> +}
> +
> +static void hbg_ethtool_get_regs(struct net_device *netdev,
> +				 struct ethtool_regs *regs, void *data)
> +{
> +	struct hbg_priv *priv = netdev_priv(netdev);
> +	const struct hbg_reg_type_info *info;
> +	u32 i, j, offset = 0;
> +
> +	regs->version = 0;
> +	for (i = 0; i < ARRAY_SIZE(hbg_type_infos); i++) {
> +		info = &hbg_type_infos[i];
> +		for (j = 0; j < info->reg_num; j++)
> +			offset += hbg_get_reg_info(priv, info,
> +						   &info->reg_maps[j],
> +						   data + offset);
> +	}
> +}

data is supposed to be just raw values, dumped from registers in the
device. You appear to be passing back ASCII text. It is supposed to be
ethtool which does the pretty print, not the kernel driver.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Jijie Shao]

on 2024/10/23 22:13, Andrew Lunn wrote:
> On Wed, Oct 23, 2024 at 09:42:10PM +0800, Jijie Shao wrote:
>> With the ethtool of a specific version,
>> the following effects are achieved:
>>
>> [root@localhost sjj]# ./ethtool -d enp131s0f1
>> [SPEC] VALID                    [0x0000]: 0x00000001
>> [SPEC] EVENT_REQ                [0x0004]: 0x00000000
>> [SPEC] MAC_ID                   [0x0008]: 0x00000002
>> [SPEC] PHY_ADDR                 [0x000c]: 0x00000002
>> [SPEC] MAC_ADDR_L               [0x0010]: 0x00000808
>> [SPEC] MAC_ADDR_H               [0x0014]: 0x08080802
>> [SPEC] UC_MAX_NUM               [0x0018]: 0x00000004
>> [SPEC] MAX_MTU                  [0x0028]: 0x00000fc2
>> [SPEC] MIN_MTU                  [0x002c]: 0x00000100
> Seems like this makes your debugfs patches redundant?

Yes, the debugfs will be removed.

>
>> +static u32 hbg_get_reg_info(struct hbg_priv *priv,
>> +			    const struct hbg_reg_type_info *type_info,
>> +			    const struct hbg_reg_offset_name_map *reg_map,
>> +			    struct hbg_reg_info *info)
>> +{
>> +	info->val = hbg_reg_read(priv, reg_map->reg_offset);
>> +	info->offset = reg_map->reg_offset - type_info->offset_base;
>> +	snprintf(info->name, sizeof(info->name),
>> +		 "[%s] %s", type_info->name, reg_map->name);
>> +
>> +	return sizeof(*info);
>> +}
>> +
>> +static void hbg_ethtool_get_regs(struct net_device *netdev,
>> +				 struct ethtool_regs *regs, void *data)
>> +{
>> +	struct hbg_priv *priv = netdev_priv(netdev);
>> +	const struct hbg_reg_type_info *info;
>> +	u32 i, j, offset = 0;
>> +
>> +	regs->version = 0;
>> +	for (i = 0; i < ARRAY_SIZE(hbg_type_infos); i++) {
>> +		info = &hbg_type_infos[i];
>> +		for (j = 0; j < info->reg_num; j++)
>> +			offset += hbg_get_reg_info(priv, info,
>> +						   &info->reg_maps[j],
>> +						   data + offset);
>> +	}
>> +}
> data is supposed to be just raw values, dumped from registers in the
> device. You appear to be passing back ASCII text. It is supposed to be
> ethtool which does the pretty print, not the kernel driver.
>
>      Andrew

We have other considerations:

If the dump register changes in the future, we hope that
only the kernel needs to be modified, and the ethtool does not need to be modified.
In this case, We do not need to consider the mapping between the ethtool and driver versions.

So in ethtool, we only need to consider basic formatted printing.
like this(not send yet):
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
#define HBG_REG_NAEM_MAX_LEN 32
  
struct hbg_reg_info {
	char name[HBG_REG_NAEM_MAX_LEN];
	u32 offset;
	u32 val;
};
  
static void hibmcge_dump_reg_info(struct hbg_reg_info *info)
{
	fprintf(stdout, "%-*s[0x%04x]: 0x%08x\n",
		HBG_REG_NAEM_MAX_LEN, info->name, info->offset, info->val);
}
  
int hibmcge_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
		      struct ethtool_regs *regs)
{
	struct hbg_reg_info *reg_info;
	u32 name_max_len;
	u32 offset = 0;
  
	if (regs->len % sizeof(*reg_info) != 0)
		return -EINVAL;
  
	while (offset < regs->len) {
		reg_info = (struct hbg_reg_info *)(regs->data + offset);
		hibmcge_dump_reg_info(reg_info);
		offset += sizeof(*reg_info);
	}
  
	return 0;
}
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

So, In this patch, pass back hbg_reg_info(name, offset, value)

Thanks,
Jijie Shao

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Andrew Lunn]

> We have other considerations:
> 
> If the dump register changes in the future, we hope that
> only the kernel needs to be modified, and the ethtool does not need to be modified.
> In this case, We do not need to consider the mapping between the ethtool and driver versions.
> 
> So in ethtool, we only need to consider basic formatted printing.
> like this(not send yet):
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 
> #define HBG_REG_NAEM_MAX_LEN 32
> struct hbg_reg_info {
> 	char name[HBG_REG_NAEM_MAX_LEN];
> 	u32 offset;
> 	u32 val;
> };
> static void hibmcge_dump_reg_info(struct hbg_reg_info *info)
> {
> 	fprintf(stdout, "%-*s[0x%04x]: 0x%08x\n",
> 		HBG_REG_NAEM_MAX_LEN, info->name, info->offset, info->val);
> }
> int hibmcge_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
> 		      struct ethtool_regs *regs)
> {
> 	struct hbg_reg_info *reg_info;
> 	u32 name_max_len;
> 	u32 offset = 0;
> 	if (regs->len % sizeof(*reg_info) != 0)
> 		return -EINVAL;
> 	while (offset < regs->len) {
> 		reg_info = (struct hbg_reg_info *)(regs->data + offset);
> 		hibmcge_dump_reg_info(reg_info);
> 		offset += sizeof(*reg_info);
> 	}
> 	return 0;
> }
> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 
> 
> So, In this patch, pass back hbg_reg_info(name, offset, value)

So this is different to all other drivers doing registers dumps.

1) Please explain this in the commit message, with a justification why
your driver is different.

2) What is actually specific to your driver here? Why not make this
available to all drivers? Maybe check if ethtool_regs.version ==
MAX_U32 is used by any of the other drivers, and if not, make that a
magic value to indicate your special format.

3) Maybe consider that there does not appear to be a netlink version
of this ethtool ioctl. Could this be nicely integrated into a netlink
version, where you have more flexibility with attributes?


	Andrew

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Jijie Shao]

on 2024/10/24 20:22, Andrew Lunn wrote:
>> We have other considerations:
>>
>> If the dump register changes in the future, we hope that
>> only the kernel needs to be modified, and the ethtool does not need to be modified.
>> In this case, We do not need to consider the mapping between the ethtool and driver versions.
>>
>> So in ethtool, we only need to consider basic formatted printing.
>> like this(not send yet):
>>
>> #define HBG_REG_NAEM_MAX_LEN 32
>> struct hbg_reg_info {
>> 	char name[HBG_REG_NAEM_MAX_LEN];
>> 	u32 offset;
>> 	u32 val;
>> };
>> static void hibmcge_dump_reg_info(struct hbg_reg_info *info)
>> {
>> 	fprintf(stdout, "%-*s[0x%04x]: 0x%08x\n",
>> 		HBG_REG_NAEM_MAX_LEN, info->name, info->offset, info->val);
>> }
>> int hibmcge_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
>> 		      struct ethtool_regs *regs)
>> {
>> 	struct hbg_reg_info *reg_info;
>> 	u32 name_max_len;
>> 	u32 offset = 0;
>> 	if (regs->len % sizeof(*reg_info) != 0)
>> 		return -EINVAL;
>> 	while (offset < regs->len) {
>> 		reg_info = (struct hbg_reg_info *)(regs->data + offset);
>> 		hibmcge_dump_reg_info(reg_info);
>> 		offset += sizeof(*reg_info);
>> 	}
>> 	return 0;
>> }
>>
>> So, In this patch, pass back hbg_reg_info(name, offset, value)
> So this is different to all other drivers doing registers dumps.
>
> 1) Please explain this in the commit message, with a justification why
> your driver is different.

In fact, we don't have anything different with other drivers.

In the customer environment, the ethtool version may not be the latest.

If the driver adds a register to the register dump, the register is unknown
when the ethtool is used to query the register.
Therefore, we want to separate the ethtool from the driver.
No matter how the driver is modified, the ethtool can display all information perfectly.

>
> 2) What is actually specific to your driver here? Why not make this
> available to all drivers? Maybe check if ethtool_regs.version ==
> MAX_U32 is used by any of the other drivers, and if not, make that a
> magic value to indicate your special format.

In fact, it would be best if ethtool could provide a unified framework to
elegantly display all register information.
The existing framework prints the information in hexadecimal format,
which is not intuitive enough.

>
> 3) Maybe consider that there does not appear to be a netlink version
> of this ethtool ioctl. Could this be nicely integrated into a netlink
> version, where you have more flexibility with attributes?

Okay, I'll analyze it.

Thanks a lot.
Jijie Shao

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[Jijie Shao]

on 2024/10/23 21:42, Jijie Shao wrote:
>   
> +#define HBG_REG_NAEM_MAX_LEN	24
> +#define HBG_REG_TYPE_MAX_LEN	8

......

> +
> +struct hbg_reg_offset_name_map {
> +	u32 reg_offset;
> +	char name[HBG_REG_NAEM_MAX_LEN];
> +};
> +
> +struct hbg_reg_type_info {
> +	char name[HBG_REG_TYPE_MAX_LEN];
> +	u32 offset_base;
> +	const struct hbg_reg_offset_name_map *reg_maps;
> +	u32 reg_num;
> +};
> +
> +struct hbg_reg_info {
> +	char name[HBG_REG_NAEM_MAX_LEN + HBG_REG_TYPE_MAX_LEN];
> +	u32 offset;
> +	u32 val;

......

> +
> +static u32 hbg_get_reg_info(struct hbg_priv *priv,
> +			    const struct hbg_reg_type_info *type_info,
> +			    const struct hbg_reg_offset_name_map *reg_map,
> +			    struct hbg_reg_info *info)
> +{
> +	info->val = hbg_reg_read(priv, reg_map->reg_offset);
> +	info->offset = reg_map->reg_offset - type_info->offset_base;
> +	snprintf(info->name, sizeof(info->name),
> +		 "[%s] %s", type_info->name, reg_map->name);
> +

In addition, there are compilation warning here:
../drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c: In function ‘hbg_ethtool_get_regs’:
../drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:322:20: warning: ‘%s’ directive output may be truncated writing up to 127 bytes into a region of size 31 [-Wformat-truncation=]
   322 |                  "[%s] %s", type_info->name, reg_map->name);
       |                    ^~
In function ‘hbg_get_reg_info’,
     inlined from ‘hbg_ethtool_get_regs’ at ../drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:338:14:
../drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:321:9: note: ‘snprintf’ output between 4 and 154 bytes into a destination of size 32
   321 |         snprintf(info->name, sizeof(info->name),
       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   322 |                  "[%s] %s", type_info->name, reg_map->name);


But in fact, sizeof(info->name) is (24+8), type_info->name length is 8, and reg_map->name length is 24.
I understand that it should be fine to use here.

Thanks,
Jijie Shao

Re: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module

[kernel test robot]

Hi Jijie,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jijie-Shao/net-hibmcge-Add-dump-statistics-supported-in-this-module/20241023-215222
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241023134213.3359092-5-shaojijie%40huawei.com
patch subject: [PATCH net-next 4/7] net: hibmcge: Add register dump supported in this module
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20241025/202410251738.mRleD5uf-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241025/202410251738.mRleD5uf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410251738.mRleD5uf-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c: In function 'hbg_ethtool_get_regs':
>> drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:322:20: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 31 [-Wformat-truncation=]
     322 |                  "[%s] %s", type_info->name, reg_map->name);
         |                    ^~
   In function 'hbg_get_reg_info',
       inlined from 'hbg_ethtool_get_regs' at drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:338:14:
   drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c:321:9: note: 'snprintf' output between 4 and 154 bytes into a destination of size 32
     321 |         snprintf(info->name, sizeof(info->name),
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     322 |                  "[%s] %s", type_info->name, reg_map->name);
         |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +322 drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c

   313	
   314	static u32 hbg_get_reg_info(struct hbg_priv *priv,
   315				    const struct hbg_reg_type_info *type_info,
   316				    const struct hbg_reg_offset_name_map *reg_map,
   317				    struct hbg_reg_info *info)
   318	{
   319		info->val = hbg_reg_read(priv, reg_map->reg_offset);
   320		info->offset = reg_map->reg_offset - type_info->offset_base;
   321		snprintf(info->name, sizeof(info->name),
 > 322			 "[%s] %s", type_info->name, reg_map->name);
   323	
   324		return sizeof(*info);
   325	}
   326	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH net-next 5/7] net: hibmcge: Add pauseparam supported in this module

[Jijie Shao]

The MAC can automatically send or respond to pause frames.
This patch supports the function of enabling pause frames
by using ethtool.

Not support pause auto-negotiation.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.c  | 24 +++++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   | 21 ++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.h   |  3 +++
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  1 +
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  2 ++
 5 files changed, 51 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
index a630c7d8ef5c..1e93d1dcf7a0 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -341,6 +341,28 @@ static void hbg_ethtool_get_regs(struct net_device *netdev,
 	}
 }
 
+static void hbg_ethtool_get_pauseparam(struct net_device *net_dev,
+				       struct ethtool_pauseparam *param)
+{
+	struct hbg_priv *priv = netdev_priv(net_dev);
+
+	hbg_hw_get_pause_enable(priv, &param->tx_pause, &param->rx_pause);
+}
+
+static int hbg_ethtool_set_pauseparam(struct net_device *net_dev,
+				      struct ethtool_pauseparam *param)
+{
+	struct hbg_priv *priv = netdev_priv(net_dev);
+
+	if (param->autoneg) {
+		netdev_err(net_dev, "autoneg unsupported\n");
+		return -EOPNOTSUPP;
+	}
+
+	hbg_hw_set_pause_enable(priv, !!param->tx_pause, !!param->rx_pause);
+	return 0;
+}
+
 static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
@@ -350,6 +372,8 @@ static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_ethtool_stats	= hbg_ethtool_get_stats,
 	.get_regs_len		= hbg_ethtool_get_regs_len,
 	.get_regs		= hbg_ethtool_get_regs,
+	.get_pauseparam         = hbg_ethtool_get_pauseparam,
+	.set_pauseparam         = hbg_ethtool_set_pauseparam,
 };
 
 void hbg_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 29d66a0ea0a6..0cbe9f7229b3 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -220,6 +220,27 @@ void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable)
 			    HBG_REG_REC_FILT_CTRL_UC_MATCH_EN_B, enable);
 }
 
+void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en)
+{
+	hbg_reg_write_field(priv, HBG_REG_PAUSE_ENABLE_ADDR,
+			    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);
+}
+
+void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en)
+{
+	*tx_en = hbg_reg_read_field(priv, HBG_REG_PAUSE_ENABLE_ADDR,
+				    HBG_REG_PAUSE_ENABLE_TX_B);
+	*rx_en = hbg_reg_read_field(priv, HBG_REG_PAUSE_ENABLE_ADDR,
+				    HBG_REG_PAUSE_ENABLE_RX_B);
+}
+
+void hbg_hw_set_rx_pause_mac_addr(struct hbg_priv *priv, u64 mac_addr)
+{
+	hbg_reg_write64(priv, HBG_REG_FD_FC_ADDR_LOW_ADDR, mac_addr);
+}
+
 static void hbg_hw_init_transmit_ctrl(struct hbg_priv *priv)
 {
 	u32 ctrl = 0;
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 6eb4b7d2cba8..a4a049b5121d 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -56,5 +56,8 @@ u32 hbg_hw_get_fifo_used_num(struct hbg_priv *priv, enum hbg_dir dir);
 void hbg_hw_set_tx_desc(struct hbg_priv *priv, struct hbg_tx_desc *tx_desc);
 void hbg_hw_fill_buffer(struct hbg_priv *priv, u32 buffer_dma_addr);
 void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable);
+void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en);
+void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en);
+void hbg_hw_set_rx_pause_mac_addr(struct hbg_priv *priv, u64 mac_addr);
 
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 0b7cfbd166ec..6331cda91575 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -196,6 +196,7 @@ static int hbg_net_set_mac_address(struct net_device *netdev, void *addr)
 	if (is_exists)
 		hbg_set_mac_to_mac_table(priv, index, NULL);
 
+	hbg_hw_set_rx_pause_mac_addr(priv, ether_addr_to_u64(mac_addr));
 	dev_addr_set(netdev, mac_addr);
 	return 0;
 }
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index bbfefe9c1e61..6088f1aef23e 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -51,6 +51,8 @@
 #define HBG_REG_PORT_ENABLE_RX_B		BIT(1)
 #define HBG_REG_PORT_ENABLE_TX_B		BIT(2)
 #define HBG_REG_PAUSE_ENABLE_ADDR		(HBG_REG_SGMII_BASE + 0x0048)
+#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_TRANSMIT_CTRL_ADDR		(HBG_REG_SGMII_BASE + 0x0060)
 #define HBG_REG_TRANSMIT_CTRL_PAD_EN_B		BIT(7)
-- 
2.33.0

Re: [PATCH net-next 5/7] net: hibmcge: Add pauseparam supported in this module

[Andrew Lunn]

> +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev,
> +				      struct ethtool_pauseparam *param)
> +{
> +	struct hbg_priv *priv = netdev_priv(net_dev);
> +
> +	if (param->autoneg) {
> +		netdev_err(net_dev, "autoneg unsupported\n");
> +		return -EOPNOTSUPP;
> +	}

Not being able to do it is not an error, so there is no need for the
netdev_err().


    Andrew

---
pw-bot: cr

Re: [PATCH net-next 5/7] net: hibmcge: Add pauseparam supported in this module

[Jijie Shao]

on 2024/10/23 22:15, Andrew Lunn wrote:
>> +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev,
>> +				      struct ethtool_pauseparam *param)
>> +{
>> +	struct hbg_priv *priv = netdev_priv(net_dev);
>> +
>> +	if (param->autoneg) {
>> +		netdev_err(net_dev, "autoneg unsupported\n");
>> +		return -EOPNOTSUPP;
>> +	}
> Not being able to do it is not an error, so there is no need for the
> netdev_err().

Ok, Thanks!

>
>
>      Andrew
>
> ---
> pw-bot: cr
>

[PATCH net-next 6/7] net: hibmcge: Add nway_reset supported in this module

[Jijie Shao]

Add nway_reset supported in this module

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
index 1e93d1dcf7a0..2fef3d161c21 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -367,6 +367,7 @@ static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
 	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.nway_reset		= phy_ethtool_nway_reset,
 	.get_sset_count		= hbg_ethtool_get_sset_count,
 	.get_strings		= hbg_ethtool_get_strings,
 	.get_ethtool_stats	= hbg_ethtool_get_stats,
-- 
2.33.0

Re: [PATCH net-next 6/7] net: hibmcge: Add nway_reset supported in this module

[Andrew Lunn]

On Wed, Oct 23, 2024 at 09:42:12PM +0800, Jijie Shao wrote:
> Add nway_reset supported in this module
> 
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
> index 1e93d1dcf7a0..2fef3d161c21 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
> @@ -367,6 +367,7 @@ static const struct ethtool_ops hbg_ethtool_ops = {
>  	.get_link		= ethtool_op_get_link,
>  	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
>  	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
> +	.nway_reset		= phy_ethtool_nway_reset,
>  	.get_sset_count		= hbg_ethtool_get_sset_count,
>  	.get_strings		= hbg_ethtool_get_strings,
>  	.get_ethtool_stats	= hbg_ethtool_get_stats,

It is odd that you have phy_ethtool_get_link_ksettings etc, so have
phylib integration, yet don't support pause autoneg?

	Andrew

[PATCH net-next 7/7] net: hibmcge: Add reset supported in this module

[Jijie Shao]

Sometimes, if the port doesn't work, we can try to fix it by resetting it.

This patch supports reset triggered by ethtool or FLR of PCIe, For example:
 ethtool --reset eth0 dedicated
 echo 1 > /sys/bus/pci/devices/0000\:83\:00.1/reset

We hope that the reset can be performed only when the port is down,
and the port cannot be up during the reset.
Therefore, the entire reset process is protected by the rtnl lock.
But the ethtool command already holds the rtnl lock in the dev_ethtool().
Therefore, the reset operation is not directly performed in
ethtool_ops.reset() function. Instead, the reset operation
is triggered by a scheduled task.

After the reset is complete, the hardware registers are restored
to their default values. Therefore, some rebuild operations are
required to rewrite the user configuration to the registers.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../net/ethernet/hisilicon/hibmcge/Makefile   |   2 +-
 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  22 +++
 .../ethernet/hisilicon/hibmcge/hbg_debugfs.c  |  14 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_err.c  | 140 ++++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_err.h  |  13 ++
 .../ethernet/hisilicon/hibmcge/hbg_ethtool.c  |  15 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_hw.c   |  10 +-
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |  47 +++---
 8 files changed, 242 insertions(+), 21 deletions(-)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_err.h

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/Makefile b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
index 1a0ec2fb8c24..7ea15f9ef849 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/Makefile
+++ b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
@@ -6,4 +6,4 @@
 obj-$(CONFIG_HIBMCGE) += hibmcge.o
 
 hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o \
-		hbg_debugfs.o
+		hbg_debugfs.o hbg_err.o
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 491192a4fc74..9eefa55fecef 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -4,6 +4,7 @@
 #ifndef __HBG_COMMON_H
 #define __HBG_COMMON_H
 
+#include <linux/ethtool.h>
 #include <linux/netdevice.h>
 #include <linux/pci.h>
 #include "hbg_reg.h"
@@ -33,6 +34,15 @@ enum hbg_tx_state {
 
 enum hbg_nic_state {
 	HBG_NIC_STATE_EVENT_HANDLING = 0,
+	HBG_NIC_STATE_NEED_RESET,
+	HBG_NIC_STATE_RESETTING,
+	HBG_NIC_STATE_RESET_FAIL,
+};
+
+enum hbg_reset_type {
+	HBG_RESET_TYPE_NONE = 0,
+	HBG_RESET_TYPE_FLR,
+	HBG_RESET_TYPE_FUNCTION,
 };
 
 struct hbg_buffer {
@@ -213,6 +223,13 @@ struct hbg_stats {
 
 	u64 tx_timeout_cnt;
 	u64 tx_dma_err_cnt;
+
+	u64 reset_fail_cnt;
+};
+
+/* saved for restore after rest */
+struct hbg_user_def {
+	struct ethtool_pauseparam pause_param;
 };
 
 struct hbg_mac_table_entry {
@@ -239,6 +256,11 @@ struct hbg_priv {
 	struct hbg_stats stats;
 	struct delayed_work service_task;
 	struct hbg_mac_filter filter;
+
+	struct hbg_user_def user_def;
+	enum hbg_reset_type reset_type;
 };
 
+void hbg_reset_task_schedule(struct hbg_priv *priv);
+
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
index 85d5cb3cd603..72a8139c49a1 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_debugfs.c
@@ -97,6 +97,8 @@ static int hbg_dbg_irq_info(struct seq_file *s, void *unused)
 	return 0;
 }
 
+static const char * const reset_type_str[] = {"None", "FLR", "Function"};
+
 static int hbg_dbg_nic_state(struct seq_file *s, void *unused)
 {
 	struct net_device *netdev = dev_get_drvdata(s->private);
@@ -105,7 +107,19 @@ static int hbg_dbg_nic_state(struct seq_file *s, void *unused)
 	seq_printf(s, "event handling state: %s\n",
 		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_EVENT_HANDLING,
 					     &priv->state)));
+	seq_printf(s, "need reset state: %s\n",
+		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_NEED_RESET,
+					     &priv->state)));
+	seq_printf(s, "resetting state: %s\n",
+		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_RESETTING,
+					     &priv->state)));
+	seq_printf(s, "reset fail state: %s\n",
+		   hbg_get_bool_str(test_bit(HBG_NIC_STATE_RESET_FAIL,
+					     &priv->state)));
+	seq_printf(s, "last reset type: %s\n",
+		   reset_type_str[priv->reset_type]);
 
+	seq_printf(s, "reset fail cnt: %llu\n", priv->stats.reset_fail_cnt);
 	seq_printf(s, "tx timeout cnt: %llu\n", priv->stats.tx_timeout_cnt);
 	return 0;
 }
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
new file mode 100644
index 000000000000..25fa25234df6
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/etherdevice.h>
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <linux/rtnetlink.h>
+#include "hbg_common.h"
+#include "hbg_err.h"
+#include "hbg_hw.h"
+
+static void hbg_restore_mac_table(struct hbg_priv *priv)
+{
+	struct hbg_mac_filter *filter = &priv->filter;
+	u64 addr;
+	u32 i;
+
+	for (i = 0; i < filter->table_max_len; i++)
+		if (!is_zero_ether_addr(filter->mac_table[i].addr)) {
+			addr = ether_addr_to_u64(filter->mac_table[i].addr);
+			hbg_hw_set_uc_addr(priv, addr, i);
+		}
+
+	hbg_hw_set_mac_filter_enable(priv, priv->filter.enabled);
+}
+
+static void hbg_restore_user_def_settings(struct hbg_priv *priv)
+{
+	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);
+}
+
+int hbg_rebuild(struct hbg_priv *priv)
+{
+	int ret;
+
+	ret = hbg_hw_init(priv);
+	if (ret)
+		return ret;
+
+	hbg_restore_user_def_settings(priv);
+	return 0;
+}
+
+static int hbg_reset_prepare(struct hbg_priv *priv, enum hbg_reset_type type)
+{
+	int ret = -EBUSY;
+
+	rtnl_lock();
+
+	set_bit(HBG_NIC_STATE_RESETTING, &priv->state);
+
+	if (netif_running(priv->netdev)) {
+		dev_warn(&priv->pdev->dev,
+			 "failed to reset because port is up\n");
+		goto unlock;
+	}
+
+	priv->reset_type = type;
+	ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_RESET);
+	if (!ret)
+		return 0;
+
+	priv->stats.reset_fail_cnt++;
+	set_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
+
+unlock:
+	clear_bit(HBG_NIC_STATE_RESETTING, &priv->state);
+	rtnl_unlock();
+	return ret;
+}
+
+static int hbg_reset_done(struct hbg_priv *priv, enum hbg_reset_type type)
+{
+	int ret;
+
+	if (!test_bit(HBG_NIC_STATE_RESETTING, &priv->state) ||
+	    type != priv->reset_type)
+		return 0;
+
+	ASSERT_RTNL();
+
+	ret = hbg_rebuild(priv);
+	if (ret) {
+		priv->stats.reset_fail_cnt++;
+		set_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
+		dev_err(&priv->pdev->dev, "failed to rebuild after reset\n");
+		goto unlock;
+	}
+
+	clear_bit(HBG_NIC_STATE_RESET_FAIL, &priv->state);
+	dev_info(&priv->pdev->dev, "reset done\n");
+
+unlock:
+	clear_bit(HBG_NIC_STATE_RESETTING, &priv->state);
+	rtnl_unlock();
+	return ret;
+}
+
+int hbg_reset(struct hbg_priv *priv)
+{
+	int ret;
+
+	ret = hbg_reset_prepare(priv, HBG_RESET_TYPE_FUNCTION);
+	if (ret)
+		return ret;
+
+	return hbg_reset_done(priv, HBG_RESET_TYPE_FUNCTION);
+}
+
+static void hbg_pci_err_reset_prepare(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_reset_prepare(priv, HBG_RESET_TYPE_FLR);
+}
+
+static void hbg_pci_err_reset_done(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	hbg_reset_done(priv, HBG_RESET_TYPE_FLR);
+}
+
+static const struct pci_error_handlers hbg_pci_err_handler = {
+	.reset_prepare = hbg_pci_err_reset_prepare,
+	.reset_done = hbg_pci_err_reset_done,
+};
+
+void hbg_set_pci_err_handler(struct pci_driver *pdrv)
+{
+	pdrv->err_handler = &hbg_pci_err_handler;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.h
new file mode 100644
index 000000000000..d7828e446308
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_err.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_ERR_H
+#define __HBG_ERR_H
+
+#include <linux/pci.h>
+
+void hbg_set_pci_err_handler(struct pci_driver *pdrv);
+int hbg_reset(struct hbg_priv *priv);
+int hbg_rebuild(struct hbg_priv *priv);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
index 2fef3d161c21..6c9a9298a2b2 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -4,6 +4,7 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 #include "hbg_common.h"
+#include "hbg_err.h"
 #include "hbg_ethtool.h"
 #include "hbg_hw.h"
 
@@ -360,6 +361,19 @@ static int hbg_ethtool_set_pauseparam(struct net_device *net_dev,
 	}
 
 	hbg_hw_set_pause_enable(priv, !!param->tx_pause, !!param->rx_pause);
+	priv->user_def.pause_param = *param;
+	return 0;
+}
+
+static int hbg_ethtool_reset(struct net_device *netdev, u32 *flags)
+{
+	struct hbg_priv *priv = netdev_priv(netdev);
+
+	if (*flags != ETH_RESET_DEDICATED)
+		return -EOPNOTSUPP;
+
+	hbg_reset_task_schedule(priv);
+	*flags = 0;
 	return 0;
 }
 
@@ -375,6 +389,7 @@ static const struct ethtool_ops hbg_ethtool_ops = {
 	.get_regs		= hbg_ethtool_get_regs,
 	.get_pauseparam         = hbg_ethtool_get_pauseparam,
 	.set_pauseparam         = hbg_ethtool_set_pauseparam,
+	.reset			= hbg_ethtool_reset,
 };
 
 void hbg_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 0cbe9f7229b3..e7798f213645 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -3,6 +3,7 @@
 
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 #include <linux/iopoll.h>
 #include <linux/minmax.h>
 #include "hbg_common.h"
@@ -167,8 +168,13 @@ static void hbg_hw_set_mac_max_frame_len(struct hbg_priv *priv,
 
 void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu)
 {
-	hbg_hw_set_pcu_max_frame_len(priv, mtu);
-	hbg_hw_set_mac_max_frame_len(priv, mtu);
+	u32 frame_len;
+
+	frame_len = mtu + VLAN_HLEN * priv->dev_specs.vlan_layers +
+		    ETH_HLEN + ETH_FCS_LEN;
+
+	hbg_hw_set_pcu_max_frame_len(priv, frame_len);
+	hbg_hw_set_mac_max_frame_len(priv, frame_len);
 }
 
 void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 6331cda91575..bd7e77e344d6 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -6,6 +6,7 @@
 #include <linux/netdevice.h>
 #include <linux/pci.h>
 #include "hbg_common.h"
+#include "hbg_err.h"
 #include "hbg_ethtool.h"
 #include "hbg_hw.h"
 #include "hbg_irq.h"
@@ -13,8 +14,6 @@
 #include "hbg_txrx.h"
 #include "hbg_debugfs.h"
 
-static void hbg_change_mtu(struct hbg_priv *priv, int new_mtu);
-
 static void hbg_all_irq_enable(struct hbg_priv *priv, bool enabled)
 {
 	struct hbg_irq_info *info;
@@ -56,11 +55,7 @@ static int hbg_hw_txrx_clear(struct hbg_priv *priv)
 		return ret;
 
 	/* After reset, regs need to be reconfigured */
-	hbg_hw_init(priv);
-	hbg_hw_set_uc_addr(priv, ether_addr_to_u64(priv->netdev->dev_addr), 0);
-	hbg_change_mtu(priv, priv->netdev->mtu);
-
-	return 0;
+	return hbg_rebuild(priv);
 }
 
 static int hbg_net_stop(struct net_device *netdev)
@@ -201,15 +196,6 @@ static int hbg_net_set_mac_address(struct net_device *netdev, void *addr)
 	return 0;
 }
 
-static void hbg_change_mtu(struct hbg_priv *priv, int new_mtu)
-{
-	u32 frame_len;
-
-	frame_len = new_mtu + VLAN_HLEN * priv->dev_specs.vlan_layers +
-		    ETH_HLEN + ETH_FCS_LEN;
-	hbg_hw_set_mtu(priv, frame_len);
-}
-
 static int hbg_net_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct hbg_priv *priv = netdev_priv(netdev);
@@ -217,7 +203,7 @@ static int hbg_net_change_mtu(struct net_device *netdev, int new_mtu)
 	if (netif_running(netdev))
 		return -EBUSY;
 
-	hbg_change_mtu(priv, new_mtu);
+	hbg_hw_set_mtu(priv, new_mtu);
 	WRITE_ONCE(netdev->mtu, new_mtu);
 
 	dev_dbg(&priv->pdev->dev,
@@ -280,6 +266,9 @@ static void hbg_service_task(struct work_struct *work)
 	struct hbg_priv *priv = container_of(work, struct hbg_priv,
 					     service_task.work);
 
+	if (test_and_clear_bit(HBG_NIC_STATE_NEED_RESET, &priv->state))
+		hbg_reset(priv);
+
 	/* The type of statistics register is u32,
 	 * and the type of driver statistics is u64.
 	 * To prevent the statistics register from overflowing,
@@ -290,6 +279,18 @@ static void hbg_service_task(struct work_struct *work)
 			      msecs_to_jiffies(5 * 60 * MSEC_PER_SEC));
 }
 
+void hbg_reset_task_schedule(struct hbg_priv *priv)
+{
+	set_bit(HBG_NIC_STATE_NEED_RESET, &priv->state);
+
+	/* Generally, service_task is scheduled every 5 minutes.
+	 * Here, we want to schedule immediately,
+	 * so cancel the original task and reschedule it.
+	 */
+	cancel_delayed_work(&priv->service_task);
+	schedule_delayed_work(&priv->service_task, 0);
+}
+
 static void hbg_delaywork_init(struct hbg_priv *priv)
 {
 	INIT_DELAYED_WORK(&priv->service_task, hbg_service_task);
@@ -320,6 +321,14 @@ static int hbg_mac_filter_init(struct hbg_priv *priv)
 	return 0;
 }
 
+static void hbg_init_user_def(struct hbg_priv *priv)
+{
+	struct ethtool_pauseparam *pause_param = &priv->user_def.pause_param;
+
+	hbg_hw_get_pause_enable(priv, &pause_param->tx_pause,
+				&pause_param->rx_pause);
+}
+
 static int hbg_init(struct hbg_priv *priv)
 {
 	int ret;
@@ -348,6 +357,7 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
+	hbg_init_user_def(priv);
 	hbg_delaywork_init(priv);
 	return devm_add_action_or_reset(&priv->pdev->dev, hbg_delaywork_uninit,
 					&priv->service_task);
@@ -414,7 +424,7 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netdev->netdev_ops = &hbg_netdev_ops;
 	netdev->watchdog_timeo = 5 * HZ;
 
-	hbg_change_mtu(priv, ETH_DATA_LEN);
+	hbg_hw_set_mtu(priv, ETH_DATA_LEN);
 	hbg_net_set_mac_address(priv->netdev, &priv->dev_specs.mac_addr);
 	hbg_ethtool_set_ops(netdev);
 
@@ -443,6 +453,7 @@ static int __init hbg_module_init(void)
 	int ret;
 
 	hbg_debugfs_register();
+	hbg_set_pci_err_handler(&hbg_driver);
 	ret = pci_register_driver(&hbg_driver);
 	if (ret)
 		hbg_debugfs_unregister();
-- 
2.33.0