Add build options and doc for mucse.
Initialize pci device access for MUCSE devices.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
 MAINTAINERS                                   |   8 +
 drivers/net/ethernet/Kconfig                  |   1 +
 drivers/net/ethernet/Makefile                 |   1 +
 drivers/net/ethernet/mucse/Kconfig            |  34 ++++
 drivers/net/ethernet/mucse/Makefile           |   7 +
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
 10 files changed, 267 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
 create mode 100644 drivers/net/ethernet/mucse/Kconfig
 create mode 100644 drivers/net/ethernet/mucse/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c

diff --git a/Documentation/networking/device_drivers/ethernet/index.rst b/Documentation/networking/device_drivers/ethernet/index.rst
index 40ac552641a3..c8abadbe15ee 100644
--- a/Documentation/networking/device_drivers/ethernet/index.rst
+++ b/Documentation/networking/device_drivers/ethernet/index.rst
@@ -47,6 +47,7 @@ Contents:
    mellanox/mlx5/index
    meta/fbnic
    microsoft/netvsc
+   mucse/rnpgbe
    neterion/s2io
    netronome/nfp
    pensando/ionic
diff --git a/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst b/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
new file mode 100644
index 000000000000..7562fb6b8f61
--- /dev/null
+++ b/Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
@@ -0,0 +1,21 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================================
+Linux Base Driver for MUCSE(R) Gigabit PCI Express Adapters
+===========================================================
+
+MUCSE Gigabit Linux driver.
+Copyright (c) 2020 - 2025 MUCSE Co.,Ltd.
+
+Identifying Your Adapter
+========================
+The driver is compatible with devices based on the following:
+
+ * MUCSE(R) Ethernet Controller N500 series
+ * MUCSE(R) Ethernet Controller N210 series
+
+Support
+=======
+ If you have problems with the software or hardware, please contact our
+ customer support team via email at techsupport@mucse.com or check our
+ website at https://www.mucse.com/en/
diff --git a/MAINTAINERS b/MAINTAINERS
index bd62ad58a47f..80c1ae2ae26c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17278,6 +17278,14 @@ T:	git git://linuxtv.org/media.git
 F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9v111.yaml
 F:	drivers/media/i2c/mt9v111.c
 
