On Wednesday 11 October 2017 02:28 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 11 October 2017 01:53 PM, Faiz Abbas wrote:
Hi,
On Tuesday 10 October 2017 07:19 PM, Marek Vasut wrote:
On 10/10/2017 12:45 PM, Faiz Abbas wrote:
Hi Marek,
On Tuesday 10 October 2017 01:30 PM, Marek Vasut wrote:
On 10/10/2017 07:48 AM, Kishon Vijay Abraham I wrote:
Hi,
Hi,
[...]
>>>>>>>>>> - dwc3_flush_cache((uintptr_t)trb, sizeof(*trb)); >>>>>>>>>> + dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2); >>>>>>>>> >>>>>>>>> Why *2 ? >>>>>>>> >>>>>>>> Because its allocated as sizeof(*dwc->ep0_trb) * 2 below. This is not >>>>>>>> strictly required as dwc3_flush_cache() rounds up the size to >>>>>>>> CACHELINE_SIZE but from a caller POV, flush everything we allocated. >>>>>>> >>>>>>> Can the other TRB be in use ? Maybe aligning the TRBs to cacheline size >>>>>>> would be better ? >>>>>>> >>>>>> A single trb is 16 bytes in size and two of them are allocated >>>>>> contiguously. >>>>> >>>>> Why are two allocated continuously ? (I am not dwc3 expert)
The TRB's should be allocated contiguously for dwc3 and only the base of the entire TRB table is programmed in the HW. ________________ <------------------ TRB table base address | TRB0 | |________________| | TRB1 | |________________| | TRB2 | |________________| | TRBn | |________________|
>>>> >>>> Neither am I. I did try to pad to the dwc_trb structure such that each >>>> trb is 64 bytes in size but this leads to failures when testing. I >>>> didn't get a chance to debug this though. I suspect its because the code >>>> expects the trbs to be contiguous and/or 16 bytes in size.
It's not the code but it's the HW.
That'd imply we need either some sort of flushing scheme or non-cachable memory allocation. What does Linux do ? The dma_alloc_coherent in linux kernel allocates non-cachable memory.
Currently, the code is using local variable trb to flush the cache. When the first trb is used, dwc3_flush_cache flushes the complete CACHELINE_SIZE (including the 2nd trb). When the 2nd trb is used to flush cache, since it is an unaligned flush, it will issue a warning and will align it to the lower cache line boundary (flushing the 1st trb in the process).
So with or without this patch, both trbs are getting flushed with every call. With the patch, we can just avoid misaligned messages by always flushing using an aligned address.
What worries me is that you can flush something into the memory while the controller is writing into it as well. Is that possible ?
No, control to the hardware is only given after all the trbs have been configured and flushed to memory. This is done by using the chain variable in the code.
dwc3_flush_cache((uintptr_t)buf_dma, len); dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2);this flush_cache can be moved after the chain so that flush is only invoked after all the TRB's has been configured.
Sure, that can be done.
Thanks, Faiz