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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 08:14:07 +0000
From: Justin Lai <justinlai0215@...ltek.com>
To: Markus Elfring <Markus.Elfring@....de>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        "Eric
 Dumazet" <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>,
        Simon Horman <horms@...nel.org>
CC: LKML <linux-kernel@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>,
        Hariprasad Kelam <hkelam@...vell.com>, Jiri Pirko <jiri@...nulli.us>,
        "Larry
 Chiu" <larry.chiu@...ltek.com>,
        Ping-Ke Shih <pkshih@...ltek.com>,
        "Ratheesh
 Kannoth" <rkannoth@...vell.com>
Subject: RE: [PATCH net-next v20 01/13] rtase: Add pci table supported in this module

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