Re: [PATCH 1/1] usb: chipidea: fix the build error with randconfig
Peter Chen 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 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 > Signed-off-by: Fabio Estevam 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 >> --- >> 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
---
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
> ---
> 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 > > --- > > 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
> ---
> 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=testing&id=d7e39d414310e098540605be2051d5187797be34
commit d7e39d414310e098540605be2051d5187797be34
Author: Felipe Balbi
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
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 r
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 > Signed-off-by: Andrzej Pietrasiewicz > Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz > --- > 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 +--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 11:01:1
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 >>> --- >>> 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 > >>> --- > >>> 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
>> ---
>> 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 > --- > 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
---
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=testing&id=d7e39d414310e098540605be2051d5187797be34
commit d7e39d414310e098540605be2051d5187797be34
Author: Felipe Balbi
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
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
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 > >> --- > >> 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 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 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 > 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 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
---
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
---
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
---
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
Signed-off-by: Roger Quadros
---
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 ");
MODULE_AUTHOR("Roger Quadros ");
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 ");
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
---
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
Signed-off-by: Roger Quadros
---
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
#include
#include
+#include
#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 > 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 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
---
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 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 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 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
---
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
---
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
---
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 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 sai
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 wrote: >> 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
> ---
> .../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
> #include
> #include
> +#include
> #include
>
> #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
Re: [PATCH] usb: phy: samsung-usb2: Toggle HSIC GPIO from device tree
Hi Julius, On Wed, Jul 10, 2013 at 2:42 PM, Julius Werner wrote: > 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. It seems that the reset gpio driver from Phillip Zabel would help in this case: http://permalink.gmane.org/gmane.linux.drivers.devicetree/36830 -- 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 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
> ---
> 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
[PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts
When the gadget role starts, we need to make sure the vbus is lower
than OTGSC_BSV, or there will be an vbus interrupt since we use
B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
When the host role starts, it may not be useful to wait vbus to lower
than OTGSC_BSV, but it can indicate some hardware problems like the
vbus is still higher than OTGSC_BSV after we disconnect to host some
time later (500 jiffies currently), which is obvious not correct.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/ci.h |3 +++
drivers/usb/chipidea/core.c | 32
drivers/usb/chipidea/otg.c |4
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 3160363..df27696 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -308,4 +308,7 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
u8 hw_port_test_get(struct ci_hdrc *ci);
+int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
+ u32 value, unsigned long timeout);
+
#endif /* __DRIVERS_USB_CHIPIDEA_CI_H */
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7a07300..1c39541 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -295,6 +295,38 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
return 0;
}
+/**
+ * hw_wait_reg: wait the register value
+ *
+ * Sometimes, it needs to wait register value before going on.
+ * Eg, when switch to device mode, the vbus value should be lower
+ * than OTGSC_BSV before connects to host.
+ *
+ * @ci: the controller
+ * @reg: register index
+ * @mask: mast bit
+ * @value: the bit value to wait
+ * @timeout: timeout to indicate an error
+ *
+ * This function returns an error code if timeout
+ */
+int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
+ u32 value, unsigned long timeout)
+{
+ unsigned long elapse = jiffies + timeout;
+
+ while (hw_read(ci, reg, mask) != value) {
+ if (time_after(jiffies, elapse)) {
+ dev_err(ci->dev, "timeout waiting for %08x in %d\n",
+ mask, reg);
+ return -ETIMEDOUT;
+ }
+ msleep(20);
+ }
+
+ return 0;
+}
+
static irqreturn_t ci_irq(int irq, void *data)
{
struct ci_hdrc *ci = data;
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index c74194d..8aa0241 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -67,6 +67,7 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
usb_gadget_vbus_disconnect(&ci->gadget);
}
+#define CI_VBUS_STABLE_TIMEOUT 500
static void ci_handle_id_switch(struct ci_hdrc *ci)
{
enum ci_role role = ci_otg_role(ci);
@@ -76,6 +77,9 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
ci_role(ci)->name, ci->roles[role]->name);
ci_role_stop(ci);
+ /* wait vbus lower than OTGSC_BSV */
+ hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
+ CI_VBUS_STABLE_TIMEOUT);
ci_role_start(ci, role);
}
}
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea
This patchset adds tested otg id switch function and vbus connect and disconnect detection for chipidea driver. And fix kinds of bugs found at chipidea drivers after enabling id and vbus detection. This patch is fully tested at imx6 sabresd platform. My chipidea repo: https://github.com/hzpeterchen/linux-usb.git Changes for v12: - Rebased greg's usb-next tree (3.10.0-rc7+) - Split more small patches for single function and fix. Changes for v11: - mark ci_handle_vbus_change as static as it is only used at core.c [3/9] - Move the vbus operation for platform code to host code, as vbus operation is common operation, and host is the only user for vbus. When it is host mode, we need to open vbus, when it is out of host mode, we need to close vbus. [6/9] [8/9] - Delete the delayed work at core.c as it is not needed. [7/9] Changes for v10: - Delete [8/9] at v9, ci core's drvdata must be set for further operation. [8/8] Changes for v9: - Some small comments from Alex like: variable comment for otg event additional newline. [3/9] - Import function tell show if the controller has otg capable, if the controller supports both host and device, we think it is otg capable, and can read OTGSC. [3/9] - Merge two otg patches [v8 3/8] and [v8 4/8] to one [v9 3/9]. [3/9] - Add inline to ci_hdrc_gadget_destroy if CONFIG_USB_CHIPIDEA_UDC is not defined, it can fix one build warning "defined but not used" [3/9] - One comment from Felipe about changing calling gadget disconnect API at chipidea's udc driver. I move calling ci->driver->disconnect from _gadget_stop_activity to which calls _gadget_stop_activity except ci13xxx_stop, as udc core will call disconnect when do rmmod gadget. [7/9] - Add ci core probe's return value to ci's platform_data, we do this for getting core's probe's result at platform layer, and quit it if the core's probe fails. [8/9] [9/9] Changes for v8: - Add ci_supports_gadget helper to know if the controller supports gadget, if the controller supports gadget, it needs to read otgsc to know the vbus value, basically, if the controller supports gadget, it will support host as well [3/8] - At ci_hdrc_probe, it needs to add free memory at error path [3/8] - Cosolidate ci->driver = NULL at ci13xxx_stop [8/8] Changes for v7: For Patch 8/8, we only need to set ci->driver to NULL when usb cable is not connected, for other changes, it will case some runtime pm unmatch and un-align with udc-core & composite driver problems. Changes for v6: - Add Alex comments for init/destroy function [3/8] [4/8] - Remove memset(&ci->gadget, 0, sizeof(ci->gadget)) at destory function [4/8] - Add Kishon's comment: Change the format of struct usb_otg otg at drivers/usb/chipidea/ci.h [1/8] - Add comments for CI_VBUS_STABLE_TIMEOUT [3/8] - Change the otg_set_peripheral return value check as the fully chipidea driver users don't need it. [4/8] - Fix one bug that the oops when re-plug in usb cable after rmmod gadget [8/8] Peter Chen (13): usb: chipidea: add vbus regulator control usb: chipidea: imx: remove vbus regulator operation usb: chipidea: udc: otg_set_peripheral is useless for some chipidea users usb: chipidea: otg: Add otg file used to access otgsc usb: chipidea: Add role init and destory APIs usb: chipidea: add otg_cap attribute for otg capable usb: chipidea: disable all interrupts and clear all interrupts status usb: chipidea: move otg relate things to otg file usb: chipidea: add vbus interrupt handler usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS usb: chipidea: udc: .pullup is valid when vbus is on at CI_HDRC_PULLUP_ON_VBUS usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget drivers/usb/chipidea/Makefile |2 +- drivers/usb/chipidea/bits.h| 10 +++ drivers/usb/chipidea/ci.h |8 ++ drivers/usb/chipidea/ci_hdrc_imx.c | 30 ++- drivers/usb/chipidea/core.c| 158 +++ drivers/usb/chipidea/host.c| 30 +++- drivers/usb/chipidea/host.h|6 ++ drivers/usb/chipidea/otg.c | 135 ++ drivers/usb/chipidea/otg.h | 22 + drivers/usb/chipidea/udc.c | 72 + drivers/usb/chipidea/udc.h |6 ++ include/linux/usb/chipidea.h | 14 +++ 12 files changed, 397 insertions(+), 96 deletions(-) create mode 100644 drivers/usb/chipidea/otg.c create mode 100644 drivers/usb/chipidea/otg.h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable
Since we need otgsc to know vbus's status at some chipidea
controllers even it is peripheral-only mode. Besides, some
SoCs (eg, AR9331 SoC) don't have otgsc register even
the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
We inroduce otg_cap attribute to indicate if the controller
is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
is exception, the platform can override it by device tree or platform data.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c | 35 ---
include/linux/usb/chipidea.h | 13 +
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 93961ff..e8ceb04 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
ci_hdrc_host_destroy(ci);
}
+static void ci_get_otg_capable(struct ci_hdrc *ci)
+{
+ if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)
+ ci->is_otg = (ci->platdata->otg_cap == OTG_CAP_ATTR_IS_TRUE);
+ else
+ ci->is_otg = (hw_read(ci, CAP_DCCPARAMS,
+ DCCPARAMS_DC | DCCPARAMS_HC)
+ == (DCCPARAMS_DC | DCCPARAMS_HC));
+ if (ci->is_otg)
+ dev_dbg(ci->dev, "It is OTG capable controller\n");
+}
+
static int ci_hdrc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -461,6 +473,9 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* To know if controller is OTG capable or not */
+ ci_get_otg_capable(ci);
+
if (!ci->platdata->phy_mode)
ci->platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
@@ -491,10 +506,19 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
- ci->is_otg = true;
- /* ID pin needs 1ms debouce time, we delay 2ms for safe */
- mdelay(2);
- ci->role = ci_otg_role(ci);
+ if (ci->is_otg) {
+ /* ID pin needs 1ms debouce time, we delay 2ms for safe
*/
+ mdelay(2);
+ ci->role = ci_otg_role(ci);
+ ci_hdrc_otg_init(ci);
+ } else {
+ /*
+* If the controller is not OTG capable, but support
+* role switch, the defalt role is gadget, and the
+* user can switch it through debugfs (proc in future?)
+*/
+ ci->role = CI_ROLE_GADGET;
+ }
} else {
ci->role = ci->roles[CI_ROLE_HOST]
? CI_ROLE_HOST
@@ -514,9 +538,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret)
goto stop;
- if (ci->is_otg)
- ci_hdrc_otg_init(ci);
-
ret = dbg_create_files(ci);
if (!ret)
return 0;
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 118bf66..0a906b4 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -7,6 +7,12 @@
#include
+enum usb_otg_cap {
+ OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
+ OTG_CAP_ATTR_IS_TRUE,
+ OTG_CAP_ATTR_IS_FALSE,
+};
+
struct ci_hdrc;
struct ci_hdrc_platform_data {
const char *name;
@@ -25,6 +31,13 @@ struct ci_hdrc_platform_data {
#define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci_hdrc *ci, unsigned event);
struct regulator *reg_vbus;
+ /*
+* Please only set otg_cap when the otg capability can't be
+* judged by CAP_DCCPARAMS, eg, the DCCPARAMS_DC and DCCPARAMS_HC
+* are both 1 at CAP_DCCPARAMS, but the controller doesn't have
+* OTGSC register (eg, AR9331 SoC).
+*/
+ enum usb_otg_cap otg_cap;
};
/* Default offset of capability registers */
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 01/13] usb: chipidea: add vbus regulator control
For boards which have board level vbus control (eg, through gpio), we
need to vbus operation according to below rules:
- For host, we need open vbus before start hcd, and close it
after remove hcd.
- For otg, the vbus needs to be on/off when usb role switches.
When the host roles begins, it opens vbus; when the host role
finishes, it closes vbus.
We put vbus operation to host as host is the only vbus user,
When we are at host mode, the vbus is on, when we are not at
host mode, vbus should be off.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/host.c | 23 ++-
include/linux/usb/chipidea.h |1 +
2 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 40d0fda..e94e52b 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include "../host/ehci.h"
@@ -64,9 +65,19 @@ static int host_start(struct ci_hdrc *ci)
ehci->caps = ci->hw_bank.cap;
ehci->has_hostpc = ci->hw_bank.lpm;
+ if (ci->platdata->reg_vbus) {
+ ret = regulator_enable(ci->platdata->reg_vbus);
+ if (ret) {
+ dev_err(ci->dev,
+ "Failed to enable vbus regulator, ret=%d\n",
+ ret);
+ goto put_hcd;
+ }
+ }
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret)
- usb_put_hcd(hcd);
+ goto disable_reg;
else
ci->hcd = hcd;
@@ -74,6 +85,14 @@ static int host_start(struct ci_hdrc *ci)
hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
return ret;
+
+disable_reg:
+ regulator_disable(ci->platdata->reg_vbus);
+
+put_hcd:
+ usb_put_hcd(hcd);
+
+ return ret;
}
static void host_stop(struct ci_hdrc *ci)
@@ -82,6 +101,8 @@ static void host_stop(struct ci_hdrc *ci)
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
+ if (ci->platdata->reg_vbus)
+ regulator_disable(ci->platdata->reg_vbus);
}
int ci_hdrc_host_init(struct ci_hdrc *ci)
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 2562994..118bf66 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -24,6 +24,7 @@ struct ci_hdrc_platform_data {
#define CI_HDRC_CONTROLLER_RESET_EVENT 0
#define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci_hdrc *ci, unsigned event);
+ struct regulator *reg_vbus;
};
/* Default offset of capability registers */
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 03/13] usb: chipidea: udc: otg_set_peripheral is useless for some chipidea users
It is useless at below cases:
- If we implement both usb host and device at chipidea driver.
- If we don't need phy->otg.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/udc.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index e475fcd..116c762 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1805,7 +1805,12 @@ static int udc_start(struct ci_hdrc *ci)
if (ci->transceiver) {
retval = otg_set_peripheral(ci->transceiver->otg,
&ci->gadget);
- if (retval)
+ /*
+* If we implement all USB functions using chipidea drivers,
+* it doesn't need to call above API, meanwhile, if we only
+* use gadget function, calling above API is useless.
+*/
+ if (retval && retval != -ENOTSUPP)
goto put_transceiver;
}
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation
Since we have added vbus reguatlor operation at common
host file (chipidea/host.c), the glue layer vbus operation
isn't needed any more.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/ci_hdrc_imx.c | 30 +++---
1 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 14362c0..d06355e 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
- struct regulator *reg_vbus;
};
static const struct usbmisc_ops *usbmisc_ops;
@@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
goto err_clk;
}
- /* we only support host now, so enable vbus here */
- data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
- if (!IS_ERR(data->reg_vbus)) {
- ret = regulator_enable(data->reg_vbus);
- if (ret) {
- dev_err(&pdev->dev,
- "Failed to enable vbus regulator, err=%d\n",
- ret);
- goto err_clk;
- }
- } else {
- data->reg_vbus = NULL;
- }
-
pdata.phy = data->phy;
+ /* Get the vbus regulator */
+ pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
+ if (IS_ERR(pdata.reg_vbus))
+ pdata.reg_vbus = NULL;
+
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
if (!pdev->dev.coherent_dma_mask)
@@ -167,7 +157,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev,
"usbmisc init failed, ret=%d\n", ret);
- goto err;
+ goto err_clk;
}
}
@@ -179,7 +169,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Can't register ci_hdrc platform device, err=%d\n",
ret);
- goto err;
+ goto err_clk;
}
if (usbmisc_ops && usbmisc_ops->post) {
@@ -200,9 +190,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
disable_device:
ci_hdrc_remove_device(data->ci_pdev);
-err:
- if (data->reg_vbus)
- regulator_disable(data->reg_vbus);
err_clk:
clk_disable_unprepare(data->clk);
return ret;
@@ -215,9 +202,6 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
ci_hdrc_remove_device(data->ci_pdev);
- if (data->reg_vbus)
- regulator_disable(data->reg_vbus);
-
if (data->phy) {
usb_phy_shutdown(data->phy);
module_put(data->phy->dev->driver->owner);
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 07/13] usb: chipidea: disable all interrupts and clear all interrupts status
During the initialization, it needs to disable all interrupts
enable bit as well as clear all interrupts status bits to avoid
exceptional interrupt.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c | 11 ++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index e8ceb04..f701e3b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -198,6 +198,12 @@ static int hw_device_init(struct ci_hdrc *ci, void __iomem
*base)
if (ci->hw_ep_max > ENDPT_MAX)
return -ENODEV;
+ /* Disable all interrupts bits */
+ hw_write(ci, OP_USBINTR, 0x, 0);
+
+ /* Clear all interrupts status bits*/
+ hw_write(ci, OP_USBSTS, 0x, 0x);
+
dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op);
@@ -413,8 +419,11 @@ static void ci_get_otg_capable(struct ci_hdrc *ci)
ci->is_otg = (hw_read(ci, CAP_DCCPARAMS,
DCCPARAMS_DC | DCCPARAMS_HC)
== (DCCPARAMS_DC | DCCPARAMS_HC));
- if (ci->is_otg)
+ if (ci->is_otg) {
dev_dbg(ci->dev, "It is OTG capable controller\n");
+ ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+ ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+ }
}
static int ci_hdrc_probe(struct platform_device *pdev)
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 04/13] usb: chipidea: otg: Add otg file used to access otgsc
This file is mainly used to access otgsc currently, it may
add otg related things in the future.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/Makefile |2 +-
drivers/usb/chipidea/bits.h | 10
drivers/usb/chipidea/core.c |3 +-
drivers/usb/chipidea/otg.c| 50 +
drivers/usb/chipidea/otg.h| 19 +++
5 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 3bbbcba..9d0e288 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -2,7 +2,7 @@ ccflags-$(CONFIG_USB_CHIPIDEA_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc.o
-ci_hdrc-y := core.o
+ci_hdrc-y := core.o otg.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)+= host.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG) += debug.o
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index aefa026..dd0cf9e 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -79,11 +79,21 @@
#define OTGSC_ASVIS BIT(18)
#define OTGSC_BSVIS BIT(19)
#define OTGSC_BSEIS BIT(20)
+#define OTGSC_1MSIS BIT(21)
+#define OTGSC_DPIS BIT(22)
#define OTGSC_IDIE BIT(24)
#define OTGSC_AVVIE BIT(25)
#define OTGSC_ASVIE BIT(26)
#define OTGSC_BSVIE BIT(27)
#define OTGSC_BSEIE BIT(28)
+#define OTGSC_1MSIE BIT(29)
+#define OTGSC_DPIE BIT(30)
+#define OTGSC_INT_EN_BITS (OTGSC_IDIE | OTGSC_AVVIE | OTGSC_ASVIE \
+ | OTGSC_BSVIE | OTGSC_BSEIE | OTGSC_1MSIE \
+ | OTGSC_DPIE)
+#define OTGSC_INT_STATUS_BITS (OTGSC_IDIS | OTGSC_AVVIS | OTGSC_ASVIS \
+ | OTGSC_BSVIS | OTGSC_BSEIS | OTGSC_1MSIS \
+ | OTGSC_DPIS)
/* USBMODE */
#define USBMODE_CM(0x03UL << 0)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index a5df24c..761f7e8 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -71,6 +71,7 @@
#include "bits.h"
#include "host.h"
#include "debug.h"
+#include "otg.h"
/* Controller register map */
static uintptr_t ci_regs_nolpm[] = {
@@ -508,7 +509,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
goto stop;
if (ci->is_otg)
- hw_write(ci, OP_OTGSC, OTGSC_IDIE, OTGSC_IDIE);
+ ci_hdrc_otg_init(ci);
ret = dbg_create_files(ci);
if (!ret)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
new file mode 100644
index 000..abefb4d
--- /dev/null
+++ b/drivers/usb/chipidea/otg.c
@@ -0,0 +1,50 @@
+/*
+ * otg.c - ChipIdea USB IP core OTG driver
+ *
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Peter Chen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * This file mainly handles otgsc register, it may include OTG operation
+ * in the future.
+ */
+
+#include
+#include
+#include
+
+#include "ci.h"
+#include "bits.h"
+
+void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+ /* Only clear request bits */
+ hw_write(ci, OP_OTGSC, OTGSC_INT_STATUS_BITS, bits);
+}
+
+void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+ hw_write(ci, OP_OTGSC, bits, bits);
+}
+
+void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+ hw_write(ci, OP_OTGSC, bits, 0);
+}
+
+/**
+ * ci_hdrc_otg_init - initialize otgsc bits
+ * ci: the controller
+ */
+int ci_hdrc_otg_init(struct ci_hdrc *ci)
+{
+ ci_enable_otg_interrupt(ci, OTGSC_IDIE);
+
+ return 0;
+}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
new file mode 100644
index 000..f24ec37
--- /dev/null
+++ b/drivers/usb/chipidea/otg.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Peter Chen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __DRIVERS_USB_CHIPIDEA_OTG_H
+#define __DRIVERS_USB_CHIPIDEA_OTG_H
+
+int ci_hdrc_otg_init(struct ci_hdrc *ci);
+void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+
+#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-inf
[PATCH v12 12/13] usb: chipidea: udc: .pullup is valid when vbus is on at CI_HDRC_PULLUP_ON_VBUS
When the flag CI_HDRC_PULLUP_ON_VBUS is set, .pullup should only be
called when the vbus is active. When the CI_HDRC_PULLUP_ON_VBUS
is set, the controller only begins to run when the vbus is on,
So, it is only meaningful software set pullup/pulldown after
the controller begins to run.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/udc.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 45abf4d..b7ead5f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1514,6 +1514,10 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int
is_on)
{
struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
+ if ((ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS)
+ && !ci->vbus_active)
+ return -EOPNOTSUPP;
+
if (is_on)
hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
else
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 05/13] usb: chipidea: Add role init and destory APIs
- The role's init will be called at probe procedure.
- The role's destory will be called at fail patch
at probe and driver's removal.
- The role's start/stop will be called when specific
role has started.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c | 10 --
drivers/usb/chipidea/host.c |7 +++
drivers/usb/chipidea/host.h |6 ++
drivers/usb/chipidea/udc.c | 36 +++-
drivers/usb/chipidea/udc.h |6 ++
5 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 761f7e8..93961ff 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -399,6 +399,12 @@ void ci_hdrc_remove_device(struct platform_device *pdev)
}
EXPORT_SYMBOL_GPL(ci_hdrc_remove_device);
+static inline void ci_role_destroy(struct ci_hdrc *ci)
+{
+ ci_hdrc_gadget_destroy(ci);
+ ci_hdrc_host_destroy(ci);
+}
+
static int ci_hdrc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -517,7 +523,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
free_irq(ci->irq, ci);
stop:
- ci_role_stop(ci);
+ ci_role_destroy(ci);
rm_wq:
flush_workqueue(ci->wq);
destroy_workqueue(ci->wq);
@@ -533,7 +539,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
flush_workqueue(ci->wq);
destroy_workqueue(ci->wq);
free_irq(ci->irq, ci);
- ci_role_stop(ci);
+ ci_role_destroy(ci);
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index e94e52b..382be5b 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -105,6 +105,13 @@ static void host_stop(struct ci_hdrc *ci)
regulator_disable(ci->platdata->reg_vbus);
}
+
+void ci_hdrc_host_destroy(struct ci_hdrc *ci)
+{
+ if (ci->role == CI_ROLE_HOST)
+ host_stop(ci);
+}
+
int ci_hdrc_host_init(struct ci_hdrc *ci)
{
struct ci_role_driver *rdrv;
diff --git a/drivers/usb/chipidea/host.h b/drivers/usb/chipidea/host.h
index 058875c..5707bf3 100644
--- a/drivers/usb/chipidea/host.h
+++ b/drivers/usb/chipidea/host.h
@@ -4,6 +4,7 @@
#ifdef CONFIG_USB_CHIPIDEA_HOST
int ci_hdrc_host_init(struct ci_hdrc *ci);
+void ci_hdrc_host_destroy(struct ci_hdrc *ci);
#else
@@ -12,6 +13,11 @@ static inline int ci_hdrc_host_init(struct ci_hdrc *ci)
return -ENXIO;
}
+static inline void ci_hdrc_host_destroy(struct ci_hdrc *ci)
+{
+
+}
+
#endif
#endif /* __DRIVERS_USB_CHIPIDEA_HOST_H */
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 116c762..24a100d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -27,6 +27,7 @@
#include "udc.h"
#include "bits.h"
#include "debug.h"
+#include "otg.h"
/* control endpoint description */
static const struct usb_endpoint_descriptor
@@ -1844,13 +1845,13 @@ free_qh_pool:
}
/**
- * udc_remove: parent remove must call this to remove UDC
+ * ci_hdrc_gadget_destroy: parent remove must call this to remove UDC
*
* No interrupts active, the IRQ has been released
*/
-static void udc_stop(struct ci_hdrc *ci)
+void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
{
- if (ci == NULL)
+ if (!ci->roles[CI_ROLE_GADGET])
return;
usb_del_gadget_udc(&ci->gadget);
@@ -1865,15 +1866,32 @@ static void udc_stop(struct ci_hdrc *ci)
if (ci->global_phy)
usb_put_phy(ci->transceiver);
}
- /* my kobject is dynamic, I swear! */
- memset(&ci->gadget, 0, sizeof(ci->gadget));
+}
+
+static int udc_id_switch_for_device(struct ci_hdrc *ci)
+{
+ if (ci->is_otg) {
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+ }
+
+ return 0;
+}
+
+static void udc_id_switch_for_host(struct ci_hdrc *ci)
+{
+ if (ci->is_otg) {
+ /* host doesn't care B_SESSION_VALID event */
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+ }
}
/**
* ci_hdrc_gadget_init - initialize device related bits
* ci: the controller
*
- * This function enables the gadget role, if the device is "device capable".
+ * This function initializes the gadget, if the device is "device capable".
*/
int ci_hdrc_gadget_init(struct ci_hdrc *ci)
{
@@ -1886,11 +1904,11 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
if (!rdrv)
return -ENOMEM;
- rdrv->start = udc_start;
- rdrv->stop = udc_stop;
+ rdrv->start = udc_id_switch_for_device;
+ rdrv->stop = udc_id_switch_for_host;
rdrv->irq = udc_irq;
rdrv->name = "gadget";
ci->roles[CI_ROLE_GADGET] = rdrv;
- return 0;
+ return udc_start(ci);
}
diff --git a/drivers/usb/ch
[PATCH v12 09/13] usb: chipidea: add vbus interrupt handler
We add vbus interrupt handler at ci_otg_work, it uses OTGSC_BSV(at otgsc)
to know it is connect or disconnet event.
Meanwhile, we introduce two flags id_event and b_sess_valid_event to
indicate it is an id interrupt or a vbus interrupt.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/ci.h |5 +
drivers/usb/chipidea/core.c | 28 +++-
drivers/usb/chipidea/otg.c | 42 +++---
drivers/usb/chipidea/otg.h |1 +
drivers/usb/chipidea/udc.c |7 +++
5 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 33cb29f..3160363 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -132,6 +132,9 @@ struct hw_bank {
* @transceiver: pointer to USB PHY, if any
* @hcd: pointer to usb_hcd for ehci host driver
* @debugfs: root dentry for this controller in debugfs
+ * @id_event: indicates there is an id event, and handled at ci_otg_work
+ * @b_sess_valid_event: indicates there is a vbus event, and handled
+ * at ci_otg_work
*/
struct ci_hdrc {
struct device *dev;
@@ -168,6 +171,8 @@ struct ci_hdrc {
struct usb_phy *transceiver;
struct usb_hcd *hcd;
struct dentry *debugfs;
+ boolid_event;
+ boolb_sess_valid_event;
};
static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 0197da2..7a07300 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -304,16 +304,34 @@ static irqreturn_t ci_irq(int irq, void *data)
if (ci->is_otg)
otgsc = hw_read(ci, OP_OTGSC, ~0);
- if (ci->role != CI_ROLE_END)
- ret = ci_role(ci)->irq(ci);
+ /*
+* Handle id change interrupt, it indicates device/host function
+* switch.
+*/
+ if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
+ ci->id_event = true;
+ ci_clear_otg_interrupt(ci, OTGSC_IDIS);
+ disable_irq_nosync(ci->irq);
+ queue_work(ci->wq, &ci->work);
+ return IRQ_HANDLED;
+ }
- if (ci->is_otg && (otgsc & OTGSC_IDIS)) {
- hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
+ /*
+* Handle vbus change interrupt, it indicates device connection
+* and disconnection events.
+*/
+ if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
+ ci->b_sess_valid_event = true;
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
disable_irq_nosync(ci->irq);
queue_work(ci->wq, &ci->work);
- ret = IRQ_HANDLED;
+ return IRQ_HANDLED;
}
+ /* Handle device/host interrupt */
+ if (ci->role != CI_ROLE_END)
+ ret = ci_role(ci)->irq(ci);
+
return ret;
}
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 68f2faf..c74194d 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -52,13 +52,23 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
return role;
}
-/**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
- */
-static void ci_role_work(struct work_struct *work)
+void ci_handle_vbus_change(struct ci_hdrc *ci)
+{
+ u32 otgsc;
+
+ if (!ci->is_otg)
+ return;
+
+ otgsc = hw_read(ci, OP_OTGSC, ~0);
+
+ if (otgsc & OTGSC_BSV)
+ usb_gadget_vbus_connect(&ci->gadget);
+ else
+ usb_gadget_vbus_disconnect(&ci->gadget);
+}
+
+static void ci_handle_id_switch(struct ci_hdrc *ci)
{
- struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
enum ci_role role = ci_otg_role(ci);
if (role != ci->role) {
@@ -68,17 +78,35 @@ static void ci_role_work(struct work_struct *work)
ci_role_stop(ci);
ci_role_start(ci, role);
}
+}
+/**
+ * ci_otg_work - perform otg (vbus/id) event handle
+ * @work: work struct
+ */
+static void ci_otg_work(struct work_struct *work)
+{
+ struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
+
+ if (ci->id_event) {
+ ci->id_event = false;
+ ci_handle_id_switch(ci);
+ } else if (ci->b_sess_valid_event) {
+ ci->b_sess_valid_event = false;
+ ci_handle_vbus_change(ci);
+ } else
+ dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
enable_irq(ci->irq);
}
+
/**
* ci_hdrc_otg_init - initialize otg struct
* ci: the controller
*/
int ci_hdrc_otg_init(struct ci_hdrc *ci)
{
- INIT_WORK(&ci->work, ci_role_work);
+ INIT_WORK(&ci->work, ci_otg_work);
[PATCH v12 13/13] usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget
When we rmmod gadget, the ci->driver needs to be cleared. Otherwise, we plug in usb cable again, the driver will consider gadget is there, in fact, it was removed. Besides, consolidate the calling of ci->driver->disconnect, when we do rmmod gadget, the gadget's disconnect should be called from udc core. Below is the oops this patch fixes. root@freescale ~$ ci_hdrc ci_hdrc.0: Connected to host Unable to handle kernel paging request at virtual address 7f01171c pgd = 80004000 [7f01171c] *pgd=4fa1e811, *pte=, *ppte= Internal error: Oops: 7 [#1] SMP ARM Modules linked in: f_acm libcomposite u_serial [last unloaded: g_serial] CPU: 0Not tainted (3.8.0-rc5+ #221) PC is at _gadget_stop_activity+0xbc/0x128 LR is at ep_fifo_flush+0x68/0xa0 pc : [<803634cc>]lr : [<80363938>]psr: 2193 sp : 806c7dc8 ip : fp : 806c7de4 r10: r9 : 80710ea4 r8 : r7 : bf834094 r6 : bf834098 r5 : bf834010 r4 : bf8340a0 r3 : 7f011708 r2 : 01940194 r1 : a193 r0 : bf834014 Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c53c7d Table: 4f06404a DAC: 0017 Process swapper/0 (pid: 0, stack limit = 0x806c6238) Stack: (0x806c7dc8 to 0x806c8000) 7dc0: bf834010 bf809950 bf834010 004b 806c7e44 806c7de8 7de0: 80365400 8036341c ec1c ec1c 0040 fff5ede0 bf834014 7e00: 000a1220 002e d958 806c7e3c 806c7e20 8004e870 bf834010 7e20: bf809950 004b 004b 80710ea4 806c7e5c 806c7e48 7e40: 80362888 80365154 bfb35180 bf809950 806c7e9c 806c7e60 8008111c 803627f4 7e60: 00989680 bf809900 bf809964 812df8c0 bf809900 bf809950 806c3f2c 7e80: 004b 412fc09a 806c7eb4 806c7ea0 80081368 800810b8 7ea0: bf809900 bf809950 806c7ecc 806c7eb8 800843c8 8008130c 004b 806cf748 7ec0: 806c7ee4 806c7ed0 80081088 8008432c 806c6000 806cf748 806c7f0c 806c7ee8 7ee0: 8000fa44 8008105c f400010c 806ce974 806c7f30 f4000110 806d29e8 412fc09a 7f00: 806c7f2c 806c7f10 80008584 8000f9f4 8000fbd0 6013 806c7f64 7f20: 806c7f84 806c7f30 8000eac0 80008554 000f 8001aea0 7f40: 806c6000 80712a48 804f0040 806d29e8 412fc09a 806c7f84 7f60: 806c7f88 806c7f78 8000fbcc 8000fbd0 6013 806c7fac 806c7f88 7f80: 8001015c 8000fb9c 806cf5b0 80712980 806a4528 8fff 1000406a 412fc09a 7fa0: 806c7fbc 806c7fb0 804e5334 800100ac 806c7ff4 806c7fc0 80668940 804e52d4 7fc0: 806684bc 806a4528 10c53c7d 806ce94c 7fe0: 806a4524 806d29dc 806c7ff8 10008078 806686b0 Backtrace: [<80363410>] (_gadget_stop_activity+0x0/0x128) from [<80365400>] (udc_irq+0x2b8/0xf38) r7:004b r6:bf834010 r5:bf809950 r4:bf834010 [<80365148>] (udc_irq+0x0/0xf38) from [<80362888>] (ci_irq+0xa0/0xf4) [<803627e8>] (ci_irq+0x0/0xf4) from [<8008111c>] (handle_irq_event_percpu+0x70/0x254) r5:bf809950 r4:bfb35180 [<800810ac>] (handle_irq_event_percpu+0x0/0x254) from [<80081368>] (handle_irq_event+0x68/0x88) [<80081300>] (handle_irq_event+0x0/0x88) from [<800843c8>] (handle_fasteoi_irq+0xa8/0x178) r5:bf809950 r4:bf809900 [<80084320>] (handle_fasteoi_irq+0x0/0x178) from [<80081088>] (generic_handle_irq+0x38/0x40) r5:806cf748 r4:004b [<80081050>] (generic_handle_irq+0x0/0x40) from [<8000fa44>] (handle_IRQ+0x5c/0xbc) r5:806cf748 r4:806c6000 [<8000f9e8>] (handle_IRQ+0x0/0xbc) from [<80008584>] (gic_handle_irq+0x3c/0x70) r9:412fc09a r8:806d29e8 r7:f4000110 r6:806c7f30 r5:806ce974 r4:f400010c [<80008548>] (gic_handle_irq+0x0/0x70) from [<8000eac0>] (__irq_svc+0x40/0x54) Exception stack(0x806c7f30 to 0x806c7f78) 7f20: 000f 8001aea0 7f40: 806c6000 80712a48 804f0040 806d29e8 412fc09a 806c7f84 7f60: 806c7f88 806c7f78 8000fbcc 8000fbd0 6013 r7:806c7f64 r6: r5:6013 r4:8000fbd0 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8) [<800100a0>] (cpu_idle+0x0/0xf8) from [<804e5334>] (rest_init+0x6c/0x84) r9:412fc09a r8:1000406a r7:8fff r6:806a4528 r5:80712980 r4:806cf5b0 [<804e52c8>] (rest_init+0x0/0x84) from [<80668940>] (start_kernel+0x29c/0x2ec) [<806686a4>] (start_kernel+0x0/0x2ec) from [<10008078>] (0x10008078) Code: e12fff33 e5953200 e353 0a02 (e5933014) ---[ end trace aade28ad434062bd ]--- Kernel panic - not syncing: 0xbf8bdfa8) df60: 000f 8001aea0 bf8bc000 80712a48 804f0040 df80: 806d29e8 412fc09a bf8bdfb4 bf8bdfb8 bf8bdfa8 8000fbcc 8000fbd0 dfa0: 6013 r7:bf8bdf94 r6: r5:6013 r4:8000fbd0 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8) [<800100a0>] (cpu_idle+0x0/0xf8) from [<804e7930>] (secondary_start_kernel+0x120/0x148) r9:412fc09a r8:1000406a r7:80712d20 r6:10c03c7d r5:0002 r4:806e2008 [<804e7810>] (secondary_start_kernel+0x0/0x148) from [<104e7148>] (0x104
[PATCH v12 08/13] usb: chipidea: move otg relate things to otg file
Move otg relate things to otg file.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c | 63 +--
drivers/usb/chipidea/otg.c | 57 +-
drivers/usb/chipidea/otg.h |2 +
3 files changed, 70 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index f701e3b..0197da2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -295,40 +295,6 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
return 0;
}
-/**
- * ci_otg_role - pick role based on ID pin state
- * @ci: the controller
- */
-static enum ci_role ci_otg_role(struct ci_hdrc *ci)
-{
- u32 sts = hw_read(ci, OP_OTGSC, ~0);
- enum ci_role role = sts & OTGSC_ID
- ? CI_ROLE_GADGET
- : CI_ROLE_HOST;
-
- return role;
-}
-
-/**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
- */
-static void ci_role_work(struct work_struct *work)
-{
- struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
- enum ci_role role = ci_otg_role(ci);
-
- if (role != ci->role) {
- dev_dbg(ci->dev, "switching from %s to %s\n",
- ci_role(ci)->name, ci->roles[role]->name);
-
- ci_role_stop(ci);
- ci_role_start(ci, role);
- }
-
- enable_irq(ci->irq);
-}
-
static irqreturn_t ci_irq(int irq, void *data)
{
struct ci_hdrc *ci = data;
@@ -409,6 +375,8 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
{
ci_hdrc_gadget_destroy(ci);
ci_hdrc_host_destroy(ci);
+ if (ci->is_otg)
+ ci_hdrc_otg_destory(ci);
}
static void ci_get_otg_capable(struct ci_hdrc *ci)
@@ -475,13 +443,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
- INIT_WORK(&ci->work, ci_role_work);
- ci->wq = create_singlethread_workqueue("ci_otg");
- if (!ci->wq) {
- dev_err(dev, "can't create workqueue\n");
- return -ENODEV;
- }
-
/* To know if controller is OTG capable or not */
ci_get_otg_capable(ci);
@@ -510,8 +471,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
- ret = -ENODEV;
- goto rm_wq;
+ return -ENODEV;
+ }
+
+ if (ci->is_otg) {
+ ret = ci_hdrc_otg_init(ci);
+ if (ret) {
+ dev_err(dev, "init otg fails, ret = %d\n", ret);
+ goto stop;
+ }
}
if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
@@ -519,7 +487,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
/* ID pin needs 1ms debouce time, we delay 2ms for safe
*/
mdelay(2);
ci->role = ci_otg_role(ci);
- ci_hdrc_otg_init(ci);
+ ci_enable_otg_interrupt(ci, OTGSC_IDIE);
} else {
/*
* If the controller is not OTG capable, but support
@@ -538,7 +506,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret) {
dev_err(dev, "can't start %s role\n", ci_role(ci)->name);
ret = -ENODEV;
- goto rm_wq;
+ goto stop;
}
platform_set_drvdata(pdev, ci);
@@ -554,9 +522,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
free_irq(ci->irq, ci);
stop:
ci_role_destroy(ci);
-rm_wq:
- flush_workqueue(ci->wq);
- destroy_workqueue(ci->wq);
return ret;
}
@@ -566,8 +531,6 @@ static int ci_hdrc_remove(struct platform_device *pdev)
struct ci_hdrc *ci = platform_get_drvdata(pdev);
dbg_remove_files(ci);
- flush_workqueue(ci->wq);
- destroy_workqueue(ci->wq);
free_irq(ci->irq, ci);
ci_role_destroy(ci);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index abefb4d..68f2faf 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -39,12 +39,65 @@ void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
}
/**
- * ci_hdrc_otg_init - initialize otgsc bits
+ * ci_otg_role - pick role based on ID pin state
+ * @ci: the controller
+ */
+enum ci_role ci_otg_role(struct ci_hdrc *ci)
+{
+ u32 sts = hw_read(ci, OP_OTGSC, ~0);
+ enum ci_role role = sts & OTGSC_ID
+ ? CI_ROLE_GADGET
+ : CI_ROLE_HOST;
+
+ return role;
+}
+
+/**
+ * ci_role_work - perform role changing based on ID pin
+ * @work: work struct
+ */
+static void ci_role_work(struct work_struct *work)
+{
+ struct ci_hdrc *ci = container_of(work, struct ci_hdrc, wor
[PATCH v12 11/13] usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS
CI_HDRC_REGS_SHARED stands for the controller registers is shared
with other USB drivers, if all USB drivers are at chipidea/, it doesn't
needed to set.
CI_HDRC_PULLUP_ON_VBUS stands for pullup dp when the vbus is on. This
flag doesn't need to set if the vbus is always on for gadget
since dp has always pulled up after the gadget has initialized.
So, the current code seems to misuse this two flags.
- When the gadget initializes, the controller doesn't need to run if
it depends on vbus (CI_HDRC_PULLUP_ON_VBUS), it does not
relate to shared register.
- When the gadget starts (load one gadget module), the controller
can run if vbus is on (CI_HDRC_PULLUP_ON_VBUS), it also does not
relate to shared register.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/udc.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index c70ce38..45abf4d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1637,8 +1637,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(&ci->gadget.dev);
if (ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS) {
if (ci->vbus_active) {
- if (ci->platdata->flags & CI_HDRC_REGS_SHARED)
- hw_device_reset(ci, USBMODE_CM_DC);
+ hw_device_reset(ci, USBMODE_CM_DC);
} else {
pm_runtime_put_sync(&ci->gadget.dev);
goto done;
@@ -1801,7 +1800,7 @@ static int udc_start(struct ci_hdrc *ci)
}
}
- if (!(ci->platdata->flags & CI_HDRC_REGS_SHARED)) {
+ if (!(ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS)) {
retval = hw_device_reset(ci, USBMODE_CM_DC);
if (retval)
goto put_transceiver;
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation
On Thu, Jul 11, 2013 at 02:27:10PM +0800, Peter Chen wrote:
> Since we have added vbus reguatlor operation at common
> host file (chipidea/host.c), the glue layer vbus operation
> isn't needed any more.
>
> Signed-off-by: Peter Chen
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 30 +++---
> 1 files changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 14362c0..d06355e 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
> struct usb_phy *phy;
> struct platform_device *ci_pdev;
> struct clk *clk;
> - struct regulator *reg_vbus;
> };
>
> static const struct usbmisc_ops *usbmisc_ops;
> @@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device
> *pdev)
> goto err_clk;
> }
>
> - /* we only support host now, so enable vbus here */
> - data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> - if (!IS_ERR(data->reg_vbus)) {
> - ret = regulator_enable(data->reg_vbus);
> - if (ret) {
> - dev_err(&pdev->dev,
> - "Failed to enable vbus regulator, err=%d\n",
> - ret);
> - goto err_clk;
> - }
> - } else {
> - data->reg_vbus = NULL;
> - }
> -
> pdata.phy = data->phy;
>
> + /* Get the vbus regulator */
> + pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> + if (IS_ERR(pdata.reg_vbus))
> + pdata.reg_vbus = NULL;
I think you should bail out at least in the -EPROBE_DEFER case.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation
On Thu, Jul 11, 2013 at 08:37:19AM +0200, Sascha Hauer wrote:
> >
> > - /* we only support host now, so enable vbus here */
> > - data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > - if (!IS_ERR(data->reg_vbus)) {
> > - ret = regulator_enable(data->reg_vbus);
> > - if (ret) {
> > - dev_err(&pdev->dev,
> > - "Failed to enable vbus regulator, err=%d\n",
> > - ret);
> > - goto err_clk;
> > - }
> > - } else {
> > - data->reg_vbus = NULL;
> > - }
> > -
> > pdata.phy = data->phy;
> >
> > + /* Get the vbus regulator */
> > + pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > + if (IS_ERR(pdata.reg_vbus))
> > + pdata.reg_vbus = NULL;
>
> I think you should bail out at least in the -EPROBE_DEFER case.
>
Oh, correct. I will add another patch behind this for fix.
In fact, I met this problem at FSL mx6-auto board which uses
gpio-expendor chip max7310 as regulator-gpio for enable vbus
output.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
