Re: [PATCH v27 net-next 02/10] net/nebula-matrix: add core driver architecture and HW layer initialization
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-11 03:41:45
Also in:
linux-doc, lkml
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 2 - [Low] Series-structure remark (validated as NOT a defect): nbl_probe() changes from an unconditional -ENODEV to a success path that enables… - [Low] Dead code introduced by this patch: (a) nbl_hw_leonis.c includes <linux/spinlock.h> and <linux/bitfield.h> although the file at this… -- net/nebula-matrix: add core driver architecture and HW layer initialization This adds the nbl_adapter/nbl_core/nbl_common_info device context, turns the previously stubbed nbl_probe() into a working PCI probe path, and adds the Leonis hardware layer that requests BAR0/BAR2 and ioremaps them via pcim helpers. A couple of small questions below about unused code added by this patch and about the point in the series where the device starts being claimed.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/nebula-matrix/nbl/Makefile b/drivers/net/ethernet/nebula-matrix/nbl/Makefile index 6c14d1071c0c2..cc060cf8bf759 100644 --- a/drivers/net/ethernet/nebula-matrix/nbl/Makefile +++ b/drivers/net/ethernet/nebula-matrix/nbl/Makefile@@ -3,4 +3,5 @@ obj-$(CONFIG_NBL) := nbl.o -nbl-objs += nbl_main.o +nbl-objs += nbl_hw/nbl_hw_leonis/nbl_hw_leonis.o \ + nbl_main.odiff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c new file mode 100644 index 0000000000000..0ba0b7f643c64 --- /dev/null +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_hw_leonis.c@@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2026 Nebula Matrix Limited. + */ +#include <linux/device.h> +#include <linux/pci.h> +#include <linux/bits.h> +#include <linux/io.h> +#include <linux/spinlock.h> +#include <linux/bitfield.h> +#include "nbl_hw_leonis.h"
[Severity: Low] This isn't a bug, but are linux/spinlock.h and linux/bitfield.h needed here yet? Grepping nbl_hw_leonis.c at this commit for spin_lock or FIELD_ returns no matches, so both look like leftovers from splitting a larger file (the spinlock and FIELD_*() users appear only in later patches). Could they be added in the patch that first uses them? [ ... ]
quoted hunk ↗ jump to hunk
+ hw_mgt->hw_size = PAGE_ALIGN_DOWN(NBL_MEM_BAR_TOTAL_SIZE - + NBL_RDMA_NOTIFY_LEN); + hw_mgt->hw_addr = + pcim_iomap(pdev, NBL_MEMORY_BAR, + hw_mgt->hw_size);
[Severity: Low] Is hw_mgt->hw_size read anywhere? It is stored in both branches but never used to bound a register access, here or later in the series, where only mailbox_bar_size ends up being checked. Passing the size straight to pcim_iomap() and dropping the field would work too. [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_include/nbl_def_common.h b/drivers/net/ethernet/nebula-matrix/nbl/nbl_include/nbl_def_common.h new file mode 100644 index 0000000000000..da30244fe75d6 --- /dev/null +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_include/nbl_def_common.h@@ -0,0 +1,32 @@
[ ... ]
quoted hunk ↗ jump to hunk
+struct nbl_common_info { + struct pci_dev *pdev; + struct device *dev; + u32 msg_enable; + u16 vsi_id;
[Severity: Low] What uses msg_enable? At this commit the only occurrence in the whole nbl directory is this declaration, and by the end of the series struct nbl_common_info no longer has the member at all, so it is added and then silently removed within the same series. Should it just be dropped here? [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c index 199626159a4ce..f2552bc732937 100644 --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c
[ ... ]
quoted hunk ↗ jump to hunk
static int nbl_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - return -ENODEV; + struct nbl_init_param param = { { 0 } }; + struct device *dev = &pdev->dev; + struct nbl_adapter *adapter; + int err; + + err = pcim_enable_device(pdev); + if (err) { + dev_err(&pdev->dev, "Failed to enable PCI dev, err=%d\n", err); + return err; + } + + nbl_get_func_param(pdev, id->driver_data, ¶m); + /* never return fail when DMA_BIT_MASK(64) */ + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); + + pci_set_master(pdev); + + adapter = nbl_core_init(pdev, ¶m); + if (IS_ERR(adapter)) { + dev_err(dev, "Nbl adapter init fail: %pe\n", adapter); + err = PTR_ERR(adapter); + goto adapter_init_err; + } + pci_set_drvdata(pdev, adapter); + return 0; +adapter_init_err: + pci_clear_master(pdev); + return err; }
[Severity: Low] This isn't a bug, but from this commit on nbl_probe() returns success after pcim_enable_device(), dma_set_mask_and_coherent(), pci_set_master() and the BAR mappings, while no network interface is created. Grepping the nbl directory for register_netdev or alloc_etherdev at the end of the series finds no matches either, so the module claims every ID in nbl_id_table[] and exposes no netdev at any point in the series. The commit message does describe this as "preparing for subsequent device implementations", so the only question is placement: would it be preferable to keep probe returning -ENODEV until the netdev registration lands, so that bisecting between these patches does not leave the hardware bound to a driver that offers nothing? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907123848.30256-1-illusion.wang%40nebula-matrix.com