Re: [PATCH v11 0/8] PHY framework

2013-08-26 Thread Kishon Vijay Abraham I
Hi Greg,

On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
 Added a generic PHY framework that provides a set of APIs for the PHY drivers
 to create/destroy a PHY and APIs for the PHY users to obtain a reference to
 the PHY with or without using phandle.
 
 This framework will be of use only to devices that uses external PHY (PHY
 functionality is not embedded within the controller).
 
 The intention of creating this framework is to bring the phy drivers spread
 all over the Linux kernel to drivers/phy to increase code re-use and to
 increase code maintainability.
 
 Comments to make PHY as bus wasn't done because PHY devices can be part of
 other bus and making a same device attached to multiple bus leads to bad
 design.
 
 If the PHY driver has to send notification on connect/disconnect, the PHY
 driver should make use of the extcon framework. Using this susbsystem
 to use extcon framwork will have to be analysed.
 
 You can find this patch series @
 git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing

Looks like there are not further comments on this series. Can you take this in
your misc tree?

Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: am335x: support for multiple USB instances, v3

2013-08-26 Thread Lars Poeschel
On Friday 02 August 2013 at 21:24:05, Sebastian Andrzej Siewior wrote:
 Hi,
 
 this series depends on Felipe's master tree and usb: phy:
 rename nop_usb_xceiv = usb_phy_gen_xceiv which I posted earlier. I
 created a -rc3 based branch for that patch [0]. If you fine with this
 please let Tony know.
 I updated the child nodes  the phy generic to use two small memory
 spaces instead of one huge as discussed on mailing list in the last
 spin. Patch contains the cppi41 dma driver which seems to work stable
 so far in host mode. Device mode is currently disabled.
 
 The complete series as used for testing is also available at [1]
 
 [0] git://git.breakpoint.cc/cgit/bigeasy/linux.git nop_phy-rename
 [1] git://git.breakpoint.cc/cgit/bigeasy/linux.git am335x_usb

Tested-by: Lars Poeschel poesc...@lemonage.de
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/11] dma: cpp41: enable pm_runtime during init

2013-08-26 Thread Vinod Koul
On Tue, Aug 20, 2013 at 06:35:53PM +0200, Sebastian Andrzej Siewior wrote:
 With enabled pm_runtime in the kernel the device won't work because it
 is not on during the probe function. This patch enables the device via
 pm_runtime on probe so it remains activated.
 
 Cc: Vinod Koul vinod.k...@intel.com
 Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Acked-by Vinod Koul vinod.k...@intel.com

~Vinod
 ---
  drivers/dma/cppi41.c | 11 +++
  1 file changed, 11 insertions(+)
 
 diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
 index e696178..6c4e64f 100644
 --- a/drivers/dma/cppi41.c
 +++ b/drivers/dma/cppi41.c
 @@ -9,6 +9,7 @@
  #include linux/dmapool.h
  #include linux/interrupt.h
  #include linux/of_address.h
 +#include linux/pm_runtime.h
  #include dmaengine.h
  
  #define DESC_TYPE27
 @@ -960,6 +961,11 @@ static int cppi41_dma_probe(struct platform_device *pdev)
   goto err_remap;
   }
  
 + pm_runtime_enable(pdev-dev);
 + ret = pm_runtime_get_sync(pdev-dev);
 + if (ret)
 + goto err_get_sync;
 +
   cdd-queues_rx = glue_info-queues_rx;
   cdd-queues_tx = glue_info-queues_tx;
   cdd-td_queue = glue_info-td_queue;
 @@ -1005,6 +1011,9 @@ static int cppi41_dma_probe(struct platform_device 
 *pdev)
  err_chans:
   deinit_cpii41(pdev, cdd);
  err_init_cppi:
 + pm_runtime_put(pdev-dev);
 +err_get_sync:
 + pm_runtime_disable(pdev-dev);
   iounmap(cdd-usbss_mem);
   iounmap(cdd-ctrl_mem);
   iounmap(cdd-sched_mem);
 @@ -1029,6 +1038,8 @@ static int cppi41_dma_remove(struct platform_device 
 *pdev)
   iounmap(cdd-ctrl_mem);
   iounmap(cdd-sched_mem);
   iounmap(cdd-qmgr_mem);
 + pm_runtime_put(pdev-dev);
 + pm_runtime_disable(pdev-dev);
   kfree(cdd);
   return 0;
  }
 -- 
 1.8.4.rc2
 

-- 
--
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: 3.10.4: kmemleak in usb_get_bos_descriptor()?

2013-08-26 Thread Xenia Ragiadakou

On 08/23/2013 05:07 PM, Alan Stern wrote:

On Fri, 23 Aug 2013, Xenia Ragiadakou wrote:


Martin is right; the BOS descriptors are leaked in
usb_reset_and_verify_device().  We need to store the old descriptor,
compare it with the new one following the reset, and delete one of them
afterward.  I haven't had time to fix this.

I'll put it in my list of future tasks to give to Xenia. :)

Alan, if you have any other small tasks for OPW interns, please let me
know.

Sarah Sharp

Hi,

I will try to fix this leak :)
First i would like to ensure that i understood the problem so ...

If i understood well the usb device's bos descriptor is allocated
everytime hub_port_init()
is called without never being freed, right?

Yes.


I do not understand why to compare the old bos with the new one, and not
just release the old one
or store the new in the memory allocated for the old one.

usb_reset_and_verify_device() checks to see if the descriptors changed
when the device was reset.  (If they did then we need to re-enumerate
the device.)  It calls descriptors_changed() to do this checking.

Originally there were no BOS descriptors, so of course the routine
didn't try to compare them.  But now that they exist, we need to
compare the old and new descriptor values, same as for the device and
config descriptors.  And of course we need to deallocate one of the BOS
descriptors, instead of leaking it.

Alan Stern



Thanks for the clarification.
I will send a patch as RFC because i am not quite familiar and because i 
have a problem to run kmemleak effectively (i can only scan for a few 
seconds, although i have set all necessary kmemleak configuration, i 
have not figured out why yet).


I perform the deallocation outside the descriptors_changed(). That leads 
to code duplication, but if i perform it inside, the function will do 
something that is not expected to do, right?


Also, another thing is that even if the device descriptors have not 
change (and thus the bcdUSB), there is still the possibility one of the 
two bos descriptors to be null while the other one is not, either due to 
a kmalloc() failure or in case the usb device version is in the range 
[0x201, 0x210) where bos descriptors are optional and if new firmware is 
downloaded in the device may has as result to add or remove the bos 
descriptor support. Am i thinking correctly here?


regards,
ksenia
--
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: Reg: USB: ehci-omap: Suspend the controller during idle.

2013-08-26 Thread Roger Quadros
Hi Bharat,

On 08/26/2013 02:29 PM, Bharathraj Nagaraju wrote:
 Dear All,
 
 With continuation to below mail, our client requirement is pushing us to 
 implement the EHCI runtime suspend/resume.
 
 I have implemented EHCI runtime suspend/resume using timer concept,Please 
 find the below design and attached patch for the same.
 
 Design Approach:
  - A timer of 10sec is scheduled.
  - During EHCI transaction (tx/rx), this timer is reset.
If the EHCI clocks are switched off, it is switched on before any 
 actual
transaction.
  - If there are no transaction on EHCI for 10secs, the timer callback is 
 invoked.
  - EHCI clocks are disabled in the timer expire function.

Why are you using your own timer? USB core already supports runtime PM which 
maintains
its own autosuspend timer. you just need to implement the runtime PM driver ops.
(i.e. .runtime_suspend and .runtime_resume)

 
 NOTE : Runtime remote wake up feature need to be implement.

Did you try the patches I pointed to you earlier? They already work with remote 
wakeup.

 
 With the above approach the ECHI is suspended,if there are no transaction for 
  10secs,during next transaction
 i am enabling the clocks and resuming the EHCI controller, but we are getting 
 below errors and its not working properly
 
 [   66.986175] catalina_wwan_ctrl_attr_wwan_on_show: wwan_on status = 1
 [   66.993743] ehci-omap ehci-omap.0: ehci_omap_bus_resume
 [   66.999542] ehci-omap ehci-omap.0: resume root hub
 [   67.024475] ehci_bus_resume: port_status[2]=0x501000, PORT_SUSPEND=0
 [   67.031311] ehci_bus_resume: port_status[1]=0x501000, PORT_SUSPEND=0
 [   67.038238] ehci_bus_resume: port_status[0]=0x601885, PORT_SUSPEND=128
 [   67.079223] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 1
 [   67.086090] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 1
 [   67.093139] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 2
 [   67.099975] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 2
 [   67.106994] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 3
 [   67.113830] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 3
 [   67.120819] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 4
 
 Can you please guide me why we are getting above errors.

These type of errors usually occur if all required clocks for the controller 
are not enabled.

cheers,
-roger

 
 
 From: Roger Quadros [rog...@ti.com]
 Sent: Tuesday, August 06, 2013 7:57 PM
 To: Bharathraj Nagaraju
 Cc: linux-o...@vger.kernel.org; linux-usb@vger.kernel.org; Kevin Hilman; Alan 
 Stern
 Subject: Re: Reg: USB: ehci-omap: Suspend the controller during idle.
 
 Hi,
 
 On 08/06/2013 03:55 PM, Bharathraj Nagaraju wrote:
   Dear All,

   We are working on omap4470 based device,kernel-3.0.31 is running on this.
   In our device modem is connected to OMAP4470 using USB EHCI.

   In our current kernel code, clocks are enabled when ports get connected
   and disabled during disconnect.Due to this current consumption is more in 
 idle use case.

   I have tried to put the host in smart idle/standby mode,due to erratas 
 around Host doesn't allow us
   use smart idle/standby capabilities.

   1) i571: USB host EHCI may stall when entering smart-standby mode
   2) i660: USBHOST Configured In Smart-Idle Can Lead To a Deadlock

   we are thinking to implement USB EHCI runtime suspend/resume feature.

   In kernel-3.0.31 architecture the usbcore driver is parent of ehci and 
 ohci drivers.
   The ehci and ohci drivers call the pm_runtime_get_sync and 
 pm_runtime_put_sync of parent device usbhs core.

   From the ehci-omap driver what i can see only partial implementation of 
 runtime PM.
   what i mean is that runtime PM methods are used only during the normal 
 suspend path
   and when nothing is connected to ports(modem disconnected in our case).

   I am thinking to change the ehci driver code, could claver enough to use 
 runtime PM
   to disable the hardware when modem is connected and the system is in idle 
 state.

   Kindly guide me how can i go about this.

   Let me know if you need further clarification from me on this.

   Thanks for spending your valuable time ...:)
 
 
 The problem with OMAP EHCI controller is that it needs to rely on IO Daisy 
 chain mechanism
 to wake up when it is suspended. Without that, we can't really put the 
 controller in to runtime
 suspend, else we will not be able to detect a new device connect.
 
 IO daisy chaining is still not supported in mainline. One approach had been 
 posted [1] and it works
 but it is still in transition.
 
 I've made some patches [2] which implement USB runtime suspend with remote 
 wakeup working
 for OMAP3 beagle-xm board. This depends on [1]. It shouldn't be hard to get 
 it working on OMAP4.
 
 [1] http://article.gmane.org/gmane.linux.ports.arm.omap/101105
 [2] http://article.gmane.org/gmane.linux.ports.arm.kernel/251250
 
 cheers,
 

Re: 3.11-rc7: compile failure in ohci

2013-08-26 Thread Greg Kroah-Hartman
On Mon, Aug 26, 2013 at 12:46:03PM +0300, Meelis Roos wrote:
 Tried 3.11-rc7 and got this compile failure on a couple of hosts where 
 -rc6 worked. config is below.
 
 This commit seems to break it:
 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9a11899c5e699a8d2551692dfcd4372e39dcbdf6
 
   CC [M]  drivers/usb/host/ohci-pci.o
 drivers/usb/host/ohci-pci.c: In function ‘ohci_pci_init’:
 drivers/usb/host/ohci-pci.c:309:35: error: ‘ohci_suspend’ undeclared (first 
 use in this function)
 drivers/usb/host/ohci-pci.c:309:35: note: each undeclared identifier is 
 reported only once for each function it appears in
 drivers/usb/host/ohci-pci.c:310:34: error: ‘ohci_resume’ undeclared (first 
 use in this function)
 make[3]: *** [drivers/usb/host/ohci-pci.o] Error 1

Sorry about that, known issue, if you enable CONFIG_PM_RUNTIME, it
should go away for now.

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 v1 23/49] hid: usbhid: prepare for enabling irq in complete()

2013-08-26 Thread Jiri Kosina
On Sun, 18 Aug 2013, Ming Lei wrote:

 Complete() will be run with interrupt enabled, so change to
 spin_lock_irqsave().
 
 Cc: Jiri Kosina jkos...@suse.cz
 Cc: linux-in...@vger.kernel.org
 Signed-off-by: Ming Lei ming@canonical.com

Acked-by: Jiri Kosina jkos...@suse.cz

 ---
  drivers/hid/usbhid/hid-core.c |5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
 index bd38cdf..2445fd6 100644
 --- a/drivers/hid/usbhid/hid-core.c
 +++ b/drivers/hid/usbhid/hid-core.c
 @@ -489,8 +489,9 @@ static void hid_ctrl(struct urb *urb)
   struct hid_device *hid = urb-context;
   struct usbhid_device *usbhid = hid-driver_data;
   int unplug = 0, status = urb-status;
 + unsigned long flags;
  
 - spin_lock(usbhid-lock);
 + spin_lock_irqsave(usbhid-lock, flags);
  
   switch (status) {
   case 0: /* success */
 @@ -525,7 +526,7 @@ static void hid_ctrl(struct urb *urb)
   }
  
   clear_bit(HID_CTRL_RUNNING, usbhid-iofl);
 - spin_unlock(usbhid-lock);
 + spin_unlock_irqrestore(usbhid-lock, flags);
   usb_autopm_put_interface_async(usbhid-intf);
   wake_up(usbhid-wait);
  }
 

-- 
Jiri Kosina
SUSE Labs

--
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 PATCH] xhci: fix usb3 streams

2013-08-26 Thread Gerd Hoffmann
xhci maintains a radix tree for each stream endpoint because it must
be able to map a trb address to the stream ring.  Each ring segment
must be added to the ring for this to work.  Currently xhci sticks
only the first segment of each stream ring into the radix tree.

Result is that things work initially, but as soon as the first segment
is full xhci can't map the trb address from the completion event to the
stream ring any more - BOOM.  You'll find this message in the logs:

  ERROR Transfer event for disabled endpoint or incorrect stream ring

This patch adds a helper function to update the radix tree.  It can
both insert and remove ring segments.  It loops over the segment list
and handles all segments instead of just the first.  It is called
whenever an update is needed:  When allocating a ring, when expanding
a ring and when releasing a ring.

It's RfC because (a) I wanna stress test this and the uas driver a bit
more before submitting for real and (b) because there is still a FIXME
to fix.  Reviews are welcome nevertheless.  Also sending to make people
aware I'm looking into this atm.

Latest bits are available from:
  git://git.kraxel.org/linux uas

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 drivers/usb/host/xhci-mem.c | 51 +
 drivers/usb/host/xhci.h |  2 ++
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6f8c2fd..44cdb1d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -154,8 +154,11 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
if (!ring)
return;
 
-   if (ring-first_seg)
+   if (ring-first_seg) {
+   if (ring-type == TYPE_STREAM)
+   xhci_update_stream_ring(ring, 0);
xhci_free_segments_for_ring(xhci, ring-first_seg);
+   }
 
kfree(ring);
 }
@@ -351,6 +354,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct 
xhci_ring *ring,
xhci_dbg(xhci, ring expansion succeed, now has %d segments\n,
ring-num_segs);
 
+   if (ring-type == TYPE_STREAM) {
+   ret = xhci_update_stream_ring(ring, 1);
+   WARN_ON(ret); /* FIXME */
+   }
+
return 0;
 }
 
@@ -601,6 +609,33 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
  * extended systems (where the DMA address can be bigger than 32-bits),
  * if we allow the PCI dma mask to be bigger than 32-bits.  So don't do that.
  */
+
+int xhci_update_stream_ring(struct xhci_ring *ring, bool insert)
+{
+   struct xhci_segment *seg;
+   unsigned long key;
+   bool present;
+   int ret;
+
+   BUG_ON(ring-trb_address_map == NULL);
+   seg = ring-first_seg;
+   do {
+   key = (unsigned long)(seg-dma  TRB_SEGMENT_SHIFT);
+   present = radix_tree_lookup(ring-trb_address_map, key) != NULL;
+   if (!present  insert) {
+   ret = radix_tree_insert(ring-trb_address_map, key, 
ring);
+   if (ret)
+   return ret;
+   }
+   if (present  !insert) {
+   radix_tree_delete(ring-trb_address_map, key);
+   }
+   seg = seg-next;
+   } while (seg != ring-first_seg);
+
+   return 0;
+}
+
 struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
unsigned int num_streams, gfp_t mem_flags)
@@ -608,7 +643,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
struct xhci_stream_info *stream_info;
u32 cur_stream;
struct xhci_ring *cur_ring;
-   unsigned long key;
u64 addr;
int ret;
 
@@ -663,6 +697,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
if (!cur_ring)
goto cleanup_rings;
cur_ring-stream_id = cur_stream;
+   cur_ring-trb_address_map = stream_info-trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
addr = cur_ring-first_seg-dma |
SCT_FOR_CTX(SCT_PRI_TR) |
@@ -672,10 +707,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
xhci_dbg(xhci, Setting stream %d ring ptr to 0x%08llx\n,
cur_stream, (unsigned long long) addr);
 
