Re: Fwd: Re: [alsa-devel] no reset_resume for driver snd-usb-audio for logitech headset H600
At Sat, 11 Jan 2014 11:08:39 +0100, wrote: > > FWD: > > Any news to the snd-usb-audio issue? > > Thanks, Bernhard > > Old Message: > - > I have tested the patch with kernel 3.12.0 with suspend and hibernate, and > the problem is gone. > > No messages "no reset_resume for driver snd-usb-audio?" or other negative > effects/messages. > > After resume, the usb_audio_device (Headset) is the master channel in KDE > Mixer, as it should be. > > => So the problem is solved for me. > > Can you please add the patch to the latest kernels and LTS-kernels or > whitelist the workaround for my device (ID 046d:0a29 Logitech, Inc. H600 > [Wireless Headset]) The "reset_resume" kernel message was known to be harmless, and now it's degraded as a debug message. That's the only change, so there is no functional change. That being said, you just need to ignore the kernel messages in the old kernels. Or, feel free to submit the commit [0a56b4fa6844: USB: change dev_warn about missing reset-resume to dev_dbg] to stable kernel. Takashi > > Thanks, Bernhard > > On 2013-10-20 23:35, Takashi Iwai wrote: > > At Sun, 20 Oct 2013 23:18:13 +0200, > > wrote: > >> > >> On 2013-10-09 11:16 (CEST,UTC+2), Takashi Iwai wrote: > >> diff --git a/sound/usb/card.c b/sound/usb/card.c > >> index 9d9de8d..c591a83 100644 > >> --- a/sound/usb/card.c > >> +++ b/sound/usb/card.c > >> @@ -747,6 +747,7 @@ static struct usb_driver usb_audio_driver = { > >>.disconnect = usb_audio_disconnect, > >>.suspend = usb_audio_suspend, > >>.resume = usb_audio_resume, > >> + .reset_resume = usb_audio_resume, > >>.id_table = usb_audio_ids, > >>.supports_autosuspend = 1, > >>}; > >> > >> Shall I add the line in b/sound/usb/card.c and try it? Which kernel > >> version shall I use? > > > > It doesn't matter which kernel version is. > > The error message should disappear, but I don't know whether any other > > positive effect you get. As Clemens pointed, there can be some > > negative effects by this, so it cannot be taken as is. But at least, > > it'd be good to know the result... > > > > > > thanks, > > > > Takashi > > > > > > -- 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: usb3 hdd docking station doesn't work with recent kernels
On Sun, 2014-01-12 at 14:12 +0400, Sergey Galanov wrote: > Hi. > My hdd docking station (AgeStar 3UBT with usb3 support) stopped > working after upgrade to linux 3.12.3. It worked fine in 3.12.0 and > before. I can't point to more specific version because it's hard to > find old packages for my distro (ArchLinux). I also tried 3.12.6, > 3.12.7 and 3.13rc7 and the problem persists. This is not good. My bet is on 60e102ac73cd40069d077014c93c86dc7205cb68 Could you revert that? If that doesn't do the job, I think you'll need to bisect. 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: [RFC 2/2] usb: core: allow a reference device for new_id
> Yes, it can, if you look at the comment in usb_match_id(), we have to > check for this there. Geez, /me blind... Will send a patch. Thanks! signature.asc Description: Digital signature
[PATCH] usb: core: check for valid id_table when using the RefId feature
When implementing the RefId feature, it was missed that id_tables can be NULL under special circumstances. Bail out in that case. Signed-off-by: Wolfram Sang --- drivers/usb/core/driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 024dbfd..3f248b6 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -73,6 +73,9 @@ ssize_t usb_store_new_id(struct usb_dynids *dynids, if (fields > 4) { const struct usb_device_id *id = id_table; + if (!id) + return -ENODEV; + for (; id->match_flags; id++) if (id->idVendor == refVendor && id->idProduct == refProduct) break; -- 1.8.5.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
From: Bjørn Mork > Thomas Kear writes: > > > On Sat, Jan 11, 2014 at 11:09 AM, Bjrn Mork wrote: > >> But looking at the code I think I found and obvious miss in the SG list > >> initialisation. I'll post a proposed fix for that. Would be good if > >> someone was able to test it. > > > > I've built 3.13.0-rc7-next-20140110 with your patch applied. > > Unfortunately since this bug has taken anywhere from minutes to days > > to manifest previously I'm not sure how quickly I'll be able to report > > on its efficacy. > > Thanks for testing it. > > If I'm correct, then your problem is caused by usbnet incrementing > urb->num_sgs above the value sg_init_table was called with. This happens > if usbnet adds padding to a fragmented skb. Unfortunately I have no > idea how you can create fragmented skbs with a certain length. But I'm > sure others here know? I've managed to send fragmented skb, but not of specific lengths. Maybe Nagle can be used on a TCP to get the required merging. Send an initial fragment to 'prime' Nagle, then send a few fragments that get sent together when the timer expires (or send data the other way to force the send). The patch you submitted is wrong. Whoever wrote the sg interface was on crack. The 'last' marker needs moving as well. David
Re: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
David Laight writes: > The patch you submitted is wrong. > Whoever wrote the sg interface was on crack. > The 'last' marker needs moving as well. I'm afraid I don't understand what you meant by this. sg_init_table() set the 'last' marker. AFAICS, you don't need to change it unless you want to chain lists. Care to explain with some code? 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
RE: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
From: Bjørn Mork > David Laight writes: > > > The patch you submitted is wrong. > > Whoever wrote the sg interface was on crack. > > The 'last' marker needs moving as well. > > I'm afraid I don't understand what you meant by this. > > sg_init_table() set the 'last' marker. AFAICS, you don't need to change > it unless you want to chain lists. > > Care to explain with some code? Just assuming that there will be some code, somewhere, that will try to process the entire sg list - so won't like the entry with a NULL pointer and zero length at the end. If all the places that process the list are given an explicit number of entries, or don't care about the NULL it doesn't matter. David
Re: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
David Laight writes: > From: Bjørn Mork >> David Laight writes: >> >> > The patch you submitted is wrong. >> > Whoever wrote the sg interface was on crack. >> > The 'last' marker needs moving as well. >> >> I'm afraid I don't understand what you meant by this. >> >> sg_init_table() set the 'last' marker. AFAICS, you don't need to change >> it unless you want to chain lists. >> >> Care to explain with some code? > > Just assuming that there will be some code, somewhere, that will try > to process the entire sg list - so won't like the entry with a > NULL pointer and zero length at the end. > > If all the places that process the list are given an explicit > number of entries, or don't care about the NULL it doesn't matter. 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. 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 0/3] Fixes for FuntionFS
This is a short series of bugfixes for FunctionFS. The first patch fixes the problem found by Dan Carpenter (or his automatic tool?) - if ffs_dev is checked for being NULL it should be done so consistently throughout the ffs_release_dev(). ffs_alloc_dev() and ffs_free_dev() are used only in f_fs.c, so they can be static and their prototypes in u_fs.h are not necessary, this is covered in the second patch. The third patch introduces a consistent naming scheme with regard to taking the ffs_lock - if a function has to be called with the lock taken, its name starts with an underscore. Andrzej Pietrasiewicz (3): usb: gadget: FunctionFS: dereference ffs_dev conditionally usb: gadget: FunctionFS: staticize functions used only in f_fs.c usb: gadget: FunctionFS: use consistent naming with regard to ffs_lock drivers/usb/gadget/f_fs.c | 39 +-- drivers/usb/gadget/u_fs.h |2 -- 2 files changed, 21 insertions(+), 20 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] usb: gadget: FunctionFS: use consistent naming with regard to ffs_lock
Consistently prefix function name with underscore if the function has to be called with ffs_lock taken. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 96bea08..fffda61 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -161,10 +161,10 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data, DEFINE_MUTEX(ffs_lock); EXPORT_SYMBOL(ffs_lock); -static struct ffs_dev *ffs_find_dev(const char *name); -static struct ffs_dev *ffs_alloc_dev(void); +static struct ffs_dev *_ffs_find_dev(const char *name); +static struct ffs_dev *_ffs_alloc_dev(void); static int _ffs_name_dev(struct ffs_dev *dev, const char *name); -static void ffs_free_dev(struct ffs_dev *dev); +static void _ffs_free_dev(struct ffs_dev *dev); static void *ffs_acquire_dev(const char *dev_name); static void ffs_release_dev(struct ffs_data *ffs_data); static int ffs_ready(struct ffs_data *ffs); @@ -2255,7 +2255,7 @@ static int ffs_func_revmap_intf(struct ffs_function *func, u8 intf) static LIST_HEAD(ffs_devices); -static struct ffs_dev *_ffs_find_dev(const char *name) +static struct ffs_dev *_ffs_do_find_dev(const char *name) { struct ffs_dev *dev; @@ -2272,7 +2272,7 @@ static struct ffs_dev *_ffs_find_dev(const char *name) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_get_single_dev(void) +static struct ffs_dev *_ffs_get_single_dev(void) { struct ffs_dev *dev; @@ -2288,15 +2288,15 @@ static struct ffs_dev *ffs_get_single_dev(void) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_find_dev(const char *name) +static struct ffs_dev *_ffs_find_dev(const char *name) { struct ffs_dev *dev; - dev = ffs_get_single_dev(); + dev = _ffs_get_single_dev(); if (dev) return dev; - return _ffs_find_dev(name); + return _ffs_do_find_dev(name); } /* Configfs support */ @@ -2332,7 +2332,7 @@ static void ffs_free_inst(struct usb_function_instance *f) opts = to_f_fs_opts(f); ffs_dev_lock(); - ffs_free_dev(opts->dev); + _ffs_free_dev(opts->dev); ffs_dev_unlock(); kfree(opts); } @@ -2387,7 +2387,7 @@ static struct usb_function_instance *ffs_alloc_inst(void) opts->func_inst.set_inst_name = ffs_set_inst_name; opts->func_inst.free_func_inst = ffs_free_inst; ffs_dev_lock(); - dev = ffs_alloc_dev(); + dev = _ffs_alloc_dev(); ffs_dev_unlock(); if (IS_ERR(dev)) { kfree(opts); @@ -2475,12 +2475,12 @@ static struct usb_function *ffs_alloc(struct usb_function_instance *fi) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_alloc_dev(void) +static struct ffs_dev *_ffs_alloc_dev(void) { struct ffs_dev *dev; int ret; - if (ffs_get_single_dev()) + if (_ffs_get_single_dev()) return ERR_PTR(-EBUSY); dev = kzalloc(sizeof(*dev), GFP_KERNEL); @@ -2508,7 +2508,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const char *name) { struct ffs_dev *existing; - existing = _ffs_find_dev(name); + existing = _ffs_do_find_dev(name); if (existing) return -EBUSY; @@ -2552,7 +2552,7 @@ EXPORT_SYMBOL(ffs_single_dev); /* * ffs_lock must be taken by the caller of this function */ -static void ffs_free_dev(struct ffs_dev *dev) +static void _ffs_free_dev(struct ffs_dev *dev) { list_del(&dev->entry); if (dev->name_allocated) @@ -2569,7 +2569,7 @@ static void *ffs_acquire_dev(const char *dev_name) ENTER(); ffs_dev_lock(); - ffs_dev = ffs_find_dev(dev_name); + ffs_dev = _ffs_find_dev(dev_name); if (!ffs_dev) ffs_dev = ERR_PTR(-ENODEV); else if (ffs_dev->mounted) -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] usb: gadget: FunctionFS: staticize functions used only in f_fs.c
ffs_alloc_dev and ffs_free_dev are used only in f_fs.c, so make them static. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c |6 -- drivers/usb/gadget/u_fs.h |2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index f0c657d..96bea08 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -162,7 +162,9 @@ DEFINE_MUTEX(ffs_lock); EXPORT_SYMBOL(ffs_lock); static struct ffs_dev *ffs_find_dev(const char *name); +static struct ffs_dev *ffs_alloc_dev(void); static int _ffs_name_dev(struct ffs_dev *dev, const char *name); +static void ffs_free_dev(struct ffs_dev *dev); static void *ffs_acquire_dev(const char *dev_name); static void ffs_release_dev(struct ffs_data *ffs_data); static int ffs_ready(struct ffs_data *ffs); @@ -2473,7 +2475,7 @@ static struct usb_function *ffs_alloc(struct usb_function_instance *fi) /* * ffs_lock must be taken by the caller of this function */ -struct ffs_dev *ffs_alloc_dev(void) +static struct ffs_dev *ffs_alloc_dev(void) { struct ffs_dev *dev; int ret; @@ -2550,7 +2552,7 @@ EXPORT_SYMBOL(ffs_single_dev); /* * ffs_lock must be taken by the caller of this function */ -void ffs_free_dev(struct ffs_dev *dev) +static void ffs_free_dev(struct ffs_dev *dev) { list_del(&dev->entry); if (dev->name_allocated) diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h index bc2d371..f418c25 100644 --- a/drivers/usb/gadget/u_fs.h +++ b/drivers/usb/gadget/u_fs.h @@ -65,10 +65,8 @@ static inline void ffs_dev_unlock(void) mutex_unlock(&ffs_lock); } -struct ffs_dev *ffs_alloc_dev(void); int ffs_name_dev(struct ffs_dev *dev, const char *name); int ffs_single_dev(struct ffs_dev *dev); -void ffs_free_dev(struct ffs_dev *dev); struct ffs_epfile; struct ffs_function; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usb: gadget: FunctionFS: dereference ffs_dev conditionally
ffs_dev->ffs_release_dev_callback should be accessed only if ffs_dev is not NULL. Additionally whitespace error correction. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 306a2b5..f0c657d 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2509,7 +2509,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const char *name) existing = _ffs_find_dev(name); if (existing) return -EBUSY; - + dev->name = name; return 0; @@ -2590,11 +2590,12 @@ static void ffs_release_dev(struct ffs_data *ffs_data) ffs_dev_lock(); ffs_dev = ffs_data->private_data; - if (ffs_dev) + if (ffs_dev) { ffs_dev->mounted = false; - - if (ffs_dev->ffs_release_dev_callback) - ffs_dev->ffs_release_dev_callback(ffs_dev); + + if (ffs_dev->ffs_release_dev_callback) + ffs_dev->ffs_release_dev_callback(ffs_dev); + } ffs_dev_unlock(); } -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: PROBLEM: usbnet / ax88179_178a: Panic in usb_hcd_map_urb_for_dma
From: Bjørn Mork [mailto:bj...@mork.no] > David Laight writes: > > From: Bjørn Mork > >> David Laight writes: > >> > >> > The patch you submitted is wrong. > >> > Whoever wrote the sg interface was on crack. > >> > The 'last' marker needs moving as well. > >> > >> I'm afraid I don't understand what you meant by this. > >> > >> sg_init_table() set the 'last' marker. AFAICS, you don't need to change > >> it unless you want to chain lists. > >> > >> Care to explain with some code? > > > > Just assuming that there will be some code, somewhere, that will try > > to process the entire sg list - so won't like the entry with a > > NULL pointer and zero length at the end. > > > > If all the places that process the list are given an explicit > > number of entries, or don't care about the NULL it doesn't matter. > > 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. David
[RFC 01/10] xhci: Use command structures when calling xhci_configure_endpoint
To create a global command queue we need to fill a command structure for each entry on the command ring. We start by requiring xhci_configure_endpoint() to be called with a proper command structure. Functions xhci_check_maxpacket and xhci_check_bandwith that called xhci_configure_endpoint without a command strucure are fixed. A special case where an endpoint needs to be configured after reset, done in interrupt context is also changed to create a command strucure. (this command structure is not used yet, but will be later when we requre all queued commands to have a command strucure) Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 10 ++--- drivers/usb/host/xhci.c | 95 +++- 2 files changed, 54 insertions(+), 51 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1e2f3f4..da83a844 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1180,12 +1180,15 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id, * because the HW can't handle two commands being queued in a row. */ if (xhci->quirks & XHCI_RESET_EP_QUIRK) { + struct xhci_command *command; + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "Queueing configure endpoint command"); xhci_queue_configure_endpoint(xhci, xhci->devs[slot_id]->in_ctx->dma, slot_id, false); xhci_ring_cmd_db(xhci); + kfree(command); } else { /* Clear our internal halted state and restart the ring(s) */ xhci->devs[slot_id]->eps[ep_index].ep_state &= ~EP_HALTED; @@ -1439,7 +1442,7 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id, add_flags - SLOT_FLAG == drop_flags) { ep_state = virt_dev->eps[ep_index].ep_state; if (!(ep_state & EP_HALTED)) - goto bandwidth_change; + return; xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "Completed config ep cmd - " "last ep index = %d, state = %d", @@ -1449,11 +1452,6 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id, ring_doorbell_for_active_rings(xhci, slot_id, ep_index); return; } -bandwidth_change: - xhci_dbg_trace(xhci, trace_xhci_dbg_context_change, - "Completed config ep cmd"); - virt_dev->cmd_status = cmd_comp_code; - complete(&virt_dev->cmd_completion); return; } diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 4265b48..5e65052 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1179,10 +1179,10 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci, static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, unsigned int ep_index, struct urb *urb) { - struct xhci_container_ctx *in_ctx; struct xhci_container_ctx *out_ctx; struct xhci_input_control_ctx *ctrl_ctx; struct xhci_ep_ctx *ep_ctx; + struct xhci_command *command; int max_packet_size; int hw_max_packet_size; int ret = 0; @@ -1207,18 +1207,24 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, unsigned int slot_id, /* FIXME: This won't work if a non-default control endpoint * changes max packet sizes. */ - in_ctx = xhci->devs[slot_id]->in_ctx; - ctrl_ctx = xhci_get_input_control_ctx(xhci, in_ctx); + + command = xhci_alloc_command(xhci, false, true, GFP_KERNEL); + if (!command) + return -ENOMEM; + + command->in_ctx = xhci->devs[slot_id]->in_ctx; + ctrl_ctx = xhci_get_input_control_ctx(xhci, command->in_ctx); if (!ctrl_ctx) { xhci_warn(xhci, "%s: Could not get input context, bad type.\n", __func__); - return -ENOMEM; + ret = -ENOMEM; + goto command_cleanup; } /* Set up the modified control endpoint 0 */ xhci_endpoint_copy(xhci, xhci->devs[slot_id]->in_ctx, xhci->devs[slot_id]->out_ctx, ep_index); - ep_ctx = xhci_get_ep_ctx(xhci, in_ctx, ep_index); + ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET_MASK); ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size)); @@ -122
[RFC 04/10] xhci: use command structures for xhci_queue_stop_endpoint()
Prepare for the global command ring by using command structures internally in functions calling xhci_queue_stop_endpoint() Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 14 -- drivers/usb/host/xhci.c | 3 +++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 805f234..f93c842 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -20,7 +20,8 @@ * Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -#include + +#include #include #include "xhci.h" @@ -284,8 +285,17 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) spin_lock_irqsave(&xhci->lock, flags); for (i = LAST_EP_INDEX; i > 0; i--) { - if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) + if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) { + struct xhci_command *command; + command = xhci_alloc_command(xhci, false, false, +GFP_NOIO); + if (!command) { + xhci_free_command(xhci, cmd); + return -ENOMEM; + } xhci_queue_stop_endpoint(xhci, slot_id, i, suspend); + kfree(command); + } } cmd->command_trb = xhci_find_next_enqueue(xhci->cmd_ring); list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index a60e432..054132b 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1467,6 +1467,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) unsigned int ep_index; struct xhci_ring *ep_ring; struct xhci_virt_ep *ep; + struct xhci_command *command; xhci = hcd_to_xhci(hcd); spin_lock_irqsave(&xhci->lock, flags); @@ -1536,6 +1537,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) * the first cancellation to be handled. */ if (!(ep->ep_state & EP_HALT_PENDING)) { + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); ep->ep_state |= EP_HALT_PENDING; ep->stop_cmds_pending++; ep->stop_cmd_timer.expires = jiffies + @@ -1543,6 +1545,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) add_timer(&ep->stop_cmd_timer); xhci_queue_stop_endpoint(xhci, urb->dev->slot_id, ep_index, 0); xhci_ring_cmd_db(xhci); + kfree(command); } done: spin_unlock_irqrestore(&xhci->lock, flags); -- 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
[RFC 09/10] xhci: Use completion and status in global command queue
Remove the per-device command list and handle_cmd_in_cmd_wait_list() and use the completion and status variables found in the command structure in the global command list. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 11 -- drivers/usb/host/xhci-mem.c | 1 - drivers/usb/host/xhci-ring.c | 79 drivers/usb/host/xhci.c | 20 ++- drivers/usb/host/xhci.h | 3 -- 5 files changed, 17 insertions(+), 97 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 3a08e26..78983d6 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -298,7 +298,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) suspend); } } - list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list); xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); @@ -310,18 +309,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) if (timeleft <= 0) { xhci_warn(xhci, "%s while waiting for stop endpoint command\n", timeleft == 0 ? "Timeout" : "Signal"); - spin_lock_irqsave(&xhci->lock, flags); - /* The timeout might have raced with the event ring handler, so -* only delete from the list if the item isn't poisoned. -*/ - if (cmd->cmd_list.next != LIST_POISON1) - list_del(&cmd->cmd_list); - spin_unlock_irqrestore(&xhci->lock, flags); ret = -ETIME; - goto command_cleanup; } - -command_cleanup: xhci_free_command(xhci, cmd); return ret; } diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 7f8e4c3..80b9c11 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -919,7 +919,6 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, dev->num_rings_cached = 0; init_completion(&dev->cmd_completion); - INIT_LIST_HEAD(&dev->cmd_list); dev->udev = udev; /* Point to output device context in dcbaa. */ diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5ccf642..6b61787 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -69,10 +69,6 @@ #include "xhci.h" #include "xhci-trace.h" -static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci, - struct xhci_virt_device *virt_dev, - struct xhci_event_cmd *event); - /* * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA * address of the TRB. @@ -761,7 +757,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, union xhci_trb *trb, struct xhci_event_cmd *event) { unsigned int ep_index; - struct xhci_virt_device *virt_dev; struct xhci_ring *ep_ring; struct xhci_virt_ep *ep; struct list_head *entry; @@ -771,11 +766,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, struct xhci_dequeue_state deq_state; if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] { - virt_dev = xhci->devs[slot_id]; - if (virt_dev) - handle_cmd_in_cmd_wait_list(xhci, virt_dev, - event); - else + if (!xhci->devs[slot_id]) xhci_warn(xhci, "Stop endpoint command " "completion for disabled slot %u\n", slot_id); @@ -1203,29 +1194,6 @@ static void xhci_complete_cmd_in_cmd_wait_list(struct xhci_hcd *xhci, } -/* Check to see if a command in the device's command queue matches this one. - * Signal the completion or free the command, and return 1. Return 0 if the - * completed command isn't at the head of the command list. - */ -static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci, - struct xhci_virt_device *virt_dev, - struct xhci_event_cmd *event) -{ - struct xhci_command *command; - - if (list_empty(&virt_dev->cmd_list)) - return 0; - - command = list_entry(virt_dev->cmd_list.next, - struct xhci_command, cmd_list); - if (xhci->cmd_ring->dequeue != command->command_trb) - return 0; - - xhci_complete_cmd_in_cmd_wait_list(xhci, command, - GET_COMP_CODE(le32_to_cpu(event->status))); - return 1; -} - /* * Finding the command trb need to be cancelled and modifying it to * NO OP command. And if the command is in device's command wait @@ -1377,7 +1345,6 @@ static void xhci_handle_cmd_enable_sl
[RFC 10/10] xhci: rework command timeout and cancellation,
Use one timer to control command timeout. start/kick the timer every time a command is completed and a new command is waiting, or a new command is added to a empty list. If the timer runs out, then tag the current command as "aborted", and start the xhci command abortion process. Previously each function that submitted a command had its own timer. If that command timed out, a new command structure for the command was created and it was put on a cancel_cmd_list list, then a pci write to abort the command ring was issued. when the ring was aborted, it checked if the current command was the one to be cancelled, later when the ring was stopped the driver got ownership of the TRBs in the command ring, compared then to the TRBs in the cancel_cmd_list, and turned them into No-ops. Now, instead, at timeout we tag the status of the command in the command queue to be aborted, and start the ring abortion. Ring abortion stops the command ring and gives control of the commands to us. All the aborted commands are now turned into No-ops. This allows us to remove the entire cancel_cmd_list code. The functions waiting for a command to finish no longer have their own timeouts. They will wait either until the command completes normally, or until the whole command abortion is done. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 11 +- drivers/usb/host/xhci-mem.c | 15 +- drivers/usb/host/xhci-ring.c | 336 +-- drivers/usb/host/xhci.c | 79 -- drivers/usb/host/xhci.h | 8 +- 5 files changed, 139 insertions(+), 310 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 78983d6..34daa5a 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -271,7 +271,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) struct xhci_virt_device *virt_dev; struct xhci_command *cmd; unsigned long flags; - int timeleft; int ret; int i; @@ -303,12 +302,10 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) spin_unlock_irqrestore(&xhci->lock, flags); /* Wait for last stop endpoint command to finish */ - timeleft = wait_for_completion_interruptible_timeout( - cmd->completion, - XHCI_CMD_DEFAULT_TIMEOUT); - if (timeleft <= 0) { - xhci_warn(xhci, "%s while waiting for stop endpoint command\n", - timeleft == 0 ? "Timeout" : "Signal"); + wait_for_completion(cmd->completion); + + if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) { + xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n"); ret = -ETIME; } xhci_free_command(xhci, cmd); diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 80b9c11..d27178b 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1692,7 +1692,6 @@ void xhci_free_command(struct xhci_hcd *xhci, void xhci_mem_cleanup(struct xhci_hcd *xhci) { struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); - struct xhci_cd *cur_cd, *next_cd; struct xhci_command *cur_cmd, *next_cmd; int size; int i, j, num_ports; @@ -1712,15 +1711,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) if (xhci->lpm_command) xhci_free_command(xhci, xhci->lpm_command); xhci->cmd_ring_reserved_trbs = 0; + + del_timer_sync(&xhci->cmd_timer); + if (xhci->cmd_ring) xhci_ring_free(xhci, xhci->cmd_ring); xhci->cmd_ring = NULL; xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring"); - list_for_each_entry_safe(cur_cd, next_cd, - &xhci->cancel_cmd_list, cancel_cmd_list) { - list_del(&cur_cd->cancel_cmd_list); - kfree(cur_cd); - } list_for_each_entry_safe(cur_cmd, next_cmd, &xhci->cmd_list, cmd_list) { @@ -2228,7 +2225,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) u32 page_size, temp; int i; - INIT_LIST_HEAD(&xhci->cancel_cmd_list); INIT_LIST_HEAD(&xhci->cmd_list); page_size = xhci_readl(xhci, &xhci->op_regs->page_size); @@ -2414,6 +2410,11 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Wrote ERST address to ir_set 0."); xhci_print_ir_set(xhci, 0); + /* init command timeout timer */ + init_timer(&xhci->cmd_timer); + xhci->cmd_timer.data = (unsigned long) xhci; + xhci->cmd_timer.function = xhci_handle_command_timeout; + /* * XXX: Might need to set the Interrupter Moderation Register to * something other than the default (~1ms minimum between interrupts). diff --git a/drivers/usb/host/xhci-rin
[RFC 08/10] xhci: Add a global command queue
Create a list to store command structures, add a strucure to it every time a command is submitted, and remove it from the list once we get a command completion event matching the command. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-mem.c | 8 drivers/usb/host/xhci-ring.c | 13 - drivers/usb/host/xhci.h | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 49b8bd0..7f8e4c3 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1694,6 +1694,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) { struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); struct xhci_cd *cur_cd, *next_cd; + struct xhci_command *cur_cmd, *next_cmd; int size; int i, j, num_ports; @@ -1722,6 +1723,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) kfree(cur_cd); } + list_for_each_entry_safe(cur_cmd, next_cmd, + &xhci->cmd_list, cmd_list) { + list_del(&cur_cmd->cmd_list); + kfree(cur_cmd); + } + for (i = 1; i < MAX_HC_SLOTS; ++i) xhci_free_virt_device(xhci, i); @@ -2223,6 +2230,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) int i; INIT_LIST_HEAD(&xhci->cancel_cmd_list); + INIT_LIST_HEAD(&xhci->cmd_list); page_size = xhci_readl(xhci, &xhci->op_regs->page_size); xhci_dbg_trace(xhci, trace_xhci_dbg_init, diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 6f6018c..5ccf642 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1502,6 +1502,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, dma_addr_t cmd_dequeue_dma; u32 cmd_comp_code; union xhci_trb *cmd_trb; + struct xhci_command *cmd; u32 cmd_type; cmd_dma = le64_to_cpu(event->cmd_trb); @@ -1519,6 +1520,13 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, return; } + cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list); + + if (cmd->command_trb != xhci->cmd_ring->dequeue) { + xhci_err(xhci, +"Command completion event does not match command\n"); + return; + } trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event); cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status)); @@ -1588,6 +1596,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci->error_bitmask |= 1 << 6; break; } + + list_del(&cmd->cmd_list); + inc_deq(xhci, xhci->cmd_ring); } @@ -3989,7 +4000,7 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, return ret; } cmd->command_trb = xhci->cmd_ring->enqueue; - + list_add_tail(&cmd->cmd_list, &xhci->cmd_list); queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3, field4 | xhci->cmd_ring->cycle_state); return 0; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index d02b73d..71aed35 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1475,6 +1475,7 @@ struct xhci_hcd { #define CMD_RING_STATE_ABORTED (1 << 1) #define CMD_RING_STATE_STOPPED (1 << 2) struct list_headcancel_cmd_list; + struct list_headcmd_list; unsigned intcmd_ring_reserved_trbs; struct xhci_ring*event_ring; struct xhci_ersterst; -- 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
[RFC 07/10] xhci: Use command structured when queuing commands
Require each queued command to have a command structure. We store the command trb pointer in the structure when queuing it, making the find_next_enqueue() function obsolete. Don't free the command strucures right away after sending the commands, We will free the commands when we receive a "command handled" event in a later patch. Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 8 ++-- drivers/usb/host/xhci-ring.c | 94 ++-- drivers/usb/host/xhci.c | 47 +++--- drivers/usb/host/xhci.h | 31 --- 4 files changed, 91 insertions(+), 89 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index f93c842..3a08e26 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -292,14 +292,14 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) if (!command) { xhci_free_command(xhci, cmd); return -ENOMEM; + } - xhci_queue_stop_endpoint(xhci, slot_id, i, suspend); - kfree(command); + xhci_queue_stop_endpoint(xhci, command, slot_id, i, +suspend); } } - cmd->command_trb = xhci_find_next_enqueue(xhci->cmd_ring); list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list); - xhci_queue_stop_endpoint(xhci, slot_id, 0, suspend); + xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bf50d28..6f6018c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -123,16 +123,6 @@ static int enqueue_is_link_trb(struct xhci_ring *ring) 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, -* we must handle that -*/ - if (TRB_TYPE_LINK_LE32(ring->enqueue->link.control)) - return ring->enq_seg->next->trbs; - return ring->enqueue; -} - /* Updates trb to point to the next TRB in the ring, and updates seg if the next * TRB is in a new segment. This does not skip over link TRBs, and it does not * effect the ring dequeue or enqueue pointers. @@ -680,12 +670,14 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, } } -static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id, +static int queue_set_tr_deq(struct xhci_hcd *xhci, + struct xhci_command *cmd, int slot_id, unsigned int ep_index, unsigned int stream_id, struct xhci_segment *deq_seg, union xhci_trb *deq_ptr, u32 cycle_state); void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci, + struct xhci_command *cmd, unsigned int slot_id, unsigned int ep_index, unsigned int stream_id, struct xhci_dequeue_state *deq_state) @@ -700,7 +692,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci, deq_state->new_deq_ptr, (unsigned long long)xhci_trb_virt_to_dma(deq_state->new_deq_seg, deq_state->new_deq_ptr), deq_state->new_cycle_state); - queue_set_tr_deq(xhci, slot_id, ep_index, stream_id, + queue_set_tr_deq(xhci, cmd, slot_id, ep_index, stream_id, deq_state->new_deq_seg, deq_state->new_deq_ptr, (u32) deq_state->new_cycle_state); @@ -857,12 +849,11 @@ remove_finished_td: if (deq_state.new_deq_ptr && deq_state.new_deq_seg) { struct xhci_command *command; command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); - xhci_queue_new_dequeue_state(xhci, + xhci_queue_new_dequeue_state(xhci, command, slot_id, ep_index, ep->stopped_td->urb->stream_id, &deq_state); xhci_ring_cmd_db(xhci); - kfree(command); } else { /* Otherwise ring the doorbell(s) to restart queued transfers */ ring_doorbell_for_active_rings(xhci, slot_id, ep_index); @@ -1187,11 +1178,10 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id, command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "Queueing configure endpoint command"); - xhci_queue_configure_endpoint(xhci, + xhci_queue_configure_endpoint(xhci, command
[RFC 06/10] xhci: use command structures for xhci_queue_reset_ep()
Prepare for the global command ring by using command structures internally in functions calling xhci_queue_reset_ep() Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 5 + drivers/usb/host/xhci.c | 6 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index df5b0f8..bf50d28 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1913,6 +1913,10 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci, struct xhci_td *td, union xhci_trb *event_trb) { struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index]; + struct xhci_command *command; + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); + if (!command) + return; ep->ep_state |= EP_HALTED; ep->stopped_td = td; ep->stopped_trb = event_trb; @@ -1926,6 +1930,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci, ep->stopped_stream = 0; xhci_ring_cmd_db(xhci); + kfree(command); } /* Check if an error has halted the endpoint ring. The class driver will diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index fa0ac48..23e057f 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2906,6 +2906,7 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, unsigned long flags; int ret; struct xhci_virt_ep *virt_ep; + struct xhci_command *command; xhci = hcd_to_xhci(hcd); udev = (struct usb_device *) ep->hcpriv; @@ -2928,6 +2929,10 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, return; } + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); + if (!command) + return; + xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, "Queueing reset endpoint command"); spin_lock_irqsave(&xhci->lock, flags); @@ -2946,6 +2951,7 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, virt_ep->stopped_trb = NULL; virt_ep->stopped_stream = 0; spin_unlock_irqrestore(&xhci->lock, flags); + kfree(command); if (ret) xhci_warn(xhci, "FIXME allocate a new ring segment\n"); -- 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
[RFC 00/10] xhci: re-work command queue management
This is an attempt to re-work and solve the issues in xhci command queue management that Sarah has descibed earlier: Right now, the command management in the xHCI driver is rather ad-hock. Different parts of the driver all submit commands, including interrupt handling routines, functions called from the USB core (with or without the bus bandwidth mutex held). Some times they need to wait for the command to complete, and sometimes they just issue the command and don't care about the result of the command. The places that wait on a command all time the command for five seconds, and then attempt to cancel the command. Unfortunately, that means if several commands are issued at once, and one of them times out, all the commands timeout, even though the host hasn't gotten a chance to service them yet. This is apparent with some devices that take a long time to respond to the Set Address command during device enumeration (when the device is plugged in). If a driver for a different device attempts to change alternate interface settings at the same time (causing a Configure Endpoint command to be issued), both commands timeout. Instead of having each command timeout after five seconds, the driver should wait indefinitely in an uninterruptible sleep on the command completion. A global command queue manager should time whatever command is currently running, and cancel that command after five seconds. If the commands were in a list, like TDs currently are, it may be easier to keep track of where the command ring dequeue pointer is, and avoid racing with events. We may need to have parts of the driver that issue commands without waiting on them still put the commands in the command list. The Implementation: --- First step is to create a list of the commands submitted to the command queue. To accomplish this each command is required to be submitted with a properly filled command structure containing completion, status variable and a pointer to the command TRB that will be used. The first 7 patches are all about creating these command structures and submitting them when we queue commands. The command structures are allocated on the fly, the commands that are submitted in interrupt context are allocated with GFP_ATOMIC. Next, the global command queue is introduced. Commands are added to the queue when trb's are queued, and remove when the commad completes. Also switch to use the status variable and completion in the command struct. A new timer handles command timeout, the timer is kicked every time when a command finishes and there's a new command waiting in the queue, or when a new command is submitted to an _empty_ command queue. Timer is deleted when the the last command on the queue finishes (empty queue) The old cancel_cmd_list is removed. When the timer expires we simply tag the current command as "ABORTED" and start the ring abortion process. Functions waiting for an aborted command to finish are called after the command abortion is completed. Mathias Nyman (10): xhci: Use command structures when calling xhci_configure_endpoint xhci: use a command structure internally in xhci_address_device() xhci: use command structures for xhci_queue_slot_control() xhci: use command structures for xhci_queue_stop_endpoint() xhci: use command structure for xhci_queue_new_dequeue_state() xhci: use command structures for xhci_queue_reset_ep() xhci: Use command structured when queuing commands xhci: Add a global command queue xhci: Use completion and status in global command queue xhci: rework command timeout and cancellation, drivers/usb/host/xhci-hub.c | 42 ++-- drivers/usb/host/xhci-mem.c | 22 +- drivers/usb/host/xhci-ring.c | 532 ++- drivers/usb/host/xhci.c | 264 +++-- drivers/usb/host/xhci.h | 43 ++-- 5 files changed, 373 insertions(+), 530 deletions(-) -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 03/10] xhci: use command structures for xhci_queue_slot_control()
Preparing for the global command queue by changing functions calling xhci_queue_slot_control() to internally use command structures Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 0167a1c..a60e432 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3562,6 +3562,11 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) unsigned long flags; u32 state; int i, ret; + struct xhci_command *command; + + command = xhci_alloc_command(xhci, false, false, GFP_KERNEL); + if (!command) + return; #ifndef CONFIG_USB_DEFAULT_PERSIST /* @@ -3577,8 +3582,10 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) /* If the host is halted due to driver unload, we still need to free the * device. */ - if (ret <= 0 && ret != -ENODEV) + if (ret <= 0 && ret != -ENODEV) { + kfree(command); return; + } virt_dev = xhci->devs[udev->slot_id]; @@ -3595,16 +3602,20 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) (xhci->xhc_state & XHCI_STATE_HALTED)) { xhci_free_virt_device(xhci, udev->slot_id); spin_unlock_irqrestore(&xhci->lock, flags); + kfree(command); return; } if (xhci_queue_slot_control(xhci, TRB_DISABLE_SLOT, udev->slot_id)) { spin_unlock_irqrestore(&xhci->lock, flags); xhci_dbg(xhci, "FIXME: allocate a command ring segment\n"); + kfree(command); return; } xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); + kfree(command); + /* * Event command completion handler will free any data structures * associated with the slot. XXX Can free sleep? @@ -3644,31 +3655,41 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) unsigned long flags; int timeleft; int ret; - union xhci_trb *cmd_trb; + struct xhci_command *command; + + command = xhci_alloc_command(xhci, false, false, GFP_KERNEL); + if (!command) + return 0; spin_lock_irqsave(&xhci->lock, flags); - cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring); + command->command_trb = xhci_find_next_enqueue(xhci->cmd_ring); + command->completion = &xhci->addr_dev; ret = xhci_queue_slot_control(xhci, TRB_ENABLE_SLOT, 0); if (ret) { spin_unlock_irqrestore(&xhci->lock, flags); xhci_dbg(xhci, "FIXME: allocate a command ring segment\n"); + kfree(command); return 0; } xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); /* XXX: how much time for xHC slot assignment? */ - timeleft = wait_for_completion_interruptible_timeout(&xhci->addr_dev, + timeleft = wait_for_completion_interruptible_timeout( + command->completion, XHCI_CMD_DEFAULT_TIMEOUT); if (timeleft <= 0) { xhci_warn(xhci, "%s while waiting for a slot\n", timeleft == 0 ? "Timeout" : "Signal"); /* cancel the enable slot request */ - return xhci_cancel_cmd(xhci, NULL, cmd_trb); + ret = xhci_cancel_cmd(xhci, NULL, command->command_trb); + kfree(command); + return ret; } if (!xhci->slot_id) { xhci_err(xhci, "Error while assigning device slot ID\n"); + kfree(command); return 0; } @@ -3703,6 +3724,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) pm_runtime_get_noresume(hcd->self.controller); #endif + + kfree(command); /* Is this a LS or FS device under a HS hub? */ /* Hub or peripherial? */ return 1; @@ -3713,6 +3736,7 @@ disable_slot: if (!xhci_queue_slot_control(xhci, TRB_DISABLE_SLOT, udev->slot_id)) xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); + kfree(command); return 0; } -- 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
[RFC 05/10] xhci: use command structure for xhci_queue_new_dequeue_state()
Prepare for the global command ring by using command structures internally in functions calling xhci_queue_new_dequeue_state() Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 3 +++ drivers/usb/host/xhci.c | 6 ++ 2 files changed, 9 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index da83a844..df5b0f8 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -855,11 +855,14 @@ remove_finished_td: /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */ if (deq_state.new_deq_ptr && deq_state.new_deq_seg) { + struct xhci_command *command; + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); xhci_queue_new_dequeue_state(xhci, slot_id, ep_index, ep->stopped_td->urb->stream_id, &deq_state); xhci_ring_cmd_db(xhci); + kfree(command); } else { /* Otherwise ring the doorbell(s) to restart queued transfers */ ring_doorbell_for_active_rings(xhci, slot_id, ep_index); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 054132b..fa0ac48 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2867,10 +2867,16 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, * issue a configure endpoint command later. */ if (!(xhci->quirks & XHCI_RESET_EP_QUIRK)) { + struct xhci_command *command; + /* Can't sleep if we're called from cleanup_halted_endpoint() */ + command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); + if (!command) + return; xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, "Queueing new dequeue state"); xhci_queue_new_dequeue_state(xhci, udev->slot_id, ep_index, ep->stopped_stream, &deq_state); + kfree(command); } else { /* Better hope no one uses the input context between now and the * reset endpoint completion! -- 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
[RFC 02/10] xhci: use a command structure internally in xhci_address_device()
Preparing for the global command queue by using command strucure in xhci_address_device Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 5e65052..0167a1c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3732,7 +3732,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) struct xhci_slot_ctx *slot_ctx; struct xhci_input_control_ctx *ctrl_ctx; u64 temp_64; - union xhci_trb *cmd_trb; + struct xhci_command *command; if (!udev->slot_id) { xhci_dbg_trace(xhci, trace_xhci_dbg_address, @@ -3753,11 +3753,19 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) return -EINVAL; } + command = xhci_alloc_command(xhci, false, false, GFP_KERNEL); + if (!command) + return -ENOMEM; + + command->in_ctx = virt_dev->in_ctx; + command->completion = &xhci->addr_dev; + slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx); ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev->in_ctx); if (!ctrl_ctx) { xhci_warn(xhci, "%s: Could not get input context, bad type.\n", __func__); + kfree(command); return -EINVAL; } /* @@ -3779,20 +3787,22 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) slot_ctx->dev_info >> 27); spin_lock_irqsave(&xhci->lock, flags); - cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring); + command->command_trb = xhci_find_next_enqueue(xhci->cmd_ring); ret = xhci_queue_address_device(xhci, virt_dev->in_ctx->dma, udev->slot_id); if (ret) { spin_unlock_irqrestore(&xhci->lock, flags); xhci_dbg_trace(xhci, trace_xhci_dbg_address, "FIXME: allocate a command ring segment"); + kfree(command); return ret; } xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); /* ctrl tx can take up to 5 sec; XXX: need more time for xHC? */ - timeleft = wait_for_completion_interruptible_timeout(&xhci->addr_dev, + timeleft = wait_for_completion_interruptible_timeout( + command->completion, XHCI_CMD_DEFAULT_TIMEOUT); /* FIXME: From section 4.3.4: "Software shall be responsible for timing * the SetAddress() "recovery interval" required by USB and aborting the @@ -3802,7 +3812,8 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_warn(xhci, "%s while waiting for address device command\n", timeleft == 0 ? "Timeout" : "Signal"); /* cancel the address device command */ - ret = xhci_cancel_cmd(xhci, NULL, cmd_trb); + ret = xhci_cancel_cmd(xhci, NULL, command->command_trb); + kfree(command); if (ret < 0) return ret; return -ETIME; @@ -3838,6 +3849,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) break; } if (ret) { + kfree(command); return ret; } temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr); @@ -3872,7 +3884,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) xhci_dbg_trace(xhci, trace_xhci_dbg_address, "Internal device address = %d", le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK); - + kfree(command); return 0; } -- 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: [PATCH 1/3] usb: gadget: FunctionFS: dereference ffs_dev conditionally
Hello. On 13-01-2014 17:26, Andrzej Pietrasiewicz wrote: ffs_dev->ffs_release_dev_callback should be accessed only if ffs_dev is not NULL. Additionally whitespace error correction. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 306a2b5..f0c657d 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2509,7 +2509,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const char *name) existing = _ffs_find_dev(name); if (existing) return -EBUSY; - + dev->name = name; return 0; Unrelated whitespace change, even in the different function. This classifies as a cleanup, not fix. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] usb: gadget: FunctionFS: staticize functions used only in f_fs.c
ffs_alloc_dev and ffs_free_dev are used only in f_fs.c, so make them static. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c |6 -- drivers/usb/gadget/u_fs.h |2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index f0c657d..96bea08 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -162,7 +162,9 @@ DEFINE_MUTEX(ffs_lock); EXPORT_SYMBOL(ffs_lock); static struct ffs_dev *ffs_find_dev(const char *name); +static struct ffs_dev *ffs_alloc_dev(void); static int _ffs_name_dev(struct ffs_dev *dev, const char *name); +static void ffs_free_dev(struct ffs_dev *dev); static void *ffs_acquire_dev(const char *dev_name); static void ffs_release_dev(struct ffs_data *ffs_data); static int ffs_ready(struct ffs_data *ffs); @@ -2473,7 +2475,7 @@ static struct usb_function *ffs_alloc(struct usb_function_instance *fi) /* * ffs_lock must be taken by the caller of this function */ -struct ffs_dev *ffs_alloc_dev(void) +static struct ffs_dev *ffs_alloc_dev(void) { struct ffs_dev *dev; int ret; @@ -2550,7 +2552,7 @@ EXPORT_SYMBOL(ffs_single_dev); /* * ffs_lock must be taken by the caller of this function */ -void ffs_free_dev(struct ffs_dev *dev) +static void ffs_free_dev(struct ffs_dev *dev) { list_del(&dev->entry); if (dev->name_allocated) diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h index bc2d371..f418c25 100644 --- a/drivers/usb/gadget/u_fs.h +++ b/drivers/usb/gadget/u_fs.h @@ -65,10 +65,8 @@ static inline void ffs_dev_unlock(void) mutex_unlock(&ffs_lock); } -struct ffs_dev *ffs_alloc_dev(void); int ffs_name_dev(struct ffs_dev *dev, const char *name); int ffs_single_dev(struct ffs_dev *dev); -void ffs_free_dev(struct ffs_dev *dev); struct ffs_epfile; struct ffs_function; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] usb: gadget: FunctionFS: use consistent naming with regard to ffs_lock
Consistently prefix function name with underscore if the function has to be called with ffs_lock taken. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 96bea08..fffda61 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -161,10 +161,10 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data, DEFINE_MUTEX(ffs_lock); EXPORT_SYMBOL(ffs_lock); -static struct ffs_dev *ffs_find_dev(const char *name); -static struct ffs_dev *ffs_alloc_dev(void); +static struct ffs_dev *_ffs_find_dev(const char *name); +static struct ffs_dev *_ffs_alloc_dev(void); static int _ffs_name_dev(struct ffs_dev *dev, const char *name); -static void ffs_free_dev(struct ffs_dev *dev); +static void _ffs_free_dev(struct ffs_dev *dev); static void *ffs_acquire_dev(const char *dev_name); static void ffs_release_dev(struct ffs_data *ffs_data); static int ffs_ready(struct ffs_data *ffs); @@ -2255,7 +2255,7 @@ static int ffs_func_revmap_intf(struct ffs_function *func, u8 intf) static LIST_HEAD(ffs_devices); -static struct ffs_dev *_ffs_find_dev(const char *name) +static struct ffs_dev *_ffs_do_find_dev(const char *name) { struct ffs_dev *dev; @@ -2272,7 +2272,7 @@ static struct ffs_dev *_ffs_find_dev(const char *name) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_get_single_dev(void) +static struct ffs_dev *_ffs_get_single_dev(void) { struct ffs_dev *dev; @@ -2288,15 +2288,15 @@ static struct ffs_dev *ffs_get_single_dev(void) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_find_dev(const char *name) +static struct ffs_dev *_ffs_find_dev(const char *name) { struct ffs_dev *dev; - dev = ffs_get_single_dev(); + dev = _ffs_get_single_dev(); if (dev) return dev; - return _ffs_find_dev(name); + return _ffs_do_find_dev(name); } /* Configfs support */ @@ -2332,7 +2332,7 @@ static void ffs_free_inst(struct usb_function_instance *f) opts = to_f_fs_opts(f); ffs_dev_lock(); - ffs_free_dev(opts->dev); + _ffs_free_dev(opts->dev); ffs_dev_unlock(); kfree(opts); } @@ -2387,7 +2387,7 @@ static struct usb_function_instance *ffs_alloc_inst(void) opts->func_inst.set_inst_name = ffs_set_inst_name; opts->func_inst.free_func_inst = ffs_free_inst; ffs_dev_lock(); - dev = ffs_alloc_dev(); + dev = _ffs_alloc_dev(); ffs_dev_unlock(); if (IS_ERR(dev)) { kfree(opts); @@ -2475,12 +2475,12 @@ static struct usb_function *ffs_alloc(struct usb_function_instance *fi) /* * ffs_lock must be taken by the caller of this function */ -static struct ffs_dev *ffs_alloc_dev(void) +static struct ffs_dev *_ffs_alloc_dev(void) { struct ffs_dev *dev; int ret; - if (ffs_get_single_dev()) + if (_ffs_get_single_dev()) return ERR_PTR(-EBUSY); dev = kzalloc(sizeof(*dev), GFP_KERNEL); @@ -2508,7 +2508,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const char *name) { struct ffs_dev *existing; - existing = _ffs_find_dev(name); + existing = _ffs_do_find_dev(name); if (existing) return -EBUSY; @@ -2552,7 +2552,7 @@ EXPORT_SYMBOL(ffs_single_dev); /* * ffs_lock must be taken by the caller of this function */ -static void ffs_free_dev(struct ffs_dev *dev) +static void _ffs_free_dev(struct ffs_dev *dev) { list_del(&dev->entry); if (dev->name_allocated) @@ -2569,7 +2569,7 @@ static void *ffs_acquire_dev(const char *dev_name) ENTER(); ffs_dev_lock(); - ffs_dev = ffs_find_dev(dev_name); + ffs_dev = _ffs_find_dev(dev_name); if (!ffs_dev) ffs_dev = ERR_PTR(-ENODEV); else if (ffs_dev->mounted) -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] Fixes for FuntionFS
This is a short series of bugfixes for FunctionFS. The first patch fixes the problem found by Dan Carpenter (or his automatic tool?) - if ffs_dev is checked for being NULL it should be done so consistently throughout the ffs_release_dev(). The second patch is a code style cleanup. ffs_alloc_dev() and ffs_free_dev() are used only in f_fs.c, so they can be static and their prototypes in u_fs.h are not necessary, this is covered in the third patch. The fourth patch introduces a consistent naming scheme with regard to taking the ffs_lock - if a function has to be called with the lock taken, its name starts with an underscore. v1..v2: - split the first patch into a fix proper and code cleanup Andrzej Pietrasiewicz (4): usb: gadget: FunctionFS: dereference ffs_dev conditionally usb: gadget: code cleanup usb: gadget: FunctionFS: staticize functions used only in f_fs.c usb: gadget: FunctionFS: use consistent naming with regard to ffs_lock drivers/usb/gadget/f_fs.c | 39 +-- drivers/usb/gadget/u_fs.h |2 -- 2 files changed, 21 insertions(+), 20 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] usb: gadget: FunctionFS: dereference ffs_dev conditionally
ffs_dev->ffs_release_dev_callback should be accessed only if ffs_dev is not NULL. Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c |9 + 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 306a2b5..78333f0 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2590,11 +2590,12 @@ static void ffs_release_dev(struct ffs_data *ffs_data) ffs_dev_lock(); ffs_dev = ffs_data->private_data; - if (ffs_dev) + if (ffs_dev) { ffs_dev->mounted = false; - - if (ffs_dev->ffs_release_dev_callback) - ffs_dev->ffs_release_dev_callback(ffs_dev); + + if (ffs_dev->ffs_release_dev_callback) + ffs_dev->ffs_release_dev_callback(ffs_dev); + } ffs_dev_unlock(); } -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] usb: gadget: code cleanup
Remove trailing whitespace Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/f_fs.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 78333f0..f0c657d 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -2509,7 +2509,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const char *name) existing = _ffs_find_dev(name); if (existing) return -EBUSY; - + dev->name = name; return 0; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] ohci and ehci-platform clks, phy and dt support
Hi, On 01/12/2014 02:04 PM, Tomasz Figa wrote: Hi, [Cc'ing DT maintainers directly] Alan Stern Wrote: I prefer the -generic option, although generic- is equally fine - Having said that, I don't really care if it's called mmio either (although this does seem less 'descriptive'). I can do a v5 changing the compatible string to generix-Xhci, if that will put an end to all this discussion, then again, there may be a better way, see below. Grepping over existing dts files, I can find several occurrences of "usb-ehci" compatible string: at91sam9g45.dtsi: compatible = "atmel,at91sam9g45-ehci", "usb-ehci"; at91sam9x5.dtsi:compatible = "atmel,at91sam9g45-ehci", "usb-ehci"; omap3.dtsi: compatible = "ti,ehci-omap", "usb-ehci"; omap4.dtsi: compatible = "ti,ehci-omap", "usb-ehci"; omap5.dtsi: compatible = "ti,ehci-omap", "usb-ehci"; sama5d3.dtsi: compatible = "atmel,at91sam9g45-ehci", "usb-ehci"; spear13xx.dtsi: compatible = "st,spear600-ehci", "usb-ehci"; spear13xx.dtsi: compatible = "st,spear600-ehci", "usb-ehci"; spear3xx.dtsi: compatible = "st,spear600-ehci", "usb-ehci"; spear600.dtsi: compatible = "st,spear600-ehci", "usb-ehci"; spear600.dtsi: compatible = "st,spear600-ehci", "usb-ehci"; tegra114.dtsi: compatible = "nvidia,tegra30-ehci", "usb-ehci"; tegra114.dtsi: compatible = "nvidia,tegra30-ehci", "usb-ehci"; tegra20.dtsi: compatible = "nvidia,tegra20-ehci", "usb-ehci"; tegra20.dtsi: compatible = "nvidia,tegra20-ehci", "usb-ehci"; tegra20.dtsi: compatible = "nvidia,tegra20-ehci", "usb-ehci"; tegra30.dtsi: compatible = "nvidia,tegra30-ehci", "usb-ehci"; tegra30.dtsi: compatible = "nvidia,tegra30-ehci", "usb-ehci"; tegra30.dtsi: compatible = "nvidia,tegra30-ehci", "usb-ehci"; Same for "usb-ohci": arch/arm/boot/dts/at91rm9200.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/at91sam9260.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/at91sam9263.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/at91sam9g45.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/at91sam9n12.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/at91sam9x5.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/lpc32xx.dtsi: compatible = "nxp,ohci-nxp", "usb-ohci"; arch/arm/boot/dts/omap3.dtsi: compatible = "ti,ohci-omap3", "usb-ohci"; arch/arm/boot/dts/omap4.dtsi: compatible = "ti,ohci-omap3", "usb-ohci"; arch/arm/boot/dts/omap5.dtsi: compatible = "ti,ohci-omap3", "usb-ohci"; arch/arm/boot/dts/sama5d3.dtsi: compatible = "atmel,at91rm9200-ohci", "usb-ohci"; arch/arm/boot/dts/spear13xx.dtsi: compatible = "st,spear600-ohci", "usb-ohci"; arch/arm/boot/dts/spear13xx.dtsi: compatible = "st,spear600-ohci", "usb-ohci"; arch/arm/boot/dts/spear3xx.dtsi:compatible = "st,spear600-ohci", "usb-ohci"; arch/arm/boot/dts/spear3xx.dtsi:compatible = "st,spear600-ohci", "usb-ohci"; arch/arm/boot/dts/spear600.dtsi:compatible = "st,spear600-ohci", "usb-ohci"; arch/arm/boot/dts/spear600.dtsi:compatible = "st,spear600-ohci", "usb-ohci"; For "usb-ehci" there is even a documentation file [1], while "usb-ohci" seems to be undocumented. [1] Documentation/devicetree/bindings/usb/usb-ehci.txt Aren't they both something that should be accounted for in this series? I agree that usb-Xhci would be the best compatible strings to use. The problem with usb-ehci is that there already is a ppc specific driver binding to that compatible string, doing various ppc specific controller initialization. Thinking more about this, there is one possible solution though, the ehci-ppc-of.c is guarded in Kconfig with: depends on PPC_OF If we add an inverted check to the Kconfig option for platform-ehci.c, ie: config USB_EHCI_HCD_PLATFORM tristate "Generic EHCI driver for a platform device" depends on !PPC_OF Then we can be certain that we don't end up with 2 drivers claiming the usb-ehci compatible on ppc platforms. I've done some quick research and it seems that ehci-platform.c is only used on arm and mips devices, so excluding its use on ppc should not be an issue. Then later on someone, who has the actual hardware to test, can merge the ppc specific quirk handling into ehci-platform,c and ehci-ppc-of.c can go
Re: [PATCH v4 0/2] ohci and ehci-platform clks, phy and dt support
On Mon, 13 Jan 2014, Hans de Goede wrote: > I agree that usb-Xhci would be the best compatible strings to use. > > The problem with usb-ehci is that there already is a ppc specific driver > binding to that > compatible string, doing various ppc specific controller initialization. > > Thinking more about this, there is one possible solution though, the > ehci-ppc-of.c is > guarded in Kconfig with: > > depends on PPC_OF > > If we add an inverted check to the Kconfig option for platform-ehci.c, ie: > > config USB_EHCI_HCD_PLATFORM > tristate "Generic EHCI driver for a platform device" > depends on !PPC_OF > > Then we can be certain that we don't end up with 2 drivers claiming the > usb-ehci compatible on ppc platforms. I've done some quick research > and it seems that ehci-platform.c is only used on arm and mips devices, so > excluding its use on ppc should not be an issue. > > Then later on someone, who has the actual hardware to test, can merge the > ppc specific quirk handling into ehci-platform,c and ehci-ppc-of.c can go > away entirely. > > Alan, if you agree this is the best way forward, I'll do a v5 with the > proposed > changes. That's okay with me. 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] xhci: fix resume issues on Renesas chips
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. Not sure if I should restrict this further or not. Probably should be sent to stable too. Signed-off-by: Don Zickus --- drivers/usb/host/xhci-pci.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 73f5208..2a21892 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -144,6 +144,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) } if (pdev->vendor == PCI_VENDOR_ID_VIA) xhci->quirks |= XHCI_RESET_ON_RESUME; + if (pdev->vendor == PCI_VENDOR_ID_RENESAS) + xhci->quirks |= XHCI_RESET_ON_RESUME; } /* called during probe() after chip reset completes */ -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] usb: gadget: code cleanup
On Mon, Jan 13 2014, Andrzej Pietrasiewicz wrote: > Remove trailing whitespace > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/f_fs.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c > index 78333f0..f0c657d 100644 > --- a/drivers/usb/gadget/f_fs.c > +++ b/drivers/usb/gadget/f_fs.c > @@ -2509,7 +2509,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, const > char *name) > existing = _ffs_find_dev(name); > if (existing) > return -EBUSY; > - > + > dev->name = name; > > return 0; > -- > 1.7.0.4 > -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH v2 1/4] usb: gadget: FunctionFS: dereference ffs_dev conditionally
On Mon, Jan 13 2014, Andrzej Pietrasiewicz wrote: > ffs_dev->ffs_release_dev_callback should be accessed only if ffs_dev > is not NULL. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/f_fs.c |9 + > 1 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c > index 306a2b5..78333f0 100644 > --- a/drivers/usb/gadget/f_fs.c > +++ b/drivers/usb/gadget/f_fs.c > @@ -2590,11 +2590,12 @@ static void ffs_release_dev(struct ffs_data *ffs_data) > ffs_dev_lock(); > > ffs_dev = ffs_data->private_data; > - if (ffs_dev) > + if (ffs_dev) { > ffs_dev->mounted = false; > - > - if (ffs_dev->ffs_release_dev_callback) > - ffs_dev->ffs_release_dev_callback(ffs_dev); > + > + if (ffs_dev->ffs_release_dev_callback) > + ffs_dev->ffs_release_dev_callback(ffs_dev); > + } > > ffs_dev_unlock(); > } > -- > 1.7.0.4 > -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH v2 4/4] usb: gadget: FunctionFS: use consistent naming with regard to ffs_lock
On Mon, Jan 13 2014, Andrzej Pietrasiewicz wrote: > Consistently prefix function name with underscore if the function has to > be called with ffs_lock taken. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/f_fs.c | 30 +++--- > 1 files changed, 15 insertions(+), 15 deletions(-) -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH v2 3/4] usb: gadget: FunctionFS: staticize functions used only in f_fs.c
On Mon, Jan 13 2014, Andrzej Pietrasiewicz wrote: > ffs_alloc_dev and ffs_free_dev are used only in f_fs.c, > so make them static. > > Signed-off-by: Andrzej Pietrasiewicz Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/f_fs.c |6 -- > drivers/usb/gadget/u_fs.h |2 -- > 2 files changed, 4 insertions(+), 4 deletions(-) -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
RE: [RFC 00/10] xhci: re-work command queue management
From: Mathias Nyman > This is an attempt to re-work and solve the issues in xhci command > queue management that Sarah has descibed earlier: > > Right now, the command management in the xHCI driver is rather ad-hock. > Different parts of the driver all submit commands, including interrupt > handling routines, functions called from the USB core (with or without the > bus bandwidth mutex held). > Some times they need to wait for the command to complete, and sometimes > they just issue the command and don't care about the result of the command. > ... > > The Implementation: > --- > > First step is to create a list of the commands submitted to the command queue. > To accomplish this each command is required to be submitted with a properly > filled command structure containing completion, status variable and a pointer > to > the command TRB that will be used. > > The first 7 patches are all about creating these command structures and > submitting them when we queue commands. > The command structures are allocated on the fly, the commands that are > submitted > in interrupt context are allocated with GFP_ATOMIC. > > Next, the global command queue is introduced. Commands are added to the queue > when trb's are queued, and remove when the commad completes. > Also switch to use the status variable and completion in the command struct. ... IMHO the xhci driver is already far too complicated, and this probably just makes it even worse. The fact that you are having to allocate memory ion an ISR ought also to be ringing alarm bells. Have you considered adding a 'software command ring' (indexed with the same value as the hardware one) and using it to hold additional parameters? It might even be worth only putting a single command into the hardware ring! That might simplify the timer code. This still has a fixed constraint on the number of queued commands, but I suspect that is bounded anyway (a few per device?). If not you can almost certainly arrange to grow the soft-ring before the isr code can run out of entries. 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 v3 00/10] Just the essential port power control fixes for 3.14
On Tue, Jan 07, 2014 at 12:29:28PM -0800, Dan Williams wrote: > Alan, Sarah, > > This revision boils down the port power control fixes to the > bare minimum to get the implementation functional and reliable. > Data structure changes are constrained to struct usb_port and > gone are the clumsier attempts at wider reworks from v1 [1] and > v2 [2]. No device model changes to consider or changes to the > meaning of 'runtime_status' for port devices. Three disconnect > bugs are fixed: > > 1/ Superspeed devices downgrade to their hi-speed connection: fix this by >preventing superspeed poweroff until the peer port is suspended. See >patch 5. > > 2/ khubd taking disconnect action on ports that are in the process of >being recovered: khubd now ignores ports in the pm-runtime-suspended >state. Alan, per your comment [3] this effectively uses the pm_usage >counter and state as a lock against khubd. See patch 7. Does that mean this patchset does not depend on the four warm port reset patches you sent on 12/20? http://marc.info/?l=linux-usb&m=138759482824618&w=2 Sarah Sharp > 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of >the port power state. Fixed by forcing a full recovery cycle of the >device before allowing the next suspend, and blocking khubd while the >resume is in progress. See patch 9. > > Patch overview: > [PATCH 01/10] usb: assign default peer ports for root hubs > [PATCH 02/10] usb: find external hub port peers > [PATCH 03/10] usb: find internal hub tier mismatch via acpi > [PATCH 04/10] usb: sysfs link peer ports > * Per our discussions of v1 these patches implement a > simple algorithm for associating peer ports across > internal and external hubs. > > [PATCH 05/10] usb: defer suspension of superspeed port while peer is > powered > * Fix case 1 > > [PATCH 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs > * Cleanup misuse of ClearPortFeature(PORT_C_ENABLE) > > [PATCH 07/10] usb: synchronize port poweroff and khubd > * Fix case 2 > > [PATCH 08/10] usb: cleanup straggling C_PORT_RESET C_PORT_LINK_STATE > notifications > * Handle some unexpected hub events encountered during testing > > [PATCH 09/10] usb: make khubd and subsequent suspension wait for port > recovery > * Fix case 3 > > [PATCH 10/10] usb: documentation for usb port power off mechanisms > > These patches were tested by repeatedly power toggling all > 35 ports in the following topology: > /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M > |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/4p, 5000M > |__ Port 1: Dev 3, If 0, Class=vend., Driver=ax88179_178a, 5000M > |__ Port 2: Dev 4, If 0, Class=stor., Driver=usb-storage, 5000M > |__ Port 4: Dev 5, If 0, Class=stor., Driver=usb-storage, 5000M > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/9p, 480M > |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/4p, 480M > |__ Port 3: Dev 4, If 0, Class=HID, Driver=usbhid, 12M > |__ Port 2: Dev 3, If 0, Class=hub, Driver=hub/4p, 480M > |__ Port 1: Dev 5, If 0, Class=HID, Driver=usbhid, 1.5M > |__ Port 2: Dev 6, If 0, Class=HID, Driver=usbhid, 1.5M > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M > |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/8p, 480M > > Each iteration of the test verifies that no disconnects occur and that > all ports reach the 'suspended' state. To force device suspension > interface drivers are unbound for power-off and then rebound. Note that > since the hub drivers are never unbound their parent ports remain active > due to ->do_remote_wakeup for the hub device, but the the 30 other ports > reliably suspend and resume now with these patches. The proposed warm > reset changes [4] do not appear to be required as long as superspeed hub > parent ports remain powered. > > [1] http://marc.info/?l=linux-usb&m=138260013707007&w=2 > [2] http://marc.info/?l=linux-usb&m=138511124910669&w=2 > [3] http://marc.info/?l=linux-usb&m=138775577717546&w=2 > [4] http://marc.info/?l=linux-usb&m=138759482824618&w=2 > > --- > > Dan Williams (9): > usb: assign default peer ports for root hubs > usb: find external hub port peers > usb: find internal hub tier mismatch via acpi > usb: sysfs link peer ports > usb: don't suspend port while peer is powered > usb: gate clearing FEAT_C_ENABLE to usb2 hubs > usb: synchronize port poweroff and khubd > usb: cleanup straggling C_PORT_RESET C_PORT_LINK_STATE notifications > usb: make khubd and subsequent suspension wait for port recovery > > Lan Tianyu (1): > USB: Documentation for USB port power off mechanisms > > > Documentation/usb/power-management.txt | 210 +++ > drivers/usb/core/hub.c | 112 ++ > drive
Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14
On Mon, Jan 13, 2014 at 11:01 AM, Sarah Sharp wrote: > On Tue, Jan 07, 2014 at 12:29:28PM -0800, Dan Williams wrote: >> Alan, Sarah, >> >> This revision boils down the port power control fixes to the >> bare minimum to get the implementation functional and reliable. >> Data structure changes are constrained to struct usb_port and >> gone are the clumsier attempts at wider reworks from v1 [1] and >> v2 [2]. No device model changes to consider or changes to the >> meaning of 'runtime_status' for port devices. Three disconnect >> bugs are fixed: >> >> 1/ Superspeed devices downgrade to their hi-speed connection: fix this by >>preventing superspeed poweroff until the peer port is suspended. See >>patch 5. >> >> 2/ khubd taking disconnect action on ports that are in the process of >>being recovered: khubd now ignores ports in the pm-runtime-suspended >>state. Alan, per your comment [3] this effectively uses the pm_usage >>counter and state as a lock against khubd. See patch 7. > > Does that mean this patchset does not depend on the four warm port reset > patches you sent on 12/20? > > http://marc.info/?l=linux-usb&m=138759482824618&w=2 Precisely. That set was aiming to solve some of the interactions by borrowing from the "libsas/libata" approach, but this take is able to achieve the same without that level of surgery. -- Dan -- 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-next 3/3] r8152: add supporting the vendor mode only
From: Hayes Wang Date: Thu, 9 Jan 2014 14:50:32 +0800 > Remove the limitation that the ecm and r8152 drivers couldn't coexist. > Besides, add the feature to support the vendor mode only. This let > someone who doesn't want to use ecm driver easy to use the vendor > driver without creating the udev rule. > > Signed-off-by: Hayes Wang This is a very bad precedence and I do not want drivers to start doing things like this. I think it would be wiser to use the existing facilities which exist to control this situation, and yes udev is one of them. I'm sorry, I'm not applying this series as long as it has this patch in it. -- 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
EG20T USB Gadget Performance
Hi all, I have an EG20T-based board and have some issues with performance on the USB device interface. I made a libusb test program (using the async interface)[0] to read data from the EG20T's USB device port which has the gadget zero source/sink function bound. In theory, one would hope this would give the fastest real-world results for the hardware connected. The test program submits 32 IN transfers and re-submits on transfer completion, counting received packets. From running my test program for a few minutes I get the following: elapsed: 548.468416 seconds packets: 21503334 packets/sec: 39206.148199 bytes/sec: 20073547.877732 MBit/sec: 160.588383 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7 transactions happening quickly (with about 14us separating them), but every 8th transaction, the EG20T will NAK between 20-80 times[1], losing 50-100us[2]. This delay happens every 8th transaction without fail[3]. I've looked at the following: 1. The f_sourcesink.c function it queues up 8 responses at the beginning. Changing this number up or down had no effect. 2. Analysis of pch_udc.c doesn't show anything which would obviously cause a delay every 8th packet. 3. f_eem seems to have roughly the same performance with ping -f -s 64000 (160Mbit/sec). The CPU load of the gadget-side Atom PC sits very close to zero. System Details: Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) Intel Atom E680 with EG20T I seem to have eliminated everything on the host side, since the host is asking for data, and the device is saying it doesn't have any for up to 100us at a time. What am I missing? Alan. [0] http://www.signal11.us/~alan/recv.c [1] The number of NAKs not important and doesn't correlate to the amount of time spent NAK-ing. [2] Analyzer screenshots: http://www.signal11.us/~alan/nak.png http://www.signal11.us/~alan/nak_open.png [3] Often it looks like it's aligned to a SOF packet, but that's only because when you delay 100us, the odds are good you will be NAK-ing when a SOF packet arrives (every 125us). Sometimes the NAK-ing will start before the SOF and sometimes after. -- 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: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs
On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote: > Суббота, 11 января 2014, 13:55 +01:00 от Uwe Kleine-König > : > > On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote: > > > Hello. > > > > > > > On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote: > > > > > This adds i.MX27 and i.MX31 as the next user of the usbmisc > > > > > driver. > > > > > > > > > > Signed-off-by: Alexander Shiyan > > > > > --- > > > > > drivers/usb/chipidea/usbmisc_imx.c | 42 > > > > > ++ > > > > > 1 file changed, 42 insertions(+) > > > ... > > > > At this point might be good to patch the imx27.dtsi with the usb > > > > defines. > > > ... > > > > > > I have a working configuration for i.MX27 USB, but I prefer to make a few > > > more > > > tests before the addition of definitions in DTS. This will be a next step. > > > Thanks. > > Any news here? > > Not ready yet. Are you still working at it? Would you mind sharing more details, like your current tree/patch stack and what works/doesn't work for you? Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] Move DWC2 driver out of staging
The DWC2 driver should now be in good enough shape to move out of staging. I have stress tested it overnight on RPI running mass storage and Ethernet transfers in parallel, and for several days on our proprietary PCI-based platform. Signed-off-by: Paul Zimmerman --- Greg, I believe I have addressed all of the feedback since I last submitted this. Is there still time to do this before you close your trees? drivers/staging/Kconfig | 2 -- drivers/staging/Makefile | 1 - drivers/staging/dwc2/TODO | 33 --- drivers/usb/Kconfig | 2 ++ drivers/usb/Makefile | 1 + drivers/{staging => usb}/dwc2/Kconfig | 0 drivers/{staging => usb}/dwc2/Makefile| 0 drivers/{staging => usb}/dwc2/core.c | 0 drivers/{staging => usb}/dwc2/core.h | 0 drivers/{staging => usb}/dwc2/core_intr.c | 0 drivers/{staging => usb}/dwc2/hcd.c | 0 drivers/{staging => usb}/dwc2/hcd.h | 0 drivers/{staging => usb}/dwc2/hcd_ddma.c | 0 drivers/{staging => usb}/dwc2/hcd_intr.c | 0 drivers/{staging => usb}/dwc2/hcd_queue.c | 0 drivers/{staging => usb}/dwc2/hw.h| 0 drivers/{staging => usb}/dwc2/pci.c | 0 drivers/{staging => usb}/dwc2/platform.c | 0 18 files changed, 3 insertions(+), 36 deletions(-) delete mode 100644 drivers/staging/dwc2/TODO rename drivers/{staging => usb}/dwc2/Kconfig (100%) rename drivers/{staging => usb}/dwc2/Makefile (100%) rename drivers/{staging => usb}/dwc2/core.c (100%) rename drivers/{staging => usb}/dwc2/core.h (100%) rename drivers/{staging => usb}/dwc2/core_intr.c (100%) rename drivers/{staging => usb}/dwc2/hcd.c (100%) rename drivers/{staging => usb}/dwc2/hcd.h (100%) rename drivers/{staging => usb}/dwc2/hcd_ddma.c (100%) rename drivers/{staging => usb}/dwc2/hcd_intr.c (100%) rename drivers/{staging => usb}/dwc2/hcd_queue.c (100%) rename drivers/{staging => usb}/dwc2/hw.h (100%) rename drivers/{staging => usb}/dwc2/pci.c (100%) rename drivers/{staging => usb}/dwc2/platform.c (100%) diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index d2beb07..4bb6b11 100644 --- a/drivers/staging/Kconfig +++ b/drivers/staging/Kconfig @@ -140,8 +140,6 @@ source "drivers/staging/netlogic/Kconfig" source "drivers/staging/mt29f_spinand/Kconfig" -source "drivers/staging/dwc2/Kconfig" - source "drivers/staging/lustre/Kconfig" source "drivers/staging/xillybus/Kconfig" diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile index bf62386..9f07e5e 100644 --- a/drivers/staging/Makefile +++ b/drivers/staging/Makefile @@ -61,7 +61,6 @@ obj-$(CONFIG_DGRP)+= dgrp/ obj-$(CONFIG_SB105X) += sb105x/ obj-$(CONFIG_FIREWIRE_SERIAL) += fwserial/ obj-$(CONFIG_GOLDFISH) += goldfish/ -obj-$(CONFIG_USB_DWC2) += dwc2/ obj-$(CONFIG_LUSTRE_FS)+= lustre/ obj-$(CONFIG_XILLYBUS) += xillybus/ obj-$(CONFIG_DGNC) += dgnc/ diff --git a/drivers/staging/dwc2/TODO b/drivers/staging/dwc2/TODO deleted file mode 100644 index 282470d..000 --- a/drivers/staging/dwc2/TODO +++ /dev/null @@ -1,33 +0,0 @@ -TODO: - - Dan Carpenter would like to see some cleanups to the microframe - scheduler code: - http://www.mail-archive.com/linux-usb@vger.kernel.org/msg26650.html - - - Should merge the NAK holdoff patch from Raspberry Pi - (http://marc.info/?l=linux-usb&m=137625067103833). But as it stands - that patch is incomplete, it needs more investigation to see if it - can be made to work for non-Raspberry Pi platforms that lack the - special FIQ interrupt that the Pi has. Without this patch, the driver - has a high interrupt rate (8K/sec). - - - The Raspberry Pi platform needs to have support for its FIQ interrupt - added, to get the same level of functionality as the downstream - driver. The raspberrypi.org developers have indicated they are - willing to help with that. - - - Some of the default driver parameters (see 'struct dwc2_core_params' - in core.h) won't work for many platforms. So DT attributes will need - to be added for some of these. But that can be done as-needed as new - platforms are added. - - - Eventually the driver should be merged with the s3c-hsotg peripheral - mode driver, so that both modes of operation can be supported with a - single driver. But I think that can wait till after the driver has - been moved to mainline. - - - After that, OTG support can be added. I'm not sure how much demand - there is for that, though, so I have that as a low priority. - -Please send any patches for this driver to Paul Zimmerman -and Greg Kroah-Hartman . And please CC linux-usb - too. diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 2642b8a..7eb909a 100644 --- a/drivers
Re: [PATCH v3 10/10] usb: documentation for usb port power off mechanisms
A couple comments below. On Tue, Jan 07, 2014 at 12:30:21PM -0800, Dan Williams wrote: > From: Lan Tianyu > > describe the mechanisms for controlling port power policy and > discovering the port power state. > > Cc: Oliver Neukum > Signed-off-by: Lan Tianyu > Signed-off-by: Sarah Sharp I think this version of the patch was changed since the last time I internally signed off on it. I'll have to be more careful about sending draft patches out with my Signed-off-by line. > Signed-off-by: Dan Williams > --- > Documentation/usb/power-management.txt | 210 > > 1 files changed, 210 insertions(+), 0 deletions(-) > > diff --git a/Documentation/usb/power-management.txt > b/Documentation/usb/power-management.txt > index 1392b61d6ebe..e5e77a67abb7 100644 > --- a/Documentation/usb/power-management.txt > +++ b/Documentation/usb/power-management.txt > @@ -5,6 +5,25 @@ ... > + User Interface for Port Power Control > + - > + > +The port power control mechanism uses the PM runtime system. Poweroff is > +requested by clearing the power/pm_qos_no_power_off flag of the port device > +(defaults to 1). If the port is disconnected it will immediately receive a > +ClearPortFeature(PORT_POWER) request. Otherwise, it will honor the pm > runtime > +rules and requrire the attached child device and all descendants to be > +suspended. > + > +Note, some interface devices/drivers do not support autosuspend. Userspace > may > +need to unbind the interface drivers before the usb_device will suspend. An > +unbound interface device is suspended by default. > + > +Example of the relevant files for port power control. > + > + child device link + > + port device + | > +parent hub + | | > + v v v > + /sys/bus/devices/usb2/2-0:1.0/port1/device > + > + /sys/bus/devices/usb2/2-0:1.0/port1/power/pm_qos_no_power_off > + /sys/bus/devices/usb2/2-0:1.0/port1/device/power/control > + /sys/bus/devices/usb2/2-0:1.0/port1/device//driver/unbind > + > +In addition to these files some ports may have a 'peer' link to a port on > +another hub. The expectation is that all superspeed ports have a > +hi-speed peer. > + > +/sys/bus/usb/devices/usb2/2-0:1.0/port1/peer -> ../../../usb3/3-0:1.0/port1 > +/sys/bus/usb/devices/usb3/3-0:1.0/port1/peer -> ../../../usb2/2-0:1.0/port1 > + > +Physically, a superspeed port is ganged with a hi-speed port to form a > +connector. Distinct from 'companion ports', peer ports share the same > +ancestor XHCI device. Whereas, with a companion port, an EHCI or XHCI > +device optionally drive the same pins. While a superspeed port is I think you mean "with a companion port, an EHCI or OHCI/UHCI device optionally drive the same pins". Or are you talking about the Intel EHCI to xHCI port switchover? We don't call those companion ports in that case. > +powered off a device may downgrade its connection and attempt to connect > +to the hi-speed pins. The implementation takes steps to prevent this > +and sequences port power to guarantee that hi-speed ports are > +powered-off before superspeed are allowed to suspend, and that > +superspeed ports are recovered/repowered before hi-speed. > + > + power/pm_qos_no_power_off: > + This writable flag controls the state of an idle port. > + Once all children and descendants have suspended the > + port may suspend/poweroff provided that > + pm_qos_no_power_off is '0'. If pm_qos_no_power_off is > + '1' the port will remain active/powered regardless of > + the stats of descendants. You probably want to mention the default policy of '1' here. > + > + power/runtime_status: > + This file reflects whether the port is 'active' (power is on) > + or 'suspended' (logically off). There is no indication to > + userspace whether VBUS is still supplied. > + > + connect_type: > + An advisory read-only flag to userspace indicating the > + location and connection type of the port. It returns > + one of four values 'hotplug', 'hardwired', 'not used', > + and 'unknown'. All values, besides unknown, are set by > + platform firmware. > + > + "hotplug" indicates an externally connectable/visible > + port on the platform. Typically userspace would choose > + to keep such a port powered to handle new device > + connection events. > + > + "hardwired" refers to a port that is not visible but > + connectable. Examples are internal ports for USB > + bluetooth that can be disconnected via an external > + switch or a port with a hardwired USB camera. It is > + expected to be safe to allow these ports to suspend
Re: EG20T USB Gadget Performance
On Mon, 13 Jan 2014, Alan Ott wrote: > Hi all, > > I have an EG20T-based board and have some issues with performance on the > USB device interface. > > I made a libusb test program (using the async interface)[0] to read data > from the EG20T's USB device port which has the gadget zero source/sink > function bound. In theory, one would hope this would give the fastest > real-world results for the hardware connected. > > The test program submits 32 IN transfers and re-submits on transfer > completion, counting received packets. > > From running my test program for a few minutes I get the following: > elapsed: 548.468416 seconds > packets: 21503334 > packets/sec: 39206.148199 > bytes/sec: 20073547.877732 > MBit/sec: 160.588383 > > 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows > 7 transactions happening quickly (with about 14us separating them), but > every 8th transaction, the EG20T will NAK between 20-80 times[1], losing > 50-100us[2]. > > This delay happens every 8th transaction without fail[3]. > > I've looked at the following: > 1. The f_sourcesink.c function it queues up 8 responses at the > beginning. Changing this number up or down had no effect. > 2. Analysis of pch_udc.c doesn't show anything which would obviously > cause a delay every 8th packet. > 3. f_eem seems to have roughly the same performance with ping -f -s > 64000 (160Mbit/sec). > > The CPU load of the gadget-side Atom PC sits very close to zero. > > System Details: > Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) > Intel Atom E680 with EG20T > > I seem to have eliminated everything on the host side, since the host is > asking for data, and the device is saying it doesn't have any for up to > 100us at a time. > > What am I missing? The analyzer clearly indicates that something is wrong on the peripheral (as opposed to the host). Since the gadget driver seems simple and straightforward, most likely the UDC driver is the cause (or the UDC hardware). You can test this by adding a few pr_debug statements at appropriate places in f_sourcesink.c. Certainly one every time the completion routine gets called, maybe also one after each request submission. You might want to combine this with changing the size of the buffer, so that the 8 packets/buffer doesn't confused with the 8 packets/delay. You might also want to try asking the driver's authors, who appear to be Toshiharu Okada and Tomoya MORINAGA , judging by the git log. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] Move DWC2 driver out of staging
The DWC2 driver should now be in good enough shape to move out of staging. I have stress tested it overnight on RPI running mass storage and Ethernet transfers in parallel, and for several days on our proprietary PCI-based platform. Signed-off-by: Paul Zimmerman --- v4: Also change directory path in MAINTAINERS Greg, I believe I have addressed all of the feedback since I last submitted this. Is there still time to do this before you close your trees? MAINTAINERS | 2 +- drivers/staging/Kconfig | 2 -- drivers/staging/Makefile | 1 - drivers/staging/dwc2/TODO | 33 --- drivers/usb/Kconfig | 2 ++ drivers/usb/Makefile | 1 + drivers/{staging => usb}/dwc2/Kconfig | 0 drivers/{staging => usb}/dwc2/Makefile| 0 drivers/{staging => usb}/dwc2/core.c | 0 drivers/{staging => usb}/dwc2/core.h | 0 drivers/{staging => usb}/dwc2/core_intr.c | 0 drivers/{staging => usb}/dwc2/hcd.c | 0 drivers/{staging => usb}/dwc2/hcd.h | 0 drivers/{staging => usb}/dwc2/hcd_ddma.c | 0 drivers/{staging => usb}/dwc2/hcd_intr.c | 0 drivers/{staging => usb}/dwc2/hcd_queue.c | 0 drivers/{staging => usb}/dwc2/hw.h| 0 drivers/{staging => usb}/dwc2/pci.c | 0 drivers/{staging => usb}/dwc2/platform.c | 0 19 files changed, 4 insertions(+), 37 deletions(-) delete mode 100644 drivers/staging/dwc2/TODO rename drivers/{staging => usb}/dwc2/Kconfig (100%) rename drivers/{staging => usb}/dwc2/Makefile (100%) rename drivers/{staging => usb}/dwc2/core.c (100%) rename drivers/{staging => usb}/dwc2/core.h (100%) rename drivers/{staging => usb}/dwc2/core_intr.c (100%) rename drivers/{staging => usb}/dwc2/hcd.c (100%) rename drivers/{staging => usb}/dwc2/hcd.h (100%) rename drivers/{staging => usb}/dwc2/hcd_ddma.c (100%) rename drivers/{staging => usb}/dwc2/hcd_intr.c (100%) rename drivers/{staging => usb}/dwc2/hcd_queue.c (100%) rename drivers/{staging => usb}/dwc2/hw.h (100%) rename drivers/{staging => usb}/dwc2/pci.c (100%) rename drivers/{staging => usb}/dwc2/platform.c (100%) diff --git a/MAINTAINERS b/MAINTAINERS index d5e4ff3..d3303eb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2635,7 +2635,7 @@ DESIGNWARE USB2 DRD IP DRIVER M: Paul Zimmerman L: linux-usb@vger.kernel.org S: Maintained -F: drivers/staging/dwc2/ +F: drivers/usb/dwc2/ DESIGNWARE USB3 DRD IP DRIVER M: Felipe Balbi diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index d2beb07..4bb6b11 100644 --- a/drivers/staging/Kconfig +++ b/drivers/staging/Kconfig @@ -140,8 +140,6 @@ source "drivers/staging/netlogic/Kconfig" source "drivers/staging/mt29f_spinand/Kconfig" -source "drivers/staging/dwc2/Kconfig" - source "drivers/staging/lustre/Kconfig" source "drivers/staging/xillybus/Kconfig" diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile index bf62386..9f07e5e 100644 --- a/drivers/staging/Makefile +++ b/drivers/staging/Makefile @@ -61,7 +61,6 @@ obj-$(CONFIG_DGRP)+= dgrp/ obj-$(CONFIG_SB105X) += sb105x/ obj-$(CONFIG_FIREWIRE_SERIAL) += fwserial/ obj-$(CONFIG_GOLDFISH) += goldfish/ -obj-$(CONFIG_USB_DWC2) += dwc2/ obj-$(CONFIG_LUSTRE_FS)+= lustre/ obj-$(CONFIG_XILLYBUS) += xillybus/ obj-$(CONFIG_DGNC) += dgnc/ diff --git a/drivers/staging/dwc2/TODO b/drivers/staging/dwc2/TODO deleted file mode 100644 index 282470d..000 --- a/drivers/staging/dwc2/TODO +++ /dev/null @@ -1,33 +0,0 @@ -TODO: - - Dan Carpenter would like to see some cleanups to the microframe - scheduler code: - http://www.mail-archive.com/linux-usb@vger.kernel.org/msg26650.html - - - Should merge the NAK holdoff patch from Raspberry Pi - (http://marc.info/?l=linux-usb&m=137625067103833). But as it stands - that patch is incomplete, it needs more investigation to see if it - can be made to work for non-Raspberry Pi platforms that lack the - special FIQ interrupt that the Pi has. Without this patch, the driver - has a high interrupt rate (8K/sec). - - - The Raspberry Pi platform needs to have support for its FIQ interrupt - added, to get the same level of functionality as the downstream - driver. The raspberrypi.org developers have indicated they are - willing to help with that. - - - Some of the default driver parameters (see 'struct dwc2_core_params' - in core.h) won't work for many platforms. So DT attributes will need - to be added for some of these. But that can be done as-needed as new - platforms are added. - - - Eventually the driver should be merged with the s3c-hsotg peripheral - mode driver, so that both modes of operation can be supported with a - single driver. But I think
Re: [PATCH v4] Move DWC2 driver out of staging
On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: > The DWC2 driver should now be in good enough shape to move out of > staging. I have stress tested it overnight on RPI running mass > storage and Ethernet transfers in parallel, and for several days > on our proprietary PCI-based platform. > > Signed-off-by: Paul Zimmerman > --- > v4: Also change directory path in MAINTAINERS > > Greg, > I believe I have addressed all of the feedback since I last > submitted this. Is there still time to do this before you > close your trees? As I want to close my tree now, let me take a look at this, as it's the perfect time to do this. One question, did you make this patch against my staging.git or usb.git tree? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] Move DWC2 driver out of staging
> From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Monday, January 13, 2014 2:04 PM > > On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: >> The DWC2 driver should now be in good enough shape to move out of >> staging. I have stress tested it overnight on RPI running mass >> storage and Ethernet transfers in parallel, and for several days >> on our proprietary PCI-based platform. >> >> Signed-off-by: Paul Zimmerman >> --- >> v4: Also change directory path in MAINTAINERS >> >> Greg, >> I believe I have addressed all of the feedback since I last >> submitted this. Is there still time to do this before you >> close your trees? > > As I want to close my tree now, let me take a look at this, as it's the > perfect time to do this. One question, did you make this patch against > my staging.git or usb.git tree? It's against your staging tree. -- Paul -- 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: EG20T USB Gadget Performance
Toshiharu and Tomoya, I sent the email below to linux-usb today in regards to some performance issues I'm seeing with the EG20T (pch_udc) driver. I appreciate any insight you may have. Alan. On 01/13/2014 03:20 PM, Alan Ott wrote: Hi all, I have an EG20T-based board and have some issues with performance on the USB device interface. I made a libusb test program (using the async interface)[0] to read data from the EG20T's USB device port which has the gadget zero source/sink function bound. In theory, one would hope this would give the fastest real-world results for the hardware connected. The test program submits 32 IN transfers and re-submits on transfer completion, counting received packets. From running my test program for a few minutes I get the following: elapsed: 548.468416 seconds packets: 21503334 packets/sec: 39206.148199 bytes/sec: 20073547.877732 MBit/sec: 160.588383 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7 transactions happening quickly (with about 14us separating them), but every 8th transaction, the EG20T will NAK between 20-80 times[1], losing 50-100us[2]. This delay happens every 8th transaction without fail[3]. I've looked at the following: 1. The f_sourcesink.c function it queues up 8 responses at the beginning. Changing this number up or down had no effect. 2. Analysis of pch_udc.c doesn't show anything which would obviously cause a delay every 8th packet. 3. f_eem seems to have roughly the same performance with ping -f -s 64000 (160Mbit/sec). The CPU load of the gadget-side Atom PC sits very close to zero. System Details: Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) Intel Atom E680 with EG20T I seem to have eliminated everything on the host side, since the host is asking for data, and the device is saying it doesn't have any for up to 100us at a time. What am I missing? Alan. [0] http://www.signal11.us/~alan/recv.c [1] The number of NAKs not important and doesn't correlate to the amount of time spent NAK-ing. [2] Analyzer screenshots: http://www.signal11.us/~alan/nak.png http://www.signal11.us/~alan/nak_open.png [3] Often it looks like it's aligned to a SOF packet, but that's only because when you delay 100us, the odds are good you will be NAK-ing when a SOF packet arrives (every 125us). Sometimes the NAK-ing will start before the SOF and sometimes after. -- 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: EG20T USB Gadget Performance
Hi, On Mon, Jan 13, 2014 at 03:20:31PM -0500, Alan Ott wrote: > I have an EG20T-based board and have some issues with performance on > the USB device interface. I don't have that hardware but ... > I made a libusb test program (using the async interface)[0] to read > data from the EG20T's USB device port which has the gadget zero > source/sink function bound. In theory, one would hope this would give > the fastest real-world results for the hardware connected. > > The test program submits 32 IN transfers and re-submits on transfer > completion, counting received packets. > > From running my test program for a few minutes I get the following: > elapsed: 548.468416 seconds > packets: 21503334 > packets/sec: 39206.148199 > bytes/sec: 20073547.877732 > MBit/sec: 160.588383 > > 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer > shows 7 transactions happening quickly (with about 14us separating > them), but every 8th transaction, the EG20T will NAK between 20-80 > times[1], losing 50-100us[2]. as Alan stated, this is a problem on the device side. The device is replying with NAK because, I believe, it has ran out of free TDs. > This delay happens every 8th transaction without fail[3]. > > I've looked at the following: > 1. The f_sourcesink.c function it queues up 8 responses at the > beginning. Changing this number up or down had no effect. > 2. Analysis of pch_udc.c doesn't show anything which would obviously > cause a delay every 8th packet. > 3. f_eem seems to have roughly the same performance with ping -f -s > 64000 (160Mbit/sec). > > The CPU load of the gadget-side Atom PC sits very close to zero. > > System Details: > Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) > Intel Atom E680 with EG20T > > I seem to have eliminated everything on the host side, since the host > is asking for data, and the device is saying it doesn't have any for > up to 100us at a time. > > What am I missing? you should probably profile your pch_udc_pcd_queue() to figure out if there's anything wasting a lot of time there. Unlike Alan, I would use trace_printk() rather than pr_debug() since trace_printk() is of much lower overhead. Google around and you'll see how to use trace_printk() and how to use the kernel function profiler. cheers -- balbi signature.asc Description: Digital signature
Re: EG20T USB Gadget Performance
On 01/13/2014 05:11 PM, Felipe Balbi wrote: 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7 transactions happening quickly (with about 14us separating them), but every 8th transaction, the EG20T will NAK between 20-80 times[1], losing 50-100us[2]. as Alan stated, this is a problem on the device side. The device is replying with NAK because, I believe, it has ran out of free TDs. What am I missing? you should probably profile your pch_udc_pcd_queue() to figure out if there's anything wasting a lot of time there. Unlike Alan, I would use trace_printk() rather than pr_debug() since trace_printk() is of much lower overhead. Google around and you'll see how to use trace_printk() and how to use the kernel function profiler. Alan and Felipe, Thanks for the insight. I'll do some tracing and try to get it narrowed down. Alan. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Move DWC2 driver out of staging
On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: > The DWC2 driver should now be in good enough shape to move out of > staging. I have stress tested it overnight on RPI running mass > storage and Ethernet transfers in parallel, and for several days > on our proprietary PCI-based platform. > > Signed-off-by: Paul Zimmerman > Cc: Felipe Balbi This all looks good to me, thanks for sticking with it, now applied. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/2] ohci and ehci-platform clks, phy and dt support
Hi all, And here is v5 of my ohci and ehci-platform clks, phy and dt support patch-set. New since the last version is that the compatibility strings now are "usb-ohci" and "usb-ehci", which should make everyone happy I hope. Other then that there are no changes compared to v4. Regards, Hans -- 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 v5 2/2] ehci-platform: Add support for clks and phy passed through devicetree
Currently ehci-platform is only used in combination with devicetree when used with some Via socs. By extending it to (optionally) get clks and a phy from devicetree, and enabling / disabling those on power_on / off, it can be used more generically. Specifically after this commit it can be used for the ehci controller on Allwinner sunxi SoCs. Since ehci-platform is intended to handle any generic enough non pci ehci device, add a "usb-ehci" compatibility string. There already is a usb-ehci device-tree bindings document, update this with clks and phy bindings info. Although actually quite generic so far the via,vt8500 compatibilty string had its own bindings document. Somehow we even ended up with 2 of them. Since these provide no extra information over the generic usb-ehci documentation, this patch removes them. The ehci-ppc-of.c driver also claims the usb-ehci compatibility string, even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid 2 drivers claiming the same compatibility string getting build on ppc. Signed-off-by: Hans de Goede --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 25 +++- .../devicetree/bindings/usb/via,vt8500-ehci.txt| 15 --- .../devicetree/bindings/usb/vt8500-ehci.txt| 12 -- drivers/usb/host/Kconfig | 1 + drivers/usb/host/ehci-platform.c | 149 + 5 files changed, 142 insertions(+), 60 deletions(-) delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index fa18612..fad10f3 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -7,13 +7,14 @@ Required properties: (debug-port or other) can be also specified here, but only after definition of standard EHCI registers. - interrupts : one EHCI interrupt should be described here. -If device registers are implemented in big endian mode, the device -node should have "big-endian-regs" property. -If controller implementation operates with big endian descriptors, -"big-endian-desc" property should be specified. -If both big endian registers and descriptors are used by the controller -implementation, "big-endian" property can be specified instead of having -both "big-endian-regs" and "big-endian-desc". + +Optional 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 + - clocks : a list of phandle + clock specifier pairs + - phys : phandle + phy specifier pair + - phy-names : "usb" Example (Sequoia 440EPx): ehci@e300 { @@ -23,3 +24,13 @@ Example (Sequoia 440EPx): reg = <0 e300 90 0 e390 70>; big-endian; }; + +Example (Allwinner sun4i A10 SoC): + ehci0: ehci@0x01c14000 { + compatible = "allwinner,sun4i-a10-ehci", "usb-ehci"; + reg = <0x01c14000 0x100>; + interrupts = <39>; + clocks = <&ahb_gates 1>; + phys = <&usbphy 1>; + phy-names = "usb"; + }; diff --git a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt deleted file mode 100644 index 17b3ad1..000 --- a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt +++ /dev/null @@ -1,15 +0,0 @@ -VIA/Wondermedia VT8500 EHCI Controller -- - -Required properties: -- compatible : "via,vt8500-ehci" -- reg : Should contain 1 register ranges(address and length) -- interrupts : ehci controller interrupt - -Example: - - ehci@d8007900 { - compatible = "via,vt8500-ehci"; - reg = <0xd8007900 0x200>; - interrupts = <43>; - }; diff --git a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/vt8500-ehci.txt deleted file mode 100644 index 5fb8fd6..000 --- a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt +++ /dev/null @@ -1,12 +0,0 @@ -VIA VT8500 and Wondermedia WM8xxx SoC USB controllers. - -Required properties: - - compatible: Should be "via,vt8500-ehci" or "wm,prizm-ehci". - - reg: Address range of the ehci registers. size should be 0x200 - - interrupts: Should contain the ehci interrupt. - -usb: ehci@D8007100 { - compatible = "wm,prizm-ehci", "usb-ehci"; - reg = <0xD8007100 0x200>; - interrupts = <1>; -}; diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index a9707da..e28cbe0 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -255,6 +255,7 @@ config USB_EHCI_ATH79 c
[PATCH v5 1/2] ohci-platform: Add support for devicetree instantiation
Add support for ohci-platform instantiation from devicetree, including optionally getting clks and a phy from devicetree, and enabling / disabling those on power_on / off. This should allow using ohci-platform from devicetree in various cases. Specifically after this commit it can be used for the ohci controller found on Allwinner sunxi SoCs. Signed-off-by: Hans de Goede --- Documentation/devicetree/bindings/usb/usb-ohci.txt | 22 +++ drivers/usb/host/ohci-platform.c | 164 ++--- 2 files changed, 162 insertions(+), 24 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/usb-ohci.txt diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt new file mode 100644 index 000..f9d6c73 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt @@ -0,0 +1,22 @@ +USB OHCI controllers + +Required properties: +- compatible : "usb-ohci" +- reg : ohci controller register range (address and length) +- interrupts : ohci controller interrupt + +Optional properties: +- clocks : a list of phandle + clock specifier pairs +- phys : phandle + phy specifier pair +- phy-names : "usb" + +Example: + + ohci0: ohci@0x01c14400 { + compatible = "allwinner,sun4i-a10-ohci", "usb-ohci"; + reg = <0x01c14400 0x100>; + interrupts = <64>; + clocks = <&usb_clk 6>, <&ahb_gates 2>; + phys = <&usbphy 1>; + phy-names = "usb"; + }; diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index f351ff5..60e125d 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -3,6 +3,7 @@ * * Copyright 2007 Michael Buesch * Copyright 2011-2012 Hauke Mehrtens + * Copyright 2014 Hans de Goede * * Derived from the OCHI-SSB driver * Derived from the OHCI-PCI driver @@ -14,11 +15,14 @@ * Licensed under the GNU/GPL. See COPYING for details. */ +#include +#include #include #include #include #include #include +#include #include #include #include @@ -27,6 +31,13 @@ #include "ohci.h" #define DRIVER_DESC "OHCI generic platform driver" +#define OHCI_MAX_CLKS 3 +#define hcd_to_ohci_priv(h) ((struct ohci_platform_priv *)hcd_to_ohci(h)->priv) + +struct ohci_platform_priv { + struct clk *clks[OHCI_MAX_CLKS]; + struct phy *phy; +}; static const char hcd_name[] = "ohci-platform"; @@ -48,11 +59,67 @@ static int ohci_platform_reset(struct usb_hcd *hcd) return ohci_setup(hcd); } +static int ohci_platform_power_on(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); + int clk, ret; + + for (clk = 0; priv->clks[clk] && clk < OHCI_MAX_CLKS; clk++) { + ret = clk_prepare_enable(priv->clks[clk]); + if (ret) + goto err_disable_clks; + } + + if (priv->phy) { + ret = phy_init(priv->phy); + if (ret) + goto err_disable_clks; + + ret = phy_power_on(priv->phy); + if (ret) + goto err_exit_phy; + } + + return 0; + +err_exit_phy: + phy_exit(priv->phy); +err_disable_clks: + while (--clk >= 0) + clk_disable_unprepare(priv->clks[clk]); + + return ret; +} + +static void ohci_platform_power_off(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); + int clk; + + if (priv->phy) { + phy_power_off(priv->phy); + phy_exit(priv->phy); + } + + for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) + if (priv->clks[clk]) + clk_disable_unprepare(priv->clks[clk]); +} + static struct hc_driver __read_mostly ohci_platform_hc_driver; static const struct ohci_driver_overrides platform_overrides __initconst = { - .product_desc = "Generic Platform OHCI controller", - .reset =ohci_platform_reset, + .product_desc = "Generic Platform OHCI controller", + .reset =ohci_platform_reset, + .extra_priv_size = sizeof(struct ohci_platform_priv), +}; + +static struct usb_ohci_pdata ohci_platform_defaults = { + .power_on = ohci_platform_power_on, + .power_suspend =ohci_platform_power_off, + .power_off =ohci_platform_power_off, }; static int ohci_platform_probe(struct platform_device *dev) @@ -60,17 +127,23 @@ static int ohci_platform_probe(struct platform_device *dev) struct usb_hcd *hcd; struct resource *res_mem; struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev); - int irq; - int err = -ENOMEM; - - if (!pdata) { -
Re: [PATCH v3 03/10] usb: find internal hub tier mismatch via acpi
On Tue, Jan 07, 2014 at 12:29:44PM -0800, Dan Williams wrote: > ACPI identifies peer ports by setting their 'group_token' and 'group_position' > _PLD data to the same value. If a platform has tier mismatch (see Appendix D > figure 131 in the xhci spec), ACPI can override the default peer port > association (for internal hubs). > > Location data is cached as an opaque cookie in usb_port_location data. I haven't looked at this too closely, but what happens if: - the USB 2.0 roothub is registered - userspace immediately sets autosuspend_delay to zero and pm_qos_no_port_poweroff to zero - usb_acpi_find_device changes the connect_type to something other than unknown - before usb_acpi_check_port_peer() is called, the port is suspended In that case, we'll potentially power off a suspended USB 2.0 port before we know if it has a USB 3.0 peer port. It seems like you shouldn't change the connection type until you set the peer port. Unless you need that connection type for matching the peer port? Also, why only match on group token and group position? The xHCI spec in Appendix D says, "The _UPC declarations for LS/FS/HS and SS ports that are paired to form a USB 3.0 compatible connector. A 'pair' is defined by two ports that declare _PLDs with identical Panel, Vertical Position, Horizontal Position, Shape, Group Orientation and Group Token parameter values." (Note, I know nothing about what these fields are actually filled in with, I'm just quoting the spec here.) Sarah Sharp > > Signed-off-by: Dan Williams > --- > drivers/usb/core/hub.h |6 +++ > drivers/usb/core/port.c | 96 > +++ > drivers/usb/core/usb-acpi.c | 35 +--- > drivers/usb/core/usb.h |2 + > 4 files changed, 131 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h > index b5efc713498e..2ba10798c943 100644 > --- a/drivers/usb/core/hub.h > +++ b/drivers/usb/core/hub.h > @@ -76,6 +76,10 @@ struct usb_hub { > struct usb_port **ports; > }; > > +struct usb_port_location { > + u32 cookie; > +}; > + > /** > * struct usb port - kernel's representation of a usb port > * @child: usb device attatched to the port > @@ -83,6 +87,7 @@ struct usb_hub { > * @port_owner: port's owner > * @peer: related usb2 and usb3 ports (share the same connector) > * @connect_type: port's connect type > + * @location: opaque representation of platform connector location > * @portnum: port index num based one > * @power_is_on: port's power state > * @did_runtime_put: port has done pm_runtime_put(). > @@ -93,6 +98,7 @@ struct usb_port { > struct dev_state *port_owner; > struct usb_port *peer; > enum usb_port_connect_type connect_type; > + struct usb_port_location location; > u8 portnum; > unsigned power_is_on:1; > unsigned did_runtime_put:1; > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > index 7fd22020d7ee..8fae3cd03305 100644 > --- a/drivers/usb/core/port.c > +++ b/drivers/usb/core/port.c > @@ -149,11 +149,14 @@ struct device_type usb_port_device_type = { > > static struct usb_port *find_peer_port(struct usb_hub *hub, int port1) > { > - struct usb_device *hdev = hub->hdev; > + struct usb_device *hdev = hub ? hub->hdev : NULL; > struct usb_device *peer_hdev; > struct usb_port *peer = NULL; > struct usb_hub *peer_hub; > > + if (!hub) > + return NULL; > + > /* set the default peer port for root hubs. Platform firmware >* can later fix this up if tier-mismatch is present. Assumes >* the primary_hcd is usb2.0 and registered first > @@ -193,6 +196,97 @@ static struct usb_port *find_peer_port(struct usb_hub > *hub, int port1) > return peer; > } > > +static void reset_peer(struct usb_port *port_dev, struct usb_port *peer) > +{ > + if (!peer) > + return; > + > + spin_lock(&peer_lock); > + if (port_dev->peer) > + put_device(&port_dev->peer->dev); > + if (peer->peer) > + put_device(&peer->peer->dev); > + port_dev->peer = peer; > + peer->peer = port_dev; > + get_device(&peer->dev); > + get_device(&port_dev->dev); > + spin_unlock(&peer_lock); > +} > + > +/* assumes that location data is only set for external connectors and that > + * external hubs never have tier mismatch > + */ > +static int redo_find_peer_port(struct device *dev, void *data) > +{ > + struct usb_port *port_dev = to_usb_port(dev); > + > + if (is_usb_port(dev)) { > + struct usb_device *hdev = to_usb_device(dev->parent->parent); > + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); > + int port1 = port_dev->portnum; > + struct usb_port *peer; > + > + peer = find_peer_port(hub, port1); > + reset_peer(port_dev, peer); > + } > + > + return device_for_each_child
Re: Bug#704242: Driver for PL-2303 HX not working
Hello together, i will close this bug at Debian now. After the last update this error seems to disappear in Debian stable. http://ftp.debian.org/debian/dists/wheezy/ChangeLog USB: pl2303: fix device initialisation at open The source for this patch can be found here: http://www.spinics.net/lists/stable/msg30311.html It's possible that this will not fix the bug under all circumstances depending on the application. Summary: == I did need some time to realize that this bug will occur only with PL2303HX chips that are China clones. In fact this type of chips run out of production at Profilic. http://www.prolific.com.tw/US/ShowProduct.aspx?pcid=41&showlevel=0017-0037-0041 Only the PL2303HXD can be buyed as original, so any new PL2303HX is a China clone. I have been contacted by Frank Schäfer and so we tested some of his driver improvements in combination with newer kernels. Up to now this could not be backported to kernel 3.2.x, but i could test a kernel 3.12.6 on Debian wheezy and the PL2303HX is running fine. There are really much products with this clone chips out there, so some further improvements of the driver would be really helpful. I will make further tests together with Frank and hope that this improvements will find a place into the next kernel versions starting with Debian jessie and 3.12.6 Best regards Karsten -- 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 Mon, Jan 13, 2014 at 02:56:57PM -0800, Sarah Sharp wrote: > On Tue, Jan 07, 2014 at 12:29:44PM -0800, Dan Williams wrote: > > ACPI identifies peer ports by setting their 'group_token' and > > 'group_position' > > _PLD data to the same value. If a platform has tier mismatch (see Appendix > > D > > figure 131 in the xhci spec), ACPI can override the default peer port > > association (for internal hubs). > > > > Location data is cached as an opaque cookie in usb_port_location data. > > I haven't looked at this too closely, but what happens if: > - the USB 2.0 roothub is registered > - userspace immediately sets autosuspend_delay to zero and >pm_qos_no_port_poweroff to zero > - usb_acpi_find_device changes the connect_type to something other than >unknown > - before usb_acpi_check_port_peer() is called, the port is suspended Or that call finishes, but no peer port is set yet, because the USB 3.0 roothub isn't registered yet. The USB 2.0 bus can be suspended before the USB 3.0 bus has finished registering, as I've seen from this thread: http://marc.info/?l=linux-usb&m=138914518219334&w=2 > In that case, we'll potentially power off a suspended USB 2.0 port > before we know if it has a USB 3.0 peer port. It seems like you > shouldn't change the connection type until you set the peer port. > Unless you need that connection type for matching the peer port? > > Also, why only match on group token and group position? The xHCI spec > in Appendix D says, "The _UPC declarations for LS/FS/HS and SS ports > that are paired to form a USB 3.0 compatible connector. A 'pair' is > defined by two ports that declare _PLDs with identical Panel, Vertical > Position, Horizontal Position, Shape, Group Orientation and Group Token > parameter values." (Note, I know nothing about what these fields are > actually filled in with, I'm just quoting the spec here.) > > Sarah Sharp > > > > > Signed-off-by: Dan Williams > > --- > > drivers/usb/core/hub.h |6 +++ > > drivers/usb/core/port.c | 96 > > +++ > > drivers/usb/core/usb-acpi.c | 35 +--- > > drivers/usb/core/usb.h |2 + > > 4 files changed, 131 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h > > index b5efc713498e..2ba10798c943 100644 > > --- a/drivers/usb/core/hub.h > > +++ b/drivers/usb/core/hub.h > > @@ -76,6 +76,10 @@ struct usb_hub { > > struct usb_port **ports; > > }; > > > > +struct usb_port_location { > > + u32 cookie; > > +}; > > + > > /** > > * struct usb port - kernel's representation of a usb port > > * @child: usb device attatched to the port > > @@ -83,6 +87,7 @@ struct usb_hub { > > * @port_owner: port's owner > > * @peer: related usb2 and usb3 ports (share the same connector) > > * @connect_type: port's connect type > > + * @location: opaque representation of platform connector location > > * @portnum: port index num based one > > * @power_is_on: port's power state > > * @did_runtime_put: port has done pm_runtime_put(). > > @@ -93,6 +98,7 @@ struct usb_port { > > struct dev_state *port_owner; > > struct usb_port *peer; > > enum usb_port_connect_type connect_type; > > + struct usb_port_location location; > > u8 portnum; > > unsigned power_is_on:1; > > unsigned did_runtime_put:1; > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > > index 7fd22020d7ee..8fae3cd03305 100644 > > --- a/drivers/usb/core/port.c > > +++ b/drivers/usb/core/port.c > > @@ -149,11 +149,14 @@ struct device_type usb_port_device_type = { > > > > static struct usb_port *find_peer_port(struct usb_hub *hub, int port1) > > { > > - struct usb_device *hdev = hub->hdev; > > + struct usb_device *hdev = hub ? hub->hdev : NULL; > > struct usb_device *peer_hdev; > > struct usb_port *peer = NULL; > > struct usb_hub *peer_hub; > > > > + if (!hub) > > + return NULL; > > + > > /* set the default peer port for root hubs. Platform firmware > > * can later fix this up if tier-mismatch is present. Assumes > > * the primary_hcd is usb2.0 and registered first > > @@ -193,6 +196,97 @@ static struct usb_port *find_peer_port(struct usb_hub > > *hub, int port1) > > return peer; > > } > > > > +static void reset_peer(struct usb_port *port_dev, struct usb_port *peer) > > +{ > > + if (!peer) > > + return; > > + > > + spin_lock(&peer_lock); > > + if (port_dev->peer) > > + put_device(&port_dev->peer->dev); > > + if (peer->peer) > > + put_device(&peer->peer->dev); > > + port_dev->peer = peer; > > + peer->peer = port_dev; > > + get_device(&peer->dev); > > + get_device(&port_dev->dev); > > + spin_unlock(&peer_lock); > > +} > > + > > +/* assumes that location data is only set for external connectors and that > > + * external hubs never have tier mismatch > > + */ > > +static int redo_find_peer_p
[PATCH] usb: dwc2: move device tree bindings doc to correct place
Now that the DWC2 driver has been moved to drivers/usb, move its bindings doc to the correct place Cc: Rob Herring Signed-off-by: Paul Zimmerman --- Greg, I missed this file when moving the rest of the dwc2 files. Documentation/devicetree/bindings/{staging => usb}/dwc2.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Documentation/devicetree/bindings/{staging => usb}/dwc2.txt (100%) diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt similarity index 100% rename from Documentation/devicetree/bindings/staging/dwc2.txt rename to Documentation/devicetree/bindings/usb/dwc2.txt -- 1.8.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: fix resume issues on Renesas chips
On Mon, Jan 13, 2014 at 10:49:49AM -0500, Don Zickus wrote: > 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. > > Not sure if I should restrict this further or not. Probably should be sent > to stable too. Can you provide the `sudo lspci -vvv -n` for the xHCI host? I would rather restrict this quirk further, since this is the first report I've seen of an NEC host needing this quirk. I would rather limit it to a Samsung subsystem vendor ID if that's available. Sarah Sharp > Signed-off-by: Don Zickus > --- > drivers/usb/host/xhci-pci.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index 73f5208..2a21892 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -144,6 +144,8 @@ static void xhci_pci_quirks(struct device *dev, struct > xhci_hcd *xhci) > } > if (pdev->vendor == PCI_VENDOR_ID_VIA) > xhci->quirks |= XHCI_RESET_ON_RESUME; > + if (pdev->vendor == PCI_VENDOR_ID_RENESAS) > + xhci->quirks |= XHCI_RESET_ON_RESUME; > } > > /* called during probe() after chip reset completes */ > -- > 1.7.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]
On 01/09/2014 03:50 PM, Sarah Sharp wrote: >>> On Tue, Jan 07, 2014 at 03:57:00PM -0800, walt wrote: >> >> I've wondered if my xhci problems might be caused by hardware quirks, and >> wondering why I seem to be the only one who has this problem. >> >> Maybe I could "take one for the team" by buying new hardware toys that I >> don't really need but I could use to test the xhci driver? (I do enjoy >> buying new toys, necessary, or, um, maybe not :) > > It would be appreciated if you could see if your device causes other > host controllers to fail. Who am I to keep a geek from new toys? ;) Sarah, I just fixed my xhci bug for US$19.99 :) #lspci | tail -1 04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 03) This new NEC usb3 controller does everything the ASMedia controller should have done from the start. I can even power-up the outboard usb3 disk docking station after the computer is running and still everything Just Works :) I appreciate all the debugging work you've done to fix the ASMedia problem but I think it's time to quit now. If hundreds of irate linux users complain about the same bug then I'll be happy to test more patches. Until then... :) -- 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 Mon, Jan 13, 2014 at 2:56 PM, Sarah Sharp wrote: > On Tue, Jan 07, 2014 at 12:29:44PM -0800, Dan Williams wrote: >> ACPI identifies peer ports by setting their 'group_token' and >> 'group_position' >> _PLD data to the same value. If a platform has tier mismatch (see Appendix D >> figure 131 in the xhci spec), ACPI can override the default peer port >> association (for internal hubs). >> >> Location data is cached as an opaque cookie in usb_port_location data. > > I haven't looked at this too closely, but what happens if: > - the USB 2.0 roothub is registered > - userspace immediately sets autosuspend_delay to zero and >pm_qos_no_port_poweroff to zero > - usb_acpi_find_device changes the connect_type to something other than >unknown Note, in this revision I've kept the established policy of not checking connect_type. We'll immediately suspend on setting pm_qos_no_poweroff to zero and device going idle. > - before usb_acpi_check_port_peer() is called, the port is suspended > > In that case, we'll potentially power off a suspended USB 2.0 port > before we know if it has a USB 3.0 peer port. It seems like you > shouldn't change the connection type until you set the peer port. > Unless you need that connection type for matching the peer port? The sequencing enforced by usb_port_runtime_suspend() is directional in that a usb2 port powering off before usb3 is always permitted (otherwise we run into cyclical dependencies). The implementation expects that the device will enforce the preference to connect on its usb3 interface, and it should as long as VBUS and the RX terminations are maintained, which they would be in this scenario. In the reverse scenario (detecting usb3 before usb2 peer) we have a check for "!peer" to block suspending the usb3 port prematurely. However, I wonder if it would be advisable to detect the case where a usb3 device has downgraded to it's usb2 connection. In that case I think we want to disable port power off because the assumption of "always enforce usb3 powered down last" has been invalidated. I can look into an incremental patch to add that safeguard for flaky devices, but it has not been a problem in testing. > Also, why only match on group token and group position? The xHCI spec > in Appendix D says, "The _UPC declarations for LS/FS/HS and SS ports > that are paired to form a USB 3.0 compatible connector. A 'pair' is > defined by two ports that declare _PLDs with identical Panel, Vertical > Position, Horizontal Position, Shape, Group Orientation and Group Token > parameter values." (Note, I know nothing about what these fields are > actually filled in with, I'm just quoting the spec here.) True, I was going off the ACPI spec (6.1.8) that says that "Group Token" is "Unique numerical value identifying a group". So my understanding is that the other fields are just positional, but group token and group position are sufficient for uniquely identifying connectors. -- 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-next] usb: core: let dynamic ids override static ids
On Sat, Jan 11, 2014 at 02:04:00AM +0100, Bjørn Mork wrote: > This modifies the probing order so that any matching > dynamic entry always will be used, even if the driver > has a matching static entry. > > It is sometimes useful to dynamically update existing > device entries. With the new ability to set the dynamic > entry driver_info field, this can be used to test new > additions to class driver exception lists or proposed > changes to existing static per-device driver_info > entries. > > Signed-off-by: Bjørn Mork > --- > drivers/usb/core/driver.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) As this could change behavior for some people, I'll wait for 3.14-rc1 to come out before adding it to my tree for 3.15, so it gets lots of testing in linux-next. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
USB, TTY, char/misc, and Staging trees now closed for 3.14
Hi all, Given that 3.13 will be out in a few days, it's time to close my trees for new patches until 3.14-rc1 is out. Please feel free to send me patches for these trees, but note that I'll be ignoring them until 3.14-rc1 is out, at which point in time I'll start reviewing them again and applying them to the various places they belong. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device
Dear friends, So far, there has been no response to this issue report. I understand this is not a help desk with guaranteed replies but the fact that there was no response whatsoever is surprising to me. I would like to at least kindly ask that someone looks at the traces and tells me if this is a possible software issue, or a hardware quirk that can hardly be worked around. A "no, we won't fix this" or even "this is not the proper mailing list for this" type of answer would be welcome at this point. It is kind of frustrating to report an USB-related issue to linux-usb mailing list, keep it updated, and yet receive absolutely no reaction in almost a month. If I knew how to fix this, I would - but I am no expert in USB stuff myself, so I am turning to experts... Thanks for understanding! Best regards, Peter On 08.01.2014 16:48, Peter Palúch wrote: Greetings, Regarding the issue with WD MyBook 1230 stalling and being reset when connected to an HP EliteBook 8560p, I have tested the 3.13-rc7 kernel from kernel.org to no avail - the drive still stalls and is reset after a couple of seconds. I am attaching a dmesg output (relevant lines regarding the drive and the xHCI reset are at the end) and the usbmon trace. The lspci and lsusb outputs have been attached to the original mail I've opened this thread with (if necessary, I will gladly repost them). The usbmon trace covers the complete session with the drive, starting with its connection to my USB 3.0 slot, then accessing the drive with "gdisk -l /dev/sdb", across its stall and reset, up to its physical disconnection. The actual transcript of the communication with the drive that ensued after the "gdisk -l /dev/sdb" command that led to the stall starts at the decimal offset 82582 in the usbmon trace file. Thank you! Best regards, Peter -- 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 Mon, 13 Jan 2014, Sarah Sharp wrote: > On Mon, Jan 13, 2014 at 02:56:57PM -0800, Sarah Sharp wrote: > > I haven't looked at this too closely, but what happens if: > > - the USB 2.0 roothub is registered > > - userspace immediately sets autosuspend_delay to zero and > >pm_qos_no_port_poweroff to zero > > - usb_acpi_find_device changes the connect_type to something other than > >unknown > > - before usb_acpi_check_port_peer() is called, the port is suspended > > Or that call finishes, but no peer port is set yet, because the USB 3.0 > roothub isn't registered yet. The USB 2.0 bus can be suspended before > the USB 3.0 bus has finished registering, as I've seen from this thread: > > http://marc.info/?l=linux-usb&m=138914518219334&w=2 It's always possible for xhci-hcd to prevent the USB-2 root hub from being suspended by calling pm_runtime_get_noresume. The corresponding _put can be done after the USB-3 root hub has been registered. 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: EG20T USB Gadget Performance
On Mon, 13 Jan 2014, Felipe Balbi wrote: > Hi, > > On Mon, Jan 13, 2014 at 03:20:31PM -0500, Alan Ott wrote: > > I have an EG20T-based board and have some issues with performance on > > the USB device interface. > > I don't have that hardware but ... > > > I made a libusb test program (using the async interface)[0] to read > > data from the EG20T's USB device port which has the gadget zero > > source/sink function bound. In theory, one would hope this would give > > the fastest real-world results for the hardware connected. > > > > The test program submits 32 IN transfers and re-submits on transfer > > completion, counting received packets. > > > > From running my test program for a few minutes I get the following: > > elapsed: 548.468416 seconds > > packets: 21503334 > > packets/sec: 39206.148199 > > bytes/sec: 20073547.877732 > > MBit/sec: 160.588383 > > > > 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer > > shows 7 transactions happening quickly (with about 14us separating > > them), but every 8th transaction, the EG20T will NAK between 20-80 > > times[1], losing 50-100us[2]. > > as Alan stated, this is a problem on the device side. The device is > replying with NAK because, I believe, it has ran out of free TDs. > > > This delay happens every 8th transaction without fail[3]. > > > > I've looked at the following: > > 1. The f_sourcesink.c function it queues up 8 responses at the > > beginning. Changing this number up or down had no effect. > > 2. Analysis of pch_udc.c doesn't show anything which would obviously > > cause a delay every 8th packet. > > 3. f_eem seems to have roughly the same performance with ping -f -s > > 64000 (160Mbit/sec). > > > > The CPU load of the gadget-side Atom PC sits very close to zero. > > > > System Details: > > Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) > > Intel Atom E680 with EG20T > > > > I seem to have eliminated everything on the host side, since the host > > is asking for data, and the device is saying it doesn't have any for > > up to 100us at a time. > > > > What am I missing? > > you should probably profile your pch_udc_pcd_queue() to figure out if > there's anything wasting a lot of time there. > > Unlike Alan, I would use trace_printk() rather than pr_debug() since > trace_printk() is of much lower overhead. Google around and you'll see > how to use trace_printk() and how to use the kernel function profiler. By the way, isn't it true that f_sourcesink uses only one request for each bulk endpoint? That would naturally lead to a delay each time the request completes and has to be resubmitted. If the driver used two requests instead, the pipeline would be much less likely to empty out. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs
Понедельник, 13 января 2014, 22:31 +01:00 от Uwe Kleine-König : > On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote: > > Суббота, 11 января 2014, 13:55 +01:00 от Uwe Kleine-König > > : > > > On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote: > > > > Hello. > > > > > > > > > On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote: > > > > > > This adds i.MX27 and i.MX31 as the next user of the usbmisc > > > > > > driver. > > > > > > > > > > > > Signed-off-by: Alexander Shiyan > > > > > > --- > > > > > > drivers/usb/chipidea/usbmisc_imx.c | 42 > > > > > > ++ > > > > > > 1 file changed, 42 insertions(+) > > > > ... > > > > > At this point might be good to patch the imx27.dtsi with the usb > > > > > defines. > > > > ... > > > > > > > > I have a working configuration for i.MX27 USB, but I prefer to make a > > > > few more > > > > tests before the addition of definitions in DTS. This will be a next > > > > step. > > > > Thanks. > > > Any news here? > > > > Not ready yet. > Are you still working at it? Would you mind sharing more details, like > your current tree/patch stack and what works/doesn't work for you? Now my work on this is suspended, but will continue later. I'll send you a personal letter with DT configuration. Ports (both Host & OTG) are detected by kernel, but works Host only. OTG not works nor as Host, nor as Device... ---
Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs
On Tuesday, January 14, 2014 11:30 AM, Alexander Shiyan wrote: Понедельник, 13 января 2014, 22:31 +01:00 от Uwe Kleine-König : On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote: Суббота, 11 января 2014, 13:55 +01:00 от Uwe Kleine-König : On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote: Hello. On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote: This adds i.MX27 and i.MX31 as the next user of the usbmisc driver. Signed-off-by: Alexander Shiyan --- drivers/usb/chipidea/usbmisc_imx.c | 42 ++ 1 file changed, 42 insertions(+) ... At this point might be good to patch the imx27.dtsi with the usb defines. ... I have a working configuration for i.MX27 USB, but I prefer to make a few more tests before the addition of definitions in DTS. This will be a next step. Thanks. Any news here? Not ready yet. Are you still working at it? Would you mind sharing more details, like your current tree/patch stack and what works/doesn't work for you? Now my work on this is suspended, but will continue later. I'll send you a personal letter with DT configuration. Ports (both Host & OTG) are detected by kernel, but works Host only. OTG not works nor as Host, nor as Device... --- hi, I'm still working on my patches for imx27 and ULPI integration. But have more urgent things in the queue. I have a running version but it cant make it in the kernel. Rework on platform device code needed. Just wait until the patches already accepted appears in the linux-next before commit new patches. Chris -- 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 v5 2/2] ehci-platform: Add support for clks and phy passed through devicetree
On 14/01/14 11:45, Hans de Goede wrote: Currently ehci-platform is only used in combination with devicetree when used with some Via socs. By extending it to (optionally) get clks and a phy from devicetree, and enabling / disabling those on power_on / off, it can be used more generically. Specifically after this commit it can be used for the ehci controller on Allwinner sunxi SoCs. Since ehci-platform is intended to handle any generic enough non pci ehci device, add a "usb-ehci" compatibility string. There already is a usb-ehci device-tree bindings document, update this with clks and phy bindings info. Although actually quite generic so far the via,vt8500 compatibilty string had its own bindings document. Somehow we even ended up with 2 of them. Since these provide no extra information over the generic usb-ehci documentation, this patch removes them. The ehci-ppc-of.c driver also claims the usb-ehci compatibility string, even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid 2 drivers claiming the same compatibility string getting build on ppc. Signed-off-by: Hans de Goede --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 25 +++- .../devicetree/bindings/usb/via,vt8500-ehci.txt| 15 --- .../devicetree/bindings/usb/vt8500-ehci.txt| 12 -- drivers/usb/host/Kconfig | 1 + drivers/usb/host/ehci-platform.c | 149 + 5 files changed, 142 insertions(+), 60 deletions(-) delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index fa18612..fad10f3 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -7,13 +7,14 @@ Required properties: (debug-port or other) can be also specified here, but only after definition of standard EHCI registers. - interrupts : one EHCI interrupt should be described here. -If device registers are implemented in big endian mode, the device -node should have "big-endian-regs" property. -If controller implementation operates with big endian descriptors, -"big-endian-desc" property should be specified. -If both big endian registers and descriptors are used by the controller -implementation, "big-endian" property can be specified instead of having -both "big-endian-regs" and "big-endian-desc". + +Optional 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 + - clocks : a list of phandle + clock specifier pairs + - phys : phandle + phy specifier pair + - phy-names : "usb" Example (Sequoia 440EPx): ehci@e300 { @@ -23,3 +24,13 @@ Example (Sequoia 440EPx): reg = <0 e300 90 0 e390 70>; big-endian; }; + +Example (Allwinner sun4i A10 SoC): + ehci0: ehci@0x01c14000 { + compatible = "allwinner,sun4i-a10-ehci", "usb-ehci"; + reg = <0x01c14000 0x100>; + interrupts = <39>; + clocks = <&ahb_gates 1>; + phys = <&usbphy 1>; + phy-names = "usb"; + }; diff --git a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt deleted file mode 100644 index 17b3ad1..000 --- a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt +++ /dev/null @@ -1,15 +0,0 @@ -VIA/Wondermedia VT8500 EHCI Controller -- - -Required properties: -- compatible : "via,vt8500-ehci" -- reg : Should contain 1 register ranges(address and length) -- interrupts : ehci controller interrupt - -Example: - - ehci@d8007900 { - compatible = "via,vt8500-ehci"; - reg = <0xd8007900 0x200>; - interrupts = <43>; - }; diff --git a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/vt8500-ehci.txt deleted file mode 100644 index 5fb8fd6..000 --- a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt +++ /dev/null @@ -1,12 +0,0 @@ -VIA VT8500 and Wondermedia WM8xxx SoC USB controllers. - -Required properties: - - compatible: Should be "via,vt8500-ehci" or "wm,prizm-ehci". - - reg: Address range of the ehci registers. size should be 0x200 - - interrupts: Should contain the ehci interrupt. - -usb: ehci@D8007100 { - compatible = "wm,prizm-ehci", "usb-ehci"; - reg = <0xD8007100 0x200>; - interrupts = <1>; -}; diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index a9707da..e28cbe0 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/h
Re: EG20T USB Gadget Performance
On 01/13/2014 09:01 PM, Alan Stern wrote: On Mon, 13 Jan 2014, Felipe Balbi wrote: Hi, On Mon, Jan 13, 2014 at 03:20:31PM -0500, Alan Ott wrote: I have an EG20T-based board and have some issues with performance on the USB device interface. I don't have that hardware but ... I made a libusb test program (using the async interface)[0] to read data from the EG20T's USB device port which has the gadget zero source/sink function bound. In theory, one would hope this would give the fastest real-world results for the hardware connected. The test program submits 32 IN transfers and re-submits on transfer completion, counting received packets. From running my test program for a few minutes I get the following: elapsed: 548.468416 seconds packets: 21503334 packets/sec: 39206.148199 bytes/sec: 20073547.877732 MBit/sec: 160.588383 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7 transactions happening quickly (with about 14us separating them), but every 8th transaction, the EG20T will NAK between 20-80 times[1], losing 50-100us[2]. as Alan stated, this is a problem on the device side. The device is replying with NAK because, I believe, it has ran out of free TDs. This delay happens every 8th transaction without fail[3]. I've looked at the following: 1. The f_sourcesink.c function it queues up 8 responses at the beginning. Changing this number up or down had no effect. 2. Analysis of pch_udc.c doesn't show anything which would obviously cause a delay every 8th packet. 3. f_eem seems to have roughly the same performance with ping -f -s 64000 (160Mbit/sec). The CPU load of the gadget-side Atom PC sits very close to zero. System Details: Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) Intel Atom E680 with EG20T I seem to have eliminated everything on the host side, since the host is asking for data, and the device is saying it doesn't have any for up to 100us at a time. What am I missing? you should probably profile your pch_udc_pcd_queue() to figure out if there's anything wasting a lot of time there. Unlike Alan, I would use trace_printk() rather than pr_debug() since trace_printk() is of much lower overhead. Google around and you'll see how to use trace_printk() and how to use the kernel function profiler. By the way, isn't it true that f_sourcesink uses only one request for each bulk endpoint? That would naturally lead to a delay each time the request completes and has to be resubmitted. That's what the comment at the top of the file says, but it doesn't appear to be true. See source_sink_start_ep(). It seems to start by queueing up 8 transactions. I've adjusted this number up with no effect (currently at 64). Maybe there's something else I'm missing. If the driver used two requests instead, the pipeline would be much less likely to empty out. Yes, I absolutely agree, the queue must be kept full, but in this case I think it is. Alan. -- 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: EG20T USB Gadget Performance
Hi I guess it is something to do with the buffer size of the gadget driver. Could you please try change the buffer size to 16K and confirm if the delay is shifting ? In this case your delay should be after 31 transfers... === 66 static struct usb_zero_options gzero_options = { 67 .isoc_interval = 4, 68 .isoc_maxpacket = 1024, 69 .bulk_buflen = 4096, => change to 16K 70 .qlen = 32, 71 }; === Rajaram On Tue, Jan 14, 2014 at 1:50 AM, Alan Ott wrote: > Hi all, > > I have an EG20T-based board and have some issues with performance on the USB > device interface. > > I made a libusb test program (using the async interface)[0] to read data > from the EG20T's USB device port which has the gadget zero source/sink > function bound. In theory, one would hope this would give the fastest > real-world results for the hardware connected. > > The test program submits 32 IN transfers and re-submits on transfer > completion, counting received packets. > > From running my test program for a few minutes I get the following: > elapsed: 548.468416 seconds > packets: 21503334 > packets/sec: 39206.148199 > bytes/sec: 20073547.877732 > MBit/sec: 160.588383 > > 160MBit/sec isn't terrible, but I hoped for better. A USB analyzer shows 7 > transactions happening quickly (with about 14us separating them), but every > 8th transaction, the EG20T will NAK between 20-80 times[1], losing > 50-100us[2]. > > This delay happens every 8th transaction without fail[3]. > > I've looked at the following: > 1. The f_sourcesink.c function it queues up 8 responses at the beginning. > Changing this number up or down had no effect. > 2. Analysis of pch_udc.c doesn't show anything which would obviously cause a > delay every 8th packet. > 3. f_eem seems to have roughly the same performance with ping -f -s 64000 > (160Mbit/sec). > > The CPU load of the gadget-side Atom PC sits very close to zero. > > System Details: > Linux 3.13.0-rc7 (With a defconfig from Yocto for Intel Crownbay) > Intel Atom E680 with EG20T > > I seem to have eliminated everything on the host side, since the host is > asking for data, and the device is saying it doesn't have any for up to > 100us at a time. > > What am I missing? > > Alan. > > [0] http://www.signal11.us/~alan/recv.c > [1] The number of NAKs not important and doesn't correlate to the amount of > time spent NAK-ing. > [2] Analyzer screenshots: > http://www.signal11.us/~alan/nak.png > http://www.signal11.us/~alan/nak_open.png > [3] Often it looks like it's aligned to a SOF packet, but that's only > because when you delay 100us, the odds are good you will be NAK-ing when a > SOF packet arrives (every 125us). Sometimes the NAK-ing will start before > the SOF and sometimes after. > > -- > 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 -- 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: EG20T USB Gadget Performance
On 01/14/2014 12:08 AM, Rajaram R wrote: I guess it is something to do with the buffer size of the gadget driver. Could you please try change the buffer size to 16K and confirm if the delay is shifting ? In this case your delay should be after 31 transfers... === 66 static struct usb_zero_options gzero_options = { 67 .isoc_interval = 4, 68 .isoc_maxpacket = 1024, 69 .bulk_buflen = 4096, => change to 16K 70 .qlen = 32, 71 }; === Hi Rajram, Yes, this does as you suspected. I now get 222Mbit/sec of data transfer. The amount of time lost during each period of NAKs also increased, with between 100 and 131us lost every 32nd transaction. So the part I misunderstood is that buflen (which can be set as a parameter to g_zero) is the number of bytes in each transfer. I wrongly assumed f_sourcesink was sending 512-byte single-transaction transfers (because the data would look the same). It looks like the extended delay time is happening once every transfer, and in looking at the driver it looks like it's because only one transfer can be active (queued) in the hardware at once. It all adds up now. Thank you all for your help. Alan. -- 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: EG20T USB Gadget Performance
On Tue, Jan 14, 2014 at 02:06:13PM +0800, Alan Ott wrote: > On 01/14/2014 12:08 AM, Rajaram R wrote: > > I guess it is something to do with the buffer size of the gadget > > driver. Could you please try change the buffer size to 16K and confirm > > if the delay is shifting ? In this case your delay should be after 31 > > transfers... > > > > === > > 66 static struct usb_zero_options gzero_options = { > > 67 .isoc_interval = 4, > > 68 .isoc_maxpacket = 1024, > > 69 .bulk_buflen = 4096, => change to 16K > > 70 .qlen = 32, > > 71 }; > > === > > Hi Rajram, > > Yes, this does as you suspected. I now get 222Mbit/sec of data transfer. See here, some way to get higher speed with zero gadget. http://www.spinics.net/lists/linux-usb/msg100588.html Regards Pratyush > -- > The amount of time lost during each period of NAKs also increased, with > between 100 and 131us lost every 32nd transaction. > > So the part I misunderstood is that buflen (which can be set as a > parameter to g_zero) is the number of bytes in each transfer. I wrongly > assumed f_sourcesink was sending 512-byte single-transaction transfers > (because the data would look the same). > > It looks like the extended delay time is happening once every transfer, > and in looking at the driver it looks like it's because only one > transfer can be active (queued) in the hardware at once. It all adds up now. > > Thank you all for your help. > > Alan. > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: gadget: s3c-hsotg: stall ep0 in set_halt function
When s3c_hsotg_ep_sethalt() function is called for ep0 it should be stalled in the same way that it is in s3c_hsotg_process_control() function, because SET_HALT for ep0 is delayed response for setup request. Endpoint 0, if halted, it doesn't need CLEAR_HALT because it clears "stalled" state automatically when next setup request is received. For this reason this patch moves code setting ep0 to "stalled" state to new function named s3c_hsotg_stall_ep0() which is called in s3c_hsotg_process_control() function as an immediate response for setup request, and in s3c_hsotg_ep_sethalt() function as a delayed response for setup request. Signed-off-by: Robert Baldyga --- drivers/usb/gadget/s3c-hsotg.c | 78 +++- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index e20bc10..a4c8c6a 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -1183,6 +1183,41 @@ static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg); static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg); /** + * s3c_hsotg_stall_ep0 - stall ep0 + * @hsotg: The device state + * + * Set stall for ep0 as response for setup request. + */ +static void s3c_hsotg_stall_ep0(struct s3c_hsotg *hsotg) { + struct s3c_hsotg_ep *ep0 = &hsotg->eps[0]; + u32 reg; + u32 ctrl; + + dev_dbg(hsotg->dev, "ep0 stall (dir=%d)\n", ep0->dir_in); + reg = (ep0->dir_in) ? DIEPCTL0 : DOEPCTL0; + + /* +* DxEPCTL_Stall will be cleared by EP once it has +* taken effect, so no need to clear later. +*/ + + ctrl = readl(hsotg->regs + reg); + ctrl |= DxEPCTL_Stall; + ctrl |= DxEPCTL_CNAK; + writel(ctrl, hsotg->regs + reg); + + dev_dbg(hsotg->dev, + "written DxEPCTL=0x%08x to %08x (DxEPCTL=0x%08x)\n", + ctrl, reg, readl(hsotg->regs + reg)); + +/* + * complete won't be called, so we enqueue + * setup request here + */ +s3c_hsotg_enqueue_setup(hsotg); +} + +/** * s3c_hsotg_process_control - process a control request * @hsotg: The device state * @ctrl: The control request received @@ -1259,38 +1294,8 @@ static void s3c_hsotg_process_control(struct s3c_hsotg *hsotg, * so respond with a STALL for the status stage to indicate failure. */ - if (ret < 0) { - u32 reg; - u32 ctrl; - - dev_dbg(hsotg->dev, "ep0 stall (dir=%d)\n", ep0->dir_in); - reg = (ep0->dir_in) ? DIEPCTL0 : DOEPCTL0; - - /* -* DxEPCTL_Stall will be cleared by EP once it has -* taken effect, so no need to clear later. -*/ - - ctrl = readl(hsotg->regs + reg); - ctrl |= DxEPCTL_Stall; - ctrl |= DxEPCTL_CNAK; - writel(ctrl, hsotg->regs + reg); - - dev_dbg(hsotg->dev, - "written DxEPCTL=0x%08x to %08x (DxEPCTL=0x%08x)\n", - ctrl, reg, readl(hsotg->regs + reg)); - - /* -* don't believe we need to anything more to get the EP -* to reply with a STALL packet -*/ - -/* - * complete won't be called, so we enqueue - * setup request here - */ -s3c_hsotg_enqueue_setup(hsotg); - } + if (ret < 0) + s3c_hsotg_stall_ep0(hsotg); } /** @@ -2826,6 +2831,15 @@ static int s3c_hsotg_ep_sethalt(struct usb_ep *ep, int value) dev_info(hs->dev, "%s(ep %p %s, %d)\n", __func__, ep, ep->name, value); + if (index == 0) { + if (value) + s3c_hsotg_stall_ep0(hs); + else + dev_warn(hs->dev, +"%s: can't clear halt on ep0\n", __func__); + return 0; + } + /* write both IN and OUT control registers */ epreg = DIEPCTL(index); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html