Marek,

> On 3 Oct 2017, at 14:04, Marek Vasut <ma...@denx.de> wrote:
> 
> On 09/19/2017 01:15 PM, Faiz Abbas wrote:
>> A flush of the cache is required before any DMA access can take place.
> 
> You mean invalidation for inbound DMA, flush for outbound DMA, right ?
> 
>> The minimum size that can be flushed from the cache is one cache line
>> size. Therefore, any buffer allocated for DMA should be in multiples
>> of cache line size.
>> 
>> Thus, allocate memory for ep0_trb in multiples of cache line size.
>> 
>> Also, when local variable trb is assigned to dwc->ep0_trb[1] and used
>> to flush cache, it leads to cache misaligned messages as only the base
>> address dwc->ep0_trb is cache aligned.
>> 
>> Therefore, flush cache using ep0_trb_addr which is always cache aligned.
>> 
>> Signed-off-by: Faiz Abbas <faiz_ab...@ti.com>
>> ---
>> drivers/usb/dwc3/ep0.c    | 7 ++++---
>> drivers/usb/dwc3/gadget.c | 3 ++-
>> 2 files changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index e61d980..f3a17a1 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -82,7 +82,7 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 
>> epnum, dma_addr_t buf_dma,
>>                              | DWC3_TRB_CTRL_LST);
>> 
>>      dwc3_flush_cache((uintptr_t)buf_dma, len);
>> -    dwc3_flush_cache((uintptr_t)trb, sizeof(*trb));
>> +    dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2);
>> 
>>      if (chain)
>>              return 0;
>> @@ -790,7 +790,7 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>>      if (!r)
>>              return;
>> 
>> -    dwc3_flush_cache((uintptr_t)trb, sizeof(*trb));
>> +    dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2);
> 
> Why *2 ?
> 
>>      status = DWC3_TRB_SIZE_TRBSTS(trb->size);
>>      if (status == DWC3_TRBSTS_SETUP_PENDING) {
>> @@ -821,7 +821,8 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>>                      ur->actual += transferred;
>> 
>>                      trb++;
>> -                    dwc3_flush_cache((uintptr_t)trb, sizeof(*trb));
>> +                    dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr,
>> +                                     sizeof(*trb) * 2);
>>                      length = trb->size & DWC3_TRB_SIZE_MASK;
>> 
>>                      ep0->free_slot = 0;
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index e065c5a..895a5bc 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2567,7 +2567,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>              goto err0;
>>      }
>> 
>> -    dwc->ep0_trb = dma_alloc_coherent(sizeof(*dwc->ep0_trb) * 2,
>> +    dwc->ep0_trb = dma_alloc_coherent(ROUND(sizeof(*dwc->ep0_trb) * 2,
>> +                                            CACHELINE_SIZE),
> 
> AFAIU dma_alloc_coherent() should mark the memory area uncachable .

We had this discussion a while back, when I submitted the fixes to make
this driver work on the RK3399: dma_alloc_coherent only performs a
memalign on ARM and ARM64:

See the following snippet in arch/arm/include/asm/dma-mapping.h:

static inline void *dma_alloc_coherent(size_t len, unsigned long *handle)
{
        *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len);
        return (void *)*handle;
}


> 
>>                                        (unsigned long *)&dwc->ep0_trb_addr);
>>      if (!dwc->ep0_trb) {
>>              dev_err(dwc->dev, "failed to allocate ep0 trb\n");
>> 
> 
> 
> -- 
> Best regards,
> Marek Vasut

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to