[U-Boot] [u-boot 30/40] dwc3: flush the buffers before using it

[Lukasz Majewski]

Hi Kishon,

> In the linux kernel, non cacheable buffers are used. However in uboot
> since there are no APIs to allocate non cacheable memory, all
> the buffers should be flushed before using it.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
> ---
>  drivers/usb/dwc3/core.c   |    7 +++++++
>  drivers/usb/dwc3/ep0.c    |    6 ++++++
>  drivers/usb/dwc3/gadget.c |    5 +++++
>  drivers/usb/dwc3/io.h     |    6 ++++++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5a8d5ea..4c3637f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -769,11 +769,18 @@ void dwc3_uboot_exit(int index)
>  void dwc3_uboot_handle_interrupt(int index)
>  {
>  	struct dwc3 *dwc = NULL;
> +	int i;
> +	struct dwc3_event_buffer *evt;
>  
>  	list_for_each_entry(dwc, &dwc3_list, list) {
>  		if (dwc->index != index)
>  			continue;
>  
> +		for (i = 0; i < dwc->num_event_buffers; i++) {
> +			evt = dwc->ev_buffs[i];
> +			dwc3_flush_cache((int)evt->buf, evt->length);
> +		}
> +
>  		dwc3_gadget_uboot_handle_interrupt(dwc);
>  		break;
>  	}
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 803ba51..977d6d4 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -74,6 +74,9 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc,
> u8 epnum, dma_addr_t buf_dma, | DWC3_TRB_CTRL_IOC
>  			| DWC3_TRB_CTRL_ISP_IMI);
>  
> +	dwc3_flush_cache((int)buf_dma, len);
> +	dwc3_flush_cache((int)trb, sizeof(*trb));
> +
>  	memset(&params, 0, sizeof(params));
>  	params.param0 = upper_32_bits(dwc->ep0_trb_addr);
>  	params.param1 = lower_32_bits(dwc->ep0_trb_addr);
> @@ -774,6 +777,8 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc, if (!r)
>  		return;
>  
> +	dwc3_flush_cache((int)trb, sizeof(*trb));
> +
>  	status = DWC3_TRB_SIZE_TRBSTS(trb->size);
>  	if (status == DWC3_TRBSTS_SETUP_PENDING) {
>  		dev_dbg(dwc->dev, "Setup Pending received");
> @@ -795,6 +800,7 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc, transfer_size += (maxp - (transfer_size % maxp));
>  		transferred = min_t(u32, ur->length,
>  				transfer_size - length);
> +		dwc3_flush_cache((int)dwc->ep0_bounce,
> DWC3_EP0_BOUNCE_SIZE); memcpy(ur->buf, dwc->ep0_bounce, transferred);
>  	} else {
>  		transferred = ur->length - length;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 1f97729..b68b6a4 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -244,6 +244,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep,
> struct dwc3_request *req, 
>  	list_del(&req->list);
>  	req->trb = NULL;
> +	dwc3_flush_cache((int)req->request.dma, req->request.length);
>  
>  	if (req->request.status == -EINPROGRESS)
>  		req->request.status = status;
> @@ -769,6 +770,9 @@ static void dwc3_prepare_one_trb(struct dwc3_ep
> *dep, trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(req->request.stream_id);
>  
>  	trb->ctrl |= DWC3_TRB_CTRL_HWO;
> +
> +	dwc3_flush_cache((int)dma, length);
> +	dwc3_flush_cache((int)trb, sizeof(*trb));
>  }
>  
>  /*
> @@ -1770,6 +1774,7 @@ static int dwc3_cleanup_done_reqs(struct dwc3
> *dwc, struct dwc3_ep *dep, slot %= DWC3_TRB_NUM;
>  		trb = &dep->trb_pool[slot];
>  
> +		dwc3_flush_cache((int)trb, sizeof(*trb));
>  		ret = __dwc3_cleanup_done_trbs(dwc, dep, req, trb,
>  				event, status);
>  		if (ret)
> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> index b6da135..31606de 100644
> --- a/drivers/usb/dwc3/io.h
> +++ b/drivers/usb/dwc3/io.h
> @@ -20,6 +20,7 @@
>  
>  #include <asm/io.h>
>  
> +#define	CACHELINE_SIZE
> CONFIG_SYS_CACHELINE_SIZE static inline u32 dwc3_readl(void __iomem
> *base, u32 offset) {
>  	u32 offs = offset - DWC3_GLOBALS_REGS_START;
> @@ -47,4 +48,9 @@ static inline void dwc3_writel(void __iomem *base,
> u32 offset, u32 value) writel(value, base + offs);
>  }
>  
> +static inline void dwc3_flush_cache(int addr, int length)
> +{
> +	flush_dcache_range(addr & ~(CACHELINE_SIZE - 1),
> +			   ALIGN(addr + length, CACHELINE_SIZE));

Is this safe cache operation?

For example, the size of struct dwc3_trb is 16B [1] and the
CACHELINE_SIZE is equal to 64B or 32B.

Since trb buffer is not padded to CACHELINE_SIZE it is perfectly
correct to flush the CACHELINE_SIZE bytes, which may be allocated to
hold other data. In this way we would have data corruption very
difficult to fix.

Please consider s3c_udc_otg_xfer_dma.c for a reference (dwc2 gadget
driver): (some useful code snippets)

flush_dcache_range((unsigned long) ep->dma_buf,
		   (unsigned long) ep->dma_buf +
		   ROUND(ep->len, CONFIG_SYS_CACHELINE_SIZE));

req = memalign(CONFIG_SYS_CACHELINE_SIZE, sizeof(*req));
if (!req)
	return 0;


And the buffers:

ALLOC_CACHE_ALIGN_BUFFER(struct data_rsp_box, rsp,
			 sizeof(struct data_rsp_box)); 

			[automatic variable allocation]

DEFINE_CACHE_ALIGN_BUFFER(unsigned char, thor_rx_data_buf,
			  sizeof(struct rqt_box));

			[global one]

All relevant structures should be allocated with using above API. Then
we can use presented above flush_dcache_range() function (with
CONFIG_SYS_CACHELINE_SIZE alignment). 

> +}
>  #endif /* __DRIVERS_USB_DWC3_IO_H */



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group