+MUCSE ETHERNET DRIVER
+M:	Yibo Dong <dong100@mucse.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+W:	https://www.mucse.com/en/
+F:	Documentation/networking/device_drivers/ethernet/mucse/*
+F:	drivers/net/ethernet/mucse/*
+
 MULTIFUNCTION DEVICES (MFD)
 M:	Lee Jones <lee@kernel.org>
 S:	Maintained
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index f86d4557d8d7..167388f9c744 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -129,6 +129,7 @@ source "drivers/net/ethernet/microchip/Kconfig"
 source "drivers/net/ethernet/mscc/Kconfig"
 source "drivers/net/ethernet/microsoft/Kconfig"
 source "drivers/net/ethernet/moxa/Kconfig"
+source "drivers/net/ethernet/mucse/Kconfig"
 source "drivers/net/ethernet/myricom/Kconfig"
 
 config FEALNX
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 67182339469a..1b8c4df3f594 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/
 obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/
 obj-$(CONFIG_NET_VENDOR_MICROSEMI) += mscc/
 obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/
+obj-$(CONFIG_NET_VENDOR_MUCSE) += mucse/
 obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/
 obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/
diff --git a/drivers/net/ethernet/mucse/Kconfig b/drivers/net/ethernet/mucse/Kconfig
new file mode 100644
index 000000000000..be0fdf268484
--- /dev/null
+++ b/drivers/net/ethernet/mucse/Kconfig
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Mucse network device configuration
+#
+
+config NET_VENDOR_MUCSE
+	bool "Mucse devices"
+	default y
+	help
+	  If you have a network (Ethernet) card from Mucse(R), say Y.
+
+	  Note that the answer to this question doesn't directly affect the
+	  kernel: saying N will just cause the configurator to skip all
+	  the questions about Mucse(R) cards. If you say Y, you will
+	  be asked for your specific card in the following questions.
+
+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.
+
+endif # NET_VENDOR_MUCSE
+
diff --git a/drivers/net/ethernet/mucse/Makefile b/drivers/net/ethernet/mucse/Makefile
new file mode 100644
index 000000000000..835771a5c374
--- /dev/null
+++ b/drivers/net/ethernet/mucse/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2020 - 2025 MUCSE Corporation.
+#
+# Makefile for the MUCSE(R) 1GbE PCI Express ethernet driver
+#
+
+obj-$(CONFIG_MGBE) += rnpgbe/
diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
new file mode 100644
index 000000000000..9df536f0d04c
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2020 - 2025 MUCSE Corporation.
+#
+# Makefile for the MUCSE(R) 1GbE PCI Express ethernet driver
+#
+
+obj-$(CONFIG_MGBE) += rnpgbe.o
+rnpgbe-objs := rnpgbe_main.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
new file mode 100644
index 000000000000..23c84454e7c7
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_H
+#define _RNPGBE_H
+
+enum rnpgbe_boards {
+	board_n500,
+	board_n210,
+	board_n210L,
+};
+
+struct mucse {
+	struct net_device *netdev;
+	struct pci_dev *pdev;
+	u16 bd_number;
+};
+
+/* Device IDs */
+#define PCI_VENDOR_ID_MUCSE 0x8848
+#define PCI_DEVICE_ID_N500_QUAD_PORT 0x8308
+#define PCI_DEVICE_ID_N500_DUAL_PORT 0x8318
+#define PCI_DEVICE_ID_N210 0x8208
+#define PCI_DEVICE_ID_N210L 0x820a
+#endif /* _RNPGBE_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
new file mode 100644
index 000000000000..c2e099bd2639
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+
+#include "rnpgbe.h"
+
+static const char rnpgbe_driver_name[] = "rnpgbe";
+
+/* rnpgbe_pci_tbl - PCI Device ID Table
+ *
+ * { PCI_DEVICE(Vendor ID, Device ID),
+ *   driver_data (used for different hw chip) }
+ */
+static struct pci_device_id rnpgbe_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N500_QUAD_PORT),
+	  .driver_data = board_n500},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N500_DUAL_PORT),
+	  .driver_data = board_n500},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N210),
+	  .driver_data = board_n210},
+	{ PCI_DEVICE(PCI_VENDOR_ID_MUCSE, PCI_DEVICE_ID_N210L),
+	  .driver_data = board_n210L},
+	/* required last entry */
+	{0, },
+};
+
+/**
+ * rnpgbe_probe - Device initialization routine
+ * @pdev: PCI device information struct
+ * @id: entry in rnpgbe_pci_tbl
+ *
+ * rnpgbe_probe initializes a PF adapter identified by a pci_dev
+ * structure.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int err;
+
+	err = pci_enable_device_mem(pdev);
+	if (err)
+		return err;
+
+	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
+	if (err) {
+		dev_err(&pdev->dev,
+			"No usable DMA configuration, aborting %d\n", err);
+		goto err_dma;
+	}
+
+	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
+	if (err) {
+		dev_err(&pdev->dev,
+			"pci_request_selected_regions failed 0x%x\n", err);
+		goto err_pci_req;
+	}
+
+	pci_set_master(pdev);
+	pci_save_state(pdev);
+
+	return 0;
+err_dma:
+err_pci_req:
+	pci_disable_device(pdev);
+	return err;
+}
+
+/**
+ * rnpgbe_remove - Device removal routine
+ * @pdev: PCI device information struct
+ *
+ * rnpgbe_remove is called by the PCI subsystem to alert the driver
+ * that it should release a PCI device.  This could be caused by a
+ * Hot-Plug event, or because the driver is going to be removed from
+ * memory.
+ **/
+static void rnpgbe_remove(struct pci_dev *pdev)
+{
+	pci_release_mem_regions(pdev);
+	pci_disable_device(pdev);
+}
+
+/**
+ * rnpgbe_dev_shutdown - Device shutdown routine
+ * @pdev: PCI device information struct
+ * @enable_wake: wakeup status
+ **/
+static void rnpgbe_dev_shutdown(struct pci_dev *pdev,
+				bool *enable_wake)
+{
+	*enable_wake = false;
+	pci_disable_device(pdev);
+}
+
+/**
+ * rnpgbe_shutdown - Device shutdown routine
+ * @pdev: PCI device information struct
+ *
+ * rnpgbe_shutdown is called by the PCI subsystem to alert the driver
+ * that os shutdown. Device should setup wakeup state here.
+ **/
+static void rnpgbe_shutdown(struct pci_dev *pdev)
+{
+	bool wake;
+
+	rnpgbe_dev_shutdown(pdev, &wake);
+
+	if (system_state == SYSTEM_POWER_OFF) {
+		pci_wake_from_d3(pdev, wake);
+		pci_set_power_state(pdev, PCI_D3hot);
+	}
+}
+
+static struct pci_driver rnpgbe_driver = {
+	.name = rnpgbe_driver_name,
+	.id_table = rnpgbe_pci_tbl,
+	.probe = rnpgbe_probe,
+	.remove = rnpgbe_remove,
+	.shutdown = rnpgbe_shutdown,
+};
+
+/**
+ * rnpgbe_init_module - Driver init routine
+ *
+ * rnpgbe_init_module is called when driver insmod
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int __init rnpgbe_init_module(void)
+{
+	int ret;
+
+	ret = pci_register_driver(&rnpgbe_driver);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+module_init(rnpgbe_init_module);
+
+/**
+ * rnpgbe_exit_module - Driver remove routine
+ *
+ * rnpgbe_exit_module is called when driver is removed
+ **/
+static void __exit rnpgbe_exit_module(void)
+{
+	pci_unregister_driver(&rnpgbe_driver);
+}
+
+module_exit(rnpgbe_exit_module);
+
+MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
+MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
+MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