-   key = (unsigned long)
-   (cur_ring-first_seg-dma  TRB_SEGMENT_SHIFT);
-   ret = radix_tree_insert(stream_info-trb_address_map,
-   key, cur_ring);
+   ret = xhci_update_stream_ring(cur_ring, 1);
if (ret) {
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;

RE: Reg: USB: ehci-omap: Suspend the controller during idle.

2013-08-26 Thread Bharathraj Nagaraju
On 08/26/2013 02:29 PM, Bharathraj Nagaraju wrote:
 Dear All,

 With continuation to below mail, our client requirement is pushing us to 
 implement the EHCI runtime suspend/resume.

 I have implemented EHCI runtime suspend/resume using timer concept,Please 
 find the below design and attached patch for the same.

 Design Approach:
  - A timer of 10sec is scheduled.
  - During EHCI transaction (tx/rx), this timer is reset.
If the EHCI clocks are switched off, it is switched on before any 
 actual
transaction.
  - If there are no transaction on EHCI for 10secs, the timer callback is 
 invoked.
  - EHCI clocks are disabled in the timer expire function.

Why are you using your own timer? USB core already supports runtime PM 
 which maintains
   its own autosuspend timer. you just need to implement the runtime PM 
 driver ops.
(i.e. .runtime_suspend and .runtime_resume)

In our current kernel code(kernel 3.0.31) clocks are enabled when port get 
connected and disabled during disconnect, hence we need to implement with my 
own timer or else we have to change back port kernel 3.8 above code.


 NOTE : Runtime remote wake up feature need to be implement.

  Did you try the patches I pointed to you earlier? They already work with 
 remote wakeup.

Not yet, i will try once this timer implementation is done. 



 With the above approach the ECHI is suspended,if there are no transaction for 
  10secs,during next transaction
 i am enabling the clocks and resuming the EHCI controller, but we are getting 
 below errors and its not working properly

 [   66.986175] catalina_wwan_ctrl_attr_wwan_on_show: wwan_on status = 1
 [   66.993743] ehci-omap ehci-omap.0: ehci_omap_bus_resume
 [   66.999542] ehci-omap ehci-omap.0: resume root hub
 [   67.024475] ehci_bus_resume: port_status[2]=0x501000, PORT_SUSPEND=0
 [   67.031311] ehci_bus_resume: port_status[1]=0x501000, PORT_SUSPEND=0
 [   67.038238] ehci_bus_resume: port_status[0]=0x601885, PORT_SUSPEND=128
 [   67.079223] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 1
 [   67.086090] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 1
 [   67.093139] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 2
 [   67.099975] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 2
 [   67.106994] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 3
 [   67.113830] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 3
 [   67.120819] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 4

 Can you please guide me why we are getting above errors.

 These type of errors usually occur if all required clocks for the 
 controller are not enabled.

Thanks i will look into this.

Cheers Bharath

From: Roger Quadros [rog...@ti.com]
Sent: Monday, August 26, 2013 5:11 PM
To: Bharathraj Nagaraju
Cc: linux-o...@vger.kernel.org; linux-usb@vger.kernel.org
Subject: Re: Reg: USB: ehci-omap: Suspend the controller during idle.

Hi Bharat,

On 08/26/2013 02:29 PM, Bharathraj Nagaraju wrote:
 Dear All,

 With continuation to below mail, our client requirement is pushing us to 
 implement the EHCI runtime suspend/resume.

 I have implemented EHCI runtime suspend/resume using timer concept,Please 
 find the below design and attached patch for the same.

 Design Approach:
  - A timer of 10sec is scheduled.
  - During EHCI transaction (tx/rx), this timer is reset.
If the EHCI clocks are switched off, it is switched on before any 
 actual
transaction.
  - If there are no transaction on EHCI for 10secs, the timer callback is 
 invoked.
  - EHCI clocks are disabled in the timer expire function.

Why are you using your own timer? USB core already supports runtime PM which 
maintains
its own autosuspend timer. you just need to implement the runtime PM driver ops.
(i.e. .runtime_suspend and .runtime_resume)


 NOTE : Runtime remote wake up feature need to be implement.

Did you try the patches I pointed to you earlier? They already work with remote 
wakeup.


 With the above approach the ECHI is suspended,if there are no transaction for 
  10secs,during next transaction
 i am enabling the clocks and resuming the EHCI controller, but we are getting 
 below errors and its not working properly

 [   66.986175] catalina_wwan_ctrl_attr_wwan_on_show: wwan_on status = 1
 [   66.993743] ehci-omap ehci-omap.0: ehci_omap_bus_resume
 [   66.999542] ehci-omap ehci-omap.0: resume root hub
 [   67.024475] ehci_bus_resume: port_status[2]=0x501000, PORT_SUSPEND=0
 [   67.031311] ehci_bus_resume: port_status[1]=0x501000, PORT_SUSPEND=0
 [   67.038238] ehci_bus_resume: port_status[0]=0x601885, PORT_SUSPEND=128
 [   67.079223] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 1
 [   67.086090] ehci-omap ehci-omap.0: detected XactErr len 0/512 retry 1
 [   67.093139] ehci-omap ehci-omap.0: detected XactErr len 0/10 retry 2
 [   67.099975] ehci-omap ehci-omap.0: detected XactErr len 

Re: I: Sierra.c USB Serial Driver FAQ/Feature Request

2013-08-26 Thread Dan Williams
On Sat, 2013-08-17 at 18:05 -0400, Peter Hyman wrote:
 On 08/17/2013 04:58 PM, Forest Bond wrote:
 
 snip...
 
 
  Okay, I published it here:
 
   
 http://www.alittletooquiet.net/media/2131024%20CDMA%20EVDO%20CnS%20Reference.pdf
 
  For the record, the document states its license as follows:
 
http://creativecommons.org/licenses/by-nd/3.0/us/
 
You are free:
 
to Share — to copy, distribute,display, and perform the work
 
Under the following conditions:
 
Attribution. You must attribute the work in the manner specified by
 the author
or licensor (but not in any way that suggests that they endorse you
 or your
use of the work).
 
No Derivative Works. You may not alter, transform, or build upon
 this work.
 
  Thanks,
  Forest
 
 Maybe this will move the ball. We'll see. Thank you.

Yes, I was given that document by Sierra a couple years ago.  No, it
doesn't help, because it only covers basic CDMA/EVDO stuff and does not
cover anything related GSM/UMTS/LTE or other basic modem operations like
mode switching.  So yes, it documents HIP and CnS, but we'd still need
to reverse engineer the mode switch packets that Peter is looking for.

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


[PATCH] USB: OHCI: fix build error related to ohci_suspend/resume

2013-08-26 Thread Alan Stern
Commit 9a11899c5e69 (USB: OHCI: add missing PCI PM callbacks to
ohci-pci.c) added missing ohci_suspend and ohci_resume callback
pointers, but forgot that these callbacks are declared and defined
only when CONFIG_PM is enabled.

This patch adds a preprocessor conditional to avoid build errors when
PM is disabled.

Signed-off-by: Alan Stern st...@rowland.harvard.edu

---


[as1702]


 drivers/usb/host/ohci-pci.c |2 ++
 1 file changed, 2 insertions(+)

Index: usb-3.11/drivers/usb/host/ohci-pci.c
===
--- usb-3.11.orig/drivers/usb/host/ohci-pci.c
+++ usb-3.11/drivers/usb/host/ohci-pci.c
@@ -305,9 +305,11 @@ static int __init ohci_pci_init(void)
 
ohci_init_driver(ohci_pci_hc_driver, pci_overrides);
 
+#ifdef CONFIG_PM
/* Entries for the PCI suspend/resume callbacks are special */
ohci_pci_hc_driver.pci_suspend = ohci_suspend;
ohci_pci_hc_driver.pci_resume = ohci_resume;
+#endif
 
return pci_register_driver(ohci_pci_driver);
 }


--
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: 3.10.4: kmemleak in usb_get_bos_descriptor()?

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Xenia Ragiadakou wrote:

 I will send a patch as RFC because i am not quite familiar and because i 
 have a problem to run kmemleak effectively (i can only scan for a few 
 seconds, although i have set all necessary kmemleak configuration, i 
 have not figured out why yet).
 
 I perform the deallocation outside the descriptors_changed(). That leads 
 to code duplication, but if i perform it inside, the function will do 
 something that is not expected to do, right?

Right.

 Also, another thing is that even if the device descriptors have not 
 change (and thus the bcdUSB), there is still the possibility one of the 
 two bos descriptors to be null while the other one is not, either due to 
 a kmalloc() failure or in case the usb device version is in the range 
 [0x201, 0x210) where bos descriptors are optional and if new firmware is 
 downloaded in the device may has as result to add or remove the bos 
 descriptor support. Am i thinking correctly here?

That's right.  Any difference at all should cause the device to be 
re-enumerated.

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 v6 1/8] usb: phy: omap-control: Get rid of platform data

2013-08-26 Thread Roger Quadros
omap-control device is present from OMAP4 onwards which
support device tree boots only. So get rid of platform data.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/usb/phy/phy-omap-control.c   |   12 +++-
 include/linux/usb/omap_control_usb.h |4 
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-control.c 
b/drivers/usb/phy/phy-omap-control.c
index a4dda8e..9772592 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -197,8 +197,6 @@ static int omap_control_usb_probe(struct platform_device 
*pdev)
 {
struct resource *res;
struct device_node *np = pdev-dev.of_node;
-   struct omap_control_usb_platform_data *pdata =
-   dev_get_platdata(pdev-dev);
 
control_usb = devm_kzalloc(pdev-dev, sizeof(*control_usb),
GFP_KERNEL);
@@ -207,14 +205,10 @@ static int omap_control_usb_probe(struct platform_device 
*pdev)
return -ENOMEM;
}
 
-   if (np) {
+   if (np)
of_property_read_u32(np, ti,type, control_usb-type);
-   } else if (pdata) {
-   control_usb-type = pdata-type;
-   } else {
-   dev_err(pdev-dev, no pdata present\n);
-   return -EINVAL;
-   }
+   else
+   return -EINVAL; /* We only support DT boot */
 
control_usb-dev= pdev-dev;
 
diff --git a/include/linux/usb/omap_control_usb.h 
b/include/linux/usb/omap_control_usb.h
index 27b5b8c..e2416b4 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -31,10 +31,6 @@ struct omap_control_usb {
u32 type;
 };
 
-struct omap_control_usb_platform_data {
-   u8 type;
-};
-
 enum omap_control_usb_mode {
USB_MODE_UNDEFINED = 0,
USB_MODE_HOST,
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 2/8] usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()

2013-08-26 Thread Roger Quadros
Add support for new device types and in the process rid of ti,type
device tree property. The correct type of device will be determined
from the compatible string instead.

Introduce a compatible string for each device type. At the moment
we support 4 types Mailbox, USB2, USB3 and DRA7.

Update DT binding information to reflect these changes.

Also get rid of omap_control_usb3_phy_power(). Just one function
i.e. omap_control_usb_phy_power() will now take care of all PHY types.

Signed-off-by: Roger Quadros rog...@ti.com
---
 Documentation/devicetree/bindings/usb/omap-usb.txt |   29 ++--
 drivers/usb/phy/phy-omap-control.c |  175 
 drivers/usb/phy/phy-omap-usb3.c|6 +-
 include/linux/usb/omap_control_usb.h   |   24 ++--
 4 files changed, 139 insertions(+), 95 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt 
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..e24078f 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -73,22 +73,23 @@ omap_dwc3 {
 OMAP CONTROL USB
 
 Required properties:
- - compatible: Should be ti,omap-control-usb
+ - compatible: Should be one of
+ ti,omap4-control-usb - if it has otghs_control mailbox register as on OMAP4.
+ ti,usb2-control-usb - if it has Power down bit in control_dev_conf register
+   e.g. USB2_PHY on OMAP5.
+ ti,usb3-control-usb - if it has DPLL and individual Rx  Tx power control
+   e.g. USB3 PHY and SATA PHY on OMAP5.
+ ti,dra7-control-usb - if it has both power down and power aux registers
+   e.g. USB2 PHY on DRA7 platform.
  - reg : Address and length of the register set for the device. It contains
-   the address of control_dev_conf and otghs_control or phy_power_usb
-   depending upon omap4 or omap5.
- - reg-names: The names of the register addresses corresponding to the 
registers
-   filled in reg.
- - ti,type: This is used to differentiate whether the control module has
-   usb mailbox or usb3 phy power. omap4 has usb mailbox in control module to
-   notify events to the musb core and omap5 has usb3 phy power register to
-   power on usb3 phy. Should be 1 if it has mailbox and 2 if it has usb3
-   phy power.
+   the address of otghs_control for omap4-control-usb or power register
+   for other types. For dra7-control-usb, it must also contain power_aux
+   register.
+ - reg-names: should be otghs_control for omap4-control-usb and power for
+   other types. For dra7-control-usb type it must also contain power_aux.
 
 omap_control_usb: omap-control-usb@4a002300 {
compatible = ti,omap-control-usb;
-   reg = 0x4a002300 0x4,
- 0x4a00233c 0x4;
-   reg-names = control_dev_conf, otghs_control;
-   ti,type = 1;
+   reg = 0x4a00233c 0x4;
+   reg-names = otghs_control;
 };
diff --git a/drivers/usb/phy/phy-omap-control.c 
b/drivers/usb/phy/phy-omap-control.c
index 9772592..4c4c85c 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -20,6 +20,7 @@
 #include linux/platform_device.h
 #include linux/slab.h
 #include linux/of.h
+#include linux/of_device.h
 #include linux/err.h
 #include linux/io.h
 #include linux/clk.h
@@ -46,61 +47,76 @@ struct device *omap_get_control_dev(void)
 EXPORT_SYMBOL_GPL(omap_get_control_dev);
 
 /**
- * omap_control_usb3_phy_power - power on/off the serializer using control
- * module
+ * omap_control_usb_phy_power - power on/off the phy using control module reg
  * @dev: the control module device
- * @on: 0 to off and 1 to on based on powering on or off the PHY
- *
- * usb3 PHY driver should call this API to power on or off the PHY.
+ * @on: 0 or 1, based on powering on or off the PHY
  */
-void omap_control_usb3_phy_power(struct device *dev, bool on)
+void omap_control_usb_phy_power(struct device *dev, int on)
 {
-   u32 val;
+   u32 val, val_aux;
unsigned long rate;
-   struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+   struct omap_control_usb *control_usb;
 
-   if (control_usb-type != OMAP_CTRL_DEV_TYPE2)
+   if (IS_ERR(dev) || !dev) {
+   pr_err(%s: invalid device\n, __func__);
return;
+   }
 
-   rate = clk_get_rate(control_usb-sys_clk);
-   rate = rate/100;
+   control_usb = dev_get_drvdata(dev);
+   if (!control_usb) {
+   dev_err(dev, %s: invalid control usb device\n, __func__);
+   return;
+   }
 
-   val = readl(control_usb-phy_power);
+   if (control_usb-type == OMAP_CTRL_TYPE_OMAP4)
+   return;
 
-   if (on) {
-   val = ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
-   OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
-   val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON 
-   OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
-   

[PATCH v6 7/8] ARM: dts: omap4: update omap-control-usb nodes

2013-08-26 Thread Roger Quadros
Split otghs_ctrl and USB2 PHY power down into separate
omap-control-usb nodes. Get rid of ti,type property.

Also get rid of ti,has-mailbox property from usb_otg_hs
node and provide the ctrl-module phandle.

CC: Benoit Cousson bcous...@baylibre.com
Signed-off-by: Roger Quadros rog...@ti.com
---
 arch/arm/boot/dts/omap4.dtsi |   20 
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..a77dd0a 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -519,7 +519,7 @@
usb2_phy: usb2phy@4a0ad080 {
compatible = ti,omap-usb2;
reg = 0x4a0ad080 0x58;
-   ctrl-module = omap_control_usb;
+   ctrl-module = omap_control_usb2phy;
};
};
 
@@ -643,12 +643,16 @@
};
};
 
-   omap_control_usb: omap-control-usb@4a002300 {
-   compatible = ti,omap-control-usb;
-   reg = 0x4a002300 0x4,
- 0x4a00233c 0x4;
-   reg-names = control_dev_conf, otghs_control;
-   ti,type = 1;
+   omap_control_usb2phy: omap-control-usb@4a002300 {
+   compatible = ti,usb2-control-usb;
+   reg = 0x4a002300 0x4;
+   reg-names = power;
+   };
+
+   omap_control_usbotg: omap-control-usb@4a00233c {
+   compatible = ti,omap4-control-usb;
+   reg = 0x4a00233c 0x4;
+   reg-names = otghs_control;
};
 
usb_otg_hs: usb_otg_hs@4a0ab000 {
@@ -661,7 +665,7 @@
multipoint = 1;
num-eps = 16;
ram-bits = 12;
-   ti,has-mailbox;
+   ctrl-module = omap_control_usbotg;
};
};
 };
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 8/8] ARM: dts: omap5: update omap-control-usb node

2013-08-26 Thread Roger Quadros
Split USB2 PHY and USB3 PHY into separate omap-control-usb
nodes. Get rid of ti,type property.

CC: Benoit Cousson bcous...@baylibre.com
Signed-off-by: Roger Quadros rog...@ti.com
---
 arch/arm/boot/dts/omap5.dtsi |   20 
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 07be2cd..7cda18b 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -626,12 +626,16 @@
hw-caps-temp-alert;
};
 
