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

[Alexander Graf]

On 02.06.14 21:24, BALATON Zoltan wrote:
> 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?

How about we leave the PCI magic be PCI magic and just provide a 
function that enables bus mastering?

>
>>> +    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.)

Ok, I think this might fall apart on real hardware, but I doubt anyone 
really cares these days, so just setting BUS_MASTER works for me.


Alex