netdev - Re: [PATCH net-next v9 1/6] net: tn40xx: add pci driver for Tehuti Networks TN40xx chips

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: Sun, 9 Jun 2024 13:17:30 +0200
From: Hans-Frieder Vogt <hfdevel@....net>
To: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org
Cc: andrew@...n.ch, horms@...nel.org, kuba@...nel.org, jiri@...nulli.us,
 pabeni@...hat.com, linux@...linux.org.uk, naveenm@...vell.com,
 jdamato@...tly.com
Subject: Re: [PATCH net-next v9 1/6] net: tn40xx: add pci driver for Tehuti
 Networks TN40xx chips

On 06.06.2024 01.26, FUJITA Tomonori wrote:
> +
> +static const struct pci_device_id tn40_id_table[] = {
> +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_TEHUTI, 0x4022,
> +			 PCI_VENDOR_ID_TEHUTI, 0x3015) },
> +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_TEHUTI, 0x4022,
> +			 PCI_VENDOR_ID_DLINK, 0x4d00) },
> +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_TEHUTI, 0x4022,
> +			 PCI_VENDOR_ID_ASUSTEK, 0x8709) },
> +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_TEHUTI, 0x4022,
> +			 PCI_VENDOR_ID_EDIMAX, 0x8103) },
> +	{ }
> +};
> +

Now that it seems we will have another revision of your patches, maybe
we can also continue to do a bit of cleanup:

Is there any reason why you specifically detail every single card with
vendor ids?
Couldn't it just do with the generic Tehuti device number, i.e.:

static const struct pci_device_id tn40_id_table[] = {
     { PCI_VDEVICE(TEHUTI, 0x4022), 0},
     { }
};

> --- /dev/null
> +++ b/drivers/net/ethernet/tehuti/tn40.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (c) Tehuti Networks Ltd. */
> +
> +#ifndef _TN40_H_
> +#define _TN40_H_
> +
> +#define TN40_DRV_NAME "tn40xx"
> +
> +#define PCI_VENDOR_ID_EDIMAX 0x1432
with my suggestion above, the definition of the vendor EDIMAX would
become obsolete. If needed, it should probably anyway rather be placed
in include/linux/pci_ids.h
> +
> +#endif /* _TN40XX_H */