Re: [BUG] FL1009: xHCI host not responding to stop endpoint command.
a...@natisbad.org (Arnaud Ebalard) writes: Sarah Sharp sarah.a.sh...@linux.intel.com writes: ... AFAICT, this is exactly what commit 35773dac5f86 does and reverting it does not help. If I am mistaken, can you point which part you want me to remove in the code to test? I am slowly starting to see a bisect session coming ;-) Try reverting commit 60e102ac73cd40069d077014c93c86dc7205cb68. AFAICT, this commit does not exist in master (Linus tree), i.e. it is not in 3.13.0-rc8. That commit is a stable backport of 9df89d85b407690afa46ddfbccc80bec6869971d which is in v3.13-rc8: bjorn@nemi:/usr/local/src/git/linux$ git tag --contains 9df89d85b407690afa46ddfbccc80bec6869971d usb-3.13-rc1 usb-3.13-rc3 usb-3.13-rc5 v3.13-rc1 v3.13-rc2 v3.13-rc3 v3.13-rc4 v3.13-rc5 v3.13-rc6 v3.13-rc7 v3.13-rc8 The stable backport is in all v3.12.x releases: bjorn@nemi:/usr/local/src/git/linux$ git tag --contains 60e102ac73cd40069d077014c93c86dc7205cb68 v3.12.1 v3.12.2 v3.12.3 v3.12.4 v3.12.5 v3.12.6 v3.12.7 v3.12.8 Bjørn -- 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 v4 2/2] usb: musb: dsps, use devm_kzalloc
Replace kzalloc by devm_kzalloc and remove the kfree() calls.
Signed-off-by: Markus Pargmann m...@pengutronix.de
---
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 d0a97d6..6cae0c8 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -668,7 +668,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;
@@ -696,7 +696,6 @@ err3:
pm_runtime_put(pdev-dev);
err2:
pm_runtime_disable(pdev-dev);
- kfree(glue);
return ret;
}
@@ -709,7 +708,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.5.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 v4 0/2] usb: musb dsps updates
Hi, The two remaining patches from the previous series usb: musb bugfixes. In v4 I used the device name for the debugfs root dir and added a commit message to the second patch. Regards, Markus Markus Pargmann (2): usb: musb: dsps, debugfs files usb: musb: dsps, use devm_kzalloc drivers/usb/musb/musb_dsps.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) -- 1.8.5.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 v4 1/2] usb: musb: dsps, debugfs files
debugfs files to show the contents of important dsps registers.
Signed-off-by: Markus Pargmann m...@pengutronix.de
---
drivers/usb/musb/musb_dsps.c | 54
1 file changed, 54 insertions(+)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 593d3c9..d0a97d6 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -46,6 +46,8 @@
#include linux/of_irq.h
#include linux/usb/of.h
+#include linux/debugfs.h
+
#include musb_core.h
static const struct of_device_id musb_dsps_of_match[];
@@ -137,6 +139,26 @@ struct dsps_glue {
unsigned long last_timer;/* last timer data for each instance */
struct dsps_context context;
+ 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 },
};
static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout)
@@ -369,6 +391,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;
@@ -378,6 +424,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)
@@ -411,6 +458,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;
}
@@ -659,6 +710,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.5.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
Re: [PATCH 1/2] USB: at91: fix the number of endpoint parameter
On 17/01/2014 03:59, Bo Shen :
In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
is only 7. So, fix it for sama5d3 SoC using the udc-num_ep.
Signed-off-by: Bo Shen voice.s...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/usb/gadget/atmel_usba_udc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
- for (i = 0; i USBA_NR_ENDPOINTS; i++)
+ for (i = 0; i udc-num_ep; i++)
if (ep_status (1 i)) {
if (ep_is_control(udc-usba_ep[i]))
usba_control_irq(udc, udc-usba_ep[i]);
--
Nicolas Ferre
--
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: at91: using USBA_NR_DMAS for DMA channels
On 17/01/2014 03:59, Bo Shen :
When the SoC is earlier than sama5d3 SoC, which have the same number
endpoints and DMAs. However for sama5d3 SoC, it has different number
for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
Signed-off-by: Bo Shen voice.s...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/usb/gadget/atmel_usba_udc.c | 2 +-
drivers/usb/gadget/atmel_usba_udc.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;
- for (i = 1; i USBA_NR_ENDPOINTS; i++)
+ for (i = 1; i USBA_NR_DMAS; i++)
if (dma_status (1 i))
usba_dma_irq(udc, udc-usba_ep[i]);
}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h
b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
#define USBA_FIFO_BASE(x)((x) 16)
/* Synth parameters */
-#define USBA_NR_ENDPOINTS7
+#define USBA_NR_DMAS 7
#define EP0_FIFO_SIZE64
#define EP0_EPT_SIZE USBA_EPT_SIZE_64
--
Nicolas Ferre
--
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] xhci: make warnings greppable
From: Sarah Sharp - xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because - slot %u was not enabled.\n, slot_id); + xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because slot + %u was not enabled.\n, slot_id); Why leave the line split at the %? I will grep for things like '.*because slot %'. Best to only split at an embedded \n. David -- 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] xhci: make warnings greppable
On Fri, 2014-01-17 at 09:58 +, David Laight wrote: From: Sarah Sharp -xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because -slot %u was not enabled.\n, slot_id); +xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because slot +%u was not enabled.\n, slot_id); Why leave the line split at the %? Because you can strictly speaking only guess that there's a '%'. I will grep for things like '.*because slot %'. Best to only split at an embedded \n. Sorry, the idea just never occured to me. I'd have never dreamt of doing such a grep. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: gadget: f_fs: fix setup request handling
On Fri, Jan 17 2014, Robert Baldyga r.bald...@samsung.com wrote:
This patch fixes __ffs_ep0_queue_wait() function, which now returns number of
bytes transferred in USB request or error code in case of failure. This is
needed by ffs_ep0_read() function, when read data is copied to userspace.
It also cleans up code by removing usused variable ep0req_status.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
Good catch, thanks.
---
drivers/usb/gadget/f_fs.c |2 +-
drivers/usb/gadget/u_fs.h |1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 306a2b5..44a6bbe 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -218,7 +218,7 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs,
char *data, size_t len)
}
ffs-setup_state = FFS_NO_SETUP;
- return ffs-ep0req_status;
+ return req-status ? req-status : req-actual;
}
static int __ffs_ep0_stall(struct ffs_data *ffs)
diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h
index bc2d371..55d2f2e 100644
--- a/drivers/usb/gadget/u_fs.h
+++ b/drivers/usb/gadget/u_fs.h
@@ -156,7 +156,6 @@ struct ffs_data {
*/
struct usb_request *ep0req;/* P: mutex */
struct completion ep0req_completion; /* P: mutex */
- int ep0req_status; /* P: mutex */
/* reference counter */
atomic_tref;
--
1.7.9.5
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
signature.asc
Description: PGP signature
Re: [PATCH v2 2/3] usb: gadget: f_fs: add poll for endpoint 0
On Fri, Jan 17 2014, Robert Baldyga wrote:
This patch adds poll function for file representing ep0.
Ability of read from or write to ep0 file is related with actual state of ffs:
- When desctiptors or strings are not written yet, POLLOUT flag is set.
- If there is any event to read, POLLIN flag is set.
- If setup request was read, POLLIN and POLLOUT flag is set, to allow
send response (by performing I/O operation consistent with setup request
direction) or set stall (by performing I/O operation opposite setup
request direction).
Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
drivers/usb/gadget/f_fs.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
@@ -558,6 +560,45 @@ static long ffs_ep0_ioctl(struct file *file, unsigned
code, unsigned long value)
return ret;
}
+static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait)
+{
+ struct ffs_data *ffs = file-private_data;
+ unsigned int mask = POLLWRNORM;
+ int ret;
+
+ ret = ffs_mutex_lock(ffs-mutex, file-f_flags O_NONBLOCK);
+ if (unlikely(ret 0))
+ return mask;
+
+ switch (ffs-state) {
+ case FFS_READ_DESCRIPTORS:
+ case FFS_READ_STRINGS:
+ mask |= POLLOUT;
+ break;
+
+ case FFS_ACTIVE:
+ switch (FFS_SETUP_STATE(ffs)) {
+ case FFS_NO_SETUP:
+ if (ffs-ev.count)
+ mask |= POLLIN;
+ break;
+
+ case FFS_SETUP_PENDING:
+ mask |= (POLLIN | POLLOUT);
+ break;
+
+ case FFS_SETUP_CANCELED:
The FFS_SETUP_CANCELED and FFS_SETUP_PENDING are the same case. ep0
never recovers from FFS_SETUP_CANCELED on its own, it waits for user
space to attempt to read or write something. So unless I'm missing
something the two cases should be merged together and in both POLLIN and
POLLOUT needs to be set.
+ break;
+ }
+ case FFS_CLOSING:
+ break;
+ }
+
+ mutex_unlock(ffs-mutex);
+
+ return mask;
+}
+
static const struct file_operations ffs_ep0_operations = {
.llseek = no_llseek,
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
signature.asc
Description: PGP signature
Re: [PATCH v2 3/3] usb: gadget: f_fs: add aio support
On Fri, Jan 17 2014, Robert Baldyga wrote:
@@ -623,8 +644,55 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep,
struct usb_request *req)
}
}
-static ssize_t ffs_epfile_io(struct file *file,
- char __user *buf, size_t len, int read)
+static void ffs_user_copy_worker(struct work_struct *work)
+{
+ size_t len = 0;
Nit: This would be better named “pos” since it's a position in
a io_data-buf buffer.
+ int i = 0;
+ int ret;
+
+ struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
+work);
+ ret = io_data-len;
+
+ use_mm(io_data-mm);
+ for (i = 0; i io_data-nr_segs; i++) {
+ if (unlikely(copy_to_user(io_data-iovec[i].iov_base,
+ io_data-buf[len],
Nit: IMO this is a bit confusing way of writing “io_data-buf + len”.
I wouldn't mention it if you cared about pointing to a single character
at position len, but here you just move the pointer of the buffer to
position len.
+ io_data-iovec[i].iov_len))) {
+ ret = -EFAULT;
+ break;
+ }
+ len += io_data-iovec[i].iov_len;
+ }
+ unuse_mm(io_data-mm);
+
+ aio_complete(io_data-kiocb, ret, ret);
+
+ kfree(io_data-iovec);
+ kfree(io_data-buf);
+ kfree(io_data);
+}
@@ -667,15 +735,32 @@ static ssize_t ffs_epfile_io(struct file *file,
* Controller may require buffer size to be aligned to
* maxpacketsize of an out endpoint.
*/
- data_len = read ? usb_ep_align_maybe(gadget, ep-ep, len) : len;
+ data_len = io_data-read ? usb_ep_align_maybe(gadget, ep-ep,
+ io_data-len) :
+io_data-len;
Nit: The following is easier to read IMO:
data_len = io_data-read ?
usb_ep_align_maybe(gadget, ep-ep, io_data-len) :
io_data-len;
data = kmalloc(data_len, GFP_KERNEL);
if (unlikely(!data))
return -ENOMEM;
-
- if (!read unlikely(copy_from_user(data, buf, len))) {
- ret = -EFAULT;
- goto error;
+ if (io_data-aio !io_data-read) {
+ int i;
+ size_t len = 0;
+ for (i = 0; i io_data-nr_segs; i++) {
+ if (unlikely(copy_from_user(data[len],
+ io_data-iovec[i].iov_base,
+ io_data-iovec[i].iov_len) != 0)) {
Nit: != 0 is not necessary here.
+ ret = -EFAULT;
+ goto error;
+ }
+ len += io_data-iovec[i].iov_len;
+ }
+ } else {
+ if (!io_data-read
+ unlikely(__copy_from_user(data, io_data-buf,
+ io_data-len))) {
+ ret = -EFAULT;
+ goto error;
+ }
}
}
@@ -741,17 +858,31 @@ static ssize_t
ffs_epfile_write(struct file *file, const char __user *buf, size_t len,
loff_t *ptr)
{
+ struct ffs_io_data io_data;
+
ENTER();
- return ffs_epfile_io(file, (char __user *)buf, len, 0);
+ io_data.aio = 0;
+ io_data.read = 0;
Use “false” since those are now bools. Same for the rest of the code.
+ io_data.buf = (char * __user)buf;
+ io_data.len = len;
+
+ return ffs_epfile_io(file, io_data);
}
@@ -770,6 +901,80 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
}
+static int ffs_aio_cancel(struct kiocb *kiocb)
+{
+ struct ffs_io_data *io_data = kiocb-private;
+ struct ffs_epfile *epfile = kiocb-ki_filp-private_data;
+ int value;
+
+ ENTER();
+
+ spin_lock_irq(epfile-ffs-eps_lock);
+
+ if (likely(io_data io_data-ep io_data-req))
+ value = usb_ep_dequeue(io_data-ep, io_data-req);
+ else
+ value = -EINVAL;
+
Nit: Trailing whitespace.
+ usb_ep_free_request(io_data-ep, io_data-req);
+
+ spin_unlock_irq(epfile-ffs-eps_lock);
+
+ return value;
+}
+
+static ssize_t ffs_epfile_aio_write(struct kiocb *kiocb,
+ const struct iovec *iovec,
+ unsigned long nr_segs, loff_t loff)
+{
+ struct ffs_io_data *io_data;
+
+ ENTER();
+
+ io_data = kmalloc(sizeof(struct ffs_io_data), GFP_KERNEL);
io_data = kmalloc(sizeof(*io_data),
Re: [PATCH v2 2/3] usb: gadget: f_fs: add poll for endpoint 0
On 01/17/2014 12:20 PM, Michal Nazarewicz wrote:
On Fri, Jan 17 2014, Robert Baldyga wrote:
This patch adds poll function for file representing ep0.
Ability of read from or write to ep0 file is related with actual state of
ffs:
- When desctiptors or strings are not written yet, POLLOUT flag is set.
- If there is any event to read, POLLIN flag is set.
- If setup request was read, POLLIN and POLLOUT flag is set, to allow
send response (by performing I/O operation consistent with setup request
direction) or set stall (by performing I/O operation opposite setup
request direction).
Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
drivers/usb/gadget/f_fs.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
@@ -558,6 +560,45 @@ static long ffs_ep0_ioctl(struct file *file, unsigned
code, unsigned long value)
return ret;
}
+static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait)
+{
+struct ffs_data *ffs = file-private_data;
+unsigned int mask = POLLWRNORM;
+int ret;
+
+ret = ffs_mutex_lock(ffs-mutex, file-f_flags O_NONBLOCK);
+if (unlikely(ret 0))
+return mask;
+
+switch (ffs-state) {
+case FFS_READ_DESCRIPTORS:
+case FFS_READ_STRINGS:
+mask |= POLLOUT;
+break;
+
+case FFS_ACTIVE:
+switch (FFS_SETUP_STATE(ffs)) {
+case FFS_NO_SETUP:
+if (ffs-ev.count)
+mask |= POLLIN;
+break;
+
+case FFS_SETUP_PENDING:
+mask |= (POLLIN | POLLOUT);
+break;
+
+case FFS_SETUP_CANCELED:
The FFS_SETUP_CANCELED and FFS_SETUP_PENDING are the same case. ep0
never recovers from FFS_SETUP_CANCELED on its own, it waits for user
space to attempt to read or write something. So unless I'm missing
something the two cases should be merged together and in both POLLIN and
POLLOUT needs to be set.
Hmm, it seems like it doesn't work this way. In both read and write
functions in ep0 file operations, there is check at the beginning:
if (FFS_SETUP_STATE(ffs) == FFS_SETUP_CANCELED)
return -EIDRM;
So read/write does nothing when FFS_SETUP_CANCELED state is set.
+break;
+}
+case FFS_CLOSING:
+break;
+}
+
+mutex_unlock(ffs-mutex);
+
+return mask;
+}
+
static const struct file_operations ffs_ep0_operations = {
.llseek = no_llseek,
--
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: sk == 0xffffffff fix - not for commit
W dniu 16.01.2014 17:29, Eric Dumazet pisze:
On Thu, 2014-01-16 at 16:21 +0100, Andrzej Pietrasiewicz wrote:
W dniu 10.12.2013 15:25, Eric Dumazet pisze:
On Tue, 2013-12-10 at 07:55 +0100, Andrzej Pietrasiewicz wrote:
W dniu 09.12.2013 16:31, Eric Dumazet pisze:
On Mon, 2013-12-09 at 12:47 +0100, Andrzej Pietrasiewicz wrote:
NOT FOR COMMITTING TO MAINLINE.
With g_ether loaded the sk occasionally becomes 0x.
It happens usually after transferring few hundreds of kilobytes to few
tens of megabytes. If sk is 0x then dereferencing it causes
kernel panic.
This is a *workaround*. I don't know enough net code to understand the core
of the problem. However, with this patch applied the problems are gone,
or at least pushed farther away.
Is it happening on SMP or UP ?
UP build, S5PC110
OK
I believe you need additional debugging to track the exact moment
0x is fed to 'sk'
It looks like a very strange bug, involving a problem in some assembly
helper, register save/restore, compiler bug or stack corruption or
something.
I started with adding WARN_ON(sk == 0x); just before return in
__inet_lookup_established(), and the problem was gone. So this looks
very strange, like a toolchain problem.
Or a timing issue. Adding a WARN_ON() adds extra instructions and might
really change the assembly output.
I used gcc-linaro-arm-linux-gnueabihf-4.8-2013.05.
If I change the toolchain to
gcc-linaro-arm-linux-gnueabihf-4.7-2013.04-20130415
the problem seems to have gone away.
Its totally possible some barrier was not properly handled by the
compiler. You could disassemble the function on both toolchains and
try to spot the issue.
So I gave it a try.
Below is a part of assembly code (ARM) which corresponds to the last
lines of the __inet_lookup_established():
C source:
=
found:
rcu_read_unlock();
return sk;
}
assembly for toolchain 4.7:
===
c0333bb8: ebf4bb6ebl c0062978 __rcu_read_unlock
c0333bbc: e51b0030ldr r0, [fp, #-48] ; 0x30
c0333bc0: e24bd028sub sp, fp, #40 ; 0x28
c0333bc4: e89daff0ldm sp, {r4, r5, r6, r7, r8, r9, sl, fp,
sp, pc}
c0333bc8: e5132018ldr r2, [r3, #-24]
assembly for toolchain 4.8:
===
c033ff5c: ebf4927ebl c006495c __rcu_read_unlock
c033ff60: e24bd028sub sp, fp, #40 ; 0x28
c033ff64: e51b0030ldr r0, [fp, #-48] ; 0x30
c033ff68: e89daff0ldm sp, {r4, r5, r6, r7, r8, r9, sl, fp,
sp, pc}
c033ff6c: e5113018ldr r3, [r1, #-24]
What can be seen is that the usage of registers is slightly different,
and, what is more important, the _order_ of ldr/sub is different.
Now, if I swap the instructions at offsets c033ff60 and c033ff64
in the 4.8-generated vmlinux, the problem seems gone! Well, at least
the binary behaves the same way as the 4.7-generated one.
Here is a _hypothesis_ of what _might_ be happening:
The function in question puts its return value in the register r0.
In both cases the return value is fetched from a memory location
relative #-48 to what the frame pointer points to. However,
in the 4.7-generated binary the ldr executes in the branch delay slot,
whereas in the 4.8-generated binary it is the sub which executes
in the branch delay slot. That way, in the 4.7-generated binary the return
value is fetched before __rcu_read_unlock begins, but in the
4.8-generated binary it is fetched some time later. Which might be
enough for someone else to schedule in and break the data to be
copied to r0 and returned from the function.
As I said, this is just a hypothesis.
AP
--
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: need to mask when writting endptflush and endptprime
On Tue, Jan 14, 2014 at 03:39:25PM +0100, Matthieu CASTET wrote:
ENDPTFLUSH and ENDPTPRIME registers are set by software and
clear by hardware.
There is a bit for each endpoint.
When we are setting a bit for an endpoint we should make sure we not touch
other endpoint bit. There is a race condition if the hardware clear the
bit between the read and the write in hw_write.
Signed-off-by: Matthieu CASTET matthieu.cas...@parrot.com
---
drivers/usb/chipidea/udc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 69d20fb..9e2e39b 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int
dir)
do {
/* flush any pending transfer */
- hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
cpu_relax();
} while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
@@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int
dir, int is_ctrl)
if (is_ctrl dir == RX hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
return -EAGAIN;
- hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
cpu_relax();
--
1.8.5.2
This patch is not working here. The whole udc stalls after this patch
is applied.
I asume it is not safe changing the bits for running endpoints back to
zero, the hardware is probably stoping the transfer in that case.
As we currently never saw real issues regarding this case, it is probably
safe to stick with the current behaviour.
It's probably also not a problem to prime endpoints which got ready in
the meantime between the ioread and the iowrite, as the software did not
add new payload data for that particular endpoint. So that the hardware
will disable the endpoint in the case the transfer descriptor is empty
for that endpoint.
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: [PATCH] net: sk == 0xffffffff fix - not for commit
W dniu 17.01.2014 13:18, Andrzej Pietrasiewicz pisze:
W dniu 16.01.2014 17:29, Eric Dumazet pisze:
On Thu, 2014-01-16 at 16:21 +0100, Andrzej Pietrasiewicz wrote:
W dniu 10.12.2013 15:25, Eric Dumazet pisze:
On Tue, 2013-12-10 at 07:55 +0100, Andrzej Pietrasiewicz wrote:
W dniu 09.12.2013 16:31, Eric Dumazet pisze:
On Mon, 2013-12-09 at 12:47 +0100, Andrzej Pietrasiewicz wrote:
NOT FOR COMMITTING TO MAINLINE.
With g_ether loaded the sk occasionally becomes 0x.
It happens usually after transferring few hundreds of kilobytes to few
tens of megabytes. If sk is 0x then dereferencing it causes
kernel panic.
This is a *workaround*. I don't know enough net code to understand the core
of the problem. However, with this patch applied the problems are gone,
or at least pushed farther away.
Is it happening on SMP or UP ?
UP build, S5PC110
OK
I believe you need additional debugging to track the exact moment
0x is fed to 'sk'
It looks like a very strange bug, involving a problem in some assembly
helper, register save/restore, compiler bug or stack corruption or
something.
I started with adding WARN_ON(sk == 0x); just before return in
__inet_lookup_established(), and the problem was gone. So this looks
very strange, like a toolchain problem.
Or a timing issue. Adding a WARN_ON() adds extra instructions and might
really change the assembly output.
I used gcc-linaro-arm-linux-gnueabihf-4.8-2013.05.
If I change the toolchain to
gcc-linaro-arm-linux-gnueabihf-4.7-2013.04-20130415
the problem seems to have gone away.
Its totally possible some barrier was not properly handled by the
compiler. You could disassemble the function on both toolchains and
try to spot the issue.
So I gave it a try.
Below is a part of assembly code (ARM) which corresponds to the last
lines of the __inet_lookup_established():
C source:
=
found:
rcu_read_unlock();
return sk;
}
assembly for toolchain 4.7:
===
c0333bb8: ebf4bb6ebl c0062978 __rcu_read_unlock
c0333bbc: e51b0030ldr r0, [fp, #-48] ; 0x30
c0333bc0: e24bd028sub sp, fp, #40 ; 0x28
c0333bc4: e89daff0ldm sp, {r4, r5, r6, r7, r8, r9, sl, fp,
sp, pc}
c0333bc8: e5132018ldr r2, [r3, #-24]
assembly for toolchain 4.8:
===
c033ff5c: ebf4927ebl c006495c __rcu_read_unlock
c033ff60: e24bd028sub sp, fp, #40 ; 0x28
c033ff64: e51b0030ldr r0, [fp, #-48] ; 0x30
c033ff68: e89daff0ldm sp, {r4, r5, r6, r7, r8, r9, sl, fp,
sp, pc}
c033ff6c: e5113018ldr r3, [r1, #-24]
What can be seen is that the usage of registers is slightly different,
and, what is more important, the _order_ of ldr/sub is different.
Now, if I swap the instructions at offsets c033ff60 and c033ff64
in the 4.8-generated vmlinux, the problem seems gone! Well, at least
the binary behaves the same way as the 4.7-generated one.
Here is a _hypothesis_ of what _might_ be happening:
The function in question puts its return value in the register r0.
In both cases the return value is fetched from a memory location
relative #-48 to what the frame pointer points to. However,
in the 4.7-generated binary the ldr executes in the branch delay slot,
whereas in the 4.8-generated binary it is the sub which executes
in the branch delay slot. That way, in the 4.7-generated binary the return
value is fetched before __rcu_read_unlock begins, but in the
4.8-generated binary it is fetched some time later. Which might be
enough for someone else to schedule in and break the data to be
copied to r0 and returned from the function.
As I said, this is just a hypothesis.
Please disregard what I have written.
There is no delay slot on ARM :O
A nice hypothesis, though ;)
AP
--
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: Stop last TRB being a LINK TRB
At least some hardware (eg ASMedia PCIe cards) don't like processing a LINK
TRB and then finding that the 'cycle' bit on the next TRB is 'unset'.
This is make much more likely by the code that adds NOP TRB in order
to avoid inappropriate data alignments at LINK TRB.
I also suspect there is a race between the xhci hardware and the host
when the termination of one command schedules the setup of the next.
This makes the the bugs very machine specific.
Fix this by remembering the address of the first trb in prepare_ring()
and using that saved pointer in giveback_first_trb().
Since the first call to queue_trb() might now not be for the actual
first trb, the callers cannot track the cycle bit so this must (and
can easily) be done by queue_trb().
Similarly queue_command() must now call giveback_first_trb(), which
must not ring the doorbell for command rings.
The 'cleanup' code at the end of xhci_queue_isoc_tx() looked problematical.
However close inspection shows that it should never be executed.
For isoc transfers prepare_ring() is called early to ensure that there
are enough ring entries for the entire transfer - so it cannot fail
when called from prepare_transfer() withing the loop. In fact the later
calls are actually unnecessary so predicate them on td_index beiong zero.
The other possible looking error return from prepare_tranfser() is if
xhci_stream_id_to_ring() fails, however all the callers have just done
this. Pass in the ep_ring as a parameter to prepare_transfer() instead
of stream_id which can be obtained from the ring.
The other 'goto cleanup' is from a coding sanity check, the test could
probably be removed, I've just left it as an xci_err() trace.
The cleanup code has been deleted.
Remove the 'flip_cycle' parameter to td_to_noop() since that was only
set when called from the cleanup code of xhci_queue_isoc_tx().
(Saves worrying about whether the flips are correct.)
The code has been simplified significantly by advancing the ring's
enqueue point past LINK TRB in queue_trb() prior to writing an entry
(instead of in prepare_ring() and inc_enq() when the caller has
indicated that another TRB will be written).
There is now only one copy of the 'advance past a LINK when writing
to a ring' code and inc_enq() collapses to 3 lines and can be inlined
directly into queue_trb().
The 'more_trbs_coming' parameter (and the code to set it) disappears
in a puff of logic.
While it doesn't matter if td-first_trb references a LINK, td-last_trb
must point to the correct place. This is now 'ring-enqueue - 1'
after queue_trb() has been called.
td-last_trb is set using a static inline function since a minor
implementation change might require a different scheme.
I've also taken the liberty of on tracing short receives when they
are unexpected. Otherwise every ethernet receive generates a trace
(when the xhci_debug() trace is enabled).
Signed-off-by: David Laight david.lai...@aculab.com
---
This patch may well fix some of the regressions that appear to have been
created by the patch that adds NOP TRB.
I've a private email that indicates that a minimal patch (that left a
lot of unused code lurking) helped one ASMedia system.
If that is the case (I don't have appropriate hardware) then this is a
strong candidate for 3.13 and stable (3.12).
I've run an ethernet soak test (asx88179_178a) for several hours (with
usbnet.c patched to correctly initialise the scatter-gather list).
drivers/usb/host/xhci-ring.c | 414 ---
drivers/usb/host/xhci.h | 7 +
2 files changed, 123 insertions(+), 298 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 53c2e29..1fa40a8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -117,12 +117,6 @@ static int last_trb(struct xhci_hcd *xhci, struct
xhci_ring *ring,
return TRB_TYPE_LINK_LE32(trb-link.control);
}
-static int enqueue_is_link_trb(struct xhci_ring *ring)
-{
- struct xhci_link_trb *link = ring-enqueue-link;
- return TRB_TYPE_LINK_LE32(link-control);
-}
-
union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
{
/* Enqueue pointer can be left pointing to the link TRB,
@@ -191,83 +185,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
}
/*
- * See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs. That would be dumb and this would
loop.
- *
- * If we've just enqueued a TRB that is in the middle of a TD (meaning the
- * chain bit is set), then set the chain bit in all the following link TRBs.
- * If we've enqueued the last TRB in a TD, make sure the following link TRBs
- * have their chain bit cleared (so that each Link TRB is a separate TD).
- *
- * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
- * set, but other sections talk about dealing with the chain bit set. This was
- * fixed in the 0.96 specification
[patch v2] usb: gadget: gr_udc: remove some unneeded error handling
Debugfs function return an ERR_PTR if they compiled out. We don't need
to test for that here because if the debugfs file are compiled out then
it is ok to pass an ERR_PTR to debugfs_create_file() since it will just
be a no-op stub.
Debugfs return NULLs on error, but we don't need to test for that either
because debugfs_create_file() will accept NULL pointers.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: remove the error handling. It's not needed
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 914cbd84ee40..f984ee75324d 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -225,14 +225,8 @@ static void gr_dfs_create(struct gr_udc *dev)
const char *name = gr_udc_state;
dev-dfs_root = debugfs_create_dir(dev_name(dev-dev), NULL);
- if (IS_ERR(dev-dfs_root)) {
- dev_err(dev-dev, Failed to create debugfs directory\n);
- return;
- }
- dev-dfs_state = debugfs_create_file(name, 0444, dev-dfs_root,
-dev, gr_dfs_fops);
- if (IS_ERR(dev-dfs_state))
- dev_err(dev-dev, Failed to create debugfs file %s\n, name);
+ dev-dfs_state = debugfs_create_file(name, 0444, dev-dfs_root, dev,
+gr_dfs_fops);
}
static void gr_dfs_delete(struct gr_udc *dev)
--
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: xhci-hcd (millions of) Too many fragments error on LUKS-encrypted device
2014/1/17 Johannes Stezenbach j...@sig21.net: I cannot reproduce the issue, just finished my weekly backup to LUKS USB3 disk with unpatched linux-3.12.7. It seems the patches are awaiting feedback before the can go to stable, since you seem to be able to reproduce it easily it would be cool if you could test. Maybe test with the first patch only to see if it fixes the endless error loop, then confirm the second patch fixes the error. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733907#25 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733826#69 Hi Johannes, during the debugging process, I happen to figure out why you cannot re-produce the bug - The bug only hits when you write directly to the mapped LUKS block device, id est `mkfs` or `dd` against /dev/mapper/luks-foo. In my tests, writing directly to the mapped LUKS block device will trigger the bug (as I wrote in previous mails - crazy loop of error messages). However, if I write into a mounted ext4 on that device, the bug will not hit. I did not test with other filesystems, but ext4 seems able to do some sorting, avoiding this bug from happening. The patches, they both do their jobs: the first does not fix the bug but Too many fragments error messages are limited to just one line. The second patch does fix the bug, great. I can see it changes two more lines in xhci.c. What's its advantage over just changing 64 to 256 in xhci.h (suggestion by David, which works as well)? I've attached the dmesg output when writing to LUKS mapped device under kernel patched with first patch. We can see there is now only one line of Too many fragements error, and the writing process does not lock up. It just failed and exited happily. After applying the second patch, everything works fine, no more error messages, no more lock-ups, and the data just get to the place. I did not try applying two patches at the same time, because it's late in my timezone... Good night. -- Sascha Weaver xhci-patched-1.dmesg Description: Binary data
RE: xhci-hcd (millions of) Too many fragments error on LUKS-encrypted device
From: Sascha Weaver [mailto:wzy...@wzyboy.org] ... The patches, they both do their jobs: the first does not fix the bug but Too many fragments error messages are limited to just one line. The second patch does fix the bug, great. I can see it changes two more lines in xhci.c. What's its advantage over just changing 64 to 256 in xhci.h (suggestion by David, which works as well)? Changing the 64 to 256 lets requests with 63 to 255 fragments complete. The other changes tell the higher layers how many fragments are supported - so should stop them sending too many fragments. Since the disk code wants to send these long fragments, and the larger rings cost almost nothing (still less than 4k for each ring) it is worth increasing the ring size as well as telling the higher layers about the limit. I also suspect no one is 100% sure the advertised limit will be observed. David
Re: [PATCH] usb: chipidea: need to mask when writting endptflush and endptprime
On Fri, Jan 17, 2014 at 01:21:45PM +0100, Michael Grzeschik wrote:
On Tue, Jan 14, 2014 at 03:39:25PM +0100, Matthieu CASTET wrote:
ENDPTFLUSH and ENDPTPRIME registers are set by software and
clear by hardware.
There is a bit for each endpoint.
When we are setting a bit for an endpoint we should make sure we not touch
other endpoint bit. There is a race condition if the hardware clear the
bit between the read and the write in hw_write.
Signed-off-by: Matthieu CASTET matthieu.cas...@parrot.com
---
drivers/usb/chipidea/udc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 69d20fb..9e2e39b 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int
dir)
do {
/* flush any pending transfer */
- hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
cpu_relax();
} while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
@@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int
dir, int is_ctrl)
if (is_ctrl dir == RX hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
return -EAGAIN;
- hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
cpu_relax();
--
1.8.5.2
This patch is not working here. The whole udc stalls after this patch
is applied.
I asume it is not safe changing the bits for running endpoints back to
zero, the hardware is probably stoping the transfer in that case.
As we currently never saw real issues regarding this case, it is probably
safe to stick with the current behaviour.
It's probably also not a problem to prime endpoints which got ready in
the meantime between the ioread and the iowrite, as the software did not
add new payload data for that particular endpoint. So that the hardware
will disable the endpoint in the case the transfer descriptor is empty
for that endpoint.
Forget my previous statements. It seems that my stack was dirty when I
compiled with that patch applied. Now that I tested again with a clean
setup:
Tested-by: Michael Grzeschik mgrzesc...@pengutronix.de
--
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 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]
From: walt Oy, Sarah! ;) I put the ASMedia adapter in my older amd64 machine, and, well, the stupid thing Just Works(TM) with kernel 3.12.7! (Yes, with the same disk docking station, too.) I can't believe the adapter works perfectly in a different computer. Have you seen this kind of thing before? Could be a horrid timing race between the cpu and xchi controller. If the cpu manages to write a NOP or LINK TRB for a following transfer before the controller polls the next entry (after raising the IRQ) then the controller might process the LINK and then get confused when it can't process the linked-to TRB. This might not sound likely, but PCIe has significant latency. At the moment I have two machines using your xhci driver and both work perfectly, so I thank you again :) I'm not sure where to go with this next. I could put the adapter back in the other machine again if you have more patches to test. Can you try the patch I posted that stops the ownership on LINK TRBs being changed before that on the linked-to TRB? I got a private mail from someone indicating that my earlier 'minimal' patch helped an ASMedia controller talking to the asx189_178a ethernet hardware. David
RE: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
From: Ming Lei On Mon, Jan 13, 2014 at 9:26 PM, David Laight david.lai...@aculab.com wrote: I believe all processing use the urb-num_sgs field to limit the number of entries. Common interfaces like dma_map_sg() and for_each_sg() limit their processing to nents entries, and the USB code use the value of urb-num_sgs for this parameter. Which mostly means that the sg_xxx functions are doing a whole load of unnecessary instructions and memory accesses... This probably has a lot to do with the significant difference in the cpu use for the usb3 and 'normal' ethernet interfaces. While each bit doesn't seem significant, they soon add up. If you plan to remove the 'nents' parameter, I am wondering if it is a good idea, because sg_nents() should be more heavy. Not mention sometimes the callers just want to map/unmap part of entries. I was thinking of using a simple address/length array without all the extra fields and flags 'overpunched' in the low address bits. I'm not even sure the current use is strictly correct. The field names of the scatterlist have a page address and offset but the buffers passed to xhci (at least by usbnet) can span multiple pages - they are physically contiguous. IIRC some traces of requests for USB disks show a separate SG entry for each 4k page - even for both virtually and physically adjacent pages. David -- 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/3] usb: gadget: f_fs: add poll for endpoint 0
On Fri, Jan 17 2014, Robert Baldyga wrote:
+ case FFS_ACTIVE:
+ switch (FFS_SETUP_STATE(ffs)) {
+ case FFS_NO_SETUP:
+ if (ffs-ev.count)
+ mask |= POLLIN;
+ break;
+
+ case FFS_SETUP_PENDING:
+ mask |= (POLLIN | POLLOUT);
+ break;
+
+ case FFS_SETUP_CANCELED:
On 01/17/2014 12:20 PM, Michal Nazarewicz wrote:
The FFS_SETUP_CANCELED and FFS_SETUP_PENDING are the same case. ep0
never recovers from FFS_SETUP_CANCELED on its own, it waits for user
space to attempt to read or write something. So unless I'm missing
something the two cases should be merged together and in both POLLIN and
POLLOUT needs to be set.
On Fri, Jan 17 2014, Robert Baldyga wrote:
Hmm, it seems like it doesn't work this way. In both read and write
functions in ep0 file operations, there is check at the beginning:
if (FFS_SETUP_STATE(ffs) == FFS_SETUP_CANCELED)
return -EIDRM;
So read/write does nothing when FFS_SETUP_CANCELED state is set.
FFS_SETUP_STATE is defined as:
#define FFS_SETUP_STATE(ffs)\
((enum ffs_setup_state)cmpxchg((ffs)-setup_state, \
FFS_SETUP_CANCELED, FFS_NO_SETUP))
So what the if condition does is:
state = ffs-setup_state;
if (state == FFS_SETUP_CANCELED) {
ffs-setup_state = FFS_NO_SETUP;
return -EIDRM;
}
Perhaps the macro should be renamed (and changed to static inline).
Plus now that I look at it, it requires memory barriers. I'll prepare
a patch for that.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
signature.asc
Description: PGP signature
Re: Bug#733826: [PATCH] xhci: Set scatter-gather limit to avoid failed block writes.
On Tue, Jan 07, 2014 at 02:40:39AM +, Ben Hutchings wrote: On Tue, 2014-01-07 at 07:01 +0800, jida...@jidanni.org wrote: Don't worry, I'll trust you! Plus I'm 53. I don't think anyone else has reproduced this problem, so you are the person in the best place to check that these changes really fix it. You can test patches against the Debian kernel package by following the instructions here: http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official Sascha Weaver found a way to reproduce the problem and was nice enough to test this patch, and also the one in Debian bug #733907. See: http://marc.info/?l=linux-usbm=138996707603975w=2 Johannes -- 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 03/10] usb: find internal hub tier mismatch via acpi
On Thu, 16 Jan 2014, Dan Williams wrote:
Maybe I need to consider it a bit longer, but introducing a per-port
mutex adds at least 2 new lock ordering constraints. A replacement of
hub-busy_bits with port_dev-lock now introduces a constraint with not
only the child lock, but also synchronous invocations of rpm_{suspend|
resume} for the port.
That second constraint is very simple: Since the resume operation will
acquire the port lock, you have to make sure you don't hold the port
lock when performing a resume.
Patch 7 would look something like this:
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d86548edcc36..da12d07da127 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4740,6 +4740,13 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i = hdev-maxchild; i++) {
+ mutex_lock(port_dev-lock);
+ if (port_dev-power_is_on)
+ do_hub_event(...);
+ else
+ hub_clear_misc_change(..);
+ mutex_unlock(port_dev-lock);
+
[..]
I wouldn't split out hub_clear_misc_change in quite that way, but never
mind.
Also, you left out the places in hub_events where the port lock needs
to be dropped: around the calls to usb_reset_device and
hub_port_connect_change. And you left out the places in the resume and
reset routines where the port lock needs to be acquired.
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 97e4939fee1a..a7f32f200d90 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -88,7 +88,7 @@ static int usb_port_runtime_resume(struct device *dev)
pm_runtime_get_sync(peer-dev);
usb_autopm_get_interface(intf);
- set_bit(port1, hub-busy_bits);
+ mutex_lock(port_dev-lock);
retval = usb_hub_set_port_power(hdev, hub, port1, true);
if (port_dev-child !retval) {
@@ -105,7 +105,7 @@ static int usb_port_runtime_resume(struct device *dev)
retval = 0;
}
- clear_bit(port1, hub-busy_bits);
+ mutex_unlock(port_dev-lock);
usb_autopm_put_interface(intf);
if (!hub_is_superspeed(hdev) peer)
@@ -138,12 +138,12 @@ static int usb_port_runtime_suspend(struct device *dev)
return -EBUSY;
usb_autopm_get_interface(intf);
- set_bit(port1, hub-busy_bits);
+ mutex_lock(port_dev-lock);
retval = usb_hub_set_port_power(hdev, hub, port1, false);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
if (!hub_is_superspeed(hdev))
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
- clear_bit(port1, hub-busy_bits);
+ mutex_unlock(port_dev-lock);
usb_autopm_put_interface(intf);
/* bounce our peer now that we are down */
---
Yes, something like that.
...not too bad, although I don't really like -power_is_on compared to
just reading the pm runtime state. If we use pm_runtime_active() then
this begins to look a lot like patch 7 again.
However, patch 9 wants khubd held off until both the port and any child
devices have had a chance to resume. It would expand the scope of the
lock from protecting concurrent port access to ordering port vs child
device resume. Patch 9 as coded does so without adding a locking
constraint (beyond flushing resume work). We certainly can't wrap a
port mutex around usb_autoresume_device(port_dev-child) given the child
synchronously resumes the port. What is the alternative I am missing?
For one thing, like I mentioned above, usb_port_resume needs to hold
the port lock. (And so does usb_reset_device, after it calls
usb_autoresume_device.)
But maybe I'm not getting your point.
Some nice benefits fall out from forcing a port+child resume cycle on
port resume:
1/ prevents usb_port_runtime_resume() from growing recovery logic that
usb_port_resume() implements, like reset and disconnect.
2/ similarly, if usb_port_resume() grows warm reset recovery logic (as
it seems to need) that is applicable to port power recovery as well.
Leaning on pm_runtime synchronization to implement new hub_events()
requirements of port pm active and flushes pending usb_device
recovery is a tighter implementation. Specifically, it is tighter than
adding a new lock and its ordering constraints between suspend, resume,
child and potentially peer port events.
If you mean that we should avoid duplicating code between
usb_port_resume/finish_port_resume and the resume routines in port.c, I
agree.
The basic idea of using runtime PM synchronization to prevent khubd and
port power operations from interfering sounds good, and it is simpler
than adding a new mutex. And I think we can also use it to prevent
port power operations and
[PATCH] usb: chipidea: need to mask when writting endptflush and endptprime
ENDPTFLUSH and ENDPTPRIME registers are set by software and clear
by hardware. There is a bit for each endpoint. When we are setting
a bit for an endpoint we should make sure we do not touch other
endpoint bit. There is a race condition if the hardware clear the
bit between the read and the write in hw_write.
Signed-off-by: Matthieu CASTET matthieu.cas...@parrot.com
Tested-by: Michael Grzeschik mgrzesc...@pengutronix.de
---
drivers/usb/chipidea/udc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 69d20fb..9e2e39b 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int dir)
do {
/* flush any pending transfer */
- hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
cpu_relax();
} while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
@@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int
dir, int is_ctrl)
if (is_ctrl dir == RX hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
return -EAGAIN;
- hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
+ hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
cpu_relax();
--
1.8.5.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
chipidea udc: ctrl traffic on endpoint 8
Hi Peter,
we recently see a lot of warnings with the chipidea udc.
The driver is running as g_ether gadget.
cat drivers/usb/chipidea/udc.c
...
if (i != 0) {
dev_warn(ci-dev, ctrl traffic at endpoint %d\n, i);
continue;
}
...
Do you know if this condition is correct anyway? It checks
for fifonumbers different to zero. As I know fifo zero is
ep0out. The core has the ep0in fifo on position 8, that
should also handle ctrl traffic IMHO.
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: [PATCH] usb: gadget: fix NULL pointer dereference
On Thu, Jan 16 2014, Andrzej Pietrasiewicz wrote:
Fix possible NULL pointer dereference introduced in
219580e64f035bb9018dbb08d340f90b0ac50f8c
usb: f_fs: check quirk to pad epout buf size when not aligned to
maxpacketsize
after 3.13-rc1.
In cases we do wait with:
wait_event_interruptible(epfile-wait, (ep = epfile-ep));
for endpoint to be enabled, functionfs_bind() has not been called yet
and epfile-ffs-gadget is still NULL and the automatic variable 'gadget'
has been initialized with NULL at the point of its definition.
Later on it is used as a parameter to:
usb_ep_align_maybe(gadget, ep-ep, len)
which in turn dereferences it.
This patch fixes it by moving the actual assignment to the local 'gadget'
variable after the potential waiting has completed.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
But since gadget is only used in the “if (!halt)” part of the code,
could you simply move definition of the variable inside the if?
---
drivers/usb/gadget/f_fs.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index fffda61..e84b73b 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -587,7 +587,7 @@ static ssize_t ffs_epfile_io(struct file *file,
char __user *buf, size_t len, int read)
{
struct ffs_epfile *epfile = file-private_data;
- struct usb_gadget *gadget = epfile-ffs-gadget;
+ struct usb_gadget *gadget;
struct ffs_ep *ep;
char *data = NULL;
ssize_t ret, data_len;
@@ -613,6 +613,11 @@ static ssize_t ffs_epfile_io(struct file *file,
goto error;
}
}
+ /*
+ * if we _do_ wait above, the epfile-ffs-gadget might be NULL
+ * before the waiting completes, so do not assign to 'gadget' earlier
+ */
+ gadget = epfile-ffs-gadget;
/* Do we halt? */
halt = !read == !epfile-in;
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
signature.asc
Description: PGP signature
[PATCH] xhci: Fix use of 'reserved' command ring entries
The xhci driver allows some command ring entries to be 'reserved' so
that some requests won't fail because the command ring is full.
However queue_command() would never allow the last few ring entries to
be used even if the caller indicated that it could use a reserved slot.
If 'command_must_succeed' is set, only check that a single entry is
available.
Note that entries are not dynamically reserved and then consumed later.
Initially one entry is reserved, later another is reserved for each active
stream.
Signed-off-by: David Laight david.lai...@aculab.com
---
drivers/usb/host/xhci-ring.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 53c2e29..c021708 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4018,23 +4018,20 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci,
gfp_t mem_flags,
/* Generic function for queueing a command TRB on the command ring.
* Check to make sure there's room on the command ring for one command TRB.
- * Also check that there's room reserved for commands that must not fail.
- * If this is a command that must not fail, meaning command_must_succeed =
TRUE,
- * then only check for the number of reserved spots.
- * Don't decrement xhci-cmd_ring_reserved_trbs after we've queued the TRB
- * because the command event handler may want to resubmit a failed command.
+ * If this isn't one of the commands that can use a reserved slot then ensure
+ * that there are enough slots for all the reserved commands to be started.
*/
static int queue_command(struct xhci_hcd *xhci, u32 field1, u32 field2,
u32 field3, u32 field4, bool command_must_succeed)
{
- int reserved_trbs = xhci-cmd_ring_reserved_trbs;
+ int trbs_needed = 1;
int ret;
if (!command_must_succeed)
- reserved_trbs++;
+ trbs_needed += xhci-cmd_ring_reserved_trbs;
ret = prepare_ring(xhci, xhci-cmd_ring, EP_STATE_RUNNING,
- reserved_trbs, GFP_ATOMIC);
+ trbs_needed, GFP_ATOMIC);
if (ret 0) {
xhci_err(xhci, ERR: No room for command on command ring\n);
if (command_must_succeed)
--
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
Re: chipidea udc: ctrl traffic on endpoint 8
Le Fri, 17 Jan 2014 16:54:07 +0100,
Michael Grzeschik m...@pengutronix.de a écrit :
Hi Peter,
we recently see a lot of warnings with the chipidea udc.
The driver is running as g_ether gadget.
cat drivers/usb/chipidea/udc.c
...
if (i != 0) {
dev_warn(ci-dev, ctrl traffic at endpoint %d\n, i);
continue;
}
...
Do you know if this condition is correct anyway? It checks
for fifonumbers different to zero. As I know fifo zero is
ep0out. The core has the ep0in fifo on position 8, that
should also handle ctrl traffic IMHO.
How many fifo have your controller (hw_ep_max) ?
When you say fifo on position 8, you mean in software (in ci_hw_ep
array), but not in hardware (ENDPTCTRLx) ?
Matthieu
--
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: AM335x USB DMA seems broken on ISOC URBs
On Sun, Dec 22, 2013 at 02:59:45AM -0300, Ezequiel Garcia wrote: Hello, While doing some experiments with the stk1160 driver (for Easycap TV video capture devices), ran into problems using v3.13-rc4. The problem is that the stk1160 IRQ handler receives no URBs at all. When configuring with CONFIG_MUSB_PIO_ONLY this looks solved, but the bandwidth requirements are too large for PIO (it's a raw video device) and the video can't get captured. Interesting enough, using an old backported for v3.2 stk1160 driver, this same setup works with TI PSP v3.2 kernel. So, I guess it's some software bug in either the cppi41 dmaengine or the musb. I tried to compare the PSP to the mainline drivers but they seems *very* different and really beyond my understanding. Any ideas on how can I help debugging this? Ping? It would be good to hunt thig bug down. The TI PSP provided USB driver works fine so at least that's a starting point... -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support
On Thu, 16 Jan 2014, Florian Fainelli wrote: Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a �crit : On Wed, 15 Jan 2014, Hans de Goede wrote: Hi All, This version of my ohci and ehci-platform clks, phy and dt support patch-set, really fixes the 2 small bugs Alan found. All okay -- this time I can't find anything to complain about. :-) There is one minor issue; which is that the ehci binding claims the driver supports the following optional boolean properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc while it does not (yet) so this is misleading. Can we at get that fixed before merging? Copy pasting the PPC ehci driver should do the job. I agree it needs to be fixed. This can be done in a follow-up patch, though. It doesn't have to be in this one because nobody is using ehci-platform with DT yet. A simple copy from the PPC driver isn't quite enough, because the platform data settings would override the DT values. ehci_platform_reset has to be changed so that it sets ehci-big_endian_desc and _mmio if the pdata flags are set, but otherwise leaves them alone. Hans, would you like to write another patch to take care of this? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.
Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().
While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.
This address the following errors Russell King has hit doing randconfig
builds:
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
'msm_otg_resume'
Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
drivers/usb/phy/phy-msm-usb.c | 57 ---
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8546c8d..7c7384f 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,32 +159,6 @@ put_3p3:
return rc;
}
-#ifdef CONFIG_PM_SLEEP
-#define USB_PHY_SUSP_DIG_VOL 50
-static int msm_hsusb_config_vddcx(int high)
-{
- int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
- int min_vol;
- int ret;
-
- if (high)
- min_vol = USB_PHY_VDD_DIG_VOL_MIN;
- else
- min_vol = USB_PHY_SUSP_DIG_VOL;
-
- ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
- if (ret) {
- pr_err(%s: unable to set the voltage for regulator
- HSUSB_VDDCX\n, __func__);
- return ret;
- }
-
- pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
-
- return ret;
-}
-#endif
-
static int msm_hsusb_ldo_set_mode(int on)
{
int ret = 0;
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
#define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
#define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
-#ifdef CONFIG_PM_SLEEP
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+
+#define USB_PHY_SUSP_DIG_VOL 50
+static int msm_hsusb_config_vddcx(int high)
+{
+ int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
+ int min_vol;
+ int ret;
+
+ if (high)
+ min_vol = USB_PHY_VDD_DIG_VOL_MIN;
+ else
+ min_vol = USB_PHY_SUSP_DIG_VOL;
+
+ ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
+ if (ret) {
+ pr_err(%s: unable to set the voltage for regulator
+ HSUSB_VDDCX\n, __func__);
+ return ret;
+ }
+
+ pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
+
+ return ret;
+}
+
static int msm_otg_suspend(struct msm_otg *motg)
{
struct usb_phy *phy = motg-phy;
@@ -1733,22 +1732,18 @@ static int msm_otg_pm_resume(struct device *dev)
}
#endif
-#ifdef CONFIG_PM
static const struct dev_pm_ops msm_otg_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(msm_otg_pm_suspend, msm_otg_pm_resume)
SET_RUNTIME_PM_OPS(msm_otg_runtime_suspend, msm_otg_runtime_resume,
msm_otg_runtime_idle)
};
-#endif
static struct platform_driver msm_otg_driver = {
.remove = msm_otg_remove,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
-#ifdef CONFIG_PM
.pm = msm_otg_dev_pm_ops,
-#endif
},
};
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Hi Josh, On 01/17/2014 12:26 PM, Josh Cartwright wrote: [...] @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy) #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000) #define PHY_RESUME_TIMEOUT_USEC (100 * 1000) -#ifdef CONFIG_PM_SLEEP +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME) Why not #ifdef CONFIG_PM here? Regards, Christopher -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Josh Cartwright jo...@codeaurora.org writes:
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.
Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().
While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.
This address the following errors Russell King has hit doing randconfig
builds:
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
'msm_otg_resume'
Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
[...]
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
#define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
#define PHY_RESUME_TIMEOUT_USEC (100 * 1000)
-#ifdef CONFIG_PM_SLEEP
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
nit: you should just use CONFIG_PM here since that is what it's for.
c.f. kernel/power/Kconfig:
config PM
def_bool y
depends on PM_SLEEP || PM_RUNTIME
Kevin
--
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] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.
Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().
While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.
This address the following errors Russell King has hit doing randconfig
builds:
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
'msm_otg_resume'
Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
v1-v2: Change conditional to simply CONFIG_PM (thanks ccov and khilman!)
drivers/usb/phy/phy-msm-usb.c | 57 ---
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8546c8d..5b169a7 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,32 +159,6 @@ put_3p3:
return rc;
}
-#ifdef CONFIG_PM_SLEEP
-#define USB_PHY_SUSP_DIG_VOL 50
-static int msm_hsusb_config_vddcx(int high)
-{
- int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
- int min_vol;
- int ret;
-
- if (high)
- min_vol = USB_PHY_VDD_DIG_VOL_MIN;
- else
- min_vol = USB_PHY_SUSP_DIG_VOL;
-
- ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
- if (ret) {
- pr_err(%s: unable to set the voltage for regulator
- HSUSB_VDDCX\n, __func__);
- return ret;
- }
-
- pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
-
- return ret;
-}
-#endif
-
static int msm_hsusb_ldo_set_mode(int on)
{
int ret = 0;
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
#define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
#define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
-#ifdef CONFIG_PM_SLEEP
+#if CONFIG_PM
+
+#define USB_PHY_SUSP_DIG_VOL 50
+static int msm_hsusb_config_vddcx(int high)
+{
+ int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
+ int min_vol;
+ int ret;
+
+ if (high)
+ min_vol = USB_PHY_VDD_DIG_VOL_MIN;
+ else
+ min_vol = USB_PHY_SUSP_DIG_VOL;
+
+ ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
+ if (ret) {
+ pr_err(%s: unable to set the voltage for regulator
+ HSUSB_VDDCX\n, __func__);
+ return ret;
+ }
+
+ pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
+
+ return ret;
+}
+
static int msm_otg_suspend(struct msm_otg *motg)
{
struct usb_phy *phy = motg-phy;
@@ -1733,22 +1732,18 @@ static int msm_otg_pm_resume(struct device *dev)
}
#endif
-#ifdef CONFIG_PM
static const struct dev_pm_ops msm_otg_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(msm_otg_pm_suspend, msm_otg_pm_resume)
SET_RUNTIME_PM_OPS(msm_otg_runtime_suspend, msm_otg_runtime_resume,
msm_otg_runtime_idle)
};
-#endif
static struct platform_driver msm_otg_driver = {
.remove = msm_otg_remove,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
-#ifdef CONFIG_PM
.pm = msm_otg_dev_pm_ops,
-#endif
},
};
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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: EHCI host broken -- interrupts disabled
On Thu, Jan 16, 2014 at 02:40:35PM -0500, Alan Stern wrote:
On Thu, 16 Jan 2014, Bjorn Helgaas wrote:
I think dev-irq is supposed to be valid after pci_enable_device(), so
it seems like it would make sense to clear PCI_COMMAND_INTX_DISABLE
there.
Okay.
I don't know why a BIOS would leave PCI_COMMAND_INTX_DISABLE set for
the EHCI device. Does it support MSI, and the BIOS assumes the OS
should use that? I don't see any MSI support in the EHCI code. In
As far as I know, there aren't any EHCI designs that use MSI. And
there is no particular reason why a BIOS would leave INTX_DISABLE set,
although come to think of it, there doesn't seem to be any reason why a
BIOS should leave that bit clear either.
any case, this doesn't seem like something we need to depend on the
BIOS to do for us.
Could you post a patch for Gaggery or Jamie to try out?
Can you try the patch below? This is against 6e2d98dc1af4, the tip of the
PCI next branch, but it should apply easily to any recent kernel.
Can you also collect the dmesg and lspci -vv output for reference, in
case we need to tweak or rework this in the future? Please remove any
non-public details from that information before posting it.
Bjorn
PCI: Enable INTx if BIOS left it disabled
From: Bjorn Helgaas bhelg...@google.com
Some firmware leaves the Interrupt Disable bit set even if the device uses
INTx interrupts. Clear Interrupt Disable so we get those interrupts.
Signed-off-by: Bjorn Helgaas bhelg...@google.com
---
drivers/pci/pci.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b6d4afa8ba40..0b6a1119b818 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1181,6 +1181,8 @@ EXPORT_SYMBOL_GPL(pci_load_and_free_saved_state);
static int do_pci_enable_device(struct pci_dev *dev, int bars)
{
int err;
+ u16 cmd;
+ u8 pin;
err = pci_set_power_state(dev, PCI_D0);
if (err 0 err != -EIO)
@@ -1190,6 +1192,14 @@ static int do_pci_enable_device(struct pci_dev *dev, int
bars)
return err;
pci_fixup_device(pci_fixup_enable, dev);
+ pci_read_config_byte(dev, PCI_INTERRUPT_PIN, pin);
+ if (pin) {
+ pci_read_config_word(dev, PCI_COMMAND, cmd);
+ if (cmd PCI_COMMAND_INTX_DISABLE)
+ pci_write_config_word(dev, PCI_COMMAND,
+ cmd ~PCI_COMMAND_INTX_DISABLE);
+ }
+
return 0;
}
--
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: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.
Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().
While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.
This address the following errors Russell King has hit doing randconfig
builds:
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
'msm_otg_resume'
Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
v1-v2: Change conditional to simply CONFIG_PM (thanks ccov and khilman!)
drivers/usb/phy/phy-msm-usb.c | 57
---
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8546c8d..5b169a7 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
[..]
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
#define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
#define PHY_RESUME_TIMEOUT_USEC (100 * 1000)
-#ifdef CONFIG_PM_SLEEP
+#if CONFIG_PM
*sigh*. This, of course, should have been #ifdef CONFIG_PM. Fixed
v3 below.
Thanks,
Josh
-- 8 --
Subject: [PATCH v3] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.
Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().
While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.
This address the following errors Russell King has hit doing randconfig
builds:
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
'msm_otg_resume'
Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
drivers/usb/phy/phy-msm-usb.c | 57 ---
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8546c8d..d204f74 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -159,32 +159,6 @@ put_3p3:
return rc;
}
-#ifdef CONFIG_PM_SLEEP
-#define USB_PHY_SUSP_DIG_VOL 50
-static int msm_hsusb_config_vddcx(int high)
-{
- int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
- int min_vol;
- int ret;
-
- if (high)
- min_vol = USB_PHY_VDD_DIG_VOL_MIN;
- else
- min_vol = USB_PHY_SUSP_DIG_VOL;
-
- ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
- if (ret) {
- pr_err(%s: unable to set the voltage for regulator
- HSUSB_VDDCX\n, __func__);
- return ret;
- }
-
- pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
-
- return ret;
-}
-#endif
-
static int msm_hsusb_ldo_set_mode(int on)
{
int ret = 0;
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
#define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
#define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
+
+#define USB_PHY_SUSP_DIG_VOL 50
+static int msm_hsusb_config_vddcx(int high)
+{
+ int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
+ int min_vol;
+ int ret;
+
+ if (high)
+ min_vol = USB_PHY_VDD_DIG_VOL_MIN;
+ else
+ min_vol = USB_PHY_SUSP_DIG_VOL;
+
+ ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
+ if (ret) {
+ pr_err(%s: unable to set the voltage for regulator
+ HSUSB_VDDCX\n, __func__);
+ return ret;
+ }
+
+ pr_debug(%s: min_vol:%d max_vol:%d\n, __func__, min_vol, max_vol);
+
+ return
RE: LOCAL MAP OPTIMIZATION FOR : mail-archive.com (Less Than $99/Month)
Good Morning Sir / Mam Is your business ranking in local maps shown on PAGE 1 of google ? With new google policies they have specifically asked local business owners to optimize their website for local maps rather than JUST organics. Do you know the reason why you are not ranked well on google MAPs or why there is drop in your website rankings? Prime reason for bad rankings for a busniess is lack of local presence and local citations ie getting your business listed on directories like YELP, MANTA Many more. These websites not just give your business a push but also help you Maintain a good Online Reputation. Why you need to optimize your website for local MAP Listings ? - MAP listings get 10 times more clicks than organic listings - Increased conversions because of real reviews posted on your Google Plus Page - Every year there is 30% increase in searches for local keywords - Increases legitimacy of a Business We will help you get your website ranked well on google for the related keywords in your niche. We specialize in LOCAL SEARCH ENGINE OPTIMIZATION increasing visibility for small businesses by ranking them for geographically-related keywords. Say for eg-: you want to search a plumber in your city, You will be typing in keywords like Plumbers + City Or Plumbers + IN + City. We make sure your website comes in google MAP listings shown on page 1 for each such keyword. Now Google believes in - BE ORIGINAL, HAVE ORIGINAL AND GIVE ORIGINAL which means that google wants to end up that frustrating experience of users who are searching for Service Or Product and seeing the results that are not even close to what they are looking for. Google only wants to give their user original and relevant results. This makes it even more important that we showcase our business in the best possible way and make sure our website in valued high by google. We at TheLOCALIST will make google feel the importance of your business by following their guidelines thus ranking your website higher in serach results. We are presently offering LOCAL OPTIMIZATION to more than 400 websites and they all rank page 1 for all possible keywords !!! Each month your website is submitted to more than 50 citations and social presence is controlled by posting videos and blogs all over the web. Email us back with your website phone number so we can discuss this further with you. Our Packages start from as low as 99$/month. Thanks For Taking Time To Read Our Email Polly Martin Local SEO Manager ( THE Localist ) Address : 24 ST Suite 32 Downtown Provo Utah --- NOT INTERESTED ? REPLY WITH NOT INTERESTED IN THE SUBJECT LINE -- 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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device
On 14-01-17 02:35 AM, Hannes Reinecke wrote: On 01/16/2014 09:48 PM, Alan Stern wrote: It's now clear that this is _not_ an XHCI issue, contrary to what $SUBJECT says. On Thu, 16 Jan 2014, Peter Palúch wrote: Alan, I am attaching the usbmon trace after the drive has been plugged into the USB-2 port. Record of the drive in stall should occur around the file offset 87808 (decimal). The log was done on the 3.12.7 kernel without CONFIG_PM. Should I do a usbmon trace on my regular kernel with CONFIG_PM as well? No need. dmesg transcript: root@bach:/tmp# dmesg usb 4-1.2: new high-speed USB device number 5 using ehci-pci usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1 usb 4-1.2: Product: My Book 1230 usb 4-1.2: Manufacturer: Western Digital usb 4-1.2: SerialNumber: 574D43344E30323438393836 usb-storage 4-1.2:1.0: USB Mass Storage device detected scsi6 : usb-storage 4-1.2:1.0 usbcore: registered new interface driver usb-storage scsi 6:0:0:0: Direct-Access WD My Book 1230 1050 PQ: 0 ANSI: 6 scsi 6:0:0:1: Enclosure WD SES Device 1050 PQ: 0 ANSI: 6 sd 6:0:0:0: Attached scsi generic sg2 type 0 scsi 6:0:0:1: Attached scsi generic sg3 type 13 sd 6:0:0:0: [sdb] Spinning up disk... .ready sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08 sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sdb: sdb1 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] Attached SCSI disk usb 4-1.2: reset high-speed USB device number 5 using ehci-pci It looks like the reset occurred because the computer sent an ATA-passthru command to the disk, and the disk wasn't prepared to handle it properly. The firmware crashed, requiring a reset. If anyone can explain, the command bytes in question were: 85082e00 ec00 SCSI ATA PASS-THROUGH(16) command issuing the mandatory ATA command 0xec which is IDENTIFY DEVICE. [See SAT-3 drafts for more information on the pass-through command.] and the sense data was: 7201001d 000e 090c 5d00 0100 0050 ATA spec says there should not (normally) be an error issued by that command; but there was: $ sg_decode_sense -n 7201001d 000e 090c 5d00 0100 0050 Descriptor format, current; Sense key: Recovered Error Additional sense: ATA pass through information available Descriptor type: ATA Status Return extend=0 error=0x0 sector_count=0x0 lba=0x00 device=0x0 status=0x50 Looks reasonable at the SCSI level, not sure about the ATA level, perhaps others can comment. I don't know what either of these means, or even what software was responsible for sending this command. It appears to have come from some user program, though, not the kernel. Possibly something run by udev. Probably smartd. The logic there is _less_ than perfect. Guilty as charged. Silly us, fancy smartmontools issuing mandatory ATA commands over a USB transport (MSC ?) and expecting the device to be well-behaved :-) Try to disable smartd and check if the issue remains. Probably will help. Throwing away all your USB storage devices (apart from those that do UAS(P)) would be even more helpful (to us). Perhaps smartmontools could do a better job of identifying USB connected storage devices and avoid them. Doug Gilbert [a smartmontools maintainer] -- 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] xhci: make warnings greppable
On Fri, Jan 17, 2014 at 11:10:56AM +0100, Oliver Neukum wrote: On Fri, 2014-01-17 at 09:58 +, David Laight wrote: From: Sarah Sharp - xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because - slot %u was not enabled.\n, slot_id); + xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because slot + %u was not enabled.\n, slot_id); Why leave the line split at the %? Because you can strictly speaking only guess that there's a '%'. I will grep for things like '.*because slot %'. Best to only split at an embedded \n. Sorry, the idea just never occured to me. I'd have never dreamt of doing such a grep. This is fixed in the revised patch, David. Please read through the entire thread before commenting, thanks. 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: [BUG] FL1009: xHCI host not responding to stop endpoint command.
On Fri, Jan 17, 2014 at 09:31:16AM +0100, Bjørn Mork wrote: a...@natisbad.org (Arnaud Ebalard) writes: Sarah Sharp sarah.a.sh...@linux.intel.com writes: ... AFAICT, this is exactly what commit 35773dac5f86 does and reverting it does not help. If I am mistaken, can you point which part you want me to remove in the code to test? I am slowly starting to see a bisect session coming ;-) Try reverting commit 60e102ac73cd40069d077014c93c86dc7205cb68. AFAICT, this commit does not exist in master (Linus tree), i.e. it is not in 3.13.0-rc8. That commit is a stable backport of 9df89d85b407690afa46ddfbccc80bec6869971d which is in v3.13-rc8: bjorn@nemi:/usr/local/src/git/linux$ git tag --contains 9df89d85b407690afa46ddfbccc80bec6869971d usb-3.13-rc1 usb-3.13-rc3 usb-3.13-rc5 v3.13-rc1 v3.13-rc2 v3.13-rc3 v3.13-rc4 v3.13-rc5 v3.13-rc6 v3.13-rc7 v3.13-rc8 Sorry for using the stable commit ID. Arnaud, please try reverting commit 9df89d85b407690afa46ddfbccc80bec6869971d usbcore: set lpm_capable field for LPM capable root hubs and see if it fixes your issues. 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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device
Gentlemen, First of all, thank you very much for looking into this issue. Alan, for the sake of keeping the thread tidy in archives, I am not going to change the $SUBJECT of this e-mail but I wholeheartedly agree that this is not an xHCI issue. Mea culpa; I did not know that at the time I first reported it. Douglas, Hannes: I believe we are definitely on to something. I do _not_ run smartd but this may be caused by something in my Gnome3 GUI environment. I have noticed that when I am not logged in to Gnome and work just in the text console, plugging in the USB drive and accessing it works without any issues. Only when I am logged into my Gnome and plug the drive in, Gnome obviously tries to do something with the drive that causes it to stall and recover in 30 seconds. I have not yet been able to identify what it is. I am not subscribed to linux-scsi mailing list so if you believe anything of this is relevant please forward that mail there. Thank you! Best regards, Peter On 17.01.2014 21:25, Douglas Gilbert wrote: On 14-01-17 02:35 AM, Hannes Reinecke wrote: On 01/16/2014 09:48 PM, Alan Stern wrote: It's now clear that this is _not_ an XHCI issue, contrary to what $SUBJECT says. On Thu, 16 Jan 2014, Peter Palúch wrote: Alan, I am attaching the usbmon trace after the drive has been plugged into the USB-2 port. Record of the drive in stall should occur around the file offset 87808 (decimal). The log was done on the 3.12.7 kernel without CONFIG_PM. Should I do a usbmon trace on my regular kernel with CONFIG_PM as well? No need. dmesg transcript: root@bach:/tmp# dmesg usb 4-1.2: new high-speed USB device number 5 using ehci-pci usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1 usb 4-1.2: Product: My Book 1230 usb 4-1.2: Manufacturer: Western Digital usb 4-1.2: SerialNumber: 574D43344E30323438393836 usb-storage 4-1.2:1.0: USB Mass Storage device detected scsi6 : usb-storage 4-1.2:1.0 usbcore: registered new interface driver usb-storage scsi 6:0:0:0: Direct-Access WD My Book 1230 1050 PQ: 0 ANSI: 6 scsi 6:0:0:1: Enclosure WD SES Device 1050 PQ: 0 ANSI: 6 sd 6:0:0:0: Attached scsi generic sg2 type 0 scsi 6:0:0:1: Attached scsi generic sg3 type 13 sd 6:0:0:0: [sdb] Spinning up disk... .ready sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08 sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sdb: sdb1 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] Attached SCSI disk usb 4-1.2: reset high-speed USB device number 5 using ehci-pci It looks like the reset occurred because the computer sent an ATA-passthru command to the disk, and the disk wasn't prepared to handle it properly. The firmware crashed, requiring a reset. If anyone can explain, the command bytes in question were: 85082e00 ec00 SCSI ATA PASS-THROUGH(16) command issuing the mandatory ATA command 0xec which is IDENTIFY DEVICE. [See SAT-3 drafts for more information on the pass-through command.] and the sense data was: 7201001d 000e 090c 5d00 0100 0050 ATA spec says there should not (normally) be an error issued by that command; but there was: $ sg_decode_sense -n 7201001d 000e 090c 5d00 0100 0050 Descriptor format, current; Sense key: Recovered Error Additional sense: ATA pass through information available Descriptor type: ATA Status Return extend=0 error=0x0 sector_count=0x0 lba=0x00 device=0x0 status=0x50 Looks reasonable at the SCSI level, not sure about the ATA level, perhaps others can comment. I don't know what either of these means, or even what software was responsible for sending this command. It appears to have come from some user program, though, not the kernel. Possibly something run by udev. Probably smartd. The logic there is _less_ than perfect. Guilty as charged. Silly us, fancy smartmontools issuing mandatory ATA commands over a USB transport (MSC ?) and expecting the device to be well-behaved :-) Try to disable smartd and check if the issue remains. Probably will help. Throwing away all your USB storage devices (apart from those that do UAS(P)) would be even more helpful (to us). Perhaps smartmontools could do a better job of identifying USB connected storage devices and avoid them. Doug Gilbert [a smartmontools maintainer] -- Ing. Peter Palúch, Ph.D. CCIE RS #23527, CCIP, CCAI, Cisco Designated VIP 2011-2014
Re: usb3 hdd docking station doesn't work with recent kernels
On Tue, Jan 14, 2014 at 02:43:40PM -0500, Alan Stern wrote:
On Tue, 14 Jan 2014, Sarah Sharp wrote:
Alan, I believe your analysis of the code was incorrect when you asked
Xenia to create commit 60e102ac73cd40069d077014c93c86dc7205cb68 in
August:
http://marc.info/?l=linux-usbm=137780837109921w=2
You asked her to set lpm_capable because you thought it simply wasn't
being set at all. You overlooked the xHCI driver setting lpm_capable in
drivers/usb/host/xhci-pci.c.
So I did.
The xHCI driver only sets lpm_capable for xHCI host controllers that it
knows how to calculate the U1/U2 timeout values for. Since those
timeout values are highly dependent on how the xHCI host hardware
scheduling works, I've only enabled USB 3.0 Link PM for Intel hosts:
static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
{
...
if (pdev-vendor == PCI_VENDOR_ID_INTEL) {
xhci-quirks |= XHCI_LPM_SUPPORT;
xhci-quirks |= XHCI_INTEL_HOST;
}
static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id
*id)
{
...
/* We know the LPM timeout algorithms for this host, let the USB
core
* enable and disable LPM for devices under the USB 3.0 roothub.
*/
if (xhci-quirks XHCI_LPM_SUPPORT)
hcd_to_bus(xhci-shared_hcd)-root_hub-lpm_capable = 1;
lpm_capable shouldn't be set for hosts where we don't know how to
calculate the U1/U2 timeout values, so commit
60e102ac73cd40069d077014c93c86dc7205cb68 should just be reverted.
Allow me to propose an alternative.
Let's do our best to treat root hubs the same as external hubs. This
means that udev-lpm_capable should be set the same way in
hub_port_init() and register_root_hub(): by calling
usb_device_supports_lpm(), which gets the information from the BOS
extended capabilities.
In particular, xhci-hcd shouldn't go messing around with the innards of
the root hub structure. Instead of setting lpm_capable when it knows
how to calculate the timeout values, it should pass the necessary
information through the ss_cap descriptor, just like an external hub
would. Either don't provide that descriptor at all if you don't know
the timeout values, or else set those fields in the descriptor to
something invalid (and make usb_device_supports_lpm() test for it).
And while we're at it, let's change the comment in
usb_device_supports_lpm() that says All USB 3.0 must support LPM.
Not only is it ungrammatical, but as you point out, it is wrong for
some root hubs.
I agree your alternative is much cleaner, and I agree we should try to
treat roothubs like external hubs. I've made a patch (to be sent
shortly) that fixes this.
However, I would like to see the original bad commit (usbcore: set
lpm_capable field for LPM capable root hubs) reverted from the stable
kernels, since it's been backported to 3.10, 3.11, and 3.12. I would
rather go for the simple revert, since that's less likely to cause
issues than new code.
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
[RFT] usb: Don't enable USB 3.0 LPM for non-Intel hosts.
Sergey, can you test whether this fixes your issue? Sarah Sharp 8---8 Commit 9df89d85b407690afa46ddfbccc80bec6869971d usbcore: set lpm_capable field for LPM capable root hubs was created under the assumption that all USB host controllers should have USB 3.0 Link PM enabled for all devices under the hosts. Unfortunately, that's not the case. The xHCI driver relies on knowledge of the host hardware scheduler to calculate the LPM U1/U2 timeout values, and it only sets lpm_capable to one for Intel host controllers (that have the XHCI_LPM_SUPPORT quirk set). When LPM is enabled for some Fresco Logic hosts, it causes failures with a AgeStar 3UBT USB 3.0 hard drive dock: Jan 11 13:59:03 sg-laptop kernel: usb 3-1: new SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:03 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:08 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:08 sg-laptop kernel: usb-storage 3-1:1.0: USB Mass Storage device detected Jan 11 13:59:08 sg-laptop mtp-probe[613]: checking bus 3, device 2: /sys/devices/pci:00/:00:1c.3/:04:00.0/usb3/3-1 Jan 11 13:59:08 sg-laptop mtp-probe[613]: bus: 3, device: 2 was not an MTP device Jan 11 13:59:08 sg-laptop kernel: scsi6 : usb-storage 3-1:1.0 Jan 11 13:59:13 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:18 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:18 sg-laptop kernel: usbcore: registered new interface driver usb-storage Jan 11 13:59:40 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:41 sg-laptop kernel: usb 3-1: device descriptor read/8, error -71 Jan 11 13:59:41 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:46 sg-laptop kernel: usb 3-1: device descriptor read/8, error -110 Jan 11 13:59:46 sg-laptop kernel: scsi 6:0:0:0: Device offlined - not ready after error recovery Jan 11 13:59:46 sg-laptop kernel: usb 3-1: USB disconnect, device number 2 lspci for the affected host: 04:00.0 0c03: 1b73:1000 (rev 04) (prog-if 30 [XHCI]) Subsystem: 1043:1039 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 19 Region 0: Memory at dd20 (32-bit, non-prefetchable) [size=64K] Capabilities: [50] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [68] MSI: Enable- Count=1/1 Maskable- 64bit+ Address: Data: Capabilities: [80] Express (v1) Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s 2us, L1 32us ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 unlimited, L1 unlimited ClockPM- Surprise- LLActRep- BwNot- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt- Kernel driver in use: xhci_hcd Kernel modules: xhci_hcd The xHCI driver shouldn't really be mucking with USB core internal fields, so rip that out. Instead, indicate the xHCI driver doesn't support Link PM on this host by clearing the U1 and U2 exit latencies in the roothub SuperSpeed Extended Capabilities BOS descriptor. This will also disable LPM for external devices that have the U1 and U2 exit latencies set to zero. This was already effectively done with commit ae8963adb4ad8c5f2a89ca1d99fb7bb721e7599f usb: Don't enable LPM if the exit latency is zero. Leave that code in place, in case there are devices out there that set one exit latency value to zero, but set the other to a valid value. Also, change messages about missing Link PM information from warning level to info level. Only print a warning about the first device that doesn't support LPM, to avoid log spam. Further, cleanup some unnecessary line breaks. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com Reported-by: Sergey Galanov sergey.e.gala...@gmail.com Cc: Alan Stern
Re: [RFT] usb: Don't enable USB 3.0 LPM for non-Intel hosts.
On Fri, Jan 17, 2014 at 02:48:55PM -0800, Sarah Sharp wrote: Sergey, can you test whether this fixes your issue? And if it works, please send me the output of `sudo lsusb -v` with the AgeStar dock plugged in. Your last lsusb output didn't include it. Sarah Sharp 8---8 Commit 9df89d85b407690afa46ddfbccc80bec6869971d usbcore: set lpm_capable field for LPM capable root hubs was created under the assumption that all USB host controllers should have USB 3.0 Link PM enabled for all devices under the hosts. Unfortunately, that's not the case. The xHCI driver relies on knowledge of the host hardware scheduler to calculate the LPM U1/U2 timeout values, and it only sets lpm_capable to one for Intel host controllers (that have the XHCI_LPM_SUPPORT quirk set). When LPM is enabled for some Fresco Logic hosts, it causes failures with a AgeStar 3UBT USB 3.0 hard drive dock: Jan 11 13:59:03 sg-laptop kernel: usb 3-1: new SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:03 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:08 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:08 sg-laptop kernel: usb-storage 3-1:1.0: USB Mass Storage device detected Jan 11 13:59:08 sg-laptop mtp-probe[613]: checking bus 3, device 2: /sys/devices/pci:00/:00:1c.3/:04:00.0/usb3/3-1 Jan 11 13:59:08 sg-laptop mtp-probe[613]: bus: 3, device: 2 was not an MTP device Jan 11 13:59:08 sg-laptop kernel: scsi6 : usb-storage 3-1:1.0 Jan 11 13:59:13 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:18 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:18 sg-laptop kernel: usbcore: registered new interface driver usb-storage Jan 11 13:59:40 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:41 sg-laptop kernel: usb 3-1: device descriptor read/8, error -71 Jan 11 13:59:41 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:46 sg-laptop kernel: usb 3-1: device descriptor read/8, error -110 Jan 11 13:59:46 sg-laptop kernel: scsi 6:0:0:0: Device offlined - not ready after error recovery Jan 11 13:59:46 sg-laptop kernel: usb 3-1: USB disconnect, device number 2 lspci for the affected host: 04:00.0 0c03: 1b73:1000 (rev 04) (prog-if 30 [XHCI]) Subsystem: 1043:1039 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 19 Region 0: Memory at dd20 (32-bit, non-prefetchable) [size=64K] Capabilities: [50] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [68] MSI: Enable- Count=1/1 Maskable- 64bit+ Address: Data: Capabilities: [80] Express (v1) Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s 2us, L1 32us ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 unlimited, L1 unlimited ClockPM- Surprise- LLActRep- BwNot- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt- Kernel driver in use: xhci_hcd Kernel modules: xhci_hcd The xHCI driver shouldn't really be mucking with USB core internal fields, so rip that out. Instead, indicate the xHCI driver doesn't support Link PM on this host by clearing the U1 and U2 exit latencies in the roothub SuperSpeed Extended Capabilities BOS descriptor. This will also disable LPM for external devices that have the U1 and U2 exit latencies set to zero. This was already effectively done with commit ae8963adb4ad8c5f2a89ca1d99fb7bb721e7599f usb: Don't enable LPM if the exit latency is zero. Leave that code in place, in case there are devices out there that set one exit latency value to zero, but set the other to a valid value. Also, change messages about missing Link PM information from warning
[RFT] xhci: Fix resume issues on Renesas chips in Samsung laptops
Don Zickus dzic...@redhat.com writes: Some co-workers of mine bought Samsung laptops that had mostly usb3 ports. Those ports did not resume correctly (the driver would timeout communicating and fail). This led to frustration as suspend/resume is a common use for laptops. Poking around, I applied the reset on resume quirk to this chipset and the resume started working. Reloading the xhci_hcd module had been the temporary workaround. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com Reported-by: Don Zickus dzic...@redhat.com Cc: stable # 2.6.37 --- Don, please ask your co-worker to apply this patch and test. Let me know if your other co-worker has a different subsystem device ID in his Samsung laptop, when he gets back from vacation. If so, we'll extend it to all Samsung laptops with that particular Renesas host. Sarah Sharp drivers/usb/host/xhci-pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index e42c318a4404..6c03584ac15f 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -142,6 +142,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) QUIRK: Resetting on resume); xhci-quirks |= XHCI_TRUST_TX_LENGTH; } + if (pdev-vendor == PCI_VENDOR_ID_RENESAS + pdev-device == 0x0015 + pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG + pdev-subsystem_device == 0xc0cd) + xhci-quirks |= XHCI_RESET_ON_RESUME; if (pdev-vendor == PCI_VENDOR_ID_VIA) xhci-quirks |= XHCI_RESET_ON_RESUME; } -- 1.8.5.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
Re: [RFT] usb: Don't enable USB 3.0 LPM for non-Intel hosts.
Hi Sarah. It works. I applied the patch to 3.13rc8 (I had to modify it a little bit because in my version xhci_readl is called rather than readl). lsusb I've sent included the dock info. It's 'JMicron Technology' device. At least it's the only difference between lsusb output with and without dock. Thanks, Sergey On 18 January 2014 02:55, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Fri, Jan 17, 2014 at 02:48:55PM -0800, Sarah Sharp wrote: Sergey, can you test whether this fixes your issue? And if it works, please send me the output of `sudo lsusb -v` with the AgeStar dock plugged in. Your last lsusb output didn't include it. Sarah Sharp 8---8 Commit 9df89d85b407690afa46ddfbccc80bec6869971d usbcore: set lpm_capable field for LPM capable root hubs was created under the assumption that all USB host controllers should have USB 3.0 Link PM enabled for all devices under the hosts. Unfortunately, that's not the case. The xHCI driver relies on knowledge of the host hardware scheduler to calculate the LPM U1/U2 timeout values, and it only sets lpm_capable to one for Intel host controllers (that have the XHCI_LPM_SUPPORT quirk set). When LPM is enabled for some Fresco Logic hosts, it causes failures with a AgeStar 3UBT USB 3.0 hard drive dock: Jan 11 13:59:03 sg-laptop kernel: usb 3-1: new SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:03 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:08 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:08 sg-laptop kernel: usb-storage 3-1:1.0: USB Mass Storage device detected Jan 11 13:59:08 sg-laptop mtp-probe[613]: checking bus 3, device 2: /sys/devices/pci:00/:00:1c.3/:04:00.0/usb3/3-1 Jan 11 13:59:08 sg-laptop mtp-probe[613]: bus: 3, device: 2 was not an MTP device Jan 11 13:59:08 sg-laptop kernel: scsi6 : usb-storage 3-1:1.0 Jan 11 13:59:13 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U1 failed. Jan 11 13:59:18 sg-laptop kernel: usb 3-1: Set SEL for device-initiated U2 failed. Jan 11 13:59:18 sg-laptop kernel: usbcore: registered new interface driver usb-storage Jan 11 13:59:40 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:41 sg-laptop kernel: usb 3-1: device descriptor read/8, error -71 Jan 11 13:59:41 sg-laptop kernel: usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd Jan 11 13:59:46 sg-laptop kernel: usb 3-1: device descriptor read/8, error -110 Jan 11 13:59:46 sg-laptop kernel: scsi 6:0:0:0: Device offlined - not ready after error recovery Jan 11 13:59:46 sg-laptop kernel: usb 3-1: USB disconnect, device number 2 lspci for the affected host: 04:00.0 0c03: 1b73:1000 (rev 04) (prog-if 30 [XHCI]) Subsystem: 1043:1039 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 19 Region 0: Memory at dd20 (32-bit, non-prefetchable) [size=64K] Capabilities: [50] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [68] MSI: Enable- Count=1/1 Maskable- 64bit+ Address: Data: Capabilities: [80] Express (v1) Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s 2us, L1 32us ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 unlimited, L1 unlimited ClockPM- Surprise- LLActRep- BwNot- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt- Kernel driver in use: xhci_hcd Kernel modules: xhci_hcd The xHCI driver shouldn't really be mucking with USB core internal fields, so rip that out. Instead, indicate the xHCI driver doesn't support Link PM on this host by clearing the U1 and U2 exit latencies in the roothub SuperSpeed Extended Capabilities BOS descriptor. This will also disable LPM for external devices that have the U1
Re: usb3 hdd docking station doesn't work with recent kernels
On Fri, 17 Jan 2014, Sarah Sharp wrote: Allow me to propose an alternative. Let's do our best to treat root hubs the same as external hubs. This means that udev-lpm_capable should be set the same way in hub_port_init() and register_root_hub(): by calling usb_device_supports_lpm(), which gets the information from the BOS extended capabilities. In particular, xhci-hcd shouldn't go messing around with the innards of the root hub structure. Instead of setting lpm_capable when it knows how to calculate the timeout values, it should pass the necessary information through the ss_cap descriptor, just like an external hub would. Either don't provide that descriptor at all if you don't know the timeout values, or else set those fields in the descriptor to something invalid (and make usb_device_supports_lpm() test for it). And while we're at it, let's change the comment in usb_device_supports_lpm() that says All USB 3.0 must support LPM. Not only is it ungrammatical, but as you point out, it is wrong for some root hubs. I agree your alternative is much cleaner, and I agree we should try to treat roothubs like external hubs. I've made a patch (to be sent shortly) that fixes this. However, I would like to see the original bad commit (usbcore: set lpm_capable field for LPM capable root hubs) reverted from the stable kernels, since it's been backported to 3.10, 3.11, and 3.12. I would rather go for the simple revert, since that's less likely to cause issues than new code. Reverting Ksenia's patch from the stable kernels is okay with me. Strictly speaking, I guess this means you have to revert it from the current kernel as well, and then write your new patch to go on top of the reversion. 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 v3 03/10] usb: find internal hub tier mismatch via acpi
On Fri, 2014-01-17 at 10:53 -0500, Alan Stern wrote:
On Thu, 16 Jan 2014, Dan Williams wrote:
Maybe I need to consider it a bit longer, but introducing a per-port
mutex adds at least 2 new lock ordering constraints. A replacement of
hub-busy_bits with port_dev-lock now introduces a constraint with not
only the child lock, but also synchronous invocations of rpm_{suspend|
resume} for the port.
That second constraint is very simple: Since the resume operation will
acquire the port lock, you have to make sure you don't hold the port
lock when performing a resume.
Patch 7 would look something like this:
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d86548edcc36..da12d07da127 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4740,6 +4740,13 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i = hdev-maxchild; i++) {
+ mutex_lock(port_dev-lock);
+ if (port_dev-power_is_on)
+ do_hub_event(...);
+ else
+ hub_clear_misc_change(..);
+ mutex_unlock(port_dev-lock);
+
[..]
I wouldn't split out hub_clear_misc_change in quite that way, but never
mind.
Also, you left out the places in hub_events where the port lock needs
to be dropped: around the calls to usb_reset_device and
hub_port_connect_change. And you left out the places in the resume and
reset routines where the port lock needs to be acquired.
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 97e4939fee1a..a7f32f200d90 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -88,7 +88,7 @@ static int usb_port_runtime_resume(struct device *dev)
pm_runtime_get_sync(peer-dev);
usb_autopm_get_interface(intf);
- set_bit(port1, hub-busy_bits);
+ mutex_lock(port_dev-lock);
retval = usb_hub_set_port_power(hdev, hub, port1, true);
if (port_dev-child !retval) {
@@ -105,7 +105,7 @@ static int usb_port_runtime_resume(struct device *dev)
retval = 0;
}
- clear_bit(port1, hub-busy_bits);
+ mutex_unlock(port_dev-lock);
usb_autopm_put_interface(intf);
if (!hub_is_superspeed(hdev) peer)
@@ -138,12 +138,12 @@ static int usb_port_runtime_suspend(struct device
*dev)
return -EBUSY;
usb_autopm_get_interface(intf);
- set_bit(port1, hub-busy_bits);
+ mutex_lock(port_dev-lock);
retval = usb_hub_set_port_power(hdev, hub, port1, false);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
if (!hub_is_superspeed(hdev))
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
- clear_bit(port1, hub-busy_bits);
+ mutex_unlock(port_dev-lock);
usb_autopm_put_interface(intf);
/* bounce our peer now that we are down */
---
Yes, something like that.
...not too bad, although I don't really like -power_is_on compared to
just reading the pm runtime state. If we use pm_runtime_active() then
this begins to look a lot like patch 7 again.
However, patch 9 wants khubd held off until both the port and any child
devices have had a chance to resume. It would expand the scope of the
lock from protecting concurrent port access to ordering port vs child
device resume. Patch 9 as coded does so without adding a locking
constraint (beyond flushing resume work). We certainly can't wrap a
port mutex around usb_autoresume_device(port_dev-child) given the child
synchronously resumes the port. What is the alternative I am missing?
For one thing, like I mentioned above, usb_port_resume needs to hold
the port lock. (And so does usb_reset_device, after it calls
usb_autoresume_device.)
But maybe I'm not getting your point.
Some nice benefits fall out from forcing a port+child resume cycle on
port resume:
1/ prevents usb_port_runtime_resume() from growing recovery logic that
usb_port_resume() implements, like reset and disconnect.
2/ similarly, if usb_port_resume() grows warm reset recovery logic (as
it seems to need) that is applicable to port power recovery as well.
Leaning on pm_runtime synchronization to implement new hub_events()
requirements of port pm active and flushes pending usb_device
recovery is a tighter implementation. Specifically, it is tighter than
adding a new lock and its ordering constraints between suspend, resume,
child and potentially peer port events.
If you mean that we should avoid duplicating code between
usb_port_resume/finish_port_resume and the resume routines in port.c, I
agree.
The basic idea of using runtime PM synchronization to
Re: [PATCH] usbcore: fix BABBLE failed enumeration of legacy USB2 devices on USB3 bus
Hi,
I encountered the same problem with another device.
If possible, it would be nice to pick Marius's patch for stable
kernels (tested here on v3.12.6).
There are chances that MacOSX is affected by a similar issue,
so if anybody has friends there...
Thanks,
--
Jérôme
On Wed, 8 Jan 2014 04:00:22 +
Marius Silaghi msila...@fit.edu wrote:
Great observation,
Sarah located a patch that is queued for the 3.14 kernel and that has
a similar effect. So future kernels could work with that one as well.
The patch I provided (being very small and safe) can still be
suggested for maintainers of older kernels in various long-term
maintained distributions (if you know who is doing that).
Here are some versions of the patch I made for current kernels:
The next one was tested on Ubuntu, applied to the source for
3.5.0-17-generic (Ubuntu)
--- linux-3.5.0/drivers/usb/core/hub.c.orig 2014-01-07
18:16:01.997031650 -0500 +++
linux-3.5.0/drivers/usb/core/hub.c2014-01-07
18:19:41.617022465 -0500 @@ -4043,7 +4043,11 @@ break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval 8) {
dev_err(udev-dev,
device descriptor read/8,
error %d\n,
For kernel 3.9.0-0.4
--- linux-3.5.0/drivers/usb/core/hub.c.orig 2014-01-07
18:16:01.997031650 -0500 +++
linux-3.5.0/drivers/usb/core/hub.c2014-01-07
18:19:41.617022465 -0500 @@ -4043,7 +4043,11 @@ break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval 8) {
dev_err(udev-dev,
device descriptor read/8,
error %d\n,
For kernel 3.10.0-5.15
--- ubuntu-saucy/drivers/usb/core/hub.c.orig 2014-01-07
16:52:41.300835262 -0500 +++
ubuntu-saucy/drivers/usb/core/hub.c 2014-01-07
16:54:53.612829730 -0500 @@ -4126,8 +4126,11 @@ if
(USE_NEW_SCHEME(retry_counter) !(hcd-driver-flags HCD_USB3))
break; }
-
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval 8) {
if (retval != -ENODEV)
dev_err(udev-dev,
For kernel 3.11
--- linux-3.11/drivers/usb/core/hub.c.orig2014-01-07
16:57:16.352823760 -0500 +++ linux-3.11/drivers/usb/core/hub.c
2014-01-07 16:58:10.168821508 -0500 @@ -4161,7 +4161,11 @@
break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval 8) {
if (retval != -ENODEV)
dev_err(udev-dev,
From: linux-usb-ow...@vger.kernel.org
[linux-usb-ow...@vger.kernel.org] on behalf of Greg Kroah-Hartman
[gre...@linuxfoundation.org] Sent: Tuesday, January 07, 2014 19:32
To: Marius Silaghi Cc: Sarah Sharp; linux-usb@vger.kernel.org;
linux-ker...@vger.kernel.org; Alan Stern; Lan Tianyu; Xenia
Ragiadakou; Jiri Kosina Subject: Re: [PATCH] usbcore: fix BABBLE
failed enumeration of legacy USB2 devices on USB3 bus
On Tue, Dec 24, 2013 at 04:19:18AM +, Marius Silaghi wrote:
From: Marius C Silaghi msila...@fit.edu
This patch is generated against the last kernel version in the
github kernel repository.
We work off of the git.kernel.org trees, not github :)
Some older families of USB2 cameras (STC-XUSB) do not support
querying only the first 8 bytes of their device descriptor and
therefore fail at enumeration on USB3 HCDs, with babble error -75
as they send more than the expected 8 bytes. The proposed patch
extends the mechanism used for non USB3 HCDs in the first part of
the same function, and successively tries to query both the 8 byte
prefix of the device
Re: [PATCH 1/2] USB: at91: fix the number of endpoint parameter
On 10:59 Fri 17 Jan , Bo Shen wrote:
In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
is only 7. So, fix it for sama5d3 SoC using the udc-num_ep.
Signed-off-by: Bo Shen voice.s...@atmel.com
---
drivers/usb/gadget/atmel_usba_udc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
- for (i = 0; i USBA_NR_ENDPOINTS; i++)
+ for (i = 0; i udc-num_ep; i++)
no the limit need to specified in the driver as a checkpoint by the compatible
or platform driver id
Best Regards,
J.
if (ep_status (1 i)) {
if (ep_is_control(udc-usba_ep[i]))
usba_control_irq(udc, udc-usba_ep[i]);
--
1.8.5.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
