Re: Linux USB file storage gadget with new UDC
Hi, > The text capture is incomplete. It is missing lots of packets. In > particular, it is missing all the packets between 202489 and 202502. The missing packets are NAK, I added the NAK after Set-Config setup stage. I hide the NAK when i export the packet capture to text format. > Also, I don't understand the "Dir H(S)" part of the capture lines. > What do they mean? They are present on every packet. Dir stands for direction, H is high speed, S is super speed. > This was never the issue. I'm sure the DATA1 packet of the Set-Config > was sent before the Get-Config request. The real question is whether > the DATA1 packet was sent before the Set-Config request had been fully > processed. > > To get more information, try adding > > msleep(100); > > just before the final "return rc;" line in do_set_config(). We should > be able to see in the analyzer log if this 100-ms delay is present. After i added msleep(100) just before final line in do_set_config(), the USB enumeration fails to complete normally. > Here's a second test you can try. In handle_exception(), the > FSG_STATE_CONFIG_CHANGE case, comment out the > > ep0_queue(fsg); > > line. Without that line the Set-Config request should time out, > because the gadget will never complete the status stage. If the > request does complete normally, it will prove that the UDC driver isn't > working right. After i comment out the ep0_queue(fsg) in FSG_STATE_CONFIG_CHANGE case of handle_exception(), the request does complete, but takes more time to complete. And UDC driver queue function is still being called after the Set-Config request. Thanks, Victor -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] quirks: add touchscreen that is dazzeled by remote wakeup
From: Oliver Neukum
The device descriptors are messed up after remote wakeup
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
---
drivers/usb/core/quirks.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index b23f71f..01fe362 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -97,6 +97,9 @@ static const struct usb_device_id usb_quirk_list[] = {
/* Alcor Micro Corp. Hub */
{ USB_DEVICE(0x058f, 0x9254), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* MicroTouch Systems touchscreen */
+ { USB_DEVICE(0x0596, 0x051e), .driver_info = USB_QUIRK_RESET_RESUME },
+
/* appletouch */
{ USB_DEVICE(0x05ac, 0x021a), .driver_info = USB_QUIRK_RESET_RESUME },
--
1.8.3.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/3] usb: musb: core: Call dma_controller_destroy() in error path only once.
In commit f3ce4d5 ("usb: musb: core: call dma_controller_destroy() in the err
path")
I erroneously assumed that the dma controller is not removed in the
error patch. This was wrong because it happens later via musb_free().
That means the original commit can be reverted because it is wrong or we
do this, so it is more obvious.
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/usb/musb/musb_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c01523e..a5f645b 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1809,9 +1809,6 @@ static void musb_free(struct musb *musb)
disable_irq_wake(musb->nIrq);
free_irq(musb->nIrq, musb);
}
- if (musb->dma_controller)
- dma_controller_destroy(musb->dma_controller);
-
musb_host_free(musb);
}
@@ -2035,6 +2032,9 @@ static int musb_remove(struct platform_device *pdev)
musb_exit_debugfs(musb);
musb_shutdown(pdev);
+ if (musb->dma_controller)
+ dma_controller_destroy(musb->dma_controller);
+
musb_free(musb);
device_init_wakeup(dev, 0);
return 0;
--
1.8.4.rc3
--
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/3] usb: musb: cppi41: allow to defer probing if DMA isn't yet available
If everything (musb, cppi41, phy) is built-in then musb will start
without the dma engine printing only
|musb-hdrc musb-hdrc.0.auto: Falied to request rx1.
The reason for this is that the musb device structs are created & probed
before those of the cppi41 device. So the cppi41 device is probed too
late.
As a workaround for this allow the musb_cppi41 part to defer the probe
if everything is fine except for the missing DMA controller. In case of
another error we continue.
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/usb/musb/musb_core.c | 8 +++-
drivers/usb/musb/musb_cppi41.c | 8 ++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index a5f645b..e5ad77d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1882,8 +1882,13 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
pm_runtime_get_sync(musb->controller);
- if (use_dma && dev->dma_mask)
+ if (use_dma && dev->dma_mask) {
musb->dma_controller = dma_controller_create(musb, musb->mregs);
+ if (IS_ERR(musb->dma_controller)) {
+ status = PTR_ERR(musb->dma_controller);
+ goto fail2_5;
+ }
+ }
/* be sure interrupts are disabled before connecting ISR */
musb_platform_disable(musb);
@@ -1976,6 +1981,7 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
fail3:
if (musb->dma_controller)
dma_controller_destroy(musb->dma_controller);
+fail2_5:
pm_runtime_put_sync(musb->controller);
fail2:
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index ae95974..ff9d6de 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -484,6 +484,7 @@ static int cppi41_dma_controller_start(struct
cppi41_dma_controller *controller)
if (ret)
goto err;
+ ret = -EINVAL;
if (port > MUSB_DMA_NUM_CHANNELS || !port)
goto err;
if (is_tx)
@@ -503,6 +504,7 @@ static int cppi41_dma_controller_start(struct
cppi41_dma_controller *controller)
dc = dma_request_slave_channel(dev, str);
if (!dc) {
dev_err(dev, "Falied to request %s.\n", str);
+ ret = -EPROBE_DEFER;
goto err;
}
cppi41_channel->dc = dc;
@@ -510,7 +512,7 @@ static int cppi41_dma_controller_start(struct
cppi41_dma_controller *controller)
return 0;
err:
cppi41_release_all_dma_chans(controller);
- return -EINVAL;
+ return ret;
}
void dma_controller_destroy(struct dma_controller *c)
@@ -526,7 +528,7 @@ struct dma_controller *dma_controller_create(struct musb
*musb,
void __iomem *base)
{
struct cppi41_dma_controller *controller;
- int ret;
+ int ret = 0;
if (!musb->controller->of_node) {
dev_err(musb->controller, "Need DT for the DMA engine.\n");
@@ -553,5 +555,7 @@ struct dma_controller *dma_controller_create(struct musb
*musb,
plat_get_fail:
kfree(controller);
kzalloc_fail:
+ if (ret == -EPROBE_DEFER)
+ return ERR_PTR(ret);
return NULL;
}
--
1.8.4.rc3
--
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/3] usb: musb: core: properly free host / device structs in err path
The patch fixes two issues in the error path cleanup: - in MUSB_PORT_MODE_DUAL_ROLE mode, if musb_gadget_setup() fails we never cleanup the host struct earlier allocated. - if musb_init_debugfs() or sysfs_create_group() fails, then we never free the host part initialization, only device part. Cc: sta...@vger.kernel.org # v3.11 Cc: Daniel Mack Signed-off-by: Sebastian Andrzej Siewior --- drivers/usb/musb/musb_core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index cd70cc8..c01523e 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1946,6 +1946,8 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) if (status < 0) goto fail3; status = musb_gadget_setup(musb); + if (status) + musb_host_cleanup(musb); break; default: dev_err(dev, "unsupported port mode %d\n", musb->port_mode); @@ -1972,6 +1974,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) fail4: musb_gadget_cleanup(musb); + musb_host_cleanup(musb); fail3: if (musb->dma_controller) -- 1.8.4.rc3 -- 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
musb: fix error path & allow to have everything built-in
Hi, this is a new series of bug fixing: - #1 fixes error path which is broken since the host / gadget split - #2 fixes an error in the error path which I introduced this merged window. If you decide instead to revert the patch please not that I will have to alter #3 (in order not to free the dma controller if musb->dma_controller == -EPROBE_DEFERED) - #3 makes cppi41 work in built-in case where both musb, phy and cppi41 is built into the kernel. I've been thinking about a select for TI_CPPI41 but then it will be forced to be built-in. Not sure if want this. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Add regulator support for pxa27x ohci and devices
This patchset adds regulator support for pxa27x ohci to make it possible to turn the ohci controller and its usb devices on/off during probe/remove, as well as resume/suspend. Cc: Alan Stern Cc: Greg Kroah-Hartman Cc: Igor Grinberg Nikita Kiryanov (2): usb: ohci-pxa27x: add regulator support usb: ohci-pxa27x: add regulators for usb devices drivers/usb/host/ohci-pxa27x.c | 62 -- 1 file changed, 59 insertions(+), 3 deletions(-) -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: ohci-pxa27x: add regulator support
Add regulator support for usb so that it would be possible to turn
it on and off (also for suspend/resume).
Cc: Alan Stern
Cc: Greg Kroah-Hartman
Signed-off-by: Nikita Kiryanov
Signed-off-by: Igor Grinberg
---
drivers/usb/host/ohci-pxa27x.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 93371a2..35e739e 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/*
* UHC: USB Host Controller (OHCI-like) register definitions
@@ -107,6 +108,7 @@ struct pxa27x_ohci {
struct device *dev;
struct clk *clk;
+ struct regulator *usb_regulator;
void __iomem*mmio_base;
};
@@ -221,6 +223,10 @@ static int pxa27x_start_hc(struct pxa27x_ohci *ohci,
struct device *dev)
inf = dev_get_platdata(dev);
+ retval = regulator_enable(ohci->usb_regulator);
+ if (retval)
+ return retval;
+
clk_prepare_enable(ohci->clk);
pxa27x_reset_hc(ohci);
@@ -272,6 +278,7 @@ static void pxa27x_stop_hc(struct pxa27x_ohci *ohci, struct
device *dev)
udelay(10);
clk_disable_unprepare(ohci->clk);
+ regulator_disable(ohci->usb_regulator);
}
#ifdef CONFIG_OF
@@ -413,10 +420,16 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
struct platform_device
ohci->dev = &pdev->dev;
ohci->clk = usb_clk;
ohci->mmio_base = (void __iomem *)hcd->regs;
+ ohci->usb_regulator = regulator_get(&pdev->dev, "vcc usb");
+ if (IS_ERR(ohci->usb_regulator)) {
+ pr_err("could not obtain usb regulator");
+ retval = PTR_ERR(ohci->usb_regulator);
+ goto err3;
+ }
if ((retval = pxa27x_start_hc(ohci, &pdev->dev)) < 0) {
pr_debug("pxa27x_start_hc failed");
- goto err3;
+ goto err4;
}
/* Select Power Management Mode */
@@ -432,6 +445,9 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
struct platform_device
return retval;
pxa27x_stop_hc(ohci, &pdev->dev);
+
+err4:
+ regulator_put(ohci->usb_regulator);
err3:
iounmap(hcd->regs);
err2:
@@ -467,6 +483,7 @@ void usb_hcd_pxa27x_remove (struct usb_hcd *hcd, struct
platform_device *pdev)
release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
clk_put(ohci->clk);
+ regulator_put(ohci->usb_regulator);
}
/*-*/
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: ohci-pxa27x: add regulators for usb devices
Add regulator support for devices that sit on USB ports,
thus allowing usb devices power management for suspend/resume.
Cc: Alan Stern
Cc: Greg Kroah-Hartman
Signed-off-by: Nikita Kiryanov
Signed-off-by: Igor Grinberg
---
drivers/usb/host/ohci-pxa27x.c | 47 ++
1 file changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 35e739e..ae4c64f 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -109,6 +109,7 @@ struct pxa27x_ohci {
struct device *dev;
struct clk *clk;
struct regulator *usb_regulator;
+ struct regulator *ext_regulator[PXA_UHC_MAX_PORTNUM];
void __iomem*mmio_base;
};
@@ -217,7 +218,7 @@ extern void pxa27x_clear_otgph(void);
static int pxa27x_start_hc(struct pxa27x_ohci *ohci, struct device *dev)
{
- int retval = 0;
+ int i, retval = 0;
struct pxaohci_platform_data *inf;
uint32_t uhchr;
@@ -227,6 +228,16 @@ static int pxa27x_start_hc(struct pxa27x_ohci *ohci,
struct device *dev)
if (retval)
return retval;
+ for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
+ retval = 0;
+ if (ohci->ext_regulator[i])
+ retval = regulator_enable(ohci->ext_regulator[i]);
+
+ if (retval)
+ pr_err("%s: regulator enable failed: port%d, err=%d\n",
+ __func__, i, retval);
+ }
+
clk_prepare_enable(ohci->clk);
pxa27x_reset_hc(ohci);
@@ -257,8 +268,22 @@ static int pxa27x_start_hc(struct pxa27x_ohci *ohci,
struct device *dev)
return 0;
}
+static void ohci_regulator_put_all(struct pxa27x_ohci *ohci)
+{
+ int i;
+
+ for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
+ if (ohci->ext_regulator[i])
+ regulator_put(ohci->ext_regulator[i]);
+ }
+
+ /* usb regulator should be present if we get here */
+ regulator_put(ohci->usb_regulator);
+}
+
static void pxa27x_stop_hc(struct pxa27x_ohci *ohci, struct device *dev)
{
+ int i;
struct pxaohci_platform_data *inf;
uint32_t uhccoms;
@@ -278,6 +303,12 @@ static void pxa27x_stop_hc(struct pxa27x_ohci *ohci,
struct device *dev)
udelay(10);
clk_disable_unprepare(ohci->clk);
+
+ for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
+ if (ohci->ext_regulator[i])
+ regulator_disable(ohci->ext_regulator[i]);
+ }
+
regulator_disable(ohci->usb_regulator);
}
@@ -360,12 +391,13 @@ static int ohci_pxa_of_init(struct platform_device *pdev)
*/
int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct
platform_device *pdev)
{
- int retval, irq;
+ int retval, irq, i;
struct usb_hcd *hcd;
struct pxaohci_platform_data *inf;
struct pxa27x_ohci *ohci;
struct resource *r;
struct clk *usb_clk;
+ char supply[7];
retval = ohci_pxa_of_init(pdev);
if (retval)
@@ -427,6 +459,13 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
struct platform_device
goto err3;
}
+ for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
+ snprintf(supply, sizeof(supply), "fsusb%d", i);
+ ohci->ext_regulator[i] = regulator_get(&pdev->dev, supply);
+ if (IS_ERR(ohci->ext_regulator[i]))
+ ohci->ext_regulator[i] = NULL;
+ }
+
if ((retval = pxa27x_start_hc(ohci, &pdev->dev)) < 0) {
pr_debug("pxa27x_start_hc failed");
goto err4;
@@ -447,7 +486,7 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
struct platform_device
pxa27x_stop_hc(ohci, &pdev->dev);
err4:
- regulator_put(ohci->usb_regulator);
+ ohci_regulator_put_all(ohci);
err3:
iounmap(hcd->regs);
err2:
@@ -483,7 +522,7 @@ void usb_hcd_pxa27x_remove (struct usb_hcd *hcd, struct
platform_device *pdev)
release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
clk_put(ohci->clk);
- regulator_put(ohci->usb_regulator);
+ ohci_regulator_put_all(ohci);
}
/*-*/
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb/chipidea: fix oops on memory allocation failure
When CMA fails to initialize in v3.12-rc4, the chipidea driver oopses
the kernel while trying to remove and put the HCD which doesn't exist:
WARNING: CPU: 0 PID: 6 at /home/rmk/git/linux-rmk/arch/arm/mm/dma-mapping.c:511
__dma_alloc+0x200/0x240()
coherent pool not initialised!
Modules linked in:
CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
Workqueue: deferwq deferred_probe_work_func
Backtrace:
[] (dump_backtrace+0x0/0x10c) from [] (show_stack+0x18/0x1c)
r6:c05fd9cc r5:01ff r4: r3:df86ad00
[] (show_stack+0x0/0x1c) from [] (dump_stack+0x70/0x8c)
[] (dump_stack+0x0/0x8c) from []
(warn_slowpath_common+0x6c/0x8c)
r4:df883a60 r3:df86ad00
[] (warn_slowpath_common+0x0/0x8c) from []
(warn_slowpath_fmt+0x38/0x40)
r8: r7:1000 r6:c083b808 r5: r4:df2efe80
[] (warn_slowpath_fmt+0x0/0x40) from []
(__dma_alloc+0x200/0x240)
r3: r2:c05fda00
[] (__dma_alloc+0x0/0x240) from [] (arm_dma_alloc+0x88/0xa0)
[] (arm_dma_alloc+0x0/0xa0) from [] (ehci_setup+0x1f4/0x438)
[] (ehci_setup+0x0/0x438) from [] (usb_add_hcd+0x18c/0x664)
[] (usb_add_hcd+0x0/0x664) from [] (host_start+0xf0/0x180)
[] (host_start+0x0/0x180) from [] (ci_hdrc_probe+0x360/0x670
)
r6:df2ef410 r5: r4:df2c3010 r3:c03e8904
[] (ci_hdrc_probe+0x0/0x670) from []
(platform_drv_probe+0x20/0x24)
[] (platform_drv_probe+0x0/0x24) from []
(driver_probe_device+0x9c/0x234)
...
---[ end trace c88ccaf3969e8422 ]---
Unable to handle kernel NULL pointer dereference at virtual address 0028
pgd = c0004000
[0028] *pgd=
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
Workqueue: deferwq deferred_probe_work_func
task: df86ad00 ti: df882000 task.ti: df882000
PC is at usb_remove_hcd+0x10/0x150
LR is at host_stop+0x1c/0x3c
pc : []lr : []psr: 6013
sp : df883b50 ip : df883b78 fp : df883b74
r10: c11f4c54 r9 : c0836450 r8 : df30c400
r7 : fff4 r6 : df2ef410 r5 : r4 : df2c3010
r3 : r2 : r1 : df86b0a0 r0 :
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 2f29404a DAC: 0015
Process kworker/u2:0 (pid: 6, stack limit = 0xdf882240)
Stack: (0xdf883b50 to 0xdf884000)
...
Backtrace:
[] (usb_remove_hcd+0x0/0x150) from [] (host_stop+0x1c/0x3c)
r6:df2ef410 r5: r4:df2c3010
[] (host_stop+0x0/0x3c) from []
(ci_hdrc_host_destroy+0x1c/0x20)
r5: r4:df2c3010
[] (ci_hdrc_host_destroy+0x0/0x20) from []
(ci_hdrc_probe+0x3ac/0x670)
[] (ci_hdrc_probe+0x0/0x670) from []
(platform_drv_probe+0x20/0x24)
[] (platform_drv_probe+0x0/0x24) from []
(driver_probe_device+0x9c/0x234)
[] (driver_probe_device+0x0/0x234) from []
(__device_attach+0x44/0x48)
...
---[ end trace c88ccaf3969e8423 ]---
Fix this so at least we can continue booting and get to a shell prompt.
Signed-off-by: Russell King
Tested-by: Russell King
---
drivers/usb/chipidea/host.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 6f96795..64d7a6d 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -100,8 +100,10 @@ static void host_stop(struct ci_hdrc *ci)
{
struct usb_hcd *hcd = ci->hcd;
- usb_remove_hcd(hcd);
- usb_put_hcd(hcd);
+ if (hcd) {
+ usb_remove_hcd(hcd);
+ usb_put_hcd(hcd);
+ }
if (ci->platdata->reg_vbus)
regulator_disable(ci->platdata->reg_vbus);
}
--
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 v3 1/4] usb: musb: gadget, stay IDLE without gadget driver
If there is no gadget driver musb should stay in B_IDLE state.
Signed-off-by: Markus Pargmann
---
drivers/usb/musb/musb_core.c | 3 ---
drivers/usb/musb/musb_gadget.c | 14 --
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 18e877f..7d40dec 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -831,12 +831,9 @@ b_host:
case OTG_STATE_B_WAIT_ACON:
dev_dbg(musb->controller, "HNP: RESET (%s), to
b_peripheral\n",
usb_otg_state_string(musb->xceiv->state));
- musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
musb_g_reset(musb);
break;
case OTG_STATE_B_IDLE:
- musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
- /* FALLTHROUGH */
case OTG_STATE_B_PERIPHERAL:
musb_g_reset(musb);
break;
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index b19ed21..fddda5d 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -2113,10 +2113,20 @@ __acquires(musb->lock)
* or else after HNP, as A-Device
*/
if (devctl & MUSB_DEVCTL_BDEVICE) {
- musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
+ if (!musb->gadget_driver) {
+ musb->is_active = 0;
+ musb->xceiv->state = OTG_STATE_B_IDLE;
+ } else {
+ musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
+ }
musb->g.is_a_peripheral = 0;
} else {
- musb->xceiv->state = OTG_STATE_A_PERIPHERAL;
+ if (!musb->gadget_driver) {
+ musb->is_active = 0;
+ musb->xceiv->state = OTG_STATE_A_IDLE;
+ } else {
+ musb->xceiv->state = OTG_STATE_A_PERIPHERAL;
+ }
musb->g.is_a_peripheral = 1;
}
--
1.8.4.rc3
--
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 v3 0/0] usb: musb bugfixes
Hi, the series contains two bugfixes and some debugfs file operations for dsps similar to the musb core regdump debugfs file. Regards, Markus Pargmann Changes in v3: - Using debugfs_reg32 for regdump debug file - Added a patch to replace kzalloc with devm_kzalloc - otg->gadget_driver NULL pointer handling was replaced by a musb_g_reset patch to force IDLE states when no gadget driver is loaded. Markus Pargmann (4): usb: musb: gadget, stay IDLE without gadget driver usb: musb: Bugfix of_node assignment usb: musb: dsps, debugfs files usb: musb: dsps, use devm_kzalloc drivers/usb/musb/musb_core.c | 15 --- drivers/usb/musb/musb_dsps.c | 60 +++--- drivers/usb/musb/musb_gadget.c | 14 -- 3 files changed, 79 insertions(+), 10 deletions(-) -- 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 v3 3/4] usb: musb: dsps, debugfs files
debugfs files to show the contents of important dsps registers.
Signed-off-by: Markus Pargmann
---
drivers/usb/musb/musb_dsps.c | 55
1 file changed, 55 insertions(+)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 189e52c..0680f0e 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -46,6 +46,8 @@
#include
#include
+#include
+
#include "musb_core.h"
static const struct of_device_id musb_dsps_of_match[];
@@ -119,6 +121,27 @@ struct dsps_glue {
const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
struct timer_list timer;/* otg_workaround timer */
unsigned long last_timer;/* last timer data for each instance */
+
+ struct debugfs_regset32 regset;
+ struct dentry *dbgfs_root;
+};
+
+static const struct debugfs_reg32 dsps_musb_regs[] = {
+ { "revision", 0x00 },
+ { "control",0x14 },
+ { "status", 0x18 },
+ { "eoi",0x24 },
+ { "intr0_stat", 0x30 },
+ { "intr1_stat", 0x34 },
+ { "intr0_set", 0x38 },
+ { "intr1_set", 0x3c },
+ { "txmode", 0x70 },
+ { "rxmode", 0x74 },
+ { "autoreq",0xd0 },
+ { "srpfixtime", 0xd4 },
+ { "tdown", 0xd8 },
+ { "phy_utmi", 0xe0 },
+ { "mode", 0xe8 },
};
/**
@@ -348,6 +371,30 @@ out:
return ret;
}
+static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
+{
+ struct dentry *root;
+ struct dentry *file;
+ char buf[128];
+
+ sprintf(buf, "%s.dsps", dev_name(musb->controller));
+ root = debugfs_create_dir(buf, NULL);
+ if (!root)
+ return -ENOMEM;
+ glue->dbgfs_root = root;
+
+ glue->regset.regs = dsps_musb_regs;
+ glue->regset.nregs = ARRAY_SIZE(dsps_musb_regs);
+ glue->regset.base = musb->ctrl_base;
+
+ file = debugfs_create_regset32("regdump", S_IRUGO, root, &glue->regset);
+ if (!file) {
+ debugfs_remove_recursive(root);
+ return -ENOMEM;
+ }
+ return 0;
+}
+
static int dsps_musb_init(struct musb *musb)
{
struct device *dev = musb->controller;
@@ -357,6 +404,7 @@ static int dsps_musb_init(struct musb *musb)
void __iomem *reg_base;
struct resource *r;
u32 rev, val;
+ int ret;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
if (!r)
@@ -390,6 +438,10 @@ static int dsps_musb_init(struct musb *musb)
val &= ~(1 << wrp->otg_disable);
dsps_writel(musb->ctrl_base, wrp->phy_utmi, val);
+ ret = dsps_musb_dbg_init(musb, glue);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -587,6 +639,9 @@ static int dsps_remove(struct platform_device *pdev)
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
kfree(glue);
+
+ debugfs_remove_recursive(glue->dbgfs_root);
+
return 0;
}
--
1.8.4.rc3
--
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 v3 4/4] usb: musb: dsps, use devm_kzalloc
Signed-off-by: Markus Pargmann
---
drivers/usb/musb/musb_dsps.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0680f0e..aae017c 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -597,7 +597,7 @@ static int dsps_probe(struct platform_device *pdev)
wrp = match->data;
/* allocate glue */
- glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+ glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&pdev->dev, "unable to allocate glue memory\n");
return -ENOMEM;
@@ -625,7 +625,6 @@ err3:
pm_runtime_put(&pdev->dev);
err2:
pm_runtime_disable(&pdev->dev);
- kfree(glue);
return ret;
}
@@ -638,7 +637,6 @@ static int dsps_remove(struct platform_device *pdev)
/* disable usbss clocks */
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
- kfree(glue);
debugfs_remove_recursive(glue->dbgfs_root);
--
1.8.4.rc3
--
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 v3 2/4] usb: musb: Bugfix of_node assignment
It is not safe to assign the of_node to a device without driver. The
device is matched against a list of drivers and the of_node could lead
to a DT match with the parent driver.
Signed-off-by: Markus Pargmann
---
drivers/usb/musb/musb_core.c | 12 +++-
drivers/usb/musb/musb_dsps.c | 1 -
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 7d40dec..651cce0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1958,6 +1958,7 @@ static int musb_probe(struct platform_device *pdev)
int irq = platform_get_irq_byname(pdev, "mc");
struct resource *iomem;
void __iomem*base;
+ int ret;
iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!iomem || irq <= 0)
@@ -1967,7 +1968,16 @@ static int musb_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
- return musb_init_controller(dev, irq, base);
+ if (dev->parent)
+ dev->of_node = dev->parent->of_node;
+
+ ret = musb_init_controller(dev, irq, base);
+ if (ret) {
+ dev->of_node = NULL;
+ return ret;
+ }
+
+ return 0;
}
static int musb_remove(struct platform_device *pdev)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index bd4138d..189e52c 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -483,7 +483,6 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
musb->dev.parent= dev;
musb->dev.dma_mask = &musb_dmamask;
musb->dev.coherent_dma_mask = musb_dmamask;
- musb->dev.of_node = of_node_get(dn);
glue->musb = musb;
--
1.8.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/7] usb: dwc3: get "usb_phy" only if the platform indicates the presence of PHY's
Hi Kishon,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
> There can be systems which does not have a external usb_phy, so get
> usb_phy only if dt data indicates the presence of PHY in the case of dt boot
> or
> if platform_data indicates the presence of PHY. Also remove checking if
> return value is -ENXIO since it's now changed to always enable usb_phy layer.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
> always
> refers to usb3 phy. Since we've lived so long with this, this patch will make
> an assumption that if only one entry is populated in *usb-phy* property, it
> will
> be usb2 phy and the next entry will be usb3 phy.
>
> drivers/usb/dwc3/Kconfig |1 +
> drivers/usb/dwc3/core.c | 72
> --
> drivers/usb/dwc3/platform_data.h |2 ++
> 3 files changed, 41 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 70fc430..8e385b4 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -1,6 +1,7 @@
> config USB_DWC3
> tristate "DesignWare USB3 DRD Core Support"
> depends on (USB || USB_GADGET) && HAS_DMA
> + select USB_PHY
> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> help
> Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 474162e..cb91d70 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
> struct device_node *node = dev->of_node;
> struct resource *res;
> struct dwc3 *dwc;
> + int count;
>
> int ret = -ENOMEM;
>
> @@ -387,16 +388,49 @@ 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);
> + count = of_count_phandle_with_args(node, "usb-phy", NULL);
> + switch (count) {
> + case 2:
> + dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev,
> + "usb-phy", 1);
> + if (IS_ERR(dwc->usb3_phy)) {
> + dev_err(dev, "usb3 phy not found\n");
> + return PTR_ERR(dwc->usb3_phy);
> + }
> + case 1:
> + dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev,
> + "usb-phy", 0);
> + if (IS_ERR(dwc->usb2_phy)) {
> + dev_err(dev, "usb2 phy not found\n");
> + return PTR_ERR(dwc->usb2_phy);
> + }
> + break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
> + default:
> + dev_err(dev, "usb phy nodes not configured\n");
> + }
>
> dwc->needs_fifo_resize = of_property_read_bool(node,
> "tx-fifo-resize");
> dwc->dr_mode = of_usb_get_dr_mode(node);
> } else if (pdata) {
> 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);
> + if (pdata->usb2_phy) {
> + dwc->usb2_phy = devm_usb_get_phy(dev,
> + USB_PHY_TYPE_USB2);
> + if (IS_ERR(dwc->usb2_phy)) {
> + dev_err(dev, "usb2 phy not found\n");
> + return PTR_ERR(dwc->usb2_phy);
> + }
> + }
> +
> + if (pdata->usb3_phy) {
> + dwc->usb3_phy = devm_usb_get_phy(dev,
> + USB_PHY_TYPE_USB3);
> + if (IS_ERR(dwc->usb3_phy)) {
> + dev_err(dev, "usb3 phy not found\n");
> + return PTR_ERR(dwc->usb3_phy);
> + }
> + }
This part looks fine to me.
>
> dwc->needs_fifo_resize = pdata->tx_fifo_resize;
> dwc->dr_mode = pdata->dr_mode;
> @@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
> if (dwc->maximum_speed == USB_SPEED_UNKNOWN)
> dwc->maximum_speed = USB_SPEED_SUPER;
>
> - if (IS_ERR(dwc->usb2_phy)) {
> - ret = PTR_ERR(dwc->usb2_phy);
> -
> - /*
> - * if -ENXIO is returned, it means PHY layer wasn't
> - * enabled, so it
Re: [Application RFC PATCH] ptp-gadget: move from gadgetfs to functionfs
Hi Guennadi, On Tue, Jun 18, 2013 at 11:54:00AM +0200, Guennadi Liakhovetski wrote: > On Mon, 17 Jun 2013, Michael Grzeschik wrote: > > > Hi Guennadi, > > > > On Thu, Jun 06, 2013 at 12:34:39AM +0200, Michael Grzeschik wrote: > > > This patch moves the ptp-gadget to use functionfs for the endpoint > > > handling. With functionfs, this patch is deleting a lot of control ep0 > > > code, which is now handled by the framework. > > > > Ping! Do you know who is maintaining this code and/or > > could be interested in this work? > > Yes, I'm still - formally - the maintainer of the code. Fortunately the > patch rate is quite low, but still - I have little time to take care of > them when they arrive, and if they're non-trivial, my expertise level in > this area isn't high enough to process patches quickly. Also in this case > to really review patch contents I would first have to study the > functionfs. Besides I don't have a platform, on which I _easily_ could > test the software, I would first have to identify, which one is best > suitable. So, if someone wants to take over mainternership of this > package, I wouldn't mind. Anatolij? Anyone@DENX? could you catch some attention for the project? I really would like to find that patch on top of this stack. git://git.denx.de/ptp-gadget.git Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/7] usb: dwc3: get "usb_phy" only if the platform indicates the presence of PHY's
Hi roger,
On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
> Hi Kishon,
>
> On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
>> There can be systems which does not have a external usb_phy, so get
>> usb_phy only if dt data indicates the presence of PHY in the case of dt boot
>> or
>> if platform_data indicates the presence of PHY. Also remove checking if
>> return value is -ENXIO since it's now changed to always enable usb_phy layer.
>>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
>> always
>> refers to usb3 phy. Since we've lived so long with this, this patch will make
>> an assumption that if only one entry is populated in *usb-phy* property, it
>> will
>> be usb2 phy and the next entry will be usb3 phy.
>>
>> drivers/usb/dwc3/Kconfig |1 +
>> drivers/usb/dwc3/core.c | 72
>> --
>> drivers/usb/dwc3/platform_data.h |2 ++
>> 3 files changed, 41 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index 70fc430..8e385b4 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -1,6 +1,7 @@
>> config USB_DWC3
>> tristate "DesignWare USB3 DRD Core Support"
>> depends on (USB || USB_GADGET) && HAS_DMA
>> +select USB_PHY
>> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>> help
>>Say Y or M here if your system has a Dual Role SuperSpeed
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 474162e..cb91d70 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
>> struct device_node *node = dev->of_node;
>> struct resource *res;
>> struct dwc3 *dwc;
>> +int count;
>>
>> int ret = -ENOMEM;
>>
>> @@ -387,16 +388,49 @@ 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);
>> +count = of_count_phandle_with_args(node, "usb-phy", NULL);
>> +switch (count) {
>> +case 2:
>> +dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev,
>> +"usb-phy", 1);
>> +if (IS_ERR(dwc->usb3_phy)) {
>> +dev_err(dev, "usb3 phy not found\n");
>> +return PTR_ERR(dwc->usb3_phy);
>> +}
>> +case 1:
>> +dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev,
>> +"usb-phy", 0);
>> +if (IS_ERR(dwc->usb2_phy)) {
>> +dev_err(dev, "usb2 phy not found\n");
>> +return PTR_ERR(dwc->usb2_phy);
>> +}
>> +break;
>
> In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
> will wrongly treat it as usb2_phy.
That was the case even before this patch no? Unfortunately the old USB PHY
library doesn't have APIs to get PHYs in a better way. If we try modifying the
USB PHY library, it'll be kind of duplicating what is already there in the
Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
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 v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
> Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
> power_on and power_off the following APIs are used phy_init(), phy_exit(),
> phy_power_on() and phy_power_off().
>
> However using the old USB phy library wont be removed till the PHYs of all
> other SoC's using dwc3 core is adapted to the Generic PHY Framework.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
> drivers/usb/dwc3/Kconfig |1 +
> drivers/usb/dwc3/core.c| 52
>
> drivers/usb/dwc3/core.h|7
> drivers/usb/dwc3/platform_data.h |2 +
> 5 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
> b/Documentation/devicetree/bindings/usb/dwc3.txt
> index e807635..471366d 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -6,11 +6,13 @@ Required properties:
> - compatible: must be "snps,dwc3"
> - reg : Address and length of the register set for the device
> - interrupts: Interrupts used by the dwc3 controller.
> +
> +Optional properties:
> - usb-phy : array of phandle for the PHY device. The first element
> in the array is expected to be a handle to the USB2/HS PHY and
> the second element is expected to be a handle to the USB3/SS PHY
> -
> -Optional properties:
> + - phys: from the *Generic PHY* bindings
> + - phy-names: from the *Generic PHY* bindings
> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>
> This is usually a subnode to DWC3 glue to which it is connected.
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 8e385b4..64eed6f 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -2,6 +2,7 @@ config USB_DWC3
> tristate "DesignWare USB3 DRD Core Support"
> depends on (USB || USB_GADGET) && HAS_DMA
> select USB_PHY
> + select GENERIC_PHY
> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> help
> Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index cb91d70..28bfa5b 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>
> usb_phy_init(dwc->usb2_phy);
> usb_phy_init(dwc->usb3_phy);
> +
> + if (dwc->usb2_generic_phy)
> + phy_init(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_init(dwc->usb3_generic_phy);
> +
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is wrong.
> mdelay(100);
>
> /* Clear USB3 PHY reset */
> @@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
> {
> usb_phy_shutdown(dwc->usb2_phy);
> usb_phy_shutdown(dwc->usb3_phy);
> +
> + if (dwc->usb2_generic_phy)
> + phy_power_off(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_power_off(dwc->usb3_generic_phy);
> }
>
> #define DWC3_ALIGN_MASK (16 - 1)
> @@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
> }
>
> + count = of_property_match_string(node, "phy-names", "usb2-phy");
> + if (count >= 0 || (pdata && pdata->usb2_generic_phy)) {
> + dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> + if (IS_ERR(dwc->usb2_generic_phy)) {
> + dev_err(dev, "no usb2 phy configured yet");
> + return PTR_ERR(dwc->usb2_generic_phy);
> + }
> + dwc->usb2_phy = NULL;
> + }
> +
> + count = of_property_match_string(node, "phy-names", "usb3-phy");
> + if (count >= 0 || (pdata && pdata->usb3_generic_phy)) {
> + dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> + if (IS_ERR(dwc->usb3_generic_phy)) {
> + dev_err(dev, "no usb3 phy configured yet");
> + return PTR_ERR(dwc->usb3_generic_phy);
> + }
> + dwc->usb3_phy = NULL;
> + }
> +
There are a couple of issues here.
- The above code must be executed only if it node is valid.
- We must either get both PHYs via old method or via new method and not support
mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
If you give the new method preference then you don't even need to attempt a
devm_usb_get_phy()
if the generic phy is present in the node.
> /* default to superspeed if no maximum_speed
Re: [PATCH v2 1/7] usb: dwc3: get "usb_phy" only if the platform indicates the presence of PHY's
On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote:
> Hi roger,
>
> On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
>> Hi Kishon,
>>
>> On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
>>> There can be systems which does not have a external usb_phy, so get
>>> usb_phy only if dt data indicates the presence of PHY in the case of dt
>>> boot or
>>> if platform_data indicates the presence of PHY. Also remove checking if
>>> return value is -ENXIO since it's now changed to always enable usb_phy
>>> layer.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I
>>> ---
>>> In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
>>> always
>>> refers to usb3 phy. Since we've lived so long with this, this patch will
>>> make
>>> an assumption that if only one entry is populated in *usb-phy* property, it
>>> will
>>> be usb2 phy and the next entry will be usb3 phy.
>>>
>>> drivers/usb/dwc3/Kconfig |1 +
>>> drivers/usb/dwc3/core.c | 72
>>> --
>>> drivers/usb/dwc3/platform_data.h |2 ++
>>> 3 files changed, 41 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>> index 70fc430..8e385b4 100644
>>> --- a/drivers/usb/dwc3/Kconfig
>>> +++ b/drivers/usb/dwc3/Kconfig
>>> @@ -1,6 +1,7 @@
>>> config USB_DWC3
>>> tristate "DesignWare USB3 DRD Core Support"
>>> depends on (USB || USB_GADGET) && HAS_DMA
>>> + select USB_PHY
>>> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>>> help
>>> Say Y or M here if your system has a Dual Role SuperSpeed
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 474162e..cb91d70 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>> struct device_node *node = dev->of_node;
>>> struct resource *res;
>>> struct dwc3 *dwc;
>>> + int count;
>>>
>>> int ret = -ENOMEM;
>>>
>>> @@ -387,16 +388,49 @@ 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);
>>> + count = of_count_phandle_with_args(node, "usb-phy", NULL);
>>> + switch (count) {
>>> + case 2:
>>> + dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev,
>>> + "usb-phy", 1);
>>> + if (IS_ERR(dwc->usb3_phy)) {
>>> + dev_err(dev, "usb3 phy not found\n");
>>> + return PTR_ERR(dwc->usb3_phy);
>>> + }
>>> + case 1:
>>> + dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev,
>>> + "usb-phy", 0);
>>> + if (IS_ERR(dwc->usb2_phy)) {
>>> + dev_err(dev, "usb2 phy not found\n");
>>> + return PTR_ERR(dwc->usb2_phy);
>>> + }
>>> + break;
>>
>> In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
>> will wrongly treat it as usb2_phy.
>
> That was the case even before this patch no? Unfortunately the old USB PHY
> library doesn't have APIs to get PHYs in a better way. If we try modifying the
> USB PHY library, it'll be kind of duplicating what is already there in the
> Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
OK. I agree with you.
Do you know if there are users of dwc3 other than exynos5250 and omap5?
If not, we should get rid of the old USB PHY altogether.
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 v2 4/7] drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
> Adapted omap-usb3 PHY driver to Generic PHY Framework and moved phy-omap-usb3
> driver in drivers/usb/phy to drivers/phy and also renamed the file to
> phy-ti-pipe3 since this same driver will be used for SATA PHY and
> PCIE PHY.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/phy/Kconfig| 10 +
> drivers/phy/Makefile |1 +
> .../phy/phy-omap-usb3.c => phy/phy-ti-pipe3.c} | 206
> +++-
> drivers/usb/phy/Kconfig| 11 --
> drivers/usb/phy/Makefile |1 -
> include/linux/phy/ti_pipe3.h | 52 +
> 6 files changed, 174 insertions(+), 107 deletions(-)
> rename drivers/{usb/phy/phy-omap-usb3.c => phy/phy-ti-pipe3.c} (57%)
> create mode 100644 include/linux/phy/ti_pipe3.h
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index ac239ac..1158943 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -27,6 +27,16 @@ config OMAP_USB2
> The USB OTG controller communicates with the comparator using this
> driver.
>
> +config TI_PIPE3
> + tristate "TI PIPE3 PHY Driver"
> + select GENERIC_PHY
> + select OMAP_CONTROL_USB
The original OMAP_USB3 config had this
depends on ARCH_OMAP2PLUS || COMPILE_TEST
You need the 'depends on ARCH_OMAP2PLUS' else you'll risk build failure on non
OMAP
platforms, because OMAP_CONTROL_USB depends on ARCH_OMAP2PLUS.
> + help
> + Enable this to support the PIPE3 PHY that is part of TI SOCs. This
> + driver takes care of all the PHY functionality apart from comparator.
> + This driver interacts with the "OMAP Control PHY Driver" to power
> + on/off the PHY.
> +
> config TWL4030_USB
> tristate "TWL4030 USB Transceiver Driver"
> depends on TWL4030_CORE && REGULATOR_TWL4030 && USB_MUSB_OMAP2PLUS
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 0dd8a98..42ccab4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,4 +4,5 @@
>
> obj-$(CONFIG_GENERIC_PHY)+= phy-core.o
> obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
> +obj-$(CONFIG_TI_PIPE3) += phy-ti-pipe3.o
> obj-$(CONFIG_TWL4030_USB)+= phy-twl4030-usb.o
> diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/phy/phy-ti-pipe3.c
> similarity index 57%
> rename from drivers/usb/phy/phy-omap-usb3.c
> rename to drivers/phy/phy-ti-pipe3.c
> index 0c6ba29..31e8397 100644
> --- a/drivers/usb/phy/phy-omap-usb3.c
> +++ b/drivers/phy/phy-ti-pipe3.c
> @@ -1,5 +1,5 @@
> /*
> - * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP.
> + * phy-ti-pipe3 - PIPE3 PHY driver for SATA, USB and PCIE.
SATA and PCIe is not yet supported so no need to mention about it now.
> *
> * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> * This program is free software; you can redistribute it and/or modify
> @@ -19,7 +19,8 @@
> #include
> #include
> #include
> -#include
> +#include
> +#include
> #include
> #include
> #include
> @@ -52,17 +53,17 @@
>
> /*
> * This is an Empirical value that works, need to confirm the actual
> - * value required for the USB3PHY_PLL_CONFIGURATION2.PLL_IDLE status
> - * to be correctly reflected in the USB3PHY_PLL_STATUS register.
> + * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status
> + * to be correctly reflected in the PIPE3PHY_PLL_STATUS register.
> */
> # define PLL_IDLE_TIME 100;
>
> -struct usb_dpll_map {
> +struct pipe3_dpll_map {
> unsigned long rate;
> - struct usb_dpll_params params;
> + struct pipe3_dpll_params params;
> };
>
> -static struct usb_dpll_map dpll_map[] = {
> +static struct pipe3_dpll_map dpll_map[] = {
> {1200, {1250, 5, 4, 20, 0} }, /* 12 MHz */
> {1680, {3125, 20, 4, 20, 0} }, /* 16.8 MHz */
> {1920, {1172, 8, 4, 20, 65537} }, /* 19.2 MHz */
> @@ -71,7 +72,7 @@ static struct usb_dpll_map dpll_map[] = {
> {3840, {3125, 47, 4, 20, 92843} }, /* 38.4 MHz */
> };
>
> -static struct usb_dpll_params *omap_usb3_get_dpll_params(unsigned long rate)
> +static struct pipe3_dpll_params *ti_pipe3_get_dpll_params(unsigned long rate)
> {
> int i;
>
> @@ -83,110 +84,113 @@ static struct usb_dpll_params
> *omap_usb3_get_dpll_params(unsigned long rate)
> return NULL;
> }
>
> -static int omap_usb3_suspend(struct usb_phy *x, int suspend)
> +static int ti_pipe3_power_off(struct phy *x)
> {
> - struct omap_usb *phy = phy_to_omapusb(x);
> - int val;
> + struct ti_pipe3 *phy = phy_get_drvdata(x);
> + int val;
> int timeout = PLL_IDLE_TIME;
>
> - if (suspend && !phy->is_suspended) {
> - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> - val |= PLL_IDLE;
> - omap_usb_write
device enumeration fail (only?) on ehci
Hello, I'm experiencing problems with our USB-CAN-hardware in a test scenario only when being attached to the ehci root-hub. When the device is enumerated and has its USB address the driver is bound and the can interface is started. So far so good. If there is a status request timeout the module firmware disconnects itself from the bus, thus reattaching to the bus, getting a new USB device address and so on... Doing this in a loops lead eventually to this error messages: > kernel: usb 1-1.1.6.1: new full-speed USB device number 21 using ehci-pci > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > kernel: usb 1-1.1.6.1: new full-speed USB device number 22 using ehci-pci > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > kernel: usb 1-1.1.6.1: new full-speed USB device number 23 using ehci-pci > kernel: usb 1-1.1.6.1: device not accepting address 23, error -110 > kernel: usb 1-1.1.6.1: new full-speed USB device number 24 using ehci-pci > kernel: usb 1-1.1.6.1: device not accepting address 24, error -110 > kernel: hub 1-1.1.6:1.0: unable to enumerate USB device on port 1 At this point the device never gets back to the bus itself. Either the whole hardware has to be power-cycled or the USB cable detached and attached again. The device is connected as follows (if done correctly) /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M |__ Port 6: Dev 14, If 0, Class=Hub, Driver=hub/1p, 480M |__ Port 1: Dev 16, If 0, Class=Vendor Specific Class, Driver=, 12M Now the funny thing: This doesn't occur if 1. The device with its internal hub (Dev 14 above) is attached to xHCI host 2. There is an Full-Speed-Hub before the device 3. The device is attached to a OHCI host The error occurs about 3-6 min after start while the test cases where it didn't happen run for longer than 1 hour. So the problem seems to occur only when the hub is connected with high-speed to any hub on an ehci bus. Has somebody an idea what might have gone wrong here? It seems the internal hub has problems with Transaction Translators. But why does this only happen when attached to ehci? Is this maybe a driver problem? I did some tests on 3.11.4 and some on 3.10.7 and both showed same results. Best regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
> Hi,
>
> On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
>> Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
>> power_on and power_off the following APIs are used phy_init(), phy_exit(),
>> phy_power_on() and phy_power_off().
>>
>> However using the old USB phy library wont be removed till the PHYs of all
>> other SoC's using dwc3 core is adapted to the Generic PHY Framework.
>>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
>> drivers/usb/dwc3/Kconfig |1 +
>> drivers/usb/dwc3/core.c| 52
>>
>> drivers/usb/dwc3/core.h|7
>> drivers/usb/dwc3/platform_data.h |2 +
>> 5 files changed, 66 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index e807635..471366d 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -6,11 +6,13 @@ Required properties:
>> - compatible: must be "snps,dwc3"
>> - reg : Address and length of the register set for the device
>> - interrupts: Interrupts used by the dwc3 controller.
>> +
>> +Optional properties:
>> - usb-phy : array of phandle for the PHY device. The first element
>> in the array is expected to be a handle to the USB2/HS PHY and
>> the second element is expected to be a handle to the USB3/SS PHY
>> -
>> -Optional properties:
>> + - phys: from the *Generic PHY* bindings
>> + - phy-names: from the *Generic PHY* bindings
>> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>>
>> This is usually a subnode to DWC3 glue to which it is connected.
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index 8e385b4..64eed6f 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -2,6 +2,7 @@ config USB_DWC3
>> tristate "DesignWare USB3 DRD Core Support"
>> depends on (USB || USB_GADGET) && HAS_DMA
>> select USB_PHY
>> +select GENERIC_PHY
>> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>> help
>>Say Y or M here if your system has a Dual Role SuperSpeed
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index cb91d70..28bfa5b 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>>
>> usb_phy_init(dwc->usb2_phy);
>> usb_phy_init(dwc->usb3_phy);
>> +
>> +if (dwc->usb2_generic_phy)
>> +phy_init(dwc->usb2_generic_phy);
>> +if (dwc->usb3_generic_phy)
>> +phy_init(dwc->usb3_generic_phy);
>> +
>
> dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
> dwc3_probe() but before the PHYs are initialized.
>
> This will cause phy power_on() to be called before phy_init() which is wrong.
>
>> mdelay(100);
>>
>> /* Clear USB3 PHY reset */
>> @@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
>> {
>> usb_phy_shutdown(dwc->usb2_phy);
>> usb_phy_shutdown(dwc->usb3_phy);
>> +
>> +if (dwc->usb2_generic_phy)
>> +phy_power_off(dwc->usb2_generic_phy);
>> +if (dwc->usb3_generic_phy)
>> +phy_power_off(dwc->usb3_generic_phy);
>> }
>>
>> #define DWC3_ALIGN_MASK (16 - 1)
>> @@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
>> dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>> }
>>
>> +count = of_property_match_string(node, "phy-names", "usb2-phy");
>> +if (count >= 0 || (pdata && pdata->usb2_generic_phy)) {
>> +dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>> +if (IS_ERR(dwc->usb2_generic_phy)) {
>> +dev_err(dev, "no usb2 phy configured yet");
>> +return PTR_ERR(dwc->usb2_generic_phy);
>> +}
>> +dwc->usb2_phy = NULL;
>> +}
>> +
>> +count = of_property_match_string(node, "phy-names", "usb3-phy");
>> +if (count >= 0 || (pdata && pdata->usb3_generic_phy)) {
>> +dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>> +if (IS_ERR(dwc->usb3_generic_phy)) {
>> +dev_err(dev, "no usb3 phy configured yet");
>> +return PTR_ERR(dwc->usb3_generic_phy);
>> +}
>> +dwc->usb3_phy = NULL;
>> +}
>> +
>
> There are a couple of issues here.
> - The above code must be executed only if it node is valid.
of_property_match_string will give a error value if the node is not present.
*count >= 0* can take care of this.
> - We must either get both PHYs via old method or via new method and not
> support mix matching
> them. e.g. it is p
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
On 10/16/2013 04:52 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
>> Hi,
>>
>> On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
>>> Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
>>> power_on and power_off the following APIs are used phy_init(), phy_exit(),
>>> phy_power_on() and phy_power_off().
>>>
>>> However using the old USB phy library wont be removed till the PHYs of all
>>> other SoC's using dwc3 core is adapted to the Generic PHY Framework.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I
>>> ---
>>> Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
>>> drivers/usb/dwc3/Kconfig |1 +
>>> drivers/usb/dwc3/core.c| 52
>>>
>>> drivers/usb/dwc3/core.h|7
>>> drivers/usb/dwc3/platform_data.h |2 +
>>> 5 files changed, 66 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index e807635..471366d 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -6,11 +6,13 @@ Required properties:
>>> - compatible: must be "snps,dwc3"
>>> - reg : Address and length of the register set for the device
>>> - interrupts: Interrupts used by the dwc3 controller.
>>> +
>>> +Optional properties:
>>> - usb-phy : array of phandle for the PHY device. The first element
>>> in the array is expected to be a handle to the USB2/HS PHY and
>>> the second element is expected to be a handle to the USB3/SS PHY
>>> -
>>> -Optional properties:
>>> + - phys: from the *Generic PHY* bindings
>>> + - phy-names: from the *Generic PHY* bindings
>>> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>>>
>>> This is usually a subnode to DWC3 glue to which it is connected.
>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>> index 8e385b4..64eed6f 100644
>>> --- a/drivers/usb/dwc3/Kconfig
>>> +++ b/drivers/usb/dwc3/Kconfig
>>> @@ -2,6 +2,7 @@ config USB_DWC3
>>> tristate "DesignWare USB3 DRD Core Support"
>>> depends on (USB || USB_GADGET) && HAS_DMA
>>> select USB_PHY
>>> + select GENERIC_PHY
>>> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>>> help
>>> Say Y or M here if your system has a Dual Role SuperSpeed
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index cb91d70..28bfa5b 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>>>
>>> usb_phy_init(dwc->usb2_phy);
>>> usb_phy_init(dwc->usb3_phy);
>>> +
>>> + if (dwc->usb2_generic_phy)
>>> + phy_init(dwc->usb2_generic_phy);
>>> + if (dwc->usb3_generic_phy)
>>> + phy_init(dwc->usb3_generic_phy);
>>> +
>>
>> dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
>> dwc3_probe() but before the PHYs are initialized.
>>
>> This will cause phy power_on() to be called before phy_init() which is wrong.
>>
You overlooked the above?
>>> mdelay(100);
>>>
>>> /* Clear USB3 PHY reset */
>>> @@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
>>> {
>>> usb_phy_shutdown(dwc->usb2_phy);
>>> usb_phy_shutdown(dwc->usb3_phy);
>>> +
>>> + if (dwc->usb2_generic_phy)
>>> + phy_power_off(dwc->usb2_generic_phy);
>>> + if (dwc->usb3_generic_phy)
>>> + phy_power_off(dwc->usb3_generic_phy);
>>> }
>>>
>>> #define DWC3_ALIGN_MASK(16 - 1)
>>> @@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
>>> dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>> }
>>>
>>> + count = of_property_match_string(node, "phy-names", "usb2-phy");
>>> + if (count >= 0 || (pdata && pdata->usb2_generic_phy)) {
>>> + dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>>> + if (IS_ERR(dwc->usb2_generic_phy)) {
>>> + dev_err(dev, "no usb2 phy configured yet");
>>> + return PTR_ERR(dwc->usb2_generic_phy);
>>> + }
>>> + dwc->usb2_phy = NULL;
>>> + }
>>> +
>>> + count = of_property_match_string(node, "phy-names", "usb3-phy");
>>> + if (count >= 0 || (pdata && pdata->usb3_generic_phy)) {
>>> + dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>>> + if (IS_ERR(dwc->usb3_generic_phy)) {
>>> + dev_err(dev, "no usb3 phy configured yet");
>>> + return PTR_ERR(dwc->usb3_generic_phy);
>>> + }
>>> + dwc->usb3_phy = NULL;
>>> + }
>>> +
>>
>> There are a couple of issues here.
>> - The above code must be executed only if it node is valid.
>
> of_property_match_string will give a error value if the node is not pre
Re: [PATCH] ehci: remove old TT sched code
On Tue, 15 Oct 2013, Dan Streetman wrote: > This removes the old EHCI transaction translator scheduling code, > leaving only the "new" (now over 7 years old) scheduling code. > > --- > > I think it's been long enough to prove the "new" tt sched > code works, and we can drop the old stuff. Alan, Greg, > and reason to keep the old tt sched code? > > This patch is against usb-next branch. I'm not so sure it's a good idea to do this now. I rather suspect there are people who still do use the "old" scheduler. Until the "new" scheduler is fixed up to work properly, I prefer to keep both options available. 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: ohci: remove ep93xx bus glue platform driver
On Tue, 15 Oct 2013, Hartley Sweeten wrote: > > This is definitely not a show-stopper in any way, but since this is > > just standard clock management, could you even move these clock ops > > into the driver? Are any other platforms already doing similar things > > so you could remove code from their platform that way as well, per > > chance? > > It does not appear any of the other users of the ohci-platform driver do > anything similar. > > The clock ops could be moved into the driver but I will need to add a > flag or something to the usb_ohci_pdata so that the platform can > indicated that a clock is required and the clock ops should be done. Adding a new entry to usb_ohci_pdata would be acceptable. However, I'm doubtful about how generic such a change would be. Some of the platform drivers don't use any clocks, and others use more than one. For now, it seems best to keep such dependencies in the platform-specific code. 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-storage: scsiglue: Changing the command result
On Wed, 16 Oct 2013, Ming Lei wrote: > > Of course the SCSI midlayer has decided to abort. That's the only way > > this bit can get set. But usb-storage doesn't know why SCSI decided to > > abort. > > usb-storage may know if it is caused by timeout via .eh_timed_out callback > if it wants to know. Ah, good point. Vishal, since the only way an abort can occur is when the .eh_timeout_out callback runs, I think it makes sense to set the result code to DID_TIME_OUT instead of DID_ABORT in all cases. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: hub: Clear Port Reset Change during init/resume
On Tue, 15 Oct 2013, Julius Werner wrote:
> This patch adds the Port Reset Change flag to the set of bits that are
> preemptively cleared on init/resume of a hub. In theory this bit should
> never be set unexpectedly... in practice it can still happen if BIOS,
> SMM or ACPI code plays around with USB devices without cleaning up
> correctly. This is especially dangerous for XHCI root hubs, which don't
> generate any more Port Status Change Events until all change bits are
> cleared, so this is a good precaution to have (similar to how it's
> already done for the Warm Port Reset Change flag).
>
> Signed-off-by: Julius Werner
> ---
> drivers/usb/core/hub.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index e6b682c..c3dd64c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1130,6 +1130,11 @@ static void hub_activate(struct usb_hub *hub, enum
> hub_activation_type type)
> usb_clear_port_feature(hub->hdev, port1,
> USB_PORT_FEAT_C_ENABLE);
> }
> + if (portchange & USB_PORT_STAT_C_RESET) {
> + need_debounce_delay = true;
> + usb_clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_RESET);
> + }
> if ((portchange & USB_PORT_STAT_C_BH_RESET) &&
> hub_is_superspeed(hub->hdev)) {
> need_debounce_delay = true;
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 v2] usb: ohci: remove ep93xx bus glue platform driver
On Wed, 16 Oct 2013, Hartley Sweeten wrote: > On Tuesday, October 15, 2013 5:57 PM, Ryan Mallon wrote: > > Gah, defconfig changes are a pest. I assume the other removed options > > are implicitly selected, and their removal is the result of > > re-minimising the defconfig? Can you put a note about this is the > > changelog please. > > Yah... It's a mess. > > All I did was: > make ARCH=arm ep93xx_defconfig > make ARCH=arm menuconfig # enable USB_OHCI_HCD_PLATFORM > make ARCH=arm savedefconfig > mv defconfig arch/arm/configs/ep93xx_defconfig > > Looks like the last time the ep93xx_defconfig was refreshed was: > > commit 07a8c03f3e06129e847acd068b8b89c13357ee64 > Author: Uwe Kleine-König > Date: Thu Jun 10 07:12:18 2010 +0200 > > ARM: reduce defconfigs > > Hence the ugly changes. It's awfully hard to believe that enabling USB_OHCI_HCD_PLATFORM is responsible for all those other changes. Maybe this should be done in two stages. First create a patch that describes the differences resulting from: make ARCH=arm ep93xx_defconfig make ARCH=arm savedefconfig mv defconfig arch/arm/configs/ep93xx_defconfig Then do this patch on top of that one. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: ohci-pxa27x: add regulator support
On Wed, 16 Oct 2013, Nikita Kiryanov wrote:
> Add regulator support for usb so that it would be possible to turn
> it on and off (also for suspend/resume).
>
> Cc: Alan Stern
> Cc: Greg Kroah-Hartman
> Signed-off-by: Nikita Kiryanov
> Signed-off-by: Igor Grinberg
> @@ -413,10 +420,16 @@ int usb_hcd_pxa27x_probe (const struct hc_driver
> *driver, struct platform_device
> ohci->dev = &pdev->dev;
> ohci->clk = usb_clk;
> ohci->mmio_base = (void __iomem *)hcd->regs;
> + ohci->usb_regulator = regulator_get(&pdev->dev, "vcc usb");
> + if (IS_ERR(ohci->usb_regulator)) {
> + pr_err("could not obtain usb regulator");
dev_err(), please.
> + retval = PTR_ERR(ohci->usb_regulator);
> + goto err3;
> + }
>
> if ((retval = pxa27x_start_hc(ohci, &pdev->dev)) < 0) {
> pr_debug("pxa27x_start_hc failed");
> - goto err3;
> + goto err4;
> }
>
> /* Select Power Management Mode */
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: Scheduling of interrupt transfer delayed on ehci when interval not 1?
On Tue, 15 Oct 2013, Marcus Overhagen wrote: > On Tue, Oct 15, 2013 at 6:07 PM, Alan Stern wrote: > > > Why does the driver make this mistake? > > My initial assumption was wrong, the driver doesn't infrequently poll, but > does so more often. I was misled by the confusing source. > However, I now made usbmon traces and analyzed them with LibreOffice. > > During data transfer, the driver does a interrupt transfer about 48ms after > the last one, and has to wait about 15ms for the result. But every 40 > transfers, > it executes 3 transfers without waiting inbetween and has to wait 63ms for the > result. This is with transfer interval set to 10 on xhci. > I also compiled the driver with transfer interval set to 1 (the > original setting before > my patch [1]), but the result is similar, an interrupt transfer every > 44ms, and waiting > of 19ms for the result. I don't have ehci hardware to test it. > > When the URB completition callback is executed, it calles complete(), > while the driver > is using wait_for_completion_interruptible_timeout() for this signal > with 100ms timeout > (originally that was 50ms but I changed it in patch [2] to make it > work with xhci) > > This function seems to timeout on ehci hardware after about 700MB to > 7GB transfered. > > I guess that the thread isn't scheduled fast enough under load, although the > USB > transfer was successful. That could be the explanation. It would also explain why increasing the timeout fixes the problem. > > bInterval = 10 means the polling period is 64 ms (for a high-speed > > device). So a timeout of 100 ms should be adequate -- provided the > > device always sends data over the interrupt endpoint without any > > further delay. > > What i do not understand: with ehci hardware, the driver works when > interval in the urb is set to 1 and the timeout is 50ms. > > With xhci, the urb appears to be scheduled not more often then every > 64ms even if the interval in the urb is set to 1. This is because ehci-hcd pays attention to urb->interval whereas xhci-hcd ignores it. > I'm just looking for the correct solution for this issue. Here's a suggestion: Have the URB's completion routine set a new flag somewhere. When the wait_for_completion_interruptible_timeout() call returns, check whether the new flag is set instead of checking whether a timeout occurred. 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 2/2] usb: ohci-pxa27x: add regulators for usb devices
On Wed, 16 Oct 2013, Nikita Kiryanov wrote:
> Add regulator support for devices that sit on USB ports,
> thus allowing usb devices power management for suspend/resume.
>
> Cc: Alan Stern
> Cc: Greg Kroah-Hartman
> Signed-off-by: Nikita Kiryanov
> Signed-off-by: Igor Grinberg
> ---
> drivers/usb/host/ohci-pxa27x.c | 47
> ++
> 1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index 35e739e..ae4c64f 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -109,6 +109,7 @@ struct pxa27x_ohci {
> struct device *dev;
> struct clk *clk;
> struct regulator *usb_regulator;
> + struct regulator *ext_regulator[PXA_UHC_MAX_PORTNUM];
> void __iomem*mmio_base;
> };
>
> @@ -217,7 +218,7 @@ extern void pxa27x_clear_otgph(void);
>
> static int pxa27x_start_hc(struct pxa27x_ohci *ohci, struct device *dev)
> {
> - int retval = 0;
> + int i, retval = 0;
> struct pxaohci_platform_data *inf;
> uint32_t uhchr;
>
> @@ -227,6 +228,16 @@ static int pxa27x_start_hc(struct pxa27x_ohci *ohci,
> struct device *dev)
> if (retval)
> return retval;
>
> + for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
> + retval = 0;
> + if (ohci->ext_regulator[i])
> + retval = regulator_enable(ohci->ext_regulator[i]);
> +
> + if (retval)
> + pr_err("%s: regulator enable failed: port%d, err=%d\n",
> +__func__, i, retval);
You could get rid of the "retval = 0;" line if you wrote this as:
if (ohci->ext_regulator[i]) {
retval = regulator_enable(...);
if (retval)
dev_err(dev, ...)
}
Note: dev_err, not pr_err.
What happens to ext_regulator[0] if you fail to enable
ext_regulator[1]? Does it remain permanently enabled?
> + }
> +
> clk_prepare_enable(ohci->clk);
>
> pxa27x_reset_hc(ohci);
> @@ -257,8 +268,22 @@ static int pxa27x_start_hc(struct pxa27x_ohci *ohci,
> struct device *dev)
> return 0;
> }
>
> +static void ohci_regulator_put_all(struct pxa27x_ohci *ohci)
> +{
> + int i;
> +
> + for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
> + if (ohci->ext_regulator[i])
> + regulator_put(ohci->ext_regulator[i]);
> + }
> +
> + /* usb regulator should be present if we get here */
> + regulator_put(ohci->usb_regulator);
> +}
What happens if you call regulator_put() for an enabled regulator?
How come you have an ohci_regulator_put_all() routine but not an
ohci_regulator_disable_all() routine?
And likewise for _get_ and _enable_?
> +
> static void pxa27x_stop_hc(struct pxa27x_ohci *ohci, struct device *dev)
> {
> + int i;
> struct pxaohci_platform_data *inf;
> uint32_t uhccoms;
>
> @@ -278,6 +303,12 @@ static void pxa27x_stop_hc(struct pxa27x_ohci *ohci,
> struct device *dev)
> udelay(10);
>
> clk_disable_unprepare(ohci->clk);
> +
> + for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
> + if (ohci->ext_regulator[i])
> + regulator_disable(ohci->ext_regulator[i]);
> + }
> +
> regulator_disable(ohci->usb_regulator);
> }
>
> @@ -360,12 +391,13 @@ static int ohci_pxa_of_init(struct platform_device
> *pdev)
> */
> int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct
> platform_device *pdev)
> {
> - int retval, irq;
> + int retval, irq, i;
> struct usb_hcd *hcd;
> struct pxaohci_platform_data *inf;
> struct pxa27x_ohci *ohci;
> struct resource *r;
> struct clk *usb_clk;
> + char supply[7];
>
> retval = ohci_pxa_of_init(pdev);
> if (retval)
> @@ -427,6 +459,13 @@ int usb_hcd_pxa27x_probe (const struct hc_driver
> *driver, struct platform_device
> goto err3;
> }
>
> + for (i = 0; i < PXA_UHC_MAX_PORTNUM; i++) {
> + snprintf(supply, sizeof(supply), "fsusb%d", i);
> + ohci->ext_regulator[i] = regulator_get(&pdev->dev, supply);
> + if (IS_ERR(ohci->ext_regulator[i]))
> + ohci->ext_regulator[i] = NULL;
Shouldn't this be a fatal error?
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 0/2] Add regulator support for pxa27x ohci and devices
On Wed, 16 Oct 2013, Nikita Kiryanov wrote: > This patchset adds regulator support for pxa27x ohci to make it possible > to turn the ohci controller and its usb devices on/off during probe/remove, > as well as resume/suspend. > > Cc: Alan Stern > Cc: Greg Kroah-Hartman > Cc: Igor Grinberg > > Nikita Kiryanov (2): > usb: ohci-pxa27x: add regulator support > usb: ohci-pxa27x: add regulators for usb devices These two patches do essentially the same thing: Add regulator support to ohci-pxa27x. As far as I'm concerned there's no reason to keep them as separate patches; you might as well combine them into a single patch. One other thing: A different patch affecting ohci-pxa27x.c was submitted about a month ago: http://marc.info/?l=linux-usb&m=137976179009631&w=2 It hasn't been merged yet, but I would like your patches to wait until it has been accepted. When that happens, you will have to rebase your changes. 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: device enumeration fail (only?) on ehci
On Wed, 16 Oct 2013, Alexander Stein wrote: > Hello, > > I'm experiencing problems with our USB-CAN-hardware in a test scenario only > when being attached to the ehci root-hub. > When the device is enumerated and has its USB address the driver is bound and > the can interface is started. So far so good. If there is a status request > timeout the module firmware disconnects itself from the bus, thus reattaching > to the bus, getting a new USB device address and so on... > Doing this in a loops lead eventually to this error messages: Do you have any idea why a status request timeout would occur? Can you capture a usbmon trace showing one of these timeouts and the following errors? > > kernel: usb 1-1.1.6.1: new full-speed USB device number 21 using ehci-pci > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > kernel: usb 1-1.1.6.1: new full-speed USB device number 22 using ehci-pci > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > kernel: usb 1-1.1.6.1: new full-speed USB device number 23 using ehci-pci > > kernel: usb 1-1.1.6.1: device not accepting address 23, error -110 > > kernel: usb 1-1.1.6.1: new full-speed USB device number 24 using ehci-pci > > kernel: usb 1-1.1.6.1: device not accepting address 24, error -110 > > kernel: hub 1-1.1.6:1.0: unable to enumerate USB device on port 1 > > At this point the device never gets back to the bus itself. Either the whole > hardware has to be power-cycled or the USB cable detached and attached again. > > The device is connected as follows (if done correctly) > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M > |__ Port 6: Dev 14, If 0, Class=Hub, Driver=hub/1p, 480M > |__ Port 1: Dev 16, If 0, Class=Vendor Specific Class, Driver=, > 12M > > Now the funny thing: This doesn't occur if > 1. The device with its internal hub (Dev 14 above) is attached to xHCI host > 2. There is an Full-Speed-Hub before the device > 3. The device is attached to a OHCI host > > The error occurs about 3-6 min after start while the test cases where it > didn't happen run for longer than 1 hour. > > So the problem seems to occur only when the hub is connected with high-speed > to any hub on an ehci bus. > Has somebody an idea what might have gone wrong here? It seems the internal > hub has problems with Transaction Translators. But why does this only happen > when attached to ehci? Is this maybe a driver problem? > I did some tests on 3.11.4 and some on 3.10.7 and both showed same results. I doubt that it is a driver problem. Much more likely is a bug in the internal hub, as you guessed. This doesn't explain why it works under xHCI but not under EHCI. Do you not get any status request timeouts under xHCI? 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
Advertencia Final.
Su contraseña caducará en 3 días formulario llenar y enviar de inmediato para validar su dirección de e-mail. Nombre de Usuario: . Contraseña anterior: . Nueva Contraseña: gracias administrador del sistema -- 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: [Pull Request] xHCI bug fixes for 3.12 (Link PM and misc)
On Tue, Oct 15, 2013 at 01:09:16PM -0700, Greg Kroah-Hartman wrote: > On Tue, Oct 15, 2013 at 12:44:08PM -0700, Sarah Sharp wrote: > > > So I'd like to take these for 3.13-rc1, and if they are fixes, take them > > > into the 3.12-stable tree (and older ones) when they hit Linus's tree > > > then. > > > > Ok, fine with me. Just to be clear though: are you asking me to delay > > these "long-standing" bugs because we're now at -rc5, or do you always > > want me to delay them for the next kernel release? > > Because we are at -rc5. Maybe something like this could go into -rc2 > and possibly -rc3, but come on, -rc5? Thank you for clarifying the line of where you cut off non-regression pulls for your usb-linus branch. I will make my future pull requests match that. > Look at all of the pushback Linus > has been giving maintainers on lkml, this shouldn't be news. I'm sorry for missing those emails; I don't normally keep up with LKML. I will strive to pay attention to future email Linus sends. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 01/17] ARM: at91: move at91_pmc.h to include/linux/clk/at91_pmc.h
On 11/10/2013 09:37, Boris BREZILLON :
This patch moves at91_pmc.h header from machine specific directory
(arch/arm/mach-at91/include/mach/at91_pmc.h) to clk include directory
(include/linux/clk/at91_pmc.h).
We need this to avoid reference to machine specific headers in clk
drivers.
Signed-off-by: Boris BREZILLON
Acked-by: Nicolas Ferre
---
arch/arm/mach-at91/at91rm9200.c|2 +-
arch/arm/mach-at91/at91sam9260.c |2 +-
arch/arm/mach-at91/at91sam9261.c |2 +-
arch/arm/mach-at91/at91sam9263.c |2 +-
arch/arm/mach-at91/at91sam9g45.c |2 +-
arch/arm/mach-at91/at91sam9n12.c |2 +-
arch/arm/mach-at91/at91sam9rl.c|2 +-
arch/arm/mach-at91/at91sam9x5.c|2 +-
arch/arm/mach-at91/clock.c |2 +-
arch/arm/mach-at91/pm.c|2 +-
arch/arm/mach-at91/pm_slowclock.S |2 +-
arch/arm/mach-at91/sama5d3.c |2 +-
arch/arm/mach-at91/setup.c |2 +-
drivers/usb/gadget/atmel_usba_udc.c|2 +-
.../include/mach => include/linux/clk}/at91_pmc.h |2 +-
15 files changed, 15 insertions(+), 15 deletions(-)
rename {arch/arm/mach-at91/include/mach => include/linux/clk}/at91_pmc.h (99%)
diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
index 4aad93d..102dac6 100644
--- a/arch/arm/mach-at91/at91rm9200.c
+++ b/arch/arm/mach-at91/at91rm9200.c
@@ -12,13 +12,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
-#include
#include
#include
diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c
index 5de6074..16d7817 100644
--- a/arch/arm/mach-at91/at91sam9260.c
+++ b/arch/arm/mach-at91/at91sam9260.c
@@ -11,6 +11,7 @@
*/
#include
+#include
#include
#include
@@ -20,7 +21,6 @@
#include
#include
#include
-#include
#include "at91_aic.h"
#include "at91_rstc.h"
diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
index 0e07932..c8f5958 100644
--- a/arch/arm/mach-at91/at91sam9261.c
+++ b/arch/arm/mach-at91/at91sam9261.c
@@ -11,6 +11,7 @@
*/
#include
+#include
#include
#include
@@ -19,7 +20,6 @@
#include
#include
#include
-#include
#include "at91_aic.h"
#include "at91_rstc.h"
diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
index 6ce7d18..cd8ad95 100644
--- a/arch/arm/mach-at91/at91sam9263.c
+++ b/arch/arm/mach-at91/at91sam9263.c
@@ -11,6 +11,7 @@
*/
#include
+#include
#include
#include
@@ -18,7 +19,6 @@
#include
#include
#include
-#include
#include "at91_aic.h"
#include "at91_rstc.h"
diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index 474ee04..9794b20 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -12,13 +12,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
-#include
#include
#include "at91_aic.h"
diff --git a/arch/arm/mach-at91/at91sam9n12.c b/arch/arm/mach-at91/at91sam9n12.c
index c7d670d..5b83c66 100644
--- a/arch/arm/mach-at91/at91sam9n12.c
+++ b/arch/arm/mach-at91/at91sam9n12.c
@@ -8,12 +8,12 @@
#include
#include
+#include
#include
#include
#include
#include
-#include
#include
#include "board.h"
diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index d4ec0d9..947da95 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -10,6 +10,7 @@
*/
#include
+#include
#include
#include
@@ -19,7 +20,6 @@
#include
#include
#include
-#include
#include "at91_aic.h"
#include "at91_rstc.h"
diff --git a/arch/arm/mach-at91/at91sam9x5.c b/arch/arm/mach-at91/at91sam9x5.c
index 916e5a1..a930d73 100644
--- a/arch/arm/mach-at91/at91sam9x5.c
+++ b/arch/arm/mach-at91/at91sam9x5.c
@@ -8,12 +8,12 @@
#include
#include
+#include
#include
#include
#include
#include
-#include
#include
#include "board.h"
diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c
index 6b2630a..5f02aea 100644
--- a/arch/arm/mach-at91/clock.c
+++ b/arch/arm/mach-at91/clock.c
@@ -24,9 +24,9 @@
#include
#include
#include
+#include
#include
-#include
#include
#include
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 15afb5d..692cacd 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -19,13 +19,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-#include
#include
#include "at91_aic.h"
diff --git a/arch/arm/mach-at91/pm_slowclock.S
b/arch/arm/mach-at91/pm_slowclock.S
index 098c28d..2001
Re: [PATCH v4 02/17] ARM: at91: add Kconfig options for common clk support
On 11/10/2013 09:37, Boris BREZILLON : This patch adds the following Kconfig options to prepare the transition to common clk framework: - AT91_USE_OLD_CLK: this option is selected by every SoC which does not support new at91 clks based on common clk framework (SoC which does not define the clock tree in its device tree). This options is also selected when the user choose non dt boards support (new at91 clks can only be registered from a device tree definition). - COMMON_CLK_AT91: this option cannot be selected directly. Instead it is enabled if these 3 conditions are met: * at least one of the selected SoCs have a PMC (Power Management Controller) Unit * device tree support is enabled * the old at91 clk implementation is disabled (every selected SoC define its clks in its device tree and non dt boards support is disabled) - OLD_CLK_AT91: this option cannot be selected directly. Instead it is enabled if these 2 conditions are met: * at least one of the selected SoCs have a PMC (Power Management Controller) Unit * at least one of the selected SoCs does not define its clks in its device tree or non dt-boards support is enabled This patch selects AT91_USE_OLD_CLK in all currently supported SoCs. These selects will be removed after clk definitions are properly added in each soc's device tree. It also selects AT91_USE_OLD_CLK in all non-dt boards support. AT91_PMC_UNIT references are replaced by OLD_CLK_AT91, because PMC Unit is enabled for both old and common clk implementations, and old clk implementation should not be compiled if COMMON_CLK is enabled. To avoid future link errors, a new stub is created for at91_dt_clock_init function if OLD_CLK_AT91 is disabled. A new check is added in dt init functions (setup.c) to prepare for SoCs supporting new clk implementation. These SoCs won't setup the register_clocks callback (clk registration is done using of_clk_init). Signed-off-by: Boris BREZILLON It seems that all cases are covered for a smooth transition to common clock framework. Acked-by: Nicolas Ferre Thanks --- arch/arm/mach-at91/Kconfig| 21 + arch/arm/mach-at91/Kconfig.non_dt |6 ++ arch/arm/mach-at91/Makefile |2 +- arch/arm/mach-at91/generic.h |3 ++- arch/arm/mach-at91/setup.c|6 -- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig index 699b71e..85b53a4 100644 --- a/arch/arm/mach-at91/Kconfig +++ b/arch/arm/mach-at91/Kconfig @@ -6,10 +6,22 @@ config HAVE_AT91_DBGU0 config HAVE_AT91_DBGU1 bool +config AT91_USE_OLD_CLK + bool + config AT91_PMC_UNIT bool default !ARCH_AT91X40 +config COMMON_CLK_AT91 + bool + default AT91_PMC_UNIT && USE_OF && !AT91_USE_OLD_CLK + select COMMON_CLK + +config OLD_CLK_AT91 + bool + default AT91_PMC_UNIT && AT91_USE_OLD_CLK + config AT91_SAM9_ALT_RESET bool default !ARCH_AT91X40 @@ -65,6 +77,7 @@ config SOC_SAMA5D3 select SOC_SAMA5 select HAVE_FB_ATMEL select HAVE_AT91_DBGU1 + select AT91_USE_OLD_CLK help Select this if you are using one of Atmel's SAMA5D3 family SoC. This support covers SAMA5D31, SAMA5D33, SAMA5D34, SAMA5D35. @@ -78,11 +91,13 @@ config SOC_AT91RM9200 select HAVE_AT91_DBGU0 select MULTI_IRQ_HANDLER select SPARSE_IRQ + select AT91_USE_OLD_CLK config SOC_AT91SAM9260 bool "AT91SAM9260, AT91SAM9XE or AT91SAM9G20" select HAVE_AT91_DBGU0 select SOC_AT91SAM9 + select AT91_USE_OLD_CLK help Select this if you are using one of Atmel's AT91SAM9260, AT91SAM9XE or AT91SAM9G20 SoC. @@ -92,6 +107,7 @@ config SOC_AT91SAM9261 select HAVE_AT91_DBGU0 select HAVE_FB_ATMEL select SOC_AT91SAM9 + select AT91_USE_OLD_CLK help Select this if you are using one of Atmel's AT91SAM9261 or AT91SAM9G10 SoC. @@ -100,18 +116,21 @@ config SOC_AT91SAM9263 select HAVE_AT91_DBGU1 select HAVE_FB_ATMEL select SOC_AT91SAM9 + select AT91_USE_OLD_CLK config SOC_AT91SAM9RL bool "AT91SAM9RL" select HAVE_AT91_DBGU0 select HAVE_FB_ATMEL select SOC_AT91SAM9 + select AT91_USE_OLD_CLK config SOC_AT91SAM9G45 bool "AT91SAM9G45 or AT91SAM9M10 families" select HAVE_AT91_DBGU1 select HAVE_FB_ATMEL select SOC_AT91SAM9 + select AT91_USE_OLD_CLK help Select this if you are using one of Atmel's AT91SAM9G45 family SoC. This support covers AT91SAM9G45, AT91SAM9G46, AT91SAM9M10 and AT91SAM9M11. @@ -121,6 +140,7 @@ config SOC_AT91SAM9X5 select HAVE_AT91_DBGU0 select HAVE_FB_ATMEL select SOC_AT91SAM9 + select AT91_USE_OLD_CLK help
Re: device enumeration fail (only?) on ehci
On Wednesday 16 October 2013 11:07:50, Alan Stern wrote: > On Wed, 16 Oct 2013, Alexander Stein wrote: > > > Hello, > > > > I'm experiencing problems with our USB-CAN-hardware in a test scenario only > > when being attached to the ehci root-hub. > > When the device is enumerated and has its USB address the driver is bound > > and the can interface is started. So far so good. If there is a status > > request timeout the module firmware disconnects itself from the bus, thus > > reattaching to the bus, getting a new USB device address and so on... > > Doing this in a loops lead eventually to this error messages: > > Do you have any idea why a status request timeout would occur? Can you > capture a usbmon trace showing one of these timeouts and the following > errors? I hope status request timeout does not interfere with some USB standard stuff. In my case this is a safety mechanism in case the host driver could not get the current status from the device. So the driver needs to poll from a specific endpoint once a second. There is no need for usbmon trace. This timeout is intentionally triggered within the test loop by simply removing the device driver without shutting down the CAN interfaces. Those dis-/reconnects are part of the problem I described. BTW: There is no shutdown callback for USB device drivers so this 'problem' even occurs during system reboot when the CAN interface is not shutdown beforehand. > > > kernel: usb 1-1.1.6.1: new full-speed USB device number 21 using ehci-pci > > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > > kernel: usb 1-1.1.6.1: new full-speed USB device number 22 using ehci-pci > > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > > kernel: usb 1-1.1.6.1: device descriptor read/64, error -110 > > > kernel: usb 1-1.1.6.1: new full-speed USB device number 23 using ehci-pci > > > kernel: usb 1-1.1.6.1: device not accepting address 23, error -110 > > > kernel: usb 1-1.1.6.1: new full-speed USB device number 24 using ehci-pci > > > kernel: usb 1-1.1.6.1: device not accepting address 24, error -110 > > > kernel: hub 1-1.1.6:1.0: unable to enumerate USB device on port 1 > > > > At this point the device never gets back to the bus itself. Either the > > whole hardware has to be power-cycled or the USB cable detached and > > attached again. > > > > The device is connected as follows (if done correctly) > > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M > > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M > > |__ Port 6: Dev 14, If 0, Class=Hub, Driver=hub/1p, 480M > > |__ Port 1: Dev 16, If 0, Class=Vendor Specific Class, Driver=, > > 12M > > > > Now the funny thing: This doesn't occur if > > 1. The device with its internal hub (Dev 14 above) is attached to xHCI host > > 2. There is an Full-Speed-Hub before the device > > 3. The device is attached to a OHCI host > > > > The error occurs about 3-6 min after start while the test cases where it > > didn't happen run for longer than 1 hour. > > > > So the problem seems to occur only when the hub is connected with > > high-speed to any hub on an ehci bus. > > Has somebody an idea what might have gone wrong here? It seems the internal > > hub has problems with Transaction Translators. But why does this only > > happen when attached to ehci? Is this maybe a driver problem? > > I did some tests on 3.11.4 and some on 3.10.7 and both showed same results. > > I doubt that it is a driver problem. Much more likely is a bug in the > internal hub, as you guessed. > > This doesn't explain why it works under xHCI but not under EHCI. Do > you not get any status request timeouts under xHCI? Those timeouts are provoked on purpose, so, yes, there also exist in the xHCI case. Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
On Wed, 16 Oct 2013, Victor Yeo wrote: > Hi, > > > The text capture is incomplete. It is missing lots of packets. In > > particular, it is missing all the packets between 202489 and 202502. > > The missing packets are NAK, I added the NAK after Set-Config setup > stage. I hide the NAK when i export the packet capture to text format. They can't all be NAKs. The device won't send a NAK unless the host sends an IN first. > > Also, I don't understand the "Dir H(S)" part of the capture lines. > > What do they mean? They are present on every packet. > > Dir stands for direction, H is high speed, S is super speed. I guessed that "Dir" stands for "direction". But which direction? It doesn't say whether the packet goes to the host or to the device -- it just says "Dir". If "H" stands for "High speed" and "S" stands for "SuperSpeed", then "H(S)" stands for "High speed(SuperSpeed)". What does that mean? Also, why does the analyzer log sometimes list the contents of a DATA0 or DATA1 packet, but other times just say "Data(8 bytes)"? By the way, you didn't mention this earlier, but the analyzer log you sent before shows a problem in packet 157241. The gadget should have sent a config descriptor, but it sent an empty data packet. > > This was never the issue. I'm sure the DATA1 packet of the Set-Config > > was sent before the Get-Config request. The real question is whether > > the DATA1 packet was sent before the Set-Config request had been fully > > processed. > > > > To get more information, try adding > > > > msleep(100); > > > > just before the final "return rc;" line in do_set_config(). We should > > be able to see in the analyzer log if this 100-ms delay is present. > > After i added msleep(100) just before final line in do_set_config(), > the USB enumeration fails to complete normally. What happens, exactly? I have asked you many times in the past to provide more debugging information. Without information, I can't help you. In this case, you should have provided the kernel log from the gadget and the analyzer log. > > Here's a second test you can try. In handle_exception(), the > > FSG_STATE_CONFIG_CHANGE case, comment out the > > > > ep0_queue(fsg); > > > > line. Without that line the Set-Config request should time out, > > because the gadget will never complete the status stage. If the > > request does complete normally, it will prove that the UDC driver isn't > > working right. > > After i comment out the ep0_queue(fsg) in FSG_STATE_CONFIG_CHANGE case > of handle_exception(), the request does complete, but takes more time > to complete. And UDC driver queue function is still being called after > the Set-Config request. Provide more information! Where does the UDC driver queue function get called from? Here's another experiment you can try. In do_set_config(), just after the "rc = do_set_interface(fsg, -1);" line, add a statement saying INFO(fsg, "fsg->config %d new_config %d\n", fsg->config, new_config); In standard_setup_req(), change the "VDBG(fsg, "get configuration\n");" line to INFO(fsg, "get configuration %d\n", fsg->config); Then let's see what shows up in the device's system log. 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: USB reset xhci_hcd for ELAN touchscreen
On Fri, Oct 04, 2013 at 11:39:47AM -0500, Drew Von Spreecken wrote: > Sarah, I've enabled debugging for the module if it helps... Log attached. Hi Drew, > On 09/30/2013 06:19 PM, Sarah Sharp wrote: > >Hi Drew! > > > >I'm the xHCI driver maintainer, and it helps to Cc me on issues with USB > >devices under xHCI hosts. > > > >On Sun, Sep 29, 2013 at 11:14:47AM -0500, Drew Von Spreecken wrote: > >>I have an Elan touchscreen that is causing USB resets and errors at > >>boot and intermittently during use. The touchscreen works as > >>expected but messages are still being logged to kern.log. > >> > >>It appears to get detected correctly: > >> > >>Bus 001 Device 002: ID 8087:8000 Intel Corp. > >>Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > >>Bus 003 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub > >>Bus 002 Device 013: ID 04f3:0089 Elan Microelectronics Corp. > >>Bus 002 Device 004: ID 2232:1049 > >>Bus 002 Device 003: ID 8087:07dc Intel Corp. > >>Bus 002 Device 017: ID 18d1:4ee2 Google Inc. Nexus 4 (debug) > >>Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > >> > >>The recurring output of dmesg: > >>[ 523.082262] usb 2-7: new full-speed USB device number 9 using xhci_hcd > >>[ 525.186961] usb 2-7: unable to read config index 0 descriptor/start: -71 > >>[ 525.186972] usb 2-7: can't read configurations, error -71 These errors look interesting: > Oct 3 21:37:46 8bit kernel: [3.861123] usb 1-7: unable to read config > index 0 descriptor/start: -71 > Oct 3 21:37:46 8bit kernel: [3.861129] usb 1-7: can't read > configurations, error -71 > Oct 3 21:37:46 8bit kernel: [4.026929] usb 1-7: new full-speed USB > device number 5 using xhci_hcd > Oct 3 21:37:46 8bit kernel: [6.060326] usb 1-7: unable to read config > index 0 descriptor/start: -71 > Oct 3 21:37:46 8bit kernel: [6.060331] usb 1-7: can't read > configurations, error -71 > Oct 3 21:37:46 8bit kernel: [6.225707] usb 1-7: new full-speed USB > device number 6 using xhci_hcd > Oct 3 21:37:46 8bit kernel: [8.259088] usb 1-7: unable to read config > index 0 descriptor/start: -71 > Oct 3 21:37:46 8bit kernel: [8.259092] usb 1-7: can't read > configurations, error -71 > Oct 3 21:37:46 8bit kernel: [8.424415] usb 1-7: new full-speed USB > device number 7 using xhci_hcd > Oct 3 21:37:46 8bit kernel: [8.586659] bio: create slab at 1 > Oct 3 21:37:46 8bit kernel: [ 10.457602] usb 1-7: unable to read config > index 0 descriptor/start: -71 > Oct 3 21:37:46 8bit kernel: [ 10.457607] usb 1-7: can't read > configurations, error -71 > Oct 3 21:37:46 8bit kernel: [ 10.457625] hub 1-0:1.0: unable to enumerate > USB device on port 7 The device isn't responding to the request for the device descriptors. It may be that it needs to use the "Windows" enumeration scheme. Can you test the following patch and see if it helps? http://marc.info/?l=linux-usb&m=138119329715367&w=2 At this point, I'm not even sure if the usb device 1-7 is the Elan touchscreen. It doesn't look like it from the lsusb output you provided (xHCI is on bus 2 and 3 in that output), but the bus numbers may have changed after you rebooted. But try the patch anyway, and see if it helps. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
xhci-hcd wakeup settings
Sarah: While working on some general fixes for wakeup races during PCI suspend, I noticed that xhci_pci_suspend() ignores its do_wakeup argument. What's the story? 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: [Application RFC PATCH] ptp-gadget: move from gadgetfs to functionfs
Hi Michael, On Wed, 16 Oct 2013, Michael Grzeschik wrote: > Hi Guennadi, > > On Tue, Jun 18, 2013 at 11:54:00AM +0200, Guennadi Liakhovetski wrote: > > On Mon, 17 Jun 2013, Michael Grzeschik wrote: > > > > > Hi Guennadi, > > > > > > On Thu, Jun 06, 2013 at 12:34:39AM +0200, Michael Grzeschik wrote: > > > > This patch moves the ptp-gadget to use functionfs for the endpoint > > > > handling. With functionfs, this patch is deleting a lot of control ep0 > > > > code, which is now handled by the framework. > > > > > > Ping! Do you know who is maintaining this code and/or > > > could be interested in this work? > > > > Yes, I'm still - formally - the maintainer of the code. Fortunately the > > patch rate is quite low, but still - I have little time to take care of > > them when they arrive, and if they're non-trivial, my expertise level in > > this area isn't high enough to process patches quickly. Also in this case > > to really review patch contents I would first have to study the > > functionfs. Besides I don't have a platform, on which I _easily_ could > > test the software, I would first have to identify, which one is best > > suitable. So, if someone wants to take over mainternership of this > > package, I wouldn't mind. Anatolij? Anyone@DENX? > > could you catch some attention for the project? I really would > like to find that patch on top of this stack. > > git://git.denx.de/ptp-gadget.git Since you seem to be the only person at the moment, interested in this project, would you like to take over its maintenance? The easiest would be for you to clone it to a server of your choice and continue development there. If you like, we can also try to pass over to you the original repository. What would be your preference? I'm adding Detlev Zundel to CC in case you'd like to continue working with the original tree. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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: musb: fix error path & allow to have everything built-in
Hi, On 10/16/2013 12:50 PM, Sebastian Andrzej Siewior wrote: > this is a new series of bug fixing: FWIW, I've tested this and your previous series in a host-only environment and it seems to work just fine. I'll rework my suspend patches on top of them. Thanks again, Daniel -- 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: device enumeration fail (only?) on ehci
On Wed, 16 Oct 2013, Alexander Stein wrote: > On Wednesday 16 October 2013 11:07:50, Alan Stern wrote: > > On Wed, 16 Oct 2013, Alexander Stein wrote: > > > > > Hello, > > > > > > I'm experiencing problems with our USB-CAN-hardware in a test scenario > > > only when being attached to the ehci root-hub. > > > When the device is enumerated and has its USB address the driver is bound > > > and the can interface is started. So far so good. If there is a status > > > request timeout the module firmware disconnects itself from the bus, thus > > > reattaching to the bus, getting a new USB device address and so on... > > > Doing this in a loops lead eventually to this error messages: > > > > Do you have any idea why a status request timeout would occur? Can you > > capture a usbmon trace showing one of these timeouts and the following > > errors? > > I hope status request timeout does not interfere with some USB > standard stuff. In my case this is a safety mechanism in case the > host driver could not get the current status from the device. So the > driver needs to poll from a specific endpoint once a second. I see. So the device itself forces the disconnect if it doesn't receive this status poll sufficiently often, right? > There is no need for usbmon trace. This timeout is intentionally > triggered within the test loop by simply removing the device driver > without shutting down the CAN interfaces. Those dis-/reconnects are > part of the problem I described. > > BTW: There is no shutdown callback for USB device drivers so this > 'problem' even occurs during system reboot when the CAN interface is > not shutdown beforehand. That's true. We could add a shutdown callback, if necessary. Or just unbind all the drivers. > > > So the problem seems to occur only when the hub is connected with > > > high-speed to any hub on an ehci bus. > > > Has somebody an idea what might have gone wrong here? It seems the > > > internal hub has problems with Transaction Translators. But why does this > > > only happen when attached to ehci? Is this maybe a driver problem? > > > I did some tests on 3.11.4 and some on 3.10.7 and both showed same > > > results. > > > > I doubt that it is a driver problem. Much more likely is a bug in the > > internal hub, as you guessed. > > > > This doesn't explain why it works under xHCI but not under EHCI. Do > > you not get any status request timeouts under xHCI? > > Those timeouts are provoked on purpose, so, yes, there also exist in the xHCI > case. But with xHCI and OHCI, the reconnect succeeds whereas with EHCI it fails? It's hard to guess why. The behavior under xHCI is slightly different from the other two, which behave the same. You can force EHCI and OHCI to work more like xHCI by doing: echo 1 >/sys/module/usbcore/parameters/old_scheme_first Maybe it's a data toggle issue involving the internal hub, but the only way to find out for sure is to use a bus analyzer. 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] ehci: remove old TT sched code
On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern wrote: > On Tue, 15 Oct 2013, Dan Streetman wrote: > >> This removes the old EHCI transaction translator scheduling code, >> leaving only the "new" (now over 7 years old) scheduling code. > > I'm not so sure it's a good idea to do this now. I rather suspect > there are people who still do use the "old" scheduler. Really? I thought the new scheduler works pretty well by now...it's the default config in at least some, if not most, distros... > Until the "new" scheduler is fixed up to work properly, I prefer to > keep both options available. Ok, no hurry. With the work you've done on it before and the new stuff in usb-next it should be working very well soon :-) -- 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] net: qmi_wwan: Olivetti Olicard 200 support
On Tue, Oct 15, 2013 at 03:06:48PM +0200, Enrico Mioso wrote: > This is a QMI device, manufactured by TCT Mobile Phones. > A companion patch blacklisting this device's QMI interface in the option.c > driver has been sent. > > Signed-off-by: Enrico Mioso > Signed-off-by: Antonella Pellizzari Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] drivers: usb: core: devio.c: Braces around if-elseif-else
On Mon, Oct 14, 2013 at 09:46:38PM +0200, Matthias Beyer wrote:
> This patch applies the rules for braces to the if-elseif-else statement
> in proc_ioctl().
>
> As the kernel styleguide says: If there is at least one multiline block
> in a if-else branching, we should add braces around all blocks. This
> includes braces around the switch-statement on the else branch, which
> needs a reindent after adding the braces.
>
> Signed-off-by: Matthias Beyer
> ---
> drivers/usb/core/devio.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index d15aa51..dd8701b 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1222,7 +1222,7 @@ static int proc_do_submiturb(struct dev_state *ps,
> struct usbdevfs_urb *uurb,
> return -ENOENT;
>
> u = 0;
> - switch(uurb->type) {
> + switch (uurb->type) {
> case USBDEVFS_URB_TYPE_CONTROL:
> if (!usb_endpoint_xfer_control(&ep->desc))
> return -EINVAL;
> @@ -1838,11 +1838,12 @@ static int proc_ioctl(struct dev_state *ps, struct
> usbdevfs_ioctl *ctl)
> return -ENODEV;
> }
>
> - if (ps->dev->state != USB_STATE_CONFIGURED)
> + if (ps->dev->state != USB_STATE_CONFIGURED) {
> retval = -EHOSTUNREACH;
> - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> + } else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno))) {
> retval = -EINVAL;
> - else switch (ctl->ioctl_code) {
> + } else {
> + switch (ctl->ioctl_code) {
>
> /* disconnect kernel driver from interface */
> case USBDEVFS_DISCONNECT:
> @@ -1874,6 +1875,7 @@ static int proc_ioctl(struct dev_state *ps, struct
> usbdevfs_ioctl *ctl)
> retval = -ENOTTY;
> }
> }
> + }
Two braces in line like that should scream at you that something is
wrong with the indentation here.
I'd leave it alone for the switch statement, as it's not needed.
Actually this whole thing should be fine, it's correct, right?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] drivers: usb: core: devio.c: Put arguments on new line
On Mon, Oct 14, 2013 at 09:46:40PM +0200, Matthias Beyer wrote:
> To fit the 80-cols convention, this patch moves the arguments (the
> second and third one) for driver->unlocked_ioctl() onto a new line.
>
> Signed-off-by: Matthias Beyer
> ---
> drivers/usb/core/devio.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 9761a27..0387948 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1870,7 +1870,8 @@ static int proc_ioctl(struct dev_state *ps, struct
> usbdevfs_ioctl *ctl)
> if (driver == NULL || driver->unlocked_ioctl == NULL) {
> retval = -ENOTTY;
> } else {
> - retval = driver->unlocked_ioctl(intf,
> ctl->ioctl_code, buf);
> + retval = driver->unlocked_ioctl(intf,
> + ctl->ioctl_code, buf);
Nah, it looks worse this way, just leave it alone.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: cdc-acm: put delayed_wb to list
On Tue, Oct 15, 2013 at 09:01:35AM +0800, xiao jin wrote: > If acm_write_start during acm suspend, write acm_wb is backuped > to delayed_wb. When acm resume, the delayed_wb will be started. > This mechanism can only record one write during acm suspend. More > acm write will be abandoned. > > This patch is to use list_head to record the write acm_wb during acm > suspend. It can ensure no acm write abandoned. > > Signed-off-by: xiao jin You obviously did not test this patch at all, as it breaks the build. Please get a senior Intel kernel developer to sign-off on your future patches so this does not happen again. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: ohci: remove ep93xx bus glue platform driver
On 17/10/13 01:43, Alan Stern wrote: > On Wed, 16 Oct 2013, Hartley Sweeten wrote: > >> On Tuesday, October 15, 2013 5:57 PM, Ryan Mallon wrote: > >>> Gah, defconfig changes are a pest. I assume the other removed options >>> are implicitly selected, and their removal is the result of >>> re-minimising the defconfig? Can you put a note about this is the >>> changelog please. >> >> Yah... It's a mess. >> >> All I did was: >> make ARCH=arm ep93xx_defconfig >> make ARCH=arm menuconfig # enable USB_OHCI_HCD_PLATFORM >> make ARCH=arm savedefconfig >> mv defconfig arch/arm/configs/ep93xx_defconfig >> >> Looks like the last time the ep93xx_defconfig was refreshed was: >> >> commit 07a8c03f3e06129e847acd068b8b89c13357ee64 >> Author: Uwe Kleine-König >> Date: Thu Jun 10 07:12:18 2010 +0200 >> >> ARM: reduce defconfigs >> >> Hence the ugly changes. > > It's awfully hard to believe that enabling USB_OHCI_HCD_PLATFORM is > responsible for all those other changes. The 'make savedefconfig' step minimises the defconfig by removing any options that are implicitly selected or options that no longer exist. E.g: 4d42942c: 'mtd: make MTD_CONCAT support mandatory' is responsible for CONFIG_MTD_CONCAT disappearing from the defconfig. Looks the other MTD options have also been removed. CONFIG_NFS_V3, for example, got removed because it is implicit as of: 981f9fac: 'NFS: Turn v3 on by default'. 'make savedefconfig' also has an annoying tendency to shuffle items around, presumably because the Kconfig files have changed. This is why, for example, the: # CONFIG_LEGACY_PTYS is not set line got moved in the diff. Interestingly CONFIG_INOTIFY got removed because that option doesn't exist anymore. I can't find the exact commit, but things have been moved around and the option is now called CONFIG_INOTIFY_USER (so whoever made the change probably didn't scan through all the defconfigs). It does mean that ep93xx lost support for inotify at some point. Since nobody has complained, I don't think it is a huge loss. In short, yes the 'make savedefconfig' step is annoying because it creates weird diffs. However, the diffs are correct (cleaning up unused options, etc). The diffs should be inspected to catch things like the inotify case, but otherwise there should be no functional changes. > > Maybe this should be done in two stages. First create a patch that > describes the differences resulting from: > > make ARCH=arm ep93xx_defconfig > make ARCH=arm savedefconfig > mv defconfig arch/arm/configs/ep93xx_defconfig > > Then do this patch on top of that one. > Yeah, I think a pre-patch for "ep93xx: make savedefconfig" would be best. Are you happy to collect all these patches once they are done? I could take the ep93xx parts, but my tree typically only has a small handful of patches each merge window, so it is probably easier if you grab them all. Thanks, ~Ryan -- 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] ehci: remove old TT sched code
On Wed, 16 Oct 2013, Dan Streetman wrote: > On Wed, Oct 16, 2013 at 10:34 AM, Alan Stern > wrote: > > On Tue, 15 Oct 2013, Dan Streetman wrote: > > > >> This removes the old EHCI transaction translator scheduling code, > >> leaving only the "new" (now over 7 years old) scheduling code. > > > > I'm not so sure it's a good idea to do this now. I rather suspect > > there are people who still do use the "old" scheduler. > > Really? I thought the new scheduler works pretty well by now...it's > the default config in at least some, if not most, distros... It would be more accurate to say that the new scheduler works sort of okay most of the time. I would hope that most distros make it the default; it is more capable than the old scheduler. Still, some people are very conservative. And it's possible that some situations are handled better by the old scheduler than the new one (although I can't think of any offhand). > > Until the "new" scheduler is fixed up to work properly, I prefer to > > keep both options available. > > Ok, no hurry. With the work you've done on it before and the new > stuff in usb-next it should be working very well soon :-) Dear me, no... The changes I have made so far amount to perhaps 25% of the work needed to make it truly reliable. It has so many bugs, I wouldn't even try to list them all. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: ohci: remove ep93xx bus glue platform driver
On Thu, 17 Oct 2013, Ryan Mallon wrote: > > It's awfully hard to believe that enabling USB_OHCI_HCD_PLATFORM is > > responsible for all those other changes. > > > The 'make savedefconfig' step minimises the defconfig by removing any > options that are implicitly selected or options that no longer exist. > E.g: 4d42942c: 'mtd: make MTD_CONCAT support mandatory' is responsible > for CONFIG_MTD_CONCAT disappearing from the defconfig. Looks the other > MTD options have also been removed. ... > In short, yes the 'make savedefconfig' step is annoying because it > creates weird diffs. However, the diffs are correct (cleaning up > unused options, etc). The diffs should be inspected to catch things > like the inotify case, but otherwise there should be no functional > changes. Sure, that's what I meant. All that churn was caused by re-minimizing the defconfig file, not by adding USB_OHCI_HCD_PLATFORM. > > Maybe this should be done in two stages. First create a patch that > > describes the differences resulting from: > > > > make ARCH=arm ep93xx_defconfig > > make ARCH=arm savedefconfig > > mv defconfig arch/arm/configs/ep93xx_defconfig > > > > > Then do this patch on top of that one. > > > > Yeah, I think a pre-patch for "ep93xx: make savedefconfig" would be > best. > > Are you happy to collect all these patches once they are done? I > could take the ep93xx parts, but my tree typically only has a > small handful of patches each merge window, so it is probably easier > if you grab them all. Greg KH and I can take both patches. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Input: usbtouchscreen - separate report and transmit buffer size handling
This patch supports the separate handling of the USB transfer buffer length
and the length of the buffer used for multi packet support. For devices
supporting multiple report or diagnostic packets, the USB transfer size is now
limited to the USB endpoints wMaxPacketSize - otherwise it defaults to the
configured report packet size as before
This fixes an issue where event reporting can be delayed for an arbitrary
time for multi packet devices. For instance the report size for eGalax devices
is defined to the 16 byte maximum diagnostic packet size as opposed to the 5
byte report packet size. In case the driver requests 16 byte from the USB
interrupt endpoint, the USB host controller driver needs to split up the
request into 2 accesses according to the endpoints wMaxPacketSize of 8 byte.
When the first transfer is answered by the eGalax device with not less than
the full 8 byte requested, the host controller has got no way of knowing
whether the touch controller has got additional data queued and will issue
the second transfer. If per example a liftoff event finishes at such a
wMaxPacketSize boundary, the data will not be available to the usbtouch driver
until a further event is triggered and transfered to the host. From user
perspective the BTN_TOUCH release event in this case is stuck until the next
touch down event.
Signed-off-by: Christian Engelmayer
---
v2: Added changes suggested by Dmitry Torokhov:
* Prefer a temporary variable over a multiple conversion of wMaxPaxetSize.
* Avoid further modifiction of the shared device info structure.
The existing violation in the probe function
if (!type->process_pkt)
type->process_pkt = usbtouch_process_pkt;
seems to be conveniance triggered and while at least at the moment applicable
for all devices of a type, could be easily removed. For a complete cleanup and
eg. setting the shared device information 'const', we would have to adapt the
Nexio support first - nexio_read_data:
if (!usbtouch->type->max_xc) {
usbtouch->type->max_xc = 2 * x_len;
input_set_abs_params(usbtouch->input, ABS_X,
0, usbtouch->type->max_xc, 0, 0);
usbtouch->type->max_yc = 2 * y_len;
input_set_abs_params(usbtouch->input, ABS_Y,
0, usbtouch->type->max_yc, 0, 0);
}
---
drivers/input/touchscreen/usbtouchscreen.c | 13 +
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c
b/drivers/input/touchscreen/usbtouchscreen.c
index 721fdb3..d632848 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -106,6 +106,7 @@ struct usbtouch_device_info {
struct usbtouch_usb {
unsigned char *data;
dma_addr_t data_dma;
+ int data_size;
unsigned char *buffer;
int buf_len;
struct urb *irq;
@@ -1523,7 +1524,7 @@ static int usbtouch_reset_resume(struct usb_interface
*intf)
static void usbtouch_free_buffers(struct usb_device *udev,
struct usbtouch_usb *usbtouch)
{
- usb_free_coherent(udev, usbtouch->type->rept_size,
+ usb_free_coherent(udev, usbtouch->data_size,
usbtouch->data, usbtouch->data_dma);
kfree(usbtouch->buffer);
}
@@ -1548,6 +1549,7 @@ static int usbtouch_probe(struct usb_interface *intf,
struct usb_endpoint_descriptor *endpoint;
struct usb_device *udev = interface_to_usbdev(intf);
struct usbtouch_device_info *type;
+ int wMaxPacketSize;
int err = -ENOMEM;
/* some devices are ignored */
@@ -1557,6 +1559,7 @@ static int usbtouch_probe(struct usb_interface *intf,
endpoint = usbtouch_get_input_endpoint(intf->cur_altsetting);
if (!endpoint)
return -ENXIO;
+ wMaxPacketSize = le16_to_cpu(endpoint->wMaxPacketSize);
usbtouch = kzalloc(sizeof(struct usbtouch_usb), GFP_KERNEL);
input_dev = input_allocate_device();
@@ -1568,7 +1571,9 @@ static int usbtouch_probe(struct usb_interface *intf,
if (!type->process_pkt)
type->process_pkt = usbtouch_process_pkt;
- usbtouch->data = usb_alloc_coherent(udev, type->rept_size,
+ usbtouch->data_size = (type->get_pkt_len) ?
+ min(type->rept_size, wMaxPacketSize) : type->rept_size;
+ usbtouch->data = usb_alloc_coherent(udev, usbtouch->data_size,
GFP_KERNEL, &usbtouch->data_dma);
if (!usbtouch->data)
goto out_free;
@@ -1628,12 +1633,12 @@ static int usbtouch_probe(struct usb_interface *intf,
if (usb_endpoint_type(endpoint) == USB_ENDPOINT_XFER_INT)
usb_fill_int_urb(usbtouch->irq, udev,
usb_rcvintpipe(udev, endpoint->bEndpointAddress),
-usbtouc
Re: [PATCH] usb/gadget: f_mass_storage: style corrections, cleanup & simplification
On Tue, Oct 15 2013, Andrzej Pietrasiewicz wrote:
> Fix spacing, improve error code returned, remove unused #define,
> use strtobool() instead of kstrtou8().
>
> Signed-off-by: Andrzej Pietrasiewicz
> Signed-off-by: Kyungmin Park
Ackedy-by: Michal Nazarewicz
> diff --git a/drivers/usb/gadget/f_mass_storage.c
> b/drivers/usb/gadget/f_mass_storage.c
> index 6b5f451..8fa8b1f 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -3209,9 +3209,9 @@ static void fsg_lun_attr_release(struct config_item
> *item)
> }
>
> static struct configfs_item_operations fsg_lun_item_ops = {
> - .release= fsg_lun_attr_release,
> - .show_attribute = fsg_lun_opts_attr_show,
> - .store_attribute = fsg_lun_opts_attr_store,
> + .release= fsg_lun_attr_release,
> + .show_attribute = fsg_lun_opts_attr_show,
> + .store_attribute= fsg_lun_opts_attr_store,
> };
>
> static ssize_t fsg_lun_opts_file_show(struct fsg_lun_opts *opts, char *page)
> @@ -3323,8 +3323,6 @@ static struct config_item_type fsg_lun_type = {
> .ct_owner = THIS_MODULE,
> };
>
> -#define MAX_NAME_LEN 40
> -
> static struct config_group *fsg_lun_make(struct config_group *group,
>const char *name)
> {
> @@ -3348,7 +3346,8 @@ static struct config_group *fsg_lun_make(struct
> config_group *group,
>
> fsg_opts = to_fsg_opts(&group->cg_item);
> if (num >= FSG_MAX_LUNS)
> - return ERR_PTR(-ENODEV);
> + return ERR_PTR(-ERANGE);
> +
> mutex_lock(&fsg_opts->lock);
> if (fsg_opts->refcnt || fsg_opts->common->luns[num]) {
> ret = -EBUSY;
> @@ -3364,7 +3363,6 @@ static struct config_group *fsg_lun_make(struct
> config_group *group,
> memset(&config, 0, sizeof(config));
> config.removable = true;
>
> -
> ret = fsg_common_create_lun(fsg_opts->common, &config, num, name,
> (const char **)&group->cg_item.ci_name);
> if (ret) {
> @@ -3418,9 +3416,9 @@ static void fsg_attr_release(struct config_item *item)
> }
>
> static struct configfs_item_operations fsg_item_ops = {
> - .release= fsg_attr_release,
> - .show_attribute = fsg_opts_attr_show,
> - .store_attribute = fsg_opts_attr_store,
> + .release= fsg_attr_release,
> + .show_attribute = fsg_opts_attr_show,
> + .store_attribute= fsg_opts_attr_store,
> };
>
> static ssize_t fsg_opts_stall_show(struct fsg_opts *opts, char *page)
> @@ -3438,22 +3436,23 @@ static ssize_t fsg_opts_stall_store(struct fsg_opts
> *opts, const char *page,
> size_t len)
> {
> int ret;
> - u8 num;
> + bool stall;
>
> mutex_lock(&opts->lock);
> +
> if (opts->refcnt) {
> - ret = -EBUSY;
> - goto end;
> + mutex_unlock(&opts->lock);
> + return -EBUSY;
> }
> - ret = kstrtou8(page, 0, &num);
> - if (ret)
> - goto end;
>
> - opts->common->can_stall = num != 0;
> - ret = len;
> + ret = strtobool(page, &stall);
> + if (!ret) {
> + opts->common->can_stall = stall;
> + ret = len;
> + }
>
> -end:
> mutex_unlock(&opts->lock);
> +
> return ret;
> }
>
> --
> 1.7.0.4
>
--
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
Re: [PATCH 1/2] xhci: fix reset for not halted endpoints
Hi Xenia,
I'm Ccing both Dan and Mathias, since your patch has triggered my "holy
crap so many race conditions" reaction, and I would like their eyes on
this too.
On Mon, Oct 07, 2013 at 06:52:38PM +0300, Xenia Ragiadakou wrote:
> If a Reset Endpoint command is issued for an endpoint that is not in
> the Halted state, xHC does nothing. Since the current implementation of
> xhci_endpoint_reset() aborts any resets that address not halted endpoints,
> the synchronization between the host-side and device-side toggle or sequence
> number is lost.
> This patch attempts to fix this issue by dropping and readding the endpoint
> in order to enforce xHC to reset its internal toggle/sequence number.
>
> Four new functions have been introduced to prevent xhci_endpoint_reset() from
> becoming huge.
>
> xhci_unlink_urbs(xhci, ring):
> unlinks the urbs from the endpoint's rings
>
> xhci_stop_endpoint_for_config(xhci, udev, ep index):
> brings the endpoint to the Stopped state and calls xhci_unlink_urbs
> to unlink any queued urbs
>
> xhci_reset_ep0(xhci, udev, ep):
> calls xhci_stop_endpoint_for_config to stop the endpoint and then
> reinitializes the ring and input context for the default control
> endpoint and issues an Evaluate Context command to reset xHC toggle
>
> xhci_reset_not_halted_ep(xhci, udev, ep):
> calls xhci_stop_endpoint_for_config and if the endpoint to be reset is
> the endpoint 0 calls xhci_reset_ep0, otherwise reinitializes the
> endpoint rings and input context and then drops and readds the endpoint
> issueing a Configure Endpoint
>
> Also, this patch introduces a new trace event for debugging output contexts
> during the process of resetting the endpoint. At this early stage, it is
> useful for debugging the xhci_reset_not_halted_ep() and it may be removed
> later when further testing and revisioning show that the function works as
> expected.
>
> Signed-off-by: Xenia Ragiadakou
> ---
>
> This patch needs to be tested further because I did not find a satisfying
> way to test it. I would appreciate if the device driver writers that had
> problems with the xhci reset endpoint in the past could test it and let me
> know if it fixes the problem or not.
>
> drivers/usb/host/xhci-ring.c | 17 +++-
> drivers/usb/host/xhci-trace.h | 6 ++
> drivers/usb/host/xhci.c | 210
> +-
> drivers/usb/host/xhci.h | 3 +
> 4 files changed, 233 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index faff6fc..f62131b 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -433,6 +433,10 @@ static void ring_doorbell_for_active_rings(struct
> xhci_hcd *xhci,
>
> ep = &xhci->devs[slot_id]->eps[ep_index];
>
> + /* If there is a pending configuration, don't ring the doorbell yet */
> + if (ep->ep_state & EP_CONFIG_PENDING)
> + return;
> +
> /* A ring has pending URBs if its TD list is not empty */
> if (!(ep->ep_state & EP_HAS_STREAMS)) {
> if (ep->ring && !(list_empty(&ep->ring->td_list)))
> @@ -715,8 +719,8 @@ static void xhci_stop_watchdog_timer_in_irq(struct
> xhci_hcd *xhci,
> }
>
> /* Must be called with xhci->lock held in interrupt context */
> -static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
> - struct xhci_td *cur_td, int status)
> +void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, struct xhci_td *cur_td,
> + int status)
> {
> struct usb_hcd *hcd;
> struct urb *urb;
> @@ -788,6 +792,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd
> *xhci, int slot_id,
> xhci_stop_watchdog_timer_in_irq(xhci, ep);
> ep->stopped_td = NULL;
> ep->stopped_trb = NULL;
> + if (ep->ep_state & EP_CONFIG_PENDING)
> + goto signal_completion;
> ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
> return;
> }
> @@ -881,6 +887,13 @@ remove_finished_td:
> return;
> } while (cur_td != last_unlinked_td);
>
> +signal_completion:
> + /* signal the completion for the Stop Endpoint command */
> + if (ep->ep_state & EP_CONFIG_PENDING) {
> + virt_dev = xhci->devs[slot_id];
> + handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
> + }
> +
> /* Return to the event handler with xhci->lock re-acquired */
> }
>
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 132abda..1f8b3fb 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -113,6 +113,12 @@ DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx,
> TP_ARGS(xhci, ctx, ep_num)
> );
>
> +DEFINE_EVENT(xhci_log_ctx, xhci_resetep_ctx,
> + TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
> +
Re: [PATCH v2 4/5] usb: xhci: change enumeration scheme to 'new scheme' by default
Hi Dan,
I'm attempting to put my queue together for usb-next, and this patch
doesn't apply, due to conflicts in the USB core. Can you rebase this
patchset on top of my for-usb-next-queue branch, and resend the last two
patches? The first three applied fine.
Thanks,
Sarah Sharp
On Tue, Oct 08, 2013 at 12:48:12AM +, Williams, Dan J wrote:
> Subject: usb: xhci: change enumeration scheme to 'new scheme' by default
>
> From: Dan Williams
>
> Change the default enumeration scheme for xhci attached non-SuperSpeed
> devices from:
>
>Reset
>SetAddress [xhci address-device BSR = 0]
>GetDescriptor(8)
>GetDescriptor(18)
>
> ...to:
>
>Reset
>[xhci address-device BSR = 1]
>GetDescriptor(64)
>Reset
>SetAddress [xhci address-device BSR = 0]
>GetDescriptor(18)
>
> ...as some devices misbehave when encountering a SetAddress command
> prior to GetDescriptor. There are known legacy devices that require
> this scheme, but testing has found at least one USB3 device that fails
> enumeration when presented with this ordering. For now, follow the ehci
> case and enable 'new scheme' by default for non-SuperSpeed devices.
>
> To support this enumeration scheme on xhci the AddressDevice operation
> needs to be performed twice. The first instance of the command enables
> the HC's device and slot context info for the device, but omits sending
> the device a SetAddress command (BSR == block set address request).
> Then, after GetDescriptor completes, follow up with the full
> AddressDevice+SetAddress operation.
>
> As mentioned before, this ordering of events with USB3 devices causes an
> extra state transition to be exposed to xhci. Previously USB3 devices
> would transition directly from 'enabled' to 'addressed' and never need
> to underrun responses to 'get descriptor'. We do see the 64-byte
> descriptor fetch the correct data, but the following 18-byte descriptor
> read after the reset gets:
>
> bLength= 0
> bDescriptorType= 0
> bcdUSB = 0
> bDeviceClass = 0
> bDeviceSubClass= 0
> bDeviceProtocol= 0
> bMaxPacketSize0= 9
>
> instead of:
>
> bLength= 12
> bDescriptorType= 1
> bcdUSB = 300
> bDeviceClass = 0
> bDeviceSubClass= 0
> bDeviceProtocol= 0
> bMaxPacketSize0= 9
>
> which results in the discovery process looping until falling back to
> 'old scheme' enumeration.
>
> Acked-by: Alan Stern
> Reported-by: David Moore
> Suggested-by: Sarah Sharp
> Signed-off-by: Dan Williams
> ---
>
> Here is the patch again with the changelog clarifications. Alan, I took
> your comments to mean acked-by, let me know it that's not the case.
>
> --
> Dan
>
>
> drivers/usb/core/hub.c | 44
> ++
> drivers/usb/host/xhci-pci.c |1 +
> drivers/usb/host/xhci-plat.c |1 +
> drivers/usb/host/xhci-ring.c |6 +++---
> drivers/usb/host/xhci.c | 19 ++
> drivers/usb/host/xhci.h | 11 ++-
> include/linux/usb/hcd.h |2 ++
> 7 files changed, 72 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c2c5f5adb464..42069ac9c6b2 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2519,6 +2519,21 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
> #define HUB_LONG_RESET_TIME 200
> #define HUB_RESET_TIMEOUT800
>
> +/*
> + * "New scheme" enumeration causes an extra state transition to be
> + * exposed to an xhci host and causes USB3 devices to receive control
> + * commands in the default state. This has been seen to cause
> + * enumeration failures, so disable this enumeration scheme for USB3
> + * devices.
> + */
> +static bool use_new_scheme(struct usb_device *udev, int retry)
> +{
> + if (udev->speed == USB_SPEED_SUPER)
> + return false;
> +
> + return USE_NEW_SCHEME(retry);
> +}
> +
> static int hub_port_reset(struct usb_hub *hub, int port1,
> struct usb_device *udev, unsigned int delay, bool warm);
>
> @@ -3951,6 +3966,20 @@ static int hub_set_address(struct usb_device *udev,
> int devnum)
> return retval;
> }
>
> +static int hub_enable_device(struct usb_device *udev)
> +{
> + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> +
> + if (!hcd->driver->enable_device)
> + return 0;
> + if (udev->state == USB_STATE_ADDRESS)
> + return 0;
> + if (udev->state != USB_STATE_DEFAULT)
> + return -EINVAL;
> +
> + return hcd->driver->enable_device(hcd, udev);
> +}
> +
> /* Reset device, (re)assign address, get device descriptor.
> * Device connection must be stable, no more debouncing needed.
> * Returns device in USB_STATE_ADDRESS, except on error.
> @@ -4063,7 +4092,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device
> *udev, int port1,
>* this area, and this is how Linux has done it for ages.
>
Re: [PATCH 9/9] usbfs: Add support for allocating / freeing streams
Alan, do you have any more feedback on this patchset?
Sarah Sharp
On Wed, Oct 09, 2013 at 05:19:31PM +0200, Hans de Goede wrote:
> Signed-off-by: Hans de Goede
> ---
> drivers/usb/core/devio.c | 118
> ++
> include/uapi/linux/usbdevice_fs.h | 7 +++
> 2 files changed, 125 insertions(+)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index bfb2821..4ca7e86 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -778,6 +778,79 @@ static struct usb_host_endpoint
> *ep_to_host_endpoint(struct usb_device *dev,
> return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK];
> }
>
> +static int parse_usbdevfs_streams(struct dev_state *ps,
> + struct usbdevfs_streams __user *streams,
> + unsigned int *num_streams_ret,
> + unsigned int *num_eps_ret,
> + struct usb_host_endpoint ***eps_ret,
> + struct usb_interface **intf_ret)
> +{
> + unsigned int i, num_streams, num_eps;
> + struct usb_host_endpoint **eps;
> + struct usb_interface *intf = NULL;
> + unsigned char ep;
> + int ifnum, ret;
> +
> + if (get_user(num_streams, &streams->num_streams) ||
> + get_user(num_eps, &streams->num_eps))
> + return -EFAULT;
> +
> + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS)
> + return -EINVAL;
> +
> + /* The XHCI controller allows max 1024 streams */
> + if (num_streams_ret && (num_streams < 2 || num_streams > 1024))
> + return -EINVAL;
> +
> + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL);
> + if (!eps)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_eps; i++) {
> + if (get_user(ep, &streams->eps[i])) {
> + ret = -EFAULT;
> + goto error;
> + }
> + eps[i] = ep_to_host_endpoint(ps->dev, ep);
> + if (!eps[i]) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /* usb_alloc/free_streams operate on an usb_interface */
> + ifnum = findintfep(ps->dev, ep);
> + if (ifnum < 0) {
> + ret = ifnum;
> + goto error;
> + }
> +
> + if (i == 0) {
> + ret = checkintf(ps, ifnum);
> + if (ret < 0)
> + goto error;
> + intf = usb_ifnum_to_if(ps->dev, ifnum);
> + } else {
> + /* Verify all eps belong to the same interface */
> + if (ifnum != intf->altsetting->desc.bInterfaceNumber) {
> + ret = -EINVAL;
> + goto error;
> + }
> + }
> + }
> +
> + if (num_streams_ret)
> + *num_streams_ret = num_streams;
> + *num_eps_ret = num_eps;
> + *eps_ret = eps;
> + *intf_ret = intf;
> +
> + return 0;
> +
> +error:
> + kfree(eps);
> + return ret;
> +}
> +
> static int match_devt(struct device *dev, void *data)
> {
> return dev->devt == (dev_t) (unsigned long) data;
> @@ -1992,6 +2065,45 @@ static int proc_disconnect_claim(struct dev_state *ps,
> void __user *arg)
> return claimintf(ps, dc.interface);
> }
>
> +static int proc_alloc_streams(struct dev_state *ps, void __user *arg)
> +{
> + unsigned num_streams, num_eps;
> + struct usb_host_endpoint **eps;
> + struct usb_interface *intf;
> + int r;
> +
> + r = parse_usbdevfs_streams(ps, arg, &num_streams, &num_eps,
> +&eps, &intf);
> + if (r)
> + return r;
> +
> + destroy_async_on_interface(ps,
> +intf->altsetting[0].desc.bInterfaceNumber);
> +
> + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL);
> + kfree(eps);
> + return r;
> +}
> +
> +static int proc_free_streams(struct dev_state *ps, void __user *arg)
> +{
> + unsigned num_eps;
> + struct usb_host_endpoint **eps;
> + struct usb_interface *intf;
> + int r;
> +
> + r = parse_usbdevfs_streams(ps, arg, NULL, &num_eps, &eps, &intf);
> + if (r)
> + return r;
> +
> + destroy_async_on_interface(ps,
> +intf->altsetting[0].desc.bInterfaceNumber);
> +
> + r = usb_free_streams(intf, eps, num_eps, GFP_KERNEL);
> + kfree(eps);
> + return r;
> +}
> +
> /*
> * NOTE: All requests here that have interface numbers as parameters
> * are assuming that somehow the configuration has been prevented from
> @@ -2168,6 +2280,12 @@ static long usbdev_do_ioctl(struct file *file,
> unsigned int cmd,
> case USBDEVFS_DISCONNECT_CLAIM:
> ret = proc_disconnect_claim(ps, p);
>
Re: [PATCH v2] xhci: fix usb3 streams
On Tue, Oct 15, 2013 at 10:53:57AM -0400, Alan Stern wrote: > On Mon, 14 Oct 2013, Gerd Hoffmann wrote: > > > Gerd, Hans, any objections to this updated patch? The warning is fixed > > with it. > > > > The patch probably still needs to address the case where the ring > > expansion fails because we can't insert the new segments into the radix > > tree. The patch should probably allocate the segments, attempt to add > > them to the radix tree, and fail without modifying the link TRBs of the > > ring. I'd have to look more deeply into the code to see what exactly > > should be done there. > > > > I would like that issue fixed before I merge these patches, so if you > > want to take a stab at fixing it, please do. > > > > Sarah Sharp > > Sarah, how did you manage to send an email with the "From:" line set to > Gerd's name and address? I sent it using git format-patch and mutt, and accidentally left Gerd's >From line intact. Looking at the headers, it seems like the default Intel exchange servers simply passed the email through. Header forging, what fun! > > 8<>8 > > > > xhci maintains a radix tree for each stream endpoint because it must > > be able to map a trb address to the stream ring. Each ring segment > > must be added to the ring for this to work. Currently xhci sticks > > only the first segment of each stream ring into the radix tree. > > > > Result is that things work initially, but as soon as the first segment > > is full xhci can't map the trb address from the completion event to the > > stream ring any more -> BOOM. You'll find this message in the logs: > > > > ERROR Transfer event for disabled endpoint or incorrect stream ring > > > > This patch adds a helper function to update the radix tree, and a > > function to remove ring segments from the tree. Both functions loop > > over the segment list and handles all segments instead of just the > > first. > > There may be a simpler approach to this problem. > > When using a new ring segment, keep the first TRB entry in reserve. > Don't put a normal TRB in there, instead leave it as a no-op entry > containing a pointer to the stream ring. (Make the prior Link TRB > point to the second entry in the new segment instead of the first.) > > Then you won't have to add to or remove anything from the radix tree. I don't understand how this would help. Are you advocating a different way of mapping TRB DMA addresses to stream rings that would allow us to ditch the radix tree all together? Ok, so with your solution, we have a virtual stream ring pointer as the first TRB of the segment. We get an event with the DMA address of a TRB in one of many stream rings on an endpoint. From that, I think we can infer the DMA address of the first TRB on the segment, due to the alignment requirements and ring size. And then what do we do with that? We don't have the virtual address of that first TRB, so the xHCI driver can't read the ring pointer from it. I'm confused as to what the next steps would be to solve this. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: hub: Clear Port Reset Change during init/resume
On Tue, Oct 15, 2013 at 05:45:00PM -0700, Julius Werner wrote:
> This patch adds the Port Reset Change flag to the set of bits that are
> preemptively cleared on init/resume of a hub. In theory this bit should
> never be set unexpectedly... in practice it can still happen if BIOS,
> SMM or ACPI code plays around with USB devices without cleaning up
> correctly. This is especially dangerous for XHCI root hubs, which don't
> generate any more Port Status Change Events until all change bits are
> cleared, so this is a good precaution to have (similar to how it's
> already done for the Warm Port Reset Change flag).
Did you run into an issue where port status change events weren't being
generated because the Port Reset flag was set? I'm trying to figure out
if this addresses a real issue you hit (and thus should be queued for
stable), or if this is just a precaution.
I do agree this is a good fix. ISTR that with some xHCI vendors, when
the host is reset, the hardware will drive a port reset down all ports.
That may cause the port reset and even warm port reset bits to be set.
I have a vague recollection that some implementations might not even
wait for the port reset to complete before saying the reset is done. We
can't tell which hosts will and won't drive a reset, so we rely on the
USB core to clear those bits if they are set.
So perhaps instead of the BIOS/SMM/ACPI code leaving the ports in an
unclean state, the root cause of your issue is actually the call to
xhci_reset? You can check the port status before that call to find out.
> Signed-off-by: Julius Werner
> ---
> drivers/usb/core/hub.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index e6b682c..c3dd64c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1130,6 +1130,11 @@ static void hub_activate(struct usb_hub *hub, enum
> hub_activation_type type)
> usb_clear_port_feature(hub->hdev, port1,
> USB_PORT_FEAT_C_ENABLE);
> }
> + if (portchange & USB_PORT_STAT_C_RESET) {
> + need_debounce_delay = true;
> + usb_clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_RESET);
> + }
> if ((portchange & USB_PORT_STAT_C_BH_RESET) &&
> hub_is_superspeed(hub->hdev)) {
> need_debounce_delay = true;
> --
> 1.7.12.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] usb: hub: Clear Port Reset Change during init/resume
On Wed, Oct 16, 2013 at 4:57 PM, Sarah Sharp wrote: > > Did you run into an issue where port status change events weren't being > generated because the Port Reset flag was set? I'm trying to figure out > if this addresses a real issue you hit (and thus should be queued for > stable), or if this is just a precaution. We ran into this on HP Chromebook 14 (Falco). The port reset flag would be set after a suspend/resume cycle with nothing attached. -- Benson Leung Software Engineer, Chrom* OS ble...@chromium.org -- 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: transmit lockup using smsc95xx ethernet on usb3
On Tue, Oct 15, 2013 at 10:59:18AM +0100, David Laight wrote: > We are seeing complete lockups of the transmit side when using > the smsc95xx driver connected to a USB3 port on an i7 (Ivybridge) cpu. > These errors are very intermittent - less than once a day, and > it isn't actually clear that they are related to traffic load. > > Most of the systems are running the 3.2 kernel from Ubuntu 12.04 > but I've seen the same problem when running a 3.4 kernel. Have you tried the latest stable kernel or the latest -rc kernel? > Looking at the changelog for xhci-ring.c I can see that some > 'nasty' bugs were fixed between 3.2 and 3.4 (and possibly since) > but the usbmon trace I've now got doesn't seem to match any > of the changelog entries. > > We are also seeing similar problems if we connect to a USB2 > header. Do you mean a USB 2.0 port under the xHCI host controller? What does `lsusb -t` show as the parent host controller in that case? > Since we can't reproduce the problem quickly it is difficult to > do any analysis. Any suggestions for increasing the error rate > would be welcome. > > Below is an annotated extract from a usbmon trace while running > a netperf test that was sending 8192 byte TCP packets (nagle off). > I've deleted the Bi entries (packets are received throughout) > and numbered all the others (modulo 1) so it is easier to > see when the requests complete, I've also calculated the elapsed > time and the number of Setup entries between the S and C traces. > > The USB ring seems to have 60 outstanding transmits in it, > each time one completes another is sent. There are a few 1 > traces of that then: > > start:9870 88020ea16000 293811125 S Bo:3:003:2 -115 1514 = > e235 e245 22003200 00224d98 > d8460002 1f0057d7 08004500 05d0ff11 >done:9811:6969:60 88020c7c8000 293811236 C Bo:3:003:2 0 1090 > > start:9871 88020ea16a80 293811242 S Bo:3:003:2 -115 1090 = > 3a34 3a44 22003200 00224d98 > d8460002 1f0057d7 08004500 0428ff12 > ... > start:9929 88020ea16780 293817964 S Bo:3:003:2 -115 1514 = > e235 e245 22003200 00224d98 > d8460002 1f0057d7 08004500 05d0ff4c > Last successful completion. >done:9870:6968:60 88020ea16000 293818093 C Bo:3:003:2 0 1514 > > start:9930 88020ea16000 293818099 S Bo:3:003:2 -115 1514 = > e235 e245 22003200 00224d98 > d8460002 1f0057d7 08004500 05d0ff4d > > At this point something (untraced) seems to have gone horribly > wrong in the transmit ring. dmesg shows nothing. > Two Bo 'fail -71', 6 succeed, one fails -32 the rest fail -104. >done:9871:6913:60 88020ea16a80 293818155 C Bo:3:003:2 -71 512 > >done:9872:6927:59 88020ea16f00 293818235 C Bo:3:003:2 -71 0 >done:9873:6875:58 88020ea16480 293818313 C Bo:3:003:2 0 1514 > >done:9874:6786:57 88020c7c83c0 293818353 C Bo:3:003:2 0 1514 > >done:9875:6794:56 88020c7c80c0 293818470 C Bo:3:003:2 0 1514 > >done:9876:6789:55 88020c7c8e40 293818589 C Bo:3:003:2 0 1514 > >done:9877:6775:54 88020c7c8240 293818702 C Bo:3:003:2 0 1090 > >done:9878:6751:53 88020c7c8180 293818803 C Bo:3:003:2 0 1514 > >done:9879:6735:52 88020c7c89c0 293818885 C Bo:3:003:2 -32 0 >done:9880:6671:51 88020c7c8900 293818925 C Bo:3:003:2 -104 0 > ... > done:9927:1292:4 88020cf0c480 293819015 C Bo:3:003:2 -104 0 > done:9928:1170:3 88020ea160c0 293819016 C Bo:3:003:2 -104 0 > Something is known to be wrong... > start:9931 88020ea160c0 293819037 S Co:3:003:0 > s 02 01 0002 0 > done:9929:1080:3 88020ea16780 293819044 C Bo:3:003:2 -104 0 > done:9930:945:2 88020ea16000 293819044 C Bo:3:003:2 -104 0 > done:9931:48:1 88020ea160c0 293819085 C Co:3:003:0 0 0 > > These 10 transmits never finish: > start:9932 88020ea160c0 293819098 S Bo:3:003:2 -115 1090 = > 3a34 3a44 22003200 00224d98 > d8460002 1f0057d7 08004500 0428ff4e > ... 9933 to 9940 deleted > start:9941 88020ea16b40 293819111 S Bo:3:003:2 -115 1514 = > e235 e245 22003200 00224d98 > d8460002 1f0057d7 08004500 05d0ff57 > > All further transmits fail immediately E -12 and generate the > 'xhci_hcd :00:14.0: ERROR no room on ep ring' message. > (There are 1070 'E' traces and 1070 'no room' messages.) > Receives are still working. > start:9942 88020ea16240 293819113 S Bo:3:003:2 -115 1514 = > e235 e245 22003200 00224d98 > d8460002 1f0057d7 08004500 05d0ff58 > done:9942:1550:1 88020ea16240 293820663 E Bo:3:003:2 -12 0 > star
Re: [PATCH v2] USB: cdc-acm: put delayed_wb to list
Hi, Greg, I am sorry for the inconvenience again. I will do as what you point to make sure the thing won't happen again in future. On Wed, 2013-10-16 at 13:44 -0700, Greg KH wrote: > On Tue, Oct 15, 2013 at 09:01:35AM +0800, xiao jin wrote: > > If acm_write_start during acm suspend, write acm_wb is backuped > > to delayed_wb. When acm resume, the delayed_wb will be started. > > This mechanism can only record one write during acm suspend. More > > acm write will be abandoned. > > > > This patch is to use list_head to record the write acm_wb during acm > > suspend. It can ensure no acm write abandoned. > > > > Signed-off-by: xiao jin > > You obviously did not test this patch at all, as it breaks the build. > > Please get a senior Intel kernel developer to sign-off on your future > patches so this does not happen again. > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: hub: Clear Port Reset Change during init/resume
> Did you run into an issue where port status change events weren't being > generated because the Port Reset flag was set? I'm trying to figure out > if this addresses a real issue you hit (and thus should be queued for > stable), or if this is just a precaution. As Benson said, we're seeing this on the HP Chromebook 14 (Intel LynxPoint xHC). It only happens after system suspend/resume, so I'm not sure if there even is an xHC reset involved (not by the kernel anyway, but with all that other stuff it's hard to say). Note that we build our own firmware (including SMM/ACPI handlers), so this does not directly translate to LynxPoint PCs, but I think we based some of our code off Intel reference code and guides which may have already included the problem. I've found port reset code in both our firmware resume path and ACPI _PS0 handler, but they both seem to clear all Port Change bits correctly, so we are not sure about our true root cause yet. -- 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 "uwb: convert bus code to use dev_groups" added to driver-core tree
This is a note to let you know that I've just added the patch titled
uwb: convert bus code to use dev_groups
to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-next branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will also be merged in the next major kernel release
during the merge window.
If you have any questions about this process, please let me know.
>From 1b604039a342a4ba465d37657bec55db476506b0 Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman
Date: Sun, 6 Oct 2013 23:55:44 -0700
Subject: uwb: convert bus code to use dev_groups
The dev_attrs field of struct bus_type is going away soon, dev_groups
should be used instead. This converts the uwb bus code to use the
correct field.
Cc: Bruno Morelli
Cc:
Signed-off-by: Greg Kroah-Hartman
---
drivers/uwb/umc-bus.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/uwb/umc-bus.c b/drivers/uwb/umc-bus.c
index 5c5b3fc9..e3ed6ff6 100644
--- a/drivers/uwb/umc-bus.c
+++ b/drivers/uwb/umc-bus.c
@@ -201,6 +201,7 @@ static ssize_t capability_id_show(struct device *dev,
struct device_attribute *a
return sprintf(buf, "0x%02x\n", umc->cap_id);
}
+static DEVICE_ATTR_RO(capability_id);
static ssize_t version_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -208,12 +209,14 @@ static ssize_t version_show(struct device *dev, struct
device_attribute *attr, c
return sprintf(buf, "0x%04x\n", umc->version);
}
+static DEVICE_ATTR_RO(version);
-static struct device_attribute umc_dev_attrs[] = {
- __ATTR_RO(capability_id),
- __ATTR_RO(version),
- __ATTR_NULL,
+static struct attribute *umc_dev_attrs[] = {
+ &dev_attr_capability_id.attr,
+ &dev_attr_version.attr,
+ NULL,
};
+ATTRIBUTE_GROUPS(umc_dev);
struct bus_type umc_bus_type = {
.name = "umc",
@@ -222,7 +225,7 @@ struct bus_type umc_bus_type = {
.remove = umc_device_remove,
.suspend= umc_device_suspend,
.resume = umc_device_resume,
- .dev_attrs = umc_dev_attrs,
+ .dev_groups = umc_dev_groups,
};
EXPORT_SYMBOL_GPL(umc_bus_type);
--
1.8.4.3.gca3854a
--
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_phy_gen: refine conditional declaration of usb_nop_xceiv_register
Commit 3fa4d734 (usb: phy: rename nop_usb_xceiv => usb_phy_gen_xceiv)
changed the conditional around the declaration of usb_nop_xceiv_register
from
#if defined(CONFIG_NOP_USB_XCEIV) ||
(defined(CONFIG_NOP_USB_XCEIV_MODULE) && defined(MODULE))
to
#if IS_ENABLED(CONFIG_NOP_USB_XCEIV)
While that looks the same, it is semantically different. The first expression
is true if CONFIG_NOP_USB_XCEIV is built as module and if the including
code is built as module. The second expression is true if code depending on
CONFIG_NOP_USB_XCEIV if built as module or into the kernel.
As a result, the arm:allmodconfig build fails with
arch/arm/mach-omap2/built-in.o: In function `omap3_evm_init':
arch/arm/mach-omap2/board-omap3evm.c:703: undefined reference to
`usb_nop_xceiv_register'
Fix the problem by reverting to the old conditional.
Cc: Josh Boyer
Signed-off-by: Guenter Roeck
---
include/linux/usb/usb_phy_gen_xceiv.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/usb/usb_phy_gen_xceiv.h
b/include/linux/usb/usb_phy_gen_xceiv.h
index f9a7e7b..11d85b9 100644
--- a/include/linux/usb/usb_phy_gen_xceiv.h
+++ b/include/linux/usb/usb_phy_gen_xceiv.h
@@ -12,7 +12,7 @@ struct usb_phy_gen_xceiv_platform_data {
unsigned int needs_reset:1;
};
-#if IS_ENABLED(CONFIG_NOP_USB_XCEIV)
+#if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE) &&
defined(MODULE))
/* sometimes transceivers are accessed only through e.g. ULPI */
extern void usb_nop_xceiv_register(void);
extern void usb_nop_xceiv_unregister(void);
--
1.7.9.7
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb/chipidea: fix oops on memory allocation failure
On Wed, Oct 16, 2013 at 01:45:15PM +0100, Russell King - ARM Linux wrote:
> When CMA fails to initialize in v3.12-rc4, the chipidea driver oopses
> the kernel while trying to remove and put the HCD which doesn't exist:
>
> WARNING: CPU: 0 PID: 6 at
> /home/rmk/git/linux-rmk/arch/arm/mm/dma-mapping.c:511
> __dma_alloc+0x200/0x240()
> coherent pool not initialised!
> Modules linked in:
> CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
> Workqueue: deferwq deferred_probe_work_func
> Backtrace:
> [] (dump_backtrace+0x0/0x10c) from []
> (show_stack+0x18/0x1c)
> r6:c05fd9cc r5:01ff r4: r3:df86ad00
> [] (show_stack+0x0/0x1c) from [] (dump_stack+0x70/0x8c)
> [] (dump_stack+0x0/0x8c) from []
> (warn_slowpath_common+0x6c/0x8c)
> r4:df883a60 r3:df86ad00
> [] (warn_slowpath_common+0x0/0x8c) from []
> (warn_slowpath_fmt+0x38/0x40)
> r8: r7:1000 r6:c083b808 r5: r4:df2efe80
> [] (warn_slowpath_fmt+0x0/0x40) from []
> (__dma_alloc+0x200/0x240)
> r3: r2:c05fda00
> [] (__dma_alloc+0x0/0x240) from []
> (arm_dma_alloc+0x88/0xa0)
> [] (arm_dma_alloc+0x0/0xa0) from []
> (ehci_setup+0x1f4/0x438)
> [] (ehci_setup+0x0/0x438) from []
> (usb_add_hcd+0x18c/0x664)
> [] (usb_add_hcd+0x0/0x664) from [] (host_start+0xf0/0x180)
> [] (host_start+0x0/0x180) from []
> (ci_hdrc_probe+0x360/0x670
> )
> r6:df2ef410 r5: r4:df2c3010 r3:c03e8904
> [] (ci_hdrc_probe+0x0/0x670) from []
> (platform_drv_probe+0x20/0x24)
> [] (platform_drv_probe+0x0/0x24) from []
> (driver_probe_device+0x9c/0x234)
> ...
> ---[ end trace c88ccaf3969e8422 ]---
> Unable to handle kernel NULL pointer dereference at virtual address 0028
> pgd = c0004000
> [0028] *pgd=
> Internal error: Oops: 17 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 6 Comm: kworker/u2:0 Tainted: GW3.12.0-rc4+ #56
> Workqueue: deferwq deferred_probe_work_func
> task: df86ad00 ti: df882000 task.ti: df882000
> PC is at usb_remove_hcd+0x10/0x150
> LR is at host_stop+0x1c/0x3c
> pc : []lr : []psr: 6013
> sp : df883b50 ip : df883b78 fp : df883b74
> r10: c11f4c54 r9 : c0836450 r8 : df30c400
> r7 : fff4 r6 : df2ef410 r5 : r4 : df2c3010
> r3 : r2 : r1 : df86b0a0 r0 :
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 10c53c7d Table: 2f29404a DAC: 0015
> Process kworker/u2:0 (pid: 6, stack limit = 0xdf882240)
> Stack: (0xdf883b50 to 0xdf884000)
> ...
> Backtrace:
> [] (usb_remove_hcd+0x0/0x150) from []
> (host_stop+0x1c/0x3c)
> r6:df2ef410 r5: r4:df2c3010
> [] (host_stop+0x0/0x3c) from []
> (ci_hdrc_host_destroy+0x1c/0x20)
> r5: r4:df2c3010
> [] (ci_hdrc_host_destroy+0x0/0x20) from []
> (ci_hdrc_probe+0x3ac/0x670)
> [] (ci_hdrc_probe+0x0/0x670) from []
> (platform_drv_probe+0x20/0x24)
> [] (platform_drv_probe+0x0/0x24) from []
> (driver_probe_device+0x9c/0x234)
> [] (driver_probe_device+0x0/0x234) from []
> (__device_attach+0x44/0x48)
> ...
> ---[ end trace c88ccaf3969e8423 ]---
>
> Fix this so at least we can continue booting and get to a shell prompt.
>
> Signed-off-by: Russell King
> Tested-by: Russell King
> ---
> drivers/usb/chipidea/host.c |6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 6f96795..64d7a6d 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -100,8 +100,10 @@ static void host_stop(struct ci_hdrc *ci)
> {
> struct usb_hcd *hcd = ci->hcd;
>
> - usb_remove_hcd(hcd);
> - usb_put_hcd(hcd);
> + if (hcd) {
> + usb_remove_hcd(hcd);
> + usb_put_hcd(hcd);
> + }
> if (ci->platdata->reg_vbus)
> regulator_disable(ci->platdata->reg_vbus);
> }
Thanks for pointing it, the regulator_disable may still be
called but the regulator_enable is not called due to hcd's
allocation is failed at this case. For this error, the
driver should not call .stop if .start has failed.
Below is my proposal fix for this problem:
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 42a0bd4..c1d05c4 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -270,16 +270,18 @@ static void host_stop(struct ci_hdrc *ci)
{
struct usb_hcd *hcd = ci->hcd;
- usb_remove_hcd(hcd);
- usb_put_hcd(hcd);
- if (ci->platdata->reg_vbus)
- regulator_disable(ci->platdata->reg_vbus);
+ if (hcd) {
+ usb_remove_hcd(hcd);
+ usb_put_hcd(hcd);
+ if (ci->platdata->reg_vbus)
+ regulator_disable(ci->platdata->reg_vbus);
+ }
}
void ci_hdrc_host_destroy(struct ci_hdrc *ci)
{
- if (ci->role == CI_ROLE_HOST)
+ if (ci->role == CI_ROLE_HOST && ci->hcd)
host_stop(ci);
}
--
Best Regards,
Peter Chen
--
To unsubscribe f
