[U-Boot] [PATCH 00/17] usb: mv_udc: Cleanup and cache ops

This series cleans up the mv_udc / chipidea UDC driver and adds support for handling caches.
Marek Vasut (17): usb: mv_udc: Unbreak the mv_udc driver usb: mv_udc: Clean up mv_udc.h usb: mv_udc: Move endpoint array into driver data usb: mv_udc: Clean up the EP initialization usb: ehci: Split out struct ehci_ctrl definition usb: mv_udc: Make use of struct ehci_ctrl usb: mv_udc: Clean up the initial variable check usb: mv_udc: Remove QH_MAXNUM macro usb: mv_udc: Init mv_drv.gadget.ops statically usb: mv_udc: Move QH and qTD into mv_drv usb: mv_udc: Properly align the endpoint QH and qTD list usb: mv_udc: Add cacheline length check usb: mv_udc: Implement better QH accessor usb: mv_udc: Improve allocation of qTD items usb: mv_udc: Implement better qTD item accessor usb: mv_udc: Add proper cache management usb: mv_udc: Add bounce buffer
drivers/serial/usbtty.h | 2 + drivers/usb/gadget/gadget_chips.h | 2 +- drivers/usb/gadget/mv_udc.c | 401 +++++++++++++++++++++++++++++-------- drivers/usb/host/ehci-hcd.c | 11 +- drivers/usb/host/ehci.h | 13 ++ include/usb/mv_udc.h | 88 ++++---- 6 files changed, 375 insertions(+), 142 deletions(-)
Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de

The mv_udc driver is broken for a while and doesn't even compile. This patch fixes the issues and gets the driver into working state again. This driver was tested on Freescale i.MX233/i.MX28 .
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/serial/usbtty.h | 2 ++ drivers/usb/gadget/gadget_chips.h | 2 +- drivers/usb/gadget/mv_udc.c | 7 ++++++- 3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/usbtty.h b/drivers/serial/usbtty.h index eb670da..4a0bd45 100644 --- a/drivers/serial/usbtty.h +++ b/drivers/serial/usbtty.h @@ -35,6 +35,8 @@ #include <usb/pxa27x_udc.h> #elif defined(CONFIG_DW_UDC) #include <usb/designware_udc.h> +#elif defined(CONFIG_MV_UDC) +#include <usb/mv_udc.h> #endif
#include <version.h> diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h index e570142..6953174 100644 --- a/drivers/usb/gadget/gadget_chips.h +++ b/drivers/usb/gadget/gadget_chips.h @@ -144,7 +144,7 @@ #define gadget_is_m66592(g) 0 #endif
-#ifdef CONFIG_USB_GADGET_MV +#ifdef CONFIG_MV_UDC #define gadget_is_mv(g) (!strcmp("mv_udc", (g)->name)) #else #define gadget_is_mv(g) 0 diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index cdbdcfa..db30bdd 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -33,6 +33,10 @@ #include <linux/types.h> #include <usb/mv_udc.h>
+#if CONFIG_USB_MAX_CONTROLLER_COUNT > 1 +#error This driver only supports one single controller. +#endif + #ifndef DEBUG #define DBG(x...) do {} while (0) #else @@ -469,6 +473,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) { struct mv_udc *udc = controller.udc; int retval; + void *ctrl;
if (!driver || driver->speed < USB_SPEED_FULL @@ -479,7 +484,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) }
if (!mvudc_probe()) { - usb_lowlevel_init(); + usb_lowlevel_init(0, &ctrl); /* select ULPI phy */ writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); }

