Re: [net-next, 01/10] bng_en: Add PCI interface

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 18/06/2025 15:47, Vikas Gupta wrote:
> Add basic pci interface to the driver which supports
> the BCM5770X NIC family.
> 
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Reviewed-by: Bhargava Chenna Marreddy <bhargava.marreddy@broadcom.com>
> Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
> ---
>   MAINTAINERS                                   |   6 +
>   drivers/net/ethernet/broadcom/Kconfig         |   8 +
>   drivers/net/ethernet/broadcom/Makefile        |   1 +
>   drivers/net/ethernet/broadcom/bnge/Makefile   |   5 +
>   drivers/net/ethernet/broadcom/bnge/bnge.h     |  16 ++
>   .../net/ethernet/broadcom/bnge/bnge_core.c    | 149 ++++++++++++++++++
>   6 files changed, 185 insertions(+)
>   create mode 100644 drivers/net/ethernet/broadcom/bnge/Makefile
>   create mode 100644 drivers/net/ethernet/broadcom/bnge/bnge.h
>   create mode 100644 drivers/net/ethernet/broadcom/bnge/bnge_core.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 507c5ff6f620..af349b77a92e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4854,6 +4854,12 @@ F:	drivers/firmware/broadcom/tee_bnxt_fw.c
>   F:	drivers/net/ethernet/broadcom/bnxt/
>   F:	include/linux/firmware/broadcom/tee_bnxt_fw.h
>   
> +BROADCOM BNG_EN 800 GIGABIT ETHERNET DRIVER
> +M:	Vikas Gupta <vikas.gupta@broadcom.com>
> +L:	netdev@vger.kernel.org
> +S:	Maintained
> +F:	drivers/net/ethernet/broadcom/bnge/
> +
>   BROADCOM BRCM80211 IEEE802.11 WIRELESS DRIVERS
>   M:	Arend van Spriel <arend.vanspriel@broadcom.com>
>   L:	linux-wireless@vger.kernel.org
> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
> index 81a74e07464f..e2c1ac91708e 100644
> --- a/drivers/net/ethernet/broadcom/Kconfig
> +++ b/drivers/net/ethernet/broadcom/Kconfig
> @@ -253,6 +253,14 @@ config BNXT_HWMON
>   	  Say Y if you want to expose the thermal sensor data on NetXtreme-C/E
>   	  devices, via the hwmon sysfs interface.
>   
> +config BNGE
> +	tristate "Broadcom Ethernet device support"
> +	depends on PCI
> +	help
> +	  This driver supports Broadcom 50/100/200/400/800 gigabit Ethernet cards.
> +	  The module will be called bng_en. To compile this driver as a module,
> +	  choose M here.
> +
>   config BCMASP
>   	tristate "Broadcom ASP 2.0 Ethernet support"
>   	depends on ARCH_BRCMSTB || COMPILE_TEST
> diff --git a/drivers/net/ethernet/broadcom/Makefile b/drivers/net/ethernet/broadcom/Makefile
> index bac5cb6ad0cd..10cc1c92ecfc 100644
> --- a/drivers/net/ethernet/broadcom/Makefile
> +++ b/drivers/net/ethernet/broadcom/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
>   obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
>   obj-$(CONFIG_BNXT) += bnxt/
>   obj-$(CONFIG_BCMASP) += asp2/
> +obj-$(CONFIG_BNGE) += bnge/
> diff --git a/drivers/net/ethernet/broadcom/bnge/Makefile b/drivers/net/ethernet/broadcom/bnge/Makefile
> new file mode 100644
> index 000000000000..0c3d632805d1
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bnge/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_BNGE) += bng_en.o
> +
> +bng_en-y := bnge_core.o
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge.h b/drivers/net/ethernet/broadcom/bnge/bnge.h
> new file mode 100644
> index 000000000000..b49c51b44473
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2025 Broadcom */
> +
> +#ifndef _BNGE_H_
> +#define _BNGE_H_
> +
> +#define DRV_NAME	"bng_en"
> +#define DRV_SUMMARY	"Broadcom 800G Ethernet Linux Driver"
> +
> +extern char bnge_driver_name[];
> +
> +enum board_idx {
> +	BCM57708,
> +};
> +
> +#endif /* _BNGE_H_ */
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_core.c b/drivers/net/ethernet/broadcom/bnge/bnge_core.c
> new file mode 100644
> index 000000000000..3778210da98d
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_core.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2025 Broadcom.
> +
> +#include <linux/init.h>
> +#include <linux/crash_dump.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +
> +#include "bnge.h"
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION(DRV_SUMMARY);
> +
> +char bnge_driver_name[] = DRV_NAME;
> +
> +static const struct {
> +	char *name;
> +} board_info[] = {
> +	[BCM57708] = { "Broadcom BCM57708 50Gb/100Gb/200Gb/400Gb/800Gb Ethernet" },
> +};
> +
> +static const struct pci_device_id bnge_pci_tbl[] = {
> +	{ PCI_VDEVICE(BROADCOM, 0x1780), .driver_data = BCM57708 },
> +	/* Required last entry */
> +	{0, }
> +};
> +MODULE_DEVICE_TABLE(pci, bnge_pci_tbl);
> +
> +static void bnge_print_device_info(struct pci_dev *pdev, enum board_idx idx)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	dev_info(dev, "%s found at mem %lx\n", board_info[idx].name,
> +		 (long)pci_resource_start(pdev, 0));
> +
> +	pcie_print_link_status(pdev);
> +}
> +
> +static void bnge_pci_disable(struct pci_dev *pdev)
> +{
> +	pci_release_regions(pdev);
> +	if (pci_is_enabled(pdev))
> +		pci_disable_device(pdev);
> +}
> +
> +static int bnge_pci_enable(struct pci_dev *pdev)
> +{
> +	int rc;
> +
> +	rc = pci_enable_device(pdev);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Cannot enable PCI device, aborting\n");
> +		return rc;
> +	}
> +
> +	if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
> +		dev_err(&pdev->dev,
> +			"Cannot find PCI device base address, aborting\n");
> +		rc = -ENODEV;
> +		goto err_pci_disable;
> +	}
> +
> +	rc = pci_request_regions(pdev, bnge_driver_name);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Cannot obtain PCI resources, aborting\n");
> +		goto err_pci_disable;
> +	}
> +
> +	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) != 0 &&
> +	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)) != 0) {
> +		dev_err(&pdev->dev, "System does not support DMA, aborting\n");
> +		rc = -EIO;
> +		goto err_pci_release;
> +	}

