RE: [PATCH net-next v20 01/13] rtase: Add pci table supported in this module

From: Justin Lai
Date: Mon Jun 17 2024 - 04:14:53 EST


> > Add pci table supported in this module, and implement pci_driver
> > function to initialize this driver, remove this driver, or shutdown this driver.
>
> Can a summary phrase like “Add support for a PCI table” be a bit nicer?
>

Yes, thank you for your suggestion. I will make the change.

>
>
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > @@ -0,0 +1,640 @@
>
> > +static int rtase_init_one(struct pci_dev *pdev,
> > + const struct pci_device_id *ent) {
>
> > + /* identify chip attached to board */
> > + if (!rtase_check_mac_version_valid(tp)) {
> > + return dev_err_probe(&pdev->dev, -ENODEV,
> > + "unknown chip version, contact
> rtase "
> > + "maintainers (see MAINTAINERS
> file)\n");
> > + }
>
>
> * May curly brackets be omitted here?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume
> ntation/process/coding-style.rst?h=v6.10-rc3#n197
>
> * Would you like to keep the message (from such string literals) in a single
> line?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume
> ntation/process/coding-style.rst?h=v6.10-rc3#n116
>

Thank you for your suggestions, I will make changes based on both of the
suggestions mentioned above.

>
>
> > + dev->features |= NETIF_F_IP_CSUM;
> > + dev->features |= NETIF_F_HIGHDMA;
>
> > + dev->hw_features |= NETIF_F_RXALL;
> > + dev->hw_features |= NETIF_F_RXFCS;
>
>
> How do you think about to reduce such assignment statements (if all desired
> software options would be passed at once)?

I think your suggestion is feasible, and I will modify it accordingly.

>
> Regards,
> Markus