-   omap_control_usb: omap-control-usb@4a002300 {
-   compatible = ti,omap-control-usb;
-   reg = 0x4a002300 0x4,
- 0x4a002370 0x4;
-   reg-names = control_dev_conf, phy_power_usb;
-   ti,type = 2;
+   omap_control_usb2phy: omap-control-usb@4a002300 {
+   compatible = ti,usb2-control-usb;
+   reg = 0x4a002300 0x4;
+   reg-names = power;
+   };
+
+   omap_control_usb3phy: omap-control-usb@0x4a002370 {
+   compatible = ti,usb3-control-usb;
+   reg = 0x4a002370 0x4;
+   reg-names = power;
};
 
omap_dwc3@4a02 {
@@ -661,7 +665,7 @@
usb2_phy: usb2phy@4a084000 {
compatible = ti,omap-usb2;
reg = 0x4a084000 0x7c;
-   ctrl-module = omap_control_usb;
+   ctrl-module = omap_control_usb2phy;
};
 
usb3_phy: usb3phy@4a084400 {
@@ -670,7 +674,7 @@
  0x4a084800 0x64,
  0x4a084c00 0x40;
reg-names = phy_rx, phy_tx, pll_ctrl;
-   ctrl-module = omap_control_usb;
+   ctrl-module = omap_control_usb3phy;
};
};
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 5/8] usb: musb: omap2430: Don't use omap_get_control_dev()

2013-08-26 Thread Roger Quadros
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

Also get rid of ti,has-mailbox property as it is redundant and
we can determine that from whether ctrl-module property is present
or not. Get rid of has_mailbox from musb_hdrc_platform_data as well.

Signed-off-by: Roger Quadros rog...@ti.com
---
 Documentation/devicetree/bindings/usb/omap-usb.txt |4 ---
 drivers/usb/musb/omap2430.c|   25 +++
 include/linux/usb/musb.h   |2 -
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt 
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index e24078f..4dc9100 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -3,9 +3,6 @@ OMAP GLUE AND OTHER OMAP SPECIFIC COMPONENTS
 OMAP MUSB GLUE
  - compatible : Should be ti,omap4-musb or ti,omap3-musb
  - ti,hwmods : must be usb_otg_hs
- - ti,has-mailbox : to specify that omap uses an external mailbox
-   (in control module) to communicate with the musb core during device connect
-   and disconnect.
  - multipoint : Should be 1 indicating the musb controller supports
multipoint. This is a MUSB configuration-specific setting.
  - num-eps : Specifies the number of endpoints. This is also a
@@ -28,7 +25,6 @@ SOC specific device node entry
 usb_otg_hs: usb_otg_hs@4a0ab000 {
compatible = ti,omap4-musb;
ti,hwmods = usb_otg_hs;
-   ti,has-mailbox;
multipoint = 1;
num-eps = 16;
ram-bits = 12;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index ebb46ec..516795b 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -38,6 +38,7 @@
 #include linux/delay.h
 #include linux/usb/musb-omap.h
 #include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
 
 #include musb_core.h
 #include omap2430.h
@@ -509,8 +510,12 @@ static int omap2430_probe(struct platform_device *pdev)
glue-dev   = pdev-dev;
glue-musb  = musb;
glue-status= OMAP_MUSB_UNKNOWN;
+   glue-control_otghs = ERR_PTR(-ENODEV);
 
if (np) {
+   struct device_node *control_node;
+   struct platform_device *control_pdev;
+
pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata) {
dev_err(pdev-dev,
@@ -539,22 +544,20 @@ static int omap2430_probe(struct platform_device *pdev)
of_property_read_u32(np, ram-bits, (u32 *)config-ram_bits);
of_property_read_u32(np, power, (u32 *)pdata-power);
config-multipoint = of_property_read_bool(np, multipoint);
-   pdata-has_mailbox = of_property_read_bool(np,
-   ti,has-mailbox);
 
pdata-board_data   = data;
pdata-config   = config;
-   }
 
-   if (pdata-has_mailbox) {
-   glue-control_otghs = omap_get_control_dev();
-   if (IS_ERR(glue-control_otghs)) {
-   dev_vdbg(pdev-dev, Failed to get control device\n);
-   ret = PTR_ERR(glue-control_otghs);
-   goto err2;
+   control_node = of_parse_phandle(np, ctrl-module, 0);
+   if (control_node) {
+   control_pdev = of_find_device_by_node(control_node);
+   if (!control_pdev) {
+   dev_err(pdev-dev, Failed to get control 
device\n);
+   ret = -EINVAL;
+   goto err2;
+   }
+   glue-control_otghs = control_pdev-dev;
}
-   } else {
-   glue-control_otghs = ERR_PTR(-ENODEV);
}
pdata-platform_ops = omap2430_ops;
 
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..eb50525 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -99,8 +99,6 @@ struct musb_hdrc_platform_data {
/* MUSB_HOST, MUSB_PERIPHERAL, or MUSB_OTG */
u8  mode;
 
-   u8  has_mailbox:1;
-
/* for clk_get() */
const char  *clock;
 
-- 
1.7.4.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: Trying to develop a Pixelsense kernel driver

2013-08-26 Thread Florian Echtler
On 22.08.2013 17:22, Florian Echtler wrote:
 On 22.08.2013 16:49, Alan Stern wrote:
 On Thu, 22 Aug 2013, Florian Echtler wrote:
 Userspace version:
 // for control commands
 usb_control_msg(handle, 0xC0, cmd, 0x00, index, (char*)buf, len, 1000);
 Kernel version:
 // for control commands
 #define surface_command(dev, command, index, buffer, size)
 usb_control_msg (dev-usbdev, usb_rcvctrlpipe (dev-usbdev, 0),
 command, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN, 0x00,
 index, buffer, size, 1000)
 This isn't equivalent to the userspace version.  Here the recipient is 
 USB_RECIP_ENDPOINT, but in the userspace version it is 
 USB_RECIP_DEVICE.  To put it another way, here the bRequestType value 
 is 0xC2 and there it is 0xC0.
 Great, thanks for spotting this. I thought I had correctly interpreted
 the original value with respect to the kernel constants - apparently not.
 Once I work up the nerve to give it another try with the updated driver,
 I'll report back.
As promised, I did fix this in the prototype driver and for the first
time, it didn't behave eratically or trash the firmware EEPROM. So I
think I'm finally back on track for a working, non-dangerous input
driver which I should presumably submit to linux-in...@vger.kernel.org
when done?

Thanks again for your help, Alan!

Best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL



signature.asc
Description: OpenPGP digital signature


[RFC] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()

2013-08-26 Thread Xenia Ragiadakou
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.

This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.

Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
 drivers/usb/core/hub.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2801619 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
 } /* usb_hub_cleanup() */
 
 static int descriptors_changed(struct usb_device *udev,
-   struct usb_device_descriptor *old_device_descriptor)
+   struct usb_device_descriptor *old_device_descriptor,
+   struct usb_host_bos *old_bos)
 {
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,19 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
 
+   if (!udev-wusb  le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201) {
+   if ((old_bos  !udev-bos) || (!old_bos  udev-bos))
+   return 1;
+   if (udev-bos) {
+   len = udev-bos-desc-wTotalLength;
+   if (len != old_bos-desc-wTotalLength)
+   return 1;
+   if (memcmp(udev-bos-desc, old_bos-desc,
+   le16_to_cpu(len)))
+   return 1;
+   }
+   }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
 * device descriptor haven't changed, we will assume the
 * Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5063,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
struct usb_hub  *parent_hub;
struct usb_hcd  *hcd = bus_to_hcd(udev-bus);
struct usb_device_descriptordescriptor = udev-descriptor;
+   struct usb_host_bos *bos = udev-bos;
int i, ret = 0;
int port1 = udev-portnum;
 
@@ -5099,12 +5114,17 @@ static int usb_reset_and_verify_device(struct 
usb_device *udev)
goto re_enumerate;
  
/* Device might have changed firmware (DFU or similar) */
-   if (descriptors_changed(udev, descriptor)) {
+   if (descriptors_changed(udev, descriptor, bos)) {
dev_info(udev-dev, device firmware changed\n);
udev-descriptor = descriptor;  /* for disconnect() calls */
+   usb_release_bos_descriptor(udev);
+   udev-bos = bos;
goto re_enumerate;
}
 
+   usb_release_bos_descriptor(udev);
+   udev-bos = bos;
+
/* Restore the device's previous configuration */
if (!udev-actconfig)
goto done;
-- 
1.8.3.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 v6 3/8] usb: phy: omap-usb2: Don't use omap_get_control_dev()

2013-08-26 Thread Roger Quadros
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

As we don't support non-DT boot, we just bail out on probe
if device node is not present.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/usb/phy/phy-omap-usb2.c |   26 --
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..e278a3d 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
 #include linux/pm_runtime.h
 #include linux/delay.h
 #include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
 
 /**
  * omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -121,8 +122,14 @@ static int omap_usb2_suspend(struct usb_phy *x, int 
suspend)
 
 static int omap_usb2_probe(struct platform_device *pdev)
 {
-   struct omap_usb *phy;
-   struct usb_otg  *otg;
+   struct omap_usb *phy;
+   struct usb_otg *otg;
+   struct device_node *node = pdev-dev.of_node;
+   struct device_node *control_node;
+   struct platform_device *control_pdev;
+
+   if (!node)
+   return -EINVAL;
 
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,11 +151,18 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.otg= otg;
phy-phy.type   = USB_PHY_TYPE_USB2;
 
-   phy-control_dev = omap_get_control_dev();
-   if (IS_ERR(phy-control_dev)) {
-   dev_dbg(pdev-dev, Failed to get control device\n);
-   return -ENODEV;
+   control_node = of_parse_phandle(node, ctrl-module, 0);
+   if (!control_node) {
+   dev_err(pdev-dev, Failed to get control device phandle\n);
+   return -EINVAL;
}
+   control_pdev = of_find_device_by_node(control_node);
+   if (!control_pdev) {
+   dev_err(pdev-dev, Failed to get control device\n);
+   return -EINVAL;
+   }
+
+   phy-control_dev = control_pdev-dev;
 
phy-is_suspended   = 1;
omap_control_usb_phy_power(phy-control_dev, 0);
-- 
1.7.4.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


Memory synchronization vs. interrupt handlers

2013-08-26 Thread Alan Stern
David and Paul:

Here's a question that doesn't seem to be answered in 
Documentation/memory-barriers.txt.  Are memory accesses within an 
interrupt handler synchronized with respect to interrupts?

In more detail, suppose we have an interrupt handler that uses a memory
variable A.  The device attached to the IRQ line sends two interrupt
requests, and we get:

CPU 0   CPU 1
-   -
Receive IRQ
Call the interrupt handler
Write A
Finish IRQ processing

Receive IRQ
Call the interrupt handler
Read A
Finish IRQ processing

Is CPU 0's write to A guaranteed to be visible on CPU 1?  Given that 
interrupts on an IRQ line are serialized, and that IRQ processing must 
involve some amount of memory barriers, I would expect the answer to be 
Yes.

Does the answer change if the IRQ line is shared?  I wouldn't expect 
it to be.

Now, if the handler were bound to multiple IRQ (or MSI) lines, then
there'd be no reason to expect this to work.  However, even in this
case, it seems that as long as we restrict our attention to handler
invocations in response to interrupt requests from one particular IRQ
line, the answer should be Yes.  (For example, if device X on IRQ I and 
device Y on IRQ J both used the same handler, a write to A in response 
to an interrupt from device X should be visible the next time X sends
an interrupt.)

Do you know the answers?

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 v6 6/8] usb: phy: omap: get rid of omap_get_control_dev()

2013-08-26 Thread Roger Quadros
This function was preventing us from supporting multiple
instances. Get rid of it. Since we support DT boots only,
users can get the control device phandle from the DT node.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/usb/phy/phy-omap-control.c   |   31 ++-
 include/linux/usb/omap_control_usb.h |5 -
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-control.c 
b/drivers/usb/phy/phy-omap-control.c
index 4c4c85c..1a7e19a 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -26,26 +26,6 @@
 #include linux/clk.h
 #include linux/usb/omap_control_usb.h
 
-static struct omap_control_usb *control_usb;
-
-/**
- * omap_get_control_dev - returns the device pointer for this control device
- *
- * This API should be called to get the device pointer for this control
- * module device. This device pointer should be used for called other
- * exported API's in this driver.
- *
- * To be used by PHY driver and glue driver.
- */
-struct device *omap_get_control_dev(void)
-{
-   if (!control_usb)
-   return ERR_PTR(-ENODEV);
-
-   return control_usb-dev;
-}
-EXPORT_SYMBOL_GPL(omap_get_control_dev);
-
 /**
  * omap_control_usb_phy_power - power on/off the phy using control module reg
  * @dev: the control module device
@@ -188,11 +168,19 @@ void omap_control_usb_set_mode(struct device *dev,
 {
struct omap_control_usb *ctrl_usb;
 
-   if (IS_ERR(dev) || control_usb-type != OMAP_CTRL_TYPE_OMAP4)
+   if (IS_ERR(dev) || !dev)
return;
 
ctrl_usb = dev_get_drvdata(dev);
 
+   if (!ctrl_usb) {
+   dev_err(dev, Invalid control usb device\n);
+   return;
+   }
+
+   if (ctrl_usb-type != OMAP_CTRL_TYPE_OMAP4)
+   return;
+
switch (mode) {
case USB_MODE_HOST:
omap_control_usb_host_mode(ctrl_usb);
@@ -215,6 +203,7 @@ static int omap_control_usb_probe(struct platform_device 
*pdev)
 {
struct resource *res;
const struct of_device_id *of_id;
+   struct omap_control_usb *control_usb;
 
of_id = of_match_device(omap_control_usb_id_table, pdev-dev);
if (!of_id)
diff --git a/include/linux/usb/omap_control_usb.h 
b/include/linux/usb/omap_control_usb.h
index 450b5c1..8008e8d 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -65,15 +65,10 @@ enum omap_control_usb_mode {
 #define OMAP_CTRL_USB2_PHY_PD  BIT(28)
 
 #if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
-extern struct device *omap_get_control_dev(void);
 extern void omap_control_usb_phy_power(struct device *dev, int on);
 extern void omap_control_usb_set_mode(struct device *dev,
enum omap_control_usb_mode mode);
 #else
-static inline struct device *omap_get_control_dev(void)
-{
-   return ERR_PTR(-ENODEV);
-}
 
 static inline void omap_control_usb_phy_power(struct device *dev, int on)
 {
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 0/8] phy: omap-usb: Support multiple instances and new types

2013-08-26 Thread Roger Quadros
Hi,

This patchset does the following:

* Get rid of omap_control_usb platform data as we support DT only.

* Restructure and add support for new PHY types. We now support the follwing
four types

 TYPE_OMAP - if it has otghs_control mailbox register (e.g. on OMAP4)
 TYPE_USB2 - if it has Power down bit in control_dev_conf register. e.g. USB2 
PHY
 TYPE_USB3 - if it has DPLL and individual Rx  Tx power control. e.g. USB3 PHY 
or SATA PHY
 TYPE_DRA7 - if it has both power down and power aux registers. e.g. USB2 PHY 
on DRA7

* Get rid of ti,type DT property and introduce compatible strings for each 
type.

* Have only one power control API omap_control_usb_phy_power() instead of a
different one for each PHY type.

* Get rid of omap_get_control_dev() so that we can support multiple instances
of the control device. We take advantage of the fact that omap control USB 
device
is only present on OMAP4 onwards and hence only supports DT boot. The users
of control USB device can get a reference to it from the device node's phandle.

Patches are based on balbi/next.

Smoke tested on OMAP4 panda with MUSB in gadget mode (g_zero).

You can find the patches in branch
 usb-control-module
in git tree
 git://github.com/rogerq/linux.git

v6:
- addressed review comment about usage of control_usb before allocation.

v5:
- fixed whitespace alignment issues.

v4:
- removed ti,has-mailbox from omap-usb binding document example.

v3:
- return -EINVAL instead of -ENODEV on probe failure.
- pass device type data through of_device_id table.
- get rid of ti,mailbox property and has_mailbox from musb platform data.

v2:
- get rid of ti,type property and introduce compatible strings for each 
device type.

cheers,
-roger

---
Roger Quadros (8):
  usb: phy: omap-control: Get rid of platform data
  usb: phy: omap: Add new device types and remove
omap_control_usb3_phy_power()
  usb: phy: omap-usb2: Don't use omap_get_control_dev()
  usb: phy: omap-usb3: Don't use omap_get_control_dev()
  usb: musb: omap2430: Don't use omap_get_control_dev()
  usb: phy: omap: get rid of omap_get_control_dev()
  ARM: dts: omap4: update omap-control-usb nodes
  ARM: dts: omap5: update omap-control-usb node

 Documentation/devicetree/bindings/usb/omap-usb.txt |   33 ++--
 arch/arm/boot/dts/omap4.dtsi   |   20 ++-
 arch/arm/boot/dts/omap5.dtsi   |   20 ++-
 drivers/usb/musb/omap2430.c|   25 ++-
 drivers/usb/phy/phy-omap-control.c |  210 +++-
 drivers/usb/phy/phy-omap-usb2.c|   26 ++-
 drivers/usb/phy/phy-omap-usb3.c|   32 +++-
 include/linux/usb/musb.h   |2 -
 include/linux/usb/omap_control_usb.h   |   33 ++--
 9 files changed, 226 insertions(+), 175 deletions(-)

-- 
1.7.4.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: [RFC] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Xenia Ragiadakou wrote:

 In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
 descriptor to hold the value read by the device. The new bos descriptor
 has to be compared with the old one in order to figure out if device 's
 firmware has changed in which case the device has to be reenumerated.
 In the original code, none of the two descriptors was deallocated leading
 to memory leaks.
 
 This patch compares the old bos descriptor with the new one to detect change
 in firmware and releases the newly allocated bos descriptor to prevent memory
 leak.

Pretty good, but a few things need to be changed.

 Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
 ---
  drivers/usb/core/hub.c | 24 ++--
  1 file changed, 22 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index 175179e..2801619 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
  } /* usb_hub_cleanup() */
  
  static int descriptors_changed(struct usb_device *udev,
 - struct usb_device_descriptor *old_device_descriptor)
 + struct usb_device_descriptor *old_device_descriptor,
 + struct usb_host_bos *old_bos)

Please leave the indentation the same as it was (two extra tab stops in 
the continuation lines).

  {
   int changed = 0;
   unsignedindex;
 @@ -4953,6 +4954,19 @@ static int descriptors_changed(struct usb_device *udev,
   sizeof(*old_device_descriptor)) != 0)
   return 1;
  
 + if (!udev-wusb  le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201) {

This test shouldn't be here.

 + if ((old_bos  !udev-bos) || (!old_bos  udev-bos))
 + return 1;
 + if (udev-bos) {
 + len = udev-bos-desc-wTotalLength;
 + if (len != old_bos-desc-wTotalLength)
 + return 1;
 + if (memcmp(udev-bos-desc, old_bos-desc,
 + le16_to_cpu(len)))
 + return 1;
 + }
 + }
 +
   /* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
 @@ -5049,6 +5063,7 @@ static int usb_reset_and_verify_device(struct 
 usb_device *udev)
   struct usb_hub  *parent_hub;
   struct usb_hcd  *hcd = bus_to_hcd(udev-bus);
   struct usb_device_descriptordescriptor = udev-descriptor;
 + struct usb_host_bos *bos = udev-bos;
   int i, ret = 0;
   int port1 = udev-portnum;
  
 @@ -5099,12 +5114,17 @@ static int usb_reset_and_verify_device(struct 
 usb_device *udev)
   goto re_enumerate;
   
   /* Device might have changed firmware (DFU or similar) */
 - if (descriptors_changed(udev, descriptor)) {
 + if (descriptors_changed(udev, descriptor, bos)) {
   dev_info(udev-dev, device firmware changed\n);
   udev-descriptor = descriptor;  /* for disconnect() calls */
 + usb_release_bos_descriptor(udev);
 + udev-bos = bos;

Don't do this here...

   goto re_enumerate;
   }
  
 + usb_release_bos_descriptor(udev);
 + udev-bos = bos;
 +

and don't do it here either.  This is tricky because we need to make 
sure the right thing happens no matter what pathway gets followed.  
Your patch misses the if (ret  0) path before the 
descriptors_changed() call.

I think the safest approach is to save the old bos value and set 
udev-bos to NULL at the same time, before the call to
usb_unlocked_disable_lpm().  Then do the release and assignment just 
after the done: and re_enumerate: statement labels.  That way they 
will always be executed, on every pathway.

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: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Ming Lei wrote:

  On Sat, 24 Aug 2013, Ming Lei wrote:
 
  But the current interrupt from EHCI isn't be acked(USBSTS isn't cleared),
  so EHCI HW don't know the irq has been handled, then it is reasonable
  that the EHCI interrupt still interrupts CPU.
 
  EHCI spec(4.15) also states the point clearly:
 
  the host controller is not required to de-assert a currently active
  interrupt condition when software sets the interrupt enables (in 
  the
  USBINR register, see Section 2.3.3) to a zero.  The only reliable
  method software should use for acknowledging an interrupt is by
  transitioning the appropriate status bits in the USBSTS register
 (Section 2.3.2) from a one to a zero.
 
  Ooh, you're right!  I had completely forgotten about this.  It's a
  serious error in the RFC patch.  Fixing it would be pretty easy,
  however.
 
 It is hard to say easy:
 
 - the USBSTS has to be cleared in top half, which means the status has
 to be stored somewhere(suppose it is ehci-irq_status)

That's right.

 - ehci-irq_status need to be read in bottom half for handling irq, so one
 lock might be required for synchronizing the access on the variable

Yes.  A new spinlock would be needed to synchronize the top half and 
the bottom half.  The same spinlock would also be used to avoid 
scheduling the tasklet when it is already running, like in your 
implementation.

 - also, once the current irq is Acked by clearing USBSTS, then later
 interrupts can come, so the irq status should have been saved into one
 queue instead of single variable(see below why disabling ehci irq isn't good)

We don't need a queue.  One variable is enough.  The order in which the 
interrupt status bits turn on doesn't matter; all we need to know are 
which bits require handing.  The top half would do:

ehci-irq_status |= ehci_read(ehci, ehci-regs-status);

The bottom half would do the same thing before checking 
ehci-irq_status, in case more bits got turned on while interrupts were 
masked.

 So when the above implementation is required in each HCDs
 change, I am wondering if it is 'pretty easy'.

I think it is pretty easy for each HCD.  Changing all of them would be 
a large job.

  Remember, more than 99% of the work an HCD does is handling URBs.
  (HCDs do a couple of other things, such as detecting connections and
  disconnection or handling wakeups, but they are very small compared to
  the number of URBs it handles.)  Consider also that an URB isn't done
  until it is given back to the driver that submitted it.
 
  Therefore, when measuring performance or latency of an HCD, what
  matters is how long the driver has to wait from URB submission to URB
  completion.  If any part of that processing takes place in a tasklet,
  the tasklet delay will add in to the total time.  It doesn't matter
  whether the tasklet handles only the giveback or some of the HCD's
  internal work as well.
 
 I am not sure if the idea of disabling EHCI irq inside ehci_irq()
 is good: one completed transfer may not be observed inside current
 tasklet handler since hardware interrupt is disabled, so the transfer
 can't be handled until next tasklet scheduled, then extra delay for
 these URBs is introduced.

Even though interrupts are masked, the tasklet can still check the EHCI
status register to see if new events have occurred while the tasklet
was running, as I described above.  The tasklet could do this check
before returning.

  At least, EHCI HW generates the fatal irq ASAP, This interrupt is not
  delayed to the next interrupt threshold.(4.15.2.4), so it is reasonable 
  for
  software to handle it asap.
 
  That doesn't seem like a good reason.  (Probably it just reflects the
  fact that once a fatal error has occurred, the controller hardware
  can't reliably determine when the next interrupt threshold will occur.)
  But of course, it would be easy to put that part of the code into the
  top half of the handler.
 
 More things done in top half, the change will become more complicated
 since more synchronizations need to consider.

Not at all.  ehci-lock will synchronize things perfectly well.

  To put it another way, which would you prefer: To have three interrupts
  while the tasklet is running, or one interrupt as soon as it finishes?
 
 I prefer to enabling EHCI interrupt during tasklet handling.

What for?  It seems likely that the top half would have to acquire the
private lock before doing any significant processing (i.e., anything
more than scheduling the tasklet).  This would force it to spin until
the tasklet was finished running, which would definitely be a
disadvantage.

 I understand it might still depend on irqs_are_masked assignment to be
 ordered before the write to the intr_enable register, but not true for
 shared interrupts.
 
 Suppose there are two barriers around irq handler, so
 
 CPU0 CPU1
 
 smp_mb()
 read A
 write A
 

[PATCH v6 4/8] usb: phy: omap-usb3: Don't use omap_get_control_dev()

2013-08-26 Thread Roger Quadros
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.

As we don't support non-DT boot, we just bail out on probe
if device node is not present.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/usb/phy/phy-omap-usb3.c |   26 --
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index 4a7f27c..4004f82 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -26,6 +26,7 @@
 #include linux/pm_runtime.h
 #include linux/delay.h
 #include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
 
 #definePLL_STATUS  0x0004
 #definePLL_GO  0x0008
@@ -196,8 +197,14 @@ static int omap_usb3_init(struct usb_phy *x)
 
 static int omap_usb3_probe(struct platform_device *pdev)
 {
-   struct omap_usb *phy;
-   struct resource *res;
+   struct omap_usb *phy;
+   struct resource *res;
+   struct device_node *node = pdev-dev.of_node;
+   struct device_node *control_node;
+   struct platform_device *control_pdev;
+
+   if (!node)
+   return -EINVAL;
 
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -239,11 +246,18 @@ static int omap_usb3_probe(struct platform_device *pdev)
return -EINVAL;
}
 
-   phy-control_dev = omap_get_control_dev();
-   if (IS_ERR(phy-control_dev)) {
-   dev_dbg(pdev-dev, Failed to get control device\n);
-   return -ENODEV;
+   control_node = of_parse_phandle(node, ctrl-module, 0);
+   if (!control_node) {
+   dev_err(pdev-dev, Failed to get control device phandle\n);
+   return -EINVAL;
}
+   control_pdev = of_find_device_by_node(control_node);
+   if (!control_pdev) {
+   dev_err(pdev-dev, Failed to get control device\n);
+   return -EINVAL;
+   }
+
+   phy-control_dev = control_pdev-dev;
 
omap_control_usb_phy_power(phy-control_dev, 0);
usb_add_phy_dev(phy-phy);
-- 
1.7.4.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] USB: OHCI: fix build error related to ohci_suspend/resume

2013-08-26 Thread Guenter Roeck
On Mon, Aug 26, 2013 at 08:38:40AM -0700, Greg Kroah-Hartman wrote:
 On Mon, Aug 26, 2013 at 10:53:53AM -0400, Alan Stern wrote:
  Commit 9a11899c5e69 (USB: OHCI: add missing PCI PM callbacks to
  ohci-pci.c) added missing ohci_suspend and ohci_resume callback
  pointers, but forgot that these callbacks are declared and defined
  only when CONFIG_PM is enabled.
  
  This patch adds a preprocessor conditional to avoid build errors when
  PM is disabled.
  
  Signed-off-by: Alan Stern st...@rowland.harvard.edu
  
  ---
 
 Guenter and Meelis, does the patch below fix the problem for you?
 
Tested-by: Guenter Roeck li...@roeck-us.net

Previously failing builds:
mips:ath79_defconfig
powerpc:ppc64e_defconfig
powerpc:cell_defconfig
powerpc:maple_defconfig
parisc:defconfig
tile:defconfig
sparc32:defconfig
sparc64:defconfig

as well as the powerpc qemu test (with the config file used
by the yocto project)

All builds pass with the patch applied, as does the qemu test.

Guenter
--
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: kernel Oops: 0003 on usbhid_submit_report

2013-08-26 Thread Andreas Lillebø Holm
On Friday, August 23, 2013 at 2:49 PM, Jiri Kosina wrote:
 On Fri, 23 Aug 2013, Benjamin Tissoires wrote:
  
  IIRC, Jiri fixed some paging requests with the two following patches:
  http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca
  http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=bc197eedef1ae082ec662c64c3f4aa302821fb7a
   
  At least, these two patches are related to hid_output_report(). I
  think they are scheduled for 3.12, so I can not find them in Linus'
  tree.
   
  It would worth trying them :)
  
 All three reported oopses happened on page boundary exactly, so this  
 pretty much looks like my implement() fix is the one.
  
 Andreas, could you please apply
  
 http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca
  
 and report back whether it makes your problem go away?
Hi,

You are my hero Jiri :-) These two patches fixes the issues I was seeing and I 
can no longer reproduce them.

I have only tried them on one hardware platform yet; the one that I had 
failing: Atom Intel e6xx CPU with EG20T PCH platform controller. I have had 
approximately 300 boots in our test rigg now and no fails yet. Before was the 
failrate 1/25 boots.

I made the kernel based on 3.10.9 stable tag and applied them directly on 
there. I do recommend that these patches makes it way into the stable tree :-)

