Re: [PATCH 01/15] net: rnpgbe: Add build support for rnpgbe
flat view
From: Andrew Lunn <andrew@lunn.ch>
Date: 2025-07-03 16:26:00
Also in:
linux-doc, lkml
quoted hunk ↗ jump to hunk
--- a/MAINTAINERS +++ b/MAINTAINERS@@ -16001,11 +16001,7 @@ F: tools/testing/vma/ MEMORY MAPPING - LOCKING M: Andrew Morton <akpm@linux-foundation.org> -M: Suren Baghdasaryan <surenb@google.com> -M: Liam R. Howlett <Liam.Howlett@oracle.com> -M: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> -R: Vlastimil Babka <vbabka@suse.cz> -R: Shakeel Butt <shakeel.butt@linux.dev> +M: Suren Baghdasaryan <surenb@google.com> M: Liam R. Howlett <Liam.Howlett@oracle.com> M: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> R: Vlastimil Babka <vbabka@suse.cz> R: Shakeel Butt <shakeel.butt@linux.dev>
You clearly have not reviewed your own patch, or you would not be changing this section of the MAINTAINERs file.
quoted hunk ↗ jump to hunk
+if NET_VENDOR_MUCSE + +config MGBE + tristate "Mucse(R) 1GbE PCI Express adapters support" + depends on PCI + select PAGE_POOL + help + This driver supports Mucse(R) 1GbE PCI Express family of + adapters. + + More specific information on configuring the driver is in + <file:Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst>. + + To compile this driver as a module, choose M here. The module + will be called rnpgbe.
There is some odd indentation here.
quoted hunk ↗ jump to hunk
+#include <linux/string.h> +#include <linux/etherdevice.h> + +#include "rnpgbe.h" + +char rnpgbe_driver_name[] = "rnpgbe"; +static const char rnpgbe_driver_string[] = + "mucse 1 Gigabit PCI Express Network Driver"; +#define DRV_VERSION "1.0.0" +const char rnpgbe_driver_version[] = DRV_VERSION;
Driver versions are pointless, since they never change, yet the kernel around the driver changes all the time. Please drop.
+static const char rnpgbe_copyright[] = + "Copyright (c) 2020-2025 mucse Corporation.";
Why do you need this as a string?
quoted hunk ↗ jump to hunk
+static int rnpgbe_add_adpater(struct pci_dev *pdev) +{ + struct mucse *mucse = NULL; + struct net_device *netdev; + static int bd_number; + + pr_info("==== add rnpgbe queues:%d ====", RNPGBE_MAX_QUEUES);
If you are still debugging this driver, please wait until it is mostly bug free before submitting. I would not expect a production quality driver to have prints like this.
quoted hunk ↗ jump to hunk
+ netdev = alloc_etherdev_mq(sizeof(struct mucse), RNPGBE_MAX_QUEUES); + if (!netdev) + return -ENOMEM; + + mucse = netdev_priv(netdev); + memset((char *)mucse, 0x00, sizeof(struct mucse));
priv is guaranteed to be zero'ed.
quoted hunk ↗ jump to hunk
+static void rnpgbe_shutdown(struct pci_dev *pdev) +{ + bool wake = false; + + __rnpgbe_shutdown(pdev, &wake);
Please avoid using __ function names. Those are supposed to be reserved for the compiler. Sometimes you will see single _ for functions which have an unlocked version and a locked version.
quoted hunk ↗ jump to hunk
+static int __init rnpgbe_init_module(void) +{ + int ret; + + pr_info("%s - version %s\n", rnpgbe_driver_string, + rnpgbe_driver_version); + pr_info("%s\n", rnpgbe_copyright);
Please don't spam the log. Only print something on error. Andrew