Received-SPF: pass (google.com: domain of linux-kernel+bounces-212909-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <bb0e9957-816e-4e36-b85c-6c5501e76a8a@web.de>
Date: Thu, 13 Jun 2024 11:01:44 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Justin Lai <justinlai0215@realtek.com>, netdev@vger.kernel.org,
 "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Simon Horman <horms@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>, Andrew Lunn <andrew@lunn.ch>,
 Hariprasad Kelam <hkelam@marvell.com>, Jiri Pirko <jiri@resnulli.us>,
 Larry Chiu <larry.chiu@realtek.com>, Ping-Ke Shih <pkshih@realtek.com>,
 Ratheesh Kannoth <rkannoth@marvell.com>
References: <20240607084321.7254-2-justinlai0215@realtek.com>
Subject: Re: [PATCH net-next v20 01/13] rtase: Add pci table supported in this
 module
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240607084321.7254-2-justinlai0215@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:M4s3FoRaGaU=;JmwsZkCNwoB8ywQRkDaspQw/4V9
 Vc4m1GsRiEu3I6hLNeNVbTbTqzr9zZU263/oFIRJViVmzbOZHbiWTn9TuNcDVwDHiNa/9IZB9
 FgUvy8iqhjengsAhUOc4v5E/ZGnhn1bUdUgKZWVfhuEdvGpYD6pBs0vcTC+F1thOUq9GEt6i2
 LCRe3e4YPWeveynrkoBF/pkCAmLCAfEDFoXoV5GUUTe+OjKUEtFrxYrEdSNo5sAYNPHdyULwc
 OhptEaMbvHJUBcbuSgqjmXXYd3ND5CfWuilsxhlyMqpUqGEi8FgSlmkN7LC5pwNpPRZhh+cfR
 +kGKHTdgOOaoiSyyowjc1fthvSLvKJIp5UBhCJRRJVLeRq5YfGB0oEMX2c3bbFds7ljvlJLvI
 d5477V6QSUcv7Rj8QipBSAzem5SXaK+H35XS7WvUj5vb/ssj/UVEy4nBvrlA1/njOs8DtY6Iz
 DkcvNv7mG1wDjmJSY6f78PcrZjgmHBVa4CPOefyIBXPWOxecUydN4MhvReEnGFvZVhaV5RRa/
 Qol+oYfYpxAQC6JO/qqY+ZGdzjyOeVhpCy/6KQtHVnGoM9S7BqyoVXrl0pl1ti+glC06GKems
 IT254qrtt/3NDapeGlFYHOIPH1LgOb4LvEYkMg2j771P+ArnQoUFVrKm3r0t33MSreNxi+dnV
 CkHGAFDnYXQPihM3UIG4DXhrdSlaXr4E2w3KOC1a+UUKK/u6vc4EUCJwTT1bA73uToHwS9/cP
 8hJacodTtMGVsTiREAqsjFiUYyOnfY7uiw1aOyWAQFapk98OBb3ZczNDmd3M83FMWauuneEH5
 GS4ienTPvNPuB7lbNqYCytJUmRnGLV5KSVGVr/KIQPwto=

> Add pci table supported in this module, and implement pci_driver functio=
n
> to initialize this driver, remove this driver, or shutdown this driver.

Can a summary phrase like =E2=80=9CAdd support for a PCI table=E2=80=9D be=
 a bit nicer?


=E2=80=A6
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> @@ -0,0 +1,640 @@
=E2=80=A6
> +static int rtase_init_one(struct pci_dev *pdev,
> +			  const struct pci_device_id *ent)
> +{
=E2=80=A6
> +	/* 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");
> +	}
=E2=80=A6

* May curly brackets be omitted here?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/=
Documentation/process/coding-style.rst?h=3Dv6.10-rc3#n197

* Would you like to keep the message (from such string literals) in a sing=
le line?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/=
Documentation/process/coding-style.rst?h=3Dv6.10-rc3#n116


=E2=80=A6
> +	dev->features |=3D NETIF_F_IP_CSUM;
> +	dev->features |=3D NETIF_F_HIGHDMA;
=E2=80=A6
> +	dev->hw_features |=3D NETIF_F_RXALL;
> +	dev->hw_features |=3D NETIF_F_RXFCS;
=E2=80=A6

How do you think about to reduce such assignment statements
(if all desired software options would be passed at once)?

Regards,
Markus