On Tue, Mar 26, 2013 at 06:33:27AM +0000, Paul Zimmerman wrote: > > From: Peter Chen <peter.chen@xxxxxxxxxxxxx> > > Sent: Monday, March 25, 2013 7:58 PM > > > > On Fri, Mar 22, 2013 at 02:20:23PM +0100, Michael Grzeschik wrote: > > > The udc uses an shared dma memory space between hard and software. This > > > memory layout is described in ci13xxx_qh and ci13xxx_td which are marked > > > with the attribute ((packed)). > > > > > > The packed attribute leads the compiler to generate one byte operations > > > for addressing the mapped memory as it believes this memory has no > > > alignement issues as common streaming data. This appeares on armv5 > > > machines where the hardware does not support unaligned 32bit operations. > > > Compilers for newer ARMs will probably still generate 32bit operations, > > > as the hardware supports it. > > > > > > The Datasheet of the synopsys core describes, that some operations on > > > the mapped memory need to be atomic double word operations. I.e. the > > > next pointer addressing in the qhead, as otherwise the hardware could > > > read wrong data and totally stuck. > > > > > > This patch fixes that issue by addressing every mapped area operation > > > with explicit readl and writel operations where needed. It also adds an > > > wmb() for the prepared TD data before it gets enqueued into the qhead. > > > > The writel includes wmb(), the wmb() isn't needed if the last instruction > > is writel. > > > > Besides, please make sure all dQH/dTD mapped area access by readl/writel in > > this patch, we don't want to add missing in future. > > Is 'sparse' happy with this? AFAIR it will complain if you use readl/writel > on memory that is not marked with __iomem. I did not try sparse jet. > Is removing the __attribute__ ((packed)) annotation not enough to fix the > problem? We just changed every user of the structure, to ensure that no compiler would ever produce a not 32bit operations in that context. Removing the attribute packed is already fixing the issue, as the gcc is producing ldr,str where needed. Beside the added wmb() is still a good idea before asigning the next pointer in the not empty list case. So removing the packed attribute, adding a proper comment to the structures and adding the wmb() would be enough for that patch. -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
