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

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 28 Feb 2025 23:41:24 +0800 Xin Tian wrote:
> +config NET_VENDOR_YUNSILICON
> +	depends on ARM64 || X86_64

|| COMPILE_TEST please ?

> diff --git a/drivers/net/ethernet/yunsilicon/Makefile b/drivers/net/ethernet/yunsilicon/Makefile
> new file mode 100644
> index 000000000..6fc8259a7
> --- /dev/null
> +++ b/drivers/net/ethernet/yunsilicon/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd.
> +# All rights reserved.
> +# Makefile for the Yunsilicon device drivers.
> +#
> +
> +# obj-$(CONFIG_YUNSILICON_XSC_ETH) += xsc/net/

Why are you adding commented out lines? Add them where needed

> +obj-$(CONFIG_YUNSILICON_XSC_PCI) += xsc/pci/
> \ No newline at end of file

new line missing

> new file mode 100644
> index 000000000..de743487e
> --- /dev/null
> +++ b/drivers/net/ethernet/yunsilicon/xsc/net/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd.
> +# All rights reserved.
> +# Yunsilicon driver configuration
> +#
> +
> +config YUNSILICON_XSC_ETH
> +	tristate "Yunsilicon XSC ethernet driver"
> +	default n

n is the default, you don't have to specify it

> +xsc_eth-y := main.o
> \ No newline at end of file

new line

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/pci/Kconfig b/drivers/net/ethernet/yunsilicon/xsc/pci/Kconfig
> new file mode 100644
> index 000000000..2b6d79905
> --- /dev/null
> +++ b/drivers/net/ethernet/yunsilicon/xsc/pci/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd.
> +# All rights reserved.
> +# Yunsilicon PCI configuration
> +#
> +
> +config YUNSILICON_XSC_PCI
> +	tristate "Yunsilicon XSC PCI driver"
> +	default n

no need

> +	select PAGE_POOL

Why is this in the PCI driver, not the ETH driver?
Please add this line in a patch which actually makes use of page pool

> +static int set_dma_caps(struct pci_dev *pdev)
> +{
> +	int err;
> +
> +	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +	if (err)
> +		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	else
> +		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));

Please grep git history for dma_set_mask_and_coherent
The fallback is unnecessary, just:

	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));

> +	if (!err)
> +		dma_set_max_seg_size(&pdev->dev, SZ_2G);
> +
> +	return err;
> +}
-- 
pw-bot: cr