Do a coding-style cleanup of this file and throw away useless defined values. These values were likely a result of a copy-paste job.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- include/usb/mv_udc.h | 60 +++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 37 deletions(-)
diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index 221e626..42dff29 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -27,28 +27,14 @@ #include <linux/usb/ch9.h> #include <linux/usb/gadget.h>
-/* Endpoint 0 states */ -#define EP0_IDLE 0 -#define EP0_IN_DATA 1 -#define EP0_OUT_DATA 2 -#define EP0_XFER_COMPLETE 3 - +#define NUM_ENDPOINTS 6
/* Endpoint parameters */ #define MAX_ENDPOINTS 4 -#define EP_MAX_PACKET_SIZE 0x200
+#define EP_MAX_PACKET_SIZE 0x200 #define EP0_MAX_PACKET_SIZE 64 -#define UDC_OUT_ENDPOINT 0x02 -#define UDC_OUT_PACKET_SIZE EP_MAX_PACKET_SIZE -#define UDC_IN_ENDPOINT 0x01 -#define UDC_IN_PACKET_SIZE EP_MAX_PACKET_SIZE -#define UDC_INT_ENDPOINT 0x05 -#define UDC_INT_PACKET_SIZE EP_MAX_PACKET_SIZE -#define UDC_BULK_PACKET_SIZE EP_MAX_PACKET_SIZE - -#define NUM_ENDPOINTS 6 -#define REQ_COUNT 12 + struct mv_ep { struct usb_ep ep; struct usb_request req; @@ -59,7 +45,7 @@ struct mv_ep { struct mv_udc { u32 pad0[80]; #define MICRO_8FRAME 0x8 -#define USBCMD_ITC(x) (((x > 0xff) ? 0xff : x) << 16) +#define USBCMD_ITC(x) ((((x) > 0xff) ? 0xff : x) << 16) #define USBCMD_FS2 (1 << 15) #define USBCMD_RST (1 << 1) #define USBCMD_RUN (1) @@ -75,25 +61,25 @@ struct mv_udc { u32 epinitaddr; /* 0x158 */ u32 pad2[10]; #define PTS_ENABLE 2 -#define PTS(x) ((x & 0x3) << 30) +#define PTS(x) (((x) & 0x3) << 30) #define PFSC (1 << 24) u32 portsc; /* 0x184 */ u32 pad3[8]; #define USBMODE_DEVICE 2 u32 usbmode; /* 0x1a8 */ u32 epstat; /* 0x1ac */ -#define EPT_TX(x) (1 << ((x & 0xffff) + 16)) -#define EPT_RX(x) (1 << (x & 0xffff)) +#define EPT_TX(x) (1 << (((x) & 0xffff) + 16)) +#define EPT_RX(x) (1 << ((x) & 0xffff)) u32 epprime; /* 0x1b0 */ u32 epflush; /* 0x1b4 */ u32 pad4; u32 epcomp; /* 0x1bc */ -#define CTRL_TXE (1 << 23) -#define CTRL_TXR (1 << 22) -#define CTRL_RXE (1 << 7) -#define CTRL_RXR (1 << 6) -#define CTRL_TXT_BULK (2 << 18) -#define CTRL_RXT_BULK (2 << 2) +#define CTRL_TXE (1 << 23) +#define CTRL_TXR (1 << 22) +#define CTRL_RXE (1 << 7) +#define CTRL_RXR (1 << 6) +#define CTRL_TXT_BULK (2 << 18) +#define CTRL_RXT_BULK (2 << 2) u32 epctrl[16]; /* 0x1c0 */ };
@@ -105,7 +91,7 @@ struct mv_drv {
struct ept_queue_head { unsigned config; - unsigned current; /* read-only */ + unsigned current; /* read-only */
unsigned next; unsigned info; @@ -124,9 +110,9 @@ struct ept_queue_head { unsigned reserved_4; };
-#define CONFIG_MAX_PKT(n) ((n) << 16) -#define CONFIG_ZLT (1 << 29) /* stop on zero-len xfer */ -#define CONFIG_IOS (1 << 15) /* IRQ on setup */ +#define CONFIG_MAX_PKT(n) ((n) << 16) +#define CONFIG_ZLT (1 << 29) /* stop on zero-len xfer */ +#define CONFIG_IOS (1 << 15) /* IRQ on setup */
struct ept_queue_item { unsigned next; @@ -140,12 +126,12 @@ struct ept_queue_item { };
#define TERMINATE 1 -#define INFO_BYTES(n) ((n) << 16) -#define INFO_IOC (1 << 15) -#define INFO_ACTIVE (1 << 7) -#define INFO_HALTED (1 << 6) -#define INFO_BUFFER_ERROR (1 << 5) -#define INFO_TX_ERROR (1 << 3) +#define INFO_BYTES(n) ((n) << 16) +#define INFO_IOC (1 << 15) +#define INFO_ACTIVE (1 << 7) +#define INFO_HALTED (1 << 6) +#define INFO_BUFFER_ERROR (1 << 5) +#define INFO_TX_ERROR (1 << 3)
extern int usb_lowlevel_init(int index, void **controller); #endif /* __MV_UDC_H__ */

The endpoints are operated on a per-controller basis, move the endpoint array into controller's private data. Also shuffle the struct mv_ep structure definition just above the definition of the struct mv_drv so they're well grouped together.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 48 +++++++++++++++++++++---------------------- include/usb/mv_udc.h | 17 +++++++-------- 2 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index db30bdd..4c0755d 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -99,10 +99,8 @@ static struct usb_ep_ops mv_ep_ops = { .free_request = mv_ep_free_request, };
-static struct mv_ep ep[2 * NUM_ENDPOINTS]; static struct mv_drv controller = { .gadget = { - .ep0 = &ep[0].ep, .name = "mv_udc", }, }; @@ -223,7 +221,7 @@ static void handle_ep_complete(struct mv_ep *ep)
static void handle_setup(void) { - struct usb_request *req = &ep[0].req; + struct usb_request *req = &controller.ep[0].req; struct mv_udc *udc = controller.udc; struct ept_queue_head *head; struct usb_ctrlrequest r; @@ -246,11 +244,11 @@ static void handle_setup(void) if ((r.wValue == 0) && (r.wLength == 0)) { req->length = 0; for (i = 0; i < NUM_ENDPOINTS; i++) { - if (!ep[i].desc) + if (!controller.ep[i].desc) continue; - num = ep[i].desc->bEndpointAddress - & USB_ENDPOINT_NUMBER_MASK; - in = (ep[i].desc->bEndpointAddress + num = controller.ep[i].desc->bEndpointAddress + & USB_ENDPOINT_NUMBER_MASK; + in = (controller.ep[i].desc->bEndpointAddress & USB_DIR_IN) != 0; if ((num == _num) && (in == _in)) { ep_enable(num, in); @@ -306,10 +304,11 @@ static void stop_activity(void) for (i = 0; i < NUM_ENDPOINTS; i++) { if (i != 0) writel(0, &udc->epctrl[i]); - if (ep[i].desc) { - num = ep[i].desc->bEndpointAddress + if (controller.ep[i].desc) { + num = controller.ep[i].desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; - in = (ep[i].desc->bEndpointAddress & USB_DIR_IN) != 0; + in = (controller.ep[i].desc->bEndpointAddress + & USB_DIR_IN) != 0; head = epts + (num * 2) + (in); head->info = INFO_ACTIVE; } @@ -340,8 +339,8 @@ void udc_irq(void) if (bit == 2) { controller.gadget.speed = USB_SPEED_HIGH; for (i = 1; i < NUM_ENDPOINTS && n; i++) - if (ep[i].desc) - ep[i].ep.maxpacket = 512; + if (controller.ep[i].desc) + controller.ep[i].ep.maxpacket = 512; } else { controller.gadget.speed = USB_SPEED_FULL; } @@ -360,14 +359,14 @@ void udc_irq(void) writel(n, &udc->epcomp);
for (i = 0; i < NUM_ENDPOINTS && n; i++) { - if (ep[i].desc) { - num = ep[i].desc->bEndpointAddress + if (controller.ep[i].desc) { + num = controller.ep[i].desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; - in = (ep[i].desc->bEndpointAddress + in = (controller.ep[i].desc->bEndpointAddress & USB_DIR_IN) != 0; bit = (in) ? EPT_TX(num) : EPT_RX(num); if (n & bit) - handle_ep_complete(&ep[i]); + handle_ep_complete(&controller.ep[i]); } } } @@ -452,19 +451,20 @@ static int mvudc_probe(void) }
INIT_LIST_HEAD(&controller.gadget.ep_list); - ep[0].ep.maxpacket = 64; - ep[0].ep.name = "ep0"; - ep[0].desc = &ep0_in_desc; + controller.gadget.ep0 = &controller.ep[0].ep; + controller.ep[0].ep.maxpacket = 64; + controller.ep[0].ep.name = "ep0"; + controller.ep[0].desc = &ep0_in_desc; INIT_LIST_HEAD(&controller.gadget.ep0->ep_list); for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { if (i != 0) { - ep[i].ep.maxpacket = 512; - ep[i].ep.name = "ep-"; - list_add_tail(&ep[i].ep.ep_list, + controller.ep[i].ep.maxpacket = 512; + controller.ep[i].ep.name = "ep-"; + list_add_tail(&controller.ep[i].ep.ep_list, &controller.gadget.ep_list); - ep[i].desc = NULL; + controller.ep[i].desc = NULL; } - ep[i].ep.ops = &mv_ep_ops; + controller.ep[i].ep.ops = &mv_ep_ops; } return 0; } diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index 42dff29..da93ad3 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -35,13 +35,6 @@ #define EP_MAX_PACKET_SIZE 0x200 #define EP0_MAX_PACKET_SIZE 64
-struct mv_ep { - struct usb_ep ep; - struct usb_request req; - struct list_head queue; - const struct usb_endpoint_descriptor *desc; -}; - struct mv_udc { u32 pad0[80]; #define MICRO_8FRAME 0x8 @@ -83,10 +76,18 @@ struct mv_udc { u32 epctrl[16]; /* 0x1c0 */ };
+struct mv_ep { + struct usb_ep ep; + struct usb_request req; + struct list_head queue; + const struct usb_endpoint_descriptor *desc; +}; + struct mv_drv { struct usb_gadget gadget; - struct usb_gadget_driver *driver; + struct usb_gadget_driver *driver; struct mv_udc *udc; + struct mv_ep ep[2 * NUM_ENDPOINTS]; };
struct ept_queue_head {

Move the constant values that are programmed into mv_ep.ep into separate static const structure so they can be memcpy()'d when the initialization happens.
Moveover, we only every init NUM_ENDPOINTS, not 2 * NUM_ENDPOINTS, so fix this bug as well.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 38 ++++++++++++++++++++++++++------------ include/usb/mv_udc.h | 2 +- 2 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 4c0755d..1164d74 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -99,6 +99,20 @@ static struct usb_ep_ops mv_ep_ops = { .free_request = mv_ep_free_request, };
+/* Init values for USB endpoints. */ +static const struct usb_ep mv_ep_init[2] = { + [0] = { /* EP 0 */ + .maxpacket = 64, + .name = "ep0", + .ops = &mv_ep_ops, + }, + [1] = { /* EP 1..n */ + .maxpacket = 512, + .name = "ep-", + .ops = &mv_ep_ops, + }, +}; + static struct mv_drv controller = { .gadget = { .name = "mv_udc", @@ -451,21 +465,21 @@ static int mvudc_probe(void) }
INIT_LIST_HEAD(&controller.gadget.ep_list); - controller.gadget.ep0 = &controller.ep[0].ep; - controller.ep[0].ep.maxpacket = 64; - controller.ep[0].ep.name = "ep0"; + + /* Init EP 0 */ + memcpy(&controller.ep[0].ep, &mv_ep_init[0], sizeof(*mv_ep_init)); controller.ep[0].desc = &ep0_in_desc; + controller.gadget.ep0 = &controller.ep[0].ep; INIT_LIST_HEAD(&controller.gadget.ep0->ep_list); - for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { - if (i != 0) { - controller.ep[i].ep.maxpacket = 512; - controller.ep[i].ep.name = "ep-"; - list_add_tail(&controller.ep[i].ep.ep_list, - &controller.gadget.ep_list); - controller.ep[i].desc = NULL; - } - controller.ep[i].ep.ops = &mv_ep_ops; + + /* Init EP 1..n */ + for (i = 1; i < NUM_ENDPOINTS; i++) { + memcpy(&controller.ep[i].ep, &mv_ep_init[1], + sizeof(*mv_ep_init)); + list_add_tail(&controller.ep[i].ep.ep_list, + &controller.gadget.ep_list); } + return 0; }
diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index da93ad3..88100ce 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -87,7 +87,7 @@ struct mv_drv { struct usb_gadget gadget; struct usb_gadget_driver *driver; struct mv_udc *udc; - struct mv_ep ep[2 * NUM_ENDPOINTS]; + struct mv_ep ep[NUM_ENDPOINTS]; };
struct ept_queue_head {

Move the struct ehci_ctrl defition from ehci-hcd.c into ehci.h so it can be re-used by drivers. In particular, the mv_udc driver can benefit from this move.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/host/ehci-hcd.c | 11 +---------- drivers/usb/host/ehci.h | 13 +++++++++++++ 2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index e0f3e4b..e0ff8cf 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -36,16 +36,7 @@ #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 #endif
-static struct ehci_ctrl { - struct ehci_hccr *hccr; /* R/O registers, not need for volatile */ - struct ehci_hcor *hcor; - int rootdev; - uint16_t portreset; - struct QH qh_list __aligned(USB_DMA_MINALIGN); - struct QH periodic_queue __aligned(USB_DMA_MINALIGN); - uint32_t *periodic_list; - int ntds; -} ehcic[CONFIG_USB_MAX_CONTROLLER_COUNT]; +static struct ehci_ctrl ehcic[CONFIG_USB_MAX_CONTROLLER_COUNT];
#define ALIGN_END_ADDR(type, ptr, size) \ ((uint32_t)(ptr) + roundup((size) * sizeof(type), USB_DMA_MINALIGN)) diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index d090f0a..bd52afe 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -22,6 +22,8 @@ #ifndef USB_EHCI_H #define USB_EHCI_H
+#include <usb.h> + #if !defined(CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS) #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 2 #endif @@ -252,6 +254,17 @@ struct QH { }; };
+struct ehci_ctrl { + struct ehci_hccr *hccr; /* R/O registers, not need for volatile */ + struct ehci_hcor *hcor; + int rootdev; + uint16_t portreset; + struct QH qh_list __aligned(USB_DMA_MINALIGN); + struct QH periodic_queue __aligned(USB_DMA_MINALIGN); + uint32_t *periodic_list; + int ntds; +}; + /* Low level init functions */ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor); int ehci_hcd_stop(int index);

The usb_lowlevel_init() call already fills and passes back struct ehci_ctrl , which readily contains correctly determined address of the port register block address computed from values from controller configuration registers. Leverage this and make use of this value as this makes the code mode universal, but also gets us rid of the CONFIG_USB_REG_BASE configuration option.
Moreover, this patch cleans up the usb_gadget_register_driver() call a little by correcting the error handling. Note the usb_lowlevel_init() and mvudc_probe() are now called in reversed order, but this has no impact on the code.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 41 +++++++++++++++++++++++------------------ include/usb/mv_udc.h | 6 +++--- 2 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 1164d74..fd3be77 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -134,7 +134,7 @@ static void mv_ep_free_request(struct usb_ep *ep, struct usb_request *_req) static void ep_enable(int num, int in) { struct ept_queue_head *head; - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; unsigned n; head = epts + 2*num + in;
@@ -170,7 +170,7 @@ static int mv_ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { struct mv_ep *mv_ep = container_of(ep, struct mv_ep, ep); - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; struct ept_queue_item *item; struct ept_queue_head *head; unsigned phys; @@ -236,7 +236,7 @@ static void handle_ep_complete(struct mv_ep *ep) static void handle_setup(void) { struct usb_request *req = &controller.ep[0].req; - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; struct ept_queue_head *head; struct usb_ctrlrequest r; int status = 0; @@ -309,7 +309,7 @@ static void stop_activity(void) { int i, num, in; struct ept_queue_head *head; - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; writel(readl(&udc->epcomp), &udc->epcomp); writel(readl(&udc->epstat), &udc->epstat); writel(0xffffffff, &udc->epflush); @@ -331,7 +331,7 @@ static void stop_activity(void)
void udc_irq(void) { - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; unsigned n = readl(&udc->usbsts); writel(n, &udc->usbsts); int bit, i, num, in; @@ -389,7 +389,7 @@ void udc_irq(void) int usb_gadget_handle_interrupts(void) { u32 value; - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor;
value = readl(&udc->usbsts); if (value) @@ -400,7 +400,7 @@ int usb_gadget_handle_interrupts(void)
static int mv_pullup(struct usb_gadget *gadget, int is_on) { - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; if (is_on) { /* RESET */ writel(USBCMD_ITC(MICRO_8FRAME) | USBCMD_RST, &udc->usbcmd); @@ -428,7 +428,7 @@ static int mv_pullup(struct usb_gadget *gadget, int is_on)
void udc_disconnect(void) { - struct mv_udc *udc = controller.udc; + struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; /* disable pullup */ stop_activity(); writel(USBCMD_FS2, &udc->usbcmd); @@ -443,7 +443,6 @@ static int mvudc_probe(void) int i;
controller.gadget.ops = &mv_udc_ops; - controller.udc = (struct mv_udc *)CONFIG_USB_REG_BASE; epts = memalign(PAGE_SIZE, QH_MAXNUM * sizeof(struct ept_queue_head)); memset(epts, 0, QH_MAXNUM * sizeof(struct ept_queue_head)); for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { @@ -485,9 +484,8 @@ static int mvudc_probe(void)
int usb_gadget_register_driver(struct usb_gadget_driver *driver) { - struct mv_udc *udc = controller.udc; - int retval; - void *ctrl; + struct mv_udc *udc; + int ret;
if (!driver || driver->speed < USB_SPEED_FULL @@ -497,15 +495,22 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) return -EINVAL; }
- if (!mvudc_probe()) { - usb_lowlevel_init(0, &ctrl); + ret = usb_lowlevel_init(0, (void **)&controller.ctrl); + if (ret) + return ret; + + ret = mvudc_probe(); + if (!ret) { + udc = (struct mv_udc *)controller.ctrl->hcor; + /* select ULPI phy */ writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc); } - retval = driver->bind(&controller.gadget); - if (retval) { - DBG("driver->bind() returned %d\n", retval); - return retval; + + ret = driver->bind(&controller.gadget); + if (ret) { + DBG("driver->bind() returned %d\n", ret); + return ret; } controller.driver = driver;
diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index 88100ce..9af9358 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -27,6 +27,8 @@ #include <linux/usb/ch9.h> #include <linux/usb/gadget.h>
+#include "../../drivers/usb/host/ehci.h" + #define NUM_ENDPOINTS 6
/* Endpoint parameters */ @@ -36,7 +38,6 @@ #define EP0_MAX_PACKET_SIZE 64
struct mv_udc { - u32 pad0[80]; #define MICRO_8FRAME 0x8 #define USBCMD_ITC(x) ((((x) > 0xff) ? 0xff : x) << 16) #define USBCMD_FS2 (1 << 15) @@ -86,7 +87,7 @@ struct mv_ep { struct mv_drv { struct usb_gadget gadget; struct usb_gadget_driver *driver; - struct mv_udc *udc; + struct ehci_ctrl *ctrl; struct mv_ep ep[NUM_ENDPOINTS]; };
@@ -134,5 +135,4 @@ struct ept_queue_item { #define INFO_BUFFER_ERROR (1 << 5) #define INFO_TX_ERROR (1 << 3)
-extern int usb_lowlevel_init(int index, void **controller); #endif /* __MV_UDC_H__ */

Clean up the code that checks the validity of a USB gadget driver in usb_gadget_register_driver(). Moreover, limit the speed of the driver to either FULL or HIGH, this is more precise and once we have xHCI support, also more correct.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index fd3be77..5b2c16a 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -487,13 +487,12 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) struct mv_udc *udc; int ret;
- if (!driver - || driver->speed < USB_SPEED_FULL - || !driver->bind - || !driver->setup) { - DBG("bad parameter.\n"); + if (!driver) + return -EINVAL; + if (!driver->bind || !driver->setup || !driver->disconnect) + return -EINVAL; + if (driver->speed != USB_SPEED_FULL && driver->speed != USB_SPEED_HIGH) return -EINVAL; - }
ret = usb_lowlevel_init(0, (void **)&controller.ctrl); if (ret)

The QH_MAXNUM is used in absolutelly incorrect manner and is not even needed. Remove it and correctly replace it's occurance with 2 * NUM_ENDPOINTS .
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 5b2c16a..04e46e1 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -60,7 +60,7 @@ static const char *reqname(unsigned r) #endif
#define PAGE_SIZE 4096 -#define QH_MAXNUM 32 + static struct usb_endpoint_descriptor ep0_out_desc = { .bLength = sizeof(struct usb_endpoint_descriptor), .bDescriptorType = USB_DT_ENDPOINT, @@ -441,10 +441,11 @@ static int mvudc_probe(void) { struct ept_queue_head *head; int i; + const int num = 2 * NUM_ENDPOINTS;
controller.gadget.ops = &mv_udc_ops; - epts = memalign(PAGE_SIZE, QH_MAXNUM * sizeof(struct ept_queue_head)); - memset(epts, 0, QH_MAXNUM * sizeof(struct ept_queue_head)); + epts = memalign(PAGE_SIZE, num * sizeof(struct ept_queue_head)); + memset(epts, 0, num * sizeof(struct ept_queue_head)); for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { /* * For item0 and item1, they are served as ep0

There is no need to init this field at runtime, so init it statically.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 04e46e1..8fa9781 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -114,8 +114,9 @@ static const struct usb_ep mv_ep_init[2] = { };
static struct mv_drv controller = { - .gadget = { - .name = "mv_udc", + .gadget = { + .name = "mv_udc", + .ops = &mv_udc_ops, }, };
@@ -443,7 +444,6 @@ static int mvudc_probe(void) int i; const int num = 2 * NUM_ENDPOINTS;
- controller.gadget.ops = &mv_udc_ops; epts = memalign(PAGE_SIZE, num * sizeof(struct ept_queue_head)); memset(epts, 0, num * sizeof(struct ept_queue_head)); for (i = 0; i < 2 * NUM_ENDPOINTS; i++) {

Both the endpoint queue head and the endpoint item list is a controller specific thing. Move them both into controller private data.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 26 +++++++++++++------------- include/usb/mv_udc.h | 2 ++ 2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 8fa9781..422d0a9 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -75,8 +75,6 @@ static struct usb_endpoint_descriptor ep0_in_desc = { .bmAttributes = USB_ENDPOINT_XFER_CONTROL, };
-struct ept_queue_head *epts; -struct ept_queue_item *items[2 * NUM_ENDPOINTS]; static int mv_pullup(struct usb_gadget *gadget, int is_on); static int mv_ep_enable(struct usb_ep *ep, const struct usb_endpoint_descriptor *desc); @@ -137,7 +135,7 @@ static void ep_enable(int num, int in) struct ept_queue_head *head; struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; unsigned n; - head = epts + 2*num + in; + head = controller.epts + 2*num + in;
n = readl(&udc->epctrl[num]); if (in) @@ -178,8 +176,8 @@ static int mv_ep_queue(struct usb_ep *ep, int bit, num, len, in; num = mv_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (mv_ep->desc->bEndpointAddress & USB_DIR_IN) != 0; - item = items[2 * num + in]; - head = epts + 2 * num + in; + item = controller.items[2 * num + in]; + head = controller.epts + 2 * num + in; phys = (unsigned)req->buf; len = req->length;
@@ -214,7 +212,7 @@ static void handle_ep_complete(struct mv_ep *ep) in = (ep->desc->bEndpointAddress & USB_DIR_IN) != 0; if (num == 0) ep->desc = &ep0_out_desc; - item = items[2 * num + in]; + item = controller.items[2 * num + in];
if (item->info & 0xff) printf("EP%d/%s FAIL nfo=%x pg0=%x\n", @@ -243,7 +241,7 @@ static void handle_setup(void) int status = 0; int num, in, _num, _in, i; char *buf; - head = epts; + head = controller.epts + 2 * 0 + 0;
flush_cache((unsigned long)head, sizeof(struct ept_queue_head)); memcpy(&r, head->setup_data, sizeof(struct usb_ctrlrequest)); @@ -324,7 +322,7 @@ static void stop_activity(void) & USB_ENDPOINT_NUMBER_MASK; in = (controller.ep[i].desc->bEndpointAddress & USB_DIR_IN) != 0; - head = epts + (num * 2) + (in); + head = controller.epts + (num * 2) + (in); head->info = INFO_ACTIVE; } } @@ -407,7 +405,7 @@ static int mv_pullup(struct usb_gadget *gadget, int is_on) writel(USBCMD_ITC(MICRO_8FRAME) | USBCMD_RST, &udc->usbcmd); udelay(200);
- writel((unsigned) epts, &udc->epinitaddr); + writel((unsigned) controller.epts, &udc->epinitaddr);
/* select DEVICE mode */ writel(USBMODE_DEVICE, &udc->usbmode); @@ -444,14 +442,15 @@ static int mvudc_probe(void) int i; const int num = 2 * NUM_ENDPOINTS;
- epts = memalign(PAGE_SIZE, num * sizeof(struct ept_queue_head)); - memset(epts, 0, num * sizeof(struct ept_queue_head)); + controller.epts = memalign(PAGE_SIZE, + num * sizeof(struct ept_queue_head)); + memset(controller.epts, 0, num * sizeof(struct ept_queue_head)); for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { /* * For item0 and item1, they are served as ep0 * out&in seperately */ - head = epts + i; + head = controller.epts + i; if (i < 2) head->config = CONFIG_MAX_PKT(EP0_MAX_PACKET_SIZE) | CONFIG_ZLT | CONFIG_IOS; @@ -461,7 +460,8 @@ static int mvudc_probe(void) head->next = TERMINATE; head->info = 0;
- items[i] = memalign(PAGE_SIZE, sizeof(struct ept_queue_item)); + controller.items[i] = memalign(PAGE_SIZE, + sizeof(struct ept_queue_item)); }
INIT_LIST_HEAD(&controller.gadget.ep_list); diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index 9af9358..78fa81c 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -88,6 +88,8 @@ struct mv_drv { struct usb_gadget gadget; struct usb_gadget_driver *driver; struct ehci_ctrl *ctrl; + struct ept_queue_head *epts; + struct ept_queue_item *items[2 * NUM_ENDPOINTS]; struct mv_ep ep[NUM_ENDPOINTS]; };

The endpoint QH list has to be aligned to 10-bit boundary. We also have to make sure the list is aligned on a cacheline boundary. Make sure it is. Furthermore, check if the memory allocation for the QH list didn't fail. Moveover, improve the comment about the QH list structure.
Finally, the qTD item list has to be aligned only to 5-bit boundary, not 10-bit as it is now, fix this as well.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 422d0a9..394af6b 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -59,8 +59,6 @@ static const char *reqname(unsigned r) } #endif
-#define PAGE_SIZE 4096 - static struct usb_endpoint_descriptor ep0_out_desc = { .bLength = sizeof(struct usb_endpoint_descriptor), .bDescriptorType = USB_DT_ENDPOINT, @@ -440,15 +438,27 @@ static int mvudc_probe(void) { struct ept_queue_head *head; int i; + const int num = 2 * NUM_ENDPOINTS;
- controller.epts = memalign(PAGE_SIZE, - num * sizeof(struct ept_queue_head)); - memset(controller.epts, 0, num * sizeof(struct ept_queue_head)); + const int eplist_min_align = 4096; + const int eplist_align = roundup(eplist_min_align, ARCH_DMA_MINALIGN); + const int eplist_raw_sz = num * sizeof(struct ept_queue_head); + const int eplist_sz = roundup(eplist_raw_sz, ARCH_DMA_MINALIGN); + + /* The QH list must be aligned to 4096 bytes. */ + controller.epts = memalign(eplist_align, eplist_sz); + if (!controller.epts) + return -ENOMEM; + memset(controller.epts, 0, eplist_sz); + for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { /* - * For item0 and item1, they are served as ep0 - * out&in seperately + * Configure QH for each endpoint. The structure of the QH list + * is such that each two subsequent fields, N and N+1 where N is + * even, in the QH list represent QH for one endpoint. The Nth + * entry represents OUT configuration and the N+1th entry does + * represent IN configuration of the endpoint. */ head = controller.epts + i; if (i < 2) @@ -460,7 +470,7 @@ static int mvudc_probe(void) head->next = TERMINATE; head->info = 0;
- controller.items[i] = memalign(PAGE_SIZE, + controller.items[i] = memalign(roundup(32, ARCH_DMA_MINALIGN), sizeof(struct ept_queue_item)); }

Check the length of system cacheline at compile-time and fail if the system uses too long cachelines.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 394af6b..2076717 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -37,6 +37,16 @@ #error This driver only supports one single controller. #endif
+/* + * Check if the system has too long cachelines. If the cachelines are + * longer then 128b, the driver will not be able flush/invalidate data + * cache over separate QH entries. We use 128b because one QH entry is + * 64b long and there are always two QH list entries for each endpoint. + */ +#if ARCH_DMA_MINALIGN > 128 +#error This driver can not work on systems with caches longer than 128b +#endif + #ifndef DEBUG #define DBG(x...) do {} while (0) #else

The code for retrieving QH for particular endpoint is hard to understand, moreover it's duplicated all over the driver. Move the code into single nice and documented function.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 2076717..68ad9f1 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -126,6 +126,19 @@ static struct mv_drv controller = { }, };
+/** + * mv_get_qh() - return queue head for endpoint + * @ep_num: Endpoint number + * @dir_in: Direction of the endpoint (IN = 1, OUT = 0) + * + * This function returns the QH associated with particular endpoint + * and it's direction. + */ +static struct ept_queue_head *mv_get_qh(int ep_num, int dir_in) +{ + return &controller.epts[(ep_num * 2) + dir_in]; +} + static struct usb_request * mv_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) { @@ -143,7 +156,7 @@ static void ep_enable(int num, int in) struct ept_queue_head *head; struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; unsigned n; - head = controller.epts + 2*num + in; + head = mv_get_qh(num, in);
n = readl(&udc->epctrl[num]); if (in) @@ -185,7 +198,7 @@ static int mv_ep_queue(struct usb_ep *ep, num = mv_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (mv_ep->desc->bEndpointAddress & USB_DIR_IN) != 0; item = controller.items[2 * num + in]; - head = controller.epts + 2 * num + in; + head = mv_get_qh(num, in); phys = (unsigned)req->buf; len = req->length;
@@ -249,7 +262,7 @@ static void handle_setup(void) int status = 0; int num, in, _num, _in, i; char *buf; - head = controller.epts + 2 * 0 + 0; + head = mv_get_qh(0, 0); /* EP0 OUT */
flush_cache((unsigned long)head, sizeof(struct ept_queue_head)); memcpy(&r, head->setup_data, sizeof(struct usb_ctrlrequest)); @@ -330,7 +343,7 @@ static void stop_activity(void) & USB_ENDPOINT_NUMBER_MASK; in = (controller.ep[i].desc->bEndpointAddress & USB_DIR_IN) != 0; - head = controller.epts + (num * 2) + (in); + head = mv_get_qh(num, in); head->info = INFO_ACTIVE; } } @@ -413,7 +426,7 @@ static int mv_pullup(struct usb_gadget *gadget, int is_on) writel(USBCMD_ITC(MICRO_8FRAME) | USBCMD_RST, &udc->usbcmd); udelay(200);
- writel((unsigned) controller.epts, &udc->epinitaddr); + writel((unsigned)controller.epts, &udc->epinitaddr);
/* select DEVICE mode */ writel(USBMODE_DEVICE, &udc->usbmode);

Allocate the qTD items all at once instead of allocating them separately. Moreover, make sure each qTD is properly aligned to 32-bytes boundary and that cache can be safely flushed over each qTD touple.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 25 +++++++++++++++++++++++-- include/usb/mv_udc.h | 1 + 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 68ad9f1..55dd5df 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -460,6 +460,7 @@ void udc_disconnect(void) static int mvudc_probe(void) { struct ept_queue_head *head; + uint8_t *imem; int i;
const int num = 2 * NUM_ENDPOINTS; @@ -469,12 +470,29 @@ static int mvudc_probe(void) const int eplist_raw_sz = num * sizeof(struct ept_queue_head); const int eplist_sz = roundup(eplist_raw_sz, ARCH_DMA_MINALIGN);
+ const int ilist_align = roundup(ARCH_DMA_MINALIGN, 32); + const int ilist_ent_raw_sz = 2 * sizeof(struct ept_queue_item); + const int ilist_ent_sz = roundup(ilist_ent_raw_sz, ARCH_DMA_MINALIGN); + const int ilist_sz = NUM_ENDPOINTS * ilist_ent_sz; + /* The QH list must be aligned to 4096 bytes. */ controller.epts = memalign(eplist_align, eplist_sz); if (!controller.epts) return -ENOMEM; memset(controller.epts, 0, eplist_sz);
+ /* + * Each qTD item must be 32-byte aligned, each qTD touple must be + * cacheline aligned. There are two qTD items for each endpoint and + * only one of them is used for the endpoint at time, so we can group + * them together. + */ + controller.items_mem = memalign(ilist_align, ilist_sz); + if (!controller.items_mem) { + free(controller.epts); + return -ENOMEM; + } + for (i = 0; i < 2 * NUM_ENDPOINTS; i++) { /* * Configure QH for each endpoint. The structure of the QH list @@ -493,8 +511,11 @@ static int mvudc_probe(void) head->next = TERMINATE; head->info = 0;
- controller.items[i] = memalign(roundup(32, ARCH_DMA_MINALIGN), - sizeof(struct ept_queue_item)); + imem = controller.items_mem + ((i >> 1) * ilist_ent_sz); + if (i & 1) + imem += sizeof(struct ept_queue_item); + + controller.items[i] = (struct ept_queue_item *)imem; }
INIT_LIST_HEAD(&controller.gadget.ep_list); diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index 78fa81c..b58a95f 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -90,6 +90,7 @@ struct mv_drv { struct ehci_ctrl *ctrl; struct ept_queue_head *epts; struct ept_queue_item *items[2 * NUM_ENDPOINTS]; + uint8_t *items_mem; struct mv_ep ep[NUM_ENDPOINTS]; };

The code for retrieving qTD item for particular endpoint is hard to understand, moreover it's duplicated all over the driver. Move the code into single nice and documented function.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 55dd5df..364c8ed 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -139,6 +139,19 @@ static struct ept_queue_head *mv_get_qh(int ep_num, int dir_in) return &controller.epts[(ep_num * 2) + dir_in]; }
+/** + * mv_get_qtd() - return queue item for endpoint + * @ep_num: Endpoint number + * @dir_in: Direction of the endpoint (IN = 1, OUT = 0) + * + * This function returns the QH associated with particular endpoint + * and it's direction. + */ +static struct ept_queue_item *mv_get_qtd(int ep_num, int dir_in) +{ + return controller.items[(ep_num * 2) + dir_in]; +} + static struct usb_request * mv_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) { @@ -197,7 +210,7 @@ static int mv_ep_queue(struct usb_ep *ep, int bit, num, len, in; num = mv_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (mv_ep->desc->bEndpointAddress & USB_DIR_IN) != 0; - item = controller.items[2 * num + in]; + item = mv_get_qtd(num, in); head = mv_get_qh(num, in); phys = (unsigned)req->buf; len = req->length; @@ -233,7 +246,7 @@ static void handle_ep_complete(struct mv_ep *ep) in = (ep->desc->bEndpointAddress & USB_DIR_IN) != 0; if (num == 0) ep->desc = &ep0_out_desc; - item = controller.items[2 * num + in]; + item = mv_get_qtd(num, in);
if (item->info & 0xff) printf("EP%d/%s FAIL nfo=%x pg0=%x\n",

Implement functions to flush/invalidate dcache over QH and qTDs and make use of them where appropriate. Also use them to replace the old incorrect cache management attempt. This is the first step towards making this driver work with data cache enabled.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 82 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 364c8ed..1eda8bc 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -152,6 +152,68 @@ static struct ept_queue_item *mv_get_qtd(int ep_num, int dir_in) return controller.items[(ep_num * 2) + dir_in]; }
+/** + * mv_flush_qh - flush cache over queue head + * @ep_num: Endpoint number + * + * This function flushes cache over QH for particular endpoint. + */ +static void mv_flush_qh(int ep_num) +{ + struct ept_queue_head *head = mv_get_qh(ep_num, 0); + const uint32_t start = (uint32_t)head; + const uint32_t end = start + 2 * sizeof(*head); + + flush_dcache_range(start, end); +} + +/** + * mv_invalidate_qh - invalidate cache over queue head + * @ep_num: Endpoint number + * + * This function invalidates cache over QH for particular endpoint. + */ +static void mv_invalidate_qh(int ep_num) +{ + struct ept_queue_head *head = mv_get_qh(ep_num, 0); + uint32_t start = (uint32_t)head; + uint32_t end = start + 2 * sizeof(*head); + + invalidate_dcache_range(start, end); +} + +/** + * mv_flush_qtd - flush cache over queue item + * @ep_num: Endpoint number + * + * This function flushes cache over qTD pair for particular endpoint. + */ +static void mv_flush_qtd(int ep_num) +{ + struct ept_queue_item *item = mv_get_qtd(ep_num, 0); + const uint32_t start = (uint32_t)item; + const uint32_t end_raw = start + 2 * sizeof(*item); + const uint32_t end = roundup(end_raw, ARCH_DMA_MINALIGN); + + flush_dcache_range(start, end); +} + +/** + * mv_invalidate_qtd - invalidate cache over queue item + * @ep_num: Endpoint number + * + * This function invalidates cache over qTD pair for particular endpoint. + */ +static void mv_invalidate_qtd(int ep_num) +{ + struct ept_queue_item *item = mv_get_qtd(ep_num, 0); + const uint32_t start = (uint32_t)item; + const uint32_t end_raw = start + 2 * sizeof(*item); + const uint32_t end = roundup(end_raw, ARCH_DMA_MINALIGN); + + invalidate_dcache_range(start, end); +} + static struct usb_request * mv_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) { @@ -177,8 +239,10 @@ static void ep_enable(int num, int in) else n |= (CTRL_RXE | CTRL_RXR | CTRL_RXT_BULK);
- if (num != 0) + if (num != 0) { head->config = CONFIG_MAX_PKT(EP_MAX_PACKET_SIZE) | CONFIG_ZLT; + mv_flush_qh(num); + } writel(n, &udc->epctrl[num]); }
@@ -231,8 +295,9 @@ static int mv_ep_queue(struct usb_ep *ep, else bit = EPT_RX(num);
- flush_cache(phys, len); - flush_cache((unsigned long)item, sizeof(struct ept_queue_item)); + mv_flush_qh(num); + mv_flush_qtd(num); + writel(bit, &udc->epprime);
return 0; @@ -247,7 +312,8 @@ static void handle_ep_complete(struct mv_ep *ep) if (num == 0) ep->desc = &ep0_out_desc; item = mv_get_qtd(num, in); - + mv_invalidate_qtd(num); + if (item->info & 0xff) printf("EP%d/%s FAIL nfo=%x pg0=%x\n", num, in ? "in" : "out", item->info, item->page0); @@ -277,7 +343,7 @@ static void handle_setup(void) char *buf; head = mv_get_qh(0, 0); /* EP0 OUT */
- flush_cache((unsigned long)head, sizeof(struct ept_queue_head)); + mv_invalidate_qh(0); memcpy(&r, head->setup_data, sizeof(struct usb_ctrlrequest)); writel(EPT_RX(0), &udc->epstat); DBG("handle setup %s, %x, %x index %x value %x\n", reqname(r.bRequest), @@ -358,6 +424,7 @@ static void stop_activity(void) & USB_DIR_IN) != 0; head = mv_get_qh(num, in); head->info = INFO_ACTIVE; + mv_flush_qh(num); } } } @@ -529,6 +596,11 @@ static int mvudc_probe(void) imem += sizeof(struct ept_queue_item);
controller.items[i] = (struct ept_queue_item *)imem; + + if (i & 1) { + mv_flush_qh(i - 1); + mv_flush_qtd(i - 1); + } }
INIT_LIST_HEAD(&controller.gadget.ep_list);

The requests sent to the controller are not properly cache aligned most of the time, thus implement a simple bounce buffer to avoid problem with cache.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Lei Wen leiwen@marvell.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/mv_udc.c | 84 +++++++++++++++++++++++++++++++++++++++---- include/usb/mv_udc.h | 6 +++- 2 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c index 1eda8bc..a87ddc7 100644 --- a/drivers/usb/gadget/mv_udc.c +++ b/drivers/usb/gadget/mv_udc.c @@ -263,6 +263,71 @@ static int mv_ep_disable(struct usb_ep *ep) return 0; }
+static int mv_bounce(struct mv_ep *ep) +{ + uint32_t addr = (uint32_t)ep->req.buf; + uint32_t ba; + + /* Input buffer address is not aligned. */ + if (addr & (ARCH_DMA_MINALIGN - 1)) + goto align; + + /* Input buffer length is not aligned. */ + if (ep->req.length & (ARCH_DMA_MINALIGN - 1)) + goto align; + + /* The buffer is well aligned, only flush cache. */ + ep->b_len = ep->req.length; + ep->b_buf = ep->req.buf; + goto flush; + +align: + /* Use internal buffer for small payloads. */ + if (ep->req.length <= 64) { + ep->b_len = 64; + ep->b_buf = ep->b_fast; + } else { + ep->b_len = roundup(ep->req.length, ARCH_DMA_MINALIGN); + ep->b_buf = memalign(ARCH_DMA_MINALIGN, ep->b_len); + if (!ep->b_buf) + return -ENOMEM; + } + + memcpy(ep->b_buf, ep->req.buf, ep->req.length); + +flush: + ba = (uint32_t)ep->b_buf; + flush_dcache_range(ba, ba + ep->b_len); + + return 0; +} + +static void mv_debounce(struct mv_ep *ep) +{ + uint32_t addr = (uint32_t)ep->req.buf; + uint32_t ba = (uint32_t)ep->b_buf; + + invalidate_dcache_range(ba, ba + ep->b_len); + + /* Input buffer address is not aligned. */ + if (addr & (ARCH_DMA_MINALIGN - 1)) + goto copy; + + /* Input buffer length is not aligned. */ + if (ep->req.length & (ARCH_DMA_MINALIGN - 1)) + goto copy; + + /* The buffer is well aligned, only invalidate cache. */ + return; + +copy: + memcpy(ep->req.buf, ep->b_buf, ep->req.length); + + /* Large payloads use allocated buffer, free it. */ + if (ep->req.length > 64) + free(ep->b_buf); +} + static int mv_ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { @@ -270,25 +335,27 @@ static int mv_ep_queue(struct usb_ep *ep, struct mv_udc *udc = (struct mv_udc *)controller.ctrl->hcor; struct ept_queue_item *item; struct ept_queue_head *head; - unsigned phys; - int bit, num, len, in; + int bit, num, len, in, ret; num = mv_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; in = (mv_ep->desc->bEndpointAddress & USB_DIR_IN) != 0; item = mv_get_qtd(num, in); head = mv_get_qh(num, in); - phys = (unsigned)req->buf; len = req->length;
+ ret = mv_bounce(mv_ep); + if (ret) + return ret; + item->next = TERMINATE; item->info = INFO_BYTES(len) | INFO_IOC | INFO_ACTIVE; - item->page0 = phys; - item->page1 = (phys & 0xfffff000) + 0x1000; + item->page0 = (uint32_t)mv_ep->b_buf; + item->page1 = ((uint32_t)mv_ep->b_buf & 0xfffff000) + 0x1000;
head->next = (unsigned) item; head->info = 0;
- DBG("ept%d %s queue len %x, buffer %x\n", - num, in ? "in" : "out", len, phys); + DBG("ept%d %s queue len %x, buffer %p\n", + num, in ? "in" : "out", len, mv_ep->b_buf);
if (in) bit = EPT_TX(num); @@ -319,6 +386,9 @@ static void handle_ep_complete(struct mv_ep *ep) num, in ? "in" : "out", item->info, item->page0);
len = (item->info >> 16) & 0x7fff; + + mv_debounce(ep); + ep->req.length -= len; DBG("ept%d %s complete %x\n", num, in ? "in" : "out", len); diff --git a/include/usb/mv_udc.h b/include/usb/mv_udc.h index b58a95f..b02b439 100644 --- a/include/usb/mv_udc.h +++ b/include/usb/mv_udc.h @@ -79,9 +79,13 @@ struct mv_udc {
struct mv_ep { struct usb_ep ep; - struct usb_request req; struct list_head queue; const struct usb_endpoint_descriptor *desc; + + struct usb_request req; + uint8_t *b_buf; + uint32_t b_len; + uint8_t b_fast[64] __aligned(ARCH_DMA_MINALIGN); };
struct mv_drv {
participants (1)
-
Marek Vasut