Re: [PATCH v8 1/8] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> writes:

> 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 compiler currently does not know about the alignment of the memory
> layout, and will create strb and ldrb operations.
>
> 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 will
> read wrong data and totally stuck.
>
> This is also possible while working with the current active td queue,
> and preparing the td->ptr.next in software while the hardware is still
> working with the current active td which is supposed to be changed:
>
> writeb(0xde, &td->ptr.next + 0x0); /* strb */
> writeb(0xad, &td->ptr.next + 0x1); /* strb */
>
> <----- hardware reads value of td->ptr.next and get stuck!
>
> writeb(0xbe, &td->ptr.next + 0x2); /* strb */
> writeb(0xef, &td->ptr.next + 0x3); /* strb */
>
> This appeares on armv5 machines where the hardware does not support
> unaligned 32bit operations.
>
> This patch adds the attribute ((aligned(4))) to the structures to tell
> the compiler to use 32bit operations. It also adds an wmb() for the
> prepared TD data before it gets enqueued into the qhead.
>
> Cc: stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> Reviewed-by: Felipe Balbi <balbi@xxxxxx>

I would change the wording a bit so that the subject indicates that it's
a fix (like "fix access width...") and that it says the important bit at
the top that on some architectures that don't do 32-bit unaligned
loads/stores such as armv5 the driver gets stuck. And you probably
should also mention for which kernels this is relevant, so that it's
more obvious that it's stable material.

That said,

Acked-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux