On Tue, Sep 16, 2025 at 09:00:23PM +0200, Jörg Sommer wrote:
> Dong Yibo schrieb am Di 16. Sep, 19:29 (+0800):
> > 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..60bbc806f17b
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2020 - 2025 Mucse Corporation. */
> > +
> > +#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_n210},
>
> Should there be a space before }?
>
ixgbe_main.c has sapce before }, but no sapce after {.
ngbe_mainc. no sapce before }, but space after {.
mlx5/core/main.c has space both.
It seems not the same....
I will add sapce before }.
> > + /* 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 errno on failure
> > + **/
> > +static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id
> > *id)
> > +{
> > + int err;
>
> In rnpgbe_mbx.c you use `int ret` for this pattern. I think you should unify
> this. But I'm more in favour of `err` than `ret`.
>
I see, I will use err in rnpgbe_mbx.c
> > +
> > + 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_disable_dev;
> > + }
> > +
> > + err = pci_request_mem_regions(pdev, rnpgbe_driver_name);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "pci_request_selected_regions failed %d\n", err);
> > + goto err_disable_dev;
> > + }
> > +
> > + pci_set_master(pdev);
> > + err = pci_save_state(pdev);
> > + if (err) {
> > + dev_err(&pdev->dev, "pci_save_state failed %d\n", err);
> > + goto err_free_regions;
> > + }
> > +
> > + return 0;
> > +err_free_regions:
> > + pci_release_mem_regions(pdev);
> > +err_disable_dev:
> > + 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
> > + **/
> > +static void rnpgbe_dev_shutdown(struct pci_dev *pdev)
> > +{
> > + 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)
> > +{
> > + rnpgbe_dev_shutdown(pdev);
>
> Is this the only user of rnpgbe_dev_shutdown?
>
No, it maybe called by suspend callback in the future.
Device relative code will be added in rnpgbe_dev_shutdown, and power state
code in rnpgbe_shutdown. This is the same like other driver did
(ixgbe_main.c)
> > +}
> > +
> > +static struct pci_driver rnpgbe_driver = {
> > + .name = rnpgbe_driver_name,
> > + .id_table = rnpgbe_pci_tbl,
> > + .probe = rnpgbe_probe,
> > + .remove = rnpgbe_remove,
> > + .shutdown = rnpgbe_shutdown,
> > +};
> > +
> > +module_pci_driver(rnpgbe_driver);
> > +
> > +MODULE_DEVICE_TABLE(pci, rnpgbe_pci_tbl);
> > +MODULE_AUTHOR("Mucse Corporation, <[email protected]>");
> > +MODULE_DESCRIPTION("Mucse(R) 1 Gigabit PCI Express Network Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
> >
>
> --
> Als deutscher Tourist im Ausland steht man vor der Frage, ob man sich
> anständig benehmen muss oder ob schon deutsche Touristen dagewesen sind.
> (Kurt Tucholsky)
Thanks for your feedback.
