Hi Marek,
On Thu, Jul 5, 2012 at 12:24 AM, Marek Vasut marex@denx.de wrote:
-static struct QH qh_list __attribute__((aligned(32))); +static char __qh_list[ALIGN(sizeof(struct QH), USB_DMA_MINALIGN)]
__attribute__((aligned(USB_DMA_MINALIGN)));+static struct QH *qh_list = (struct QH *)__qh_list;
Maybe we should create DEFINE_ALIGNED_VARIABLE as a common.h macro?
Yep. I even thought about this but decided not to do... can't recall why. Now I think it's really a good idea.
static struct descriptor { struct usb_hub_descriptor hub; @@ -172,13 +174,13 @@ 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);
size_t rsz = roundup(sz, USB_DMA_MINALIGN); int idx; if (sz != rsz) debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz);
if (addr & 31)
if (addr != ALIGN(addr, USB_DMA_MINALIGN)) debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf);Good :)
Well, thinking more about this, it's actually a wrong place to check this... It can be setup packet buffer, which can be unaligned (this is ok cause it's only flushed and never invalidated). And size can always be unaligned...
/* Flush dcache */
flush_dcache_range((uint32_t)&qh_list,(uint32_t)&qh_list + sizeof(struct QH));flush_dcache_range((uint32_t)&qh, (uint32_t)&qh + sizeof(structQH));
flush_dcache_range((uint32_t)qtd, (uint32_t)qtd + sizeof(qtd));
flush_dcache_range((uint32_t)qh_list,(uint32_t)qh_list + ALIGN(sizeof(struct QH),USB_DMA_MINALIGN));
flush_dcache_range((uint32_t)qh, (uint32_t)qh +ALIGN(sizeof(struct QH), USB_DMA_MINALIGN));flush_dcache_range((uint32_t)qtd, (uint32_t)qtd +ALIGN(3*sizeof(*qtd), USB_DMA_MINALIGN));Maybe we should make a macro for those things here to prevent such spaghetti of code ?
Hm.. Maybe. Ideas? ;) Actually I also thought about moving all this stuff to a single proper aligned buffer and do flush/invalidate for a whole buffer at once. It can save us some space... but it's BSS anyway... Don't know if it's worth it...
Regards, Ilya.