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

[Mark Cave-Ayland]

On 02/06/14 13:07, 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>
>> ---
>>   openbios-devel/config/examples/ppc_config.xml |   3 +
>>   openbios-devel/drivers/Kconfig                |  20 +
>>   openbios-devel/drivers/build.xml              |   4 +
>>   openbios-devel/drivers/pci.c                  |  18 +
>>   openbios-devel/drivers/pci.h                  |   1 +
>>   openbios-devel/drivers/pci_database.c         |  18 +-
>>   openbios-devel/drivers/pci_database.h         |   1 +
>>   openbios-devel/drivers/usb.c                  | 587 ++++++++++++++++
>>   openbios-devel/drivers/usb.h                  | 357 ++++++++++
>>   openbios-devel/drivers/usbhid.c               | 579 ++++++++++++++++
>>   openbios-devel/drivers/usbohci.c              | 920
>> ++++++++++++++++++++++++++
>>   openbios-devel/drivers/usbohci.h              |  45 ++
>>   openbios-devel/drivers/usbohci_private.h      | 270 ++++++++
>>   openbios-devel/drivers/usbohci_rh.c           | 212 ++++++
>>   openbios-devel/include/drivers/pci.h          |   1 +
>>   openbios-devel/include/drivers/usb.h          |   8 +
>>   16 files changed, 3043 insertions(+), 1 deletion(-)
>>   create mode 100644 openbios-devel/drivers/usb.c
>>   create mode 100644 openbios-devel/drivers/usb.h
>>   create mode 100644 openbios-devel/drivers/usbhid.c
>>   create mode 100644 openbios-devel/drivers/usbohci.c
>>   create mode 100644 openbios-devel/drivers/usbohci.h
>>   create mode 100644 openbios-devel/drivers/usbohci_private.h
>>   create mode 100644 openbios-devel/drivers/usbohci_rh.c
>>   create mode 100644 openbios-devel/include/drivers/usb.h
>>
>> diff --git a/openbios-devel/config/examples/ppc_config.xml
>> b/openbios-devel/config/examples/ppc_config.xml
>> index 621b65d..4c14eb6 100644
>> --- a/openbios-devel/config/examples/ppc_config.xml
>> +++ b/openbios-devel/config/examples/ppc_config.xml
>> @@ -80,3 +80,6 @@
>>     <option name="CONFIG_DRIVER_ESCC" type="boolean" value="true"/>
>>     <option name="CONFIG_DRIVER_FW_CFG" type="boolean" value="true"/>
>>     <option name="CONFIG_FW_CFG_ADDR" type="integer" value="0xf0000510"/>
>> +  <option name="CONFIG_DRIVER_USB" type="boolean" value="true"/>
>> +  <option name="CONFIG_DEBUG_USB" type="boolean" value="false"/>
>> +  <option name="CONFIG_USB_HID" type="boolean" value="true"/>
>> diff --git a/openbios-devel/drivers/Kconfig
>> b/openbios-devel/drivers/Kconfig
>> index 7be334b..ce7e7ac 100644
>> --- a/openbios-devel/drivers/Kconfig
>> +++ b/openbios-devel/drivers/Kconfig
>> @@ -36,4 +36,24 @@ config DEBUG_IDE
>>       help
>>         Debug IDE driver
>>
>> +config DRIVER_USB
>> +    bool "USB Support"
>> +    default n
>> +    help
>> +      If you want to be able to use USB devices, enable this option.
>> +
>> +config DEBUG_USB
>> +    depends DRIVER_USB
>> +    bool "Debug USB driver"
>> +    default n
>> +    help
>> +      Debug USB driver
>> +
>> +config USB_HID
>> +    depends DRIVER_USB
>> +    bool "USB driver for HID devices"
>> +    default n
>> +    help
>> +      If you want to be able to use USB keyboard, enable this option.
>> +
>>   endmenu

Does Kconfig actually work with OpenBIOS? I don't think I've ever used 
it...?

>> diff --git a/openbios-devel/drivers/build.xml
>> b/openbios-devel/drivers/build.xml
>> index edec6b5..bd1abd3 100644
>> --- a/openbios-devel/drivers/build.xml
>> +++ b/openbios-devel/drivers/build.xml
>> @@ -22,6 +22,10 @@
>>     <object source="pc_serial.c" condition="DRIVER_PC_SERIAL"/>
>>     <object source="escc.c" condition="DRIVER_ESCC"/>
>>     <object source="fw_cfg.c" condition="DRIVER_FW_CFG"/>
>> +  <object source="usb.c" condition="DRIVER_USB"/>
>> +  <object source="usbhid.c" condition="USB_HID"/>
>> +  <object source="usbohci.c" condition="DRIVER_USB"/>
>> +  <object source="usbohci_rh.c" condition="DRIVER_USB"/>
>>    </library>
>>    <dictionary name="openbios" target="forth">
>> diff --git a/openbios-devel/drivers/pci.c b/openbios-devel/drivers/pci.c
>> index ca92d63..ca4de87 100644
>> --- a/openbios-devel/drivers/pci.c
>> +++ b/openbios-devel/drivers/pci.c
>> @@ -34,6 +34,9 @@
>>   #include "cuda.h"
>>   #include "macio.h"
>>   #endif
>> +#ifdef CONFIG_DRIVER_USB
>> +#include "drivers/usb.h"
>> +#endif
>>   #if defined (CONFIG_DEBUG_PCI)
>>   # define PCI_DPRINTF(format, ...) printk(format, ## __VA_ARGS__)
>> @@ -883,6 +886,21 @@ int i82378_config_cb(const pci_config_t *config)
>>       return 0;
>>   }
>> +int usb_ohci_config_cb(const pci_config_t *config)
>> +{
>> +#ifdef CONFIG_DRIVER_USB
>> +    pci_addr addr = 0x80000000u | config->dev;
>
> Where does this offset come from? Don't we have proper helpers for this?
>
>> +    uint16_t cmd;
>> +
>> +    cmd = pci_config_read16(addr, PCI_COMMAND);
>> +    cmd |= PCI_COMMAND_BUS_MASTER;
>
> Is this really the only bit that should be enabled? Who maps the BARs?
>
> Apart from these minor nits I am in favor of this patch, but would like
> an ack from Mark before I apply it ;).

Sorry I'm a bit late to this - busy with sponsored work at the moment. 
I'm quite surprised/impressed by the size of the patch as I was 
expecting it to be a lot larger which makes me happier we're not adding 
a massive chunk of extra C code to the codebase.

My only query is whether the coreboot licence is compatible with 
OpenBIOS (we should check with Stefan), otherwise there's nothing 
obvious to pick out apart from the Kconfig (although I should add I am 
far from a PCI/USB expert). Other than that, I would be okay if you 
wanted to commit this.


ATB,

Mark.