Re: [PATCH] usb: gadget: fotg210-udc: Use module_platform_driver_probe macro
Hi, On Thu, Jun 20, 2013 at 8:16 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, Jun 20, 2013 at 01:53:13PM +, Yuan-Hsin Chen wrote: Because fotg210-udc is configured as a non-hotpluggable device, that makes sense to use module_platform_driver_probe() instead of module_platform_driver(). This also fixes the section mismatch issue. Signed-off-by: Yuan-Hsin Chen yhc...@faraday-tech.com I would rather see you remove __init annotation from your probe() function. The reason being that it will users to bind/unbind the driver through sysfs, which is pretty good for testing, at least. I see and will send a patch later. Thanks. Yuan-Hsin -- balbi -- 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] jacinto6 : usb3_phy: Updated dpll M,N values.
On 5/31/2013 1:24 AM, Ruchika Kharwar wrote:
Addition of the M and N recommended values for the USB3 PHY DPLL.
Sysclk for DRA7xx is 20MHz.
This yields:
Clk = 20MHz * M/(N+1) = 20MHz * 1000 /(7+1) = 2.5 Ghz
Signed-off-by: Ruchika Kharwar ruch...@ti.com
---
drivers/usb/phy/phy-omap-usb3.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index a6e60b1..efe6e14 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -27,7 +27,7 @@
#include linux/delay.h
#include linux/usb/omap_control_usb.h
-#define NUM_SYS_CLKS 5
+#defineNUM_SYS_CLKS6
#define PLL_STATUS 0x0004
#define PLL_GO 0x0008
#define PLL_CONFIGURATION1 0x000C
@@ -62,6 +62,7 @@ enum sys_clk_rate {
CLK_RATE_12MHZ,
CLK_RATE_16MHZ,
CLK_RATE_19MHZ,
+ CLK_RATE_20MHZ,
CLK_RATE_26MHZ,
CLK_RATE_38MHZ
};
@@ -72,6 +73,8 @@ static struct usb_dpll_params
omap_usb3_dpll_params[NUM_SYS_CLKS] = {
{1172, 8, 4, 20, 65537},/* 19.2 MHz */
{1250, 12, 4, 20, 0}, /* 26 MHz */
{3125, 47, 4, 20, 92843}, /* 38.4 MHz */
+ {1000, 7, 4, 10, 0},/* 20 MHz */
+
};
CLK_RATE_20MHZ is 3 but 20MHz value added in array at offset 5???
static int omap_usb3_suspend(struct usb_phy *x, int suspend)
@@ -122,6 +125,8 @@ static inline enum sys_clk_rate
__get_sys_clk_index(unsigned long rate)
return CLK_RATE_16MHZ;
case 1920:
return CLK_RATE_19MHZ;
+ case 2000:
+ return CLK_RATE_20MHZ;
case 2600:
return CLK_RATE_26MHZ;
case 3840:
--
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: host: xhci-plat: release mem region while removing module
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 --- drivers/usb/host/xhci-plat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index df90fe5..93ad67e 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -179,6 +179,7 @@ static int xhci_plat_remove(struct platform_device *dev) usb_remove_hcd(hcd); iounmap(hcd-regs); + release_mem_region(hcd-rsrc_start, hcd-rsrc_len); usb_put_hcd(hcd); kfree(xhci); -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Friday 21 June 2013 09:12:38 Ming Lei wrote: This should be enough since during remove path usbcore will wait for all URBs' completion which is only triggered by tasklet, so once all URBs are finished, the tasklet list has been empty already. No, usbcore only waits until the urb_list for each endpoint is empty. It doesn't keep track of the individual URB completions. Look at usb_hcd_flush_endpoint() and you'll see. But, if URBs still aren't completed after usb_disconnect(), it should be bug of usbcore or driver, shouldn't it? A driver cannot know what caused the call to disconnect(). That includes driver unbind. Doing IO to a device that another driver may be bound to is certainly a bug. Nevertheless it usually works and it is desirable to keep that behavior. 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: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Fri, Jun 21, 2013 at 4:33 PM, Oliver Neukum oneu...@suse.de wrote: On Friday 21 June 2013 09:12:38 Ming Lei wrote: This should be enough since during remove path usbcore will wait for all URBs' completion which is only triggered by tasklet, so once all URBs are finished, the tasklet list has been empty already. No, usbcore only waits until the urb_list for each endpoint is empty. It doesn't keep track of the individual URB completions. Look at usb_hcd_flush_endpoint() and you'll see. But, if URBs still aren't completed after usb_disconnect(), it should be bug of usbcore or driver, shouldn't it? A driver cannot know what caused the call to disconnect(). That includes driver unbind. Doing IO to a device that another driver may be bound to is certainly a bug. Nevertheless it usually works and it is desirable to keep that behavior. I mean once driver is unbound, URB's complete() in that driver shouldn't be called. The situation might happen when driver-remove() doesn't kill the URBs with the patch applied. 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: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Friday 21 June 2013 17:13:48 Ming Lei wrote: On Fri, Jun 21, 2013 at 4:33 PM, Oliver Neukum oneu...@suse.de wrote: On Friday 21 June 2013 09:12:38 Ming Lei wrote: This should be enough since during remove path usbcore will wait for all URBs' completion which is only triggered by tasklet, so once all URBs are finished, the tasklet list has been empty already. No, usbcore only waits until the urb_list for each endpoint is empty. It doesn't keep track of the individual URB completions. Look at usb_hcd_flush_endpoint() and you'll see. But, if URBs still aren't completed after usb_disconnect(), it should be bug of usbcore or driver, shouldn't it? A driver cannot know what caused the call to disconnect(). That includes driver unbind. Doing IO to a device that another driver may be bound to is certainly a bug. Nevertheless it usually works and it is desirable to keep that behavior. I mean once driver is unbound, URB's complete() in that driver shouldn't be This is highly problematic. It is bound to cause resource leaks. called. The situation might happen when driver-remove() doesn't kill the URBs with the patch applied. Well, there is no good way to handle this. But we have a simple rule. If usb_submit_urb() succeeds, complete() will be called. Breaking this rule is a bad idea. The best way is really to make sure that no URB survive. 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] staging: usbip: replace pr_warning() with pr_warn()
pr_warn() is preferred over pr_warning().
Signed-off-by: navin patidar nav...@cdac.in
---
drivers/staging/usbip/usbip_event.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/usbip_event.c
b/drivers/staging/usbip/usbip_event.c
index 82123be..64933b9 100644
--- a/drivers/staging/usbip/usbip_event.c
+++ b/drivers/staging/usbip/usbip_event.c
@@ -85,7 +85,7 @@ int usbip_start_eh(struct usbip_device *ud)
ud-eh = kthread_run(event_handler_loop, ud, usbip_eh);
if (IS_ERR(ud-eh)) {
- pr_warning(Unable to start control thread\n);
+ pr_warn(Unable to start control thread\n);
return PTR_ERR(ud-eh);
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Fri, Jun 21, 2013 at 5:20 PM, Oliver Neukum oneu...@suse.de wrote: This is highly problematic. It is bound to cause resource leaks. called. The situation might happen when driver-remove() doesn't kill the URBs with the patch applied. Well, there is no good way to handle this. But we have a simple rule. If usb_submit_urb() succeeds, complete() will be called. Breaking this rule is a bad idea. The rule should be enhanced by calling complete() before completing driver unbind. The best way is really to make sure that no URB survive. Drivers may let usbcore to do that by not setting driver-soft_unbind. I will fix the problem in v2, one solution I thought of is to flush the endpoint's URBs which have been added to tasklet list at the end of usb_hcd_flush_endpoint(). Any comments? 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: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Fri, Jun 21, 2013 at 5:43 PM, Ming Lei ming@canonical.com wrote: On Fri, Jun 21, 2013 at 5:20 PM, Oliver Neukum oneu...@suse.de wrote: Drivers may let usbcore to do that by not setting driver-soft_unbind. I will fix the problem in v2, one solution I thought of is to flush the endpoint's URBs which have been added to tasklet list at the end of usb_hcd_flush_endpoint(). Any comments? Or we can wait for completion of all URBs in the endpoint at the end of usb_hcd_flush_endpoint(), I think this approach should be easier. 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] usb: phy: msm: Move mach depndend code to platform data
From: Ivan T. Ivanov iiva...@mm-sol.com
This patch fix compilation error and is an intermediate step
before the addition of DeviceTree support for newer targets.
Fix suggested here: https://lkml.org/lkml/2013/6/19/381
Cc: David Brown dav...@codeaurora.org
Cc: Daniel Walker dwal...@fifo99.com
Cc: Bryan Huntsman bry...@codeaurora.org
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
arch/arm/mach-msm/board-msm7x30.c | 35 +++
arch/arm/mach-msm/board-qsd8x50.c | 34 +++
drivers/usb/phy/phy-msm-usb.c | 55 ++---
include/linux/usb/msm_hsusb.h |2 ++
4 files changed, 85 insertions(+), 41 deletions(-)
diff --git a/arch/arm/mach-msm/board-msm7x30.c
b/arch/arm/mach-msm/board-msm7x30.c
index db3d8c0..4df7daa 100644
--- a/arch/arm/mach-msm/board-msm7x30.c
+++ b/arch/arm/mach-msm/board-msm7x30.c
@@ -31,6 +31,7 @@
#include asm/setup.h
#include mach/board.h
+#include mach/clk.h
#include mach/msm_iomap.h
#include mach/dma.h
@@ -61,10 +62,44 @@ static int hsusb_phy_init_seq[] = {
-1
};
+static int hsusb_phy_link_clk_reset(struct clk *link_clk, bool assert)
+{
+ int ret;
+
+ if (assert) {
+ ret = clk_reset(link_clk, CLK_RESET_ASSERT);
+ if (ret)
+ pr_err(usb hs_clk assert failed\n);
+ } else {
+ ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err(usb hs_clk deassert failed\n);
+ }
+ return ret;
+}
+
+static int hsusb_phy_clk_reset(struct clk *phy_clk)
+{
+ int ret;
+
+ ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
+ if (ret) {
+ pr_err(usb phy clk assert failed\n);
+ return ret;
+ }
+ usleep_range(1, 12000);
+ ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err(usb phy clk deassert failed\n);
+ return ret;
+}
+
static struct msm_otg_platform_data msm_otg_pdata = {
.phy_init_seq = hsusb_phy_init_seq,
.mode = USB_PERIPHERAL,
.otg_control= OTG_PHY_CONTROL,
+ .phy_link_clk_reset = hsusb_phy_link_clk_reset,
+ .phy_phy_clk_reset = hsusb_phy_clk_reset,
};
struct msm_gpiomux_config msm_gpiomux_configs[GPIOMUX_NGPIOS] = {
diff --git a/arch/arm/mach-msm/board-qsd8x50.c
b/arch/arm/mach-msm/board-qsd8x50.c
index f14a73d..d3d92ab 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -82,10 +82,44 @@ static int hsusb_phy_init_seq[] = {
-1
};
+static int hsusb_phy_link_clk_reset(struct clk *link_clk, bool assert)
+{
+ int ret;
+
+ if (assert) {
+ ret = clk_reset(link_clk, CLK_RESET_ASSERT);
+ if (ret)
+ pr_err(usb hs_clk assert failed\n);
+ } else {
+ ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err(usb hs_clk deassert failed\n);
+ }
+ return ret;
+}
+
+static int hsusb_phy_clk_reset(struct clk *phy_clk)
+{
+ int ret;
+
+ ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
+ if (ret) {
+ pr_err(usb phy clk assert failed\n);
+ return ret;
+ }
+ usleep_range(1, 12000);
+ ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
+ if (ret)
+ pr_err(usb phy clk deassert failed\n);
+ return ret;
+}
+
static struct msm_otg_platform_data msm_otg_pdata = {
.phy_init_seq = hsusb_phy_init_seq,
.mode = USB_PERIPHERAL,
.otg_control= OTG_PHY_CONTROL,
+ .phy_link_clk_reset = hsusb_phy_link_clk_reset,
+ .phy_phy_clk_reset = hsusb_phy_clk_reset,
};
static struct platform_device *devices[] __initdata = {
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index d08f334..ab1b880 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -40,8 +40,6 @@
#include linux/usb/msm_hsusb_hw.h
#include linux/regulator/consumer.h
-#include mach/clk.h
-
#define MSM_USB_BASE (motg-regs)
#define DRIVER_NAMEmsm_otg
@@ -306,51 +304,20 @@ static void ulpi_init(struct msm_otg *motg)
}
}
-static int msm_otg_link_clk_reset(struct msm_otg *motg, bool assert)
-{
- int ret;
-
- if (assert) {
- ret = clk_reset(motg-clk, CLK_RESET_ASSERT);
- if (ret)
- dev_err(motg-phy.dev, usb hs_clk assert failed\n);
- } else {
- ret = clk_reset(motg-clk, CLK_RESET_DEASSERT);
- if (ret)
- dev_err(motg-phy.dev, usb hs_clk
usb: UHCI: fix pkt size in TD for a non-aligned sg element
From: Konstantin Filatov kfila...@parallels.com
Commit 689d6eac introduced an implementation of scatter-gather list
for UHCI. This implementation has a bug in case when a non-last sg
element was not aligned by TD's max-pkt-size. This bug was latent
till commit 2851784f which initializes sg_table and enables using
the implementation.
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 crashes with SIGBUS.
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
CC: Alan Stern st...@rowland.harvard.edu
CC: linux-usb@vger.kernel.org
---
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: Chipidea usb otg support for IMX/MXS (device functionality)
Hi, Hi, On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote: Indeed, I've been using the patchset Add tested id switch and vbus connect detect support for Chipidea from Peter for quite some time on top of 3.9 and it works like a charm for the gadget mode on an MX28 platform. BTW, Peter, I've seen that these patches are still not merged in 3.10, is there a reason for that? do you plan on sending a version rebased on top of 3.10 some time in the future? I tried to do the rebasing myself, but the chipidea driver seems to have changed quite heavily, which makes the process quite difficult when you don't know what you're doing :) we already have Peter's patches on v3.10-rc3 [1]. I can spend few bandwidth on upstream work recently, I may have more bandwidth after June 15th. Currently, we still have no conclusion for chipidea core driver's coming work, like Device tree support, how to identify if the controller is OTG supported. Yes, the next important step is getting the of propertys dr_mode and phy_type properly used in the chipidea core. [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary - v3.10/topic/usb-peterchen Is anyone planning to work on this stuff and start pushing it mainline or is this effort stalled completely? What is it that's missing before these can be applied? 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
[PATCH v5] usb: dwc3: use extcon fwrk to receive connect/disconnect
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 failed\n);
+ return;
+ }
+ }
val =
Re: Chipidea usb otg support for IMX/MXS (device functionality)
Hi Marek, On Fri, Jun 21, 2013 at 01:26:10PM +0200, Marek Vasut wrote: Hi, Hi, On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote: Indeed, I've been using the patchset Add tested id switch and vbus connect detect support for Chipidea from Peter for quite some time on top of 3.9 and it works like a charm for the gadget mode on an MX28 platform. BTW, Peter, I've seen that these patches are still not merged in 3.10, is there a reason for that? do you plan on sending a version rebased on top of 3.10 some time in the future? I tried to do the rebasing myself, but the chipidea driver seems to have changed quite heavily, which makes the process quite difficult when you don't know what you're doing :) we already have Peter's patches on v3.10-rc3 [1]. I can spend few bandwidth on upstream work recently, I may have more bandwidth after June 15th. Currently, we still have no conclusion for chipidea core driver's coming work, like Device tree support, how to identify if the controller is OTG supported. Yes, the next important step is getting the of propertys dr_mode and phy_type properly used in the chipidea core. [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary - v3.10/topic/usb-peterchen Is anyone planning to work on this stuff and start pushing it mainline or is this effort stalled completely? What is it that's missing before these can be applied? AFAIK the latest commit to that work is: http://permalink.gmane.org/gmane.linux.usb.general/88121 Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb: UHCI: fix pkt size in TD for a non-aligned sg element
Hello. On 21-06-2013 15:00, Denis V. Lunev wrote: From: Konstantin Filatov kfila...@parallels.com Commit 689d6eac Please also specify that commit's summary line in parens. introduced an implementation of scatter-gather list for UHCI. This implementation has a bug in case when a non-last sg element was not aligned by TD's max-pkt-size. This bug was latent till commit 2851784f And this one too. which initializes sg_table and enables using the implementation. 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 crashes with SIGBUS. 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 CC: Alan Stern st...@rowland.harvard.edu CC: linux-usb@vger.kernel.org WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Chipidea usb otg support for IMX/MXS (device functionality)
Dear Michael Grzeschik, Hi Marek, On Fri, Jun 21, 2013 at 01:26:10PM +0200, Marek Vasut wrote: Hi, Hi, On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote: Indeed, I've been using the patchset Add tested id switch and vbus connect detect support for Chipidea from Peter for quite some time on top of 3.9 and it works like a charm for the gadget mode on an MX28 platform. BTW, Peter, I've seen that these patches are still not merged in 3.10, is there a reason for that? do you plan on sending a version rebased on top of 3.10 some time in the future? I tried to do the rebasing myself, but the chipidea driver seems to have changed quite heavily, which makes the process quite difficult when you don't know what you're doing :) we already have Peter's patches on v3.10-rc3 [1]. I can spend few bandwidth on upstream work recently, I may have more bandwidth after June 15th. Currently, we still have no conclusion for chipidea core driver's coming work, like Device tree support, how to identify if the controller is OTG supported. Yes, the next important step is getting the of propertys dr_mode and phy_type properly used in the chipidea core. [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary - v3.10/topic/usb-peterchen Is anyone planning to work on this stuff and start pushing it mainline or is this effort stalled completely? What is it that's missing before these can be applied? AFAIK the latest commit to that work is: http://permalink.gmane.org/gmane.linux.usb.general/88121 Cool, so it seems Peter is back at it. Thanks 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Fri, Jun 21, 2013 at 8:35 PM, Denis V. Lunev d...@openvz.org wrote:
From: Denis V. Lunev d...@openvz.org
From: Konstantin Filatov kfila...@parallels.com
The following commit
commit 689d6eacd1b7c3677bfe6ee367766f21c3c80e26
Author: Ming Lei tom.leim...@gmail.com
Date: Thu Sep 30 20:32:44 2010 +0800
USB: UHCI: add native scatter-gather support(v1)
introduced an implementation of scatter-gather list for UHCI.
This implementation has a bug in case when a non-last sg
element was not aligned by TD's max-pkt-size. This bug was latent
till the
commit 2851784f4d820bc697a8cc608509f9e3975c80e5
Author: Sebastian Andrzej Siewior bige...@linutronix.de
Date: Fri Jan 13 19:04:17 2012 +0100
usb/uhci: initialize sg_table properly
which initializes really initializes sg_table and enables using
the implementation.
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
I am wondering if it is a valid test, since the built sg list has one middle sg
which size is not maxp aligned.
Did you run the test on ehci controller? and is the transfer OK on ehci
controller?
The test crashes with SIGBUS.
IMO, the test may cause buffer crash.
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
CC: 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 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
The attached patch may avoid the memory crash, but I am wondering if
we need to consider the situation, or a warning should be needed.
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 041c6dd..5c98044 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -969,6 +969,13 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
pktsze = len;
if (!(urb-transfer_flags URB_SHORT_NOT_OK))
status = ~TD_CTRL_SPD;
+ } else {
+ /* skip the middle short transfer */
+ if (this_sg_len maxsze) {
+ len -= this_sg_len;
+ this_sg_len = 0;
+ goto skip;
+ }
}
if (plink) {
@@ -989,6 +996,7 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
data += pktsze;
this_sg_len -= pktsze;
len -= maxsze;
+skip:
if (this_sg_len = 0) {
if (--i = 0 || len = 0)
break;
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: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Fri, 21 Jun 2013, Ming Lei wrote: On Fri, Jun 21, 2013 at 5:20 PM, Oliver Neukum oneu...@suse.de wrote: This is highly problematic. It is bound to cause resource leaks. called. The situation might happen when driver-remove() doesn't kill the URBs with the patch applied. Well, there is no good way to handle this. But we have a simple rule. If usb_submit_urb() succeeds, complete() will be called. Breaking this rule is a bad idea. The rule should be enhanced by calling complete() before completing driver unbind. The best way is really to make sure that no URB survive. Drivers may let usbcore to do that by not setting driver-soft_unbind. I will fix the problem in v2, one solution I thought of is to flush the endpoint's URBs which have been added to tasklet list at the end of usb_hcd_flush_endpoint(). Any comments? Come to think of it, this shouldn't be a problem. Drivers _must_ insure that all their URBs have completed before their disconnect routine returns; otherwise the completion handler could get called after the driver has been unloaded from memory. Currently we have no way to enforce this in usbcore, although with the tasklets we could enforce it. But I don't think it is necessary. It would be sufficient to log a warning if the tasklet's URB list isn't empty when exit_giveback_urb_bh() runs. It won't happen unless there's a buggy driver. Besides, even if you try to flush all the URBs on the tasklet's list at the end of usb_hcd_flush_endpoint(), you'll still miss the URBs which have been moved to the local_list in usb_giveback_urb_bh(). You'd have to wait until the tasklet wasn't running, and it would most likely be a waste of time. 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Fri, 21 Jun 2013, Konstantin Filatov wrote:
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
It _isn't_ correct. But the fault lies in the driver that submitted
the SG transfer in the first place, not in uhci-hcd. This is the right
thing to do.
Also, its the same as what ehci-hcd does. Oddly enough, it looks like
nobody ever added SG support to ohci-hcd.
By the way, the patch could be improved if it removed the line
pktsze = len;
This won't be needed, because the code is careful to guarantee that
this_sg_len = len always.
Your patch skips a chunk of data to transfer. This is a corruption of
data. It's unacceptable. We can return an error code if we treat the
request as malformed, but we can't transfer data selectively.
I agree. Fix up the patch description as Sergei suggested, and you can
add
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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Fri, Jun 21, 2013 at 10:23 PM, Konstantin Filatov
kfila...@parallels.com wrote:
On 06/21/2013 05:47 PM, Ming Lei wrote:
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
I am wondering if it is a valid test, since the built sg list has one
middle sg
which size is not maxp aligned.
Did you run the test on ehci controller? and is the transfer OK on ehci
controller?
Yes, for ehci the test succeded.
Did you compare the transfer data? And what is the actual transfer length?
This test exists inside linux kernel source tree for long time. It is
important to check usb infrastructure integrity.
The test utility 'testusb' uses ioctl call to a driver 'usbtest', and
actually the driver run a test (in the kernel space) on the host. The
hardware (Gadget Zero device) verifies the test transfer.
The test crashes with SIGBUS.
IMO, the test may cause buffer crash.
In the current implementation, no mater what 'this_sg_len' is, the transfer
size of td is always set as maxp of the endpoint if it isn't the last packet, so
the middle sg which size isn't maxp aligned will be overflowed for IN transfer,
then buffer crash is caused.
No buffer crash. The driver signals about the test failed in such way.
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
See 5.3.2 Pipes of USB 1.1 spec:
An IRP may require multiple data payloads to move the client data over the bus.
The data payloads for such a multiple data payload IRP are expected to be of
the maximum packet size until the last data payload that contains the remainder
of the overall IRP.
And same description can be found in USB 2.0 spec.
The attached patch may avoid the memory crash, but I am wondering if
we need to consider the situation, or a warning should be needed.
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 041c6dd..5c98044 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -969,6 +969,13 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
pktsze = len;
if (!(urb-transfer_flags URB_SHORT_NOT_OK))
status = ~TD_CTRL_SPD;
+ } else {
+ /* skip the middle short transfer */
+ if (this_sg_len maxsze) {
+ len -= this_sg_len;
+ this_sg_len = 0;
+ goto skip;
+ }
}
if (plink) {
@@ -989,6 +996,7 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
data += pktsze;
this_sg_len -= pktsze;
len -= maxsze;
+skip:
if (this_sg_len = 0) {
if (--i = 0 || len = 0)
break;
Your patch skips a chunk of data to transfer. This is a corruption of data.
How can a corruption of data be caused?
It's unacceptable. We can return an error code if we treat the request as
malformed, but we can't transfer data selectively.
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 Fri, Jun 21, 2013 at 11:19 PM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 21 Jun 2013, Konstantin Filatov wrote:
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
It _isn't_ correct. But the fault lies in the driver that submitted
the SG transfer in the first place, not in uhci-hcd. This is the right
thing to do.
Also, its the same as what ehci-hcd does. Oddly enough, it looks like
nobody ever added SG support to ohci-hcd.
By the way, the patch could be improved if it removed the line
pktsze = len;
This won't be needed, because the code is careful to guarantee that
this_sg_len = len always.
Your patch skips a chunk of data to transfer. This is a corruption of
data. It's unacceptable. We can return an error code if we treat the
request as malformed, but we can't transfer data selectively.
I agree. Fix up the patch description as Sergei suggested, and you can
add
Acked-by: Alan Stern st...@rowland.harvard.edu
But the patch violates USB spec, doesn't it?
Also it sets the later packet size of TD as the small one, is it correct?
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] jacinto6 : usb3_phy: Updated dpll M,N values.
Addition of the M and N recommended values for the USB3 PHY DPLL.
Sysclk for DRA7xx is 20MHz.
This yields:
Clk = 20MHz * M/(N+1) = 20MHz * 1000 /(7+1) = 2.5 Ghz
Signed-off-by: Nikhil Devshatwar nikhil...@ti.com
Signed-off-by: Ruchika Kharwar ruch...@ti.com
---
drivers/usb/phy/phy-omap-usb3.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index a6e60b1..a2fb30b 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -27,7 +27,7 @@
#include linux/delay.h
#include linux/usb/omap_control_usb.h
-#defineNUM_SYS_CLKS5
+#defineNUM_SYS_CLKS6
#definePLL_STATUS 0x0004
#definePLL_GO 0x0008
#definePLL_CONFIGURATION1 0x000C
@@ -62,6 +62,7 @@ enum sys_clk_rate {
CLK_RATE_12MHZ,
CLK_RATE_16MHZ,
CLK_RATE_19MHZ,
+ CLK_RATE_20MHZ,
CLK_RATE_26MHZ,
CLK_RATE_38MHZ
};
@@ -70,8 +71,10 @@ static struct usb_dpll_params
omap_usb3_dpll_params[NUM_SYS_CLKS] = {
{1250, 5, 4, 20, 0},/* 12 MHz */
{3125, 20, 4, 20, 0}, /* 16.8 MHz */
{1172, 8, 4, 20, 65537},/* 19.2 MHz */
+ {1000, 7, 4, 10, 0},/* 20 MHz */
{1250, 12, 4, 20, 0}, /* 26 MHz */
{3125, 47, 4, 20, 92843}, /* 38.4 MHz */
+
};
static int omap_usb3_suspend(struct usb_phy *x, int suspend)
@@ -122,6 +125,8 @@ static inline enum sys_clk_rate
__get_sys_clk_index(unsigned long rate)
return CLK_RATE_16MHZ;
case 1920:
return CLK_RATE_19MHZ;
+ case 2000:
+ return CLK_RATE_20MHZ;
case 2600:
return CLK_RATE_26MHZ;
case 3840:
--
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 Fri, Jun 21, 2013 at 11:19 PM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 21 Jun 2013, Konstantin Filatov wrote:
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
It _isn't_ correct. But the fault lies in the driver that submitted
the SG transfer in the first place, not in uhci-hcd. This is the right
thing to do.
The correct fix should be: not using sg under this situation by
patching usb_sg_init(),
shouldn't it?
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 Fri, 21 Jun 2013, Ming Lei wrote:
On Fri, Jun 21, 2013 at 11:19 PM, Alan Stern st...@rowland.harvard.edu
wrote:
On Fri, 21 Jun 2013, Konstantin Filatov wrote:
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
It _isn't_ correct. But the fault lies in the driver that submitted
the SG transfer in the first place, not in uhci-hcd. This is the right
thing to do.
Also, its the same as what ehci-hcd does. Oddly enough, it looks like
nobody ever added SG support to ohci-hcd.
By the way, the patch could be improved if it removed the line
pktsze = len;
This won't be needed, because the code is careful to guarantee that
this_sg_len = len always.
Your patch skips a chunk of data to transfer. This is a corruption of
data. It's unacceptable. We can return an error code if we treat the
request as malformed, but we can't transfer data selectively.
I agree. Fix up the patch description as Sergei suggested, and you can
add
Acked-by: Alan Stern st...@rowland.harvard.edu
But the patch violates USB spec, doesn't it?
No, the client driver violates the kernel's requirements by submitting
an SG transfer that can't be carried out. Although it probably isn't
documented anywhere, the USB stack requires that the size of each SG
element except the last one must be a multiple of the endpoint's
maxpacket value.
(The wireless USB stack may have a weaker requirement here. I'm
talking about wired USB only.)
We could fail the transfer request if the SG element length violates
this requirement. That would be acceptable. But then ehci-hcd and
xhci-hcd should be updated to do the same thing.
Also it sets the later packet size of TD as the small one, is it correct?
I don't understand the question. TDs don't have a later or an
earlier packet size. Are you referring to the ActLen and MaxLen
fields in the TD? And what do you mean by the small one?
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: [RFC PATCH v1 1/6] USB: HCD: support giveback of URB in tasklet context
On Fri, Jun 21, 2013 at 10:48 PM, Alan Stern st...@rowland.harvard.edu wrote: Come to think of it, this shouldn't be a problem. Drivers _must_ insure that all their URBs have completed before their disconnect routine returns; otherwise the completion handler could get called after the driver has been unloaded from memory. Currently we have no way to enforce this in usbcore, although with the tasklets we could enforce it. But I don't think it is necessary. It One way of enforcing this I thought of is to wait for completions of all unlinking URBs at the end of usb_hcd_flush_endpoint(). But as you said, it may not be necessary. would be sufficient to log a warning if the tasklet's URB list isn't empty when exit_giveback_urb_bh() runs. It won't happen unless there's a buggy driver. Besides, even if you try to flush all the URBs on the tasklet's list at the end of usb_hcd_flush_endpoint(), you'll still miss the URBs which have been moved to the local_list in usb_giveback_urb_bh(). You'd have to wait until the tasklet wasn't running, and it would most likely be a waste of time. 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 Fri, 21 Jun 2013, Ming Lei wrote:
On Fri, Jun 21, 2013 at 11:19 PM, Alan Stern st...@rowland.harvard.edu
wrote:
On Fri, 21 Jun 2013, Konstantin Filatov wrote:
--- 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;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
It _isn't_ correct. But the fault lies in the driver that submitted
the SG transfer in the first place, not in uhci-hcd. This is the right
thing to do.
The correct fix should be: not using sg under this situation by
patching usb_sg_init(),
shouldn't it?
I'm not sure what you mean. Do you mean that usb_sg_init() should fail
if the SG element length isn't a multiple of the maxpacket size? That
probably will break wireless USB.
Do you mean that usb_sg_init() should break the transfer up into a
bunch of separate URBs in this case (the way it does when the HCD
doesn't have native support for SG)? That won't work because one of
the URBs would have a transfer length that wasn't a multiple of the
maxpacket size.
Furthermore, URBs using SG can be submitted directly, without going
through usb_sg_init() at all. So changing usb_sg_init() won't fix the
problem.
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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, Jun 22, 2013 at 12:08 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 21 Jun 2013, Ming Lei wrote: But the patch violates USB spec, doesn't it? No, the client driver violates the kernel's requirements by submitting an SG transfer that can't be carried out. Although it probably isn't documented anywhere, the USB stack requires that the size of each SG element except the last one must be a multiple of the endpoint's maxpacket value. Look there is the description: 5.3.2 Pipes of USB 1.1 spec: An IRP may require multiple data payloads to move the client data over the bus. The data payloads for such a multiple data payload IRP are expected to be of the maximum packet size until the last data payload that contains the remainder of the overall IRP. (The wireless USB stack may have a weaker requirement here. I'm talking about wired USB only.) We could fail the transfer request if the SG element length violates this requirement. That would be acceptable. But then ehci-hcd and xhci-hcd should be updated to do the same thing. Also it sets the later packet size of TD as the small one, is it correct? I don't understand the question. TDs don't have a later or an earlier packet size. Are you referring to the ActLen and MaxLen fields in the TD? And what do you mean by the small one? I mean MaxLen, the patch changes 'pktsze' as 'this_sg_len' if 'this_sg_len' is less than 'pktsze', then the maxlen of all following TDs will use the value smaller than maxp. 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 Sat, Jun 22, 2013 at 12:13 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 21 Jun 2013, Ming Lei wrote: I'm not sure what you mean. Do you mean that usb_sg_init() should fail if the SG element length isn't a multiple of the maxpacket size? That probably will break wireless USB. Do you mean that usb_sg_init() should break the transfer up into a bunch of separate URBs in this case (the way it does when the HCD doesn't have native support for SG)? That won't work because one of the URBs would have a transfer length that wasn't a multiple of the maxpacket size. Yes. Sorry, could you explain why that won't work? I understand the URBs with maxp-unaligned length still can be completed. Furthermore, URBs using SG can be submitted directly, without going through usb_sg_init() at all. So changing usb_sg_init() won't fix the problem. Looks usbtest use it, so the reported problem can be fixed. 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 5/6] USB: OHCI: make ohci-at91 a separate driver
On Fri, 21 Jun 2013, Manjunath Goudar wrote:
On 20 June 2013 22:23, Alan Stern st...@rowland.harvard.edu wrote:
On Thu, 20 Jun 2013, Manjunath Goudar wrote:
@@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device
*pdev, pm_message_t mesg)
* REVISIT: some boards will be able to turn VBUS off...
*/
if (at91_suspend_entering_slow_clock()) {
- ohci_usb_reset (ohci);
+ ohci_restart(ohci);
Why did you change this? Did we discuss it earlier?
We are not discussed regarding this,I think we need to call
use ohci_resume() instead of ohci_restart().
Why? Don't you think the current code has a good reason for calling
ohci_usb_reset()?
Here ohci_usb_reset() is static function,that is what I am planing to call
ohci_setup() or ohci_restart() because it is calling ohci_usb_reset(),
If not calling, we can make ohci_usb_reset() function as non-static
function
or use directly ohci_usb_reset() function code here.
Let me know which one is good approach.
As a general rule, you should never change code that you don't
understand. Do you _know_ that it will be safe to call ohci_setup() or
ohci_restart() at this point?
It might be a good idea to get in touch with the person who wrote that
routine originally and ask why they used ohci_usb_reset().
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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, 22 Jun 2013, Ming Lei wrote: On Sat, Jun 22, 2013 at 12:08 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 21 Jun 2013, Ming Lei wrote: But the patch violates USB spec, doesn't it? No, the client driver violates the kernel's requirements by submitting an SG transfer that can't be carried out. Although it probably isn't documented anywhere, the USB stack requires that the size of each SG element except the last one must be a multiple of the endpoint's maxpacket value. Look there is the description: 5.3.2 Pipes of USB 1.1 spec: An IRP may require multiple data payloads to move the client data over the bus. The data payloads for such a multiple data payload IRP are expected to be of the maximum packet size until the last data payload that contains the remainder of the overall IRP. Yes, I know. So what? The spec doesn't say anything about the requirements of Linux's USB stack. (The wireless USB stack may have a weaker requirement here. I'm talking about wired USB only.) We could fail the transfer request if the SG element length violates this requirement. That would be acceptable. But then ehci-hcd and xhci-hcd should be updated to do the same thing. Also it sets the later packet size of TD as the small one, is it correct? I don't understand the question. TDs don't have a later or an earlier packet size. Are you referring to the ActLen and MaxLen fields in the TD? And what do you mean by the small one? I mean MaxLen, the patch changes 'pktsze' as 'this_sg_len' if 'this_sg_len' is less than 'pktsze', then the maxlen of all following TDs will use the value smaller than maxp. No, they won't. pktsze gets reinitialized each time through the loop. 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, 22 Jun 2013, Ming Lei wrote: On Sat, Jun 22, 2013 at 12:13 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 21 Jun 2013, Ming Lei wrote: I'm not sure what you mean. Do you mean that usb_sg_init() should fail if the SG element length isn't a multiple of the maxpacket size? That probably will break wireless USB. Do you mean that usb_sg_init() should break the transfer up into a bunch of separate URBs in this case (the way it does when the HCD doesn't have native support for SG)? That won't work because one of the URBs would have a transfer length that wasn't a multiple of the maxpacket size. Yes. Sorry, could you explain why that won't work? I understand the URBs with maxp-unaligned length still can be completed. Suppose the SG list has two elements, where the first element's length is 1000 (not a multiple of 64) and the second element's length is 512. Then usb_sg_init() will create two URBs to carry out the transfer. The transfer_length of the first URB will be 1000 and the transfer_length of the second URB will be 512. When the first URB is submitted to uhci-hcd, it will be broken up into 15 TDs of length 64 (the maxpacket size) followed by a TD of length 40. The second URB will be broken up into 8 TDs of length 64. Therefore the device will see 15 full-size packets, a short packet, and then 8 more full-size packets. This is not what you want. (By the way, this is exactly what would happen when the same transfer is submitted to an HCD that doesn't have SG support, like ohci-hcd. As you can see, the problem doesn't lie in the fact that SG is being used; it lies in the fact that the SG element length isn't a multiple of the maxpacket length.) Furthermore, URBs using SG can be submitted directly, without going through usb_sg_init() at all. So changing usb_sg_init() won't fix the problem. Looks usbtest use it, so the reported problem can be fixed. Sometimes it's hard to tell whether you are serious or joking. 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, Jun 22, 2013 at 12:36 AM, Alan Stern st...@rowland.harvard.edu wrote: No, they won't. pktsze gets reinitialized each time through the loop. You are right, the pktsze will get reinitialized, then there is a short packet in the middle of transfer, so it violates the above description of USB spec which only allows that short packet appears the end of 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
Re: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On 6/21/13 7:19 PM, Alan Stern wrote: On Fri, 21 Jun 2013, Konstantin Filatov wrote: Your patch skips a chunk of data to transfer. This is a corruption of data. It's unacceptable. We can return an error code if we treat the request as malformed, but we can't transfer data selectively. I agree. Fix up the patch description as Sergei suggested, and you can add Acked-by: Alan Stern st...@rowland.harvard.edu Alan, simple stupid question. Does [v2] patch description is good or I should re-write it like 689d6eac (USB: UHCI: add native scatter-gather support(v1)) Regards, Den -- 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 Sat, Jun 22, 2013 at 12:45 AM, Alan Stern st...@rowland.harvard.edu wrote: Sorry, could you explain why that won't work? I understand the URBs with maxp-unaligned length still can be completed. Suppose the SG list has two elements, where the first element's length is 1000 (not a multiple of 64) and the second element's length is 512. Then usb_sg_init() will create two URBs to carry out the transfer. The transfer_length of the first URB will be 1000 and the transfer_length of the second URB will be 512. When the first URB is submitted to uhci-hcd, it will be broken up into 15 TDs of length 64 (the maxpacket size) followed by a TD of length 40. The second URB will be broken up into 8 TDs of length 64. Therefore the device will see 15 full-size packets, a short packet, and then 8 more full-size packets. This is not what you want. From view of driver, maybe they only want to transfer 1512 bytes, and it is hard to say what the driver or device wants. Maybe they don't depend on the middle short packet at all, maybe they do, who knows? Denis's patch still can't do what you hope(short packet is at the end of the transfer), can it? (By the way, this is exactly what would happen when the same transfer is submitted to an HCD that doesn't have SG support, like ohci-hcd. As you can see, the problem doesn't lie in the fact that SG is being used; it lies in the fact that the SG element length isn't a multiple of the maxpacket length.) Furthermore, URBs using SG can be submitted directly, without going through usb_sg_init() at all. So changing usb_sg_init() won't fix the problem. Looks usbtest use it, so the reported problem can be fixed. Sometimes it's hard to tell whether you are serious or joking. I mean other places still can do this way like usb_sg_init(). 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 Fri, 21 Jun 2013, Denis V. Lunev wrote: Alan, simple stupid question. Does [v2] patch description is good or I should re-write it like 689d6eac (USB: UHCI: add native scatter-gather support(v1)) It would be better to change the patch title. The problem isn't related to alignment; the problem is that the element size isn't a multiple of the maxpacket size. 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: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, 22 Jun 2013, Ming Lei wrote: On Sat, Jun 22, 2013 at 12:45 AM, Alan Stern st...@rowland.harvard.edu wrote: Sorry, could you explain why that won't work? I understand the URBs with maxp-unaligned length still can be completed. Suppose the SG list has two elements, where the first element's length is 1000 (not a multiple of 64) and the second element's length is 512. Then usb_sg_init() will create two URBs to carry out the transfer. The transfer_length of the first URB will be 1000 and the transfer_length of the second URB will be 512. When the first URB is submitted to uhci-hcd, it will be broken up into 15 TDs of length 64 (the maxpacket size) followed by a TD of length 40. The second URB will be broken up into 8 TDs of length 64. Therefore the device will see 15 full-size packets, a short packet, and then 8 more full-size packets. This is not what you want. From view of driver, maybe they only want to transfer 1512 bytes, and it is hard to say what the driver or device wants. Maybe they don't depend on the middle short packet at all, maybe they do, who knows? Nonsense. Transfers do not have short packets in the middle, only at the end. If the driver wants to have 1512 bytes in a single transfer then there must be 23 packets of length 64 followed one packet of length 40. If the driver wants to send 15 packets of length 64 followed by one packet of length 40 and 8 more packets of length 64, then it must use two transfers. Denis's patch still can't do what you hope(short packet is at the end of the transfer), can it? What I hope is that we can prevent SIGBUS or other memory access errors. The patch will do that. The test case mentioned in the patch description (testusb -a -t 7 -c 2000 -s 4096 -v 41) is expected to fail, because 41 is not a multiple of 64. But it shouldn't crash. Maybe the usbtest driver should refuse to run test 7 and 8 if the vary parameter isn't a multiple of the endpoint's maxpacket length. Furthermore, URBs using SG can be submitted directly, without going through usb_sg_init() at all. So changing usb_sg_init() won't fix the problem. Looks usbtest use it, so the reported problem can be fixed. Sometimes it's hard to tell whether you are serious or joking. I mean other places still can do this way like usb_sg_init(). You mean other places still can submit SG transfers with invalid element lengths? Yes, they can. That's why the fix has to be in uhci-hcd, not in usb_sg_init(). 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: [RFC PATCH v1 5/6] USB: EHCI: improve interrupt qh unlink
On Thu, 20 Jun 2013, Ming Lei wrote:
IMO, there is no any side effect when we change the state to
QH_STATE_UNLINK_WAIT and later change back to QH_STATE_LINKED
later under this situation.
I don't like the idea of changing an invariant.
The reason why I use QH_STATE_UNLINK_WAIT here is for avoiding
unnecessary CPU wakeup. Without the state, the unlink wait timer
is still triggered to check if there are intr QHs to be unlinked, but in most
of
situations, there aren't QHs to be unlinked since tasklet is surely run
before the wait timer is expired. So generally, without knowing the wait
state,
CPU wakeup events may be introduced unnecessarily.
Avoiding unnecessary timer events is a good thing, but I don't
understand how it is connected with QH_STATE_UNLINK_WAIT. Doesn't this
revised patch avoid the events without using this state?
Besides, don't you already know the wait state by checking whether
qh-unlink_node is empty?
Considered that the interval of interrupt endpoint might be very
small(e.g. 125us,
1ms) and some devices have very frequent intr event, I think we
need to pay attention to the problem.
Yes, we do. The hrtimer code in ehci-timer is written specifically to
handle that sort of situation.
Even on async QH situation, we might need to consider the problem too
since some application(eg. bulk only mass storage on xhci) may have
thousands of interrupts per seconds during data transfer, so CPU wakeup
events could be increased much by letting wait timer expire unnecessarily.
I suspect it's the other way around. Letting the timer expire
_reduces_ the amount of work, because we don't have to start and stop
the timer as often.
It's a tradeoff. One approach starts and cancels the timer N times
(where N can be fairly large); the other approach starts the timer
once and lets it expire, and then the handler routine does almost no
work. Which approach uses more CPU time? I don't know; I haven't
measured it.
Also the async QH unlink approach has another disadvantage:
- if there are several QHs which are become empty during one wait period,
the hrtimer has to be scheduled several times for starting unlink one qh
each time.
That's because the driver unlinks only one async QH at a time. It is
unavoidable. In earlier kernels the driver would unlink multiple async
QHs simultaneously, and it needed to schedule the timer only once.
For some reason (I still don't understand why), this did not work on
some systems.
And after introducing the unlink wait list like the patch, we only
need schedule the hrtimer one time for unlinking all these empty QHs.
The async code used to work that way, before I changed it to unlink
only one async QH at a time.
Finally, unlink wait for intr qh is only needed when the qh is completed
right now, and other cases(eg. unlink) needn't the wait.
Right.
The attached patch removes the QH_STATE_UNLINK_WAIT, and can
avoid the above disadvantages of async QH unlink, could you comment
on the new patch?
Okay...
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 246e124..35b4148 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -304,7 +304,8 @@ static void end_unlink_async(struct ehci_hcd *ehci);
static void unlink_empty_async(struct ehci_hcd *ehci);
static void unlink_empty_async_suspended(struct ehci_hcd *ehci);
static void ehci_work(struct ehci_hcd *ehci);
-static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh);
+static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh,
+ bool wait);
Adding a wait argument is a bad approach. You should create a new
function: start_unlink_intr_wait() or something like that. After all,
it is used in only one place.
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index acff5b8..5dfda56 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -548,6 +548,9 @@ static void qh_link_periodic(struct ehci_hcd
*ehci, struct ehci_qh *qh)
list_add(qh-intr_node, ehci-intr_qh_list);
+ /* unlink need this node to be initialized */
+ INIT_LIST_HEAD(qh-unlink_node);
This should be done only once, when the QH is created. And the comment
is unnecessary.
@@ -98,6 +99,17 @@ static void ehci_enable_event(struct ehci_hcd
*ehci, unsigned event,
}
}
+/* Warning: don't call this function from hrtimer handler context */
+static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
+{
+ if (!hcd_giveback_urb_in_bh(ehci_to_hcd(ehci)))
+ return;
This test looks really ugly, and it won't be needed after the driver
switches over to tasklets. Of course, there's a problem before we
switch over: this routine will be called by an interrupt URB
submission, which could occur during a giveback in the timer handler
context.
Perhaps the best approach is to leave this routine out
Re: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sat, Jun 22, 2013 at 1:42 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 22 Jun 2013, Ming Lei wrote: On Sat, Jun 22, 2013 at 12:45 AM, Alan Stern st...@rowland.harvard.edu wrote: Sorry, could you explain why that won't work? I understand the URBs with maxp-unaligned length still can be completed. Suppose the SG list has two elements, where the first element's length is 1000 (not a multiple of 64) and the second element's length is 512. Then usb_sg_init() will create two URBs to carry out the transfer. The transfer_length of the first URB will be 1000 and the transfer_length of the second URB will be 512. When the first URB is submitted to uhci-hcd, it will be broken up into 15 TDs of length 64 (the maxpacket size) followed by a TD of length 40. The second URB will be broken up into 8 TDs of length 64. Therefore the device will see 15 full-size packets, a short packet, and then 8 more full-size packets. This is not what you want. From view of driver, maybe they only want to transfer 1512 bytes, and it is hard to say what the driver or device wants. Maybe they don't depend on the middle short packet at all, maybe they do, who knows? Nonsense. Transfers do not have short packets in the middle, only at the end. If the driver wants to have 1512 bytes in a single transfer then there must be 23 packets of length 64 followed one packet of length 40. If the driver wants to send 15 packets of length 64 followed by one packet of length 40 and 8 more packets of length 64, then it must use two transfers. Denis's patch still can't do what you hope(short packet is at the end of the transfer), can it? What I hope is that we can prevent SIGBUS or other memory access errors. The patch will do that. If we only want to prevent this errors, there should be better approach, IMO. Since you mentioned it doesn't make sense to generate short packet in the middle of transfer, also it may not be what the driver/device expected, I suggest to add a check in usb_submit_urb() for this case and returns failure on it simply, because all HCDs shouldn't support this sort of thing. The check in usb_submit_urb() can avoid unnecessary change in HCD. Any comments on the idea? The test case mentioned in the patch description (testusb -a -t 7 -c 2000 -s 4096 -v 41) is expected to fail, because 41 is not a multiple of 64. But it shouldn't crash. Maybe the usbtest driver should refuse to run test 7 and 8 if the vary parameter isn't a multiple of the endpoint's maxpacket length. Furthermore, URBs using SG can be submitted directly, without going through usb_sg_init() at all. So changing usb_sg_init() won't fix the problem. Looks usbtest use it, so the reported problem can be fixed. Sometimes it's hard to tell whether you are serious or joking. I mean other places still can do this way like usb_sg_init(). You mean other places still can submit SG transfers with invalid element lengths? Yes, they can. That's why the fix has to be in uhci-hcd, not in usb_sg_init(). Alan Stern 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 0/9] usb: gadget/uvc: stability and usability fixes
Hi Michael, On Tuesday 18 June 2013 17:12:53 Michael Grzeschik wrote: On Tue, Jun 04, 2013 at 05:08:19PM +0200, Michael Grzeschik wrote: Hi, this series is fixing some stability and usability issues found with the usb uvc-gadget. It's needed to make the device usable as an v4l2sink with gstreamer. Thanks, Michael Laurent Pinchart (1): usb: gadget/uvc: Fix error handling in uvc_queue_buffer() Michael Grzeschik (8): usb: gadget/uvc: cancel the video queue if buffers could not be enqueued usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT usb: gadget/uvc: free buffers after streamoff usb: gadget/uvc: Add monotonic time stamp flag usb: gadget/uvc: change KERN_INFO to KERN_DEBUG on request shutdown usb: gadget/uvc: set sizes in queue_setup to 0 when memorytype is USERPTR usb: gadget/uvc: fix S_FMT always assume sizeimage for MJPEG usb: gadget/uvc: add uvc suspend resume functions drivers/usb/gadget/f_uvc.c | 17 + drivers/usb/gadget/uvc_queue.c | 9 +++-- drivers/usb/gadget/uvc_v4l2.c | 33 +++-- drivers/usb/gadget/uvc_video.c | 5 - 4 files changed, 59 insertions(+), 5 deletions(-) Ping! Did you find time to look into that series? I'll try to review it on Sunday. -- Regards, Laurent Pinchart -- 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)
Hi, Dear Michael Grzeschik, Hi Marek, On Fri, Jun 21, 2013 at 01:26:10PM +0200, Marek Vasut wrote: Hi, Hi, On Wed, May 29, 2013 at 08:07:55AM +, Chen Peter-B29397 wrote: Indeed, I've been using the patchset Add tested id switch and vbus connect detect support for Chipidea from Peter for quite some time on top of 3.9 and it works like a charm for the gadget mode on an MX28 platform. BTW, Peter, I've seen that these patches are still not merged in 3.10, is there a reason for that? do you plan on sending a version rebased on top of 3.10 some time in the future? I tried to do the rebasing myself, but the chipidea driver seems to have changed quite heavily, which makes the process quite difficult when you don't know what you're doing :) we already have Peter's patches on v3.10-rc3 [1]. I can spend few bandwidth on upstream work recently, I may have more bandwidth after June 15th. Currently, we still have no conclusion for chipidea core driver's coming work, like Device tree support, how to identify if the controller is OTG supported. Yes, the next important step is getting the of propertys dr_mode and phy_type properly used in the chipidea core. [1] http://git.pengutronix.de/?p=mgr/linux.git;a=summary - v3.10/topic/usb-peterchen Is anyone planning to work on this stuff and start pushing it mainline or is this effort stalled completely? What is it that's missing before these can be applied? AFAIK the latest commit to that work is: http://permalink.gmane.org/gmane.linux.usb.general/88121 Cool, so it seems Peter is back at it. Thanks 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. 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