On 8/12/2025 3:09 PM, Dong Yibo wrote:
> Add build options and doc for mucse.
> Initialize pci device access for MUCSE devices.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  .../device_drivers/ethernet/index.rst         |   1 +
>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
>  MAINTAINERS                                   |   8 +
>  drivers/net/ethernet/Kconfig                  |   1 +
>  drivers/net/ethernet/Makefile                 |   1 +
>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
>  drivers/net/ethernet/mucse/Makefile           |   7 +
>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
>  10 files changed, 267 insertions(+)
>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
>  create mode 100644 drivers/net/ethernet/mucse/Makefile
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c

[ ... ]

> + **/
> +static int __init rnpgbe_init_module(void)
> +{
> +	int ret;
> +
> +	ret = pci_register_driver(&rnpgbe_driver);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

Unnecessary code - can be simplified to just `return
pci_register_driver(&rnpgbe_driver);`

> +
> +module_init(rnpgbe_init_module);
> +
> +/**
> + * rnpgbe_exit_module - Driver remove routine
> + *
> + * rnpgbe_exit_module is called when driver is removed
> + **/
> +static void __exit rnpgbe_exit_module(void)
> +{
> +	pci_unregister_driver(&rnpgbe_driver);
> +}
> +
> +module_exit(rnpgbe_exit_module);
> +
> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> +MODULE_LICENSE("GPL");

-- 
Thanks and Regards,
Md Danish Anwar

On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Add build options and doc for mucse.
> > Initialize pci device access for MUCSE devices.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  .../device_drivers/ethernet/index.rst         |   1 +
> >  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
> >  MAINTAINERS                                   |   8 +
> >  drivers/net/ethernet/Kconfig                  |   1 +
> >  drivers/net/ethernet/Makefile                 |   1 +
> >  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
> >  drivers/net/ethernet/mucse/Makefile           |   7 +
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
> >  10 files changed, 267 insertions(+)
> >  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
> >  create mode 100644 drivers/net/ethernet/mucse/Kconfig
> >  create mode 100644 drivers/net/ethernet/mucse/Makefile
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> 
> [ ... ]
> 
> > + **/
> > +static int __init rnpgbe_init_module(void)
> > +{
> > +	int ret;
> > +
> > +	ret = pci_register_driver(&rnpgbe_driver);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> 
> Unnecessary code - can be simplified to just `return
> pci_register_driver(&rnpgbe_driver);`
> 

Yes, but if I add some new codes which need some free after
pci_register_driver failed, the new patch will be like this:

-return pci_register_driver(&rnpgbe_driver);
+int ret:
+wq = create_singlethread_workqueue(rnpgbe_driver_name);
+ret = pci_register_driver(&rnpgbe_driver);
+if (ret) {
+	destroy_workqueue(wq);
+	return ret;
+}
+return 0;

Is this ok? Maybe not good to delete code for adding new feature?
This is what Andrew suggested not to do.

> > +
> > +module_init(rnpgbe_init_module);
> > +
> > +/**
> > + * rnpgbe_exit_module - Driver remove routine
> > + *
> > + * rnpgbe_exit_module is called when driver is removed
> > + **/
> > +static void __exit rnpgbe_exit_module(void)
> > +{
> > +	pci_unregister_driver(&rnpgbe_driver);
> > +}
> > +
> > +module_exit(rnpgbe_exit_module);
> > +
> > +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> > +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> > +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> > +MODULE_LICENSE("GPL");
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

On 13/08/25 12:14 pm, Yibo Dong wrote:
> On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
>> On 8/12/2025 3:09 PM, Dong Yibo wrote:
>>> Add build options and doc for mucse.
>>> Initialize pci device access for MUCSE devices.
>>>
>>> Signed-off-by: Dong Yibo <dong100@mucse.com>
>>> ---
>>>  .../device_drivers/ethernet/index.rst         |   1 +
>>>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
>>>  MAINTAINERS                                   |   8 +
>>>  drivers/net/ethernet/Kconfig                  |   1 +
>>>  drivers/net/ethernet/Makefile                 |   1 +
>>>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
>>>  drivers/net/ethernet/mucse/Makefile           |   7 +
>>>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
>>>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
>>>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
>>>  10 files changed, 267 insertions(+)
>>>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
>>>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
>>>  create mode 100644 drivers/net/ethernet/mucse/Makefile
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
>>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
>>
>> [ ... ]
>>
>>> + **/
>>> +static int __init rnpgbe_init_module(void)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = pci_register_driver(&rnpgbe_driver);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return 0;
>>> +}
>>
>> Unnecessary code - can be simplified to just `return
>> pci_register_driver(&rnpgbe_driver);`
>>
> 
> Yes, but if I add some new codes which need some free after
> pci_register_driver failed, the new patch will be like this:
> 
> -return pci_register_driver(&rnpgbe_driver);
> +int ret:
> +wq = create_singlethread_workqueue(rnpgbe_driver_name);
> +ret = pci_register_driver(&rnpgbe_driver);
> +if (ret) {
> +	destroy_workqueue(wq);
> +	return ret;
> +}
> +return 0;
> 
> Is this ok? Maybe not good to delete code for adding new feature?
> This is what Andrew suggested not to do.
> 

In this patch series you are not modifying rnpgbe_init_module() again.
If you define a function as something in one patch and in later patches
you change it to something else - That is not encouraged, you should not
remove the code that you added in previous patches.

However here throughout your series you are not modifying this function.
Now the diff that you are showing, I don't know when you plan to post
that but as far as this series is concerned this diff is not part of the
series.

static int __init rnpgbe_init_module(void)
{
	int ret;

	ret = pci_register_driver(&rnpgbe_driver);
	if (ret)
		return ret;

	return 0;
}

This to me just seems unnecessary. You can just return
pci_register_driver() now and in future whenever you add other code you
can modify the function.

It would have  made sense for you to keep it as it is if some later
patch in your series would have modified it.

>>> +
>>> +module_init(rnpgbe_init_module);
>>> +
>>> +/**
>>> + * rnpgbe_exit_module - Driver remove routine
>>> + *
>>> + * rnpgbe_exit_module is called when driver is removed
>>> + **/
>>> +static void __exit rnpgbe_exit_module(void)
>>> +{
>>> +	pci_unregister_driver(&rnpgbe_driver);
>>> +}
>>> +
>>> +module_exit(rnpgbe_exit_module);
>>> +
>>> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
>>> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
>>> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
>>> +MODULE_LICENSE("GPL");
>>
>> -- 
>> Thanks and Regards,
>> Md Danish Anwar
>>
>>
> 
> Thanks for your feedback.

-- 
Thanks and Regards,
Danish

On Wed, Aug 13, 2025 at 01:21:26PM +0530, MD Danish Anwar wrote:
> On 13/08/25 12:14 pm, Yibo Dong wrote:
> > On Tue, Aug 12, 2025 at 09:48:07PM +0530, Anwar, Md Danish wrote:
> >> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> >>> Add build options and doc for mucse.
> >>> Initialize pci device access for MUCSE devices.
> >>>
> >>> Signed-off-by: Dong Yibo <dong100@mucse.com>
> >>> ---
> >>>  .../device_drivers/ethernet/index.rst         |   1 +
> >>>  .../device_drivers/ethernet/mucse/rnpgbe.rst  |  21 +++
> >>>  MAINTAINERS                                   |   8 +
> >>>  drivers/net/ethernet/Kconfig                  |   1 +
> >>>  drivers/net/ethernet/Makefile                 |   1 +
> >>>  drivers/net/ethernet/mucse/Kconfig            |  34 ++++
> >>>  drivers/net/ethernet/mucse/Makefile           |   7 +
> >>>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   8 +
> >>>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  25 +++
> >>>  .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c   | 161 ++++++++++++++++++
> >>>  10 files changed, 267 insertions(+)
> >>>  create mode 100644 Documentation/networking/device_drivers/ethernet/mucse/rnpgbe.rst
> >>>  create mode 100644 drivers/net/ethernet/mucse/Kconfig
> >>>  create mode 100644 drivers/net/ethernet/mucse/Makefile
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/Makefile
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> >>>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> >>
> >> [ ... ]
> >>
> >>> + **/
> >>> +static int __init rnpgbe_init_module(void)
> >>> +{
> >>> +	int ret;
> >>> +
> >>> +	ret = pci_register_driver(&rnpgbe_driver);
> >>> +	if (ret)
> >>> +		return ret;
> >>> +
> >>> +	return 0;
> >>> +}
> >>
> >> Unnecessary code - can be simplified to just `return
> >> pci_register_driver(&rnpgbe_driver);`
> >>
> > 
> > Yes, but if I add some new codes which need some free after
> > pci_register_driver failed, the new patch will be like this:
> > 
> > -return pci_register_driver(&rnpgbe_driver);
> > +int ret:
> > +wq = create_singlethread_workqueue(rnpgbe_driver_name);
> > +ret = pci_register_driver(&rnpgbe_driver);
> > +if (ret) {
> > +	destroy_workqueue(wq);
> > +	return ret;
> > +}
> > +return 0;
> > 
> > Is this ok? Maybe not good to delete code for adding new feature?
> > This is what Andrew suggested not to do.
> > 
> 
> In this patch series you are not modifying rnpgbe_init_module() again.
> If you define a function as something in one patch and in later patches
> you change it to something else - That is not encouraged, you should not
> remove the code that you added in previous patches.
> 
> However here throughout your series you are not modifying this function.
> Now the diff that you are showing, I don't know when you plan to post
> that but as far as this series is concerned this diff is not part of the
> series.
> 
> static int __init rnpgbe_init_module(void)
> {
> 	int ret;
> 
> 	ret = pci_register_driver(&rnpgbe_driver);
> 	if (ret)
> 		return ret;
> 
> 	return 0;
> }
> 
> This to me just seems unnecessary. You can just return
> pci_register_driver() now and in future whenever you add other code you
> can modify the function.
> 
> It would have  made sense for you to keep it as it is if some later
> patch in your series would have modified it.
> 

Ok, I got it, thanks. I will just return pci_register_driver().

> >>> +
> >>> +module_init(rnpgbe_init_module);
> >>> +
> >>> +/**
> >>> + * rnpgbe_exit_module - Driver remove routine
> >>> + *
> >>> + * rnpgbe_exit_module is called when driver is removed
> >>> + **/
> >>> +static void __exit rnpgbe_exit_module(void)
> >>> +{
> >>> +	pci_unregister_driver(&rnpgbe_driver);
> >>> +}
> >>> +
> >>> +module_exit(rnpgbe_exit_module);
> >>> +
> >>> +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> >>> +MODULE_AUTHOR("Mucse Corporation, <techsupport@mucse.com>");
> >>> +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> >>> +MODULE_LICENSE("GPL");
> >>
> >> -- 
> >> Thanks and Regards,
> >> Md Danish Anwar
> >>
> >>
> > 
> > Thanks for your feedback.
> 
> -- 
> Thanks and Regards,
> Danish
> 
> 

Thanks for your feedback.

On 12/08/2025 10:39, Dong Yibo wrote:
> Add build options and doc for mucse.
> Initialize pci device access for MUCSE devices.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---

[...]

> +/**
> + * rnpgbe_probe - Device initialization routine
> + * @pdev: PCI device information struct
> + * @id: entry in rnpgbe_pci_tbl
> + *
> + * rnpgbe_probe initializes a PF adapter identified by a pci_dev
> + * structure.
> + *
> + * @return: 0 on success, negative on failure
> + **/
> +static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	int err;
> +
> +	err = pci_enable_device_mem(pdev);
> +	if (err)
> +		return err;
> +
> +	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"No usable DMA configuration, aborting %d\n", err);
> +		goto err_dma;
> +	}
> +
> +	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"pci_request_selected_regions failed 0x%x\n", err);
> +		goto err_pci_req;
> +	}
> +
> +	pci_set_master(pdev);
> +	pci_save_state(pdev);
> +
> +	return 0;
> +err_dma:
> +err_pci_req:
> +	pci_disable_device(pdev);
> +	return err;
> +}

