[Qemu-devel] Revert bogus usb-ohci change

[Qemu-devel] Revert bogus usb-ohci change

[Paul Brook]

Commit f1698408 "Compile usb-ohci only once" introduces a "be" property to the 
pci-ohci device. The PCI bus is always little-endian, so this is completely 
bogus. A hypothetical big-endian device would actually introduce another 
byteswap.

As discussed previously, if you really want to avoid compiling the device 
multiple times then this needs to be fixed elsewhere. Adding user visible 
properties to devices is definitely not the right solution.

I have reverted this change. If there are other devices that have been hacked 
in the same way then please revert those too.

Paul

[Qemu-devel] Re: Revert bogus usb-ohci change

[Blue Swirl]

On 4/4/10, Paul Brook <paul@codesourcery.com> wrote:
> Commit f1698408 "Compile usb-ohci only once" introduces a "be" property to the
>  pci-ohci device. The PCI bus is always little-endian, so this is completely
>  bogus. A hypothetical big-endian device would actually introduce another
>  byteswap.

Then why there are again byte swaps with big endian target, aren't
they bogus too? Because we don't have the byte swapping bus yet.
Current system is bogus in this respect, so I claim that in general,
the level of bogosity was not changed.

>  As discussed previously, if you really want to avoid compiling the device
>  multiple times then this needs to be fixed elsewhere. Adding user visible
>  properties to devices is definitely not the right solution.
>
>  I have reverted this change. If there are other devices that have been hacked
>  in the same way then please revert those too.

I agree that the user visible property solution was wrong, sorry.

I had a patch "Compile rtl8139 and e1000 only once", which had "be"
property too but I didn't commit it because the property passing
didn't feel right and I didn't have a plan how to pass that from
higher levels.

Other patches didn't use the property method.

[Qemu-devel] Re: Revert bogus usb-ohci change

[Paul Brook]

> On 4/4/10, Paul Brook <paul@codesourcery.com> wrote:
> > Commit f1698408 "Compile usb-ohci only once" introduces a "be" property
> > to the pci-ohci device. The PCI bus is always little-endian, so this is
> > completely bogus. A hypothetical big-endian device would actually
> > introduce another byteswap.
> 
> Then why there are again byte swaps with big endian target, aren't
> they bogus too? Because we don't have the byte swapping bus yet.
> Current system is bogus in this respect, so I claim that in general,
> the level of bogosity was not changed.

The difference is that your patch makes this visible to the user.
Internal warts are only annoying to developers. User visible warts annoy 
and/or confuse everyone. Improving compile time at the expense of end user 
happiness is something I consider to be a bad idea.

Compare this with e.g. pflash_cfi01 where you're trading compile time for code 
size (~100 extra lines).  I'm not entirely convinced this is worth it, but am 
not going to object too strongly. I do however think that your naming 
convention is wrong. Instead of be/le it should be native/cross endian.
It just happens that all the current cross cases are for LE devices on a BE 
target.

Paul