USB mass storage and ARM cache coherency
in [Kernel]
|
Prev: [PATCH 0/3] HID: make raw output callback more flexible
Next: kprobes: Disable booster when CONFIG_PREEMPT=y
From: Sebastian Andrzej Siewior on 2 Feb 2010 04:20 * Catalin Marinas | 2010-02-01 17:29:14 [+0000]: >> So, let's put this in the HCD drivers and be done with it. That is the correct place. MMC -hcd drivers for instance are doing this way. >The patch below is what fixes the I-D cache incoherency issues on ARM. I >don't particularly like the solution but it seems to be the only one >available. The PIO-MMC drivers walk through a scatter list via sg_miter_start() and friends. Those helpers take care of this automaticly. >IMHO, Linux should have functions similar to the DMA API but for PIO >drivers (e.g. pio_map_single/pio_unmap_single) that non-coherent >architectures can define, otherwise being no-ops. Any thoughts? What is wrong with flush_dcache_page() ? And I think linux-arch is the appropriate place. >isp1760: Flush the D-cache for the pipe-in transfer buffers > >From: Catalin Marinas <catalin.marinas(a)arm.com> > >When the HDC driver writes the data to the transfer buffers it pollutes >the D-cache (unlike DMA drivers where the device writes the data). If >the corresponding pages get mapped into user space, there are no >additional cache flushing operations performed and this causes random >user space faults on architectures with separate I and D caches >(Harvard) or those with aliasing D-cache. > >Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com> >Cc: Matthew Dharm <mdharm-kernel(a)one-eyed-alien.net> >Cc: Greg KH <greg(a)kroah.com> >Cc: Sebastian Siewior <bigeasy(a)linutronix.de> >--- > drivers/usb/host/isp1760-hcd.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > >diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c >index 27b8f7c..4d3eeee 100644 >--- a/drivers/usb/host/isp1760-hcd.c >+++ b/drivers/usb/host/isp1760-hcd.c >@@ -18,6 +18,8 @@ > #include <linux/uaccess.h> > #include <linux/io.h> > #include <asm/unaligned.h> >+#include <asm/cacheflush.h> >+#include <asm/memory.h> I'm fine with the patch generally but I don't like the asm headers. cacheflush.h is available on most architectures as far as I can see it but memory.h is only available on arm. So you break the build on !arm and therefore I NAK this. > #include "../core/hcd.h" > #include "isp1760-hcd.h" >@@ -904,6 +906,14 @@ __acquires(priv->lock) > status = 0; > } > >+ if (usb_pipein(urb->pipe) && usb_pipetype(urb->pipe) == PIPE_BULK) { >+ void *ptr; >+ for (ptr = urb->transfer_buffer; >+ ptr < urb->transfer_buffer + urb->transfer_buffer_length; >+ ptr += PAGE_SIZE) >+ flush_dcache_page(virt_to_page(ptr)); >+ } >+ > /* complete() can reenter this HCD */ > usb_hcd_unlink_urb_from_ep(priv_to_hcd(priv), urb); > spin_unlock(&priv->lock); > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Florian Fainelli on 2 Feb 2010 04:40 On Tuesday 02 February 2010 07:58:42 Oliver Neukum wrote: > Am Montag, 1. Februar 2010 23:30:01 schrieb Andreas Mohr: > > I took some time to add your patch to ehci-q.c / ohci-q.c > > (for my *hci-ssb.c ASUS WL-500gP v2), on my now heavily patched-up > > 2.6.31.9, but UNFORTUNATELY it kept locking up the same way as always > > when stopping playback despite being damn sure this time that this patch > > could have the potential to finally fix it ;) > > (I had to replace memory.h with page.h on my arch though, to fix the > > build) > > A moment please. You are using ehci and ohci. Both are using dma. > Why does this issue arise? Because the BCM4710 CPU core is know to have cache problems and we have been trying to workaround this, your problem Andreas is imho a different one. -- Regards, Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Catalin Marinas on 2 Feb 2010 05:00 On Tue, 2010-02-02 at 04:24 +0000, Paul Mundt wrote: > On Mon, Feb 01, 2010 at 03:14:04PM -0500, Alan Stern wrote: > > On Mon, 1 Feb 2010, Catalin Marinas wrote: > > > > > On Fri, 2010-01-29 at 18:54 +0000, Matthew Dharm wrote: > > > > HOWEVER, I firmly believe that the cache-management functions belong with > > > > the driver that actually talks to the low-level hardware, as that's the > > > > only place where you can be 100% certain of what cache operations are > > > > needed. After all, I think someone is working on a USB-over-IP transport, > > > > and trying to manage cache at the usb-storage level in that scenario is > > > > just silly. > > > > > > > > So, let's put this in the HCD drivers and be done with it. > > > > > > The patch below is what fixes the I-D cache incoherency issues on ARM. I > > > don't particularly like the solution but it seems to be the only one > > > available. > > > > > > IMHO, Linux should have functions similar to the DMA API but for PIO > > > drivers (e.g. pio_map_single/pio_unmap_single) that non-coherent > > > architectures can define, otherwise being no-ops. Any thoughts? > > > > You should bring this up on the linux-arm-kernel mailing list and CC: > > the ARM maintainer. They are the ones most directly affected. > > No, this belongs on linux-arch, as it's something that impacts a lot of > people besides ARM. I agree. I'll try to come up with a proposal and post it there. BTW, this was already raised on the ARM Linux lists and people there are aware of these problems. Their suggestion was to take it to LKML. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Catalin Marinas on 2 Feb 2010 06:10 On Tue, 2010-02-02 at 06:39 +0000, Paul Mundt wrote: > On Mon, Feb 01, 2010 at 05:29:14PM +0000, Catalin Marinas wrote: > > diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c > > index 27b8f7c..4d3eeee 100644 > > --- a/drivers/usb/host/isp1760-hcd.c > > +++ b/drivers/usb/host/isp1760-hcd.c > > @@ -18,6 +18,8 @@ > > #include <linux/uaccess.h> > > #include <linux/io.h> > > #include <asm/unaligned.h> > > +#include <asm/cacheflush.h> > > +#include <asm/memory.h> > > asm/memory.h isn't a portable header. If you are including it for > virt_to_page(), linux/io.h should already bring that in via asm/io.h. > If arm doesn't bring in virt_to_page() through its asm/io.h, then fix the > headers there please. In the ARM case, yes, it brings virt_to_page() but I'm not sure that's the case for the other architectures. I think a better header is linux/mm.h which already uses this function in virt_to_head_page(). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Paul Mundt on 2 Feb 2010 06:20
On Tue, Feb 02, 2010 at 11:05:39AM +0000, Catalin Marinas wrote: > On Tue, 2010-02-02 at 06:39 +0000, Paul Mundt wrote: > > On Mon, Feb 01, 2010 at 05:29:14PM +0000, Catalin Marinas wrote: > > > diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c > > > index 27b8f7c..4d3eeee 100644 > > > --- a/drivers/usb/host/isp1760-hcd.c > > > +++ b/drivers/usb/host/isp1760-hcd.c > > > @@ -18,6 +18,8 @@ > > > #include <linux/uaccess.h> > > > #include <linux/io.h> > > > #include <asm/unaligned.h> > > > +#include <asm/cacheflush.h> > > > +#include <asm/memory.h> > > > > asm/memory.h isn't a portable header. If you are including it for > > virt_to_page(), linux/io.h should already bring that in via asm/io.h. > > If arm doesn't bring in virt_to_page() through its asm/io.h, then fix the > > headers there please. > > In the ARM case, yes, it brings virt_to_page() but I'm not sure that's > the case for the other architectures. I think a better header is > linux/mm.h which already uses this function in virt_to_head_page(). > For some reason I was thinking virt_to_phys() instead of virt_to_page() when I wrote that, so just ignore me. linux/mm.h is obviously fine. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |