[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3e4dc0f5c9a449f7905f436e097f80f6@realtek.com>
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
Powered by blists - more mailing lists