Re: [PATCH v3 01/14] net-next/yunsilicon: Add xsc driver basic framework

[Simon Horman <horms@kernel.org>]

On Wed, Jan 15, 2025 at 06:22:44PM +0800, Xin Tian wrote:
> Add yunsilicon xsc driver basic framework, including xsc_pci driver
> and xsc_eth driver
> 
> Co-developed-by: Honggang Wei <weihg@yunsilicon.com>
> Signed-off-by: Honggang Wei <weihg@yunsilicon.com>
> Co-developed-by: Lei Yan <jacky@yunsilicon.com>
> Signed-off-by: Lei Yan <jacky@yunsilicon.com>
> Signed-off-by: Xin Tian <tianx@yunsilicon.com>

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h b/drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h

...

> +struct xsc_dev_resource {
> +	struct mutex alloc_mutex;	/* protect buffer alocation according to numa node */

nit: allocation

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/pci/main.c b/drivers/net/ethernet/yunsilicon/xsc/pci/main.c

...

> +static int xsc_pci_init(struct xsc_core_device *xdev, const struct pci_device_id *id)
> +{
> +	struct pci_dev *pdev = xdev->pdev;
> +	void __iomem *bar_base;
> +	int bar_num = 0;
> +	int err;
> +
> +	xdev->numa_node = dev_to_node(&pdev->dev);
> +
> +	err = xsc_pci_enable_device(xdev);
> +	if (err) {
> +		pci_err(pdev, "failed to enable PCI device: err=%d\n", err);
> +		goto err_ret;
> +	}
> +
> +	err = pci_request_region(pdev, bar_num, KBUILD_MODNAME);
> +	if (err) {
> +		pci_err(pdev, "failed to request %s pci_region=%d: err=%d\n",
> +			KBUILD_MODNAME, bar_num, err);
> +		goto err_disable;
> +	}
> +
> +	pci_set_master(pdev);
> +
> +	err = set_dma_caps(pdev);
> +	if (err) {
> +		pci_err(pdev, "failed to set DMA capabilities mask: err=%d\n", err);
> +		goto err_clr_master;
> +	}
> +
> +	bar_base = pci_ioremap_bar(pdev, bar_num);
> +	if (!bar_base) {
> +		pci_err(pdev, "failed to ioremap %s bar%d\n", KBUILD_MODNAME, bar_num);

Should err, which will be the return value of the function,
be set to a negative error value here? As is, the function will
return 0.

> +		goto err_clr_master;
> +	}
> +
> +	err = pci_save_state(pdev);
> +	if (err) {
> +		pci_err(pdev, "pci_save_state failed: err=%d\n", err);
> +		goto err_io_unmap;
> +	}
> +
> +	xdev->bar_num = bar_num;
> +	xdev->bar = bar_base;
> +
> +	return 0;
> +
> +err_io_unmap:
> +	pci_iounmap(pdev, bar_base);
> +err_clr_master:
> +	pci_clear_master(pdev);
> +	pci_release_region(pdev, bar_num);
> +err_disable:
> +	xsc_pci_disable_device(xdev);
> +err_ret:
> +	return err;
> +}

...