Why do you need 2 different labels pointing to the very same line? The
code is not changed through patchset, I see no reasons to have it like
this

On Tue, Aug 12, 2025 at 04:37:52PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Add build options and doc for mucse.
> > Initialize pci device access for MUCSE devices.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> 
> [...]
> 
> > +/**
> > + * rnpgbe_probe - Device initialization routine
> > + * @pdev: PCI device information struct
> > + * @id: entry in rnpgbe_pci_tbl
> > + *
> > + * rnpgbe_probe initializes a PF adapter identified by a pci_dev
> > + * structure.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > +{
> > +	int err;
> > +
> > +	err = pci_enable_device_mem(pdev);
> > +	if (err)
> > +		return err;
> > +
> > +	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(56));
> > +	if (err) {
> > +		dev_err(&pdev->dev,
> > +			"No usable DMA configuration, aborting %d\n", err);
> > +		goto err_dma;
> > +	}
> > +
> > +	err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> > +	if (err) {
> > +		dev_err(&pdev->dev,
> > +			"pci_request_selected_regions failed 0x%x\n", err);
> > +		goto err_pci_req;
> > +	}
> > +
> > +	pci_set_master(pdev);
> > +	pci_save_state(pdev);
> > +
> > +	return 0;
> > +err_dma:
> > +err_pci_req:
> > +	pci_disable_device(pdev);
> > +	return err;
> > +}
> 
> Why do you need 2 different labels pointing to the very same line? The
> code is not changed through patchset, I see no reasons to have it like
> this
> 
> 
> 
You are right, I should only 1 label here.

Thanks for your feedback.