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@xxxxxxxxx>
> >>> ---
> >>> .../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@xxxxxxxxx>");
> >>> +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.
