On Tuesday 23 June 2009 10:35:42 Piotr Zięcik wrote: > --- a/sys/dev/usb/usb_busdma.c > +++ b/sys/dev/usb/usb_busdma.c > @@ -658,8 +658,7 @@ usb_pc_cpu_invalidate(struct usb_page_cache *pc) > /* nothing has been loaded into this page cache! */ > return; > } > - bus_dmamap_sync(pc->tag, pc->map, > - BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); > + bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREREAD); > } > > /*------------------------------------------------------------------------ >* @@ -672,8 +671,7 @@ usb_pc_cpu_flush(struct usb_page_cache *pc) > /* nothing has been loaded into this page cache! */ > return; > } > - bus_dmamap_sync(pc->tag, pc->map, > - BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); > + bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE); > }
Hi, Let's restart the discussion with the initial patch. You want to change the flags passed to bus_dmamap_sync() so that the flush/invalidate mapping gets right. I understand why your patch makes it work. That's not the problem. In "src/sys/arm/arm/busdma_machdep.c" there is a function called "_bus_dmamap_sync_bp()". If you look at that function you see that it only triggers on the "BUS_DMASYNC_PREWRITE" and "BUS_DMASYNC_POSTREAD" flags. After your patching only the PREXXXX flags are used, so if bouce pages are used on ARM and x86 and amd64 +++, then only BUS_DMASYNC_PREWRITE will do anything. This indicates that your patch is not fully correct. Grepping through the source code for ARM, I found a line like this: /*XXX*/ arm9_dcache_wbinv_range, /* dcache_inv_range */ which is not correct. If we are only invalidating, then it is not correct to do a flush first. Summed up: static void bus_dmamap_sync_buf(void *buf, int len, bus_dmasync_op_t op) { char _tmp_cl[arm_dcache_align], _tmp_clend[arm_dcache_align]; if ((op & BUS_DMASYNC_PREWRITE) && !(op & BUS_DMASYNC_PREREAD)) { cpu_dcache_wb_range((vm_offset_t)buf, len); cpu_l2cache_wb_range((vm_offset_t)buf, len); } if (op & BUS_DMASYNC_PREREAD) { if (!(op & BUS_DMASYNC_PREWRITE) && ((((vm_offset_t)(buf) | len) & arm_dcache_align_mask) == 0)) { cpu_dcache_inv_range((vm_offset_t)buf, len); cpu_l2cache_inv_range((vm_offset_t)buf, len); } else { Because the USB code specifies both PREREAD and PREWRITE we end up in the following case, which is not implemented correctly. The function name indicates write back first, then invalidate, but when looking at the implementation: <example> arm8_cache_purgeID, /* idcache_wbinv_all */ (void *)arm8_cache_purgeID, /* idcache_wbinv_range */ </example> You see that it only performs purge and no prior flush. This is what needs fixing! If semihalf could go through the "arm/arm/cpufunc.c" file and fix those flush and invalidate functions then many people would become happy! Again, it is not a problem in USB firstly, it is a problem in "arm/xxx". cpu_dcache_wbinv_range((vm_offset_t)buf, len); cpu_l2cache_wbinv_range((vm_offset_t)buf, len); } } if (op & BUS_DMASYNC_POSTREAD) { if ((vm_offset_t)buf & arm_dcache_align_mask) { memcpy(_tmp_cl, (void *)((vm_offset_t)buf & ~ arm_dcache_align_mask), (vm_offset_t)buf & arm_dcache_align_mask); } if (((vm_offset_t)buf + len) & arm_dcache_align_mask) { memcpy(_tmp_clend, (void *)((vm_offset_t)buf + len), arm_dcache_align - (((vm_offset_t)(buf) + len) & arm_dcache_align_mask)); } cpu_dcache_inv_range((vm_offset_t)buf, len); cpu_l2cache_inv_range((vm_offset_t)buf, len); if ((vm_offset_t)buf & arm_dcache_align_mask) memcpy((void *)((vm_offset_t)buf & ~arm_dcache_align_mask), _tmp_cl, (vm_offset_t)buf & arm_dcache_align_mask); if (((vm_offset_t)buf + len) & arm_dcache_align_mask) memcpy((void *)((vm_offset_t)buf + len), _tmp_clend, arm_dcache_align - (((vm_offset_t)(buf) + len) & arm_dcache_align_mask)); } } --HPS _______________________________________________ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"