Re: Linux USB file storage gadget with new UDC
Hi, > All I can tell is that the gadget got hung after receiving the second > WRITE command. Can you figure out where it got hung and why? > > Victor, you don't seem to get the big pattern that keeps repeating > here. Every time something does wrong, you tell me about it. Then I > point out that you didn't include any debugging information, so you > send part of a log. Then I point out that you didn't send the entire > log, or you didn't send logs for both the gadget and the host. You end > up losing a day or two each time this happens. > > There's a very simple lesson: When you're asking for help in debugging > a problem, _always_ include _all_ the data that might be relevant. > > Here's another lesson, which I have pointed out a few times before but > you still don't seem to have understood: When you want to know where > your driver is hanging up, put a bunch of printk statements in it, at > all the important spots. Then you'll be able to see, in the log, the > last printk that was executed before the hang. That will tell you > where the problem is. Thanks. I will add more printk statements gradually. Now i discover if i write to a large text file (> 48k) on USB gadget, linux will crash. The full log of UDC and gadget driver when linux crashes, and corresponding usbmon trace are attached. If these logs are not helpful, i shall add more printk. thanks, victor bulk_in_complete --> 0, 13/13 bulk_in_complete --> 0, 13/13 EP1 OUT IRQ 0x28 ep1_out: RX DMA done : NULL REQ on OUT EP-1 Unable to handle kernel paging request at virtual address 4000 pgd = c0204000 [4000] *pgd= Internal error: Oops - BUG: 817 [#1] PREEMPT ARM Modules linked in: g_file_storage kagen2_udc ath6kl_sdio ath6kl_core ka2000_sdio ka2000_sdhc CPU: 0Not tainted (3.4.4+ #41) PC is at th, wValue, wIndex; unsigned int rdata, rdata1; // setup data valid val = readl(dev->base_addr + 0+0xfb0/0x199c [kagen2_udc] LR is at console_unlock+0x208/0x218 pc : []lr : []psr: 2093 sp : c1347c68 ip : c1347b98 fp : c1347eb4 r10: c1328000 r9 : c12b4db4 r8 : 0001 r7 : c12fedd0 r6 : 0200 r5 : c1346000 r4 : c12b4d80 r3 : r2 : 0001 r1 : 015bb795 r0 : 4000 Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 0005717f Table: 01314000 DAC: 0017 Process file-storage-ga (pid: 122, stack limit = 0xc1346270) Stack: (0xc1347c68 to 0xc1348000) 7c60: 0200 c1347c78 000a 6013 7c80: c1328000 c12b4db4 4f203150 50205455 0a474e49 61760909 203d206c 64616572 7ca0: 6564286c 623e2d76 5f657361 72646461 30202b20 63383178 090a3b29 6c617609 7cc0: 203d2620 7830 3030 0a3b 61760909 3d7c206c 30783020 30323030 7ce0: 3b303030 090a0909 69727709 286c6574 2c6c6176 76656420 61623e2d 615f6573 7d00: 20726464 7830202b 29633831 7d090a3b 6c65090a 69206573 76282066 3d206c61 7d20: 7830203d 0a293832 700a7b09 746e6972 4522286b 4f203150 49205455 30205152 7d40: 5c782578 202c226e 296c6176 09090a3b 70652f2f 756f5f31 65642874 0a3b2976 7d60: 73740909 74654c6b 4253555f 7461642e 203d2061 736e7528 656e6769 6f6c2064 7d80: 6429676e 0a3b7665 61740909 656c6b73 63735f74 75646568 2628656c 4c6b7374 7da0: 555f7465 3b294253 09090a0a 206c6176 6572203d 286c6461 2d766564 7361623e 7dc0: 64615f65 2b207264 31783020 3b293061 7d090a09 6c65090a 69206573 76282066 7de0: 3d206c61 7830203d 0a293432 090a7b09 452f2f09 69203150 5249206e 09090a51 7e00: 206c6176 6572203d 286c6461 2d766564 7361623e 64615f65 2b207264 31783020 7e20: 3b293838 7609090a 26206c61 7830203d 3030 09090a3b 206c6176 7e40: 30203d7c 30303078 30303030 09093b32 7709090a 65746972 6176286c 64202c6c 7e60: 3e2d7665 65736162 6464615f 202b2072 38317830 0a3b2938 090a0909 65090a7d 7e80: 2065736c 28206669 c03ef7b0 c12b4d80 c12fedd0 c1289600 c12896f8 c12896e0 7ea0: 7e00 c1346018 c1347eec c1347eb8 bf035f9c bf02fa88 c12896dc 7ec0: c1289700 c1289600 00c8c000 c12896dc c1289700 c1289600 00c8c000 7ee0: c1347f54 c1347ef0 bf036b14 bf035e64 c12896e0 000a c1347f04 c0209bd4 7f00: c1347fbc be00 7e00 0001 00c8c000 00c88000 7f20: 0001 005f c12896dc c1289600 0001 005f c12896dc 7f40: c1346018 c1347fbc c1347f58 bf038ce8 bf0368d8 bf03a316 bf03a29f 7f60: 0015 c127ea80 c1347f8c c1347f78 c02349c8 c1289604 c13207e0 c1320540 7f80: c1347fac c1347f90 c03f2fc0 c02365d0 c1337e00 c1337e00 c1289600 bf037bc0 7fa0: 0013 c1347ff4 c1347fc0 c022f8f4 bf037bd0 7fc0: c1337e00 c1289600 c1347fd0 c1347fd0 c1337e00 7fe0: c022f860 c02191c8 c1347ff8 c02191c8 c022f870 Backtrace: [] (th, wValue, wIndex; unsigned int rdata, rdata1; // setup data valid val = readl(dev->base_addr + 0+0xa1c/0x199c [kagen2_udc]) from [] (bulk_in_complete+0x24c/0x1010 [g_file_storage
Re: undefined reference to `usb_gadget_unmap_request' regression
Hi, On Fri, May 10, 2013 at 8:35 PM, Vivek Gautam wrote: > Hi Felipe, > > > On Thu, May 9, 2013 at 8:18 PM, Felipe Balbi wrote: >> Hi, >> >> On Wed, May 08, 2013 at 10:04:56PM +0530, Vivek Gautam wrote: >>> Commit 388e5c51135f817f01177c42261f1116a6d7f2ad usb: dwc3: remove >>> dwc3 dependency on host AND gadget >>> by me breaks compilation when USB_DWC3=y, USB=y but USB_GADGET=m, >>> additionally when either of USB_DWC3_GADGET=y or USB_DWC3_DUAL_ROLE=y >>> :-( >>> >>> I had some confusion with this actually. >>> >>> We started with making USB_DWC3 independent of USB and USB_GADGET. >>> Thereby added some helping configs USB_DWC3_HOST, USB_DWC3_GADGET and >>> USB_DWC3_DUAL_ROLE. >>> Now when USB_GADGET=m and USB=y; USB_DWC3=y >>> Now, USB_DWC3_DUAL_ROLE=y or USB_DWC3_GADGET=y will make things >>> worse, since it will compile >>> dwc3/gadget.c and dwc3/ep0.c >>> This lets the compilation to break since usb/gadget/** is still >>> compiling as module. >>> But the errors it throws for functions are already exported with >>> EXPORT_SYMBOL_GPL(), then why these errors then. >>> >>> A possible flaw with this could be that "USB_DWC3_HOST, >>> USB_DWC3_GADGET and USB_DWC3_DUAL_ROLE" >>> are still 'bool' type configs. But making this as 'tristate' too >>> doesn't makes our life easier. >>> Ideally dwc3/gadget.c and dwc3/ep0.c should be compiling as modules >>> when USB_GADGET=m. >>> But then we have functions like dwc3_host_init() and >>> dwc3_gadget_init() and other similar functions in dwc3/core.c which >>> give >>> compilation break. >>> >>> Help please :-) >> >> I guess we need to make USB_GADGET bool then. udc-core would always be >> built-in or not built at all. Can you see if that helps ? > > True, this definitely helps. Thanks !! > But i was worrying do we really want to do this :-( > > I could come with below change too (although seems a bit silly, adding > a dummy config and stuff :-( ) Sorry for such a bad solution here. This definitely is not the right way. I could use Peter's suggestion to get to a solution. My following mail will contain a patch to that. -- Best Regards Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: Fix compilation break when building with USB_DWC3_DUAL_ROLE=y
The commit: 388e5c5 usb: dwc3: remove dwc3 dependency on host AND gadget breaks compilation when USB=y, USB_GADGET=m, USB_DWC3=y and USB_DWC3_DUAL_ROLE=y. drivers/built-in.o: In function `dwc3_gadget_giveback': drivers/usb/dwc3/gadget.c:271: undefined reference to `usb_gadget_unmap_request' drivers/built-in.o: In function `__dwc3_gadget_kick_transfer': drivers/usb/dwc3/gadget.c:1005: undefined reference to `usb_gadget_unmap_request' drivers/built-in.o: In function `__dwc3_gadget_ep_queue': drivers/usb/dwc3/gadget.c:1073: undefined reference to `usb_gadget_map_request' drivers/built-in.o: In function `dwc3_gadget_reset_interrupt': drivers/usb/dwc3/gadget.c:2165: undefined reference to `usb_gadget_set_state' drivers/built-in.o: In function `dwc3_gadget_init': drivers/usb/dwc3/gadget.c:2647: undefined reference to `usb_add_gadget_udc' drivers/built-in.o: In function `dwc3_gadget_exit': drivers/usb/dwc3/gadget.c:2681: undefined reference to `usb_del_gadget_udc' drivers/built-in.o: In function `__dwc3_ep0_do_control_data': drivers/usb/dwc3/ep0.c:929: undefined reference to `usb_gadget_map_request' drivers/usb/dwc3/ep0.c:906: undefined reference to `usb_gadget_map_request' drivers/built-in.o: In function `dwc3_ep0_set_config': drivers/usb/dwc3/ep0.c:575: undefined reference to `usb_gadget_set_state' drivers/built-in.o: In function `dwc3_ep0_set_address': drivers/usb/dwc3/ep0.c:520: undefined reference to `usb_gadget_set_state' drivers/usb/dwc3/ep0.c:522: undefined reference to `usb_gadget_set_state' drivers/built-in.o: In function `dwc3_ep0_set_config': drivers/usb/dwc3/ep0.c:556: undefined reference to `usb_gadget_set_state' Making changes similar to patch: 71a5e61 usb: chipidea: fix and improve dependencies if usb host or gadget support is built as module Let us limit the DWC3 mode to depend on corresponding usb-subsystem and USB_DWC3. Signed-off-by: Vivek Gautam Cc: Felipe Balbi Cc: Fengguang Wu --- drivers/usb/dwc3/Kconfig |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index ea5ee9c..757aa18 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -19,21 +19,21 @@ choice config USB_DWC3_HOST bool "Host only mode" - depends on USB + depends on USB=y || USB=USB_DWC3 help Select this when you want to use DWC3 in host mode only, thereby the gadget feature will be regressed. config USB_DWC3_GADGET bool "Gadget only mode" - depends on USB_GADGET + depends on USB_GADGET=y || USB_GADGET=USB_DWC3 help Select this when you want to use DWC3 in gadget mode only, thereby the host feature will be regressed. config USB_DWC3_DUAL_ROLE bool "Dual Role mode" - depends on (USB && USB_GADGET) + depends on ((USB=y || USB=USB_DWC3) && (USB_GADGET=y || USB_GADGET=USB_DWC3)) help This is the default mode of working of DWC3 controller where both host and gadget features are enabled. -- 1.7.6.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] usb: chipidea: ci13xxx_imx: Let device core handle pinctrl
Fabio Estevam writes: > Since commit ab78029 (drivers/pinctrl: grab default handles from device core), > we can rely on device core for handling pinctrl. > > So remove devm_pinctrl_get_select_default() from the driver. I'm taking this patch into my tree unless there are objections. Regards, -- Alex -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
On Tue, 14 May 2013, victor yeo wrote: > Thanks. I will add more printk statements gradually. Now i discover if > i write to a large text file (> 48k) on USB gadget, linux will crash. > The full log of UDC and gadget driver when linux crashes, and > corresponding usbmon trace are attached. If these logs are not > helpful, i shall add more printk. Unfortunately, I can't get anything useful out of the UDC driver crash log. It looks like the crash occurred somewhere inside the do_write() routine. Perhaps within the call to start_transfer(), or perhaps within the vfs_write() call. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/8] usb/isp116x-hcd: take msecs_to_jiffies_timeout into use
Use msecs_to_jiffies_timeout instead of open-coding the same.
Signed-off-by: Imre Deak
---
drivers/usb/host/isp116x-hcd.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c
index b64e661..de5807a 100644
--- a/drivers/usb/host/isp116x-hcd.c
+++ b/drivers/usb/host/isp116x-hcd.c
@@ -620,7 +620,7 @@ static irqreturn_t isp116x_irq(struct usb_hcd *hcd)
to come out of suspend, it may take more
than 10ms for status bits to stabilize. */
mod_timer(&hcd->rh_timer, jiffies
- + msecs_to_jiffies(20) + 1);
+ + msecs_to_jiffies_timeout(20));
if (intstat & HCINT_RD) {
DBG(" remote wakeup\n");
usb_hcd_resume_root_hub(hcd);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
Hi, >> Thanks. I will add more printk statements gradually. Now i discover if >> i write to a large text file (> 48k) on USB gadget, linux will crash. >> The full log of UDC and gadget driver when linux crashes, and >> corresponding usbmon trace are attached. If these logs are not >> helpful, i shall add more printk. > > Unfortunately, I can't get anything useful out of the UDC driver crash > log. > > It looks like the crash occurred somewhere inside the do_write() > routine. Perhaps within the call to start_transfer(), or perhaps > within the vfs_write() call. > > Alan Stern > Just curious. The crash log shows the following UDC driver code which is responsible to receive endpoint 0 setup data. However, the host PC is sending SCSI_WRITE_10 command at the time of the crash. These two does not correlate, right? unsigned int rdata, rdata1; // setup data valid val = readl(dev->base_addr + 0+0xfb0/0x199c [kagen2_udc] thanks, victor -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
On Tue, 14 May 2013, victor yeo wrote: > Just curious. The crash log shows the following UDC driver code which > is responsible to receive endpoint 0 setup data. However, the host PC > is sending SCSI_WRITE_10 command at the time of the crash. These two > does not correlate, right? > > unsigned int rdata, rdata1; > > // setup data valid > val = readl(dev->base_addr + 0+0xfb0/0x199c [kagen2_udc] The crash log should not include any driver code. What data were you writing to the gadget when the crash occurred? Were you sending the source code for the UDC driver? If you were, maybe that data got written to a wrong area in memory. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: udc: configure iso endpoints
The implementation is derived from the fsl_udc_core code in
fsl_ep_enable and makes basic iso handling possible.
Signed-off-by: Michael Grzeschik
Signed-off-by: Marc Kleine-Budde
Reviewed-by: Felipe Balbi
---
drivers/usb/chipidea/core.c | 2 +-
drivers/usb/chipidea/udc.c | 19 ++-
drivers/usb/chipidea/udc.h | 1 +
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 450107e..3cdb889 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -43,7 +43,7 @@
*
* TODO List
* - OTG
- * - Isochronous & Interrupt Traffic
+ * - Interrupt Traffic
* - Handle requests which spawns into several TDs
* - GET_STATUS(device) - always reports 0
* - Gadget API (majority of optional features)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 3d90e61..84f5ec0 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
struct ci13xxx_req *mReq)
mEp->qh.ptr->td.token &=
cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
+ if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
+ u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
+ if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
+ mul++;
+ mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
+ }
+
wmb(); /* synchronize before ep prime */
ret = hw_ep_prime(ci, mEp->num, mEp->dir,
@@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request
*req,
}
}
+ if (usb_endpoint_xfer_isoc(mEp->ep.desc)
+ && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
+ dev_err(mEp->ci->dev, "request length to big for
isochronous\n");
+ return -EMSGSIZE;
+ }
+
/* first nuke then test link, e.g. previous status has not sent */
if (!list_empty(&mReq->queue)) {
dev_err(mEp->ci->dev, "request already in queue\n");
@@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
mEp->num = usb_endpoint_num(desc);
mEp->type = usb_endpoint_type(desc);
- mEp->ep.maxpacket = usb_endpoint_maxp(desc);
+ mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
+ mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
cap |= QH_IOS;
@@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
if (ep == NULL || mEp->ep.desc == NULL)
return -EINVAL;
+ if (usb_endpoint_xfer_isoc(mEp->ep.desc))
+ return -EOPNOTSUPP;
+
spin_lock_irqsave(mEp->lock, flags);
#ifndef STALL_IN
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index d12e8b5..a75724a 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -50,6 +50,7 @@ struct ci13xxx_qh {
#define QH_MAX_PKT(0x07FFUL << 16)
#define QH_ZLTBIT(29)
#define QH_MULT (0x0003UL << 30)
+#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
/* 1 */
u32 curr;
/* 2 - 8 */
--
1.8.2.rc2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: udc: configure iso endpoints
Hi,
On Tue, May 14, 2013 at 07:22:56PM +0200, Michael Grzeschik wrote:
> The implementation is derived from the fsl_udc_core code in
> fsl_ep_enable and makes basic iso handling possible.
>
> Signed-off-by: Michael Grzeschik
> Signed-off-by: Marc Kleine-Budde
> Reviewed-by: Felipe Balbi
> ---
> drivers/usb/chipidea/core.c | 2 +-
> drivers/usb/chipidea/udc.c | 19 ++-
> drivers/usb/chipidea/udc.h | 1 +
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 450107e..3cdb889 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -43,7 +43,7 @@
> *
> * TODO List
> * - OTG
> - * - Isochronous & Interrupt Traffic
> + * - Interrupt Traffic
> * - Handle requests which spawns into several TDs
> * - GET_STATUS(device) - always reports 0
> * - Gadget API (majority of optional features)
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 3d90e61..84f5ec0 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
> struct ci13xxx_req *mReq)
> mEp->qh.ptr->td.token &=
> cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
>
> + if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
> + u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
> + if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
> + mul++;
> + mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
> + }
> +
> wmb(); /* synchronize before ep prime */
>
> ret = hw_ep_prime(ci, mEp->num, mEp->dir,
> @@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct
> usb_request *req,
> }
> }
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc)
> + && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
> + dev_err(mEp->ci->dev, "request length to big for
> isochronous\n");
> + return -EMSGSIZE;
> + }
> +
> /* first nuke then test link, e.g. previous status has not sent */
> if (!list_empty(&mReq->queue)) {
> dev_err(mEp->ci->dev, "request already in queue\n");
> @@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
> mEp->num = usb_endpoint_num(desc);
> mEp->type = usb_endpoint_type(desc);
>
> - mEp->ep.maxpacket = usb_endpoint_maxp(desc);
> + mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
> + mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
>
> if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
> cap |= QH_IOS;
> @@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
> if (ep == NULL || mEp->ep.desc == NULL)
> return -EINVAL;
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc))
> + return -EOPNOTSUPP;
> +
> spin_lock_irqsave(mEp->lock, flags);
>
> #ifndef STALL_IN
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index d12e8b5..a75724a 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -50,6 +50,7 @@ struct ci13xxx_qh {
> #define QH_MAX_PKT(0x07FFUL << 16)
> #define QH_ZLTBIT(29)
> #define QH_MULT (0x0003UL << 30)
> +#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> /* 1 */
> u32 curr;
> /* 2 - 8 */
> --
> 1.8.2.rc2
Please ignore that patch. I have to reword the patch description and
the remove the history.
Thanks,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: udc: configure iso endpoints
This patch adds iso endpoint support to the device controller.
It makes use of the multiplication bits in the maxpacket field
of the endpoint and calculates the multiplier bits for each
transfer description on every request.
Signed-off-by: Michael Grzeschik
---
drivers/usb/chipidea/core.c | 2 +-
drivers/usb/chipidea/udc.c | 19 ++-
drivers/usb/chipidea/udc.h | 1 +
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 450107e..3cdb889 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -43,7 +43,7 @@
*
* TODO List
* - OTG
- * - Isochronous & Interrupt Traffic
+ * - Interrupt Traffic
* - Handle requests which spawns into several TDs
* - GET_STATUS(device) - always reports 0
* - Gadget API (majority of optional features)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 3d90e61..84f5ec0 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
struct ci13xxx_req *mReq)
mEp->qh.ptr->td.token &=
cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
+ if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
+ u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
+ if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
+ mul++;
+ mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
+ }
+
wmb(); /* synchronize before ep prime */
ret = hw_ep_prime(ci, mEp->num, mEp->dir,
@@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request
*req,
}
}
+ if (usb_endpoint_xfer_isoc(mEp->ep.desc)
+ && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
+ dev_err(mEp->ci->dev, "request length to big for
isochronous\n");
+ return -EMSGSIZE;
+ }
+
/* first nuke then test link, e.g. previous status has not sent */
if (!list_empty(&mReq->queue)) {
dev_err(mEp->ci->dev, "request already in queue\n");
@@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
mEp->num = usb_endpoint_num(desc);
mEp->type = usb_endpoint_type(desc);
- mEp->ep.maxpacket = usb_endpoint_maxp(desc);
+ mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
+ mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
cap |= QH_IOS;
@@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
if (ep == NULL || mEp->ep.desc == NULL)
return -EINVAL;
+ if (usb_endpoint_xfer_isoc(mEp->ep.desc))
+ return -EOPNOTSUPP;
+
spin_lock_irqsave(mEp->lock, flags);
#ifndef STALL_IN
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index d12e8b5..a75724a 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -50,6 +50,7 @@ struct ci13xxx_qh {
#define QH_MAX_PKT(0x07FFUL << 16)
#define QH_ZLTBIT(29)
#define QH_MULT (0x0003UL << 30)
+#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
/* 1 */
u32 curr;
/* 2 - 8 */
--
1.8.2.rc2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: udc: configure iso endpoints
Hello.
On 05/14/2013 09:31 PM, Michael Grzeschik wrote:
This patch adds iso endpoint support to the device controller.
It makes use of the multiplication bits in the maxpacket field
of the endpoint and calculates the multiplier bits for each
transfer description on every request.
Signed-off-by: Michael Grzeschik
[...]
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 450107e..3cdb889 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -43,7 +43,7 @@
*
* TODO List
* - OTG
- * - Isochronous & Interrupt Traffic
+ * - Interrupt Traffic
* - Handle requests which spawns into several TDs
* - GET_STATUS(device) - always reports 0
* - Gadget API (majority of optional features)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 3d90e61..84f5ec0 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
struct ci13xxx_req *mReq)
mEp->qh.ptr->td.token &=
cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
+ if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
+ u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
Empty line wouldn't hurt here, after declaration.
+ if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
+ mul++;
+ mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
+ }
+
wmb(); /* synchronize before ep prime */
ret = hw_ep_prime(ci, mEp->num, mEp->dir,
@@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request
*req,
}
}
+ if (usb_endpoint_xfer_isoc(mEp->ep.desc)
+ && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
Somewhat better style would be to leave the operator on a previous
line.
+ dev_err(mEp->ci->dev, "request length to big for
isochronous\n");
s/to/too/
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: fix Kconfig logic for USB_UHCI_HCD
The Kconfig settings for uhci-hcd are too permissive; they allow the driver to be built without any bus-glue modules configured (USB_UHCI_HCD enabled, PCI disabled, SPARC_LEON disabled, ARCH_VT8500 enabled, and USB_UHCI_PLATFORM disabled). This patch fixes the problem by rearranging the dependencies. Now the platform-dependent config options don't depend on USB_UHCI_HCD; instead it depends on them. Furthermore, there is no user-selectable choice as to which glue modules will be built. If USB_UHCI_HCD is enabled then all applicable bus glues will be built. Signed-off-by: Alan Stern CC: Arnd Bergmann --- [as1679] drivers/usb/host/Kconfig | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) Index: usb-3.9/drivers/usb/host/Kconfig === --- usb-3.9.orig/drivers/usb/host/Kconfig +++ usb-3.9/drivers/usb/host/Kconfig @@ -507,7 +507,7 @@ endif # USB_OHCI_HCD config USB_UHCI_HCD tristate "UHCI HCD (most Intel and VIA) support" - depends on PCI || SPARC_LEON || ARCH_VT8500 + depends on PCI || USB_UHCI_SUPPORT_NON_PCI_HC ---help--- The Universal Host Controller Interface is a standard by Intel for accessing the USB hardware in the PC (which is also called the USB @@ -524,26 +524,19 @@ config USB_UHCI_HCD config USB_UHCI_SUPPORT_NON_PCI_HC bool - depends on USB_UHCI_HCD - default y if (SPARC_LEON || ARCH_VT8500) + default y if (SPARC_LEON || USB_UHCI_PLATFORM) config USB_UHCI_PLATFORM - bool "Generic UHCI Platform Driver support" - depends on USB_UHCI_SUPPORT_NON_PCI_HC + bool default y if ARCH_VT8500 - ---help--- - Enable support for generic UHCI platform devices that require no - additional configuration. config USB_UHCI_BIG_ENDIAN_MMIO bool - depends on USB_UHCI_SUPPORT_NON_PCI_HC && SPARC_LEON - default y + default y if SPARC_LEON config USB_UHCI_BIG_ENDIAN_DESC bool - depends on USB_UHCI_SUPPORT_NON_PCI_HC && SPARC_LEON - default y + default y if SPARC_LEON config USB_FHCI_HCD tristate "Freescale QE USB Host Controller support" -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: UHCI: fix for suspend of virtual HP controller
HP's virtual UHCI host controller takes a long time to suspend (several hundred microseconds), even when no devices are attached. This provokes a warning message from uhci-hcd in the auto-stop case. To prevent this from happening, this patch adds a test to avoid performing an auto-stop when the wait_for_hp quirk flag is set. The controller will still suspend through the normal runtime PM mechanism. And since that pathway includes a 1-ms delay, the slowness of the virtual hardware won't matter. Signed-off-by: Alan Stern Reported-and-tested-by: ZhenHua CC: --- [as1680] drivers/usb/host/uhci-hub.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: usb-3.9/drivers/usb/host/uhci-hub.c === --- usb-3.9.orig/drivers/usb/host/uhci-hub.c +++ usb-3.9/drivers/usb/host/uhci-hub.c @@ -225,7 +225,8 @@ static int uhci_hub_status_data(struct u /* auto-stop if nothing connected for 1 second */ if (any_ports_active(uhci)) uhci->rh_state = UHCI_RH_RUNNING; - else if (time_after_eq(jiffies, uhci->auto_stop_time)) + else if (time_after_eq(jiffies, uhci->auto_stop_time) && + !uhci->wait_for_hp) suspend_rh(uhci, UHCI_RH_AUTO_STOPPED); break; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: fix latency in uhci-hcd and ohci-hcd
Commits c44b225077bb1fb25ed5cd5c4f226897b91bedd4 (UHCI: implement new
semantics for URB_ISO_ASAP) and
6a41b4d3fe8cd4cc95181516fc6fba7b1747a27c (OHCI: implement new
semantics for URB_ISO_ASAP) increased the latency for isochronous URBs
in uhci-hcd and ohci-hcd respectively to 2 milliseconds, in an
attempt to avoid underruns. It turns out that not only was this
unnecessary -- 1-ms latency works okay -- it also causes problems with
certain application loads such as real-time audio.
This patch changes the latency for both drivers back to 1 ms.
This should be applied to -stable kernels going back to 3.8.
Signed-off-by: Alan Stern
Reported-and-tested-by: Joe Rayhawk
CC: Clemens Ladisch
CC:
---
[as1681]
drivers/usb/host/ohci-hcd.c |2 +-
drivers/usb/host/uhci-q.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Index: usb-3.9/drivers/usb/host/ohci-hcd.c
===
--- usb-3.9.orig/drivers/usb/host/ohci-hcd.c
+++ usb-3.9/drivers/usb/host/ohci-hcd.c
@@ -233,7 +233,7 @@ static int ohci_urb_enqueue (
urb->start_frame = frame;
}
} else if (ed->type == PIPE_ISOCHRONOUS) {
- u16 next = ohci_frame_no(ohci) + 2;
+ u16 next = ohci_frame_no(ohci) + 1;
u16 frame = ed->last_iso + ed->interval;
/* Behind the scheduling threshold? */
Index: usb-3.9/drivers/usb/host/uhci-q.c
===
--- usb-3.9.orig/drivers/usb/host/uhci-q.c
+++ usb-3.9/drivers/usb/host/uhci-q.c
@@ -1287,7 +1287,7 @@ static int uhci_submit_isochronous(struc
return -EINVAL; /* Can't change the period */
} else {
- next = uhci->frame_number + 2;
+ next = uhci->frame_number + 1;
/* Find the next unused frame */
if (list_empty(&qh->queue)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: OHCI: fix logic for scheduling isochronous URBs
The isochronous scheduling logic in ohci-hcd has a bug. The
calculation for skipping TDs that are too late should be carried out
only in the !URB_ISO_ASAP case. When URB_ISO_ASAP is set, the URB is
pushed back so that none of the TDs are too late, which would cause
the calculation to overflow.
The patch also fixes the calculation to avoid overflow in the case
where the frame value wraps around.
This should be applied to -stable kernels going back to 3.8.
Signed-off-by: Alan Stern
CC:
---
[as1682]
drivers/usb/host/ohci-hcd.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
Index: usb-3.9/drivers/usb/host/ohci-hcd.c
===
--- usb-3.9.orig/drivers/usb/host/ohci-hcd.c
+++ usb-3.9/drivers/usb/host/ohci-hcd.c
@@ -240,7 +240,7 @@ static int ohci_urb_enqueue (
if (unlikely(tick_before(frame, next))) {
/* USB_ISO_ASAP: Round up to the first available slot */
- if (urb->transfer_flags & URB_ISO_ASAP)
+ if (urb->transfer_flags & URB_ISO_ASAP) {
frame += (next - frame + ed->interval - 1) &
-ed->interval;
@@ -248,21 +248,25 @@ static int ohci_urb_enqueue (
* Not ASAP: Use the next slot in the stream. If
* the entire URB falls before the threshold, fail.
*/
- else if (tick_before(frame + ed->interval *
+ } else {
+ if (tick_before(frame + ed->interval *
(urb->number_of_packets - 1), next)) {
- retval = -EXDEV;
- usb_hcd_unlink_urb_from_ep(hcd, urb);
- goto fail;
- }
+ retval = -EXDEV;
+ usb_hcd_unlink_urb_from_ep(hcd, urb);
+ goto fail;
+ }
- /*
-* Some OHCI hardware doesn't handle late TDs
-* correctly. After retiring them it proceeds to
-* the next ED instead of the next TD. Therefore
-* we have to omit the late TDs entirely.
-*/
- urb_priv->td_cnt = DIV_ROUND_UP(next - frame,
- ed->interval);
+ /*
+* Some OHCI hardware doesn't handle late TDs
+* correctly. After retiring them it proceeds
+* to the next ED instead of the next TD.
+* Therefore we have to omit the late TDs
+* entirely.
+*/
+ urb_priv->td_cnt = DIV_ROUND_UP(
+ (u16) (next - frame),
+ ed->interval);
+ }
}
urb->start_frame = frame;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 0/2] TI compliance mode fixes
On 05/09/2013 07:31 PM, Sarah Sharp wrote: > Hi Tony, Don, and Oliver, > > Can one of you double check these two patches resolve the resume from S3 > issue on effected HP systems with the TI compliance mode quirk? > > Oliver has confirmed that the first patch works, but no one has tested > the patch to disable D3cold for the xHCI host on those systems. Ying, > can you double check that the code to disable D3cold looks sane? > > If these patches work on the systems, I will send them out next week to > Greg. > > Thanks, > Sarah Sharp > Just a quick note. Both patches work correctly on 3.8. However, I'm experiencing a hang when returning from hibernate in 3.10-rc1. I will troubleshoot this tomorrow. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: chipidea: udc: configure iso endpoints
>
> This patch adds iso endpoint support to the device controller.
> It makes use of the multiplication bits in the maxpacket field
> of the endpoint and calculates the multiplier bits for each
> transfer description on every request.
>
> Signed-off-by: Michael Grzeschik
> ---
> drivers/usb/chipidea/core.c | 2 +-
> drivers/usb/chipidea/udc.c | 19 ++-
> drivers/usb/chipidea/udc.h | 1 +
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 450107e..3cdb889 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -43,7 +43,7 @@
> *
> * TODO List
> * - OTG
> - * - Isochronous & Interrupt Traffic
> + * - Interrupt Traffic
> * - Handle requests which spawns into several TDs
> * - GET_STATUS(device) - always reports 0
> * - Gadget API (majority of optional features)
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 3d90e61..84f5ec0 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -466,6 +466,13 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp,
> struct ci13xxx_req *mReq)
> mEp->qh.ptr->td.token &=
> cpu_to_le32(~(TD_STATUS_HALTED|TD_STATUS_ACTIVE));
>
> + if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
> + u32 mul = mReq->req.length >> __ffs(mEp->ep.maxpacket);
> + if (mReq->req.length & ~(mul << __ffs(mEp->ep.maxpacket)))
> + mul++;
> + mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
> + }
> +
I think it is a little hard to read, why not
if (mEp->type == USB_ENDPOINT_XFER_ISOC) {
u32 mul = mReq->req.length / mEp->ep.maxpacket;
if (mReq->req.length % mEp->ep.maxpacket)
mul++;
mEp->qh.ptr->cap |= mul << __ffs(QH_MULT);
}
> wmb(); /* synchronize before ep prime */
>
> ret = hw_ep_prime(ci, mEp->num, mEp->dir,
> @@ -678,6 +685,12 @@ static int _ep_queue(struct usb_ep *ep, struct
> usb_request *req,
> }
> }
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc)
> + && mReq->req.length > (1 + mEp->ep.mult) * mEp->ep.maxpacket) {
> + dev_err(mEp->ci->dev, "request length to big for
> isochronous\n");
> + return -EMSGSIZE;
> + }
> +
typo, "too big"
the request length should be less than 3*1024.
> /* first nuke then test link, e.g. previous status has not sent */
> if (!list_empty(&mReq->queue)) {
> dev_err(mEp->ci->dev, "request already in queue\n");
> @@ -1060,7 +1073,8 @@ static int ep_enable(struct usb_ep *ep,
> mEp->num = usb_endpoint_num(desc);
> mEp->type = usb_endpoint_type(desc);
>
> - mEp->ep.maxpacket = usb_endpoint_maxp(desc);
> + mEp->ep.maxpacket = usb_endpoint_maxp(desc) & 0x07ff;
> + mEp->ep.mult = QH_ISO_MULT(usb_endpoint_maxp(desc));
>
The above change doesn't need.
maxpacket <= 1024 for high speed
maxpacket <= 1023 for full speed
For high speed high bandwidth, it just has two or three transactions per
microframe
> if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
> cap |= QH_IOS;
> @@ -1246,6 +1260,9 @@ static int ep_set_halt(struct usb_ep *ep, int value)
> if (ep == NULL || mEp->ep.desc == NULL)
> return -EINVAL;
>
> + if (usb_endpoint_xfer_isoc(mEp->ep.desc))
> + return -EOPNOTSUPP;
> +
> spin_lock_irqsave(mEp->lock, flags);
>
> #ifndef STALL_IN
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index d12e8b5..a75724a 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -50,6 +50,7 @@ struct ci13xxx_qh {
> #define QH_MAX_PKT(0x07FFUL << 16)
> #define QH_ZLTBIT(29)
> #define QH_MULT (0x0003UL << 30)
> +#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> /* 1 */
> u32 curr;
> /* 2 - 8 */
> --
> 1.8.2.rc2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 0/2] TI compliance mode fixes
On Tue, May 14, 2013 at 03:49:40PM -0400, Tony Camuso wrote: > > On 05/09/2013 07:31 PM, Sarah Sharp wrote: > > Hi Tony, Don, and Oliver, > > > > Can one of you double check these two patches resolve the resume from S3 > > issue on effected HP systems with the TI compliance mode quirk? > > > > Oliver has confirmed that the first patch works, but no one has tested > > the patch to disable D3cold for the xHCI host on those systems. Ying, > > can you double check that the code to disable D3cold looks sane? > > > > If these patches work on the systems, I will send them out next week to > > Greg. > > > > Thanks, > > Sarah Sharp > > > > Just a quick note. > > Both patches work correctly on 3.8. > > However, I'm experiencing a hang when returning from hibernate in > 3.10-rc1. > > I will troubleshoot this tomorrow. Thanks for letting me know Tony! Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: Consolidate XHCI TD Size calculation
On Thu, May 09, 2013 at 09:42:51PM -0700, Julius Werner wrote:
> The TD Size field in an XHCI TRB is calculated in two different ways,
> depending on the XHCI version. The current code does this version check
> in every caller, which makes it easy to forget (such as in the function
> for control transfers, which still erroneously uses the old calculation
> for its data stage TRB). The 1.0 calculation has also been corrected and
> amended several times, growing slightly more complicated than necessary.
Does this patch fix the control transfer data stage issue? Please break
this into a bug fix for that, with your refactoring on top of that. We
need to backport the bug fix to the stable kernels, and queue the
refactoring for 3.11.
Sarah Sharp
>
> This patch moves the XHCI version check into a single, unified (and
> slightly simplified) xhci_td_remainder() function, which deduplicates
> code and looks cleaner in the callers.
>
> Signed-off-by: Julius Werner
> ---
> drivers/usb/host/xhci-ring.c | 111
> +++
> 1 file changed, 29 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 1969c00..32629b2 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3074,54 +3074,33 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t
> mem_flags,
> }
>
> /*
> - * The TD size is the number of bytes remaining in the TD (including this
> TRB),
> - * right shifted by 10.
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> - */
> -static u32 xhci_td_remainder(unsigned int remainder)
> -{
> - u32 max = (1 << (21 - 17 + 1)) - 1;
> -
> - if ((remainder >> 10) >= max)
> - return max << 17;
> - else
> - return (remainder >> 10) << 17;
> -}
> -
> -/*
> - * For xHCI 1.0 host controllers, TD size is the number of max packet sized
> - * packets remaining in the TD (*not* including this TRB).
> - *
> - * Total TD packet count = total_packet_count =
> - * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
> + * Calculate the TD Size field of a TRB, shifted to its correct position
> + * in bits 21:17. In pre-1.0 XHCI versions, this field contains the
> + * number of bytes left in the TD, including the current TRB, in kB.
> *
> - * Packets transferred up to and including this TRB = packets_transferred =
> - * rounddown(total bytes transferred including this TRB / wMaxPacketSize)
> + * In XHCI 1.0, it contains the amount of packets left in the TD,
> + * excluding the current TRB. Even though the spec explains this in a
> + * roundabout way, the end result is just the amount of bytes left after
> + * this TRB divided by the maximum packet size, rounded up. For the last
> + * TRB this is naturally zero since there cannot be any more bytes left.
> *
> - * TD size = total_packet_count - packets_transferred
> - *
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> - * The last TRB in a TD must have the TD size set to zero.
> + * @running_total: amount of bytes in the TD before this TRB
> + * @trb_len: amount of bytes in the current TRB
> + * @total_len: total amount of bytes in the TD
> */
> -static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
> - unsigned int total_packet_count, struct urb *urb,
> - unsigned int num_trbs_left)
> +static u32 xhci_td_remainder(int running_total, int trb_len, int total_len,
> + struct urb *urb)
> {
> - int packets_transferred;
> + u32 result;
>
> - /* One TRB with a zero-length data packet. */
> - if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> - return 0;
> -
> - /* All the TRB queueing functions don't count the current TRB in
> - * running_total.
> - */
> - packets_transferred = (running_total + trb_buff_len) /
> - GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc));
> + if (hcd_to_xhci(bus_to_hcd(urb->dev->bus))->hci_version < 0x100)
> + result = (total_len - running_total) >> 10;
> + else
> + result = DIV_ROUND_UP(total_len - running_total - trb_len,
> + GET_MAX_PACKET(usb_endpoint_maxp(&urb->ep->desc)));
>
> - if ((total_packet_count - packets_transferred) > 31)
> - return 31 << 17;
> - return (total_packet_count - packets_transferred) << 17;
> + /* TD size field is only 5 bit wide, so cap this at 31 */
> + return min(result, 31u) << 17;
> }
>
> static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> @@ -3134,7 +3113,6 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
> gfp_t mem_flags,
> struct scatterlist *sg;
> int num_sgs;
> int trb_buff_len, this_sg_len, running_total;
> - unsigned int total_packet_count;
> bool first_trb;
> u64 addr;
> bool more_trbs_coming;
> @@ -3148,8 +3126,6 @@
Re: [OPW kernel] xhci.c: dma_set_mask
On Wed, May 15, 2013 at 04:56:00AM +0300, Xenia Ragiadakou wrote: > Hi Sarah, Hi Ksenia! > In xhci.c line 4665, the function dma_set_mask is used to enable > 64-bit DMA addressing > (if i understood correctly). > > However, accordingly to the Documentation/DMA-API-HOWTO.txt, there > is possibility > that the platform cannot support the DMA addressing of the device. > So the return value of dma_set_mask needs to be tested if it equals > zero and if not > go to error label. Am I right? > > Can we assume that 64 bit or 32 bit DMA addressing will always be supported? > This is the first time i come through these stuff so my intuition is > limited. All xHCI hosts are required to support DMA, by the xHCI specification. You just can't get USB 3.0 speeds without DMA. Or are you concerned that the xHCI host will support 64-bit DMA, but the system only supports 32-bit DMA? Because that would be a question for Alan Stern, who has worked on the USB core for far longer than I have. The xHCI PCI driver sets the hc_driver->flag for HCD_MEMORY, which means the USB host controller requires DMA. In drivers/usb/core/hcd-pci.c (which is what the xHCI PCI driver calls into in its probe function), the USB core will fail the init if the device doesn't support DMA. Separately from the PCI probe function, the xHCI driver in xhci.c:4665 tests whether the host supports 32-bit DMA or 64-bit DMA. This is an xHCI-specific DMA capability, not a generic PCI DMA capability. Does that answer your questions? Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OPW kernel] dma_set_coherent_mask
On Wed, May 15, 2013 at 05:41:12AM +0300, Xenia Ragiadakou wrote:
> Hi Sarah, (again)
>
> Also I noticed that dma_set_coherent_mask() is not called somewhere,
> which according to DMA-API-HOWTO.txt, means the even if the DMA mask
> is set to 64 bits by dma_set_mask, dma_alloc_coherent and
> dma_pool_alloc wont return 64 bit addresses (32 MSbits wont be
> addressed).
Another good question for Alan, and the USB and PCI list.
(Alan, Ksenia is one of the applicants for the FOSS Outreach Program for
Women that I've been coordinating: http://kernelnewbies.org/OPWIntro)
We do allocate memory using DMA pools, and we do want 64-bit context
addresses if the xHCI host controller can handle it.
The xHCI driver calls dma_set_mask, but not dma_set_coherent_mask():
temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params);
if (HCC_64BIT_ADDR(temp)) {
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64));
} else {
dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
}
Alan, should it be calling dma_set_coherent_mask()? I think I may have
noticed the context addresses were never 64-bit addresses, but I didn't
think to look whether the host supported 64-bit addresses. I just
assumed it could only handle 32-bit addresses.
> I hope I'm not doing a huge mistake in my reasonance, and you loose
> your time with these emails.
Nope, I want you to ask questions, so don't worry about that.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OPW kernel] xhci.c: dma_set_mask
[Please leave Alan and the Linux USB mailing list Cc'ed, since I asked
for Alan's advice on this.]
On Wed, May 15, 2013 at 07:50:39AM +0300, Xenia Ragiadakou wrote:
> I will try to clarify more my question.
>
> In xhci.c: 4665 states that:
>
> if (HCC_64BIT_ADDR(temp)) {
> xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
> dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64));
> } else {
> dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
> }
> }
>
> Is there a possibility for the dma_set_mask function to fail,
> so that it will be necessary to check for this condition :
>
> if (HCC_64BIT_ADDR(temp)) {
> xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
> if (dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)))
>goto error;
> } else {
> if (dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32)))
> goto error;
> }
> }
In theory, yes, dma_set_mask() could fail. It would have to be a pretty
broken xHCI host that indicates it supports 64-bits when it doesn't.
Perhaps in the 64-bit case, you want to try to enable 32-bit DMA on
failure? If that fails, then bail out.
Since the DMA setup code is copy-pasted twice in xhci_gen_setup(),
perhaps you want to refactor the DMA setup into its own function as a
separate patch on top of your bug fix?
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: OMAP: Add usb_otg and glue data to OMAP3+ boards
On Saturday 11 May 2013 08:05 AM, Tony Lindgren wrote:
Commit ad871c10 (ARM: dts: OMAP: Add usb_otg and glue data to
OMAP3+ boards) added support for MUSB on omap3 for device tree,
but added the interrupts the wrong way probably as they were
copied from the omap4.dtsi file. On omap3 we have TI specific
interrupt controller, not GIC.
Fix this by specifying the interrupt following the TI INTC
binding.
Without this fix MUSB won't work as it is trying to use
irq0 instead of irq92.
Signed-off-by: Tony Lindgren
Tested in beagleboard-xm
Tested-by: Kishon Vijay Abraham I
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -516,7 +516,7 @@
usb_otg_hs: usb_otg_hs@480ab000 {
compatible = "ti,omap3-musb";
reg = <0x480ab000 0x1000>;
- interrupts = <0 92 0x4>, <0 93 0x4>;
+ interrupts = <92>, <93>;
interrupt-names = "mc", "dma";
ti,hwmods = "usb_otg_hs";
multipoint = <1>;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
