Re: Re: [PATCH] USB2NET : SR9700 : One chip USB 1.1 USB2NETSR9700Device Driver Support
Dear Miller : I'm sorry to trouble you that How about the process for SR9700 Device Driver release? Thanks a lot. 2013-09-22 liujunliang_ljl 发件人： David Miller 发送时间： 2013-09-04 10:27:14 收件人： liujunliang_ljl 抄送： horms; joe; romieu; gregkh; netdev; linux-usb; linux-kernel; sunhecheng 主题： Re: [PATCH] USB2NET : SR9700 : One chip USB 1.1 USB2NETSR9700Device Driver Support From: liujunliang_...@163.com Date: Sun, 1 Sep 2013 19:38:08 +0800 From: Liu Junliang liujunliang_...@163.com Signed-off-by: Liu Junliang liujunliang_...@163.com Applied, thanks.
[PATCH] usb/core/devio.c:tolerate wrong direction flag in control endpoints
Hi,
USB devio rejects control messages when the index does not have the
direction bit set correctly.
This breaks windows apps in KVM -- and might be overly strict according
to my reading of USB HID spec.
Attached patch makes the kernel tolerant against it and makes the app
work for me.
More details in the patch header.
USB experts: Please review this and judge whether this is correct,
applies more generically,
or maybe needs to be special cased (only for USB HID devices?) or
implemented as quirk
or module/kernel parameter.
Once in the final form, this *might* be stable material.
Please keep me in copy for the discussion, my participation on LKML is
mostly reading summaries
from Jonathan and Thorsten these days, unfortunately.
--
Kurt Garloff k...@garloff.de
Cologne, Germany
commit bc1e4e1ae1d5a4f9b2d263f22c651dd5ba4f8ff9
Author: Kurt Garloff k...@garloff.de
Date: Sun Sep 22 11:54:59 2013 +0200
From: Kurt Garloff k...@garloff.de
Subject: Tolerate wrong direction bit endpoint index for control messages
Signed-off-by: Kurt Garloff k...@garloff.de
Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101)
[1] with the Windows App (EasyNote) works natively but fails when
WIndows is running under KVM (and the USB device handed to KVM).
The reason is a USB control message
usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008
This goes to endpoint 1 (wIndex), however, the endpoint is an input
endpoint and thus enumerated 0x81.
The kernel thus rejects the IO and thus we see the failure.
Apparently, Linux is more strict here than Windows ... we can't change
the Win app easily, so that's a problem.
However, the app might not even be buggy here. Browsing the USB HID
spec, there's a note that the bit for direction is ignored for control
endpoints ... so Linux might be overly strict?
With attached patch, everything works.
usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep 01 but needs 81 (or 00)
Notes:
- I have not checked whether the ignorance of the direction bit for
control endpoints only applies to USB HID devices, thus I have not
special-cased depending on the device type.
- We do output a warning into the kernel log, as this might still be
caused by an application error.
- We don't risk sending to a wrong endpoint, as there can't be two
endpoints with same index and just different direction.
- I suspect this will mostly affect apps in virtual environments; as on
Linux the apps would have been adapted to the stricter handling of the
kernel. I have done that for mine[2], although delaying the release by
two years
[1} http://www.pegatech.com/
[2] https://sourceforge.net/projects/notetakerpen/
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index c2f62a3..8acbc2f 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -623,6 +623,19 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype,
switch (requesttype USB_RECIP_MASK) {
case USB_RECIP_ENDPOINT:
ret = findintfep(ps-dev, index);
+ if (ret 0) {
+ /* OK, give it a second try -- user might have confused
+ * direction -- this happens from virtualized win apps
+ * e.g. -- warn, but be graceful */
+ ret = findintfep(ps-dev, index ^ 0x80);
+ if (ret = 0)
+dev_info(ps-dev-dev ,
+ %s: process %i (%s) requesting ep %02x but needs %02x (or 00)\n,
+ __func__,
+ task_pid_nr(current),
+ current-comm,
+ index, index ^ 0x80);
+ }
if (ret = 0)
ret = checkintf(ps, ret);
break;
[PATCH v2 2/5] dma: cppi41: s/deinit_cpii41/deinit_cppi41/
Fix a misspelled function name.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 53d1d31..5469a15 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -741,7 +741,7 @@ static void disable_sched(struct cppi41_dd *cdd)
cppi_writel(0, cdd-sched_mem + DMA_SCHED_CTRL);
}
-static void deinit_cpii41(struct device *dev, struct cppi41_dd *cdd)
+static void deinit_cppi41(struct device *dev, struct cppi41_dd *cdd)
{
disable_sched(cdd);
@@ -835,7 +835,7 @@ static int init_cppi41(struct device *dev, struct cppi41_dd
*cdd)
init_sched(cdd);
return 0;
err_td:
- deinit_cpii41(dev, cdd);
+ deinit_cppi41(dev, cdd);
return ret;
}
@@ -1009,7 +1009,7 @@ err_irq:
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
cleanup_chans(cdd);
err_chans:
- deinit_cpii41(pdev-dev, cdd);
+ deinit_cppi41(pdev-dev, cdd);
err_init_cppi:
pm_runtime_put(pdev-dev);
err_get_sync:
@@ -1033,7 +1033,7 @@ static int cppi41_dma_remove(struct platform_device *pdev)
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
free_irq(cdd-irq, cdd);
cleanup_chans(cdd);
- deinit_cpii41(pdev-dev, cdd);
+ deinit_cppi41(pdev-dev, cdd);
iounmap(cdd-usbss_mem);
iounmap(cdd-ctrl_mem);
iounmap(cdd-sched_mem);
--
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 v2 1/5] dma: cppi41: pass around device instead of platform_device
Instead of passing around struct plafform_device, use struct device and
save one level of dereferencing. This affects the following functions:
* cppi41_add_chans
* purge_descs
* deinit_cpii41
* init_descs
* init_cppi41
* cppi_glue_infos
It's just a cosmetic cleanup that makes the code more readable.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 7c82b92..53d1d31 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -674,14 +674,14 @@ static void cleanup_chans(struct cppi41_dd *cdd)
}
}
-static int cppi41_add_chans(struct platform_device *pdev, struct cppi41_dd
*cdd)
+static int cppi41_add_chans(struct device *dev, struct cppi41_dd *cdd)
{
struct cppi41_channel *cchan;
int i;
int ret;
u32 n_chans;
- ret = of_property_read_u32(pdev-dev.of_node, #dma-channels,
+ ret = of_property_read_u32(dev-of_node, #dma-channels,
n_chans);
if (ret)
return ret;
@@ -719,7 +719,7 @@ err:
return -ENOMEM;
}
-static void purge_descs(struct platform_device *pdev, struct cppi41_dd *cdd)
+static void purge_descs(struct device *dev, struct cppi41_dd *cdd)
{
unsigned int mem_decs;
int i;
@@ -731,7 +731,7 @@ static void purge_descs(struct platform_device *pdev,
struct cppi41_dd *cdd)
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMCTRL(i));
- dma_free_coherent(pdev-dev, mem_decs, cdd-cd,
+ dma_free_coherent(dev, mem_decs, cdd-cd,
cdd-descs_phys);
}
}
@@ -741,19 +741,19 @@ static void disable_sched(struct cppi41_dd *cdd)
cppi_writel(0, cdd-sched_mem + DMA_SCHED_CTRL);
}
-static void deinit_cpii41(struct platform_device *pdev, struct cppi41_dd *cdd)
+static void deinit_cpii41(struct device *dev, struct cppi41_dd *cdd)
{
disable_sched(cdd);
- purge_descs(pdev, cdd);
+ purge_descs(dev, cdd);
cppi_writel(0, cdd-qmgr_mem + QMGR_LRAM0_BASE);
cppi_writel(0, cdd-qmgr_mem + QMGR_LRAM0_BASE);
- dma_free_coherent(pdev-dev, QMGR_SCRATCH_SIZE, cdd-qmgr_scratch,
+ dma_free_coherent(dev, QMGR_SCRATCH_SIZE, cdd-qmgr_scratch,
cdd-scratch_phys);
}
-static int init_descs(struct platform_device *pdev, struct cppi41_dd *cdd)
+static int init_descs(struct device *dev, struct cppi41_dd *cdd)
{
unsigned int desc_size;
unsigned int mem_decs;
@@ -777,7 +777,7 @@ static int init_descs(struct platform_device *pdev, struct
cppi41_dd *cdd)
reg |= ilog2(ALLOC_DECS_NUM) - 5;
BUILD_BUG_ON(DESCS_AREAS != 1);
- cdd-cd = dma_alloc_coherent(pdev-dev, mem_decs,
+ cdd-cd = dma_alloc_coherent(dev, mem_decs,
cdd-descs_phys, GFP_KERNEL);
if (!cdd-cd)
return -ENOMEM;
@@ -813,12 +813,12 @@ static void init_sched(struct cppi41_dd *cdd)
cppi_writel(reg, cdd-sched_mem + DMA_SCHED_CTRL);
}
-static int init_cppi41(struct platform_device *pdev, struct cppi41_dd *cdd)
+static int init_cppi41(struct device *dev, struct cppi41_dd *cdd)
{
int ret;
BUILD_BUG_ON(QMGR_SCRATCH_SIZE ((1 14) - 1));
- cdd-qmgr_scratch = dma_alloc_coherent(pdev-dev, QMGR_SCRATCH_SIZE,
+ cdd-qmgr_scratch = dma_alloc_coherent(dev, QMGR_SCRATCH_SIZE,
cdd-scratch_phys, GFP_KERNEL);
if (!cdd-qmgr_scratch)
return -ENOMEM;
@@ -827,7 +827,7 @@ static int init_cppi41(struct platform_device *pdev, struct
cppi41_dd *cdd)
cppi_writel(QMGR_SCRATCH_SIZE, cdd-qmgr_mem + QMGR_LRAM_SIZE);
cppi_writel(0, cdd-qmgr_mem + QMGR_LRAM1_BASE);
- ret = init_descs(pdev, cdd);
+ ret = init_descs(dev, cdd);
if (ret)
goto err_td;
@@ -835,7 +835,7 @@ static int init_cppi41(struct platform_device *pdev, struct
cppi41_dd *cdd)
init_sched(cdd);
return 0;
err_td:
- deinit_cpii41(pdev, cdd);
+ deinit_cpii41(dev, cdd);
return ret;
}
@@ -914,11 +914,11 @@ static const struct of_device_id cppi41_dma_ids[] = {
};
MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
-static const struct cppi_glue_infos *get_glue_info(struct platform_device
*pdev)
+static const struct cppi_glue_infos *get_glue_info(struct device *dev)
{
const struct of_device_id *of_id;
- of_id = of_match_node(cppi41_dma_ids, pdev-dev.of_node);
+ of_id = of_match_node(cppi41_dma_ids, dev-of_node);
if (!of_id)
return NULL;
return of_id-data;
@@ -931,7 +931,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
int irq;
[PATCH v2 0/5] dma: cppi41: some trivial fixes and support for suspend/resume
Here are some patches to teach the cppi41 DMA driver support for suspend and resume. Patches 1-3 are simply cosmetic things that emerged during my debugging sessions. Patch #4 is actually a real bugfix which I would like Sebastian Andrzej Siewior to have a look at. Quite frankly, the allocation scheme in this driver and the logic to determine a descriptor's index number seems quite complicated to me, but that's probably a different story. Allocating and freeing the exact same pointer more than once is certainly a bug. Patch #5 adds support for suspend and resume. As the commit log says, the code needed to achive this is the result of a trial-and-error session. This is the minimum that's I'm left with now and which works for me. On a different note, I'm also working on patches for the musb core to make it suspend and resume. Currently, I still have to rmmod/insmod musb_dsps before/after the resume cycle, but I'm hoping to make progress here soon. Thanks, Daniel v1 - v2: * Patch #5: depend on CONFIG_PM_SLEEP rather than on CONFIG_PM and use SIMPLE_DEV_PM_OPS (Reported by Sergei Shtylyov) Daniel Mack (5): dma: cppi41: pass around device instead of platform_device dma: cppi41: s/deinit_cpii41/deinit_cppi41/ dma: cppi41: add shortcut to pdev-dev in cppi41_dma_probe() dma: cppi41: only allocate descriptor memory once dma: cppi41: add support for suspend and resume drivers/dma/cppi41.c | 120 --- 1 file changed, 75 insertions(+), 45 deletions(-) -- 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 v2 3/5] dma: cppi41: add shortcut to pdev-dev in cppi41_dma_probe()
Makes the code more readable and compact. No functional change.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 5469a15..d689706 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -927,11 +927,12 @@ static const struct cppi_glue_infos *get_glue_info(struct
device *dev)
static int cppi41_dma_probe(struct platform_device *pdev)
{
struct cppi41_dd *cdd;
+ struct device *dev = pdev-dev;
const struct cppi_glue_infos *glue_info;
int irq;
int ret;
- glue_info = get_glue_info(pdev-dev);
+ glue_info = get_glue_info(dev);
if (!glue_info)
return -EINVAL;
@@ -946,14 +947,14 @@ static int cppi41_dma_probe(struct platform_device *pdev)
cdd-ddev.device_issue_pending = cppi41_dma_issue_pending;
cdd-ddev.device_prep_slave_sg = cppi41_dma_prep_slave_sg;
cdd-ddev.device_control = cppi41_dma_control;
- cdd-ddev.dev = pdev-dev;
+ cdd-ddev.dev = dev;
INIT_LIST_HEAD(cdd-ddev.channels);
cpp41_dma_info.dma_cap = cdd-ddev.cap_mask;
- cdd-usbss_mem = of_iomap(pdev-dev.of_node, 0);
- cdd-ctrl_mem = of_iomap(pdev-dev.of_node, 1);
- cdd-sched_mem = of_iomap(pdev-dev.of_node, 2);
- cdd-qmgr_mem = of_iomap(pdev-dev.of_node, 3);
+ cdd-usbss_mem = of_iomap(dev-of_node, 0);
+ cdd-ctrl_mem = of_iomap(dev-of_node, 1);
+ cdd-sched_mem = of_iomap(dev-of_node, 2);
+ cdd-qmgr_mem = of_iomap(dev-of_node, 3);
if (!cdd-usbss_mem || !cdd-ctrl_mem || !cdd-sched_mem ||
!cdd-qmgr_mem) {
@@ -961,8 +962,8 @@ static int cppi41_dma_probe(struct platform_device *pdev)
goto err_remap;
}
- pm_runtime_enable(pdev-dev);
- ret = pm_runtime_get_sync(pdev-dev);
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
if (ret)
goto err_get_sync;
@@ -970,22 +971,22 @@ static int cppi41_dma_probe(struct platform_device *pdev)
cdd-queues_tx = glue_info-queues_tx;
cdd-td_queue = glue_info-td_queue;
- ret = init_cppi41(pdev-dev, cdd);
+ ret = init_cppi41(dev, cdd);
if (ret)
goto err_init_cppi;
- ret = cppi41_add_chans(pdev-dev, cdd);
+ ret = cppi41_add_chans(dev, cdd);
if (ret)
goto err_chans;
- irq = irq_of_parse_and_map(pdev-dev.of_node, 0);
+ irq = irq_of_parse_and_map(dev-of_node, 0);
if (!irq)
goto err_irq;
cppi_writel(USBSS_IRQ_PD_COMP, cdd-usbss_mem + USBSS_IRQ_ENABLER);
ret = request_irq(irq, glue_info-isr, IRQF_SHARED,
- dev_name(pdev-dev), cdd);
+ dev_name(dev), cdd);
if (ret)
goto err_irq;
cdd-irq = irq;
@@ -994,7 +995,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
if (ret)
goto err_dma_reg;
- ret = of_dma_controller_register(pdev-dev.of_node,
+ ret = of_dma_controller_register(dev-of_node,
cppi41_dma_xlate, cpp41_dma_info);
if (ret)
goto err_of;
@@ -1009,11 +1010,11 @@ err_irq:
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
cleanup_chans(cdd);
err_chans:
- deinit_cppi41(pdev-dev, cdd);
+ deinit_cppi41(dev, cdd);
err_init_cppi:
- pm_runtime_put(pdev-dev);
+ pm_runtime_put(dev);
err_get_sync:
- pm_runtime_disable(pdev-dev);
+ pm_runtime_disable(dev);
iounmap(cdd-usbss_mem);
iounmap(cdd-ctrl_mem);
iounmap(cdd-sched_mem);
--
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 v2 4/5] dma: cppi41: only allocate descriptor memory once
cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This leads to both
memory leaks and double-frees.
Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
loops.
While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index d689706..3347321 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -727,13 +727,12 @@ static void purge_descs(struct device *dev, struct
cppi41_dd *cdd)
mem_decs = ALLOC_DECS_NUM * sizeof(struct cppi41_desc);
for (i = 0; i DESCS_AREAS; i++) {
-
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMCTRL(i));
-
- dma_free_coherent(dev, mem_decs, cdd-cd,
- cdd-descs_phys);
}
+
+ dma_free_coherent(dev, ALLOC_DECS_NUM * sizeof(struct cppi41_desc),
+ cdd-cd, cdd-descs_phys);
}
static void disable_sched(struct cppi41_dd *cdd)
@@ -755,8 +754,7 @@ static void deinit_cppi41(struct device *dev, struct
cppi41_dd *cdd)
static int init_descs(struct device *dev, struct cppi41_dd *cdd)
{
- unsigned int desc_size;
- unsigned int mem_decs;
+ unsigned int desc_size = sizeof(struct cppi41_desc);
int i;
u32 reg;
u32 idx;
@@ -765,28 +763,25 @@ static int init_descs(struct device *dev, struct
cppi41_dd *cdd)
(sizeof(struct cppi41_desc) - 1));
BUILD_BUG_ON(sizeof(struct cppi41_desc) 32);
BUILD_BUG_ON(ALLOC_DECS_NUM 32);
+ BUILD_BUG_ON(DESCS_AREAS != 1);
- desc_size = sizeof(struct cppi41_desc);
- mem_decs = ALLOC_DECS_NUM * desc_size;
+ cdd-cd = dma_alloc_coherent(dev, ALLOC_DECS_NUM * desc_size,
+cdd-descs_phys, GFP_KERNEL);
+ if (!cdd-cd)
+ return -ENOMEM;
idx = 0;
for (i = 0; i DESCS_AREAS; i++) {
-
reg = idx QMGR_MEMCTRL_IDX_SH;
reg |= (ilog2(desc_size) - 5) QMGR_MEMCTRL_DESC_SH;
reg |= ilog2(ALLOC_DECS_NUM) - 5;
- BUILD_BUG_ON(DESCS_AREAS != 1);
- cdd-cd = dma_alloc_coherent(dev, mem_decs,
- cdd-descs_phys, GFP_KERNEL);
- if (!cdd-cd)
- return -ENOMEM;
-
cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(reg, cdd-qmgr_mem + QMGR_MEMCTRL(i));
idx += ALLOC_DECS_NUM;
}
+
return 0;
}
--
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 v2 5/5] dma: cppi41: add support for suspend and resume
This patch adds support for suspend/resume functionality to the cppi41
DMA driver. The steps neccessary to make the system resume properly were
figured out by hefty trial-and-error. The code as it stands now is the
minimum that has to be done to put the musb host system on an AM33xx
system into an operable state after resume.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 3347321..59dfa8e 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -1040,12 +1040,45 @@ static int cppi41_dma_remove(struct platform_device
*pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int cppi41_suspend(struct device *dev)
+{
+ struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+ cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
+ disable_sched(cdd);
+
+ return 0;
+}
+
+static int cppi41_resume(struct device *dev)
+{
+ struct cppi41_dd *cdd = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i DESCS_AREAS; i++)
+ cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
+
+ init_sched(cdd);
+ cppi_writel(USBSS_IRQ_PD_COMP, cdd-usbss_mem + USBSS_IRQ_ENABLER);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
+
+#define DEV_PM_OPS (cppi41_pm_ops)
+#else
+#define DEV_PM_OPS NULL
+#endif
+
static struct platform_driver cpp41_dma_driver = {
.probe = cppi41_dma_probe,
.remove = cppi41_dma_remove,
.driver = {
.name = cppi41-dma-engine,
.owner = THIS_MODULE,
+ .pm = DEV_PM_OPS,
.of_match_table = of_match_ptr(cppi41_dma_ids),
},
};
--
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
Re: [PATCH 22/51] DMA-API: amba: get rid of separate dma_mask
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King rmk+ker...@arm.linux.org.uk wrote: AMBA Primecell devices always treat streaming and coherent DMA exactly the same, so there's no point in having the masks separated. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk for the drivers/of/platform.c portion: Acked-by: Grant Likely grant.lik...@linaro.org g. -- 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] dma: cppi41: add support for suspend and resume
Hello. On 22-09-2013 15:57, Daniel Mack wrote: This patch adds support for suspend/resume functionality to the cppi41 DMA driver. The steps neccessary to make the system resume properly were Necessary. figured out by hefty trial-and-error. The code as it stands now is the minimum that has to be done to put the musb host system on an AM33xx system into an operable state after resume. Signed-off-by: Daniel Mack zon...@gmail.com --- drivers/dma/cppi41.c | 33 + 1 file changed, 33 insertions(+) diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c index 3347321..59dfa8e 100644 --- a/drivers/dma/cppi41.c +++ b/drivers/dma/cppi41.c @@ -1040,12 +1040,45 @@ static int cppi41_dma_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM_SLEEP [...] +static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume); + +#define DEV_PM_OPS (cppi41_pm_ops) +#else +#define DEV_PM_OPS NULL +#endif You don't need that with SIMPLE_DEV_PM_OPS(), just get it out of #ifdef. WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] dma: cppi41: add support for suspend and resume
On 22.09.2013 16:25, Sergei Shtylyov wrote: On 22-09-2013 15:57, Daniel Mack wrote: +#ifdef CONFIG_PM_SLEEP [...] +static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume); + +#define DEV_PM_OPS (cppi41_pm_ops) +#else +#define DEV_PM_OPS NULL +#endif You don't need that with SIMPLE_DEV_PM_OPS(), just get it out of #ifdef. Ah, now I see it. Thanks for the heads-up! 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
[PATCH v2 1/4] usb: musb: am35x: use SIMPLE_DEV_PM_OPS
This makes am35x_pm_ops const and will stub the struct out in case
CONFIG_PM_SLEEP is not set.
Signed-off-by: Daniel Mack zon...@gmail.com
---
I'm resending this series because I've just learned that
SIMPLE_DEV_PM_OPS will stub itself out in case CONFIG_PM_SLEEP is
not set. That makes the the code even smaller.
drivers/usb/musb/am35x.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 5c310c6..7f1e990 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -615,23 +615,16 @@ static int am35x_resume(struct device *dev)
return 0;
}
-
-static struct dev_pm_ops am35x_pm_ops = {
- .suspend= am35x_suspend,
- .resume = am35x_resume,
-};
-
-#define DEV_PM_OPS am35x_pm_ops
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(am35x_pm_ops, am35x_suspend, am35x_resume);
+
static struct platform_driver am35x_driver = {
.probe = am35x_probe,
.remove = am35x_remove,
.driver = {
.name = musb-am35x,
- .pm = DEV_PM_OPS,
+ .pm = am35x_pm_ops,
},
};
--
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 v2 4/4] usb: musb: ux500: use SIMPLE_DEV_PM_OPS
This removes the DEV_PM_OPS macro and brings this file in line with the
other musb platform drivers.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/musb/ux500.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 59256b1..f483d19 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -376,17 +376,10 @@ static int ux500_resume(struct device *dev)
return 0;
}
-
-static const struct dev_pm_ops ux500_pm_ops = {
- .suspend= ux500_suspend,
- .resume = ux500_resume,
-};
-
-#define DEV_PM_OPS (ux500_pm_ops)
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(ux500_pm_ops, ux500_suspend, ux500_resume);
+
static const struct of_device_id ux500_match[] = {
{ .compatible = stericsson,db8500-musb, },
{}
@@ -397,7 +390,7 @@ static struct platform_driver ux500_driver = {
.remove = ux500_remove,
.driver = {
.name = musb-ux500,
- .pm = DEV_PM_OPS,
+ .pm = ux500_pm_ops,
.of_match_table = ux500_match,
},
};
--
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 v2 2/4] usb: musb: blackfin: use SIMPLE_DEV_PM_OPS
This makes bfin_pm_ops const and will stub the struct out in case
CONFIG_PM_SLEEP is not set.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/musb/blackfin.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
index 72e2056..d9692f7 100644
--- a/drivers/usb/musb/blackfin.c
+++ b/drivers/usb/musb/blackfin.c
@@ -561,23 +561,16 @@ static int bfin_resume(struct device *dev)
return 0;
}
-
-static struct dev_pm_ops bfin_pm_ops = {
- .suspend= bfin_suspend,
- .resume = bfin_resume,
-};
-
-#define DEV_PM_OPS bfin_pm_ops
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(bfin_pm_ops, bfin_suspend, bfin_resume);
+
static struct platform_driver bfin_driver = {
.probe = bfin_probe,
.remove = __exit_p(bfin_remove),
.driver = {
.name = musb-blackfin,
- .pm = DEV_PM_OPS,
+ .pm = bfin_pm_ops,
},
};
--
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 v2 3/4] usb: musb: omap2430: use SIMPLE_DEV_PM_OPS
This makes omap2430_pm_ops const and will stub the struct out in case
CONFIG_PM_SLEEP is not set.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/musb/omap2430.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 59d2245..be42460 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -659,17 +659,12 @@ static int omap2430_runtime_resume(struct device *dev)
return 0;
}
-
-static struct dev_pm_ops omap2430_pm_ops = {
- .runtime_suspend = omap2430_runtime_suspend,
- .runtime_resume = omap2430_runtime_resume,
-};
-
-#define DEV_PM_OPS (omap2430_pm_ops)
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(omap2430_pm_ops,
+omap2430_runtime_suspend,
+omap2430_runtime_resume);
+
#ifdef CONFIG_OF
static const struct of_device_id omap2430_id_table[] = {
{
@@ -688,7 +683,7 @@ static struct platform_driver omap2430_driver = {
.remove = omap2430_remove,
.driver = {
.name = musb-omap2430,
- .pm = DEV_PM_OPS,
+ .pm = omap2430_pm_ops,
.of_match_table = of_match_ptr(omap2430_id_table),
},
};
--
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 v3 0/5] dma: cppi41: some trivial fixes and support for suspend/resume
Here are some patches to teach the cppi41 DMA driver support for suspend and resume. Patches 1-3 are simply cosmetic things that emerged during my debugging sessions. Patch #4 is actually a real bugfix which I would like Sebastian Andrzej Siewior to have a look at. Quite frankly, the allocation scheme in this driver and the logic to determine a descriptor's index number seems quite complicated to me, but that's probably a different story. Allocating and freeing the exact same pointer more than once is certainly a bug. Patch #5 adds support for suspend and resume. As the commit log says, the code needed to achive this is the result of a trial-and-error session. This is the minimum that's I'm left with now and which works for me. On a different note, I'm also working on patches for the musb core to make it suspend and resume. Currently, I still have to rmmod/insmod musb_dsps before/after the resume cycle, but I'm hoping to make progress here soon. Thanks, Daniel v2 - v3: * Patch #5: kill DEV_PM_OPS macro hack and fix a typo (Reported by Sergei Shtylyov) v1 - v2: * Patch #5: depend on CONFIG_PM_SLEEP rather than on CONFIG_PM and use SIMPLE_DEV_PM_OPS (Reported by Sergei Shtylyov) Daniel Mack (5): dma: cppi41: pass around device instead of platform_device dma: cppi41: s/deinit_cpii41/deinit_cppi41/ dma: cppi41: add shortcut to pdev-dev in cppi41_dma_probe() dma: cppi41: only allocate descriptor memory once dma: cppi41: add support for suspend and resume drivers/dma/cppi41.c | 115 +++ 1 file changed, 70 insertions(+), 45 deletions(-) -- 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 v3 2/5] dma: cppi41: s/deinit_cpii41/deinit_cppi41/
Fix a misspelled function name.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 53d1d31..5469a15 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -741,7 +741,7 @@ static void disable_sched(struct cppi41_dd *cdd)
cppi_writel(0, cdd-sched_mem + DMA_SCHED_CTRL);
}
-static void deinit_cpii41(struct device *dev, struct cppi41_dd *cdd)
+static void deinit_cppi41(struct device *dev, struct cppi41_dd *cdd)
{
disable_sched(cdd);
@@ -835,7 +835,7 @@ static int init_cppi41(struct device *dev, struct cppi41_dd
*cdd)
init_sched(cdd);
return 0;
err_td:
- deinit_cpii41(dev, cdd);
+ deinit_cppi41(dev, cdd);
return ret;
}
@@ -1009,7 +1009,7 @@ err_irq:
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
cleanup_chans(cdd);
err_chans:
- deinit_cpii41(pdev-dev, cdd);
+ deinit_cppi41(pdev-dev, cdd);
err_init_cppi:
pm_runtime_put(pdev-dev);
err_get_sync:
@@ -1033,7 +1033,7 @@ static int cppi41_dma_remove(struct platform_device *pdev)
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
free_irq(cdd-irq, cdd);
cleanup_chans(cdd);
- deinit_cpii41(pdev-dev, cdd);
+ deinit_cppi41(pdev-dev, cdd);
iounmap(cdd-usbss_mem);
iounmap(cdd-ctrl_mem);
iounmap(cdd-sched_mem);
--
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 v3 4/5] dma: cppi41: only allocate descriptor memory once
cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This leads to both
memory leaks and double-frees.
Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
loops.
While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index d689706..3347321 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -727,13 +727,12 @@ static void purge_descs(struct device *dev, struct
cppi41_dd *cdd)
mem_decs = ALLOC_DECS_NUM * sizeof(struct cppi41_desc);
for (i = 0; i DESCS_AREAS; i++) {
-
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMCTRL(i));
-
- dma_free_coherent(dev, mem_decs, cdd-cd,
- cdd-descs_phys);
}
+
+ dma_free_coherent(dev, ALLOC_DECS_NUM * sizeof(struct cppi41_desc),
+ cdd-cd, cdd-descs_phys);
}
static void disable_sched(struct cppi41_dd *cdd)
@@ -755,8 +754,7 @@ static void deinit_cppi41(struct device *dev, struct
cppi41_dd *cdd)
static int init_descs(struct device *dev, struct cppi41_dd *cdd)
{
- unsigned int desc_size;
- unsigned int mem_decs;
+ unsigned int desc_size = sizeof(struct cppi41_desc);
int i;
u32 reg;
u32 idx;
@@ -765,28 +763,25 @@ static int init_descs(struct device *dev, struct
cppi41_dd *cdd)
(sizeof(struct cppi41_desc) - 1));
BUILD_BUG_ON(sizeof(struct cppi41_desc) 32);
BUILD_BUG_ON(ALLOC_DECS_NUM 32);
+ BUILD_BUG_ON(DESCS_AREAS != 1);
- desc_size = sizeof(struct cppi41_desc);
- mem_decs = ALLOC_DECS_NUM * desc_size;
+ cdd-cd = dma_alloc_coherent(dev, ALLOC_DECS_NUM * desc_size,
+cdd-descs_phys, GFP_KERNEL);
+ if (!cdd-cd)
+ return -ENOMEM;
idx = 0;
for (i = 0; i DESCS_AREAS; i++) {
-
reg = idx QMGR_MEMCTRL_IDX_SH;
reg |= (ilog2(desc_size) - 5) QMGR_MEMCTRL_DESC_SH;
reg |= ilog2(ALLOC_DECS_NUM) - 5;
- BUILD_BUG_ON(DESCS_AREAS != 1);
- cdd-cd = dma_alloc_coherent(dev, mem_decs,
- cdd-descs_phys, GFP_KERNEL);
- if (!cdd-cd)
- return -ENOMEM;
-
cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(reg, cdd-qmgr_mem + QMGR_MEMCTRL(i));
idx += ALLOC_DECS_NUM;
}
+
return 0;
}
--
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 v3 3/5] dma: cppi41: add shortcut to pdev-dev in cppi41_dma_probe()
Makes the code more readable and compact. No functional change.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 5469a15..d689706 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -927,11 +927,12 @@ static const struct cppi_glue_infos *get_glue_info(struct
device *dev)
static int cppi41_dma_probe(struct platform_device *pdev)
{
struct cppi41_dd *cdd;
+ struct device *dev = pdev-dev;
const struct cppi_glue_infos *glue_info;
int irq;
int ret;
- glue_info = get_glue_info(pdev-dev);
+ glue_info = get_glue_info(dev);
if (!glue_info)
return -EINVAL;
@@ -946,14 +947,14 @@ static int cppi41_dma_probe(struct platform_device *pdev)
cdd-ddev.device_issue_pending = cppi41_dma_issue_pending;
cdd-ddev.device_prep_slave_sg = cppi41_dma_prep_slave_sg;
cdd-ddev.device_control = cppi41_dma_control;
- cdd-ddev.dev = pdev-dev;
+ cdd-ddev.dev = dev;
INIT_LIST_HEAD(cdd-ddev.channels);
cpp41_dma_info.dma_cap = cdd-ddev.cap_mask;
- cdd-usbss_mem = of_iomap(pdev-dev.of_node, 0);
- cdd-ctrl_mem = of_iomap(pdev-dev.of_node, 1);
- cdd-sched_mem = of_iomap(pdev-dev.of_node, 2);
- cdd-qmgr_mem = of_iomap(pdev-dev.of_node, 3);
+ cdd-usbss_mem = of_iomap(dev-of_node, 0);
+ cdd-ctrl_mem = of_iomap(dev-of_node, 1);
+ cdd-sched_mem = of_iomap(dev-of_node, 2);
+ cdd-qmgr_mem = of_iomap(dev-of_node, 3);
if (!cdd-usbss_mem || !cdd-ctrl_mem || !cdd-sched_mem ||
!cdd-qmgr_mem) {
@@ -961,8 +962,8 @@ static int cppi41_dma_probe(struct platform_device *pdev)
goto err_remap;
}
- pm_runtime_enable(pdev-dev);
- ret = pm_runtime_get_sync(pdev-dev);
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
if (ret)
goto err_get_sync;
@@ -970,22 +971,22 @@ static int cppi41_dma_probe(struct platform_device *pdev)
cdd-queues_tx = glue_info-queues_tx;
cdd-td_queue = glue_info-td_queue;
- ret = init_cppi41(pdev-dev, cdd);
+ ret = init_cppi41(dev, cdd);
if (ret)
goto err_init_cppi;
- ret = cppi41_add_chans(pdev-dev, cdd);
+ ret = cppi41_add_chans(dev, cdd);
if (ret)
goto err_chans;
- irq = irq_of_parse_and_map(pdev-dev.of_node, 0);
+ irq = irq_of_parse_and_map(dev-of_node, 0);
if (!irq)
goto err_irq;
cppi_writel(USBSS_IRQ_PD_COMP, cdd-usbss_mem + USBSS_IRQ_ENABLER);
ret = request_irq(irq, glue_info-isr, IRQF_SHARED,
- dev_name(pdev-dev), cdd);
+ dev_name(dev), cdd);
if (ret)
goto err_irq;
cdd-irq = irq;
@@ -994,7 +995,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
if (ret)
goto err_dma_reg;
- ret = of_dma_controller_register(pdev-dev.of_node,
+ ret = of_dma_controller_register(dev-of_node,
cppi41_dma_xlate, cpp41_dma_info);
if (ret)
goto err_of;
@@ -1009,11 +1010,11 @@ err_irq:
cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
cleanup_chans(cdd);
err_chans:
- deinit_cppi41(pdev-dev, cdd);
+ deinit_cppi41(dev, cdd);
err_init_cppi:
- pm_runtime_put(pdev-dev);
+ pm_runtime_put(dev);
err_get_sync:
- pm_runtime_disable(pdev-dev);
+ pm_runtime_disable(dev);
iounmap(cdd-usbss_mem);
iounmap(cdd-ctrl_mem);
iounmap(cdd-sched_mem);
--
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
Re: [PATCH v2 3/4] usb: musb: omap2430: use SIMPLE_DEV_PM_OPS
Hello.
On 22-09-2013 18:43, Daniel Mack wrote:
This makes omap2430_pm_ops const and will stub the struct out in case
CONFIG_PM_SLEEP is not set.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/musb/omap2430.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 59d2245..be42460 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -659,17 +659,12 @@ static int omap2430_runtime_resume(struct device *dev)
return 0;
}
-
-static struct dev_pm_ops omap2430_pm_ops = {
- .runtime_suspend = omap2430_runtime_suspend,
- .runtime_resume = omap2430_runtime_resume,
-};
-
-#define DEV_PM_OPS (omap2430_pm_ops)
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(omap2430_pm_ops,
+omap2430_runtime_suspend,
+omap2430_runtime_resume);
No, SIMPLE_DEV_PM_OPS() won't do here, it's runtime PM, didn't you see?
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] usb: musb: omap2430: use SIMPLE_DEV_PM_OPS
On 22.09.2013 16:59, Sergei Shtylyov wrote:
On 22-09-2013 18:43, Daniel Mack wrote:
This makes omap2430_pm_ops const and will stub the struct out in case
CONFIG_PM_SLEEP is not set.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/musb/omap2430.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 59d2245..be42460 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -659,17 +659,12 @@ static int omap2430_runtime_resume(struct device *dev)
return 0;
}
-
-static struct dev_pm_ops omap2430_pm_ops = {
-.runtime_suspend = omap2430_runtime_suspend,
-.runtime_resume = omap2430_runtime_resume,
-};
-
-#define DEV_PM_OPS (omap2430_pm_ops)
-#else
-#define DEV_PM_OPS NULL
#endif
+static SIMPLE_DEV_PM_OPS(omap2430_pm_ops,
+ omap2430_runtime_suspend,
+ omap2430_runtime_resume);
No, SIMPLE_DEV_PM_OPS() won't do here, it's runtime PM, didn't you see?
Eh, you're right, sorry. Whoever applies this series, just drop that one
patch. The rest of this series is not affected.
Sergei, thanks for the review 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: [PATCH] usb/core/devio.c:tolerate wrong direction flag in control endpoints
On Sun, 22 Sep 2013, Kurt Garloff wrote:
Hi,
USB devio rejects control messages when the index does not have the
direction bit set correctly.
I wouldn't describe it that way. It would be more accurate to say that
the message is rejected when wIndex contains an invalid endpoint
address.
This breaks windows apps in KVM -- and might be overly strict according
to my reading of USB HID spec.
It is not overly strict.
Attached patch makes the kernel tolerant against it and makes the app
work for me.
More details in the patch header.
USB experts: Please review this and judge whether this is correct,
applies more generically,
or maybe needs to be special cased (only for USB HID devices?) or
implemented as quirk
or module/kernel parameter.
The patch seems reasonable enough, although the description needs
improvement and a couple of minor things should be fixed. More
comments below.
In the future, please put patches inline with the rest of the email
message. Don't make them attachments unless you have to; that way they
become harder to read and comment on.
Once in the final form, this *might* be stable material.
Yes, it should be.
commit bc1e4e1ae1d5a4f9b2d263f22c651dd5ba4f8ff9
Author: Kurt Garloff k...@garloff.de
Date: Sun Sep 22 11:54:59 2013 +0200
From: Kurt Garloff k...@garloff.de
Subject: Tolerate wrong direction bit endpoint index for control messages
Signed-off-by: Kurt Garloff k...@garloff.de
Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101)
[1] with the Windows App (EasyNote) works natively but fails when
WIndows is running under KVM (and the USB device handed to KVM).
The reason is a USB control message
usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200
wIndex=0001 wLength=0008
This goes to endpoint 1 (wIndex), however, the endpoint is an input
endpoint and thus enumerated 0x81.
The kernel thus rejects the IO and thus we see the failure.
Apparently, Linux is more strict here than Windows ... we can't change
the Win app easily, so that's a problem.
Indeed, this indicates that the device itself is also buggy. If it
worked correctly, it would reject the incorrect control message.
However, the app might not even be buggy here. Browsing the USB HID
spec, there's a note that the bit for direction is ignored for control
endpoints ... so Linux might be overly strict?
No, Linux is correct. While it is true that the direction bit is
ignored for control endpoints, in this case we are talking about
endpoint 1, which is not a control endpoint. In an HID device, it is
almost certainly an interrupt endpoint.
With attached patch, everything works.
usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep 01
but needs 81 (or 00)
Notes:
- I have not checked whether the ignorance of the direction bit for
control endpoints only applies to USB HID devices, thus I have not
special-cased depending on the device type.
It applies to all devices, but it isn't relevant here.
- We do output a warning into the kernel log, as this might still be
caused by an application error.
- We don't risk sending to a wrong endpoint, as there can't be two
endpoints with same index and just different direction.
Of course there can be. It is entirely possible to have one endpoint
with address 0x01 and another with address 0x81.
However, your patch doesn't run into this problem. If both endpoints
exist, the routine will use the one with the address given by wIndex
and ignore the other -- your new code won't run at all.
- I suspect this will mostly affect apps in virtual environments; as on
Linux the apps would have been adapted to the stricter handling of the
kernel. I have done that for mine[2], although delaying the release by
two years
[1} http://www.pegatech.com/
[2] https://sourceforge.net/projects/notetakerpen/
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index c2f62a3..8acbc2f 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -623,6 +623,19 @@ static int check_ctrlrecip(struct dev_state *ps,
unsigned int requesttype,
switch (requesttype USB_RECIP_MASK) {
case USB_RECIP_ENDPOINT:
ret = findintfep(ps-dev, index);
+ if (ret 0) {
+ /* OK, give it a second try -- user might have confused
+ * direction -- this happens from virtualized win apps
+ * e.g. -- warn, but be graceful */
/*
* The accepted format for multiline comments
* looks like this.
*/
+ ret = findintfep(ps-dev, index ^ 0x80);
+ if (ret = 0)
+ dev_info(ps-dev-dev ,
+
Re: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
On Wed, 18 Sep 2013, Marc MERLIN wrote: Howdy, I have a T530 with a built in webcam that uses the uvcvideo driver. Kernel 3.10.6, but the problem has been there for many kernel versions. From time to time (not always) it shows up at the top of powertop with an unexplained high power use while I'm not using the camera. Powertop says autosuspend is active: Note that tunables says: Good Autosuspend for USB device Integrated Camera [Chicony Electronics Co., Ltd.] Unloading uvcvideo makes no difference. However if I never load uvcvideo and never use the camera after booting the laptop, then the device doesn't use any power. This is better than nothing, but not very desirable since I do use the camera from time to google for video conferencing. Here's the module info gandalfthegreat:/sys/bus/usb/drivers/uvcvideo# l total 0 drwxr-xr-x 2 root root0 Sep 18 17:48 ./ drwxr-xr-x 12 root root0 Sep 17 15:10 ../ lrwxrwxrwx 1 root root0 Sep 18 21:30 3-1.6:1.0 - ../../../../devices/pci:00/:00:1a.0/usb3/3-1/3-1.6/3-1.6:1.0/ lrwxrwxrwx 1 root root0 Sep 18 21:30 3-1.6:1.1 - ../../../../devices/pci:00/:00:1a.0/usb3/3-1/3-1.6/3-1.6:1.1/ --w--- 1 root root 4096 Sep 18 21:30 bind lrwxrwxrwx 1 root root0 Sep 18 21:30 module - ../../../../module/uvcvideo/ -rw-r--r-- 1 root root 4096 Sep 18 21:30 new_id -rw-r--r-- 1 root root 4096 Sep 18 21:30 remove_id --w--- 1 root root 4096 Sep 18 17:48 uevent --w--- 1 root root 4096 Sep 18 21:30 unbind gandalfthegreat:/sys/bus/usb/devices/3-1.6/power# grep . * active_duration:61227648 async:enabled autosuspend:2 autosuspend_delay_ms:2000 connected_duration:66830880 control:auto level:auto persist:1 runtime_active_kids:0 runtime_active_time:18870052 runtime_enabled:enabled runtime_status:active runtime_suspended_time:5324088 runtime_usage:0 This all looks correct. Any ideas? You might get more information from a kernel with CONFIG_USB_DEBUG enabled. Especially if you add #define VERBOSE_DEBUG to drivers/usb/core/driver.c before the first #include line. 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: problematic device
Cleware GMBH is a German company producing all kinds of USB devices. Switches, lights, sensors, etc. As a hobbyist, I do the Linux support for them (I wrote a userspace program to control their devices). Now a new range of products are to be released with new firmware. I think I'm seeing problems with them and I would like to discuss them here before bothering Cleware. When the device is plugged in, I can access it, somewhat. Why somewhat? Because it looks as if it is thrown out and reconnected each 6-7 seconds: [715607.610601] usb 3-3: USB disconnect, device number 93 [715607.976312] usb 3-3: new low-speed USB device number 94 using xhci_hcd [715607.997641] usb 3-3: New USB device found, idVendor=0d50, idProduct=0010 [715607.997644] usb 3-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [715607.997645] usb 3-3: Product: USB-Temp [715607.997647] usb 3-3: Manufacturer: Cleware GmbH [715607.997648] usb 3-3: SerialNumber: 63813 [715607.997807] usb 3-3: ep 0x81 - rounding interval to 64 microframes, ep desc says 80 microframes [715607.997810] usb 3-3: ep 0x2 - rounding interval to 64 microframes, ep desc says 80 microframes [715608.000742] hid-generic 0003:0D50:0010.0934: hiddev0,hidraw1: USB HID v1.11 Device [Cleware GmbH USB-Temp] on usb-:00:14.0-3/input0 [715614.701038] usb 3-3: USB disconnect, device number 94 [715615.070236] usb 3-3: new low-speed USB device number 95 using xhci_hcd Does the device behave the same way if you plug it into a USB-2 port instead of a USB-3 port? This alienware laptop seems to have only usb3 ports: they look all the same for color and description. So I tried it on my asus eee pc 1015 (seashell) and there it behaves the same with kernel 3.9.1 and 3.2.0. Oh and on my raspberry pi with 3.9.8 kernel it's also doing this. My question now is: is this a Linux kernel problem? Or a problem with the firmware of these devices? If it is the latter case, please (please) tell me (if possible) what it is doing wrong. There's no way to tell for certain from just the kernel log. However, if the device behaves differently when plugged into a USB-2 port then most likely the problem isn't in the device. Folkert van Heusden -- www.vanheusden.com/multitail - win een vlaai van multivlaai! zorg ervoor dat multitail opgenomen wordt in Fedora Core, AIX, Solaris of HP/UX en win een vlaai naar keuze -- Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: Fix race between ep halt and URB cancellation
Hi Sarah,
the described Problem also exisits on actual Linux Kernel 3.12-rc so I
rebased my patch to this.
--
The halted state of a endpoint cannot be cleared over CLEAR_HALT from a
user process, because the stopped_td variable was overwritten in the
handle_stopped_endpoint() function. So the xhci_endpoint_reset() function will
refuse the reset and communication with device can not run over this endpoint.
https://bugzilla.kernel.org/show_bug.cgi?id=60699
Signed-off-by: Florian Wolter woll...@web.de
---
drivers/usb/host/xhci-ring.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c47f90e..411da1f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -859,8 +859,12 @@ remove_finished_td:
/* Otherwise ring the doorbell(s) to restart queued transfers */
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
- ep-stopped_td = NULL;
- ep-stopped_trb = NULL;
+
+ /* Clear stopped_td and stopped_trb if endpoint is not halted */
+ if (!(ep-ep_state EP_HALTED)) {
+ ep-stopped_td = NULL;
+ ep-stopped_trb = NULL;
+ }
/*
* Drop the lock and complete the URBs in the cancelled TD list.
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
On Sun, Sep 22, 2013 at 12:38:56PM -0400, Alan Stern wrote: gandalfthegreat:/sys/bus/usb/devices/3-1.6/power# grep . * active_duration:61227648 async:enabled autosuspend:2 autosuspend_delay_ms:2000 connected_duration:66830880 control:auto level:auto persist:1 runtime_active_kids:0 runtime_active_time:18870052 runtime_enabled:enabled runtime_status:active runtime_suspended_time:5324088 runtime_usage:0 This all looks correct. Since then, I've confirmed that I don't have the problem some time after reboot. It may be that the device doesn't seem to sleep well after I've used it once. What's interesting, is that I see this when power is plugged in: Power est. Events/sCategory Description 8.18 W100.0% Device USB device: Yubico Yubikey II (Yubico) 8.13 W100.0% Device USB device: Integrated Camera (Chicony Electronics Co., Ltd.) Somehow I know that my Yubikey isn't using 8W, so powertop numbers need to be taken with a grain of salt. Once I go to batteries, I see this: Summary: 760.1 wakeups/second, 718.9 GPU ops/seconds, 0.0 VFS ops/sec and 6.9% CPU use Power est. Usage Events/sCategory Description 8.32 W100.0% Device USB device: Yubico Yubikey II (Yubico) 2.52 W 73.3% Device Display backlight So at least for now, the camera does sleep ok, until later when it probably won't again. I'm somehow thinking there is a driver or hardware problem when the device does get stuck in a mode where it won't sleep properly again until the next reboot (just unloading/loading the driver does not fix this). Any ideas? You might get more information from a kernel with CONFIG_USB_DEBUG enabled. Especially if you add #define VERBOSE_DEBUG to drivers/usb/core/driver.c before the first #include line. Do you think thaty would help debug the problem above, or not really? I'm starting to think that the USB layer is not at fault, although I could be wrong I suppose. Thanks, Marc -- A mouse is a device used to point at the xterm you want to type in - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.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: problematic device
On Sun, 22 Sep 2013, folkert wrote: Cleware GMBH is a German company producing all kinds of USB devices. Switches, lights, sensors, etc. As a hobbyist, I do the Linux support for them (I wrote a userspace program to control their devices). Now a new range of products are to be released with new firmware. I think I'm seeing problems with them and I would like to discuss them here before bothering Cleware. When the device is plugged in, I can access it, somewhat. Why somewhat? Because it looks as if it is thrown out and reconnected each 6-7 seconds: [715607.610601] usb 3-3: USB disconnect, device number 93 [715607.976312] usb 3-3: new low-speed USB device number 94 using xhci_hcd [715607.997641] usb 3-3: New USB device found, idVendor=0d50, idProduct=0010 [715607.997644] usb 3-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [715607.997645] usb 3-3: Product: USB-Temp [715607.997647] usb 3-3: Manufacturer: Cleware GmbH [715607.997648] usb 3-3: SerialNumber: 63813 [715607.997807] usb 3-3: ep 0x81 - rounding interval to 64 microframes, ep desc says 80 microframes [715607.997810] usb 3-3: ep 0x2 - rounding interval to 64 microframes, ep desc says 80 microframes [715608.000742] hid-generic 0003:0D50:0010.0934: hiddev0,hidraw1: USB HID v1.11 Device [Cleware GmbH USB-Temp] on usb-:00:14.0-3/input0 [715614.701038] usb 3-3: USB disconnect, device number 94 [715615.070236] usb 3-3: new low-speed USB device number 95 using xhci_hcd Does the device behave the same way if you plug it into a USB-2 port instead of a USB-3 port? This alienware laptop seems to have only usb3 ports: they look all the same for color and description. So I tried it on my asus eee pc 1015 (seashell) and there it behaves the same with kernel 3.9.1 and 3.2.0. Oh and on my raspberry pi with 3.9.8 kernel it's also doing this. Then the problem is most likely in the device or the firmware. If you get a usbmon trace, it might provide more information. You asked what the firmware is doing wrong, but you already know the answer: It disconnects the device every 6-7 seconds. Perhaps it does this because it doesn't like the commands it receives from the computer. 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: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
On Sun, 22 Sep 2013, Marc MERLIN wrote: On Sun, Sep 22, 2013 at 12:38:56PM -0400, Alan Stern wrote: gandalfthegreat:/sys/bus/usb/devices/3-1.6/power# grep . * active_duration:61227648 async:enabled autosuspend:2 autosuspend_delay_ms:2000 connected_duration:66830880 control:auto level:auto persist:1 runtime_active_kids:0 runtime_active_time:18870052 runtime_enabled:enabled runtime_status:active runtime_suspended_time:5324088 runtime_usage:0 This all looks correct. Since then, I've confirmed that I don't have the problem some time after reboot. It may be that the device doesn't seem to sleep well after I've used it once. What's interesting, is that I see this when power is plugged in: Power est. Events/sCategory Description 8.18 W100.0% Device USB device: Yubico Yubikey II (Yubico) 8.13 W100.0% Device USB device: Integrated Camera (Chicony Electronics Co., Ltd.) Somehow I know that my Yubikey isn't using 8W, so powertop numbers need to be taken with a grain of salt. I don't know where powertop gets its numbers from. Perhaps it uses the value reported by the device (bMaxPower). That value is only a maximum; it doesn't change to reflect the actual usage. Once I go to batteries, I see this: Summary: 760.1 wakeups/second, 718.9 GPU ops/seconds, 0.0 VFS ops/sec and 6.9% CPU use Power est. Usage Events/sCategory Description 8.32 W100.0% Device USB device: Yubico Yubikey II (Yubico) 2.52 W 73.3% Device Display backlight So at least for now, the camera does sleep ok, until later when it probably won't again. I'm somehow thinking there is a driver or hardware problem when the device does get stuck in a mode where it won't sleep properly again until the next reboot (just unloading/loading the driver does not fix this). That's quite possible. But if it is a driver problem, wouldn't unloading and reloading the driver fix it? You might get more information from a kernel with CONFIG_USB_DEBUG enabled. Especially if you add #define VERBOSE_DEBUG to drivers/usb/core/driver.c before the first #include line. Do you think thaty would help debug the problem above, or not really? I'm There's no way to know in advance. starting to think that the USB layer is not at fault, although I could be wrong I suppose. You asked for advice on things to try, and I suggested something. That's the best I can do. 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: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
On Sun, Sep 22, 2013 at 04:32:08PM -0400, Alan Stern wrote: I'm somehow thinking there is a driver or hardware problem when the device does get stuck in a mode where it won't sleep properly again until the next reboot (just unloading/loading the driver does not fix this). That's quite possible. But if it is a driver problem, wouldn't unloading and reloading the driver fix it? You'd think that but it hasn't so far with this one device. You might get more information from a kernel with CONFIG_USB_DEBUG enabled. Especially if you add #define VERBOSE_DEBUG to drivers/usb/core/driver.c before the first #include line. Do you think thaty would help debug the problem above, or not really? I'm There's no way to know in advance. starting to think that the USB layer is not at fault, although I could be wrong I suppose. You asked for advice on things to try, and I suggested something. That's the best I can do. Understood, just making sure this was still potentially useful considering what I found out since my last message. Thanks for your help, Marc -- A mouse is a device used to point at the xterm you want to type in - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.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: [PATCH] usb/core/devio.c:tolerate wrong direction flag in control endpoints
Hi Alan, thanks for your review and your constructive comments! Alan Stern st...@rowland.harvard.edu schrieb: On Sun, 22 Sep 2013, Kurt Garloff wrote: Hi, USB devio rejects control messages when the index does not have the direction bit set correctly. I wouldn't describe it that way. It would be more accurate to say that the message is rejected when wIndex contains an invalid endpoint address. Well, this seems to be a question of terminology, no? I saw the endpoint byte as consisting of endpoint index plus the direction bit. This breaks windows apps in KVM -- and might be overly strict according to my reading of USB HID spec. It is not overly strict. OK, you certainly know the USB specs better than I do. If the message is not according to spec because the windex byte (which should reference the endpoint) has the endpoint direction flag wrong, then the question may become whether the kernel should reject it or leave that to the device. Rejecting by the kernel has the risk that somewhat non-compliant devices with somewhat non-compliant (userspace) drivers are prevented from working. While not rejecting has the risk that overly sensitive devices might freak out on receiving a non-compliant transfer. The fact that Win does not not seem to filter here however might make that risk rather small. (Long years have taught us that companies don't test against the spec but this OS from Redmond.) Attached patch makes the kernel tolerant against it and makes the app work for me. More details in the patch header. USB experts: Please review this and judge whether this is correct, applies more generically, or maybe needs to be special cased (only for USB HID devices?) or implemented as quirk or module/kernel parameter. The patch seems reasonable enough, although the description needs improvement and a couple of minor things should be fixed. More comments below. In the future, please put patches inline with the rest of the email message. Don't make them attachments unless you have to; that way they become harder to read and comment on. Seems I have not sent a patch for too long, so I failed to remember this longstanding rule when sending. Sorry for this! Once in the final form, this *might* be stable material. Yes, it should be. commit bc1e4e1ae1d5a4f9b2d263f22c651dd5ba4f8ff9 Author: Kurt Garloff k...@garloff.de Date: Sun Sep 22 11:54:59 2013 +0200 From: Kurt Garloff k...@garloff.de Subject: Tolerate wrong direction bit endpoint index for control messages Signed-off-by: Kurt Garloff k...@garloff.de Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101) [1] with the Windows App (EasyNote) works natively but fails when WIndows is running under KVM (and the USB device handed to KVM). The reason is a USB control message usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008 This goes to endpoint 1 (wIndex), however, the endpoint is an input endpoint and thus enumerated 0x81. The kernel thus rejects the IO and thus we see the failure. Apparently, Linux is more strict here than Windows ... we can't change the Win app easily, so that's a problem. Indeed, this indicates that the device itself is also buggy. If it worked correctly, it would reject the incorrect control message. It seems to interpret wIndex differently indeed. Not sure whether that qualifies as a bug or not. Maybe it should not claim to be a HID device then? Looking at the code, it seems that printers do something strange here, and it might be that the device in question here is not 100% HID in that it also assumes a non-standard meaning to this byte. Strange enough, the app does want to talk to the control interface it seems: Sep 19 09:47:21 nehalem kernel: [44539.730269] usb 4-2.2: usbdev_do_ioctl: SUBMITURB Sep 19 09:47:21 nehalem kernel: [44539.730276] usb 4-2.2: proc_submiturb: URB 02 00 0 01a83420 16 0 Sep 19 09:47:21 nehalem kernel: [44539.730280] usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008 Sep 19 09:47:21 nehalem kernel: [44539.730285] usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep 01 but needs 81 (or 00) Sep 19 09:47:21 nehalem kernel: [44539.730290] usb 4-2.2: userurb 01b56f00, ep0 ctrl-out, length 8 Sep 19 09:47:21 nehalem kernel: [44539.730294] data: 02 01 b5 00 00 00 00 00 Sep 19 09:47:21 nehalem kernel: [44539.730301] usb 4-2.2: proc_submiturb: return 0 The second line here comes from instrumentation I have inserted in proc_submiturb(): snoop(ps-dev-dev, %s: URB %02x %02x %i %08x %p %i %i\n, __func__, uurb.type, uurb.endpoint, uurb.status, uurb.flags, uurb.buffer, uurb.buffer_length, uurb.actual_length); and it shows that
Re: [alsa-devel] Buffer size for ALSA USB PCM audio
On Thu, 29 Aug 2013, Alan Stern wrote: On Thu, 29 Aug 2013, James Stone wrote: On Wed, Aug 28, 2013 at 7:46 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 28 Aug 2013, Clemens Ladisch wrote: Sorry, what I said applies more to explicit sync endpoints. When using implicit sync, a playback URB is submitted for each completed capture URB, with the number of samples per packet identical to the corresponding capture packet, so the parameters must be identical. Got it. Below is an updated patch. James, the problem you encountered was most likely a result of the faulty treatment of capture endpoints that Clemens pointed out. It was quite obvious in the usbmon traces that the unpatched driver used 8 packets per URB whereas the patched driver used 22. This updated patch should fix that problem. Works fine. With this, it is possible to get clear playback at 64 frames/period too. With vanilla 3.10.9, there was some glitchy distortion to the sound at that latency, so this seems to be an improvement. That's good. What happens if you push frames/period even lower? Daniel and Eldad, more testing of the most recent proposed patch would be welcome. Sorry for the long response time but I've been busy lately. I see a good improvement on my card (M-Audio C400, implicit feedback) - using a 48 frame period (@48kHz) I get no xruns with jack running without clients. Without the patch, the above test generates constant xruns. I tested the patch with mainline 3.11.0, and let jack run about 5 minutes. Thank you for working on this! FWIW, feel free to add: Tested-by: Eldad Zack el...@fogrefinery.com Cheers, Eldad -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: use usb_composite_dev for usb_function_{de}activate
On Tue, Sep 17, 2013 at 12:39:51PM -0500, Felipe Balbi wrote:
Hi,
On Tue, Sep 17, 2013 at 07:08:12PM +0200, Michael Grzeschik wrote:
On Wed, Sep 11, 2013 at 09:01:39AM +0200, Michael Grzeschik wrote:
On Tue, Sep 10, 2013 at 04:19:40PM +0200, Michael Grzeschik wrote:
usb_function_{de}activate makes it possible to have several gadget
functions, inside one composite device, to be able to prevent the
gadget
to enumerate until an prepared condition has triggered (i.e. userspace
daemon has started)
This patch simplifies and renames the functions to its actual meaning
usb_cdev_{de}activate and fixes all its users. As the code is counting
the deactivations in the current cdev.
I was working on this also to be used in udc-core for udc_bind_to_driver.
Because the current implementation will call usb_gadget_connect without
checking for delaying functions. But for that the whole mechanism needs
to be ported to gadget level.
What do you think about usb_gadget_deactivate/usb_gadget_activate ?
Just a short ping on that mails. You probably mist them, as
I replied to myself in the first place! ;-)
We could certainly add usb_gadget_activate/deactivate functions, but I'm
not sure what's best to implement:
a) entirely new functions
b) add refcounting to usb_gadget_connect/disconnect
Also, we can't simply force all functions to be deactivated and forget
about everything else. Not all functions are using
usb_function_activate/deactivate as of today. We could, rather easily,
just add those calls to such functions and force the conversion.
what do you think ?
I think we will only need usb_gadget_activate/deactivate in the end.
Forcing the conversion with all functions implementing
activate/deactivate is necessary, to let the framework behave reliable.
This way there will be no need for the seperate set of
usb_function_activate/dectivate anyway.
Regards,
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: Cannot shutdown power use from built in webcam in thinkpad T530 questions]
On Thu, Sep 19, 2013 at 12:35 PM, Marc MERLIN m...@merlins.org wrote: gandalfthegreat:/sys/bus/usb/devices/3-1.6/power# grep . * active_duration:61227648 async:enabled autosuspend:2 autosuspend_delay_ms:2000 connected_duration:66830880 control:auto level:auto persist:1 runtime_active_kids:0 runtime_active_time:18870052 runtime_enabled:enabled runtime_status:active runtime_suspended_time:5324088 runtime_usage:0 The uvc device should have been put in auto-suspend state if no application is using the device. You can check if there is application which is using the device by 'lsof | grep /dev/video'. Also looks power usage estimation of 'power top' should be used when battery provides power, and the data isn't correct if power is provided from power adapter, if I remember correctly. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb/core/devio.c:tolerate wrong direction flag in control endpoints
On Sun, 22 Sep 2013, Kurt Garloff wrote: Hi Alan, thanks for your review and your constructive comments! You're welcome. Hi, USB devio rejects control messages when the index does not have the direction bit set correctly. I wouldn't describe it that way. It would be more accurate to say that the message is rejected when wIndex contains an invalid endpoint address. Well, this seems to be a question of terminology, no? I saw the endpoint byte as consisting of endpoint index plus the direction bit. See the entry for Endpoint Address in Chapter 2 (Terms and Abbreviations) of the USB 2.0 specification. The definition says: The combination of an endpoint number and an endpoint direction on a USB device. Each endpoint address supports data transfer in one direction. This breaks windows apps in KVM -- and might be overly strict according to my reading of USB HID spec. It is not overly strict. OK, you certainly know the USB specs better than I do. If the message is not according to spec because the windex byte (which should reference the endpoint) has the endpoint direction flag wrong, then the question may become whether the kernel should reject it or leave that to the device. Rejecting by the kernel has the risk that somewhat non-compliant devices with somewhat non-compliant (userspace) drivers are prevented from working. While not rejecting has the risk that overly sensitive devices might freak out on receiving a non-compliant transfer. The fact that Win does not not seem to filter here however might make that risk rather small. (Long years have taught us that companies don't test against the spec but this OS from Redmond.) This is a good explanation of why the patch should be accepted. From: Kurt Garloff k...@garloff.de Subject: Tolerate wrong direction bit endpoint index for control messages Signed-off-by: Kurt Garloff k...@garloff.de Trying to read data from the Pegasus Technologies NoteTaker (0e20:0101) [1] with the Windows App (EasyNote) works natively but fails when WIndows is running under KVM (and the USB device handed to KVM). The reason is a USB control message usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008 This goes to endpoint 1 (wIndex), however, the endpoint is an input endpoint and thus enumerated 0x81. The kernel thus rejects the IO and thus we see the failure. Apparently, Linux is more strict here than Windows ... we can't change the Win app easily, so that's a problem. Indeed, this indicates that the device itself is also buggy. If it worked correctly, it would reject the incorrect control message. It seems to interpret wIndex differently indeed. Not sure whether that qualifies as a bug or not. Maybe it should not claim to be a HID device then? Maybe not. This particular combination of bRequestType and bRequest values (0x22, 0x09) is not defined in the HID 1.11 spec. Do you know if it is defined somewhere else? Looking at the code, it seems that printers do something strange here, and it might be that the device in question here is not 100% HID in that it also assumes a non-standard meaning to this byte. Strange enough, the app does want to talk to the control interface it seems: Sep 19 09:47:21 nehalem kernel: [44539.730269] usb 4-2.2: usbdev_do_ioctl: SUBMITURB Sep 19 09:47:21 nehalem kernel: [44539.730276] usb 4-2.2: proc_submiturb: URB 02 00 0 01a83420 16 0 Sep 19 09:47:21 nehalem kernel: [44539.730280] usb 4-2.2: control urb: bRequestType=22 bRequest=09 wValue=0200 wIndex=0001 wLength=0008 Sep 19 09:47:21 nehalem kernel: [44539.730285] usb 4-2.2: check_ctrlrecip: process 13073 (qemu-kvm) requesting ep 01 but needs 81 (or 00) Sep 19 09:47:21 nehalem kernel: [44539.730290] usb 4-2.2: userurb 01b56f00, ep0 ctrl-out, length 8 Sep 19 09:47:21 nehalem kernel: [44539.730294] data: 02 01 b5 00 00 00 00 00 Sep 19 09:47:21 nehalem kernel: [44539.730301] usb 4-2.2: proc_submiturb: return 0 The second line here comes from instrumentation I have inserted in proc_submiturb(): snoop(ps-dev-dev, %s: URB %02x %02x %i %08x %p %i %i\n, __func__, uurb.type, uurb.endpoint, uurb.status, uurb.flags, uurb.buffer, uurb.buffer_length, uurb.actual_length); and it shows that uurb.endpoint actually is set to 0. That's right. The URB is sent to endpoint 0. The value in bRequestType indicates that the recipient of the request is an endpoint, and the value of wIndex indicates which endpoint. So even though the request is sent to endpoint 0, it actually refers to endpoint 0x01 (or 0x81, as the case may be). As an analogy, consider the Clear-Halt request. This
[PATCH] usb: dwc3: Remove additional delay of 100ms when resuming
This delay got introduced in: 7415f17 usb: dwc3: core: add power management support which reflected similar code in dwc3_core_soft_reset() function. However, originally the delay of 100ms in dwc3_core_soft_reset() was meant to assist USB2PHY and USB3PHY reset, not for usb_phy_init() sequence. We should get rid of this delay, since things will still work fine without this. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- Hi Felipe, I remember this change for phy_init including msleep(100) was suggested by me, after testing the patch-series for PM support to dwc3. Sorry for that !! drivers/usb/dwc3/core.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 474162e..e88ffae 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -691,7 +691,6 @@ static int dwc3_resume(struct device *dev) usb_phy_init(dwc-usb3_phy); usb_phy_init(dwc-usb2_phy); - msleep(100); spin_lock_irqsave(dwc-lock, flags); -- 1.7.6.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No keyboard and mouse usb detected at startup.
Hi Alan, I have one question just for knowledge. I was looking for the reason of this bug but wasn't even able to identify which driver (ehci,xhci etc) these devices are using. I tried inspecting with my USB mouse and found out it was ehci (i believe it depends on devices). My configuration files are almost similar between 3.10 and 3.11... Attached 2 lsusb -v files, 1 that works, the other doesn't. I use debian wheezy with LUKS. Thank you, best regards Note: Same problem with the kernel 3.12rc1. It looks like your system did not load the ohci-pci driver. Is CONFIG_USB_OHCI_HCD_PCI enabled? how did you identified it was using ohci? Please reply. Regards Kumar Gaurav -- 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 v3 5/5] dma: cppi41: add support for suspend and resume
On Sun, Sep 22, 2013 at 04:50:04PM +0200, Daniel Mack wrote:
This patch adds support for suspend/resume functionality to the cppi41
DMA driver. The steps necessary to make the system resume properly were
figured out by trial-and-error. The code as it stands now is the
minimum that has to be done to put the musb host system on an AM33xx
system into an operable state after resume.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/dma/cppi41.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 3347321..89decc9 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -1040,12 +1040,41 @@ static int cppi41_dma_remove(struct platform_device
*pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
a
+static int cppi41_suspend(struct device *dev)
+{
+ struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+ cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
+ disable_sched(cdd);
+
+ return 0;
+}
+
+static int cppi41_resume(struct device *dev)
+{
+ struct cppi41_dd *cdd = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i DESCS_AREAS; i++)
+ cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
+
+ init_sched(cdd);
+ cppi_writel(USBSS_IRQ_PD_COMP, cdd-usbss_mem + USBSS_IRQ_ENABLER);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
Here is the macro in pm.h
#ifdef CONFIG_PM_SLEEP
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
.suspend = suspend_fn, \
.resume = resume_fn, \
.freeze = suspend_fn, \
.thaw = resume_fn, \
.poweroff = suspend_fn, \
.restore = resume_fn,
#else
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
#endif
/*
* Use this if you want to use the same suspend and resume callbacks for suspend
* to RAM and hibernation.
*/
#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
const struct dev_pm_ops name = { \
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
Now since you are using the macro there should be no need to wrap ifdef around
your code, the macro will take care of it.
~Vinod
+
static struct platform_driver cpp41_dma_driver = {
.probe = cppi41_dma_probe,
.remove = cppi41_dma_remove,
.driver = {
.name = cppi41-dma-engine,
.owner = THIS_MODULE,
+ .pm = cppi41_pm_ops,
.of_match_table = of_match_ptr(cppi41_dma_ids),
},
};
--
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
Re: [PATCH v3 2/5] dma: cppi41: s/deinit_cpii41/deinit_cppi41/
On Sun, Sep 22, 2013 at 04:50:01PM +0200, Daniel Mack wrote: Fix a misspelled function name. Signed-off-by: Daniel Mack zon...@gmail.com Applied, thanks ~Vinod -- -- 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 v3 1/5] dma: cppi41: pass around device instead of platform_device
On Sun, Sep 22, 2013 at 04:50:00PM +0200, Daniel Mack wrote: Instead of passing around struct plafform_device, use struct device and save one level of dereferencing. This affects the following functions: * cppi41_add_chans * purge_descs * deinit_cpii41 * init_descs * init_cppi41 * cppi_glue_infos It's just a cosmetic cleanup that makes the code more readable. Applied, thanks ~Vinod -- 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 v3 3/5] dma: cppi41: add shortcut to pdev-dev in cppi41_dma_probe()
On Sun, Sep 22, 2013 at 04:50:02PM +0200, Daniel Mack wrote: Makes the code more readable and compact. No functional change. Signed-off-by: Daniel Mack zon...@gmail.com Applied, thanks ~Vinod -- -- 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 v3 4/5] dma: cppi41: only allocate descriptor memory once
On Sun, Sep 22, 2013 at 04:50:03PM +0200, Daniel Mack wrote:
cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This leads to both
memory leaks and double-frees.
Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
loops.
While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).
Looks fine, Sebastian cna you test it pls
~Vinod
--
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 v3 5/5] dma: cppi41: add support for suspend and resume
On 23.09.2013 06:09, Vinod Koul wrote:
On Sun, Sep 22, 2013 at 04:50:04PM +0200, Daniel Mack wrote:
+#ifdef CONFIG_PM_SLEEP
a
+static int cppi41_suspend(struct device *dev)
+{
+struct cppi41_dd *cdd = dev_get_drvdata(dev);
+
+cppi_writel(0, cdd-usbss_mem + USBSS_IRQ_CLEARR);
+disable_sched(cdd);
+
+return 0;
+}
+
+static int cppi41_resume(struct device *dev)
+{
+struct cppi41_dd *cdd = dev_get_drvdata(dev);
+int i;
+
+for (i = 0; i DESCS_AREAS; i++)
+cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
+
+init_sched(cdd);
+cppi_writel(USBSS_IRQ_PD_COMP, cdd-usbss_mem + USBSS_IRQ_ENABLER);
+
+return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cppi41_pm_ops, cppi41_suspend, cppi41_resume);
Here is the macro in pm.h
[...]
Now since you are using the macro there should be no need to wrap ifdef around
your code, the macro will take care of it.
Well yes, which is why I put the macro itself *outside* of the #ifdef
block. Without that #ifdef, however, and with CONFIG_PM_SLEEP unset, I get:
drivers/dma/cppi41.c:1043:12: warning: ‘cppi41_suspend’ defined but not
used [-Wunused-function]
static int cppi41_suspend(struct device *dev)
^
drivers/dma/cppi41.c:1053:12: warning: ‘cppi41_resume’ defined but not
used [-Wunused-function]
static int cppi41_resume(struct device *dev)
^
... which doesn't surprise me much. Or do I still not get your point?
Thanks,
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