Thanks again :)

Best regards
Andreas Lillebø Holm

--
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 v15 00/13] Add tested id switch and vbus connect detect support for Chipidea

2013-08-26 Thread Hector Palacios

Dear Peter,

On 08/12/2013 03:22 AM, Peter Chen wrote:

This patchset adds tested otg id switch function and vbus connect
and disconnect detection for chipidea driver. And fix kinds of
bugs found at chipidea drivers after enabling id and vbus detection.

This patch are fully tested at imx6 sabresd and imx28evk platform by me.
Besides, marek tested it on two STMP3780-based boards (not yet mainline)
and two MX28-based boards.

My chipidea repo: https://github.com/hzpeterchen/linux-usb.git

Changes for v15:
- Mark otgsc API as static line, and move them to otg.h [4/13]
- Remove some redundant comments [6/13]
- Fix some typos [8/13]
- Retire flag CI_HDRC_PULLUP_ON_VBUS [12/13] [13/13]


Patches 6 and 12 in this series seem to be incomplete or malformed. Please 
check.

Best regards,
--
Hector Palacios
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: Fix compilation error when CONFIG_PM disabled

2013-08-26 Thread Shawn Bohrer
Commit 9a11899c5e699a8d USB: OHCI: add missing PCI PM callbacks to
ohci-pci.c Introduced the following compilation errors when power
management is disabled:

drivers/usb/host/ohci-pci.c: In function 'ohci_pci_init':
drivers/usb/host/ohci-pci.c:309:35: error: 'ohci_suspend' undeclared (first use 
in this function)
drivers/usb/host/ohci-pci.c:309:35: note: each undeclared identifier is 
reported only once for each function it appears in
drivers/usb/host/ohci-pci.c:310:34: error: 'ohci_resume' undeclared (first use 
in this function)

ohci_suspend and ohci_resume are only defined when CONFIG_PM is defined
so only use them under CONFIG_PM.

Signed-off-by: Shawn Bohrer sboh...@rgmadvisors.com
---
 drivers/usb/host/ohci-pci.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 0f1d193..062b410 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -305,9 +305,11 @@ static int __init ohci_pci_init(void)
 
ohci_init_driver(ohci_pci_hc_driver, pci_overrides);
 
+#ifdef CONFIG_PM
/* Entries for the PCI suspend/resume callbacks are special */
ohci_pci_hc_driver.pci_suspend = ohci_suspend;
ohci_pci_hc_driver.pci_resume = ohci_resume;
+#endif
 
return pci_register_driver(ohci_pci_driver);
 }
-- 
1.7.7.6


-- 

---
This email, along with any attachments, is confidential. If you 
believe you received this message in error, please contact the 
sender immediately and delete all copies of the message.  
Thank you.
--
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: Fix compilation error when CONFIG_PM disabled

2013-08-26 Thread Greg KH
On Mon, Aug 26, 2013 at 11:35:14AM -0500, Shawn Bohrer wrote:
 Commit 9a11899c5e699a8d USB: OHCI: add missing PCI PM callbacks to
 ohci-pci.c Introduced the following compilation errors when power
 management is disabled:
 
 drivers/usb/host/ohci-pci.c: In function 'ohci_pci_init':
 drivers/usb/host/ohci-pci.c:309:35: error: 'ohci_suspend' undeclared (first 
 use in this function)
 drivers/usb/host/ohci-pci.c:309:35: note: each undeclared identifier is 
 reported only once for each function it appears in
 drivers/usb/host/ohci-pci.c:310:34: error: 'ohci_resume' undeclared (first 
 use in this function)
 
 ohci_suspend and ohci_resume are only defined when CONFIG_PM is defined
 so only use them under CONFIG_PM.
 
 Signed-off-by: Shawn Bohrer sboh...@rgmadvisors.com

Alan sent this patch an hour or so ago, sorry.

I'll add your Signed-off-by: to his patch though, when I queue it up for
Linus in a few hours (I'm in an airport at the moment...)

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] USB: Fix compilation error when CONFIG_PM disabled

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Shawn Bohrer wrote:

 Commit 9a11899c5e699a8d USB: OHCI: add missing PCI PM callbacks to
 ohci-pci.c Introduced the following compilation errors when power
 management is disabled:
 
 drivers/usb/host/ohci-pci.c: In function 'ohci_pci_init':
 drivers/usb/host/ohci-pci.c:309:35: error: 'ohci_suspend' undeclared (first 
 use in this function)
 drivers/usb/host/ohci-pci.c:309:35: note: each undeclared identifier is 
 reported only once for each function it appears in
 drivers/usb/host/ohci-pci.c:310:34: error: 'ohci_resume' undeclared (first 
 use in this function)
 
 ohci_suspend and ohci_resume are only defined when CONFIG_PM is defined
 so only use them under CONFIG_PM.

Heh -- beat you to it.

http://marc.info/?l=linux-usbm=137752885015904w=2

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: OHCI: fix build error related to ohci_suspend/resume

2013-08-26 Thread Greg Kroah-Hartman
On Mon, Aug 26, 2013 at 09:06:27AM -0700, Guenter Roeck wrote:
 On Mon, Aug 26, 2013 at 08:38:40AM -0700, Greg Kroah-Hartman wrote:
  On Mon, Aug 26, 2013 at 10:53:53AM -0400, Alan Stern wrote:
   Commit 9a11899c5e69 (USB: OHCI: add missing PCI PM callbacks to
   ohci-pci.c) added missing ohci_suspend and ohci_resume callback
   pointers, but forgot that these callbacks are declared and defined
   only when CONFIG_PM is enabled.
   
   This patch adds a preprocessor conditional to avoid build errors when
   PM is disabled.
   
   Signed-off-by: Alan Stern st...@rowland.harvard.edu
   
   ---
  
  Guenter and Meelis, does the patch below fix the problem for you?
  
 Tested-by: Guenter Roeck li...@roeck-us.net
 
 Previously failing builds:
   mips:ath79_defconfig
   powerpc:ppc64e_defconfig
   powerpc:cell_defconfig
   powerpc:maple_defconfig
   parisc:defconfig
   tile:defconfig
   sparc32:defconfig
   sparc64:defconfig
 
   as well as the powerpc qemu test (with the config file used
   by the yocto project)
 
 All builds pass with the patch applied, as does the qemu test.

Thanks for testing, I'll queue it up later today (at an airport right
now, about to board a flight), and send it to Linus later tonight if no
one else has any problems with it.

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


[RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()

2013-08-26 Thread Xenia Ragiadakou
In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
descriptor to hold the value read by the device. The new bos descriptor
has to be compared with the old one in order to figure out if device 's
firmware has changed in which case the device has to be reenumerated.
In the original code, none of the two descriptors was deallocated leading
to memory leaks.

This patch compares the old bos descriptor with the new one to detect change
in firmware and releases the newly allocated bos descriptor to prevent memory
leak.

Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---

Differences from version 2:

- fix identation
- initialize udev-bos to null so that check fails if bos is uninitialized
- move bos deallocation inside 'done' and 're_enumerate' paths to ensure
  that the deallocation will be performed even if hub_port_init() fails
- remove check (!udev-wusb  le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201)
  since the checks that follow suffice

 drivers/usb/core/hub.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..2455001 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
 } /* usb_hub_cleanup() */
 
 static int descriptors_changed(struct usb_device *udev,
-   struct usb_device_descriptor *old_device_descriptor)
+   struct usb_device_descriptor *old_device_descriptor,
+   struct usb_host_bos *old_bos)
 {
int changed = 0;
unsignedindex;
@@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
sizeof(*old_device_descriptor)) != 0)
return 1;
 
+   if ((old_bos  !udev-bos) || (!old_bos  udev-bos))
+   return 1;
+   if (udev-bos) {
+   len = udev-bos-desc-wTotalLength;
+   if (len != old_bos-desc-wTotalLength)
+   return 1;
+   if (memcmp(udev-bos-desc, old_bos-desc, le16_to_cpu(len)))
+   return 1;
+   }
+
/* Since the idVendor, idProduct, and bcdDevice values in the
 * device descriptor haven't changed, we will assume the
 * Manufacturer and Product strings haven't changed either.
@@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
struct usb_hub  *parent_hub;
struct usb_hcd  *hcd = bus_to_hcd(udev-bus);
struct usb_device_descriptordescriptor = udev-descriptor;
+   struct usb_host_bos *bos;
int i, ret = 0;
int port1 = udev-portnum;
 
@@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
}
parent_hub = usb_hub_to_struct_hub(parent_hdev);
 
+   bos = udev-bos;
+   udev-bos = NULL;
+
/* Disable LPM and LTM while we reset the device and reinstall the alt
 * settings.  Device-initiated LPM settings, and system exit latency
 * settings are cleared when the device is reset, so we have to set
@@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device 
*udev)
goto re_enumerate;
  
/* Device might have changed firmware (DFU or similar) */
-   if (descriptors_changed(udev, descriptor)) {
+   if (descriptors_changed(udev, descriptor, bos)) {
dev_info(udev-dev, device firmware changed\n);
udev-descriptor = descriptor;  /* for disconnect() calls */
goto re_enumerate;
@@ -5172,11 +5187,15 @@ done:
/* Now that the alt settings are re-installed, enable LTM and LPM. */
usb_unlocked_enable_lpm(udev);
usb_enable_ltm(udev);
+   usb_release_bos_descriptor(udev);
+   udev-bos = bos;
return 0;
  
 re_enumerate:
/* LPM state doesn't matter when we're about to destroy the device. */
hub_port_logical_disconnect(parent_hub, port1);
+   usb_release_bos_descriptor(udev);
+   udev-bos = bos;
return -ENODEV;
 }
 
-- 
1.8.3.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] USB: OHCI: fix build error related to ohci_suspend/resume

2013-08-26 Thread Guenter Roeck
On Mon, Aug 26, 2013 at 09:50:22AM -0700, Greg Kroah-Hartman wrote:
 On Mon, Aug 26, 2013 at 09:06:27AM -0700, Guenter Roeck wrote:
  On Mon, Aug 26, 2013 at 08:38:40AM -0700, Greg Kroah-Hartman wrote:
   On Mon, Aug 26, 2013 at 10:53:53AM -0400, Alan Stern wrote:
Commit 9a11899c5e69 (USB: OHCI: add missing PCI PM callbacks to
ohci-pci.c) added missing ohci_suspend and ohci_resume callback
pointers, but forgot that these callbacks are declared and defined
only when CONFIG_PM is enabled.

This patch adds a preprocessor conditional to avoid build errors when
PM is disabled.

Signed-off-by: Alan Stern st...@rowland.harvard.edu

---
   
   Guenter and Meelis, does the patch below fix the problem for you?
   
  Tested-by: Guenter Roeck li...@roeck-us.net
  
  Previously failing builds:
  mips:ath79_defconfig
  powerpc:ppc64e_defconfig
  powerpc:cell_defconfig
  powerpc:maple_defconfig
  parisc:defconfig
  tile:defconfig
  sparc32:defconfig
  sparc64:defconfig
  
  as well as the powerpc qemu test (with the config file used
  by the yocto project)
  
  All builds pass with the patch applied, as does the qemu test.
 
 Thanks for testing, I'll queue it up later today (at an airport right
 now, about to board a flight), and send it to Linus later tonight if no
 one else has any problems with it.
 
To simplify this kind of testing, I added a 'testing' branch to the list of
branches I am auto-building. Right now it runs a complete test cycle with the
patch applied on top of v3.11-rc7. This should ensure that the patch doesn't
break any previously passing builds. You should see the results in the 'testing'
column of the stable queue build table on 
http://server.roeck-us.net:8010/builders
when you get out of the plane.

Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Sarah Sharp
On Thu, Aug 22, 2013 at 02:55:07PM -0700, Greg KH wrote:
 On Thu, Aug 22, 2013 at 02:49:07PM -0700, Sarah Sharp wrote:
  On Thu, Aug 22, 2013 at 10:42:49AM -0400, Alan Stern wrote:
   On Wed, 21 Aug 2013, Sarah Sharp wrote:
   
Possible fixes
--

The USB core obviously needs to be changed to check the port status
after the TRSMRCY timeout, and continue to wait if the port is still in
the resuming state.  I will have to study the EHCI port status diagrams
in detail to figure out how the USB core can do this.
   
   As far as EHCI is concerned, this is a non-problem.  The closest
   analogy to the RExit-U0 transition is in the description of the Force
   Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says
   that the host controller must complete the transition to the high-speed
   idle state within 2 milliseconds of software setting the bit to a zero
   (which happens when the hub driver does its Get-Port-Status call).
   
   Thus, as soon as the TRSMRCY delay is finished, the device and the port
   are supposed to be ready.  In fact, the hardware doesn't provide any
   means of telling whether they are ready or not.
  
  Well, shoot, I thought I had solved world hunger, or at least USB power
  management issues. :)
  
  So basically it sounds like this is an xHCI specific issue, and probably
  not the root cause of the USB device disconnects we see under EHCI
  hosts.  I guess the xHCI hardware engineers just assumed software would
  always wait for the interrupt from the port status change event, rather
  than using a simple 10 ms timer.  I bet they didn't even realize that
  that the transition took longer than 10ms, because Windows waited for
  the port status change event.

One last thought on this note:  We know Windows doesn't have high-res
timers, and Arjan says asking for a 10 ms delay will produce a delay
around 17 ms.  Since Linux is busy waiting, we may be communicating with
the device sooner than Windows does.

 Why can't Linux do the same thing, and not worry about any timeout at
 all?

We can for xHCI hosts, but not EHCI hosts.  EHCI hosts only send an
interrupt when the suspend change bit is set, which is when software
needs to start the 10ms timer.  It doesn't send an interrupt when that
10ms expires, unlike xHCI.

We could add a new xHCI driver call, to resume the port, which would
only return to the USB core when the port status change event occurs
after the 10 ms (or longer) delay has finished.

  Should I print a debugging message if the xHCI host exceeds 10ms?  I
  would be nice to let hardware engineers know they're out of spec.
 
 Sure, but the odds of anyone of them enabling debugging, and then
 noticing this are slim to none.  But if it makes us feel better pointing
 out hardware bugs (I know it makes me feel good), please do so.

It will make it easier to convince xHCI host designers that they need to
improve their link training timing, so I'll leave it in.  Besides, it's
much more likely that people will have debugging enabled now that Xenia
has made the driver use dynamic debug rather than a config option. :)

Sarah Sharp
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2] usbcore: compare and release one bos descriptor in usb_reset_and_verify_device()

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Xenia Ragiadakou wrote:

 In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
 descriptor to hold the value read by the device. The new bos descriptor
 has to be compared with the old one in order to figure out if device 's
 firmware has changed in which case the device has to be reenumerated.
 In the original code, none of the two descriptors was deallocated leading
 to memory leaks.
 
 This patch compares the old bos descriptor with the new one to detect change
 in firmware and releases the newly allocated bos descriptor to prevent memory
 leak.
 
 Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
 ---
 
 Differences from version 2:
 
 - fix identation
 - initialize udev-bos to null so that check fails if bos is uninitialized
 - move bos deallocation inside 'done' and 're_enumerate' paths to ensure
   that the deallocation will be performed even if hub_port_init() fails
 - remove check (!udev-wusb  le16_to_cpu(udev-descriptor.bcdUSB) = 0x0201)
   since the checks that follow suffice

This looks good to me.  Martin, can you verify that it fixes your 
kmemleak issue?

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: I: Sierra.c USB Serial Driver FAQ/Feature Request

2013-08-26 Thread pete

Quoting Dan Williams d...@redhat.com:


On Sat, 2013-08-17 at 18:05 -0400, Peter Hyman wrote:


snip

Maybe this will move the ball. We'll see. Thank you.


Yes, I was given that document by Sierra a couple years ago.  No, it
doesn't help, because it only covers basic CDMA/EVDO stuff and does not
cover anything related GSM/UMTS/LTE or other basic modem operations like
mode switching.  So yes, it documents HIP and CnS, but we'd still need
to reverse engineer the mode switch packets that Peter is looking for.

Dan




Diminishing returns. For a three year old product which is not  
actively supported by the vendor (linux drivers are old), I recommend  
we just adopt the workaround I wrote and leave it there. Obviously  
proprietary things are done in the proprietary Windows software.  
Thanks all for taking a look.


--
Peter

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Sarah Sharp wrote:

 One last thought on this note:  We know Windows doesn't have high-res
 timers, and Arjan says asking for a 10 ms delay will produce a delay
 around 17 ms.  Since Linux is busy waiting, we may be communicating with
 the device sooner than Windows does.
 
  Why can't Linux do the same thing, and not worry about any timeout at
  all?
 
 We can for xHCI hosts, but not EHCI hosts.  EHCI hosts only send an
 interrupt when the suspend change bit is set, which is when software
 needs to start the 10ms timer.  It doesn't send an interrupt when that
 10ms expires, unlike xHCI.
 
 We could add a new xHCI driver call, to resume the port, which would
 only return to the USB core when the port status change event occurs
 after the 10 ms (or longer) delay has finished.

This sounds a little confused.  The end of the TRSMRCY period isn't
signalled by an interrupt on any host controller, not even on xHCI.  
It's just a regular kernel timer.  We could increase it from 10 ms to
17 (or 20), but only at the cost of increasing resume latency for every
USB device.

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: [alsa-devel] Buffer size for ALSA USB PCM audio

2013-08-26 Thread Alan Stern
Clemens and everyone else:

Not having heard any responses to the patch posted last Wednesday, I 
have updated and completed it.  The version below is ready for testing.  
Please let me know what you find.

It is not very different from the previous version.  I got rid of the
nrpacks module parameter and figured out how to pass the frames/period
and periods/buffer values to data_ep_set_params().  (Maybe this isn't
the best way to do it, but it works.)  I also fixed up the calculation 
that limits URBs to a sync interval.

Of greater interest, I decided to limit each URB to 6 ms and the total
queue to 18 ms, thereby encouraging the driver to use three URBs rather
than two.  (These values could be increased to 8 and 24, respectively.)  
This helps to reduce the effects of an underrun, as follows:

With only two URBs, following an underrun the driver would submit URB1
and URB2.  Because of the delay, it can easily happen that the last
packet of URB1 comes before the isochronous scheduling threshold.  The
URB gets scheduled, but the hardware never sees it.  Consequently there
is no completion interrupt until URB2 finishes, at which point the
queue is empty, causing a secondary underrun.  I actually saw this
happen several times during testing.

James, this patch should be tested along with Clemens's original
maxpacket is too large patch and my EHCI accepts late iso URBs  
patches.  It should allow you to go down to ridiculously low parameter
values, provided only that the total latency is higher than ~1.2 ms.  
For example, at 48 KHz this patch should work okay with 8 frames/period
and 8 periods/buffer.  Of course, larger values will provide greater
resilience against underruns.

Alan Stern



