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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
