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

[Lukasz Majewski <l.majewski@samsung.com>]

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@ti.com>
> ---
>  drivers/usb/dwc3/core.c   |   11 +++++++++--
>  drivers/usb/dwc3/ep0.c    |    6 ++++++
>  drivers/usb/dwc3/gadget.c |    8 +++++++-
>  drivers/usb/dwc3/io.h     |    5 +++++
>  4 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5a8d5ea..78fce1b 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -157,8 +157,8 @@ static int dwc3_alloc_event_buffers(struct dwc3
> *dwc, unsigned length) num = DWC3_NUM_INT(dwc->hwparams.hwparams1);
>  	dwc->num_event_buffers = num;
>  
> -	dwc->ev_buffs = devm_kzalloc(dwc->dev,
> sizeof(*dwc->ev_buffs) * num,
> -			GFP_KERNEL);
> +	dwc->ev_buffs = memalign(CONFIG_SYS_CACHELINE_SIZE,
> +				 sizeof(*dwc->ev_buffs) * num);
>  	if (!dwc->ev_buffs)
>  		return -ENOMEM;
>  
> @@ -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..8560b88 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));

My only concern is with sizeof(*trb), which is 16B.

On Exynos cache line size is 32B.
With this particular case we would flush one extra TRB struct to main
memory.

Since we have per EP trb_pools aligned with memalign calls (and
capable of storing up to 32 TRB requests) and perform one TRB
transmission at a time, we can leave this code as it is.

Hence, 

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

>  }
>  
>  /*
> @@ -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)
> @@ -2583,7 +2588,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>  		goto err1;
>  	}
>  
> -	dwc->setup_buf = kzalloc(DWC3_EP0_BOUNCE_SIZE, GFP_KERNEL);
> +	dwc->setup_buf = memalign(CONFIG_SYS_CACHELINE_SIZE,
> +				  DWC3_EP0_BOUNCE_SIZE);
>  	if (!dwc->setup_buf) {
>  		ret = -ENOMEM;
>  		goto err2;
> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> index b6da135..5042a24 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,8 @@ 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, addr + ROUND(length,
> CACHELINE_SIZE)); +}
>  #endif /* __DRIVERS_USB_DWC3_IO_H */



-- 
Best regards,

Lukasz Majewski

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