diff mbox

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

Message ID 1423212497-11970-31-git-send-email-kishon@ti.com
State Accepted
Delegated to: Łukasz Majewski
Headers show

Commit Message

Kishon Vijay Abraham I Feb. 6, 2015, 8:48 a.m. UTC
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   |    7 +++++++
 drivers/usb/dwc3/ep0.c    |    6 ++++++
 drivers/usb/dwc3/gadget.c |    5 +++++
 drivers/usb/dwc3/io.h     |    6 ++++++
 4 files changed, 24 insertions(+)

Comments

Łukasz Majewski Feb. 16, 2015, 11:39 a.m. UTC | #1
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   |    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 */
diff mbox

Patch

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));
+}
 #endif /* __DRIVERS_USB_DWC3_IO_H */