Re: [PATCH 1/1] usb: chipidea: fix the build error with randconfig
Peter Chen peter.c...@freescale.com writes: Using below configs, the compile will have error: ERROR: ehci_init_driver undefined! .config: CONFIG_USB_CHIPIDEA=m CONFIG_USB_CHIPIDEA_HOST=y CONFIG_USB_CHIPIDEA_DEBUG=y The reason is chipidea host uses symbol from ehci, but ehci is not compiled. Let the chipidea host depend on ehci even it is built as module. Looks good, I'll send it to Greg after -rc1 is out. 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] chipidea: bits: Cast PORTSC_PTS and DEVLC_PTS macros
Fabio Estevam fabio.este...@freescale.com writes: Fix the following build warnings on x86: drivers/usb/chipidea/core.c: In function 'hw_phymode_configure': drivers/usb/chipidea/core.c:226:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:230:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:243:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] drivers/usb/chipidea/core.c:246:3: warning: large integer implicitly truncated to unsigned type [-Woverflow] Reported-by: Felipe Balbi ba...@ti.com Signed-off-by: Fabio Estevam fabio.este...@freescale.com Thanks, I'll send this to Greg for usb-linus after the -rc1 is out. Regards, -- Alex -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: host: Use usb_hcd_platform_shutdown() wherever possible
On 07/09/2013 05:16 PM, Alan Stern wrote: On Tue, 9 Jul 2013, Roger Quadros wrote: Most HCD drivers are doing the same thing in their .shutdown callback so it makes sense to use the generic usb_hcd_platform_shutdown() handler there. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/host/ehci-grlib.c | 11 +-- drivers/usb/host/ehci-mxc.c | 10 +- drivers/usb/host/ehci-omap.c | 10 +- drivers/usb/host/ehci-ppc-of.c| 11 +-- drivers/usb/host/ehci-s5p.c | 10 +- drivers/usb/host/ehci-tegra.c | 10 +- drivers/usb/host/ehci-xilinx-of.c | 17 + drivers/usb/host/ohci-omap3.c | 10 +- 8 files changed, 8 insertions(+), 81 deletions(-) This all looks fine. But unless my kernel tree is out of date, you missed ohci-ppc-of.c. You are right. I missed it and will send a revision. I've also noticed some drivers doing non-standard stuff. e.g. - ehci-ps3.c and ohci-pst set .shutdown as well as .remove to to ps3_ehci_remove - ehci-tilegx.c and ohci-tilegx call .remove in the .shutdown path - ehci-mv.c checks for (!hcd-rh_registered) in the shudown remove patch. Is this necessary? cheers, -roger -- 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] musb: omap: Fix: pass all the resources to musb core
commit 09fc7d (usb: musb: fix incorrect usage of resource pointer)
assumes musb core will always have only 2 resources. But for OMAP
platforms there can be 3 resources (2 irq resource and 1 iomem
resource). Fixed it here.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Changes from v1:
*) Removed redundant initialization of *i*
drivers/usb/musb/omap2430.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 5b6113a..5bbef78 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -481,7 +481,7 @@ static u64 omap2430_dmamask = DMA_BIT_MASK(32);
static int omap2430_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[3];
struct musb_hdrc_platform_data *pdata = pdev-dev.platform_data;
struct omap_musb_board_data *data;
struct platform_device *musb;
@@ -489,6 +489,7 @@ static int omap2430_probe(struct platform_device *pdev)
struct device_node *np = pdev-dev.of_node;
struct musb_hdrc_config *config;
int ret = -ENOMEM;
+ int i;
glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
@@ -571,15 +572,12 @@ static int omap2430_probe(struct platform_device *pdev)
memset(musb_resources, 0x00, sizeof(*musb_resources) *
ARRAY_SIZE(musb_resources));
- musb_resources[0].name = pdev-resource[0].name;
- musb_resources[0].start = pdev-resource[0].start;
- musb_resources[0].end = pdev-resource[0].end;
- musb_resources[0].flags = pdev-resource[0].flags;
-
- musb_resources[1].name = pdev-resource[1].name;
- musb_resources[1].start = pdev-resource[1].start;
- musb_resources[1].end = pdev-resource[1].end;
- musb_resources[1].flags = pdev-resource[1].flags;
+ for (i = 0; i ARRAY_SIZE(musb_resources); i++) {
+ musb_resources[i].name = pdev-resource[i].name;
+ musb_resources[i].start = pdev-resource[i].start;
+ musb_resources[i].end = pdev-resource[i].end;
+ musb_resources[i].flags = pdev-resource[i].flags;
+ }
ret = platform_device_add_resources(musb, musb_resources,
ARRAY_SIZE(musb_resources));
--
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: [PATCH v2] musb: omap: Fix: pass all the resources to musb core
On Wed, Jul 10, 2013 at 04:29:22PM +0530, Kishon Vijay Abraham I wrote:
commit 09fc7d (usb: musb: fix incorrect usage of resource pointer)
assumes musb core will always have only 2 resources. But for OMAP
platforms there can be 3 resources (2 irq resource and 1 iomem
resource). Fixed it here.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Changes from v1:
*) Removed redundant initialization of *i*
drivers/usb/musb/omap2430.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 5b6113a..5bbef78 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -481,7 +481,7 @@ static u64 omap2430_dmamask = DMA_BIT_MASK(32);
static int omap2430_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[3];
struct musb_hdrc_platform_data *pdata = pdev-dev.platform_data;
struct omap_musb_board_data *data;
struct platform_device *musb;
@@ -489,6 +489,7 @@ static int omap2430_probe(struct platform_device *pdev)
struct device_node *np = pdev-dev.of_node;
struct musb_hdrc_config *config;
int ret = -ENOMEM;
+ int i;
glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
@@ -571,15 +572,12 @@ static int omap2430_probe(struct platform_device *pdev)
memset(musb_resources, 0x00, sizeof(*musb_resources) *
ARRAY_SIZE(musb_resources));
- musb_resources[0].name = pdev-resource[0].name;
- musb_resources[0].start = pdev-resource[0].start;
- musb_resources[0].end = pdev-resource[0].end;
- musb_resources[0].flags = pdev-resource[0].flags;
-
- musb_resources[1].name = pdev-resource[1].name;
- musb_resources[1].start = pdev-resource[1].start;
- musb_resources[1].end = pdev-resource[1].end;
- musb_resources[1].flags = pdev-resource[1].flags;
+ for (i = 0; i ARRAY_SIZE(musb_resources); i++) {
then this is not enough, what if one device using omap2430.c has 2
resources and the other has 3 ? and what if a new one has 4 ?
How about using pdev-num_resources to dynamically allocate
musb_resources array and using the same thing iterate here ?
Something like:
struct resource *musb_resources;
musb_resources = kcalloc(pdev-num_resources, struct resource,
GPF_KERNEL);
if (!musb_resources)
bail();
for (i = 0; i pdev-num_resources, i++) {
musb_resources[i].name = pdev-resource[i].name;
musb_resources[i].start = pdev-resource[i].start;
musb_resources[i].end = pdev-resource[i].end;
musb_resources[i].flags = pdev-resource[i].flags;
}
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [RFC v3] xhci: fix dma mask setup in xhci.c
Hi, On Tue, Jul 09, 2013 at 02:43:45PM -0700, Sarah Sharp wrote: On Tue, Jul 09, 2013 at 02:59:38PM +0300, Felipe Balbi wrote: Hi, On Mon, Jul 08, 2013 at 09:55:15PM -0700, Sarah Sharp wrote: Felipe, Andy, and Seb, I have a couple questions below. On Fri, Jul 05, 2013 at 08:24:56PM +0300, Xenia Ragiadakou wrote: Felipe, Andy, is there any chance that a platform_device dma_mask pointer would already be initialized by the time the probe function is called? We wouldn't want to overwrite it. Can you please check the xhci_plat_probe code? yes there is. At least if you're booting with Devicetree, OF core sets *all* dma_masks to 32-bits (erroneously IMO): $ git grep -e dma_mask drivers/of/ drivers/of/platform.c: dev-dev.dma_mask = dev-archdata.dma_mask; drivers/of/platform.c: dev-archdata.dma_mask = 0xUL; drivers/of/platform.c: dev-dev.coherent_dma_mask = DMA_BIT_MASK(32); drivers/of/platform.c: dev-dev.coherent_dma_mask = ~0; drivers/of/platform.c: dev-dma_mask = ~0; Ok, so that means Xenia needs to make sure to check whether the dma_mask pointer is non-NULL in xhci-plat.c, so that she doesn't overwrite it with a pointer to the coherent dma mask, correct? Or should she just overwrite the pointer because the OF core really shouldn't be setting the DMA mask pointer? I think it's safe to overwrite (and perhaps even better to do so) provided OF core enables 32-bits blindly, without checking anything about the device. Eventually that dma_mask setting should, likely, come via DeviceTree to the platform_devices. Until, we can overwrite with whatever we can discover out of the device's registers. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/2] usb: dwc3: adapt to use dr_mode device tree helper
On Tue, Jul 09, 2013 at 02:16:32PM +0300, Roger Quadros wrote: Hi, On 07/06/2013 03:52 PM, Ruchika Kharwar wrote: This patch adapts the dwc3 to use the device tree helper of_usb_get_dr_mode for the mode of operation of the dwc3 instance being probed. Signed-off-by: Ruchika Kharwar ruch...@ti.com --- drivers/usb/dwc3/core.c | 51 +-- drivers/usb/dwc3/core.h |5 - 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c35d49d..7b98e4f 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -517,14 +517,17 @@ static int dwc3_probe(struct platform_device *pdev) } if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) - mode = DWC3_MODE_HOST; + mode = USB_DR_MODE_HOST; else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) - mode = DWC3_MODE_DEVICE; + mode = USB_DR_MODE_PERIPHERAL; else - mode = DWC3_MODE_DRD; + mode = of_usb_get_dr_mode(node); You need to check if node is not NULL before using of_usb_get_dr_mode. Also what would happen if DT passes a mode which can't be supported due to missing device driver? e.g. DT passes mode = host whereas HOST is not enabled. hmm... look closely, she's already handling that, right ? If DWC3 Host-only, then mode is hardcoded to host, if DWC3 is Gadget-only, then mode is hardcoded to peripheral and if DWC3 is DRD, then she checks DeviceTree. -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/2] [RFC] usb: dwc3: using the maximum_speed to determine if the usb3 phy is needed
Hi,
On Sat, Jul 06, 2013 at 07:53:57AM -0500, Ruchika Kharwar wrote:
When the initialization of usb3 phy fails, when enabled in the system
the dwc3_probe deferral is further qualified by the maximum speed.
In devices such as dra7xx, there are multiple dwc3 instances where the
maximum_speed is different between the instances.
This patch depends on http://www.spinics.net/lists/linux-usb/msg88627.html
Signed-off-by: Ruchika Kharwar ruch...@ti.com
---
drivers/usb/dwc3/core.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 7b98e4f..05f2205 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -460,8 +460,11 @@ static int dwc3_probe(struct platform_device *pdev)
if (ret == -ENXIO)
return ret;
- dev_err(dev, no usb3 phy configured\n);
- return -EPROBE_DEFER;
+ if (dwc-maximum_speed == USB_SPEED_SUPER) {
+ dev_err(dev, no usb3 phy configured\n);
+ return -EPROBE_DEFER;
+ } else
+ dev_dbg(dev, maximum speed is super\n);
I don't think this is enough. We don't even want to try getting the PHY
if maximum-speed isn't SuperSpeed. I have written this patch a couple
weeks back and have been meaning to send, but still didn't. It's also
available on my testing branch at [1]
[1]
http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testingid=d7e39d414310e098540605be2051d5187797be34
commit d7e39d414310e098540605be2051d5187797be34
Author: Felipe Balbi ba...@ti.com
Date: Sun Jun 30 18:39:23 2013 +0300
usb: dwc3: core: make USB3 PHY optional
If we want a port to work at any speed lower
than Superspeed, it makes no sense to even
initialize/power up the USB3 transceiver,
provided it won't be used.
We can use the oportunity to save some power
and leave the superspeed transceiver powered
off.
There is at least one such case which is
Texas Instruments' AM437x which has one
of its USB3 ports without a matching USB3
PHY (that port is hardwired to work on USB2
only).
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9893301..c86ae12 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -411,15 +411,33 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ switch (dwc-maximum_speed) {
+ case USB_SPEED_SUPER:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 0);
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 1);
+ break;
+ case USB_SPEED_HIGH:
+ case USB_SPEED_FULL:
+ case USB_SPEED_LOW:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 0);
+ break;
+ }
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
} else {
dwc-maximum_speed = pdata-maximum_speed;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ switch (dwc-maximum_speed) {
+ case USB_SPEED_SUPER:
+ dwc-usb2_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB2);
+ dwc-usb3_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB3);
+ break;
+ case USB_SPEED_HIGH:
+ case USB_SPEED_FULL:
+ case USB_SPEED_LOW:
+ dwc-usb2_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB2);
+ break;
+ }
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
}
@@ -443,19 +461,21 @@ static int dwc3_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
- if (IS_ERR(dwc-usb3_phy)) {
- ret = PTR_ERR(dwc-usb2_phy);
+ if (dwc-maximum_speed == USB_SPEED_SUPER) {
+ if (IS_ERR(dwc-usb3_phy)) {
+ ret = PTR_ERR(dwc-usb2_phy);
- /*
-* if -ENXIO is returned, it means PHY layer wasn't
-* enabled, so it makes no sense to return -EPROBE_DEFER
-* in that case, since no PHY driver will ever probe.
-*/
- if (ret == -ENXIO)
- return ret;
+ /*
+* if -ENXIO is returned, it means PHY layer
Re: [PATCH v2 1/1] usb/gadget: Kconfig: Fix configfs-based RNDIS function build
On Tue, Jul 09 2013, Andrzej Pietrasiewicz wrote: USB_CONFIGFS_RNDIS depends on USB_U_RNDIS. Select it. Reported-by: Fengguang Wu fengguang...@intel.com Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Acked-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/Kconfig |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 6123c16..abe905f 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -684,6 +684,7 @@ config USB_CONFIGFS_RNDIS depends on USB_CONFIGFS depends on NET select USB_U_ETHER + select USB_U_RNDIS select USB_F_RNDIS help Microsoft Windows XP bundles the Remote NDIS (RNDIS) protocol, -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Edgeport 416 USB to serial convertor
I am looking to use an Edgeport 416 USB to serial convertor under SLES SP2. When I plug in the device all 16 serial ports are discovered but after about 2 minutes they spontaneously disconnect and reconnect. Is this a known problem? Is there a newer driver I should be using? Any and all help appreciated. thanks, bfp uname -a returns: Linux VideoEdge 3.0.74-0.6.8-default #1 SMP Wed May 15 07:26:33 UTC 2013 (5e244d7) x86_64 x86_64 x86_64 GNU/Linux Here is excerpt from /var/log/messages showing discovery, disconnection, and reconnection: Jul 10 11:01:11 info kernel: [88862.360504] usb 1-1.5: new full-speed USB device number 4 using ehci_hcd Jul 10 11:01:11 info kernel: [88862.452642] usb 1-1.5: New USB device found, idVendor=1608, idProduct=0184 Jul 10 11:01:11 info kernel: [88862.452647] usb 1-1.5: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Jul 10 11:01:11 info kernel: [88862.452881] hub 1-1.5:1.0: USB hub found Jul 10 11:01:11 info kernel: [88862.453078] hub 1-1.5:1.0: 6 ports detected Jul 10 11:01:16 info kernel: [88867.201777] usb 1-1.5.1: new full-speed USB device number 5 using ehci_hcd Jul 10 11:01:16 info kernel: [88867.293997] usb 1-1.5.1: New USB device found, idVendor=1608, idProduct=02c7 Jul 10 11:01:16 info kernel: [88867.294002] usb 1-1.5.1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Jul 10 11:01:16 info kernel: [88867.294238] hub 1-1.5.1:1.0: USB hub found Jul 10 11:01:16 info kernel: [88867.294454] hub 1-1.5.1:1.0: 4 ports detected Jul 10 11:01:16 info kernel: [88867.381264] usb 1-1.5.2: new full-speed USB device number 6 using ehci_hcd Jul 10 11:01:16 info kernel: [88867.473478] usb 1-1.5.2: New USB device found, idVendor=1608, idProduct=02c7 Jul 10 11:01:16 info kernel: [88867.473483] usb 1-1.5.2: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Jul 10 11:01:16 info kernel: [88867.473730] hub 1-1.5.2:1.0: USB hub found Jul 10 11:01:16 info kernel: [88867.473966] hub 1-1.5.2:1.0: 4 ports detected Jul 10 11:01:16 info kernel: [88867.564739] usb 1-1.5.1.1: new full-speed USB device number 7 using ehci_hcd Jul 10 11:01:16 info kernel: [88867.663707] usb 1-1.5.1.1: New USB device found, idVendor=1608, idProduct=0247 Jul 10 11:01:16 info kernel: [88867.663712] usb 1-1.5.1.1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 Jul 10 11:01:16 info kernel: [88867.687844] USB Serial support registered for Edgeport TI 1 port adapter Jul 10 11:01:16 info kernel: [88867.687862] USB Serial support registered for Edgeport TI 2 port adapter Jul 10 11:01:16 info kernel: [88867.687892] io_ti 1-1.5.1.1:1.0: Edgeport TI 2 port adapter converter detected Jul 10 11:01:16 info kernel: [88867.737007] usb 1-1.5.1.2: new full-speed USB device number 8 using ehci_hcd Jul 10 11:01:16 info kernel: [88867.995727] usb 1-1.5.1.2: New USB device found, idVendor=1608, idProduct=0247 Jul 10 11:01:16 info kernel: [88867.995732] usb 1-1.5.1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 10 11:01:16 info kernel: [88867.995735] usb 1-1.5.1.2: Product: Edgeport/416 Jul 10 11:01:16 info kernel: [88867.995737] usb 1-1.5.1.2: Manufacturer: Digi International Jul 10 11:01:16 info kernel: [88867.995740] usb 1-1.5.1.2: SerialNumber: W83721134-1 Jul 10 11:01:16 info kernel: [88868.068265] usb 1-1.5.1.3: new full-speed USB device number 9 using ehci_hcd Jul 10 11:01:17 info kernel: [88868.327779] usb 1-1.5.1.3: New USB device found, idVendor=1608, idProduct=0247 Jul 10 11:01:17 info kernel: [88868.327784] usb 1-1.5.1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 10 11:01:17 info kernel: [88868.327787] usb 1-1.5.1.3: Product: Edgeport/416 Jul 10 11:01:17 info kernel: [88868.327789] usb 1-1.5.1.3: Manufacturer: Digi International Jul 10 11:01:17 info kernel: [88868.327792] usb 1-1.5.1.3: SerialNumber: W83721134-2 Jul 10 11:01:17 info kernel: [88868.399322] usb 1-1.5.1.4: new full-speed USB device number 10 using ehci_hcd Jul 10 11:01:17 info kernel: [88868.658578] usb 1-1.5.1.4: New USB device found, idVendor=1608, idProduct=0247 Jul 10 11:01:17 info kernel: [88868.658583] usb 1-1.5.1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 10 11:01:17 info kernel: [88868.658586] usb 1-1.5.1.4: Product: Edgeport/416 Jul 10 11:01:17 info kernel: [88868.658588] usb 1-1.5.1.4: Manufacturer: Digi International Jul 10 11:01:17 info kernel: [88868.658591] usb 1-1.5.1.4: SerialNumber: W83721134-3 Jul 10 11:01:17 warning kernel: [88868.714180] io_ti: probe of 1-1.5.1.1:1.0 failed with error -5 Jul 10 11:01:17 info kernel: [88868.714199] io_ti 1-1.5.1.2:1.0: Edgeport TI 2 port adapter converter detected Jul 10 11:01:17 info kernel: [88868.730392] usb 1-1.5.2.1: new full-speed USB device number 11 using ehci_hcd Jul 10 11:01:17 info kernel: [88868.854737] usb 1-1.5.1.2: Edgeport TI 2 port adapter converter now attached to ttyUSB4 Jul 10 11:01:17 info kernel: [88868.854783] usb 1-1.5.1.2: Edgeport TI 2 port adapter converter now attached to ttyUSB5 Jul 10
Re: [PATCH 1/2] usb: dwc3: adapt to use dr_mode device tree helper
On 07/10/2013 02:42 PM, Felipe Balbi wrote: On Tue, Jul 09, 2013 at 02:16:32PM +0300, Roger Quadros wrote: Hi, On 07/06/2013 03:52 PM, Ruchika Kharwar wrote: This patch adapts the dwc3 to use the device tree helper of_usb_get_dr_mode for the mode of operation of the dwc3 instance being probed. Signed-off-by: Ruchika Kharwar ruch...@ti.com --- drivers/usb/dwc3/core.c | 51 +-- drivers/usb/dwc3/core.h |5 - 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c35d49d..7b98e4f 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -517,14 +517,17 @@ static int dwc3_probe(struct platform_device *pdev) } if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) - mode = DWC3_MODE_HOST; + mode = USB_DR_MODE_HOST; else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) - mode = DWC3_MODE_DEVICE; + mode = USB_DR_MODE_PERIPHERAL; else - mode = DWC3_MODE_DRD; + mode = of_usb_get_dr_mode(node); You need to check if node is not NULL before using of_usb_get_dr_mode. Also what would happen if DT passes a mode which can't be supported due to missing device driver? e.g. DT passes mode = host whereas HOST is not enabled. hmm... look closely, she's already handling that, right ? If DWC3 Host-only, then mode is hardcoded to host, if DWC3 is Gadget-only, then mode is hardcoded to peripheral and if DWC3 is DRD, then she checks DeviceTree. In the above example if DT passes mode = host but CONFIG_USB_DWC3_GADGET is enabled in config, the dwc3 driver will work in gadget only mode. There is no warning/information to the user that it can't be enabled in host mode. All I'm saying is that there must be some kind of indication about the requested mode not being available because of a configuration issue. cheers, -roger -- 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 1/2] usb: dwc3: adapt to use dr_mode device tree helper
Hi, On Wed, Jul 10, 2013 at 04:11:52PM +0300, Roger Quadros wrote: On 07/10/2013 02:42 PM, Felipe Balbi wrote: On Tue, Jul 09, 2013 at 02:16:32PM +0300, Roger Quadros wrote: Hi, On 07/06/2013 03:52 PM, Ruchika Kharwar wrote: This patch adapts the dwc3 to use the device tree helper of_usb_get_dr_mode for the mode of operation of the dwc3 instance being probed. Signed-off-by: Ruchika Kharwar ruch...@ti.com --- drivers/usb/dwc3/core.c | 51 +-- drivers/usb/dwc3/core.h |5 - 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c35d49d..7b98e4f 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -517,14 +517,17 @@ static int dwc3_probe(struct platform_device *pdev) } if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) - mode = DWC3_MODE_HOST; + mode = USB_DR_MODE_HOST; else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) - mode = DWC3_MODE_DEVICE; + mode = USB_DR_MODE_PERIPHERAL; else - mode = DWC3_MODE_DRD; + mode = of_usb_get_dr_mode(node); You need to check if node is not NULL before using of_usb_get_dr_mode. Also what would happen if DT passes a mode which can't be supported due to missing device driver? e.g. DT passes mode = host whereas HOST is not enabled. hmm... look closely, she's already handling that, right ? If DWC3 Host-only, then mode is hardcoded to host, if DWC3 is Gadget-only, then mode is hardcoded to peripheral and if DWC3 is DRD, then she checks DeviceTree. In the above example if DT passes mode = host but CONFIG_USB_DWC3_GADGET is enabled in config, the dwc3 driver will work in gadget only mode. There is no warning/information to the user that it can't be enabled in host mode. All I'm saying is that there must be some kind of indication about the requested mode not being available because of a configuration issue. oh, alright. But that's not part of $subject ;-) So there are just two fixes which needs to be made to this patch: 1) call of_usb_get_dr_mode() conditionally on dev-of_node 2) make sure platform_data can be used to pass the mode cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] musb: omap: Fix: pass all the resources to musb core
On Wednesday 10 July 2013 04:57 PM, Felipe Balbi wrote:
On Wed, Jul 10, 2013 at 04:29:22PM +0530, Kishon Vijay Abraham I wrote:
commit 09fc7d (usb: musb: fix incorrect usage of resource pointer)
assumes musb core will always have only 2 resources. But for OMAP
platforms there can be 3 resources (2 irq resource and 1 iomem
resource). Fixed it here.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Changes from v1:
*) Removed redundant initialization of *i*
drivers/usb/musb/omap2430.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 5b6113a..5bbef78 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -481,7 +481,7 @@ static u64 omap2430_dmamask = DMA_BIT_MASK(32);
static int omap2430_probe(struct platform_device *pdev)
{
-struct resource musb_resources[2];
+struct resource musb_resources[3];
struct musb_hdrc_platform_data *pdata = pdev-dev.platform_data;
struct omap_musb_board_data *data;
struct platform_device *musb;
@@ -489,6 +489,7 @@ static int omap2430_probe(struct platform_device *pdev)
struct device_node *np = pdev-dev.of_node;
struct musb_hdrc_config *config;
int ret = -ENOMEM;
+int i;
glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
@@ -571,15 +572,12 @@ static int omap2430_probe(struct platform_device *pdev)
memset(musb_resources, 0x00, sizeof(*musb_resources) *
ARRAY_SIZE(musb_resources));
-musb_resources[0].name = pdev-resource[0].name;
-musb_resources[0].start = pdev-resource[0].start;
-musb_resources[0].end = pdev-resource[0].end;
-musb_resources[0].flags = pdev-resource[0].flags;
-
-musb_resources[1].name = pdev-resource[1].name;
-musb_resources[1].start = pdev-resource[1].start;
-musb_resources[1].end = pdev-resource[1].end;
-musb_resources[1].flags = pdev-resource[1].flags;
+for (i = 0; i ARRAY_SIZE(musb_resources); i++) {
then this is not enough, what if one device using omap2430.c has 2
resources and the other has 3 ? and what if a new one has 4 ?
How about using pdev-num_resources to dynamically allocate
musb_resources array and using the same thing iterate here ?
Yeah. This looks better.
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: [PATCH 1/2] usb: dwc3: adapt to use dr_mode device tree helper
On 07/10/2013 04:35 PM, Felipe Balbi wrote: Hi, On Wed, Jul 10, 2013 at 04:11:52PM +0300, Roger Quadros wrote: On 07/10/2013 02:42 PM, Felipe Balbi wrote: On Tue, Jul 09, 2013 at 02:16:32PM +0300, Roger Quadros wrote: Hi, On 07/06/2013 03:52 PM, Ruchika Kharwar wrote: This patch adapts the dwc3 to use the device tree helper of_usb_get_dr_mode for the mode of operation of the dwc3 instance being probed. Signed-off-by: Ruchika Kharwar ruch...@ti.com --- drivers/usb/dwc3/core.c | 51 +-- drivers/usb/dwc3/core.h |5 - 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c35d49d..7b98e4f 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -517,14 +517,17 @@ static int dwc3_probe(struct platform_device *pdev) } if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) - mode = DWC3_MODE_HOST; + mode = USB_DR_MODE_HOST; else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) - mode = DWC3_MODE_DEVICE; + mode = USB_DR_MODE_PERIPHERAL; else - mode = DWC3_MODE_DRD; + mode = of_usb_get_dr_mode(node); You need to check if node is not NULL before using of_usb_get_dr_mode. Also what would happen if DT passes a mode which can't be supported due to missing device driver? e.g. DT passes mode = host whereas HOST is not enabled. hmm... look closely, she's already handling that, right ? If DWC3 Host-only, then mode is hardcoded to host, if DWC3 is Gadget-only, then mode is hardcoded to peripheral and if DWC3 is DRD, then she checks DeviceTree. In the above example if DT passes mode = host but CONFIG_USB_DWC3_GADGET is enabled in config, the dwc3 driver will work in gadget only mode. There is no warning/information to the user that it can't be enabled in host mode. All I'm saying is that there must be some kind of indication about the requested mode not being available because of a configuration issue. oh, alright. But that's not part of $subject ;-) Agreed. Warning the user can be a different patch. So there are just two fixes which needs to be made to this patch: 1) call of_usb_get_dr_mode() conditionally on dev-of_node 2) make sure platform_data can be used to pass the mode perfect :) cheers, -roger -- 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 2/2] [RFC] usb: dwc3: using the maximum_speed to determine if the usb3 phy is needed
On 07/10/2013 06:46 AM, Felipe Balbi wrote:
Hi,
On Sat, Jul 06, 2013 at 07:53:57AM -0500, Ruchika Kharwar wrote:
When the initialization of usb3 phy fails, when enabled in the system
the dwc3_probe deferral is further qualified by the maximum speed.
In devices such as dra7xx, there are multiple dwc3 instances where the
maximum_speed is different between the instances.
This patch depends on http://www.spinics.net/lists/linux-usb/msg88627.html
Signed-off-by: Ruchika Kharwar ruch...@ti.com
---
drivers/usb/dwc3/core.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 7b98e4f..05f2205 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -460,8 +460,11 @@ static int dwc3_probe(struct platform_device *pdev)
if (ret == -ENXIO)
return ret;
- dev_err(dev, no usb3 phy configured\n);
- return -EPROBE_DEFER;
+ if (dwc-maximum_speed == USB_SPEED_SUPER) {
+ dev_err(dev, no usb3 phy configured\n);
+ return -EPROBE_DEFER;
+ } else
+ dev_dbg(dev, maximum speed is super\n);
I don't think this is enough. We don't even want to try getting the PHY
if maximum-speed isn't SuperSpeed. I have written this patch a couple
weeks back and have been meaning to send, but still didn't. It's also
available on my testing branch at [1]
[1]
http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testingid=d7e39d414310e098540605be2051d5187797be34
commit d7e39d414310e098540605be2051d5187797be34
Author: Felipe Balbi ba...@ti.com
Date: Sun Jun 30 18:39:23 2013 +0300
usb: dwc3: core: make USB3 PHY optional
If we want a port to work at any speed lower
than Superspeed, it makes no sense to even
initialize/power up the USB3 transceiver,
provided it won't be used.
We can use the oportunity to save some power
and leave the superspeed transceiver powered
off.
There is at least one such case which is
Texas Instruments' AM437x which has one
of its USB3 ports without a matching USB3
PHY (that port is hardwired to work on USB2
only).
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9893301..c86ae12 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -411,15 +411,33 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ switch (dwc-maximum_speed) {
+ case USB_SPEED_SUPER:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 0);
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 1);
+ break;
+ case USB_SPEED_HIGH:
+ case USB_SPEED_FULL:
+ case USB_SPEED_LOW:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
usb-phy, 0);
+ break;
+ }
dwc-needs_fifo_resize = of_property_read_bool(node, tx-fifo-resize);
} else {
dwc-maximum_speed = pdata-maximum_speed;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ switch (dwc-maximum_speed) {
+ case USB_SPEED_SUPER:
+ dwc-usb2_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB2);
+ dwc-usb3_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB3);
+ break;
+ case USB_SPEED_HIGH:
+ case USB_SPEED_FULL:
+ case USB_SPEED_LOW:
+ dwc-usb2_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB2);
+ break;
+ }
This is definitely better..
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
}
@@ -443,19 +461,21 @@ static int dwc3_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
- if (IS_ERR(dwc-usb3_phy)) {
- ret = PTR_ERR(dwc-usb2_phy);
+ if (dwc-maximum_speed == USB_SPEED_SUPER) {
+ if (IS_ERR(dwc-usb3_phy)) {
+ ret = PTR_ERR(dwc-usb2_phy);
- /*
-* if -ENXIO is returned, it means PHY layer wasn't
-* enabled, so it makes no sense to return -EPROBE_DEFER
-* in that case, since no PHY driver will ever probe.
-*/
- if (ret == -ENXIO)
- return ret;
+ /*
+* if -ENXIO is
Re: Video corruption varies by system load
On Tue, 9 Jul 2013, Devin Heitmueller wrote: So I hooked up the video and wrote a bit of Perl to parse the ISOC stream and render the underlying video frames. I can see definitively that the video returned from the device contains the corruption. This rules out any sort of DMA or memory related issue (proving that the data is not being mangled by the host on receipt). Now that I have the raw USB trace though including timing data, I started looking at the actual underlying ISOC traffic at the time of the corruption, and found something interesting: Despite having five URBs queued at all times with an interval of 1, there are cases where the URB isn't being sent. The corruption consistently follows one of these intervals where a URB was skipped. We're expecting the host controller to request to pull the buffer every 125us, and in instances where the corruption is exhibited immediately follow a 250us gap between URBs. See attached screenshot: http://devinheitmueller.com/isoc_loss.png Packet 27082 is the packet that contains the corruption. The previous URB was received exactly 250us prior (whereas it should have been only 125us). 349.594 - 349.344 = 250. I suspect the FIFO is overflowing on the chip as a result of the host controller not asking for the buffer when it's supposed to. It's worth mentioning that the corrupt bytes are actually also found several packets later in the correct place, suggesting the chip is probably employing some sort of circular buffer which is wrapping around. So should I be digging into the EHCI URB scheduling code? Any suggestions on where else I should be poking around would be very welcome. Digging into the scheduling code probably won't help much. However you could try collecting a usbmon trace (see Documentation/usb/usbmon.txt). This would clearly show the timing of URB submissions and completions. I'll be the first to admit that this isn't my particular area of expertise - so if I've made some stupid assumption about the expected behavior for the URB timing on the bus, don't hesitate to point that out. The transfers should occur regularly at 1-microframe intervals. If they don't then something is wrong somewhere. But I think the problem is more likely to lie in the upper-level driver than in ehci-hcd. You're using the em28xx driver? At first glance, there is one obvious bug in that driver (probably not at all related to your problem, though). The em28xx_irq_callback() routine should not set urb-status. I bet the problem is related to the usage of the URB_ISO_ASAP flag. em28xx_alloc_urbs() sets URB_ISO_ASAP in urb-transfer_flags, and the value never gets cleared. In fact, that flag bit is supposed to be set only in the first URB of a stream, not in the following URBs. (The same mistake is present for the URBs in the audio stream.) 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: Use usb_hcd_platform_shutdown() wherever possible
On Wed, 10 Jul 2013, Roger Quadros wrote: On 07/09/2013 05:16 PM, Alan Stern wrote: On Tue, 9 Jul 2013, Roger Quadros wrote: Most HCD drivers are doing the same thing in their .shutdown callback so it makes sense to use the generic usb_hcd_platform_shutdown() handler there. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/host/ehci-grlib.c | 11 +-- drivers/usb/host/ehci-mxc.c | 10 +- drivers/usb/host/ehci-omap.c | 10 +- drivers/usb/host/ehci-ppc-of.c| 11 +-- drivers/usb/host/ehci-s5p.c | 10 +- drivers/usb/host/ehci-tegra.c | 10 +- drivers/usb/host/ehci-xilinx-of.c | 17 + drivers/usb/host/ohci-omap3.c | 10 +- 8 files changed, 8 insertions(+), 81 deletions(-) This all looks fine. But unless my kernel tree is out of date, you missed ohci-ppc-of.c. You are right. I missed it and will send a revision. I've also noticed some drivers doing non-standard stuff. e.g. - ehci-ps3.c and ohci-pst set .shutdown as well as .remove to to ps3_ehci_remove I don't know why they do that. There not be any good reason. You could try asking the PS3 platform maintainer. - ehci-tilegx.c and ohci-tilegx call .remove in the .shutdown path Again, I don't know why. The TILE architecture maintainer might know. - ehci-mv.c checks for (!hcd-rh_registered) in the shudown remove patch. Is this necessary? I suspect this is because the driver is trying to cope with switching between host mode and device (peripheral) mode. This doesn't seem like a good way to implement OTG, but until it gets changed we'll have to live with the driver the way 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: Video corruption varies by system load
Hi Alan, On Wed, Jul 10, 2013 at 10:48 AM, Alan Stern st...@rowland.harvard.edu wrote: Digging into the scheduling code probably won't help much. However you could try collecting a usbmon trace (see Documentation/usb/usbmon.txt). This would clearly show the timing of URB submissions and completions. Good suggestion. I'll do that. I'll be the first to admit that this isn't my particular area of expertise - so if I've made some stupid assumption about the expected behavior for the URB timing on the bus, don't hesitate to point that out. The transfers should occur regularly at 1-microframe intervals. If they don't then something is wrong somewhere. But I think the problem is more likely to lie in the upper-level driver than in ehci-hcd. You're using the em28xx driver? Yes. At first glance, there is one obvious bug in that driver (probably not at all related to your problem, though). The em28xx_irq_callback() routine should not set urb-status. I noticed that already and have it fixed in my local tree. It'll be in my next pull request. That said, fixing that had no effect. I bet the problem is related to the usage of the URB_ISO_ASAP flag. em28xx_alloc_urbs() sets URB_ISO_ASAP in urb-transfer_flags, and the value never gets cleared. In fact, that flag bit is supposed to be set only in the first URB of a stream, not in the following URBs. (The same mistake is present for the URBs in the audio stream.) Wow, REALLY? Ok, if that's the case then I will fix that and see if it makes any difference. That really should be documented somewhere because I've seen it done that way in a bunch of different drivers (and in fact done it myself that way in several drivers I wrote in the media tree). Just so I'm understanding what is supposed to be the expected behavior - so it should be set in the first URB, but what about when we resubmit the URBs? Should I be clearing the flag prior to resubmitting the first URB (since it will be unchanged)? Or is the expected behavior that I set it on the first URB, and then nothing is supposed to ever touch transfer flags on any URB from that point forward? While on that topic, I'm clearing the status and actual_length fields in all of the iso_frame_desc[] fields of the URB prior to resubmitting - should I be doing that? Are there other fields I should be resetting? Thank you so much for your help! Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: Edgeport 416 USB to serial convertor
On Wed, Jul 10, 2013 at 12:09:35PM +, Brian Peters wrote: I am looking to use an Edgeport 416 USB to serial convertor under SLES SP2. Great, then please contact SuSE for support, as you are paying for it already :) There's nothing we can do with their kernel, as it is different from the latest kernel release (3.10). When I plug in the device all 16 serial ports are discovered but after about 2 minutes they spontaneously disconnect and reconnect. Is this a known problem? Is there a newer driver I should be using? Sounds like an electrical problem with the device, or the hub you are plugging it into. Have you checked that? good luck, 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: Video corruption varies by system load
On Wed, Jul 10, 2013 at 10:58 AM, Devin Heitmueller dheitmuel...@kernellabs.com wrote: I bet the problem is related to the usage of the URB_ISO_ASAP flag. em28xx_alloc_urbs() sets URB_ISO_ASAP in urb-transfer_flags, and the value never gets cleared. In fact, that flag bit is supposed to be set only in the first URB of a stream, not in the following URBs. (The same mistake is present for the URBs in the audio stream.) Wow, REALLY? Ok, if that's the case then I will fix that and see if it makes any difference. Nope, that wasn't it. I am now only setting ISO_ASAP in the first packet, and I tried both leaving it as in on resubmit and clearing the flag prior to resubmit. In retrospect, it seems a bit strange that if that were the problem the behavior would have changed when introducing system load. Regardless, I'm happy to fix any conformance issues you see with the driver (even if they don't fix the problem at hand). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: I bet the problem is related to the usage of the URB_ISO_ASAP flag. em28xx_alloc_urbs() sets URB_ISO_ASAP in urb-transfer_flags, and the value never gets cleared. In fact, that flag bit is supposed to be set only in the first URB of a stream, not in the following URBs. (The same mistake is present for the URBs in the audio stream.) Wow, REALLY? Ok, if that's the case then I will fix that and see if it makes any difference. That really should be documented somewhere because I've seen it done that way in a bunch of different drivers (and in fact done it myself that way in several drivers I wrote in the media tree). It _is_ documented, in a couple of places. It is discussed in the kerneldoc preceding the definition of struct urb in include/linux/usb.h and in the kerneldoc for usb_submit_urb() in drivers/usb/core/urb.c. However, the meaning now is different from what it used to be in the past, and it is liable to undergo a slight change in the future. Stay tuned. Basically, URB_ISO_ASAP now means that the driver wants the URB to be scheduled for the next definitely available time slot, and is willing to skip some time slots if necessary to insure this. Not setting URB_ISO_ASAP means the driver wants the URB to be scheduled for the time slot following the end of the preceding URB, even if that slot may already be expired. There is a certain ambiguity about what to do when a stream is started or restarted, because then the notion of preceding URB doesn't make sense. Even worse, the host controller driver can't always tell when a stream is being restarted -- that's why I suggest avoiding the whole issue by always setting URB_ISO_ASAP on the first URB of a new or restarting stream. The definitely available phrase above is probably the source of your trouble. A time slot that is already in the past certainly isn't available. However, a time slot that is still in the future may not be definitely available either, because the hardware requires that URBs be submitted somewhat in advance of when they will be used. Depending on the hardware, a time slot may not be definitely available unless it is over 1 ms in the future. I recommend setting up isochronous pipelines to be at least 2 ms long. You can use less if 2 ms is more latency than you want, but don't go under 1 ms. Just so I'm understanding what is supposed to be the expected behavior - so it should be set in the first URB, but what about when we resubmit the URBs? Should I be clearing the flag prior to resubmitting the first URB (since it will be unchanged)? Or is the expected behavior that I set it on the first URB, and then nothing is supposed to ever touch transfer flags on any URB from that point forward? Given that so many drivers do this wrong, it might be worthwhile for the USB core to clear the URB_ISO_ASAP flag before calling the URB's completion handler. That way drivers won't have to worry about clearing it themselves. (On the other hand, there is at least one driver that probably does want to have the flag set during resubmissions, so then it would have to be updated.) Alternatively, and for now at least, you can try clearing that flag yourself before resubmitting the URBs. While on that topic, I'm clearing the status and actual_length fields in all of the iso_frame_desc[] fields of the URB prior to resubmitting - should I be doing that? You don't have to; the USB core does that for you. There's nothing _wrong_ with it; it's just a waste of time. Are there other fields I should be resetting? For an isochronous stream, all you really need to do for resubmission is make sure the transfer_buffer (and possibly transfer_dma), transfer_buffer_length, number_of_packets, and iso_frame_desc[i].offset and .length values are set properly. Everything else can be left as is. Thank you so much for your help! You're welcome. 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: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: On Wed, Jul 10, 2013 at 10:58 AM, Devin Heitmueller dheitmuel...@kernellabs.com wrote: I bet the problem is related to the usage of the URB_ISO_ASAP flag. em28xx_alloc_urbs() sets URB_ISO_ASAP in urb-transfer_flags, and the value never gets cleared. In fact, that flag bit is supposed to be set only in the first URB of a stream, not in the following URBs. (The same mistake is present for the URBs in the audio stream.) Wow, REALLY? Ok, if that's the case then I will fix that and see if it makes any difference. Nope, that wasn't it. I am now only setting ISO_ASAP in the first packet, and I tried both leaving it as in on resubmit and clearing the flag prior to resubmit. usbmon is the best debugging tool at this point. In retrospect, it seems a bit strange that if that were the problem the behavior would have changed when introducing system load. It's possible that increasing the system load can cause resubmissions to be delayed for too long. Regardless, I'm happy to fix any conformance issues you see with the driver (even if they don't fix the problem at hand). For now, let's just try to find the source of 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: Video corruption varies by system load
On Wed, Jul 10, 2013 at 11:40 AM, Alan Stern st...@rowland.harvard.edu wrote: Nope, that wasn't it. I am now only setting ISO_ASAP in the first packet, and I tried both leaving it as in on resubmit and clearing the flag prior to resubmit. usbmon is the best debugging tool at this point. http://www.devinheitmueller.com/em28xx_usbmon.log (only about 290KB). I'm just starting to read through it now, but figured it would be better to send it along while doing such. Probably the biggest issue at this point is that while I was definitely seeing the corruption on the screen while creating the trace, I don't yet have a way to correlate that corruption to a particular spot in the usbmon trace. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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 1/6] ARM: OMAP3: Enable Hardware Save and Restore for USB Host
To ensure hardware context is restored while resuming from
OFF mode we need to enable the Hardware SAR bit for the
USB Host power domain.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/powerdomains3xxx_data.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c
b/arch/arm/mach-omap2/powerdomains3xxx_data.c
index e2d4bd8..7b44d1f 100644
--- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
@@ -289,13 +289,7 @@ static struct powerdomain usbhost_pwrdm = {
.prcm_offs= OMAP3430ES2_USBHOST_MOD,
.pwrsts = PWRSTS_OFF_RET_ON,
.pwrsts_logic_ret = PWRSTS_RET,
- /*
-* REVISIT: Enabling usb host save and restore mechanism seems to
-* leave the usb host domain permanently in ACTIVE mode after
-* changing the usb host power domain state from OFF to active once.
-* Disabling for now.
-*/
- /*.flags = PWRDM_HAS_HDWR_SAR,*/ /* for USBHOST ctrlr only */
+ .flags= PWRDM_HAS_HDWR_SAR, /* for USBHOST ctrlr only */
.banks= 1,
.pwrsts_mem_ret = {
[0] = PWRSTS_RET, /* MEMRETSTATE */
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] USB: Implement runtime idling and remote wakeup for OMAP EHCI controller
Hi, This series implements suspend/resume for the OMAP EHCI host controller during runtime idle. This will cause its parent, the OMAP USB Host Module as well as the USB TLL Module to be put in suspend and hence allow the USB power domain to be put in a lower power state. Then we no longer prevent the rest of the OMAP SoC from entering lower power states like RETention or OFF mode when USB (or system) is suspended. This was one of the main reason why EHCI_OMAP is still not enabled in OMAP2 defconfig. In order for remote wakeup or hub events (connect/disconnect) to be detected while in suspend, we need to rely on the IO daisy chaining mechanism on OMAP. This is nothing but configuring a pin to be wakeup capable and triggering an interrupt on any pin activity while the hardware module or the entire SoC is in sleep state. For this to work, we rely on the wakeup feature added to the omap-pinctrl-single driver in [1]. This takes care of routing IO pad wakeup interrupt to the right driver's interrupt handler (i.e. ehci_irq in our case). The pin state information for DEFAULT and IDLE is specified for the omap3beagle-xm board in patch 2. So this is tested only on omap3beagle-xm board. Patch 4 takes care of switching the pin states between DEFAULT and IDLE during idle/active mode using the pinctrl framework. Patch 5 takes care of handling the wakeup IRQ when the USB controller is suspended and Hardware is not accessible. Patch 6 implements runtime and system suspend/resume for the OMAP EHCI controller Changelog: v1: - addressed review comments on RFC patchset - Special case handling of controllers that can generate wakeup IRQ when suspended is done in HCD core instead of ehci-hcd. - Patches based on linus/master commit 496322bc91e35007ed754184dcd447a02b6dd685 with the following patches on top [1] - OMAP pinctrl wakeup support http://thread.gmane.org/gmane.linux.ports.arm.omap/99010/focus=99041 This does not apply cleanly though. Tony has agreed to post a revision anytime soon. [2] - [PATCH v5 0/2] ARM: dts: Add USB host support for Beagle-xm http://thread.gmane.org/gmane.linux.drivers.devicetree/39291 [3] - USB-EHCI-Fix-resume-signalling-on-remote-wakeup.patch http://thread.gmane.org/gmane.linux.usb.general/89608 cheers, -roger --- Roger Quadros (6): ARM: OMAP3: Enable Hardware Save and Restore for USB Host ARM: dts: omap3beagle-xm: Add idle state pins for USB host mfd: omap-usb-host: move initialization to module_init() mfd: omap-usb-host: Put pins in IDLE state on suspend USB: Support wakeup IRQ for suspended controllers USB: ehci-omap: Implement suspend/resume arch/arm/boot/dts/omap3-beagle-xm.dts | 29 ++--- arch/arm/mach-omap2/powerdomains3xxx_data.c |8 +--- drivers/mfd/omap-usb-host.c | 45 ++ drivers/mfd/omap-usb-tll.c | 20 + drivers/usb/core/hcd.c | 21 - drivers/usb/host/ehci-omap.c| 64 ++- include/linux/usb/hcd.h |3 + 7 files changed, 136 insertions(+), 54 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] ARM: dts: omap3beagle-xm: Add idle state pins for USB host
Add the Idle state pins for USB host and enable WAKEUP on
DIR, DAT0-3, so that the PHY can wakeup the OMAP SoC from
sleep on any USB activity (e.g. remote wakeup or connect/disconnect).
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap3-beagle-xm.dts | 29 +++--
1 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts
b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 371b1e2..91d19fa 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -113,11 +113,6 @@
};
omap3_pmx_core {
- pinctrl-names = default;
- pinctrl-0 =
- hsusbb2_pins
- ;
-
uart3_pins: pinmux_uart3_pins {
pinctrl-single,pins =
0x16e (PIN_INPUT | PIN_OFF_WAKEUPENABLE | MUX_MODE0) /*
uart3_rx_irrx.uart3_rx_irrx */
@@ -125,7 +120,7 @@
;
};
- hsusbb2_pins: pinmux_hsusbb2_pins {
+ hsusb2_pins: pinmux_hsusb2_pins {
pinctrl-single,pins =
0x5c0 (PIN_OUTPUT | MUX_MODE3) /*
etk_d10.hsusb2_clk */
0x5c2 (PIN_OUTPUT | MUX_MODE3) /*
etk_d11.hsusb2_stp */
@@ -141,6 +136,25 @@
0x1ae (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs1.hsusb2_data3 */
;
};
+
+ /* WAKEUP enabled on DIR, DAT0-3 */
+ hsusb2_idle_pins: pinmux_hsusb2_idle_pins {
+ pinctrl-single,pins =
+ 0x5c0 (PIN_OUTPUT | MUX_MODE3) /*
etk_d10.hsusb2_clk */
+ 0x5c2 (PIN_OUTPUT | MUX_MODE3) /*
etk_d11.hsusb2_stp */
+ 0x5c4 (PIN_INPUT_PULLDOWN | WAKEUP_EN | MUX_MODE3)
/* etk_d12.hsusb2_dir */
+ 0x5c6 (PIN_INPUT_PULLDOWN | MUX_MODE3)
/* etk_d13.hsusb2_nxt */
+ 0x5c8 (PIN_INPUT_PULLDOWN | WAKEUP_EN | MUX_MODE3)
/* etk_d14.hsusb2_data0 */
+ 0x5cA (PIN_INPUT_PULLDOWN | WAKEUP_EN | MUX_MODE3)
/* etk_d15.hsusb2_data1 */
+ 0x1a4 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi1_cs3.hsusb2_data2 */
+ 0x1a6 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_clk.hsusb2_data7 */
+ 0x1a8 (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_simo.hsusb2_data4 */
+ 0x1aa (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_somi.hsusb2_data5 */
+ 0x1ac (PIN_INPUT_PULLDOWN | MUX_MODE3) /*
mcspi2_cs0.hsusb2_data6 */
+ 0x1ae (PIN_INPUT_PULLDOWN | WAKEUP_EN | MUX_MODE3)
/* mcspi2_cs1.hsusb2_data3 */
+ ;
+ interrupts = 77; /* route padconf wakeup to EHCI IRQ */
+ };
};
i2c1 {
@@ -223,6 +237,9 @@
};
usbhshost {
+ pinctrl-names = default, idle;
+ pinctrl-0 = hsusb2_pins;
+ pinctrl-1 = hsusb2_idle_pins;
port2-mode = ehci-phy;
};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] USB: Support wakeup IRQ for suspended controllers
Some platforms e.g. ehci-omap can generate an interrupt
(i.e. remote wakeup) even when the controller is suspended i.e.
HW_ACCESSIBLE is cleared.
Introduce a flag has_wakeup_irq in struct usb_hcd to indicate
such cases.
We tackle this case by disabling the IRQ, scheduling a
hub resume and enabling back the IRQ after the controller has
resumed. This ensures that the IRQ handler runs only after the
controller is accessible.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/core/hcd.c | 21 -
include/linux/usb/hcd.h |3 +++
2 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 014dc99..933d8f1 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2161,6 +2161,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)) {
+ /* We can now process IRQs so enable IRQ */
+ clear_bit(HCD_FLAG_IRQ_DISABLED, hcd-flags);
+ enable_irq(hcd-irq);
+ }
}
/**
@@ -2248,7 +2253,21 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
*/
local_irq_save(flags);
- if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd)))
+ if (unlikely(HCD_DEAD(hcd)))
+ rc = IRQ_NONE;
+ else if (unlikely(!HCD_HW_ACCESSIBLE(hcd) hcd-has_wakeup_irq)) {
+ /*
+* We got a wakeup interrupt while the controller was
+* suspending or suspended. We can't handle it now, so
+* disable the IRQ and resume the root hub (and hence
+* the controller too).
+*/
+ disable_irq_nosync(hcd-irq);
+ set_bit(HCD_FLAG_IRQ_DISABLED, hcd-flags);
+ usb_hcd_resume_root_hub(hcd);
+
+ rc = IRQ_HANDLED;
+ } else if (unlikely(!HCD_HW_ACCESSIBLE(hcd)))
rc = IRQ_NONE;
else if (hcd-driver-irq(hcd) == IRQ_NONE)
rc = IRQ_NONE;
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 1e88377..5eb9f85 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? */
@@ -132,6 +134,7 @@ struct usb_hcd {
unsignedwireless:1; /* Wireless USB HCD */
unsignedauthorized_default:1;
unsignedhas_tt:1; /* Integrated TT in root hub */
+ unsignedhas_wakeup_irq:1; /* Can IRQ when suspended */
unsigned intirq;/* irq allocated */
void __iomem*regs; /* device memory/io */
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] mfd: omap-usb-host: move initialization to module_init()
We no longer need to be initialized in any particular order
so move driver initialization to the standard place i.e. module_init()
CC: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mfd/omap-usb-host.c | 23 +++
drivers/mfd/omap-usb-tll.c | 20 ++--
2 files changed, 5 insertions(+), 38 deletions(-)
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 759fae3..fb2b3d8 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -895,31 +895,14 @@ static struct platform_driver usbhs_omap_driver = {
.pm = usbhsomap_dev_pm_ops,
.of_match_table = of_match_ptr(usbhs_omap_dt_ids),
},
+ .probe = usbhs_omap_probe,
.remove = usbhs_omap_remove,
};
+module_platform_driver(usbhs_omap_driver);
+
MODULE_AUTHOR(Keshava Munegowda keshava_mgo...@ti.com);
MODULE_AUTHOR(Roger Quadros rog...@ti.com);
MODULE_ALIAS(platform: USBHS_DRIVER_NAME);
MODULE_LICENSE(GPL v2);
MODULE_DESCRIPTION(usb host common core driver for omap EHCI and OHCI);
-
-static int __init omap_usbhs_drvinit(void)
-{
- return platform_driver_probe(usbhs_omap_driver, usbhs_omap_probe);
-}
-
-/*
- * init before ehci and ohci drivers;
- * The usbhs core driver should be initialized much before
- * the omap ehci and ohci probe functions are called.
- * This usbhs core driver should be initialized after
- * usb tll driver
- */
-fs_initcall_sync(omap_usbhs_drvinit);
-
-static void __exit omap_usbhs_drvexit(void)
-{
- platform_driver_unregister(usbhs_omap_driver);
-}
-module_exit(omap_usbhs_drvexit);
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index e59ac4c..1a3a26f 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
@@ -326,6 +326,8 @@ static struct platform_driver usbtll_omap_driver = {
.remove = usbtll_omap_remove,
};
+module_platform_driver(usbtll_omap_driver);
+
int omap_tll_init(struct usbhs_omap_platform_data *pdata)
{
int i;
@@ -477,21 +479,3 @@ MODULE_AUTHOR(Roger Quadros rog...@ti.com);
MODULE_ALIAS(platform: USBHS_DRIVER_NAME);
MODULE_LICENSE(GPL v2);
MODULE_DESCRIPTION(usb tll driver for TI OMAP EHCI and OHCI controllers);
-
-static int __init omap_usbtll_drvinit(void)
-{
- return platform_driver_register(usbtll_omap_driver);
-}
-
-/*
- * init before usbhs core driver;
- * The usbtll driver should be initialized before
- * the usbhs core driver probe function is called.
- */
-fs_initcall(omap_usbtll_drvinit);
-
-static void __exit omap_usbtll_drvexit(void)
-{
- platform_driver_unregister(usbtll_omap_driver);
-}
-module_exit(omap_usbtll_drvexit);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] USB: ehci-omap: Implement suspend/resume
Call ehci_suspend/resume() during runtime suspend/resume
as well as system suspend/resume.
Use a flag bound to indicate that the HCD structures are valid.
This is only true between usb_add_hcd() and usb_remove_hcd() calls.
The flag can be used by omap_ehci_runtime_suspend/resume() handlers
to avoid calling ehci_suspend/resume() when we are no longer bound
to the HCD e.g. during probe() and remove();
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/host/ehci-omap.c | 64 -
1 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 9bd7dfe..6d1fde97 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -69,6 +69,7 @@ static const char hcd_name[] = ehci-omap;
struct omap_hcd {
struct usb_phy *phy[OMAP3_HS_USB_PORTS]; /* one PHY for each port */
int nports;
+ bool bound; /* HCD structures are valid */
};
static inline void ehci_write(void __iomem *base, u32 reg, u32 val)
@@ -159,6 +160,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
hcd-rsrc_start = res-start;
hcd-rsrc_len = resource_size(res);
hcd-regs = regs;
+ hcd-has_wakeup_irq = true;
hcd_to_ehci(hcd)-caps = regs;
omap = (struct omap_hcd *)hcd_to_ehci(hcd)-priv;
@@ -216,6 +218,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
goto err_pm_runtime;
}
+ omap-bound = true;
+
/*
* Bring PHYs out of reset for non PHY modes.
* Even though HSIC mode is a PHY-less mode, the reset
@@ -232,6 +236,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:
@@ -264,7 +270,9 @@ static int ehci_hcd_omap_remove(struct platform_device
*pdev)
struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)-priv;
int i;
+ pm_runtime_get_sync(dev);
usb_remove_hcd(hcd);
+ omap-bound = false;
for (i = 0; i omap-nports; i++) {
if (omap-phy[i])
@@ -293,15 +301,67 @@ static const struct of_device_id omap_ehci_dt_ids[] = {
MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids);
+static int omap_ehci_suspend(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+ dev_dbg(dev, %s\n, __func__);
+
+ return ehci_suspend(hcd, true);
+}
+
+static int omap_ehci_resume(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+ dev_dbg(dev, %s\n, __func__);
+
+ return ehci_resume(hcd, false);
+}
+
+static int omap_ehci_runtime_suspend(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)-priv;
+ bool do_wakeup = device_may_wakeup(dev);
+
+ dev_dbg(dev, %s\n, __func__);
+
+ if (omap-bound)
+ ehci_suspend(hcd, do_wakeup);
+
+ return 0;
+}
+
+static int omap_ehci_runtime_resume(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)-priv;
+
+ dev_dbg(dev, %s\n, __func__);
+
+ if (omap-bound)
+ ehci_resume(hcd, false);
+
+ return 0;
+}
+
+
+static const struct dev_pm_ops omap_ehci_pm_ops = {
+ .suspend = omap_ehci_suspend,
+ .resume = omap_ehci_resume,
+ .runtime_suspend = omap_ehci_runtime_suspend,
+ .runtime_resume = omap_ehci_runtime_resume,
+};
+
static struct platform_driver ehci_hcd_omap_driver = {
.probe = ehci_hcd_omap_probe,
.remove = ehci_hcd_omap_remove,
.shutdown = ehci_hcd_omap_shutdown,
- /*.suspend = ehci_hcd_omap_suspend, */
- /*.resume = ehci_hcd_omap_resume, */
.driver = {
.name = hcd_name,
.of_match_table = omap_ehci_dt_ids,
+ .pm = omap_ehci_pm_ops,
}
};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] mfd: omap-usb-host: Put pins in IDLE state on suspend
In order to support wake up from suspend use the pinctrl
framework to put the USB host pins in IDLE state during suspend.
CC: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mfd/omap-usb-host.c | 22 ++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index fb2b3d8..3734d16 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -31,6 +31,7 @@
#include linux/of.h
#include linux/of_platform.h
#include linux/err.h
+#include linux/pinctrl/consumer.h
#include omap-usb.h
@@ -111,6 +112,7 @@ struct usbhs_hcd_omap {
struct usbhs_omap_platform_data *pdata;
u32 usbhs_rev;
+ boolno_idle;
};
/*-*/
@@ -325,6 +327,7 @@ static int usbhs_runtime_resume(struct device *dev)
dev_dbg(dev, usbhs_runtime_resume\n);
+ pinctrl_pm_select_default_state(dev);
omap_tll_enable(pdata);
if (!IS_ERR(omap-ehci_logic_fck))
@@ -378,6 +381,12 @@ static int usbhs_runtime_suspend(struct device *dev)
dev_dbg(dev, usbhs_runtime_suspend\n);
+ if (omap-no_idle) {
+ dev_dbg(dev, %s: Not suspending as IDLE pins not available\n,
+ __func__);
+ return -EBUSY;
+ }
+
for (i = 0; i omap-nports; i++) {
switch (pdata-port_mode[i]) {
case OMAP_EHCI_PORT_MODE_HSIC:
@@ -401,6 +410,7 @@ static int usbhs_runtime_suspend(struct device *dev)
clk_disable(omap-ehci_logic_fck);
omap_tll_disable(pdata);
+ pinctrl_pm_select_idle_state(dev);
return 0;
}
@@ -608,6 +618,14 @@ static int usbhs_omap_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ if (!dev-pins || !dev-pins-idle_state) {
+ /* If IDLE pins are not available, we can't remote wakeup,
+* so prevent idling in that case.
+*/
+ omap-no_idle = true;
+ dev_info(dev, No IDLE pins so runtime idle disabled\n);
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
omap-uhh_base = devm_ioremap_resource(dev, res);
if (IS_ERR(omap-uhh_base))
@@ -796,6 +814,8 @@ static int usbhs_omap_probe(struct platform_device *pdev)
}
}
+ pinctrl_pm_select_default_state(dev);
+
return 0;
err_alloc:
@@ -872,6 +892,8 @@ static int usbhs_omap_remove(struct platform_device *pdev)
/* remove children */
device_for_each_child(pdev-dev, NULL, usbhs_omap_remove_child);
+ pinctrl_pm_select_idle_state(pdev-dev);
+
return 0;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: On Wed, Jul 10, 2013 at 11:40 AM, Alan Stern st...@rowland.harvard.edu wrote: Nope, that wasn't it. I am now only setting ISO_ASAP in the first packet, and I tried both leaving it as in on resubmit and clearing the flag prior to resubmit. usbmon is the best debugging tool at this point. http://www.devinheitmueller.com/em28xx_usbmon.log (only about 290KB). I'm just starting to read through it now, but figured it would be better to send it along while doing such. Probably the biggest issue at this point is that while I was definitely seeing the corruption on the screen while creating the trace, I don't yet have a way to correlate that corruption to a particular spot in the usbmon trace. If you use the bus analyzer at the same time, you could compare microframe numbers. You're using 64 packets/URB, so each URB lasts 8 ms. In addition, you have a pipeline of 5 URBs, for a total of 40 ms. That should be plenty. I don't see any problems in the usbmon trace, but they might not show up there. In particular, the trace includes the status values only for the first 5 packets in each URB. Does the driver encounter any packets with a nonzero status? It could be that you're facing some sort of hardware limitation of the host controller. Can you try running the test on a computer with a different brand of motherboard? 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: Video corruption varies by system load
On Wed, Jul 10, 2013 at 12:30 PM, Alan Stern st...@rowland.harvard.edu wrote: If you use the bus analyzer at the same time, you could compare microframe numbers. Ah, again, good suggestion. I'll get a usbmon trace in parallel to the Beagle. I'll have to move some stuff around though because I don't want to run the Beagle on the same system the em28xx device is connected to. You're using 64 packets/URB, so each URB lasts 8 ms. In addition, you have a pipeline of 5 URBs, for a total of 40 ms. That should be plenty. Yeah, it felt pretty conservative. 64 packets/URB is what the Windows driver does, and I tried higher numbers of URBs (up to 12) with no change in behavior. I don't see any problems in the usbmon trace, but they might not show up there. In particular, the trace includes the status values only for the first 5 packets in each URB. Does the driver encounter any packets with a nonzero status? No. I added code to the driver to do a printk() on any error conditions (both at the URB level and at the isoc packet level), and the log has been clean. It could be that you're facing some sort of hardware limitation of the host controller. Can you try running the test on a computer with a different brand of motherboard? Ruled that out already. I'm seeing the behavior on my 2010 MacBook Pro (Intel EHCI), a Dell XPS system (also Intel EHCI), and a TI 8147 Davinci embedded system (using the musb HCD), I've also tried three different Empia designs and they all exhibit the same behavior (em2820, em2861, em2883). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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] option: add Olivetti Olicard 200
Speaks AT on interfaces 5 (command PPP) and 3 (secondary), other
interface protocols are unknown.
Cc: sta...@vger.kernel.org
Signed-off-by: Dan Williams d...@redhat.com
---
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 5dd857d..feda580 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -341,6 +341,7 @@ static void option_instat_callback(struct urb *urb);
#define OLIVETTI_VENDOR_ID 0x0b3c
#define OLIVETTI_PRODUCT_OLICARD1000xc000
#define OLIVETTI_PRODUCT_OLICARD1450xc003
+#define OLIVETTI_PRODUCT_OLICARD2000xc005
/* Celot products */
#define CELOT_VENDOR_ID0x211f
@@ -1256,6 +1257,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD100) },
{ USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD145) },
+ { USB_DEVICE(OLIVETTI_VENDOR_ID, OLIVETTI_PRODUCT_OLICARD200) },
{ USB_DEVICE(CELOT_VENDOR_ID, CELOT_PRODUCT_CT680M) }, /* CT-650 CDMA
450 1xEVDO modem */
{ USB_DEVICE(ONDA_VENDOR_ID, ONDA_MT825UP) }, /* ONDA MT825UP modem */
{ USB_DEVICE_AND_INTERFACE_INFO(SAMSUNG_VENDOR_ID,
SAMSUNG_PRODUCT_GT_B3730, USB_CLASS_CDC_DATA, 0x00, 0x00) }, /* Samsung
GT-B3730 LTE USB modem.*/
--
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: phy: samsung-usb2: Toggle HSIC GPIO from device tree
Hi Felipe, This is intended to pull down a reset signal line, not to switch power to the device. I could implement that with the regulator framework too, but I think that would just be confusing and harder to understand without providing any benefit. It's really just a plain old GPIO. -- 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: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: I don't see any problems in the usbmon trace, but they might not show up there. In particular, the trace includes the status values only for the first 5 packets in each URB. Does the driver encounter any packets with a nonzero status? No. I added code to the driver to do a printk() on any error conditions (both at the URB level and at the isoc packet level), and the log has been clean. You inspired me to take a closer look at the usbmon log you made available. There _is_ an error indication after all; the line with timestamp 397263317 got an error in one of its 64 packets (but this was the only error in the entire trace). The trace doesn't say what the error was or which packet it occurred in. This error should have shown up in your driver as a nonzero value for the packet status. If you want to investigate further, you can capture the entire data stream using Wireshark. (Of course, that means capturing 22 MB/s of data.) It could be that you're facing some sort of hardware limitation of the host controller. Can you try running the test on a computer with a different brand of motherboard? Ruled that out already. I'm seeing the behavior on my 2010 MacBook Pro (Intel EHCI), a Dell XPS system (also Intel EHCI), and a TI 8147 Davinci embedded system (using the musb HCD), I've also tried three different Empia designs and they all exhibit the same behavior (em2820, em2861, em2883). It's hard to see how they could all suffer from the same hardware bug. 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 5/6] USB: Support wakeup IRQ for suspended controllers
On Wed, 10 Jul 2013, Roger Quadros wrote:
Some platforms e.g. ehci-omap can generate an interrupt
(i.e. remote wakeup) even when the controller is suspended i.e.
HW_ACCESSIBLE is cleared.
Introduce a flag has_wakeup_irq in struct usb_hcd to indicate
such cases.
We tackle this case by disabling the IRQ, scheduling a
hub resume and enabling back the IRQ after the controller has
resumed. This ensures that the IRQ handler runs only after the
controller is accessible.
@@ -2248,7 +2253,21 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
*/
local_irq_save(flags);
- if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd)))
+ if (unlikely(HCD_DEAD(hcd)))
+ rc = IRQ_NONE;
+ else if (unlikely(!HCD_HW_ACCESSIBLE(hcd) hcd-has_wakeup_irq)) {
+ /*
+ * We got a wakeup interrupt while the controller was
+ * suspending or suspended. We can't handle it now, so
+ * disable the IRQ and resume the root hub (and hence
+ * the controller too).
+ */
+ disable_irq_nosync(hcd-irq);
+ set_bit(HCD_FLAG_IRQ_DISABLED, hcd-flags);
+ usb_hcd_resume_root_hub(hcd);
+
+ rc = IRQ_HANDLED;
+ } else if (unlikely(!HCD_HW_ACCESSIBLE(hcd)))
rc = IRQ_NONE;
In the interest of minimizing the amount of work needed in the most
common case, this should be written as:
else if (unlikely(!HCD_HW_ACCESSIBLE(hcd))) {
if (hcd-has_wakeup_irq) {
...
} else
rc = IRQ_NONE;
else if (hcd-driver-irq(hcd) == IRQ_NONE)
rc = IRQ_NONE;
Apart from that, the patch is okay. When you rearrange the logic, you
can add my Acked-by.
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 6/6] USB: ehci-omap: Implement suspend/resume
On Wed, 10 Jul 2013, Roger Quadros wrote:
Call ehci_suspend/resume() during runtime suspend/resume
as well as system suspend/resume.
Use a flag bound to indicate that the HCD structures are valid.
This is only true between usb_add_hcd() and usb_remove_hcd() calls.
The flag can be used by omap_ehci_runtime_suspend/resume() handlers
to avoid calling ehci_suspend/resume() when we are no longer bound
to the HCD e.g. during probe() and remove();
@@ -293,15 +301,67 @@ static const struct of_device_id omap_ehci_dt_ids[] = {
MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids);
+static int omap_ehci_suspend(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+ dev_dbg(dev, %s\n, __func__);
+
+ return ehci_suspend(hcd, true);
+}
+
+static int omap_ehci_resume(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+ dev_dbg(dev, %s\n, __func__);
+
+ return ehci_resume(hcd, false);
+}
There are three problems here. The first is very simple: the wakeup
settings are messed up. Wakeup is supposed to be enabled always for
runtime suspend, whereas it is controlled by device_may_wakeup() for
system suspend. You reversed them.
The other two problems are both related to the interaction between
system PM and runtime PM. Suppose the controller is already runtime
suspended when the system goes to sleep. Because it is runtime
suspended, it is enabled for wakeup. But device_may_wakeup() could
return false; if this happens then you have to do a runtime-resume in
omap_ehci_suspend() before calling ehci_suspend(), so that the
controller can be suspended again with wakeups disabled. (Or you could
choose an alternative method for accomplishing the same result, such as
disabling the wakeup signal from the pad without going through a whole
EHCI resume/suspend cycle.) Conversely, if device_may_wakeup() returns
true then you shouldn't do anything at all, because the controller is
already suspended with the correct wakeup setting.
For the third problem, suppose the controller was runtime suspended
when the system went to sleep. When the system wakes up, the
controller will become active, so you have to inform the runtime PM
core about its change of state. Basically, if omap_ehci_resume() sees
that ehci_resume() returned 0 then it must do:
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
All of these issues are discussed (among lots of other material) in
Documentation/power/runtime_pm.txt.
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: Video corruption varies by system load
Hi Alan, On Wed, Jul 10, 2013 at 2:27 PM, Alan Stern st...@rowland.harvard.edu wrote: You inspired me to take a closer look at the usbmon log you made available. There _is_ an error indication after all; the line with timestamp 397263317 got an error in one of its 64 packets (but this was the only error in the entire trace). The trace doesn't say what the error was or which packet it occurred in. This error should have shown up in your driver as a nonzero value for the packet status. I probably should have better qualified my previous statement. Every once in a while there is an actual -EOVERFLOW condition, which will result in that one frame being a bit corrupted. Those cases do show up in the URB handler via the status field. That said though, the problem I'm debugging happens 20-30 times a second, so I'm looking for something much more prevalent than one error in a five second capture. If you want to investigate further, you can capture the entire data stream using Wireshark. (Of course, that means capturing 22 MB/s of data.) Will that get me any new data/context I'm not already getting with the bus analyzer? I'm not against doing a Wireshark capture if it gets me access to some internal state of the USB stack that wouldn't be available to the Beagle, but at this point I've got scripts written to parse the Beagle data so it's not clear what the benefit would be. It could be that you're facing some sort of hardware limitation of the host controller. Can you try running the test on a computer with a different brand of motherboard? Ruled that out already. I'm seeing the behavior on my 2010 MacBook Pro (Intel EHCI), a Dell XPS system (also Intel EHCI), and a TI 8147 Davinci embedded system (using the musb HCD), I've also tried three different Empia designs and they all exhibit the same behavior (em2820, em2861, em2883). It's hard to see how they could all suffer from the same hardware bug. Agreed. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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 5/6] USB: Support wakeup IRQ for suspended controllers
On Wed, 10 Jul 2013, Roger Quadros wrote:
Some platforms e.g. ehci-omap can generate an interrupt
(i.e. remote wakeup) even when the controller is suspended i.e.
HW_ACCESSIBLE is cleared.
Introduce a flag has_wakeup_irq in struct usb_hcd to indicate
such cases.
We tackle this case by disabling the IRQ, scheduling a
hub resume and enabling back the IRQ after the controller has
resumed. This ensures that the IRQ handler runs only after the
controller is accessible.
Oh yes, one more thing...
@@ -132,6 +134,7 @@ struct usb_hcd {
unsignedwireless:1; /* Wireless USB HCD */
unsignedauthorized_default:1;
unsignedhas_tt:1; /* Integrated TT in root hub */
+ unsignedhas_wakeup_irq:1; /* Can IRQ when suspended */
Please add a highly visible comment here, warning that has_wakeup_irq
should never be set on systems with shared IRQs. Having both would ...
well, it would indicate a really bad system design.
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: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: Hi Alan, On Wed, Jul 10, 2013 at 2:27 PM, Alan Stern st...@rowland.harvard.edu wrote: You inspired me to take a closer look at the usbmon log you made available. There _is_ an error indication after all; the line with timestamp 397263317 got an error in one of its 64 packets (but this was the only error in the entire trace). The trace doesn't say what the error was or which packet it occurred in. This error should have shown up in your driver as a nonzero value for the packet status. I probably should have better qualified my previous statement. Every once in a while there is an actual -EOVERFLOW condition, which will result in that one frame being a bit corrupted. Those cases do show up in the URB handler via the status field. That said though, the problem I'm debugging happens 20-30 times a second, so I'm looking for something much more prevalent than one error in a five second capture. 20-30 times each second? Okay... I didn't realize the errors were that frequent. It's rather disturbing that they don't show up at all in the usbmon trace. This suggests two possibilities: The ehci-hcd driver is so messed up that not only did it fail to tell the hardware to send those missing packets; it also thought they had been sent and gotten replies! The controller hardware is so messed up that it failed to send the missing packets but then stored a status value indicating that they had been sent and replies had been received! Both possibilities are rather unsettling. Fortunately, they aren't exhaustive -- something else might be going on. If you want to investigate further, you can capture the entire data stream using Wireshark. (Of course, that means capturing 22 MB/s of data.) Will that get me any new data/context I'm not already getting with the bus analyzer? I'm not against doing a Wireshark capture if it gets me access to some internal state of the USB stack that wouldn't be available to the Beagle, but at this point I've got scripts written to parse the Beagle data so it's not clear what the benefit would be. Here's an approach that might prove informative. Clear the transfer buffer to all zeros before resubmitting each URB (or store some other recognizable pattern in the buffer). When checking the completed URB, verify that each packet not only has a 0 status value but also has the right actual_length value and has new data in the transfer buffer. Finally, compare the data received in the packets surrounding a gap with the data you see from the Beagle-480. Look especially for signs indicating whether the gap represents a packet that was skipped over entirely, or whether the packet was delayed by one microframe and then all the following packets in the URB shifted back accordingly. To do this, you'll need the full data stream recorded by wireshark. 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: Video corruption varies by system load
On Wed, Jul 10, 2013 at 3:37 PM, Alan Stern st...@rowland.harvard.edu wrote: 20-30 times each second? Okay... I didn't realize the errors were that frequent. Yeah, now you see why I'm freaking out. If this were one corrupt line every 20 seconds, I wouldn't care less. But there are lines every few frames of video (and we're doing 30 frames/second). This isn't the sort of problem that is only apparent to a video engineer who knows what to look for - It's highly visible even to non-technical viewers. It's rather disturbing that they don't show up at all in the usbmon trace. This suggests two possibilities: The ehci-hcd driver is so messed up that not only did it fail to tell the hardware to send those missing packets; it also thought they had been sent and gotten replies! The controller hardware is so messed up that it failed to send the missing packets but then stored a status value indicating that they had been sent and replies had been received! Either of these are possibilities. Both possibilities are rather unsettling. Fortunately, they aren't exhaustive -- something else might be going on. If you want to investigate further, you can capture the entire data stream using Wireshark. (Of course, that means capturing 22 MB/s of data.) Will that get me any new data/context I'm not already getting with the bus analyzer? I'm not against doing a Wireshark capture if it gets me access to some internal state of the USB stack that wouldn't be available to the Beagle, but at this point I've got scripts written to parse the Beagle data so it's not clear what the benefit would be. Here's an approach that might prove informative. Clear the transfer buffer to all zeros before resubmitting each URB (or store some other recognizable pattern in the buffer). When checking the completed URB, verify that each packet not only has a 0 status value but also has the right actual_length value and has new data in the transfer buffer. Yeah, I tried that a few days ago. I did a memset() on the transfer_buffer prior to every resubmit, because at one point I thought perhaps I was getting back the buffer without it having been filled in. Finally, compare the data received in the packets surrounding a gap with the data you see from the Beagle-480. Look especially for signs indicating whether the gap represents a packet that was skipped over entirely, or whether the packet was delayed by one microframe and then all the following packets in the URB shifted back accordingly. To do this, you'll need the full data stream recorded by wireshark. From a data perspective, I don't think that the data is being skipped. If the URB were being lost entirely then the rest of the video frame would be skewed by the amount of the missing data. The raw video frame only tells me where the frame starts and ends, so if data is missing then the video data ends up being out of alignment from that point forward. Throw a URB on the floor and the entire rest of the frame will be shifted. I'm receiving the correct number of bytes, but the buffer contains the wrong data. It's probably also worth noting that I added some debug code to the URB completion handler to inject a couple of pixels to mark where the isoc packets start/end in the resulting video. That allowed me to see that the corrupt bytes always start on a packet boundary, and extend only partially into that packet. It's not like the entire packet is corrupt - only a portion of it. Once you get past the corrupt portion of the packet, the correct video picks up right where it left off. This all raises a good question - does the usbmon or wireshark trace show isoc data arriving every 125us? If it does, then the host controller is magically making up packets and announcing them to em28xx even though they were never actually sent across the wire. I'll have to take a closer look into that. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: USB host support should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma': drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single' drivers/built-in.o: In function `usb_hcd_unmap_urb_for_dma': drivers/usb/core/hcd.c:1393: undefined reference to `dma_unmap_sg' drivers/usb/core/hcd.c:1398: undefined reference to `dma_unmap_page' drivers/usb/core/hcd.c:1403: undefined reference to `dma_unmap_single' drivers/built-in.o: In function `usb_hcd_map_urb_for_dma': drivers/usb/core/hcd.c:1445: undefined reference to `dma_map_single' drivers/usb/core/hcd.c:1450: undefined reference to `dma_mapping_error' drivers/usb/core/hcd.c:1480: undefined reference to `dma_map_sg' drivers/usb/core/hcd.c:1495: undefined reference to `dma_map_page' drivers/usb/core/hcd.c:1501: undefined reference to `dma_mapping_error' drivers/usb/core/hcd.c:1507: undefined reference to `dma_map_single' drivers/usb/core/hcd.c:1512: undefined reference to `dma_mapping_error' drivers/built-in.o: In function `hcd_buffer_free': drivers/usb/core/buffer.c:146: undefined reference to `dma_pool_free' drivers/usb/core/buffer.c:150: undefined reference to `dma_free_coherent' drivers/built-in.o: In function `hcd_buffer_destroy': drivers/usb/core/buffer.c:90: undefined reference to `dma_pool_destroy' drivers/built-in.o: In function `hcd_buffer_create': drivers/usb/core/buffer.c:65: undefined reference to `dma_pool_create' drivers/built-in.o: In function `hcd_buffer_alloc': drivers/usb/core/buffer.c:120: undefined reference to `dma_pool_alloc' drivers/usb/core/buffer.c:122: undefined reference to `dma_alloc_coherent' ,,, Commit d9ea21a779278da06d0cbe989594bf542ed213d7 (usb: host: make USB_ARCH_HAS_?HCI obsolete) allowed to enable USB on platforms with NO_DMA=y, and exposed several input and media USB drivers that just select USB if USB_ARCH_HAS_HCD, without checking HAS_DMA. Fix the former by making USB depend on HAS_DMA. To fix the latter, instead of adding lots of depends on HAS_DMA, make those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and selecting USB. Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already handle this in a similar way. Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org --- drivers/input/joystick/Kconfig|3 +-- drivers/input/misc/Kconfig| 15 +-- drivers/input/mouse/Kconfig |9 +++-- drivers/input/tablet/Kconfig | 15 +-- drivers/input/touchscreen/Kconfig |3 +-- drivers/media/rc/Kconfig | 21 +++-- drivers/usb/Kconfig |2 +- 7 files changed, 23 insertions(+), 45 deletions(-) diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig index 56eb471..d7e36fb 100644 --- a/drivers/input/joystick/Kconfig +++ b/drivers/input/joystick/Kconfig @@ -278,8 +278,7 @@ config JOYSTICK_JOYDUMP config JOYSTICK_XPAD tristate X-Box gamepad support - depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you want to use the X-Box pad with your computer. Make sure to say Y to Joystick support (CONFIG_INPUT_JOYDEV) diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 0b541cd..00cdecb 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -286,8 +286,7 @@ config INPUT_ATLAS_BTNS config INPUT_ATI_REMOTE2 tristate ATI / Philips USB RF remote control - depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you want to use an ATI or Philips USB RF remote control. These are RF remotes with USB receivers. @@ -301,8 +300,7 @@ config INPUT_ATI_REMOTE2 config INPUT_KEYSPAN_REMOTE tristate Keyspan DMR USB remote control - depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you want to use a Keyspan DMR USB remote control. Currently only the UIA-11 type of receiver has been tested. The tag @@ -333,8 +331,7 @@ config INPUT_KXTJ9_POLLED_MODE config INPUT_POWERMATE tristate Griffin PowerMate and Contour Jog support - depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you want to use Griffin PowerMate or Contour Jog devices. These are aluminum dials which can measure clockwise and anticlockwise @@ -349,8 +346,7 @@ config INPUT_POWERMATE config INPUT_YEALINK tristate Yealink usb-p1k voip phone - depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you want to enable keyboard and LCD functions of the Yealink usb-p1k usb phones. The audio part is enabled by the generic @@ -364,8 +360,7 @@ config INPUT_YEALINK config INPUT_CM109 tristate C-Media CM109 USB I/O Controller - depends on USB_ARCH_HAS_HCD - select USB + depends on USB
Re: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: It's rather disturbing that they don't show up at all in the usbmon trace. This suggests two possibilities: The ehci-hcd driver is so messed up that not only did it fail to tell the hardware to send those missing packets; it also thought they had been sent and gotten replies! The controller hardware is so messed up that it failed to send the missing packets but then stored a status value indicating that they had been sent and replies had been received! Either of these are possibilities. I'd prefer to think of them as very unlikely possibilities. :-) Here's an approach that might prove informative. Clear the transfer buffer to all zeros before resubmitting each URB (or store some other recognizable pattern in the buffer). When checking the completed URB, verify that each packet not only has a 0 status value but also has the right actual_length value and has new data in the transfer buffer. Yeah, I tried that a few days ago. I did a memset() on the transfer_buffer prior to every resubmit, because at one point I thought perhaps I was getting back the buffer without it having been filled in. And you found that the buffer was getting filled in, although some of the data values were wrong? Finally, compare the data received in the packets surrounding a gap with the data you see from the Beagle-480. Look especially for signs indicating whether the gap represents a packet that was skipped over entirely, or whether the packet was delayed by one microframe and then all the following packets in the URB shifted back accordingly. To do this, you'll need the full data stream recorded by wireshark. From a data perspective, I don't think that the data is being skipped. If the URB were being lost entirely then the rest of the video frame would be skewed by the amount of the missing data. The raw video frame only tells me where the frame starts and ends, so if data is missing then the video data ends up being out of alignment from that point forward. Throw a URB on the floor and the entire rest of the frame will be shifted. I'm receiving the correct number of bytes, but the buffer contains the wrong data. It's probably also worth noting that I added some debug code to the URB completion handler to inject a couple of pixels to mark where the isoc packets start/end in the resulting video. That allowed me to see that the corrupt bytes always start on a packet boundary, and extend only partially into that packet. It's not like the entire packet is corrupt - only a portion of it. Once you get past the corrupt portion of the packet, the correct video picks up right where it left off. Could that happen if no packet was transmitted? The only way I can see would be if the buffer already contained valid-looking data and it didn't get overwritten, or if the buffer was overwritten with stale data left over from the preceding packet. Can you rule out both of these possibilities? So far, your analysis of the received data buffer seems to show that the computer does receive data from the device, but the Beagle-480 shows that no data was sent. This is inconsistent. This all raises a good question - does the usbmon or wireshark trace show isoc data arriving every 125us? If it does, then the host controller is magically making up packets and announcing them to em28xx even though they were never actually sent across the wire. I'll have to take a closer look into that. usbmon and wireshark show essentially the same things; the only difference being that usbmon abbreviates the data (on the theory that excessive detail is rarely useful). They both look only at URB submissions and completions; neither one can show what happened while an URB was in progress. So for example, your usbmon trace shows that the URBs did complete at intervals of 8000 us, although the timing wasn't entirely regular -- some completions were delayed by as much as 120 us or so relative to their expected time. The delays did not accumulate, however, and with a pipeline 40-ms long, they should not have mattered. The microframe numbers in the trace show that the URBs were scheduled at regular intervals of 64 microframes. Lack of errors indicates the controller hardware reported that it executed the transfers at the proper times. 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: USB host support should depend on HAS_DMA
On Wed, 10 Jul 2013, Geert Uytterhoeven wrote: If NO_DMA=y: drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma': drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single' ,,, Commit d9ea21a779278da06d0cbe989594bf542ed213d7 (usb: host: make USB_ARCH_HAS_?HCI obsolete) allowed to enable USB on platforms with NO_DMA=y, and exposed several input and media USB drivers that just select USB if USB_ARCH_HAS_HCD, without checking HAS_DMA. Fix the former by making USB depend on HAS_DMA. This isn't right. There are USB host controllers that use PIO, not DMA. The HAS_DMA dependency should go with the controller driver, not the USB core. On the other hand, the USB core does call various routines like dma_unmap_single. It ought to be possible to compile these calls even when DMA isn't enabled. That is, they should be defined as do-nothing stubs. To fix the latter, instead of adding lots of depends on HAS_DMA, make those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and selecting USB. Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already handle this in a similar way. That seems reasonable. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fotg210-udc: Remove bogus __init/__exit annotations
When builtin (CONFIG_USB_FOTG210_UDC=y):
LD drivers/usb/gadget/built-in.o
WARNING: drivers/usb/gadget/built-in.o(.data+0xbf8): Section mismatch in
reference from the variable fotg210_driver to the function
.init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
LD drivers/usb/built-in.o
WARNING: drivers/usb/built-in.o(.data+0x14684): Section mismatch in reference
from the variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
LD drivers/built-in.o
WARNING: drivers/built-in.o(.data+0x8b0c8): Section mismatch in reference from
the variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
CHK include/generated/uapi/linux/version.h
LINKvmlinux
LD vmlinux.o
MODPOST vmlinux.o
WARNING: vmlinux.o(.data+0xc6730): Section mismatch in reference from the
variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
GEN .version
CHK include/generated/compile.h
UPD include/generated/compile.h
CC init/version.o
LD init/built-in.o
`.exit.text' referenced in section `.data' of drivers/built-in.o: defined in
discarded section `.exit.text' of drivers/built-in.o
make[3]: *** [vmlinux] Error 1
Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org
---
drivers/usb/gadget/fotg210-udc.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/fotg210-udc.c b/drivers/usb/gadget/fotg210-udc.c
index cce5535..10cd18d 100644
--- a/drivers/usb/gadget/fotg210-udc.c
+++ b/drivers/usb/gadget/fotg210-udc.c
@@ -1074,7 +1074,7 @@ static struct usb_gadget_ops fotg210_gadget_ops = {
.udc_stop = fotg210_udc_stop,
};
-static int __exit fotg210_udc_remove(struct platform_device *pdev)
+static int fotg210_udc_remove(struct platform_device *pdev)
{
struct fotg210_udc *fotg210 = dev_get_drvdata(pdev-dev);
@@ -1088,7 +1088,7 @@ static int __exit fotg210_udc_remove(struct
platform_device *pdev)
return 0;
}
-static int __init fotg210_udc_probe(struct platform_device *pdev)
+static int fotg210_udc_probe(struct platform_device *pdev)
{
struct resource *res, *ires;
struct fotg210_udc *fotg210 = NULL;
--
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: [PATCH] usb: gadget: fotg210-udc: Remove bogus __init/__exit annotations
On 07/11/2013 01:45 AM, Geert Uytterhoeven wrote:
When builtin (CONFIG_USB_FOTG210_UDC=y):
LD drivers/usb/gadget/built-in.o
WARNING: drivers/usb/gadget/built-in.o(.data+0xbf8): Section mismatch in
reference from the variable fotg210_driver to the function
.init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
LD drivers/usb/built-in.o
WARNING: drivers/usb/built-in.o(.data+0x14684): Section mismatch in reference
from the variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
LD drivers/built-in.o
WARNING: drivers/built-in.o(.data+0x8b0c8): Section mismatch in reference from
the variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
CHK include/generated/uapi/linux/version.h
LINKvmlinux
LD vmlinux.o
MODPOST vmlinux.o
WARNING: vmlinux.o(.data+0xc6730): Section mismatch in reference from the
variable fotg210_driver to the function .init.text:fotg210_udc_probe()
The variable fotg210_driver references
the function __init fotg210_udc_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
GEN .version
CHK include/generated/compile.h
UPD include/generated/compile.h
CC init/version.o
LD init/built-in.o
`.exit.text' referenced in section `.data' of drivers/built-in.o: defined in
discarded section `.exit.text' of drivers/built-in.o
make[3]: *** [vmlinux] Error 1
Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org
---
drivers/usb/gadget/fotg210-udc.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/fotg210-udc.c b/drivers/usb/gadget/fotg210-udc.c
index cce5535..10cd18d 100644
--- a/drivers/usb/gadget/fotg210-udc.c
+++ b/drivers/usb/gadget/fotg210-udc.c
@@ -1074,7 +1074,7 @@ static struct usb_gadget_ops fotg210_gadget_ops = {
.udc_stop = fotg210_udc_stop,
};
-static int __exit fotg210_udc_remove(struct platform_device *pdev)
+static int fotg210_udc_remove(struct platform_device *pdev)
I think you can leave __exit annotation here, if you enclose the
reference in the driver structure in __exit_p()...
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: Video corruption varies by system load
On Wed, Jul 10, 2013 at 5:21 PM, Alan Stern st...@rowland.harvard.edu wrote: Yeah, I tried that a few days ago. I did a memset() on the transfer_buffer prior to every resubmit, because at one point I thought perhaps I was getting back the buffer without it having been filled in. And you found that the buffer was getting filled in, although some of the data values were wrong? Correct. Finally, compare the data received in the packets surrounding a gap with the data you see from the Beagle-480. Look especially for signs indicating whether the gap represents a packet that was skipped over entirely, or whether the packet was delayed by one microframe and then all the following packets in the URB shifted back accordingly. To do this, you'll need the full data stream recorded by wireshark. From a data perspective, I don't think that the data is being skipped. If the URB were being lost entirely then the rest of the video frame would be skewed by the amount of the missing data. The raw video frame only tells me where the frame starts and ends, so if data is missing then the video data ends up being out of alignment from that point forward. Throw a URB on the floor and the entire rest of the frame will be shifted. I'm receiving the correct number of bytes, but the buffer contains the wrong data. It's probably also worth noting that I added some debug code to the URB completion handler to inject a couple of pixels to mark where the isoc packets start/end in the resulting video. That allowed me to see that the corrupt bytes always start on a packet boundary, and extend only partially into that packet. It's not like the entire packet is corrupt - only a portion of it. Once you get past the corrupt portion of the packet, the correct video picks up right where it left off. Could that happen if no packet was transmitted? The only way I can see would be if the buffer already contained valid-looking data and it didn't get overwritten, or if the buffer was overwritten with stale data left over from the preceding packet. Can you rule out both of these possibilities? So far, your analysis of the received data buffer seems to show that the computer does receive data from the device, but the Beagle-480 shows that no data was sent. This is inconsistent. I think I did not explain this clearly. Let me try again: It would appear that the data received by the host and announced in the URB completion handler matches the data received by the Beagle. I see the incorrect bytes in the a microframe in both cases. In fact I've got a script that reconstructs the video frames based solely on the Beagle trace, and I see the same video corruption as I'm seeing on the host. Given that, it is my assertion that the problem has nothing to do with the way the HCD receives the buffers and passes them off to the completion handler. So one might ask: why is the em28xx device sending a microframe with corrupt bytes? One thing I've noticed is immediately prior to any microframe containing corruption, there was a missing microframe - the time between the microframe containing corruption and the previously received microframe was 250us, and not 125us as expected. My speculation is that the failure of the host controller to ask for that missing microframe causes some confusion in the em28xx, which results in it sending the wrong bytes in the next microframe. Alternatively the em28xx has some sort of circular buffer, and failing to ask for the microframe causes the circular buffer to wrap around (so the next microframe sent contains bad data). This all raises a good question - does the usbmon or wireshark trace show isoc data arriving every 125us? If it does, then the host controller is magically making up packets and announcing them to em28xx even though they were never actually sent across the wire. I'll have to take a closer look into that. usbmon and wireshark show essentially the same things; the only difference being that usbmon abbreviates the data (on the theory that excessive detail is rarely useful). They both look only at URB submissions and completions; neither one can show what happened while an URB was in progress. So for example, your usbmon trace shows that the URBs did complete at intervals of 8000 us, although the timing wasn't entirely regular -- some completions were delayed by as much as 120 us or so relative to their expected time. The delays did not accumulate, however, and with a pipeline 40-ms long, they should not have mattered. The microframe numbers in the trace show that the URBs were scheduled at regular intervals of 64 microframes. Lack of errors indicates the controller hardware reported that it executed the transfers at the proper times. Wireshark appears to be showing the URB timing, but really what we're interested in is the microframe timing. That said, it would be good to know what is in the microframe that is
Re: [PATCH] usb: USB host support should depend on HAS_DMA
On Wednesday 10 July 2013, Alan Stern wrote:
This isn't right. There are USB host controllers that use PIO, not
DMA. The HAS_DMA dependency should go with the controller driver, not
the USB core.
On the other hand, the USB core does call various routines like
dma_unmap_single. It ought to be possible to compile these calls even
when DMA isn't enabled. That is, they should be defined as do-nothing
stubs.
The asm-generic/dma-mapping-broken.h file intentionally causes link
errors, but that could be changed.
The better approach in my mind would be to replace code like
if (hcd-self.uses_dma)
with
if (IS_ENABLED(CONFIG_HAS_DMA) hcd-self.uses_dma) {
which will reliably cause that reference to be omitted from object code,
but not stop giving link errors for drivers that actually require
DMA.
Arnd
--
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: Audio I/O parameters
Hi Clemens, On Mon, Jul 8, 2013 at 2:12 PM, James Stone jamesmst...@gmail.com wrote: Snip! Acquire audio card Audio0 creating alsa driver ... hw:USB,0|-|64|2|44100|0|0|nomon|swmeter|-|16bit Using ALSA driver USB-Audio running on card 0 - Focusrite Scarlett 2i4 USB at usb-:00:12.2-3, high speed configuring for 44100Hz, period = 64 frames (1.5 ms), buffer = 2 periods ALSA: final selected sample format for playback: 32bit integer little-endian ALSA: use 2 periods for playback ALSA: poll time out, polled for 2176094 usecs JackAudioDriver::ProcessAsync: read error, stopping... This is a definite reduction in performance compared to earlier kernels. Some further info - on 3.5.0-28, I can start jackd in playback only with 8 frames/period, and capture only at 16 frames/period. Any thoughts on further investigating this bug with the 3.8.0 kernel with the Focusrite Scarlett 2i4? I'm happy to continue with any further testing if it would be helpful.. One thing is that another person affected by the same bug reports that it may be hardware-specific: See: https://bugs.launchpad.net/ubuntu/+source/linux-lowlatency/+bug/1185563 Jori Neimi reports: My laptop can handle jackd with a latency of 32 samples on my Focusrite Scarlett 2i2 and 3.8.0-25 lowlatency kernel. On my desktop jackd won't even start with a latency of less than 512 samples using the same kernel and same USB audio device. No help from the proposed 3.8.0-26, so I'll continue using 3.5.x kernels on my desktop. Could this mean it is specific to some type of USB hardware on the motherboard?? James -- 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: phy: samsung-usb2: Toggle HSIC GPIO from device tree
On Wednesday, July 10, 2013 9:34 AM, Julius Werner wrote:
This patch adds support for a new 'samsung,hsic-reset-gpio' in the
device tree, which will be interpreted as an active-low reset pin during
PHY initialization when it exists. Useful for intergrated HSIC devices
like an SMSC 3503 hub. It is necessary to add this directly to the PHY
initialization to get the timing right, since resetting a HSIC device
after it has already been enumerated can confuse the USB stack.
Also fixes PHY semaphore code to make sure we always go through the
setup at least once, even if it was already turned on (e.g. by
firmware), and changes a spinlock to a mutex to allow sleeping in the
critical section.
CC'ed Thomas Abraham,
This reset signal pin seems 'HUB_RESET' pin to reset SMSC 3503 hub
on Arndale board. This reset pin is described that 'This active low signal
is used by the system to reset the chip' in SMSC 3503 hub datasheet.
One more thing,
'phy-samsung-usb*.c' files are used and designed to control USB PHY controller
in Exynos SoCs; thus, these files should control only internal USB PHY
controller
in Exynos SoCs.
However, the reset pin is used for reset external SMSC 3503 hub chip
that is not placed in Exynos SoC.
Thus, there should not be HUB reset code
in ./drivers/usb/phy/phy-samsung-usb*.c
This topic was already discussed one month ago.
As Olof Johansson mentioned, 'drivers/platform/arm/' would be
a good alternative; thus, USB hub reset code should be moved
to 'drivers/platform/arm/'. Please refer to the discussion.
(http://patches.linaro.org/16856/)
Best regards,
Jingoo Han
Change-Id: Ieecac52c27daa7a17a7ed3b2863ddba3aeb8d16f
Signed-off-by: Julius Werner jwer...@chromium.org
---
.../devicetree/bindings/usb/samsung-usbphy.txt | 10 ++
drivers/usb/phy/phy-samsung-usb.c | 17 ++
drivers/usb/phy/phy-samsung-usb.h | 7 ++--
drivers/usb/phy/phy-samsung-usb2.c | 38
++
drivers/usb/phy/phy-samsung-usb3.c | 12 +++
5 files changed, 55 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 33fd354..82e2e16 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -31,6 +31,12 @@ Optional properties:
- ranges: allows valid translation between child's address space and parent's
address space.
+- samsung,hsic-reset-gpio: an active low GPIO pin that resets a device
+ connected to the HSIC port. Useful for things like
+ an on-board SMSC3503 hub.
+- pinctrl-0: Pin control group containing the HSIC reset GPIO pin.
+- pinctrl-names: Should contain only one value - default.
+
- The child node 'usbphy-sys' to the node 'usbphy' is for the system
controller
interface for usb-phy. It should provide the following information
required by
usb-phy controller to control phy.
@@ -56,6 +62,10 @@ Example:
clocks = clock 2, clock 305;
clock-names = xusbxti, otg;
+ samsung,hsic-reset-gpio = gpx2 4 1;
+ pinctrl-names = default;
+ pinctrl-0 = hsic_reset;
+
usbphy-sys {
/* USB device and host PHY_CONTROL registers */
reg = 0x10020704 0x8;
diff --git a/drivers/usb/phy/phy-samsung-usb.c
b/drivers/usb/phy/phy-samsung-usb.c
index ac025ca..23f1d70 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -27,6 +27,7 @@
#include linux/io.h
#include linux/of.h
#include linux/of_address.h
+#include linux/of_gpio.h
#include linux/usb/samsung_usb_phy.h
#include phy-samsung-usb.h
@@ -58,6 +59,22 @@ int samsung_usbphy_parse_dt(struct samsung_usbphy *sphy)
if (sphy-sysreg == NULL)
dev_warn(sphy-dev, Can't get usb-phy sysreg cfg register\n);
+ /*
+ * Some boards have a separate active-low reset GPIO for their HSIC USB
+ * devices. If they don't, this will just stay at an invalid value and
+ * the init code will ignore it.
+ */
+ sphy-hsic_reset_gpio = of_get_named_gpio(sphy-dev-of_node,
+ samsung,hsic-reset-gpio, 0);
+ if (gpio_is_valid(sphy-hsic_reset_gpio)) {
+ if (devm_gpio_request_one(sphy-dev, sphy-hsic_reset_gpio,
+ GPIOF_OUT_INIT_LOW, samsung_hsic_reset)) {
+ dev_err(sphy-dev, can't request hsic reset gpio %d\n,
+ sphy-hsic_reset_gpio);
+ sphy-hsic_reset_gpio = -EINVAL;
+ }
+ }
+
of_node_put(usbphy_sys);
return 0;
diff --git a/drivers/usb/phy/phy-samsung-usb.h
b/drivers/usb/phy/phy-samsung-usb.h
index
Re: [PATCH] usb: USB host support should depend on HAS_DMA
On Thu, 11 Jul 2013, Arnd Bergmann wrote:
On Wednesday 10 July 2013, Alan Stern wrote:
This isn't right. There are USB host controllers that use PIO, not
DMA. The HAS_DMA dependency should go with the controller driver, not
the USB core.
On the other hand, the USB core does call various routines like
dma_unmap_single. It ought to be possible to compile these calls even
when DMA isn't enabled. That is, they should be defined as do-nothing
stubs.
The asm-generic/dma-mapping-broken.h file intentionally causes link
errors, but that could be changed.
The better approach in my mind would be to replace code like
if (hcd-self.uses_dma)
with
if (IS_ENABLED(CONFIG_HAS_DMA) hcd-self.uses_dma) {
which will reliably cause that reference to be omitted from object code,
but not stop giving link errors for drivers that actually require
DMA.
How will it give link errors for drivers that require DMA?
Besides, wouldn't it be better to get an error at config time rather
than at link time? Or even better still, not to be allowed to
configure drivers that depend on DMA if DMA isn't available?
If we add an explicit dependency for HAS_DMA to the Kconfig entries for
these drivers, then your suggestion would be a good way to allow
usbcore to be built independently of DMA support.
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 1/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
Hello Maintainers:
Please help check this patch when you have time, thanks.
BTW: this uninitialized variable warning may not be found by gcc
compiler (which a gcc bug exists almost 10 years).
Thanks.
On 07/02/2013 12:06 PM, Chen Gang wrote:
The variable 'actual' is only used in checking 'buffer[1]' code
block, so need move it into, or it may not be initialized.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 609dbc2..42d62c9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -786,7 +786,7 @@ usbtmc_clear_check_status:
goto exit;
}
- if (buffer[1] == 1)
+ if (buffer[1] == 1) {
do {
dev_dbg(dev, Reading from bulk in EP\n);
@@ -805,11 +805,13 @@ usbtmc_clear_check_status:
} while ((actual == max_size)
(n USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
- if (actual == max_size) {
- dev_err(dev, Couldn't clear device buffer within %d cycles\n,
- USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
- rv = -EPERM;
- goto exit;
+ if (actual == max_size) {
+ dev_err(dev,
+ Couldn't clear device buffer within %d
cycles\n,
+ USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+ rv = -EPERM;
+ goto exit;
+ }
}
goto usbtmc_clear_check_status;
--
Chen Gang
--
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: Video corruption varies by system load
On Wed, 10 Jul 2013, Devin Heitmueller wrote: So one might ask: why is the em28xx device sending a microframe with corrupt bytes? One thing I've noticed is immediately prior to any microframe containing corruption, there was a missing microframe - the time between the microframe containing corruption and the previously received microframe was 250us, and not 125us as expected. My speculation is that the failure of the host controller to ask for that missing microframe causes some confusion in the em28xx, which results in it sending the wrong bytes in the next microframe. Alternatively the em28xx has some sort of circular buffer, and failing to ask for the microframe causes the circular buffer to wrap around (so the next microframe sent contains bad data). This is what I've been trying to get at. The usbmon timing data shows that the host controller thinks a packet was sent and received during the missing microframe. The data you saw in the buffer proves that the controller did write _something_ to memory during the missing microframe -- where did that something come from? Wireshark appears to be showing the URB timing, but really what we're interested in is the microframe timing. That said, it would be good to know what is in the microframe that is immediately prior to the microframe containing corruption. Is it a zero length microframe? Is That's why I suggested you zero the buffer before resubmission and have the driver check the packet descriptor's actual_length value. it the frame that was received 250us earlier? That's why I suggested you examine the wireshark data in detail. That might help understand whether the host controller believes that the IN transaction occurred. The host controller undoubtedly _does_ believe this. Or at least, that's what it indicates to the CPU, by setting the iTD's transaction status to 0. Unless for reason the iTD data structure was set up wrongly in the first place... I'll work on seeing if I can correlate the data between the Beagle trace and the usbmon trace, so I can see what the host *thinks* happened immediately prior to the corrupt microframe. Good. 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
