Re: [PATCH] net: tulip: Set PCI revision to match dec21143

[Marek Vasut]

On 6/11/20 12:27 AM, Philippe Mathieu-Daudé wrote:
> On 6/10/20 11:13 PM, Sven Schnelle wrote:
>> On Mon, Jun 08, 2020 at 12:17:11AM +0200, Philippe Mathieu-Daudé wrote:
>>> Hi Sven, could you review thiw one-line patch?
>>>
>>> On 4/18/20 2:25 AM, Marek Vasut wrote:
>>>> The tulip driver claims to emulate dec21143 and it does not emulate 
>>>> dec21142.
>>>> The dec21142 and dec21143 can be discerned by the PCI revision register,
>>>> where dec21142 reports value < 0x20 and dec21143 value >= 0x20. E.g. the
>>>> U-Boot 'tulip' driver also only supports dec21143 and verifies that the
>>>> PCI revision ID is >= 0x20, otherwise refuses to operate such a card.
>>>>
>>>> This patch sets the PCI revision ID to 0x20 to match the dec21143 and
>>>> thus also permits e.g. U-Boot to work with the tulip emulation.
>>>>
>>>> Fixes: 34ea023d4b95 ("net: add tulip (dec21143) driver")
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>> Cc: Prasad J Pandit <pjp@fedoraproject.org>
>>>> Cc: Sven Schnelle <svens@stackframe.org>
>>>> ---
>>>>  hw/net/tulip.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>>>> index 1295f51d07..ffb6c2479a 100644
>>>> --- a/hw/net/tulip.c
>>>> +++ b/hw/net/tulip.c
>>>> @@ -962,6 +962,8 @@ static void pci_tulip_realize(PCIDevice *pci_dev, 
>>>> Error **errp)
>>>>  
>>>>      pci_conf = s->dev.config;
>>>>      pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
>>>> +    /* Anything with revision < 0x20 is DC21142, anything >= 0x20 is 
>>>> DC21143 */
>>>> +    pci_conf[PCI_REVISION_ID] = 0x20;
>>>>  
>>>>      s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
>>>>      tulip_fill_eeprom(s);
>>>>
>>>
>>
>> The intel datasheet for the DEC21143 lists only Rev IDs > 30 for this 
>> particular family:
>>
>> 21143-PB,TB,PC,TC - 0x30
>> 21143-PD,TD - x041
>>
>> but maybe older DEC chips used 0x20 - don't know. The most interesting 
>> question is
>> whether ancient OS' like HP-UX or Windows XP would still work with this 
>> patch, but
>> i don't have test images at hand right now.
> 
> So the question is whether your HP-UX/WinXP images also boot with a
> DEC21142 (you aimed to model a DEC21143, and it is tested anyway).
> 
> Marek, suggestion:
> 
> Make pci_tulip_realize() abstract, add dec21142 and dec21143 models as
> you suggested, making 'tulip' an alias of dec21142 for backward
> compatibility. You can then use the dec21143.

I don't have any way to test dec21142 , I only have dec21143 support in
U-Boot. U-Boot actually checks for this revision field and does not work
with dec21142 , so these older models must be somehow incompatible.
Hence, if we model only the dec21143 anyway, we should set that revision
ID to model it fully.