date:20140113

From: Bjørn Mork
 Thomas Kear tho...@kear.co.nz writes:
 
  On Sat, Jan 11, 2014 at 11:09 AM, Bjrn Mork bj...@mork.no 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

2014-01-13 Thread Bjørn Mork

David Laight david.lai...@aculab.com 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 david.lai...@aculab.com 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

2014-01-13 Thread Bjørn Mork

David Laight david.lai...@aculab.com writes:
 From: Bjørn Mork
 David Laight david.lai...@aculab.com 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 andrze...@samsung.com
---
 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 andrze...@samsung.com
---
 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 andrze...@samsung.com
---
 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 david.lai...@aculab.com writes:
  From: Bjørn Mork
  David Laight david.lai...@aculab.com 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 mathias.ny...@linux.intel.com
---
 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));
 
@@ -1226,16

[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 mathias.ny...@linux.intel.com
---
 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 linux/gfp.h
+
+#include linux/slab.h
 #include asm/unaligned.h
 
 #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 mathias.ny...@linux.intel.com
---
 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_slot(struct

[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 mathias.ny...@linux.intel.com
---
 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-ring.c

[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 mathias.ny...@linux.intel.com
---
 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 mathias.ny...@linux.intel.com
---
 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 mathias.ny...@linux.intel.com
---
 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 mathias.ny...@linux.intel.com
---
 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 mathias.ny...@linux.intel.com
---
 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 mathias.ny...@linux.intel.com
---
 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

2014-01-13 Thread Sergei Shtylyov


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 andrze...@samsung.com
---
  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 andrze...@samsung.com
---
 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 andrze...@samsung.com
---
 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 andrze...@samsung.com
---
 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 andrze...@samsung.com
---
 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]



snip


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
away entirely.

Alan, if you agree this is the best way forward, I'll do a v5 with the proposed
changes.

Regards,

Hans
--
To unsubscribe from this

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

2014-01-13 Thread Don Zickus

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 dzic...@redhat.com
---
 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 andrze...@samsung.com

Acked-by: Michal Nazarewicz min...@mina86.com

 ---
  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 +--m...@google.com--xmpp:min...@jabber.org--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 andrze...@samsung.com

Acked-by: Michal Nazarewicz min...@mina86.com

 ---
  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 +--m...@google.com--xmpp:min...@jabber.org--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 andrze...@samsung.com

Acked-by: Michal Nazarewicz min...@mina86.com

 ---
  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 +--m...@google.com--xmpp:min...@jabber.org--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 andrze...@samsung.com

Acked-by: Michal Nazarewicz min...@mina86.com

 ---
  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 +--m...@google.com--xmpp:min...@jabber.org--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-usbm=138759482824618w=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-usbm=138260013707007w=2
 [2] http://marc.info/?l=linux-usbm=138511124910669w=2
 [3] http://marc.info/?l=linux-usbm=138775577717546w=2
 [4] http://marc.info/?l=linux-usbm=138759482824618w=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 ++
  drivers/usb/core/hub.h |   14 ++
  drivers/usb/core/port.c|  252

Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14

2014-01-13 Thread Dan Williams

On Mon, Jan 13, 2014 at 11:01 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com 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-usbm=138759482824618w=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

2014-01-13 Thread David Miller

From: Hayes Wang hayesw...@realtek.com
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 hayesw...@realtek.com

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

2014-01-13 Thread 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 
 u.kleine-koe...@pengutronix.de:
  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 Shiyanshc_w...@mail.ru
 ---
 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 pa...@synopsys.com
---
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-usbm=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 pa...@synopsys.com
-and Greg Kroah-Hartman gre...@linuxfoundation.org. And please CC linux-usb
-linux-usb@vger.kernel.org too.
diff --git a/drivers/usb/Kconfig

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 tianyu@intel.com
 
 describe the mechanisms for controlling port power policy and
 discovering the port power state.
 
 Cc: Oliver Neukum oneu...@suse.de
 Signed-off-by: Lan Tianyu tianyu@intel.com
 Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com

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 dan.j.willi...@intel.com
 ---
  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/intf/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

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 toshiharu-li...@dsn.okisemi.com and Tomoya 
MORINAGA tomoya-li...@dsn.okisemi.com, 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 pa...@synopsys.com
---
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 pa...@synopsys.com
 L: linux-usb@vger.kernel.org
 S: Maintained
-F: drivers/staging/dwc2/
+F: drivers/usb/dwc2/
 
 DESIGNWARE USB3 DRD IP DRIVER
 M: Felipe Balbi ba...@ti.com
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-usbm=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.

Re: [PATCH v4] Move DWC2 driver out of staging

2014-01-13 Thread Greg KH

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 pa...@synopsys.com
 ---
 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 pa...@synopsys.com
 ---
 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

2014-01-13 Thread Felipe Balbi

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

2014-01-13 Thread Greg KH

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 pa...@synopsys.com
 Cc: Felipe Balbi ba...@ti.com

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 hdego...@redhat.com
---
 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
 
 config USB_EHCI_HCD_PLATFORM

[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 hdego...@redhat.com
---
 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 m...@bues.ch
  * Copyright 2011-2012 Hauke Mehrtens ha...@hauke-m.de
+ * Copyright 2014 Hans de Goede hdego...@redhat.com
  *
  * 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 linux/clk.h
+#include linux/dma-mapping.h
 #include linux/hrtimer.h
 #include linux/io.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/err.h
+#include linux/phy/phy.h
 #include linux/platform_device.h
 #include linux/usb/ohci_pdriver.h
 #include linux/usb.h
@@ -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

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 dan.j.willi...@intel.com
 ---
  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(dev, NULL, redo_find_peer_port);
 +}
 +
 +static int pair_port(struct device *dev, void *data)
 +{
 +

Re: Bug#704242: Driver for PL-2303 HX not working

2014-01-13 Thread Karsten Malcher

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=41showlevel=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-usbm=138914518219334w=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 dan.j.willi...@intel.com
  ---
   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);
  +

[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 rob.herr...@calxeda.com
Signed-off-by: Paul Zimmerman pa...@synopsys.com
---
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 dzic...@redhat.com
 ---
  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]

2014-01-13 Thread walt

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

2014-01-13 Thread Dan Williams

On Mon, Jan 13, 2014 at 2:56 PM, Sarah Sharp
sarah.a.sh...@linux.intel.com 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

2014-01-13 Thread Greg KH

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 bj...@mork.no
 ---
  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

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-usbm=138914518219334w=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

2014-01-13 Thread Alexander Shiyan

Понедельник, 13 января 2014, 22:31 +01:00 от Uwe Kleine-König 
u.kleine-koe...@pengutronix.de:
 On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
  Суббота, 11 января 2014, 13:55 +01:00 от Uwe Kleine-König 
  u.kleine-koe...@pengutronix.de:
   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 Shiyanshc_w...@mail.ru
  ---
  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

2014-01-13 Thread Chris Ruehl


On Tuesday, January 14, 2014 11:30 AM, Alexander Shiyan wrote:

Понедельник, 13 января 2014, 22:31 +01:00 от Uwe Kleine-König 
u.kleine-koe...@pengutronix.de:

On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:

Суббота, 11 января 2014, 13:55 +01:00 от Uwe Kleine-König 
u.kleine-koe...@pengutronix.de:

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 Shiyanshc_w...@mail.ru
---
 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

2014-01-13 Thread Tony Prisk


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 hdego...@redhat.com
---
  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 @@

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

2014-01-13 Thread Rajaram R

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 a...@signal11.us 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