| Message ID | 1342363433-9284-5-git-send-email-ilya.yanok@cogentembedded.com |
|---|---|
| State | Accepted |
| Commit | 189a6956ebbd7820afe5fa45a64ca495e6cefd9c |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Ilya Yanok, > Buffer coming from upper layers should be cacheline aligned/padded > to perform safe cache operations. For now we don't do bounce > buffering so getting unaligned buffer is an upper layer error. > We can't check if the buffer is properly padded with current > interface so just assume it is (consider changing with in the > future). The following changes are done: > > 1. Remove useless length alignment check. We get actual transfer > length not the size of the underlying buffer so it's perfectly > valid for it to be unaligned. > 2. Move flush_dcache_range() out of while loop or it will > flush too much. > 3. Don't try to fix buffer address before calling invalidate: > if it's unaligned it's an error anyway so let cache subsystem > cry about that. > 4. Fix end buffer address to be cacheline aligned assuming upper > layer reserved enough space. This is potentially dangerous > operation so upper layers should be careful about that. > > Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com> > --- > drivers/usb/host/ehci-hcd.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c > index 59039f4..a6cd5e3 100644 > --- a/drivers/usb/host/ehci-hcd.c > +++ b/drivers/usb/host/ehci-hcd.c > @@ -175,18 +175,15 @@ static int ehci_td_buffer(struct qTD *td, void *buf, > size_t sz) { > uint32_t delta, next; > uint32_t addr = (uint32_t)buf; > - size_t rsz = roundup(sz, 32); > int idx; > > - if (sz != rsz) > - debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz); Shall we not also test if addr + sz is aligned? > - if (addr & 31) > + if (addr != ALIGN(addr, ARCH_DMA_MINALIGN)) > debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf); [...] Best regards, Marek Vasut
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 59039f4..a6cd5e3 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -175,18 +175,15 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz) { uint32_t delta, next; uint32_t addr = (uint32_t)buf; - size_t rsz = roundup(sz, 32); int idx; - if (sz != rsz) - debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz); - - if (addr & 31) + if (addr != ALIGN(addr, ARCH_DMA_MINALIGN)) debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf); + flush_dcache_range(addr, ALIGN(addr + sz, ARCH_DMA_MINALIGN)); + idx = 0; while (idx < 5) { - flush_dcache_range(addr, addr + rsz); td->qt_buffer[idx] = cpu_to_hc32(addr); td->qt_buffer_hi[idx] = 0; next = (addr + 4096) & ~4095; @@ -386,9 +383,17 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, WATCHDOG_RESET(); } while (get_timer(ts) < timeout); - /* Invalidate the memory area occupied by buffer */ - invalidate_dcache_range(((uint32_t)buffer & ~31), - ((uint32_t)buffer & ~31) + roundup(length, 32)); + /* + * Invalidate the memory area occupied by buffer + * Don't try to fix the buffer alignment, if it isn't properly + * aligned it's upper layer's fault so let invalidate_dcache_range() + * vow about it. But we have to fix the length as it's actual + * transfer length and can be unaligned. This is potentially + * dangerous operation, it's responsibility of the calling + * code to make sure enough space is reserved. + */ + invalidate_dcache_range((uint32_t)buffer, + ALIGN((uint32_t)buffer + length, ARCH_DMA_MINALIGN)); /* Check that the TD processing happened */ if (token & 0x80) {
Buffer coming from upper layers should be cacheline aligned/padded to perform safe cache operations. For now we don't do bounce buffering so getting unaligned buffer is an upper layer error. We can't check if the buffer is properly padded with current interface so just assume it is (consider changing with in the future). The following changes are done: 1. Remove useless length alignment check. We get actual transfer length not the size of the underlying buffer so it's perfectly valid for it to be unaligned. 2. Move flush_dcache_range() out of while loop or it will flush too much. 3. Don't try to fix buffer address before calling invalidate: if it's unaligned it's an error anyway so let cache subsystem cry about that. 4. Fix end buffer address to be cacheline aligned assuming upper layer reserved enough space. This is potentially dangerous operation so upper layers should be careful about that. Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com> --- drivers/usb/host/ehci-hcd.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)