date:20130328

Michael Grzeschik m...@pengutronix.de writes:

 On Wed, Mar 27, 2013 at 10:24:04PM +0200, Alexander Shishkin wrote:
 Michael Grzeschik m...@pengutronix.de writes:
 
  On Wed, Mar 27, 2013 at 06:35:30PM +0200, Alexander Shishkin wrote:
  Michael Grzeschik m.grzesc...@pengutronix.de writes:
  
   Hi,
  
   this series solve some issues with the chipidea udc.
  
   The series is based on v3.9-rc4.
  
   Michael Grzeschik (6):
 usb: chipidea: udc: add attribute aligned(4) to shared memory structs
 usb: chipidea: udc: only clear active and halted bits in qhead
 usb: chipidea: udc: move ZLT flag change to ep_enable
 usb: chipidea: udc: read status of td only once in hardware_dequeue
 usb: chipidea: udc: don't truncate requests to single tds
 usb: chipidea: udc: move _ep_queue into an unlocked function
  
  Actually, do any of these patches fix tangible bugs, that can be
  reproduced? If so, you should certainly mention that. But somehow I doubt
  that, because otherwise the driver will be totally unusable.
  
  What I see is patches that bring some parts of udc code in accordance
  with the spec, which is a good thing to do, but doesn't fix any real
  bugs that bother people. Or does it?
 
  Obviously we were hunting a real issue while writing that patches. The
  most essential one is Patch 1/6, which definitely is a stable patch. I
  will clearify that in the patch description.
 
 Does it then fall in never worked before category, since it looks like
 it has never worked before on armv5 or did it work with older compilers?

 We used gcc-4.6.2 and gcc-4.7.2. In both cases we tested on the mx28 and
 the system got stuck after some transfers when using the following test:

 user@target$ getty /dev/ttyGS0
 user@host$ microcom -p /dev/ttyACM0

 Within the shell we started the following command:
 $(while true; do find /; done)

 At least the xscale version seems to be fine.

 Could you test the above with the xscale?

Just checked, gcc does generate a bunch of byte loads and stores on
xscale too (which makes sense, of course) for TD/QH accesses.
But since we don't support xscale in chipidea in mainline yet, it wasn't
an issue. It's different for imx, however. So now I'm convinced this
should go to stable.

 I will refer to another patch which is pretty similar
 and adressing in detail the same problamatic situation:

 http://www.spinics.net/lists/linux-usb/msg60768.html

 The only reason the fsl_udc_core driver does not
 have run into that, is that its shared structures
 are not marked with the attribute ((packed)).

  The others though should go into v3.10, agreed?

Waaait, go into v3.10 means those patches need to be based on my tree
and not v3.9-rc4. Those that are based on -rc4 are fixes for v3.9 and
are intending to go into v3.9-rcX. I see you just sent 8 patches based
on rc4 and only the first one should be.

 Agreed. What about the multi-td patch and the rest? Are you going to
 resend them still?

 I will clean up this series first and add the following patch to it:

 usb: chipidea: udc: prepare qhead with dma_alloc_coherent

 The rest are cleanup and feature patches which will come in seperate
 series, as Felipe suggested.

Felipe, since you have brought this up, which of these patches seem like
v3.9 fixes to you? They are all reasonable improvements, but I see no
reason sending them to v3.9, save for the first one.

Regards,
--
Alex
--
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 v8 1/8] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

Michael Grzeschik m.grzesc...@pengutronix.de writes:

 The udc uses an shared dma memory space between hard and software. This
 memory layout is described in ci13xxx_qh and ci13xxx_td which are marked
 with the attribute ((packed)).

 The compiler currently does not know about the alignment of the memory
 layout, and will create strb and ldrb operations.

 The Datasheet of the synopsys core describes, that some operations on
 the mapped memory need to be atomic double word operations. I.e. the
 next pointer addressing in the qhead, as otherwise the hardware will
 read wrong data and totally stuck.

 This is also possible while working with the current active td queue,
 and preparing the td-ptr.next in software while the hardware is still
 working with the current active td which is supposed to be changed:

 writeb(0xde, td-ptr.next + 0x0); /* strb */
 writeb(0xad, td-ptr.next + 0x1); /* strb */

 - hardware reads value of td-ptr.next and get stuck!

 writeb(0xbe, td-ptr.next + 0x2); /* strb */
 writeb(0xef, td-ptr.next + 0x3); /* strb */

 This appeares on armv5 machines where the hardware does not support
 unaligned 32bit operations.

 This patch adds the attribute ((aligned(4))) to the structures to tell
 the compiler to use 32bit operations. It also adds an wmb() for the
 prepared TD data before it gets enqueued into the qhead.

 Cc: stable sta...@vger.kernel.org
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Reviewed-by: Felipe Balbi ba...@ti.com

I would change the wording a bit so that the subject indicates that it's
a fix (like fix access width...) and that it says the important bit at
the top that on some architectures that don't do 32-bit unaligned
loads/stores such as armv5 the driver gets stuck. And you probably
should also mention for which kernels this is relevant, so that it's
more obvious that it's stable material.

That said,

Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb video capture issue due to uvc_complete callback spends more time

Hi Ravi,

