diff mbox series

[v2,02/11] pci_ids/tulip: Add PCI vendor ID for HP and use it in tulip

Message ID 20231017154645.95844-3-deller@kernel.org (mailing list archive)
State New, archived
Headers show
Series target/hppa: Add emulation of a C3700 HP-PARISC workstation | expand

Commit Message

Helge Deller Oct. 17, 2023, 3:46 p.m. UTC 
From: Helge Deller <deller@gmx.de>

Signed-off-by: Helge Deller <deller@gmx.de>
---
 hw/net/tulip.c           | 2 +-
 include/hw/pci/pci_ids.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

BALATON Zoltan Oct. 17, 2023, 4:13 p.m. UTC | #1 
On Tue, 17 Oct 2023, deller@kernel.org wrote:
> From: Helge Deller <deller@gmx.de>
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
> hw/net/tulip.c           | 2 +-
> include/hw/pci/pci_ids.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 915e5fb595..11d866e431 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, void *data)
>     k->exit = pci_tulip_exit;
>     k->vendor_id = PCI_VENDOR_ID_DEC;
>     k->device_id = PCI_DEVICE_ID_DEC_21143;
> -    k->subsystem_vendor_id = 0x103c;
> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>     k->subsystem_id = 0x104f;
>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>     dc->vmsd = &vmstate_pci_tulip;
> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 85469b9b53..3c0e72df0e 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -171,6 +171,8 @@
> #define PCI_VENDOR_ID_DEC                0x1011
> #define PCI_DEVICE_ID_DEC_21143          0x0019
>
> +#define PCI_VENDOR_ID_HP                 0x103c
> +

