Re: Linux USB file storage gadget with new UDC
Hi, The problem is in UDC driver. i made the change, it is ok now. Good. I noticed that the usb_ep_set_wedge routine still isn't working right. You might try to fix that. Alan Stern Ok, is the usb_ep_set_wedge routine not working? I can't see that in the log file. Now, in USB 2.0 CV test, there is an error about GET_STATUS request, as shown below. g_file_storage gadget: ep0-setup, length 8: : 82 00 00 00 81 00 02 00 g_file_storage gadget: unknown control req 82.00 v i0081 l2 handle_setup status -95 I think the GET_STATUS request is not handled by the gadget driver. Isn't it so? 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: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework
Hi, On Mon, Jun 24, 2013 at 11:01:56AM +0530, Kishon Vijay Abraham I wrote: @@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb) if (glue-status != OMAP_MUSB_UNKNOWN) omap_musb_set_mailbox(glue); - usb_phy_init(musb-xceiv); + phy_init(musb-phy); pm_runtime_put_noidle(musb-controller); + phy_pm_runtime_put(musb-phy); see, weird unbalanced calls :-) Make it all explicit: phy_pm_runtime_get_sync(phy); phy_init(phy); phy_pm_runtime_put(phy); I think then it makes sense to drop get_sync from phy_init()? maybe not, you don't know if the phy has already autosuspended or not. -- balbi signature.asc Description: Digital signature
Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework
On Wednesday 19 June 2013 02:52 AM, Sylwester Nawrocki wrote: Hi Kishon, I've noticed there is a little inconsistency between the code and documentation. On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote: +3. Creating the PHY + +The PHY driver should create the PHY in order for other peripheral controllers +to make use of it. The PHY framework provides 2 APIs to create the PHY. + +struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops, +void *priv); +struct phy *devm_phy_create(struct device *dev, int id, +const struct phy_ops *ops, void *priv); The 'label' argument is missing in those function prototypes. It seems the labels must be unique, I guess this needs to be documented. And do we allow NULL labels ? If so, then there is probably a check for NULL label needed in phy_lookup() and if not, then phy_create() should fail when NULL label is passed ? Yeah. Label is used only for non-dt case so NULL should be allowed while phy_create(). Thanks Kishon -- 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: Chipidea usb otg support for IMX/MXS (device functionality)
On Mon, Jun 24, 2013 at 01:37:59AM +, Chen Peter-B29397 wrote: Add shawn. Marek, have you tried mx23 evk? Shawn, marek reported the udc function at mx23 works abnormal, but it works good at mx28. Have you tried mx23 udc recently? Sorry, I never tried usb on my imx23-evk due to some setup problem on the board. Shawn -- 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: [RESEND PATCH v2 1/1] usb: fix build error without CONFIG_USB_PHY
Peter Chen peter.c...@freescale.com writes:
on i386:
drivers/built-in.o: In function `ci_hdrc_probe':
core.c:(.text+0x20446b): undefined reference to `of_usb_get_phy_mode'
Signed-off-by: Peter Chen peter.c...@freescale.com
Reported-by: Randy Dunlap rdun...@infradead.org
Acked-by: Randy Dunlap rdun...@infradead.org
It's actually Felipe's turf, so needs either his ack or target his tree.
I'm ok with this.
FWIW,
Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com
---
Changes for v2:
- Using IS_ENABLED to MACRO define
include/linux/usb/of.h | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index e460a24..a0ef405 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -10,19 +10,23 @@
#include linux/usb/otg.h
#include linux/usb/phy.h
-#ifdef CONFIG_OF
-enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+#if IS_ENABLED(CONFIG_OF)
enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np);
#else
-static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node
*np)
+static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
{
- return USBPHY_INTERFACE_MODE_UNKNOWN;
+ return USB_DR_MODE_UNKNOWN;
}
+#endif
-static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+#if IS_ENABLED(CONFIG_OF) IS_ENABLED(CONFIG_USB_PHY)
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+#else
+static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node
*np)
{
- return USB_DR_MODE_UNKNOWN;
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
}
+
#endif
#endif /* __LINUX_USB_OF_H */
--
1.7.0.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
[PATCH v2 0/4] USB: HCD/EHCI: giveback of URB in tasklet context
Hi, The patchset supports to run giveback of URB in tasklet context, so that DMA unmapping/mapping on transfer buffer and compelte() callback can be run with interrupt enabled, then time of HCD interrupt handler(IRQs disabled time) can be saved much. Also this approach may simplify HCD since HCD lock needn't be released any more before calling usb_hcd_giveback_urb(). The patchset enables the mechanism on EHCI HCD now. In the commit log of patch 4/4, detailed test result on three machines (ARM A9/A15 dual core, X86) are provided about transfer performance and ehci irq handling time. From the result, no transfer performance loss is found and ehci irq handling time drops much with the patchset. V2: - 1/4: always run URB complete() of root-hub in tasklet - 1/4: store urb status in urb-unlinked - 1/4: don't allocate 'struct giveback_urb_bh' dynamically - 1/4: other minor changes - 2/4: descript changes simply - 3/4: don't use QH_STATE_UNLINK_WAIT to implement intr qh unlink wait - 3/4: cancel unlink wait change - 4/4: merge HCD private lock changes - rebase on 3.10-rc7-next20130624 V1: - change percput tasklet into tasklet in HCD to avoid out of order of URB-complete() for same endpoint - disable local IRQs when calling complete() from tasklet to avoid deadlock which is caused by holding lock via spin_lock and the same lock might be acquired in hard irq context - document coming change about calling complete() with irq enabled so that we can start to clean up USB drivers which call spin_lock() in complete() Documentation/usb/URB.txt | 21 +++-- drivers/usb/core/hcd.c| 169 ++--- drivers/usb/host/ehci-fsl.c |2 +- drivers/usb/host/ehci-grlib.c |2 +- drivers/usb/host/ehci-hcd.c |3 +- drivers/usb/host/ehci-hub.c |1 + drivers/usb/host/ehci-mem.c |1 + drivers/usb/host/ehci-mv.c|2 +- drivers/usb/host/ehci-octeon.c|2 +- drivers/usb/host/ehci-pmcmsp.c|2 +- drivers/usb/host/ehci-ppc-of.c|2 +- drivers/usb/host/ehci-ps3.c |2 +- drivers/usb/host/ehci-q.c |5 -- drivers/usb/host/ehci-sched.c | 62 +- drivers/usb/host/ehci-sead3.c |2 +- drivers/usb/host/ehci-sh.c|2 +- drivers/usb/host/ehci-tilegx.c|2 +- drivers/usb/host/ehci-timer.c | 45 +- drivers/usb/host/ehci-w90x900.c |2 +- drivers/usb/host/ehci-xilinx-of.c |2 +- drivers/usb/host/ehci.h |3 + include/linux/usb/hcd.h | 17 22 files changed, 287 insertions(+), 64 deletions(-) Thanks, -- Ming Lei -- 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 1/4] USB: HCD: support giveback of URB in tasklet context
This patch implements the mechanism of giveback of URB in
tasklet context, so that hardware interrupt handling time for
usb host controller can be saved much, and HCD interrupt handling
can be simplified.
Motivations:
1), on some arch(such as ARM), DMA mapping/unmapping is a bit
time-consuming, for example: when accessing usb mass storage
via EHCI on pandaboard, the common length of transfer buffer is 120KB,
the time consumed on DMA unmapping may reach hundreds of microseconds;
even on A15 based box, the time is still about scores of microseconds
2), on some arch, reading DMA coherent memoery is very time-consuming,
the most common example is usb video class driver[1]
3), driver's complete() callback may do much things which is driver
specific, so the time is consumed unnecessarily in hardware irq context.
4), running driver's complete() callback in hardware irq context causes
that host controller driver has to release its lock in interrupt handler,
so reacquiring the lock after return may busy wait a while and increase
interrupt handling time. More seriously, releasing the HCD lock makes
HCD becoming quite complicated to deal with introduced races.
So the patch proposes to run giveback of URB in tasklet context, then
time consumed in HCD irq handling doesn't depend on drivers' complete and
DMA mapping/unmapping any more, also we can simplify HCD since the HCD
lock isn't needed to be released during irq handling.
The patch should be reasonable and doable:
1), for drivers, they don't care if the complete() is called in hard irq
context or softirq context
2), the biggest change is the situation in which usb_submit_urb() is called
in complete() callback, so the introduced tasklet schedule delay might be a
con, but it shouldn't be a big deal:
- control/bulk asynchronous transfer isn't sensitive to schedule
delay
- the patch schedules giveback of periodic URBs using
tasklet_hi_schedule, so the introduced delay should be very
small
- for ISOC transfer, generally, drivers submit several URBs
concurrently to avoid interrupt delay, so it is OK with the
little schedule delay.
- for interrupt transfer, generally, drivers only submit one URB
at the same time, but interrupt transfer is often used in event
report, polling, ... situations, and a little delay should be OK.
Considered that HCDs may optimize on submitting URB in complete(), the
patch may cause the optimization not working, so introduces one flag to mark
if the HCD supports to run giveback URB in tasklet context. When all HCDs
are ready, the flag can be removed.
[1], http://marc.info/?t=136438111600010r=1w=2
Cc: Oliver Neukum oli...@neukum.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ming Lei ming@canonical.com
---
drivers/usb/core/hcd.c | 169 ++-
include/linux/usb/hcd.h | 17 +
2 files changed, 156 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 014dc99..1fbd193 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -694,15 +694,7 @@ error:
/* any errors get returned through the urb completion */
spin_lock_irq(hcd_root_hub_lock);
usb_hcd_unlink_urb_from_ep(hcd, urb);
-
- /* This peculiar use of spinlocks echoes what real HC drivers do.
-* Avoiding calls to local_irq_disable/enable makes the code
-* RT-friendly.
-*/
- spin_unlock(hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, status);
- spin_lock(hcd_root_hub_lock);
-
spin_unlock_irq(hcd_root_hub_lock);
return 0;
}
@@ -742,9 +734,7 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
memcpy(urb-transfer_buffer, buffer, length);
usb_hcd_unlink_urb_from_ep(hcd, urb);
- spin_unlock(hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, 0);
- spin_lock(hcd_root_hub_lock);
} else {
length = 0;
set_bit(HCD_FLAG_POLL_PENDING, hcd-flags);
@@ -834,10 +824,7 @@ static int usb_rh_urb_dequeue(struct usb_hcd *hcd, struct
urb *urb, int status)
if (urb == hcd-status_urb) {
hcd-status_urb = NULL;
usb_hcd_unlink_urb_from_ep(hcd, urb);
-
- spin_unlock(hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, status);
- spin_lock(hcd_root_hub_lock);
}
}
done:
@@ -1648,6 +1635,73 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
/*-*/
+static void __usb_hcd_giveback_urb(struct urb *urb)
+{
+ struct usb_hcd *hcd = bus_to_hcd(urb-dev-bus);
+ int status = urb-unlinked;
+
[PATCH v2 4/4] USB: EHCI: support running URB giveback in tasklet context
All 4 transfer types can work well on EHCI HCD after switching to run URB giveback in tasklet context, so mark all HCD drivers to support it. At the same time, don't release ehci-lock during URB giveback, and remove the check on HCD_BH in ehci_disable_event(). From below test results on 3 machines(2 ARM and one x86), time consumed by EHCI interrupt handler droped much without performance loss. 1 test description 1.1 mass storage performance test: - run below command 10 times and compute the average performance dd if=/dev/sdN iflag=direct of=/dev/null bs=200M count=1 - two usb mass storage device: A: sandisk extreme USB 3.0 16G(used in test case 1 case 2) B: kingston DataTraveler G2 4GB(only used in test case 2) 1.2 uvc function test: - run one simple capture program in the below link http://kernel.ubuntu.com/~ming/up/capture.c - capture format 640*480 and results in High Bandwidth mode on the uvc device: Z-Star 0x0ac8/0x3450 - on T410(x86) laptop, also use guvcview to watch video capture/playback 1.3 about test2 and test4 - both two devices involved are tested concurrently by above test items 1.4 how to compute irq time(the time consumed by ehci_irq) - use trace points of irq:irq_handler_entry and irq:irq_handler_exit 1.5 kernel 3.10.0-rc3-next-20130528 1.6 test machines Pandaboard A1: ARM CortexA9 dural core Arndale board: ARM CortexA15 dural core T410: i5 CPU 2.67GHz quad core 2 test result 2.1 test case1: single mass storage device performance test upstream| patched perf(MB/s)+irq time(us) | perf(MB/s)+irq time(us) Pandaboard A1: 25.280(avg:145,max:772) | 25.540(avg:14, max:75) Arndale board: 29.700(avg:33, max:129) | 29.700(avg:10, max:50) T410: 34.430(avg:17, max:154*)| 34.660(avg:12, max:155) - 2.2 test case2: two mass storage devices' performance test upstream| patched perf(MB/s)+irq time(us) | perf(MB/s)+irq time(us) Pandaboard A1: 15.840/15.580(avg:158,max:1216) | 16.500/16.160(avg:15,max:139) Arndale board: 17.370/16.220(avg:33 max:234) | 17.480/16.200(avg:11, max:91) T410: 21.180/19.820(avg:18 max:160) | 21.220/19.880(avg:11, max:149) - 2.3 test case3: one uvc streaming test - uvc device works well(on x86, luvcview can be used too and has same result with uvc capture) upstream| patched irq time(us)| irq time(us) Pandaboard A1: (avg:445, max:873) | (avg:33, max:44) Arndale board: (avg:316, max:630) | (avg:20, max:27) T410: (avg:39, max:107) | (avg:10, max:65) - 2.4 test case4: one uvc streaming plus one mass storage device test upstream| patched perf(MB/s)+irq time(us) | perf(MB/s)+irq time(us) Pandaboard A1: 20.340(avg:259,max:1704)| 20.390(avg:24, max:101) Arndale board: 23.460(avg:124,max:726) | 23.370(avg:15, max:52) T410: 28.520(avg:27, max:169) | 28.630(avg:13, max:160) - * On T410, sometimes read ehci status register in ehci_irq takes more than 100us, and the problem has been reported on the link: http://marc.info/?t=13706586731r=1w=2 Cc: Alan Stern st...@rowland.harvard.edu Signed-off-by: Ming Lei ming@canonical.com --- drivers/usb/host/ehci-fsl.c |2 +- drivers/usb/host/ehci-grlib.c |2 +- drivers/usb/host/ehci-hcd.c |2 +- drivers/usb/host/ehci-mv.c|2 +- drivers/usb/host/ehci-octeon.c|2 +- drivers/usb/host/ehci-pmcmsp.c|2 +- drivers/usb/host/ehci-ppc-of.c|2 +- drivers/usb/host/ehci-ps3.c |2 +- drivers/usb/host/ehci-q.c |5 - drivers/usb/host/ehci-sead3.c |2 +- drivers/usb/host/ehci-sh.c|2 +- drivers/usb/host/ehci-tilegx.c|2 +- drivers/usb/host/ehci-timer.c |3 --- drivers/usb/host/ehci-w90x900.c |2 +- drivers/usb/host/ehci-xilinx-of.c |2 +- 15 files changed, 13 insertions(+), 21 deletions(-) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index bd831ec..330274a 100644 ---
[PATCH v2 3/4] USB: EHCI: improve interrupt qh unlink
Given interrupt URB will be resubmitted from tasklet context which
is scheduled by ehci hardware interrupt handler, and commonly only
one interrupt URB is scheduled on qh, so the qh may be unlinked
immediately once qh_completions() returns from ehci_irq(), then
the intr URB to be resubmitted in complete() can only be scheduled
and linked to hardware until the qh unlink is completed.
This patch improves this above situation, and the qh will wait for 5
milliseconds before being unlinked from hardware, if one URB is submitted
during the period, the qh is move out of unlink wait list and the
interrupt transfer can be scheduled immediately, not like before: the
transfer is only linked to hardware until previous unlink is completed.
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ming Lei ming@canonical.com
---
drivers/usb/host/ehci-hcd.c |1 +
drivers/usb/host/ehci-hub.c |1 +
drivers/usb/host/ehci-mem.c |1 +
drivers/usb/host/ehci-sched.c | 62 ++---
drivers/usb/host/ehci-timer.c | 48 ++-
drivers/usb/host/ehci.h |3 ++
6 files changed, 111 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 7abf1ce..35f1372 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -487,6 +487,7 @@ static int ehci_init(struct usb_hcd *hcd)
ehci-periodic_size = DEFAULT_I_TDPS;
INIT_LIST_HEAD(ehci-async_unlink);
INIT_LIST_HEAD(ehci-async_idle);
+ INIT_LIST_HEAD(ehci-intr_unlink_wait);
INIT_LIST_HEAD(ehci-intr_unlink);
INIT_LIST_HEAD(ehci-intr_qh_list);
INIT_LIST_HEAD(ehci-cached_itd_list);
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 2b70277..6037f84 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -345,6 +345,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
end_unlink_async(ehci);
unlink_empty_async_suspended(ehci);
+ ehci_handle_start_intr_unlinks(ehci);
ehci_handle_intr_unlinks(ehci);
end_free_itds(ehci);
diff --git a/drivers/usb/host/ehci-mem.c b/drivers/usb/host/ehci-mem.c
index ef2c3a1..52a7773 100644
--- a/drivers/usb/host/ehci-mem.c
+++ b/drivers/usb/host/ehci-mem.c
@@ -93,6 +93,7 @@ static struct ehci_qh *ehci_qh_alloc (struct ehci_hcd *ehci,
gfp_t flags)
qh-qh_dma = dma;
// INIT_LIST_HEAD (qh-qh_list);
INIT_LIST_HEAD (qh-qtd_list);
+ INIT_LIST_HEAD(qh-unlink_node);
/* dummy td enables safe urb queuing */
qh-dummy = ehci_qtd_alloc (ehci, flags);
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index f80d033..5bf67e2 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -601,12 +601,24 @@ static void qh_unlink_periodic(struct ehci_hcd *ehci,
struct ehci_qh *qh)
list_del(qh-intr_node);
}
-static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
+/* must be called with holding ehci-lock */
+static void cancel_unlink_wait_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
{
- /* If the QH isn't linked then there's nothing we can do. */
- if (qh-qh_state != QH_STATE_LINKED)
+ if (qh-qh_state != QH_STATE_LINKED || list_empty(qh-unlink_node))
return;
+ list_del_init(qh-unlink_node);
+
+ /* avoid unnecessary CPU wakeup */
+ if (list_empty(ehci-intr_unlink_wait))
+ ehci_disable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR);
+}
+
+static void start_do_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
+{
+ /* if the qh is waitting for unlink, cancel it now */
+ cancel_unlink_wait_intr(ehci, qh);
+
qh_unlink_periodic (ehci, qh);
/* Make sure the unlinks are visible before starting the timer */
@@ -632,6 +644,45 @@ static void start_unlink_intr(struct ehci_hcd *ehci,
struct ehci_qh *qh)
}
}
+/*
+ * It is common only one intr URB is scheduled on one qh, and
+ * given complete() is run in tasklet context, introduce a bit
+ * delay to avoid unlink qh too early.
+ */
+static void __start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh,
+ bool wait)
+{
+ /* If the QH isn't linked then there's nothing we can do. */
+ if (qh-qh_state != QH_STATE_LINKED)
+ return;
+
+ if (!wait)
+ return start_do_unlink_intr(ehci, qh);
+
+ qh-unlink_cycle = ehci-intr_unlink_wait_cycle;
+
+ /* New entries go at the end of the intr_unlink_wait list */
+ list_add_tail(qh-unlink_node, ehci-intr_unlink_wait);
+
+ if (ehci-rh_state EHCI_RH_RUNNING)
+ ehci_handle_start_intr_unlinks(ehci);
+ else if (ehci-intr_unlink_wait.next == qh-unlink_node) {
+ ehci_enable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR, true);
+
[PATCH v2 2/4] USB: URB documentation: claim complete() will be run with IRQs enabled
There is no good reason to run complete() in hard interrupt disabled context. After switch to run complete() in tasklet, we will enable local IRQs when calling complete() since we can do it at that time. Even though we still disable IRQs now when calling complete() in tasklet, the URB documentation is updated to claim complete() will be run in tasklet context and local IRQs will be enabled, so that USB drivers can know the change and avoid one deadlock caused by: assume IRQs disabled in complete() and call spin_lock() to hold lock which might be acquired in interrupt context. Current spin_lock() usages in drivers' complete() will be cleaned up at the same time, and once the cleanup is finished, local IRQs will be enabled when calling complete() in tasklet. Also fix description about type of usb_complete_t, and remove the advice of running completion handler in tasklet for decreasing system latency. Cc: Oliver Neukum oli...@neukum.org Cc: Alan Stern st...@rowland.harvard.edu Signed-off-by: Ming Lei ming@canonical.com --- Documentation/usb/URB.txt | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Documentation/usb/URB.txt b/Documentation/usb/URB.txt index 00d2c64..50da0d4 100644 --- a/Documentation/usb/URB.txt +++ b/Documentation/usb/URB.txt @@ -195,13 +195,12 @@ by the completion handler. The handler is of the following type: - typedef void (*usb_complete_t)(struct urb *, struct pt_regs *) + typedef void (*usb_complete_t)(struct urb *) -I.e., it gets the URB that caused the completion call, plus the -register values at the time of the corresponding interrupt (if any). -In the completion handler, you should have a look at urb-status to -detect any USB errors. Since the context parameter is included in the URB, -you can pass information to the completion handler. +I.e., it gets the URB that caused the completion call. In the completion +handler, you should have a look at urb-status to detect any USB errors. +Since the context parameter is included in the URB, you can pass +information to the completion handler. Note that even when an error (or unlink) is reported, data may have been transferred. That's because USB transfers are packetized; it might take @@ -210,12 +209,12 @@ have transferred successfully before the completion was called. NOTE: * WARNING * -NEVER SLEEP IN A COMPLETION HANDLER. These are normally called -during hardware interrupt processing. If you can, defer substantial -work to a tasklet (bottom half) to keep system latencies low. You'll -probably need to use spinlocks to protect data structures you manipulate -in completion handlers. +NEVER SLEEP IN A COMPLETION HANDLER. These are often called in atomic +context. +In the current kernel, completion handlers run with local interrupts +disabled, but in the future this will be changed, so don't assume that +local IRQs are always disabled inside completion handlers. 1.8. How to do isochronous (ISO) transfers? -- 1.7.9.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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On 06/23/2013 07:05 PM, Alan Stern wrote: That's why, if the check is checked, I feel it should be added to each HCD driver separately. Maybe I'm wrong. But before doing anything, you should check with Thomas Pugliese. He recently added SG support to the wireless USB driver. Suppose wireless USB is one exception, it should the only one, so we can rule it out easily by using hcd-wireless, right? Yes, that's true. Alan Stern This test passed for many years. It became to fail only for the recent kernels. So this event looks like a broken thing. It's not a good idea to force a higher driver to care about the max pkt size in a dynamically selected usb configuration. Note that a sg-list could be created even in another subsystem. If we decided to treat such sg-lists as acceptable, then we have only two options: 1. Send a shorter packet between two full packets of a TD list. 2. Introduce a bounce buffer similarly in lib/swiotlb.c Option 2 is very costed. If the option 1 is acceptable, then we should choose it. But ehci does resolve this issue in such way already, so the option 1 is acceptable. From my point of view this choice is unrelated to the wireless USB requirements. Konstantin Filatov -- 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 v2 4/4] USB: EHCI: support running URB giveback in tasklet context
On Monday 24 June 2013 17:42:05 Ming Lei wrote: All 4 transfer types can work well on EHCI HCD after switching to run URB giveback in tasklet context, so mark all HCD drivers to support it. At the same time, don't release ehci-lock during URB giveback, and remove the check on HCD_BH in ehci_disable_event(). From below test results on 3 machines(2 ARM and one x86), time consumed by EHCI interrupt handler droped much without performance loss. 1 test description 1.1 mass storage performance test: - run below command 10 times and compute the average performance dd if=/dev/sdN iflag=direct of=/dev/null bs=200M count=1 It would be nice to get worst case numbers. How bad does it get if you reduce the sg size in usb-storage from 120K to 4K? Regards Oliver -- 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 v2 3/4] USB: EHCI: improve interrupt qh unlink
On Monday 24 June 2013 17:42:04 Ming Lei wrote: This patch improves this above situation, and the qh will wait for 5 milliseconds before being unlinked from hardware, if one URB is submitted during the period, the qh is move out of unlink wait list and the interrupt transfer can be scheduled immediately, not like before: the transfer is only linked to hardware until previous unlink is completed. It seems to me that this logic should be used only if the URB completed without error. Regards Oliver -- 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 / RFC] qcserial: Add support for ONYX 3G (Alfa network)
Hello guys!
Description:
This patch adds support for the ONYX 3G device (first version).
Both the Mangementand the Modem interfaces seem to work fine, still can't
figure out if the Qualcomm Binary interface does work.
And that's why this is also an RFC mail.
The device is cheap and small, the firmware seems minimal and no particular
modifications are made by the product vendor, at least apparently.
Any suggestion would be GREATLY appreciated!
A link to the MSI package by Qualcomm can be found here - I post this so that a
more skilled INF-analyser can help me!!
http://www.gstorm.eu/dgm/ALFA_Onyx3G_USB_Modem.msi
The in-device SD slot and virtual CD are both working with this interface
setup.
Patch:
diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
index bd794b4..bba2fc0 100644
--- a/drivers/usb/serial/qcserial.c
+++ b/drivers/usb/serial/qcserial.c
@@ -133,6 +133,9 @@ static const struct usb_device_id id_table[] = {
{USB_DEVICE_INTERFACE_NUMBER(0x1199, 0x901c, 0)}, /* Sierra
Wireless EM7700 Device Management */
{USB_DEVICE_INTERFACE_NUMBER(0x1199, 0x901c, 2)}, /* Sierra
Wireless EM7700 NMEA */
{USB_DEVICE_INTERFACE_NUMBER(0x1199, 0x901c, 3)}, /* Sierra
Wireless EM7700 Modem */
+ {USB_DEVICE_INTERFACE_NUMBER(0x05c6, 0x0023, 0)}, /* ALFA Network ONYX
3G Device management */
+ {USB_DEVICE_INTERFACE_NUMBER(0x05c6, 0x0023, 2)}, /* ALFA Network ONYX
3G NMEA*/
+ {USB_DEVICE_INTERFACE_NUMBER(0x05c6, 0x0023, 3)}, /* ALFA Network ONYX
3G MDM */
{ } /* Terminating entry */
};
--
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 v5] usb: dwc3: use extcon fwrk to receive connect/disconnect
On 06/21/2013 08:58 PM, Kishon Vijay Abraham I wrote:
Modified dwc3-omap to receive connect and disconnect notification using
extcon framework. Also did the necessary cleanups required after
adapting to extcon framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
Acked-by: Felipe Balbi ba...@ti.com
Acked-by: Chanwoo Choi cw00.c...@samsung.com
---
This patch should be applied after all of the extcon patchset will be applied
because this patch has dependency of extcon patch related to DT.
http://goo.gl/Tu3qW
Changes from v4:
* checked the return values of extcon_register_interest and print an error
message. Note that I dint do return since there might be cases where
one of USB (device mode) or USB-HOST (host mode) might succeed.
* Added depends on of EXTCON in usb_dwc3. Only some platforms might
be using EXTCON, but inorder to avoid compilation errors, added
depends on
Changes from v3:
* did #include of of_extcon.h after Chanwoo resent the patch separating
extcon-class.c from of_extcon.c
Changes from v2:
* updated the Documentation with dwc3 dt binding information.
* used of_extcon_get_extcon_dev to get extcon device from device tree data.
Changes from v1:
* regulator enable/disable is now done here instead of palmas-usb as some
users
of palmas-usb wont need regulator.
Documentation/devicetree/bindings/usb/omap-usb.txt |5 +
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/dwc3-omap.c | 125
+---
include/linux/usb/dwc3-omap.h | 30 -
4 files changed, 112 insertions(+), 49 deletions(-)
delete mode 100644 include/linux/usb/dwc3-omap.h
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index d4769f3..f1c15f3 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -53,6 +53,11 @@ OMAP DWC3 GLUE
It should be set to 1 for HW mode and 2 for SW mode.
- ranges: the child address space are mapped 1:1 onto the parent address
space
+Optional Properties:
+ - extcon : phandle for the extcon device omap dwc3 uses to detect
+ connect/disconnect events.
+ - vbus-supply : phandle to the regulator device tree node if needed.
+
Sub-nodes:
The dwc3 core should be added as subnode to omap dwc3 glue.
- dwc3 :
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 757aa18..08a9fab 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) GENERIC_HARDIRQS
+ depends on EXTCON
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index f8f76e6..80b5780 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -43,13 +43,15 @@
#include linux/spinlock.h
#include linux/platform_device.h
#include linux/platform_data/dwc3-omap.h
-#include linux/usb/dwc3-omap.h
#include linux/pm_runtime.h
#include linux/dma-mapping.h
#include linux/ioport.h
#include linux/io.h
#include linux/of.h
#include linux/of_platform.h
+#include linux/extcon.h
+#include linux/extcon/of_extcon.h
+#include linux/regulator/consumer.h
#include linux/usb/otg.h
@@ -124,9 +126,21 @@ struct dwc3_omap {
u32 utmi_otg_status;
u32 dma_status:1;
+
+ struct extcon_specific_cable_nb extcon_vbus_dev;
+ struct extcon_specific_cable_nb extcon_id_dev;
+ struct notifier_block vbus_nb;
+ struct notifier_block id_nb;
+
+ struct regulator*vbus_reg;
};
-static struct dwc3_omap *_omap;
+enum omap_dwc3_vbus_id_status {
+ OMAP_DWC3_ID_FLOAT,
+ OMAP_DWC3_ID_GROUND,
+ OMAP_DWC3_VBUS_OFF,
+ OMAP_DWC3_VBUS_VALID,
+};
static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
{
@@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base,
u32 offset, u32 value)
writel(value, base + offset);
}
-int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
+static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
+ enum omap_dwc3_vbus_id_status status)
{
- u32 val;
- struct dwc3_omap*omap = _omap;
-
- if (!omap)
- return -EPROBE_DEFER;
+ int ret;
+ u32 val;
switch (status) {
case OMAP_DWC3_ID_GROUND:
dev_dbg(omap-dev, ID GND\n);
+ if (omap-vbus_reg) {
+ ret = regulator_enable(omap-vbus_reg);
+ if (ret) {
+ dev_dbg(omap-dev, regulator enable
Re: [PATCH v2 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, Jun 24, 2013 at 6:28 PM, Oliver Neukum oli...@neukum.org wrote: On Monday 24 June 2013 17:42:04 Ming Lei wrote: This patch improves this above situation, and the qh will wait for 5 milliseconds before being unlinked from hardware, if one URB is submitted during the period, the qh is move out of unlink wait list and the interrupt transfer can be scheduled immediately, not like before: the transfer is only linked to hardware until previous unlink is completed. It seems to me that this logic should be used only if the URB completed without error. The current completed URB with error doesn't mean the later submitted URB will complete with error, so I don't think the logic is related with URB completion error. Thanks, -- Ming Lei -- 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Mon, Jun 24, 2013 at 6:10 PM, Konstantin Filatov kfila...@parallels.com wrote: On 06/23/2013 07:05 PM, Alan Stern wrote: That's why, if the check is checked, I feel it should be added to each HCD driver separately. Maybe I'm wrong. But before doing anything, you should check with Thomas Pugliese. He recently added SG support to the wireless USB driver. Suppose wireless USB is one exception, it should the only one, so we can rule it out easily by using hcd-wireless, right? Yes, that's true. Alan Stern This test passed for many years. It became to fail only for the recent kernels. So this event looks like a broken thing. It's not a good idea to force a higher driver to care about the max pkt size in a dynamically selected usb configuration. Note that a sg-list could be The constraint is from USB spec(1.1/2.0/3.0), and a short packet simply means the end of one transfer, I think we can see it as a API about sg list passed to usb_submit_urb(). created even in another subsystem. Considered that there isn't much such kind of usages now, we need to consider the problem, and introducing check in usb_submit_urb() is necessary, so we can avoid and kill such mistaken usages at early stage. Otherwise, we have to introduce buffer debounce in usbcore or hcd to work around the problem when many users start to do that. If we decided to treat such sg-lists as acceptable, then we have only two options: 1. Send a shorter packet between two full packets of a TD list. 2. Introduce a bounce buffer similarly in lib/swiotlb.c Option 2 is very costed. If the option 1 is acceptable, then we should choose it. No, option 1 isn't accepted, and option 2 isn't necessary since there aren't much such usage. In fact, except for usbtest, I don't know there are other drivers which may pass such kind of sg list. If anyone knows, please share it. But ehci does resolve this issue in such way already, so the option 1 is acceptable. No, ehci only doesn't cause memory access error but the way is wrong, and it does violate USB spec. From my point of view this choice is unrelated to the wireless USB requirements. We just want to confirm if wireless USB need the same check. Thanks, -- Ming Lei -- 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 20/20] usb: chipidea: drop 13xxx infix
Greg KH gre...@linuxfoundation.org writes: On Thu, Jun 13, 2013 at 06:00:04PM +0300, Alexander Shishkin wrote: ci13xxx is bad for at least the following reasons: * people often mistype it * it doesn't add any informational value to the names it's used in * it needlessly attracts mail filters This patch replaces it with ci_hdrc, ci_udc or ci_hw, depending on the situation. Modules with ci13xxx prefix are also renamed accordingly and aliases are added for compatibility. Otherwise, no functional changes. Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com This one didn't apply cleanly, and so I tried to do it by hand, and edited the file and totally messed it up :( Weirdness, I just cherry-picked it on top of usb-next and it worked without any conflicts. So, can you regenerate it against my usb-next branch and resend it so that I can apply it? Sorry about that, it's my fault for loosing the patch. Sure, resending it now. Thanks, -- 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: [PATCH v2 1/4] USB: HCD: support giveback of URB in tasklet context
On Mon, Jun 24, 2013 at 5:42 PM, Ming Lei ming@canonical.com wrote:
+
+static void init_giveback_urb_bh(struct usb_hcd *hcd)
+{
+ if (!hcd_giveback_urb_in_bh(hcd))
+ return;
Sorry, the above check isn't needed, and the below check isn't
needed too. I will fix it in V3.
+
+ __init_giveback_urb_bh(hcd-high_prio_bh);
+ __init_giveback_urb_bh(hcd-low_prio_bh);
+}
+
+static void exit_giveback_urb_bh(struct usb_hcd *hcd)
+{
+ if (!hcd_giveback_urb_in_bh(hcd))
+ return;
+
+ /*
+* tasklet_kill() isn't needed here because:
+* - driver's disconnec() called from usb_disconnect() should
+* make sure its URBs are completed during the disconnect()
+* callback
+*
+* - it is too late to run complete() here since driver may have
+* been removed already now
+*
+* Using tasklet to run URB's complete() doesn't change this
+* behavior of usbcore.
+*/
+}
+
Thanks,
--
Ming Lei
--
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 v2 3/4] USB: EHCI: improve interrupt qh unlink
On Monday 24 June 2013 19:16:43 Ming Lei wrote: On Mon, Jun 24, 2013 at 6:28 PM, Oliver Neukum oli...@neukum.org wrote: On Monday 24 June 2013 17:42:04 Ming Lei wrote: This patch improves this above situation, and the qh will wait for 5 milliseconds before being unlinked from hardware, if one URB is submitted during the period, the qh is move out of unlink wait list and the interrupt transfer can be scheduled immediately, not like before: the transfer is only linked to hardware until previous unlink is completed. It seems to me that this logic should be used only if the URB completed without error. The current completed URB with error doesn't mean the later submitted URB will complete with error, so I don't think the logic is related with URB completion error. But a) it is likelier b) the driver will start error handling And what about the bandwidth? At least if you unlink the URB, its bandwidth should be given back immediately. Regards Oliver -- 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: [RESEND PATCH v2 1/1] usb: fix build error without CONFIG_USB_PHY
On Mon, Jun 24, 2013 at 11:15:35AM +0300, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: on i386: drivers/built-in.o: In function `ci_hdrc_probe': core.c:(.text+0x20446b): undefined reference to `of_usb_get_phy_mode' Signed-off-by: Peter Chen peter.c...@freescale.com Reported-by: Randy Dunlap rdun...@infradead.org Acked-by: Randy Dunlap rdun...@infradead.org It's actually Felipe's turf, so needs either his ack or target his tree. I'm ok with this. FWIW, Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com since the original patch went through your tree, it's better if you take it. Here's my Ack: Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 4/4] USB: EHCI: support running URB giveback in tasklet context
On Mon, Jun 24, 2013 at 6:24 PM, Oliver Neukum oli...@neukum.org wrote: On Monday 24 June 2013 17:42:05 Ming Lei wrote: All 4 transfer types can work well on EHCI HCD after switching to run URB giveback in tasklet context, so mark all HCD drivers to support it. At the same time, don't release ehci-lock during URB giveback, and remove the check on HCD_BH in ehci_disable_event(). From below test results on 3 machines(2 ARM and one x86), time consumed by EHCI interrupt handler droped much without performance loss. 1 test description 1.1 mass storage performance test: - run below command 10 times and compute the average performance dd if=/dev/sdN iflag=direct of=/dev/null bs=200M count=1 It would be nice to get worst case numbers. How bad does it get if you reduce the sg size in usb-storage from 120K to 4K? A quick test on one arm A15 box shows that the average speed over 10 times 'dd' becomes 8.0MB/sec from 8.160MB/sec when 'bs' parameter of 'dd' changes to 4K, so there is ~1.9% performance loss with the patch under the worst case. Same test on my T410(x86), the speed difference is only 40K. I will collect the worst case numbers and include it in the commit log of V3. Thanks, -- Ming Lei -- 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 v2 4/4] USB: EHCI: support running URB giveback in tasklet context
On Monday 24 June 2013 20:58:26 Ming Lei wrote:
On Mon, Jun 24, 2013 at 6:24 PM, Oliver Neukum oli...@neukum.org wrote:
On Monday 24 June 2013 17:42:05 Ming Lei wrote:
All 4 transfer types can work well on EHCI HCD after switching to run
URB giveback in tasklet context, so mark all HCD drivers to support
it.
At the same time, don't release ehci-lock during URB giveback,
and remove the check on HCD_BH in ehci_disable_event().
From below test results on 3 machines(2 ARM and one x86), time
consumed by EHCI interrupt handler droped much without performance
loss.
1 test description
1.1 mass storage performance test:
- run below command 10 times and compute the average performance
dd if=/dev/sdN iflag=direct of=/dev/null bs=200M count=1
It would be nice to get worst case numbers. How bad does it get
if you reduce the sg size in usb-storage from 120K to 4K?
A quick test on one arm A15 box shows that the average speed over
10 times 'dd' becomes 8.0MB/sec from 8.160MB/sec when 'bs'
parameter of 'dd' changes to 4K, so there is ~1.9% performance
loss with the patch under the worst case.
Same test on my T410(x86), the speed difference is only 40K.
I will collect the worst case numbers and include it in the commit
log of V3.
Sorry,
I was referring to scsiglue.c
struct scsi_host_template usb_stor_host_template = {
/* basic userland interface stuff */
.name = usb-storage,
.proc_name =usb-storage,
.proc_info =proc_info,
.info = host_info,
/* command interface -- queued only */
.queuecommand = queuecommand,
/* error and abort handlers */
.eh_abort_handler = command_abort,
.eh_device_reset_handler = device_reset,
.eh_bus_reset_handler = bus_reset,
/* queue commands only, only one command per LUN */
.can_queue =1,
.cmd_per_lun = 1,
/* unknown initiator id */
.this_id = -1,
.slave_alloc = slave_alloc,
.slave_configure = slave_configure,
/* lots of sg segments can be handled */
.sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,
/* limit the total size of a transfer to 120 KB */
.max_sectors = 240,
If you go to 8 sectors here, you should get the absolute worst case.
Regards
Oliver
--
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 v2 4/4] USB: EHCI: support running URB giveback in tasklet context
On Mon, Jun 24, 2013 at 9:06 PM, Oliver Neukum oli...@neukum.org wrote:
On Monday 24 June 2013 20:58:26 Ming Lei wrote:
On Mon, Jun 24, 2013 at 6:24 PM, Oliver Neukum oli...@neukum.org wrote:
On Monday 24 June 2013 17:42:05 Ming Lei wrote:
All 4 transfer types can work well on EHCI HCD after switching to run
URB giveback in tasklet context, so mark all HCD drivers to support
it.
At the same time, don't release ehci-lock during URB giveback,
and remove the check on HCD_BH in ehci_disable_event().
From below test results on 3 machines(2 ARM and one x86), time
consumed by EHCI interrupt handler droped much without performance
loss.
1 test description
1.1 mass storage performance test:
- run below command 10 times and compute the average performance
dd if=/dev/sdN iflag=direct of=/dev/null bs=200M count=1
It would be nice to get worst case numbers. How bad does it get
if you reduce the sg size in usb-storage from 120K to 4K?
A quick test on one arm A15 box shows that the average speed over
10 times 'dd' becomes 8.0MB/sec from 8.160MB/sec when 'bs'
parameter of 'dd' changes to 4K, so there is ~1.9% performance
loss with the patch under the worst case.
Same test on my T410(x86), the speed difference is only 40K.
I will collect the worst case numbers and include it in the commit
log of V3.
Sorry,
I was referring to scsiglue.c
struct scsi_host_template usb_stor_host_template = {
/* basic userland interface stuff */
.name = usb-storage,
.proc_name =usb-storage,
.proc_info =proc_info,
.info = host_info,
/* command interface -- queued only */
.queuecommand = queuecommand,
/* error and abort handlers */
.eh_abort_handler = command_abort,
.eh_device_reset_handler = device_reset,
.eh_bus_reset_handler = bus_reset,
/* queue commands only, only one command per LUN */
.can_queue =1,
.cmd_per_lun = 1,
/* unknown initiator id */
.this_id = -1,
.slave_alloc = slave_alloc,
.slave_configure = slave_configure,
/* lots of sg segments can be handled */
.sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,
/* limit the total size of a transfer to 120 KB */
.max_sectors = 240,
If you go to 8 sectors here, you should get the absolute worst case.
If you check trace of usbmon, 'dd if=/dev/sda iflag=direct bs=4k count=xxx'
generates the 4k data stage per transfer(cbw/data/csw).
So there is no difference between them.
Thanks,
--
Ming Lei
--
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: Chipidea usb otg support for IMX/MXS (device functionality)
Hello Peter, Hi Peter, Peter, I dunno if you are already aware of it, but the USB peripheral mode hangs on MX233. It's easy to replicate for example if you try to run CDC ethernet over the USB peripheral mode, then telnet into the board and run dmesg . This will trigger a larger data transfer which will make the UDC driver hang. This doesn't happen on MX28 so it must be some MX233-specific thing. Marek, Have you tried below? http://marc.info/?l=linux-usbm=136537318510847w=2 It may not the USB itself problem, the mx23's usb is almost the same with mx28's. This also happens on a different MX23-based board for me. This other board I tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing people complained about in the Olimex forum is resolved now. Add shawn. Marek, have you tried mx23 evk? No, I don't have this one available right now. I tried both MX23 Olinuxino maxi (I just mutilated the board so that I cut traces to the USB devices on the board and routed out USB gadget connector) and a custom MX23 board. Note that both have working USB peripheral mode in U-Boot too, but the driver in U-Boot is very minimalistic. Shawn, marek reported the udc function at mx23 works abnormal, but it works good at mx28. Have you tried mx23 udc recently? Fabio, can you possibly test on MX23EVK please? Best regards, Marek Vasut -- 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 Mon, 24 Jun 2013, victor yeo wrote: Hi, The problem is in UDC driver. i made the change, it is ok now. Good. I noticed that the usb_ep_set_wedge routine still isn't working right. You might try to fix that. Alan Stern Ok, is the usb_ep_set_wedge routine not working? I can't see that in the log file. It is not working. This can be seen in the usbmon log. Now, in USB 2.0 CV test, there is an error about GET_STATUS request, as shown below. g_file_storage gadget: ep0-setup, length 8: : 82 00 00 00 81 00 02 00 g_file_storage gadget: unknown control req 82.00 v i0081 l2 handle_setup status -95 I think the GET_STATUS request is not handled by the gadget driver. Isn't it so? That's right. Get-Status, Set-Feature, and Clear-Feature requests must be handled by the UDC driver. 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
Re: [PATCH] usb: host: xhci-plat: release mem region while removing module
On Fri, Jun 21, 2013 at 01:59:08PM +0530, George Cherian wrote: Do a release_mem_region of the hcd resource. Without this the subsequent insertion of module fails in request_mem_region. Signed-off-by: George Cherian george.cher...@ti.com very nice catch. Acked-by: Felipe Balbi ba...@ti.com You need to resend with: Cc: sta...@vger.kernel.org # v3.4+ Not sure if Sarah wants to take care of the stable tagging, though. -- balbi signature.asc Description: Digital signature
Re: [PATCH 8/8] usb: musb: omap2430: make it compile again
Hi, On Wed, Jun 19, 2013 at 05:38:16PM +0200, Sebastian Andrzej Siewior wrote: it does not compile since 09fc7d (usb: musb: fix incorrect usage of resource pointer). What makes me wonder most is if source of the Tested-by tag :) Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de which tree are you using as your base ? I don't have that hunk below on my v3.10-rc7. $ git grep -e struct resource drivers/usb/musb/omap2430.c $ echo $? 1 -- balbi signature.asc Description: Digital signature
From: Konstantin Filatov kfila...@parallels.com
with a length that isn't multiply by max pkt size for this endpoint.
The commit 689d6eac (USB: UHCI: add native scatter-gather support(v1))
introduced an implementation of scatter-gather list for UHCI.
This implementation has a bug when the size of a non-last sg
element was not multiply by TD's max-pkt-size. This bug was latent
till the commit 2851784f (usb/uhci: initialize sg_table properly)
which really initializes sg_table and enables SG lists in UHCI.
The bug can be easily reproduced with Gadget Zero in full_speed mode
connected to a host with UHCI controller by the standard test from
tools/usb/ with command line
testusb -a -t 7 -c 2000 -s 4096 -v 41
The test sends trash bytes to the gadget instead of only zero bytes
and so the test fails.
This patch shortens TD's packet not only for the last TD in sg list,
but also for the last TD in sg element.
Signed-off-by: Konstantin Filatov kfila...@parallels.com
Signed-off-by: Denis V. Lunev d...@openvz.org
Acked-by: Alan Stern st...@rowland.harvard.edu
CC: Ming Lei tom.leim...@gmail.com
CC: Sebastian Andrzej Siewior bige...@linutronix.de
CC: linux-usb@vger.kernel.org
---
Changes from v2:
- changed subject as suggested by Alan Stern
- edited description
- shortened the referenced commit descriptions
Changes from v1:
- added short commit descriptions as requested by Sergei Shtylyov
- CC list extended according to the list of original authors
drivers/usb/host/uhci-q.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 6e6ea21..e0ebc80 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -977,6 +977,9 @@ static int uhci_submit_common(struct uhci_hcd *uhci, struct
urb *urb,
for (;;) { /* Allow zero length packets */
int pktsze = maxsze;
+ if (this_sg_len pktsze) /* The short packet */
+ pktsze = this_sg_len;
+
if (len = pktsze) {/* The last packet */
pktsze = len;
if (!(urb-transfer_flags URB_SHORT_NOT_OK))
--
1.8.1.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
[v4] usb: UHCI: fix pkt size in TD for a sg element
From: Konstantin Filatov kfila...@parallels.com
with a length that isn't multiply by max pkt size for this endpoint.
The commit 689d6eac (USB: UHCI: add native scatter-gather support(v1))
introduced an implementation of scatter-gather list for UHCI.
This implementation has a bug when the size of a non-last sg
element was not multiply by TD's max-pkt-size. This bug was latent
till the commit 2851784f (usb/uhci: initialize sg_table properly)
which really initializes sg_table and enables SG lists in UHCI.
The bug can be easily reproduced with Gadget Zero in full_speed mode
connected to a host with UHCI controller by the standard test from
tools/usb/ with command line
testusb -a -t 7 -c 2000 -s 4096 -v 41
The test sends trash bytes to the gadget instead of only zero bytes
and so the test fails.
This patch shortens TD's packet not only for the last TD in sg list,
but also for the last TD in sg element.
Signed-off-by: Konstantin Filatov kfila...@parallels.com
Signed-off-by: Denis V. Lunev d...@openvz.org
Acked-by: Alan Stern st...@rowland.harvard.edu
CC: Ming Lei tom.leim...@gmail.com
CC: Sebastian Andrzej Siewior bige...@linutronix.de
CC: linux-usb@vger.kernel.org
---
Changes from v3:
- fixed subject
Changes from v2:
- changed subject as suggested by Alan Stern
- edited description
- shortened the referenced commit descriptions
Changes from v1:
- added short commit descriptions as requested by Sergei Shtylyov
- CC list extended according to the list of original authors
drivers/usb/host/uhci-q.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 6e6ea21..e0ebc80 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -977,6 +977,9 @@ static int uhci_submit_common(struct uhci_hcd *uhci, struct
urb *urb,
for (;;) { /* Allow zero length packets */
int pktsze = maxsze;
+ if (this_sg_len pktsze) /* The short packet */
+ pktsze = this_sg_len;
+
if (len = pktsze) {/* The last packet */
pktsze = len;
if (!(urb-transfer_flags URB_SHORT_NOT_OK))
--
1.8.1.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 8/8] usb: musb: omap2430: make it compile again
On 06/24/2013 04:27 PM, Felipe Balbi wrote: Hi, Hi Felipe, which tree are you using as your base ? I don't have that hunk below on my v3.10-rc7. $ git grep -e struct resource drivers/usb/musb/omap2430.c $ echo $? 1 I have here git merge fel/master gusb/usb-linus gusb/usb-next where fel is you and gusb is Greg. It came from that commit https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/drivers/usb/musb/omap2430.c?h=nextid=09fc7d22b024692b2fe8a943b246de1af307132b and if I look at your master branch https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/tree/drivers/usb/musb/omap2430.c the typo is still there. Sebastian -- 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: From: Konstantin Filatov kfila...@parallels.com
Hi, On Mon, Jun 24, 2013 at 06:31:07PM +0400, Denis V. Lunev wrote: with a length that isn't multiply by max pkt size for this endpoint. The commit 689d6eac (USB: UHCI: add native scatter-gather support(v1)) introduced an implementation of scatter-gather list for UHCI. This implementation has a bug when the size of a non-last sg element was not multiply by TD's max-pkt-size. This bug was latent till the commit 2851784f (usb/uhci: initialize sg_table properly) which really initializes sg_table and enables SG lists in UHCI. The bug can be easily reproduced with Gadget Zero in full_speed mode connected to a host with UHCI controller by the standard test from tools/usb/ with command line testusb -a -t 7 -c 2000 -s 4096 -v 41 The test sends trash bytes to the gadget instead of only zero bytes and so the test fails. This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Acked-by: Alan Stern st...@rowland.harvard.edu CC: Ming Lei tom.leim...@gmail.com CC: Sebastian Andrzej Siewior bige...@linutronix.de CC: linux-usb@vger.kernel.org this is not the correct way to send a patch. Subject line is wrong, please fix that. -- balbi signature.asc Description: Digital signature
Re: URB completion order, normal behavior or bug?
Well, our project needs more than two USB ports, and we were looking at the model A anyway, so maybe I should just find some nice USB hub chip and/or PIC that uses SPI and has a good stable driver and hook it up via the Pi's SPI interface? I know that sounds kinda stupid when it already has the USB interface on it, but if it's an unstable thing, it may be good since we're going to be moving big telescopes around with these things and it would be bad if it suddenly whacked somebody in the head because of a bad USB message. :) I would trust SPI over USB, especially as you have more direct control over SPI than USB. But if you only have USB, then perhaps you should use a different device than the RPI? Like a BeagleBoneBlack? Or some other tiny Linux machine, there are a lot of them under 100 USD these days. Thanks for the advice! I'm going to focus on perfecting my driver for the moment and starting the work of talking to SPI devices with it and making sure that all is good. My brother and I were originally thinking of going SPI to our peripherals, but the issues of distance (electrical noise capacitance) was seeming increasingly daunting until I discovered cheap USB-to-SPI bridges like this one (under $2, but no I2C). So I'm glad we're taking this approach now since there are so many cheap Linux devices with USB. But back to the MCP2210 driver its self, once I get this a little more perfected, I'll submit it to linux-usb for RFC and start the gradual process of making it fit for integration in the mainline. -- 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 8/8] usb: musb: omap2430: make it compile again
On Wed, Jun 19, 2013 at 05:38:16PM +0200, Sebastian Andrzej Siewior wrote: it does not compile since 09fc7d (usb: musb: fix incorrect usage of resource pointer). What makes me wonder most is if source of the Tested-by tag :) Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 8/8] usb: musb: omap2430: make it compile again
On Mon, Jun 24, 2013 at 04:33:42PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 04:27 PM, Felipe Balbi wrote: Hi, Hi Felipe, which tree are you using as your base ? I don't have that hunk below on my v3.10-rc7. $ git grep -e struct resource drivers/usb/musb/omap2430.c $ echo $? 1 I have here git merge fel/master gusb/usb-linus gusb/usb-next where fel is you and gusb is Greg. It came from that commit https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/drivers/usb/musb/omap2430.c?h=nextid=09fc7d22b024692b2fe8a943b246de1af307132b and if I look at your master branch https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/tree/drivers/usb/musb/omap2430.c the typo is still there. it's in my tree, but not in v3.10-rc7 clean, so that typo is going on v3.11 merge window... Looks like I forgot to update the patch, I had seen that problem before and had fixed it :-( Looks like your patch needs to go in fast, I'll ack it. -- balbi signature.asc Description: Digital signature
Re: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Mon, 24 Jun 2013, Konstantin Filatov wrote: On 06/23/2013 07:05 PM, Alan Stern wrote: That's why, if the check is checked, I feel it should be added to each HCD driver separately. Maybe I'm wrong. But before doing anything, you should check with Thomas Pugliese. He recently added SG support to the wireless USB driver. Suppose wireless USB is one exception, it should the only one, so we can rule it out easily by using hcd-wireless, right? Yes, that's true. Alan Stern This test passed for many years. It became to fail only for the recent kernels. So this event looks like a broken thing. Looks can be deceiving. Even though the test used to pass, it wasn't doing what it was supposed to do. Also, if you tried test 8 (read) instead of test 7 (write), using the same test parameters, you would have found that it has _never_ worked. Not even on EHCI. The error is in the parameters, not just in the new kernel code. It's not a good idea to force a higher driver to care about the max pkt size in a dynamically selected usb configuration. Note that a sg-list could be created even in another subsystem. (In fact, SG lists are usually created in other subsystems.) There have always been cases where this sort of thing is done. Consider for example the restrictions on the address value when using MMAP_FIXED in the mmap(2) system call. The fact is, most USB host controller hardware is not capable of sending or receiving a packet if the data buffer isn't contiguous in memory (or at best, virtually contiguous, meaning that any discontinuities must occur exactly on page boundaries). If we decided to treat such sg-lists as acceptable, then we have only two options: 1. Send a shorter packet between two full packets of a TD list. 2. Introduce a bounce buffer similarly in lib/swiotlb.c Option 2 is very costed. If the option 1 is acceptable, then we should choose it. But ehci does resolve this issue in such way already, so the option 1 is acceptable. The other choice is not to regard such SG lists as acceptable. In the end, that's a safer approach. The client will be notified at once that something went wrong, instead of thinking that the transfer succeeded when the data was not sent as intended. From my point of view this choice is unrelated to the wireless USB requirements. The choice is not related to wireless USB, but the best way to implement it is. 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
Re: [PATCH 8/8] usb: musb: omap2430: make it compile again
Hi, On Mon, Jun 24, 2013 at 04:42:52PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 04:38 PM, Felipe Balbi wrote: it's in my tree, but not in v3.10-rc7 clean, so that typo is going on v3.11 merge window... Looks like I forgot to update the patch, I had seen that problem before and had fixed it :-( Looks like your patch needs to go in fast, I'll ack it. So you are going to take only 8/8 for the upcoming merge window? In that case I have the slight feeling that someone should resend that patch with Greg in To: or it won't make it… yeah, the others can wait until v3.12, I'll apply them as soon as v3.11-rc1 is tagged. -- balbi signature.asc Description: Digital signature
Re: From: Konstantin Filatov kfila...@parallels.com
On Mon, Jun 24, 2013 at 06:42:28PM +0400, Denis V. Lunev wrote: On 6/24/13 6:35 PM, Felipe Balbi wrote: Hi, On Mon, Jun 24, 2013 at 06:31:07PM +0400, Denis V. Lunev wrote: with a length that isn't multiply by max pkt size for this endpoint. The commit 689d6eac (USB: UHCI: add native scatter-gather support(v1)) introduced an implementation of scatter-gather list for UHCI. This implementation has a bug when the size of a non-last sg element was not multiply by TD's max-pkt-size. This bug was latent till the commit 2851784f (usb/uhci: initialize sg_table properly) which really initializes sg_table and enables SG lists in UHCI. The bug can be easily reproduced with Gadget Zero in full_speed mode connected to a host with UHCI controller by the standard test from tools/usb/ with command line testusb -a -t 7 -c 2000 -s 4096 -v 41 The test sends trash bytes to the gadget instead of only zero bytes and so the test fails. This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Acked-by: Alan Stern st...@rowland.harvard.edu CC: Ming Lei tom.leim...@gmail.com CC: Sebastian Andrzej Siewior bige...@linutronix.de CC: linux-usb@vger.kernel.org this is not the correct way to send a patch. Subject line is wrong, please fix that. yep, sorry, fixed already. saw as soon as I replied :-) -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 10:33 PM, Denis V. Lunev d...@openvz.org wrote: From: Konstantin Filatov kfila...@parallels.com This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Considered that: - the change violates USB spec(1.1/2.0/3.0) - the problem should be avoided in usbcore since it isn't a uhci specific problem - this patch only hides problem, doesn't help to fix real problem. so NAK. I will cook a patch to prevent such bad sg list from reaching HCDs tomorrow. Thanks, -- Ming Lei -- 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 8/8] usb: musb: omap2430: make it compile again
On 06/24/2013 04:38 PM, Felipe Balbi wrote: it's in my tree, but not in v3.10-rc7 clean, so that typo is going on v3.11 merge window... Looks like I forgot to update the patch, I had seen that problem before and had fixed it :-( Looks like your patch needs to go in fast, I'll ack it. So you are going to take only 8/8 for the upcoming merge window? In that case I have the slight feeling that someone should resend that patch with Greg in To: or it won't make it… Sebastian -- 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: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 10:56:30PM +0800, Ming Lei wrote: On Mon, Jun 24, 2013 at 10:33 PM, Denis V. Lunev d...@openvz.org wrote: From: Konstantin Filatov kfila...@parallels.com This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Considered that: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. - the problem should be avoided in usbcore since it isn't a uhci specific problem to this I agree. - this patch only hides problem, doesn't help to fix real problem. what is the problem then ? Are you saying that it's wrong to have an sg which is not aligned to wMaxPacketSize somewhere in the middle of an sg-table ? How so ? What does that have to do with USB at all ? -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
Hi Alan,
On 06/20/2013 08:33 PM, Alan Stern wrote:
On Thu, 20 Jun 2013, Roger Quadros wrote:
runtime_resume(dev)
{
...
if (omap-flags OMAP_EHCI_IRQ_PENDING) {
process_pending_irqs(omap);
OK, thanks.
But I'm not sure if the generic ehci_irq handler is able to
run in a process context. Maybe if we replace spin_lock(ehci-lock);
with spin_lock_irqsave() there, it will work.
Alan is this a doable option?
ehci_irq() will work okay in process context, provided the caller
disables interrupts.
But maybe none of this will be needed after Roger redesigns the
controller suspend to work at the right time. Or if it is, we could
adopt a simpler alternative: the controller's resume routine could
always call usb_hcd_resume_root_hub(). After all, about the only
reason for doing a runtime resume of the controller is because the root
hub needs to do something.
OK I've tried to handle all this in an alternate way. Now the controller
suspend/resume
and runtime suspend/resume is independent of bus suspend.
The controller now runtime suspends when all devices on the bus have suspended
and
the hub auto suspends. NOTE: HW_ACCESSIBLE is still set on runtime_suspend.
The challenge here is to process the interrupt in this state.
I've tried to handle this state. (i.e. interrupt while controller is runtime
suspended),
by disabling the interrupt till we are ready and calling
usb_hcd_resume_root_hub().
We mark a flag (HW_IRQ_DISABLED) stating that the interrupt was disabled and
based on
that enable the IRQ and clear the flag in hcd_resume_work().
Do let me know what you think of this approach.
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index d53547d..8879cd2 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2136,6 +2136,11 @@ static void hcd_resume_work(struct work_struct *work)
usb_lock_device(udev);
usb_remote_wakeup(udev);
usb_unlock_device(udev);
+ if (HCD_IRQ_DISABLED(hcd)) {
+ /* Interrupt was disabled */
+ clear_bit(HCD_FLAG_IRQ_DISABLED, hcd-flags);
+ enable_irq(hcd-irq);
+ }
}
/**
@@ -2225,6 +2230,16 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd)))
rc = IRQ_NONE;
+ else if (pm_runtime_status_suspended(hcd-self.controller)) {
+ /*
+* We can't handle it yet so disable IRQ, make note of it
+* and resume root hub (i.e. controller as well)
+*/
+ disable_irq_nosync(hcd-irq);
+ set_bit(HCD_FLAG_IRQ_DISABLED, hcd-flags);
+ usb_hcd_resume_root_hub(hcd);
+ rc = IRQ_HANDLED;
+ }
else if (hcd-driver-irq(hcd) == IRQ_NONE)
rc = IRQ_NONE;
else
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index f5f5c7d..6c6287e 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -110,6 +110,7 @@ struct usb_hcd {
#define HCD_FLAG_WAKEUP_PENDING4 /* root hub is
resuming? */
#define HCD_FLAG_RH_RUNNING5 /* root hub is running? */
#define HCD_FLAG_DEAD 6 /* controller has died? */
+#define HCD_FLAG_IRQ_DISABLED 7 /* Interrupt was disabled */
/* The flags can be tested using these macros; they are likely to
* be slightly faster than test_bit().
@@ -120,6 +121,7 @@ struct usb_hcd {
#define HCD_WAKEUP_PENDING(hcd)((hcd)-flags (1U
HCD_FLAG_WAKEUP_PENDING))
#define HCD_RH_RUNNING(hcd)((hcd)-flags (1U HCD_FLAG_RH_RUNNING))
#define HCD_DEAD(hcd) ((hcd)-flags (1U HCD_FLAG_DEAD))
+#define HCD_IRQ_DISABLED(hcd) ((hcd)-flags (1U HCD_FLAG_IRQ_DISABLED))
/* Flags that get set only during HCD registration or removal. */
unsignedrh_registered:1;/* is root hub registered? */
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 16d7150..c5320c7 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -225,6 +225,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
usb_phy_set_suspend(omap-phy[i], 0);
}
+ pm_runtime_put_sync(dev);
+
return 0;
err_pm_runtime:
--
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: [v4] usb: UHCI: fix pkt size in TD for a sg element
On 06/24/2013 05:04 PM, Felipe Balbi wrote: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. It is a little. The first USB packet has 512 vs 502 bytes on the wire. Depending on the protocol on the wire it is an issue. But this is almost kidergarder. We have only mass storage using this which does not do this. You can't even merge sgs at that level so that you run in such a problem. So this is simply a tool used wrong. I agree with the patch Alan acked. Sebastian -- 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: From: Konstantin Filatov kfila...@parallels.com
On 6/24/13 6:35 PM, Felipe Balbi wrote: Hi, On Mon, Jun 24, 2013 at 06:31:07PM +0400, Denis V. Lunev wrote: with a length that isn't multiply by max pkt size for this endpoint. The commit 689d6eac (USB: UHCI: add native scatter-gather support(v1)) introduced an implementation of scatter-gather list for UHCI. This implementation has a bug when the size of a non-last sg element was not multiply by TD's max-pkt-size. This bug was latent till the commit 2851784f (usb/uhci: initialize sg_table properly) which really initializes sg_table and enables SG lists in UHCI. The bug can be easily reproduced with Gadget Zero in full_speed mode connected to a host with UHCI controller by the standard test from tools/usb/ with command line testusb -a -t 7 -c 2000 -s 4096 -v 41 The test sends trash bytes to the gadget instead of only zero bytes and so the test fails. This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Acked-by: Alan Stern st...@rowland.harvard.edu CC: Ming Lei tom.leim...@gmail.com CC: Sebastian Andrzej Siewior bige...@linutronix.de CC: linux-usb@vger.kernel.org this is not the correct way to send a patch. Subject line is wrong, please fix that. yep, sorry, fixed already. -- 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 v2 1/4] USB: HCD: support giveback of URB in tasklet context
On Mon, 24 Jun 2013, Ming Lei wrote:
This patch implements the mechanism of giveback of URB in
tasklet context, so that hardware interrupt handling time for
usb host controller can be saved much, and HCD interrupt handling
can be simplified.
Changes from v1 to v2?
+static void usb_giveback_urb_bh(unsigned long param)
+{
+ struct giveback_urb_bh *bh = (struct giveback_urb_bh *)param;
+ unsigned long flags;
+ struct list_head local_list;
+
+ spin_lock_irqsave(bh-lock, flags);
+ bh-running = 1;
+ restart:
+ list_replace_init(bh-head, local_list);
+ spin_unlock_irqrestore(bh-lock, flags);
Tasklet routines are always called with interrupts enabled, right?
Therefore you don't need to use the flags argument here or below.
@@ -1667,25 +1721,40 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
*/
void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
{
- urb-hcpriv = NULL;
- if (unlikely(urb-unlinked))
- status = urb-unlinked;
- else if (unlikely((urb-transfer_flags URB_SHORT_NOT_OK)
- urb-actual_length urb-transfer_buffer_length
- !status))
- status = -EREMOTEIO;
+ struct giveback_urb_bh *bh;
+ bool sched, high_prio_bh;
- unmap_urb_for_dma(hcd, urb);
- usbmon_urb_complete(hcd-self, urb, status);
- usb_unanchor_urb(urb);
+ /* pass status to tasklet via unlinked */
+ if (likely(!urb-unlinked))
+ urb-unlinked = status;
- /* pass ownership to the completion handler */
- urb-status = status;
- urb-complete (urb);
- atomic_dec (urb-use_count);
- if (unlikely(atomic_read(urb-reject)))
- wake_up (usb_kill_urb_queue);
- usb_put_urb (urb);
+ if (!hcd_giveback_urb_in_bh(hcd) !is_root_hub(urb-dev)) {
+ __usb_hcd_giveback_urb(urb);
+ return;
+ }
+
+ if (usb_pipeisoc(urb-pipe) || usb_pipeint(urb-pipe)) {
+ bh = hcd-high_prio_bh;
+ high_prio_bh = 1;
+ } else {
+ bh = hcd-low_prio_bh;
+ high_prio_bh = 0;
+ }
Bool values should be assigned true or false, not 1 or 0.
+
+ spin_lock(bh-lock);
+ list_add_tail(urb-urb_list, bh-head);
+ if (bh-running)
+ sched = 0;
+ else
+ sched = 1;
+ spin_unlock(bh-lock);
How about calling this variable running instead of sched? Then you
could just say:
running = bh-running;
with no if statement.
+
+ if (!sched)
+ ;
+ else if (high_prio_bh)
+ tasklet_hi_schedule(bh-bh);
+ else
+ tasklet_schedule(bh-bh);
}
EXPORT_SYMBOL_GPL(usb_hcd_giveback_urb);
@@ -2307,6 +2376,42 @@ EXPORT_SYMBOL_GPL (usb_hc_died);
/*-*/
+static void __init_giveback_urb_bh(struct giveback_urb_bh *bh)
+{
+
+ spin_lock_init(bh-lock);
+ INIT_LIST_HEAD(bh-head);
+ tasklet_init(bh-bh, usb_giveback_urb_bh, (unsigned long)bh);
+}
+
+static void init_giveback_urb_bh(struct usb_hcd *hcd)
+{
+ if (!hcd_giveback_urb_in_bh(hcd))
+ return;
As you mentioned, there's not much point in skipping this code when
it's not needed. In fact, you could just put the two lines below
directly into usb_create_shared_hcd(), and get rid of the __ from the
beginning of the name.
+
+ __init_giveback_urb_bh(hcd-high_prio_bh);
+ __init_giveback_urb_bh(hcd-low_prio_bh);
+}
+
+static void exit_giveback_urb_bh(struct usb_hcd *hcd)
+{
+ if (!hcd_giveback_urb_in_bh(hcd))
+ return;
+
+ /*
+ * tasklet_kill() isn't needed here because:
+ * - driver's disconnec() called from usb_disconnect() should
Misspelled disconnect.
+ * make sure its URBs are completed during the disconnect()
+ * callback
+ *
+ * - it is too late to run complete() here since driver may have
+ * been removed already now
+ *
+ * Using tasklet to run URB's complete() doesn't change this
+ * behavior of usbcore.
+ */
+}
Why have a separate subroutine for doing nothing? Simply put this
comment directly into usb_remove_hcd(). (And you can remove the last
two lines of the comment; they don't make sense in this context.)
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
Re: [PATCH v2 2/4] USB: URB documentation: claim complete() will be run with IRQs enabled
On Mon, 24 Jun 2013, Ming Lei wrote: There is no good reason to run complete() in hard interrupt disabled context. After switch to run complete() in tasklet, we will enable local IRQs when calling complete() since we can do it at that time. Even though we still disable IRQs now when calling complete() in tasklet, the URB documentation is updated to claim complete() will be run in tasklet context and local IRQs will be enabled, so that USB drivers can know the change and avoid one deadlock caused by: assume IRQs disabled in complete() and call spin_lock() to hold lock which might be acquired in interrupt context. Current spin_lock() usages in drivers' complete() will be cleaned up at the same time, and once the cleanup is finished, local IRQs will be enabled when calling complete() in tasklet. Also fix description about type of usb_complete_t, and remove the advice of running completion handler in tasklet for decreasing system latency. Cc: Oliver Neukum oli...@neukum.org Cc: Alan Stern st...@rowland.harvard.edu Signed-off-by: Ming Lei ming@canonical.com --- Documentation/usb/URB.txt | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Documentation/usb/URB.txt b/Documentation/usb/URB.txt index 00d2c64..50da0d4 100644 --- a/Documentation/usb/URB.txt +++ b/Documentation/usb/URB.txt @@ -195,13 +195,12 @@ by the completion handler. The handler is of the following type: - typedef void (*usb_complete_t)(struct urb *, struct pt_regs *) + typedef void (*usb_complete_t)(struct urb *) -I.e., it gets the URB that caused the completion call, plus the -register values at the time of the corresponding interrupt (if any). -In the completion handler, you should have a look at urb-status to -detect any USB errors. Since the context parameter is included in the URB, -you can pass information to the completion handler. +I.e., it gets the URB that caused the completion call. In the completion +handler, you should have a look at urb-status to detect any USB errors. +Since the context parameter is included in the URB, you can pass +information to the completion handler. Note that even when an error (or unlink) is reported, data may have been transferred. That's because USB transfers are packetized; it might take @@ -210,12 +209,12 @@ have transferred successfully before the completion was called. NOTE: * WARNING * -NEVER SLEEP IN A COMPLETION HANDLER. These are normally called -during hardware interrupt processing. If you can, defer substantial -work to a tasklet (bottom half) to keep system latencies low. You'll -probably need to use spinlocks to protect data structures you manipulate -in completion handlers. +NEVER SLEEP IN A COMPLETION HANDLER. These are often called in atomic +context. +In the current kernel, completion handlers run with local interrupts +disabled, but in the future this will be changed, so don't assume that +local IRQs are always disabled inside completion handlers. 1.8. How to do isochronous (ISO) transfers? Acked-by: Alan Stern st...@rowland.harvard.edu -- 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: [v4] usb: UHCI: fix pkt size in TD for a sg element
Hi, On Mon, Jun 24, 2013 at 05:12:50PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 05:04 PM, Felipe Balbi wrote: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. But this is almost kidergarder. We have only mass storage using this which does not do this. You can't even merge sgs at that level so that you run in such a problem. So this is simply a tool used wrong. I agree with the patch Alan acked. right, $subject is just a new version with improved subject line :-) -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 11:04 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Jun 24, 2013 at 10:56:30PM +0800, Ming Lei wrote: On Mon, Jun 24, 2013 at 10:33 PM, Denis V. Lunev d...@openvz.org wrote: From: Konstantin Filatov kfila...@parallels.com This patch shortens TD's packet not only for the last TD in sg list, but also for the last TD in sg element. Signed-off-by: Konstantin Filatov kfila...@parallels.com Signed-off-by: Denis V. Lunev d...@openvz.org Considered that: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. Per USB spec, short packet can only be the last packet of one transfer. In our linux USB implementation, we think the sg list passed to URB or one URB as one single transfer, so the middle short packet will cause the URB to be understood as two or more transfers by device. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. Suggest to see the discussion in the below link: http://marc.info/?t=13718225823r=1w=2 - the problem should be avoided in usbcore since it isn't a uhci specific problem to this I agree. - this patch only hides problem, doesn't help to fix real problem. what is the problem then ? Are you saying that it's wrong to have an sg which is not aligned to wMaxPacketSize somewhere in the middle of an sg-table ? How so ? What does that have to do with USB at all ? The problem is that short packet can only be the last packet of one transfer. Thanks, -- Ming Lei -- 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 7/7] usb: phy: msm: Lindent the code
From: Ivan T. Ivanov iiva...@mm-sol.com
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/usb/phy/phy-msm-usb.c | 99 ++---
1 file changed, 52 insertions(+), 47 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 6d05085..111f454 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -45,18 +45,18 @@
#define ULPI_IO_TIMEOUT_USEC (10 * 1000)
-#define USB_PHY_3P3_VOL_MIN305 /* uV */
-#define USB_PHY_3P3_VOL_MAX330 /* uV */
+#define USB_PHY_3P3_VOL_MIN305 /* uV */
+#define USB_PHY_3P3_VOL_MAX330 /* uV */
#define USB_PHY_3P3_HPM_LOAD 5 /* uA */
#define USB_PHY_3P3_LPM_LOAD 4000/* uA */
-#define USB_PHY_1P8_VOL_MIN180 /* uV */
-#define USB_PHY_1P8_VOL_MAX180 /* uV */
+#define USB_PHY_1P8_VOL_MIN180 /* uV */
+#define USB_PHY_1P8_VOL_MAX180 /* uV */
#define USB_PHY_1P8_HPM_LOAD 5 /* uA */
#define USB_PHY_1P8_LPM_LOAD 4000/* uA */
-#define USB_PHY_VDD_DIG_VOL_MIN100 /* uV */
-#define USB_PHY_VDD_DIG_VOL_MAX132 /* uV */
+#define USB_PHY_VDD_DIG_VOL_MIN100 /* uV */
+#define USB_PHY_VDD_DIG_VOL_MAX132 /* uV */
static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init)
{
@@ -64,8 +64,8 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
if (init) {
ret = regulator_set_voltage(motg-vddcx,
- USB_PHY_VDD_DIG_VOL_MIN,
- USB_PHY_VDD_DIG_VOL_MAX);
+ USB_PHY_VDD_DIG_VOL_MIN,
+ USB_PHY_VDD_DIG_VOL_MAX);
if (ret) {
dev_err(motg-phy.dev, Cannot set vddcx voltage\n);
return ret;
@@ -73,15 +73,17 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
ret = regulator_enable(motg-vddcx);
if (ret)
- dev_err(motg-phy.dev, unable to enable hsusb
vddcx\n);
+ dev_err(motg-phy.dev,
+ unable to enable hsusb vddcx\n);
} else {
ret = regulator_set_voltage(motg-vddcx, 0,
- USB_PHY_VDD_DIG_VOL_MAX);
+ USB_PHY_VDD_DIG_VOL_MAX);
if (ret)
dev_err(motg-phy.dev, Cannot set vddcx voltage\n);
ret = regulator_disable(motg-vddcx);
if (ret)
- dev_err(motg-phy.dev, unable to disable hsusb
vddcx\n);
+ dev_err(motg-phy.dev,
+ unable to disable hsusb vddcx\n);
}
return ret;
@@ -93,26 +95,28 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int
init)
if (init) {
rc = regulator_set_voltage(motg-v3p3, USB_PHY_3P3_VOL_MIN,
- USB_PHY_3P3_VOL_MAX);
+ USB_PHY_3P3_VOL_MAX);
if (rc) {
dev_err(motg-phy.dev, Cannot set v3p3 voltage\n);
return rc;
}
rc = regulator_enable(motg-v3p3);
if (rc) {
- dev_err(motg-phy.dev, unable to enable the hsusb
3p3\n);
+ dev_err(motg-phy.dev,
+ unable to enable the hsusb 3p3\n);
return rc;
}
rc = regulator_set_voltage(motg-v1p8, USB_PHY_1P8_VOL_MIN,
- USB_PHY_1P8_VOL_MAX);
+ USB_PHY_1P8_VOL_MAX);
if (rc) {
dev_err(motg-phy.dev, Cannot set v1p8 voltage\n);
goto disable_3p3;
}
rc = regulator_enable(motg-v1p8);
if (rc) {
- dev_err(motg-phy.dev, unable to enable the hsusb
1p8\n);
+ dev_err(motg-phy.dev,
+ unable to enable the hsusb 1p8\n);
goto disable_3p3;
}
@@ -156,26 +160,26 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
if (on) {
ret = regulator_set_optimum_mode(motg-v1p8,
- USB_PHY_1P8_HPM_LOAD);
+USB_PHY_1P8_HPM_LOAD);
if (ret 0) {
pr_err(Could not set HPM for v1p8\n);
return ret;
}
ret = regulator_set_optimum_mode(motg-v3p3,
-
[PATCH 6/7] usb: phy: msm: Fix WARNING: Prefer seq_puts to seq_printf
From: Ivan T. Ivanov iiva...@mm-sol.com
This fixes checkpatch.pl warnings.
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/usb/phy/phy-msm-usb.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 41938e6..6d05085 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1204,13 +1204,13 @@ static int msm_otg_mode_show(struct seq_file *s, void
*unused)
switch (otg-phy-state) {
case OTG_STATE_A_HOST:
- seq_printf(s, host\n);
+ seq_puts(s, host\n);
break;
case OTG_STATE_B_PERIPHERAL:
- seq_printf(s, peripheral\n);
+ seq_puts(s, peripheral\n);
break;
default:
- seq_printf(s, none\n);
+ seq_puts(s, none\n);
break;
}
--
1.7.9.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
[PATCH 5/7] usb: phy: msm: Fix WARNING: quoted string split across lines
From: Ivan T. Ivanov iiva...@mm-sol.com
This fixes checkpatch.pl warnings.
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/usb/phy/phy-msm-usb.c | 33 +++--
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 0e7d7ab..41938e6 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -67,8 +67,7 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
USB_PHY_VDD_DIG_VOL_MIN,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret) {
- dev_err(motg-phy.dev, unable to set the voltage
- for hsusb vddcx\n);
+ dev_err(motg-phy.dev, Cannot set vddcx voltage\n);
return ret;
}
@@ -79,8 +78,7 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int
init)
ret = regulator_set_voltage(motg-vddcx, 0,
USB_PHY_VDD_DIG_VOL_MAX);
if (ret)
- dev_err(motg-phy.dev, unable to set the voltage
- for hsusb vddcx\n);
+ dev_err(motg-phy.dev, Cannot set vddcx voltage\n);
ret = regulator_disable(motg-vddcx);
if (ret)
dev_err(motg-phy.dev, unable to disable hsusb
vddcx\n);
@@ -97,8 +95,7 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int init)
rc = regulator_set_voltage(motg-v3p3, USB_PHY_3P3_VOL_MIN,
USB_PHY_3P3_VOL_MAX);
if (rc) {
- dev_err(motg-phy.dev, unable to set voltage level
- for hsusb 3p3\n);
+ dev_err(motg-phy.dev, Cannot set v3p3 voltage\n);
return rc;
}
rc = regulator_enable(motg-v3p3);
@@ -110,8 +107,7 @@ static int msm_hsusb_ldo_init(struct msm_otg *motg, int
init)
rc = regulator_set_voltage(motg-v1p8, USB_PHY_1P8_VOL_MIN,
USB_PHY_1P8_VOL_MAX);
if (rc) {
- dev_err(motg-phy.dev, unable to set voltage level
- for hsusb 1p8\n);
+ dev_err(motg-phy.dev, Cannot set v1p8 voltage\n);
goto disable_3p3;
}
rc = regulator_enable(motg-v1p8);
@@ -144,8 +140,7 @@ static int msm_hsusb_config_vddcx(struct msm_otg *motg, int
high)
ret = regulator_set_voltage(motg-vddcx, min_vol, max_vol);
if (ret) {
- pr_err(%s: unable to set the voltage for regulator
- HSUSB_VDDCX\n, __func__);
+ dev_err(motg-phy.dev, Cannot set vddcx voltage\n);
return ret;
}
@@ -163,15 +158,13 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
ret = regulator_set_optimum_mode(motg-v1p8,
USB_PHY_1P8_HPM_LOAD);
if (ret 0) {
- pr_err(%s: Unable to set HPM of the regulator
- HSUSB_1p8\n, __func__);
+ pr_err(Could not set HPM for v1p8\n);
return ret;
}
ret = regulator_set_optimum_mode(motg-v3p3,
USB_PHY_3P3_HPM_LOAD);
if (ret 0) {
- pr_err(%s: Unable to set HPM of the regulator
- HSUSB_3p3\n, __func__);
+ pr_err(Could not set HPM for v3p3\n);
regulator_set_optimum_mode(motg-v1p8,
USB_PHY_1P8_LPM_LOAD);
return ret;
@@ -180,13 +173,11 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
ret = regulator_set_optimum_mode(motg-v1p8,
USB_PHY_1P8_LPM_LOAD);
if (ret 0)
- pr_err(%s: Unable to set LPM of the regulator
- HSUSB_1p8\n, __func__);
+ pr_err(Could not set LPM for v1p8\n);
ret = regulator_set_optimum_mode(motg-v3p3,
USB_PHY_3P3_LPM_LOAD);
if (ret 0)
- pr_err(%s: Unable to set LPM of the regulator
- HSUSB_3p3\n, __func__);
+ pr_err(Could not set LPM for v3p3\n);
}
pr_debug(reg (%s)\n, on ? HPM : LPM);
@@ -519,8 +510,7 @@ static int msm_otg_resume(struct msm_otg *motg)
[PATCH 2/7] usb: phy: msm: Migrate to Managed Device Resource allocation
From: Ivan T. Ivanov iiva...@mm-sol.com
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/usb/phy/phy-msm-usb.c | 78 +++--
1 file changed, 20 insertions(+), 58 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ab1b880..cc37f5e 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1397,13 +1397,14 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
return -ENODEV;
}
- motg = kzalloc(sizeof(struct msm_otg), GFP_KERNEL);
+ motg = devm_kzalloc(pdev-dev, sizeof(*motg), GFP_KERNEL);
if (!motg) {
dev_err(pdev-dev, unable to allocate msm_otg\n);
return -ENOMEM;
}
- motg-phy.otg = kzalloc(sizeof(struct usb_otg), GFP_KERNEL);
+ motg-phy.otg = devm_kzalloc(pdev-dev, sizeof(*motg-phy.otg),
+GFP_KERNEL);
if (!motg-phy.otg) {
dev_err(pdev-dev, unable to allocate msm_otg\n);
return -ENOMEM;
@@ -1413,18 +1414,16 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
phy = motg-phy;
phy-dev = pdev-dev;
- motg-phy_reset_clk = clk_get(pdev-dev, usb_phy_clk);
+ motg-phy_reset_clk = devm_clk_get(pdev-dev, usb_phy_clk);
if (IS_ERR(motg-phy_reset_clk)) {
dev_err(pdev-dev, failed to get usb_phy_clk\n);
- ret = PTR_ERR(motg-phy_reset_clk);
- goto free_motg;
+ return PTR_ERR(motg-phy_reset_clk);
}
- motg-clk = clk_get(pdev-dev, usb_hs_clk);
+ motg-clk = devm_clk_get(pdev-dev, usb_hs_clk);
if (IS_ERR(motg-clk)) {
dev_err(pdev-dev, failed to get usb_hs_clk\n);
- ret = PTR_ERR(motg-clk);
- goto put_phy_reset_clk;
+ return PTR_ERR(motg-clk);
}
clk_set_rate(motg-clk, 6000);
@@ -1436,21 +1435,19 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
* on pclk source
*/
if (motg-pdata-pclk_src_name) {
- motg-pclk_src = clk_get(pdev-dev,
+ motg-pclk_src = devm_clk_get(pdev-dev,
motg-pdata-pclk_src_name);
if (IS_ERR(motg-pclk_src))
- goto put_clk;
+ return PTR_ERR(motg-pclk_src);
clk_set_rate(motg-pclk_src, INT_MAX);
clk_prepare_enable(motg-pclk_src);
} else
motg-pclk_src = ERR_PTR(-ENOENT);
-
- motg-pclk = clk_get(pdev-dev, usb_hs_pclk);
+ motg-pclk = devm_clk_get(pdev-dev, usb_hs_pclk);
if (IS_ERR(motg-pclk)) {
dev_err(pdev-dev, failed to get usb_hs_pclk\n);
- ret = PTR_ERR(motg-pclk);
- goto put_pclk_src;
+ return PTR_ERR(motg-pclk);
}
/*
@@ -1458,30 +1455,27 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
* clock is introduced to remove the dependency on AXI
* bus frequency.
*/
- motg-core_clk = clk_get(pdev-dev, usb_hs_core_clk);
+ motg-core_clk = devm_clk_get(pdev-dev, usb_hs_core_clk);
if (IS_ERR(motg-core_clk))
motg-core_clk = NULL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(pdev-dev, failed to get platform resource mem\n);
- ret = -ENODEV;
- goto put_core_clk;
+ return -ENODEV;
}
- motg-regs = ioremap(res-start, resource_size(res));
+ motg-regs = devm_ioremap_resource(pdev-dev, res);
if (!motg-regs) {
dev_err(pdev-dev, ioremap failed\n);
- ret = -ENOMEM;
- goto put_core_clk;
+ return PTR_ERR(motg-regs);
}
dev_info(pdev-dev, OTG regs = %p\n, motg-regs);
motg-irq = platform_get_irq(pdev, 0);
- if (!motg-irq) {
+ if (motg-irq 0) {
dev_err(pdev-dev, platform_get_irq failed\n);
- ret = -ENODEV;
- goto free_regs;
+ return motg-irq;
}
clk_prepare_enable(motg-clk);
@@ -1490,7 +1484,7 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
ret = msm_hsusb_init_vddcx(motg, 1);
if (ret) {
dev_err(pdev-dev, hsusb vddcx configuration failed\n);
- goto free_regs;
+ return ret;
}
ret = msm_hsusb_ldo_init(motg, 1);
@@ -1512,7 +1506,7 @@ static int __init msm_otg_probe(struct platform_device
*pdev)
INIT_WORK(motg-sm_work, msm_otg_sm_work);
INIT_DELAYED_WORK(motg-chg_work, msm_chg_detect_work);
-
[PATCH 4/7] usb: phy: msm: Remove unnecessarily check for valid regulators.
From: Ivan T. Ivanov iiva...@mm-sol.com
Whether regulators are available or not is checked at driver
probe. If they are not available driver will refuse to load,
so no need to check them again.
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/usb/phy/phy-msm-usb.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8289270..0e7d7ab 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,16 +159,6 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg,
int on)
{
int ret = 0;
- if (!motg-v1p8 || IS_ERR(motg-v1p8)) {
- pr_err(%s: HSUSB_1p8 is not initialized\n, __func__);
- return -ENODEV;
- }
-
- if (!motg-v3p3 || IS_ERR(motg-v3p3)) {
- pr_err(%s: HSUSB_3p3 is not initialized\n, __func__);
- return -ENODEV;
- }
-
if (on) {
ret = regulator_set_optimum_mode(motg-v1p8,
USB_PHY_1P8_HPM_LOAD);
--
1.7.9.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: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, 24 Jun 2013, Felipe Balbi wrote: Hi, On Mon, Jun 24, 2013 at 05:12:50PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 05:04 PM, Felipe Balbi wrote: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. That simply isn't true. The DMA engine in EHCI, for example, is not capable of constructing a packet out of discontiguous memory buffers. (Unless the discontinuities all occur exactly on page boundaries, but that's not what we're discussing here.) Furthermore, if the HCD doesn't have DMA support then we break the transfer up into multiple URBs, each corresponding to a single SG element. The data in the SG buffers do not get rearranged, so you would inevitably end up with a short packet at the end of one of the intermediate URBs. 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
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On 06/24/2013 05:18 PM, Felipe Balbi wrote: It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. The DMA engines (intern or extern) do not merge transfers. Some of them are clever enough to transfer a single 1024 as two 512 requests. But two 256 bytes requests are not merged into one 512 request. I believe the device side of dwc3 is able to do so if marked properly but musb host side won't be able to do so. Sebastian -- 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, Ok, is the usb_ep_set_wedge routine not working? I can't see that in the log file. It is not working. This can be seen in the usbmon log. I re-attach the usbmon log. If possible, please show me which line indicates that usb_ep_set_wedge routine is not working, or how to look for the clue. Is it from the control transfer line? Now, in USB 2.0 CV test, there is an error about GET_STATUS request, as shown below. g_file_storage gadget: ep0-setup, length 8: : 82 00 00 00 81 00 02 00 g_file_storage gadget: unknown control req 82.00 v i0081 l2 handle_setup status -95 I think the GET_STATUS request is not handled by the gadget driver. Isn't it so? That's right. Get-Status, Set-Feature, and Clear-Feature requests must be handled by the UDC driver. Alan Stern Should the UDC driver handle Get-Status before or after the call to fsg_setup()? thanks, victor f2e9da80 3086290883 S Ci:1:001:0 s a3 00 0001 0004 4 f2e9da80 3086290911 C Ci:1:001:0 0 4 = 0001 f2e9da80 3086290919 S Ci:1:001:0 s a3 00 0002 0004 4 f2e9da80 3086290923 C Ci:1:001:0 0 4 = 0001 f2e9da80 3086290927 S Ci:1:001:0 s a3 00 0003 0004 4 f2e9da80 3086290931 C Ci:1:001:0 0 4 = 0001 f2e9da80 3086290936 S Ci:1:001:0 s a3 00 0004 0004 4 f2e9da80 3086290940 C Ci:1:001:0 0 4 = 0001 f2e9da80 3086290944 S Ci:1:001:0 s a3 00 0005 0004 4 f2e9da80 3086290958 C Ci:1:001:0 0 4 = 03050400 f2e9da80 3086290963 S Ci:1:001:0 s a3 00 0006 0004 4 f2e9da80 3086290967 C Ci:1:001:0 0 4 = 0001 f6816a80 3086290972 S Ii:1:001:1 -115:2048 4 f2e9da80 3086291359 S Ci:1:001:0 s a3 00 0005 0004 4 f2e9da80 3086291366 C Ci:1:001:0 0 4 = 03050400 f2e9da80 3086291372 S Co:1:001:0 s 23 01 0012 0005 0 f2e9da80 3086291378 C Co:1:001:0 0 0 f2e9d100 3086307426 S Ci:1:001:0 s a3 00 0005 0004 4 f2e9d100 3086307441 C Ci:1:001:0 0 4 = 0305 f2e9d100 3086307450 S Ci:1:003:0 s 80 00 0002 2 f2e9d100 3086307599 C Ci:1:003:0 0 2 = 0300 f2e9d100 3086308101 S Co:1:003:0 s 00 01 0001 0 f2e9d100 3086308971 C Co:1:003:0 0 0 f2e9d100 3086308995 S Ci:1:003:0 s a3 00 0001 0004 4 f2e9d100 3086309344 C Ci:1:003:0 0 4 = 0001 f2e9d100 3086309362 S Ci:1:003:0 s a3 00 0002 0004 4 f2e9d100 3086309718 C Ci:1:003:0 0 4 = 01010100 f2e9d100 3086309736 S Co:1:003:0 s 23 01 0010 0002 0 f2e9d100 3086310093 C Co:1:003:0 0 0 f2e9d100 3086310110 S Ci:1:003:0 s a3 00 0003 0004 4 f2e9d100 3086310466 C Ci:1:003:0 0 4 = 0001 f2e9d100 3086310478 S Ci:1:003:0 s a3 00 0004 0004 4 f2e9d100 3086310842 C Ci:1:003:0 0 4 = 0001 f3369680 3086414872 S Ii:1:003:1 -115:2048 1 f33a1c00 3086414952 S Ci:1:003:0 s a3 00 0002 0004 4 f33a1c00 3086415358 C Ci:1:003:0 0 4 = 0101 f33a1c00 3086415413 S Co:1:003:0 s 23 03 0016 0002 0 f33a1c00 3086415717 C Co:1:003:0 0 0 f33a1c00 3086415763 S Co:1:003:0 s 23 03 0004 0002 0 f33a1c00 3086416093 C Co:1:003:0 0 0 f33a1600 3086430814 S Ci:1:003:0 s a3 00 0002 0004 4 f33a1600 3086431836 C Ci:1:003:0 0 4 = 03011000 f3369680 3086450282 C Ii:1:003:1 0:2048 1 = 04 f3369680 3086450297 S Ii:1:003:1 -115:2048 1 f33fa980 3086486796 S Co:1:003:0 s 23 01 0014 0002 0 f33fa980 3086487826 C Co:1:003:0 0 0 f33fa980 3086487934 S Ci:1:000:0 s 80 06 0100 0040 64 f33fa980 3086488571 C Ci:1:000:0 0 18 = 12010002 0040 2505a5a4 33030102 0001 f33fa980 3086488632 S Co:1:003:0 s 23 03 0004 0002 0 f33fa980 3086488943 C Co:1:003:0 0 0 f33fa080 3086503390 S Ci:1:003:0 s a3 00 0002 0004 4 f33fa080 3086503964 C Ci:1:003:0 0 4 = 03011000 f4147b80 3086558950 S Co:1:003:0 s 23 01 0014 0002 0 f4147b80 3086559441 C Co:1:003:0 0 0 f4147b80 3086559513 S Co:1:000:0 s 00 05 000b 0 f4147b80 3086559683 C Co:1:000:0 0 0 f2eeba00 3086578950 S Ci:1:011:0 s 80 06 0100 0012 18 f2eeba00 3086580189 C Ci:1:011:0 0 18 = 12010002 0040 2505a5a4 33030102 0001 f2eeba00 3086580259 S Ci:1:011:0 s 80 06 0600 000a 10 f2eeba00 3086580806 C Ci:1:011:0 0 10 = 0a060002 0040 0100 f2eeba00 3086580883 S Ci:1:011:0 s 80 06 0200 0009 9 f2eeb500 3086580900 S Co:1:003:0 s 23 03 0016 0202 0 f2eeb500 3086581180 C Co:1:003:0 0 0 f2eeba00 3086581558 C Ci:1:011:0 0 9 = 09022000 010104c0 01 f2eeba00 3086581604 S Ci:1:011:0 s 80 06 0200 0020 32 f2eeba00 3086582182 C Ci:1:011:0 0 32 = 09022000 010104c0 01090400 00020806 50050705 81024000 00070501 0240 f2eeba00 3086582259 S Ci:1:011:0 s 80 06 0300 00ff 255 f2eeba00 3086582933 C Ci:1:011:0 0 4 = 04030904 f2eeba00 3086583014 S Ci:1:011:0 s 80 06 0302 0409 00ff 255 f2eeba00 3086583558 C Ci:1:011:0 0 54 = 36034600 69006c00 65002d00 62006100 63006b00 65006400 20005300 74006f00 f2eeba00 3086583633 S Ci:1:011:0 s 80 06 0301 0409 00ff 255 f2eeba00 3086584558 C Ci:1:011:0 0 58 = 3a034c00 69006e00 75007800 20003300 2e003400 2e003400 2b002000 77006900 f2eeba00 3086584905 S Co:1:011:0 s 00 09 0001 0 f2eeba00 3086585055 C Co:1:011:0 0 0
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
Hi, On Mon, Jun 24, 2013 at 11:32:16AM -0400, Alan Stern wrote: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. That simply isn't true. The DMA engine in EHCI, for example, is not capable of constructing a packet out of discontiguous memory buffers. /me goes read EHCI... that's a limitation on EHCI though, which can't support data scattered around memory. It must be contiguous within that page. It really doesn't have anything to do with SGs, right ? If EHCI has such a limitation than EHCI alone should have means to handle drivers which send data scaterred around memory in small chunks. Furthermore, if the HCD doesn't have DMA support then we break the transfer up into multiple URBs, each corresponding to a single SG element. The data in the SG buffers do not get rearranged, so you would inevitably end up with a short packet at the end of one of the intermediate URBs. that's, again, a limitation of the HCD. MUSB can't handle SGs at all, so we have to use a contiguous buffer which can hold the entire data to do the transfer in that case. DWC3, OTOH, can handle anything. Your data can be anywhere, and it'll just transfer it, we just need to set the 'chain' bit properly to tell the internal DMA controller that the USB transfer isn't over yet. -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 06:42:05PM +0300, Felipe Balbi wrote: On Mon, Jun 24, 2013 at 11:32:16AM -0400, Alan Stern wrote: - the change violates USB spec(1.1/2.0/3.0) I can't see how this would violate USB spec. USB specifications have no knowledge of scatter-gather. It really doesn't matter how the data gets into the HW's FIFO, as long as it *does* get there. IOW an SG table like below: sg[0].length = 512 sg[1].length = 512 sg[2].length = 20 is no different than: sg[0].length = 502 sg[1].length = 512 sg[2].length = 30 from the USB perspective, all is sees is 1044 bytes being shifted through the data lines. It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. That simply isn't true. The DMA engine in EHCI, for example, is not capable of constructing a packet out of discontiguous memory buffers. /me goes read EHCI... that's a limitation on EHCI though, which can't support data scattered around memory. It must be contiguous within that page. It really doesn't have anything to do with SGs, right ? If EHCI has such a limitation than EHCI alone should have means to handle drivers which send data scaterred around memory in small chunks. BTW, EHCI can handle short SGs in the first page. See page 97 of EHCI 1.0 spec. -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 05:35:42PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 05:18 PM, Felipe Balbi wrote: It is a little. The first USB packet has 512 vs 502 bytes on the wire. you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. The DMA engines (intern or extern) do not merge transfers. Some of them are clever enough to transfer a single 1024 as two 512 requests. But two 256 bytes requests are not merged into one 512 request. I believe the device side of dwc3 is able to do so if marked properly but musb host side won't be able to do so. right, that's why we use a temporary buffer to copy data to and do the transfer. -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On 06/24/2013 05:46 PM, Felipe Balbi wrote: The DMA engines (intern or extern) do not merge transfers. Some of them are clever enough to transfer a single 1024 as two 512 requests. But two 256 bytes requests are not merged into one 512 request. I believe the device side of dwc3 is able to do so if marked properly but musb host side won't be able to do so. right, that's why we use a temporary buffer to copy data to and do the transfer. I remember this is done on dwc3's gadget RX side because the dma engine expects a multiple of max packet size. You do this on musb as well? Sebastian -- 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] xhci: Compute last_ctx from complete set of configured endpoints.
On Tue, Jun 18, 2013 at 02:09:13PM -0700, Reilly Grant wrote:
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.
The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former as when the xHC writes
changes into the output device context array it must know how large it
is. It does not make sense for this field to only refer to the input
context.
My interpretation of the spec is that the last valid context index in
the input context was supposed to be used as the last endpoint context
that is valid for this command, not for the output context.
Looking at the spec revision from 08/2012, there is an amendment to
section 4.6.6.1 that makes this even more clear. The part of the
section that talks about what the hardware must do in order to evaluate
each endpoint and update the output context says:
Set the Context Entries field in the Output Slot Context to the index
of the last valid Endpoint Context in its Output Device Context
structure, which shall be greater to or equal to the value of the
Context Entries field in the Input Slot Context.
That makes it fairly clear that software needs to set the last valid
endpoint context index based on what endpoints are added or dropped in
the input context, not which endpoints are still valid in the output
context.
So, no, I can't accept this patch. If this fixes a real problem in some
hardware, we'll add a quirk for that hardware.
Sarah Sharp
---
drivers/usb/host/xhci.c | 52
+
1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d8f640b..aa117d1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1571,12 +1571,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_slot_ctx *slot_ctx;
- unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
int ret;
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1617,24 +1615,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
ctrl_ctx-add_flags = cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx-add_flags);
- last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx-add_flags));
- slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
- /* Update the last valid endpoint context, if we deleted the last one */
- if ((le32_to_cpu(slot_ctx-dev_info) LAST_CTX_MASK)
- LAST_CTX(last_ctx)) {
- slot_ctx-dev_info = cpu_to_le32(~LAST_CTX_MASK);
- slot_ctx-dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
- }
- new_slot_info = le32_to_cpu(slot_ctx-dev_info);
-
xhci_endpoint_zero(xhci, xhci-devs[udev-slot_id], ep);
- xhci_dbg(xhci, drop ep 0x%x, slot id %d, new drop flags = %#x, new add
flags = %#x, new slot info = %#x\n,
+ xhci_dbg(xhci, drop ep 0x%x, slot id %d, new drop flags = %#x, new add
flags = %#x\n,
(unsigned int) ep-desc.bEndpointAddress,
udev-slot_id,
(unsigned int) new_drop_flags,
- (unsigned int) new_add_flags,
- (unsigned int) new_slot_info);
+ (unsigned int) new_add_flags);
return 0;
}
@@ -1657,11 +1644,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
- struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
- unsigned int last_ctx;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
@@ -1676,7 +1661,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
return -ENODEV;
added_ctxs = xhci_get_endpoint_flag(ep-desc);
- last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
* deal with ep0 max packet size changing once we get the
@@ -1737,24 +1721,14 @@ int
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
Hi, On Mon, Jun 24, 2013 at 05:53:25PM +0200, Sebastian Andrzej Siewior wrote: On 06/24/2013 05:46 PM, Felipe Balbi wrote: The DMA engines (intern or extern) do not merge transfers. Some of them are clever enough to transfer a single 1024 as two 512 requests. But two 256 bytes requests are not merged into one 512 request. I believe the device side of dwc3 is able to do so if marked properly but musb host side won't be able to do so. right, that's why we use a temporary buffer to copy data to and do the transfer. I remember this is done on dwc3's gadget RX side because the dma engine expects a multiple of max packet size. You do this on musb as well? that's not related to SG, it's OUT transfers must be aligned on wMaxPacketSize, the size that is. Which means that even if you have an SG like so: SG[0] = 1023 SG[1] = 1025 the transfer size is still 2048 so that would work :-) For MUSB, take a look at musb_host.c and grep for memcpy() -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, 24 Jun 2013, Felipe Balbi wrote: you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. That simply isn't true. The DMA engine in EHCI, for example, is not capable of constructing a packet out of discontiguous memory buffers. /me goes read EHCI... that's a limitation on EHCI though, which can't support data scattered around memory. It must be contiguous within that page. It really doesn't have anything to do with SGs, right ? Right. If EHCI has such a limitation than EHCI alone should have means to handle drivers which send data scaterred around memory in small chunks. Not only EHCI; also UHCI, OHCI, and I don't know how many others. This is one of the reasons why David Brownell wrote the USB Scatter-Gather library code in usbcore. Furthermore, if the HCD doesn't have DMA support then we break the transfer up into multiple URBs, each corresponding to a single SG element. The data in the SG buffers do not get rearranged, so you would inevitably end up with a short packet at the end of one of the intermediate URBs. that's, again, a limitation of the HCD. MUSB can't handle SGs at all, so we have to use a contiguous buffer which can hold the entire data to do the transfer in that case. MUSB shouldn't be copying data to and from bounce buffers just to be able to handle SG tranfers. DWC3, OTOH, can handle anything. Your data can be anywhere, and it'll just transfer it, we just need to set the 'chain' bit properly to tell the internal DMA controller that the USB transfer isn't over yet. Maybe the same is true of xHCI; I don't know. In any case, those HCDs are definitely in the minority. 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
Re: [Bug] USB 2.0 Ports Dont Work on Sony Vaio Laptop
On Wed, Jun 19, 2013 at 11:52:10AM -0400, Alan Stern wrote: On Wed, 19 Jun 2013, Ming Lei wrote: There's a good chance this problem was caused by a change in the xhci-hcd driver. I am wondering why xhci-hcd may cause the problem since the affected hub is 'Intel Corp. Integrated Rate Matching Hub' which is connected to EHCI root hub. It's just a guess. You pointed out that the problem seems to be related to port routing, and the port routing code is part of the xHCI driver. The original bug report on Launchpad doesn't mention any earlier kernels working. Maybe the problem was caused by a change somewhere else, such as a change in the BIOS. It's entirely possible that the BIOS is not setting the xHCI port routing mask properly, and the xHCI driver is not switching the ports from EHCI to xHCI. My guess would be that the BIOS sets the USB 3.0 port mask, but not the USB 2.0 port mask, which would cause the USB 3.0 ports to be switched over, and not the USB 2.0 ports. It could also be that the BIOS is completely ignoring the writes to the port registers as well. Can the reporters recompile with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on? That will tell me which ports are being switched over. 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: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, Jun 24, 2013 at 11:44 PM, Felipe Balbi ba...@ti.com wrote: BTW, EHCI can handle short SGs in the first page. See page 97 of EHCI 1.0 spec. That isn't enough, EHCI requires that size of the 3 middle buffers are 4K. Thanks, -- Ming Lei -- 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: [v4] usb: UHCI: fix pkt size in TD for a sg element
Hi, On Mon, Jun 24, 2013 at 12:04:19PM -0400, Alan Stern wrote: On Mon, 24 Jun 2013, Felipe Balbi wrote: you wouldn't notice the difference. The DMA engine is the one which would read the sgtable to figure where the data is scattered, at the end of the day, SW only knows of a single 1044bytes URB and controller is required to generate proper USB packets out of that. That simply isn't true. The DMA engine in EHCI, for example, is not capable of constructing a packet out of discontiguous memory buffers. /me goes read EHCI... that's a limitation on EHCI though, which can't support data scattered around memory. It must be contiguous within that page. It really doesn't have anything to do with SGs, right ? Right. If EHCI has such a limitation than EHCI alone should have means to handle drivers which send data scaterred around memory in small chunks. Not only EHCI; also UHCI, OHCI, and I don't know how many others. This is one of the reasons why David Brownell wrote the USB Scatter-Gather library code in usbcore. fair enough. Furthermore, if the HCD doesn't have DMA support then we break the transfer up into multiple URBs, each corresponding to a single SG element. The data in the SG buffers do not get rearranged, so you would inevitably end up with a short packet at the end of one of the intermediate URBs. that's, again, a limitation of the HCD. MUSB can't handle SGs at all, so we have to use a contiguous buffer which can hold the entire data to do the transfer in that case. MUSB shouldn't be copying data to and from bounce buffers just to be able to handle SG tranfers. well, if we can assume all SG elements to be wMaxPacketSize aligned, we could just break it up into smaller URBs and avoid the copy, but I wasn't aware we could make that assumption in the host side. DWC3, OTOH, can handle anything. Your data can be anywhere, and it'll just transfer it, we just need to set the 'chain' bit properly to tell the internal DMA controller that the USB transfer isn't over yet. Maybe the same is true of xHCI; I don't know. In any case, those HCDs are definitely in the minority. not fair :-p -- balbi signature.asc Description: Digital signature
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, 24 Jun 2013, Felipe Balbi wrote: BTW, EHCI can handle short SGs in the first page. See page 97 of EHCI 1.0 spec. I'm not sure what you're talking about. SG is used for bulk transfers, whereas p. 97 is about interrupt transfers. It's true that the hardware's abilities aren't fully utilized by ehci-hcd. But since other HCDs have even stronger restrictions, I don't see much point in trying to make ehci-hcd more capable. Besides, what happens if the short SG element is in the middle of the transfer, rather than at the very beginning? Or if the short first SG element starts at a page boundary and ends somewhere inside the page? 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
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Tue, Jun 25, 2013 at 12:09:38AM +0800, Ming Lei wrote: On Mon, Jun 24, 2013 at 11:44 PM, Felipe Balbi ba...@ti.com wrote: BTW, EHCI can handle short SGs in the first page. See page 97 of EHCI 1.0 spec. That isn't enough, EHCI requires that size of the 3 middle buffers are 4K. right, I figured that much :-) -- balbi signature.asc Description: Digital signature
Re: Fwd: Re: Bug#704242: Driver for PL-2303 HX not working
On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Yesterday i got this interesting mail from Aric, who has analyzed a similar problem with this chip. I think something went wrong in the usb serial handling, but not in the driver itself. But the error is really complicate to figure out in the depth of the software. The really annoying thing is that it worked with kernel 2.6.32. That's a really long time ago, loads of things have changed since then. If someone can run 'git bisect' to track down the problem, that would be wonderful. thanks, greg k-h -- 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: [Bug] USB 2.0 Ports Dont Work on Sony Vaio Laptop
On Tue, Jun 25, 2013 at 12:06 AM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Wed, Jun 19, 2013 at 11:52:10AM -0400, Alan Stern wrote: On Wed, 19 Jun 2013, Ming Lei wrote: There's a good chance this problem was caused by a change in the xhci-hcd driver. I am wondering why xhci-hcd may cause the problem since the affected hub is 'Intel Corp. Integrated Rate Matching Hub' which is connected to EHCI root hub. It's just a guess. You pointed out that the problem seems to be related to port routing, and the port routing code is part of the xHCI driver. The original bug report on Launchpad doesn't mention any earlier kernels working. Maybe the problem was caused by a change somewhere else, such as a change in the BIOS. It's entirely possible that the BIOS is not setting the xHCI port routing mask properly, and the xHCI driver is not switching the ports from EHCI to xHCI. My guess would be that the BIOS sets the USB 3.0 port mask, but not the USB 2.0 port mask, which would cause the USB 3.0 ports to be switched over, and not the USB 2.0 ports. So looks there is still the port routing thing between xhci hub and ehci rate matching hub? It could also be that the BIOS is completely ignoring the writes to the port registers as well. Can the reporters recompile with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on? That will tell me which ports are being switched over. I will post your suggestion on Launchpad later. Thanks, -- Ming Lei -- 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 Mon, 24 Jun 2013, victor yeo wrote: Hi, Ok, is the usb_ep_set_wedge routine not working? I can't see that in the log file. It is not working. This can be seen in the usbmon log. I re-attach the usbmon log. If possible, please show me which line indicates that usb_ep_set_wedge routine is not working, or how to look for the clue. Is it from the control transfer line? Here's an example: f4148f80 308532 S Bo:1:011:1 -115 31 = 55534243 0600 c000 861a 003f00c0 00 f4148f80 308652 C Bo:1:011:1 0 31 f14c5600 308676 S Bi:1:011:1 -115 192 f14c5600 3087787651 C Bi:1:011:1 -121 16 = 0f00 080a0400 f4148f80 3087787674 S Bi:1:011:1 -115 13 f4148f80 3087803018 C Bi:1:011:1 0 13 = 55534253 0600 b000 00 The last line should have failed with a -32 error code, because the IN endpoint is supposed to be halted at this point. I think the GET_STATUS request is not handled by the gadget driver. Isn't it so? That's right. Get-Status, Set-Feature, and Clear-Feature requests must be handled by the UDC driver. Alan Stern Should the UDC driver handle Get-Status before or after the call to fsg_setup()? For these requests, the UDC driver should not call fsg_setup() at all. It should handle the request entirely by itself. 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
Re: [PATCH] xhci: Compute last_ctx from complete set of configured endpoints.
On Mon, Jun 24, 2013 at 08:59AM -0700, Sarah Sharp wrote:
On Tue, Jun 18, 2013 at 02:09:13PM -0700, Reilly Grant wrote:
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.
The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former as when the xHC writes
changes into the output device context array it must know how large it
is. It does not make sense for this field to only refer to the input
context.
My interpretation of the spec is that the last valid context index in
the input context was supposed to be used as the last endpoint context
that is valid for this command, not for the output context.
Looking at the spec revision from 08/2012, there is an amendment to
section 4.6.6.1 that makes this even more clear. The part of the
section that talks about what the hardware must do in order to evaluate
each endpoint and update the output context says:
Set the Context Entries field in the Output Slot Context to the index
of the last valid Endpoint Context in its Output Device Context
structure, which shall be greater to or equal to the value of the
Context Entries field in the Input Slot Context.
That makes it fairly clear that software needs to set the last valid
endpoint context index based on what endpoints are added or dropped in
the input context, not which endpoints are still valid in the output
context.
So, no, I can't accept this patch. If this fixes a real problem in some
hardware, we'll add a quirk for that hardware.
Thank you for the clarification. I was not aware of this addendum. Could you
forward me a link to it? The last revision I can find on the Intel web site is
from 05/2010. This fixed a problem on VMware's virtual hardware. Assuming
changing behavior will not affect any other systems I will fix it based on this
information.
---
drivers/usb/host/xhci.c | 52
+
1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d8f640b..aa117d1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1571,12 +1571,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_slot_ctx *slot_ctx;
- unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
int ret;
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1617,24 +1615,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
ctrl_ctx-add_flags = cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx-add_flags);
- last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx-add_flags));
- slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
- /* Update the last valid endpoint context, if we deleted the last one */
- if ((le32_to_cpu(slot_ctx-dev_info) LAST_CTX_MASK)
- LAST_CTX(last_ctx)) {
- slot_ctx-dev_info = cpu_to_le32(~LAST_CTX_MASK);
- slot_ctx-dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
- }
- new_slot_info = le32_to_cpu(slot_ctx-dev_info);
-
xhci_endpoint_zero(xhci, xhci-devs[udev-slot_id], ep);
- xhci_dbg(xhci, drop ep 0x%x, slot id %d, new drop flags = %#x, new add
flags = %#x, new slot info = %#x\n,
+ xhci_dbg(xhci, drop ep 0x%x, slot id %d, new drop flags = %#x, new add
flags = %#x\n,
(unsigned int) ep-desc.bEndpointAddress,
udev-slot_id,
(unsigned int) new_drop_flags,
- (unsigned int) new_add_flags,
- (unsigned int) new_slot_info);
+ (unsigned int) new_add_flags);
return 0;
}
@@ -1657,11 +1644,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
- struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
- unsigned int last_ctx;
- u32 new_add_flags, new_drop_flags, new_slot_info;
+ u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
@@ -1676,7 +1661,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct
usb_device *udev,
Re: [v4] usb: UHCI: fix pkt size in TD for a sg element
On Mon, 24 Jun 2013, Felipe Balbi wrote: MUSB shouldn't be copying data to and from bounce buffers just to be able to handle SG tranfers. well, if we can assume all SG elements to be wMaxPacketSize aligned, we could just break it up into smaller URBs and avoid the copy, but I wasn't aware we could make that assumption in the host side. Other than usbtest, the only driver using SG that I know of is usb-storage, and it does make that assumption. It works because the block layer packages the I/O up into groups of pages and the bulk maxpacket values always divide 4096 (or whatever the page size happens to be) -- except for wireless USB. And MUSB doesn't have to break anything up into smaller URBs; that's what the SG library does. DWC3, OTOH, can handle anything. Your data can be anywhere, and it'll just transfer it, we just need to set the 'chain' bit properly to tell the internal DMA controller that the USB transfer isn't over yet. Maybe the same is true of xHCI; I don't know. In any case, those HCDs are definitely in the minority. not fair :-p Ironically, it's the least-capable hardware which should have the fewest difficulties. It's easy to support SG when you're forced to use PIO. :-) 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
Subject: [v3.8 Regression] UHCI: OHCI: implement new semantics for URB_ISO_ASAP
Hi Alan, A bug was opened against the Ubuntu distro[0]. It is believed that the following commits introduced the regression: commit c44b225077bb1fb25ed5cd5c4f226897b91bedd4 Author: Alan Stern st...@rowland.harvard.edu Date: Mon Oct 1 10:32:09 2012 -0400 UHCI: implement new semantics for URB_ISO_ASAP commit 6a41b4d3fe8cd4cc95181516fc6fba7b1747a27c Author: Alan Stern st...@rowland.harvard.edu Date: Mon Oct 1 10:32:15 2012 -0400 OHCI: implement new semantics for URB_ISO_ASAP The regression was introduced as of v3.8-rc1. I see the following commit was created to address a similar issue to this: commit e1944017839d7dfbf7329fac4bdec8b4050edf5e Author: Alan Stern st...@rowland.harvard.edu Date: Tue May 14 13:57:19 2013 -0400 USB: fix latency in uhci-hcd and ohci-hcd I created a test kernel with commit e1944017839d7dfbf7329fac4bdec8b4050edf5e. However, folks affected state this bug still exists with commit e194401. It is also reported that this bug exists in the latest Mainline kernel. I see that you are the author of these patches, so I wanted to get your opinion on the issue. Thanks, Joe [0] http://pad.lv/1191603 -- 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: Hardware bug in Intel USB-2 hub?
On Wed, Jun 19, 2013 at 03:03:23PM -0400, Alan Stern wrote: Sarah: This report surfaced in Bugzilla #59011 (see especially comments #38 and #39). Toralf reports, among other things, that the integrated rate-matching hub in his ThinkPad T420 (6 Series/C200 Series chipset) isn't behaving the way it should. The particular symptom is that when the hub is suspended, it does not relay wakeup requests from a downstream port to its upstream port, if the downstream device is connected at low speed and the downstream port's suspend feature isn't set. This happens with the 3.10-rc kernels, because commit 0aa2832dd0d9 changed system suspend to do a USB global bus suspend. None of the ports on non-SuperSpeed hubs are explicitly put into suspend mode; instead, everything on the bus goes into suspend when the root hub stops sending packets. You can't do the global suspend for USB 3.0 devices. Each USB 3.0 device has to be enabled for remote wakeup by sending it a Function Wake request, and then suspended. A global suspend will not work properly, because the devices will not be configured to send a wake request. That means that if you have a USB 3.0 hub that isn't suspended when the system is placed in S3, then it will not send remote wake requests. The other problem is that USB 3.0 hubs are all self-powered. What happens to their ports when the SoFs stop on the upstream port of the hub, but the downstream ports are still active? I'm not sure the USB 3.0 spec even defines this behavior. Perhaps that's the problem with the rate matching hub as well: it just isn't expecting to stop receiving SoFs when the downstream ports are in U0. The problem is easy to test on any system running a 3.10-rc kernel. Simply plug a low-speed USB keyboard (almost all keyboards are low speed) into a USB-2 port, suspend the system, and then see if typing on the keyboard will wake up the computer. You have to enable remote wakeup via a sysfs file as well, right? I have tested a couple of external USB-2 hubs; one of them behaved correctly and one didn't. Regardless, it was surprising to see Toralf's report that an Intel hub doesn't work right. I don't have any machines with a comparable chipset, so I can't test one of those integrated hubs directly. Can somebody at Intel look into this? I don't think this is a hardware issue. I think the hardware just doesn't expect global suspend. I will ask the hardware engineers I know about it though. 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: Fwd: Re: Bug#704242: Driver for PL-2303 HX not working
Hello Greg, Am 24.06.2013 18:17, schrieb Greg KH: On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Just take this one: http://www.ebay.com/itm/USB-To-RS232-TTL-PL2303HX-Auto-Converter-Module-5V-3-3V-Output-Converter-Adapter-/370812126675 US $1.67 and it is yours. Yesterday i got this interesting mail from Aric, who has analyzed a similar problem with this chip. I think something went wrong in the usb serial handling, but not in the driver itself. But the error is really complicate to figure out in the depth of the software. The really annoying thing is that it worked with kernel 2.6.32. That's a really long time ago, loads of things have changed since then. If someone can run 'git bisect' to track down the problem, that would be wonderful. Yes - that would reduce the problem. But i think it must be debugged with the correspondend hardware too, or not? thanks, greg k-h Cheers Karsten -- 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: Bug#704242: Driver for PL-2303 HX not working
I will gladly ship you one. Give me address details, and I'll go to the post office to send it. Personally, I have resolved to abandon the PL2303 chip, and move to FTDI instead. But for the sake of other Linux users who might buy that adapter by mistake, I am willing to donate the hardware to an able kernel hacker :-) Aric Fedida -- Mobile: +1.609.422.0311 Skype: AricFedidaNSA On Jun 24, 2013, at 12:17 PM, Greg KH g...@kroah.com wrote: On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Yesterday i got this interesting mail from Aric, who has analyzed a similar problem with this chip. I think something went wrong in the usb serial handling, but not in the driver itself. But the error is really complicate to figure out in the depth of the software. The really annoying thing is that it worked with kernel 2.6.32. That's a really long time ago, loads of things have changed since then. If someone can run 'git bisect' to track down the problem, that would be wonderful. thanks, greg k-h -- 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: Bug#704242: Driver for PL-2303 HX not working
Am 24.06.2013 18:38, schrieb Aric Fedida: I will gladly ship you one. Give me address details, and I'll go to the post office to send it. Personally, I have resolved to abandon the PL2303 chip, and move to FTDI instead. But for the sake of other Linux users who might buy that adapter by mistake, I am willing to donate the hardware to an able kernel hacker :-) Yes - i will also spend $1,67 and buy one to send to the right destination address to help to solve the problem. Just let me know ... The advantage of this universal adapters is that you can simply shorten RXD and TXD for a loop with one of the included cables. Karsten -- 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: HWA: fix device probe failure
This patch fixes a race condition that caused the HWA_HC interface probe
function to occasionally fail. The HWA_HC would attempt to register
itself with the HWA_RC by searching for a uwb_rc class device with the
same parent device ptr. If the probe function for the HWA_RC interface
had yet to run, the uwb_rc class device would not have been created
causing the look up to fail and the HWA_HC probe function to return an
error causing the device to be unusable.
The fix is for the HWA to delay registering with the HWA_RC until
receiving the command from userspace to start the wireless channel. It is
the responsibility of userspace to ensure that the uwb_rc class device has
been created before starting the HWA channel.
diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c
index 4af750e..483990c 100644
--- a/drivers/usb/host/hwa-hc.c
+++ b/drivers/usb/host/hwa-hc.c
@@ -683,12 +683,9 @@ static int hwahc_create(struct hwahc *hwahc, struct
usb_interface *iface)
wa-usb_dev = usb_get_dev(usb_dev); /* bind the USB device */
wa-usb_iface = usb_get_intf(iface);
wusbhc-dev = dev;
- wusbhc-uwb_rc = uwb_rc_get_by_grandpa(iface-dev.parent);
- if (wusbhc-uwb_rc == NULL) {
- result = -ENODEV;
- dev_err(dev, Cannot get associated UWB Host Controller\n);
- goto error_rc_get;
- }
+ /* defer getting the uwb_rc handle until it is needed since it
+* may not have been registered by the hwa_rc driver yet. */
+ wusbhc-uwb_rc = NULL;
result = wa_fill_descr(wa); /* Get the device descriptor */
if (result 0)
goto error_fill_descriptor;
@@ -731,8 +728,6 @@ error_wusbhc_create:
/* WA Descr fill allocs no resources */
error_security_create:
error_fill_descriptor:
- uwb_rc_put(wusbhc-uwb_rc);
-error_rc_get:
usb_put_intf(iface);
usb_put_dev(usb_dev);
return result;
diff --git a/drivers/usb/wusbcore/mmc.c b/drivers/usb/wusbcore/mmc.c
index 021467f..b71760c 100644
--- a/drivers/usb/wusbcore/mmc.c
+++ b/drivers/usb/wusbcore/mmc.c
@@ -195,6 +195,7 @@ int wusbhc_start(struct wusbhc *wusbhc)
struct device *dev = wusbhc-dev;
WARN_ON(wusbhc-wuie_host_info != NULL);
+ BUG_ON(wusbhc-uwb_rc == NULL);
result = wusbhc_rsv_establish(wusbhc);
if (result 0) {
@@ -276,12 +277,38 @@ int wusbhc_chid_set(struct wusbhc *wusbhc, const struct
wusb_ckhdid *chid)
}
wusbhc-chid = *chid;
}
+
+ /* register with UWB if we haven't already since we are about to start
+ the radio. */
+ if ((chid) (wusbhc-uwb_rc == NULL)) {
+ wusbhc-uwb_rc = uwb_rc_get_by_grandpa(wusbhc-dev-parent);
+ if (wusbhc-uwb_rc == NULL) {
+ result = -ENODEV;
+ dev_err(wusbhc-dev, Cannot get associated UWB Host
Controller\n);
+ goto error_rc_get;
+ }
+
+ result = wusbhc_pal_register(wusbhc);
+ if (result 0) {
+ dev_err(wusbhc-dev, Cannot register as a UWB PAL\n);
+ goto error_pal_register;
+ }
+ }
mutex_unlock(wusbhc-mutex);
if (chid)
result = uwb_radio_start(wusbhc-pal);
else
uwb_radio_stop(wusbhc-pal);
+
+ return result;
+
+error_pal_register:
+ uwb_rc_put(wusbhc-uwb_rc);
+ wusbhc-uwb_rc = NULL;
+error_rc_get:
+ mutex_unlock(wusbhc-mutex);
+
return result;
}
EXPORT_SYMBOL_GPL(wusbhc_chid_set);
diff --git a/drivers/usb/wusbcore/pal.c b/drivers/usb/wusbcore/pal.c
index d0b172c..59e100c 100644
--- a/drivers/usb/wusbcore/pal.c
+++ b/drivers/usb/wusbcore/pal.c
@@ -45,10 +45,11 @@ int wusbhc_pal_register(struct wusbhc *wusbhc)
}
/**
- * wusbhc_pal_register - unregister the WUSB HC as a UWB PAL
+ * wusbhc_pal_unregister - unregister the WUSB HC as a UWB PAL
* @wusbhc: the WUSB HC
*/
void wusbhc_pal_unregister(struct wusbhc *wusbhc)
{
- uwb_pal_unregister(wusbhc-pal);
+ if (wusbhc-uwb_rc)
+ uwb_pal_unregister(wusbhc-pal);
}
diff --git a/drivers/usb/wusbcore/reservation.c
b/drivers/usb/wusbcore/reservation.c
index 6f4fafd..ead79f7 100644
--- a/drivers/usb/wusbcore/reservation.c
+++ b/drivers/usb/wusbcore/reservation.c
@@ -80,6 +80,9 @@ int wusbhc_rsv_establish(struct wusbhc *wusbhc)
struct uwb_dev_addr bcid;
int ret;
+ if (rc == NULL)
+ return -ENODEV;
+
rsv = uwb_rsv_create(rc, wusbhc_rsv_complete_cb, wusbhc);
if (rsv == NULL)
return -ENOMEM;
diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c
index e712af3..742c607 100644
--- a/drivers/usb/wusbcore/wusbhc.c
+++ b/drivers/usb/wusbcore/wusbhc.c
@@ -325,13 +325,7 @@ int wusbhc_b_create(struct wusbhc *wusbhc)
Re: Hardware bug in Intel USB-2 hub?
On Mon, 24 Jun 2013, Sarah Sharp wrote: On Wed, Jun 19, 2013 at 03:03:23PM -0400, Alan Stern wrote: Sarah: This report surfaced in Bugzilla #59011 (see especially comments #38 and #39). Toralf reports, among other things, that the integrated rate-matching hub in his ThinkPad T420 (6 Series/C200 Series chipset) isn't behaving the way it should. The particular symptom is that when the hub is suspended, it does not relay wakeup requests from a downstream port to its upstream port, if the downstream device is connected at low speed and the downstream port's suspend feature isn't set. This happens with the 3.10-rc kernels, because commit 0aa2832dd0d9 changed system suspend to do a USB global bus suspend. None of the ports on non-SuperSpeed hubs are explicitly put into suspend mode; instead, everything on the bus goes into suspend when the root hub stops sending packets. You can't do the global suspend for USB 3.0 devices. Each USB 3.0 device has to be enabled for remote wakeup by sending it a Function Wake request, and then suspended. A global suspend will not work properly, because the devices will not be configured to send a wake request. That means that if you have a USB 3.0 hub that isn't suspended when the system is placed in S3, then it will not send remote wake requests. The other problem is that USB 3.0 hubs are all self-powered. What happens to their ports when the SoFs stop on the upstream port of the hub, but the downstream ports are still active? I'm not sure the USB 3.0 spec even defines this behavior. Perhaps that's the problem with the rate matching hub as well: it just isn't expecting to stop receiving SoFs when the downstream ports are in U0. Sure. The problem I'm talking about is specific to USB-2 hubs. Or more precisely, to non-SuperSpeed hubs -- it may well apply to the non-SuperSpeed portions of a USB-3 hub (I don't have any to test). In this case, I'm particularly concerned about the rate-matching hubs used in Intel chipsets; these are always USB-2 because they are hard-wired to the EHCI controllers. [Is there any reason you can't use global suspend with a USB-3 hub if it is attached to the computer by a USB-2 cable? As I understand it, in that case the hub's behavior is supposed to be governed by the USB-2 spec. Or is there any reason you can't use global suspend with the ports on a USB-3 hub that are currently connected at low, full, or high speed?] The problem is easy to test on any system running a 3.10-rc kernel. Simply plug a low-speed USB keyboard (almost all keyboards are low speed) into a USB-2 port, suspend the system, and then see if typing on the keyboard will wake up the computer. You have to enable remote wakeup via a sysfs file as well, right? Keyboards are enabled for remote wakeup by default. If you used a mouse instead of a keyboard then yes, you would have to enable it manually. I have tested a couple of external USB-2 hubs; one of them behaved correctly and one didn't. Regardless, it was surprising to see Toralf's report that an Intel hub doesn't work right. I don't have any machines with a comparable chipset, so I can't test one of those integrated hubs directly. Can somebody at Intel look into this? I don't think this is a hardware issue. I think the hardware just doesn't expect global suspend. If the hardware doesn't expect global suspend, that's a hardware issue. After all, global suspend is part of the USB-2 spec and we're talking about USB-2 hubs. I will ask the hardware engineers I know about it though. Thanks. 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
Re: Fwd: Re: Bug#704242: Driver for PL-2303 HX not working
On Mon, Jun 24, 2013 at 06:41:38PM +0200, Karsten Malcher wrote: Hello Greg, Am 24.06.2013 18:17, schrieb Greg KH: On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Just take this one: http://www.ebay.com/itm/USB-To-RS232-TTL-PL2303HX-Auto-Converter-Module-5V-3-3V-Output-Converter-Adapter-/370812126675 US $1.67 and it is yours. But wiring that up to a real 9pin serial port would be a pain (doable, but a pain). Is there any devices out there with a DB-9 output connector? thanks, greg k-h -- 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] staging: dwc2: fix thinko in dwc2_hc_set_even_odd_frame()
I screwed up the sense of this if() statement while porting our
vendor driver to create the dwc2 driver. This caused frame overrun
errors on periodic transfers when there were other transfers
active in the same (micro)frame.
With this fix, the dwc2 driver now works on the Raspberry Pi
platform even with the USB Ethernet controller enabled, where
before that would cause all USB devices to stop working.
Thanks to Ray Jui and Jerry Lin at Broadcom for tracking this down.
Reported-by: Ray Jui r...@broadcom.com
Signed-off-by: Paul Zimmerman pa...@synopsys.com
---
drivers/staging/dwc2/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index ac8ed15..e3a0e77 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -1148,7 +1148,7 @@ static void dwc2_hc_set_even_odd_frame(struct dwc2_hsotg
*hsotg,
if (chan-ep_type == USB_ENDPOINT_XFER_INT ||
chan-ep_type == USB_ENDPOINT_XFER_ISOC) {
/* 1 if _next_ frame is odd, 0 if it's even */
- if (dwc2_hcd_get_frame_number(hsotg) 0x1)
+ if (!(dwc2_hcd_get_frame_number(hsotg) 0x1))
*hcchar |= HCCHAR_ODDFRM;
}
}
--
1.8.2.rc0.16.g20a599e
--
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: Fwd: Re: Bug#704242: Driver for PL-2303 HX not working
Am 24.06.2013 20:18, schrieb Greg KH: On Mon, Jun 24, 2013 at 06:41:38PM +0200, Karsten Malcher wrote: Hello Greg, Am 24.06.2013 18:17, schrieb Greg KH: On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Just take this one: http://www.ebay.com/itm/USB-To-RS232-TTL-PL2303HX-Auto-Converter-Module-5V-3-3V-Output-Converter-Adapter-/370812126675 US $1.67 and it is yours. But wiring that up to a real 9pin serial port would be a pain (doable, but a pain). Is there any devices out there with a DB-9 output connector? You mean if you want to test full handshake with all signals - that's true. The advantage of the adapter is to get directly 5V and 3,3V. I don't know some other adapter with exactly this chip, because i don't need them. Only this Adapter which use the other chip ch341-uart which works without problems http://www.ebay.com/itm/USB-to-RS232-Serial-9-Pin-DB9-Adapter-PC-PDA-RS-232-/180399132966 When i think about it i never seen that someone uses the handshake for this simple serial communication. Not in communication with mobiles, ARM-boards or AVR microcontroller. The built-in UART's in the ARM and AVR don't suffer handshake signals, so that you must do it by software with additional IO ports. thanks, greg k-h Cheers Karsten -- 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 v2 3/4] USB: EHCI: improve interrupt qh unlink
On Mon, 24 Jun 2013, Ming Lei wrote:
Given interrupt URB will be resubmitted from tasklet context which
is scheduled by ehci hardware interrupt handler, and commonly only
one interrupt URB is scheduled on qh, so the qh may be unlinked
immediately once qh_completions() returns from ehci_irq(), then
the intr URB to be resubmitted in complete() can only be scheduled
and linked to hardware until the qh unlink is completed.
This patch improves this above situation, and the qh will wait for 5
milliseconds before being unlinked from hardware, if one URB is submitted
during the period, the qh is move out of unlink wait list and the
interrupt transfer can be scheduled immediately, not like before: the
transfer is only linked to hardware until previous unlink is completed.
This description is very hard to understand. I suggest rewriting it,
something like this:
ehci-hcd currently unlinks an interrupt QH when it becomes empty, that
is, after its last URB completes. This works well because in almost
all cases, the completion handler for an interrupt URB resubmits the
URB; therefore the QH doesn't become empty and doesn't get unlinked.
When we start using tasklets for URB completion, this scheme won't work
as well. The resubmission won't occur until the tasklet runs, which
will be some time after the completion is queued with the tasklet.
During that delay, the QH will be empty and so will be unlinked
unnecessarily.
To prevent this problem, this patch adds a 5-ms time delay before empty
interrupt QHs are unlinked. Most often, during that time the interrupt
URB will be resubmitted and thus we can avoid unlinking the QH.
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index f80d033..5bf67e2 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -601,12 +601,24 @@ static void qh_unlink_periodic(struct ehci_hcd *ehci,
struct ehci_qh *qh)
list_del(qh-intr_node);
}
-static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
+/* must be called with holding ehci-lock */
The comment should be:
/* caller must hold ehci-lock */
But in fact you can leave it out. Almost all the code in this file
runs with the lock held.
+static void cancel_unlink_wait_intr(struct ehci_hcd *ehci, struct ehci_qh
*qh)
{
- /* If the QH isn't linked then there's nothing we can do. */
- if (qh-qh_state != QH_STATE_LINKED)
+ if (qh-qh_state != QH_STATE_LINKED || list_empty(qh-unlink_node))
return;
+ list_del_init(qh-unlink_node);
+
+ /* avoid unnecessary CPU wakeup */
+ if (list_empty(ehci-intr_unlink_wait))
+ ehci_disable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR);
If you don't mind, can we leave out ehci_disable_event() for now and
add it after the rest of this series is merged? It will keeps things a
little simpler, and then we'll be able to use ehci_disable_event() for
the IAA watchdog timer event as well as using it here.
+}
+
+static void start_do_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
+{
+ /* if the qh is waitting for unlink, cancel it now */
s/waitt/wait/
+ cancel_unlink_wait_intr(ehci, qh);
+
qh_unlink_periodic (ehci, qh);
/* Make sure the unlinks are visible before starting the timer */
@@ -632,6 +644,45 @@ static void start_unlink_intr(struct ehci_hcd *ehci,
struct ehci_qh *qh)
}
}
+/*
+ * It is common only one intr URB is scheduled on one qh, and
+ * given complete() is run in tasklet context, introduce a bit
+ * delay to avoid unlink qh too early.
+ */
+static void __start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh,
+ bool wait)
+{
+ /* If the QH isn't linked then there's nothing we can do. */
+ if (qh-qh_state != QH_STATE_LINKED)
+ return;
+
+ if (!wait)
+ return start_do_unlink_intr(ehci, qh);
+
+ qh-unlink_cycle = ehci-intr_unlink_wait_cycle;
+
+ /* New entries go at the end of the intr_unlink_wait list */
+ list_add_tail(qh-unlink_node, ehci-intr_unlink_wait);
+
+ if (ehci-rh_state EHCI_RH_RUNNING)
+ ehci_handle_start_intr_unlinks(ehci);
+ else if (ehci-intr_unlink_wait.next == qh-unlink_node) {
+ ehci_enable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR, true);
+ ++ehci-intr_unlink_wait_cycle;
+ }
+}
+
+static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
+{
+ __start_unlink_intr(ehci, qh, false);
+}
+
+static void start_unlink_intr_wait(struct ehci_hcd *ehci,
+struct ehci_qh *qh)
+{
+ __start_unlink_intr(ehci, qh, true);
+}
This is not what I had in mind.
Instead, copy the qh-qh_state test into the beginning of
start_unlink_intr() and move the rest of start_do_unlink_intr() there.
Then you can rename __start_unlink_intr() to start_unlink_intr_wait()
Re: Fwd: Re: Bug#704242: Driver for PL-2303 HX not working
On Mon, Jun 24, 2013 at 08:37:59PM +0200, Karsten Malcher wrote: Am 24.06.2013 20:18, schrieb Greg KH: On Mon, Jun 24, 2013 at 06:41:38PM +0200, Karsten Malcher wrote: Hello Greg, Am 24.06.2013 18:17, schrieb Greg KH: On Mon, Jun 24, 2013 at 10:41:07AM +0200, Karsten Malcher wrote: Hello Greg, have you buyed one of this jinxed PL-2303 HX adapters? No, I have not been able to find one. Does anyone know where I can purchase one? Just take this one: http://www.ebay.com/itm/USB-To-RS232-TTL-PL2303HX-Auto-Converter-Module-5V-3-3V-Output-Converter-Adapter-/370812126675 US $1.67 and it is yours. But wiring that up to a real 9pin serial port would be a pain (doable, but a pain). Is there any devices out there with a DB-9 output connector? You mean if you want to test full handshake with all signals - that's true. Which is what the driver needs to test, right? And of course, everone with this problem is properly hooking up these signals, right? Otherwise, there will be problems as the pins could float and cause transmissions / receive data to stop happening. At the least, loop-back testing is important. The advantage of the adapter is to get directly 5V and 3,3V. What do you mean by this? I don't know some other adapter with exactly this chip, because i don't need them. Only this Adapter which use the other chip ch341-uart which works without problems http://www.ebay.com/itm/USB-to-RS232-Serial-9-Pin-DB9-Adapter-PC-PDA-RS-232-/180399132966 When i think about it i never seen that someone uses the handshake for this simple serial communication. Not in communication with mobiles, ARM-boards or AVR microcontroller. The built-in UART's in the ARM and AVR don't suffer handshake signals, so that you must do it by software with additional IO ports. What's wrong with software flow control? Why reinvent something that has been successfully used for 30+ years? thanks, greg k-h -- 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: HWA: fix device probe failure
On Mon, Jun 24, 2013 at 12:18:04PM -0500, Thomas Pugliese wrote: This patch fixes a race condition that caused the HWA_HC interface probe function to occasionally fail. The HWA_HC would attempt to register itself with the HWA_RC by searching for a uwb_rc class device with the same parent device ptr. If the probe function for the HWA_RC interface had yet to run, the uwb_rc class device would not have been created causing the look up to fail and the HWA_HC probe function to return an error causing the device to be unusable. The fix is for the HWA to delay registering with the HWA_RC until receiving the command from userspace to start the wireless channel. It is the responsibility of userspace to ensure that the uwb_rc class device has been created before starting the HWA channel. diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c index 4af750e..483990c 100644 -ENOSIGNEDOFFBY :( -- 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: HWA: fix device probe failure
On Mon, 24 Jun 2013, Greg KH wrote:
On Mon, Jun 24, 2013 at 12:18:04PM -0500, Thomas Pugliese wrote:
This patch fixes a race condition that caused the HWA_HC interface probe
function to occasionally fail. The HWA_HC would attempt to register
itself with the HWA_RC by searching for a uwb_rc class device with the
same parent device ptr. If the probe function for the HWA_RC interface
had yet to run, the uwb_rc class device would not have been created
causing the look up to fail and the HWA_HC probe function to return an
error causing the device to be unusable.
The fix is for the HWA to delay registering with the HWA_RC until
receiving the command from userspace to start the wireless channel. It is
the responsibility of userspace to ensure that the uwb_rc class device has
been created before starting the HWA channel.
diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c
index 4af750e..483990c 100644
-ENOSIGNEDOFFBY :(
Oops.
Signed-off-by: Thomas Pugliese thomas.pugli...@gmail.com
diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c
index 4af750e..483990c 100644
--- a/drivers/usb/host/hwa-hc.c
+++ b/drivers/usb/host/hwa-hc.c
@@ -683,12 +683,9 @@ static int hwahc_create(struct hwahc *hwahc, struct
usb_interface *iface)
wa-usb_dev = usb_get_dev(usb_dev); /* bind the USB device */
wa-usb_iface = usb_get_intf(iface);
wusbhc-dev = dev;
- wusbhc-uwb_rc = uwb_rc_get_by_grandpa(iface-dev.parent);
- if (wusbhc-uwb_rc == NULL) {
- result = -ENODEV;
- dev_err(dev, Cannot get associated UWB Host Controller\n);
- goto error_rc_get;
- }
+ /* defer getting the uwb_rc handle until it is needed since it
+* may not have been registered by the hwa_rc driver yet. */
+ wusbhc-uwb_rc = NULL;
result = wa_fill_descr(wa); /* Get the device descriptor */
if (result 0)
goto error_fill_descriptor;
@@ -731,8 +728,6 @@ error_wusbhc_create:
/* WA Descr fill allocs no resources */
error_security_create:
error_fill_descriptor:
- uwb_rc_put(wusbhc-uwb_rc);
-error_rc_get:
usb_put_intf(iface);
usb_put_dev(usb_dev);
return result;
diff --git a/drivers/usb/wusbcore/mmc.c b/drivers/usb/wusbcore/mmc.c
index 021467f..b71760c 100644
--- a/drivers/usb/wusbcore/mmc.c
+++ b/drivers/usb/wusbcore/mmc.c
@@ -195,6 +195,7 @@ int wusbhc_start(struct wusbhc *wusbhc)
struct device *dev = wusbhc-dev;
WARN_ON(wusbhc-wuie_host_info != NULL);
+ BUG_ON(wusbhc-uwb_rc == NULL);
result = wusbhc_rsv_establish(wusbhc);
if (result 0) {
@@ -276,12 +277,38 @@ int wusbhc_chid_set(struct wusbhc *wusbhc, const struct
wusb_ckhdid *chid)
}
wusbhc-chid = *chid;
}
+
+ /* register with UWB if we haven't already since we are about to start
+ the radio. */
+ if ((chid) (wusbhc-uwb_rc == NULL)) {
+ wusbhc-uwb_rc = uwb_rc_get_by_grandpa(wusbhc-dev-parent);
+ if (wusbhc-uwb_rc == NULL) {
+ result = -ENODEV;
+ dev_err(wusbhc-dev, Cannot get associated UWB Host
Controller\n);
+ goto error_rc_get;
+ }
+
+ result = wusbhc_pal_register(wusbhc);
+ if (result 0) {
+ dev_err(wusbhc-dev, Cannot register as a UWB PAL\n);
+ goto error_pal_register;
+ }
+ }
mutex_unlock(wusbhc-mutex);
if (chid)
result = uwb_radio_start(wusbhc-pal);
else
uwb_radio_stop(wusbhc-pal);
+
+ return result;
+
+error_pal_register:
+ uwb_rc_put(wusbhc-uwb_rc);
+ wusbhc-uwb_rc = NULL;
+error_rc_get:
+ mutex_unlock(wusbhc-mutex);
+
return result;
}
EXPORT_SYMBOL_GPL(wusbhc_chid_set);
diff --git a/drivers/usb/wusbcore/pal.c b/drivers/usb/wusbcore/pal.c
index d0b172c..59e100c 100644
--- a/drivers/usb/wusbcore/pal.c
+++ b/drivers/usb/wusbcore/pal.c
@@ -45,10 +45,11 @@ int wusbhc_pal_register(struct wusbhc *wusbhc)
}
/**
- * wusbhc_pal_register - unregister the WUSB HC as a UWB PAL
+ * wusbhc_pal_unregister - unregister the WUSB HC as a UWB PAL
* @wusbhc: the WUSB HC
*/
void wusbhc_pal_unregister(struct wusbhc *wusbhc)
{
- uwb_pal_unregister(wusbhc-pal);
+ if (wusbhc-uwb_rc)
+ uwb_pal_unregister(wusbhc-pal);
}
diff --git a/drivers/usb/wusbcore/reservation.c
b/drivers/usb/wusbcore/reservation.c
index 6f4fafd..ead79f7 100644
--- a/drivers/usb/wusbcore/reservation.c
+++ b/drivers/usb/wusbcore/reservation.c
@@ -80,6 +80,9 @@ int wusbhc_rsv_establish(struct wusbhc *wusbhc)
struct uwb_dev_addr bcid;
int ret;
+ if (rc == NULL)
+ return -ENODEV;
+
rsv = uwb_rsv_create(rc, wusbhc_rsv_complete_cb,
Re: Bug#704242: Driver for PL-2303 HX not working
On Mon, Jun 24, 2013 at 12:38:17PM -0400, Aric Fedida wrote: I will gladly ship you one. Give me address details, and I'll go to the post office to send it. I'll send it off-list, thanks. Personally, I have resolved to abandon the PL2303 chip, and move to FTDI instead. But for the sake of other Linux users who might buy that adapter by mistake, I am willing to donate the hardware to an able kernel hacker :-) Yes, I recommend the ftdi devices as well, the specs are availble, and the driver seems to work better because of it. thanks, greg k-h -- 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] chipidea: ci13xxx_imx: Access phy via private data
commit ea1418b5f1a (usb: chipidea: i.MX: use devm_usb_get_phy_by_phandle to get
phy) causes the USB host to miss the disconnect/connect events.
In order to reproduce this problem:
- Insert a USB thumb into the USB host port (connection is detected)
- Remove it (no disconnect event will be reported)
- Insert the USB thumb again (connection is not detected)
Fix this problem by accessing the usb_phy structure using the private data
instead of accessing a local structure.
Tested on a mx28evk board.
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
drivers/usb/chipidea/ci13xxx_imx.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 7e6f067..46f273e 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -98,7 +98,6 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
};
struct resource *res;
int ret;
- struct usb_phy *phy;
if (of_find_property(pdev-dev.of_node, fsl,usbmisc, NULL)
!usbmisc_ops)
@@ -130,14 +129,14 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
- phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
- if (!IS_ERR(phy)) {
- ret = usb_phy_init(phy);
+ data-phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
+ if (!IS_ERR(data-phy)) {
+ ret = usb_phy_init(data-phy);
if (ret) {
dev_err(pdev-dev, unable to init phy: %d\n, ret);
goto err_clk;
}
- } else if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ } else if (PTR_ERR(data-phy) == -EPROBE_DEFER) {
ret = -EPROBE_DEFER;
goto err_clk;
}
--
1.8.1.2
--
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: HWA: fix device probe failure
On Mon, Jun 24, 2013 at 02:00:06PM -0500, Thomas Pugliese wrote: On Mon, 24 Jun 2013, Greg KH wrote: On Mon, Jun 24, 2013 at 12:18:04PM -0500, Thomas Pugliese wrote: This patch fixes a race condition that caused the HWA_HC interface probe function to occasionally fail. The HWA_HC would attempt to register itself with the HWA_RC by searching for a uwb_rc class device with the same parent device ptr. If the probe function for the HWA_RC interface had yet to run, the uwb_rc class device would not have been created causing the look up to fail and the HWA_HC probe function to return an error causing the device to be unusable. The fix is for the HWA to delay registering with the HWA_RC until receiving the command from userspace to start the wireless channel. It is the responsibility of userspace to ensure that the uwb_rc class device has been created before starting the HWA channel. diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c index 4af750e..483990c 100644 -ENOSIGNEDOFFBY :( Oops. Signed-off-by: Thomas Pugliese thomas.pugli...@gmail.com Please resend in a format that I can apply it in, without having to edit the patch... third time's a charm? greg k-h -- 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: HWA: fix device probe failure (resubmit)
This patch fixes a race condition that caused the HWA_HC interface probe
function to occasionally fail. The HWA_HC would attempt to register
itself with the HWA_RC by searching for a uwb_rc class device with the
same parent device ptr. If the probe function for the HWA_RC interface
had yet to run, the uwb_rc class device would not have been created
causing the look up to fail and the HWA_HC probe function to return an
error causing the device to be unusable.
The fix is for the HWA to delay registering with the HWA_RC until
receiving the command from userspace to start the wireless channel. It is
the responsibility of userspace to ensure that the uwb_rc class device has
been created before starting the HWA channel.
Signed-off-by: Thomas Pugliese thomas.pugli...@gmail.com
diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c
index 4af750e..483990c 100644
--- a/drivers/usb/host/hwa-hc.c
+++ b/drivers/usb/host/hwa-hc.c
@@ -683,12 +683,9 @@ static int hwahc_create(struct hwahc *hwahc, struct
usb_interface *iface)
wa-usb_dev = usb_get_dev(usb_dev); /* bind the USB device */
wa-usb_iface = usb_get_intf(iface);
wusbhc-dev = dev;
- wusbhc-uwb_rc = uwb_rc_get_by_grandpa(iface-dev.parent);
- if (wusbhc-uwb_rc == NULL) {
- result = -ENODEV;
- dev_err(dev, Cannot get associated UWB Host Controller\n);
- goto error_rc_get;
- }
+ /* defer getting the uwb_rc handle until it is needed since it
+* may not have been registered by the hwa_rc driver yet. */
+ wusbhc-uwb_rc = NULL;
result = wa_fill_descr(wa); /* Get the device descriptor */
if (result 0)
goto error_fill_descriptor;
@@ -731,8 +728,6 @@ error_wusbhc_create:
/* WA Descr fill allocs no resources */
error_security_create:
error_fill_descriptor:
- uwb_rc_put(wusbhc-uwb_rc);
-error_rc_get:
usb_put_intf(iface);
usb_put_dev(usb_dev);
return result;
diff --git a/drivers/usb/wusbcore/mmc.c b/drivers/usb/wusbcore/mmc.c
index 021467f..b71760c 100644
--- a/drivers/usb/wusbcore/mmc.c
+++ b/drivers/usb/wusbcore/mmc.c
@@ -195,6 +195,7 @@ int wusbhc_start(struct wusbhc *wusbhc)
struct device *dev = wusbhc-dev;
WARN_ON(wusbhc-wuie_host_info != NULL);
+ BUG_ON(wusbhc-uwb_rc == NULL);
result = wusbhc_rsv_establish(wusbhc);
if (result 0) {
@@ -276,12 +277,38 @@ int wusbhc_chid_set(struct wusbhc *wusbhc, const struct
wusb_ckhdid *chid)
}
wusbhc-chid = *chid;
}
+
+ /* register with UWB if we haven't already since we are about to start
+ the radio. */
+ if ((chid) (wusbhc-uwb_rc == NULL)) {
+ wusbhc-uwb_rc = uwb_rc_get_by_grandpa(wusbhc-dev-parent);
+ if (wusbhc-uwb_rc == NULL) {
+ result = -ENODEV;
+ dev_err(wusbhc-dev, Cannot get associated UWB Host
Controller\n);
+ goto error_rc_get;
+ }
+
+ result = wusbhc_pal_register(wusbhc);
+ if (result 0) {
+ dev_err(wusbhc-dev, Cannot register as a UWB PAL\n);
+ goto error_pal_register;
+ }
+ }
mutex_unlock(wusbhc-mutex);
if (chid)
result = uwb_radio_start(wusbhc-pal);
else
uwb_radio_stop(wusbhc-pal);
+
+ return result;
+
+error_pal_register:
+ uwb_rc_put(wusbhc-uwb_rc);
+ wusbhc-uwb_rc = NULL;
+error_rc_get:
+ mutex_unlock(wusbhc-mutex);
+
return result;
}
EXPORT_SYMBOL_GPL(wusbhc_chid_set);
diff --git a/drivers/usb/wusbcore/pal.c b/drivers/usb/wusbcore/pal.c
index d0b172c..59e100c 100644
--- a/drivers/usb/wusbcore/pal.c
+++ b/drivers/usb/wusbcore/pal.c
@@ -45,10 +45,11 @@ int wusbhc_pal_register(struct wusbhc *wusbhc)
}
/**
- * wusbhc_pal_register - unregister the WUSB HC as a UWB PAL
+ * wusbhc_pal_unregister - unregister the WUSB HC as a UWB PAL
* @wusbhc: the WUSB HC
*/
void wusbhc_pal_unregister(struct wusbhc *wusbhc)
{
- uwb_pal_unregister(wusbhc-pal);
+ if (wusbhc-uwb_rc)
+ uwb_pal_unregister(wusbhc-pal);
}
diff --git a/drivers/usb/wusbcore/reservation.c
b/drivers/usb/wusbcore/reservation.c
index 6f4fafd..ead79f7 100644
--- a/drivers/usb/wusbcore/reservation.c
+++ b/drivers/usb/wusbcore/reservation.c
@@ -80,6 +80,9 @@ int wusbhc_rsv_establish(struct wusbhc *wusbhc)
struct uwb_dev_addr bcid;
int ret;
+ if (rc == NULL)
+ return -ENODEV;
+
rsv = uwb_rsv_create(rc, wusbhc_rsv_complete_cb, wusbhc);
if (rsv == NULL)
return -ENOMEM;
diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c
index e712af3..742c607 100644
--- a/drivers/usb/wusbcore/wusbhc.c
+++ b/drivers/usb/wusbcore/wusbhc.c
@@ -325,13 +325,7 @@ int
Re: [PATCH v7 0/9] Generic PHY Framework
Hi, On 06/18/2013 11:49 AM, Felipe Balbi wrote: On Mon, Jun 17, 2013 at 12:16:35PM +0200, Sylwester Nawrocki wrote: I have already used this API for our MIPI CSI-2/DSIM DPHYs driver, the RFC patch series can be found at [1]. Thanks, Sylwester [1] http://www.spinics.net/lists/arm-kernel/msg251666.html one comment to that series: let's make the phy names standardized, how about phy-exynos-video-mipi.c or phy-mipi-csi-dsim.c ? Yes, I have been thinking about that, wasn't sure exactly what pattern to chose. I would make it phy-exynos-mipi-video.c, phy-exynos-dsim-csis.c feels a bit odd. phy-mipi-csis-dsim.c might be to generic as MIPI CSIS stands for MIPI CSI Slave and MIPI DSIM - MIPI DSI Master. And there might be (actually are) IP blocks in an SoC that use the other MIPI Aliance standards. For the HDMI PHY I guess just phy-exynos-hdmi.c could be used. Thanks, Sylwester -- 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] staging: usbip: replace pr_warning() with pr_warn()
On Fri, Jun 21, 2013 at 03:01:04PM +0530, navin patidar wrote: pr_warn() is preferred over pr_warning(). And dev_warn() is preferred over both of them, can you convert the code to use that instead? thanks, greg k-h -- 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: [RESEND PATCH v2 1/1] usb: fix build error without CONFIG_USB_PHY
On Mon, Jun 24, 2013 at 03:23:25PM +0300, Felipe Balbi wrote: On Mon, Jun 24, 2013 at 11:15:35AM +0300, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: on i386: drivers/built-in.o: In function `ci_hdrc_probe': core.c:(.text+0x20446b): undefined reference to `of_usb_get_phy_mode' Signed-off-by: Peter Chen peter.c...@freescale.com Reported-by: Randy Dunlap rdun...@infradead.org Acked-by: Randy Dunlap rdun...@infradead.org It's actually Felipe's turf, so needs either his ack or target his tree. I'm ok with this. FWIW, Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com since the original patch went through your tree, it's better if you take it. Here's my Ack: Acked-by: Felipe Balbi ba...@ti.com No need, I'll take it, thanks. greg k-h -- 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] build some drivers only when compile-testing
On Wed, Jun 19, 2013 at 08:50:08AM +0200, Jiri Slaby wrote: On 06/18/2013 06:04 PM, Greg Kroah-Hartman wrote: So currently I have what is attached... Comments? Looks good to me, want me to queue it up through my char/misc driver tree for 3.11? If there are no objections... Whoever picks that up, I would be happy 8-). I've taken it, without the chipidea portion, through my driver-core tree. thanks, greg k-h -- 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: URB completion order, normal behavior or bug?
On 06/23/2013 09:08 PM, Alan Stern wrote:
On Sun, 23 Jun 2013, Daniel Santos wrote:
So I'm working on this MCP2210 driver which talks in 64-byte
request/response pairs using interrupt URBs (it can be used with
hid-generic, but I don't want to do that). I'm testing on a Raspberry
Pi Model B using the Raspbian 3.9.7 kernel. My usual order of things is:
Maybe you don't care about this any more, but in case you still do,
here's my perspective. This applies to the drivers in the standard
kernel; I don't know anything in particular about the driver used in
the RPi.
I submit an out URB (to rx my response)
No, an OUT URB transfers data _out_ from the computer to the device.
The response goes from the device _in_ to the computer.
I sure do appreciate this because I was initially confused about this
and am now re-confused. I presumed that out meant out of the
computer and into the USB device. However, I indeed have the variable
labeled out that I am using to receive responses and visa versa. I
have this little struct I use for both the IN and OUT URBs in my device
struct:
struct mcp2210_endpoint_proxy {
const struct usb_host_endpoint *ep;
struct urb *urb;
struct mcp2210_msg *buffer;
/* FIXME: inefficient */
u8 submitted:1;
u8 completed:1;
u8 kill:1;
u8 retry_count:4;
u8 is_dir_in:1;
};
/* then in my device struct: */
struct mcp2210_device {
struct usb_device *udev;
struct usb_interface *intf;
struct spi_master *spi_master;
struct gpio_chip *gpio_chip;
...
struct mcp2210_endpoint_proxy in;
struct mcp2210_endpoint_proxy out;
...
};
I initialize them both with a single function and it works! This is
what caused me to believe that the URB directions were described from
the USB device standpoint. (please pardon my mailer's mangling of
whitespace)
static __always_inline int init_endpoint(struct mcp2210_device *dev,
const struct usb_host_endpoint *ep)
{
int is_dir_in = !!usb_endpoint_dir_in(ep-desc);
struct mcp2210_endpoint_proxy *dest;
unsigned int pipe;
usb_complete_t complete_fn;
struct urb *urb;
if (is_dir_in) {
dest = dev-in;
pipe = usb_sndintpipe(dev-udev, ep-desc.bEndpointAddress);
complete_fn = urb_complete_in;
} else {
dest = dev-out;
pipe = usb_rcvintpipe(dev-udev, ep-desc.bEndpointAddress);
complete_fn = urb_complete_out;
}
dest-is_dir_in = is_dir_in;
if (unlikely(dest-ep)) {
mcp2210_warn(Unexpected: more than one interrupt %s endpoint
discovered, ingoring them\n,
urb_dir_str[is_dir_in]);
return 0;
}
urb = usb_alloc_urb(0, GFP_KERNEL);
if (unlikely(!urb)) {
mcp2210_err(Failed to alloc %s URB\n, urb_dir_str[is_dir_in]);
return -ENOMEM;
}
dest-buffer = usb_alloc_coherent(dev-udev, 64, GFP_KERNEL,
urb-transfer_dma);
if (unlikely(!dest-buffer)) {
usb_free_urb(urb);
mcp2210_err(Failed to alloc %s URB DMA buffer\n,
urb_dir_str[is_dir_in]);
return -ENOMEM;
}
dest-ep = ep;
dest-urb = urb;
usb_fill_int_urb(urb,
dev-udev,
pipe,
dest-buffer,
64,
complete_fn,
dev,
ep-desc.bInterval);
urb-transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
return 0;
}
This is how I use it in my probe function:
int mcp2210_probe(struct usb_interface *intf, const struct usb_device_id
*id)
{
struct usb_device *udev = interface_to_usbdev(intf);
struct usb_host_interface *intf_desc = intf-cur_altsetting;
struct mcp2210_device *dev;
struct usb_host_endpoint *ep, *ep_end;
int ret = -ENODEV;
int i;
printk(mcp2210_probe\n);
...
/* Set up interrupt endpoint information. */
ep_end = intf_desc-endpoint[intf_desc-desc.bNumEndpoints];
for (ep = intf_desc-endpoint; ep != ep_end; ++ep) {
if (!usb_endpoint_xfer_int(ep-desc))
continue;
if (init_endpoint(dev, ep))
goto error0;
}
if (unlikely(!dev-in.ep || !dev-out.ep)) {
mcp2210_err(could not find in and/or out interrupt endpoints);
goto error0;
}
...
}
I don't understand why this works and the previous didn't in light of
your comment (as well as my original assumptions). Can you see any
flaws in this? I've used !!usb_endpoint_dir_in(ep-desc) to get a one
or zero for rather or not I have the IN endpoint since I store this in a
u8:1 bitfield.
I submit an in URB (with my request)
Likewise, your request goes _out_ from the computer to the device.
My in URB completion handler is called
My out URB completion handler is called.
Because of the confusion in the terminology, it's hard to tell what
this means. I take it that normally, the request completes before the
response does (which is what you would expect).
But occasionally
Re: [PATCH V2] USB: initialize or shutdown PHY when add or remove host controller
On Tue, Jun 25, 2013 at 3:45 AM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Fri, Jun 21, 2013 at 09:07:59AM +0800, Chao Xie wrote:
On Fri, Jun 21, 2013 at 1:25 AM, Alan Stern st...@rowland.harvard.edu
wrote:
On Thu, 20 Jun 2013, Felipe Balbi wrote:
In fact, the PHY setting and handling is related to platform or SOC,
and for different SOC they can
have same EHCI HCD but they PHY handling can be different.
Omap'a case is the example, and i think some other vendors may have
silimar cases.
From above point, It is better to leave the PHY initialization and
shutdown to be done by each echi-xxx driver.
So Alan and Felipe
What are your ideas about it?
If we have so many exceptions, then sure. But eventually, the common
case should be added generically with a flag so that non-generic cases
(like OMAP) can request to handle the PHY by themselves.
Alan ?
I don't have very strong feelings about this; Felipe has much more
experience with these things.
However, when the common case is added into the core, the simplest way
to indicate that the HCD wants to handle the PHY(s) by itself will be
to leave hcd-phy set to NULL or an ERR_PTR value.
One important thing that hasn't been pointed out yet: When we move
these calls into the core, the same patch must also remove those calls
from the glue drivers that currently do set hcd-phy. And it must make
sure that the glue drivers which handle the PHY by themselves do not
set hcd-phy.
From device point of view, EHCI is a standlone component. It has the
standard sepcification, so each
SOC vendor has EHCI HCD need to follow the standards. Then we have
common EHCI HCD driver.
The PHY is outside of EHCI component, each SOC vendor may have
different PHY implementation. Then
we have PHY driver.
The EHCI glue driver ehci-xxx works like a SOC depended driver. It is
its duty to handle the'
relationship between the EHCI HCD driver and PHY driver.
that's not entirely true. We build abstractions layers so that the
commonalities can be written generically. Just look at the amount of
code I removed on v3.10 merge window by moving all other UDC drivers to
use generic constructs I introduced earlier.
It just so happens that OMAP's EHCI has two different working modes
which mandates different ways to handle the PHY, one is pretty much the
generic way (power up EHCI, then power up PHY) the other is inverted
(PHY, then EHCI), that's the only reason (as of today) we're having this
thread.
It is same as clk, irq requested by ehci-xxx driver.
clocks could be handled generically in some cases, we have pm_clk_add()
for a reason ;-)
Also, clock handling can be hidden under pm_runtime callbacks (say,
clk_enable() on -runtime_resume(), clk_disable() on
-runtime_suspend()). IRQ is actually handled by usbcore, you just pass
a handler which, in most cases, is the normal ehci_irq() handler.
But we'll get to those later, let's focus on PHY for now.
clock is another story, and i know that OMAP has full system to handle
the clock with PM runtime,
i would like to discuss it when one day you want to do it.
So i think add a flag and use usb_get_phy() is not very good.
Alan was talking about use hcd-phy as that flag, no flag would be
added. But why isn't it very good ? you didn't mention your resoning.
I maybe understand something wrong.
Using hcd-phy as a flag to indicates whether the gule driver need
EHCI HCD to help
phy operation, such as initialization and shutdown, i think it is fine.
If add another member as a flag in EHCI HCD to indicates the PHY
differences of each echi-xxx.c driver,
and handle them in EHCI HCD, i think that is not very good. Because as
you said that make
common part into EHCI HCD is the target, but this member will import
all the differences to EHCI HCD.
It is better to let the ehci-xxx.c driver to handle the differences if
it does not fit EHCI HCD's requirment
for common PHY handling just as this patch did.
It is bette to make ehci-xxx to do the phy getting and EHCI HCD
initialize it and shut down as the patch did, or let ehci-xxx to
handle the PHY as Roger said.
right, so this is what Alan suggested:
ehci-xxx.c does usb_get_phy() (or any of those variants) and sets the
returned pointer to hcd-phy. From that point on, ehci-hcd will play
with the phy, resuming and suspending at the proper locations, asking
the phy to enable wakeup capabilities and the like.
In fact, because of that, I was just considering if I should protect
usb_phy* against NULL pointers, just to make EHCI's life easier, I mean:
static inline int usb_phy_set_suspend(struct usb_phy *phy, int suspend)
{
if (!phy)
return 0;
return phy-suspend(phy, suspend);
}
This patch does not include the suspending/resumeing. It is great that you are
woking at it.
Based on the generic work is not too much, and does not look so
meaningful. I suggest that let to
RE: Chipidea usb otg support for IMX/MXS (device functionality)
No, I don't have this one available right now. I tried both MX23 Olinuxino maxi (I just mutilated the board so that I cut traces to the USB devices on the board and routed out USB gadget connector) and a custom MX23 board. Note that both have working USB peripheral mode in U-Boot too, but the driver in U-Boot is very minimalistic. You mean, the larger data transfer with USB peripheral mode is ok at u-boot? but it can't work at Linux Kernel? Best regards, Peter -- 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] chipidea: ci13xxx_imx: Access phy via private data
On Tue, Jun 25, 2013 at 2:58 AM, Fabio Estevam
fabio.este...@freescale.com wrote:
commit ea1418b5f1a (usb: chipidea: i.MX: use devm_usb_get_phy_by_phandle to
get
phy) causes the USB host to miss the disconnect/connect events.
In order to reproduce this problem:
- Insert a USB thumb into the USB host port (connection is detected)
- Remove it (no disconnect event will be reported)
- Insert the USB thumb again (connection is not detected)
Fix this problem by accessing the usb_phy structure using the private data
instead of accessing a local structure.
Tested on a mx28evk board.
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
drivers/usb/chipidea/ci13xxx_imx.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 7e6f067..46f273e 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -98,7 +98,6 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
};
struct resource *res;
int ret;
- struct usb_phy *phy;
if (of_find_property(pdev-dev.of_node, fsl,usbmisc, NULL)
!usbmisc_ops)
@@ -130,14 +129,14 @@ static int ci13xxx_imx_probe(struct platform_device
*pdev)
return ret;
}
- phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
- if (!IS_ERR(phy)) {
- ret = usb_phy_init(phy);
+ data-phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
+ if (!IS_ERR(data-phy)) {
+ ret = usb_phy_init(data-phy);
if (ret) {
dev_err(pdev-dev, unable to init phy: %d\n, ret);
goto err_clk;
}
- } else if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ } else if (PTR_ERR(data-phy) == -EPROBE_DEFER) {
ret = -EPROBE_DEFER;
goto err_clk;
}
--
1.8.1.2
Acked-by: Peter Chen peter.c...@freescale.com
--
BR,
Peter Chen
--
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 V2] USB: initialize or shutdown PHY when add or remove host controller
Hi,
On Tue, Jun 25, 2013 at 09:25:30AM +0800, Chao Xie wrote:
It is same as clk, irq requested by ehci-xxx driver.
clocks could be handled generically in some cases, we have pm_clk_add()
for a reason ;-)
Also, clock handling can be hidden under pm_runtime callbacks (say,
clk_enable() on -runtime_resume(), clk_disable() on
-runtime_suspend()). IRQ is actually handled by usbcore, you just pass
a handler which, in most cases, is the normal ehci_irq() handler.
But we'll get to those later, let's focus on PHY for now.
clock is another story, and i know that OMAP has full system to handle
the clock with PM runtime,
i would like to discuss it when one day you want to do it.
sure, anytime.
So i think add a flag and use usb_get_phy() is not very good.
Alan was talking about use hcd-phy as that flag, no flag would be
added. But why isn't it very good ? you didn't mention your resoning.
I maybe understand something wrong.
Using hcd-phy as a flag to indicates whether the gule driver need
EHCI HCD to help
phy operation, such as initialization and shutdown, i think it is fine.
If add another member as a flag in EHCI HCD to indicates the PHY
differences of each echi-xxx.c driver,
and handle them in EHCI HCD, i think that is not very good. Because as
no argument there :-)
you said that make
common part into EHCI HCD is the target, but this member will import
all the differences to EHCI HCD.
oh no, by 'flag' I meant something to tell ehci-hcd that we want to
handle PHY by ourselves, but as Alan pointed out, we don't need a
separate flag.
IOW, I didn't mean to cater for OMAP's peculiarities in the generic code
:-)
It is better to let the ehci-xxx.c driver to handle the differences if
it does not fit EHCI HCD's requirment
for common PHY handling just as this patch did.
right :-)
It is bette to make ehci-xxx to do the phy getting and EHCI HCD
initialize it and shut down as the patch did, or let ehci-xxx to
handle the PHY as Roger said.
right, so this is what Alan suggested:
ehci-xxx.c does usb_get_phy() (or any of those variants) and sets the
returned pointer to hcd-phy. From that point on, ehci-hcd will play
with the phy, resuming and suspending at the proper locations, asking
the phy to enable wakeup capabilities and the like.
In fact, because of that, I was just considering if I should protect
usb_phy* against NULL pointers, just to make EHCI's life easier, I mean:
static inline int usb_phy_set_suspend(struct usb_phy *phy, int suspend)
{
if (!phy)
return 0;
return phy-suspend(phy, suspend);
}
This patch does not include the suspending/resumeing. It is great that you are
woking at it.
yeah, I'll add that part so that ehci-hcd doesn't have to add if
(hcd-phy) all over the place.
Based on the generic work is not too much, and does not look so
meaningful. I suggest that let to echi-xxx
do it.
we'll end up with a boilerplate code in every single ehci-xxx doing
exactly the same thing. By building the common case in ehci-hcd, we can
make sure to focus efforts wrt power consumption, proper use of the phy
layer, etc in a single location which (almost) everybody shares.
The other bits which are non-generic, can use ehci-hcd as a reference to
build their own stuff.
my 2 cents
OK. I understand. I am not very fimilar with PHY suspending/resuming.
I hope that i can see the patch move all PHY handling to EHCI HCD
including suspending/resuming, so
i can change our ehci driver to fit it and continuing to push the USB
patches ;-)
suspend/resume is usually very tricky, so I'd rather leave it for later.
For now, let's just build enough ground-work as to make it easier to
think about suspend/resume later :-)
Meaning that we can just add the bare minimum (init on probe and
shutdown on remove) and add more support as we go :-)
cheers
--
balbi
signature.asc
Description: Digital signature
