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

Lukasz Majewski
2015-02-16T11:39:16Z

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 */

--
Best regards,

Lukasz Majewski

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