Messages in this thread | | | Date | Mon, 28 Apr 2008 07:10:06 -0700 | From | Arjan van de Ven <> | Subject | Re: [patch] x86, voyager: fix ioremap_nocache() |
| |
On Sun, 27 Apr 2008 18:39:24 -0400 Jeff Garzik <jeff@garzik.org> wrote:
> James Bottomley wrote: > > Here's another piece of the x86 API that's designed to be cached. > > The dma_declare_coherent_memory() usually represents behind bridge > > memory that's fully participatory in the coherence model. > > > > Making it uncached damages the utility of this memory because doing > > cacheline sized burst cycles when needed to it is far faster than > > individual byte/word/quad writes. > > > > Signed-off-by: James Bottomley > > <James.Bottomley@HansenPartnership.com> > > > > --- > > > > diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c > > index 388b113..df83ffd 100644 > > --- a/arch/x86/kernel/pci-dma.c > > +++ b/arch/x86/kernel/pci-dma.c > > @@ -214,7 +214,7 @@ int dma_declare_coherent_memory(struct device > > *dev, dma_addr_t bus_addr, > > /* FIXME: this routine just ignores > > DMA_MEMORY_INCLUDES_CHILDREN */ > > - mem_base = ioremap(bus_addr, size); > > + mem_base = ioremap_cache(bus_addr, size); > > if (!mem_base) > > goto out;
this patch patch is likely broken on x86; or rather, anyone who uses it is... thinking you can find cache coherent memory on a PCI or similar bus that is actually cachable... keep dreaming. (for now; there's talk about extending PCI)
> I would rather change drivers to use ioremap_nocache(), and leave the > API as-is. > > Isn't there Yet More Breakage in lib/iomap.c, given these new > semantics? > > if (flags & IORESOURCE_MEM) { > if (flags & IORESOURCE_CACHEABLE) > return ioremap(start, len); > return ioremap_nocache(start, len); > } > > Any driver using pci_iomap() (libata, and others) is affected.
only if you map ROM's. Anything but ROMs you cannot set IORESOURCE_CACHEABLE on... since PCI MMIO memory isn't cache coherent per se. (it's cache coherent on x86 by virtue of being uncachable ;-) Thankfully Linux doesn't do that.
> I disagree with this semantics change. A number of code places _and > drivers_ GET IT RIGHT, and these are all broken now?
Can you list one that gets it actually right ? (cachable pretty much means: "the cpu is the only one changing it AND there is no side effect of reading or writing")
-- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org
| |