[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240609.211335.6182990734312389.fujita.tomonori@gmail.com>
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.
Powered by blists - more mailing lists