[Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Stefan Weil]

The new devices added here are still not functional -
partially because some patches are still missing,
partially because I cannot test them. Nevertheless
they belong to the same family and will be supported
by this driver some day.

As soon as they work, they will also be added to hw/pci.c.

This patch is a step to synchronize my maintainer version
of eepro100.c (git://repo.or.cz/qemu/ar7.git) with the
version integrated in QEMU.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 3f84e26..1d66edb 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -78,10 +78,15 @@
 #define MAX_ETH_FRAME_SIZE 1514
 
 /* This driver supports several different devices which are declared here. */
+#define i82550          0x82550
 #define i82551          0x82551
+#define i82557A         0x82557a
 #define i82557B         0x82557b
 #define i82557C         0x82557c
+#define i82558A         0x82558a
 #define i82558B         0x82558b
+#define i82559A         0x82559a
+#define i82559B         0x82559b
 #define i82559C         0x82559c
 #define i82559ER        0x82559e
 #define i82562          0x82562
@@ -1758,9 +1763,9 @@ static void nic_cleanup(VLANClientState *vc)
     eeprom93xx_free(s->eeprom);
 }
 
-static int pci_nic_uninit(PCIDevice *dev)
+static int pci_nic_uninit(PCIDevice *pci_dev)
 {
-    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
+    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
 
     cpu_unregister_io_memory(s->mmio_index);
 
@@ -1814,35 +1819,116 @@ static int nic_init(PCIDevice *pci_dev, uint32_t device)
     return 0;
 }
 
-static int pci_i82551_init(PCIDevice *dev)
+static int pci_i82550_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82550);
+}
+
+static int pci_i82551_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82551);
+}
+
+static int pci_i82557a_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82557A);
+}
+
+static int pci_i82557b_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82557B);
+}
+
+static int pci_i82557c_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82557C);
+}
+
+static int pci_i82558a_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82558A);
+}
+
+static int pci_i82558b_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82558B);
+}
+
+static int pci_i82559a_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82559A);
+}
+
+static int pci_i82559b_init(PCIDevice *pci_dev)
+{
+    return nic_init(pci_dev, i82559B);
+}
+
+static int pci_i82559c_init(PCIDevice *pci_dev)
 {
-    return nic_init(dev, i82551);
+    return nic_init(pci_dev, i82559C);
 }
 
-static int pci_i82557b_init(PCIDevice *dev)
+static int pci_i82559er_init(PCIDevice *pci_dev)
 {
-    return nic_init(dev, i82557B);
+    return nic_init(pci_dev, i82559ER);
 }
 
-static int pci_i82559er_init(PCIDevice *dev)
+static int pci_i82562_init(PCIDevice *pci_dev)
 {
-    return nic_init(dev, i82559ER);
+    return nic_init(pci_dev, i82562);
 }
 
 static PCIDeviceInfo eepro100_info[] = {
     {
+        .qdev.name = "i82550",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82550_init,
+    },{
         .qdev.name = "i82551",
         .qdev.size = sizeof(EEPRO100State),
         .init      = pci_i82551_init,
     },{
+        .qdev.name = "i82557a",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82557a_init,
+    },{
         .qdev.name = "i82557b",
         .qdev.size = sizeof(EEPRO100State),
         .init      = pci_i82557b_init,
     },{
+        .qdev.name = "i82557c",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82557c_init,
+    },{
+        .qdev.name = "i82558a",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82558a_init,
+    },{
+        .qdev.name = "i82558b",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82558b_init,
+    },{
+        .qdev.name = "i82559a",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82559a_init,
+    },{
+        .qdev.name = "i82559b",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82559b_init,
+    },{
+        .qdev.name = "i82559c",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82559c_init,
+    },{
         .qdev.name = "i82559er",
         .qdev.size = sizeof(EEPRO100State),
         .init      = pci_i82559er_init,
     },{
+        .qdev.name = "i82562",
+        .qdev.size = sizeof(EEPRO100State),
+        .init      = pci_i82562_init,
+    },{
         /* end of list */
     }
 };
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Anthony Liguori]

Stefan Weil wrote:
> The new devices added here are still not functional -
> partially because some patches are still missing,
> partially because I cannot test them. Nevertheless
> they belong to the same family and will be supported
> by this driver some day.
>
> As soon as they work, they will also be added to hw/pci.c.
>   

I don't like the idea of adding devices that are known to not work.  
This is why PPC is such a mess.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Stefan Weil]

Anthony Liguori schrieb:
> Stefan Weil wrote:
>> The new devices added here are still not functional -
>> partially because some patches are still missing,
>> partially because I cannot test them. Nevertheless
>> they belong to the same family and will be supported
>> by this driver some day.
>>
>> As soon as they work, they will also be added to hw/pci.c.
>>   
>
> I don't like the idea of adding devices that are known to not work. 
> This is why PPC is such a mess.
>
> Regards,
>
> Anthony Liguori
>

Anthony, you asked me to send my maintainer version from git://repo.or.cz/qemu/ar7.git
in small patches. Of course, the final goal is to have devices which work.

To have a list of devices which should be supported helps other people who want
to contribute to eepro100.c. They won't write code just for i82557c if the same
code should be applied to i82557[ab] as well, for example.

Regards
Stefan

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Anthony Liguori]

