[OpenBIOS] [PATCH v3] Add USB OHCI + HID driver

BALATON Zoltan balaton at eik.bme.hu
Thu Jun 26 13:56:40 CEST 2014


On Thu, 26 Jun 2014, Alexander Graf wrote:
> On 26.06.14 12:55, BALATON Zoltan wrote:
>> On Thu, 26 Jun 2014, Alexander Graf wrote:
>>> On 26.06.14 00:36, BALATON Zoltan wrote:
>>>> On Thu, 26 Jun 2014, Alexander Graf wrote:
>>>>> On 01.06.14 23:55, BALATON Zoltan wrote:
>>>>>> This driver is ported from CoreBoot's libpayload and fixed to work on
>>>>>> big endian CPUs (tested on PPC with QEMU). It is enough to support a
>>>>>> USB keyboard on an Apple Keylargo OHCI compliant USB host and makes
>>>>>> OpenBIOS usable on qemu-system-ppc64 with mac99 machine type as well
>>>>>> as allows to emulate PowerMac3,1 better.
>>>>>> 
>>>>>> Signed-off-by: BALATON Zoltan <balaton at eik.bme.hu>
>>>>>> Acked-by: Alexander Graf <agraf at suse.de>
>>>>>> ---
>>>>>> 
>>>>>> v2: improve init and teardown by moving pci configuration to the driver
>>>>>>      and giving up using the controller on quiesce interface call also
>>>>>>      resetting it to avoid runaway DMA until the OS driver takes over
>>>>>> v3: Only enable bus master bit in PCI config for QEMU
>>>>>> ---
>>>>> 
>>>>> [...]
>>>>> 
>>>>>> +hci_t *
>>>>>> +ohci_pci_init (pci_addr addr)
>>>>>> +{
>>>>>> +    u32 reg_base;
>>>>>> +#if defined(CONFIG_QEMU)
>>>>>> +    uint16_t cmd;
>>>>>> +
>>>>>> +    cmd = pci_config_read16(addr, PCI_COMMAND);
>>>>>> +    cmd |= PCI_COMMAND_BUS_MASTER;
>>>>>> +    pci_config_write16(addr, PCI_COMMAND, cmd);
>>>>>> +#endif
>>>>> 
>>>>> I don't think the driver will work without bus mastering.
>>>> 
>>>> So then apply the first version without this #if.
>>>> 
>>>>> Just don't match on it on non-QEMU machines and add a comment why you 
>>>>> don't.
>>>> 
>>>> Why not? Coreboot is supposed to work on real hardware too.
>>> 
>>> No, it won't because it won't have bus mastering enabled. Do you want to 
>>> be the guy trying to debug why OHCI support doesn't only to find out that 
>>> it's because bus mastering isn't on?
>> 
>> I'm afraid I'll be the one to debug it if it breaks anyway... If it needs 
>> bus mastering everywhere then apply v2 without special casing QEMU. (You 
>> asked to put ifdefs around enabling bus mastering.) What I don't see is why 
>> should we special case this driver for QEMU if
>> 
>> 1. It's supposed to works on real hardware too as it is used in CoreBoot
>>    in such situations
>> 2. I'm not aware of any usage of OpenBIOS on anything else than QEMU
>
> OpenBIOS started off on real Macs :). I don't know if anyone still uses those 
> targets.

So why not let them try and yell if it breaks if they are still around? A 
USB driver would be useful for real Macs with USB keyboard so they might 
want to try and fix it anyway. (Maybe that's why noone uses those targets 
any more.)

>> So restricting this driver for QEMU seems an unnecessary complication.
>> What am I missing?
>
> If you enable bus mastering for the device without taking a lot of care for 
> its PCI host controller, the system might just crash.

If OpenBIOS takes enough care with configuring the host controller then it 
should not crash. If it doesn't then that's not a bug in the OHCI driver 
so why should it be disabled?

> That's why we don't 
> want to enable bus mastering on devices for configurations where we aren't 
> sure it works (read: non-QEMU targets).

Isn't that overly cautious and just making code more complicated by 
putting conditionals everywhere for no good reason other then the fear of 
breaking something that might not be used any more anyway? I could put 
another conditional around the definition of OHCI in pci_database.c and 
have it only enabled for QEMU but I'm arguing that it should work on real 
hardware and thus I don't see why it should not be enabled until someone 
finds it crashes there and even then it would be better to fix the host 
controller config than disabling this driver.

> So either you can give me a lot of confidence that the OHCI driver does not 
> do any DMA and thus works just fine without bus mastering enabled and only

No, I think it needs to do DMA and bus mastering enabled. Here is the 
thread where it was established:

http://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg06587.html

> requires it enabled because of MorphOS again (in which case that should be 
> documented in a comment!)

MorphOS is a different case. It has it's own driver but that currently 
cannot use the OHCI controller on QEMU, maybe because it expects 
OpenFirmware to initalise an IRQ which this driver does not do. I haven't 
investigated that yet as it is not needed for using VNC which is the only 
way to access MorphOS at the moment. So it is not needed for MorphOS but 
for the driver itself.

> or the driver is completely useless on non-QEMU 
> platforms and shouldn't be around because it would probably just time out.

It shouldn't be useless and should work as it is identical to what's in 
CoreBoot (plus big endian fixes) where it is supposedly used on real 
hardware.

Regards,
BALATON Zoltan



More information about the OpenBIOS mailing list