According to https://docs.kernel.org/core-api/dma-api-howto.html the
code above is not correct. dma_set_mask_and_coherent() will never return
fail when DMA_BIT_MASK(64).


> +
> +	pci_set_master(pdev);
> +
> +	return 0;
> +
> +err_pci_release:
> +	pci_release_regions(pdev);
> +
> +err_pci_disable:
> +	pci_disable_device(pdev);
> +	return rc;
> +}
> +
> +static int bnge_probe_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	int rc;
> +
> +	if (pci_is_bridge(pdev))
> +		return -ENODEV;
> +
> +	if (!pdev->msix_cap) {
> +		dev_err(&pdev->dev, "MSIX capability missing, aborting\n");
> +		return -ENODEV;
> +	}
> +
> +	if (is_kdump_kernel()) {
> +		pci_clear_master(pdev);
> +		pcie_flr(pdev);
> +	}
> +
> +	rc = bnge_pci_enable(pdev);
> +	if (rc)
> +		return rc;
> +
> +	bnge_print_device_info(pdev, ent->driver_data);
> +
> +	pci_save_state(pdev);
> +
> +	return 0;
> +}
> +
> +static void bnge_remove_one(struct pci_dev *pdev)
> +{
> +	bnge_pci_disable(pdev);
> +}
> +
> +static void bnge_shutdown(struct pci_dev *pdev)
> +{
> +	pci_disable_device(pdev);
> +
> +	if (system_state == SYSTEM_POWER_OFF) {
> +		pci_wake_from_d3(pdev, 0);
> +		pci_set_power_state(pdev, PCI_D3hot);
> +	}
> +}
> +
> +static struct pci_driver bnge_driver = {
> +	.name		= bnge_driver_name,
> +	.id_table	= bnge_pci_tbl,
> +	.probe		= bnge_probe_one,
> +	.remove		= bnge_remove_one,
> +	.shutdown	= bnge_shutdown,
> +};
> +
> +static int __init bnge_init_module(void)
> +{
> +	return pci_register_driver(&bnge_driver);
> +}
> +module_init(bnge_init_module);
> +
> +static void __exit bnge_exit_module(void)
> +{
> +	pci_unregister_driver(&bnge_driver);
> +}
> +module_exit(bnge_exit_module);