* [PATCH net-next 2/3] net/ethernet/zte/dinghai: add logging infrastructure
2026-04-15 1:53 [PATCH net-next 1/3] net/ethernet: add ZTE network driver support Junyang Han
@ 2026-04-15 1:53 ` Junyang Han
2026-04-15 14:19 ` Andrew Lunn
2026-04-15 1:53 ` [PATCH net-next 3/3] net/ethernet/zte/dinghai: add hardware register access and PCI capability scanning Junyang Han
2026-04-15 14:10 ` [PATCH net-next 1/3] net/ethernet: add ZTE network driver support Andrew Lunn
2 siblings, 1 reply; 6+ messages in thread
From: Junyang Han @ 2026-04-15 1:53 UTC (permalink / raw)
To: netdev
Cc: davem, andrew+netdev, edumazet, kuba, pabeni, han.junyang,
ran.ming, han.chengfei, zhang.yanze
[-- Attachment #1.1.1: Type: text/plain, Size: 10819 bytes --]
Introduce logging macros (DH_LOG_EMERG/ALERT/CRIT/ERR/WARN/INFO/DBG)
and helper definitions for ZTE DingHai driver debugging.
Signed-off-by: Junyang Han <han.junyang@zte.com.cn>
---
drivers/net/ethernet/zte/dinghai/Makefile | 2 +-
drivers/net/ethernet/zte/dinghai/dh_helper.h | 79 ++++++++++++++++++++
drivers/net/ethernet/zte/dinghai/dh_log.c | 10 +++
drivers/net/ethernet/zte/dinghai/dh_log.h | 64 ++++++++++++++++
drivers/net/ethernet/zte/dinghai/en_pf.c | 49 ++++++++++--
5 files changed, 196 insertions(+), 8 deletions(-)
create mode 100644 drivers/net/ethernet/zte/dinghai/dh_helper.h
create mode 100644 drivers/net/ethernet/zte/dinghai/dh_log.c
create mode 100644 drivers/net/ethernet/zte/dinghai/dh_log.h
diff --git a/drivers/net/ethernet/zte/dinghai/Makefile b/drivers/net/ethernet/zte/dinghai/Makefile
index f55a8de518be..c2a815427c24 100644
--- a/drivers/net/ethernet/zte/dinghai/Makefile
+++ b/drivers/net/ethernet/zte/dinghai/Makefile
@@ -6,5 +6,5 @@
ccflags-y += -I$(src)
obj-$(CONFIG_DINGHAI_PF) += dinghai10e.o
-dinghai10e-y := en_pf.o
+dinghai10e-y := en_pf.o dh_log.o
diff --git a/drivers/net/ethernet/zte/dinghai/dh_helper.h b/drivers/net/ethernet/zte/dinghai/dh_helper.h
new file mode 100644
index 000000000000..3933e6d79460
--- /dev/null
+++ b/drivers/net/ethernet/zte/dinghai/dh_helper.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ZTE DingHai Ethernet driver - device-level logging helpers
+ * Copyright (c) 2022-2024, ZTE Corporation.
+ */
+
+#ifndef __DH_HELPER_H__
+#define __DH_HELPER_H__
+
+#include <linux/dev_printk.h>
+#include <linux/types.h>
+#include "dh_log.h"
+#include "en_pf.h"
+
+extern u32 dh_debug_mask;
+
+#define dh_dbg(__dev, format, ...) \
+ dev_dbg((__dev)->device, "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_dbg_once(__dev, format, ...) \
+ dev_dbg_once((__dev)->device, \
+ "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_dbg_mask(__dev, mask, format, ...) \
+do { \
+ if ((mask) & dh_debug_mask) \
+ dh_dbg(__dev, format, ##__VA_ARGS__); \
+} while (0)
+
+#define dh_err(__dev, format, ...) \
+ dev_err((__dev)->device, "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_err_rl(__dev, format, ...) \
+ dev_err_ratelimited((__dev)->device, \
+ "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_warn(__dev, format, ...) \
+ dev_warn((__dev)->device, "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_warn_once(__dev, format, ...) \
+ dev_warn_once((__dev)->device, "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_warn_rl(__dev, format, ...) \
+ dev_warn_ratelimited((__dev)->device, \
+ "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+#define dh_info(__dev, format, ...) \
+ dev_info((__dev)->device, format, ##__VA_ARGS__)
+
+#define dh_info_rl(__dev, format, ...) \
+ dev_info_ratelimited((__dev)->device, \
+ "%s:%d:(pid %d): " format, \
+ __func__, __LINE__, current->pid, \
+ ##__VA_ARGS__)
+
+enum {
+ ZXDH_PCI_DEV_IS_VF = 1 << 0,
+};
+
+static inline bool dh_core_is_sf(const struct dh_core_dev *dev)
+{
+ return dev->coredev_type == DH_COREDEV_SF;
+}
+
+#endif /* __DH_HELPER_H__ */
diff --git a/drivers/net/ethernet/zte/dinghai/dh_log.c b/drivers/net/ethernet/zte/dinghai/dh_log.c
new file mode 100644
index 000000000000..5e6e42175e37
--- /dev/null
+++ b/drivers/net/ethernet/zte/dinghai/dh_log.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ZTE DingHai Ethernet driver - logging runtime control
+ * Copyright (c) 2022-2024, ZTE Corporation.
+ */
+
+#include <linux/module.h>
+
+int debug_print;
+module_param(debug_print, int, 0644);
diff --git a/drivers/net/ethernet/zte/dinghai/dh_log.h b/drivers/net/ethernet/zte/dinghai/dh_log.h
new file mode 100644
index 000000000000..295ee306fa0d
--- /dev/null
+++ b/drivers/net/ethernet/zte/dinghai/dh_log.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ZTE DingHai Ethernet driver - logging infrastructure
+ * Copyright (c) 2022-2024, ZTE Corporation.
+ */
+
+#ifndef __DH_LOG_H__
+#define __DH_LOG_H__
+
+#include <linux/kernel.h>
+#include <linux/printk.h>
+
+#define MODULE_CMD "zxdh_cmd"
+#define MODULE_NP "zxdh_np"
+#define MODULE_PF "zxdh_pf"
+#define MODULE_PTP "zxdh_ptp"
+#define MODULE_TSN "zxdh_tsn"
+#define MODULE_LAG "zxdh_lag"
+#define MODULE_DHTOOLS "zxdh_tool"
+#define MODULE_SEC "zxdh_sec"
+#define MODULE_MPF "zxdh_mpf"
+#define MODULE_FUC_HP "zxdh_func_hp"
+#define MODULE_UACCE "zxdh_uacce"
+#define MODULE_HEAL "zxdh_health"
+
+extern int debug_print;
+
+#define DH_LOG_EMERG(module, fmt, arg...) \
+ printk(KERN_EMERG "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_ALERT(module, fmt, arg...) \
+ printk(KERN_ALERT "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_CRIT(module, fmt, arg...) \
+ printk(KERN_CRIT "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_ERR(module, fmt, arg...) \
+ printk(KERN_ERR "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_WARNING(module, fmt, arg...) \
+ printk(KERN_WARNING "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_INFO(module, fmt, arg...) \
+ printk(KERN_INFO "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg)
+
+#define DH_LOG_DEBUG(module, fmt, arg...) \
+do { \
+ if (debug_print) \
+ printk(KERN_DEBUG "[%s][%s][%d] " fmt, \
+ module, __func__, __LINE__, ##arg); \
+} while (0)
+
+#define LOG_ERR(fmt, arg...) DH_LOG_ERR(MODULE_PF, fmt, ##arg)
+#define LOG_INFO(fmt, arg...) DH_LOG_INFO(MODULE_PF, fmt, ##arg)
+#define LOG_DEBUG(fmt, arg...) DH_LOG_DEBUG(MODULE_PF, fmt, ##arg)
+#define LOG_WARN(fmt, arg...) DH_LOG_WARNING(MODULE_PF, fmt, ##arg)
+
+#endif /* __DH_LOG_H__ */
diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.c b/drivers/net/ethernet/zte/dinghai/en_pf.c
index d3a4298fa927..2d2740223401 100644
--- a/drivers/net/ethernet/zte/dinghai/en_pf.c
+++ b/drivers/net/ethernet/zte/dinghai/en_pf.c
@@ -8,6 +8,7 @@
#include <linux/pci.h>
#include <net/devlink.h>
#include "en_pf.h"
+#include "dh_log.h"
#define DRV_VERSION "1.0-1"
#define DRV_SUMMARY "ZTE(R) zxdh-net driver"
@@ -21,6 +22,13 @@ MODULE_DESCRIPTION(DRV_SUMMARY);
MODULE_VERSION(DRV_VERSION);
MODULE_LICENSE("GPL");
+u32 dh_debug_mask;
+module_param_named(debug_mask, dh_debug_mask, uint, 0644);
+MODULE_PARM_DESC(debug_mask, "debug mask: 1 = dump cmd data, 2 = dump cmd exec time, 3 = both. Default=0");
+static bool probe_vf = 1;
+module_param(probe_vf, bool, 0644);
+MODULE_PARM_DESC(probe_vf, "probe_vf: 0 = N, 1 = Y");
+
static const struct devlink_ops dh_pf_devlink_ops = {};
const struct pci_device_id dh_pf_pci_table[] = {
@@ -39,26 +47,34 @@ static int dh_pf_pci_init(struct dh_core_dev *dev)
pci_set_drvdata(dev->pdev, dev);
ret = pci_enable_device(dev->pdev);
- if (ret)
+ if (ret) {
+ LOG_ERR("pci_enable_device failed: %d\n", ret);
return -ENOMEM;
+ }
ret = dma_set_mask_and_coherent(dev->device, DMA_BIT_MASK(64));
if (ret) {
ret = dma_set_mask_and_coherent(dev->device, DMA_BIT_MASK(32));
- if (ret)
+ if (ret) {
+ LOG_ERR("dma_set_mask_and_coherent failed: %d\n", ret);
goto err_pci;
+ }
}
ret = pci_request_selected_regions(dev->pdev,
pci_select_bars(dev->pdev, IORESOURCE_MEM),
"dh-pf");
- if (ret)
+ if (ret) {
+ LOG_ERR("pci_request_selected_regions failed: %d\n", ret);
goto err_pci;
+ }
pci_set_master(dev->pdev);
ret = pci_save_state(dev->pdev);
- if (ret)
+ if (ret) {
+ LOG_ERR("pci_save_state failed: %d\n", ret);
goto err_pci_save_state;
+ }
pf_dev = dh_core_priv(dev);
pf_dev->pci_ioremap_addr[0] =
@@ -66,6 +82,9 @@ static int dh_pf_pci_init(struct dh_core_dev *dev)
pci_resource_len(dev->pdev, 0));
if (!pf_dev->pci_ioremap_addr[0]) {
ret = -ENOMEM;
+ LOG_ERR("ioremap(0x%llx, 0x%llx) failed\n",
+ pci_resource_start(dev->pdev, 0),
+ pci_resource_len(dev->pdev, 0));
goto err_pci_save_state;
}
@@ -95,10 +114,13 @@ static int dh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
struct devlink *devlink = NULL;
int ret = 0;
+ LOG_INFO("pf level start\n");
devlink = devlink_alloc(&dh_pf_devlink_ops, sizeof(struct zxdh_pf_device),
&pdev->dev);
- if (!devlink)
+ if (!devlink) {
+ LOG_ERR("devlink alloc failed\n");
return -ENOMEM;
+ }
dh_dev = devlink_priv(devlink);
dh_dev->device = &pdev->dev;
@@ -112,10 +134,17 @@ static int dh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
mutex_init(&pf_dev->irq_lock);
dh_dev->coredev_type = GET_COREDEV_TYPE(pdev);
+ LOG_DEBUG("%s device: %s\n",
+ (dh_dev->coredev_type == DH_COREDEV_PF) ? "PF" : "VF",
+ pci_name(pdev));
ret = dh_pf_pci_init(dh_dev);
- if (ret)
+ if (ret) {
+ LOG_ERR("dh_pf_pci_init failed: %d\n", ret);
goto err_cfg_init;
+ }
+
+ LOG_INFO("pf level completed\n");
return 0;
@@ -133,14 +162,17 @@ static void dh_pf_remove(struct pci_dev *pdev)
struct devlink *devlink = priv_to_devlink(dh_dev);
struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
- if (!dh_dev)
+ if (!pf_dev)
return;
+ LOG_INFO("pf level start\n");
+
dh_pf_pci_close(dh_dev);
mutex_destroy(&pf_dev->irq_lock);
mutex_destroy(&dh_dev->lock);
devlink_free(devlink);
pci_set_drvdata(pdev, NULL);
+ LOG_INFO("pf level completed\n");
}
static void dh_pf_shutdown(struct pci_dev *pdev)
@@ -149,12 +181,15 @@ static void dh_pf_shutdown(struct pci_dev *pdev)
struct devlink *devlink = priv_to_devlink(dh_dev);
struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ LOG_INFO("pf level start\n");
+
dh_pf_pci_close(dh_dev);
mutex_destroy(&pf_dev->irq_lock);
mutex_destroy(&dh_dev->lock);
devlink_free(devlink);
pci_set_drvdata(pdev, NULL);
+ LOG_INFO("pf level completed\n");
}
static int dh_pf_suspend(struct pci_dev *pdev, pm_message_t state)
--
2.43.0
[-- Attachment #1.1.2: Type: text/html , Size: 26984 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next 2/3] net/ethernet/zte/dinghai: add logging infrastructure
2026-04-15 1:53 ` [PATCH net-next 2/3] net/ethernet/zte/dinghai: add logging infrastructure Junyang Han
@ 2026-04-15 14:19 ` Andrew Lunn
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2026-04-15 14:19 UTC (permalink / raw)
To: Junyang Han
Cc: netdev, davem, andrew+netdev, edumazet, kuba, pabeni, ran.ming,
han.chengfei, zhang.yanze
> +#define dh_dbg(__dev, format, ...) \
> + dev_dbg((__dev)->device, "%s:%d:(pid %d): " format, \
> + __func__, __LINE__, current->pid, \
> + ##__VA_ARGS__)
> +
> +#define dh_dbg_once(__dev, format, ...) \
> + dev_dbg_once((__dev)->device, \
> + "%s:%d:(pid %d): " format, \
> + __func__, __LINE__, current->pid, \
> + ##__VA_ARGS__)
There is a dislike for adding wrappers around the existing kernel log
functions. What value does this add?
> +int debug_print;
> +module_param(debug_print, int, 0644);
Module parameters are not liked.
> +module_param_named(debug_mask, dh_debug_mask, uint, 0644);
> +MODULE_PARM_DESC(debug_mask, "debug mask: 1 = dump cmd data, 2 =
> dump cmd exec time, 3 = both. Default=0");
> +static bool probe_vf = 1;
> +module_param(probe_vf, bool, 0644);
> +MODULE_PARM_DESC(probe_vf, "probe_vf: 0 = N, 1 = Y");
I suggest you remove all these.
Debug code is reasonable to have during development. But once the
driver actually works, it should not be needed.
> const struct pci_device_id dh_pf_pci_table[] = {
> @@ -39,26 +47,34 @@ static int dh_pf_pci_init(struct dh_core_dev *dev)
> pci_set_drvdata(dev->pdev, dev);
>
> ret = pci_enable_device(dev->pdev);
> - if (ret)
> + if (ret) {
> + LOG_ERR("pci_enable_device failed: %d\n", ret);
dev_err(dev, ..)
> return -ENOMEM;
> + }
>
> ret = dma_set_mask_and_coherent(dev->device, DMA_BIT_MASK(64));
> if (ret) {
> ret = dma_set_mask_and_coherent(dev->device, DMA_BIT_MASK(32));
> - if (ret)
> + if (ret) {
> + LOG_ERR("dma_set_mask_and_coherent failed: %d\n", ret);
dev_err(dev, ...
And maybe it should actually be dev_err_probe() if this is a probe
function, and you need to handle -EPROBE_DEFER.
Debug logging should be limited to actual debug stuff. Please use the
standard logging primitives for anything which is not debug, so
dev_err(), dev_warn(), dev_info(). netdev_err(), netdev_warn() etc.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 3/3] net/ethernet/zte/dinghai: add hardware register access and PCI capability scanning
2026-04-15 1:53 [PATCH net-next 1/3] net/ethernet: add ZTE network driver support Junyang Han
2026-04-15 1:53 ` [PATCH net-next 2/3] net/ethernet/zte/dinghai: add logging infrastructure Junyang Han
@ 2026-04-15 1:53 ` Junyang Han
2026-04-15 14:31 ` Andrew Lunn
2026-04-15 14:10 ` [PATCH net-next 1/3] net/ethernet: add ZTE network driver support Andrew Lunn
2 siblings, 1 reply; 6+ messages in thread
From: Junyang Han @ 2026-04-15 1:53 UTC (permalink / raw)
To: netdev
Cc: davem, andrew+netdev, edumazet, kuba, pabeni, han.junyang,
ran.ming, han.chengfei, zhang.yanze
[-- Attachment #1.1.1: Type: text/plain, Size: 18746 bytes --]
Implement PCI configuration space access, BAR mapping, capability
scanning (common/notify/device), and hardware queue register
definitions for DingHai PF device.
Signed-off-by: Junyang Han <han.junyang@zte.com.cn>
---
drivers/net/ethernet/zte/dinghai/dh_queue.h | 71 ++++
drivers/net/ethernet/zte/dinghai/en_pf.c | 411 ++++++++++++++++++++
drivers/net/ethernet/zte/dinghai/en_pf.h | 41 ++
3 files changed, 523 insertions(+)
create mode 100644 drivers/net/ethernet/zte/dinghai/dh_queue.h
diff --git a/drivers/net/ethernet/zte/dinghai/dh_queue.h b/drivers/net/ethernet/zte/dinghai/dh_queue.h
new file mode 100644
index 000000000000..1e7d64ecbbf3
--- /dev/null
+++ b/drivers/net/ethernet/zte/dinghai/dh_queue.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ZTE DingHai Ethernet driver - PCI capability definitions
+ * Copyright (c) 2022-2024, ZTE Corporation.
+ */
+
+#ifndef __DH_QUEUE_H__
+#define __DH_QUEUE_H__
+
+/* Vector value used to disable MSI for queue */
+#define ZXDH_MSI_NO_VECTOR 0xff
+
+/* Status byte for guest to report progress, and synchronize features */
+/* We have seen device and processed generic fields */
+#define ZXDH_CONFIG_S_ACKNOWLEDGE 1
+/* We have found a driver for the device. */
+#define ZXDH_CONFIG_S_DRIVER 2
+/* Driver has used its parts of the config, and is happy */
+#define ZXDH_CONFIG_S_DRIVER_OK 4
+/* Driver has finished configuring features */
+#define ZXDH_CONFIG_S_FEATURES_OK 8
+/* Device entered invalid state, driver must reset it */
+#define ZXDH_CONFIG_S_NEEDS_RESET 0x40
+/* We've given up on this device */
+#define ZXDH_CONFIG_S_FAILED 0x80
+
+/* This is the PCI capability header: */
+struct zxdh_pf_pci_cap {
+ __u8 cap_vndr; /* Generic PCI field: PCI_CAP_ID_VNDR */
+ __u8 cap_next; /* Generic PCI field: next ptr. */
+ __u8 cap_len; /* Generic PCI field: capability length */
+ __u8 cfg_type; /* Identifies the structure. */
+ __u8 bar; /* Where to find it. */
+ __u8 id; /* Multiple capabilities of the same type */
+ __u8 padding[2]; /* Pad to full dword. */
+ __le32 offset; /* Offset within bar. */
+ __le32 length; /* Length of the structure, in bytes. */
+};
+
+/* Fields in ZXDH_PF_PCI_CAP_COMMON_CFG: */
+struct zxdh_pf_pci_common_cfg {
+ /* About the whole device. */
+ __le32 device_feature_select; /* read-write */
+ __le32 device_feature; /* read-only */
+ __le32 guest_feature_select; /* read-write */
+ __le32 guest_feature; /* read-write */
+ __le16 msix_config; /* read-write */
+ __le16 num_queues; /* read-only */
+ __u8 device_status; /* read-write */
+ __u8 config_generation; /* read-only */
+
+ /* About a specific virtqueue. */
+ __le16 queue_select; /* read-write */
+ __le16 queue_size; /* read-write, power of 2. */
+ __le16 queue_msix_vector; /* read-write */
+ __le16 queue_enable; /* read-write */
+ __le16 queue_notify_off; /* read-only */
+ __le32 queue_desc_lo; /* read-write */
+ __le32 queue_desc_hi; /* read-write */
+ __le32 queue_avail_lo; /* read-write */
+ __le32 queue_avail_hi; /* read-write */
+ __le32 queue_used_lo; /* read-write */
+ __le32 queue_used_hi; /* read-write */
+};
+
+struct zxdh_pf_pci_notify_cap {
+ struct zxdh_pf_pci_cap cap;
+ __le32 notify_off_multiplier; /* Multiplier for queue_notify_off. */
+};
+
+#endif /* __DH_QUEUE_H__ */
diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.c b/drivers/net/ethernet/zte/dinghai/en_pf.c
index 2d2740223401..c29299ad629e 100644
--- a/drivers/net/ethernet/zte/dinghai/en_pf.c
+++ b/drivers/net/ethernet/zte/dinghai/en_pf.c
@@ -107,6 +107,417 @@ void dh_pf_pci_close(struct dh_core_dev *dev)
pci_disable_device(dev->pdev);
}
+int32_t zxdh_pf_pci_find_capability(struct pci_dev *pdev, uint8_t cfg_type,
+ uint32_t ioresource_types, int32_t *bars)
+{
+ int32_t pos = 0;
+ uint8_t type = 0;
+ uint8_t bar = 0;
+
+ for (pos = pci_find_capability(pdev, PCI_CAP_ID_VNDR); pos > 0;
+ pos = pci_find_next_capability(pdev, pos, PCI_CAP_ID_VNDR)) {
+ pci_read_config_byte(pdev, pos + offsetof(struct zxdh_pf_pci_cap, cfg_type), &type);
+ pci_read_config_byte(pdev, pos + offsetof(struct zxdh_pf_pci_cap, bar), &bar);
+
+ /* ignore structures with reserved BAR values */
+ if (bar > ZXDH_PF_MAX_BAR_VAL)
+ continue;
+
+ if (type == cfg_type) {
+ if (pci_resource_len(pdev, bar) &&
+ pci_resource_flags(pdev, bar) & ioresource_types) {
+ *bars |= (1 << bar);
+ return pos;
+ }
+ }
+ }
+
+ return 0;
+}
+
+void __iomem *zxdh_pf_map_capability(struct dh_core_dev *dh_dev, int32_t off,
+ size_t minlen, uint32_t align,
+ uint32_t start, uint32_t size,
+ size_t *len, resource_size_t *pa,
+ uint32_t *bar_off)
+{
+ struct pci_dev *pdev = dh_dev->pdev;
+ uint8_t bar = 0;
+ uint32_t offset = 0;
+ uint32_t length = 0;
+ void __iomem *p = NULL;
+
+ pci_read_config_byte(pdev, off + offsetof(struct zxdh_pf_pci_cap, bar), &bar);
+ pci_read_config_dword(pdev, off + offsetof(struct zxdh_pf_pci_cap, offset), &offset);
+ pci_read_config_dword(pdev, off + offsetof(struct zxdh_pf_pci_cap, length), &length);
+
+ if (bar_off)
+ *bar_off = offset;
+
+ if (length <= start) {
+ LOG_ERR("bad capability len %u (>%u expected)\n", length, start);
+ return NULL;
+ }
+
+ if (length - start < minlen) {
+ LOG_ERR("bad capability len %u (>=%zu expected)\n", length, minlen);
+ return NULL;
+ }
+
+ length -= start;
+ if (start + offset < offset) {
+ LOG_ERR("map wrap-around %u+%u\n", start, offset);
+ return NULL;
+ }
+
+ offset += start;
+ if (offset & (align - 1)) {
+ LOG_ERR("offset %u not aligned to %u\n", offset, align);
+ return NULL;
+ }
+
+ if (length > size)
+ length = size;
+
+ if (len)
+ *len = length;
+
+ if (minlen + offset < minlen || minlen + offset > pci_resource_len(pdev, bar)) {
+ LOG_ERR("map custom queue %zu@%u out of range on bar %i length %lu\n",
+ minlen, offset, bar, (unsigned long)pci_resource_len(pdev, bar));
+ return NULL;
+ }
+
+ p = pci_iomap_range(pdev, bar, offset, length);
+ if (unlikely(!p)) {
+ LOG_ERR("unable to map custom queue %u@%u on bar %i\n", length, offset, bar);
+ } else if (pa) {
+ *pa = pci_resource_start(pdev, bar) + offset;
+ }
+
+ return p;
+}
+
+int32_t zxdh_pf_common_cfg_init(struct dh_core_dev *dh_dev)
+{
+ int32_t common = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ struct pci_dev *pdev = dh_dev->pdev;
+
+ /* check for a common config: if not, use legacy mode (bar 0). */
+ common = zxdh_pf_pci_find_capability(pdev, ZXDH_PCI_CAP_COMMON_CFG,
+ IORESOURCE_IO | IORESOURCE_MEM,
+ &pf_dev->modern_bars);
+ if (common == 0) {
+ LOG_ERR("missing capabilities %i, leaving for legacy driver\n", common);
+ return -ENODEV;
+ }
+
+ pf_dev->common = zxdh_pf_map_capability(dh_dev, common,
+ sizeof(struct zxdh_pf_pci_common_cfg),
+ ZXDH_PF_ALIGN4, 0,
+ sizeof(struct zxdh_pf_pci_common_cfg),
+ NULL, NULL, NULL);
+ if (unlikely(!pf_dev->common)) {
+ LOG_ERR("pf_dev->common is null\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int32_t zxdh_pf_notify_cfg_init(struct dh_core_dev *dh_dev)
+{
+ int32_t notify = 0;
+ uint32_t notify_length = 0;
+ uint32_t notify_offset = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ struct pci_dev *pdev = dh_dev->pdev;
+
+ /* If common is there, these should be too... */
+ notify = zxdh_pf_pci_find_capability(pdev, ZXDH_PCI_CAP_NOTIFY_CFG,
+ IORESOURCE_IO | IORESOURCE_MEM,
+ &pf_dev->modern_bars);
+ if (notify == 0) {
+ LOG_ERR("missing capabilities %i\n", notify);
+ return -EINVAL;
+ }
+
+ pci_read_config_dword(pdev, notify + offsetof(struct zxdh_pf_pci_notify_cap,
+ notify_off_multiplier), &pf_dev->notify_offset_multiplier);
+ pci_read_config_dword(pdev, notify + offsetof(struct zxdh_pf_pci_notify_cap,
+ cap.length), ¬ify_length);
+ pci_read_config_dword(pdev, notify + offsetof(struct zxdh_pf_pci_notify_cap,
+ cap.offset), ¬ify_offset);
+
+ /* We don't know how many VQs we'll map, ahead of the time.
+ * If notify length is small, map it all now. Otherwise, map each VQ individually later.
+ */
+ if ((uint64_t)notify_length + (notify_offset % PAGE_SIZE) <= PAGE_SIZE) {
+ pf_dev->notify_base = zxdh_pf_map_capability(dh_dev, notify,
+ ZXDH_PF_MAP_MINLEN2,
+ ZXDH_PF_ALIGN2, 0,
+ notify_length,
+ &pf_dev->notify_len,
+ &pf_dev->notify_pa, NULL);
+ if (unlikely(!pf_dev->notify_base)) {
+ LOG_ERR("pf_dev->notify_base is null\n");
+ return -EINVAL;
+ }
+ } else {
+ pf_dev->notify_map_cap = notify;
+ }
+
+ return 0;
+}
+
+int32_t zxdh_pf_device_cfg_init(struct dh_core_dev *dh_dev)
+{
+ int32_t device = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ struct pci_dev *pdev = dh_dev->pdev;
+
+ /* Device capability is only mandatory for devices that have device-specific configuration. */
+ device = zxdh_pf_pci_find_capability(pdev, ZXDH_PCI_CAP_DEVICE_CFG,
+ IORESOURCE_IO | IORESOURCE_MEM,
+ &pf_dev->modern_bars);
+
+ /* we don't know how much we should map, but PAGE_SIZE is more than enough for all existing devices. */
+ if (device) {
+ pf_dev->device = zxdh_pf_map_capability(dh_dev, device, 0,
+ ZXDH_PF_ALIGN4, 0, PAGE_SIZE,
+ &pf_dev->device_len, NULL,
+ &pf_dev->dev_cfg_bar_off);
+ if (unlikely(!pf_dev->device)) {
+ LOG_ERR("pf_dev->device is null\n");
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
+void zxdh_pf_modern_cfg_uninit(struct dh_core_dev *dh_dev)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ struct pci_dev *pdev = dh_dev->pdev;
+
+ if (pf_dev->device)
+ pci_iounmap(pdev, pf_dev->device);
+ if (pf_dev->notify_base)
+ pci_iounmap(pdev, pf_dev->notify_base);
+ pci_iounmap(pdev, pf_dev->common);
+}
+
+int32_t zxdh_pf_modern_cfg_init(struct dh_core_dev *dh_dev)
+{
+ int32_t ret = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ struct pci_dev *pdev = dh_dev->pdev;
+
+ ret = zxdh_pf_common_cfg_init(dh_dev);
+ if (ret != 0) {
+ LOG_ERR("zxdh_pf_common_cfg_init failed: %d\n", ret);
+ return -EINVAL;
+ }
+
+ ret = zxdh_pf_notify_cfg_init(dh_dev);
+ if (ret != 0) {
+ LOG_ERR("zxdh_pf_notify_cfg_init failed: %d\n", ret);
+ goto err_map_notify;
+ }
+
+ ret = zxdh_pf_device_cfg_init(dh_dev);
+ if (ret != 0) {
+ LOG_ERR("zxdh_pf_device_cfg_init failed: %d\n", ret);
+ goto err_map_device;
+ }
+
+ return 0;
+
+err_map_device:
+ if (pf_dev->notify_base)
+ pci_iounmap(pdev, pf_dev->notify_base);
+err_map_notify:
+ pci_iounmap(pdev, pf_dev->common);
+ return -EINVAL;
+}
+
+uint16_t zxdh_pf_get_queue_notify_off(struct dh_core_dev *dh_dev,
+ uint16_t phy_index, uint16_t index)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ if (pf_dev->packed_status)
+ iowrite16(phy_index, &pf_dev->common->queue_select);
+ else
+ iowrite16(index, &pf_dev->common->queue_select);
+
+ return ioread16(&pf_dev->common->queue_notify_off);
+}
+
+void __iomem *zxdh_pf_map_vq_notify(struct dh_core_dev *dh_dev,
+ uint16_t phy_index, uint16_t index,
+ resource_size_t *pa)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ uint16_t off = 0;
+
+ off = zxdh_pf_get_queue_notify_off(dh_dev, phy_index, index);
+
+ if (pf_dev->notify_base) {
+ /* offset should not wrap */
+ if ((uint64_t)off * pf_dev->notify_offset_multiplier + 2 > pf_dev->notify_len) {
+ LOG_ERR("bad notification offset %u (x %u) for queue %u > %zd",
+ off, pf_dev->notify_offset_multiplier, phy_index,
+ pf_dev->notify_len);
+ return NULL;
+ }
+
+ if (pa)
+ *pa = pf_dev->notify_pa + off * pf_dev->notify_offset_multiplier;
+
+ return pf_dev->notify_base + off * pf_dev->notify_offset_multiplier;
+ } else {
+ return zxdh_pf_map_capability(dh_dev, pf_dev->notify_map_cap, 2, 2,
+ off * pf_dev->notify_offset_multiplier,
+ 2, NULL, pa, NULL);
+ }
+}
+
+void zxdh_pf_unmap_vq_notify(struct dh_core_dev *dh_dev, void *priv)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ if (!pf_dev->notify_base)
+ pci_iounmap(dh_dev->pdev, priv);
+}
+
+void zxdh_pf_set_status(struct dh_core_dev *dh_dev, uint8_t status)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ iowrite8(status, &pf_dev->common->device_status);
+}
+
+uint8_t zxdh_pf_get_status(struct dh_core_dev *dh_dev)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ return ioread8(&pf_dev->common->device_status);
+}
+
+static uint8_t zxdh_pf_get_cfg_gen(struct dh_core_dev *dh_dev)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ uint8_t config_generation = 0;
+
+ config_generation = ioread8(&pf_dev->common->config_generation);
+ LOG_INFO("config_generation is %d\n", config_generation);
+
+ return config_generation;
+}
+
+void zxdh_pf_get_vf_mac(struct dh_core_dev *dh_dev, uint8_t *mac, int32_t vf_id)
+{
+ uint32_t DEV_MAC_L = 0;
+ uint16_t DEV_MAC_H = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ if (pf_dev->pf_sriov_cap_base) {
+ DEV_MAC_L = ioread32((void __iomem *)(pf_dev->pf_sriov_cap_base +
+ (pf_dev->sriov_bar_size) * vf_id +
+ pf_dev->dev_cfg_bar_off));
+ mac[0] = DEV_MAC_L & 0xff;
+ mac[1] = (DEV_MAC_L >> 8) & 0xff;
+ mac[2] = (DEV_MAC_L >> 16) & 0xff;
+ mac[3] = (DEV_MAC_L >> 24) & 0xff;
+ DEV_MAC_H = ioread16((void __iomem *)(pf_dev->pf_sriov_cap_base +
+ (pf_dev->sriov_bar_size) * vf_id +
+ pf_dev->dev_cfg_bar_off +
+ ZXDH_DEV_MAC_HIGH_OFFSET));
+ mac[4] = DEV_MAC_H & 0xff;
+ mac[5] = (DEV_MAC_H >> 8) & 0xff;
+ }
+}
+
+void zxdh_pf_set_vf_mac_reg(struct zxdh_pf_device *pf_dev, uint8_t *mac, int32_t vf_id)
+{
+ uint32_t DEV_MAC_L = 0;
+ uint16_t DEV_MAC_H = 0;
+
+ if (pf_dev->pf_sriov_cap_base) {
+ DEV_MAC_L = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
+ DEV_MAC_H = mac[4] | (mac[5] << 8);
+ iowrite32(DEV_MAC_L, (void __iomem *)(pf_dev->pf_sriov_cap_base +
+ (pf_dev->sriov_bar_size) * vf_id +
+ pf_dev->dev_cfg_bar_off));
+ iowrite16(DEV_MAC_H, (void __iomem *)(pf_dev->pf_sriov_cap_base +
+ (pf_dev->sriov_bar_size) * vf_id +
+ pf_dev->dev_cfg_bar_off +
+ ZXDH_DEV_MAC_HIGH_OFFSET));
+ }
+}
+
+void zxdh_pf_set_vf_mac(struct dh_core_dev *dh_dev, uint8_t *mac, int32_t vf_id)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ zxdh_pf_set_vf_mac_reg(pf_dev, mac, vf_id);
+}
+
+void zxdh_set_mac(struct dh_core_dev *dh_dev, uint8_t *mac)
+{
+ uint32_t DEV_MAC_L = 0;
+ uint16_t DEV_MAC_H = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ DEV_MAC_L = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
+ DEV_MAC_H = mac[4] | (mac[5] << 8);
+ iowrite32(DEV_MAC_L, pf_dev->device);
+ iowrite16(DEV_MAC_H, (void __iomem *)((uint8_t *)pf_dev->device +
+ ZXDH_DEV_MAC_HIGH_OFFSET));
+}
+
+void zxdh_get_mac(struct dh_core_dev *dh_dev, uint8_t *mac)
+{
+ uint32_t DEV_MAC_L = 0;
+ uint16_t DEV_MAC_H = 0;
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ DEV_MAC_L = ioread32(pf_dev->device);
+ mac[0] = DEV_MAC_L & 0xff;
+ mac[1] = (DEV_MAC_L >> 8) & 0xff;
+ mac[2] = (DEV_MAC_L >> 16) & 0xff;
+ mac[3] = (DEV_MAC_L >> 24) & 0xff;
+ DEV_MAC_H = ioread16((void __iomem *)((uint8_t *)pf_dev->device +
+ ZXDH_DEV_MAC_HIGH_OFFSET));
+ mac[4] = DEV_MAC_H & 0xff;
+ mac[5] = (DEV_MAC_H >> 8) & 0xff;
+}
+
+uint64_t zxdh_pf_get_features(struct dh_core_dev *dh_dev)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+ uint64_t device_feature = 0;
+
+ iowrite32(0, &pf_dev->common->device_feature_select);
+ device_feature = ioread32(&pf_dev->common->device_feature);
+ iowrite32(1, &pf_dev->common->device_feature_select);
+ device_feature |= ((uint64_t)ioread32(&pf_dev->common->device_feature) << 32);
+
+ return device_feature;
+}
+
+void zxdh_pf_set_features(struct dh_core_dev *dh_dev, uint64_t features)
+{
+ struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
+
+ iowrite32(0, &pf_dev->common->guest_feature_select);
+ iowrite32((uint32_t)features, &pf_dev->common->guest_feature);
+ iowrite32(1, &pf_dev->common->guest_feature_select);
+ iowrite32(features >> 32, &pf_dev->common->guest_feature);
+}
+
static int dh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct dh_core_dev *dh_dev = NULL;
diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.h b/drivers/net/ethernet/zte/dinghai/en_pf.h
index 0d3880b0aede..197b21788576 100644
--- a/drivers/net/ethernet/zte/dinghai/en_pf.h
+++ b/drivers/net/ethernet/zte/dinghai/en_pf.h
@@ -10,11 +10,31 @@
#include <linux/types.h>
#include <linux/pci.h>
#include <linux/mutex.h>
+#include "dh_log.h"
+#include "dh_queue.h"
#define ZXDH_PF_VENDOR_ID 0x1cf2
#define ZXDH_PF_DEVICE_ID 0x8040
#define ZXDH_VF_DEVICE_ID 0x8041
+/* Common configuration */
+#define ZXDH_PCI_CAP_COMMON_CFG 1
+/* Notifications */
+#define ZXDH_PCI_CAP_NOTIFY_CFG 2
+/* ISR access */
+#define ZXDH_PCI_CAP_ISR_CFG 3
+/* Device specific configuration */
+#define ZXDH_PCI_CAP_DEVICE_CFG 4
+/* PCI configuration access */
+#define ZXDH_PCI_CAP_PCI_CFG 5
+
+#define ZXDH_PF_MAX_BAR_VAL 0x5
+#define ZXDH_PF_ALIGN4 4
+#define ZXDH_PF_ALIGN2 2
+#define ZXDH_PF_MAP_MINLEN2 2
+
+#define ZXDH_DEV_MAC_HIGH_OFFSET 4
+
enum dh_coredev_type {
DH_COREDEV_PF,
DH_COREDEV_VF,
@@ -34,6 +54,27 @@ struct dh_core_dev {
};
struct zxdh_pf_device {
+ struct zxdh_pf_pci_common_cfg __iomem *common;
+ /* Device-specific data (non-legacy mode) */
+ /* Base of vq notifications (non-legacy mode). */
+ void __iomem *device;
+ void __iomem *notify_base;
+ void __iomem *pf_sriov_cap_base;
+ /* Physical base of vq notifications */
+ resource_size_t notify_pa;
+ /* So we can sanity-check accesses. */
+ size_t notify_len;
+ size_t device_len;
+ /* Capability for when we need to map notifications per-vq. */
+ int32_t notify_map_cap;
+ uint32_t notify_offset_multiplier;
+ /* Multiply queue_notify_off by this value. (non-legacy mode). */
+ int32_t modern_bars;
+
+ uint64_t pci_ioremap_addr[6];
+ uint64_t sriov_bar_size;
+ uint32_t dev_cfg_bar_off;
+ bool packed_status;
bool bar_chan_valid;
bool vepa;
struct mutex irq_lock; /* Protects IRQ operations */
--
2.43.0
[-- Attachment #1.1.2: Type: text/html , Size: 45357 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next 3/3] net/ethernet/zte/dinghai: add hardware register access and PCI capability scanning
2026-04-15 1:53 ` [PATCH net-next 3/3] net/ethernet/zte/dinghai: add hardware register access and PCI capability scanning Junyang Han
@ 2026-04-15 14:31 ` Andrew Lunn
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2026-04-15 14:31 UTC (permalink / raw)
To: Junyang Han
Cc: netdev, davem, andrew+netdev, edumazet, kuba, pabeni, ran.ming,
han.chengfei, zhang.yanze
> +int32_t zxdh_pf_pci_find_capability(struct pci_dev *pdev, uint8_t cfg_type,
> + uint32_t ioresource_types, int32_t *bars)
> +{
> + int32_t pos = 0;
> + uint8_t type = 0;
> + uint8_t bar = 0;
> +
> + for (pos = pci_find_capability(pdev, PCI_CAP_ID_VNDR); pos > 0;
> + pos = pci_find_next_capability(pdev, pos, PCI_CAP_ID_VNDR)) {
> + pci_read_config_byte(pdev, pos + offsetof
> (struct zxdh_pf_pci_cap, cfg_type), &type);
> + pci_read_config_byte(pdev, pos + offsetof
> (struct zxdh_pf_pci_cap, bar), &bar);
Something odd going on with indentation? Has the mailer corrupted it?
> +
> + /* ignore structures with reserved BAR values */
> + if (bar > ZXDH_PF_MAX_BAR_VAL)
> + continue;
> +
> + if (type == cfg_type) {
> + if (pci_resource_len(pdev, bar) &&
> + pci_resource_flags(pdev, bar) & ioresource_types) {
> + *bars |= (1 << bar);
> + return pos;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +void __iomem *zxdh_pf_map_capability(struct dh_core_dev *dh_dev, int32_t off,
> + size_t minlen, uint32_t align,
> + uint32_t start, uint32_t size,
> + size_t *len, resource_size_t *pa,
> + uint32_t *bar_off)
> + p = pci_iomap_range(pdev, bar, offset, length);
> + if (unlikely(!p)) {
Is this hot path? Please only use unlikely() when dealing with frames
in the hot path.
> +int32_t zxdh_pf_common_cfg_init(struct dh_core_dev *dh_dev)
> +{
> + int32_t common = 0;
> + struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
> + struct pci_dev *pdev = dh_dev->pdev;
> +
> + /* check for a common config: if not, use legacy mode (bar 0). */
> + common = zxdh_pf_pci_find_capability(pdev, ZXDH_PCI_CAP_COMMON_CFG,
> + IORESOURCE_IO | IORESOURCE_MEM,
> + &pf_dev->modern_bars);
> + if (common == 0) {
> + LOG_ERR("missing capabilities %i, leaving for legacy driver\
> n", common);
> + return -ENODEV;
> + }
> +
> + pf_dev->common = zxdh_pf_map_capability(dh_dev, common,
> + sizeof(struct zxdh_pf_pci_common_cfg),
> + ZXDH_PF_ALIGN4, 0,
> + sizeof(struct zxdh_pf_pci_common_cfg),
> + NULL, NULL, NULL);
> + if (unlikely(!pf_dev->common)) {
> + LOG_ERR("pf_dev->common is null\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +int32_t zxdh_pf_notify_cfg_init(struct dh_core_dev *dh_dev)
> +{
> + /* We don't know how many VQs we'll map, ahead of the time.
> + * If notify length is small, map it all now. Otherwise, map each VQ individually later.
> + */
> + if ((uint64_t)notify_length + (notify_offset % PAGE_SIZE) <= PAGE_SIZE) {
Please try to avoid casts. They suggest the types are wrong. You will
probably have better code if you don't need the cast.
> +int32_t zxdh_pf_modern_cfg_init(struct dh_core_dev *dh_dev)
> +{
> + int32_t ret = 0;
> + struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
> + struct pci_dev *pdev = dh_dev->pdev;
> +
> + ret = zxdh_pf_common_cfg_init(dh_dev);
> + if (ret != 0) {
if (ret)
would be more normal.
> +void zxdh_pf_get_vf_mac
> (struct dh_core_dev *dh_dev, uint8_t *mac, int32_t vf_id)
> +{
> + uint32_t DEV_MAC_L = 0;
> + uint16_t DEV_MAC_H = 0;
> + struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
> +
> + if (pf_dev->pf_sriov_cap_base) {
> + DEV_MAC_L = ioread32((void __iomem *)(pf_dev->pf_sriov_cap_base +
> + (pf_dev->sriov_bar_size) * vf_id +
> + pf_dev->dev_cfg_bar_off));
Is the cast needed? pf_dev->pf_sriov_cap_base should already be void *
__iomem.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/3] net/ethernet: add ZTE network driver support
2026-04-15 1:53 [PATCH net-next 1/3] net/ethernet: add ZTE network driver support Junyang Han
2026-04-15 1:53 ` [PATCH net-next 2/3] net/ethernet/zte/dinghai: add logging infrastructure Junyang Han
2026-04-15 1:53 ` [PATCH net-next 3/3] net/ethernet/zte/dinghai: add hardware register access and PCI capability scanning Junyang Han
@ 2026-04-15 14:10 ` Andrew Lunn
2 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2026-04-15 14:10 UTC (permalink / raw)
To: Junyang Han
Cc: netdev, davem, andrew+netdev, edumazet, kuba, pabeni, ran.ming,
han.chengfei, zhang.yanze
On Wed, Apr 15, 2026 at 09:53:32AM +0800, Junyang Han wrote:
> Add basic framework for ZTE DingHai ethernet PF driver, including
> Kconfig/Makefile build support and PCIe device probe/remove skeleton.
Please take a read on
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
and
https://docs.kernel.org/process/submitting-patches.html
Please always include a patch 0/X in a patch set, explaining the big
picture.
Thanks for keeping the driver small and easy to review.
> + * ZTE DingHai Ethernet driver
> + * Copyright (c) 2022-2024, ZTE Corporation.
And the last two years?
> +#define DRV_VERSION "1.0-1"
Driver versions are generally useless. What does this actually mean
for the given very limited driver? Are you going to change the version
with each patchset?
> +#define DRV_SUMMARY "ZTE(R) zxdh-net driver"
> +
> +const char zxdh_pf_driver_version[] = DRV_VERSION;
> +static const char zxdh_pf_driver_string[] = DRV_SUMMARY;
> +static const char zxdh_pf_copyright[] = "Copyright (c)
> 2022-2024, ZTE Corporation.";
You don't need this, you have the copyright above.
> +MODULE_AUTHOR("ZTE");
Author is a person, with an email address.
> +MODULE_DESCRIPTION(DRV_SUMMARY);
Please just put the string here, not #define.
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_LICENSE("GPL");
> +static int dh_pf_pci_init(struct dh_core_dev *dev)
> +{
> + int ret = 0;
> + struct zxdh_pf_device *pf_dev = NULL;
Reverse Christmas tree. This applies everywhere for a netdev driver.
> +static int dh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +err_cfg_init:
> + mutex_destroy(&pf_dev->irq_lock);
> + mutex_destroy(&dh_dev->lock);
> + devlink_free(devlink);
> + pf_dev = NULL;
Since this is a probe function, do you really need to set pf_dev to
NULL? How is it going to keep a value over EPROBE_DEFER cycles?
> +static void dh_pf_remove(struct pci_dev *pdev)
> +{
> + struct dh_core_dev *dh_dev = pci_get_drvdata(pdev);
> + struct devlink *devlink = priv_to_devlink(dh_dev);
> + struct zxdh_pf_device *pf_dev = dh_core_priv(dh_dev);
> +
> + if (!dh_dev)
> + return;
How does that happen?
> + dh_pf_pci_close(dh_dev);
> + mutex_destroy(&pf_dev->irq_lock);
> + mutex_destroy(&dh_dev->lock);
> + devlink_free(devlink);
> + pci_set_drvdata(pdev, NULL);
> +}
> +static int dh_pf_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + return 0;
> +}
> +
> +static int dh_pf_resume(struct pci_dev *pdev)
> +{
> + return 0;
> +}
If they do nothing, don't provide them. You can add them when you add
suspend/resume support.
> +static int __init dh_pf_pci_init_module(void)
> +{
> + return pci_register_driver(&dh_pf_driver);
> +}
> +
> +static void __exit dh_pf_pci_exit_module(void)
> +{
> + pci_unregister_driver(&dh_pf_driver);
> +}
> +
> +module_init(dh_pf_pci_init_module);
> +module_exit(dh_pf_pci_exit_module);
The PCI subsystem offers a wrapper to do this.
> +struct dh_core_dev {
> + struct device *device;
> + enum dh_coredev_type coredev_type;
> + struct pci_dev *pdev;
> + struct devlink *devlink;
> + struct mutex lock; /* Protects device configuration */
> + char priv[] __aligned(32);
That is unusual. priv is usually a void * and allocated. If you want
an actual array, you might want to have a second member indicate the
size of the array, look at all the work done recently on flexible
arrays.
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 6+ messages in thread