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

[Justin Lai <justinlai0215@realtek.com>]

> > 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