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, 09 Jun 2024 21:13:35 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: hfdevel@....net
Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org, 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 Sun, 9 Jun 2024 13:17:30 +0200
Hans-Frieder Vogt <hfdevel@....net> wrote:

> 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},
>     { }
> };

The original driver tells that different hardware initialization
routines are required depending on the type of PHY hardware (e.g.,
mdio speed, register value for interrupt). I plan to add such
information to driver_data in the id table when I add new PHY support.

Seems that you could add AQR105 PHY soon so I'll update the table to
put QT2025 PHY specific info into the table.


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

Surely, I'll put it in include/linux/pci_ids.h in v10.