[OpenBIOS] [PATCH] Add USB OHCI + HID driver
Alexander Graf
agraf at suse.de
Tue Jun 3 00:04:43 CEST 2014
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
More information about the OpenBIOS
mailing list