Index: usb-3.11/sound/usb/usbaudio.h
===
--- usb-3.11.orig/sound/usb/usbaudio.h
+++ usb-3.11/sound/usb/usbaudio.h
@@ -55,7 +55,6 @@ struct snd_usb_audio {
struct list_head mixer_list;/* list of mixer interfaces */
 
int setup;  /* from the 'device_setup' module param 
*/
-   int nrpacks;/* from the 'nrpacks' module param */
bool autoclock; /* from the 'autoclock' module param */
 
struct usb_host_interface *ctrl_intf;   /* the audio control interface 
*/
Index: usb-3.11/sound/usb/card.c
===
--- usb-3.11.orig/sound/usb/card.c
+++ usb-3.11/sound/usb/card.c
@@ -79,7 +79,6 @@ static bool enable[SNDRV_CARDS] = SNDRV_
 /* Vendor/product IDs for this card */
 static int vid[SNDRV_CARDS] = { [0 ... (SNDRV_CARDS-1)] = -1 };
 static int pid[SNDRV_CARDS] = { [0 ... (SNDRV_CARDS-1)] = -1 };
-static int nrpacks = 8;/* max. number of packets per urb */
 static int device_setup[SNDRV_CARDS]; /* device parameter for this card */
 static bool ignore_ctl_error;
 static bool autoclock = true;
@@ -94,8 +93,6 @@ module_param_array(vid, int, NULL, 0444)
 MODULE_PARM_DESC(vid, Vendor ID for the USB audio device.);
 module_param_array(pid, int, NULL, 0444);
 MODULE_PARM_DESC(pid, Product ID for the USB audio device.);
-module_param(nrpacks, int, 0644);
-MODULE_PARM_DESC(nrpacks, Max. number of packets per URB.);
 module_param_array(device_setup, int, NULL, 0444);
 MODULE_PARM_DESC(device_setup, Specific device setup (if needed).);
 module_param(ignore_ctl_error, bool, 0444);
@@ -374,7 +371,6 @@ static int snd_usb_audio_create(struct u
chip-dev = dev;
chip-card = card;
chip-setup = device_setup[idx];
-   chip-nrpacks = nrpacks;
chip-autoclock = autoclock;
chip-probing = 1;
 
@@ -756,10 +752,6 @@ static struct usb_driver usb_audio_drive
 
 static int __init snd_usb_audio_init(void)
 {
-   if (nrpacks  1 || nrpacks  MAX_PACKS) {
-   printk(KERN_WARNING invalid nrpacks value.\n);
-   return -EINVAL;
-   }
return usb_register(usb_audio_driver);
 }
 
Index: usb-3.11/sound/usb/endpoint.h
===
--- usb-3.11.orig/sound/usb/endpoint.h
+++ usb-3.11/sound/usb/endpoint.h
@@ -12,6 +12,8 @@ int snd_usb_endpoint_set_params(struct s
snd_pcm_format_t pcm_format,
unsigned int channels,
unsigned int period_bytes,
+   unsigned int period_frames,
+   unsigned int buffer_periods,
unsigned int rate,
struct audioformat *fmt,
struct snd_usb_endpoint *sync_ep);
Index: usb-3.11/sound/usb/card.h
===
--- usb-3.11.orig/sound/usb/card.h
+++ usb-3.11/sound/usb/card.h
@@ -2,11 +2,11 @@
 #define __USBAUDIO_CARD_H
 
 #define MAX_NR_RATES   1024
-#define MAX_PACKS  20
+#define MAX_PACKS  

[RFC 3/3] EHCI: handle late isochronous submissions

2013-08-26 Thread Alan Stern
This patch does the real work.  It fixes up ehci-hcd so that an URB 
submitted by a completion handler will keep the isochronous stream 
alive, even if the handler was delayed by running in a tasklet and the 
queue has emptied out.

As I mentioned earlier, this is not a simple change.

Alan Stern



 drivers/usb/host/ehci-sched.c |  164 +-
 drivers/usb/host/ehci.h   |1 
 2 files changed, 101 insertions(+), 64 deletions(-)

Index: usb-3.11/drivers/usb/host/ehci.h
===
--- usb-3.11.orig/drivers/usb/host/ehci.h
+++ usb-3.11/drivers/usb/host/ehci.h
@@ -433,6 +433,7 @@ struct ehci_iso_packet {
 struct ehci_iso_sched {
struct list_headtd_list;
unsignedspan;
+   unsignedfirst_packet;
struct ehci_iso_packet  packet [0];
 };
 
Index: usb-3.11/drivers/usb/host/ehci-sched.c
===
--- usb-3.11.orig/drivers/usb/host/ehci-sched.c
+++ usb-3.11/drivers/usb/host/ehci-sched.c
@@ -1379,8 +1379,6 @@ sitd_slot_ok (
  * given EHCI_TUNE_FLS and the slop).  Or, write a smarter scheduler!
  */
 
-#define SCHEDULING_DELAY   40  /* microframes */
-
 static int
 iso_stream_schedule (
struct ehci_hcd *ehci,
@@ -1388,10 +1386,12 @@ iso_stream_schedule (
struct ehci_iso_stream  *stream
 )
 {
-   u32 now, base, next, start, period, span;
-   int status;
+   u32 now, base, next, start, period, span, now2;
+   u32 wrap = 0, skip = 0;
+   int status = 0;
unsignedmod = ehci-periodic_size  3;
struct ehci_iso_sched   *sched = urb-hcpriv;
+   boolempty = list_empty(stream-td_list);
 
period = urb-interval;
span = sched-span;
@@ -1402,6 +1402,19 @@ iso_stream_schedule (
 
now = ehci_read_frame_index(ehci)  (mod - 1);
 
+   /* Take the isochronous scheduling threshold into account */
+   if (ehci-i_thresh)
+   next = now + ehci-i_thresh;/* uframe cache */
+   else
+   next = (now + 2 + 7)  ~0x07;   /* full frame cache */
+
+   /*
+* Use ehci-last_iso_frame as the base.  There can't be any
+* TDs scheduled for earlier than that.
+*/
+   base = ehci-last_iso_frame  3;
+   next = (next - base)  (mod - 1);
+
/*
 * Need to schedule; when's the next (u)frame we could start?
 * This is bigger than ehci-i_thresh allows; scheduling itself
@@ -1409,19 +1422,17 @@ iso_stream_schedule (
 * can also help high bandwidth if the dma and irq loads don't
 * jump until after the queue is primed.
 */
-   if (unlikely(list_empty(stream-td_list))) {
-   int done = 0;
-
-   base = now  ~0x07;
-   start = base + SCHEDULING_DELAY;
+   if (unlikely(empty  !urb-ep-giveback_in_progress)) {
+   int done = 0;
+   u32 last;
 
/* find a uframe slot with enough bandwidth.
 * Early uframes are more precious because full-speed
 * iso IN transfers can't use late uframes,
 * and therefore they should be allocated last.
 */
-   next = start;
-   start += period;
+   last = ((++ehci-random_frame)  3)  (period - 1);
+   start = last + period;
do {
start--;
/* check schedule: enough space? */
@@ -1436,7 +1447,7 @@ iso_stream_schedule (
start, sched, period))
done = 1;
}
-   } while (start  next  !done);
+   } while (start  last  !done);
 
/* no room in the schedule */
if (!done) {
@@ -1444,6 +1455,9 @@ iso_stream_schedule (
status = -ENOSPC;
goto fail;
}
+
+   start -= base;
+   goto do_ASAP;
}
 
/*
@@ -1452,72 +1466,85 @@ iso_stream_schedule (
 * (irq delays etc).  If there are, the behavior depends on
 * whether URB_ISO_ASAP is set.
 */
-   else {
+   start = (stream-next_uframe - base)  (mod - 1);
+   now2 = (now - base)  (mod - 1);
 
-   /* Take the isochronous scheduling threshold into account */
-   if (ehci-i_thresh)
-   next = now + ehci-i_thresh;/* uframe cache */
-   else
-   next = (now + 2 + 7)  ~0x07;   /* full frame cache */
+   /* Is the schedule already full? */
+   if (unlikely(!empty  start  period)) {
+ 

[RFC 2/3] EHCI: rearrange some code

2013-08-26 Thread Alan Stern
This patch merely interchanges the if and else branches of the big 
if statement in iso_stream_schedule(), in order to simplify the 
following patch.

Alan Stern


 drivers/usb/host/ehci-sched.c |   92 +-
 1 file changed, 47 insertions(+), 45 deletions(-)

Index: usb-3.11/drivers/usb/host/ehci-sched.c
===
--- usb-3.11.orig/drivers/usb/host/ehci-sched.c
+++ usb-3.11/drivers/usb/host/ehci-sched.c
@@ -1402,12 +1402,57 @@ iso_stream_schedule (
 
now = ehci_read_frame_index(ehci)  (mod - 1);
 
-   /* Typical case: reuse current schedule, stream is still active.
+   /*
+* Need to schedule; when's the next (u)frame we could start?
+* This is bigger than ehci-i_thresh allows; scheduling itself
+* isn't free, the delay should handle reasonably slow cpus.  It
+* can also help high bandwidth if the dma and irq loads don't
+* jump until after the queue is primed.
+*/
+   if (unlikely(list_empty(stream-td_list))) {
+   int done = 0;
+
+   base = now  ~0x07;
+   start = base + SCHEDULING_DELAY;
+
+   /* find a uframe slot with enough bandwidth.
+* Early uframes are more precious because full-speed
+* iso IN transfers can't use late uframes,
+* and therefore they should be allocated last.
+*/
+   next = start;
+   start += period;
+   do {
+   start--;
+   /* check schedule: enough space? */
+   if (stream-highspeed) {
+   if (itd_slot_ok(ehci, mod, start,
+   stream-usecs, period))
+   done = 1;
+   } else {
+   if ((start % 8) = 6)
+   continue;
+   if (sitd_slot_ok(ehci, mod, stream,
+   start, sched, period))
+   done = 1;
+   }
+   } while (start  next  !done);
+
+   /* no room in the schedule */
+   if (!done) {
+   ehci_dbg(ehci, iso sched full %p, urb);
+   status = -ENOSPC;
+   goto fail;
+   }
+   }
+
+   /*
+* Typical case: reuse current schedule, stream is still active.
 * Hopefully there are no gaps from the host falling behind
 * (irq delays etc).  If there are, the behavior depends on
 * whether URB_ISO_ASAP is set.
 */
-   if (likely (!list_empty (stream-td_list))) {
+   else {
 
/* Take the isochronous scheduling threshold into account */
if (ehci-i_thresh)
@@ -1454,49 +1499,6 @@ iso_stream_schedule (
start += base;
}
 
-   /* need to schedule; when's the next (u)frame we could start?
-* this is bigger than ehci-i_thresh allows; scheduling itself
-* isn't free, the delay should handle reasonably slow cpus.  it
-* can also help high bandwidth if the dma and irq loads don't
-* jump until after the queue is primed.
-*/
-   else {
-   int done = 0;
-
-   base = now  ~0x07;
-   start = base + SCHEDULING_DELAY;
-
-   /* find a uframe slot with enough bandwidth.
-* Early uframes are more precious because full-speed
-* iso IN transfers can't use late uframes,
-* and therefore they should be allocated last.
-*/
-   next = start;
-   start += period;
-   do {
-   start--;
-   /* check schedule: enough space? */
-   if (stream-highspeed) {
-   if (itd_slot_ok(ehci, mod, start,
-   stream-usecs, period))
-   done = 1;
-   } else {
-   if ((start % 8) = 6)
-   continue;
-   if (sitd_slot_ok(ehci, mod, stream,
-   start, sched, period))
-   done = 1;
-   }
-   } while (start  next  !done);
-
-   /* no room in the schedule */
-   if (!done) {
-   ehci_dbg(ehci, iso sched full %p, urb);
-   status = -ENOSPC;
-   goto fail;
-   }
-   }
-
/* Tried to schedule too far into the future? */
if (unlikely(start - base + span - period 

[RFC 1/3] Keep track of givebacks in progress

2013-08-26 Thread Alan Stern
This first proposed patch for solving the isochronous stream problem 
adds a mechanism for seeing whether or not an URB is currently being 
given back for a particular endpoint.

The mechanism isn't perfect.  It doesn't use any synchronization, so
it's possible that a CPU may think a giveback is still in progress even
after it has finished.  This shouldn't matter.  If an isochronous queue
empties out and then the driver starts a new stream within a few
milliseconds -- quickly enough that the change to giveback_in_progress
hasn't propagated to all the CPUs -- there won't be any harm in 
scheduling the new stream as a continuation of the old one.

Alan Stern


 drivers/usb/core/hcd.c |3 +++
 include/linux/usb.h|5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Index: usb-3.11/include/linux/usb.h
===
--- usb-3.11.orig/include/linux/usb.h
+++ usb-3.11/include/linux/usb.h
@@ -68,9 +68,10 @@ struct usb_host_endpoint {
void*hcpriv;
struct ep_device*ep_dev;/* For sysfs info */
 
-   unsigned char *extra;   /* Extra descriptors */
-   int extralen;
-   int enabled;
+   unsigned char   *extra; /* Extra descriptors */
+   int extralen;
+   int enabled;
+   boolgiveback_in_progress;
 };
 
 /* host-side wrapper for one interface setting's parsed descriptors */
Index: usb-3.11/drivers/usb/core/hcd.c
===
--- usb-3.11.orig/drivers/usb/core/hcd.c
+++ usb-3.11/drivers/usb/core/hcd.c
@@ -1638,6 +1638,7 @@ int usb_hcd_unlink_urb (struct urb *urb,
 static void __usb_hcd_giveback_urb(struct urb *urb)
 {
struct usb_hcd *hcd = bus_to_hcd(urb-dev-bus);
+   struct usb_host_endpoint *ep = urb-ep;
int status = urb-unlinked;
unsigned long flags;
 
@@ -1653,6 +1654,7 @@ static void __usb_hcd_giveback_urb(struc
 
/* pass ownership to the completion handler */
urb-status = status;
+   ep-giveback_in_progress = true;
 
/*
 * We disable local IRQs here avoid possible deadlock because
@@ -1668,6 +1670,7 @@ static void __usb_hcd_giveback_urb(struc
urb-complete(urb);
local_irq_restore(flags);
 
+   ep-giveback_in_progress = false;
atomic_dec(urb-use_count);
if (unlikely(atomic_read(urb-reject)))
wake_up(usb_kill_urb_queue);


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: OHCI: Allow runtime PM without system sleep

2013-08-26 Thread Alan Stern
Since ohci-hcd supports runtime PM, the .pm field in its pci_driver
structure should be protected by CONFIG_PM rather than
CONFIG_PM_SLEEP.

Without this change, OHCI controllers won't do runtime suspend if
system suspend or hibernation isn't enabled.

Signed-off-by: Alan Stern st...@rowland.harvard.edu
cc: sta...@vger.kernel.org

---

Greg:

I marked this for -stable because it fixes a bug.  But the bug is a 
benign one (under some configs, OHCI controllers won't go to low power 
at times when they could) and nobody has complained about it.  You can 
decide whether or not this is really suitable for the stable kernels.


[as1703]


 drivers/usb/host/ohci-pci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: usb-3.11/drivers/usb/host/ohci-pci.c
===
--- usb-3.11.orig/drivers/usb/host/ohci-pci.c
+++ usb-3.11/drivers/usb/host/ohci-pci.c
@@ -289,7 +289,7 @@ static struct pci_driver ohci_pci_driver
.remove =   usb_hcd_pci_remove,
.shutdown = usb_hcd_pci_shutdown,
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
.driver =   {
.pm =   usb_hcd_pci_pm_ops
},

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Sarah Sharp
On Fri, Aug 23, 2013 at 10:43:19AM -0400, Alan Stern wrote:
 On Wed, 21 Aug 2013, Sarah Sharp wrote:
 
  Background
  --
  
  The USB 2.0 specification, section 7.1.7.7, says that upon device remote
  wakeup signaling, the first active hub (which is often the roothub) must
  rebroadcast the resume signaling for at least 20 ms (TDRSMDN).  After
  that's done, the hub's suspend status change bit will be set, and system
  software must not access the device for at least 10 ms (TRSMRCY).
  
  It turns out that TRSMRCY is a *minimum*, not a *maximum*, according to
  Table 7-14.  That means the port can actually take longer than TRSMRCY
  to resume.  Any attempt to communicate with the device, or reset the
  device, will result in a USB device disconnect.
 
 By the way, I just noticed your Google+ posting about this.  I think 
 you (and perhaps the engineers you spoke with) may have misunderstood 
 what Table 7-14 means when it lists 10 ms as the _minimum_ value for 
 TRSMRCY.
 
 This delay value is a requirement on the OS.  The host system must not
 access the device until at least 10 ms after the resume is complete.  
 The system can wait longer than that if it wants -- that's why 10 ms is
 a minimum.  It just has to avoid accessing the device sooner.
 
 A _minimum_ value on the host side translates into a _maximum_ value on 
 the device side.  The device can safely assume that it can spend up to 
 10 ms getting back into shape after a resume, but no more.  After 10 
 ms, the host may try to communicate with it.

After re-reading the spec, I agree with your analysis.  However, the
fact that chipset designers misinterpreted the spec means there may be
hardware out there that needs a longer timeout.  The spec should have
been normative on both the software and the hardware, saying something
like:

The USB System Software must provide a 10 ms resume recovery time
(TRSMRCY) during which it will not attempt to access any device
connected to the affected (just-activated) bus segment.  The host
controller and device must be ready for communication after the resume
recovery time (TRSMRCY) expires.

I have heard reports of USB devices disconnecting from the bus and
reconnecting after remote wakeup.  I've personally experienced this with
one of my PL2303 USB serial adapters, although it has since died, so I
can't retest.

Another company (whose email I ironically lost due to a failed transfer
to a USB 3.0 backup drive) had bus traces showing the root cause of a
disconnect on resume from remote wakeup.  Occasionally, the host
controller was sending the SoFs too soon on resume, and the device would
interpret it as a low-speed chirp.  The device would disconnect, and
transform from a high speed device to a low speed device.  I don't
think increasing the 10 ms time out will help at all in this case, but
you did ask what USB device disconnect scenarios I've seen.

If users do see device disconnects on remote wakeup resume, we should
see if increasing the timeout helps.

  Then, when the USB core calls into get port status, it transitions the
  port from the Resume state to the RExit state by changing the port link
  state to U0.  The xHCI driver will get a port status change event when
  that transition is complete, but that port status change event is
  currently ignored.
 
 The excess delay you observe with xHCI is the time spent in the RExit
 substate?  That probably should not be counted as part of the TRSMRCY
 period.  It's hard to say for certain, because TRSMRCY is described
 only in the USB-2 spec and not in the xHCI spec, and vice versa for
 RExit.  Still, it's reasonable to assume that the TRSMRCY period should
 begin when the port changes back to U0, not when it leaves the RESUME
 state and enters RExit.
 
 So in the end this appears to be a simple bug in xhci-hcd.  The
 Get-Port-Status request that terminates the resume signalling should
 wait until the port goes back into U0 (which agrees with what you have
 already decided, of course).  ehci-hcd does something similar:
 
   /* stop resume signaling */
   temp = ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
   ehci_writel(ehci, temp, status_reg);
   clear_bit(wIndex, ehci-resuming_ports);
   retval = ehci_handshake(ehci, status_reg,
   PORT_RESUME, 0, 2000 /* 2msec */);
 
 The ehci_handshake call busy-waits until the controller turns off the
 PORT_RESUME bit, which happens when the port has switched to a
 high-speed idle.  It's supposed to take no more than 2 ms but hopefully
 is a lot faster.  (Hmmm, maybe the private lock should be dropped
 during this handshake...)

Ah, so there is an analogous issue in EHCI.  Basically, the EHCI driver
waiting for the PORT_RESUME bit to be clear is equivalent to the xHCI
driver waiting for the port to enter U0.  I agree that this seems like
an xHCI driver issue, and I'll fix it in 

Re: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs

2013-08-26 Thread Sarah Sharp
On Thu, Aug 22, 2013 at 05:11:49AM +, Paul Zimmerman wrote:
  From: Julius Werner
  Sent: Wednesday, August 21, 2013 9:22 PM
  
   You need the USB 2.0 spec errata from 2011-11 that describes the changes
   made for BESL as well.  It's in the USB2-LPM-Errata-final.pdf and
   USB2_LinkPowerMangement_ECN[final].pdf files in this zip file:
  
   http://www.usb.org/developers/docs/usb_20_070113.zip
  
   I agree though, it's all a confusing mess for documentation on USB 2.0
   Link PM.
  
  Thanks, I hadn't found that first one yet. Do you also have a link for
  the updated XHCI specification or a separate erratum document
  describing the PORTHLPMC register referenced in the BESL patches (they
  don't exist on DWC3, but I'm still curious)?
 
 Just to set the record straight :)
 
 The PORTHLPMC registers do exist on DWC3, starting with the 1.90a version
 of the core or thereabouts. They only supported the HIRD flavor of LPM,
 though. Only fairly recently has support for BESL been added, around
 version 2.41a or so.

And the 2.41a version that supports BESL properly sets the BLC flag in
the USB 2.0 Protocol extended capabilities for all the USB 2.0 ports?
Has this functionality been well-tested?

Sarah Sharp
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs

2013-08-26 Thread Paul Zimmerman
 From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com]
 Sent: Monday, August 26, 2013 12:14 PM
 
 On Thu, Aug 22, 2013 at 05:11:49AM +, Paul Zimmerman wrote:
   From: Julius Werner
   Sent: Wednesday, August 21, 2013 9:22 PM
  
You need the USB 2.0 spec errata from 2011-11 that describes the changes
made for BESL as well.  It's in the USB2-LPM-Errata-final.pdf and
USB2_LinkPowerMangement_ECN[final].pdf files in this zip file:
   
http://www.usb.org/developers/docs/usb_20_070113.zip
   
I agree though, it's all a confusing mess for documentation on USB 2.0
Link PM.
  
   Thanks, I hadn't found that first one yet. Do you also have a link for
   the updated XHCI specification or a separate erratum document
   describing the PORTHLPMC register referenced in the BESL patches (they
   don't exist on DWC3, but I'm still curious)?
 
  Just to set the record straight :)
 
  The PORTHLPMC registers do exist on DWC3, starting with the 1.90a version
  of the core or thereabouts. They only supported the HIRD flavor of LPM,
  though. Only fairly recently has support for BESL been added, around
  version 2.41a or so.
 
 And the 2.41a version that supports BESL properly sets the BLC flag in
 the USB 2.0 Protocol extended capabilities for all the USB 2.0 ports?
 Has this functionality been well-tested?

In 2.41a it is described as an early adopter feature in the databook,
and no mention is made of the BLC flag. So the answer there is maybe.
Starting with 2.50a it is a full-fledged feature and the databook does
describe the BLC flag.

So it may be safer to say that the feature is present starting with 2.50a.

In 2.51a it has been well-tested in simulation. In actual hardware, it
has only been briefly tested in an ad-hoc sort of way, since none of the
standard drivers in the market support the feature yet, as far as we know.

Once the support is fully working in the Linux driver we can try testing
it there.

-- 
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: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs

2013-08-26 Thread Paul Zimmerman
 From: Paul Zimmerman
 Sent: Monday, August 26, 2013 12:38 PM

...

 So it may be safer to say that the feature is present starting with 2.50a.
 
 In 2.51a it has been well-tested in simulation. In actual hardware, it

That should have said:

In 2.50a it has been well-tested in simulation...

-- 
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: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Sarah Sharp wrote:

  By the way, I just noticed your Google+ posting about this.  I think 
  you (and perhaps the engineers you spoke with) may have misunderstood 
  what Table 7-14 means when it lists 10 ms as the _minimum_ value for 
  TRSMRCY.
  
  This delay value is a requirement on the OS.  The host system must not
  access the device until at least 10 ms after the resume is complete.  
  The system can wait longer than that if it wants -- that's why 10 ms is
  a minimum.  It just has to avoid accessing the device sooner.
  
  A _minimum_ value on the host side translates into a _maximum_ value on 
  the device side.  The device can safely assume that it can spend up to 
  10 ms getting back into shape after a resume, but no more.  After 10 
  ms, the host may try to communicate with it.
 
 After re-reading the spec, I agree with your analysis.  However, the
 fact that chipset designers misinterpreted the spec means there may be
 hardware out there that needs a longer timeout.  The spec should have
 been normative on both the software and the hardware, saying something
 like:
 
 The USB System Software must provide a 10 ms resume recovery time
 (TRSMRCY) during which it will not attempt to access any device
 connected to the affected (just-activated) bus segment.  The host
 controller and device must be ready for communication after the resume
 recovery time (TRSMRCY) expires.

See, that's not quite right.  The host controller has to be ready for
communication long before TRSMRCY expires.  In fact, the host
controller has to start sending SOF packets within 3 ms of the end of
the resume signalling (to prevent the device from going back into
suspend).

It _would_ be correct to add that the device must be ready for
communication after TRSMRCY expires, but this should be understood
implicitly.  I think the spec is sufficiently clear in this regard.  
And it's not obvious (to me at least) that the chipset designers
misinterpreted it -- but then, I haven't talked to them directly.

 I have heard reports of USB devices disconnecting from the bus and
 reconnecting after remote wakeup.  I've personally experienced this with
 one of my PL2303 USB serial adapters, although it has since died, so I
 can't retest.

Such behavior wouldn't be surprising in the least.  It's the sort of 
thing that caused us to add the RESET_RESUME quirk.

 Another company (whose email I ironically lost due to a failed transfer
 to a USB 3.0 backup drive) had bus traces showing the root cause of a
 disconnect on resume from remote wakeup.  Occasionally, the host
 controller was sending the SoFs too soon on resume, and the device would
 interpret it as a low-speed chirp.

?  There is no such thing as a low-speed chirp.  Did you mean a 
high-speed chirp?

  The device would disconnect, and
 transform from a high speed device to a low speed device.  I don't
 think increasing the 10 ms time out will help at all in this case, but
 you did ask what USB device disconnect scenarios I've seen.

Yeah, that's a bad one.  There's not much we can do about the HC 
sending SOF packets too soon.

 If users do see device disconnects on remote wakeup resume, we should
 see if increasing the timeout helps.

Certainly.

 Ah, so there is an analogous issue in EHCI.  Basically, the EHCI driver
 waiting for the PORT_RESUME bit to be clear is equivalent to the xHCI
 driver waiting for the port to enter U0.  I agree that this seems like
 an xHCI driver issue, and I'll fix it in the driver.

Okay.  It may turn out that some devices still require TRSMRCY to be
longer than 10 ms, even after your fix.  Such behavior wouldn't be
specific to xHCI, though; it would apply equally well to EHCI
connections.

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


[RFC 1/4] xhci: replace USB_MAXINTERFACES with config-desc.bNumInterface

2013-08-26 Thread Xenia Ragiadakou
This patch replaces USB_MAXINTERFACES with config-desc.bNumInterface in
the termination condition for the loop that updates the LPM timeout of the
endpoints on the cofiguration's interfaces, in xhci_calculate_lpm_timeout(),
to avoid unnecessary loop cycles since most configurations come with 1-2
interfaces while USB_MAXINTERFACES is 32.

Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ba0ec0a..787076e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4574,7 +4574,7 @@ static u16 xhci_calculate_lpm_timeout(struct usb_hcd *hcd,
if (!config)
return timeout;
 
-   for (i = 0; i  USB_MAXINTERFACES; i++) {
+   for (i = 0; i  config-desc.bNumInterfaces; i++) {
struct usb_driver *driver;
struct usb_interface *intf = config-interface[i];
 
-- 
1.8.3.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


[RFC 4/4] xhci: remove conversion from generic to pci device in xhci_mem.c

2013-08-26 Thread Xenia Ragiadakou
This patch removes the to_pci_dev() conversion performed to generic struct
device since it is not actually useful (the pointer to the generic device
can be used directly rather through a conversion to pci_dev) and it is pci
bus specific.

Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
 drivers/usb/host/xhci-mem.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6dc5c8b..f201990 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -432,10 +432,10 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
 {
-   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller);
+   struct device *dev = xhci_to_hcd(xhci)-self.controller;
 
if (num_stream_ctxs  MEDIUM_STREAM_ARRAY_SIZE)
-   dma_free_coherent(pdev-dev,
+   dma_free_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
stream_ctx, dma);
else if (num_stream_ctxs = SMALL_STREAM_ARRAY_SIZE)
@@ -460,10 +460,10 @@ static struct xhci_stream_ctx 
*xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs, dma_addr_t *dma,
gfp_t mem_flags)
 {
-   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller);
+   struct device *dev = xhci_to_hcd(xhci)-self.controller;
 
if (num_stream_ctxs  MEDIUM_STREAM_ARRAY_SIZE)
-   return dma_alloc_coherent(pdev-dev,
+   return dma_alloc_coherent(dev,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs,
dma, mem_flags);
else if (num_stream_ctxs = SMALL_STREAM_ARRAY_SIZE)
@@ -1615,7 +1615,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
 {
int num_sp;
int i;
-   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)-self.controller);
+   struct device *dev = xhci_to_hcd(xhci)-self.controller;
 
if (!xhci-scratchpad)
return;
@@ -1623,13 +1623,13 @@ static void scratchpad_free(struct xhci_hcd *xhci)
num_sp = HCS_MAX_SCRATCHPAD(xhci-hcs_params2);
 
for (i = 0; i  num_sp; i++) {
-   dma_free_coherent(pdev-dev, xhci-page_size,
+   dma_free_coherent(dev, xhci-page_size,
xhci-scratchpad-sp_buffers[i],
xhci-scratchpad-sp_dma_buffers[i]);
}
kfree(xhci-scratchpad-sp_dma_buffers);
kfree(xhci-scratchpad-sp_buffers);
-   dma_free_coherent(pdev-dev, num_sp * sizeof(u64),
+   dma_free_coherent(dev, num_sp * sizeof(u64),
xhci-scratchpad-sp_array,
xhci-scratchpad-sp_dma);
kfree(xhci-scratchpad);
@@ -1691,7 +1691,7 @@ 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 device   *dev = xhci_to_hcd(xhci)-self.controller;
struct dev_info *dev_info, *next;
struct xhci_cd  *cur_cd, *next_cd;
unsigned long   flags;
@@ -1701,7 +1701,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci-erst.num_entries);
if (xhci-erst.entries)
-   dma_free_coherent(pdev-dev, size,
+   dma_free_coherent(dev, size,
xhci-erst.entries, xhci-erst.erst_dma_addr);
xhci-erst.entries = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, Freed ERST);
@@ -1749,7 +1749,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
Freed medium stream array pool);
 
if (xhci-dcbaa)
-   dma_free_coherent(pdev-dev, sizeof(*xhci-dcbaa),
+   dma_free_coherent(dev, sizeof(*xhci-dcbaa),
xhci-dcbaa, xhci-dcbaa-dma);
xhci-dcbaa = NULL;
 
-- 
1.8.3.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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread David Miller
From: Enrico Mioso mrkiko...@gmail.com
Date: Fri, 23 Aug 2013 09:56:29 +0200

 + if ((on  atomic_add_return(1, drvstate-pmcount) == 1) || (!on  
 atomic_dec_and_test(drvstate-pmcount))) {

These line significantly exceeds 80 columns.

 + subdriver = usb_cdc_wdm_register(ctx-control,
 +  usbnet_dev-status-desc,
 +  256, /* CDC-WMC r1.1 requires 
 wMaxCommand to be at least 256 decimal (0x100) */
 +  
 huawei_cdc_ncm_wdm_manage_power);

Likewise.

 + if (intf == ctx-control  drvstate-subdriver  
 drvstate-subdriver-suspend)

Likewise.

 + int ret = 0;
 + struct usbnet *usbnet_dev = usb_get_intfdata(intf);
 + struct huawei_cdc_ncm_state *drvstate = (void *)usbnet_dev-data;
 + struct cdc_ncm_ctx *ctx = drvstate-ctx;
 + bool callsub = (intf == ctx-control  drvstate-subdriver  
 drvstate-subdriver-resume); /* should we call subdriver's resume? ? */

Likewise, and order local function variable declarations by line
length, longest to shortest.

--
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 3/4] xhci: remove unnecessary check in xhci_free_stream_info()

2013-08-26 Thread Xenia Ragiadakou
This patch removes the unneccessary check 'if (stream_info)' because
there is already a check few lines above which ensures that stream_info
is not NULL.

Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
 drivers/usb/host/xhci-mem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 53b972c..6dc5c8b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -721,8 +721,7 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
stream_info-stream_ctx_array,
stream_info-ctx_array_dma);
 
-   if (stream_info)
-   kfree(stream_info-stream_rings);
+   kfree(stream_info-stream_rings);
kfree(stream_info);
 }
 
-- 
1.8.3.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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread Joe Perches
On Mon, 2013-08-26 at 16:31 -0400, David Miller wrote:
 From: Enrico Mioso mrkiko...@gmail.com
[]
  + int ret = 0;
  + struct usbnet *usbnet_dev = usb_get_intfdata(intf);
  + struct huawei_cdc_ncm_state *drvstate = (void *)usbnet_dev-data;
  + struct cdc_ncm_ctx *ctx = drvstate-ctx;
  + bool callsub = (intf == ctx-control  drvstate-subdriver  
  drvstate-subdriver-resume); /* should we call subdriver's resume? ? */
 
 Likewise, and order local function variable declarations by line
 length, longest to shortest.

I think the premise of local variable
declaration by line length is flawed.

It can't work when local variables are
dependent on initialization order as
above.

int ret; could be moved down below callsub
if really desired though.



--
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: 045e:0745 [HP TouchSmart tm2t-2200] No scancode for several keys of Microsoft Comfort Desktop 5000

2013-08-26 Thread Ari Fogel
It seems I had trouble parsing the maze of instructions I've been
navigating. I suppose I should have emailed the driver maintainer instead.

--
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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread David Miller
From: Joe Perches j...@perches.com
Date: Mon, 26 Aug 2013 13:45:13 -0700

 I think the premise of local variable
 declaration by line length is flawed.

I disagree.

 It can't work when local variables are
 dependent on initialization order as
 above.

Move the initialization below the declarations.
--
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: failure to resume from suspend cause by 84ebc102

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Adam Borowski wrote:

 Hi!
 
 I'm afraid that your kernel patch 84ebc10294a3d7be4c66f51070b7aedbaa24de9b
 (USB: remove CONFIG_USB_SUSPEND option) causes lockups on resume from
 suspend on at least some machines.
 
 On kernels from this patch forward up to some point in 3.11-rc, there is
 nothing but a blank screen after resuming; in later kernels the display
 returns but that's it.  With no_console_suspend=1, it appears like it starts
 to dump a trace, yet fails to do so.  120 seconds later, I get a hung_task
 message with the following trace:
 
 schedule_timeout+0x1f9/0x2c0
 set_cpus_allowed_ptr+0x6e/0x100
 kthread_create_on_node+0x100/0x110
 wait_for_completion+0x9a/0x100
 wake_up_state+0x10/0x10
 device_resume+0x42/0x180
 async_resume+0x14/0x40
 async_run_entry_fn+0x2d/0x120
 process_one_work+0x167/0x410
 worker_thread+0x116/0x3a0
 manage_workers.isra.25+0x290/0x290
 kthread+0xaf/0xc0
 kthread_create_on_node+0x110/0x110
 ret_from_fork+0x7c/0xb0
 kthread_create_on_node+0x110/0x110
 
 The config comes from Debian kernel packages, it includes:
 CONFIG_PM_RUNTIME=y
 CONFIG_PM=y

I presume CONFIG_PM_SLEEP is also enabled, since otherwise you wouldn't 
be able to suspend the machine.

What happens if go back to a kernel without that commit and enable
CONFIG_USB_SUSPEND?  The behavior should be identical -- basically the
commit is supposed to have the effect of always assuming that
CONFIG_USB_SUSPEND has the same value as CONFIG_PM_RUNTIME, except in 
one spot where it is assumed to have the same value as CONFIG_PM.

Also, bear in mind that the commit you mentioned was incomplete.  It
has to be applied along with 98f541c6e390 (USB: remove remaining
instances of USB_SUSPEND).  Of course, the later kernels all have both
commits.

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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread Joe Perches
On Mon, 2013-08-26 at 16:58 -0400, David Miller wrote:
 From: Joe Perches j...@perches.com
 Date: Mon, 26 Aug 2013 13:45:13 -0700
 
  I think the premise of local variable
  declaration by line length is flawed.
 
 I disagree.
 
  It can't work when local variables are
  dependent on initialization order as
  above.
 
 Move the initialization below the declarations.

So all initializations should be on separate
lines from declarations?

ick.

It's possible, but it doubles the associated
line count and I still think it's a very
dubious premise/practice/style.

As a general rule, it's I suppose OK, but as
a hardened rule, it's like most all hard rules.
It's made to be broken.



--
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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread David Miller
From: Joe Perches j...@perches.com
Date: Mon, 26 Aug 2013 14:05:55 -0700

 On Mon, 2013-08-26 at 16:58 -0400, David Miller wrote:
 From: Joe Perches j...@perches.com
 Date: Mon, 26 Aug 2013 13:45:13 -0700
 
  I think the premise of local variable
  declaration by line length is flawed.
 
 I disagree.
 
  It can't work when local variables are
  dependent on initialization order as
  above.
 
 Move the initialization below the declarations.
 
 So all initializations should be on separate
 lines from declarations?

No, you can often make it work out when the initialization
occurs on the same time.

Like everything else in life, you evaluate it on a case by
case basis.

Don't try to over-simplify things and act as if they are black
and white when they aren't.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bug] [Sony VAIO SVT15117CXS] USB 2.0 ports not working with any USB device

2013-08-26 Thread Kevin Archer
I think the link in my first email was wrong
here is the correct link
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1210858

I did run 2.6.33 and 2.6.39 in which the 2 usb ports did appear to
work, but the Xserver did not work correctly so I only had command
line.


On Sat, Aug 24, 2013 at 12:10 PM, Kevin Archer kevin.arche...@gmail.com wrote:
 dmesg with device plugged in
 I don't think system saw it.
 The usb 2.0 devices the log sees are all built in.
 maybe some kind of conflict issue with the 2 ports?

 Thanks


 On Sat, Aug 24, 2013 at 11:44 AM, Kevin Archer kevin.arche...@gmail.com 
 wrote:
 I have seen a power flash on the devices at startup then nothing.
 usbmon for bus 1 gives no output what so ever.
 doing the modprobe for dummy-irq irq=16 gives me the one extra line on dmesg
 [] dummy-irq: registered for IRQ 16
 attaching lsusb output as well as the dmesg for the tests (nothing
 installed during boot), finally I am will reboot with device installed
 and post that dmesg output

 On Wed, Aug 21, 2013 at 11:13 AM, Alan Stern st...@rowland.harvard.edu 
 wrote:
 On Tue, 20 Aug 2013, Kevin Archer wrote:

 DMESG log from CONFIG_DUMMY_IRQ per your instructions

 [ 8561.353244] ath9k: ath9k: Driver unloaded
 [ 8627.529948] ehci-pci :00:1a.0: remove, state 1
 [ 8627.529961] ehci-pci :00:1a.0: roothub graceful disconnect
 [ 8627.529973] usb usb1: USB disconnect, device number 1
 [ 8627.529979] usb 1-1: USB disconnect, device number 2
 [ 8627.529984] usb 1-1.1: USB disconnect, device number 3
 [ 8627.529988] usb 1-1.1: unregistering device
 [ 8627.529994] usb 1-1.1: unregistering interface 1-1.1:1.0
 [ 8627.530101] usb 1-1.1: unlink qh1-0e01/880241e95180 start 0 [1/3 us]
 [ 8627.530110] ehci-pci :00:1a.0: shutdown urb 88023f922600 
 ep1in-intr
 [ 8627.559559] usbhid 1-1.1:1.0: removing 0 minor
 [ 8627.560402] usb 1-1.1: usb_disable_device nuking all URBs
 [ 8627.561008] usb 1-1.2: USB disconnect, device number 4
 [ 8627.561022] usb 1-1.2: unregistering device
 [ 8627.561034] usb 1-1.2: unregistering interface 1-1.2:1.0
 [ 8627.561432] usb 1-1.2: unlink qh1-0e01/88023fe57500 start 0 [1/2 us]
 [ 8627.561456] ehci-pci :00:1a.0: shutdown urb 880243aceb40 
 ep1in-intr
 [ 8627.563903] ehci-pci :00:1a.0: shutdown urb 880243ace6c0 
 ep2in-bulk
 [ 8627.563918] ehci-pci :00:1a.0: shutdown urb 880243ace9c0 
 ep2in-bulk
 [ 8627.564084] usb 1-1: clear tt buffer port 2, a4 ep2 t04048d80
 [ 8627.566562] usb 1-1.2: unregistering interface 1-1.2:1.1
 [ 8627.566715] usb 1-1.2: usb_disable_device nuking all URBs
 [ 8627.566993] usb 1-1.3: USB disconnect, device number 5
 [ 8627.567003] usb 1-1.3: unregistering device
 [ 8627.567011] usb 1-1.3: unregistering interface 1-1.3:1.0
 [ 8627.615764] usb 1-1.3: unregistering interface 1-1.3:1.1
 [ 8627.615844] usb 1-1.3: usb_disable_device nuking all URBs
 [ 8627.616210] usb 1-1: unregistering device
 [ 8627.616219] usb 1-1: unregistering interface 1-1:1.0
 [ 8627.616303] usb 1-1: unlink qh256-0001/88023fb6e280 start 1 [1/0 us]
 [ 8627.616327] ehci-pci :00:1a.0: shutdown urb 88023fe9a240 
 ep1in-intr
 [ 8627.618111] usb 1-1: usb_disable_device nuking all URBs
 [ 8627.618990] usb usb1: unregistering device
 [ 8627.619001] usb usb1: unregistering interface 1-0:1.0
 [ 8627.619100] ehci-pci :00:1a.0: shutdown urb 88023fc0a300 
 ep1in-intr
 [ 8627.619230] usb usb1: usb_disable_device nuking all URBs
 [ 8627.619873] ehci-pci :00:1a.0: stop
 [ 8627.620405] ehci-pci :00:1a.0: reset command 0010002 (park)=0
 ithresh=1 period=1024 Reset HALT
 [ 8627.624708] ehci-pci :00:1a.0: irq normal 193 err 3 iaa 90 (lost 0)
 [ 8627.624719] ehci-pci :00:1a.0: complete 195 unlink 8
 [ 8627.624732] ehci-pci :00:1a.0: ehci_stop completed status 1000 Halt
 [ 8627.624760] ehci-pci :00:1a.0: USB bus 1 deregistered
 [ 8652.756509] dummy-irq: registered for IRQ 16

 And nothing more?  Hmmm...

 What shows up if you do just the modprobe dummy-irq step (skip the
 rmmod and echo steps)?  In particular, do any more irq 16: nobody
 cared messages show up?

 If they don't, does loading the dummy-irq module cause your USB-2
 devices to start working again?

 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 RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver

2013-08-26 Thread Joe Perches
On Mon, 2013-08-26 at 17:09 -0400, David Miller wrote:
 From: Joe Perches j...@perches.com
[]
 Don't try to over-simplify things and act as if they are black
 and white when they aren't.

It wasn't me simplifying.

I commented on your request to:

order local function variable declarations by line
 length, longest to shortest.

as it seemed a bit rigid.

And perhaps you didn't read the rest of my email.

 As a general rule, it's I suppose OK, but as
 a hardened rule, it's like most all hard rules.
 It's made to be broken.



--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Greg KH
On Mon, Aug 26, 2013 at 10:45:30AM -0700, Sarah Sharp wrote:
 On Thu, Aug 22, 2013 at 02:55:07PM -0700, Greg KH wrote:
  On Thu, Aug 22, 2013 at 02:49:07PM -0700, Sarah Sharp wrote:
   On Thu, Aug 22, 2013 at 10:42:49AM -0400, Alan Stern wrote:
On Wed, 21 Aug 2013, Sarah Sharp wrote:

 Possible fixes
 --
 
 The USB core obviously needs to be changed to check the port status
 after the TRSMRCY timeout, and continue to wait if the port is still 
 in
 the resuming state.  I will have to study the EHCI port status 
 diagrams
 in detail to figure out how the USB core can do this.

As far as EHCI is concerned, this is a non-problem.  The closest
analogy to the RExit-U0 transition is in the description of the Force
Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says
that the host controller must complete the transition to the high-speed
idle state within 2 milliseconds of software setting the bit to a zero
(which happens when the hub driver does its Get-Port-Status call).

Thus, as soon as the TRSMRCY delay is finished, the device and the port
are supposed to be ready.  In fact, the hardware doesn't provide any
means of telling whether they are ready or not.
   
   Well, shoot, I thought I had solved world hunger, or at least USB power
   management issues. :)
   
   So basically it sounds like this is an xHCI specific issue, and probably
   not the root cause of the USB device disconnects we see under EHCI
   hosts.  I guess the xHCI hardware engineers just assumed software would
   always wait for the interrupt from the port status change event, rather
   than using a simple 10 ms timer.  I bet they didn't even realize that
   that the transition took longer than 10ms, because Windows waited for
   the port status change event.
 
 One last thought on this note:  We know Windows doesn't have high-res
 timers, and Arjan says asking for a 10 ms delay will produce a delay
 around 17 ms.  Since Linux is busy waiting, we may be communicating with
 the device sooner than Windows does.

What does Windows do for this type of thing?  How long do they wait
before reporting an error?  We probably need to do the same as they do,
no matter what the spec says.

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: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Sarah Sharp
On Mon, Aug 26, 2013 at 03:07:25PM -0700, Greg KH wrote:
 On Mon, Aug 26, 2013 at 10:45:30AM -0700, Sarah Sharp wrote:
  
  One last thought on this note:  We know Windows doesn't have high-res
  timers, and Arjan says asking for a 10 ms delay will produce a delay
  around 17 ms.  Since Linux is busy waiting, we may be communicating with
  the device sooner than Windows does.
 
 What does Windows do for this type of thing?  How long do they wait
 before reporting an error?  We probably need to do the same as they do,
 no matter what the spec says.

I double checked with the Intel Windows devs, and they say that Arjan is
mistaken, and they'll get around 11 ms, just like we do.

They say:

The EHCI driver stack applies the 10 ms starting at the point that it
sees C_PORT_SUSPEND asserted for the port.

The inaccuracy in Windows 8 is generally no more than 1 system clock
period late, but it can be delayed by interrupt processing for other
devices. I would expect the additional average delay to be no more than
1 ms.

They didn't mention waiting for the PORT_RESUME bit to be cleared, like
the Linux EHCI driver does.  Alan, would you like me to ask them about
that?

Sarah Sharp
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet

2013-08-26 Thread Ming Lei
On Mon, Aug 26, 2013 at 11:29 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Mon, 26 Aug 2013, Ming Lei wrote:

  On Sat, 24 Aug 2013, Ming Lei wrote:
 
  But the current interrupt from EHCI isn't be acked(USBSTS isn't cleared),
  so EHCI HW don't know the irq has been handled, then it is reasonable
  that the EHCI interrupt still interrupts CPU.
 
  EHCI spec(4.15) also states the point clearly:
 
  the host controller is not required to de-assert a currently 
  active
  interrupt condition when software sets the interrupt enables (in 
  the
  USBINR register, see Section 2.3.3) to a zero.  The only reliable
  method software should use for acknowledging an interrupt is by
  transitioning the appropriate status bits in the USBSTS register
 (Section 2.3.2) from a one to a zero.
 
  Ooh, you're right!  I had completely forgotten about this.  It's a
  serious error in the RFC patch.  Fixing it would be pretty easy,
  however.

 It is hard to say easy:

 - the USBSTS has to be cleared in top half, which means the status has
 to be stored somewhere(suppose it is ehci-irq_status)

 That's right.

 - ehci-irq_status need to be read in bottom half for handling irq, so one
 lock might be required for synchronizing the access on the variable

 Yes.  A new spinlock would be needed to synchronize the top half and
 the bottom half.  The same spinlock would also be used to avoid
 scheduling the tasklet when it is already running, like in your
 implementation.

Then every HCD need to copy these kind of implementation...


 - also, once the current irq is Acked by clearing USBSTS, then later
 interrupts can come, so the irq status should have been saved into one
 queue instead of single variable(see below why disabling ehci irq isn't good)

 We don't need a queue.  One variable is enough.  The order in which the
 interrupt status bits turn on doesn't matter; all we need to know are
 which bits require handing.  The top half would do:

 ehci-irq_status |= ehci_read(ehci, ehci-regs-status);

Looks OK.


 The bottom half would do the same thing before checking
 ehci-irq_status, in case more bits got turned on while interrupts were
 masked.

 So when the above implementation is required in each HCDs
 change, I am wondering if it is 'pretty easy'.

 I think it is pretty easy for each HCD.  Changing all of them would be
 a large job.

Still not sure it is pretty easy since extra lock things have to be considered:
(such as, order between two locks, disabling irq and acquiring lock)


  Remember, more than 99% of the work an HCD does is handling URBs.
  (HCDs do a couple of other things, such as detecting connections and
  disconnection or handling wakeups, but they are very small compared to
  the number of URBs it handles.)  Consider also that an URB isn't done
  until it is given back to the driver that submitted it.
 
  Therefore, when measuring performance or latency of an HCD, what
  matters is how long the driver has to wait from URB submission to URB
  completion.  If any part of that processing takes place in a tasklet,
  the tasklet delay will add in to the total time.  It doesn't matter
  whether the tasklet handles only the giveback or some of the HCD's
  internal work as well.

 I am not sure if the idea of disabling EHCI irq inside ehci_irq()
 is good: one completed transfer may not be observed inside current
 tasklet handler since hardware interrupt is disabled, so the transfer
 can't be handled until next tasklet scheduled, then extra delay for
 these URBs is introduced.

 Even though interrupts are masked, the tasklet can still check the EHCI
 status register to see if new events have occurred while the tasklet
 was running, as I described above.  The tasklet could do this check
 before returning.

Yes, the tasklet can do it but some events(IAA, connection, fata error, ...)
are handled with delay.


  At least, EHCI HW generates the fatal irq ASAP, This interrupt is not
  delayed to the next interrupt threshold.(4.15.2.4), so it is reasonable 
  for
  software to handle it asap.
 
  That doesn't seem like a good reason.  (Probably it just reflects the
  fact that once a fatal error has occurred, the controller hardware
  can't reliably determine when the next interrupt threshold will occur.)
  But of course, it would be easy to put that part of the code into the
  top half of the handler.

 More things done in top half, the change will become more complicated
 since more synchronizations need to consider.

 Not at all.  ehci-lock will synchronize things perfectly well.

It isn't good to hold ehci-lock in ehci_irq(), otherwise, ehci_irq has to
spin on this lock when is held in tasklet.


  To put it another way, which would you prefer: To have three interrupts
  while the tasklet is running, or one interrupt as soon as it finishes?

 I prefer to enabling EHCI interrupt during tasklet handling.

 What for?  It seems likely that the top half would have 

[RFC v2] usb: Fix xHCI host issues on remote wakeup.

2013-08-26 Thread Sarah Sharp
When a device signals remote wakeup on a roothub, and the suspend change
bit is set, the host controller driver must not give control back to the
USB core until the port goes back into the active state.

EHCI accomplishes this by waiting in the get port status function until
the PORT_RESUME bit is cleared:

/* stop resume signaling */
temp = ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
ehci_writel(ehci, temp, status_reg);
clear_bit(wIndex, ehci-resuming_ports);
retval = ehci_handshake(ehci, status_reg,
PORT_RESUME, 0, 2000 /* 2msec */);

Similarly, the xHCI host should wait until the port goes into U0, before
passing control up to the USB core.  When the port transitions from the
RExit state to U0, the xHCI driver will get a port status change event.
We need to wait for that event before passing control up to the USB
core.

After the port transitions to the active state, the USB core should time
a recovery interval before it talks to the device.  The length of that
recovery interval is TRSMRCY, 10 ms, mentioned in the USB 2.0 spec,
section 7.1.7.7.  The previous xHCI code (which did not wait for the
port to go into U0) would cause the USB core to violate that recovery
interval.

This bug caused numerous USB device disconnects on remote wakeup under
ChromeOS and a Lynx Point LP xHCI host that takes up to 20 ms to move
from RExit to U0.  ChromeOS is very aggressive about power savings, and
sets the autosuspend_delay to 100 ms, and disables USB persist.

I attempted to replicate this bug with Ubuntu 12.04, but could not.  I
used Ubuntu 12.04 on the same platform, with the same BIOS that the bug
was triggered on ChromeOS with.  I also changed the USB sysfs settings
as described above, but still could not reproduce the bug under Ubuntu.
It may be that ChromeOS userspace triggers this bug through additional
settings.

This patch should be backported to kernels as old as 2.6.37, that
contain the commit 561925318725a41189a69f36ebe99199b3fb84c4 USB: xHCI:
port remote wakeup implementation.  It may be difficult to backport
this due to xhci-bus_state being unavailable in older kernels, and the
code to split xHCI roothubs into a USB 2.0 roothub and a USB 3.0 roothub
not going in until 2.6.39.

Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: sta...@vger.kernel.org
---
 drivers/usb/host/xhci-hub.c  | 37 +
 drivers/usb/host/xhci-mem.c  |  2 ++
 drivers/usb/host/xhci-ring.c | 13 +
 drivers/usb/host/xhci.h  | 10 ++
 4 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 187a3ec..b2fb67d 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -634,21 +634,42 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
goto error;
if (time_after_eq(jiffies,
bus_state-resume_done[wIndex])) {
+   int time_left;
+
xhci_dbg(xhci, Resume USB2 port %d\n,
wIndex + 1);
bus_state-resume_done[wIndex] = 0;
clear_bit(wIndex, bus_state-resuming_ports);
+
+   set_bit(wIndex, bus_state-rexit_ports);
xhci_set_link_state(xhci, port_array, wIndex,
XDEV_U0);
-   xhci_dbg(xhci, set port %d resume\n,
-   wIndex + 1);
-   slot_id = xhci_find_slot_id_by_port(hcd, xhci,
-wIndex + 1);
-   if (!slot_id) {
-   xhci_dbg(xhci, slot_id is zero\n);
-   goto error;
+
+   spin_unlock_irqrestore(xhci-lock, flags);
+   time_left = wait_for_completion_timeout(
+   bus_state-rexit_done[wIndex],
+   msecs_to_jiffies(
+   
XHCI_MAX_REXIT_TIMEOUT));
+   spin_lock_irqsave(xhci-lock, flags);
+
+   if (time_left) {
+   slot_id = xhci_find_slot_id_by_port(hcd,
+   xhci, wIndex + 1);
+   if (!slot_id) {
+   xhci_dbg(xhci, slot_id is 
zero\n);
+   goto error;
+ 

[GIT PATCH] USB fixe for 3.11-rc8

2013-08-26 Thread Greg KH
The following changes since commit d8dfad3876e438b759da3c833d62fb8b2267:

  Linux 3.11-rc7 (2013-08-25 17:43:22 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ 
tags/usb-3.11-rc8

for you to fetch changes up to d3474049ab6cfcf14274f5ab9f20c8f50b083eab:

  USB: OHCI: fix build error related to ohci_suspend/resume (2013-08-26 
15:22:15 -0700)


USB bugfix for 3.11-rc8

Here is a single bugfix that resolves the can not build the OHCI driver
with CONFIG_PM disabled problem that lots of people have been reporting
with 3.11-rc7.  Sorry about that one, it missed my build tests, and it
seems, a number of others as well.

Thank goodness for Guenter :)

Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org


Alan Stern (1):
  USB: OHCI: fix build error related to ohci_suspend/resume

 drivers/usb/host/ohci-pci.c | 2 ++
 1 file changed, 2 insertions(+)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT RFC] USB: Fix USB device disconnects on resume.

2013-08-26 Thread Alan Stern
On Mon, 26 Aug 2013, Sarah Sharp wrote:

 I double checked with the Intel Windows devs, and they say that Arjan is
 mistaken, and they'll get around 11 ms, just like we do.
 
 They say:
 
 The EHCI driver stack applies the 10 ms starting at the point that it
 sees C_PORT_SUSPEND asserted for the port.
 
 The inaccuracy in Windows 8 is generally no more than 1 system clock
 period late, but it can be delayed by interrupt processing for other
 devices. I would expect the additional average delay to be no more than
 1 ms.
 
 They didn't mention waiting for the PORT_RESUME bit to be cleared, like
 the Linux EHCI driver does.  Alan, would you like me to ask them about
 that?

Yes, please.  You might also ask them what they meant by it sees
C_PORT_SUSPEND asserted for the port, since there is no C_PORT_SUSPEND
bit in the EHCI Port Status and Control registers.  Were they talking 
about external hubs instead of the root hub?

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 v15 00/13] Add tested id switch and vbus connect detect support for Chipidea

2013-08-26 Thread Peter Chen
On Mon, Aug 26, 2013 at 06:24:48PM +0200, Hector Palacios wrote:
 Dear Peter,
 
 On 08/12/2013 03:22 AM, Peter Chen wrote:
 This patchset adds tested otg id switch function and vbus connect
 and disconnect detection for chipidea driver. And fix kinds of
 bugs found at chipidea drivers after enabling id and vbus detection.
 
 This patch are fully tested at imx6 sabresd and imx28evk platform by me.
 Besides, marek tested it on two STMP3780-based boards (not yet mainline)
 and two MX28-based boards.
 
 My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
 
 Changes for v15:
 - Mark otgsc API as static line, and move them to otg.h [4/13]
 - Remove some redundant comments [6/13]
 - Fix some typos [8/13]
 - Retire flag CI_HDRC_PULLUP_ON_VBUS [12/13] [13/13]
 
 Patches 6 and 12 in this series seem to be incomplete or malformed. Please 
 check.
 

Would you post what's the problem for them or what's problem
at your system after using them?

-- 

Best Regards,
Peter Chen

--
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