On Thursday 28 March 2013 06:48:09 B, Ravi wrote:
  On Wednesday 27 March 2013 10:43:24 B, Ravi wrote:
  Hi
  
  I am observing issue while streaming video from usb camera connected to
  host controller based on mentor graphics. The issue is root caused that
  there are huge SOF gaps seen between the two isochronous IN token issued
  by host controller. This is due to fact, significant amount of time is
  spent in uvc callback function handler. Due to this next request
  programmed is delayed leads to this failure of video streaming. I have
  measured time taken by uvc callback function is in range minimum of 11
  microsecond to maximum of 3318 microsecond. Looks like callback handler
  doing some processing and takes more time to return rather than giveback
  immediately. Need your help to understand why uvc callback handler takes
  much time, if it does any processing can it move to different task
  context and return immediately, this will reduce the latency.
  
  I'm surprised by that large delay. A quick look at the UVC URB completion
  handler (I assume you're talking about the host driver, not the gadget
  driver)
  
  Thanks for reply, Yes, I am using the musb host controller driver
  (driver/usb/musb).
  
  doesn't show any significant source of delay. You will likely need to
  profile the code to find out where the problem comes from.
  
  I have profiled by adding timestamp before and after the
  usb_hcd_giveback_urb() @ drivers/usb/musb/musb_host.c and found the
  microsecond delay is ranging from 11usec to 3318usec while capturing video
  stream frames of resolution 640x480 from usb camera.
  
  static void musb_giveback(struct musb *musb, struct urb *urb, int status)
  
   __releases(musb-lock)
   __acquires(musb-lock)
   {
  
  +   struct timeval st, et, t;
  +   int is_isoc;
  .
  +   if (is_isoc)
  +   do_gettimeofday(st); /* get start time */
  
  usb_hcd_giveback_urb(musb_to_hcd(musb), urb, status);
  
  +   if (is_isoc) {
  +   do_gettimeofday(et); /* get end time */
  +   t.tv_sec = et.tv_sec - st.tv_sec;
  +   t.tv_usec = et.tv_usec - st.tv_usec;
  +   gtime[gIndex++ % MAX_LOG_TIME] = t; /* compute diff time
  */
  +   }
  
  Anything I miss here?
  
  You should also make sure that no other IRQ handler on the system keeps
  IRQs disabled for a long time.
 
 Some more debugging, Most of the time spend in stream-decode() function

That points to uvc_video_decode_isoc().

 in uvc_video_complete() callback handler.

It's not very surprising, but doesn't tell where the problem comes from. As 
Ming Lei pointed out, slow access to coherent memory might be an explanation. 
Could you find out how the time is spent between uvc_video_decode_start(), 
uvc_video_decode_data() and uvc_video_decode_data() ? It might also be worth 
it timing the uvc_queue_next_buffer() calls, in case the function is delayed 
by spinlock contention (if you're running on an SMP system).

-- 
Regards,

Laurent Pinchart

--
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: usb video capture issue due to uvc_complete callback spends more time

2013-03-28 Thread B, Ravi

Laurent

 On Thursday 28 March 2013 06:48:09 B, Ravi wrote:
   On Wednesday 27 March 2013 10:43:24 B, Ravi wrote:
   Hi
  
   I am observing issue while streaming video from usb camera connected
 to
   host controller based on mentor graphics. The issue is root caused
 that
   there are huge SOF gaps seen between the two isochronous IN token
 issued
   by host controller. This is due to fact, significant amount of time
 is
   spent in uvc callback function handler. Due to this next request
   programmed is delayed leads to this failure of video streaming. I
 have
   measured time taken by uvc callback function is in range minimum of
 11
   microsecond to maximum of 3318 microsecond. Looks like callback
 handler
   doing some processing and takes more time to return rather than
 giveback
   immediately. Need your help to understand why uvc callback handler
 takes
   much time, if it does any processing can it move to different task
   context and return immediately, this will reduce the latency.
  
   I'm surprised by that large delay. A quick look at the UVC URB
 completion
   handler (I assume you're talking about the host driver, not the
 gadget
   driver)
  
   Thanks for reply, Yes, I am using the musb host controller driver
   (driver/usb/musb).
  
   doesn't show any significant source of delay. You will likely need to
   profile the code to find out where the problem comes from.
  
   I have profiled by adding timestamp before and after the
   usb_hcd_giveback_urb() @ drivers/usb/musb/musb_host.c and found the
   microsecond delay is ranging from 11usec to 3318usec while capturing
 video
   stream frames of resolution 640x480 from usb camera.
  
   static void musb_giveback(struct musb *musb, struct urb *urb, int
 status)
  
__releases(musb-lock)
__acquires(musb-lock)
{
  
   +   struct timeval st, et, t;
   +   int is_isoc;
   .
   +   if (is_isoc)
   +   do_gettimeofday(st); /* get start time */
  
   usb_hcd_giveback_urb(musb_to_hcd(musb), urb, status);
  
   +   if (is_isoc) {
   +   do_gettimeofday(et); /* get end time */
   +   t.tv_sec = et.tv_sec - st.tv_sec;
   +   t.tv_usec = et.tv_usec - st.tv_usec;
   +   gtime[gIndex++ % MAX_LOG_TIME] = t; /* compute diff
 time
   */
   +   }
  
   Anything I miss here?
  
   You should also make sure that no other IRQ handler on the system
 keeps
   IRQs disabled for a long time.
 
  Some more debugging, Most of the time spend in stream-decode() function
 
 That points to uvc_video_decode_isoc().

You are correct, that points to uvc_video_decode_isoc()

 
  in uvc_video_complete() callback handler.
 
 It's not very surprising, but doesn't tell where the problem comes from.
 As
 Ming Lei pointed out, slow access to coherent memory might be an
 explanation.
 Could you find out how the time is spent between uvc_video_decode_start(),
 uvc_video_decode_data() and uvc_video_decode_data() ? It might also be
 worth
 it timing the uvc_queue_next_buffer() calls, in case the function is
 delayed
 by spinlock contention (if you're running on an SMP system).

I did not dig further, I try to get this timing info.   

Quickly I tried another experiment, instead of calling stream-decode() in 
callback, initiated work thread, which perform stream-decode()  re-submit 
urb. This reduces the uvc_video_callback() time to 12usec, But after 900 urb 
completion, submit_urb() failed with -EBUSY error. Further I stopped debugging, 
something I may not be doing right at uvc level.

Thanks Laurent.

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

Re: [PATCH v5 06/12] usb: chipidea: udc: configure iso endpoints

2013-03-28 Thread Peter Chen

On Tue, Mar 26, 2013 at 07:49:07PM +0200, Felipe Balbi wrote:
 Hi,
 
 On Tue, Mar 26, 2013 at 05:58:42PM +0100, Michael Grzeschik wrote:
  The implementation is derived from the fsl_udc_core code in
  fsl_ep_enable and makes basic iso handling possible.
  
  Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
  Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
  ---
  Changes since v4:
   - rebased on the new alignment patch
   - changed indention to tabs
   - removed spare brakets
   - added special handling for iso in ep_queue and ep_halt
   - changed TODO list entry in core.c
  Changes since v3:
   - added QH_ISO_TRANS macro
   - removed unused operations mentioned by Peter
  Changes since v2:
   - fixed usage of variable max
   - reworked on writel/readl patches
  Changes since v1:
   - fixed coding style issues mentioned by Sergei
  
   drivers/usb/chipidea/core.c |  2 +-
   drivers/usb/chipidea/udc.c  | 15 ---
   drivers/usb/chipidea/udc.h  |  1 +
   3 files changed, 14 insertions(+), 4 deletions(-)
  
  diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
  index 47b8da2..82de38b 100644
  --- a/drivers/usb/chipidea/core.c
  +++ b/drivers/usb/chipidea/core.c
  @@ -43,7 +43,7 @@
*
* TODO List
* - OTG
  - * - Isochronous  Interrupt Traffic
  + * - Interrupt Traffic
* - Handle requests which spawns into several TDs
* - GET_STATUS(device) - always reports 0
* - Gadget API (majority of optional features)
  diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
  index 567ce89..cab349b 100644
  --- a/drivers/usb/chipidea/udc.c
  +++ b/drivers/usb/chipidea/udc.c
  @@ -1010,6 +1010,8 @@ static int ep_enable(struct usb_ep *ep,
  unsigned long flags;
  u32 val = 0;
   
  +   unsigned short max;
  +
  if (ep == NULL || desc == NULL)
  return -EINVAL;
   
  @@ -1026,19 +1028,19 @@ static int ep_enable(struct usb_ep *ep,
  mEp-num  = usb_endpoint_num(desc);
  mEp-type = usb_endpoint_type(desc);
   
  -   mEp-ep.maxpacket = usb_endpoint_maxp(desc);
  +   max = mEp-ep.maxpacket = usb_endpoint_maxp(desc);
   
  trace_ci_ep_enable(mEp, 0);
   
  if (mEp-type == USB_ENDPOINT_XFER_CONTROL)
  val |= QH_IOS;
  else if (mEp-type == USB_ENDPOINT_XFER_ISOC)
  -   val = ~QH_MULT;
  +   val |= QH_ISO_TRANS(max)  __ffs(QH_MULT);
   
  if (mEp-num)
  val |= QH_ZLT;
   
  -   val |= (mEp-ep.maxpacket  __ffs(QH_MAX_PKT))  QH_MAX_PKT;
  +   val |= (max  __ffs(QH_MAX_PKT))  QH_MAX_PKT;
  mEp-qh.ptr-cap = val;
   
  mEp-qh.ptr-td.next |= TD_TERMINATE;   /* needed? */
  @@ -1182,6 +1184,10 @@ static int ep_queue(struct usb_ep *ep, struct 
  usb_request *req,
  }
  }
   
  +   if (usb_endpoint_xfer_isoc(mEp-ep.desc)
  +mReq-req.length  mEp-ep.maxpacket)
  +   return -EMSGSIZE;
 
 this HW really can't handle requests greater max packet size for
 isochronous ?

I re-check the spec, the calculate for Mult and max packet size are both wrong.
Maximum Packet Length is = 1024
Mult's calculate should according to req.length and max packet size,
eg, if req.length is 2060, and max packet size is 1024.
then, the mult is 1 + 2060/1024 = 3. And the Total Bytes is 2060 at dTD.
The check at ep_queue should like below:

if (usb_endpoint_xfer_isoc(mEp-ep.desc) {
 (mReq-req.length  3 * mEp-ep.maxpacket)
return -EMSGSIZE;

-- 

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

Re: [PATCH V5 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

Hi Bhupesh,

Thanks for the patch. We're almost there :-)

On Thursday 28 March 2013 10:39:13 Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework for
 video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from
 the V4L2 domain to UVC domain via a user-space application), we need to
 support USER_PTR IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAP IO
 method and now the user-space application can just pass a pointer to the
 video buffers being dequeued from the V4L2 device side while queueing
 them at the UVC gadget end. This ensures that we have a zero-copy
 design as the videobuffers pass from the V4L2 capture device to the UVC
 gadget.
 
 Note that there will still be a need to apply UVC specific payload
 headers on top of each UVC payload data, which will still require a copy
 operation to be performed in the 'encode' routines of the UVC gadget.
 
 This patch also addresses one issue found out while porting the UVC
 gadget to videobuf2 framework:
   - In case the usb requests queued by the gadget get completed
 with a status of -ESHUTDOWN (disconnected from host),
 the queue of videobuf2 should be cancelled to ensure that the
 application space daemon is not left in a state waiting for
 a vb2 to be successfully absorbed at the USB side.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

[snip]

 diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
 index 104ae9c..0d30171 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c

[snip]


 +static int uvc_queue_buffer(struct uvc_video_queue *queue,
 + struct v4l2_buffer *buf)
 +{
 + unsigned long flags;
 + int ret;
 
 + mutex_lock(queue-mutex);
   spin_lock_irqsave(queue-irqlock, flags);
 + ret = vb2_qbuf(queue-queue, buf);

vb2_qbuf() must be called before spin_lock_irqsave(), as the 
uvc_buffer_queue() operation takes the same lock. The spin lock must thus 
protect the two lines below only.

   ret = (queue-flags  UVC_QUEUE_PAUSED) != 0;
   queue-flags = ~UVC_QUEUE_PAUSED;
   spin_unlock_irqrestore(queue-irqlock, flags);
   mutex_unlock(queue-mutex);

 + return ret;
  }

-- 
Regards,

Laurent Pinchart

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

RE: [PATCH V5 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2013-03-28 Thread Bhupesh SHARMA

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Thursday, March 28, 2013 2:35 PM
 To: Bhupesh SHARMA
 Cc: linux-usb@vger.kernel.org; ba...@ti.com; bhupesh.li...@gmail.com;
 m...@pengutronix.de; gang.c...@asianux.com; tipec...@gmail.com
 Subject: Re: [PATCH V5 1/2] usb: gadget/uvc: Port UVC webcam gadget to
 use videobuf2 framework

 Hi Bhupesh,

 Thanks for the patch. We're almost there :-)

 On Thursday 28 March 2013 10:39:13 Bhupesh Sharma wrote:
  This patch reworks the videobuffer management logic present in the UVC
  webcam gadget and ports it to use the more apt videobuf2 framework
  for video buffer management.

  To support routing video data captured from a real V4L2 video capture
  device with a zero copy operation on videobuffers (as they pass from
  the V4L2 domain to UVC domain via a user-space application), we need
  to support USER_PTR IO method at the UVC gadget side.

  So the V4L2 capture device driver can still continue to use MMAP IO
  method and now the user-space application can just pass a pointer to
  the video buffers being dequeued from the V4L2 device side while
  queueing them at the UVC gadget end. This ensures that we have a zero-
 copy
  design as the videobuffers pass from the V4L2 capture device to the
  UVC gadget.

  Note that there will still be a need to apply UVC specific payload
  headers on top of each UVC payload data, which will still require a
  copy operation to be performed in the 'encode' routines of the UVC
 gadget.

  This patch also addresses one issue found out while porting the UVC
  gadget to videobuf2 framework:
  - In case the usb requests queued by the gadget get completed
with a status of -ESHUTDOWN (disconnected from host),
the queue of videobuf2 should be cancelled to ensure that the
application space daemon is not left in a state waiting for
a vb2 to be successfully absorbed at the USB side.

  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

 [snip]

  diff --git a/drivers/usb/gadget/uvc_queue.c
  b/drivers/usb/gadget/uvc_queue.c index 104ae9c..0d30171 100644
  --- a/drivers/usb/gadget/uvc_queue.c
  +++ b/drivers/usb/gadget/uvc_queue.c

 [snip]

  +static int uvc_queue_buffer(struct uvc_video_queue *queue,
  +   struct v4l2_buffer *buf)
  +{
  +   unsigned long flags;
  +   int ret;

  +   mutex_lock(queue-mutex);
  spin_lock_irqsave(queue-irqlock, flags);
  +   ret = vb2_qbuf(queue-queue, buf);

 vb2_qbuf() must be called before spin_lock_irqsave(), as the
 uvc_buffer_queue() operation takes the same lock. The spin lock must thus
 protect the two lines below only.

Ok. V6 will fix this issue.

Regards,
Bhupesh

  ret = (queue-flags  UVC_QUEUE_PAUSED) != 0;
  queue-flags = ~UVC_QUEUE_PAUSED;
  spin_unlock_irqrestore(queue-irqlock, flags);
  mutex_unlock(queue-mutex);

  +   return ret;
   }

 --
 Regards,

 Laurent Pinchart

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

Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

On Thu, Mar 28, 2013 at 08:55:07AM +0200, Alexander Shishkin wrote:
  Agreed. What about the multi-td patch and the rest? Are you going to
  resend them still?
 
  I will clean up this series first and add the following patch to it:
 
  usb: chipidea: udc: prepare qhead with dma_alloc_coherent
 
  The rest are cleanup and feature patches which will come in seperate
  series, as Felipe suggested.
 
 Felipe, since you have brought this up, which of these patches seem like
 v3.9 fixes to you? They are all reasonable improvements, but I see no
 reason sending them to v3.9, save for the first one.

right. Looks like except patch 1, they should all go to v3.10

-- 
balbi


signature.asc
Description: Digital signature

Re: Renesas sparse errors

Hi,

On Thu, Mar 28, 2013 at 11:18:11AM +0200, Felipe Balbi wrote:
 Hi,
 
 On Wed, Mar 27, 2013 at 06:21:19PM -0700, Kuninori Morimoto wrote:
   linux/drivers/usb/renesas_usbhs/common.c:313:17: error: incompatible 
   types in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:322:17: error: incompatible 
   types in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:384:17: error: incompatible 
   types in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:524:9: error: incompatible types 
   in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:545:9: error: incompatible types 
   in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:574:9: error: incompatible types 
   in conditional expression (different base types)
   linux/drivers/usb/renesas_usbhs/common.c:606:9: error: incompatible types 
   in conditional expression (different base types)
   
   Could you look into fixing them for v3.10 or v3.11 ? That would be
   great as it would make my build-testing scripts a lot happier :-p
  
  Hmmm... strange...
  
  I can't get this error.
  I tried x86(32bit/64bit)/sh/arm compiler on your next and master branch.
  
  1b0563f888d14f877ef0b5602ba240f3e857df06
  (Merge branch 'next')
  
  6b0cfc656f8a649fbfbe11e76e0aa301ee26879e
  (usb: musb: ux500_dma: fix sparse warning)
  
  my all result are..
  
...
CC  drivers/usb/renesas_usbhs/common.o
CC  drivers/usb/renesas_usbhs/mod.o
CC  drivers/usb/renesas_usbhs/pipe.o
CC  drivers/usb/renesas_usbhs/fifo.o
CC  drivers/usb/renesas_usbhs/mod_host.o
LD  drivers/usb/renesas_usbhs/renesas_usbhs.o
LD  drivers/usb/renesas_usbhs/built-in.o
 
 I don't see the CHECKs here, you need to ask sparse to run :-) try make
 C=2. Please be sure 'sparse' is in your PATH.

should look like this:

  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/common.c
  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/mod.c
  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/pipe.c
  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/fifo.c
  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/mod_host.c
  CHECK   /home/balbi/workspace/linux/drivers/usb/renesas_usbhs/mod_gadget.c
  CC  drivers/usb/renesas_usbhs/mod.o
  CC  drivers/usb/renesas_usbhs/pipe.o
  CC  drivers/usb/renesas_usbhs/fifo.o
drivers/usb/renesas_usbhs/mod_gadget.c:233:28: warning: symbol 
'req_clear_feature' was not declared. Should it be static?
drivers/usb/renesas_usbhs/mod_gadget.c:274:28: warning: symbol 
'req_set_feature' was not declared. Should it be static?
drivers/usb/renesas_usbhs/mod_gadget.c:375:28: warning: symbol 'req_get_status' 
was not declared. Should it be static?
drivers/usb/renesas_usbhs/common.c:313:17: error: incompatible types in 
conditional expression (different base types)
  CC  drivers/usb/renesas_usbhs/mod_gadget.o
drivers/usb/renesas_usbhs/common.c:322:17: error: incompatible types in 
conditional expression (different base types)
drivers/usb/renesas_usbhs/common.c:384:17: error: incompatible types in 
conditional expression (different base types)
drivers/usb/renesas_usbhs/common.c:524:9: error: incompatible types in 
conditional expression (different base types)
drivers/usb/renesas_usbhs/common.c:545:9: error: incompatible types in 
conditional expression (different base types)
drivers/usb/renesas_usbhs/common.c:574:9: error: incompatible types in 
conditional expression (different base types)
drivers/usb/renesas_usbhs/common.c:606:9: error: incompatible types in 
conditional expression (different base types)
  CC  drivers/usb/renesas_usbhs/common.o
  CC  drivers/usb/renesas_usbhs/mod_host.o
  LD  drivers/usb/renesas_usbhs/renesas_usbhs.o
  LD  drivers/usb/renesas_usbhs/built-in.o

(errors aren't following CHECKs because I run 18 build jobs)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

Svetoslav Neykov svetos...@neykov.name writes:

 Convert between big-endian and little-endian format when accessing the usb 
 controller structures which are little-endian by specification.
 Fix cases where the little-endian memory layout is taken for granted.
 The patch doesn't have any effect on the already supported
 little-endian architectures.

Applied to my branch of things that are aiming at v3.10. Next time
please make sure that it applies cleanly.

 (no changes since last version)

No need to mention this here, you can use cover letter and/or below the
diffstat, so that it's not part of the commit message.

Thanks,
--
Alex
--
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/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-28 Thread Matthijs Kooijman

Hi Paul,

 Overall this seems to be an improvement, so I have no objections. There
 is a pretty high likelihood of something getting broken in the translation,
 however.
 
 For example, this:
 + hw-host_nperio_tx_fifo_size = (readl(hsotg-regs + GNPTXFSIZ)  
 GNPTXFSIZ_NP_TXF_DEP_MASK)  GNPTXFSIZ_NP_TXF_DEP_SHIFT;
 looks like a bug (shifted the wrong direction).
Yeah, I'll make sure to doublecheck the patch when it is finished. I
already found the above one after I sent over the patch, but since I was
just looking to collect feedback on the basic idea behind the patch, I
hadn't thorougly checked it before sending it.

 You probably should use u32 instead of int for the members of struct
 dwc2_hw_params, otherwise you might introduce a sign bug if any of the
 values get shifted into the high bit and become unexpectedly negative. And
 I would use either u8, bool, or bitfields for the true/false values, to
 save a little space.
I don't think any of the values will exceed 2^31, but using more
appropriate types makes sense. I'll see about using properly sized
bitfields to minimize the memory overhead.

Gr.

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

Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

On Thu, Mar 28, 2013 at 11:12:51AM +0200, Felipe Balbi wrote:
 On Thu, Mar 28, 2013 at 08:55:07AM +0200, Alexander Shishkin wrote:
   Agreed. What about the multi-td patch and the rest? Are you going to
   resend them still?
  
   I will clean up this series first and add the following patch to it:
  
   usb: chipidea: udc: prepare qhead with dma_alloc_coherent
  
   The rest are cleanup and feature patches which will come in seperate
   series, as Felipe suggested.
  
  Felipe, since you have brought this up, which of these patches seem like
  v3.9 fixes to you? They are all reasonable improvements, but I see no
  reason sending them to v3.9, save for the first one.
 
 right. Looks like except patch 1, they should all go to v3.10

What about [PATCH v8 7/8] usb: chipidea: udc: fix possible memory leak in 
_ep_nuke ?
I added the Cc: stable sta...@vger.kernel.org on purpose.

Worth it?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V6 0/2] UVC webcam gadget related changes

2013-03-28 Thread Bhupesh Sharma

This patchset tries to enhance the UVC webcam gadget driver and is based on
Laurent's git tree available here (head uvc-gadget):
git://linuxtv.org/pinchartl/uvcvideo.git

Note that to ease review and integration of these patches, I have rebased them
on Laurent's repo and all the relevant patches after review can be pushed in
Felipe's repo in one go.

The patch 1/2 in this patchset tries to handle all the review comments
received on the V5 of the following UVC gadget related patch:

[PATCH V5 1/2] usb: gadget/uvc: Port UVC webcam gadget to use
videobuf2 framework

which can be viewed here:
[1] http://www.spinics.net/lists/linux-usb/msg83430.html

The patch 2/2 in this patchset has already received an Acked-By from
Laurent.

I have tested this patchset on a super-speed compliant USB device controller
(DWC3), with the VIVI capture device acting as a dummy source of video data and
I also have modified the 'uvc-gadget' application written by Laurent
(original application available here: 
http://git.ideasonboard.org/uvc-gadget.git)
for testing the complete flow from V4L2 to UVC domain and vice versa.

I will send a patch for the modified 'uvc-gadget' application in a separate
mail thread.

Changes since V5:
- Addresses the review comments received on V5 of this patchset from Laurent.

- The changes suggested by Laurent and Alan to remove WARN_ON messages
  from the UDC controller drivers in case the gadget tries to dequeue a USB
  request which was never queued to the UDC controller, will be floated
  as a separate patch.

Bhupesh Sharma (2):
  usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework
  usb: gadget/uvc: Add support for 'get_unmapped_area' for MMUless
architectures

 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/uvc_queue.c |  538 +---
 drivers/usb/gadget/uvc_queue.h |   32 +--
 drivers/usb/gadget/uvc_v4l2.c  |   48 ++--
 drivers/usb/gadget/uvc_video.c |   17 +-
 5 files changed, 218 insertions(+), 418 deletions(-)

-- 
1.7.2.2

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

[PATCH V6 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2013-03-28 Thread Bhupesh Sharma

This patch reworks the videobuffer management logic present in the UVC
webcam gadget and ports it to use the more apt videobuf2 framework for
video buffer management.

To support routing video data captured from a real V4L2 video capture
device with a zero copy operation on videobuffers (as they pass from
the V4L2 domain to UVC domain via a user-space application), we need to
support USER_PTR IO method at the UVC gadget side.

So the V4L2 capture device driver can still continue to use MMAP IO
method and now the user-space application can just pass a pointer to the
video buffers being dequeued from the V4L2 device side while queueing
them at the UVC gadget end. This ensures that we have a zero-copy
design as the videobuffers pass from the V4L2 capture device to the UVC
gadget.

Note that there will still be a need to apply UVC specific payload
headers on top of each UVC payload data, which will still require a copy
operation to be performed in the 'encode' routines of the UVC gadget.

This patch also addresses one issue found out while porting the UVC
gadget to videobuf2 framework:
- In case the usb requests queued by the gadget get completed
  with a status of -ESHUTDOWN (disconnected from host),
  the queue of videobuf2 should be cancelled to ensure that the
  application space daemon is not left in a state waiting for
  a vb2 to be successfully absorbed at the USB side.

Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
---
 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/uvc_queue.c |  532 
 drivers/usb/gadget/uvc_queue.h |   32 +--
 drivers/usb/gadget/uvc_v4l2.c  |   37 +--
 drivers/usb/gadget/uvc_video.c |   17 +-
 5 files changed, 193 insertions(+), 426 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 14625fd..3186967 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -928,6 +928,7 @@ config USB_G_WEBCAM
tristate USB Webcam Gadget
depends on VIDEO_DEV
select USB_LIBCOMPOSITE
+   select VIDEOBUF2_VMALLOC
help
  The Webcam Gadget acts as a composite USB Audio and Video Class
  device. It provides a userspace API to process UVC control requests
diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
index 104ae9c..3139795 100644
--- a/drivers/usb/gadget/uvc_queue.c
+++ b/drivers/usb/gadget/uvc_queue.c
@@ -10,6 +10,7 @@
  * (at your option) any later version.
  */
 
+#include linux/atomic.h
 #include linux/kernel.h
 #include linux/mm.h
 #include linux/list.h
@@ -18,7 +19,8 @@
 #include linux/videodev2.h
 #include linux/vmalloc.h
 #include linux/wait.h
-#include linux/atomic.h
+
+#include media/videobuf2-vmalloc.h
 
 #include uvc.h
 
@@ -28,330 +30,175 @@
  * Video queues is initialized by uvc_queue_init(). The function performs
  * basic initialization of the uvc_video_queue struct and never fails.
  *
- * Video buffer allocation and freeing are performed by uvc_alloc_buffers and
- * uvc_free_buffers respectively. The former acquires the video queue lock,
- * while the later must be called with the lock held (so that allocation can
- * free previously allocated buffers). Trying to free buffers that are mapped
- * to user space will return -EBUSY.
- *
- * Video buffers are managed using two queues. However, unlike most USB video
- * drivers that use an in queue and an out queue, we use a main queue to hold
- * all queued buffers (both 'empty' and 'done' buffers), and an irq queue to
- * hold empty buffers. This design (copied from video-buf) minimizes locking
- * in interrupt, as only one queue is shared between interrupt and user
- * contexts.
- *
- * Use cases
- * -
- *
- * Unless stated otherwise, all operations that modify the irq buffers queue
- * are protected by the irq spinlock.
- *
- * 1. The user queues the buffers, starts streaming and dequeues a buffer.
- *
- *The buffers are added to the main and irq queues. Both operations are
- *protected by the queue lock, and the later is protected by the irq
- *spinlock as well.
- *
- *The completion handler fetches a buffer from the irq queue and fills it
- *with video data. If no buffer is available (irq queue empty), the handler
- *returns immediately.
- *
- *When the buffer is full, the completion handler removes it from the irq
- *queue, marks it as ready (UVC_BUF_STATE_DONE) and wakes its wait queue.
- *At that point, any process waiting on the buffer will be woken up. If a
- *process tries to dequeue a buffer after it has been marked ready, the
- *dequeing will succeed immediately.
- *
- * 2. Buffers are queued, user is waiting on a buffer and the device gets
- *disconnected.
- *
- *When the device is disconnected, the kernel calls the completion handler
- *with an appropriate status code. The handler marks all buffers in the
- *irq queue as being

[PATCH V6 2/2] usb: gadget/uvc: Add support for 'get_unmapped_area' for MMUless architectures

2013-03-28 Thread Bhupesh Sharma

This patch adds the support for 'get_unmapped_area' in UVC gadget
which is called when the 'mmap' system call is executed on MMUless
architectures.

Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
 drivers/usb/gadget/uvc_queue.c |   18 ++
 drivers/usb/gadget/uvc_v4l2.c  |   15 +++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
index 3139795..7ce27e3 100644
--- a/drivers/usb/gadget/uvc_queue.c
+++ b/drivers/usb/gadget/uvc_queue.c
@@ -232,6 +232,24 @@ static int uvc_queue_mmap(struct uvc_video_queue *queue,
return ret;
 }
 
+#ifndef CONFIG_MMU
+/*
+ * Get unmapped area.
+ *
+ * NO-MMU arch need this function to make mmap() work correctly.
+ */
+static unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
+   unsigned long pgoff)
+{
+   unsigned long ret;
+
+   mutex_lock(queue-mutex);
+   ret = vb2_get_unmapped_area(queue-queue, 0, 0, pgoff, 0);
+   mutex_unlock(queue-mutex);
+   return ret;
+}
+#endif
+
 /*
  * Cancel the video buffers queue.
  *
diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
index 7d91740..b83d0ad 100644
--- a/drivers/usb/gadget/uvc_v4l2.c
+++ b/drivers/usb/gadget/uvc_v4l2.c
@@ -340,6 +340,18 @@ uvc_v4l2_poll(struct file *file, poll_table *wait)
return uvc_queue_poll(uvc-video.queue, file, wait);
 }
 
+#ifndef CONFIG_MMU
+static unsigned long uvc_v4l2_get_unmapped_area(struct file *file,
+   unsigned long addr, unsigned long len, unsigned long pgoff,
+   unsigned long flags)
+{
+   struct video_device *vdev = video_devdata(file);
+   struct uvc_device *uvc = video_get_drvdata(vdev);
+
+   return uvc_queue_get_unmapped_area(uvc-video.queue, pgoff);
+}
+#endif
+
 static struct v4l2_file_operations uvc_v4l2_fops = {
.owner  = THIS_MODULE,
.open   = uvc_v4l2_open,
@@ -347,5 +359,8 @@ static struct v4l2_file_operations uvc_v4l2_fops = {
.ioctl  = uvc_v4l2_ioctl,
.mmap   = uvc_v4l2_mmap,
.poll   = uvc_v4l2_poll,
+#ifndef CONFIG_MMU
+   .get_unmapped_area = uvc_v4l2_get_unmapped_area,
+#endif
 };
 
-- 
1.7.2.2

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

Re: [PATCH V6 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

Hi Bhupesh,

Thanks for the patch.

On Thursday 28 March 2013 15:11:52 Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework for
 video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from
 the V4L2 domain to UVC domain via a user-space application), we need to
 support USER_PTR IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAP IO
 method and now the user-space application can just pass a pointer to the
 video buffers being dequeued from the V4L2 device side while queueing
 them at the UVC gadget end. This ensures that we have a zero-copy
 design as the videobuffers pass from the V4L2 capture device to the UVC
 gadget.
 
 Note that there will still be a need to apply UVC specific payload
 headers on top of each UVC payload data, which will still require a copy
 operation to be performed in the 'encode' routines of the UVC gadget.
 
 This patch also addresses one issue found out while porting the UVC
 gadget to videobuf2 framework:
   - In case the usb requests queued by the gadget get completed
 with a status of -ESHUTDOWN (disconnected from host),
 the queue of videobuf2 should be cancelled to ensure that the
 application space daemon is not left in a state waiting for
 a vb2 to be successfully absorbed at the USB side.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

Finally,

Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

:-)

-- 
Regards,

Laurent Pinchart

--
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 v8 7/8] usb: chipidea: udc: fix possible memory leak in _ep_nuke

Michael Grzeschik m.grzesc...@pengutronix.de writes:

 In hardware_enqueue code adds one extra td with dma_pool_alloc if
 mReq-req.zero is true. When _ep_nuke will be called for that endpoint,
 dma_pool_free will not be called to free that memory again. That patch
 fixes this.

Okay, drop the possible from subject and you're good to go.

Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com


 Cc: stable sta...@vger.kernel.org
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 ---
  drivers/usb/chipidea/udc.c | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
 index b40c259..a0a0a64 100644
 --- a/drivers/usb/chipidea/udc.c
 +++ b/drivers/usb/chipidea/udc.c
 @@ -564,6 +564,12 @@ __acquires(mEp-lock)
   struct ci13xxx_req *mReq = \
   list_entry(mEp-qh.queue.next,
  struct ci13xxx_req, queue);
 +
 + if (mReq-zptr) {
 + dma_pool_free(mEp-td_pool, mReq-zptr, mReq-zdma);
 + mReq-zptr = NULL;
 + }
 +
   list_del_init(mReq-queue);
   mReq-req.status = -ESHUTDOWN;
  
 -- 
 1.8.2.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: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

Michael Grzeschik m...@pengutronix.de writes:

 On Thu, Mar 28, 2013 at 11:12:51AM +0200, Felipe Balbi wrote:
 On Thu, Mar 28, 2013 at 08:55:07AM +0200, Alexander Shishkin wrote:
   Agreed. What about the multi-td patch and the rest? Are you going to
   resend them still?
  
   I will clean up this series first and add the following patch to it:
  
   usb: chipidea: udc: prepare qhead with dma_alloc_coherent
  
   The rest are cleanup and feature patches which will come in seperate
   series, as Felipe suggested.
  
  Felipe, since you have brought this up, which of these patches seem like
  v3.9 fixes to you? They are all reasonable improvements, but I see no
  reason sending them to v3.9, save for the first one.
 
 right. Looks like except patch 1, they should all go to v3.10

 What about [PATCH v8 7/8] usb: chipidea: udc: fix possible memory leak in 
 _ep_nuke ?
 I added the Cc: stable sta...@vger.kernel.org on purpose.

 Worth it?

Looks like a good cadidate for stable as well. I was mislead by
possible in the subject when scanning through the patchset. It's a
very real memory leak.

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

[PATCH 0/9] Equivalent of g_serial.ko with configfs

Dear All,

I have changed the way of aiming at a configurable configfs-based gadget.

After Sebastian has laid the foundation for the conversion  I thought of
doing the conversion to configfs like this: 1) convert the functions to
the new function interface from Sebastian, 2) add configfs support.

I have submitted more than 50 patches which did the step 1) for all functions
but hid and multimedia. The patches are here:

http://www.spinics.net/lists/linux-usb/msg82418.html
http://www.spinics.net/lists/linux-usb/msg83094.html

For your convenience I have set up a branch which contains all of them:

git://git.infradead.org/users/kmpark/linux-samsung usb-gadgdet-configfs-old

But now I think it is better to think of the conversion having the
perspective of old gadgets in mind. So the process would be like this:
provide the equivalent of g_serial.ko, provide the equivalent of
usb ethernet gadgets, provide the equivalent of g_multi.ko and so on.

Here I present the conversion of everthing that is required to provide
the equivalent of g_serial.ko with configfs.

A branch will be available here (from 29th March 2013):

git://git.infradead.org/users/kmpark/linux-samsung usb-gadget-configfs


BACKWARD COMPATIBILITY
==

Please note that the old g_serial.ko is still available and works.


USING THE NEW GADGET
==

Please refer to this post:

http://www.spinics.net/lists/linux-usb/msg76388.html

for general information from Sebastian on how to use configfs-based
gadgets (*).

Here is the description specific to using g_serial.ko equivalent.

The old g_serial.ko during runtime provides exclusively one of the
following functions:

- acm
- serial
- obex

The selection is based on the module parameters use_acm and use_obex.
By default acm is used, if use_acm is false then serial is used unless
use_obex is true.

There is also an n_ports parameter, which controls how many ports
will be created for the actually used function.

With configfs the procedure is as follows, compared to the information
mentioned above (*):

instead of mkdir functions/acm.ttyS1 do

mkdir functions/acm|gser|obex.instance name

e.g. mkdir functions/obex.tty1

To achieve the equivalent of n_ports  1 repeat the above step
the desired number of times, each time using a new instance name.

The rest of the procedure (*) remains the same.

After unbinding the gadget with echo   UDC
the symbolic links in the configuration directory can be removed,
the strings/* subdirectories in the configuration directory can
be removed, the strings/* subdirectories at the gadget level can
be removed and the configs/* subdirectories can be removed.
The functions/* subdirectories can be removed.
After that the gadget directory can be removed.
After that the respective modules can be unloaded.


TESTING THE FUNCTIONS


acm)

On host: cat  /dev/ttyACMX
On target: cat /dev/ttyGSY

then the other way round

On target: cat  /dev/ttyGSY
On host: cat /dev/ttyACMX

serial)

On host: insmod usbserial vendor=vendorID product=productID
On host: cat  /dev/ttyUSBX
On target: cat /dev/ttyGSY

then the other way round

On target: cat  /dev/ttyGSY
On host: cat /dev/ttyUSBX

obex)

On target: seriald -f /dev/ttyGSY -s 1024
On host: serialc -v vendorID -p productID -iinterface# -a1 -s1024 \
 -tout endpoint addr -rin endpoint addr

where seriald and serialc are Felipe's utilities found here:

https://git.gitorious.org/usb/usb-tools.git master


This patch series depends on the following patches:

http://www.spinics.net/lists/linux-usb/msg78752.html
usb/gadget: nokia: use function framework for ACM

http://www.spinics.net/lists/linux-usb/msg78733.html
usb/gadget: move the global the_dev variable to their users

http://www.spinics.net/lists/linux-usb/msg78734.html
usb/gadget: push tty port allocation from gadget into f_acm

http://www.spinics.net/lists/linux-usb/msg76388.html
usb/gadget: the start of the configfs interface


Andrzej Pietrasiewicz (9):
  usb/gadget: use consistent naming scheme for usb function modules
  usb/gadget: nokia: remove unused include
  usb/gadget: f_serial: convert to new function interface with backward
compatibility
  usb/gadget: serial: convert to new interface of f_serial
  usb/gadget: f_serial: remove compatibility layer
  usb/gadget: f_serial: add configfs support
  usb/gadget: f_obex: convert to new function interface with backward
compatibility
  usb/gadget: serial: convert to new interface of f_obex
  usb/gadget: f_obex: add configfs support

 drivers/usb/gadget/Kconfig|8 ++
 drivers/usb/gadget/Makefile   |   11 ++-
 drivers/usb/gadget/f_obex.c   |  226 -
 drivers/usb/gadget/f_serial.c |  174 +++
 drivers/usb/gadget/nokia.c|3 +-
 drivers/usb/gadget/serial.c   |   59 ++--
 6 files changed, 334 insertions(+), 147 deletions(-)

--
To unsubscribe from this list: send the line unsubscribe

[PATCH 6/9] usb/gadget: f_serial: add configfs support

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/f_serial.c |   55 +
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/f_serial.c b/drivers/usb/gadget/f_serial.c
index 49c3d14..f14a5d9 100644
--- a/drivers/usb/gadget/f_serial.c
+++ b/drivers/usb/gadget/f_serial.c
@@ -258,6 +258,59 @@ fail:
return status;
 }
 
+static inline struct f_serial_opts *to_f_serial_opts(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct f_serial_opts,
+   func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(f_serial_opts);
+static ssize_t f_serial_attr_show(struct config_item *item,
+ struct configfs_attribute *attr,
+ char *page)
+{
+   struct f_serial_opts *opts = to_f_serial_opts(item);
+   struct f_serial_opts_attribute *f_serial_opts_attr =
+   container_of(attr, struct f_serial_opts_attribute, attr);
+   ssize_t ret = 0;
+
+   if (f_serial_opts_attr-show)
+   ret = f_serial_opts_attr-show(opts, page);
+   
+   return ret;
+}
+
+static void serial_attr_release(struct config_item *item)
+{
+   struct f_serial_opts *opts = to_f_serial_opts(item);
+
+   usb_put_function_instance(opts-func_inst);
+}
+
+static struct configfs_item_operations serial_item_ops = {
+   .release= serial_attr_release,
+   .show_attribute = f_serial_attr_show,
+};
+
+static ssize_t f_serial_port_num_show(struct f_serial_opts *opts, char *page)
+{
+   return sprintf(page, %u\n, opts-port_num);
+}
+
+static struct f_serial_opts_attribute f_serial_port_num =
+   __CONFIGFS_ATTR_RO(port_num, f_serial_port_num_show);
+
+static struct configfs_attribute *acm_attrs[] = {
+   f_serial_port_num.attr,
+   NULL,
+};
+
+static struct config_item_type serial_func_type = {
+   .ct_item_ops= serial_item_ops,
+   .ct_attrs   = acm_attrs,
+   .ct_owner   = THIS_MODULE,
+};
+
 static void gser_free_inst(struct usb_function_instance *f)
 {
struct f_serial_opts *opts;
@@ -282,6 +335,8 @@ static struct usb_function_instance *gser_alloc_inst(void)
kfree(opts);
return ERR_PTR(ret);
}
+   config_group_init_type_name(opts-func_inst.group, ,
+   serial_func_type);
 
return opts-func_inst;
 }
-- 
1.7.0.4

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

[PATCH 5/9] usb/gadget: f_serial: remove compatibility layer

There are no old function interface users left, so the old interface
can be removed.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/f_serial.c |   46 -
 1 files changed, 0 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/gadget/f_serial.c b/drivers/usb/gadget/f_serial.c
index 465789b..49c3d14 100644
--- a/drivers/usb/gadget/f_serial.c
+++ b/drivers/usb/gadget/f_serial.c
@@ -258,50 +258,6 @@ fail:
return status;
 }
 
-#ifdef USB_FSERIAL_INCLUDED
-
-static void
-gser_old_unbind(struct usb_configuration *c, struct usb_function *f)
-{
-   usb_free_all_descriptors(f);
-   kfree(func_to_gser(f));
-}
-
-/**
- * gser_bind_config - add a generic serial function to a configuration
- * @c: the configuration to support the serial instance
- * @port_num: /dev/ttyGS* port this interface will use
- * Context: single threaded during gadget setup
- *
- * Returns zero on success, else negative errno.
- */
-int __init gser_bind_config(struct usb_configuration *c, u8 port_num)
-{
-   struct f_gser   *gser;
-   int status;
-
-   /* allocate and initialize one new instance */
-   gser = kzalloc(sizeof *gser, GFP_KERNEL);
-   if (!gser)
-   return -ENOMEM;
-
-   gser-port_num = port_num;
-
-   gser-port.func.name = gser;
-   gser-port.func.strings = gser_strings;
-   gser-port.func.bind = gser_bind;
-   gser-port.func.unbind = gser_old_unbind;
-   gser-port.func.set_alt = gser_set_alt;
-   gser-port.func.disable = gser_disable;
-
-   status = usb_add_function(c, gser-port.func);
-   if (status)
-   kfree(gser);
-   return status;
-}
-
-#else
-
 static void gser_free_inst(struct usb_function_instance *f)
 {
struct f_serial_opts *opts;
@@ -372,5 +328,3 @@ DECLARE_USB_FUNCTION_INIT(gser, gser_alloc_inst, 
gser_alloc);
 MODULE_LICENSE(GPL);
 MODULE_AUTHOR(Al Borchers);
 MODULE_AUTHOR(David Brownell);
-
-#endif
-- 
1.7.0.4

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

[PATCH 4/9] usb/gadget: serial: convert to new interface of f_serial

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/Kconfig  |1 +
 drivers/usb/gadget/serial.c |   25 +++--
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 737232c..ae4cc11 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -814,6 +814,7 @@ config USB_G_SERIAL
depends on TTY
select USB_U_SERIAL
select USB_F_ACM
+   select USB_F_SERIAL
select USB_LIBCOMPOSITE
help
  The Serial Gadget talks to the Linux-USB generic serial driver.
diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c
index 07c1e80..9d215c4 100644
--- a/drivers/usb/gadget/serial.c
+++ b/drivers/usb/gadget/serial.c
@@ -37,8 +37,6 @@
  * a gcc --combine ... part1.c part2.c part3.c ...  build would.
  */
 #include f_obex.c
-#define USB_FSERIAL_INCLUDED
-#include f_serial.c
 
 /*-*/
 USB_GADGET_COMPOSITE_OPTIONS();
@@ -139,16 +137,6 @@ static int __init serial_bind_obex_config(struct 
usb_configuration *c)
return status;
 }
 
-static int __init serial_bind_gser_config(struct usb_configuration *c)
-{
-   unsigned i;
-   int status = 0;
-
-   for (i = 0; i  n_ports  status == 0; i++)
-   status = gser_bind_config(c, tty_lines[i]);
-   return status;
-}
-
 static struct usb_configuration serial_config_driver = {
/* .label = f(use_acm) */
/* .bConfigurationValue = f(use_acm) */
@@ -212,7 +200,7 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
int status;
int cur_line = 0;
 
-   if (!use_acm) {
+   if (use_obex) {
for (cur_line = 0; cur_line  n_ports; cur_line++) {
status = gserial_alloc_line(tty_lines[cur_line]);
if (status)
@@ -245,9 +233,10 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
} else if (use_obex)
status = usb_add_config(cdev, serial_config_driver,
serial_bind_obex_config);
-   else
-   status = usb_add_config(cdev, serial_config_driver,
-   serial_bind_gser_config);
+   else {
+   status = serial_register_ports(cdev, serial_config_driver,
+   gser);
+   }
if (status  0)
goto fail;
 
@@ -258,7 +247,7 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
 
 fail:
cur_line--;
-   while (cur_line = 0  !use_acm)
+   while (cur_line = 0  use_obex)
gserial_free_line(tty_lines[cur_line--]);
return status;
 }
@@ -270,7 +259,7 @@ static int gs_unbind(struct usb_composite_dev *cdev)
for (i = 0; i  n_ports; i++) {
usb_put_function(f_serial[i]);
usb_put_function_instance(fi_serial[i]);
-   if (!use_acm)
+   if (use_obex)
gserial_free_line(tty_lines[i]);
}
return 0;
-- 
1.7.0.4

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

[PATCH 8/9] usb/gadget: serial: convert to new interface of f_obex

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/Kconfig  |1 +
 drivers/usb/gadget/serial.c |   42 +++---
 2 files changed, 4 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index ed68dc6..e8e12fd 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -818,6 +818,7 @@ config USB_G_SERIAL
select USB_U_SERIAL
select USB_F_ACM
select USB_F_SERIAL
+   select USB_F_OBEX
select USB_LIBCOMPOSITE
help
  The Serial Gadget talks to the Linux-USB generic serial driver.
diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c
index 664da64..1f5f978 100644
--- a/drivers/usb/gadget/serial.c
+++ b/drivers/usb/gadget/serial.c
@@ -12,6 +12,7 @@
 
 #include linux/kernel.h
 #include linux/device.h
+#include linux/module.h
 #include linux/tty.h
 #include linux/tty_flip.h
 
@@ -28,18 +29,6 @@
 #define GS_VERSION_NAMEGS_LONG_NAME   GS_VERSION_STR
 
 /*-*/
-
-/*
- * Kbuild is not very cooperative with respect to linking separately
- * compiled library objects into one module.  So for now we won't use
- * separate compilation ... ensuring init/exit sections work to shrink
- * the runtime footprint, and giving us at least some parts of what
- * a gcc --combine ... part1.c part2.c part3.c ...  build would.
- */
-#define USBF_OBEX_INCLUDED
-#include f_obex.c
-
-/*-*/
 USB_GADGET_COMPOSITE_OPTIONS();
 
 /* Thanks to NetChip Technologies for donating this product ID.
@@ -126,17 +115,6 @@ module_param(n_ports, uint, 0);
 MODULE_PARM_DESC(n_ports, number of ports to create, default=1);
 
 /*-*/
-static unsigned char tty_lines[MAX_U_SERIAL_PORTS];
-
-static int __init serial_bind_obex_config(struct usb_configuration *c)
-{
-   unsigned i;
-   int status = 0;
-
-   for (i = 0; i  n_ports  status == 0; i++)
-   status = obex_bind_config(c, tty_lines[i]);
-   return status;
-}
 
 static struct usb_configuration serial_config_driver = {
/* .label = f(use_acm) */
@@ -199,15 +177,6 @@ out:
 static int __init gs_bind(struct usb_composite_dev *cdev)
 {
int status;
-   int cur_line = 0;
-
-   if (use_obex) {
-   for (cur_line = 0; cur_line  n_ports; cur_line++) {
-   status = gserial_alloc_line(tty_lines[cur_line]);
-   if (status)
-   goto fail;
-   }
-   }
 
/* Allocate string descriptor numbers ... note that string
 * contents can be overridden by the composite_dev glue.
@@ -232,8 +201,8 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
acm);
usb_ep_autoconfig_reset(cdev-gadget);
} else if (use_obex)
-   status = usb_add_config(cdev, serial_config_driver,
-   serial_bind_obex_config);
+   status = serial_register_ports(cdev, serial_config_driver,
+   obex);
else {
status = serial_register_ports(cdev, serial_config_driver,
gser);
@@ -247,9 +216,6 @@ static int __init gs_bind(struct usb_composite_dev *cdev)
return 0;
 
 fail:
-   cur_line--;
-   while (cur_line = 0  use_obex)
-   gserial_free_line(tty_lines[cur_line--]);
return status;
 }
 
@@ -260,8 +226,6 @@ static int gs_unbind(struct usb_composite_dev *cdev)
for (i = 0; i  n_ports; i++) {
usb_put_function(f_serial[i]);
usb_put_function_instance(fi_serial[i]);
-   if (use_obex)
-   gserial_free_line(tty_lines[i]);
}
return 0;
 }
-- 
1.7.0.4

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

[PATCH 3/9] usb/gadget: f_serial: convert to new function interface with backward compatibility

Converting f_serial to the new function interface requires converting
the f_serial's function code and its users.
This patch converts the f_serial.c to the new function interface.
The file is now compiled into a separate usb_f_serial.ko module.
The old function interface is provided by means of preprocessor
conditional directives. After all users are converted, the old interface
can be removed.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/Kconfig|3 +
 drivers/usb/gadget/Makefile   |2 +
 drivers/usb/gadget/f_serial.c |  131 -
 drivers/usb/gadget/serial.c   |1 +
 4 files changed, 110 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index a22809e..737232c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -557,6 +557,9 @@ config USB_F_SS_LB
 config USB_U_SERIAL
tristate
 
+config USB_F_SERIAL
+   tristate
+
 choice
tristate USB Gadget Drivers
default USB_ETH
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index f494757..d643510 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -82,3 +82,5 @@ obj-$(CONFIG_USB_F_ACM)   += usb_f_acm.o
 usb_f_ss_lb-y  := f_loopback.o f_sourcesink.o
 obj-$(CONFIG_USB_F_SS_LB)  += usb_f_ss_lb.o
 obj-$(CONFIG_USB_U_SERIAL) += u_serial.o
+usb_f_serial-y := f_serial.o
+obj-$(CONFIG_USB_F_SERIAL) += usb_f_serial.o
diff --git a/drivers/usb/gadget/f_serial.c b/drivers/usb/gadget/f_serial.c
index da33cfb..465789b 100644
--- a/drivers/usb/gadget/f_serial.c
+++ b/drivers/usb/gadget/f_serial.c
@@ -12,6 +12,7 @@
 
 #include linux/slab.h
 #include linux/kernel.h
+#include linux/module.h
 #include linux/device.h
 
 #include u_serial.h
@@ -42,7 +43,7 @@ static inline struct f_gser *func_to_gser(struct usb_function 
*f)
 
 /* interface descriptor: */
 
-static struct usb_interface_descriptor gser_interface_desc __initdata = {
+static struct usb_interface_descriptor gser_interface_desc = {
.bLength =  USB_DT_INTERFACE_SIZE,
.bDescriptorType =  USB_DT_INTERFACE,
/* .bInterfaceNumber = DYNAMIC */
@@ -55,21 +56,21 @@ static struct usb_interface_descriptor gser_interface_desc 
__initdata = {
 
 /* full speed support: */
 
-static struct usb_endpoint_descriptor gser_fs_in_desc __initdata = {
+static struct usb_endpoint_descriptor gser_fs_in_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
.bEndpointAddress = USB_DIR_IN,
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_endpoint_descriptor gser_fs_out_desc __initdata = {
+static struct usb_endpoint_descriptor gser_fs_out_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
.bEndpointAddress = USB_DIR_OUT,
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *gser_fs_function[] __initdata = {
+static struct usb_descriptor_header *gser_fs_function[] = {
(struct usb_descriptor_header *) gser_interface_desc,
(struct usb_descriptor_header *) gser_fs_in_desc,
(struct usb_descriptor_header *) gser_fs_out_desc,
@@ -78,47 +79,47 @@ static struct usb_descriptor_header *gser_fs_function[] 
__initdata = {
 
 /* high speed support: */
 
-static struct usb_endpoint_descriptor gser_hs_in_desc __initdata = {
+static struct usb_endpoint_descriptor gser_hs_in_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
.bmAttributes = USB_ENDPOINT_XFER_BULK,
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_endpoint_descriptor gser_hs_out_desc __initdata = {
+static struct usb_endpoint_descriptor gser_hs_out_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
.bmAttributes = USB_ENDPOINT_XFER_BULK,
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *gser_hs_function[] __initdata = {
+static struct usb_descriptor_header *gser_hs_function[] = {
(struct usb_descriptor_header *) gser_interface_desc,
(struct usb_descriptor_header *) gser_hs_in_desc,
(struct usb_descriptor_header *) gser_hs_out_desc,
NULL,
 };
 
-static struct usb_endpoint_descriptor gser_ss_in_desc __initdata = {
+static struct usb_endpoint_descriptor gser_ss_in_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
.bmAttributes = USB_ENDPOINT_XFER_BULK,
.wMaxPacketSize =   cpu_to_le16(1024),
 };
 
-static struct usb_endpoint_descriptor

[PATCH 9/9] usb/gadget: f_obex: add configfs support

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/usb/gadget/f_obex.c |   55 +++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/f_obex.c b/drivers/usb/gadget/f_obex.c
index 439b666..ea56ab1 100644
--- a/drivers/usb/gadget/f_obex.c
+++ b/drivers/usb/gadget/f_obex.c
@@ -456,6 +456,59 @@ int __init obex_bind_config(struct usb_configuration *c, 
u8 port_num)
 
 #else
 
+static inline struct f_serial_opts *to_f_serial_opts(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct f_serial_opts,
+   func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(f_serial_opts);
+static ssize_t f_obex_attr_show(struct config_item *item,
+   struct configfs_attribute *attr,
+   char *page)
+{
+   struct f_serial_opts *opts = to_f_serial_opts(item);
+   struct f_serial_opts_attribute *f_serial_opts_attr =
+   container_of(attr, struct f_serial_opts_attribute, attr);
+   ssize_t ret = 0;
+
+   if (f_serial_opts_attr-show)
+   ret = f_serial_opts_attr-show(opts, page);
+   
+   return ret;
+}
+
+static void obex_attr_release(struct config_item *item)
+{
+   struct f_serial_opts *opts = to_f_serial_opts(item);
+
+   usb_put_function_instance(opts-func_inst);
+}
+
+static struct configfs_item_operations obex_item_ops = {
+   .release= obex_attr_release,
+   .show_attribute = f_obex_attr_show,
+};
+
+static ssize_t f_obex_port_num_show(struct f_serial_opts *opts, char *page)
+{
+   return sprintf(page, %u\n, opts-port_num);
+}
+
+static struct f_serial_opts_attribute f_obex_port_num =
+   __CONFIGFS_ATTR_RO(port_num, f_obex_port_num_show);
+
+static struct configfs_attribute *acm_attrs[] = {
+   f_obex_port_num.attr,
+   NULL,
+};
+
+static struct config_item_type obex_func_type = {
+   .ct_item_ops= obex_item_ops,
+   .ct_attrs   = acm_attrs,
+   .ct_owner   = THIS_MODULE,
+};
+
 static void obex_free_inst(struct usb_function_instance *f)
 {
struct f_serial_opts *opts;
@@ -480,6 +533,8 @@ static struct usb_function_instance *obex_alloc_inst(void)
kfree(opts);
return ERR_PTR(ret);
}
+   config_group_init_type_name(opts-func_inst.group, ,
+   obex_func_type);
 
return opts-func_inst;
 }
-- 
1.7.0.4

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

RE: [PATCH V6 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2013-03-28 Thread Bhupesh SHARMA

Hi Laurent,

 Hi Bhupesh,
 
 Thanks for the patch.
 
 On Thursday 28 March 2013 15:11:52 Bhupesh Sharma wrote:
  This patch reworks the videobuffer management logic present in the UVC
  webcam gadget and ports it to use the more apt videobuf2 framework
  for video buffer management.
 
  To support routing video data captured from a real V4L2 video capture
  device with a zero copy operation on videobuffers (as they pass from
  the V4L2 domain to UVC domain via a user-space application), we need
  to support USER_PTR IO method at the UVC gadget side.
 
  So the V4L2 capture device driver can still continue to use MMAP IO
  method and now the user-space application can just pass a pointer to
  the video buffers being dequeued from the V4L2 device side while
  queueing them at the UVC gadget end. This ensures that we have a zero-
 copy
  design as the videobuffers pass from the V4L2 capture device to the
  UVC gadget.
 
  Note that there will still be a need to apply UVC specific payload
  headers on top of each UVC payload data, which will still require a
  copy operation to be performed in the 'encode' routines of the UVC
 gadget.
 
  This patch also addresses one issue found out while porting the UVC
  gadget to videobuf2 framework:
  - In case the usb requests queued by the gadget get completed
with a status of -ESHUTDOWN (disconnected from host),
the queue of videobuf2 should be cancelled to ensure that the
application space daemon is not left in a state waiting for
a vb2 to be successfully absorbed at the USB side.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 
 Finally,
 
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 
 :-)
 

Thanks for your time in reviewing this and previous patchsets :).

If you can apply these 2 patches to your git tree 
(git://linuxtv.org/pinchartl/uvcvideo.git),
I want to prepare and send out two more UVC patches:
- One that supports UVC Bulk endpoint implementation.
- Second that supports MEMCPY less implementation for adding UVC header to the 
UVC payload for UDC controllers which support SG implementation.

In the meantime, probably we can request Felipe to pull UVC gadget related 
patches from your git repo.

Regards,
Bhupesh

--
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 v9 2/2] usb: chipidea: udc: fix memory leak in _ep_nuke

In hardware_enqueue code adds one extra td with dma_pool_alloc if
mReq-req.zero is true. When _ep_nuke will be called for that endpoint,
dma_pool_free will not be called to free that memory again. That patch
fixes this.

Cc: stable sta...@vger.kernel.org # v3.5
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com
---
This patch cannot be backportet further than
v3.5, since the location of the source files
changed.

 drivers/usb/chipidea/udc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 77fb66f..d86333b 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -563,6 +563,12 @@ __acquires(mEp-lock)
struct ci13xxx_req *mReq = \
list_entry(mEp-qh.queue.next,
   struct ci13xxx_req, queue);
+
+   if (mReq-zptr) {
+   dma_pool_free(mEp-td_pool, mReq-zptr, mReq-zdma);
+   mReq-zptr = NULL;
+   }
+
list_del_init(mReq-queue);
mReq-req.status = -ESHUTDOWN;
 
-- 
1.8.2.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

[PATCH v9 0/2] usb: chipidea: udc: bugfixes

Hi,

this series solves some memroy issues with the chipidea udc

The series is based on v3.9-rc4.

Thanks,
Michael

Michael Grzeschik (2):
  usb: chipidea: udc: fix memory access of shared memory on armv5 machines
  usb: chipidea: udc: fix memory leak in _ep_nuke

 drivers/usb/chipidea/udc.c | 8 
 drivers/usb/chipidea/udc.h | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

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

[PATCH v9 1/2] usb: chipidea: udc: fix memory access of shared memory on armv5 machines

The udc uses an shared dma memory space between hard and software. This
memory layout is described in ci13xxx_qh and ci13xxx_td which are marked
with the attribute ((packed)).

The compiler currently does not know about the alignment of the memory
layout, and will create strb and ldrb operations.

The Datasheet of the synopsys core describes, that some operations on
the mapped memory need to be atomic double word operations. I.e. the
next pointer addressing in the qhead, as otherwise the hardware will
read wrong data and totally stuck.

This is also possible while working with the current active td queue,
and preparing the td-ptr.next in software while the hardware is still
working with the current active td which is supposed to be changed:

writeb(0xde, td-ptr.next + 0x0); /* strb */
writeb(0xad, td-ptr.next + 0x1); /* strb */

- hardware reads value of td-ptr.next and get stuck!

writeb(0xbe, td-ptr.next + 0x2); /* strb */
writeb(0xef, td-ptr.next + 0x3); /* strb */

This appeares on armv5 machines where the hardware does not support
unaligned 32bit operations.

This patch adds the attribute ((aligned(4))) to the structures to tell
the compiler to use 32bit operations. It also adds an wmb() for the
prepared TD data before it gets enqueued into the qhead.

Cc: stable sta...@vger.kernel.org # v3.5
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Alexander Shishkin alexander.shish...@linux.intel.com
---
This patch cannot be backportet further than
v3.5, since the location of the source files
changed.

 drivers/usb/chipidea/udc.c | 2 ++
 drivers/usb/chipidea/udc.h | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index f64fbea..77fb66f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -461,6 +461,8 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
mReq-ptr-page[i] =
(mReq-req.dma + i * CI13XXX_PAGE_SIZE)  
~TD_RESERVED_MASK;
 
+   wmb();
+
if (!list_empty(mEp-qh.queue)) {
struct ci13xxx_req *mReqPrev;
int n = hw_ep_bit(mEp-num, mEp-dir);
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index 4ff2384d..d12e8b5 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -40,7 +40,7 @@ struct ci13xxx_td {
 #define TD_CURR_OFFSET(0x0FFFUL   0)
 #define TD_FRAME_NUM  (0x07FFUL   0)
 #define TD_RESERVED_MASK  (0x0FFFUL   0)
-} __attribute__ ((packed));
+} __attribute__ ((packed, aligned(4)));
 
 /* DMA layout of queue heads */
 struct ci13xxx_qh {
@@ -57,7 +57,7 @@ struct ci13xxx_qh {
/* 9 */
u32 RESERVED;
struct usb_ctrlrequest   setup;
-} __attribute__ ((packed));
+} __attribute__ ((packed, aligned(4)));
 
 /**
  * struct ci13xxx_req - usb request representation
-- 
1.8.2.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: [PATCH V6 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

Hi Bhupesh,

On Thursday 28 March 2013 18:39:54 Bhupesh SHARMA wrote:
  On Thursday 28 March 2013 15:11:52 Bhupesh Sharma wrote:
   This patch reworks the videobuffer management logic present in the UVC
   webcam gadget and ports it to use the more apt videobuf2 framework
   for video buffer management.
   
   To support routing video data captured from a real V4L2 video capture
   device with a zero copy operation on videobuffers (as they pass from
   the V4L2 domain to UVC domain via a user-space application), we need
   to support USER_PTR IO method at the UVC gadget side.
   
   So the V4L2 capture device driver can still continue to use MMAP IO
   method and now the user-space application can just pass a pointer to
   the video buffers being dequeued from the V4L2 device side while
   queueing them at the UVC gadget end. This ensures that we have a zero-
   copy design as the videobuffers pass from the V4L2 capture device to
   the UVC gadget.
   
   Note that there will still be a need to apply UVC specific payload
   headers on top of each UVC payload data, which will still require a
   copy operation to be performed in the 'encode' routines of the UVC
   gadget.
  
   This patch also addresses one issue found out while porting the UVC
   
   gadget to videobuf2 framework:
 - In case the usb requests queued by the gadget get completed
 
   with a status of -ESHUTDOWN (disconnected from host),
   the queue of videobuf2 should be cancelled to ensure that the
   application space daemon is not left in a state waiting for
   a vb2 to be successfully absorbed at the USB side.
   
   Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  
  Finally,
  
  Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  
  :-)
 
 Thanks for your time in reviewing this and previous patchsets :).

You're welcome.

 If you can apply these 2 patches to your git tree
 (git://linuxtv.org/pinchartl/uvcvideo.git),

Done. They're in the uvc-gadget branch, based on Felipe's master branch.

 I want to prepare and send out two more UVC patches:
 - One that supports UVC Bulk endpoint implementation.
 - Second that supports MEMCPY less implementation for adding UVC header to
 the UVC payload for UDC controllers which support SG implementation.
 
 In the meantime, probably we can request Felipe to pull UVC gadget related
 patches from your git repo.

Felipe, can you pull from

git://linuxtv.org/pinchartl/uvcvideo.git uvc-gadget branch

or do you want a proper pull request ?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 3/5] USB chipidea: introduce dual role mode pdata flags

Felipe Balbi ba...@ti.com writes:

 Hi,

 On Fri, Mar 08, 2013 at 10:55:46PM +0200, Alexander Shishkin wrote:
  + dr_mode = ci-platdata-dr_mode;
  + if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == 
  USB_DR_MODE_DUAL_ROLE)
  + dr_mode = USB_DR_MODE_OTG;
  +
/* initialize role(s) before the interrupt is requested */
  - ret = ci_hdrc_host_init(ci);
  - if (ret)
  - dev_info(dev, doesn't support host\n);
  + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
 
  this is not something you should be passing via pdata; chipidea core
  should know how to read this data by itself. Meaning that chipidea core
  should be taught about devicetree. But make it optional since now all
  users use DT.
 
  And I don't think I like the idea of chipidea core calling into device
  tree code directly.
 
  Hmmmthis means draw :)
 
 Well, we could go for something like
 
 ci_hdrc-$(CONFIG_OF) += of.o
 
 and try to contain the damage there, maybe? Ideas? I would very much
 like to keep the clutter away from the core probe if possible.

 damage, what damage ? DeviceTree is quite real and drivers need to cope
 with it. If not all platforms support devicetree, make it optional. It's
 easy enough to make the choice based on device.of_node being valid or
 not.

We have dr_mode and phy_mode (so far). The latter is simple, but the
former one needs to see some special cases, based on its setting. Now,
if we're a pci device, for example, we don't have phandles and stuff and
we will still get this information via platform data.

So, what we'll end up with is some glue drivers (that don't have device
tree) passing all sorts of stuff via platform data and others just
expecting the chipidea to take care of it. That's inconsistent at best.

 At the end of the day, it's the chipidea IP which needs dr_mode, not the
 glue. Passing the responsability of decoding dr_mode to the glue is
 moronic. It's just like asking the glue to control chipidea's clocks.

Now, now. There's something to be said about stuffing core drivers with
support for all sorts of resource management protocols du jour, but
we'll leave that for another day.

As for the clocks, if they are external to chipidea controller, the
latter has no business messing with them. It's like asking chipidea to
do power management on your SoC for you. :)

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

Re: [PATCH 2/5] USB chipidea: add PTW and PTS handling

Marc Kleine-Budde m...@pengutronix.de writes:

 From: Michael Grzeschik m.grzesc...@pengutronix.de

 This patch makes it possible to configure the PTW and PTS bits inside
 the portsc register for host and device mode before the driver starts
 and the phy can be addressed as hardware implementation is designed.

Is anybody working on this? Now that the otg and phy bits are in
Felipe's next, we can think of applying these too. This needs some work,
though.

Firstly, it would be really nice to have the devicetree bit and imx bit
split to separate patches, so that if we're to revert one or the other,
we don't end up reverting both.

 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 ---
  .../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
  drivers/usb/chipidea/bits.h|   14 ++-
  drivers/usb/chipidea/ci13xxx_imx.c |3 ++
  drivers/usb/chipidea/core.c|   39 
 
  include/linux/usb/chipidea.h   |1 +
  5 files changed, 61 insertions(+), 1 deletion(-)

 diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt 
 b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 index 5778b9c..dd42ccd 100644
 --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 @@ -5,6 +5,11 @@ Required properties:
  - reg: Should contain registers location and length
  - interrupts: Should contain controller interrupt
  
 +Recommended properies:
 +- phy_type: the type of the phy connected to the core. Should be one
 +  of utmi, utmi_wide, ulpi, serial or hsic. Without this
 +  property the PORTSC register won't be touched
 +
  Optional properties:
  - fsl,usbphy: phandler of usb phy that connects to the only one port
  - fsl,usbmisc: phandler of non-core register device, with one argument
 diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
 index 050de85..d8ffc2f 100644
 --- a/drivers/usb/chipidea/bits.h
 +++ b/drivers/usb/chipidea/bits.h
 @@ -48,10 +48,22 @@
  #define PORTSC_SUSP   BIT(7)
  #define PORTSC_HSPBIT(9)
  #define PORTSC_PTC(0x0FUL  16)
 +/* PTS and PTW for non lpm version only */
 +#define PORTSC_PTS(d) d)  0x3)  30) | (((d)  0x4) ? BIT(25) 
 : 0))
 +#define PORTSC_PTWBIT(28)
  
  /* DEVLC */
  #define DEVLC_PSPD(0x03UL  25)
 -#defineDEVLC_PSPD_HS  (0x02UL  25)
 +#define DEVLC_PSPD_HS (0x02UL  25)
 +#define DEVLC_PTW BIT(27)
 +#define DEVLC_STS BIT(28)
 +#define DEVLC_PTS(d)  (((d)  0x7)  29)
 +
 +/* Encoding for DEVLC_PTS and PORTSC_PTS */
 +#define PTS_UTMI  0
 +#define PTS_ULPI  2
 +#define PTS_SERIAL3
 +#define PTS_HSIC  4
  
  /* OTGSC */
  #define OTGSC_IDPU BIT(5)
 diff --git a/drivers/usb/chipidea/ci13xxx_imx.c 
 b/drivers/usb/chipidea/ci13xxx_imx.c
 index 69024e0..ebc1148 100644
 --- a/drivers/usb/chipidea/ci13xxx_imx.c
 +++ b/drivers/usb/chipidea/ci13xxx_imx.c
 @@ -21,6 +21,7 @@
  #include linux/clk.h
  #include linux/regulator/consumer.h
  #include linux/pinctrl/consumer.h
 +#include linux/usb/of.h
  
  #include ci.h
  #include ci13xxx_imx.h
 @@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
  CI13XXX_PULLUP_ON_VBUS |
  CI13XXX_DISABLE_STREAMING;
  
 + pdata-phy_mode = of_usb_get_phy_mode(pdev-dev.of_node);
 +
   data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
   if (!data) {
   dev_err(pdev-dev, Failed to allocate CI13xxx-IMX data!\n);
 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index 57cae1f..04d68cb 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -67,6 +67,8 @@
  #include linux/usb/gadget.h
  #include linux/usb/otg.h
  #include linux/usb/chipidea.h
 +#include linux/usb/of.h
 +#include linux/phy.h

Is of.h actually needed here?

  
  #include ci.h
  #include udc.h
 @@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void 
 __iomem *base)
   return 0;
  }
  
 +static void hw_phymode_configure(struct ci13xxx *ci)
 +{
 + u32 portsc, lpm;
 +
 + switch (ci-platdata-phy_mode) {
 + case USBPHY_INTERFACE_MODE_UTMI:
 + portsc = PORTSC_PTS(PTS_UTMI);
 + lpm = DEVLC_PTS(PTS_UTMI);
 + break;
 + case USBPHY_INTERFACE_MODE_UTMIW:
 + portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
 + lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
 + break;
 + case USBPHY_INTERFACE_MODE_ULPI:
 + portsc = PORTSC_PTS(PTS_ULPI);
 + lpm = DEVLC_PTS(PTS_ULPI);
 + break;
 + case USBPHY_INTERFACE_MODE_SERIAL:
 + portsc =

Re: [PATCH v9 0/2] usb: chipidea: udc: bugfixes

Michael Grzeschik m.grzesc...@pengutronix.de writes:

 Hi,

 this series solves some memroy issues with the chipidea udc

 The series is based on v3.9-rc4.

Looks good.

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

Re: [PATCH 3/5] USB chipidea: introduce dual role mode pdata flags

Hi,

On Thu, Mar 28, 2013 at 01:13:00PM +0200, Alexander Shishkin wrote:
   + dr_mode = ci-platdata-dr_mode;
   + if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == 
   USB_DR_MODE_DUAL_ROLE)
   + dr_mode = USB_DR_MODE_OTG;
   +
 /* initialize role(s) before the interrupt is requested */
   - ret = ci_hdrc_host_init(ci);
   - if (ret)
   - dev_info(dev, doesn't support host\n);
   + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
  
   this is not something you should be passing via pdata; chipidea core
   should know how to read this data by itself. Meaning that chipidea core
   should be taught about devicetree. But make it optional since now all
   users use DT.
  
   And I don't think I like the idea of chipidea core calling into device
   tree code directly.
  
   Hmmmthis means draw :)
  
  Well, we could go for something like
  
  ci_hdrc-$(CONFIG_OF) += of.o
  
  and try to contain the damage there, maybe? Ideas? I would very much
  like to keep the clutter away from the core probe if possible.
 
  damage, what damage ? DeviceTree is quite real and drivers need to cope
  with it. If not all platforms support devicetree, make it optional. It's
  easy enough to make the choice based on device.of_node being valid or
  not.
 
 We have dr_mode and phy_mode (so far). The latter is simple, but the
 former one needs to see some special cases, based on its setting. Now,
 if we're a pci device, for example, we don't have phandles and stuff and
 we will still get this information via platform data.

fair enough:

if (pdev-dev.of_node)
chipidea_init_from_dt(ci);
else
chipidea_init_from_pdata(ci);

 So, what we'll end up with is some glue drivers (that don't have device
 tree) passing all sorts of stuff via platform data and others just
 expecting the chipidea to take care of it. That's inconsistent at best.

it's not inconsistent at all.

Some drivers pass data through DT and some pass data through pdata.

Regardless of which driver type you have, chipidea core still needs to
fetch the data, either by of_property_*() calls or by reading
pdata-$field.

I wouldn't call it inconsistency, it's just coping with both ways of
receiving data.

  At the end of the day, it's the chipidea IP which needs dr_mode, not the
  glue. Passing the responsability of decoding dr_mode to the glue is
  moronic. It's just like asking the glue to control chipidea's clocks.
 
 Now, now. There's something to be said about stuffing core drivers with
 support for all sorts of resource management protocols du jour, but
 we'll leave that for another day.
 
 As for the clocks, if they are external to chipidea controller, the
 latter has no business messing with them. It's like asking chipidea to

heh, that's not what I said...

 do power management on your SoC for you. :)

right, asking other layers to do your work is stupid, that's exactly
what I said. Shuving DT knowledge in glue layer just so chipidea core
only understands pdata is stupid. You end up allocating memory twice to
hold the same data (once for DT and once for the pdata copies of it).

-- 
balbi


signature.asc
Description: Digital signature

[PATCH/RFC] uvcvideo: Disable USB autosuspend for Creative Live! Cam Optia AF

The camera fails to start video streaming after having been autosuspend.
Add a new quirk to selectively disable autosuspend for devices that
don't support it.

Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
 drivers/media/usb/uvc/uvc_driver.c | 14 +-
 drivers/media/usb/uvc/uvcvideo.h   |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

I've tried to set the reset resume quirk for this device in the USB core but
the camera still failed to start video streaming after having been
autosuspended. Regardless of whether the reset resume quirk was set, it would
respond to control messages but wouldn't send video data.

This solution below is a hack, but I'm not sure what else I can try. Crazy
ideas are welcome.

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 5dbefa6..99e2de0 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1913,8 +1913,11 @@ static int uvc_probe(struct usb_interface *intf,
supported.\n, ret);
}
 
+   if (!(dev-quirks  UVC_QUIRK_DISABLE_AUTOSUSPEND))
+   usb_enable_autosuspend(udev);
+
uvc_trace(UVC_TRACE_PROBE, UVC device initialized.\n);
-   usb_enable_autosuspend(udev);
+
return 0;
 
 error:
@@ -2061,6 +2064,15 @@ static struct usb_device_id uvc_ids[] = {
  .bInterfaceSubClass   = 1,
  .bInterfaceProtocol   = 0,
  .driver_info  = UVC_QUIRK_PROBE_MINMAX },
+   /* Creative Live! Cam Optia AF */
+   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
+   | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x041e,
+ .idProduct= 0x4058,
+ .bInterfaceClass  = USB_CLASS_VIDEO,
+ .bInterfaceSubClass   = 1,
+ .bInterfaceProtocol   = 0,
+ .driver_info  = UVC_QUIRK_DISABLE_AUTOSUSPEND },
/* Genius eFace 2025 */
{ .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index af505fd..9cd584a 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -137,6 +137,7 @@
 #define UVC_QUIRK_FIX_BANDWIDTH0x0080
 #define UVC_QUIRK_PROBE_DEF0x0100
 #define UVC_QUIRK_RESTRICT_FRAME_RATE  0x0200
+#define UVC_QUIRK_DISABLE_AUTOSUSPEND  0x0400
 
 /* Format flags */
 #define UVC_FMT_FLAG_COMPRESSED0x0001
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 2/2] usb: chipidea: AR933x platform support for the chipidea driver

Svetoslav Neykov svetos...@neykov.name writes:

 Support host and device usb modes for the chipidea controller in AR933x.

 Changes since last version of the patch:
   * conditionally include ci13xxx_ar933x.c for compilation
   * removed __devinit/__devexit/__devexit_p()
   * use a dynamically allocated structure for ci13xxx_platform_data
   * move controller mode check to platform usb registration
   * pick a different name for the ar933x chipidea driver
   * use a correct MODE_ALIAS name
   * use the dr_mode changes in [PATCH 0/3] otg-for-v3.10-v2:
 separate phy code and add DT helper

 Signed-off-by: Svetoslav Neykov svetos...@neykov.name

This can go in either through chipidea or mips tree, but in either case
it has to have an Acked-by from the maintainer of the other tree.

 ---
  arch/mips/ath79/dev-usb.c  |   50 +
  arch/mips/include/asm/mach-ath79/ar71xx_regs.h |3 +
  .../asm/mach-ath79/ar933x_chipidea_platform.h  |   18 +
  drivers/usb/chipidea/Makefile  |5 ++
  drivers/usb/chipidea/ci13xxx_ar933x.c  |   75 
 
  5 files changed, 151 insertions(+)
  create mode 100644 
 arch/mips/include/asm/mach-ath79/ar933x_chipidea_platform.h
  create mode 100644 drivers/usb/chipidea/ci13xxx_ar933x.c

 diff --git a/arch/mips/ath79/dev-usb.c b/arch/mips/ath79/dev-usb.c
 index bd2bc10..0c285d9 100644
 --- a/arch/mips/ath79/dev-usb.c
 +++ b/arch/mips/ath79/dev-usb.c
 @@ -19,9 +19,11 @@
  #include linux/platform_device.h
  #include linux/usb/ehci_pdriver.h
  #include linux/usb/ohci_pdriver.h
 +#include linux/usb/otg.h
  
  #include asm/mach-ath79/ath79.h
  #include asm/mach-ath79/ar71xx_regs.h
 +#include asm/mach-ath79/ar933x_chipidea_platform.h
  #include common.h
  #include dev-usb.h
  
 @@ -68,6 +70,22 @@ static struct platform_device ath79_ehci_device = {
   },
  };
  
 +static struct resource ar933x_chipidea_resources[2];
 +
 +static struct ar933x_chipidea_platform_data ar933x_chipidea_data = {
 +};

No need to initialize it like this, it should save a few bytes in
.data.

 +
 +static struct platform_device ar933x_chipidea_device = {
 + .name   = ar933x-chipidea,
 + .id = -1,
 + .resource   = ar933x_chipidea_resources,
 + .num_resources  = ARRAY_SIZE(ar933x_chipidea_resources),
 + .dev = {
 + .dma_mask   = ath79_ehci_dmamask,
 + .coherent_dma_mask  = DMA_BIT_MASK(32),
 + },
 +};
 +
  static void __init ath79_usb_init_resource(struct resource res[2],
  unsigned long base,
  unsigned long size,
 @@ -174,8 +192,32 @@ static void __init ar913x_usb_setup(void)
   platform_device_register(ath79_ehci_device);
  }
  
 +static void __init ar933x_usb_setup_ctrl_config(void)
 +{
 + void __iomem *usb_ctrl_base, *usb_config_reg;
 + u32 usb_config;
 +
 + usb_ctrl_base = ioremap(AR71XX_USB_CTRL_BASE, AR71XX_USB_CTRL_SIZE);
 + usb_config_reg = usb_ctrl_base + AR71XX_USB_CTRL_REG_CONFIG;
 + usb_config = __raw_readl(usb_config_reg);
 + usb_config = ~AR933X_USB_CONFIG_HOST_ONLY;
 + __raw_writel(usb_config, usb_config_reg);
 + iounmap(usb_ctrl_base);
 +}
 +
  static void __init ar933x_usb_setup(void)
  {
 + u32 bootstrap;
 + enum usb_dr_mode dr_mode;
 +
 + bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
 + if (bootstrap  AR933X_BOOTSTRAP_USB_MODE_HOST) {
 + dr_mode = USB_DR_MODE_HOST;
 + } else {
 + dr_mode = USB_DR_MODE_PERIPHERAL;
 + ar933x_usb_setup_ctrl_config();
 + }
 +
   ath79_device_reset_set(AR933X_RESET_USBSUS_OVERRIDE);
   mdelay(10);
  
 @@ -187,8 +229,16 @@ static void __init ar933x_usb_setup(void)
  
   ath79_usb_init_resource(ath79_ehci_resources, AR933X_EHCI_BASE,
   AR933X_EHCI_SIZE, ATH79_CPU_IRQ_USB);
 +
   ath79_ehci_device.dev.platform_data = ath79_ehci_pdata_v2;
   platform_device_register(ath79_ehci_device);
 +
 + ath79_usb_init_resource(ar933x_chipidea_resources, AR933X_EHCI_BASE,
 + AR933X_EHCI_SIZE, ATH79_CPU_IRQ_USB);
 +
 + ar933x_chipidea_data.dr_mode = dr_mode;
 + ar933x_chipidea_device.dev.platform_data = ar933x_chipidea_data;
 + platform_device_register(ar933x_chipidea_device);
  }
  
  static void __init ar934x_usb_setup(void)
 diff --git a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h 
 b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
 index a5e0f17..13eb2d9 100644
 --- a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
 +++ b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
 @@ -297,6 +297,7 @@
  #define AR934X_RESET_USB_PHY BIT(4)
  #define AR934X_RESET_USBSUS_OVERRIDE BIT(3)
  
 +#define AR933X_BOOTSTRAP_USB_MODE_HOST   BIT(3)
  #define AR933X_BOOTSTRAP_REF_CLK_40  BIT(0)

RE: usb video capture issue due to uvc_complete callback spends more time

2013-03-28 Thread B, Ravi

Laurent

  
   Some more debugging, Most of the time spend in stream-decode()
 function
 
  That points to uvc_video_decode_isoc().
 
 You are correct, that points to uvc_video_decode_isoc()
 
 
   in uvc_video_complete() callback handler.
 
  It's not very surprising, but doesn't tell where the problem comes from.
  As
  Ming Lei pointed out, slow access to coherent memory might be an
  explanation.
  Could you find out how the time is spent between
 uvc_video_decode_start(),
  uvc_video_decode_data() and uvc_video_decode_data() ? It might also be
  worth
  it timing the uvc_queue_next_buffer() calls, in case the function is
  delayed
  by spinlock contention (if you're running on an SMP system).
 
 I did not dig further, I try to get this timing info.
 
 Quickly I tried another experiment, instead of calling stream-decode() in
 callback, initiated work thread, which perform stream-decode()  re-
 submit urb. This reduces the uvc_video_callback() time to 12usec, But
 after 900 urb completion, submit_urb() failed with -EBUSY error. Further I
 stopped debugging, something I may not be doing right at uvc level.
 
 Thanks Laurent.

I profiled the uvc_video_decode_isoc(), the maximum time was taken by 
uvc_video_decode_data() function. 

For example, in one iteration I have observed, the time taken by 
uvc_video_decode_isoc() was 2175 usec. In this maximum amount of time was 
consumed by uvc_video_decode_data() around 1792 usec.

The function uvc_video_decode_data() was called in loop, below the time taken 
in each iteration. The total was around 1792 usec.

SUM(108, 59, 72, 57, 108, 58, 108, 58, 
72, 87, 72, 58, 108, 59, 82, 58, 
108,59, 72, 87, 74, 59, 109) = 1792 usec

let me know if you need any further info.

--
Ravi B
--
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: usb video capture issue due to uvc_complete callback spends more time

2013-03-28 Thread Ming Lei

On Thu, Mar 28, 2013 at 8:30 PM, B, Ravi ravib...@ti.com wrote:

 For example, in one iteration I have observed, the time taken by 
 uvc_video_decode_isoc() was 2175 usec. In this maximum amount of time was 
 consumed by uvc_video_decode_data() around 1792 usec.

uvc_video_decode_data() is basically a memcpy() from coherent buffer to
normal buffer.


 The function uvc_video_decode_data() was called in loop, below the time taken 
 in each iteration. The total was around 1792 usec.

 SUM(108, 59, 72, 57, 108, 58, 108, 58,
 72, 87, 72, 58, 108, 59, 82, 58,
 108,59, 72, 87, 74, 59, 109) = 1792 usec

Looks that just means very slow read data from coherent buffer, or
bad memcpy?

Also you might need to check if you use gp_timer as default
clocksource, otherwise the default 32K timer only gives you a
~30us accuracy.

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

Re: [PATCH 3/5] USB chipidea: introduce dual role mode pdata flags

Felipe Balbi ba...@ti.com writes:

 Hi,

Hi,

 On Thu, Mar 28, 2013 at 01:13:00PM +0200, Alexander Shishkin wrote:
   + dr_mode = ci-platdata-dr_mode;
   + if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == 
   USB_DR_MODE_DUAL_ROLE)
   + dr_mode = USB_DR_MODE_OTG;
   +
 /* initialize role(s) before the interrupt is requested */
   - ret = ci_hdrc_host_init(ci);
   - if (ret)
   - dev_info(dev, doesn't support host\n);
   + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) 
   {
  
   this is not something you should be passing via pdata; chipidea core
   should know how to read this data by itself. Meaning that chipidea 
   core
   should be taught about devicetree. But make it optional since now all
   users use DT.
  
   And I don't think I like the idea of chipidea core calling into device
   tree code directly.
  
   Hmmmthis means draw :)
  
  Well, we could go for something like
  
  ci_hdrc-$(CONFIG_OF) += of.o
  
  and try to contain the damage there, maybe? Ideas? I would very much
  like to keep the clutter away from the core probe if possible.
 
  damage, what damage ? DeviceTree is quite real and drivers need to cope
  with it. If not all platforms support devicetree, make it optional. It's
  easy enough to make the choice based on device.of_node being valid or
  not.
 
 We have dr_mode and phy_mode (so far). The latter is simple, but the
 former one needs to see some special cases, based on its setting. Now,
 if we're a pci device, for example, we don't have phandles and stuff and
 we will still get this information via platform data.

 fair enough:

 if (pdev-dev.of_node)
   chipidea_init_from_dt(ci);
 else
   chipidea_init_from_pdata(ci);

You mean, you want to have two instances of the similar logic? Don't
forget that they might fail to fetch certain phandles and still
continue, but failing to fetch other phandles will be fatal for
probe(). The above snipped can also be shortened to

  chipidea_just_do_the_right_thing(ci); /* I'd like that, btw */

The devil is in the details.

Then, I hate to bring it up, but what do you do for acpi devices? PnP
devices? PCMCIA devices?

Right now, the core is a platform driver. It gets all the information
from platform data. That's all it needs for its purpose, and all the
platform specific details are abstracted away. It's the purpose of the
glue layer's existance to fetch all the relevant bits from the glue
driver knows where and supply it in a *consistent* manner to the core.

Note, it's totally different for regulators or clocks or phys. It is
totally unacceptable to pass objects around between glue and core and
glue shouldn't have to deal with those. And, of course, you can request
all those in the core code in a platform-agnostic manner.

 So, what we'll end up with is some glue drivers (that don't have device
 tree) passing all sorts of stuff via platform data and others just
 expecting the chipidea to take care of it. That's inconsistent at best.

 it's not inconsistent at all.

 Some drivers pass data through DT and some pass data through pdata.

 Regardless of which driver type you have, chipidea core still needs to
 fetch the data, either by of_property_*() calls or by reading
 pdata-$field.

 I wouldn't call it inconsistency, it's just coping with both ways of
 receiving data.

  At the end of the day, it's the chipidea IP which needs dr_mode, not the
  glue. Passing the responsability of decoding dr_mode to the glue is
  moronic. It's just like asking the glue to control chipidea's clocks.
 
 Now, now. There's something to be said about stuffing core drivers with
 support for all sorts of resource management protocols du jour, but
 we'll leave that for another day.
 
 As for the clocks, if they are external to chipidea controller, the
 latter has no business messing with them. It's like asking chipidea to

 heh, that's not what I said...

 do power management on your SoC for you. :)

 right, asking other layers to do your work is stupid, that's exactly
 what I said. Shuving DT knowledge in glue layer just so chipidea core
 only understands pdata is stupid.

Why? Especially if the glue drivers have to fetch stuff for their own
needs from DT anyway. Might easily happen.

 You end up allocating memory twice to
 hold the same data (once for DT and once for the pdata copies of it).

It would have been one valid reason for teaching chipidea core about DT,
if we could duplicate *all* of the pdata fields in DT. Otherwise we
still need pdata. And supposing that we can (which we can't) do that,
and supposing that extra 32 bytes of memory actually matter, it still
doesn't justify the extra code in the core to deal with DT. I'm still
not convinced.

Regards,
--
Alex
--
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: usb video capture issue due to uvc_complete callback spends more time

Hi,

On Thu, Mar 28, 2013 at 08:53:03PM +0800, Ming Lei wrote:
 On Thu, Mar 28, 2013 at 8:30 PM, B, Ravi ravib...@ti.com wrote:
 
  For example, in one iteration I have observed, the time taken by
  uvc_video_decode_isoc() was 2175 usec. In this maximum amount of
  time was consumed by uvc_video_decode_data() around 1792 usec.
 
 uvc_video_decode_data() is basically a memcpy() from coherent buffer to
 normal buffer.

if that's the case, this should show, right ?

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 3394c34..fdba0b7 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1490,12 +1490,7 @@ static int uvc_init_video_isoc(struct uvc_streaming 
*stream,
urb-context = stream;
urb-pipe = usb_rcvisocpipe(stream-dev-udev,
ep-desc.bEndpointAddress);
-#ifndef CONFIG_DMA_NONCOHERENT
-   urb-transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
-   urb-transfer_dma = stream-urb_dma[i];
-#else
urb-transfer_flags = URB_ISO_ASAP;
-#endif
urb-interval = ep-desc.bInterval;
urb-transfer_buffer = stream-urb_buffer[i];
urb-complete = uvc_video_complete;
@@ -1555,10 +1550,6 @@ static int uvc_init_video_bulk(struct uvc_streaming 
*stream,
usb_fill_bulk_urb(urb, stream-dev-udev, pipe,
stream-urb_buffer[i], size, uvc_video_complete,
stream);
-#ifndef CONFIG_DMA_NONCOHERENT
-   urb-transfer_flags = URB_NO_TRANSFER_DMA_MAP;
-   urb-transfer_dma = stream-urb_dma[i];
-#endif
 
stream-urb[i] = urb;
}

-- 
balbi


signature.asc
Description: Digital signature

Re: usb video capture issue due to uvc_complete callback spends more time

On Thu, Mar 28, 2013 at 03:23:46PM +0200, Felipe Balbi wrote:
 Hi,
 
 On Thu, Mar 28, 2013 at 08:53:03PM +0800, Ming Lei wrote:
  On Thu, Mar 28, 2013 at 8:30 PM, B, Ravi ravib...@ti.com wrote:
  
   For example, in one iteration I have observed, the time taken by
   uvc_video_decode_isoc() was 2175 usec. In this maximum amount of
   time was consumed by uvc_video_decode_data() around 1792 usec.
  
  uvc_video_decode_data() is basically a memcpy() from coherent buffer to
  normal buffer.
 
 if that's the case, this should show, right ?
 
 diff --git a/drivers/media/usb/uvc/uvc_video.c 
 b/drivers/media/usb/uvc/uvc_video.c
 index 3394c34..fdba0b7 100644
 --- a/drivers/media/usb/uvc/uvc_video.c
 +++ b/drivers/media/usb/uvc/uvc_video.c
 @@ -1490,12 +1490,7 @@ static int uvc_init_video_isoc(struct uvc_streaming 
 *stream,
   urb-context = stream;
   urb-pipe = usb_rcvisocpipe(stream-dev-udev,
   ep-desc.bEndpointAddress);
 -#ifndef CONFIG_DMA_NONCOHERENT
 - urb-transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
 - urb-transfer_dma = stream-urb_dma[i];
 -#else
   urb-transfer_flags = URB_ISO_ASAP;
 -#endif
   urb-interval = ep-desc.bInterval;
   urb-transfer_buffer = stream-urb_buffer[i];
   urb-complete = uvc_video_complete;
 @@ -1555,10 +1550,6 @@ static int uvc_init_video_bulk(struct uvc_streaming 
 *stream,
   usb_fill_bulk_urb(urb, stream-dev-udev, pipe,
   stream-urb_buffer[i], size, uvc_video_complete,
   stream);
 -#ifndef CONFIG_DMA_NONCOHERENT
 - urb-transfer_flags = URB_NO_TRANSFER_DMA_MAP;
 - urb-transfer_dma = stream-urb_dma[i];
 -#endif
  
   stream-urb[i] = urb;
   }

actually this is more correct I guess:

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 3394c34..5c0e3a6 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1387,14 +1387,8 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming 
*stream,
for (; npackets  1; npackets /= 2) {
for (i = 0; i  UVC_URBS; ++i) {
stream-urb_size = psize * npackets;
-#ifndef CONFIG_DMA_NONCOHERENT
-   stream-urb_buffer[i] = usb_alloc_coherent(
-   stream-dev-udev, stream-urb_size,
-   gfp_flags | __GFP_NOWARN, stream-urb_dma[i]);
-#else
stream-urb_buffer[i] =
kmalloc(stream-urb_size, gfp_flags | __GFP_NOWARN);
-#endif
if (!stream-urb_buffer[i]) {
uvc_free_urb_buffers(stream);
break;
@@ -1490,12 +1484,7 @@ static int uvc_init_video_isoc(struct uvc_streaming 
*stream,
urb-context = stream;
urb-pipe = usb_rcvisocpipe(stream-dev-udev,
ep-desc.bEndpointAddress);
-#ifndef CONFIG_DMA_NONCOHERENT
-   urb-transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
-   urb-transfer_dma = stream-urb_dma[i];
-#else
urb-transfer_flags = URB_ISO_ASAP;
-#endif
urb-interval = ep-desc.bInterval;
urb-transfer_buffer = stream-urb_buffer[i];
urb-complete = uvc_video_complete;
@@ -1555,10 +1544,6 @@ static int uvc_init_video_bulk(struct uvc_streaming 
*stream,
usb_fill_bulk_urb(urb, stream-dev-udev, pipe,
stream-urb_buffer[i], size, uvc_video_complete,
stream);
-#ifndef CONFIG_DMA_NONCOHERENT
-   urb-transfer_flags = URB_NO_TRANSFER_DMA_MAP;
-   urb-transfer_dma = stream-urb_dma[i];
-#endif
 
stream-urb[i] = urb;
}

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 2/4] usb: introduce usb force power off mechanism


On 2013/3/28 2:45, Alan Stern wrote:

+int usb_hub_port_power_reset(struct usb_device *hdev, int port1)
+{
+   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+   struct usb_interface *intf = to_usb_interface(hub-intfdev);
+   int ret;
+
+   usb_autopm_get_interface(intf);


What happens if hdev is NULL?

Yes, This will be a problem. The repower cmd to root
hub should be ignored.




+   hub_port_logical_disconnect(hub, port1);
+   ret = usb_hub_set_port_power(hdev, port1, false);


How long do you think the power should remain turned off?  This code
will leave it off for only a few milliseconds at most.  That may not
even be long enough for the voltage to drop all the way to 0.

The delay probably should be at least 100 ms.  Maybe more, I don't
know.

I think this depends on the device. I don't find the answer on spec.
I find the cool down delay of over-current for port is 100ms
and one for hub is 500ms. Could we reference these delay?
500ms would be safe.



+   ret |= usb_hub_set_port_power(hdev, port1, true);


Don't use |=.  Skip the second call if the first one fails.


OK.

+   usb_autopm_put_interface(intf);
+
+   return ret;
+}


If you placed this function later on in the source file, you wouldn't
need the forward declaration of hub_port_logical_disconnect().

Yes, I will do this.


Alan Stern



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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 v2 1/1] usb: musb: implement (un)map_urb_for_dma hooks

2013-03-28 Thread Ruslan Bilovol

Hi Felipe,

On Wed, Mar 27, 2013 at 3:17 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Thu, Mar 14, 2013 at 08:12:09PM +0200, Ruslan Bilovol wrote:
 MUSB controller cannot work in DMA mode with misaligned buffers,
 switching in PIO mode.

 HCD core has hooks that allow to override the default DMA
 mapping and unmapping routines for host controllers that have
 special DMA requirements, such as alignment contraints.

 It is observed that work in PIO mode is slow and it's better
 to align buffers properly before passing them to MUSB

 This increased throughput 80-120 MBits/s over musb@omap4 with
 USB Gigabit ethernet adapter attached.

 Some ideas taken from ehci-tegra.c

 Signed-off-by: Ruslan Bilovol ruslan.bilo...@ti.com
 ---
  drivers/usb/musb/musb_core.c |   14 ++
  drivers/usb/musb/musb_host.c |  102 
 +-
  drivers/usb/musb/musb_host.h |2 +-
  3 files changed, 116 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
 index 60b41cc..91ac166 100644
 --- a/drivers/usb/musb/musb_core.c
 +++ b/drivers/usb/musb/musb_core.c
 @@ -1431,6 +1431,20 @@ static int musb_core_init(u16 musb_type, struct musb 
 *musb)

   /* log release info */
   musb-hwvers = musb_read_hwvers(mbase);
 +
 +#ifndef CONFIG_MUSB_PIO_ONLY
 + /*
 +  * The DMA engine in RTL1.8 and above cannot handle
 +  * DMA addresses that are not aligned to a 4 byte boundary.
 +  * For such engine implemented (un)map_urb_for_dma hooks.
 +  * Do not use these hooks for RTL1.8
 +  */
 + if (musb-hwvers  MUSB_HWVERS_1800) {

 if you move this check to map/unmap and always return error if this is
 true, you can avoid removing 'const' from our struct hc_driver. Would
 that work ?

If we return an error in map/unmap callbacks, this will break urb transferring,
however I can call core function usb_hcd_(un)map_urb_for_dma() instead of
returning the error (and that is default behavior if we do not have
map/unmap callbacks
set for the hc driver) so I can avoid removing 'const' from our struct
hc_driver and this will work.
The side effect will be only in small overhead for this path.

So, will be this OK for you? I will send v3 in this case.

-- 
Best regards,
Ruslan Bilvol


 --
 balbi
--
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 v2 1/1] usb: musb: implement (un)map_urb_for_dma hooks

On Thu, Mar 28, 2013 at 03:44:50PM +0200, Ruslan Bilovol wrote:
 Hi Felipe,
 
 On Wed, Mar 27, 2013 at 3:17 PM, Felipe Balbi ba...@ti.com wrote:
  Hi,
 
  On Thu, Mar 14, 2013 at 08:12:09PM +0200, Ruslan Bilovol wrote:
  MUSB controller cannot work in DMA mode with misaligned buffers,
  switching in PIO mode.
 
  HCD core has hooks that allow to override the default DMA
  mapping and unmapping routines for host controllers that have
  special DMA requirements, such as alignment contraints.
 
  It is observed that work in PIO mode is slow and it's better
  to align buffers properly before passing them to MUSB
 
  This increased throughput 80-120 MBits/s over musb@omap4 with
  USB Gigabit ethernet adapter attached.
 
  Some ideas taken from ehci-tegra.c
 
  Signed-off-by: Ruslan Bilovol ruslan.bilo...@ti.com
  ---
   drivers/usb/musb/musb_core.c |   14 ++
   drivers/usb/musb/musb_host.c |  102 
  +-
   drivers/usb/musb/musb_host.h |2 +-
   3 files changed, 116 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
  index 60b41cc..91ac166 100644
  --- a/drivers/usb/musb/musb_core.c
  +++ b/drivers/usb/musb/musb_core.c
  @@ -1431,6 +1431,20 @@ static int musb_core_init(u16 musb_type, struct 
  musb *musb)
 
/* log release info */
musb-hwvers = musb_read_hwvers(mbase);
  +
  +#ifndef CONFIG_MUSB_PIO_ONLY
  + /*
  +  * The DMA engine in RTL1.8 and above cannot handle
  +  * DMA addresses that are not aligned to a 4 byte boundary.
  +  * For such engine implemented (un)map_urb_for_dma hooks.
  +  * Do not use these hooks for RTL1.8
  +  */
  + if (musb-hwvers  MUSB_HWVERS_1800) {
 
  if you move this check to map/unmap and always return error if this is
  true, you can avoid removing 'const' from our struct hc_driver. Would
  that work ?
 
 If we return an error in map/unmap callbacks, this will break urb 
 transferring,
 however I can call core function usb_hcd_(un)map_urb_for_dma() instead of
 returning the error (and that is default behavior if we do not have
 map/unmap callbacks
 set for the hc driver) so I can avoid removing 'const' from our struct
 hc_driver and this will work.
 The side effect will be only in small overhead for this path.
 
 So, will be this OK for you? I will send v3 in this case.

should be alright. Thanks

-- 
balbi


signature.asc
Description: Digital signature

Re: xhci page fault panic on Ubuntu kernel with HP desktop hardware

2013-03-28 Thread Yann Sionneau


Le 27/03/2013 23:40, Sarah Sharp a écrit :
 On Wed, Mar 27, 2013 at 02:24:24PM +0100, Yann Sionneau wrote:
 Le 26/03/2013 17:30, Sarah Sharp a écrit :
 On Tue, Mar 26, 2013 at 12:11:13PM +0100, Yann Sionneau wrote:
 Le 25/03/2013 19:13, Sarah Sharp a écrit :
 On Mon, Mar 25, 2013 at 05:43:40PM +0100, Yann Sionneau wrote:
 Please compile with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING
 turned on.  If you can't reproduce your bug on 3.8.4, turn them off,
 recompile, and see if you can reproduce it again.
 We just reproduced it with a 3.8.4 kernel with CONFIG_USB_DEBUG and
 CONFIG_USB_XHCI_HCD_DEBUGGING turned on.
 I am sending you the picture as attached file.
 Thanks.  I need more information though.  Can you follow these
 directions for setting up netconsole, and send me the full log file from
 boot to when the machine hangs?  Make sure the kernel still has
 CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on.

 http://kernelnewbies.org/KernelDebug

 That will help me figure out what command is causing the null pointer
 dereference.

 Sarah Sharp
 Hi Sarah,

 I've got some xhci debug traces:
 ysionneau@yann-HP:~/ssd/olivier$ nc -l -u  | tee 
 ~/ssd/olivier/dmesg-`date +%Y-%m-%d-%H-%M`.txt
 That was helpful, thanks!  However, next time will you send the log file
 as an attachment?  Your mail client is wrapping lines, and it makes it
 hard for me to read the logs.

 [ 1375.761963] xhci_hcd :00:14.0: Timeout while waiting for address 
 device command
 [ 1375.761974] xhci_hcd :00:14.0: Abort command ring
 [ 1375.762029] BUG: unable to handle kernel NULL pointer dereference at 
 0018
 [ 1375.762102] IP: [c14854b0] xhci_stream_id_to_ring+0x30/0x40
 Ok, so it may be to be related to xHCI command cancellation.  That's an
 area that I've been working on, and I've run across a couple bugs.  I'm
 not sure yet if they're the particular bug you're running into though.

 Could you please test this branch, and let me know if it solves your
 issue?  If not please use netconsole to capture the output again.

 git clone git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git -b 
 ful-command-queue-fixes-simple

 Sarah Sharp

Here it is (it does seem to crash in a different function this time though)!
Sorry for the line wrapping!

Thanks again for your help :)

Best regards,

-- 
Yann Sionneau
[ 1700.820864] sd 7:0:0:0: [sdb] No Caching mode page present
[ 1700.820874] sd 7:0:0:0: [sdb] Assuming drive cache: write through
[ 1700.824501] sd 7:0:0:0: [sdb] No Caching mode page present
[ 1700.824512] sd 7:0:0:0: [sdb] Assuming drive cache: write through
[ 1700.828896] sd 7:0:0:0: [sdb] No Caching mode page present
[ 1700.828906] sd 7:0:0:0: [sdb] Assuming drive cache: write through
[ 1755.917600] xhci_hcd :00:14.0: ERROR: unexpected command completion code 
0x18.
[ 1756.121523] usb 3-4: device not accepting address 27, error -22
[ 1761.235939] xhci_hcd :00:14.0: @349997e0 349990a0  
1800 1b008401
[ 1761.236332] BUG: unable to handle kernel NULL pointer dereference at 0024
[ 1761.236401] IP: [c1496c0e] handle_cmd_completion+0x91e/0xaf0
[ 1761.236453] *pdpt =  *pde = f0009bd0f0009bd0 
[ 1761.236504] Oops: 0002 [#1] SMP 
[ 1761.236538] Modules linked in: nls_iso8859_1 nls_cp437 vfat fat usb_storage 
netconsole configfs rfcomm bnep bluetooth parport_pc ppdev binfmt_misc coretemp 
kvm radeon ttm drm_kms_helper drm snd_hda_codec_realtek snd_hda_intel 
snd_hda_codec aesni_intel snd_hwdep ablk_helper snd_pcm snd_seq_midi 
snd_rawmidi snd_seq_midi_event mac_hid snd_seq tpm_infineon snd_timer 
snd_seq_device lpc_ich snd hp_wmi sparse_keymap cryptd video psmouse serio_raw 
i2c_algo_bit tpm_tis lp wmi soundcore mei snd_page_alloc lrw aes_i586 xts 
gf128mul parport microcode e1000e ptp pps_core
[ 1761.237111] Pid: 0, comm: swapper/0 Not tainted 3.9.0-rc4+ #1 
Hewlett-Packard HP Compaq Elite 8300 SFF/3397
[ 1761.237178] EIP: 0060:[c1496c0e] EFLAGS: 00210006 CPU: 0
[ 1761.237219] EIP is at handle_cmd_completion+0x91e/0xaf0
[ 1761.237258] EAX:  EBX:  ECX: 0007 EDX: 
[ 1761.237302] ESI: f49997e0 EDI: f49d8000 EBP: f5009ef8 ESP: f5009eac
[ 1761.237347]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 1761.237387] CR0: 80050033 CR2: 0024 CR3: 019b7000 CR4: 001407f0
[ 1761.237431] DR0:  DR1:  DR2:  DR3: 
[ 1761.237475] DR6: 0ff0 DR7: 0400
[ 1761.237505] Process swapper/0 (pid: 0, ti=f5008000 task=c18791a0 
task.ti=c186c000)
[ 1761.237557] Stack:
[ 1761.237575]  f504a864 c18172ac  f492 f492179c f5009ed8 f5009ecc 
0001
[ 1761.237661]  f49990a0  0014   f5009f00 c1074435 
f49d8000
[ 1761.237749]  f49d8000 f49997e0 0001 f5009f84 c14978e1 f5009f50 c13ad5cb 

[ 1761.237837] Call Trace:
[ 1761.237864]  [c1074435] ? __wake_up+0x45/0x60
[ 1761.237899]  [c14978e1] xhci_irq+0x7b1/0x16c0
[ 1761.237937]  [c13ad5cb] ? credit_entropy_bits+0xfb/0x1d0
[

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

On Thu, Mar 28, 2013 at 11:28:32AM +0200, Alexander Shishkin wrote:
 Svetoslav Neykov svetos...@neykov.name writes:
 
  Convert between big-endian and little-endian format when accessing the usb 
  controller structures which are little-endian by specification.
  Fix cases where the little-endian memory layout is taken for granted.
  The patch doesn't have any effect on the already supported
  little-endian architectures.
 
 Applied to my branch of things that are aiming at v3.10. Next time
 please make sure that it applies cleanly.

I am currently rebasing my fix/cleanup/feature patches against your
ci-for-greg and realised that this patch missed to fix debug.c with
cpu_le_32 action. Is someone volunteering to cook a patch?

Thanks,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

2013-03-28 Thread Marc Kleine-Budde

On 03/28/2013 10:28 AM, Alexander Shishkin wrote:
 Svetoslav Neykov svetos...@neykov.name writes:
 
 Convert between big-endian and little-endian format when accessing
 the usb controller structures which are little-endian by
 specification. Fix cases where the little-endian memory layout is
 taken for granted. The patch doesn't have any effect on the already
 supported little-endian architectures.

Has anyone tested how the cpu_to_le32 and vice versa effects the
load/store operations? Does the compiler generate full 32 bit accesses
on mips (and big endian arm) or is a byte-shift-or pattern used?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: 3.8.4: ohci question

On Wed, 27 Mar 2013, Bjorn Helgaas wrote:

 [+cc linux-usb]
 
 On Wed, Mar 27, 2013 at 11:37 AM, Udo van den Heuvel udo...@xs4all.nl wrote:
  Hello,
 
  When my dmesg gives me a growing number of lines like the one below,
  what is going on?
 
   ohci_hcd :00:12.0: urb 88023025c500 path 2 ep1in 6c16 cc 6
  -- status -71
 
  Please let me know!

-71 errors indicate a low-level problem on the USB bus.  Generally it
means that the device isn't sending any packets.  Maybe the device's
firmware has crashed or maybe it has been disconnected.

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 V2 3/6] usb: ehci: mv_ehci: remove unused clock

On Thu, 28 Mar 2013, Chao Xie wrote:

 hi, Alan
 
 This is the patch for EHCI clock fix. Can you help to review and ack it? 
 Thanks.
 
 On Mon, Mar 25, 2013 at 3:06 PM, Chao Xie chao@marvell.com wrote:
  The origianl understanding of clock is wrong. The EHCI controller
  only have one clock input.
  Passing clock name by pdata is wrong. The clock is defined by device
  iteself.
 
  Signed-off-by: Chao Xie chao@marvell.com
  ---
   drivers/usb/host/ehci-mv.c |   35 ++-
   1 files changed, 10 insertions(+), 25 deletions(-)
 
  diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
  index 3804820..6bad41a 100644
  --- a/drivers/usb/host/ehci-mv.c
  +++ b/drivers/usb/host/ehci-mv.c
  @@ -33,25 +33,17 @@ struct ehci_hcd_mv {
 
  struct mv_usb_platform_data *pdata;
 
  -   /* clock source and total clock number */
  -   unsigned int clknum;
  -   struct clk *clk[0];
  +   struct clk *clk;
   };
 
   static void ehci_clock_enable(struct ehci_hcd_mv *ehci_mv)
   {
  -   unsigned int i;
  -
  -   for (i = 0; i  ehci_mv-clknum; i++)
  -   clk_prepare_enable(ehci_mv-clk[i]);
  +   clk_prepare_enable(ehci_mv-clk);
   }
 
   static void ehci_clock_disable(struct ehci_hcd_mv *ehci_mv)
   {
  -   unsigned int i;
  -
  -   for (i = 0; i  ehci_mv-clknum; i++)
  -   clk_disable_unprepare(ehci_mv-clk[i]);
  +   clk_disable_unprepare(ehci_mv-clk);
   }
 
   static int mv_ehci_enable(struct ehci_hcd_mv *ehci_mv)
  @@ -144,9 +136,8 @@ static int mv_ehci_probe(struct platform_device *pdev)
  struct ehci_hcd *ehci;
  struct ehci_hcd_mv *ehci_mv;
  struct resource *r;
  -   int clk_i, retval = -ENODEV;
  +   int retval = -ENODEV;
  u32 offset;
  -   size_t size;
 
  if (!pdata) {
  dev_err(pdev-dev, missing platform_data\n);
  @@ -160,8 +151,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
  if (!hcd)
  return -ENOMEM;
 
  -   size = sizeof(*ehci_mv) + sizeof(struct clk *) * pdata-clknum;
  -   ehci_mv = devm_kzalloc(pdev-dev, size, GFP_KERNEL);
  +   ehci_mv = devm_kzalloc(pdev-dev, sizeof(*ehci_mv), GFP_KERNEL);
  if (ehci_mv == NULL) {
  dev_err(pdev-dev, cannot allocate ehci_hcd_mv\n);
  retval = -ENOMEM;
  @@ -172,16 +162,11 @@ static int mv_ehci_probe(struct platform_device *pdev)
  ehci_mv-pdata = pdata;
  ehci_mv-hcd = hcd;
 
  -   ehci_mv-clknum = pdata-clknum;
  -   for (clk_i = 0; clk_i  ehci_mv-clknum; clk_i++) {
  -   ehci_mv-clk[clk_i] =
  -   devm_clk_get(pdev-dev, pdata-clkname[clk_i]);
  -   if (IS_ERR(ehci_mv-clk[clk_i])) {
  -   dev_err(pdev-dev, error get clck \%s\\n,
  -   pdata-clkname[clk_i]);
  -   retval = PTR_ERR(ehci_mv-clk[clk_i]);
  -   goto err_clear_drvdata;
  -   }
  +   ehci_mv-clk = devm_clk_get(pdev-dev, NULL);
  +   if (IS_ERR(ehci_mv-clk)) {
  +   dev_err(pdev-dev, error getting clock\n);
  +   retval = PTR_ERR(ehci_mv-clk);
  +   goto err_clear_drvdata;
  }
 
  r = platform_get_resource_byname(pdev, IORESOURCE_MEM, phyregs);

Looks okay to me.

Acked-by: Alan Stern st...@rowland.harvard.edu

--
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: Piggy-backing new hardware using old usb-serial

On Wed, 2013-03-27 at 16:13 -0700, Greg KH wrote:
 On Wed, Mar 27, 2013 at 09:28:11PM +0100, Wesley W. Terpstra wrote:
  On Wed, 2013-03-27 at 13:06 -0700, Greg KH wrote:
Our current prototypes borrow the Sierra VID
   And the USB-IF might revoke your vendor id, if they find you shipping a
   device with a different vendor id than the one you have been assigned.
  
  One of the reasons we borrowed that VID is that we do not currently have
  a VID assigned. We are still deciding whether it is worth joining the
  USB-IF just to get a vendor ID for a few thousands of devices.
  
  I am of the opinion that it is, but I cannot sign the membership forms
  on behalf of GSI. We probably will end up buying a VID soon.
  
   Why not just add your device id to an existing driver, sending in a
   patch to do so.  All distros will pick that up and then your device will
   just work on all Linux distros.
  
  I was under the impression that drivers in the linux mainline had to be
  for hardware that was widely available.
 
 Not true at all, we take drivers for _anything_ :)
 
  I take it then, that just adding support to an existing driver is
  acceptable?
 
 That's also ok, if you are using the same chip that the driver supports.
 
  That wouldn't address people with older linux distributions, but would
  definitely be a good long term solution. It's really a shame there is no
  USB-IF standard for usb-serial... then things would even potentially
  work out of the box on windows.
 
 You can use the CDC-ACM driver, as you have pointed out, which should
 allow your device to work on any OS with no driver needed, so maybe just
 use that instead.  There's no need to support modem commands in your
 device if you use it.

Greg's right, there's no reason not to use cdc-acm if you want to do
that, since not all cdc-acm devices are modems.  If you get a USBIF
vendor ID, then I'll happily add your device to the ModemManager probing
blacklist too.  That's kind of hard to do without a unique USB VID/PID
though :)

Dan

What driver should I target?
   What chip do you use for your device?
  
  The device I am concerned about right now has an Altera arria2 connected
  to a cy7c68013a-56baxc (fx2lp). We have several form factor variations.
  A few have FTDI chips where I don't need to care, but can also do less.
 
 If you use the ftdi chip, then use the ftdi driver, and add the device
 id that ftdi gives you to it.
 
 Hope this helps,
 
 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


--
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: Linux USB file storage gadget with new UDC

On Thu, 28 Mar 2013, victor yeo wrote:

 There is a problem with SCSI_READ_10 command if looking at usbmon. I
 pasted the usbmon log that starts from SCSI_READ_10. Basically, the
 SCSI_READ_10 was received by gadget, processed, sent CSW, followed by
 control packets. Then another SCSI_READ_10, sent CSW, followed by
 control packets. Then another SCSI_READ_10, but CSW is not received by
 host.
 
 There must be problems in the UDC driver. CSW is sent by the UDC
 driver but it is not received by the Linux host.
 
 Thanks,
 victor
 
 f59e13c0 3246885432 S Bo:2:046:1 -115 31 = 55534243 0c00 0010
 8a28  0008  00
 f59e13c0 3246885582 C Bo:2:046:1 0 31 
 f59e15c0 3246885594 S Bi:2:046:1 -115 4096 
 f59e15c0 3247150217 C Bi:2:046:1 -75 0

This is the problem.  -75 is -EOVERFLOW; it means the host received a
packet that was larger than expected.  Since the maxpacket value for a
bulk-in endpoint is 512, your UDC must be sending bulk DATA packets
with more than 512 bytes.

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/RFC] uvcvideo: Disable USB autosuspend for Creative Live! Cam Optia AF

On Thu, 28 Mar 2013, Laurent Pinchart wrote:

 The camera fails to start video streaming after having been autosuspend.
 Add a new quirk to selectively disable autosuspend for devices that
 don't support it.
 
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/usb/uvc/uvc_driver.c | 14 +-
  drivers/media/usb/uvc/uvcvideo.h   |  1 +
  2 files changed, 14 insertions(+), 1 deletion(-)
 
 I've tried to set the reset resume quirk for this device in the USB core but
 the camera still failed to start video streaming after having been
 autosuspended. Regardless of whether the reset resume quirk was set, it would
 respond to control messages but wouldn't send video data.

Presumably the camera won't work after a system suspend, either.

 This solution below is a hack, but I'm not sure what else I can try. Crazy
 ideas are welcome.

It's not a hack; it's a normal use for a quirk flag.  Of course, if you 
can figure out what's wrong with the camera and see how to fix it, that 
would be best.

How does the camera perform on a Windows system after being put to
sleep and then woken up?

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 2/4] usb: introduce usb force power off mechanism

On Thu, 28 Mar 2013, Lan Tianyu wrote:

  How long do you think the power should remain turned off?  This code
  will leave it off for only a few milliseconds at most.  That may not
  even be long enough for the voltage to drop all the way to 0.
 
  The delay probably should be at least 100 ms.  Maybe more, I don't
  know.
 I think this depends on the device. I don't find the answer on spec.
 I find the cool down delay of over-current for port is 100ms
 and one for hub is 500ms. Could we reference these delay?
 500ms would be safe.

Okay, then use 500 ms.

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: Piggy-backing new hardware using old usb-serial

2013-03-28 Thread Wesley W. Terpstra

On Thu, 2013-03-28 at 09:39 -0500, Dan Williams wrote:
 Greg's right, there's no reason not to use cdc-acm if you want to do
 that, since not all cdc-acm devices are modems.  If you get a USBIF
 vendor ID, then I'll happily add your device to the ModemManager probing
 blacklist too.

Yes, the cdc-acm approach has the distinct advantage of not having to
write one driver for each OS.

What does ModemManager probing consist of? Sending a few ATx commands to
see what the device is? On the interactive console that wouldn't hurt,
but on the proprietary data channel it could break things.

I assume that it poses no problem to the linux cdc enumeration if my
device reports two data interfaces (with two endpoints each).

Once I have added an interrupt endpoint and ignore the class specific
requests to meet the CDC-ACM interface and have a valid VID+PID pair,
I'll get back to you.

In case anyone else cares, I found some nice Dutch folks who resell PIDs
under their VID for cheap:
http://www.mcselec.com/index.php?page=shop.product_detailsflypage=shop.flypageproduct_id=92option=com_phpshopItemid=1

I will probably go this route for now.


--
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: ftdi_sio: Add support for Mitsubishi FX-USB-AW/-BD

2013-03-28 Thread Konstantin Holoborodko

It enhances the driver for FTDI-based USB serial adapters
to recognize Mitsubishi Electric Corp. USB/RS422 Converters
as FT232BM chips and support them.
https://search.meau.com/?q=FX-USB-AW

Signed-off-by: Konstantin Holoborodko klh.ker...@gmail.com
Tested-by: Konstantin Holoborodko klh.ker...@gmail.com
---
 drivers/usb/serial/ftdi_sio.c |1 +
 drivers/usb/serial/ftdi_sio_ids.h |7 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index d4809d5..9886180 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -640,6 +640,7 @@ static struct usb_device_id id_table_combined [] = {
{ USB_DEVICE(FTDI_VID, FTDI_RM_CANVIEW_PID) },
{ USB_DEVICE(ACTON_VID, ACTON_SPECTRAPRO_PID) },
{ USB_DEVICE(CONTEC_VID, CONTEC_COM1USBH_PID) },
+   { USB_DEVICE(MITSUBISHI_VID, MITSUBISHI_FXUSB_PID) },
{ USB_DEVICE(BANDB_VID, BANDB_USOTL4_PID) },
{ USB_DEVICE(BANDB_VID, BANDB_USTL4_PID) },
{ USB_DEVICE(BANDB_VID, BANDB_USO9ML2_PID) },
diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index 9d359e1..e79861e 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -584,6 +584,13 @@
 #define CONTEC_COM1USBH_PID0x8311  /* COM-1(USB)H */
 
 /*
+ * Mitsubishi Electric Corp. (http://www.meau.com)
+ * Submitted by Konstantin Holoborodko
+ */
+#define MITSUBISHI_VID 0x06D3
+#define MITSUBISHI_FXUSB_PID   0x0284 /* USB/RS422 converters: FX-USB-AW/-BD */
+
+/*
  * Definitions for BB Electronics products.
  */
 #define BANDB_VID  0x0856  /* BB Electronics Vendor ID */
-- 
1.7.9.5

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

Re: Piggy-backing new hardware using old usb-serial

On Thu, 2013-03-28 at 15:58 +0100, Wesley W. Terpstra wrote:
On Thu, 2013-03-28 at 09:39 -0500, Dan Williams wrote:
Greg's right, there's no reason not to use cdc-acm if you want to do
that, since not all cdc-acm devices are modems. If you get a USBIF
vendor ID, then I'll happily add your device to the ModemManager probing
blacklist too.

Yes, the cdc-acm approach has the distinct advantage of not having to
write one driver for each OS.

What does ModemManager probing consist of? Sending a few ATx commands to
see what the device is? On the interactive console that wouldn't hurt,
but on the proprietary data channel it could break things.

ModemManager looks at a certain set of serial devices and probes each
TTY the device provides with AT commands, Qualcomm DIAG (QCDM) requests,
Qualcomm MSM Interface (QMI) requests, and potentially other protocols.
It does filter devices based on driver name (eg, 'sierra' is certainly a
modem while 'whiteheat' certainly is not).

Probing is necessary because there are a *ton* of devices out there,
manufacturers often use the same VID/PID for wildly different devices
(even completely different modem chipsets *cough* TCT Mobile *cough*),
or don't bother to use a unique VID/PID at all. Sometimes firmware
updates change the USB interface layout, so what used to be an AT port
is now a DIAG port, or even the PID has changed. We've seen it all.

We've tried the tag-every-known-modem-by-USB-details approach before,
and that simply isn't scalable, especially when you throw tethering your
handset into the mix. There are even more phones than there are USB
dongles.

Windows doesn't have this problem, because your dongle provider or
cellular provider provides the drivers for their specific device, and
you're never expected to install more than one set of drivers at a time.
Get a new device? Great! Install the connection manager specific for
that device and uninstall the old one.

Probing does break things on devices that don't expect it, like UPSs or
other stuff, which is why we have a blacklist for stuff we know MM
shouldn't probe. Happy to add to it.

Dan

I assume that it poses no problem to the linux cdc enumeration if my
device reports two data interfaces (with two endpoints each).

Once I have added an interrupt endpoint and ignore the class specific
requests to meet the CDC-ACM interface and have a valid VID+PID pair,
I'll get back to you.

In case anyone else cares, I found some nice Dutch folks who resell PIDs
under their VID for cheap:
http://www.mcselec.com/index.php?page=shop.product_detailsflypage=shop.flypageproduct_id=92option=com_phpshopItemid=1

I will probably go this route for now.

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

Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework

2013-03-28 Thread Stephen Warren

On 03/27/2013 11:43 PM, Kishon Vijay Abraham I wrote:
 The PHY framework 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

 diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt 
 b/Documentation/devicetree/bindings/phy/phy-bindings.txt

 +This document explains only the dt data binding. For general information 
 about
 +PHY subsystem refer Documentation/phy.txt
 +
 +PHY device node
 +===
 +
 +Optional Properties:
 +#phy-cells:  Number of cells in a PHY specifier;  The meaning of all those
 + cells is defined by the binding for the phy node. However
 + in-order to return the correct PHY, the PHY susbsystem
 + requires the first cell always refers to the port.

Why impose that restriction? Other DT bindings do not.

This is typically implemented by having each provider driver implement a
.of_xlate() operation, which parses all of the specifier cells, and
returns the ID of the object it represents. This allows bindings to use
whatever arbitrary representation they want.

For the common/simple cases where #phy-cells==0, or #phy-cells==1 and
directly represents the PHY ID, the PHY core can provide an
implementation of that common .of_xlate() function, which PHY provider
drivers can simply plug in as their .of_xlate() function.

 +This property is optional because it is needed only for the case where a
 +single IP implements multiple PHYs.

The property should always exist so that the DT-parsing code in the PHY
core can always validate exactly how many cells are present in the PHY
specifier.

 +
 +For example:
 +
 +phys: phy {
 +compatible = xxx;
 +reg1 = ...;
 +reg2 = ...;
 +reg3 = ...;

3 separate reg values should be 3 separate entries in a single reg
property, not 3 separate reg properties, with non-standard names.

 +.
 +.
 +#phy-cells = 1;
 +.
 +.
 +};
 +
 +That node describes an IP block that implements 3 different PHYs. In order to
 +differentiate between these 3 PHYs, an additonal specifier should be given
 +while trying to get a reference to it. (The PHY subsystem assumes the
 +specifier is port id).

A single DT node would typically represent a single HW IP block, and
hence typically have a single reg property. If there are 3 separate HW
IP blocks, and their register aren't interleaved, and hence can be
represented by 3 separate reg values, then I'd typically expect to see 3
separate DT nodes, one for each independent register range.

The only case where I'd expect a single DT node to provide multipe PHYs,
is when a single HW IP block actually implements multiple PHYs /and/ the
registers for those 3 PHYs are interleaved (or share bits in the same
registers) such that each PHY can't be represented by a separate
non-overlapping reg property.

 +example1:
 +phys: phy {

How about:

Example 1:

usb1: usb@xxx {

 +};
 +This node represents a controller that uses two PHYs one for usb2 and one for

Blank line after }?

 +usb3. The controller driver can get the appropriate PHY either by using
 +devm_of_phy_get/of_phy_get by passing the correct index or by using
 +of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
 +*phy-names*.

Discussions of Linux-specific driver APIs should be in the Linux API
documentation, not the DT binding documentation, which is supposed to be
OS-agnostic. Instead, perhaps say:

Individual bindings must specify the required set of entries the phys
property. Bindings must also specify either a required order for those
entries in the phys property, or specify required set of names that must
be present in the phy-names property, in which case the order is arbitrary.

 +example2:
 +phys: phy {

How about:

Example 2:

usb2: usb@yyy {

 +This node represents a controller that uses one of the PHYs which is defined
 +previously. Note that the phy handle has an additional specifier 1 to
 +differentiate between the three PHYs. For this case, the controller driver
 +should use of_phy_get_with_args/devm_of_phy_get_with_args.

I think tha last sentence should be removed, and perhaps the previous
sentence extended with:

, as required by #phy-cells = 1 in the PHY provider node.

 diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c

 +subsys_initcall(phy_core_init);

Why not make that module_init(); device probe() ordering should be
handled using -EPROBE_DEFER these days, so the exact initcall used
doesn't really matter, and hence it'd be best to use the most common one
rather than something unusual.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

Michael Grzeschik m...@pengutronix.de writes:

 On Thu, Mar 28, 2013 at 11:28:32AM +0200, Alexander Shishkin wrote:
 Svetoslav Neykov svetos...@neykov.name writes:
 
  Convert between big-endian and little-endian format when accessing the usb 
  controller structures which are little-endian by specification.
  Fix cases where the little-endian memory layout is taken for granted.
  The patch doesn't have any effect on the already supported
  little-endian architectures.
 
 Applied to my branch of things that are aiming at v3.10. Next time
 please make sure that it applies cleanly.

 I am currently rebasing my fix/cleanup/feature patches against your
 ci-for-greg and realised that this patch missed to fix debug.c with
 cpu_le_32 action. Is someone volunteering to cook a patch?

Nice catch. If nobody beats me to it, I'll probably just amend that
patch in my branch in a couple of hours.

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

Re: [PATCH 2/4] usb: introduce usb force power off mechanism


On 2013/3/28 22:46, Alan Stern wrote:

On Thu, 28 Mar 2013, Lan Tianyu wrote:


How long do you think the power should remain turned off?  This code
will leave it off for only a few milliseconds at most.  That may not
even be long enough for the voltage to drop all the way to 0.

The delay probably should be at least 100 ms.  Maybe more, I don't
know.

I think this depends on the device. I don't find the answer on spec.
I find the cool down delay of over-current for port is 100ms
and one for hub is 500ms. Could we reference these delay?
500ms would be safe.


Okay, then use 500 ms.

Alan Stern


Ok.

About the path usb: Add usb port system pm support, do you think it's
ok?

On 2013/3/28 2:47, Alan Stern wrote:

What happens if there's no device plugged in to the port, but the hub
is enabled for remote wakeup?  How will the hub be able to detect a
plug-in event if the port isn't powered?

Alan Stern


Hi Alan:
 Great thanks for your review.
 The hub will not detect the new devices. From my opinion, this
depends on the user space since the port only will be powered off when
pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset
the flag, losing plug-in event should have been took into account.




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

[PATCH 1/2 v3] usbnet: allow status interrupt URB to always be active

Some drivers (sierra_net) need the status interrupt URB
active even when the device is closed, because they receive
custom indications from firmware.  Add functions to refcount
the status interrupt URB submit/kill operation so that
sub-drivers and the generic driver don't fight over whether
the status interrupt URB is active or not.

A sub-driver can call usbnet_status_start() at any time, but
the URB is only submitted the first time the function is
called.  Likewise, when the sub-driver is done with the URB,
it calls usbnet_status_stop() but the URB is only killed when
all users have stopped it.  The URB is still killed and
re-submitted for suspend/resume, as before, with the same
refcount it had at suspend.

Signed-off-by: Dan Williams d...@redhat.com
---
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 51f3192..6431a03 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -252,6 +252,43 @@ static int init_status (struct usbnet *dev, struct 
usb_interface *intf)
return 0;
 }
 
+/* Submit the interrupt URB if it hasn't been submitted yet */
+int usbnet_status_start(struct usbnet *dev, gfp_t mem_flags)
+{
+   int ret = 0;
+
+   /* Only drivers that implement a status hook should call this */
+   BUG_ON(dev-interrupt == NULL);
+
+   if (test_bit (EVENT_DEV_ASLEEP, dev-flags))
+   return -EINVAL;
+
+   mutex_lock (dev-interrupt_mutex);
+   if (++dev-interrupt_count == 1)
+   ret = usb_submit_urb (dev-interrupt, mem_flags);
+   dev_dbg(dev-udev-dev, incremented interrupt URB count to %d\n,
+   dev-interrupt_count);
+   mutex_unlock (dev-interrupt_mutex);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(usbnet_status_start);
+
+/* Kill the interrupt URB if all submitters want it killed */
+void usbnet_status_stop(struct usbnet *dev)
+{
+   if (dev-interrupt) {
+   mutex_lock (dev-interrupt_mutex);
+   BUG_ON(dev-interrupt_count == 0);
+   if (dev-interrupt_count  --dev-interrupt_count == 0)
+   usb_kill_urb(dev-interrupt);
+   dev_dbg(dev-udev-dev,
+   decremented interrupt URB count to %d\n,
+   dev-interrupt_count);
+   mutex_unlock (dev-interrupt_mutex);
+   }
+}
+EXPORT_SYMBOL_GPL(usbnet_status_stop);
+
 /* Passes this packet up the stack, updating its accounting.
  * Some link protocols batch packets, so their rx_fixup paths
  * can return clones as well as just modify the original skb.
@@ -725,7 +762,7 @@ int usbnet_stop (struct net_device *net)
if (!(info-flags  FLAG_AVOID_UNLINK_URBS))
usbnet_terminate_urbs(dev);
 
-   usb_kill_urb(dev-interrupt);
+   usbnet_status_stop(dev);
 
usbnet_purge_paused_rxq(dev);
 
@@ -787,7 +824,7 @@ int usbnet_open (struct net_device *net)
 
/* start any status interrupt transfer */
if (dev-interrupt) {
-   retval = usb_submit_urb (dev-interrupt, GFP_KERNEL);
+   retval = usbnet_status_start(dev, GFP_KERNEL);
if (retval  0) {
netif_err(dev, ifup, dev-net,
  intr submit %d\n, retval);
@@ -1430,6 +1467,8 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
dev-delay.data = (unsigned long) dev;
init_timer (dev-delay);
mutex_init (dev-phy_mutex);
+   mutex_init (dev-interrupt_mutex);
+   dev-interrupt_count = 0;
 
dev-net = net;
strcpy (net-name, usb%d);
@@ -1585,9 +1624,13 @@ int usbnet_resume (struct usb_interface *intf)
int retval;
 
if (!--dev-suspend_count) {
-   /* resume interrupt URBs */
-   if (dev-interrupt  test_bit(EVENT_DEV_OPEN, dev-flags))
-   usb_submit_urb(dev-interrupt, GFP_NOIO);
+   /* resume interrupt URBs if they were submitted at suspend */
+   if (dev-interrupt) {
+   mutex_lock (dev-interrupt_mutex);
+   if (dev-interrupt_count)
+   usb_submit_urb(dev-interrupt, GFP_NOIO);
+   mutex_unlock (dev-interrupt_mutex);
+   }
 
spin_lock_irq(dev-txq.lock);
while ((res = usb_get_from_anchor(dev-deferred))) {
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 0e5ac93..d71f44c 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -56,6 +56,8 @@ struct usbnet {
struct sk_buff_head done;
struct sk_buff_head rxq_pause;
struct urb  *interrupt;
+   unsignedinterrupt_count;
+   struct mutexinterrupt_mutex;
struct usb_anchor   deferred;
struct tasklet_struct   bh;
 
@@ -246,4 +248,7 @@ extern int usbnet_nway_reset(struct net_device *net);

[PATCH 2/2 v3] sierra_net: keep status interrupt URB active

The driver and firmware sync up through SYNC messages, and the
firmware's affirmative reply to these SYNC messages appears to be the
Reset indication received via the status interrupt endpoint.  Thus the
driver needs the status interrupt endpoint always active so that the
Reset indication can be received even if the netdev is closed, which is
the case right after device insertion.

Signed-off-by: Dan Williams d...@redhat.com
---
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 79ab243..c707225 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -427,6 +427,13 @@ static void sierra_net_dosync(struct usbnet *dev)
 
dev_dbg(dev-udev-dev, %s, __func__);
 
+   /* The SIERRA_NET_HIP_MSYNC_ID command appears to request that the
+* firmware restart itself.  After restarting, the modem will respond
+* with the SIERRA_NET_HIP_RESTART_ID indication.  The driver continues
+* sending MSYNC commands every few seconds until it receives the
+* RESTART event from the firmware
+*/
+
/* tell modem we are ready */
status = sierra_net_send_sync(dev);
if (status  0)
@@ -705,6 +712,9 @@ static int sierra_net_bind(struct usbnet *dev, struct 
usb_interface *intf)
/* set context index initially to 0 - prepares tx hdr template */
sierra_net_set_ctx_index(priv, 0);
 
+   /* prepare sync message template */
+   memcpy(priv-sync_msg, sync_tmplate, sizeof(priv-sync_msg));
+
/* decrease the rx_urb_size and max_tx_size to 4k on USB 1.1 */
dev-rx_urb_size  = SIERRA_NET_RX_URB_SIZE;
if (dev-udev-speed != USB_SPEED_HIGH)
@@ -740,11 +750,6 @@ static int sierra_net_bind(struct usbnet *dev, struct 
usb_interface *intf)
kfree(priv);
return -ENODEV;
}
-   /* prepare sync message from template */
-   memcpy(priv-sync_msg, sync_tmplate, sizeof(priv-sync_msg));
-
-   /* initiate the sync sequence */
-   sierra_net_dosync(dev);
 
return 0;
 }
@@ -767,8 +772,9 @@ static void sierra_net_unbind(struct usbnet *dev, struct 
usb_interface *intf)
netdev_err(dev-net,
usb_control_msg failed, status %d\n, status);
 
-   sierra_net_set_private(dev, NULL);
+   usbnet_status_stop(dev);
 
+   sierra_net_set_private(dev, NULL);
kfree(priv);
 }
 
@@ -909,6 +915,24 @@ static const struct driver_info sierra_net_info_direct_ip 
= {
.tx_fixup = sierra_net_tx_fixup,
 };
 
+static int
+sierra_net_probe (struct usb_interface *udev, const struct usb_device_id *prod)
+{
+   int ret;
+
+   ret = usbnet_probe(udev, prod);
+   if (ret == 0) {
+   struct usbnet *dev = usb_get_intfdata(udev);
+
+   ret = usbnet_status_start(dev, GFP_KERNEL);
+   if (ret == 0) {
+   /* Interrupt URB now set up; initiate sync sequence */
+   sierra_net_dosync(dev);
+   }
+   }
+   return ret;
+}
+
 #define DIRECT_IP_DEVICE(vend, prod) \
{USB_DEVICE_INTERFACE_NUMBER(vend, prod, 7), \
.driver_info = (unsigned long)sierra_net_info_direct_ip}, \
@@ -931,7 +955,7 @@ MODULE_DEVICE_TABLE(usb, products);
 static struct usb_driver sierra_net_driver = {
.name = sierra_net,
.id_table = products,
-   .probe = usbnet_probe,
+   .probe = sierra_net_probe,
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,


--
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/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-28 Thread Matthijs Kooijman

Hi Paul,

while continuing this patch, I stumbled upon a bit of code which doesn't
make sense to me. In dwc2_dump_global_registers is the following bit:

if (hsotg-core_params-en_multiple_tx_fifo = 0) {
ep_num = hsotg-hwcfg4  GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT 
 GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK 
 GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT;
txfsiz = DPTXFSIZ;
} else {
ep_num = hsotg-hwcfg4  GHWCFG4_NUM_IN_EPS_SHIFT 
 GHWCFG4_NUM_IN_EPS_MASK  GHWCFG4_NUM_IN_EPS_SHIFT;
txfsiz = DIENPTXF;
}

for (i = 0; i  ep_num; i++) {
addr = hsotg-regs + DPTXFSIZN(i + 1);
dev_dbg(hsotg-dev, %s[%d] @0x%08lX : 0x%08X\n, txfsiz, i + 1,
(unsigned long)addr, readl(addr));
}

There's a number of things:
 - In the else above, the register name is set to DIENPTXF, but the values are
   always read from DPTXFSIZN?
 - According to my docs for GHWCFG4_NUM_IN_EPS, the actual number of endpoints
   is the value read + 1.
 - This would result in 1-16 eps, but FIFO numbers are from 1-15 for DPTXFSIZ.

Do you know what the deal is here? Or, given that this is only debug output for
device mode only, shall we just remove this code for now?

Gr.

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

Re: [PATCH 2/4] usb: introduce usb force power off mechanism

On Fri, 29 Mar 2013, Lan Tianyu wrote:

 About the path usb: Add usb port system pm support, do you think it's
 ok?

Generally yes.  But why doesn't usb_port_system_suspend check for any
PM_QOS constraints?  Either on the port itself or on the child 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: [PATCH 2/4] usb: introduce usb force power off mechanism


On 2013/3/29 0:50, Alan Stern wrote:

On Fri, 29 Mar 2013, Lan Tianyu wrote:


About the path usb: Add usb port system pm support, do you think it's
ok?


Generally yes.  But why doesn't usb_port_system_suspend check for any
PM_QOS constraints?  Either on the port itself or on the child device.

Because usb_port_runtime_suspend() will PM Qos flags. If add check in
usb_port_system_suspend(), PM Qos flags would be checked twice and
this seems redundant.


Alan Stern



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)

2013-03-28 Thread David Linares

On 27 March 2013 20:00, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 27 Mar 2013, David Linares wrote:

 Please correct me if I am wrong.
 On my iMX25 board, I have got a 1-port root_hub which will provide
 500mA max to its unique child.

 You should check that against the iMX25's specs.  It's possible that
 the root hub has a current limit lower than 500 mA.

  Now that's where I am getting confused.
 Its child, a 2-port hub, will then be bus-powered. But if it is, it
 won't provide these 500mA shared between the 2 ports. It will only
 provide 100mA per port, right?

 Actually, I don't know.  What I said earlier was under the assumption
 that the 2-port hub receives all its power from the USB bus.  But in
 theory, it's possible for the hub to receive power directly from the
 iMX25 board's power supply.  So the answer depends on how the hardware
 is wired up.

 If the 2-port hub doesn't have an additional power source then you are
 right -- it is bus powered and should provide only 100 mA to each port.
 But if it does have an external (to the USB bus) supply then it could
 very well try to provide 500 mA to each port.  Even if the external
 supply isn't strong enough to permit this.

 But for me to be able to plug my wireless dongle, in the code, I guess
 I will need to have hub-mA_per_port = 500; and therefore have the
 USB_DEVICE_SELF_POWERED bit set in the hubstatus. Is that correct?
 Because otherwise, my dongle will always pass the test if
 (c-desc.bMaxPower * 2  udev-bus_mA) if this udev-bus_mA is set to
 100mA and always end up in insufficient power.

 Sort of.  Your emphasis is backward; this is a hardware issue, and
 whether or not a software test is passed won't affect the underlying
 problem.  That test is there so that the system will avoid trying to
 use a device when there isn't enough power for it.  Unfortunately the
 test doesn't always work, because many devices lie about their power
 sources.

 Also, hubs don't always limit the amount of current to their downstream
 ports the way they should.  A bus-powered hub is supposed to get an
 over-current condition and turn off port power if the device attached
 to the port tries to draw more than 100 mA.  But not all of them do
 this.

 In any case, you said there were occasional problems even without the
 dongle attached.  In that situation, power should not be an issue.

  You never posted any usbmon results.  If you could see the actual data
  sent by the hub when it's not working, you might get a better idea of
  where the problem is.

 True. Didn't get a chance to do this experiment yet. As you can read,
 I am still having some difficulties understanding how it should work
 in practice and who gives which power :-)
 Will do that tomorrow too.

 Okay.

Ok so here is what I did:
- compiled all USB stuff as modules
- boot up the device with nothing plugged in
- Checked that lsmod | grep usb didn't return anything
- modprobe usbmon (this will also load usbcore)
- start the capture : cat /sys/kernel/debug/usb/usbmon/0u  mon.out 
- modprobe ehci-hcd
- kill the cat ;p

I managed to get the capture for the failing case (NOK) and the OK
case. Here are the results:
OK case, dmesg output: http://pastebin.com/8DRP9L4E
OK case, lsusb -v output:   http://pastebin.com/HcFKhVeW
OK case, usbmon output:   http://pastebin.com/LrFuJ2cd

NOK case, dmesg output:   http://pastebin.com/7kXDCDmr
NOK case, lsusb -v output: http://pastebin.com/zachs6FX
NOK case, usbmon output: http://pastebin.com/RtyLjKt9

I still haven't analysed the results, but there are definitely some
differences between the two usbmon results.
Just wanted to post them as soon as possible, just in case I did
something wrong.
Cheers.
David


 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

[GIT PATCH] USB fixes for 3.9-rc4

The following changes since commit 8bb9660418e05bb1845ac1a2428444d78e322cc7:

  Linux 3.9-rc4 (2013-03-23 16:52:44 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-3.9-rc4

for you to fetch changes up to 482b0b5d82bd916cc0c55a2abf65bdc69023b843:

  usb: ftdi_sio: Add support for Mitsubishi FX-USB-AW/-BD (2013-03-28 08:50:22 
-0700)


USB fixes for 3.9-rc4

Here are some USB fixes to resolve issues reported recently, as well as a new
device id for the ftdi_sio driver.

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


Greg Kroah-Hartman (1):
  Merge tag 'for-usb-linus-2013-03-26' of 
git://git.kernel.org/.../sarah/xhci into usb-linus

Konstantin Holoborodko (1):
  usb: ftdi_sio: Add support for Mitsubishi FX-USB-AW/-BD

Lan Tianyu (2):
  usb: add find_raw_port_number callback to struct hc_driver()
  usb/acpi: binding xhci root hub usb port with ACPI

Ming Lei (1):
  USB: serial: fix hang when opening port

Peter Chen (1):
  usb: xhci: fix build warning

Roland Stigge (1):
  usb: Fix compile error by selecting USB_OTG_UTILS

Sarah Sharp (1):
  xhci: Don't warn on empty ring for suspended devices.

Soeren Moch (1):
  USB: EHCI: fix bug in iTD/siTD DMA pool allocation

Vivek Gautam (1):
  usb: xhci: Fix TRB transfer length macro used for Event TRB.

 drivers/usb/core/hcd.c|  8 +
 drivers/usb/core/usb-acpi.c   |  8 -
 drivers/usb/gadget/Kconfig|  1 +
 drivers/usb/host/ehci-sched.c |  2 ++
 drivers/usb/host/xhci-mem.c   | 36 +--
 drivers/usb/host/xhci-pci.c   |  1 +
 drivers/usb/host/xhci-ring.c  | 61 ++-
 drivers/usb/host/xhci.c   | 22 ++
 drivers/usb/host/xhci.h   |  5 
 drivers/usb/phy/Kconfig   |  1 +
 drivers/usb/serial/ftdi_sio.c |  1 +
 drivers/usb/serial/ftdi_sio_ids.h |  7 +
 drivers/usb/serial/usb-serial.c   |  1 +
 include/linux/usb/hcd.h   |  2 ++
 14 files changed, 101 insertions(+), 55 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 3.8.4: ohci question

On Thu, 28 Mar 2013, Udo van den Heuvel wrote:

 On 2013-03-28 15:35, Alan Stern wrote:
  When my dmesg gives me a growing number of lines like the one below,
  what is going on?
 
   ohci_hcd :00:12.0: urb 88023025c500 path 2 ep1in 6c16 cc 6
  -- status -71
 
  Please let me know!
  
  -71 errors indicate a low-level problem on the USB bus.  Generally it
  means that the device isn't sending any packets.  Maybe the device's
  firmware has crashed or maybe it has been disconnected.
 
 Could it be due to a defect in the mainboard?
 Or the device connected to that port?

Yes.  Or it could be due to any number of other things.  Do you want to 
post a complete dmesg log?

Alan Stern

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

Re: [PATCH 2/4] usb: introduce usb force power off mechanism

On Fri, 29 Mar 2013, Lan Tianyu wrote:

 On 2013/3/29 0:50, Alan Stern wrote:
  On Fri, 29 Mar 2013, Lan Tianyu wrote:
 
  About the path usb: Add usb port system pm support, do you think it's
  ok?
 
  Generally yes.  But why doesn't usb_port_system_suspend check for any
  PM_QOS constraints?  Either on the port itself or on the child device.
 Because usb_port_runtime_suspend() will PM Qos flags. If add check in
 usb_port_system_suspend(), PM Qos flags would be checked twice and
 this seems redundant.

Yes, that's right -- I noticed this the first time I read the patch and 
then forgot it again.

I don't see any other problems with that patch.

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 2/4] usb: introduce usb force power off mechanism


On 2013/3/29 1:49, Alan Stern wrote:

On Fri, 29 Mar 2013, Lan Tianyu wrote:


On 2013/3/29 0:50, Alan Stern wrote:

On Fri, 29 Mar 2013, Lan Tianyu wrote:


About the path usb: Add usb port system pm support, do you think it's
ok?


Generally yes.  But why doesn't usb_port_system_suspend check for any
PM_QOS constraints?  Either on the port itself or on the child device.

Because usb_port_runtime_suspend() will PM Qos flags. If add check in
usb_port_system_suspend(), PM Qos flags would be checked twice and
this seems redundant.


Yes, that's right -- I noticed this the first time I read the patch and
then forgot it again.

I don't see any other problems with that patch.

Alan Stern



Ok. I just refresh patch usb: introduce usb force power off mechanism
Please have a look.

From 16f5c7c6dd00830530a9ac758af25b575e0b8731 Mon Sep 17 00:00:00 2001
From: Lan Tianyu tianyu@intel.com
Date: Tue, 26 Feb 2013 11:12:09 +0800
Subject: [PATCH] usb: introduce usb force power off mechanism

Some devices' firmware will be broken at some points. Power down
and power on device can help device to rework in this case.

This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node
to repower usb device. First, call hub_port_logical_disconnect() to
disconnect device. Second, Power down and up usb port.

This patch is also helpful fo some QAs who want to do hcd's memleak
test(Plug and unplug device thousand times.)

Signed-off-by: Lan Tianyu tianyu@intel.com
---
 drivers/usb/core/devio.c  |   16 
 drivers/usb/core/hub.c|   27 +++
 drivers/usb/core/usb.h|2 ++
 include/uapi/linux/usbdevice_fs.h |1 +
 4 files changed, 46 insertions(+)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 8823e98..951e904 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1102,6 +1102,17 @@ static int proc_resetdevice(struct dev_state *ps)
return usb_reset_device(ps-dev);
 }

+static int proc_resetdevicepower(struct dev_state *ps)
+{
+   struct usb_device *udev = ps-dev;
+   struct usb_device *hdev = udev-parent;
+
+   if (!hdev)
+   return -EINVAL;
+
+   return usb_hub_port_power_reset(hdev, udev-portnum);
+}
+
 static int proc_setintf(struct dev_state *ps, void __user *arg)
 {
struct usbdevfs_setinterface setintf;
@@ -2011,6 +2022,11 @@ static long usbdev_do_ioctl(struct file *file, unsigned 
int cmd,
ret = proc_resetdevice(ps);
break;

+   case USBDEVFS_POWER_RESET:
+   snoop(dev-dev, %s: RESET POWER\n, __func__);
+   ret = proc_resetdevicepower(ps);
+   break;
+
case USBDEVFS_CLEAR_HALT:
snoop(dev-dev, %s: CLEAR_HALT\n, __func__);
ret = proc_clearhalt(ps, p);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 6b7f5b9..9729e36 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -948,6 +948,33 @@ static void hub_port_logical_disconnect(struct usb_hub 
*hub, int port1)
 }

 /**
+ * usb_hub_port_power_reset - repower hub port
+ * @hdev: USB device belonging to the usb hub
+ * @port1: port num to be repowered.
+ *
+ * This routine will try to disconnect the device on
+ * the port and then repower the port.
+ */
+int usb_hub_port_power_reset(struct usb_device *hdev, int port1)
+{
+   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+   struct usb_interface *intf = to_usb_interface(hub-intfdev);
+   int ret;
+
+   usb_autopm_get_interface(intf);
+   hub_port_logical_disconnect(hub, port1);
+   ret = usb_hub_set_port_power(hdev, port1, false);
+   if (!ret) {
+   /* Wait for power down device */
+   msleep(500);
+   ret = usb_hub_set_port_power(hdev, port1, true);
+   }
+   usb_autopm_put_interface(intf);
+
+   return ret;
+}
+
+/**
  * usb_remove_device - disable a device's port on its parent hub
  * @udev: device to be disabled and removed
  * Context: @udev locked, must be able to sleep.
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index a7f20bd..4e7785c 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct usb_device 
*hdev, int port1,
enum usb_port_connect_type type);
 extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
struct usb_hub_descriptor *desc);
+extern int usb_hub_port_power_reset(struct usb_device *hdev,
+   int port1);

 #ifdef CONFIG_ACPI
 extern int usb_acpi_register(void);
diff --git a/include/uapi/linux/usbdevice_fs.h 
b/include/uapi/linux/usbdevice_fs.h
index 0c65e4b..b6e0d17 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -176,5 +176,6 @@ struct usbdevfs_disconnect_claim {
 #define USBDEVFS_RELEASE_PORT  _IOR('U', 25, unsigned int)
 #define

Re: [PATCH 4/4] usb: register usb port to usb_bus_type

On Thu, Mar 28, 2013 at 01:11:04AM +0800, Lan Tianyu wrote:
 Usb port isn't assigned to any bus_type. This seems not good from
 Greg's comments.
   http://marc.info/?l=linux-usbm=136200364929942w=2
 
 This patch is to register usb port to usb_bus_type. The usb port's
 original name is portX. This will cause name confilct after adding
 usb port to usb_bus_type since the usb ports with same port num under
 different hub have the same name. So change the usb port's name format
 to port + (hub dev name) + '.' + (port num) for non-root hub and
 port + (usb bus num) + '-' + (port num) for root hub.
 
 ls /sys/bus/usb/devices
 1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
 1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
 1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
 1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
 1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
 1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
 1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2

What does it look like if you reverse the naming scheme (hub dev name +
port)?  Doesn't that show the devices in a bit more logical way?

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: improve port transitions when EHCI starts up

On Wed, Mar 27, 2013 at 04:13:36PM -0400, Alan Stern wrote:
 It seems to be getting more common recently for EHCI host controllers
 to be probed after their companion UHCI or OHCI controllers.  This may
 be caused partly by splitting the ehci-pci driver out from ehci-hcd,
 or it may be caused by changes in the way the kernel does driver
 probing.
 
 Regardless, it has a tendency to cause problems.  When an EHCI
 controller is initialized, it takes ownership of all the ports away
 from the companions.  In effect, it forcefully disconnects all the USB
 devices that may already be using a companion controller.
 
 This patch (as1672) tries to make the transition more orderly by
 deconfiguring the root hubs for all the companion controllers before
 initializing the EHCI controller, and reconfiguring them afterward.
 The result is a soft disconnect rather than a hard one.
 
 Internally, the patch refactors the code involved in associating EHCI
 controllers with their companions.  The old approach, in which a
 single function is called with an argument telling it what to do (the
 companion_action enum), has been replaced with a scheme using multiple
 callback functions, each performing a single task.
 
 This patch won't solve all the problems people encounter when their
 EHCI controllers start up, but it will at least reduce the number of
 error messages generated by the unexpected disconnections.
 
 Signed-off-by: Alan Stern st...@rowland.harvard.edu
 Tested-by: Jenya Y jy.gerstma...@gmail.com
 
 ---
 
  drivers/usb/core/hcd-pci.c |  216 
 ++---
  1 file changed, 129 insertions(+), 87 deletions(-)

This patch doesn't apply to my tree:

checking file drivers/usb/core/hcd-pci.c
Hunk #2 succeeded at 221 (offset 5 lines).
Hunk #3 FAILED at 243.
Hunk #4 succeeded at 270 (offset 5 lines).
Hunk #5 succeeded at 313 (offset 5 lines).
Hunk #6 succeeded at 481 (offset 5 lines).
1 out of 6 hunks FAILED

What branch did you make it against?  Just to be sure, I merged both
branches together, and that still didn't work.

confused,

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 4/4] usb: register usb port to usb_bus_type

On Fri, Mar 29, 2013 at 02:17:02AM +0800, Lan Tianyu wrote:
 On 2013/3/29 1:59, Greg KH wrote:
 On Thu, Mar 28, 2013 at 01:11:04AM +0800, Lan Tianyu wrote:
 Usb port isn't assigned to any bus_type. This seems not good from
 Greg's comments.
 http://marc.info/?l=linux-usbm=136200364929942w=2
 
 This patch is to register usb port to usb_bus_type. The usb port's
 original name is portX. This will cause name confilct after adding
 usb port to usb_bus_type since the usb ports with same port num under
 different hub have the same name. So change the usb port's name format
 to port + (hub dev name) + '.' + (port num) for non-root hub and
 port + (usb bus num) + '-' + (port num) for root hub.
 
 ls /sys/bus/usb/devices
 1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
 1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
 1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
 1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
 1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
 1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
 1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2
 
 What does it look like if you reverse the naming scheme (hub dev name +
 port)?  Doesn't that show the devices in a bit more logical way?
 Hi Greg:
   Do you mean e.g port1.2-1, originally it's port2-1.1.
 2-1 is hub dev name?

No, I mean 2-1.port1 as these are the ports on the device, the device
prefix should go first, right?

   If right, how about root hub port and it should be port2.usb1?

usb1.port2
--
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 4/4] usb: register usb port to usb_bus_type


On 2013/3/29 2:21, Greg KH wrote:

What does it look like if you reverse the naming scheme (hub dev name +
port)?  Doesn't that show the devices in a bit more logical way?

Hi Greg:
Do you mean e.g port1.2-1, originally it's port2-1.1.
2-1 is hub dev name?


No, I mean 2-1.port1 as these are the ports on the device, the device
prefix should go first, right?


If right, how about root hub port and it should be port2.usb1?


usb1.port2

Ok. I get it. Yes, this looks more logical. Thanks. Will refresh soon.




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

Re: [PATCH v4 0/6] Generic PHY Framework

2013-03-28 Thread David Miller


You really need to CC: net...@vger.kernel.org rather than me explicitly
on this patch set.

Thanks.
--
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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)

On Thu, 28 Mar 2013, David Linares wrote:

 Ok so here is what I did:
 - compiled all USB stuff as modules
 - boot up the device with nothing plugged in
 - Checked that lsmod | grep usb didn't return anything
 - modprobe usbmon (this will also load usbcore)
 - start the capture : cat /sys/kernel/debug/usb/usbmon/0u  mon.out 
 - modprobe ehci-hcd
 - kill the cat ;p
 
 I managed to get the capture for the failing case (NOK) and the OK
 case. Here are the results:
 OK case, dmesg output: http://pastebin.com/8DRP9L4E
 OK case, lsusb -v output:   http://pastebin.com/HcFKhVeW
 OK case, usbmon output:   http://pastebin.com/LrFuJ2cd
 
 NOK case, dmesg output:   http://pastebin.com/7kXDCDmr
 NOK case, lsusb -v output: http://pastebin.com/zachs6FX
 NOK case, usbmon output: http://pastebin.com/RtyLjKt9
 
 I still haven't analysed the results, but there are definitely some
 differences between the two usbmon results.
 Just wanted to post them as soon as possible, just in case I did
 something wrong.

You did everything right.

There are two important differences between the usbmon traces.  In the
okay case, we have this:

c1b2e240 75754579 S Ci:1:002:0 s 80 06 0100  0012 18 
c1b2e240 75755779 C Ci:1:002:0 0 18 =     
c1b2e240 75755915 S Ci:1:002:0 s 80 06 0100  0012 18 
c1b2e240 75756777 C Ci:1:002:0 0 18 = 12010002 0940 24041225  0001

This shows the host asking for the 2-port hub's device descriptor twice 
and getting two different answers.  The first response is obviously 
wrong, because it is all zeros.  The second is right.

I can't imagine why it went wrong the first time, nor why there was a 
second attempt -- the driver only tries once.  The corresponding part 
of the not-okay usbmon trace shows only one attempt:

c146a420 72878459 S Ci:1:002:0 s 80 06 0100  0012 18 
c146a420 72879654 C Ci:1:002:0 0 18 = 12010002 0940 24041225  0001

The second difference occurs in the response to the Get-Device-Status 
request.  The okay case shows:

c1af6820 75799421 S Ci:1:002:0 s 80 00   0002 2 
c1af6820 75800790 C Ci:1:002:0 0 2 = 0100

which looks right.  There's only one bit set in the response, to 
indicate the 2-port hub is self-powered.  The not-okay case shows:

c14365c0 72920304 S Ci:1:002:0 s 80 00   0002 2 
c14365c0 72920676 C Ci:1:002:0 0 2 = 6063

This response is complete garbage.  I have no idea where it came from, 
but it clearly indicates a bug somewhere.

Everything else was identical.  However, there was one more problem,
which showed up in both traces: The 2-port hub connected as a
full-speed device instead of high-speed.  You can see it in the dmesg
logs:

usb 1-1: new full-speed USB device number 2 using mxc-ehci
usb 1-1: not running at top speed; connect to a high speed hub

This is another bug.  There's no way to tell if the flaw lies in the 
2-port hub or in the iMX25's EHCI controller.  It's even possible that 
the problem lies in the connection between them; it may be picking up 
electromagnetic noise from the other components on the board.

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 4/4] usb: register usb port to usb_bus_type

On Thu, 28 Mar 2013, Greg KH wrote:

  ls /sys/bus/usb/devices
  1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
  1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
  1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
  1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
  1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
  1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
  1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2
  
  What does it look like if you reverse the naming scheme (hub dev name +
  port)?  Doesn't that show the devices in a bit more logical way?
  Hi Greg:
  Do you mean e.g port1.2-1, originally it's port2-1.1.
  2-1 is hub dev name?
 
 No, I mean 2-1.port1 as these are the ports on the device, the device
 prefix should go first, right?
 
  If right, how about root hub port and it should be port2.usb1?
 
 usb1.port2

Is this a good idea?  There are userspace programs that look through
the list of files in /sys/bus/usb/devices, and they probably expect
filenames beginning with a number or with 'usb' to be USB devices and
interfaces.

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 2/2] xhci: Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h

2013-03-28 Thread David Howells

Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h.

Signed-off-by: David Howells dhowe...@redhat.com
cc: Sarah Sharp sarah.a.sh...@linux.intel.com
cc: Greg Kroah-Hartman gre...@linuxfoundation.org
cc: linux-usb@vger.kernel.org
---

 drivers/usb/host/xhci-mem.c |   16 
 drivers/usb/host/xhci.h |4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 35616ff..e11e2af 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -51,7 +51,7 @@ static struct xhci_segment *xhci_segment_alloc(struct 
xhci_hcd *xhci,
return NULL;
}
 
-   memset(seg-trbs, 0, SEGMENT_SIZE);
+   memset(seg-trbs, 0, TRB_SEGMENT_SIZE);
/* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
if (cycle_state == 0) {
for (i = 0; i  TRBS_PER_SEGMENT; i++)
@@ -467,7 +467,7 @@ struct xhci_ring *xhci_dma_to_transfer_ring(
 {
if (ep-ep_state  EP_HAS_STREAMS)
return radix_tree_lookup(ep-stream_info-trb_address_map,
-   address  SEGMENT_SHIFT);
+   address  TRB_SEGMENT_SHIFT);
return ep-ring;
 }
 
@@ -478,7 +478,7 @@ static struct xhci_ring *dma_to_stream_ring(
u64 address)
 {
return radix_tree_lookup(stream_info-trb_address_map,
-   address  SEGMENT_SHIFT);
+   address  TRB_SEGMENT_SHIFT);
 }
 #endif /* CONFIG_USB_XHCI_HCD_DEBUGGING */
 
@@ -514,7 +514,7 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
 
cur_ring = stream_info-stream_rings[cur_stream];
for (addr = cur_ring-first_seg-dma;
-   addr  cur_ring-first_seg-dma + SEGMENT_SIZE;
+   addr  cur_ring-first_seg-dma + 
TRB_SEGMENT_SIZE;
addr += trb_size) {
mapped_ring = dma_to_stream_ring(stream_info, addr);
if (cur_ring != mapped_ring) {
@@ -662,7 +662,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
cur_stream, (unsigned long long) addr);
 
key = (unsigned long)
-   (cur_ring-first_seg-dma  SEGMENT_SHIFT);
+   (cur_ring-first_seg-dma  TRB_SEGMENT_SHIFT);
ret = radix_tree_insert(stream_info-trb_address_map,
key, cur_ring);
if (ret) {
@@ -693,7 +693,7 @@ cleanup_rings:
if (cur_ring) {
addr = cur_ring-first_seg-dma;
radix_tree_delete(stream_info-trb_address_map,
-   addr  SEGMENT_SHIFT);
+   addr  TRB_SEGMENT_SHIFT);
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;
}
@@ -764,7 +764,7 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
if (cur_ring) {
addr = cur_ring-first_seg-dma;
radix_tree_delete(stream_info-trb_address_map,
-   addr  SEGMENT_SHIFT);
+   addr  TRB_SEGMENT_SHIFT);
xhci_ring_free(xhci, cur_ring);
stream_info-stream_rings[cur_stream] = NULL;
}
@@ -2325,7 +2325,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 * so we pick the greater alignment need.
 */
xhci-segment_pool = dma_pool_create(xHCI ring segments, dev,
-   SEGMENT_SIZE, 64, xhci-page_size);
+   TRB_SEGMENT_SIZE, 64, xhci-page_size);
 
/* See Table 46 and Note on Figure 55 */
xhci-device_pool = dma_pool_create(xHCI input/output contexts, dev,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 558ba50..6bf2257 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1234,8 +1234,8 @@ union xhci_trb {
 #define TRBS_PER_SEGMENT   64
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS  (TRBS_PER_SEGMENT - 3)
-#define SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
-#define SEGMENT_SHIFT  (ilog2(SEGMENT_SIZE))
+#define TRB_SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
+#define TRB_SEGMENT_SHIFT  (ilog2(TRB_SEGMENT_SIZE))
 /* TRB buffer pointers can't cross 64KB boundaries */
 #define TRB_MAX_BUFF_SHIFT 16
 #define TRB_MAX_BUFF_SIZE  (1  TRB_MAX_BUFF_SHIFT)

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

[PATCH 1/2] xhci: Use ilog2() rather than __ffs() for calculating SEGMENT_SHIFT

2013-03-28 Thread David Howells

Use ilog2() rather than __ffs() for calculating SEGMENT_SHIFT as ilog2() can
be worked out at compile time, whereas __ffs() must be calculated at runtime.

Signed-off-by: David Howells dhowe...@redhat.com
cc: Sarah Sharp sarah.a.sh...@linux.intel.com
cc: Greg Kroah-Hartman gre...@linuxfoundation.org
cc: linux-usb@vger.kernel.org
---

 drivers/usb/host/xhci.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2c510e4..558ba50 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1235,7 +1235,7 @@ union xhci_trb {
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS  (TRBS_PER_SEGMENT - 3)
 #define SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
-#define SEGMENT_SHIFT  (__ffs(SEGMENT_SIZE))
+#define SEGMENT_SHIFT  (ilog2(SEGMENT_SIZE))
 /* TRB buffer pointers can't cross 64KB boundaries */
 #define TRB_MAX_BUFF_SHIFT 16
 #define TRB_MAX_BUFF_SIZE  (1  TRB_MAX_BUFF_SHIFT)

--
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: improve port transitions when EHCI starts up

On Thu, 28 Mar 2013, Greg KH wrote:

 On Wed, Mar 27, 2013 at 04:13:36PM -0400, Alan Stern wrote:
  It seems to be getting more common recently for EHCI host controllers
  to be probed after their companion UHCI or OHCI controllers.  This may
  be caused partly by splitting the ehci-pci driver out from ehci-hcd,
  or it may be caused by changes in the way the kernel does driver
  probing.
  
  Regardless, it has a tendency to cause problems.  When an EHCI
  controller is initialized, it takes ownership of all the ports away
  from the companions.  In effect, it forcefully disconnects all the USB
  devices that may already be using a companion controller.
  
  This patch (as1672) tries to make the transition more orderly by
  deconfiguring the root hubs for all the companion controllers before
  initializing the EHCI controller, and reconfiguring them afterward.
  The result is a soft disconnect rather than a hard one.
  
  Internally, the patch refactors the code involved in associating EHCI
  controllers with their companions.  The old approach, in which a
  single function is called with an argument telling it what to do (the
  companion_action enum), has been replaced with a scheme using multiple
  callback functions, each performing a single task.
  
  This patch won't solve all the problems people encounter when their
  EHCI controllers start up, but it will at least reduce the number of
  error messages generated by the unexpected disconnections.
  
  Signed-off-by: Alan Stern st...@rowland.harvard.edu
  Tested-by: Jenya Y jy.gerstma...@gmail.com
  
  ---
  
   drivers/usb/core/hcd-pci.c |  216 
  ++---
   1 file changed, 129 insertions(+), 87 deletions(-)
 
 This patch doesn't apply to my tree:
 
 checking file drivers/usb/core/hcd-pci.c
 Hunk #2 succeeded at 221 (offset 5 lines).
 Hunk #3 FAILED at 243.
 Hunk #4 succeeded at 270 (offset 5 lines).
 Hunk #5 succeeded at 313 (offset 5 lines).
 Hunk #6 succeeded at 481 (offset 5 lines).
 1 out of 6 hunks FAILED
 
 What branch did you make it against?  Just to be sure, I merged both
 branches together, and that still didn't work.
 
 confused,

It's the old moving target problem.  My patch was made against your
usb-next branch _before_ commit 00eed9c814cb was applied.  I will
refresh it and resend.

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 2/2] USB: remove CONFIG_USB_SUSPEND option

On Thu, 28 Mar 2013, Greg KH wrote:

 On Wed, Mar 27, 2013 at 04:14:46PM -0400, Alan Stern wrote:
  This patch (as1675) removes the CONFIG_USB_SUSPEND option, essentially
  replacing it everywhere with CONFIG_PM_RUNTIME (except for one place
  in hub.c, where it is replaced with CONFIG_PM because the code needs
  to be used in both runtime and system PM).  The net result is code
  shrinkage and simplification.
  
  There's very little point in keeping CONFIG_USB_SUSPEND because almost
  everybody enables it.  The few that don't will find that the usbcore
  module has gotten somewhat bigger and they will have to take active
  measures if they want to prevent hubs from being runtime suspended.
  
  Signed-off-by: Alan Stern st...@rowland.harvard.edu
  CC: Peter Chen peter.c...@freescale.com
 
 After applying this patch, there is still some instances of
 CONFIG_USB_SUSPEND in the Documentation/ directory, care to also remove
 those?

Ah, good point.  I checked the .c and .h files but not anything else.

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 4/4] usb: register usb port to usb_bus_type

On Thu, Mar 28, 2013 at 02:44:01PM -0400, Alan Stern wrote:
 On Thu, 28 Mar 2013, Greg KH wrote:
 
   ls /sys/bus/usb/devices
   1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
   1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
   1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
   1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
   1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
   1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
   1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2
   
   What does it look like if you reverse the naming scheme (hub dev name +
   port)?  Doesn't that show the devices in a bit more logical way?
   Hi Greg:
 Do you mean e.g port1.2-1, originally it's port2-1.1.
   2-1 is hub dev name?
  
  No, I mean 2-1.port1 as these are the ports on the device, the device
  prefix should go first, right?
  
 If right, how about root hub port and it should be port2.usb1?
  
  usb1.port2
 
 Is this a good idea?  There are userspace programs that look through
 the list of files in /sys/bus/usb/devices, and they probably expect
 filenames beginning with a number or with 'usb' to be USB devices and
 interfaces.

What userspace programs?

And if they do that, then we shouldn't put the ports in here at all.

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 4/4] usb: register usb port to usb_bus_type


On 2013/3/29 2:44, Alan Stern wrote:

On Thu, 28 Mar 2013, Greg KH wrote:


ls /sys/bus/usb/devices
1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2


What does it look like if you reverse the naming scheme (hub dev name +
port)?  Doesn't that show the devices in a bit more logical way?

Hi Greg:
Do you mean e.g port1.2-1, originally it's port2-1.1.
2-1 is hub dev name?


No, I mean 2-1.port1 as these are the ports on the device, the device
prefix should go first, right?


If right, how about root hub port and it should be port2.usb1?


usb1.port2


Is this a good idea?  There are userspace programs that look through
the list of files in /sys/bus/usb/devices, and they probably expect
filenames beginning with a number or with 'usb' to be USB devices and

After updating

ls /sys/bus/usb/devices
1-0:1.0  1-1.1:1.0  1-1.4  1-1.port3  2-1:1.02-1.6  2-1.port2  
2-1.port6  usb1.port1  usb2.port1  usb3.port1  usb4 usb4.port4
1-1  1-1.1:1.1  1-1.4:1.0  1-1.port4  2-1.5  2-1.6:1.0 2-1.port3  
3-0:1.0usb1.port2  usb2.port2  usb3.port2  usb4.port1
1-1.11-1.2  1-1.port1  2-0:1.02-1.5:1.0  2-1.6:1.1 2-1.port4  
4-0:1.0usb1.port3  usb2.port3  usb3.port3  usb4.port2
1-1:1.0  1-1.2:1.0  1-1.port2  2-12-1.5:1.1  2-1.port1 2-1.port5  usb1  
 usb2usb3usb3.port4  usb4.port3

interfaces.

Alan Stern



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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 refreshed] USB: improve port transitions when EHCI starts up

It seems to be getting more common recently for EHCI host controllers
to be probed after their companion UHCI or OHCI controllers.  This may
be caused partly by splitting the ehci-pci driver out from ehci-hcd,
or it may be caused by changes in the way the kernel does driver
probing.

Regardless, it has a tendency to cause problems.  When an EHCI
controller is initialized, it takes ownership of all the ports away
from the companions.  In effect, it forcefully disconnects all the USB
devices that may already be using a companion controller.

This patch (as1672b) tries to make the transition more orderly by
deconfiguring the root hubs for all the companion controllers before
initializing the EHCI controller, and reconfiguring them afterward.
The result is a soft disconnect rather than a hard one.

Internally, the patch refactors the code involved in associating EHCI
controllers with their companions.  The old approach, in which a
single function is called with an argument telling it what to do (the
companion_action enum), has been replaced with a scheme using multiple
callback functions, each performing a single task.

This patch won't solve all the problems people encounter when their
EHCI controllers start up, but it will at least reduce the number of
error messages generated by the unexpected disconnections.

Signed-off-by: Alan Stern st...@rowland.harvard.edu
Tested-by: Jenya Y jy.gerstma...@gmail.com

---

 drivers/usb/core/hcd-pci.c |  214 +++--
 1 file changed, 129 insertions(+), 85 deletions(-)

Index: usb-3.9/drivers/usb/core/hcd-pci.c
===
--- usb-3.9.orig/drivers/usb/core/hcd-pci.c
+++ usb-3.9/drivers/usb/core/hcd-pci.c
@@ -37,119 +37,123 @@
 
 /* PCI-based HCs are common, but plenty of non-PCI HCs are used too */
 
-#ifdef CONFIG_PM_SLEEP
-
-/* Coordinate handoffs between EHCI and companion controllers
- * during system resume
+/*
+ * Coordinate handoffs between EHCI and companion controllers
+ * during EHCI probing and system resume.
  */
 
-static DEFINE_MUTEX(companions_mutex);
+static DECLARE_RWSEM(companions_rwsem);
 
 #define CL_UHCIPCI_CLASS_SERIAL_USB_UHCI
 #define CL_OHCIPCI_CLASS_SERIAL_USB_OHCI
 #define CL_EHCIPCI_CLASS_SERIAL_USB_EHCI
 
-enum companion_action {
-   SET_HS_COMPANION, CLEAR_HS_COMPANION, WAIT_FOR_COMPANIONS
-};
+static inline int is_ohci_or_uhci(struct pci_dev *pdev)
+{
+   return pdev-class == CL_OHCI || pdev-class == CL_UHCI;
+}
 
-static void companion_common(struct pci_dev *pdev, struct usb_hcd *hcd,
-   enum companion_action action)
+typedef void (*companion_fn)(struct pci_dev *pdev, struct usb_hcd *hcd,
+   struct pci_dev *companion, struct usb_hcd *companion_hcd);
+
+/* Iterate over PCI devices in the same slot as pdev and call fn for each */
+static void for_each_companion(struct pci_dev *pdev, struct usb_hcd *hcd,
+   companion_fn fn)
 {
struct pci_dev  *companion;
struct usb_hcd  *companion_hcd;
unsigned intslot = PCI_SLOT(pdev-devfn);
 
-   /* Iterate through other PCI functions in the same slot.
-* If pdev is OHCI or UHCI then we are looking for EHCI, and
-* vice versa.
+   /*
+* Iterate through other PCI functions in the same slot.
+* If the function's drvdata isn't set then it isn't bound to
+* a USB host controller driver, so skip it.
 */
companion = NULL;
for_each_pci_dev(companion) {
if (companion-bus != pdev-bus ||
PCI_SLOT(companion-devfn) != slot)
continue;
-
companion_hcd = pci_get_drvdata(companion);
if (!companion_hcd)
continue;
-
-   /* For SET_HS_COMPANION, store a pointer to the EHCI bus in
-* the OHCI/UHCI companion bus structure.
-* For CLEAR_HS_COMPANION, clear the pointer to the EHCI bus
-* in the OHCI/UHCI companion bus structure.
-* For WAIT_FOR_COMPANIONS, wait until the OHCI/UHCI
-* companion controllers have fully resumed.
-*/
-
-   if ((pdev-class == CL_OHCI || pdev-class == CL_UHCI) 
-   companion-class == CL_EHCI) {
-   /* action must be SET_HS_COMPANION */
-   dev_dbg(companion-dev, HS companion for %s\n,
-   dev_name(pdev-dev));
-   hcd-self.hs_companion = companion_hcd-self;
-
-   } else if (pdev-class == CL_EHCI 
-   (companion-class == CL_OHCI ||
-   companion-class == CL_UHCI)) {
-   switch (action) {
-   case SET_HS_COMPANION:
-

Re: [PATCH 4/4] usb: register usb port to usb_bus_type


On 2013/3/29 2:53, Greg KH wrote:

On Thu, Mar 28, 2013 at 02:44:01PM -0400, Alan Stern wrote:

On Thu, 28 Mar 2013, Greg KH wrote:


ls /sys/bus/usb/devices
1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  port4-3
1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  port4-4
1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2


What does it look like if you reverse the naming scheme (hub dev name +
port)?  Doesn't that show the devices in a bit more logical way?

Hi Greg:
Do you mean e.g port1.2-1, originally it's port2-1.1.
2-1 is hub dev name?


No, I mean 2-1.port1 as these are the ports on the device, the device
prefix should go first, right?


If right, how about root hub port and it should be port2.usb1?


usb1.port2


Is this a good idea?  There are userspace programs that look through
the list of files in /sys/bus/usb/devices, and they probably expect
filenames beginning with a number or with 'usb' to be USB devices and
interfaces.


What userspace programs?

And if they do that, then we shouldn't put the ports in here at all.

This means usb port should be assigned to usb_bus_type.
How about creating a usb_port class and assign usb port devices to it?
ATA layer does something like this.


greg k-h



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-28 Thread Paul Zimmerman

Whoops, resending as text instead of html.

 From: Matthijs Kooijman [mailto:matth...@stdin.nl]
 Sent: Thursday, March 28, 2013 9:32 AM
 
 Hi Paul,
 
 while continuing this patch, I stumbled upon a bit of code which doesn't
 make sense to me. In dwc2_dump_global_registers is the following bit:
 
 if (hsotg-core_params-en_multiple_tx_fifo = 0) {
 ep_num = hsotg-hwcfg4  GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT 
  GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK 
  GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT;
 txfsiz = DPTXFSIZ;
 } else {
 ep_num = hsotg-hwcfg4  GHWCFG4_NUM_IN_EPS_SHIFT 
  GHWCFG4_NUM_IN_EPS_MASK  GHWCFG4_NUM_IN_EPS_SHIFT;
 txfsiz = DIENPTXF;
 }
 
 for (i = 0; i  ep_num; i++) {
 addr = hsotg-regs + DPTXFSIZN(i + 1);
 dev_dbg(hsotg-dev, %s[%d] @0x%08lX : 0x%08X\n, txfsiz, i + 
 1,
 (unsigned long)addr, readl(addr));
 }
 
 There's a number of things:
  - In the else above, the register name is set to DIENPTXF, but the values are
always read from DPTXFSIZN?

If you look closely at the databook, you will see that DPTXFSIZn and
DIEPTXFn are both aliases for the same register address.

  - According to my docs for GHWCFG4_NUM_IN_EPS, the actual number of endpoints
is the value read + 1.
  - This would result in 1-16 eps, but FIFO numbers are from 1-15 for DPTXFSIZ.

That's a bug. There are a max of 15 registers, covering EPs 1-15. EP 0
is handled by GNPTXFSIZ. So I think the loop should be:

for (i = 0; i  ep_num + 1; i++) {
addr = hsotg-regs + DPTXFSIZN(i);
dev_dbg(hsotg-dev, %s[%d] @0x%08lX : 0x%08X\n, txfsiz, i,
(unsigned long)addr, readl(addr));
}

 Do you know what the deal is here? Or, given that this is only debug output 
 for
 device mode only, shall we just remove this code for now?

Sure, that's fine with me too. Will you submit a patch for that?

-- 
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 4/4] usb: register usb port to usb_bus_type

On Thu, 28 Mar 2013, Greg KH wrote:

 On Thu, Mar 28, 2013 at 02:44:01PM -0400, Alan Stern wrote:
  On Thu, 28 Mar 2013, Greg KH wrote:
  
ls /sys/bus/usb/devices
1-0:1.02-0:1.0  port1-1  port1-1.3  port2-1.2  port2-2  
port4-3
1-12-1  port1-1.1port1-1.4  port2-1.3  port3-1  
port4-4
1-1.1  2-1:1.0  port1-1.2port1-1.5  port2-1.4  port3-2  usb1
1-1:1.03-0:1.0  port1-1.2.1  port1-1.6  port2-1.5  port3-3  usb2
1-1.1:1.0  3-1  port1-1.2.2  port1-2port2-1.6  port3-4  usb3
1-1.2  3-1:1.0  port1-1.2.3  port2-1port2-1.7  port4-1  usb4
1-1.2:1.0  4-0:1.0  port1-1.2.4  port2-1.1  port2-1.8  port4-2

What does it look like if you reverse the naming scheme (hub dev name +
port)?  Doesn't that show the devices in a bit more logical way?
Hi Greg:
Do you mean e.g port1.2-1, originally it's port2-1.1.
2-1 is hub dev name?
   
   No, I mean 2-1.port1 as these are the ports on the device, the device
   prefix should go first, right?
   
If right, how about root hub port and it should be port2.usb1?
   
   usb1.port2
  
  Is this a good idea?  There are userspace programs that look through
  the list of files in /sys/bus/usb/devices, and they probably expect
  filenames beginning with a number or with 'usb' to be USB devices and
  interfaces.
 
 What userspace programs?

In the past I have answered questions from people wanting to know how
to write a program that could go from bus  device numbers to paths in
sysfs.  The answer was to look at all the links in /sys/bus/usb/devices
for names beginning with a digit or with usb, eliminate those whose
names contain ':' as they are interfaces rather than devices, and then
check the busnum and devnum files in each of the remaining directories.

Also, libusb/libusbx uses a similar scheme to search through the
entries in /sys/bus/usb/devices, looking for parent-child
relationships.

 And if they do that, then we shouldn't put the ports in here at all.

This is one of those cloudy issues.  We're probably okay if the name 
begins with something other than a digit or a 'u'.

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/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-28 Thread Matthijs Kooijman

Hi Paul,

 If you look closely at the databook, you will see that DPTXFSIZn and
 DIEPTXFn are both aliases for the same register address.
Ah, right. Remebmer I don't have the databook, only the register
descriptions from the RT3052 datasheet, which are riddled with typos, so
I had assumed the identical register offsets were just another typo :-)

  Do you know what the deal is here? Or, given that this is only debug output 
  for
  device mode only, shall we just remove this code for now?
 Sure, that's fine with me too. Will you submit a patch for that?

I'll opt for this option, since I don't really understand the device
part of the hardware. Let's tackle this when adding device mode later.

I'll include a patch to remove this code in the series I'm working on.

Gr.

Matthijs
--
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: remove CONFIG_USB_SUSPEND from Documentation

An earlier patch removed the CONFIG_USB_SUSPEND symbol but forgot to
update the Documentation files.  This patch (as1676) rectifies that
omission.

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

---

 Documentation/ABI/testing/sysfs-bus-usb |6 +++---
 Documentation/usb/power-management.txt  |   10 ++
 2 files changed, 9 insertions(+), 7 deletions(-)

Index: usb-3.9/Documentation/ABI/testing/sysfs-bus-usb
===
--- usb-3.9.orig/Documentation/ABI/testing/sysfs-bus-usb
+++ usb-3.9/Documentation/ABI/testing/sysfs-bus-usb
@@ -32,7 +32,7 @@ Date: January 2008
 KernelVersion: 2.6.25
 Contact:   Sarah Sharp sarah.a.sh...@intel.com
 Description:
-   If CONFIG_PM and CONFIG_USB_SUSPEND are enabled, then this file
+   If CONFIG_PM_RUNTIME is enabled then this file
is present.  When read, it returns the total time (in msec)
that the USB device has been connected to the machine.  This
file is read-only.
@@ -45,7 +45,7 @@ Date: January 2008
 KernelVersion: 2.6.25
 Contact:   Sarah Sharp sarah.a.sh...@intel.com
 Description:
-   If CONFIG_PM and CONFIG_USB_SUSPEND are enabled, then this file
+   If CONFIG_PM_RUNTIME is enabled then this file
is present.  When read, it returns the total time (in msec)
that the USB device has been active, i.e. not in a suspended
state.  This file is read-only.
@@ -187,7 +187,7 @@ What:   /sys/bus/usb/devices/.../power/us
 Date:  September 2011
 Contact:   Andiry Xu andiry...@amd.com
 Description:
-   If CONFIG_USB_SUSPEND is set and a USB 2.0 lpm-capable device
+   If CONFIG_PM_RUNTIME is set and a USB 2.0 lpm-capable device
is plugged in to a xHCI host which support link PM, it will
perform a LPM test; if the test is passed and host supports
USB2 hardware LPM (xHCI 1.0 feature), USB2 hardware LPM will
Index: usb-3.9/Documentation/usb/power-management.txt
===
--- usb-3.9.orig/Documentation/usb/power-management.txt
+++ usb-3.9/Documentation/usb/power-management.txt
@@ -33,6 +33,10 @@ built with CONFIG_USB_SUSPEND enabled (w
 CONFIG_PM_RUNTIME).  System PM support is present only if the kernel
 was built with CONFIG_SUSPEND or CONFIG_HIBERNATION enabled.
 
+(Starting with the 3.10 kernel release, dynamic PM support for USB is
+present whenever the kernel was built with CONFIG_PM_RUNTIME enabled.
+The CONFIG_USB_SUSPEND option has been eliminated.)
+
 
What is Remote Wakeup?
--
@@ -206,10 +210,8 @@ initialized to 5.  (The idle-delay value
 will not be affected.)
 
 Setting the initial default idle-delay to -1 will prevent any
-autosuspend of any USB device.  This is a simple alternative to
-disabling CONFIG_USB_SUSPEND and rebuilding the kernel, and it has the
-added benefit of allowing you to enable autosuspend for selected
-devices.
+autosuspend of any USB device.  This has the benefit of allowing you
+then to enable autosuspend for selected devices.
 
 
Warnings

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

Re: [PATCH 2/4] usb: introduce usb force power off mechanism

On Fri, 29 Mar 2013, Lan Tianyu wrote:

 Ok. I just refresh patch usb: introduce usb force power off mechanism
 Please have a look.

  From 16f5c7c6dd00830530a9ac758af25b575e0b8731 Mon Sep 17 00:00:00 2001
 From: Lan Tianyu tianyu@intel.com
 Date: Tue, 26 Feb 2013 11:12:09 +0800
 Subject: [PATCH] usb: introduce usb force power off mechanism

 Some devices' firmware will be broken at some points. Power down
 and power on device can help device to rework in this case.

 This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node
 to repower usb device. First, call hub_port_logical_disconnect() to
 disconnect device. Second, Power down and up usb port.

 This patch is also helpful fo some QAs who want to do hcd's memleak
 test(Plug and unplug device thousand times.)

 Signed-off-by: Lan Tianyu tianyu@intel.com

...

 --- a/drivers/usb/core/usb.h
 +++ b/drivers/usb/core/usb.h
 @@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct 
 usb_device *hdev, int port1,
   enum usb_port_connect_type type);
   extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
   struct usb_hub_descriptor *desc);
 +extern int usb_hub_port_power_reset(struct usb_device *hdev,
 + int port1);

This can all go on one line.

It looks okay.  When you test it, does the attached device get detected 
and initialized properly?

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 2/4] usb: introduce usb force power off mechanism


On 2013/3/29 3:38, Alan Stern wrote:

On Fri, 29 Mar 2013, Lan Tianyu wrote:


Ok. I just refresh patch usb: introduce usb force power off mechanism
Please have a look.

  From 16f5c7c6dd00830530a9ac758af25b575e0b8731 Mon Sep 17 00:00:00 2001
From: Lan Tianyu tianyu@intel.com
Date: Tue, 26 Feb 2013 11:12:09 +0800
Subject: [PATCH] usb: introduce usb force power off mechanism

Some devices' firmware will be broken at some points. Power down
and power on device can help device to rework in this case.

This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node
to repower usb device. First, call hub_port_logical_disconnect() to
disconnect device. Second, Power down and up usb port.

This patch is also helpful fo some QAs who want to do hcd's memleak
test(Plug and unplug device thousand times.)

Signed-off-by: Lan Tianyu tianyu@intel.com


...


--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct usb_device 
*hdev, int port1,
enum usb_port_connect_type type);
   extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
struct usb_hub_descriptor *desc);
+extern int usb_hub_port_power_reset(struct usb_device *hdev,
+   int port1);


This can all go on one line.


It looks okay.  When you test it, does the attached device get detected
and initialized properly?

I test usb2.0 key on my machine. It works.


Alan Stern



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

Re: [PATCH 2/2] xhci: Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h

Hi Dave,

I'm a little bit confused about your description for the second one.
Did you need to change the #defines names because they could conflict
with other drivers when the xHCI driver is built in?  Or is there some
other point I'm missing?

Are these feature patches for 3.10, or bug fixes for 3.9?  If they're
for 3.9, should they go into stable?  My inclination is the first patch
shouldn't but I'm not sure about this one.

On Thu, Mar 28, 2013 at 06:48:35PM +, David Howells wrote:
 Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h.

 Signed-off-by: David Howells dhowe...@redhat.com
 cc: Sarah Sharp sarah.a.sh...@linux.intel.com
 cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 cc: linux-usb@vger.kernel.org
...
 --- a/drivers/usb/host/xhci.h
 +++ b/drivers/usb/host/xhci.h
 @@ -1234,8 +1234,8 @@ union xhci_trb {
  #define TRBS_PER_SEGMENT 64
  /* Allow two commands + a link TRB, along with any reserved command TRBs */
  #define MAX_RSVD_CMD_TRBS(TRBS_PER_SEGMENT - 3)
 -#define SEGMENT_SIZE (TRBS_PER_SEGMENT*16)
 -#define SEGMENT_SHIFT(ilog2(SEGMENT_SIZE))
 +#define TRB_SEGMENT_SIZE (TRBS_PER_SEGMENT*16)
 +#define TRB_SEGMENT_SHIFT(ilog2(TRB_SEGMENT_SIZE))

I would prefer an XHCI_ prefix instead of a TRB_ prefix.  Segments are
comprised of TRBs, so my brain doesn't parse this well. :)

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

Re: [PATCH 2/4] usb: introduce usb force power off mechanism

On Thu, Mar 28, 2013 at 01:11:02AM +0800, Lan Tianyu wrote:
 Some devices' firmware will be broken at some points. Power down
 and power on device can help device to rework in this case.
 
 This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node
 to repower usb device. First, call hub_port_logical_disconnect() to
 disconnect device. Second, Power down and up usb port.

I don't think this is the right approach.  We want to be able to power
off a port, even if no devices are attached.  One of the use case
scenarios we discussed was allowing distros to turn off empty USB ports
according to some policy (e.g. screen blank, lost bluetooth connection
to their phone that indicates the user walked away from the computer,
server admin wants to save power, etc).

With the current patches in 3.9, I don't see a way we can do that.  The
ports have power/control, which can only be set to 'on' or 'auto'.  You
should add an 'off' option to that file.

I don't particularly care if usbfs gets a new port power ioctl, I just
want the port power sysfs files to allow power off of an empty port.

 This patch is also helpful fo some QAs who want to do hcd's memleak
 test(Plug and unplug device thousand times.)

What we'd like to also do in our internal Intel QA is measure the USB
host power, manually power off all ports, and ensure the host power
drops.  That way we can be sure the BIOS and your ACPI code is working
properly.

Sarah Sharp

 
 Signed-off-by: Lan Tianyu tianyu@intel.com
 ---
  drivers/usb/core/devio.c  |   13 +
  drivers/usb/core/hub.c|   16 
  drivers/usb/core/usb.h|2 ++
  include/uapi/linux/usbdevice_fs.h |1 +
  4 files changed, 32 insertions(+)
 
 diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
 index 8823e98..a76b326 100644
 --- a/drivers/usb/core/devio.c
 +++ b/drivers/usb/core/devio.c
 @@ -1102,6 +1102,14 @@ static int proc_resetdevice(struct dev_state *ps)
   return usb_reset_device(ps-dev);
  }
  
 +static int proc_resetdevicepower(struct dev_state *ps)
 +{
 + struct usb_device *udev = ps-dev;
 + struct usb_device *hdev = udev-parent;
 +
 + return usb_hub_port_power_reset(hdev, udev-portnum);
 +}
 +
  static int proc_setintf(struct dev_state *ps, void __user *arg)
  {
   struct usbdevfs_setinterface setintf;
 @@ -2011,6 +2019,11 @@ static long usbdev_do_ioctl(struct file *file, 
 unsigned int cmd,
   ret = proc_resetdevice(ps);
   break;
  
 + case USBDEVFS_POWER_RESET:
 + snoop(dev-dev, %s: RESET POWER\n, __func__);
 + ret = proc_resetdevicepower(ps);
 + break;
 +
   case USBDEVFS_CLEAR_HALT:
   snoop(dev-dev, %s: CLEAR_HALT\n, __func__);
   ret = proc_clearhalt(ps, p);
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index 6b7f5b9..c228e69 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -114,6 +114,7 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem);
  #define HUB_DEBOUNCE_STABLE   100
  
  static int usb_reset_and_verify_device(struct usb_device *udev);
 +static void hub_port_logical_disconnect(struct usb_hub *hub, int port1);
  
  static inline char *portspeed(struct usb_hub *hub, int portstatus)
  {
 @@ -741,6 +742,21 @@ int usb_hub_set_port_power(struct usb_device *hdev, int 
 port1,
   return ret;
  }
  
 +int usb_hub_port_power_reset(struct usb_device *hdev, int port1)
 +{
 + struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 + struct usb_interface *intf = to_usb_interface(hub-intfdev);
 + int ret;
 +
 + usb_autopm_get_interface(intf);
 + hub_port_logical_disconnect(hub, port1);
 + ret = usb_hub_set_port_power(hdev, port1, false);
 + ret |= usb_hub_set_port_power(hdev, port1, true);
 + usb_autopm_put_interface(intf);
 +
 + return ret;
 +}
 +
  /**
   * usb_hub_clear_tt_buffer - clear control/bulk TT state in high speed hub
   * @urb: an URB associated with the failed or incomplete split transaction
 diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
 index a7f20bd..4e7785c 100644
 --- a/drivers/usb/core/usb.h
 +++ b/drivers/usb/core/usb.h
 @@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct 
 usb_device *hdev, int port1,
   enum usb_port_connect_type type);
  extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
   struct usb_hub_descriptor *desc);
 +extern int usb_hub_port_power_reset(struct usb_device *hdev,
 + int port1);
  
  #ifdef CONFIG_ACPI
  extern int usb_acpi_register(void);
 diff --git a/include/uapi/linux/usbdevice_fs.h 
 b/include/uapi/linux/usbdevice_fs.h
 index 0c65e4b..b6e0d17 100644
 --- a/include/uapi/linux/usbdevice_fs.h
 +++ b/include/uapi/linux/usbdevice_fs.h
 @@ -176,5 +176,6 @@ struct usbdevfs_disconnect_claim {
  #define USBDEVFS_RELEASE_PORT  _IOR('U', 25, unsigned int)
  #define USBDEVFS_GET_CAPABILITIES  _IOR('U', 26,

Re: [PATCH 2/4] usb: introduce usb force power off mechanism

On Fri, Mar 29, 2013 at 03:51:50AM +0800, Lan Tianyu wrote:
 On 2013/3/29 3:38, Alan Stern wrote:
 On Fri, 29 Mar 2013, Lan Tianyu wrote:
 It looks okay.  When you test it, does the attached device get detected
 and initialized properly?
 I test usb2.0 key on my machine. It works.

Did you test USB 3.0 as well?

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 3/4] usb: Add usb port system pm support

On Thu, Mar 28, 2013 at 07:58:47AM +0800, Lan Tianyu wrote:
 On 2013/3/28 2:47, Alan Stern wrote:
 On Thu, 28 Mar 2013, Lan Tianyu wrote
 What happens if there's no device plugged in to the port, but the hub
 is enabled for remote wakeup?  How will the hub be able to detect a
 plug-in event if the port isn't powered?
 
 Alan Stern
 
 Hi Alan:
   Great thanks for your review.
   The hub will not detect the new devices. From my opinion, this
 depends on the user space since the port only will be powered off when
 pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset
 the flag, losing plug-in event should have been took into account.

So basically, you're saying that any new distro policy that turns off
port power will need to re-enable it if the user wants hotplug events
from a hub?  I think that's a very important thing to document.

I really think we need to add more description about the port power off
mechanisms to Documentation/usb/power-management.txt.  There's a little
bit in Documentation/ABI/testing/sysfs-bus-usb, but since this mechanism
is so new, I think we need to educate distro users on its effects and
suggestions on how to use it.  Can you take a first stab at an overview,
and I'll let you know what's missing?

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 2/4] usb: introduce usb force power off mechanism