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

[BALATON Zoltan]

On Mon, 2 Jun 2014, Alexander Graf wrote:
>> @@ -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?

In include/arch/ppc/pci.h:
#define PCI_ADDR(bus, dev, fn) \
     ((pci_addr) (0x80000000u \
                 | (uint32_t) (bus) << 16 \
                 | (uint32_t) (dev) << 11 \
                 | (uint32_t) (fn) << 8))

#define PCI_BUS(pcidev) ((uint8_t) ((pcidev) >> 16))
#define PCI_DEV(pcidev) ((uint8_t) ((pcidev) >> 11) & 0x1f)
#define PCI_FN(pcidev) ((uint8_t) ((pcidev) >> 8) & 7)

In drivers/pci.c:ob_configure_pci_device()

config.dev = addr & 0x00FFFFFF;

A possible alternative would be

PCI_ADDR(PCI_BUS(config->dev), PCI_DEV(config->dev), PCI_FN(config->dev))

Is that any better?

>> +    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?

The existing code already takes care of configuring pci devices but did 
not set the bus master bit. (I also had to do the same for the network 
card for DMA to work correctly. This will be in a separate patch.)

Regards,
BALATON Zoltan