* [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets
@ 2014-04-24 23:52 Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 2/5] usb: ci_udc: set ep->req.actual after transfer Stephen Warren
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Stephen Warren @ 2014-04-24 23:52 UTC (permalink / raw
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
ci_ep_queue() currently only fills in the page0/page1 fields in the
queue item. If the buffer is larger than 4KiB (unaligned) or 8KiB
(page-aligned), then this prevents the HW from knowing where to write
the balance of the data.
Fix this by initializing all 5 pageN pointers, which allows up to
16KiB (potentially non-page-aligned) buffers.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/usb/gadget/ci_udc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index 14b1e9b8bf11..815ce7b262ca 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -350,6 +350,9 @@ static int ci_ep_queue(struct usb_ep *ep,
item->info = INFO_BYTES(len) | INFO_IOC | INFO_ACTIVE;
item->page0 = (uint32_t)ci_ep->b_buf;
item->page1 = ((uint32_t)ci_ep->b_buf & 0xfffff000) + 0x1000;
+ item->page2 = ((uint32_t)ci_ep->b_buf & 0xfffff000) + 0x2000;
+ item->page3 = ((uint32_t)ci_ep->b_buf & 0xfffff000) + 0x3000;
+ item->page4 = ((uint32_t)ci_ep->b_buf & 0xfffff000) + 0x4000;
ci_flush_qtd(num);
head->next = (unsigned) item;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/5] usb: ci_udc: set ep->req.actual after transfer
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
@ 2014-04-24 23:52 ` Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 3/5] usb: ci_udc: make PHY initialization conditional Stephen Warren
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-04-24 23:52 UTC (permalink / raw
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
At least drivers/usb/gadget/storage_common.c expects that ep->req.actual
contain the number of bytes actually transferred. (At least in practice,
I observed it failing to work correctly unless this was the case).
However, ci_udc.c modifies ep->req.length instead. I assume that .length
is supposed to represent the allocated buffer size, whereas .actual is
supposed to represent the actual number of bytes transferred. In the OUT
transaction case, this may happen simply because the host sends a smaller
packet than the max possible size, which is quite legal. In the IN case,
transferring fewer bytes than requested could presumably happen as an
error.
Modify handle_ep_complete() to write to .actual rather than modifying
.length.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/usb/gadget/ci_udc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index 815ce7b262ca..832606f5e441 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -321,7 +321,7 @@ static void ci_debounce(struct ci_ep *ep, int in)
if (addr == ba)
return; /* not a bounce */
- memcpy(ep->req.buf, ep->b_buf, ep->req.length);
+ memcpy(ep->req.buf, ep->b_buf, ep->req.actual);
free:
/* Large payloads use allocated buffer, free it. */
if (ep->b_buf != ep->b_fast)
@@ -388,7 +388,7 @@ static void handle_ep_complete(struct ci_ep *ep)
num, in ? "in" : "out", item->info, item->page0);
len = (item->info >> 16) & 0x7fff;
- ep->req.length -= len;
+ ep->req.actual = ep->req.length - len;
ci_debounce(ep, in);
DBG("ept%d %s complete %x\n",
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/5] usb: ci_udc: make PHY initialization conditional
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 2/5] usb: ci_udc: set ep->req.actual after transfer Stephen Warren
@ 2014-04-24 23:52 ` Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 4/5] usb: ci_udc: support variants with hostpc register Stephen Warren
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-04-24 23:52 UTC (permalink / raw
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
usb_gadget_register_driver() currently unconditionally programs PORTSC
to select a ULPI PHY. This is incorrect on at least the Tegra boards I
am testing with, which use a UTMI PHY for the OTG ports. Make the PHY
selection code conditional upon the specific EHCI controller that is in
use.
Ideally, I believe that the PHY initialization code should be part of
ehci_hcd_init() in the relevant EHCI controller driver, or some board-
specific function that ehci_hcd_init() calls.
For MX6, I'm not sure this PHY initialization code is correct even before
this patch, since ehci-mx6's ehci_hcd_init() already configures PORTSC to
a board-specific value, and it seems likely that the code in ci_udc.c is
incorrectly undoing this. Perhaps this is not an issue if the PHY
selection register bits aren't implemented on this instance of the MX6
USB controller?
ehci-mxs.c doens't appear to touch PORTSC, so this code is likely still
required there.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/usb/gadget/ci_udc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index 832606f5e441..fd751a242639 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -702,7 +702,6 @@ static int ci_udc_probe(void)
int usb_gadget_register_driver(struct usb_gadget_driver *driver)
{
- struct ci_udc *udc;
int ret;
if (!driver)
@@ -717,12 +716,18 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
return ret;
ret = ci_udc_probe();
+#if defined(CONFIG_USB_EHCI_MX6) || defined(CONFIG_USB_EHCI_MXS)
+ /*
+ * FIXME: usb_lowlevel_init()->ehci_hcd_init() should be doing all
+ * HW-specific initialization, e.g. ULPI-vs-UTMI PHY selection
+ */
if (!ret) {
- udc = (struct ci_udc *)controller.ctrl->hcor;
+ struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor;
/* select ULPI phy */
writel(PTS(PTS_ENABLE) | PFSC, &udc->portsc);
}
+#endif
ret = driver->bind(&controller.gadget);
if (ret) {
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 4/5] usb: ci_udc: support variants with hostpc register
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 2/5] usb: ci_udc: set ep->req.actual after transfer Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 3/5] usb: ci_udc: make PHY initialization conditional Stephen Warren
@ 2014-04-24 23:52 ` Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 5/5] usb: ums: use only 1 buffer for CI_UDC Stephen Warren
2014-04-28 5:34 ` [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Marek Vasut
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-04-24 23:52 UTC (permalink / raw
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
Tegra's USB controller appears to be a variant of the ChipIdea
controller; perhaps derived from it, or simply a different version of
the IP core to what U-Boot supports today.
In this variant, at least the following difference are present:
- Some registers are moved about.
- Setup transaction completion is reported in a separate 'epsetupstat'
register, rather than in 'epstat' (which still exists, perhaps for
other transaction types).
- USB connection speed is reported in a separate 'hostpc1_devlc'
register, rather than 'portsc'.
- The registers used by ci_udc.c begin at offset 0x130 from the USB
register base, rather than offset 0x140. However, this is handled
by the associated EHCI controller driver, since the register address
is stored in controller.ctrl->hcor.
Introduce define CONFIG_CI_UDC_HAS_HOSTPC to indicate which variant of
the controller should be supported. The "HAS_HOSTPC" part of this name
mirrors the similar "has_hostpc" field used by the Linux EHCI controller
core to represent the presence/absence of the hostpc1_devlc register.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/usb/gadget/ci_udc.c | 15 +++++++++++
drivers/usb/gadget/ci_udc.h | 65 ++++++++++++++++++++++++++++++++-------------
2 files changed, 62 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index fd751a242639..02d3fdade86c 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -416,7 +416,11 @@ static void handle_setup(void)
ci_invalidate_qh(0);
memcpy(&r, head->setup_data, sizeof(struct usb_ctrlrequest));
+#ifdef CONFIG_CI_UDC_HAS_HOSTPC
+ writel(EPT_RX(0), &udc->epsetupstat);
+#else
writel(EPT_RX(0), &udc->epstat);
+#endif
DBG("handle setup %s, %x, %x index %x value %x\n", reqname(r.bRequest),
r.bRequestType, r.bRequest, r.wIndex, r.wValue);
@@ -483,6 +487,9 @@ static void stop_activity(void)
struct ept_queue_head *head;
struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor;
writel(readl(&udc->epcomp), &udc->epcomp);
+#ifdef CONFIG_CI_UDC_HAS_HOSTPC
+ writel(readl(&udc->epsetupstat), &udc->epsetupstat);
+#endif
writel(readl(&udc->epstat), &udc->epstat);
writel(0xffffffff, &udc->epflush);
@@ -524,7 +531,11 @@ void udc_irq(void)
int max = 64;
int speed = USB_SPEED_FULL;
+#ifdef CONFIG_CI_UDC_HAS_HOSTPC
+ bit = (readl(&udc->hostpc1_devlc) >> 25) & 3;
+#else
bit = (readl(&udc->portsc) >> 26) & 3;
+#endif
DBG("-- portchange %x %s\n", bit, (bit == 2) ? "High" : "Full");
if (bit == 2) {
speed = USB_SPEED_HIGH;
@@ -541,7 +552,11 @@ void udc_irq(void)
printf("<UEI %x>\n", readl(&udc->epcomp));
if ((n & STS_UI) || (n & STS_UEI)) {
+#ifdef CONFIG_CI_UDC_HAS_HOSTPC
+ n = readl(&udc->epsetupstat);
+#else
n = readl(&udc->epstat);
+#endif
if (n & EPT_RX(0))
handle_setup();
diff --git a/drivers/usb/gadget/ci_udc.h b/drivers/usb/gadget/ci_udc.h
index 42f6ef4ab30a..4425fd934570 100644
--- a/drivers/usb/gadget/ci_udc.h
+++ b/drivers/usb/gadget/ci_udc.h
@@ -8,45 +8,74 @@
#define NUM_ENDPOINTS 6
+#ifdef CONFIG_CI_UDC_HAS_HOSTPC
+struct ci_udc {
+ u32 usbcmd; /* 0x130 */
+ u32 usbsts; /* 0x134 */
+ u32 pad1[3];
+ u32 devaddr; /* 0x144 */
+ u32 epinitaddr; /* 0x148 */
+ u32 pad2[10];
+ u32 portsc; /* 0x174 */
+ u32 pad178[(0x1b4 - (0x174 + 4)) / 4];
+ u32 hostpc1_devlc; /* 0x1b4 */
+ u32 pad1b8[(0x1f8 - (0x1b4 + 4)) / 4];
+ u32 usbmode; /* 0x1f8 */
+ u32 pad1fc[(0x208 - (0x1f8 + 4)) / 4];
+ u32 epsetupstat; /* 0x208 */
+ u32 epprime; /* 0x20c */
+ u32 epflush; /* 0x210 */
+ u32 epstat; /* 0x214 */
+ u32 epcomp; /* 0x218 */
+ u32 epctrl[16]; /* 0x21c */
+};
+#else
struct ci_udc {
-#define MICRO_8FRAME 0x8
-#define USBCMD_ITC(x) ((((x) > 0xff) ? 0xff : x) << 16)
-#define USBCMD_FS2 (1 << 15)
-#define USBCMD_RST (1 << 1)
-#define USBCMD_RUN (1)
u32 usbcmd; /* 0x140 */
-#define STS_SLI (1 << 8)
-#define STS_URI (1 << 6)
-#define STS_PCI (1 << 2)
-#define STS_UEI (1 << 1)
-#define STS_UI (1 << 0)
u32 usbsts; /* 0x144 */
u32 pad1[3];
u32 devaddr; /* 0x154 */
u32 epinitaddr; /* 0x158 */
u32 pad2[10];
-#define PTS_ENABLE 2
-#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))
u32 epprime; /* 0x1b0 */
u32 epflush; /* 0x1b4 */
u32 pad4;
u32 epcomp; /* 0x1bc */
+ u32 epctrl[16]; /* 0x1c0 */
+};
+
+#define PTS_ENABLE 2
+#define PTS(x) (((x) & 0x3) << 30)
+#define PFSC (1 << 24)
+#endif
+
+#define MICRO_8FRAME 0x8
+#define USBCMD_ITC(x) ((((x) > 0xff) ? 0xff : x) << 16)
+#define USBCMD_FS2 (1 << 15)
+#define USBCMD_RST (1 << 1)
+#define USBCMD_RUN (1)
+
+#define STS_SLI (1 << 8)
+#define STS_URI (1 << 6)
+#define STS_PCI (1 << 2)
+#define STS_UEI (1 << 1)
+#define STS_UI (1 << 0)
+
+#define USBMODE_DEVICE 2
+
+#define EPT_TX(x) (1 << (((x) & 0xffff) + 16))
+#define EPT_RX(x) (1 << ((x) & 0xffff))
+
#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 */
-};
struct ci_ep {
struct usb_ep ep;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 5/5] usb: ums: use only 1 buffer for CI_UDC
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
` (2 preceding siblings ...)
2014-04-24 23:52 ` [U-Boot] [PATCH 4/5] usb: ci_udc: support variants with hostpc register Stephen Warren
@ 2014-04-24 23:52 ` Stephen Warren
2014-04-28 5:34 ` [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Marek Vasut
4 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-04-24 23:52 UTC (permalink / raw
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
ci_udc.c allocates only a single buffer for each endpoint, which
ci_ep_alloc_request() returns as a hard-coded value rather than
dynamically allocating. Consequently, storage_common.c must limit
itself to using a single buffer at a time. Add a special case
to the definition of FSG_NUM_BUFFERS for this.
Another option would be to fix ci_ep_alloc_request() to dynamically
allocate the buffers like some/all(?) other device mode drivers do.
However, I don't think that ci_ep_queue() supports queueing up
multiple buffers either yet, and I'm not familiar enough with the
controller yet to implement that. As such, any attempt to use multiple
buffers simply results in data corruption and other errors.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/usb/gadget/storage_common.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 02803df23c52..74300746b9db 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -311,7 +311,11 @@ static struct fsg_lun *fsg_lun_from_dev(struct device *dev)
#define DELAYED_STATUS (EP0_BUFSIZE + 999) /* An impossibly large value */
/* Number of buffers we will use. 2 is enough for double-buffering */
+#ifndef CONFIG_CI_UDC
#define FSG_NUM_BUFFERS 2
+#else
+#define FSG_NUM_BUFFERS 1 /* ci_udc only allows 1 req per ep at present */
+#endif
/* Default size of buffer length. */
#define FSG_BUFLEN ((u32)16384)
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
` (3 preceding siblings ...)
2014-04-24 23:52 ` [U-Boot] [PATCH 5/5] usb: ums: use only 1 buffer for CI_UDC Stephen Warren
@ 2014-04-28 5:34 ` Marek Vasut
4 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2014-04-28 5:34 UTC (permalink / raw
To: u-boot
On Friday, April 25, 2014 at 01:52:36 AM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> ci_ep_queue() currently only fills in the page0/page1 fields in the
> queue item. If the buffer is larger than 4KiB (unaligned) or 8KiB
> (page-aligned), then this prevents the HW from knowing where to write
> the balance of the data.
>
> Fix this by initializing all 5 pageN pointers, which allows up to
> 16KiB (potentially non-page-aligned) buffers.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Applied all, thanks. But can you please check if we can get the buffer
allocation from 5/5 sorted please ? I'd really like that instead of such hacks.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-28 5:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-24 23:52 [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 2/5] usb: ci_udc: set ep->req.actual after transfer Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 3/5] usb: ci_udc: make PHY initialization conditional Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 4/5] usb: ci_udc: support variants with hostpc register Stephen Warren
2014-04-24 23:52 ` [U-Boot] [PATCH 5/5] usb: ums: use only 1 buffer for CI_UDC Stephen Warren
2014-04-28 5:34 ` [U-Boot] [PATCH 1/5] usb: ci_udc: Support larger packets Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.