[PATCH][2.4] VIO infrastructure update - review request

[Santiago Leon]

Hollis...

Thanks for the extra explanations... I think I agree with you that the
irq and dma should be unconditionally set the way you explained it...
I'll make that change to the patch...

And yes, I have modified and tested this changes on veth and vscsi...

Hollis Blanchard wrote:

> On Wednesday, Oct 29, 2003, at 19:30 US/Central, Santiago Leon wrote:
>
>>
>> The attached patch is a reconstruction of the virtual I/O
>> infrastructure, along with the proper changes to the Virtual Ethernet
>> driver... The objective was to clean up the interface, structs, and
>> functions... Some of the device initialization code was moved to each
>> virtual driver (sort of what other buses like PCI do), making the
>> drivers module-friendly and ready for hotplug...
>
>
> This patch is important because the current VIO bus code in 2.4 has a
> lot of PCI-specific features which don't make sense for virtual devices.
> For example, it maintains separate global and per-bus lists of devices
> (even though logically it's hard to see how we could end up with
> multiple virtual busses), and it tries to emulate PCI's vendor/device
> identifiers with hardcoded constants. That changes the drivers' device
> tables like so:
>
>>  static struct vio_device_id ibmveth_device_table[] __devinitdata= {
>> -    { 0x1014, 0x0001, 0, 0, 0, 0, 0},
>> +    { "network", "IBM,l-lan"},
>>      { 0,}
>>  };
>
> All those 0's were unused, and the vendor/device IDs were completely
> artificial. I think this patch is clearly better.
>
> Also, there was some driver-specific code in vio.c, for example
> knowledge about irq and dma properties between different types of devices.
>
> One change I'd like to see: instead of having each driver call
> vio_get_irq(), rather have vio.c set the irq for it. Something like
> this, unconditionally in vio_register_device():
>
>     uint *intrs = (uint *)get_property(node, "interrupts", NULL);
>     if (intrs) {
>         vio_dev->interrupts = *intrs;
>     } else {
>         vio_dev->interrupts = -1;
>     }
> In other words, if the OF node has an "interrupts" property, store the
> value into struct vio_dev. If not, store -1 (or whatever). Same for DMA,
> but not for things that really are device-specific, e.g. MAC address.
>
> Also, Santi didn't mention in the mail that he's modified and tested
> ibmveth.c and I believe VSCSI as well.
>

--
********************************************************************
Santiago A. Leon
Power Linux Development
IBM Linux Technology Center
Off: (919) 254-6048  T/L: 444-6048  Fax: (919) 543-7378


** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/