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