| Message ID | 20140506224833.17968.87108.stgit@bhelgaas-glaptop.roam.corp.google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tuesday 06 May 2014 16:48:33 Bjorn Helgaas wrote: > @@ -51,8 +53,12 @@ static void gapspci_fixup_resources(struct pci_dev *dev) > /* > * Redirect dma memory allocations to special memory window. > */ > + region.start = GAPSPCI_DMA_BASE; > + region.end = GAPSPCI_DMA_BASE + GAPSPCI_DMA_SIZE - 1; > + res.flags = IORESOURCE_MEM; > + pcibios_bus_to_resource(dev->bus, &res, ®ion); > BUG_ON(!dma_declare_coherent_memory(&dev->dev, > - GAPSPCI_DMA_BASE, > + res->start, > GAPSPCI_DMA_BASE, > GAPSPCI_DMA_SIZE, > DMA_MEMORY_MAP | Not sure if this is an improvement. I understand the intention, but it's actually possible for the offset to be different both ways: Your patch applies the outbound mem_offset that was provided when registering the MMIO resource for the PCI host bridge. What the driver needs instead is the inbound DMA offset that gets applied by some host bridges that don't have a 1:1 mapping but also don't have an IOMMU. We know that on this particular platform, both are zero, so the original code works and it will keep working with your change, but I think it's a mistake anyway. I have seen both kinds of offsets in the past on real machines, but I am not aware of any where they are both nonzero and have the same value. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 07 May 2014 09:55:16 Arnd Bergmann wrote: > On Tuesday 06 May 2014 16:48:33 Bjorn Helgaas wrote: > > @@ -51,8 +53,12 @@ static void gapspci_fixup_resources(struct pci_dev *dev) > > /* > > * Redirect dma memory allocations to special memory window. > > */ > > + region.start = GAPSPCI_DMA_BASE; > > + region.end = GAPSPCI_DMA_BASE + GAPSPCI_DMA_SIZE - 1; > > + res.flags = IORESOURCE_MEM; > > + pcibios_bus_to_resource(dev->bus, &res, ®ion); > > BUG_ON(!dma_declare_coherent_memory(&dev->dev, > > - GAPSPCI_DMA_BASE, > > + res->start, > > GAPSPCI_DMA_BASE, > > GAPSPCI_DMA_SIZE, > > DMA_MEMORY_MAP | > > Not sure if this is an improvement. I understand the intention, but > it's actually possible for the offset to be different both ways: > > Your patch applies the outbound mem_offset that was provided when > registering the MMIO resource for the PCI host bridge. What the driver > needs instead is the inbound DMA offset that gets applied by some > host bridges that don't have a 1:1 mapping but also don't have an > IOMMU. We know that on this particular platform, both are zero, so > the original code works and it will keep working with your change, > but I think it's a mistake anyway. I have seen both kinds of offsets > in the past on real machines, but I am not aware of any where they > are both nonzero and have the same value. Hmm, looking at it again, it seem even weirder: the address we register for DMA allocations is the same that gets passed into the PCI host for MMIO resources. Something is very strange here. Still, I'd rather not touch it at all, whatever it does. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sh/drivers/pci/fixups-dreamcast.c b/arch/sh/drivers/pci/fixups-dreamcast.c index d6cde700e316..987c788f5a4f 100644 --- a/arch/sh/drivers/pci/fixups-dreamcast.c +++ b/arch/sh/drivers/pci/fixups-dreamcast.c @@ -31,6 +31,8 @@ static void gapspci_fixup_resources(struct pci_dev *dev) { struct pci_channel *p = dev->sysdata; + struct pci_bus_region region; + struct resource res; printk(KERN_NOTICE "PCI: Fixing up device %s\n", pci_name(dev)); @@ -51,8 +53,12 @@ static void gapspci_fixup_resources(struct pci_dev *dev) /* * Redirect dma memory allocations to special memory window. */ + region.start = GAPSPCI_DMA_BASE; + region.end = GAPSPCI_DMA_BASE + GAPSPCI_DMA_SIZE - 1; + res.flags = IORESOURCE_MEM; + pcibios_bus_to_resource(dev->bus, &res, ®ion); BUG_ON(!dma_declare_coherent_memory(&dev->dev, - GAPSPCI_DMA_BASE, + res->start, GAPSPCI_DMA_BASE, GAPSPCI_DMA_SIZE, DMA_MEMORY_MAP |
dma_declare_coherent_memory() needs both the CPU physical address and the bus address of the device memory. They are likely the same on this platform, but in general we should use pcibios_bus_to_resource() to account for any address translation done by the PCI host bridge. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> CC: Magnus Damm <damm@opensource.se> CC: linux-sh@vger.kernel.org --- arch/sh/drivers/pci/fixups-dreamcast.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html