Did not notice this in first round, sorry. These seems to be sorted 
(there's a comment further up about that) so this should be between AMD 
and TI a bit more down.

Regards,
BALATON Zoltan

> #define PCI_VENDOR_ID_CIRRUS             0x1013
>
> #define PCI_VENDOR_ID_IBM                0x1014
>
Helge Deller Oct. 17, 2023, 5:58 p.m. UTC | #2 
On 10/17/23 18:13, BALATON Zoltan wrote:
> On Tue, 17 Oct 2023, deller@kernel.org wrote:
>> From: Helge Deller <deller@gmx.de>
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>> hw/net/tulip.c           | 2 +-
>> include/hw/pci/pci_ids.h | 2 ++
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>> index 915e5fb595..11d866e431 100644
>> --- a/hw/net/tulip.c
>> +++ b/hw/net/tulip.c
>> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, void *data)
>>     k->exit = pci_tulip_exit;
>>     k->vendor_id = PCI_VENDOR_ID_DEC;
>>     k->device_id = PCI_DEVICE_ID_DEC_21143;
>> -    k->subsystem_vendor_id = 0x103c;
>> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>>     k->subsystem_id = 0x104f;
>>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>>     dc->vmsd = &vmstate_pci_tulip;
>> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>> index 85469b9b53..3c0e72df0e 100644
>> --- a/include/hw/pci/pci_ids.h
>> +++ b/include/hw/pci/pci_ids.h
>> @@ -171,6 +171,8 @@
>> #define PCI_VENDOR_ID_DEC                0x1011
>> #define PCI_DEVICE_ID_DEC_21143          0x0019
>>
>> +#define PCI_VENDOR_ID_HP                 0x103c
>> +
>
> Did not notice this in first round, sorry. These seems to be sorted
> (there's a comment further up about that) so this should be between
> AMD and TI a bit more down.

The list isn't sorted at all. I think the comment just wants to
say that you should mention the vendor before the devices.
Anyway, as the list currently is, there are multiple positions
where HP could be added...

Helge
BALATON Zoltan Oct. 17, 2023, 7:19 p.m. UTC | #3 
On Tue, 17 Oct 2023, Helge Deller wrote:
> On 10/17/23 18:13, BALATON Zoltan wrote:
>> On Tue, 17 Oct 2023, deller@kernel.org wrote:
>>> From: Helge Deller <deller@gmx.de>
>>> 
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> ---
>>> hw/net/tulip.c           | 2 +-
>>> include/hw/pci/pci_ids.h | 2 ++
>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>> index 915e5fb595..11d866e431 100644
>>> --- a/hw/net/tulip.c
>>> +++ b/hw/net/tulip.c
>>> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, 
>>> void *data)
>>>     k->exit = pci_tulip_exit;
>>>     k->vendor_id = PCI_VENDOR_ID_DEC;
>>>     k->device_id = PCI_DEVICE_ID_DEC_21143;
>>> -    k->subsystem_vendor_id = 0x103c;
>>> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>>>     k->subsystem_id = 0x104f;
>>>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>>>     dc->vmsd = &vmstate_pci_tulip;
>>> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>>> index 85469b9b53..3c0e72df0e 100644
>>> --- a/include/hw/pci/pci_ids.h
>>> +++ b/include/hw/pci/pci_ids.h
>>> @@ -171,6 +171,8 @@
>>> #define PCI_VENDOR_ID_DEC                0x1011
>>> #define PCI_DEVICE_ID_DEC_21143          0x0019
>>> 
>>> +#define PCI_VENDOR_ID_HP                 0x103c
>>> +
>> 
>> Did not notice this in first round, sorry. These seems to be sorted
>> (there's a comment further up about that) so this should be between
>> AMD and TI a bit more down.
>
> The list isn't sorted at all. I think the comment just wants to

It is still mostly sorted except where people did not get how. Try

grep PCI_VENDOR_ID_ pci_ids.h

> say that you should mention the vendor before the devices.

I think it says that PCI_VENDOR_IDs should be sorted and then DEVICE_IDs 
within them should also be sorted but device IDs intervene VENDOR_IDs so 
the sorting of VENDOR_IDs may not be obvious at first sight.

> Anyway, as the list currently is, there are multiple positions
> where HP could be added...

Yes, some IDs already break this sorting but we could still avoid breaking 
it more. Not a big deal though.

Regards,
BALATON Zoltan
Helge Deller Oct. 17, 2023, 7:25 p.m. UTC | #4 
On 10/17/23 21:19, BALATON Zoltan wrote:
> On Tue, 17 Oct 2023, Helge Deller wrote:
>> On 10/17/23 18:13, BALATON Zoltan wrote:
>>> On Tue, 17 Oct 2023, deller@kernel.org wrote:
>>>> From: Helge Deller <deller@gmx.de>
>>>>
>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>> ---
>>>> hw/net/tulip.c           | 2 +-
>>>> include/hw/pci/pci_ids.h | 2 ++
>>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>>> index 915e5fb595..11d866e431 100644
>>>> --- a/hw/net/tulip.c
>>>> +++ b/hw/net/tulip.c
>>>> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, void *data)
>>>>     k->exit = pci_tulip_exit;
>>>>     k->vendor_id = PCI_VENDOR_ID_DEC;
>>>>     k->device_id = PCI_DEVICE_ID_DEC_21143;
>>>> -    k->subsystem_vendor_id = 0x103c;
>>>> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>>>>     k->subsystem_id = 0x104f;
>>>>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>>>>     dc->vmsd = &vmstate_pci_tulip;
>>>> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>>>> index 85469b9b53..3c0e72df0e 100644
>>>> --- a/include/hw/pci/pci_ids.h
>>>> +++ b/include/hw/pci/pci_ids.h
>>>> @@ -171,6 +171,8 @@
>>>> #define PCI_VENDOR_ID_DEC                0x1011
>>>> #define PCI_DEVICE_ID_DEC_21143          0x0019
>>>>
>>>> +#define PCI_VENDOR_ID_HP                 0x103c
>>>> +
>>>
>>> Did not notice this in first round, sorry. These seems to be sorted
>>> (there's a comment further up about that) so this should be between
>>> AMD and TI a bit more down.
>>
>> The list isn't sorted at all. I think the comment just wants to
>
> It is still mostly sorted except where people did not get how. Try
>
> grep PCI_VENDOR_ID_ pci_ids.h
>
>> say that you should mention the vendor before the devices.
>
> I think it says that PCI_VENDOR_IDs should be sorted and then DEVICE_IDs within them should also be sorted but device IDs intervene VENDOR_IDs so the sorting of VENDOR_IDs may not be obvious at first sight.
>
>> Anyway, as the list currently is, there are multiple positions
>> where HP could be added...
>
> Yes, some IDs already break this sorting but we could still avoid breaking it more. [...]

... that's why I added "HP" it after "DEC" :-)

Helge
BALATON Zoltan Oct. 17, 2023, 8:21 p.m. UTC | #5 
On Tue, 17 Oct 2023, Helge Deller wrote:
> On 10/17/23 21:19, BALATON Zoltan wrote:
>> On Tue, 17 Oct 2023, Helge Deller wrote:
>>> On 10/17/23 18:13, BALATON Zoltan wrote:
>>>> On Tue, 17 Oct 2023, deller@kernel.org wrote:
>>>>> From: Helge Deller <deller@gmx.de>
>>>>> 
>>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>>> ---
>>>>> hw/net/tulip.c           | 2 +-
>>>>> include/hw/pci/pci_ids.h | 2 ++
>>>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>>>> index 915e5fb595..11d866e431 100644
>>>>> --- a/hw/net/tulip.c
>>>>> +++ b/hw/net/tulip.c
>>>>> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, 
>>>>> void *data)
>>>>>     k->exit = pci_tulip_exit;
>>>>>     k->vendor_id = PCI_VENDOR_ID_DEC;
>>>>>     k->device_id = PCI_DEVICE_ID_DEC_21143;
>>>>> -    k->subsystem_vendor_id = 0x103c;
>>>>> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>>>>>     k->subsystem_id = 0x104f;
>>>>>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>>>>>     dc->vmsd = &vmstate_pci_tulip;
>>>>> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>>>>> index 85469b9b53..3c0e72df0e 100644
>>>>> --- a/include/hw/pci/pci_ids.h
>>>>> +++ b/include/hw/pci/pci_ids.h
>>>>> @@ -171,6 +171,8 @@
>>>>> #define PCI_VENDOR_ID_DEC                0x1011
>>>>> #define PCI_DEVICE_ID_DEC_21143          0x0019
>>>>> 
>>>>> +#define PCI_VENDOR_ID_HP                 0x103c
>>>>> +
>>>> 
>>>> Did not notice this in first round, sorry. These seems to be sorted
>>>> (there's a comment further up about that) so this should be between
>>>> AMD and TI a bit more down.
>>> 
>>> The list isn't sorted at all. I think the comment just wants to
>> 
>> It is still mostly sorted except where people did not get how. Try
>> 
>> grep PCI_VENDOR_ID_ pci_ids.h
>> 
>>> say that you should mention the vendor before the devices.
>> 
>> I think it says that PCI_VENDOR_IDs should be sorted and then DEVICE_IDs 
>> within them should also be sorted but device IDs intervene VENDOR_IDs so 
>> the sorting of VENDOR_IDs may not be obvious at first sight.
>> 
>>> Anyway, as the list currently is, there are multiple positions
>>> where HP could be added...
>> 
>> Yes, some IDs already break this sorting but we could still avoid breaking 
>> it more. [...]
>
> ... that's why I added "HP" it after "DEC" :-)

But it should be sorted by ID number not name that's why it should be 
between AMD 0x1022 and TI 0x104c.

Regards,
BALATON Zoltan
Helge Deller Oct. 17, 2023, 8:48 p.m. UTC | #6 
On 10/17/23 22:21, BALATON Zoltan wrote:
> On Tue, 17 Oct 2023, Helge Deller wrote:
>> On 10/17/23 21:19, BALATON Zoltan wrote:
>>> On Tue, 17 Oct 2023, Helge Deller wrote:
>>>> On 10/17/23 18:13, BALATON Zoltan wrote:
>>>>> On Tue, 17 Oct 2023, deller@kernel.org wrote:
>>>>>> From: Helge Deller <deller@gmx.de>
>>>>>>
>>>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>>>> ---
>>>>>> hw/net/tulip.c           | 2 +-
>>>>>> include/hw/pci/pci_ids.h | 2 ++
>>>>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>>>>> index 915e5fb595..11d866e431 100644
>>>>>> --- a/hw/net/tulip.c
>>>>>> +++ b/hw/net/tulip.c
>>>>>> @@ -1020,7 +1020,7 @@ static void tulip_class_init(ObjectClass *klass, void *data)
>>>>>>     k->exit = pci_tulip_exit;
>>>>>>     k->vendor_id = PCI_VENDOR_ID_DEC;
>>>>>>     k->device_id = PCI_DEVICE_ID_DEC_21143;
>>>>>> -    k->subsystem_vendor_id = 0x103c;
>>>>>> +    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
>>>>>>     k->subsystem_id = 0x104f;
>>>>>>     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>>>>>>     dc->vmsd = &vmstate_pci_tulip;
>>>>>> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>>>>>> index 85469b9b53..3c0e72df0e 100644
>>>>>> --- a/include/hw/pci/pci_ids.h
>>>>>> +++ b/include/hw/pci/pci_ids.h
>>>>>> @@ -171,6 +171,8 @@
>>>>>> #define PCI_VENDOR_ID_DEC                0x1011
>>>>>> #define PCI_DEVICE_ID_DEC_21143          0x0019
>>>>>>
>>>>>> +#define PCI_VENDOR_ID_HP                 0x103c
>>>>>> +
>>>>>
>>>>> Did not notice this in first round, sorry. These seems to be sorted
>>>>> (there's a comment further up about that) so this should be between
>>>>> AMD and TI a bit more down.
>>>>
>>>> The list isn't sorted at all. I think the comment just wants to
>>>
>>> It is still mostly sorted except where people did not get how. Try
>>>
>>> grep PCI_VENDOR_ID_ pci_ids.h
>>>
>>>> say that you should mention the vendor before the devices.
>>>
>>> I think it says that PCI_VENDOR_IDs should be sorted and then DEVICE_IDs within them should also be sorted but device IDs intervene VENDOR_IDs so the sorting of VENDOR_IDs may not be obvious at first sight.
>>>
>>>> Anyway, as the list currently is, there are multiple positions
>>>> where HP could be added...
>>>
>>> Yes, some IDs already break this sorting but we could still avoid breaking it more. [...]
>>
>> ... that's why I added "HP" it after "DEC" :-)
>
> But it should be sorted by ID number not name that's why it should be between AMD 0x1022 and TI 0x104c.

Oh, I see!
It's sorted by *numeric* PCI ID number.
Will fix in next round or before final pull request.

Helge
diff mbox series

Patch

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 915e5fb595..11d866e431 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -1020,7 +1020,7 @@  static void tulip_class_init(ObjectClass *klass, void *data)
     k->exit = pci_tulip_exit;
     k->vendor_id = PCI_VENDOR_ID_DEC;
     k->device_id = PCI_DEVICE_ID_DEC_21143;
-    k->subsystem_vendor_id = 0x103c;
+    k->subsystem_vendor_id = PCI_VENDOR_ID_HP;
     k->subsystem_id = 0x104f;
     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
     dc->vmsd = &vmstate_pci_tulip;
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index 85469b9b53..3c0e72df0e 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -171,6 +171,8 @@ 
 #define PCI_VENDOR_ID_DEC                0x1011
 #define PCI_DEVICE_ID_DEC_21143          0x0019
 
+#define PCI_VENDOR_ID_HP                 0x103c
+
 #define PCI_VENDOR_ID_CIRRUS             0x1013
 
 #define PCI_VENDOR_ID_IBM                0x1014