Am Freitag, den 22.03.2013, 14:20 +0100 schrieb Michael Grzeschik:
> 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.
>
> We also remove the packed attribute as other patches could not address
> this memory handling correct and leak new causes for the issue to come
> back.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
[...]
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index d644574..be60464 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -38,7 +38,7 @@ struct ci13xxx_td {
> #define TD_CURR_OFFSET (0x0FFFUL << 0)
> #define TD_FRAME_NUM (0x07FFUL << 0)
> #define TD_RESERVED_MASK (0x0FFFUL << 0)
> -} __attribute__ ((packed));
> +};
>
> /* DMA layout of queue heads */
> struct ci13xxx_qh {
> @@ -55,7 +55,7 @@ struct ci13xxx_qh {
> /* 9 */
> u32 RESERVED;
> struct usb_ctrlrequest setup;
> -} __attribute__ ((packed));
> +};
>
Please don't remove the packed attribute, as this is a structure which
is shared between hardware and software and really needs a fixed layout.
As all the members of this struct are natural aligned, it should be
enough to add attribute aligned(4) to tell the compiler that the start
address of the struct is also 32bit aligned and this way cause it to
emit 32bit load/stores.
Though, you have to make sure your DMA buffers are really aligned on a
32bit boundary or you'll run into the kernels alignment fixup handlers
or a data abort.
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
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
