* [RFC PATCH net-next 01/10] net: hibmcge: Add pci table supported in this module
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space Jijie Shao
` (8 subsequent siblings)
9 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Add pci table supported in this module, and implement pci_driver function
to initialize this driver.
hibmcge is a passthrough network device. Its software runs
on the host side, and the MAC hardware runs on the BMC side
to reduce the host CPU area. The software interacts with the
MAC hardware through the PCIe.
┌─────────────────────────┐
│ HOST CPU network device │
│ ┌──────────────┐ │
│ │hibmcge driver│ │
│ └─────┬─┬──────┘ │
│ │ │ │
│HOST ┌───┴─┴───┐ │
│ │ PCIE RC │ │
└──────┴───┬─┬───┴────────┘
│ │
PCIE
│ │
┌──────┬───┴─┴───┬────────┐
│ │ PCIE EP │ │
│BMC └───┬─┬───┘ │
│ │ │ │
│ ┌────────┴─┴──────────┐ │
│ │ GE │ │
│ │ ┌─────┐ ┌─────┐ │ │
│ │ │ MAC │ │ MAC │ │ │
└─┴─┼─────┼────┼─────┼──┴─┘
│ PHY │ │ PHY │
└─────┘ └─────┘
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 16 ++++
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 83 +++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_main.h | 13 +++
3 files changed, 112 insertions(+)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
new file mode 100644
index 000000000000..e08e28f25f3c
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_COMMON_H
+#define __HBG_COMMON_H
+
+#include <linux/etherdevice.h>
+#include <linux/pci.h>
+
+struct hbg_priv {
+ struct net_device *netdev;
+ struct pci_dev *pdev;
+ u8 __iomem *io_base;
+};
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
new file mode 100644
index 000000000000..30cc19e71c54
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/etherdevice.h>
+#include <linux/netdevice.h>
+#include <linux/pci.h>
+#include "hbg_common.h"
+#include "hbg_main.h"
+
+static int hbg_pci_init(struct pci_dev *pdev)
+{
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct hbg_priv *priv = netdev_priv(netdev);
+ int ret;
+
+ ret = pcim_enable_device(pdev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to enable PCI device\n");
+
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to set consistent PCI DMA\n");
+
+ ret = pcim_iomap_regions(pdev, BIT(HBG_MEM_BAR), HBG_DEV_NAME);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to map PCI bar space");
+
+ priv->io_base = pcim_iomap_table(pdev)[HBG_MEM_BAR];
+ pci_set_master(pdev);
+ return 0;
+}
+
+static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+ struct net_device *netdev;
+ struct hbg_priv *priv;
+ int ret;
+
+ netdev = devm_alloc_etherdev_mqs(&pdev->dev,
+ sizeof(struct hbg_priv), 1, 1);
+ if (!netdev)
+ return -ENOMEM;
+
+ pci_set_drvdata(pdev, netdev);
+
+ SET_NETDEV_DEV(netdev, &pdev->dev);
+
+ priv = netdev_priv(netdev);
+ priv->netdev = netdev;
+ priv->pdev = pdev;
+
+ ret = hbg_pci_init(pdev);
+ if (ret)
+ return ret;
+
+ ret = devm_register_netdev(&pdev->dev, netdev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register netdev\n");
+
+ return 0;
+}
+
+static const struct pci_device_id hbg_pci_tbl[] = {
+ {PCI_VDEVICE(HUAWEI, 0x3730), 0},
+ { }
+};
+MODULE_DEVICE_TABLE(pci, hbg_pci_tbl);
+
+static struct pci_driver hbg_driver = {
+ .name = HBG_DEV_NAME,
+ .id_table = hbg_pci_tbl,
+ .probe = hbg_probe,
+};
+module_pci_driver(hbg_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
+MODULE_DESCRIPTION("hibmcge driver");
+MODULE_VERSION(HBG_MOD_VERSION);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
new file mode 100644
index 000000000000..f9652e5c06b2
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_MAIN_H
+#define __HBG_MAIN_H
+
+#include <linux/io.h>
+
+#define HBG_DEV_NAME "hibmcge"
+#define HBG_MEM_BAR 0
+#define HBG_MOD_VERSION "1.0"
+
+#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 01/10] net: hibmcge: Add pci table supported in this module Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-08-05 12:55 ` Simon Horman
2024-07-31 9:42 ` [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module Jijie Shao
` (7 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Add support for to read and write registers through the pic bar space.
Some driver parameters, such as mac_id, are determined by the
board form. Therefore, these parameters are initialized
from the register as device specifications.
the device specifications register are initialized and writed by bmc.
driver will read these registers when loading.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 26 +++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 72 +++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 61 ++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 18 +++++
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 19 +++++
5 files changed, 196 insertions(+)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index e08e28f25f3c..b56b5179f735 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -7,10 +7,36 @@
#include <linux/etherdevice.h>
#include <linux/pci.h>
+enum hbg_nic_state {
+ HBG_NIC_STATE_INITED = 0,
+ HBG_NIC_STATE_EVENT_HANDLING,
+};
+
+enum hbg_hw_event_type {
+ HBG_HW_EVENT_NONE = 0,
+ HBG_HW_EVENT_INIT, /* driver is loading */
+};
+
+struct hbg_dev_specs {
+ u32 mac_id;
+ struct sockaddr mac_addr;
+ u32 phy_addr;
+ u32 rx_fifo_num;
+ u32 tx_fifo_num;
+ u32 vlan_layers;
+ u32 max_mtu;
+ u32 min_mtu;
+
+ u32 max_frame_len;
+ u32 rx_buf_size;
+};
+
struct hbg_priv {
struct net_device *netdev;
struct pci_dev *pdev;
u8 __iomem *io_base;
+ struct hbg_dev_specs dev_specs;
+ unsigned long state;
};
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
new file mode 100644
index 000000000000..ca1cb09c90ff
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/minmax.h>
+#include <linux/netdevice.h>
+#include <linux/pci.h>
+#include "hbg_common.h"
+#include "hbg_hw.h"
+#include "hbg_reg.h"
+
+static bool hbg_hw_spec_is_valid(struct hbg_priv *priv)
+{
+ return hbg_reg_read(priv, HBG_REG_SPEC_VALID_ADDR) &&
+ !hbg_reg_read(priv, HBG_REG_EVENT_REQ_ADDR);
+}
+
+int hbg_hw_event_notify(struct hbg_priv *priv, enum hbg_hw_event_type event_type)
+{
+#define HBG_HW_EVENT_WAIT_TIMEOUT_MS (2 * 1000)
+#define HBG_HW_EVENT_WAIT_INTERVAL_MS 100
+
+ u32 timeout = 0;
+
+ if (test_and_set_bit(HBG_NIC_STATE_EVENT_HANDLING, &priv->state))
+ return -EBUSY;
+
+ /* notify */
+ hbg_reg_write(priv, HBG_REG_EVENT_REQ_ADDR, event_type);
+
+ do {
+ msleep(HBG_HW_EVENT_WAIT_INTERVAL_MS);
+ timeout += HBG_HW_EVENT_WAIT_INTERVAL_MS;
+
+ if (hbg_hw_spec_is_valid(priv))
+ break;
+ } while (timeout <= HBG_HW_EVENT_WAIT_TIMEOUT_MS);
+
+ clear_bit(HBG_NIC_STATE_EVENT_HANDLING, &priv->state);
+
+ if (!hbg_hw_spec_is_valid(priv)) {
+ dev_err(&priv->pdev->dev, "event %d wait timeout\n", event_type);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+int hbg_hw_dev_specs_init(struct hbg_priv *priv)
+{
+ struct hbg_dev_specs *dev_specs = &priv->dev_specs;
+ u64 mac_addr;
+
+ if (!hbg_hw_spec_is_valid(priv)) {
+ dev_err(&priv->pdev->dev, "dev_specs not init\n");
+ return -EINVAL;
+ }
+
+ dev_specs->mac_id = hbg_reg_read(priv, HBG_REG_MAC_ID_ADDR);
+ dev_specs->phy_addr = hbg_reg_read(priv, HBG_REG_PHY_ID_ADDR);
+ dev_specs->max_mtu = hbg_reg_read(priv, HBG_REG_MAX_MTU_ADDR);
+ dev_specs->min_mtu = hbg_reg_read(priv, HBG_REG_MIN_MTU_ADDR);
+ dev_specs->vlan_layers = hbg_reg_read(priv, HBG_REG_VLAN_LAYERS_ADDR);
+ dev_specs->rx_fifo_num = hbg_reg_read(priv, HBG_REG_RX_FIFO_NUM_ADDR);
+ dev_specs->tx_fifo_num = hbg_reg_read(priv, HBG_REG_TX_FIFO_NUM_ADDR);
+ mac_addr = hbg_reg_read64(priv, HBG_REG_MAC_ADDR_ADDR);
+ u64_to_ether_addr(mac_addr, (u8 *)dev_specs->mac_addr.sa_data);
+
+ if (!is_valid_ether_addr((u8 *)dev_specs->mac_addr.sa_data))
+ return -EADDRNOTAVAIL;
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
new file mode 100644
index 000000000000..61c6db948364
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_HW_H
+#define __HBG_HW_H
+
+#include <linux/io-64-nonatomic-lo-hi.h>
+
+static inline u32 hbg_reg_read(struct hbg_priv *priv, u32 reg_addr)
+{
+ return readl(priv->io_base + reg_addr);
+}
+
+static inline void hbg_reg_write(struct hbg_priv *priv, u32 reg_addr, u32 value)
+{
+ writel(value, priv->io_base + reg_addr);
+}
+
+static inline u32 hbg_reg_read_field(struct hbg_priv *priv, u32 reg_addr,
+ u32 mask)
+{
+ return FIELD_GET(mask, hbg_reg_read(priv, reg_addr));
+}
+
+static inline void hbg_reg_write_field(struct hbg_priv *priv, u32 reg_addr,
+ u32 mask, u32 val)
+{
+ u32 reg_value = hbg_reg_read(priv, reg_addr);
+
+ reg_value &= ~mask;
+ reg_value |= FIELD_PREP(mask, val);
+ hbg_reg_write(priv, reg_addr, reg_value);
+}
+
+static inline u32 hbg_reg_read_bit(struct hbg_priv *priv, u32 reg_addr,
+ u32 mask)
+{
+ return hbg_reg_read_field(priv, reg_addr, mask);
+}
+
+static inline void hbg_reg_write_bit(struct hbg_priv *priv, u32 reg_addr,
+ u32 mask, u32 val)
+{
+ hbg_reg_write_field(priv, reg_addr, mask, val);
+}
+
+static inline u64 hbg_reg_read64(struct hbg_priv *priv, u32 reg_addr)
+{
+ return lo_hi_readq(priv->io_base + reg_addr);
+}
+
+static inline void hbg_reg_write64(struct hbg_priv *priv, u32 reg_addr,
+ u64 value)
+{
+ lo_hi_writeq(value, priv->io_base + reg_addr);
+}
+
+int hbg_hw_event_notify(struct hbg_priv *priv, enum hbg_hw_event_type event_type);
+int hbg_hw_dev_specs_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 30cc19e71c54..df0fc6a1059b 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -5,8 +5,21 @@
#include <linux/netdevice.h>
#include <linux/pci.h>
#include "hbg_common.h"
+#include "hbg_hw.h"
#include "hbg_main.h"
+static int hbg_init(struct net_device *netdev)
+{
+ struct hbg_priv *priv = netdev_priv(netdev);
+ int ret;
+
+ ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_INIT);
+ if (ret)
+ return ret;
+
+ return hbg_hw_dev_specs_init(priv);
+}
+
static int hbg_pci_init(struct pci_dev *pdev)
{
struct net_device *netdev = pci_get_drvdata(pdev);
@@ -56,11 +69,16 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
return ret;
+ ret = hbg_init(netdev);
+ if (ret)
+ return ret;
+
ret = devm_register_netdev(&pdev->dev, netdev);
if (ret)
return dev_err_probe(&pdev->dev, ret,
"failed to register netdev\n");
+ set_bit(HBG_NIC_STATE_INITED, &priv->state);
return 0;
}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
new file mode 100644
index 000000000000..4c34516e0c89
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_REG_H
+#define __HBG_REG_H
+
+/* DEV SPEC */
+#define HBG_REG_SPEC_VALID_ADDR 0x0000
+#define HBG_REG_EVENT_REQ_ADDR 0x0004
+#define HBG_REG_MAC_ID_ADDR 0x0008
+#define HBG_REG_PHY_ID_ADDR 0x000C
+#define HBG_REG_MAC_ADDR_ADDR 0x0010
+#define HBG_REG_MAX_MTU_ADDR 0x0028
+#define HBG_REG_MIN_MTU_ADDR 0x002C
+#define HBG_REG_TX_FIFO_NUM_ADDR 0x0030
+#define HBG_REG_RX_FIFO_NUM_ADDR 0x0034
+#define HBG_REG_VLAN_LAYERS_ADDR 0x0038
+
+#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space
2024-07-31 9:42 ` [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space Jijie Shao
@ 2024-08-05 12:55 ` Simon Horman
2024-08-05 13:08 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Simon Horman @ 2024-08-05 12:55 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Wed, Jul 31, 2024 at 05:42:37PM +0800, Jijie Shao wrote:
> Add support for to read and write registers through the pic bar space.
>
> Some driver parameters, such as mac_id, are determined by the
> board form. Therefore, these parameters are initialized
> from the register as device specifications.
>
> the device specifications register are initialized and writed by bmc.
> driver will read these registers when loading.
>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
...
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
...
> +static inline void hbg_reg_write_field(struct hbg_priv *priv, u32 reg_addr,
> + u32 mask, u32 val)
> +{
> + u32 reg_value = hbg_reg_read(priv, reg_addr);
> +
> + reg_value &= ~mask;
> + reg_value |= FIELD_PREP(mask, val);
Hi,
I may well be wrong but I think that FIELD_PREP can only be used with
a compile-time constant as the mask.
In any case, with Clang-18 W=1 allmodconfig builds on x86_64 I see:
CC drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.o
CC drivers/net/ethernet/hisilicon/hibmcge/hbg_main.o
In file included from drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c:8:
drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h:31:15: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
31 | reg_value |= FIELD_PREP(mask, val);
| ^~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:72:53: note: expanded from macro '__BF_FIELD_CHECK'
72 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
73 | __bf_cast_unsigned(_reg, ~0ull), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
74 | _pfx "type of reg too small for mask"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
././include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert'
498 | __compiletime_assert(condition, msg, prefix, suffix)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
1 warning generated.
In file included from drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c:8:
drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h:31:15: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
31 | reg_value |= FIELD_PREP(mask, val);
| ^~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:72:53: note: expanded from macro '__BF_FIELD_CHECK'
72 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
73 | __bf_cast_unsigned(_reg, ~0ull), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
74 | _pfx "type of reg too small for mask"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
././include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert'
498 | __compiletime_assert(condition, msg, prefix, suffix)
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
1 warning generated.
...
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space
2024-08-05 12:55 ` Simon Horman
@ 2024-08-05 13:08 ` Jijie Shao
0 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-08-05 13:08 UTC (permalink / raw)
To: Simon Horman
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/5 20:55, Simon Horman wrote:
> Hi,
>
> I may well be wrong but I think that FIELD_PREP can only be used with
> a compile-time constant as the mask.
>
> In any case, with Clang-18 W=1 allmodconfig builds on x86_64 I see:
Thanks， I'll fix this warning in V2.
>
> CC drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.o
> CC drivers/net/ethernet/hisilicon/hibmcge/hbg_main.o
> In file included from drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c:8:
> drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h:31:15: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
> 31 | reg_value |= FIELD_PREP(mask, val);
> | ^~~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
> 115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitfield.h:72:53: note: expanded from macro '__BF_FIELD_CHECK'
> 72 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> 73 | __bf_cast_unsigned(_reg, ~0ull), \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 74 | _pfx "type of reg too small for mask"); \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> ././include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert'
> 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ././include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert'
> 498 | __compiletime_assert(condition, msg, prefix, suffix)
> | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ././include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert'
> 490 | if (!(condition)) \
> | ^~~~~~~~~
> 1 warning generated.
> In file included from drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c:8:
> drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h:31:15: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
> 31 | reg_value |= FIELD_PREP(mask, val);
> | ^~~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
> 115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitfield.h:72:53: note: expanded from macro '__BF_FIELD_CHECK'
> 72 | BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> 73 | __bf_cast_unsigned(_reg, ~0ull), \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 74 | _pfx "type of reg too small for mask"); \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> ././include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert'
> 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ././include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert'
> 498 | __compiletime_assert(condition, msg, prefix, suffix)
> | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ././include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert'
> 490 | if (!(condition)) \
> | ^~~~~~~~~
> 1 warning generated.
>
> ...
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 01/10] net: hibmcge: Add pci table supported in this module Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 02/10] net: hibmcge: Add read/write registers supported through the bar space Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-08-01 0:42 ` Andrew Lunn
2024-07-31 9:42 ` [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt " Jijie Shao
` (6 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
this driver using phy through genphy device. Implements the C22
read and write PHY registers interfaces.
Some hardware interfaces related to the PHY are also implemented
in this patch.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 34 +++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 170 +++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 3 +
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 26 +-
.../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 276 ++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_mdio.h | 13 +
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 46 +++
.../hisilicon/hibmcge/hbg_reg_union.h | 112 +++++++
8 files changed, 678 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.h
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index b56b5179f735..364aab4d61b9 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -5,7 +5,27 @@
#define __HBG_COMMON_H
#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/linkmode.h>
#include <linux/pci.h>
+#include "hbg_reg.h"
+
+#define HBG_STATUS_DISABLE 0x0
+#define HBG_STATUS_ENABLE 0x1
+#define HBG_DEFAULT_MTU_SIZE 1500
+#define HBG_RX_SKIP1 0x00
+#define HBG_RX_SKIP2 0x01
+#define HBG_LINK_DOWN 0
+#define HBG_LINK_UP 1
+
+enum hbg_dir {
+ HBG_DIR_TX = 1 << 0,
+ HBG_DIR_RX = 1 << 1,
+ HBG_DIR_TX_RX = HBG_DIR_TX | HBG_DIR_RX,
+};
+
+#define hbg_dir_has_tx(dir) ((dir) & HBG_DIR_TX)
+#define hbg_dir_has_rx(dir) ((dir) & HBG_DIR_RX)
enum hbg_nic_state {
HBG_NIC_STATE_INITED = 0,
@@ -31,12 +51,26 @@ struct hbg_dev_specs {
u32 rx_buf_size;
};
+struct hbg_mac {
+ struct mii_bus *mdio_bus;
+ struct phy_device *phydev;
+ u8 phy_addr;
+
+ u32 speed;
+ u32 duplex;
+ u32 autoneg;
+ u32 link_status;
+
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+};
+
struct hbg_priv {
struct net_device *netdev;
struct pci_dev *pdev;
u8 __iomem *io_base;
struct hbg_dev_specs dev_specs;
unsigned long state;
+ struct hbg_mac mac;
};
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index ca1cb09c90ff..b2465ba06cee 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -70,3 +70,173 @@ int hbg_hw_dev_specs_init(struct hbg_priv *priv)
return 0;
}
+
+int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
+{
+ if (speed != HBG_PORT_MODE_SGMII_10M &&
+ speed != HBG_PORT_MODE_SGMII_100M &&
+ speed != HBG_PORT_MODE_SGMII_1000M)
+ return -EOPNOTSUPP;
+
+ if (duplex != DUPLEX_FULL && duplex != DUPLEX_HALF)
+ return -EOPNOTSUPP;
+
+ if (speed == HBG_PORT_MODE_SGMII_1000M && duplex == DUPLEX_HALF)
+ return -EOPNOTSUPP;
+
+ hbg_reg_write_field(priv, HBG_REG_PORT_MODE_ADDR,
+ HBG_REG_PORT_MODE_M, speed);
+ hbg_reg_write_bit(priv, HBG_REG_DUPLEX_TYPE_ADDR,
+ HBG_REG_DUPLEX_B, duplex);
+
+ priv->mac.speed = speed;
+ priv->mac.duplex = duplex;
+
+ return 0;
+}
+
+/* sgmii autoneg always enable */
+int hbg_hw_sgmii_autoneg(struct hbg_priv *priv)
+{
+#define HBG_HW_DEFAULT_16_BIT_CNTR 0x3F
+#define HBG_HW_AUTONEG_TIMEOUT_MS 1000
+#define HBG_HW_AUTONEG_TIMEOUT_STEP 5
+
+ struct hbg_transmit_control control;
+ struct hbg_an_state an_state;
+ u32 wait_time, speed;
+
+ /* if already autoneg ok, return directly */
+ an_state.bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
+ if (an_state.an_done)
+ return 0;
+
+ hbg_reg_write(priv, HBG_REG_TX_LOCAL_PAGE_ADDR, HBG_STATUS_ENABLE);
+ hbg_reg_write(priv, HBG_REG_16_BIT_CNTR_ADDR, HBG_HW_DEFAULT_16_BIT_CNTR);
+ /* set BIT(1) */
+ hbg_reg_write(priv, HBG_REG_LD_LINK_COUNTER_ADDR, 0x2);
+
+ control.bits = hbg_reg_read(priv, HBG_REG_TRANSMIT_CONTROL_ADDR);
+ control.an_enable = HBG_STATUS_DISABLE;
+ hbg_reg_write(priv, HBG_REG_TRANSMIT_CONTROL_ADDR, control.bits);
+ control.an_enable = HBG_STATUS_ENABLE;
+ hbg_reg_write(priv, HBG_REG_TRANSMIT_CONTROL_ADDR, control.bits);
+
+ wait_time = 0;
+ do {
+ msleep(HBG_HW_AUTONEG_TIMEOUT_STEP);
+ wait_time += HBG_HW_AUTONEG_TIMEOUT_STEP;
+
+ an_state.bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
+ if (an_state.an_done)
+ break;
+ } while (wait_time < HBG_HW_AUTONEG_TIMEOUT_MS);
+
+ if (!an_state.an_done)
+ return -ETIMEDOUT;
+
+ switch (an_state.speed) {
+ case 0x0:
+ speed = HBG_PORT_MODE_SGMII_10M;
+ break;
+ case 0x1:
+ speed = HBG_PORT_MODE_SGMII_100M;
+ break;
+ case 0x2:
+ speed = HBG_PORT_MODE_SGMII_1000M;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return hbg_hw_adjust_link(priv, speed, an_state.duplex);
+}
+
+static void hbg_hw_init_transmit_control(struct hbg_priv *priv)
+{
+ struct hbg_transmit_control control = {
+ .bits = 0,
+ .pad_enalbe = HBG_STATUS_ENABLE,
+ .crc_add = HBG_STATUS_ENABLE,
+ .an_enable = HBG_STATUS_ENABLE,
+ };
+
+ hbg_reg_write(priv, HBG_REG_TRANSMIT_CONTROL_ADDR, control.bits);
+}
+
+static void hbg_hw_init_rx_ctrl(struct hbg_priv *priv)
+{
+ struct hbg_rx_ctrl ctrl = {
+ .bits = 0,
+ .rx_get_addr_mode = HBG_STATUS_ENABLE,
+ .time_inf_en = HBG_STATUS_DISABLE,
+ .rx_align_num = NET_IP_ALIGN,
+ .rxbuf_1st_skip_size = HBG_RX_SKIP1,
+ .rxbuf_1st_skip_size2 = HBG_RX_SKIP2,
+ .port_num = priv->dev_specs.mac_id,
+ };
+
+ hbg_reg_write(priv, HBG_REG_RX_CTRL_ADDR, ctrl.bits);
+}
+
+static void hbg_hw_init_rx_pkt_mode(struct hbg_priv *priv)
+{
+/* 0x0: no parse, 0x1: parse from L2 layer, 0x10: parse from IP layer */
+#define HBG_RX_PKT_PARSE_MODE 0x1
+
+ struct hbg_rx_pkt_mode mode = {
+ .bits = 0,
+ .parse_mode = HBG_RX_PKT_PARSE_MODE,
+ };
+
+ hbg_reg_write(priv, HBG_REG_RX_PKT_MODE_ADDR, mode.bits);
+}
+
+static void hbg_hw_init_recv_ctrl(struct hbg_priv *priv)
+{
+ struct hbg_recv_control ctrl = {
+ .bits = 0,
+ .strip_pad_en = HBG_STATUS_ENABLE,
+ };
+
+ hbg_reg_write(priv, HBG_REG_RECV_CONTROL_ADDR, ctrl.bits);
+}
+
+static void hbg_hw_init_rx_control(struct hbg_priv *priv)
+{
+ hbg_reg_write_field(priv, HBG_REG_RX_BUF_SIZE_ADDR,
+ HBG_REG_RX_BUF_SIZE_M, priv->dev_specs.rx_buf_size);
+ hbg_hw_init_rx_ctrl(priv);
+ hbg_hw_init_rx_pkt_mode(priv);
+ hbg_hw_init_recv_ctrl(priv);
+ hbg_reg_write_bit(priv, HBG_REG_CF_CRC_STRIP_ADDR,
+ HBG_REG_CF_CRC_STRIP_B, HBG_STATUS_DISABLE);
+}
+
+int hbg_hw_init(struct hbg_priv *priv)
+{
+/* little endian or big endian.
+ * ctrl means packet description, data means skb packet data
+ */
+#define HBG_ENDIAN_CTRL_LE_DATA_BE 0x0
+
+ int ret;
+
+ ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_INIT);
+ if (ret)
+ return ret;
+
+ ret = hbg_hw_dev_specs_init(priv);
+ if (ret)
+ return ret;
+
+ hbg_reg_write_field(priv, HBG_REG_BUS_CTRL_ADDR,
+ HBG_REG_BUS_CTRL_ENDIAN_M,
+ HBG_ENDIAN_CTRL_LE_DATA_BE);
+ hbg_reg_write_bit(priv, HBG_REG_MODE_CHANGE_EN_ADDR,
+ HBG_REG_MODE_CHANGE_EN_B, HBG_STATUS_ENABLE);
+
+ hbg_hw_init_rx_control(priv);
+ hbg_hw_init_transmit_control(priv);
+ return 0;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 61c6db948364..79a529d7212b 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -57,5 +57,8 @@ static inline void hbg_reg_write64(struct hbg_priv *priv, u32 reg_addr,
int hbg_hw_event_notify(struct hbg_priv *priv, enum hbg_hw_event_type event_type);
int hbg_hw_dev_specs_init(struct hbg_priv *priv);
+int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
+int hbg_hw_sgmii_autoneg(struct hbg_priv *priv);
+int hbg_hw_init(struct hbg_priv *pri);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index df0fc6a1059b..940e1eef70a4 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -7,17 +7,39 @@
#include "hbg_common.h"
#include "hbg_hw.h"
#include "hbg_main.h"
+#include "hbg_mdio.h"
+
+static const u32 hbg_mode_ability[] = {
+ ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+ ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+ ETHTOOL_LINK_MODE_Autoneg_BIT,
+ ETHTOOL_LINK_MODE_TP_BIT,
+};
+
+static int hbg_mac_init(struct hbg_priv *priv)
+{
+ struct hbg_mac *mac = &priv->mac;
+ u32 i;
+
+ for (i = 0; i < ARRAY_SIZE(hbg_mode_ability); i++)
+ linkmode_set_bit(hbg_mode_ability[i], mac->supported);
+
+ return hbg_mdio_init(priv);
+}
static int hbg_init(struct net_device *netdev)
{
struct hbg_priv *priv = netdev_priv(netdev);
int ret;
- ret = hbg_hw_event_notify(priv, HBG_HW_EVENT_INIT);
+ ret = hbg_hw_init(priv);
if (ret)
return ret;
- return hbg_hw_dev_specs_init(priv);
+ return hbg_mac_init(priv);
}
static int hbg_pci_init(struct pci_dev *pdev)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
new file mode 100644
index 000000000000..2d6f3ad10087
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/phy.h>
+#include "hbg_common.h"
+#include "hbg_hw.h"
+#include "hbg_mdio.h"
+#include "hbg_reg.h"
+
+#define HBG_MAC_GET_PRIV(mac) ((mac)->mdio_bus->priv)
+#define HBG_MII_BUS_GET_MAC(bus) (&((struct hbg_priv *)(bus)->priv)->mac)
+
+#define HBG_MDIO_FREQUENCE_2_5M 0x1
+#define HBG_MDIO_C22_MODE 0x1
+
+#define HBG_MDIO_C22_REG_WRITE 0x1
+#define HBG_MDIO_C22_REG_READ 0x2
+
+static void hbg_mdio_set_command(struct hbg_mac *mac,
+ struct hbg_mdio_command *command)
+{
+ hbg_reg_write(HBG_MAC_GET_PRIV(mac), HBG_REG_MDIO_COMMAND_REG_ADDR,
+ command->bits);
+}
+
+static void hbg_mdio_get_command(struct hbg_mac *mac,
+ struct hbg_mdio_command *command)
+{
+ command->bits = hbg_reg_read(HBG_MAC_GET_PRIV(mac),
+ HBG_REG_MDIO_COMMAND_REG_ADDR);
+}
+
+static void hbg_mdio_set_wdata_reg(struct hbg_mac *mac, u16 wdata_value)
+{
+ hbg_reg_write_field(HBG_MAC_GET_PRIV(mac), HBG_REG_MDIO_WDATA_REG_ADDR,
+ HBG_REG_MDIO_WDATA_M, wdata_value);
+}
+
+static u32 hbg_mdio_get_rdata_reg(struct hbg_mac *mac)
+{
+ return hbg_reg_read_field(HBG_MAC_GET_PRIV(mac),
+ HBG_REG_MDIO_RDATA_REG_ADDR,
+ HBG_REG_MDIO_WDATA_M);
+}
+
+static int hbg_mdio_wait_ready(struct hbg_mac *mac)
+{
+#define HBG_MDIO_OP_TIMEOUT_MS 1000
+#define HBG_MDIO_OP_INTERVAL_MS 5
+
+ struct hbg_mdio_command command;
+ u32 timeout = 0;
+
+ do {
+ hbg_mdio_get_command(mac, &command);
+ if (command.mdio_start == 0)
+ /* the operation is complete */
+ return 0;
+
+ msleep(HBG_MDIO_OP_INTERVAL_MS);
+ timeout += HBG_MDIO_OP_INTERVAL_MS;
+ } while (timeout < HBG_MDIO_OP_TIMEOUT_MS);
+
+ return -ETIMEDOUT;
+}
+
+static int hbg_mdio_check_op_status(struct hbg_mac *mac)
+{
+ if (hbg_reg_read(HBG_MAC_GET_PRIV(mac), HBG_REG_MDIO_STA_REG_ADDR))
+ return -EBUSY;
+
+ return 0;
+}
+
+static int hbg_mdio_check_send_result(struct hbg_mac *mac)
+{
+ int ret;
+
+ ret = hbg_mdio_wait_ready(mac);
+ if (ret)
+ return ret;
+
+ return hbg_mdio_check_op_status(mac);
+}
+
+static int hbg_mdio_cmd_send(struct hbg_mac *mac, u32 prt_addr, u32 dev_addr,
+ u32 type, u32 op_code)
+{
+ struct hbg_mdio_command mdio_cmd;
+
+ hbg_mdio_get_command(mac, &mdio_cmd);
+ mdio_cmd.mdio_st = type;
+ /* if auto scan enabled, this value need fix to 0 */
+ mdio_cmd.mdio_start = 0x1;
+ mdio_cmd.mdio_op = op_code;
+ mdio_cmd.mdio_prtad = prt_addr;
+ mdio_cmd.mdio_devad = dev_addr;
+ hbg_mdio_set_command(mac, &mdio_cmd);
+
+ /* wait operation complete and check the result */
+ return hbg_mdio_check_send_result(mac);
+}
+
+static int hbg_mdio_read22(struct mii_bus *bus, int phy_addr, int regnum)
+{
+ struct hbg_mac *mac = HBG_MII_BUS_GET_MAC(bus);
+ int ret;
+
+ ret = hbg_mdio_check_op_status(mac);
+ if (ret)
+ return ret;
+
+ ret = hbg_mdio_cmd_send(mac, phy_addr, regnum, HBG_MDIO_C22_MODE,
+ HBG_MDIO_C22_REG_READ);
+ if (ret)
+ return ret;
+
+ return hbg_mdio_get_rdata_reg(mac);
+}
+
+static int hbg_mdio_write22(struct mii_bus *bus, int phy_addr, int regnum,
+ u16 val)
+{
+ struct hbg_mac *mac = HBG_MII_BUS_GET_MAC(bus);
+ int ret;
+
+ ret = hbg_mdio_check_op_status(mac);
+ if (ret)
+ return ret;
+
+ hbg_mdio_set_wdata_reg(mac, val);
+ return hbg_mdio_cmd_send(mac, phy_addr, regnum, HBG_MDIO_C22_MODE,
+ HBG_MDIO_C22_REG_WRITE);
+}
+
+static int hbg_mdio_init_hw(struct hbg_mac *mac)
+{
+ u32 freq = HBG_MDIO_FREQUENCE_2_5M;
+ struct hbg_mdio_command cmd;
+
+ cmd.bits = 0;
+ cmd.mdio_auto_scan = HBG_STATUS_DISABLE;
+ cmd.mdio_st = HBG_MDIO_C22_MODE;
+
+ /* freq use two bits, which are stored in clk_sel and clk_sel_exp */
+ cmd.mdio_clk_sel = freq & 0x1;
+ cmd.mdio_clk_sel_exp = (((u32)freq) >> 1) & 0x1;
+
+ hbg_mdio_set_command(mac, &cmd);
+ return 0;
+}
+
+static void hbg_phy_adjust_link(struct net_device *netdev)
+{
+ struct hbg_priv *priv = netdev_priv(netdev);
+ struct phy_device *phydev = priv->mac.phydev;
+ u32 speed;
+
+ switch (phydev->speed) {
+ case SPEED_10:
+ speed = HBG_PORT_MODE_SGMII_10M;
+ break;
+ case SPEED_100:
+ speed = HBG_PORT_MODE_SGMII_100M;
+ break;
+ case SPEED_1000:
+ speed = HBG_PORT_MODE_SGMII_1000M;
+ break;
+ default:
+ return;
+ }
+
+ priv->mac.autoneg = phydev->autoneg;
+ hbg_hw_adjust_link(priv, speed, phydev->duplex);
+}
+
+static void hbg_phy_disconnect(void *data)
+{
+ phy_disconnect((struct phy_device *)data);
+}
+
+static int hbg_phy_connect(struct hbg_priv *priv)
+{
+ struct phy_device *phydev = priv->mac.phydev;
+ struct hbg_mac *mac = &priv->mac;
+ int ret;
+
+ linkmode_copy(phydev->supported, mac->supported);
+ linkmode_copy(phydev->advertising, mac->supported);
+
+ phy_connect_direct(priv->netdev, mac->phydev, hbg_phy_adjust_link,
+ PHY_INTERFACE_MODE_SGMII);
+ ret = devm_add_action_or_reset(&priv->pdev->dev,
+ hbg_phy_disconnect, mac->phydev);
+ if (ret)
+ return ret;
+
+ phy_attached_info(phydev);
+ return 0;
+}
+
+/* include phy link and mac link */
+u32 hbg_get_link_status(struct hbg_priv *priv)
+{
+ struct phy_device *phydev = priv->mac.phydev;
+ int ret;
+
+ if (!phydev)
+ return HBG_LINK_DOWN;
+
+ phy_read_status(phydev);
+ if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
+ !phydev->link)
+ return HBG_LINK_DOWN;
+
+ ret = hbg_hw_sgmii_autoneg(priv);
+ if (ret)
+ return HBG_LINK_DOWN;
+
+ return HBG_LINK_UP;
+}
+
+void hbg_phy_start(struct hbg_priv *priv)
+{
+ if (!priv->mac.phydev)
+ return;
+
+ phy_start(priv->mac.phydev);
+}
+
+void hbg_phy_stop(struct hbg_priv *priv)
+{
+ if (!priv->mac.phydev)
+ return;
+
+ phy_stop(priv->mac.phydev);
+}
+
+int hbg_mdio_init(struct hbg_priv *priv)
+{
+ struct hbg_mac *mac = &priv->mac;
+ struct phy_device *phydev;
+ struct mii_bus *mdio_bus;
+ int ret;
+
+ mac->phy_addr = priv->dev_specs.phy_addr;
+ mdio_bus = devm_mdiobus_alloc(&priv->pdev->dev);
+ if (!mdio_bus)
+ return dev_err_probe(&priv->pdev->dev, -ENOMEM,
+ "failed to alloc MDIO bus\n");
+
+ mdio_bus->parent = &priv->pdev->dev;
+ mdio_bus->priv = priv;
+ mdio_bus->phy_mask = ~(1 << mac->phy_addr);
+ mdio_bus->name = "hibmcge mii bus";
+ mac->mdio_bus = mdio_bus;
+
+ mdio_bus->read = hbg_mdio_read22;
+ mdio_bus->write = hbg_mdio_write22;
+ snprintf(mdio_bus->id, MII_BUS_ID_SIZE, "%s-%s", "mii",
+ dev_name(&priv->pdev->dev));
+
+ ret = devm_mdiobus_register(&priv->pdev->dev, mdio_bus);
+ if (ret)
+ return dev_err_probe(&priv->pdev->dev, ret,
+ "failed to register MDIO bus\n");
+
+ phydev = mdiobus_get_phy(mdio_bus, mac->phy_addr);
+ if (!phydev)
+ return dev_err_probe(&priv->pdev->dev, -EIO,
+ "failed to get phy device\n");
+
+ mac->phydev = phydev;
+ hbg_mdio_init_hw(mac);
+ return hbg_phy_connect(priv);
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.h
new file mode 100644
index 000000000000..bca38c7fe14b
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_MDIO_H
+#define __HBG_MDIO_H
+
+#include "hbg_common.h"
+
+int hbg_mdio_init(struct hbg_priv *priv);
+u32 hbg_get_link_status(struct hbg_priv *priv);
+void hbg_phy_start(struct hbg_priv *priv);
+void hbg_phy_stop(struct hbg_priv *priv);
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index 4c34516e0c89..f56893424da2 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -4,6 +4,8 @@
#ifndef __HBG_REG_H
#define __HBG_REG_H
+#include "hbg_reg_union.h"
+
/* DEV SPEC */
#define HBG_REG_SPEC_VALID_ADDR 0x0000
#define HBG_REG_EVENT_REQ_ADDR 0x0004
@@ -16,4 +18,48 @@
#define HBG_REG_RX_FIFO_NUM_ADDR 0x0034
#define HBG_REG_VLAN_LAYERS_ADDR 0x0038
+/* MDIO */
+#define HBG_REG_MDIO_BASE 0x8000
+#define HBG_REG_MDIO_COMMAND_REG_ADDR (HBG_REG_MDIO_BASE + 0x0000)
+#define HBG_REG_MDIO_ADDR_REG_ADDR (HBG_REG_MDIO_BASE + 0x0004)
+#define HBG_REG_MDIO_WDATA_REG_ADDR (HBG_REG_MDIO_BASE + 0x0008)
+#define HBG_REG_MDIO_RDATA_REG_ADDR (HBG_REG_MDIO_BASE + 0x000C)
+#define HBG_REG_MDIO_STA_REG_ADDR (HBG_REG_MDIO_BASE + 0x0010)
+
+/* GMAC */
+#define HBG_REG_SGMII_BASE 0x10000
+#define HBG_REG_DUPLEX_TYPE_ADDR (HBG_REG_SGMII_BASE + 0x0008)
+#define HBG_REG_PORT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x0040)
+#define HBG_REG_AN_NEG_STATE_ADDR (HBG_REG_SGMII_BASE + 0x0058)
+#define HBG_REG_TX_LOCAL_PAGE_ADDR (HBG_REG_SGMII_BASE + 0x005C)
+#define HBG_REG_TRANSMIT_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x0060)
+#define HBG_REG_CF_CRC_STRIP_ADDR (HBG_REG_SGMII_BASE + 0x01B0)
+#define HBG_REG_MODE_CHANGE_EN_ADDR (HBG_REG_SGMII_BASE + 0x01B4)
+#define HBG_REG_16_BIT_CNTR_ADDR (HBG_REG_SGMII_BASE + 0x01CC)
+#define HBG_REG_LD_LINK_COUNTER_ADDR (HBG_REG_SGMII_BASE + 0x01D0)
+#define HBG_REG_RECV_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x01E0)
+
+/* PCU */
+#define HBG_REG_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
+#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
+#define HBG_REG_RX_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04F0)
+#define HBG_REG_RX_PKT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x04F4)
+
+/* mask */
+#define HBG_REG_PORT_MODE_M GENMASK(3, 0)
+#define HBG_REG_MODE_CHANGE_EN_B BIT(0)
+#define HBG_REG_RX_BUF_SIZE_M GENMASK(15, 0)
+#define HBG_REG_BUS_CTRL_ENDIAN_M GENMASK(2, 1)
+#define HBG_REG_DUPLEX_B BIT(0)
+#define HBG_REG_CF_CRC_STRIP_B BIT(1)
+#define HBG_REG_MDIO_WDATA_M GENMASK(15, 0)
+#define HBG_REG_IND_INTR_MASK_B BIT(0)
+
+enum hbg_port_mode {
+ /* 0x0 ~ 0x5 are reserved */
+ HBG_PORT_MODE_SGMII_10M = 0x6,
+ HBG_PORT_MODE_SGMII_100M = 0x7,
+ HBG_PORT_MODE_SGMII_1000M = 0x8,
+};
+
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
new file mode 100644
index 000000000000..fc6cad15438d
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_REG_UNION_H
+#define __HBG_REG_UNION_H
+
+struct hbg_rx_ctrl {
+ union {
+ struct {
+ u32 rxbuf_1st_skip_size2 : 4;
+ u32 cache_line_l : 3;
+ u32 rx_cfg_req_en : 1;
+ u32 cache_line_h : 2;
+ u32 addr_mode : 2;
+ u32 rx_get_addr_mode : 1;
+ u32 port_num : 4;
+ u32 rx_align_num : 2;
+ u32 pool_num : 4;
+ u32 time_inf_en : 1;
+ u32 rxbuf_no_1st_skip_size : 4;
+ u32 rxbuf_1st_skip_size : 4;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_rx_pkt_mode {
+ union {
+ struct {
+ u32 gen_id : 8;
+ u32 rsv_0 : 4;
+ u32 match_offset : 9;
+ u32 parse_mode : 2;
+ u32 skip_len : 7;
+ u32 rsv_1 : 1;
+ u32 instr_head_mode : 1;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_transmit_control {
+ union {
+ struct {
+ u32 rsv_0 : 5;
+ u32 an_enable : 1;
+ u32 crc_add : 1;
+ u32 pad_enalbe : 1;
+ u32 rsv_1 : 24;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_mdio_command {
+ union {
+ struct {
+ u32 mdio_devad : 5;
+ u32 mdio_prtad :5;
+ u32 mdio_op : 2;
+ u32 mdio_st : 2;
+ u32 mdio_start : 1;
+ u32 mdio_clk_sel : 1;
+ u32 mdio_auto_scan : 1;
+ u32 mdio_clk_sel_exp : 1;
+ u32 rev : 14;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_an_state {
+ union {
+ struct {
+ u32 reserved_0 : 5;
+ /* SerDes autoneg */
+ u32 half_duplex : 1;
+ /* SerDes autoneg */
+ u32 full_duplex : 1;
+ /* SerDes autoneg */
+ u32 support_pause_frame : 2;
+ u32 reserved_1 : 1;
+ /* SerDes autoneg, b10: 1000M; b01: 100M; b00: 10M */
+ u32 speed : 2;
+ /* SGMII autoneg, 0: half duplex; 1: full duplex */
+ u32 duplex : 1;
+ u32 rf2 : 1;
+ u32 reserved_2 : 1;
+ u32 link_ok : 1;
+ u32 reserved_3 : 4;
+ u32 rx_sync_ok : 1;
+ u32 an_done : 1;
+ u32 reserved_4 : 10;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_recv_control {
+ union {
+ struct {
+ u32 reserved_0 : 3;
+ u32 strip_pad_en : 1;
+ /* short frame transparent transmission enable */
+ u32 runt_pkt_en : 1;
+ u32 reserved_1 : 27;
+ };
+ u32 bits;
+ };
+};
+
+#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module
2024-07-31 9:42 ` [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module Jijie Shao
@ 2024-08-01 0:42 ` Andrew Lunn
2024-08-01 9:04 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 0:42 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> +int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
> +{
> + if (speed != HBG_PORT_MODE_SGMII_10M &&
> + speed != HBG_PORT_MODE_SGMII_100M &&
> + speed != HBG_PORT_MODE_SGMII_1000M)
> + return -EOPNOTSUPP;
> +
> + if (duplex != DUPLEX_FULL && duplex != DUPLEX_HALF)
> + return -EOPNOTSUPP;
> +
> + if (speed == HBG_PORT_MODE_SGMII_1000M && duplex == DUPLEX_HALF)
> + return -EOPNOTSUPP;
If you tell phylib you don't support 1G/Half, this will not happen.
> +/* sgmii autoneg always enable */
> +int hbg_hw_sgmii_autoneg(struct hbg_priv *priv)
> +{
> + wait_time = 0;
> + do {
> + msleep(HBG_HW_AUTONEG_TIMEOUT_STEP);
> + wait_time += HBG_HW_AUTONEG_TIMEOUT_STEP;
> +
> + an_state.bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
> + if (an_state.an_done)
> + break;
> + } while (wait_time < HBG_HW_AUTONEG_TIMEOUT_MS);
include/linux/iopoll.h
> +static const u32 hbg_mode_ability[] = {
> + ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_100baseT_Half_BIT,
> + ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_10baseT_Half_BIT,
> + ETHTOOL_LINK_MODE_Autoneg_BIT,
> + ETHTOOL_LINK_MODE_TP_BIT,
> +};
> +
> +static int hbg_mac_init(struct hbg_priv *priv)
> +{
> + struct hbg_mac *mac = &priv->mac;
> + u32 i;
> +
> + for (i = 0; i < ARRAY_SIZE(hbg_mode_ability); i++)
> + linkmode_set_bit(hbg_mode_ability[i], mac->supported);
Humm, odd. Where is this leading...
> +#define HBG_MDIO_FREQUENCE_2_5M 0x1
I assume it supports other frequencies. You might want to implement
the DT property 'clock-frequency'. Many modern PHY will work faster
than 2.5Mhz.
> +static int hbg_phy_connect(struct hbg_priv *priv)
> +{
> + struct phy_device *phydev = priv->mac.phydev;
> + struct hbg_mac *mac = &priv->mac;
> + int ret;
> +
> + linkmode_copy(phydev->supported, mac->supported);
> + linkmode_copy(phydev->advertising, mac->supported);
And here it is. Why? Do you see any other MAC driver doing this?
What you probably want is:
phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
which is what other MAC drivers do.
> +
> + phy_connect_direct(priv->netdev, mac->phydev, hbg_phy_adjust_link,
> + PHY_INTERFACE_MODE_SGMII);
> + ret = devm_add_action_or_reset(&priv->pdev->dev,
> + hbg_phy_disconnect, mac->phydev);
> + if (ret)
> + return ret;
> +
> + phy_attached_info(phydev);
> + return 0;
> +}
> +
> +/* include phy link and mac link */
> +u32 hbg_get_link_status(struct hbg_priv *priv)
> +{
> + struct phy_device *phydev = priv->mac.phydev;
> + int ret;
> +
> + if (!phydev)
> + return HBG_LINK_DOWN;
> +
> + phy_read_status(phydev);
> + if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
> + !phydev->link)
> + return HBG_LINK_DOWN;
> +
> + ret = hbg_hw_sgmii_autoneg(priv);
> + if (ret)
> + return HBG_LINK_DOWN;
> +
> + return HBG_LINK_UP;
> +}
There should not be any need for this. So why do you have it?
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module
2024-08-01 0:42 ` Andrew Lunn
@ 2024-08-01 9:04 ` Jijie Shao
2024-08-01 12:07 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 9:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Thanks for reviewing
on 2024/8/1 8:42, Andrew Lunn wrote:
>> +int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
>> +{
>> + if (speed != HBG_PORT_MODE_SGMII_10M &&
>> + speed != HBG_PORT_MODE_SGMII_100M &&
>> + speed != HBG_PORT_MODE_SGMII_1000M)
>> + return -EOPNOTSUPP;
>> +
>> + if (duplex != DUPLEX_FULL && duplex != DUPLEX_HALF)
>> + return -EOPNOTSUPP;
>> +
>> + if (speed == HBG_PORT_MODE_SGMII_1000M && duplex == DUPLEX_HALF)
>> + return -EOPNOTSUPP;
> If you tell phylib you don't support 1G/Half, this will not happen.
ok, I will delete it in V2
>
>> +/* sgmii autoneg always enable */
>> +int hbg_hw_sgmii_autoneg(struct hbg_priv *priv)
>> +{
>> + wait_time = 0;
>> + do {
>> + msleep(HBG_HW_AUTONEG_TIMEOUT_STEP);
>> + wait_time += HBG_HW_AUTONEG_TIMEOUT_STEP;
>> +
>> + an_state.bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
>> + if (an_state.an_done)
>> + break;
>> + } while (wait_time < HBG_HW_AUTONEG_TIMEOUT_MS);
> include/linux/iopoll.h
Thanks for the guide. These macros will simplify the code.
>
>> +static const u32 hbg_mode_ability[] = {
>> + ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
>> + ETHTOOL_LINK_MODE_100baseT_Full_BIT,
>> + ETHTOOL_LINK_MODE_100baseT_Half_BIT,
>> + ETHTOOL_LINK_MODE_10baseT_Full_BIT,
>> + ETHTOOL_LINK_MODE_10baseT_Half_BIT,
>> + ETHTOOL_LINK_MODE_Autoneg_BIT,
>> + ETHTOOL_LINK_MODE_TP_BIT,
>> +};
>> +
>> +static int hbg_mac_init(struct hbg_priv *priv)
>> +{
>> + struct hbg_mac *mac = &priv->mac;
>> + u32 i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(hbg_mode_ability); i++)
>> + linkmode_set_bit(hbg_mode_ability[i], mac->supported);
> Humm, odd. Where is this leading...
>
>> +#define HBG_MDIO_FREQUENCE_2_5M 0x1
> I assume it supports other frequencies. You might want to implement
> the DT property 'clock-frequency'. Many modern PHY will work faster
> than 2.5Mhz.
it's a good idea,
>
>> +static int hbg_phy_connect(struct hbg_priv *priv)
>> +{
>> + struct phy_device *phydev = priv->mac.phydev;
>> + struct hbg_mac *mac = &priv->mac;
>> + int ret;
>> +
>> + linkmode_copy(phydev->supported, mac->supported);
>> + linkmode_copy(phydev->advertising, mac->supported);
> And here it is. Why? Do you see any other MAC driver doing this?
>
> What you probably want is:
>
> phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
>
> which is what other MAC drivers do.
Yeah， using phy_remove_link_mode, the hbg_mode_ability[] and linkmode_set_bit
of the previous code can be removed together.
Thank you.
>
>> +
>> + phy_connect_direct(priv->netdev, mac->phydev, hbg_phy_adjust_link,
>> + PHY_INTERFACE_MODE_SGMII);
>> + ret = devm_add_action_or_reset(&priv->pdev->dev,
>> + hbg_phy_disconnect, mac->phydev);
>> + if (ret)
>> + return ret;
>> +
>> + phy_attached_info(phydev);
>> + return 0;
>> +}
>> +
>> +/* include phy link and mac link */
>> +u32 hbg_get_link_status(struct hbg_priv *priv)
>> +{
>> + struct phy_device *phydev = priv->mac.phydev;
>> + int ret;
>> +
>> + if (!phydev)
>> + return HBG_LINK_DOWN;
>> +
>> + phy_read_status(phydev);
>> + if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
>> + !phydev->link)
>> + return HBG_LINK_DOWN;
>> +
>> + ret = hbg_hw_sgmii_autoneg(priv);
>> + if (ret)
>> + return HBG_LINK_DOWN;
>> +
>> + return HBG_LINK_UP;
>> +}
> There should not be any need for this. So why do you have it?
I'll move this to another patch where it's more appropriate.
>
> Andrew
Thanks, Jijie
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module
2024-08-01 9:04 ` Jijie Shao
@ 2024-08-01 12:07 ` Andrew Lunn
0 siblings, 0 replies; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 12:07 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> > > +/* include phy link and mac link */
> > > +u32 hbg_get_link_status(struct hbg_priv *priv)
> > > +{
> > > + struct phy_device *phydev = priv->mac.phydev;
> > > + int ret;
> > > +
> > > + if (!phydev)
> > > + return HBG_LINK_DOWN;
> > > +
> > > + phy_read_status(phydev);
> > > + if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
> > > + !phydev->link)
> > > + return HBG_LINK_DOWN;
> > > +
> > > + ret = hbg_hw_sgmii_autoneg(priv);
> > > + if (ret)
> > > + return HBG_LINK_DOWN;
> > > +
> > > + return HBG_LINK_UP;
> > > +}
> > There should not be any need for this. So why do you have it?
>
> I'll move this to another patch where it's more appropriate.
That does not explain why it is needed. Generally, phylib tells you if
the link is up, as part of the adjust_link callback. Why do you need
to do something which no other driver does?
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt supported in this module
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (2 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 03/10] net: hibmcge: Add mdio and hardware configuration supported in this module Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-07-31 13:14 ` Joe Damato
2024-08-05 12:57 ` Simon Horman
2024-07-31 9:42 ` [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions Jijie Shao
` (5 subsequent siblings)
9 siblings, 2 replies; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
The driver supports four interrupts: TX interrupt, RX interrupt,
mdio interrupt, and error interrupt.
Actually, the driver does not use the mdio interrupt.
Therefore, the driver does not request the mdio interrupt.
The error interrupt distinguishes different error information
by using different masks. To distinguish different errors,
the statistics count is added for each error.
To ensure the consistency of the code process, masks are added for the
TX interrupt and RX interrupt.
This patch implements interrupt request and free, and provides a
unified entry for the interrupt handler function. However,
the specific interrupt handler function of each interrupt
is not implemented currently.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 20 ++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 59 ++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 8 +
.../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 189 ++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_irq.h | 13 ++
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 9 +
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 34 ++++
.../hisilicon/hibmcge/hbg_reg_union.h | 60 ++++++
8 files changed, 392 insertions(+)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 364aab4d61b9..6a3e647cd27c 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -51,6 +51,25 @@ struct hbg_dev_specs {
u32 rx_buf_size;
};
+struct hbg_irq_info {
+ const char *name;
+ enum hbg_irq_mask mask;
+ u64 count;
+};
+
+struct hbg_irq {
+ u32 id;
+ char name[32];
+};
+
+struct hbg_vector {
+ struct hbg_irq *irqs;
+ u32 irq_count;
+
+ struct hbg_irq_info *info_array;
+ u32 info_array_len;
+};
+
struct hbg_mac {
struct mii_bus *mdio_bus;
struct phy_device *phydev;
@@ -71,6 +90,7 @@ struct hbg_priv {
struct hbg_dev_specs dev_specs;
unsigned long state;
struct hbg_mac mac;
+ struct hbg_vector vectors;
};
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index b2465ba06cee..d9bed7cc7790 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -71,6 +71,65 @@ int hbg_hw_dev_specs_init(struct hbg_priv *priv)
return 0;
}
+void hbg_hw_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status)
+{
+ status->bits = hbg_reg_read(priv, HBG_REG_CF_INTRPT_STAT_ADDR);
+}
+
+void hbg_hw_set_err_intr_clear(struct hbg_priv *priv, struct hbg_intr_status *status)
+{
+ hbg_reg_write(priv, HBG_REG_CF_INTRPT_CLR_ADDR, status->bits);
+}
+
+void hbg_hw_set_err_intr_mask(struct hbg_priv *priv, const struct hbg_intr_mask *msk)
+{
+ hbg_reg_write(priv, HBG_REG_CF_INTRPT_MSK_ADDR, msk->bits);
+}
+
+void hbg_hw_get_err_intr_mask(struct hbg_priv *priv, struct hbg_intr_mask *msk)
+{
+ msk->bits = hbg_reg_read(priv, HBG_REG_CF_INTRPT_MSK_ADDR);
+}
+
+void hbg_hw_set_txrx_intr_enable(struct hbg_priv *priv,
+ enum hbg_dir dir, bool enabld)
+{
+ u32 addr = (dir == HBG_DIR_TX) ? HBG_REG_CF_IND_TXINT_MSK_ADDR :
+ HBG_REG_CF_IND_RXINT_MSK_ADDR;
+
+ hbg_reg_write_bit(priv, addr, HBG_REG_IND_INTR_MASK_B, enabld);
+}
+
+bool hbg_hw_txrx_intr_is_enabled(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ u32 addr = (dir == HBG_DIR_TX) ? HBG_REG_CF_IND_TXINT_MSK_ADDR :
+ HBG_REG_CF_IND_RXINT_MSK_ADDR;
+
+ return hbg_reg_read_bit(priv, addr, HBG_REG_IND_INTR_MASK_B);
+}
+
+void hbg_hw_set_txrx_intr_clear(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ u32 addr = (dir == HBG_DIR_TX) ? HBG_REG_CF_IND_TXINT_CLR_ADDR :
+ HBG_REG_CF_IND_RXINT_CLR_ADDR;
+
+ hbg_reg_write_bit(priv, addr, HBG_REG_IND_INTR_MASK_B, 0x1);
+}
+
+void hbg_hw_get_txrx_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status)
+{
+ status->tx = hbg_reg_read_bit(priv, HBG_REG_CF_IND_TXINT_STAT_ADDR,
+ HBG_REG_IND_INTR_MASK_B);
+
+ status->rx = hbg_reg_read_bit(priv, HBG_REG_CF_IND_RXINT_STAT_ADDR,
+ HBG_REG_IND_INTR_MASK_B);
+}
+
+void hbg_hw_get_auto_neg_state(struct hbg_priv *priv, struct hbg_an_state *state)
+{
+ state->bits = hbg_reg_read(priv, HBG_REG_AN_NEG_STATE_ADDR);
+}
+
int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
{
if (speed != HBG_PORT_MODE_SGMII_10M &&
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 79a529d7212b..e2a08dc5d883 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -57,6 +57,14 @@ static inline void hbg_reg_write64(struct hbg_priv *priv, u32 reg_addr,
int hbg_hw_event_notify(struct hbg_priv *priv, enum hbg_hw_event_type event_type);
int hbg_hw_dev_specs_init(struct hbg_priv *priv);
+void hbg_hw_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status);
+void hbg_hw_get_err_intr_mask(struct hbg_priv *priv, struct hbg_intr_mask *msk);
+void hbg_hw_set_err_intr_mask(struct hbg_priv *priv, const struct hbg_intr_mask *msk);
+void hbg_hw_set_err_intr_clear(struct hbg_priv *priv, struct hbg_intr_status *status);
+void hbg_hw_set_txrx_intr_enable(struct hbg_priv *priv, enum hbg_dir dir, bool enabld);
+bool hbg_hw_txrx_intr_is_enabled(struct hbg_priv *priv, enum hbg_dir dir);
+void hbg_hw_set_txrx_intr_clear(struct hbg_priv *priv, enum hbg_dir dir);
+void hbg_hw_get_txrx_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status);
int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
int hbg_hw_sgmii_autoneg(struct hbg_priv *priv);
int hbg_hw_init(struct hbg_priv *pri);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
new file mode 100644
index 000000000000..5e90e72d7f24
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include "hbg_irq.h"
+#include "hbg_hw.h"
+#include "hbg_main.h"
+
+#define HBG_VECTOR_NUM 4
+
+static struct hbg_irq_info hbg_irqs[] = {
+ { "TX", HBG_IRQ_TX, 0 },
+ { "RX", HBG_IRQ_RX, 0 },
+ { "RX_BUF_AVL", HBG_IRQ_BUF_AVL, 0 },
+ { "MAC_MII_FIFO_ERR", HBG_IRQ_MAC_MII_FIFO_ERR, 0 },
+ { "MAC_PCS_RX_FIFO_ERR", HBG_IRQ_MAC_PCS_RX_FIFO_ERR, 0 },
+ { "MAC_PCS_TX_FIFO_ERR", HBG_IRQ_MAC_PCS_TX_FIFO_ERR, 0 },
+ { "MAC_APP_RX_FIFO_ERR", HBG_IRQ_MAC_APP_RX_FIFO_ERR, 0 },
+ { "MAC_APP_TX_FIFO_ERR", HBG_IRQ_MAC_APP_TX_FIFO_ERR, 0 },
+ { "SRAM_PARITY_ERR", HBG_IRQ_SRAM_PARITY_ERR, 0 },
+ { "TX_AHB_ERR", HBG_IRQ_TX_AHB_ERR, 0 },
+ { "REL_BUF_ERR", HBG_IRQ_REL_BUF_ERR, 0 },
+ { "TXCFG_AVL", HBG_IRQ_TXCFG_AVL, 0 },
+ { "TX_DROP", HBG_IRQ_TX_DROP, 0 },
+ { "RX_DROP", HBG_IRQ_RX_DROP, 0 },
+ { "RX_AHB_ERR", HBG_IRQ_RX_AHB_ERR, 0 },
+ { "MAC_FIFO_ERR", HBG_IRQ_MAC_FIFO_ERR, 0 },
+ { "RBREQ_ERR", HBG_IRQ_RBREQ_ERR, 0 },
+ { "WE_ERR", HBG_IRQ_WE_ERR, 0 },
+};
+
+void hbg_irq_enable(struct hbg_priv *priv, enum hbg_irq_mask mask, bool enable)
+{
+ struct hbg_intr_mask intr_mask;
+
+ if (mask == HBG_IRQ_TX)
+ return hbg_hw_set_txrx_intr_enable(priv, HBG_DIR_TX, enable);
+
+ if (mask == HBG_IRQ_RX)
+ return hbg_hw_set_txrx_intr_enable(priv, HBG_DIR_RX, enable);
+
+ hbg_hw_get_err_intr_mask(priv, &intr_mask);
+ if (enable)
+ intr_mask.bits = intr_mask.bits | mask;
+ else
+ intr_mask.bits = intr_mask.bits & (~mask);
+
+ hbg_hw_set_err_intr_mask(priv, &intr_mask);
+}
+
+bool hbg_irq_is_enabled(struct hbg_priv *priv, enum hbg_irq_mask mask)
+{
+ struct hbg_intr_mask intr_mask;
+
+ if (mask == HBG_IRQ_TX)
+ return hbg_hw_txrx_intr_is_enabled(priv, HBG_DIR_TX);
+
+ if (mask == HBG_IRQ_RX)
+ return hbg_hw_txrx_intr_is_enabled(priv, HBG_DIR_RX);
+
+ hbg_hw_get_err_intr_mask(priv, &intr_mask);
+ return intr_mask.bits & mask;
+}
+
+static void hbg_irq_clear_src(struct hbg_priv *priv, enum hbg_irq_mask mask)
+{
+ struct hbg_intr_status intr_clear;
+
+ if (mask == HBG_IRQ_TX)
+ return hbg_hw_set_txrx_intr_clear(priv, HBG_DIR_TX);
+
+ if (mask == HBG_IRQ_RX)
+ return hbg_hw_set_txrx_intr_clear(priv, HBG_DIR_RX);
+
+ intr_clear.bits = mask;
+ hbg_hw_set_err_intr_clear(priv, &intr_clear);
+}
+
+static void hbg_irq_info_handle(struct hbg_priv *priv,
+ struct hbg_irq_info *irq_info)
+{
+ if (!hbg_irq_is_enabled(priv, irq_info->mask))
+ return;
+
+ hbg_irq_enable(priv, irq_info->mask, false);
+ hbg_irq_clear_src(priv, irq_info->mask);
+
+ irq_info->count++;
+ hbg_irq_enable(priv, irq_info->mask, true);
+}
+
+static irqreturn_t hbg_irq_handle(int irq_num, void *p)
+{
+ struct hbg_intr_status intr_status;
+ struct hbg_irq_info *irq_info;
+ struct hbg_priv *priv = p;
+ u32 i;
+
+ hbg_hw_get_err_intr_status(priv, &intr_status);
+ hbg_hw_get_txrx_intr_status(priv, &intr_status);
+
+ for (i = 0; i < priv->vectors.info_array_len; i++) {
+ irq_info = &priv->vectors.info_array[i];
+ if (intr_status.bits & irq_info->mask)
+ hbg_irq_info_handle(priv, irq_info);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static const char *irq_names_map[HBG_VECTOR_NUM] = { "tx", "rx", "err", "mdio" };
+
+int hbg_irq_init(struct hbg_priv *priv)
+{
+ struct hbg_vector *vectors = &priv->vectors;
+ struct hbg_irq *irq;
+ int ret;
+ int i;
+
+ ret = pci_alloc_irq_vectors(priv->pdev, HBG_VECTOR_NUM, HBG_VECTOR_NUM,
+ PCI_IRQ_MSI);
+ if (ret < 0) {
+ dev_err(&priv->pdev->dev,
+ "failed to allocate MSI vectors, vectors = %d\n", ret);
+ return ret;
+ }
+
+ if (ret != HBG_VECTOR_NUM) {
+ dev_err(&priv->pdev->dev,
+ "requested %u MSI, but allocated %d MSI\n",
+ HBG_VECTOR_NUM, ret);
+ ret = -EINVAL;
+ goto free_vectors;
+ }
+
+ vectors->irqs = devm_kcalloc(&priv->pdev->dev, HBG_VECTOR_NUM,
+ sizeof(struct hbg_irq), GFP_KERNEL);
+ if (!vectors->irqs) {
+ ret = -ENOMEM;
+ goto free_vectors;
+ }
+
+ /* mdio irq not request */
+ vectors->irq_count = HBG_VECTOR_NUM - 1;
+ for (i = 0; i < vectors->irq_count; i++) {
+ irq = &vectors->irqs[i];
+ snprintf(irq->name, sizeof(irq->name) - 1, "%s-%s-%s",
+ HBG_DEV_NAME, pci_name(priv->pdev), irq_names_map[i]);
+
+ irq->id = pci_irq_vector(priv->pdev, i);
+ irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
+ ret = request_irq(irq->id, hbg_irq_handle,
+ 0, irq->name, priv);
+ if (ret) {
+ dev_err(&priv->pdev->dev,
+ "failed to requset irq(%d), ret = %d\n",
+ irq->id, ret);
+ goto free_vectors;
+ }
+ }
+
+ vectors->info_array = hbg_irqs;
+ vectors->info_array_len = ARRAY_SIZE(hbg_irqs);
+ return 0;
+
+free_vectors:
+ hbg_irq_uninit(priv);
+ return ret;
+}
+
+void hbg_irq_uninit(void *data)
+{
+ struct hbg_priv *priv = data;
+ struct hbg_vector *vectors;
+ struct hbg_irq *irq;
+ int i;
+
+ vectors = &priv->vectors;
+ for (i = 0; i < vectors->irq_count; i++) {
+ irq = &vectors->irqs[i];
+ if (irq->id > 0) {
+ free_irq(irq->id, priv);
+ irq->id = 0;
+ }
+ }
+
+ pci_free_irq_vectors(priv->pdev);
+ vectors->irqs = NULL;
+ vectors->irq_count = 0;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.h
new file mode 100644
index 000000000000..27c5e2eca99f
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_IRQ_H
+#define __HBG_IRQ_H
+#include "hbg_common.h"
+
+void hbg_irq_enable(struct hbg_priv *priv, enum hbg_irq_mask mask, bool enable);
+bool hbg_irq_is_enabled(struct hbg_priv *priv, enum hbg_irq_mask mask);
+int hbg_irq_init(struct hbg_priv *priv);
+void hbg_irq_uninit(void *data);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 940e1eef70a4..059ea155572f 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/pci.h>
#include "hbg_common.h"
#include "hbg_hw.h"
+#include "hbg_irq.h"
#include "hbg_main.h"
#include "hbg_mdio.h"
@@ -39,6 +40,14 @@ static int hbg_init(struct net_device *netdev)
if (ret)
return ret;
+ ret = hbg_irq_init(priv);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&priv->pdev->dev, hbg_irq_uninit, priv);
+ if (ret)
+ return ret;
+
return hbg_mac_init(priv);
}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index f56893424da2..b422fa990270 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -40,10 +40,19 @@
#define HBG_REG_RECV_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x01E0)
/* PCU */
+#define HBG_REG_CF_INTRPT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x042C)
+#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_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
#define HBG_REG_RX_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04F0)
#define HBG_REG_RX_PKT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x04F4)
+#define HBG_REG_CF_IND_TXINT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x0694)
+#define HBG_REG_CF_IND_TXINT_STAT_ADDR (HBG_REG_SGMII_BASE + 0x0698)
+#define HBG_REG_CF_IND_TXINT_CLR_ADDR (HBG_REG_SGMII_BASE + 0x069C)
+#define HBG_REG_CF_IND_RXINT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x06a0)
+#define HBG_REG_CF_IND_RXINT_STAT_ADDR (HBG_REG_SGMII_BASE + 0x06a4)
+#define HBG_REG_CF_IND_RXINT_CLR_ADDR (HBG_REG_SGMII_BASE + 0x06a8)
/* mask */
#define HBG_REG_PORT_MODE_M GENMASK(3, 0)
@@ -62,4 +71,29 @@ enum hbg_port_mode {
HBG_PORT_MODE_SGMII_1000M = 0x8,
};
+enum hbg_irq_mask {
+ HBG_IRQ_TX = BIT_MASK(0),
+ HBG_IRQ_RX = BIT_MASK(1),
+
+ /* others in err irq */
+ HBG_IRQ_TX_PKG = BIT_MASK(14),
+ HBG_IRQ_MAC_MII_FIFO_ERR = BIT_MASK(15),
+ HBG_IRQ_MAC_PCS_RX_FIFO_ERR = BIT_MASK(16),
+ HBG_IRQ_MAC_PCS_TX_FIFO_ERR = BIT_MASK(17),
+ HBG_IRQ_MAC_APP_RX_FIFO_ERR = BIT_MASK(18),
+ HBG_IRQ_MAC_APP_TX_FIFO_ERR = BIT_MASK(19),
+ HBG_IRQ_SRAM_PARITY_ERR = BIT_MASK(20),
+ HBG_IRQ_TX_AHB_ERR = BIT_MASK(21),
+ HBG_IRQ_BUF_AVL = BIT_MASK(22),
+ HBG_IRQ_REL_BUF_ERR = BIT_MASK(23),
+ HBG_IRQ_TXCFG_AVL = BIT_MASK(24),
+ HBG_IRQ_TX_DROP = BIT_MASK(25),
+ HBG_IRQ_RX_DROP = BIT_MASK(26),
+ HBG_IRQ_RX_PKG = BIT_MASK(27),
+ HBG_IRQ_RX_AHB_ERR = BIT_MASK(28),
+ HBG_IRQ_MAC_FIFO_ERR = BIT_MASK(29),
+ HBG_IRQ_RBREQ_ERR = BIT_MASK(30),
+ HBG_IRQ_WE_ERR = BIT_MASK(31),
+};
+
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
index fc6cad15438d..406eac051c10 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
@@ -109,4 +109,64 @@ struct hbg_recv_control {
};
};
+struct hbg_intr_status {
+ union {
+ struct {
+ u32 tx : 1;
+ u32 rx : 1;
+
+ /* others in err irq */
+ u32 rsv_0 : 12;
+ u32 tx_pkt_cpl : 1;
+ u32 mac_mii_ff_err : 1;
+ u32 mac_pcs_rxff_err : 1;
+ u32 mac_pcs_txff_err : 1;
+ u32 mac_app_rxff_err : 1;
+ u32 mac_app_txff_err : 1;
+ u32 sram_parity_err : 1;
+ u32 tx_ahb_err : 1;
+ u32 buf_avl : 1;
+ u32 rel_err : 1;
+ u32 txcfg_avl : 1;
+ u32 tx_drop : 1;
+ u32 rx_drop : 1;
+ u32 rx_frm : 1;
+ u32 ahb_err : 1;
+ u32 mac_fifo_err : 1;
+ u32 rbreq_err : 1;
+ u32 we_err : 1;
+ };
+ u32 bits;
+ };
+};
+
+struct hbg_intr_mask {
+ union {
+ struct {
+ u32 intr_time : 6;
+ u32 intr_pkt : 6;
+ u32 rsv0 : 2;
+ u32 tx_pkg_intr_msk : 1;
+ u32 mac_mii_ff_err_msk : 1;
+ u32 mac_pcs_rxff_err_msk : 1;
+ u32 mac_pcs_txff_err_msk : 1;
+ u32 mac_app_rxff_err_msk : 1;
+ u32 mac_app_txff_err_msk : 1;
+ u32 sram_parity_err_int_msk : 1;
+ u32 tx_ahb_err_int_msk : 1;
+ u32 buf_avl_msk : 1;
+ u32 rel_buf_msk : 1;
+ u32 txcfg_avl_msk : 1;
+ u32 tx_drop_int_msk : 1;
+ u32 rx_drop_int_msk : 1;
+ u32 rx_pkg_intr_msk : 1;
+ u32 ahb_err_int_msk : 1;
+ u32 mac_ff_err_int_msk : 1;
+ u32 rbreq_err_int_msk : 1;
+ u32 we_err_int_msk : 1;
+ };
+ u32 bits;
+ };
+};
+
#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt supported in this module
2024-07-31 9:42 ` [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt " Jijie Shao
@ 2024-07-31 13:14 ` Joe Damato
2024-08-01 11:31 ` Jijie Shao
2024-08-05 12:57 ` Simon Horman
1 sibling, 1 reply; 36+ messages in thread
From: Joe Damato @ 2024-07-31 13:14 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Wed, Jul 31, 2024 at 05:42:39PM +0800, Jijie Shao wrote:
> The driver supports four interrupts: TX interrupt, RX interrupt,
> mdio interrupt, and error interrupt.
>
> Actually, the driver does not use the mdio interrupt.
> Therefore, the driver does not request the mdio interrupt.
I might be reading this wrong, but the commit message seems a bit
confusing? If it's not used then why allocate it?
[...]
> ---
> .../ethernet/hisilicon/hibmcge/hbg_common.h | 20 ++
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 59 ++++++
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 8 +
> .../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 189 ++++++++++++++++++
> .../net/ethernet/hisilicon/hibmcge/hbg_irq.h | 13 ++
> .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 9 +
> .../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 34 ++++
> .../hisilicon/hibmcge/hbg_reg_union.h | 60 ++++++
> 8 files changed, 392 insertions(+)
> create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
> create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.h
[...]
> +
> +static const char *irq_names_map[HBG_VECTOR_NUM] = { "tx", "rx", "err", "mdio" };
> +
> +int hbg_irq_init(struct hbg_priv *priv)
> +{
> + struct hbg_vector *vectors = &priv->vectors;
> + struct hbg_irq *irq;
> + int ret;
> + int i;
> +
> + ret = pci_alloc_irq_vectors(priv->pdev, HBG_VECTOR_NUM, HBG_VECTOR_NUM,
> + PCI_IRQ_MSI);
No MSI-X ?
This seems to request HBG_VECTOR_NUM (4) IRQs and errors out if ret
!= HBG_VECTOR_NUM, but ...
> + if (ret < 0) {
> + dev_err(&priv->pdev->dev,
> + "failed to allocate MSI vectors, vectors = %d\n", ret);
> + return ret;
> + }
> +
> + if (ret != HBG_VECTOR_NUM) {
> + dev_err(&priv->pdev->dev,
> + "requested %u MSI, but allocated %d MSI\n",
> + HBG_VECTOR_NUM, ret);
> + ret = -EINVAL;
> + goto free_vectors;
> + }
> +
> + vectors->irqs = devm_kcalloc(&priv->pdev->dev, HBG_VECTOR_NUM,
> + sizeof(struct hbg_irq), GFP_KERNEL);
> + if (!vectors->irqs) {
> + ret = -ENOMEM;
> + goto free_vectors;
> + }
> +
> + /* mdio irq not request */
> + vectors->irq_count = HBG_VECTOR_NUM - 1;
Here the comment says mdio is not requested? But it does seem like
the IRQ is allocated above, it's just unused?
Maybe above you should remove mdio completely if its not in use?
Or is it used later in some other patch or something?
> + for (i = 0; i < vectors->irq_count; i++) {
> + irq = &vectors->irqs[i];
> + snprintf(irq->name, sizeof(irq->name) - 1, "%s-%s-%s",
> + HBG_DEV_NAME, pci_name(priv->pdev), irq_names_map[i]);
> +
> + irq->id = pci_irq_vector(priv->pdev, i);
> + irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
> + ret = request_irq(irq->id, hbg_irq_handle,
> + 0, irq->name, priv);
> + if (ret) {
> + dev_err(&priv->pdev->dev,
> + "failed to requset irq(%d), ret = %d\n",
> + irq->id, ret);
> + goto free_vectors;
> + }
> + }
> +
> + vectors->info_array = hbg_irqs;
> + vectors->info_array_len = ARRAY_SIZE(hbg_irqs);
> + return 0;
> +
> +free_vectors:
> + hbg_irq_uninit(priv);
> + return ret;
> +}
[...]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt supported in this module
2024-07-31 13:14 ` Joe Damato
@ 2024-08-01 11:31 ` Jijie Shao
0 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 11:31 UTC (permalink / raw)
To: Joe Damato, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Cc: shaojijie
on 2024/7/31 21:14, Joe Damato wrote:
>> + dev_err(&priv->pdev->dev,
>> + "failed to allocate MSI vectors, vectors = %d\n", ret);
>> + return ret;
>> + }
>> +
>> + if (ret != HBG_VECTOR_NUM) {
>> + dev_err(&priv->pdev->dev,
>> + "requested %u MSI, but allocated %d MSI\n",
>> + HBG_VECTOR_NUM, ret);
>> + ret = -EINVAL;
>> + goto free_vectors;
>> + }
>> +
>> + vectors->irqs = devm_kcalloc(&priv->pdev->dev, HBG_VECTOR_NUM,
>> + sizeof(struct hbg_irq), GFP_KERNEL);
>> + if (!vectors->irqs) {
>> + ret = -ENOMEM;
>> + goto free_vectors;
>> + }
>> +
>> + /* mdio irq not request */
>> + vectors->irq_count = HBG_VECTOR_NUM - 1;
> Here the comment says mdio is not requested? But it does seem like
> the IRQ is allocated above, it's just unused?
Yes, not request_irq only alloc
>
> Maybe above you should remove mdio completely if its not in use?
>
> Or is it used later in some other patch or something?
MDIO is required because the PHY is used.
However, the mdio interrupt provided by the hardware is not used.
If only 3 interrupts are alloc, an error is reported
when the driver is loaded and unloaded again,
log:
insmod hibmcge.ko
Generic PHY mii-0000:83:00.1:02: attached PHY driver (mii_bus:phy_addr=mii-0000:83:00.1:02, irq=POLL)
hibmcge 0000:83:00.1 enp131s0f1: renamed from eth0
IPv6: ADDRCONF(NETDEV_CHANGE): enp131s0f1: link becomes ready
hibmcge 0000:83:00.1: link up!
rmmod hibmcge.ko
insmod hibmcge.ko
hibmcge 0000:83:00.1: failed to allocate MSI vectors, vectors = -28
hibmcge: probe of 0000:83:00.1 failed with error -28
>
>> + for (i = 0; i < vectors->irq_count; i++) {
Therefore, only three interrupts are requested by request_irq
because "vectors->irq_count = HBG_VECTOR_NUM - 1"
Thanks,
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt supported in this module
2024-07-31 9:42 ` [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt " Jijie Shao
2024-07-31 13:14 ` Joe Damato
@ 2024-08-05 12:57 ` Simon Horman
2024-08-05 13:29 ` Jijie Shao
1 sibling, 1 reply; 36+ messages in thread
From: Simon Horman @ 2024-08-05 12:57 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Wed, Jul 31, 2024 at 05:42:39PM +0800, Jijie Shao wrote:
> The driver supports four interrupts: TX interrupt, RX interrupt,
> mdio interrupt, and error interrupt.
>
> Actually, the driver does not use the mdio interrupt.
> Therefore, the driver does not request the mdio interrupt.
>
> The error interrupt distinguishes different error information
> by using different masks. To distinguish different errors,
> the statistics count is added for each error.
>
> To ensure the consistency of the code process, masks are added for the
> TX interrupt and RX interrupt.
>
> This patch implements interrupt request and free, and provides a
> unified entry for the interrupt handler function. However,
> the specific interrupt handler function of each interrupt
> is not implemented currently.
>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
...
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
...
> +int hbg_irq_init(struct hbg_priv *priv)
> +{
> + struct hbg_vector *vectors = &priv->vectors;
> + struct hbg_irq *irq;
> + int ret;
> + int i;
> +
> + ret = pci_alloc_irq_vectors(priv->pdev, HBG_VECTOR_NUM, HBG_VECTOR_NUM,
> + PCI_IRQ_MSI);
> + if (ret < 0) {
> + dev_err(&priv->pdev->dev,
> + "failed to allocate MSI vectors, vectors = %d\n", ret);
> + return ret;
> + }
> +
> + if (ret != HBG_VECTOR_NUM) {
> + dev_err(&priv->pdev->dev,
> + "requested %u MSI, but allocated %d MSI\n",
> + HBG_VECTOR_NUM, ret);
> + ret = -EINVAL;
> + goto free_vectors;
> + }
> +
> + vectors->irqs = devm_kcalloc(&priv->pdev->dev, HBG_VECTOR_NUM,
> + sizeof(struct hbg_irq), GFP_KERNEL);
> + if (!vectors->irqs) {
> + ret = -ENOMEM;
> + goto free_vectors;
> + }
> +
> + /* mdio irq not request */
> + vectors->irq_count = HBG_VECTOR_NUM - 1;
> + for (i = 0; i < vectors->irq_count; i++) {
> + irq = &vectors->irqs[i];
> + snprintf(irq->name, sizeof(irq->name) - 1, "%s-%s-%s",
> + HBG_DEV_NAME, pci_name(priv->pdev), irq_names_map[i]);
> +
> + irq->id = pci_irq_vector(priv->pdev, i);
> + irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
I think that you <linux/irq.h> needs to be included.
Else allmodconfig builds - on x86_64 but curiously not ARM64 - fail.
CC drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.o
drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c: In function 'hbg_irq_init':
drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c:150:17: error: implicit declaration of function 'irq_set_status_flags' [-Wimplicit-function-declaration]
150 | irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
| ^~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c:150:47: error: 'IRQ_NOAUTOEN' undeclared (first use in this function); did you mean 'IRQF_NO_AUTOEN'?
150 | irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
| ^~~~~~~~~~~~
| IRQF_NO_AUTOEN
...
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt supported in this module
2024-08-05 12:57 ` Simon Horman
@ 2024-08-05 13:29 ` Jijie Shao
0 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-08-05 13:29 UTC (permalink / raw)
To: Simon Horman
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/5 20:57, Simon Horman wrote:
> On Wed, Jul 31, 2024 at 05:42:39PM +0800, Jijie Shao wrote:
>> The driver supports four interrupts: TX interrupt, RX interrupt,
>> mdio interrupt, and error interrupt.
>>
>> Actually, the driver does not use the mdio interrupt.
>> Therefore, the driver does not request the mdio interrupt.
>>
>> The error interrupt distinguishes different error information
>> by using different masks. To distinguish different errors,
>> the statistics count is added for each error.
>>
>> To ensure the consistency of the code process, masks are added for the
>> TX interrupt and RX interrupt.
>>
>> This patch implements interrupt request and free, and provides a
>> unified entry for the interrupt handler function. However,
>> the specific interrupt handler function of each interrupt
>> is not implemented currently.
>>
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ...
>
>> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
> ...
>
>> +int hbg_irq_init(struct hbg_priv *priv)
>> +{
>> + struct hbg_vector *vectors = &priv->vectors;
>> + struct hbg_irq *irq;
>> + int ret;
>> + int i;
>> +
>> + ret = pci_alloc_irq_vectors(priv->pdev, HBG_VECTOR_NUM, HBG_VECTOR_NUM,
>> + PCI_IRQ_MSI);
>> + if (ret < 0) {
>> + dev_err(&priv->pdev->dev,
>> + "failed to allocate MSI vectors, vectors = %d\n", ret);
>> + return ret;
>> + }
>> +
>> + if (ret != HBG_VECTOR_NUM) {
>> + dev_err(&priv->pdev->dev,
>> + "requested %u MSI, but allocated %d MSI\n",
>> + HBG_VECTOR_NUM, ret);
>> + ret = -EINVAL;
>> + goto free_vectors;
>> + }
>> +
>> + vectors->irqs = devm_kcalloc(&priv->pdev->dev, HBG_VECTOR_NUM,
>> + sizeof(struct hbg_irq), GFP_KERNEL);
>> + if (!vectors->irqs) {
>> + ret = -ENOMEM;
>> + goto free_vectors;
>> + }
>> +
>> + /* mdio irq not request */
>> + vectors->irq_count = HBG_VECTOR_NUM - 1;
>> + for (i = 0; i < vectors->irq_count; i++) {
>> + irq = &vectors->irqs[i];
>> + snprintf(irq->name, sizeof(irq->name) - 1, "%s-%s-%s",
>> + HBG_DEV_NAME, pci_name(priv->pdev), irq_names_map[i]);
>> +
>> + irq->id = pci_irq_vector(priv->pdev, i);
>> + irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
> I think that you <linux/irq.h> needs to be included.
> Else allmodconfig builds - on x86_64 but curiously not ARM64 - fail.
Thank you, this is very confusing. I built successfully on both ARM and x86.
I will rebuild it according to your Clang version to fix this error
Jijie Shao
>
> CC drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.o
> drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c: In function 'hbg_irq_init':
> drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c:150:17: error: implicit declaration of function 'irq_set_status_flags' [-Wimplicit-function-declaration]
> 150 | irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
> | ^~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c:150:47: error: 'IRQ_NOAUTOEN' undeclared (first use in this function); did you mean 'IRQF_NO_AUTOEN'?
> 150 | irq_set_status_flags(irq->id, IRQ_NOAUTOEN);
> | ^~~~~~~~~~~~
> | IRQF_NO_AUTOEN
>
> ...
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (3 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 04/10] net: hibmcge: Add interrupt " Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-08-01 0:51 ` Andrew Lunn
2024-07-31 9:42 ` [RFC PATCH net-next 06/10] net: hibmcge: Implement .ndo_start_xmit function Jijie Shao
` (4 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Implement the .ndo_open .ndo_stop .ndo_set_mac_address
and .ndo_change_mtu functions.
And .ndo_validate_addr calls the eth_validate_addr function directly
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 4 +
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 40 +++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 3 +
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 105 ++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 8 ++
5 files changed, 160 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 6a3e647cd27c..22d5ce310a3f 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -30,8 +30,12 @@ enum hbg_dir {
enum hbg_nic_state {
HBG_NIC_STATE_INITED = 0,
HBG_NIC_STATE_EVENT_HANDLING,
+ HBG_NIC_STATE_OPEN,
};
+#define hbg_nic_is_open(priv) test_bit(HBG_NIC_STATE_OPEN, &(priv)->state)
+#define hbg_nic_is_inited(priv) test_bit(HBG_NIC_STATE_INITED, &(priv)->state)
+
enum hbg_hw_event_type {
HBG_HW_EVENT_NONE = 0,
HBG_HW_EVENT_INIT, /* driver is loading */
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index d9bed7cc7790..f06c74d59c02 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -71,6 +71,46 @@ int hbg_hw_dev_specs_init(struct hbg_priv *priv)
return 0;
}
+void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr)
+{
+ hbg_reg_write64(priv, HBG_REG_STATION_ADDR_LOW_2_ADDR, mac_addr);
+}
+
+static void hbg_hw_set_pcu_max_frame_len(struct hbg_priv *priv,
+ u16 max_frame_len)
+{
+#define HBG_PCU_FRAME_LEN_PLUS 4
+
+ max_frame_len = max_t(u32, max_frame_len, HBG_DEFAULT_MTU_SIZE);
+
+ /* lower two bits of value must be set to 0. Otherwise, the value is ignored */
+ max_frame_len = round_up(max_frame_len, HBG_PCU_FRAME_LEN_PLUS);
+
+ hbg_reg_write_field(priv, HBG_REG_MAX_FRAME_LEN_ADDR,
+ HBG_REG_MAX_FRAME_LEN_M, max_frame_len);
+}
+
+static void hbg_hw_set_mac_max_frame_len(struct hbg_priv *priv,
+ u16 max_frame_size)
+{
+ hbg_reg_write_field(priv, HBG_REG_MAX_FRAME_SIZE_ADDR,
+ HBG_REG_MAX_FRAME_LEN_M, max_frame_size);
+}
+
+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);
+}
+
+void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable)
+{
+ hbg_reg_write_bit(priv, HBG_REG_PORT_ENABLE_ADDR,
+ HBG_REG_PORT_ENABLE_TX_B, enable);
+ hbg_reg_write_bit(priv, HBG_REG_PORT_ENABLE_ADDR,
+ HBG_REG_PORT_ENABLE_RX_B, enable);
+}
+
void hbg_hw_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status)
{
status->bits = hbg_reg_read(priv, HBG_REG_CF_INTRPT_STAT_ADDR);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index e2a08dc5d883..556f479bc094 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -57,6 +57,8 @@ static inline void hbg_reg_write64(struct hbg_priv *priv, u32 reg_addr,
int hbg_hw_event_notify(struct hbg_priv *priv, enum hbg_hw_event_type event_type);
int hbg_hw_dev_specs_init(struct hbg_priv *priv);
+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_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status);
void hbg_hw_get_err_intr_mask(struct hbg_priv *priv, struct hbg_intr_mask *msk);
void hbg_hw_set_err_intr_mask(struct hbg_priv *priv, const struct hbg_intr_mask *msk);
@@ -67,6 +69,7 @@ void hbg_hw_set_txrx_intr_clear(struct hbg_priv *priv, enum hbg_dir dir);
void hbg_hw_get_txrx_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status);
int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
int hbg_hw_sgmii_autoneg(struct hbg_priv *priv);
+void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu);
int hbg_hw_init(struct hbg_priv *pri);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 059ea155572f..0184ea5d563e 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -2,6 +2,7 @@
// Copyright (c) 2024 Hisilicon Limited.
#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
#include <linux/netdevice.h>
#include <linux/pci.h>
#include "hbg_common.h"
@@ -10,6 +11,105 @@
#include "hbg_main.h"
#include "hbg_mdio.h"
+static void hbg_enable_intr(struct hbg_priv *priv, bool enabled)
+{
+ u32 i;
+
+ for (i = 0; i < priv->vectors.info_array_len; i++)
+ hbg_irq_enable(priv, priv->vectors.info_array[i].mask,
+ enabled);
+
+ for (i = 0; i < priv->vectors.irq_count; i++) {
+ if (enabled)
+ enable_irq(priv->vectors.irqs[i].id);
+ else
+ disable_irq(priv->vectors.irqs[i].id);
+ }
+}
+
+static int hbg_net_open(struct net_device *dev)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+
+ if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
+ return 0;
+
+ netif_carrier_off(dev);
+ hbg_enable_intr(priv, true);
+ hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
+ netif_start_queue(dev);
+ hbg_phy_start(priv);
+ return 0;
+}
+
+static int hbg_net_stop(struct net_device *dev)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+
+ if (!hbg_nic_is_open(priv))
+ return 0;
+
+ clear_bit(HBG_NIC_STATE_OPEN, &priv->state);
+ netif_carrier_off(dev);
+ netif_stop_queue(dev);
+ hbg_hw_mac_enable(priv, HBG_STATUS_DISABLE);
+ hbg_enable_intr(priv, false);
+ hbg_phy_stop(priv);
+ return 0;
+}
+
+static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+ u8 *mac_addr;
+
+ mac_addr = ((struct sockaddr *)addr)->sa_data;
+ if (ether_addr_equal(dev->dev_addr, mac_addr))
+ return 0;
+
+ if (!is_valid_ether_addr(mac_addr))
+ return -EADDRNOTAVAIL;
+
+ hbg_hw_set_uc_addr(priv, ether_addr_to_u64(mac_addr));
+ dev_addr_set(dev, mac_addr);
+ return 0;
+}
+
+static int hbg_net_change_mtu(struct net_device *dev, int new_mtu)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+ bool is_opened = hbg_nic_is_open(priv);
+ u32 frame_len;
+
+ if (new_mtu == dev->mtu)
+ return 0;
+
+ if (new_mtu < priv->dev_specs.min_mtu || new_mtu > priv->dev_specs.max_mtu)
+ return -EINVAL;
+
+ hbg_net_stop(dev);
+
+ frame_len = new_mtu + VLAN_HLEN * priv->dev_specs.vlan_layers +
+ ETH_HLEN + ETH_FCS_LEN;
+ hbg_hw_set_mtu(priv, frame_len);
+
+ dev_info(&priv->pdev->dev,
+ "change mtu from %u to %u\n", dev->mtu, new_mtu);
+ WRITE_ONCE(dev->mtu, new_mtu);
+
+ if (is_opened)
+ hbg_net_open(dev);
+ return 0;
+}
+
+static const struct net_device_ops hbg_netdev_ops = {
+ .ndo_open = hbg_net_open,
+ .ndo_stop = hbg_net_stop,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = hbg_net_set_mac_address,
+ .ndo_change_mtu = hbg_net_change_mtu,
+};
+
static const u32 hbg_mode_ability[] = {
ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
ETHTOOL_LINK_MODE_100baseT_Full_BIT,
@@ -95,6 +195,7 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
priv = netdev_priv(netdev);
priv->netdev = netdev;
priv->pdev = pdev;
+ netdev->netdev_ops = &hbg_netdev_ops;
ret = hbg_pci_init(pdev);
if (ret)
@@ -104,6 +205,10 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
return ret;
+ netdev->max_mtu = priv->dev_specs.max_mtu;
+ netdev->min_mtu = priv->dev_specs.min_mtu;
+ hbg_net_change_mtu(netdev, HBG_DEFAULT_MTU_SIZE);
+ hbg_net_set_mac_address(priv->netdev, &priv->dev_specs.mac_addr);
ret = devm_register_netdev(&pdev->dev, netdev);
if (ret)
return dev_err_probe(&pdev->dev, ret,
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index b422fa990270..86f1157a2af2 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -29,7 +29,9 @@
/* GMAC */
#define HBG_REG_SGMII_BASE 0x10000
#define HBG_REG_DUPLEX_TYPE_ADDR (HBG_REG_SGMII_BASE + 0x0008)
+#define HBG_REG_MAX_FRAME_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x003C)
#define HBG_REG_PORT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x0040)
+#define HBG_REG_PORT_ENABLE_ADDR (HBG_REG_SGMII_BASE + 0x0044)
#define HBG_REG_AN_NEG_STATE_ADDR (HBG_REG_SGMII_BASE + 0x0058)
#define HBG_REG_TX_LOCAL_PAGE_ADDR (HBG_REG_SGMII_BASE + 0x005C)
#define HBG_REG_TRANSMIT_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x0060)
@@ -38,11 +40,14 @@
#define HBG_REG_16_BIT_CNTR_ADDR (HBG_REG_SGMII_BASE + 0x01CC)
#define HBG_REG_LD_LINK_COUNTER_ADDR (HBG_REG_SGMII_BASE + 0x01D0)
#define HBG_REG_RECV_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x01E0)
+#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)
/* PCU */
#define HBG_REG_CF_INTRPT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x042C)
#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_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
#define HBG_REG_RX_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04F0)
@@ -57,12 +62,15 @@
/* mask */
#define HBG_REG_PORT_MODE_M GENMASK(3, 0)
#define HBG_REG_MODE_CHANGE_EN_B BIT(0)
+#define HBG_REG_MAX_FRAME_LEN_M GENMASK(15, 0)
#define HBG_REG_RX_BUF_SIZE_M GENMASK(15, 0)
#define HBG_REG_BUS_CTRL_ENDIAN_M GENMASK(2, 1)
#define HBG_REG_DUPLEX_B BIT(0)
#define HBG_REG_CF_CRC_STRIP_B BIT(1)
#define HBG_REG_MDIO_WDATA_M GENMASK(15, 0)
#define HBG_REG_IND_INTR_MASK_B BIT(0)
+#define HBG_REG_PORT_ENABLE_RX_B BIT(1)
+#define HBG_REG_PORT_ENABLE_TX_B BIT(2)
enum hbg_port_mode {
/* 0x0 ~ 0x5 are reserved */
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-07-31 9:42 ` [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions Jijie Shao
@ 2024-08-01 0:51 ` Andrew Lunn
2024-08-01 9:13 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 0:51 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
> +{
> + struct hbg_priv *priv = netdev_priv(dev);
> + u8 *mac_addr;
> +
> + mac_addr = ((struct sockaddr *)addr)->sa_data;
> + if (ether_addr_equal(dev->dev_addr, mac_addr))
> + return 0;
> +
> + if (!is_valid_ether_addr(mac_addr))
> + return -EADDRNOTAVAIL;
How does the core pass you an invalid MAC address?
> +static int hbg_net_change_mtu(struct net_device *dev, int new_mtu)
> +{
> + struct hbg_priv *priv = netdev_priv(dev);
> + bool is_opened = hbg_nic_is_open(priv);
> + u32 frame_len;
> +
> + if (new_mtu == dev->mtu)
> + return 0;
> +
> + if (new_mtu < priv->dev_specs.min_mtu || new_mtu > priv->dev_specs.max_mtu)
> + return -EINVAL;
You just need to set dev->min_mtu and dev->max_mtu, and the core will
do this validation for you.
> + dev_info(&priv->pdev->dev,
> + "change mtu from %u to %u\n", dev->mtu, new_mtu);
dev_dbg() Don't spam the log for normal operations.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-08-01 0:51 ` Andrew Lunn
@ 2024-08-01 9:13 ` Jijie Shao
2024-08-01 12:18 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 9:13 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 8:51, Andrew Lunn wrote:
>> +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
>> +{
>> + struct hbg_priv *priv = netdev_priv(dev);
>> + u8 *mac_addr;
>> +
>> + mac_addr = ((struct sockaddr *)addr)->sa_data;
>> + if (ether_addr_equal(dev->dev_addr, mac_addr))
>> + return 0;
>> +
>> + if (!is_valid_ether_addr(mac_addr))
>> + return -EADDRNOTAVAIL;
> How does the core pass you an invalid MAC address?
According to my test,
in the 6.4 rc4 kernel version, invalid mac address is allowed to be configured.
An error is reported only when ifconfig ethx up.
>
>> +static int hbg_net_change_mtu(struct net_device *dev, int new_mtu)
>> +{
>> + struct hbg_priv *priv = netdev_priv(dev);
>> + bool is_opened = hbg_nic_is_open(priv);
>> + u32 frame_len;
>> +
>> + if (new_mtu == dev->mtu)
>> + return 0;
>> +
>> + if (new_mtu < priv->dev_specs.min_mtu || new_mtu > priv->dev_specs.max_mtu)
>> + return -EINVAL;
> You just need to set dev->min_mtu and dev->max_mtu, and the core will
> do this validation for you.
Thanks, I'll test it，and if it works I'll remove the judgement
>
>> + dev_info(&priv->pdev->dev,
>> + "change mtu from %u to %u\n", dev->mtu, new_mtu);
> dev_dbg() Don't spam the log for normal operations.
okay, Thanks!
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-08-01 9:13 ` Jijie Shao
@ 2024-08-01 12:18 ` Andrew Lunn
2024-08-01 12:33 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 12:18 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Thu, Aug 01, 2024 at 05:13:33PM +0800, Jijie Shao wrote:
>
> on 2024/8/1 8:51, Andrew Lunn wrote:
> > > +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
> > > +{
> > > + struct hbg_priv *priv = netdev_priv(dev);
> > > + u8 *mac_addr;
> > > +
> > > + mac_addr = ((struct sockaddr *)addr)->sa_data;
> > > + if (ether_addr_equal(dev->dev_addr, mac_addr))
> > > + return 0;
> > > +
> > > + if (!is_valid_ether_addr(mac_addr))
> > > + return -EADDRNOTAVAIL;
> > How does the core pass you an invalid MAC address?
>
> According to my test,
> in the 6.4 rc4 kernel version, invalid mac address is allowed to be configured.
> An error is reported only when ifconfig ethx up.
Ah, interesting.
I see a test in __dev_open(), which is what you are saying here. But i
would also expect a test in rtnetlink, or maybe dev_set_mac_address().
We don't want every driver having to repeat this test in their
.ndo_set_mac_address, when it could be done once in the core.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-08-01 12:18 ` Andrew Lunn
@ 2024-08-01 12:33 ` Jijie Shao
2024-08-01 12:36 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 12:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 20:18, Andrew Lunn wrote:
> On Thu, Aug 01, 2024 at 05:13:33PM +0800, Jijie Shao wrote:
>> on 2024/8/1 8:51, Andrew Lunn wrote:
>>>> +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
>>>> +{
>>>> + struct hbg_priv *priv = netdev_priv(dev);
>>>> + u8 *mac_addr;
>>>> +
>>>> + mac_addr = ((struct sockaddr *)addr)->sa_data;
>>>> + if (ether_addr_equal(dev->dev_addr, mac_addr))
>>>> + return 0;
>>>> +
>>>> + if (!is_valid_ether_addr(mac_addr))
>>>> + return -EADDRNOTAVAIL;
>>> How does the core pass you an invalid MAC address?
>> According to my test,
>> in the 6.4 rc4 kernel version, invalid mac address is allowed to be configured.
>> An error is reported only when ifconfig ethx up.
> Ah, interesting.
>
> I see a test in __dev_open(), which is what you are saying here. But i
> would also expect a test in rtnetlink, or maybe dev_set_mac_address().
> We don't want every driver having to repeat this test in their
> .ndo_set_mac_address, when it could be done once in the core.
>
> Andrew
Hi:
I did the following test on my device:
insmod hibmcge.ko
hibmcge: no symbol version for module_layout
hibmcge: loading out-of-tree module taints kernel.
hibmcge: module verification failed: signature and/or required key missing - tainting kernel
hibmcge 0000:83:00.1: enabling device (0140 -> 0142)
Generic PHY mii-0000:83:00.1:02: attached PHY driver (mii_bus:phy_addr=mii-0000:83:00.1:02, irq=POLL)
hibmcge 0000:83:00.1 enp131s0f1: renamed from eth0
IPv6: ADDRCONF(NETDEV_CHANGE): enp131s0f1: link becomes ready
hibmcge 0000:83:00.1: link up!
ifconfig enp131s0f1 hw ether FF:FF:FF:FF:FF:FF
ip a
6: enp131s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000
link/ether ff:ff:ff:ff:ff:ff brd ff:ff:ff:ff:ff:ff permaddr 08:02:00:00:08:08
ifconfig enp131s0f1 up
ifconfig enp131s0f1 down up
SIOCSIFFLAGS: Cannot assign requested address
hibmcge 0000:83:00.1: link down!
uname -a
Linux localhost.localdomain 6.4.0+ #1 SMP Fri Mar 15 14:44:20 CST 2024 aarch64 aarch64 aarch64 GNU/Linux
So I'm not sure what's wrong. I also implemented ndo_validate_addr by eth_validate_addr.
Thanks
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-08-01 12:33 ` Jijie Shao
@ 2024-08-01 12:36 ` Andrew Lunn
2024-08-01 13:08 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 12:36 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Thu, Aug 01, 2024 at 08:33:38PM +0800, Jijie Shao wrote:
>
> on 2024/8/1 20:18, Andrew Lunn wrote:
> > On Thu, Aug 01, 2024 at 05:13:33PM +0800, Jijie Shao wrote:
> > > on 2024/8/1 8:51, Andrew Lunn wrote:
> > > > > +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
> > > > > +{
> > > > > + struct hbg_priv *priv = netdev_priv(dev);
> > > > > + u8 *mac_addr;
> > > > > +
> > > > > + mac_addr = ((struct sockaddr *)addr)->sa_data;
> > > > > + if (ether_addr_equal(dev->dev_addr, mac_addr))
> > > > > + return 0;
> > > > > +
> > > > > + if (!is_valid_ether_addr(mac_addr))
> > > > > + return -EADDRNOTAVAIL;
> > > > How does the core pass you an invalid MAC address?
> > > According to my test,
> > > in the 6.4 rc4 kernel version, invalid mac address is allowed to be configured.
> > > An error is reported only when ifconfig ethx up.
> > Ah, interesting.
> >
> > I see a test in __dev_open(), which is what you are saying here. But i
> > would also expect a test in rtnetlink, or maybe dev_set_mac_address().
> > We don't want every driver having to repeat this test in their
> > .ndo_set_mac_address, when it could be done once in the core.
> >
> > Andrew
>
> Hi:
> I did the following test on my device:
>
> insmod hibmcge.ko
> hibmcge: no symbol version for module_layout
> hibmcge: loading out-of-tree module taints kernel.
> hibmcge: module verification failed: signature and/or required key missing - tainting kernel
> hibmcge 0000:83:00.1: enabling device (0140 -> 0142)
> Generic PHY mii-0000:83:00.1:02: attached PHY driver (mii_bus:phy_addr=mii-0000:83:00.1:02, irq=POLL)
> hibmcge 0000:83:00.1 enp131s0f1: renamed from eth0
> IPv6: ADDRCONF(NETDEV_CHANGE): enp131s0f1: link becomes ready
> hibmcge 0000:83:00.1: link up!
>
> ifconfig enp131s0f1 hw ether FF:FF:FF:FF:FF:FF
>
> ip a
> 6: enp131s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000
> link/ether ff:ff:ff:ff:ff:ff brd ff:ff:ff:ff:ff:ff permaddr 08:02:00:00:08:08
> ifconfig enp131s0f1 up
> ifconfig enp131s0f1 down up
> SIOCSIFFLAGS: Cannot assign requested address
> hibmcge 0000:83:00.1: link down!
>
> uname -a
> Linux localhost.localdomain 6.4.0+ #1 SMP Fri Mar 15 14:44:20 CST 2024 aarch64 aarch64 aarch64 GNU/Linux
>
>
>
> So I'm not sure what's wrong. I also implemented ndo_validate_addr by eth_validate_addr.
I agree. I don't see a test. Please could you include a patch to
dev_set_mac_address() to validate the address there before calling
into the driver. It might also be worth a search to see if anybody
else has tried this before, and failed. There might be a good reason
you cannot validate it.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions
2024-08-01 12:36 ` Andrew Lunn
@ 2024-08-01 13:08 ` Jijie Shao
0 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 13:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 20:36, Andrew Lunn wrote:
> On Thu, Aug 01, 2024 at 08:33:38PM +0800, Jijie Shao wrote:
>> on 2024/8/1 20:18, Andrew Lunn wrote:
>>> On Thu, Aug 01, 2024 at 05:13:33PM +0800, Jijie Shao wrote:
>>>> on 2024/8/1 8:51, Andrew Lunn wrote:
>>>>>> +static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
>>>>>> +{
>>>>>> + struct hbg_priv *priv = netdev_priv(dev);
>>>>>> + u8 *mac_addr;
>>>>>> +
>>>>>> + mac_addr = ((struct sockaddr *)addr)->sa_data;
>>>>>> + if (ether_addr_equal(dev->dev_addr, mac_addr))
>>>>>> + return 0;
>>>>>> +
>>>>>> + if (!is_valid_ether_addr(mac_addr))
>>>>>> + return -EADDRNOTAVAIL;
>>>>> How does the core pass you an invalid MAC address?
>>>> According to my test,
>>>> in the 6.4 rc4 kernel version, invalid mac address is allowed to be configured.
>>>> An error is reported only when ifconfig ethx up.
>>> Ah, interesting.
>>>
>>> I see a test in __dev_open(), which is what you are saying here. But i
>>> would also expect a test in rtnetlink, or maybe dev_set_mac_address().
>>> We don't want every driver having to repeat this test in their
>>> .ndo_set_mac_address, when it could be done once in the core.
>>>
>>> Andrew
>> Hi:
>> I did the following test on my device:
>>
>> insmod hibmcge.ko
>> hibmcge: no symbol version for module_layout
>> hibmcge: loading out-of-tree module taints kernel.
>> hibmcge: module verification failed: signature and/or required key missing - tainting kernel
>> hibmcge 0000:83:00.1: enabling device (0140 -> 0142)
>> Generic PHY mii-0000:83:00.1:02: attached PHY driver (mii_bus:phy_addr=mii-0000:83:00.1:02, irq=POLL)
>> hibmcge 0000:83:00.1 enp131s0f1: renamed from eth0
>> IPv6: ADDRCONF(NETDEV_CHANGE): enp131s0f1: link becomes ready
>> hibmcge 0000:83:00.1: link up!
>>
>> ifconfig enp131s0f1 hw ether FF:FF:FF:FF:FF:FF
>>
>> ip a
>> 6: enp131s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000
>> link/ether ff:ff:ff:ff:ff:ff brd ff:ff:ff:ff:ff:ff permaddr 08:02:00:00:08:08
>> ifconfig enp131s0f1 up
>> ifconfig enp131s0f1 down up
>> SIOCSIFFLAGS: Cannot assign requested address
>> hibmcge 0000:83:00.1: link down!
>>
>> uname -a
>> Linux localhost.localdomain 6.4.0+ #1 SMP Fri Mar 15 14:44:20 CST 2024 aarch64 aarch64 aarch64 GNU/Linux
>>
>>
>>
>> So I'm not sure what's wrong. I also implemented ndo_validate_addr by eth_validate_addr.
> I agree. I don't see a test. Please could you include a patch to
> dev_set_mac_address() to validate the address there before calling
> into the driver. It might also be worth a search to see if anybody
> else has tried this before, and failed. There might be a good reason
> you cannot validate it.
>
> Andrew
sure, I will include that in V2
Thanks，
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 06/10] net: hibmcge: Implement .ndo_start_xmit function
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (4 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 05/10] net: hibmcge: Implement some .ndo functions Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets Jijie Shao
` (3 subsequent siblings)
9 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Implement .ndo_start_xmit function to fill the information of the packet
to be transmitted into the tx descriptor, and then the hardware will
transmit the packet using the information in the tx descriptor.
In addition, we also implemented the tx_handler function to enable the
tx descriptor to be reused, and .ndo_tx_timeout function to print some
information when the hardware is busy.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 46 ++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 18 ++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 2 +
.../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 47 ++--
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 33 +++
.../net/ethernet/hisilicon/hibmcge/hbg_main.h | 1 +
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 6 +
.../hisilicon/hibmcge/hbg_reg_union.h | 29 ++
.../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 255 ++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_txrx.h | 37 +++
10 files changed, 456 insertions(+), 18 deletions(-)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 22d5ce310a3f..760dcf88c0cf 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -15,8 +15,16 @@
#define HBG_DEFAULT_MTU_SIZE 1500
#define HBG_RX_SKIP1 0x00
#define HBG_RX_SKIP2 0x01
+#define HBG_PCU_CACHE_LINE_SIZE 32
+
#define HBG_LINK_DOWN 0
#define HBG_LINK_UP 1
+#define HBG_TX_TIMEOUT_BUF_LEN 1024
+
+enum hbg_tx_state {
+ HBG_TX_STATE_COMPLETE = 0, /* clear state, must fix to 0 */
+ HBG_TX_STATE_START,
+};
enum hbg_dir {
HBG_DIR_TX = 1 << 0,
@@ -36,6 +44,41 @@ enum hbg_nic_state {
#define hbg_nic_is_open(priv) test_bit(HBG_NIC_STATE_OPEN, &(priv)->state)
#define hbg_nic_is_inited(priv) test_bit(HBG_NIC_STATE_INITED, &(priv)->state)
+struct hbg_priv;
+struct hbg_ring;
+struct hbg_buffer {
+ u32 state;
+ dma_addr_t state_dma;
+
+ struct sk_buff *skb;
+ dma_addr_t skb_dma;
+ u32 skb_len;
+
+ enum hbg_dir dir;
+ struct hbg_ring *ring;
+ struct hbg_priv *priv;
+};
+
+struct hbg_ring {
+ struct hbg_buffer *queue;
+ dma_addr_t queue_dma;
+
+ union {
+ u32 head;
+ u32 ntc;
+ };
+ union {
+ u32 tail;
+ u32 ntu;
+ };
+ u32 len;
+
+ enum hbg_dir dir;
+ struct hbg_priv *priv;
+ struct napi_struct napi;
+ char *tout_log_buf; /* tx timeout log buffer */
+};
+
enum hbg_hw_event_type {
HBG_HW_EVENT_NONE = 0,
HBG_HW_EVENT_INIT, /* driver is loading */
@@ -59,6 +102,8 @@ struct hbg_irq_info {
const char *name;
enum hbg_irq_mask mask;
u64 count;
+
+ void (*irq_handle)(struct hbg_priv *priv, struct hbg_irq_info *irq_info);
};
struct hbg_irq {
@@ -95,6 +140,7 @@ struct hbg_priv {
unsigned long state;
struct hbg_mac mac;
struct hbg_vector vectors;
+ struct hbg_ring tx_ring;
};
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index f06c74d59c02..683ad03f47d7 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -68,6 +68,7 @@ int hbg_hw_dev_specs_init(struct hbg_priv *priv)
if (!is_valid_ether_addr((u8 *)dev_specs->mac_addr.sa_data))
return -EADDRNOTAVAIL;
+ dev_specs->max_frame_len = HBG_PCU_CACHE_LINE_SIZE + dev_specs->max_mtu;
return 0;
}
@@ -111,6 +112,23 @@ void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable)
HBG_REG_PORT_ENABLE_RX_B, enable);
}
+u32 hbg_hw_get_fifo_used_num(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ if (hbg_dir_has_tx(dir))
+ return hbg_reg_read_field(priv, HBG_REG_CF_CFF_DATA_NUM_ADDR,
+ HBG_REG_CF_CFF_DATA_NUM_ADDR_TX_M);
+
+ return 0;
+}
+
+void hbg_hw_set_tx_desc(struct hbg_priv *priv, struct hbg_tx_desc *tx_desc)
+{
+ hbg_reg_write(priv, HBG_REG_TX_CFF_ADDR_0_ADDR, tx_desc->word0);
+ hbg_reg_write(priv, HBG_REG_TX_CFF_ADDR_1_ADDR, tx_desc->word1);
+ hbg_reg_write(priv, HBG_REG_TX_CFF_ADDR_2_ADDR, tx_desc->pkt_addr);
+ hbg_reg_write(priv, HBG_REG_TX_CFF_ADDR_3_ADDR, tx_desc->clear_addr);
+}
+
void hbg_hw_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status)
{
status->bits = hbg_reg_read(priv, HBG_REG_CF_INTRPT_STAT_ADDR);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 556f479bc094..cb23b239f42a 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -71,5 +71,7 @@ int hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex);
int hbg_hw_sgmii_autoneg(struct hbg_priv *priv);
void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu);
int hbg_hw_init(struct hbg_priv *pri);
+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);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
index 5e90e72d7f24..ed51b4f5d5ec 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
@@ -7,25 +7,30 @@
#define HBG_VECTOR_NUM 4
+static void hbg_irq_handle_tx(struct hbg_priv *priv, struct hbg_irq_info *irq_info)
+{
+ napi_schedule(&priv->tx_ring.napi);
+}
+
static struct hbg_irq_info hbg_irqs[] = {
- { "TX", HBG_IRQ_TX, 0 },
- { "RX", HBG_IRQ_RX, 0 },
- { "RX_BUF_AVL", HBG_IRQ_BUF_AVL, 0 },
- { "MAC_MII_FIFO_ERR", HBG_IRQ_MAC_MII_FIFO_ERR, 0 },
- { "MAC_PCS_RX_FIFO_ERR", HBG_IRQ_MAC_PCS_RX_FIFO_ERR, 0 },
- { "MAC_PCS_TX_FIFO_ERR", HBG_IRQ_MAC_PCS_TX_FIFO_ERR, 0 },
- { "MAC_APP_RX_FIFO_ERR", HBG_IRQ_MAC_APP_RX_FIFO_ERR, 0 },
- { "MAC_APP_TX_FIFO_ERR", HBG_IRQ_MAC_APP_TX_FIFO_ERR, 0 },
- { "SRAM_PARITY_ERR", HBG_IRQ_SRAM_PARITY_ERR, 0 },
- { "TX_AHB_ERR", HBG_IRQ_TX_AHB_ERR, 0 },
- { "REL_BUF_ERR", HBG_IRQ_REL_BUF_ERR, 0 },
- { "TXCFG_AVL", HBG_IRQ_TXCFG_AVL, 0 },
- { "TX_DROP", HBG_IRQ_TX_DROP, 0 },
- { "RX_DROP", HBG_IRQ_RX_DROP, 0 },
- { "RX_AHB_ERR", HBG_IRQ_RX_AHB_ERR, 0 },
- { "MAC_FIFO_ERR", HBG_IRQ_MAC_FIFO_ERR, 0 },
- { "RBREQ_ERR", HBG_IRQ_RBREQ_ERR, 0 },
- { "WE_ERR", HBG_IRQ_WE_ERR, 0 },
+ { "TX", HBG_IRQ_TX, 0, hbg_irq_handle_tx },
+ { "RX", HBG_IRQ_RX, 0, NULL },
+ { "RX_BUF_AVL", HBG_IRQ_BUF_AVL, 0, NULL },
+ { "MAC_MII_FIFO_ERR", HBG_IRQ_MAC_MII_FIFO_ERR, 0, NULL },
+ { "MAC_PCS_RX_FIFO_ERR", HBG_IRQ_MAC_PCS_RX_FIFO_ERR, 0, NULL },
+ { "MAC_PCS_TX_FIFO_ERR", HBG_IRQ_MAC_PCS_TX_FIFO_ERR, 0, NULL },
+ { "MAC_APP_RX_FIFO_ERR", HBG_IRQ_MAC_APP_RX_FIFO_ERR, 0, NULL },
+ { "MAC_APP_TX_FIFO_ERR", HBG_IRQ_MAC_APP_TX_FIFO_ERR, 0, NULL },
+ { "SRAM_PARITY_ERR", HBG_IRQ_SRAM_PARITY_ERR, 0, NULL },
+ { "TX_AHB_ERR", HBG_IRQ_TX_AHB_ERR, 0, NULL },
+ { "REL_BUF_ERR", HBG_IRQ_REL_BUF_ERR, 0, NULL },
+ { "TXCFG_AVL", HBG_IRQ_TXCFG_AVL, 0, NULL },
+ { "TX_DROP", HBG_IRQ_TX_DROP, 0, NULL },
+ { "RX_DROP", HBG_IRQ_RX_DROP, 0, NULL },
+ { "RX_AHB_ERR", HBG_IRQ_RX_AHB_ERR, 0, NULL },
+ { "MAC_FIFO_ERR", HBG_IRQ_MAC_FIFO_ERR, 0, NULL },
+ { "RBREQ_ERR", HBG_IRQ_RBREQ_ERR, 0, NULL },
+ { "WE_ERR", HBG_IRQ_WE_ERR, 0, NULL },
};
void hbg_irq_enable(struct hbg_priv *priv, enum hbg_irq_mask mask, bool enable)
@@ -85,6 +90,12 @@ static void hbg_irq_info_handle(struct hbg_priv *priv,
hbg_irq_clear_src(priv, irq_info->mask);
irq_info->count++;
+ if (irq_info->irq_handle)
+ irq_info->irq_handle(priv, irq_info);
+
+ if (irq_info->mask == HBG_IRQ_TX)
+ return;
+
hbg_irq_enable(priv, irq_info->mask, true);
}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 0184ea5d563e..8efeea9b0c26 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -10,6 +10,7 @@
#include "hbg_irq.h"
#include "hbg_main.h"
#include "hbg_mdio.h"
+#include "hbg_txrx.h"
static void hbg_enable_intr(struct hbg_priv *priv, bool enabled)
{
@@ -35,6 +36,7 @@ static int hbg_net_open(struct net_device *dev)
return 0;
netif_carrier_off(dev);
+ napi_enable(&priv->tx_ring.napi);
hbg_enable_intr(priv, true);
hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
netif_start_queue(dev);
@@ -54,6 +56,7 @@ static int hbg_net_stop(struct net_device *dev)
netif_stop_queue(dev);
hbg_hw_mac_enable(priv, HBG_STATUS_DISABLE);
hbg_enable_intr(priv, false);
+ napi_disable(&priv->tx_ring.napi);
hbg_phy_stop(priv);
return 0;
}
@@ -102,12 +105,33 @@ static int hbg_net_change_mtu(struct net_device *dev, int new_mtu)
return 0;
}
+static void hbg_net_tx_timeout(struct net_device *dev, unsigned int txqueue)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+ struct hbg_ring *ring = &priv->tx_ring;
+ char *buf = ring->tout_log_buf;
+ u32 pos = 0;
+
+ pos += scnprintf(buf + pos, HBG_TX_TIMEOUT_BUF_LEN - pos,
+ "ring used num: %u, fifo used num: %u\n",
+ hbg_get_queue_used_num(ring),
+ hbg_hw_get_fifo_used_num(priv, HBG_DIR_TX));
+ pos += scnprintf(buf + pos, HBG_TX_TIMEOUT_BUF_LEN - pos,
+ "ntc: %u, ntu: %u, irq enabled: %u\n",
+ ring->ntc, ring->ntu,
+ hbg_irq_is_enabled(priv, HBG_IRQ_TX));
+
+ netdev_info(dev, "%s", buf);
+}
+
static const struct net_device_ops hbg_netdev_ops = {
.ndo_open = hbg_net_open,
.ndo_stop = hbg_net_stop,
+ .ndo_start_xmit = hbg_net_start_xmit,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = hbg_net_set_mac_address,
.ndo_change_mtu = hbg_net_change_mtu,
+ .ndo_tx_timeout = hbg_net_tx_timeout,
};
static const u32 hbg_mode_ability[] = {
@@ -140,6 +164,14 @@ static int hbg_init(struct net_device *netdev)
if (ret)
return ret;
+ ret = hbg_txrx_init(priv);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&priv->pdev->dev, hbg_txrx_uninit, priv);
+ if (ret)
+ return ret;
+
ret = hbg_irq_init(priv);
if (ret)
return ret;
@@ -205,6 +237,7 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
return ret;
+ netdev->watchdog_timeo = HBG_TX_TIMEOUT;
netdev->max_mtu = priv->dev_specs.max_mtu;
netdev->min_mtu = priv->dev_specs.min_mtu;
hbg_net_change_mtu(netdev, HBG_DEFAULT_MTU_SIZE);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
index f9652e5c06b2..05be9ef5f2fc 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.h
@@ -9,5 +9,6 @@
#define HBG_DEV_NAME "hibmcge"
#define HBG_MEM_BAR 0
#define HBG_MOD_VERSION "1.0"
+#define HBG_TX_TIMEOUT (5 * HZ)
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index 86f1157a2af2..b5a78ea8927a 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -48,6 +48,11 @@
#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_CF_CFF_DATA_NUM_ADDR (HBG_REG_SGMII_BASE + 0x045C)
+#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)
+#define HBG_REG_TX_CFF_ADDR_3_ADDR (HBG_REG_SGMII_BASE + 0x0494)
#define HBG_REG_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
#define HBG_REG_RX_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04F0)
@@ -71,6 +76,7 @@
#define HBG_REG_IND_INTR_MASK_B BIT(0)
#define HBG_REG_PORT_ENABLE_RX_B BIT(1)
#define HBG_REG_PORT_ENABLE_TX_B BIT(2)
+#define HBG_REG_CF_CFF_DATA_NUM_ADDR_TX_M GENMASK(8, 0)
enum hbg_port_mode {
/* 0x0 ~ 0x5 are reserved */
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
index 406eac051c10..6a2a585451d7 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
@@ -169,4 +169,33 @@ struct hbg_intr_mask {
};
};
+struct hbg_tx_desc {
+ union {
+ struct {
+ u32 l4_cs : 1;
+ u32 wb : 1;
+ u32 l3_cs : 1;
+ u32 rsv0 : 16;
+ u32 ds : 6;
+ u32 sd : 1;
+ u32 ipoff : 5;
+ u32 rsv1 : 1;
+ };
+ u32 word0;
+ };
+ union {
+ struct {
+ u32 rsv2 : 4;
+ u32 len : 16;
+ u32 back : 6;
+ u32 fm : 1;
+ u32 l2cache : 1;
+ u32 rsv3 : 4;
+ };
+ u32 word1;
+ };
+ u32 pkt_addr; /* word2 */
+ u32 clear_addr; /* word3 */
+};
+
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
new file mode 100644
index 000000000000..00b4d5951c1e
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include "hbg_common.h"
+#include "hbg_irq.h"
+#include "hbg_reg.h"
+#include "hbg_txrx.h"
+
+#define netdev_get_tx_ring(netdev) (&(((struct hbg_priv *)netdev_priv(netdev))->tx_ring))
+
+#define buffer_to_dma_dir(buffer) (((buffer)->dir == HBG_DIR_RX) ? \
+ DMA_FROM_DEVICE : DMA_TO_DEVICE)
+
+#define hbg_queue_is_full(head, tail, ring) ((head) == ((tail) + 1) % (ring)->len)
+#define hbg_queue_is_empty(head, tail) ((head) == (tail))
+#define hbg_queue_next_prt(p, ring) (((p) + 1) % (ring)->len)
+
+static int hbg_dma_map(struct hbg_buffer *buffer)
+{
+ struct hbg_priv *priv = buffer->priv;
+
+ 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)))
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void hbg_dma_unmap(struct hbg_buffer *buffer)
+{
+ struct hbg_priv *priv = buffer->priv;
+
+ if (unlikely(!buffer->skb_dma))
+ return;
+
+ dma_unmap_single(&priv->pdev->dev, buffer->skb_dma, buffer->skb_len,
+ buffer_to_dma_dir(buffer));
+ buffer->skb_dma = 0;
+}
+
+static void hbg_init_tx_desc(struct hbg_buffer *buffer,
+ struct hbg_tx_desc *tx_desc)
+{
+ tx_desc->word0 = 0;
+ tx_desc->word1 = 0;
+
+ tx_desc->wb = HBG_STATUS_ENABLE;
+ tx_desc->len = buffer->skb->len;
+ tx_desc->clear_addr = buffer->state_dma;
+ tx_desc->pkt_addr = buffer->skb_dma;
+ tx_desc->ipoff = buffer->skb->network_header - buffer->skb->mac_header;
+
+ if (likely(buffer->skb->ip_summed == CHECKSUM_PARTIAL)) {
+ tx_desc->l3_cs = 1;
+ tx_desc->l4_cs = 1;
+ }
+}
+
+netdev_tx_t hbg_net_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
+{
+ struct hbg_ring *ring = netdev_get_tx_ring(net_dev);
+ struct hbg_priv *priv = netdev_priv(net_dev);
+ /* This smp_load_acquire() pairs with smp_store_release() in
+ * hbg_tx_buffer_recycle() called in tx interrupt handle process.
+ */
+ u32 ntc = smp_load_acquire(&ring->ntc);
+ struct hbg_buffer *buffer;
+ struct hbg_tx_desc tx_desc;
+ u32 ntu = ring->ntu;
+
+ if (unlikely(!hbg_nic_is_open(priv))) {
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
+ }
+
+ if (unlikely(!skb->len || skb->len > hbg_spec_max_frame_len(priv, HBG_DIR_TX))) {
+ dev_kfree_skb_any(skb);
+ net_dev->stats.tx_errors++;
+ return NETDEV_TX_OK;
+ }
+
+ if (unlikely(hbg_queue_is_full(ntc, ntu, ring) ||
+ hbg_fifo_is_full(ring->priv, ring->dir))) {
+ netif_stop_queue(net_dev);
+ return NETDEV_TX_BUSY;
+ }
+
+ buffer = &ring->queue[ntu];
+ buffer->skb = skb;
+ buffer->skb_len = skb->len;
+ if (unlikely(hbg_dma_map(buffer))) {
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
+ }
+
+ buffer->state = HBG_TX_STATE_START;
+ hbg_init_tx_desc(buffer, &tx_desc);
+ hbg_hw_set_tx_desc(priv, &tx_desc);
+
+ /* This smp_store_release() pairs with smp_load_acquire() in
+ * hbg_tx_buffer_recycle() called in tx interrupt handle process.
+ */
+ smp_store_release(&ring->ntu, hbg_queue_next_prt(ntu, ring));
+ net_dev->stats.tx_bytes += skb->len;
+ net_dev->stats.tx_packets++;
+ return NETDEV_TX_OK;
+}
+
+static void hbg_buffer_free_skb(struct hbg_buffer *buffer)
+{
+ if (unlikely(!buffer->skb))
+ return;
+
+ dev_kfree_skb_any(buffer->skb);
+ buffer->skb = NULL;
+}
+
+static void hbg_buffer_free(struct hbg_buffer *buffer)
+{
+ hbg_dma_unmap(buffer);
+ hbg_buffer_free_skb(buffer);
+}
+
+static int hbg_napi_tx_recycle(struct napi_struct *napi, int budget)
+{
+ struct hbg_ring *ring = container_of(napi, struct hbg_ring, napi);
+ struct hbg_priv *priv = ring->priv;
+ /* This smp_load_acquire() pairs with smp_store_release() in
+ * hbg_start_xmit() called in xmit process.
+ */
+ u32 ntu = smp_load_acquire(&ring->ntu);
+ struct hbg_buffer *buffer;
+ u32 ntc = ring->ntc;
+ int packet_done = 0;
+
+ while (!hbg_queue_is_empty(ntc, ntu)) {
+ /* make sure HW write desc complete */
+ dma_rmb();
+
+ buffer = &ring->queue[ntc];
+ if (buffer->state != HBG_TX_STATE_COMPLETE)
+ break;
+
+ hbg_buffer_free(buffer);
+ ntc = hbg_queue_next_prt(ntc, ring);
+ packet_done++;
+ };
+
+ /* This smp_store_release() pairs with smp_load_acquire() in
+ * hbg_start_xmit() called in xmit process.
+ */
+ smp_store_release(&ring->ntc, ntc);
+ netif_wake_queue(priv->netdev);
+ napi_complete(napi);
+ hbg_irq_enable(priv, HBG_IRQ_TX, true);
+ return packet_done;
+}
+
+static void hbg_ring_uninit(struct hbg_ring *ring)
+{
+ struct hbg_buffer *buffer;
+ u32 i;
+
+ if (!ring->queue)
+ return;
+
+ netif_napi_del(&ring->napi);
+
+ for (i = 0; i < ring->len; i++) {
+ buffer = &ring->queue[i];
+ hbg_buffer_free(buffer);
+ buffer->ring = NULL;
+ buffer->priv = NULL;
+ }
+
+ dma_free_coherent(&ring->priv->pdev->dev,
+ ring->len * sizeof(struct hbg_buffer),
+ ring->queue, ring->queue_dma);
+ ring->queue = NULL;
+ ring->queue_dma = 0;
+ ring->len = 0;
+ ring->priv = NULL;
+}
+
+static int hbg_ring_init(struct hbg_priv *priv, struct hbg_ring *ring,
+ enum hbg_dir dir)
+{
+ struct hbg_buffer *buffer;
+ u32 i, len;
+
+ len = hbg_get_spec_fifo_max_num(priv, dir) + 1;
+ ring->queue = dma_alloc_coherent(&priv->pdev->dev,
+ len * sizeof(struct hbg_buffer),
+ &ring->queue_dma, GFP_KERNEL);
+ if (!ring->queue)
+ return -ENOMEM;
+
+ for (i = 0; i < len; i++) {
+ buffer = &ring->queue[i];
+ buffer->skb_len = 0;
+ buffer->dir = dir;
+ buffer->ring = ring;
+ buffer->priv = priv;
+ buffer->state_dma = ring->queue_dma + (i * sizeof(*buffer));
+ }
+
+ ring->dir = dir;
+ ring->priv = priv;
+ ring->ntc = 0;
+ ring->ntu = 0;
+ ring->len = len;
+ return 0;
+}
+
+static int hbg_tx_ring_init(struct hbg_priv *priv)
+{
+ struct hbg_ring *tx_ring = &priv->tx_ring;
+ int ret;
+
+ if (!tx_ring->tout_log_buf)
+ tx_ring->tout_log_buf = devm_kzalloc(&priv->pdev->dev,
+ HBG_TX_TIMEOUT_BUF_LEN,
+ GFP_KERNEL);
+
+ if (!tx_ring->tout_log_buf)
+ return -ENOMEM;
+
+ ret = hbg_ring_init(priv, tx_ring, HBG_DIR_TX);
+ if (ret)
+ return ret;
+
+ netif_napi_add_tx(priv->netdev, &tx_ring->napi, hbg_napi_tx_recycle);
+ return 0;
+}
+
+int hbg_txrx_init(struct hbg_priv *priv)
+{
+ int ret;
+
+ ret = hbg_tx_ring_init(priv);
+ if (ret)
+ dev_err(&priv->pdev->dev,
+ "failed to init tx ring, ret = %d\n", ret);
+
+ return ret;
+}
+
+void hbg_txrx_uninit(void *data)
+{
+ struct hbg_priv *priv = data;
+
+ hbg_ring_uninit(&priv->tx_ring);
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.h
new file mode 100644
index 000000000000..fa00a74a451a
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_TXRX_H
+#define __HBG_TXRX_H
+
+#include <linux/etherdevice.h>
+#include "hbg_hw.h"
+
+static inline u32 hbg_spec_max_frame_len(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ return (dir == HBG_DIR_TX) ? priv->dev_specs.max_frame_len :
+ priv->dev_specs.rx_buf_size;
+}
+
+static inline u32 hbg_get_spec_fifo_max_num(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ return (dir == HBG_DIR_TX) ? priv->dev_specs.tx_fifo_num :
+ priv->dev_specs.rx_fifo_num;
+}
+
+static inline bool hbg_fifo_is_full(struct hbg_priv *priv, enum hbg_dir dir)
+{
+ return hbg_hw_get_fifo_used_num(priv, dir) >=
+ hbg_get_spec_fifo_max_num(priv, dir);
+}
+
+static inline u32 hbg_get_queue_used_num(struct hbg_ring *ring)
+{
+ return (ring->ntu + ring->len - ring->ntc) % ring->len;
+}
+
+netdev_tx_t hbg_net_start_xmit(struct sk_buff *skb, struct net_device *net_dev);
+int hbg_txrx_init(struct hbg_priv *priv);
+void hbg_txrx_uninit(void *data);
+
+#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (5 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 06/10] net: hibmcge: Implement .ndo_start_xmit function Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-07-31 13:23 ` Joe Damato
2024-07-31 9:42 ` [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions Jijie Shao
` (2 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Implement rx_poll function to read the rx descriptor after
receiving the rx interrupt. Adjust the skb based on the
descriptor to complete the reception of the packet.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 5 +
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 10 ++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 1 +
.../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 9 +-
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 2 +
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 2 +
.../hisilicon/hibmcge/hbg_reg_union.h | 65 ++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 157 +++++++++++++++++-
8 files changed, 248 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 760dcf88c0cf..25563af04897 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -16,11 +16,15 @@
#define HBG_RX_SKIP1 0x00
#define HBG_RX_SKIP2 0x01
#define HBG_PCU_CACHE_LINE_SIZE 32
+#define HBG_RX_DESCR 0x01
#define HBG_LINK_DOWN 0
#define HBG_LINK_UP 1
#define HBG_TX_TIMEOUT_BUF_LEN 1024
+#define HBG_PACKET_HEAD_SIZE ((HBG_RX_SKIP1 + HBG_RX_SKIP2 + HBG_RX_DESCR) * \
+ HBG_PCU_CACHE_LINE_SIZE)
+
enum hbg_tx_state {
HBG_TX_STATE_COMPLETE = 0, /* clear state, must fix to 0 */
HBG_TX_STATE_START,
@@ -141,6 +145,7 @@ struct hbg_priv {
struct hbg_mac mac;
struct hbg_vector vectors;
struct hbg_ring tx_ring;
+ struct hbg_ring rx_ring;
};
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index 683ad03f47d7..0a26055337b6 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -69,6 +69,7 @@ int hbg_hw_dev_specs_init(struct hbg_priv *priv)
return -EADDRNOTAVAIL;
dev_specs->max_frame_len = HBG_PCU_CACHE_LINE_SIZE + dev_specs->max_mtu;
+ dev_specs->rx_buf_size = HBG_PACKET_HEAD_SIZE + dev_specs->max_frame_len;
return 0;
}
@@ -118,6 +119,10 @@ u32 hbg_hw_get_fifo_used_num(struct hbg_priv *priv, enum hbg_dir dir)
return hbg_reg_read_field(priv, HBG_REG_CF_CFF_DATA_NUM_ADDR,
HBG_REG_CF_CFF_DATA_NUM_ADDR_TX_M);
+ if (hbg_dir_has_rx(dir))
+ return hbg_reg_read_field(priv, HBG_REG_CF_CFF_DATA_NUM_ADDR,
+ HBG_REG_CF_CFF_DATA_NUM_ADDR_RX_M);
+
return 0;
}
@@ -129,6 +134,11 @@ void hbg_hw_set_tx_desc(struct hbg_priv *priv, struct hbg_tx_desc *tx_desc)
hbg_reg_write(priv, HBG_REG_TX_CFF_ADDR_3_ADDR, tx_desc->clear_addr);
}
+void hbg_hw_fill_buffer(struct hbg_priv *priv, u32 buffer_dma_addr)
+{
+ hbg_reg_write(priv, HBG_REG_RX_CFF_ADDR_ADDR, buffer_dma_addr);
+}
+
void hbg_hw_get_err_intr_status(struct hbg_priv *priv, struct hbg_intr_status *status)
{
status->bits = hbg_reg_read(priv, HBG_REG_CF_INTRPT_STAT_ADDR);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index cb23b239f42a..1e14d0dbeb26 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -73,5 +73,6 @@ void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu);
int hbg_hw_init(struct hbg_priv *pri);
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);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
index ed51b4f5d5ec..59d28e306a98 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
@@ -12,9 +12,14 @@ static void hbg_irq_handle_tx(struct hbg_priv *priv, struct hbg_irq_info *irq_in
napi_schedule(&priv->tx_ring.napi);
}
+static void hbg_irq_handle_rx(struct hbg_priv *priv, struct hbg_irq_info *irq_info)
+{
+ napi_schedule(&priv->rx_ring.napi);
+}
+
static struct hbg_irq_info hbg_irqs[] = {
{ "TX", HBG_IRQ_TX, 0, hbg_irq_handle_tx },
- { "RX", HBG_IRQ_RX, 0, NULL },
+ { "RX", HBG_IRQ_RX, 0, hbg_irq_handle_rx },
{ "RX_BUF_AVL", HBG_IRQ_BUF_AVL, 0, NULL },
{ "MAC_MII_FIFO_ERR", HBG_IRQ_MAC_MII_FIFO_ERR, 0, NULL },
{ "MAC_PCS_RX_FIFO_ERR", HBG_IRQ_MAC_PCS_RX_FIFO_ERR, 0, NULL },
@@ -93,7 +98,7 @@ static void hbg_irq_info_handle(struct hbg_priv *priv,
if (irq_info->irq_handle)
irq_info->irq_handle(priv, irq_info);
- if (irq_info->mask == HBG_IRQ_TX)
+ if (irq_info->mask == HBG_IRQ_TX || irq_info->mask == HBG_IRQ_RX)
return;
hbg_irq_enable(priv, irq_info->mask, true);
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 8efeea9b0c26..bb5f8321da8a 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -36,6 +36,7 @@ static int hbg_net_open(struct net_device *dev)
return 0;
netif_carrier_off(dev);
+ napi_enable(&priv->rx_ring.napi);
napi_enable(&priv->tx_ring.napi);
hbg_enable_intr(priv, true);
hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
@@ -57,6 +58,7 @@ static int hbg_net_stop(struct net_device *dev)
hbg_hw_mac_enable(priv, HBG_STATUS_DISABLE);
hbg_enable_intr(priv, false);
napi_disable(&priv->tx_ring.napi);
+ napi_disable(&priv->rx_ring.napi);
hbg_phy_stop(priv);
return 0;
}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index b5a78ea8927a..7f579de39ff5 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -53,6 +53,7 @@
#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)
#define HBG_REG_TX_CFF_ADDR_3_ADDR (HBG_REG_SGMII_BASE + 0x0494)
+#define HBG_REG_RX_CFF_ADDR_ADDR (HBG_REG_SGMII_BASE + 0x04A0)
#define HBG_REG_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
#define HBG_REG_RX_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04F0)
@@ -77,6 +78,7 @@
#define HBG_REG_PORT_ENABLE_RX_B BIT(1)
#define HBG_REG_PORT_ENABLE_TX_B BIT(2)
#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)
enum hbg_port_mode {
/* 0x0 ~ 0x5 are reserved */
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
index 6a2a585451d7..d6d511f836b4 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg_union.h
@@ -198,4 +198,69 @@ struct hbg_tx_desc {
u32 clear_addr; /* word3 */
};
+struct hbg_rx_desc {
+ union {
+ struct {
+ u32 rsv : 3;
+ u32 tt : 2;
+ u32 group : 4;
+ u32 qos : 3;
+ u32 gen_id : 8;
+ u32 rsv1 : 12;
+ };
+ u32 word0;
+ };
+ u32 tag; /* word1 */
+ union {
+ struct {
+ u32 all_skip_len : 9;
+ u32 rsv2 : 3;
+ u32 port_num : 4;
+ u32 len : 16;
+ };
+ u32 word2;
+ };
+ union {
+ struct {
+ u16 vlan;
+ u8 ip_offset;
+ u8 buf_num;
+ };
+ u32 word3;
+ };
+ union {
+ struct {
+ u32 rsv3 : 5;
+ u32 pm : 2;
+ u32 index_match : 1;
+ u32 l2_error : 1;
+ u32 l3_error_code : 4;
+ u32 drop : 1;
+ u32 vlan_flag : 1;
+ u32 icmp : 1;
+ u32 rarp : 1;
+ u32 arp : 1;
+ u32 mul_cst : 1;
+ u32 brd_cst : 1;
+ u32 ip_version_err : 1;
+ u32 opt : 1;
+ u32 frag : 1;
+ u32 l4_error_code : 4;
+ u32 rsv4 : 1;
+ u32 ip_version : 1;
+ u32 ipsec : 1;
+ u32 ip_tcp_udp : 2;
+ };
+ u32 word4;
+ };
+ union {
+ struct {
+ u16 size;
+ u8 rsv5;
+ u8 back;
+ };
+ u32 word5;
+ };
+};
+
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
index 00b4d5951c1e..1bd480e516ec 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_txrx.c
@@ -14,6 +14,7 @@
#define hbg_queue_is_full(head, tail, ring) ((head) == ((tail) + 1) % (ring)->len)
#define hbg_queue_is_empty(head, tail) ((head) == (tail))
#define hbg_queue_next_prt(p, ring) (((p) + 1) % (ring)->len)
+#define hbg_queue_move_next(p, ring) ((ring)->p = hbg_queue_next_prt((ring)->p, (ring)))
static int hbg_dma_map(struct hbg_buffer *buffer)
{
@@ -117,6 +118,20 @@ static void hbg_buffer_free_skb(struct hbg_buffer *buffer)
buffer->skb = NULL;
}
+static int hbg_buffer_alloc_skb(struct hbg_buffer *buffer)
+{
+ u32 len = hbg_spec_max_frame_len(buffer->priv, buffer->dir);
+ struct hbg_priv *priv = buffer->priv;
+
+ buffer->skb = netdev_alloc_skb(priv->netdev, len);
+ if (unlikely(!buffer->skb))
+ return -ENOMEM;
+
+ buffer->skb_len = len;
+ memset(buffer->skb->data, 0, HBG_PACKET_HEAD_SIZE);
+ return 0;
+}
+
static void hbg_buffer_free(struct hbg_buffer *buffer)
{
hbg_dma_unmap(buffer);
@@ -158,6 +173,110 @@ static int hbg_napi_tx_recycle(struct napi_struct *napi, int budget)
return packet_done;
}
+static int hbg_rx_fill_one_buffer(struct hbg_priv *priv)
+{
+ struct hbg_ring *ring = &priv->rx_ring;
+ struct hbg_buffer *buffer;
+ int ret;
+
+ buffer = &ring->queue[ring->ntu];
+ ret = hbg_buffer_alloc_skb(buffer);
+ if (unlikely(ret))
+ return ret;
+
+ ret = hbg_dma_map(buffer);
+ if (unlikely(ret)) {
+ hbg_buffer_free_skb(buffer);
+ return ret;
+ }
+
+ hbg_hw_fill_buffer(priv, buffer->skb_dma);
+ hbg_queue_move_next(ntu, ring);
+ return 0;
+}
+
+static int hbg_rx_fill_buffers(struct hbg_priv *priv)
+{
+ struct hbg_ring *ring = &priv->rx_ring;
+ int ret;
+
+ while (!(hbg_fifo_is_full(priv, ring->dir) ||
+ hbg_queue_is_full(ring->ntc, ring->ntu, ring))) {
+ ret = hbg_rx_fill_one_buffer(priv);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static bool hbg_sync_data_from_hw(struct hbg_priv *priv,
+ struct hbg_buffer *buffer)
+{
+ struct hbg_rx_desc *rx_desc;
+
+ /* make sure HW write desc complete */
+ dma_rmb();
+
+ dma_sync_single_for_cpu(&priv->pdev->dev, buffer->skb_dma,
+ buffer->skb_len, DMA_FROM_DEVICE);
+
+ rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
+ return rx_desc->len != 0;
+}
+
+static int hbg_napi_rx_poll(struct napi_struct *napi, int budget)
+{
+ struct hbg_ring *ring = container_of(napi, struct hbg_ring, napi);
+ struct hbg_priv *priv = ring->priv;
+ struct hbg_rx_desc *rx_desc;
+ struct hbg_buffer *buffer;
+ u32 packet_done = 0;
+
+ if (unlikely(!hbg_nic_is_open(priv))) {
+ napi_complete(napi);
+ return 0;
+ }
+
+ while (packet_done < budget) {
+ if (unlikely(hbg_queue_is_empty(ring->ntc, ring->ntu)))
+ break;
+
+ buffer = &ring->queue[ring->ntc];
+ if (unlikely(!buffer->skb))
+ goto next_buffer;
+
+ if (unlikely(!hbg_sync_data_from_hw(priv, buffer)))
+ break;
+
+ hbg_dma_unmap(buffer);
+
+ rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
+ skb_reserve(buffer->skb, HBG_PACKET_HEAD_SIZE + NET_IP_ALIGN);
+ skb_put(buffer->skb, rx_desc->len);
+ buffer->skb->protocol = eth_type_trans(buffer->skb, priv->netdev);
+
+ priv->netdev->stats.rx_bytes += rx_desc->len;
+ priv->netdev->stats.rx_packets++;
+ netif_receive_skb(buffer->skb);
+ buffer->skb = NULL;
+ hbg_rx_fill_one_buffer(priv);
+
+next_buffer:
+ hbg_queue_move_next(ntc, ring);
+ packet_done++;
+ }
+
+ hbg_rx_fill_buffers(priv);
+ if (packet_done >= budget)
+ return packet_done;
+
+ napi_complete(napi);
+ hbg_irq_enable(priv, HBG_IRQ_RX, true);
+
+ return packet_done;
+}
+
static void hbg_ring_uninit(struct hbg_ring *ring)
{
struct hbg_buffer *buffer;
@@ -235,15 +354,50 @@ static int hbg_tx_ring_init(struct hbg_priv *priv)
return 0;
}
+static int hbg_rx_ring_init(struct hbg_priv *priv)
+{
+ struct hbg_ring *rx_ring = &priv->rx_ring;
+ int ret;
+
+ ret = hbg_ring_init(priv, rx_ring, HBG_DIR_RX);
+ if (ret)
+ return ret;
+
+ netif_napi_add(priv->netdev, &priv->rx_ring.napi, hbg_napi_rx_poll);
+ return 0;
+}
+
int hbg_txrx_init(struct hbg_priv *priv)
{
int ret;
ret = hbg_tx_ring_init(priv);
- if (ret)
+ if (ret) {
dev_err(&priv->pdev->dev,
"failed to init tx ring, ret = %d\n", ret);
+ return ret;
+ }
+
+ ret = hbg_rx_ring_init(priv);
+ if (ret) {
+ dev_err(&priv->pdev->dev,
+ "failed to init rx ring, ret = %d\n", ret);
+ goto err_uninit_tx;
+ }
+
+ ret = hbg_rx_fill_buffers(priv);
+ if (ret) {
+ dev_err(&priv->pdev->dev,
+ "failed to fill rx buffers, ret = %d\n", ret);
+ goto err_uninit_rx;
+ }
+ return 0;
+
+err_uninit_rx:
+ hbg_ring_uninit(&priv->rx_ring);
+err_uninit_tx:
+ hbg_ring_uninit(&priv->tx_ring);
return ret;
}
@@ -252,4 +406,5 @@ void hbg_txrx_uninit(void *data)
struct hbg_priv *priv = data;
hbg_ring_uninit(&priv->tx_ring);
+ hbg_ring_uninit(&priv->rx_ring);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets
2024-07-31 9:42 ` [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets Jijie Shao
@ 2024-07-31 13:23 ` Joe Damato
2024-08-01 11:58 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Joe Damato @ 2024-07-31 13:23 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Wed, Jul 31, 2024 at 05:42:42PM +0800, Jijie Shao wrote:
> Implement rx_poll function to read the rx descriptor after
> receiving the rx interrupt. Adjust the skb based on the
> descriptor to complete the reception of the packet.
>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> .../ethernet/hisilicon/hibmcge/hbg_common.h | 5 +
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 10 ++
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 1 +
> .../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 9 +-
> .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 2 +
> .../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 2 +
> .../hisilicon/hibmcge/hbg_reg_union.h | 65 ++++++++
> .../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 157 +++++++++++++++++-
> 8 files changed, 248 insertions(+), 3 deletions(-)
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> index 8efeea9b0c26..bb5f8321da8a 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> @@ -36,6 +36,7 @@ static int hbg_net_open(struct net_device *dev)
> return 0;
>
> netif_carrier_off(dev);
> + napi_enable(&priv->rx_ring.napi);
> napi_enable(&priv->tx_ring.napi);
> hbg_enable_intr(priv, true);
> hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
In the future, it might be good to consider using:
- netif_napi_set_irq
- netif_queue_set_napi
to link NAPIs with IRQs and queues.
For an example, see 64b62146ba9e ("net/mlx4: link NAPI instances to
queues and IRQs).
[...]
> +static int hbg_rx_fill_buffers(struct hbg_priv *priv)
> +{
> + struct hbg_ring *ring = &priv->rx_ring;
> + int ret;
> +
> + while (!(hbg_fifo_is_full(priv, ring->dir) ||
> + hbg_queue_is_full(ring->ntc, ring->ntu, ring))) {
> + ret = hbg_rx_fill_one_buffer(priv);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static bool hbg_sync_data_from_hw(struct hbg_priv *priv,
> + struct hbg_buffer *buffer)
> +{
> + struct hbg_rx_desc *rx_desc;
> +
> + /* make sure HW write desc complete */
> + dma_rmb();
> +
> + dma_sync_single_for_cpu(&priv->pdev->dev, buffer->skb_dma,
> + buffer->skb_len, DMA_FROM_DEVICE);
> +
> + rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
> + return rx_desc->len != 0;
> +}
Have you looked into using the page pool to simplify some of the
logic above?
> +static int hbg_napi_rx_poll(struct napi_struct *napi, int budget)
> +{
> + struct hbg_ring *ring = container_of(napi, struct hbg_ring, napi);
> + struct hbg_priv *priv = ring->priv;
> + struct hbg_rx_desc *rx_desc;
> + struct hbg_buffer *buffer;
> + u32 packet_done = 0;
> +
> + if (unlikely(!hbg_nic_is_open(priv))) {
> + napi_complete(napi);
> + return 0;
> + }
> +
> + while (packet_done < budget) {
> + if (unlikely(hbg_queue_is_empty(ring->ntc, ring->ntu)))
> + break;
> +
> + buffer = &ring->queue[ring->ntc];
> + if (unlikely(!buffer->skb))
> + goto next_buffer;
> +
> + if (unlikely(!hbg_sync_data_from_hw(priv, buffer)))
> + break;
> +
> + hbg_dma_unmap(buffer);
> +
> + rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
> + skb_reserve(buffer->skb, HBG_PACKET_HEAD_SIZE + NET_IP_ALIGN);
> + skb_put(buffer->skb, rx_desc->len);
> + buffer->skb->protocol = eth_type_trans(buffer->skb, priv->netdev);
> +
> + priv->netdev->stats.rx_bytes += rx_desc->len;
> + priv->netdev->stats.rx_packets++;
> + netif_receive_skb(buffer->skb);
Any reason why not napi_gro_receive ?
> + buffer->skb = NULL;
> + hbg_rx_fill_one_buffer(priv);
> +
> +next_buffer:
> + hbg_queue_move_next(ntc, ring);
> + packet_done++;
> + }
> +
> + hbg_rx_fill_buffers(priv);
> + if (packet_done >= budget)
> + return packet_done;
> +
> + napi_complete(napi);
Maybe:
if (napi_complete_done(napi))
hbg_irq_enable(priv, HBG_IRQ_RX, true);
> + hbg_irq_enable(priv, HBG_IRQ_RX, true);
> +
> + return packet_done;
> +}
[...]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets
2024-07-31 13:23 ` Joe Damato
@ 2024-08-01 11:58 ` Jijie Shao
0 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 11:58 UTC (permalink / raw)
To: Joe Damato
Cc: shaojijie, yisen.zhuang@huawei.com, salil.mehta@huawei.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, shenjian15@huawei.com, wangpeiyang1@huawei.com,
liuyonglong@huawei.com, sudongming (A), xujunsheng,
shiyongbang (A), netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
on 2024/7/31 21:23, Joe Damato wrote:
> On Wed, Jul 31, 2024 at 05:42:42PM +0800, Jijie Shao wrote:
>> Implement rx_poll function to read the rx descriptor after
>> receiving the rx interrupt. Adjust the skb based on the
>> descriptor to complete the reception of the packet.
>>
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>> ---
>> .../ethernet/hisilicon/hibmcge/hbg_common.h | 5 +
>> .../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 10 ++
>> .../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 1 +
>> .../net/ethernet/hisilicon/hibmcge/hbg_irq.c | 9 +-
>> .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 2 +
>> .../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 2 +
>> .../hisilicon/hibmcge/hbg_reg_union.h | 65 ++++++++
>> .../net/ethernet/hisilicon/hibmcge/hbg_txrx.c | 157 +++++++++++++++++-
>> 8 files changed, 248 insertions(+), 3 deletions(-)
>
>> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
>> index 8efeea9b0c26..bb5f8321da8a 100644
>> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
>> @@ -36,6 +36,7 @@ static int hbg_net_open(struct net_device *dev)
>> return 0;
>>
>> netif_carrier_off(dev);
>> + napi_enable(&priv->rx_ring.napi);
>> napi_enable(&priv->tx_ring.napi);
>> hbg_enable_intr(priv, true);
>> hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
> In the future, it might be good to consider using:
> - netif_napi_set_irq
> - netif_queue_set_napi
>
> to link NAPIs with IRQs and queues.
>
Sounds good, but I can't find these two functions in 6.4 kernel?
>
>> +static int hbg_rx_fill_buffers(struct hbg_priv *priv)
>> +{
>> + struct hbg_ring *ring = &priv->rx_ring;
>> + int ret;
>> +
>> + while (!(hbg_fifo_is_full(priv, ring->dir) ||
>> + hbg_queue_is_full(ring->ntc, ring->ntu, ring))) {
>> + ret = hbg_rx_fill_one_buffer(priv);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool hbg_sync_data_from_hw(struct hbg_priv *priv,
>> + struct hbg_buffer *buffer)
>> +{
>> + struct hbg_rx_desc *rx_desc;
>> +
>> + /* make sure HW write desc complete */
>> + dma_rmb();
>> +
>> + dma_sync_single_for_cpu(&priv->pdev->dev, buffer->skb_dma,
>> + buffer->skb_len, DMA_FROM_DEVICE);
>> +
>> + rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
>> + return rx_desc->len != 0;
>> +}
> Have you looked into using the page pool to simplify some of the
> logic above?
Thanks, but I probably won't use it at the moment.
>
>> +static int hbg_napi_rx_poll(struct napi_struct *napi, int budget)
>> +{
>> + struct hbg_ring *ring = container_of(napi, struct hbg_ring, napi);
>> + struct hbg_priv *priv = ring->priv;
>> + struct hbg_rx_desc *rx_desc;
>> + struct hbg_buffer *buffer;
>> + u32 packet_done = 0;
>> +
>> + if (unlikely(!hbg_nic_is_open(priv))) {
>> + napi_complete(napi);
>> + return 0;
>> + }
>> +
>> + while (packet_done < budget) {
>> + if (unlikely(hbg_queue_is_empty(ring->ntc, ring->ntu)))
>> + break;
>> +
>> + buffer = &ring->queue[ring->ntc];
>> + if (unlikely(!buffer->skb))
>> + goto next_buffer;
>> +
>> + if (unlikely(!hbg_sync_data_from_hw(priv, buffer)))
>> + break;
>> +
>> + hbg_dma_unmap(buffer);
>> +
>> + rx_desc = (struct hbg_rx_desc *)buffer->skb->data;
>> + skb_reserve(buffer->skb, HBG_PACKET_HEAD_SIZE + NET_IP_ALIGN);
>> + skb_put(buffer->skb, rx_desc->len);
>> + buffer->skb->protocol = eth_type_trans(buffer->skb, priv->netdev);
>> +
>> + priv->netdev->stats.rx_bytes += rx_desc->len;
>> + priv->netdev->stats.rx_packets++;
>> + netif_receive_skb(buffer->skb);
> Any reason why not napi_gro_receive ?
Is it OK if the MAC does not support gro?
>
>> + buffer->skb = NULL;
>> + hbg_rx_fill_one_buffer(priv);
>> +
>> +next_buffer:
>> + hbg_queue_move_next(ntc, ring);
>> + packet_done++;
>> + }
>> +
>> + hbg_rx_fill_buffers(priv);
>> + if (packet_done >= budget)
>> + return packet_done;
>> +
>> + napi_complete(napi);
> Maybe:
>
> if (napi_complete_done(napi))
> hbg_irq_enable(priv, HBG_IRQ_RX, true);
okay, I will fix it in v2
Thanks again,
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (6 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 07/10] net: hibmcge: Implement rx_poll function to receive packets Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-08-01 1:10 ` Andrew Lunn
2024-07-31 9:42 ` [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge Jijie Shao
2024-07-31 9:42 ` [RFC PATCH net-next 10/10] net: hibmcge: Add maintainer " Jijie Shao
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Implement a workqueue in this module, The workqueue is invoked
once every second to update link status.
Implement the .get_drvinfo .get_link .get_link_ksettings to get
the basic information and working status of the driver.
Implement the .set_link_ksettings to modify the rate, duplex,
and auto-negotiation status.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 1 +
.../ethernet/hisilicon/hibmcge/hbg_ethtool.c | 56 +++++++++++
.../ethernet/hisilicon/hibmcge/hbg_ethtool.h | 11 +++
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 96 ++++++++++++++++++-
4 files changed, 161 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 25563af04897..45ec4b463e70 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -146,6 +146,7 @@ struct hbg_priv {
struct hbg_vector vectors;
struct hbg_ring tx_ring;
struct hbg_ring rx_ring;
+ 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
new file mode 100644
index 000000000000..3acd6eae189e
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+#include "hbg_common.h"
+#include "hbg_ethtool.h"
+#include "hbg_hw.h"
+#include "hbg_main.h"
+#include "hbg_mdio.h"
+
+static void hbg_ethtool_get_drvinfo(struct net_device *netdev,
+ struct ethtool_drvinfo *drvinfo)
+{
+ strscpy(drvinfo->version, HBG_MOD_VERSION, sizeof(drvinfo->version));
+ drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
+}
+
+static u32 hbg_ethtool_get_link(struct net_device *netdev)
+{
+ struct hbg_priv *priv = netdev_priv(netdev);
+
+ return priv->mac.link_status;
+}
+
+static int hbg_ethtool_get_ksettings(struct net_device *netdev,
+ struct ethtool_link_ksettings *ksettings)
+{
+ struct hbg_priv *priv = netdev_priv(netdev);
+
+ phy_ethtool_ksettings_get(priv->mac.phydev, ksettings);
+ return 0;
+}
+
+static int hbg_ethtool_set_ksettings(struct net_device *netdev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct hbg_priv *priv = netdev_priv(netdev);
+
+ if (cmd->base.speed == SPEED_1000 && cmd->base.duplex == DUPLEX_HALF)
+ return -EINVAL;
+
+ return phy_ethtool_ksettings_set(priv->mac.phydev, cmd);
+}
+
+static const struct ethtool_ops hbg_ethtool_ops = {
+ .get_drvinfo = hbg_ethtool_get_drvinfo,
+ .get_link = hbg_ethtool_get_link,
+ .get_link_ksettings = hbg_ethtool_get_ksettings,
+ .set_link_ksettings = hbg_ethtool_set_ksettings,
+};
+
+void hbg_ethtool_set_ops(struct net_device *netdev)
+{
+ netdev->ethtool_ops = &hbg_ethtool_ops;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
new file mode 100644
index 000000000000..628707ec2686
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef __HBG_ETHTOOL_H
+#define __HBG_ETHTOOL_H
+
+#include <linux/netdevice.h>
+
+void hbg_ethtool_set_ops(struct net_device *netdev);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index bb5f8321da8a..bea596123c37 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -6,12 +6,15 @@
#include <linux/netdevice.h>
#include <linux/pci.h>
#include "hbg_common.h"
+#include "hbg_ethtool.h"
#include "hbg_hw.h"
#include "hbg_irq.h"
#include "hbg_main.h"
#include "hbg_mdio.h"
#include "hbg_txrx.h"
+static struct workqueue_struct *hbg_workqueue;
+
static void hbg_enable_intr(struct hbg_priv *priv, bool enabled)
{
u32 i;
@@ -136,6 +139,52 @@ static const struct net_device_ops hbg_netdev_ops = {
.ndo_tx_timeout = hbg_net_tx_timeout,
};
+static void hbg_update_link_status(struct hbg_priv *priv)
+{
+ u32 link;
+
+ link = hbg_get_link_status(priv);
+ if (link == priv->mac.link_status)
+ return;
+
+ priv->mac.link_status = link;
+ if (link == HBG_LINK_DOWN) {
+ netif_carrier_off(priv->netdev);
+ netif_tx_stop_all_queues(priv->netdev);
+ dev_info(&priv->pdev->dev, "link down!");
+ } else {
+ netif_tx_wake_all_queues(priv->netdev);
+ netif_carrier_on(priv->netdev);
+ dev_info(&priv->pdev->dev, "link up!");
+ }
+}
+
+static void hbg_service_task(struct work_struct *work)
+{
+ struct hbg_priv *priv =
+ container_of(work, struct hbg_priv, service_task.work);
+
+ hbg_update_link_status(priv);
+
+ mod_delayed_work(hbg_workqueue, &priv->service_task,
+ round_jiffies_relative(HZ));
+}
+
+static void hbg_delaywork_init(struct hbg_priv *priv)
+{
+ INIT_DELAYED_WORK(&priv->service_task, hbg_service_task);
+ mod_delayed_work(hbg_workqueue, &priv->service_task,
+ round_jiffies_relative(HZ));
+}
+
+static void hbg_delaywork_uninit(void *data)
+{
+ struct hbg_priv *priv = data;
+
+ if (priv->service_task.work.func)
+ cancel_delayed_work_sync(&priv->service_task);
+}
+
static const u32 hbg_mode_ability[] = {
ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
ETHTOOL_LINK_MODE_100baseT_Full_BIT,
@@ -177,12 +226,17 @@ static int hbg_init(struct net_device *netdev)
ret = hbg_irq_init(priv);
if (ret)
return ret;
-
ret = devm_add_action_or_reset(&priv->pdev->dev, hbg_irq_uninit, priv);
if (ret)
return ret;
- return hbg_mac_init(priv);
+ ret = hbg_mac_init(priv);
+ if (ret)
+ return ret;
+
+ hbg_delaywork_init(priv);
+ return devm_add_action_or_reset(&priv->pdev->dev,
+ hbg_delaywork_uninit, priv);
}
static int hbg_pci_init(struct pci_dev *pdev)
@@ -249,6 +303,7 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return dev_err_probe(&pdev->dev, ret,
"failed to register netdev\n");
+ hbg_ethtool_set_ops(netdev);
set_bit(HBG_NIC_STATE_INITED, &priv->state);
return 0;
}
@@ -264,7 +319,42 @@ 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_workqueue = alloc_workqueue("%s", WQ_UNBOUND, 0, HBG_DEV_NAME);
+ if (!hbg_workqueue) {
+ pr_err("%s: failed to create workqueue\n", HBG_DEV_NAME);
+ return -ENOMEM;
+ }
+
+ ret = pci_register_driver(&hbg_driver);
+ if (ret) {
+ pr_err("%s: failed to register PCI driver, ret = %d\n",
+ HBG_DEV_NAME, ret);
+ goto err_destroy_workqueue;
+ }
+
+ return 0;
+
+err_destroy_workqueue:
+ destroy_workqueue(hbg_workqueue);
+ hbg_workqueue = NULL;
+
+ return ret;
+}
+module_init(hbg_module_init);
+
+static void __exit hbg_module_exit(void)
+{
+ pci_unregister_driver(&hbg_driver);
+ flush_workqueue(hbg_workqueue);
+ destroy_workqueue(hbg_workqueue);
+ hbg_workqueue = NULL;
+}
+module_exit(hbg_module_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-07-31 9:42 ` [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions Jijie Shao
@ 2024-08-01 1:10 ` Andrew Lunn
2024-08-01 11:10 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 1:10 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> +static void hbg_ethtool_get_drvinfo(struct net_device *netdev,
> + struct ethtool_drvinfo *drvinfo)
> +{
> + strscpy(drvinfo->version, HBG_MOD_VERSION, sizeof(drvinfo->version));
> + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
A version is pointless, it tells you nothing useful. If you don't
touch version, the core will fill it with the uname, so you know
exactly what kernel this is, which is useful.
> +static u32 hbg_ethtool_get_link(struct net_device *netdev)
> +{
> + struct hbg_priv *priv = netdev_priv(netdev);
> +
> + return priv->mac.link_status;
> +}
> +
> +static int hbg_ethtool_get_ksettings(struct net_device *netdev,
> + struct ethtool_link_ksettings *ksettings)
> +{
> + struct hbg_priv *priv = netdev_priv(netdev);
> +
> + phy_ethtool_ksettings_get(priv->mac.phydev, ksettings);
You can avoid this wrapper since phy_attach_direct sets netdev->phydev
to phydev. You can then call phy_ethtool_get_link_ksettings() etc.
> +static int hbg_ethtool_set_ksettings(struct net_device *netdev,
> + const struct ethtool_link_ksettings *cmd)
> +{
> + struct hbg_priv *priv = netdev_priv(netdev);
> +
> + if (cmd->base.speed == SPEED_1000 && cmd->base.duplex == DUPLEX_HALF)
> + return -EINVAL;
So long as you have told phylib about not supporting 1000Base/Half,
phy_ethtool_set_link_ksettings() will return an error for you.
> +static const struct ethtool_ops hbg_ethtool_ops = {
> + .get_drvinfo = hbg_ethtool_get_drvinfo,
> + .get_link = hbg_ethtool_get_link,
Why not ethtool_op_get_link() ?
> + .get_link_ksettings = hbg_ethtool_get_ksettings,
> + .set_link_ksettings = hbg_ethtool_set_ksettings,
> +};
> +static void hbg_update_link_status(struct hbg_priv *priv)
> +{
> + u32 link;
> +
> + link = hbg_get_link_status(priv);
> + if (link == priv->mac.link_status)
> + return;
> +
> + priv->mac.link_status = link;
> + if (link == HBG_LINK_DOWN) {
> + netif_carrier_off(priv->netdev);
> + netif_tx_stop_all_queues(priv->netdev);
> + dev_info(&priv->pdev->dev, "link down!");
> + } else {
> + netif_tx_wake_all_queues(priv->netdev);
> + netif_carrier_on(priv->netdev);
> + dev_info(&priv->pdev->dev, "link up!");
> + }
> +}
Why do you need this? phylib will poll the PHY once per second and
call the adjust_link callback whenever the link changes state.
> @@ -177,12 +226,17 @@ static int hbg_init(struct net_device *netdev)
> ret = hbg_irq_init(priv);
> if (ret)
> return ret;
> -
> ret = devm_add_action_or_reset(&priv->pdev->dev, hbg_irq_uninit, priv);
This white space change does not belong here.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-08-01 1:10 ` Andrew Lunn
@ 2024-08-01 11:10 ` Jijie Shao
2024-08-01 12:26 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 11:10 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 9:10, Andrew Lunn wrote:
>> +static void hbg_ethtool_get_drvinfo(struct net_device *netdev,
>> + struct ethtool_drvinfo *drvinfo)
>> +{
>> + strscpy(drvinfo->version, HBG_MOD_VERSION, sizeof(drvinfo->version));
>> + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
> A version is pointless, it tells you nothing useful. If you don't
> touch version, the core will fill it with the uname, so you know
> exactly what kernel this is, which is useful.
>
>> +static u32 hbg_ethtool_get_link(struct net_device *netdev)
>> +{
>> + struct hbg_priv *priv = netdev_priv(netdev);
>> +
>> + return priv->mac.link_status;
>> +}
>> +
>> +static int hbg_ethtool_get_ksettings(struct net_device *netdev,
>> + struct ethtool_link_ksettings *ksettings)
>> +{
>> + struct hbg_priv *priv = netdev_priv(netdev);
>> +
>> + phy_ethtool_ksettings_get(priv->mac.phydev, ksettings);
> You can avoid this wrapper since phy_attach_direct sets netdev->phydev
> to phydev. You can then call phy_ethtool_get_link_ksettings() etc.
Yes, It`s ok
>
>> +static int hbg_ethtool_set_ksettings(struct net_device *netdev,
>> + const struct ethtool_link_ksettings *cmd)
>> +{
>> + struct hbg_priv *priv = netdev_priv(netdev);
>> +
>> + if (cmd->base.speed == SPEED_1000 && cmd->base.duplex == DUPLEX_HALF)
>> + return -EINVAL;
> So long as you have told phylib about not supporting 1000Base/Half,
> phy_ethtool_set_link_ksettings() will return an error for you.
okay,
>
>> +static const struct ethtool_ops hbg_ethtool_ops = {
>> + .get_drvinfo = hbg_ethtool_get_drvinfo,
>> + .get_link = hbg_ethtool_get_link,
> Why not ethtool_op_get_link() ?
It`s a good idea
>
>> + .get_link_ksettings = hbg_ethtool_get_ksettings,
>> + .set_link_ksettings = hbg_ethtool_set_ksettings,
>> +};
>> +static void hbg_update_link_status(struct hbg_priv *priv)
>> +{
>> + u32 link;
>> +
>> + link = hbg_get_link_status(priv);
>> + if (link == priv->mac.link_status)
>> + return;
>> +
>> + priv->mac.link_status = link;
>> + if (link == HBG_LINK_DOWN) {
>> + netif_carrier_off(priv->netdev);
>> + netif_tx_stop_all_queues(priv->netdev);
>> + dev_info(&priv->pdev->dev, "link down!");
>> + } else {
>> + netif_tx_wake_all_queues(priv->netdev);
>> + netif_carrier_on(priv->netdev);
>> + dev_info(&priv->pdev->dev, "link up!");
>> + }
>> +}
> Why do you need this? phylib will poll the PHY once per second and
> call the adjust_link callback whenever the link changes state.
However, we hope that the network port can be linked only when
the PHY and MAC are linked.
The adjust_link callback can ensure that the PHY status is normal,
but cannot ensure that the MAC address is linked.
so, in hbg_get_link_status:
+/* include phy link and mac link */
+u32 hbg_get_link_status(struct hbg_priv *priv)
+{
+ struct phy_device *phydev = priv->mac.phydev;
+ int ret;
+
+ if (!phydev)
+ return HBG_LINK_DOWN;
+
+ phy_read_status(phydev);
+ if ((phydev->state != PHY_UP && phydev->state != PHY_RUNNING) ||
+ !phydev->link)
+ return HBG_LINK_DOWN;
+
+ ret = hbg_hw_sgmii_autoneg(priv);
+ if (ret)
+ return HBG_LINK_DOWN;
+
+ return HBG_LINK_UP;
+}
>
>> @@ -177,12 +226,17 @@ static int hbg_init(struct net_device *netdev)
>> ret = hbg_irq_init(priv);
>> if (ret)
>> return ret;
>> -
>> ret = devm_add_action_or_reset(&priv->pdev->dev, hbg_irq_uninit, priv);
> This white space change does not belong here.
Yes, I'll fix that in V2
Thanks again,
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-08-01 11:10 ` Jijie Shao
@ 2024-08-01 12:26 ` Andrew Lunn
2024-08-01 13:06 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 12:26 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> > Why do you need this? phylib will poll the PHY once per second and
> > call the adjust_link callback whenever the link changes state.
>
> However, we hope that the network port can be linked only when
> the PHY and MAC are linked.
> The adjust_link callback can ensure that the PHY status is normal,
> but cannot ensure that the MAC address is linked.
So why would the SGMII link be down? My experience with SGMII is that
the link comes up as soon as both ends have power. You are also not
using in-band signalling, you configure the MAC based on the
adjust_link callback.
Basically, whenever you do something which no other driver does, you
need to explain why. Do you see any other MAC driver using SGMII doing
this?
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-08-01 12:26 ` Andrew Lunn
@ 2024-08-01 13:06 ` Jijie Shao
2024-08-01 20:16 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 13:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 20:26, Andrew Lunn wrote:
>>> Why do you need this? phylib will poll the PHY once per second and
>>> call the adjust_link callback whenever the link changes state.
>> However, we hope that the network port can be linked only when
>> the PHY and MAC are linked.
>> The adjust_link callback can ensure that the PHY status is normal,
>> but cannot ensure that the MAC address is linked.
> So why would the SGMII link be down? My experience with SGMII is that
> the link comes up as soon as both ends have power. You are also not
> using in-band signalling, you configure the MAC based on the
> adjust_link callback.
>
> Basically, whenever you do something which no other driver does, you
> need to explain why. Do you see any other MAC driver using SGMII doing
> this?
>
> Andrew
Yes, it was my mistake, I should explain why.
If the network port is linked, but the link fails between the SGMII and PHY,
is there any method to find out the cause?
I've had a problem with phy link but SGMII no link due to poor contact.
In this case, the network port no link. Therefore, we can quickly find and analyze the cause.
Or maybe we shouldn't think about the case. because the link is up but packets cannot be received or sent.
Thanks
Jijie Shao
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions
2024-08-01 13:06 ` Jijie Shao
@ 2024-08-01 20:16 ` Andrew Lunn
0 siblings, 0 replies; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 20:16 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
> If the network port is linked, but the link fails between the SGMII and PHY,
> is there any method to find out the cause?
>
> I've had a problem with phy link but SGMII no link due to poor contact.
So a hardware design issue? Has it been solved now, or is there
shipped hardware with this problem?
I would say there is a difference between the first prototype board,
and real products. If the real product has issues, then we should try
to address it. If it is just a prototype board, i would ignore it.
One option might be to expose your PCS as a phylib PCS. Do you get
interrupts from it when it has link up? Link down? phylink will then
combine media side status with PCS status.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (7 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 08/10] net: hibmcge: Implement workqueue and some ethtool_ops functions Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
2024-08-01 1:13 ` Andrew Lunn
2024-07-31 9:42 ` [RFC PATCH net-next 10/10] net: hibmcge: Add maintainer " Jijie Shao
9 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Add a Makefile and update Kconfig to build hibmcge driver.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
drivers/net/ethernet/hisilicon/Kconfig | 17 ++++++++++++++++-
drivers/net/ethernet/hisilicon/Makefile | 1 +
drivers/net/ethernet/hisilicon/hibmcge/Makefile | 10 ++++++++++
3 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/Makefile
diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
index 3312e1d93c3b..372854d15481 100644
--- a/drivers/net/ethernet/hisilicon/Kconfig
+++ b/drivers/net/ethernet/hisilicon/Kconfig
@@ -7,7 +7,7 @@ config NET_VENDOR_HISILICON
bool "Hisilicon devices"
default y
depends on OF || ACPI
- depends on ARM || ARM64 || COMPILE_TEST
+ depends on ARM || ARM64 || COMPILE_TEST || X86_64
help
If you have a network (Ethernet) card belonging to this class, say Y.
@@ -18,6 +18,8 @@ config NET_VENDOR_HISILICON
if NET_VENDOR_HISILICON
+if ARM || ARM64 || COMPILE_TEST
+
config HIX5HD2_GMAC
tristate "Hisilicon HIX5HD2 Family Network Device Support"
select PHYLIB
@@ -141,4 +143,17 @@ config HNS3_ENET
endif #HNS3
+endif # ARM || ARM64 || COMPILE_TEST
+
+config HIBMCGE
+ tristate "Hisilicon BMC Gigabit Ethernet Device Support"
+ default m
+ depends on PCI && PCI_MSI
+ help
+ If you wish to compile a kernel for a BMC with HIBMC-xx_gmac
+ then you should answer Y to this. This makes this driver suitable for use
+ on certain boards such as the HIBMC-210.
+
+ If you are unsure, say N.
+
endif # NET_VENDOR_HISILICON
diff --git a/drivers/net/ethernet/hisilicon/Makefile b/drivers/net/ethernet/hisilicon/Makefile
index 7f76d412047a..0e2cadfea8ff 100644
--- a/drivers/net/ethernet/hisilicon/Makefile
+++ b/drivers/net/ethernet/hisilicon/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_HNS_MDIO) += hns_mdio.o
obj-$(CONFIG_HNS) += hns/
obj-$(CONFIG_HNS3) += hns3/
obj-$(CONFIG_HISI_FEMAC) += hisi_femac.o
+obj-$(CONFIG_HIBMCGE) += hibmcge/
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/Makefile b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
new file mode 100644
index 000000000000..ea223b7207af
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Makefile for the HISILICON BMC GE network device drivers.
+#
+
+ccflags-y += -I$(src)
+
+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
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge
2024-07-31 9:42 ` [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge Jijie Shao
@ 2024-08-01 1:13 ` Andrew Lunn
2024-08-01 12:15 ` Jijie Shao
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 1:13 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Wed, Jul 31, 2024 at 05:42:44PM +0800, Jijie Shao wrote:
> Add a Makefile and update Kconfig to build hibmcge driver.
>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/Kconfig | 17 ++++++++++++++++-
> drivers/net/ethernet/hisilicon/Makefile | 1 +
> drivers/net/ethernet/hisilicon/hibmcge/Makefile | 10 ++++++++++
> 3 files changed, 27 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/Makefile
>
> diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
> index 3312e1d93c3b..372854d15481 100644
> --- a/drivers/net/ethernet/hisilicon/Kconfig
> +++ b/drivers/net/ethernet/hisilicon/Kconfig
> @@ -7,7 +7,7 @@ config NET_VENDOR_HISILICON
> bool "Hisilicon devices"
> default y
> depends on OF || ACPI
> - depends on ARM || ARM64 || COMPILE_TEST
> + depends on ARM || ARM64 || COMPILE_TEST || X86_64
It is normal to have COMPILE_TEST last.
Any reason this won't work on S390, PowerPC etc?
> +if ARM || ARM64 || COMPILE_TEST
> +
You would normally express this with a depends on.
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge
2024-08-01 1:13 ` Andrew Lunn
@ 2024-08-01 12:15 ` Jijie Shao
2024-08-01 12:33 ` Andrew Lunn
0 siblings, 1 reply; 36+ messages in thread
From: Jijie Shao @ 2024-08-01 12:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: shaojijie, yisen.zhuang, salil.mehta, davem, edumazet, kuba,
pabeni, horms, shenjian15, wangpeiyang1, liuyonglong, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
on 2024/8/1 9:13, Andrew Lunn wrote:
> On Wed, Jul 31, 2024 at 05:42:44PM +0800, Jijie Shao wrote:
>> Add a Makefile and update Kconfig to build hibmcge driver.
>>
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>> ---
>> drivers/net/ethernet/hisilicon/Kconfig | 17 ++++++++++++++++-
>> drivers/net/ethernet/hisilicon/Makefile | 1 +
>> drivers/net/ethernet/hisilicon/hibmcge/Makefile | 10 ++++++++++
>> 3 files changed, 27 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/Makefile
>>
>> diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
>> index 3312e1d93c3b..372854d15481 100644
>> --- a/drivers/net/ethernet/hisilicon/Kconfig
>> +++ b/drivers/net/ethernet/hisilicon/Kconfig
>> @@ -7,7 +7,7 @@ config NET_VENDOR_HISILICON
>> bool "Hisilicon devices"
>> default y
>> depends on OF || ACPI
>> - depends on ARM || ARM64 || COMPILE_TEST
>> + depends on ARM || ARM64 || COMPILE_TEST || X86_64
> It is normal to have COMPILE_TEST last.
ok，
>
> Any reason this won't work on S390, PowerPC etc?
I have only compiled and tested on arm or x86.
I can't ensure that compile or work ok on other platforms.
>
>> +if ARM || ARM64 || COMPILE_TEST
>> +
> You would normally express this with a depends on.
Sorry, I can't understand how to convert if to depends on?
Thanks，
Jijie shao
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge
2024-08-01 12:15 ` Jijie Shao
@ 2024-08-01 12:33 ` Andrew Lunn
0 siblings, 0 replies; 36+ messages in thread
From: Andrew Lunn @ 2024-08-01 12:33 UTC (permalink / raw)
To: Jijie Shao
Cc: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms,
shenjian15, wangpeiyang1, liuyonglong, sudongming1, xujunsheng,
shiyongbang, netdev, linux-kernel
On Thu, Aug 01, 2024 at 08:15:43PM +0800, Jijie Shao wrote:
>
> on 2024/8/1 9:13, Andrew Lunn wrote:
> > On Wed, Jul 31, 2024 at 05:42:44PM +0800, Jijie Shao wrote:
> > > Add a Makefile and update Kconfig to build hibmcge driver.
> > >
> > > Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> > > ---
> > > drivers/net/ethernet/hisilicon/Kconfig | 17 ++++++++++++++++-
> > > drivers/net/ethernet/hisilicon/Makefile | 1 +
> > > drivers/net/ethernet/hisilicon/hibmcge/Makefile | 10 ++++++++++
> > > 3 files changed, 27 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/Makefile
> > >
> > > diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
> > > index 3312e1d93c3b..372854d15481 100644
> > > --- a/drivers/net/ethernet/hisilicon/Kconfig
> > > +++ b/drivers/net/ethernet/hisilicon/Kconfig
> > > @@ -7,7 +7,7 @@ config NET_VENDOR_HISILICON
> > > bool "Hisilicon devices"
> > > default y
> > > depends on OF || ACPI
> > > - depends on ARM || ARM64 || COMPILE_TEST
> > > + depends on ARM || ARM64 || COMPILE_TEST || X86_64
> > It is normal to have COMPILE_TEST last.
>
> ok，
>
> >
> > Any reason this won't work on S390, PowerPC etc?
>
> I have only compiled and tested on arm or x86.
> I can't ensure that compile or work ok on other platforms.
It is a sign of quality if it compiles on other platforms. The kernel
APIs should hide away all the differences, if you are using them
correctly. So i would take away all the platform depends at this
level. Toolchains are easy to install apt-get
c-compiler-s390x-linux-gnu etc. And 0-day will build it for you after
a while on various platforms.
>
> >
> > > +if ARM || ARM64 || COMPILE_TEST
> > > +
> > You would normally express this with a depends on.
>
> Sorry, I can't understand how to convert if to depends on?
Kconfig is not my speciality, but cannot you using
depends on ARM || ARM64 || COMPILE_TEST
inside the symbol?
Andrew
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH net-next 10/10] net: hibmcge: Add maintainer for hibmcge
2024-07-31 9:42 [RFC PATCH net-next 00/10] Add support of HIBMCGE Ethernet Driver Jijie Shao
` (8 preceding siblings ...)
2024-07-31 9:42 ` [RFC PATCH net-next 09/10] net: hibmcge: Add a Makefile and update Kconfig for hibmcge Jijie Shao
@ 2024-07-31 9:42 ` Jijie Shao
9 siblings, 0 replies; 36+ messages in thread
From: Jijie Shao @ 2024-07-31 9:42 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, edumazet, kuba, pabeni, horms
Cc: shenjian15, wangpeiyang1, liuyonglong, shaojijie, sudongming1,
xujunsheng, shiyongbang, netdev, linux-kernel
Add myself as the maintainer for the hibmcge ethernet driver.
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index dd4297ea41f9..fa7a52a03945 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9956,6 +9956,13 @@ S: Maintained
W: http://www.hisilicon.com
F: drivers/net/ethernet/hisilicon/hns3/
+HISILICON NETWORK HIBMCGE DRIVER
+M: Jijie Shao <shaojijie@huawei.com>
+L: netdev@vger.kernel.org
+S: Maintained
+W: http://www.hisilicon.com
+F: drivers/net/ethernet/hisilicon/hibmcge/
+
HISILICON NETWORK SUBSYSTEM DRIVER
M: Yisen Zhuang <yisen.zhuang@huawei.com>
M: Salil Mehta <salil.mehta@huawei.com>
--
2.33.0
^ permalink raw reply related [flat|nested] 36+ messages in thread