Re: [PATCH net-next v9 1/2] dinghai: add ZTE network driver support

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 20 Jul 2026 18:10:53 +0800 (CST) han.junyang@zte.com.cn wrote:
> From: Junyang Han <han.junyang@zte.com.cn>
> 
> Add basic framework for ZTE DingHai ethernet PF driver, including
> Kconfig/Makefile build support and PCIe device probe/remove skeleton.
> 
> Signed-off-by: Junyang Han <han.junyang@zte.com.cn>

> +ccflags-y += -I$(src)

Why?

> +obj-$(CONFIG_DINGHAI_PF) += dinghai10e.o
> +dinghai10e-y := en_pf.o

> +static int zxdh_pf_pci_init(struct zxdh_core_dev *zxdh_dev)
> +{
> +	struct zxdh_pf_dev *pf_dev = zxdh_dev->priv;
> +	int ret;
> +
> +	pci_set_drvdata(zxdh_dev->pdev, zxdh_dev);
> +
> +	ret = pci_enable_device(zxdh_dev->pdev);
> +	if (ret) {
> +		dev_err(zxdh_dev->device, "pci_enable_device failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = dma_set_mask_and_coherent(zxdh_dev->device, DMA_BIT_MASK(64));
> +	if (ret) {
> +		ret = dma_set_mask_and_coherent(zxdh_dev->device, DMA_BIT_MASK(32));
> +		if (ret) {

read Documentation/core-api/dma-api-howto.rst
these can't fail

> +			dev_err(zxdh_dev->device, "dma_set_mask_and_coherent failed: %d\n", ret);
> +			goto err_pci;
> +		}
> +	}
> +

> +static int zxdh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct zxdh_pf_dev *pf_dev;
> +	struct zxdh_core_dev *zxdh_dev;

reverse xmas tree ordering, please

> +	struct devlink *devlink;
> +	int ret;
> +
> +	devlink = devlink_alloc(&zxdh_pf_devlink_ops, sizeof(struct zxdh_core_dev),
> +				&pdev->dev);
> +	if (!devlink) {
> +		dev_err(&pdev->dev, "zxdh_pf devlink alloc failed\n");

no errors on allocation failures please, core will print an OOM

> +		return -ENOMEM;
> +	}
> +
> +	zxdh_dev = devlink_priv(devlink);
> +	zxdh_dev->device = &pdev->dev;
> +	zxdh_dev->pdev = pdev;
> +	zxdh_dev->devlink = devlink;
> +
> +	pf_dev = zxdh_core_alloc_priv(zxdh_dev, sizeof(*pf_dev));
> +	if (!pf_dev) {
> +		dev_err(&pdev->dev, "zxdh_pf_dev alloc failed\n");
> +		ret = -ENOMEM;
> +		goto err_pf_dev;
> +	}
> +
> +	pf_dev->bar_chan_valid = false;
> +	pf_dev->vepa = false;

don't zero init fields in zalloc'ed structs

> +	mutex_init(&zxdh_dev->lock);
> +	mutex_init(&pf_dev->irq_lock);
> +
> +	zxdh_dev->coredev_type = GET_COREDEV_TYPE(pdev);
> +
> +	ret = zxdh_pf_pci_init(zxdh_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "zxdh_pf_pci_init failed: %d\n", ret);
> +		goto err_cfg_init;
> +	}
> +
> +	devlink_register(devlink);
> +
> +	return 0;
> +
> +err_cfg_init:
> +	mutex_destroy(&pf_dev->irq_lock);
> +	mutex_destroy(&zxdh_dev->lock);
> +	zxdh_core_free_priv(zxdh_dev);
> +err_pf_dev:
> +	devlink_free(devlink);
> +	return ret;

> diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.h b/drivers/net/ethernet/zte/dinghai/en_pf.h
> new file mode 100644
> index 000000000000..65eac936505b
> --- /dev/null
> +++ b/drivers/net/ethernet/zte/dinghai/en_pf.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * ZTE DingHai Ethernet driver - PF header
> + * Copyright (c) 2022-2026, ZTE Corporation.
> + */
> +
> +#ifndef __ZXDH_EN_PF_H__
> +#define __ZXDH_EN_PF_H__
> +
> +#include <linux/types.h>
> +#include <linux/pci.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>

Why do you need device.h and slab.h here?

> +#define ZXDH_PF_VENDOR_ID	0x1cf2
> +#define ZXDH_PF_DEVICE_ID	0x8040
> +#define ZXDH_VF_DEVICE_ID	0x8041
> +
> +enum zxdh_coredev_type {
> +	DH_COREDEV_PF,
> +	DH_COREDEV_VF,
> +	DH_COREDEV_SF,
> +	DH_COREDEV_MPF

I suspect it will take some time/development to add SF support.
Please don't add unused enum entries

> +};
> +
> +struct devlink;

No need to forward declare types used as members in structs.

> +struct zxdh_core_dev {
> +	struct device *device;
> +	enum zxdh_coredev_type coredev_type;
> +	struct pci_dev *pdev;
> +	struct devlink *devlink;
> +	struct mutex lock; /* Protects device configuration */
> +	void *priv;
> +};
> +
> +struct zxdh_pf_dev {
> +	void __iomem *pci_ioremap_addr[6];
> +	bool bar_chan_valid;
> +	bool vepa;

same here, please don't add fields which are obviously unused

> +	struct mutex irq_lock; /* Protects IRQ operations */

and here.. you init / destroy this mutex but never take it

> +};
> +
> +#define GET_COREDEV_TYPE(pdev) \
> +	((pdev)->device == ZXDH_VF_DEVICE_ID ? DH_COREDEV_VF : DH_COREDEV_PF)
> +
> +void *zxdh_core_alloc_priv(struct zxdh_core_dev *zxdh_dev, size_t size);
> +void zxdh_core_free_priv(struct zxdh_core_dev *zxdh_dev);
> +void zxdh_pf_pci_close(struct zxdh_core_dev *zxdh_dev);
> +
> +#endif /* __ZXDH_EN_PF_H__ */