Re: [PATCH v27 net-next 01/10] net/nebula-matrix: add minimum nbl build framework

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Documentation/Kconfig prose added by this patch describes
  functionality that does not exist at this commit:…

--

net/nebula-matrix: add minimum nbl build framework

This patch adds the Kconfig, Makefile and MAINTAINERS entries for a new
nebula-matrix ethernet vendor directory, a documentation file for the nbl
driver, and a PCI driver skeleton whose probe and remove callbacks are
stubs.

> diff --git a/Documentation/networking/device_drivers/ethernet/nebula-matrix/nbl.rst b/Documentation/networking/device_drivers/ethernet/nebula-matrix/nbl.rst
> new file mode 100644
> index 0000000000000..5540291cf3766
> --- /dev/null
> +++ b/Documentation/networking/device_drivers/ethernet/nebula-matrix/nbl.rst
> @@ -0,0 +1,28 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +================================================================
> +Linux Base Driver for Nebula-matrix m18110-NIC/m18000-NIC family
> +================================================================
> +
> +Overview:
> +=========
> +The m18110-NIC/m18000-NIC (marketed as SNIC s1000) is a series
> +of network interface cards for the Data Center Area.
> +
> +This driver provides the core infrastructure for m18110/m18000-NIC
> +devices, including:
> +
> +- PCI device enumeration and resource (BAR) management
> +- Firmware command interface via PF mailbox
> +- Channel-based communication between driver and firmware
> +- Device initialization and teardown

[Severity: Low]

This isn't a bug, but does the documentation added here describe code that
exists at this commit?  The only C file added is nbl_main.c, where
nbl_probe() returns -ENODEV and nbl_remove() is empty, so there is no BAR
management, no PF mailbox command interface, no channel communication and no
device init/teardown yet.

The same wording appears in the CONFIG_NBL help text below.

All four items are implemented by the end of the series (nbl_probe() ends up
calling nbl_core_init(), which chains nbl_hw_init_leonis(),
nbl_chan_init_common(), nbl_res_init_leonis(), nbl_disp_init() and
nbl_dev_init()), and the .rst text is not touched again, so this is only a
skew within the series.

Would it be preferable to word the first patch's doc and help text to match
the stub state, or to add this prose together with the code it describes?

[ ... ]

> diff --git a/drivers/net/ethernet/nebula-matrix/Kconfig b/drivers/net/ethernet/nebula-matrix/Kconfig
> new file mode 100644
> index 0000000000000..dd0e91d14131f
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/Kconfig
> @@ -0,0 +1,32 @@

[ ... ]

> +config NBL
> +	tristate "Nebula-matrix Ethernet Controller m18110/m18000 support"
> +	depends on PCI && (64BIT || COMPILE_TEST) && !CPU_BIG_ENDIAN
> +	help
> +	  This driver provides the core infrastructure for Nebula-matrix
> +	  Ethernet Controller m18110/m18000 Family of devices, including
> +	  PCI enumeration, firmware mailbox and channel communication.

This is the menuconfig text mentioned above; at this commit selecting
CONFIG_NBL=m builds a module that only registers a pci_driver with an
always-failing probe.

[ ... ]

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c
> new file mode 100644
> index 0000000000000..199626159a4ce
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_main.c
> @@ -0,0 +1,100 @@

[ ... ]

> +static int nbl_probe(struct pci_dev *pdev,
> +		     const struct pci_device_id *id)
> +{
> +	return -ENODEV;
> +}
> +
> +static void nbl_remove(struct pci_dev *pdev)
> +{
> +}

[ ... ]

> +static const struct pci_device_id nbl_id_table[] = {
> +	{ PCI_DEVICE(NBL_VENDOR_ID, NBL_DEVICE_ID_M18110),
> +	  .driver_data = BIT(NBL_CAP_HAS_NET_BIT) },

[ ... ]

> +	/* required as sentinel */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(pci, nbl_id_table);

Since MODULE_DEVICE_TABLE() publishes all 16 IDs in the 0x1F0F:0x3403-0x3412
range, udev will autoload nbl on matching hardware at this commit and the
device stays unbound because of the -ENODEV above.  Is that the intended
intermediate state for a bisect point?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907123848.30256-1-illusion.wang%40nebula-matrix.com