Stefan Weil wrote:
> Anthony, you asked me to send my maintainer version from git://repo.or.cz/qemu/ar7.git
> in small patches. Of course, the final goal is to have devices which work.
>
> To have a list of devices which should be supported helps other people who want
> to contribute to eepro100.c. They won't write code just for i82557c if the same
> code should be applied to i82557[ab] as well, for example.
>   

You can do that with comments instead of exposing broken devices to an 
end-user.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Stefan Weil]

Anthony Liguori schrieb:
> Stefan Weil wrote:
>> Anthony, you asked me to send my maintainer version from
>> git://repo.or.cz/qemu/ar7.git
>> in small patches. Of course, the final goal is to have devices which
>> work.
>>
>> To have a list of devices which should be supported helps other
>> people who want
>> to contribute to eepro100.c. They won't write code just for i82557c
>> if the same
>> code should be applied to i82557[ab] as well, for example.
>>   
>
> You can do that with comments instead of exposing broken devices to an
> end-user.
>
> Regards,
>
> Anthony Liguori
>

1. The new devices are not exposed to end-users, at least not for the
moment.

2. Many qemu devices are more or less "broken". This is quite normal for an
    emulation, because developers of those emulations only have limited
    documentation / resources / testing capabilities.

Many users of qemu don't expect a perfect system, and those who do will
stick to those devices which work for them.

So a reasonable way might be to expose many devices to end-users, but
to classify them as stable / testing / experimental (like it is done for the
host and target support).

Regards
Stefan Weil

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Anthony Liguori]

Stefan Weil wrote:
> Anthony Liguori schrieb:
>   
>> Stefan Weil wrote:
>>     
>>> Anthony, you asked me to send my maintainer version from
>>> git://repo.or.cz/qemu/ar7.git
>>> in small patches. Of course, the final goal is to have devices which
>>> work.
>>>
>>> To have a list of devices which should be supported helps other
>>> people who want
>>> to contribute to eepro100.c. They won't write code just for i82557c
>>> if the same
>>> code should be applied to i82557[ab] as well, for example.
>>>   
>>>       
>> You can do that with comments instead of exposing broken devices to an
>> end-user.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>     
>
> 1. The new devices are not exposed to end-users, at least not for the
> moment.
>   

They are, or will be soon, thanks to qdev.

> 2. Many qemu devices are more or less "broken". This is quite normal for an
>     emulation, because developers of those emulations only have limited
>     documentation / resources / testing capabilities.
>   

Works in some circumstances and completely untested and expected to fail 
are two separate things.

> So a reasonable way might be to expose many devices to end-users, but
> to classify them as stable / testing / experimental (like it is done for the
> host and target support).
>   

Code that has no chance of working shouldn't be in the tree because it's 
untestable.  Untestable code will rot.  More to the point, what's the 
point of having untested code in the tree when the expectation is that 
it won't work at all?  Who does it benefit?

Regards,

Anthony Liguori

> Regards
> Stefan Weil
>
>   

Re: [Qemu-devel] [PATCH] eepro100: Add more i825xx devices

[Stefan Weil]

Anthony Liguori schrieb:
> Stefan Weil wrote:
>> Anthony Liguori schrieb:
>>  
>>> Stefan Weil wrote:
>>>    
>>>> Anthony, you asked me to send my maintainer version from
>>>> git://repo.or.cz/qemu/ar7.git
>>>> in small patches. Of course, the final goal is to have devices which
>>>> work.
>>>>
>>>> To have a list of devices which should be supported helps other
>>>> people who want
>>>> to contribute to eepro100.c. They won't write code just for i82557c
>>>> if the same
>>>> code should be applied to i82557[ab] as well, for example.
>>>>         
>>> You can do that with comments instead of exposing broken devices to an
>>> end-user.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>     
>>
>> 1. The new devices are not exposed to end-users, at least not for the
>> moment.
>>   
>
> They are, or will be soon, thanks to qdev.

I thought so.

>
>> 2. Many qemu devices are more or less "broken". This is quite normal
>> for an
>>     emulation, because developers of those emulations only have limited
>>     documentation / resources / testing capabilities.
>>   
>
> Works in some circumstances and completely untested and expected to
> fail are two separate things.
>
>> So a reasonable way might be to expose many devices to end-users, but
>> to classify them as stable / testing / experimental (like it is done
>> for the
>> host and target support).
>>   
>
> Code that has no chance of working shouldn't be in the tree because
> it's untestable.  Untestable code will rot.  More to the point, what's
> the point of having untested code in the tree when the expectation is
> that it won't work at all?  Who does it benefit?
>
> Regards,
>
> Anthony Liguori
>

There is a misunderstanding: all i825xx belong to the same ethernet
controller family,
and all of them will work with drivers which only use standard (or
basic) features,
(as soon as you have applied all patches which are still needed -
without them,
some very basic features like endianess, multicast or simplified mode
don't work) .

So the emulation will already work basically and provide the correct PCI
ids.
Some very simple fixes needed are already in my code base (which still has
to be integrated into qemu), for example checking for i82557c has to be
extended
to check for i8255a, i8255b, too (in most cases).

Nevertheless I don't claim that everything works out of the box because
I cannot test every device with all applications. Other users who need
a special device can test it, because a large part of the code will work
and the missing parts can be fixed thanks to extensive log messages.

Special features of those devices will only work as far as they are
implemented.
Among those features are things like wake-on-lan or power save modi, things
which none of qemu's ethernet drivers emulates today - so no reason for
panic :-)

Regards
Stefan Weil