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); if (chain) return 0; memset(¶ms, 0, sizeof(params)); params.param0 = upper_32_bits(dwc->ep0_trb_addr); params.param1 = lower_32_bits(dwc->ep0_trb_addr); ret = dwc3_send_gadget_ep_cmd(dwc, dep->number, DWC3_DEPCMD_STARTTRANSFER, ¶ms); Thanks, Faiz _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot