[PATCH v2] usb: ehci-sh: Fix build error due to comma has been deleted

2013-03-27 Thread Nobuhiro Iwamatsu
By commit 39d3568 (USB: remove incorrect __exit markups), comma following
ehci_hcd_sh_remove has been deleted. This fixes the error by the correction.

Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com
CC: Dmitry Torokhov dmitry.torok...@gmail.com
---
v2:  Added commit's summary and fixed sentence.

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

diff --git a/drivers/usb/host/ehci-sh.c b/drivers/usb/host/ehci-sh.c
index e30e396..b0f2268 100644
--- a/drivers/usb/host/ehci-sh.c
+++ b/drivers/usb/host/ehci-sh.c
@@ -196,7 +196,7 @@ static void ehci_hcd_sh_shutdown(struct platform_device 
*pdev)
 
 static struct platform_driver ehci_hcd_sh_driver = {
.probe  = ehci_hcd_sh_probe,
-   .remove = ehci_hcd_sh_remove
+   .remove = ehci_hcd_sh_remove,
.shutdown   = ehci_hcd_sh_shutdown,
.driver = {
.name   = sh_ehci,
-- 
1.7.10.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 v2 09/12] usb: chipidea: udc: add the define TD_PAGE_COUNT and fix all users

2013-03-27 Thread Felipe Balbi
HI,

On Tue, Mar 26, 2013 at 09:24:30PM +0100, Michael Grzeschik wrote:
 On Tue, Mar 26, 2013 at 07:55:46PM +0200, Felipe Balbi wrote:
  Hi,
  
  On Tue, Mar 26, 2013 at 05:58:45PM +0100, Michael Grzeschik wrote:
   A static count of transfer descriptors was used everywhere in the driver
   with the fixed number 5. This patch adds a define, named TD_PAGE_COUNT,
   and replaces all users of this value. This way its possible to have only
   one parameter to change and limit the amount of buffer pointers per TD.
   
   Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
   ---
   Changes since v1:
- renamed TD_COUNT to more precise TD_PAGE_COUNT
- changed TD_PAGE_COUNT to 5 based on the previous patch
- fixed patch description to address the tds buffers not the td itself
   
drivers/usb/chipidea/ci.h  | 1 +
drivers/usb/chipidea/udc.c | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
   index 7dcd390..4ca3373 100644
   --- a/drivers/usb/chipidea/ci.h
   +++ b/drivers/usb/chipidea/ci.h
   @@ -21,6 +21,7 @@

   /**
 * DEFINE
 
   */
   +#define TD_PAGE_COUNT  5
#define CI13XXX_PAGE_SIZE  4096ul /* page size for TD's */
#define ENDPT_MAX  32
#define RX0  /* similar to USB_DIR_OUT but can be used as an 
   index */
   diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
   index e779870..4294c2c 100644
   --- a/drivers/usb/chipidea/udc.c
   +++ b/drivers/usb/chipidea/udc.c
   @@ -445,7 +445,7 @@ _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
   ci13xxx_req *mReq,
 mReq-ptr-token  |= TD_IOC;
 }
 mReq-ptr-page[0]  = mReq-req.dma;
   - for (i = 1; i  5; i++)
   + for (i = 1; i  TD_PAGE_COUNT; i++)
 mReq-ptr-page[i] =
 (mReq-req.dma + i * CI13XXX_PAGE_SIZE)  
   ~TD_RESERVED_MASK;

   @@ -697,8 +697,8 @@ static int _ep_queue(struct usb_ep *ep, struct 
   usb_request *req,
 goto done;
 }

   - if (req-length  4 * CI13XXX_PAGE_SIZE) {
   - req-length = 4 * CI13XXX_PAGE_SIZE;
   + if (req-length  (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE) {
   + req-length = (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE;
  
  this is a bug. You shouldn't change req-length, you might completely
  screw up a gadget driver. Don't change anything what the gadget driver
  passes to you, ever. Change some private structure instead.
  
  (yeah, I know it's not part of $subject)
 
 Beside that bug get fixed by the patch
 usb: chipidea: udc: add multiple td support to hardware_{en,de}queue
 in that series.

*ALL BUG FIXES DESERVE THEIR OWN PATCH*. Burn this into non-volatile
memory.

When writing patches, whenever you find a bug fix, you create a separate
branch on top of newest -rc (or Greg's usb-linus or my 'fixes' branch
depending on which driver you're fixing) fix the bug with as few lines
as possible and send the patch so it can be applied during the -rc,
without creating dependencies with patches going for next merge window.

If bug was introduced prior to last merge window (e.g. you figure bug
was introduced in v3.6, instead of v3.8) then you add Cc
sta...@vger.kernel.org to make sure folks using older kernels also have
the fix.

I would suggest Alex doesn't take this series until it's properly split
into a bugfixes series and another series of cleanups/new features for
next.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] usb: Fix compile error by selecting USB_OTG_UTILS

2013-03-27 Thread Felipe Balbi
On Tue, Mar 26, 2013 at 01:37:41PM -0700, Greg KH wrote:
 On Tue, Mar 26, 2013 at 09:25:35PM +0100, Roland Stigge wrote:
  On 03/26/2013 08:24 PM, Felipe Balbi wrote:
   commit ?
  
  1c2088812f095df77f4b3224b65db79d7111a300
 
 Showed up in 3.9-rc3.
 
 Felipe, I can take this directly if you don't have any other USB patches
 for me at the moment.

please go ahead Greg:

Acked-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v2 09/12] usb: chipidea: udc: add the define TD_PAGE_COUNT and fix all users

2013-03-27 Thread Alexander Shishkin
Felipe Balbi ba...@ti.com writes:

 HI,

 On Tue, Mar 26, 2013 at 09:24:30PM +0100, Michael Grzeschik wrote:
 On Tue, Mar 26, 2013 at 07:55:46PM +0200, Felipe Balbi wrote:
  Hi,
  
  On Tue, Mar 26, 2013 at 05:58:45PM +0100, Michael Grzeschik wrote:
   A static count of transfer descriptors was used everywhere in the driver
   with the fixed number 5. This patch adds a define, named TD_PAGE_COUNT,
   and replaces all users of this value. This way its possible to have only
   one parameter to change and limit the amount of buffer pointers per TD.
   
   Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
   ---
   Changes since v1:
- renamed TD_COUNT to more precise TD_PAGE_COUNT
- changed TD_PAGE_COUNT to 5 based on the previous patch
- fixed patch description to address the tds buffers not the td itself
   
drivers/usb/chipidea/ci.h  | 1 +
drivers/usb/chipidea/udc.c | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
   index 7dcd390..4ca3373 100644
   --- a/drivers/usb/chipidea/ci.h
   +++ b/drivers/usb/chipidea/ci.h
   @@ -21,6 +21,7 @@

   /**
 * DEFINE
 
   */
   +#define TD_PAGE_COUNT  5
#define CI13XXX_PAGE_SIZE  4096ul /* page size for TD's */
#define ENDPT_MAX  32
#define RX0  /* similar to USB_DIR_OUT but can be used as an 
   index */
   diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
   index e779870..4294c2c 100644
   --- a/drivers/usb/chipidea/udc.c
   +++ b/drivers/usb/chipidea/udc.c
   @@ -445,7 +445,7 @@ _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
   ci13xxx_req *mReq,
mReq-ptr-token  |= TD_IOC;
}
mReq-ptr-page[0]  = mReq-req.dma;
   -for (i = 1; i  5; i++)
   +for (i = 1; i  TD_PAGE_COUNT; i++)
mReq-ptr-page[i] =
(mReq-req.dma + i * CI13XXX_PAGE_SIZE)  
   ~TD_RESERVED_MASK;

   @@ -697,8 +697,8 @@ static int _ep_queue(struct usb_ep *ep, struct 
   usb_request *req,
goto done;
}

   -if (req-length  4 * CI13XXX_PAGE_SIZE) {
   -req-length = 4 * CI13XXX_PAGE_SIZE;
   +if (req-length  (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE) {
   +req-length = (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE;
  
  this is a bug. You shouldn't change req-length, you might completely
  screw up a gadget driver. Don't change anything what the gadget driver
  passes to you, ever. Change some private structure instead.
  
  (yeah, I know it's not part of $subject)
 
 Beside that bug get fixed by the patch
 usb: chipidea: udc: add multiple td support to hardware_{en,de}queue
 in that series.

 *ALL BUG FIXES DESERVE THEIR OWN PATCH*. Burn this into non-volatile
 memory.

 When writing patches, whenever you find a bug fix, you create a separate
 branch on top of newest -rc (or Greg's usb-linus or my 'fixes' branch
 depending on which driver you're fixing) fix the bug with as few lines
 as possible and send the patch so it can be applied during the -rc,
 without creating dependencies with patches going for next merge window.

 If bug was introduced prior to last merge window (e.g. you figure bug
 was introduced in v3.6, instead of v3.8) then you add Cc
 sta...@vger.kernel.org to make sure folks using older kernels also have
 the fix.

 I would suggest Alex doesn't take this series until it's properly split
 into a bugfixes series and another series of cleanups/new features for
 next.

I hear ya

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 v4 00/12] usb: chipidea: udc: fixes, iso ep and multi td support

2013-03-27 Thread Michael Grzeschik
On Tue, Mar 26, 2013 at 03:50:47PM -0700, Greg KH wrote:
 On Tue, Mar 26, 2013 at 11:57:48PM +0200, Alexander Shishkin wrote:
  Also, when you resend this patchset, *please* make sure all the patches
  in it have the same version number, which is greater than *any* of these
  patches had before. Right now your 0/12 is v4, and the patches contained
  therein are anything between v2 and v5 and 1/12 doesn't have a
  version. This sends my head spinning.
 
 Welcome to my world :)

Ok, i messed it up this time, the series should have been without any
version number, like the other series before.

But anyway, i will rework the whole thing and split into bugfix and
feature patches to repost. For that i will set the series number
beginning with the oldest patch version number.

Thanks for the review,
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: Linux USB file storage gadget with new UDC

2013-03-27 Thread victor yeo
Hi,

 Understand, UDC driver will call driver-setup(), and if the return
 value is negative, UDC driver has to set dev-protocol_stall = 1 and
 maybe call usb_ep_set_halt(). However, the hardware won't be able to
 send out zero length response.

 Don't be silly; of course it can.  Nobody would be foolish enough to
 design a piece of USB hardware that couldn't send a zero-length DATA
 packet.

  I think the purpose of zero length
 response is to get an ACK from the host.

 The purpose of the zero-length response is to complete the Status stage
 of a control-OUT transfer.

Thanks, the halt feature is ok now. The SCSI_READ_10 command has a
problem. The reply value from do_read is -5, which means -EIO. The
for(;;) loop in do_read() was probably broken at if (amount_left ==
0). Is this if-statement valid?

Here is the gadget log when receiving SCSI_READ_10 from Linux host.

g_file_storage gadget: bulk-out, length 31:
: 55 53 42 43 0f 00 00 00 00 10 00 00 80 00 0a 28
0010: 00 00 00 00 00 00 00 08 00 00 00 00 e0 f8 02
SCSI CDB: 28 00 00 00 00 00 00 00 08 00
g_file_storage gadget: SCSI command: READ(10);  Dc=10, Di=4096;  Hc=10, Hi=4096
 driver read from SD card .
g_file_storage gadget-lun0: file read 4096 @ 0 - 4096
READ_10 reply -5   *** printk added by me***
g_file_storage gadget: bulk-in, length 13:
: 55 53 42 53 0f 00 00 00 00 00 00 00 00

Thanks,
victor
--
To unsubscribe from this list: send the line unsubscribe 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: EHCI: DT support for generic bus glue

2013-03-27 Thread Matthijs Kooijman
Hi folks,

 I don't think we are consistent in any way. PowerPC sets up a 32 bit
 DMA mask for all devices during DT probe from arch code, while the
 common code sets up coherent_dma_mask but not dma_mask, except
 for AMBA devices, which also get the 32 bit mask.
 
 The MIPS Octeon and PowerPC PS3 EHCI backends set up the dma mask
 because platform code doesn't do it for them, but both drivers are
 not using DT. The Xilinx and PPC-OF EHCI back-end do not set it up,
 because on microblaze and powerpc it does come from the platform
 code.
 
 I think it's a horrible mess and if anyone has an idea of what the
 right solution is, we should probably implement that, but from what
 I see here, setting a 32-bit dma mask unless there is already one
 is a reasonable choice.

I also ran into this issue with loading the dwc2 driver from OF
recently, and stumbled upon this (unfinished) patch that sets up the
dma_mask using a dma-mask property in the DT, which looks like the
proper way to do this to me:

https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013172.html
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013179.html
https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013293.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/135991.html

It's probably not up to you guys to implement this, but perhaps it helps
to get some perspective?

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 v5 06/12] usb: chipidea: udc: configure iso endpoints

2013-03-27 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 ?
 
 please clarify what you're doing here and, possibly, add a comment.
 
 Other than that:
 
 Reviewed-by: Felipe Balbi ba...@ti.com
 
 -- 
 balbi



-- 

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


[PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-27 Thread Matthijs Kooijman
Hi folks,

while going through the dwc2 driver, I've found that there is a lot of
places that need info from the hwcfg registers. Currently, each of these
places do their own bitshifting and masking, but it occurs to me that
the code would become cleaner if all this happened in single place, so
all the other spots can just refer to a proper variable instead.

I've made a start at implementing this, but before I finish up this
patch, I'd like to know what other people think. Is this a good idea, or
would you prefer to keep it like this?

The advantage of this patch would be that the code is more clear. The
disadvantage is a couple of dozen bytes of extra memory used (though
this could be reduced to just a few to no bytes by using bitfields to
store the values instead, if preferred). Another disadvantage could be
that it would be a pretty big, but simple, patch to review.

I've put my WIP patch below, let me know what you think.

Gr.

Matthijs

commit 5f67447a06581bf9d9d5f154e635a45a1406994c
Author: Matthijs Kooijman matth...@stdin.nl
Date:   Tue Mar 26 17:35:34 2013 +0100

staging: dwc2: Interpret all hwcfg and related register at init time

Before, the hwcfg4 registers were read at device init time, but
interpreted at various parts in the code. This commit unpacks the hwcfg
register values into a struct with properly labeled variables, which
makes all the other code using these values more consise and easier to
read.

This commit mostly moves code, but also attempts to simplify some
expressions, from (val  shift)  (mask  shift) to
(val  mask)  shift.

Note: This patch is unfinished, it does not convert all hwcfg accesses
yet.

diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index 71a6547..3a9a9ee 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -90,12 +90,10 @@ static void dwc2_enable_common_interrupts(struct dwc2_hsotg 
*hsotg)
  */
 static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
 {
-   u32 hs_phy_type = hsotg-hwcfg2  GHWCFG2_HS_PHY_TYPE_MASK;
-   u32 fs_phy_type = hsotg-hwcfg2  GHWCFG2_FS_PHY_TYPE_MASK;
u32 hcfg, val;
 
-   if ((hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI 
-fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED 
+   if ((hsotg-hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI 
+hsotg-hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED 
 hsotg-core_params-ulpi_fs_ls  0) ||
hsotg-core_params-phy_type == DWC2_PHY_TYPE_PARAM_FS) {
/* Full speed PHY */
@@ -245,7 +243,7 @@ static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool 
select_phy)
 
 static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 {
-   u32 usbcfg, hs_phy_type, fs_phy_type;
+   u32 usbcfg;
 
if (hsotg-core_params-speed == DWC2_SPEED_PARAM_FULL 
hsotg-core_params-phy_type == DWC2_PHY_TYPE_PARAM_FS) {
@@ -256,11 +254,8 @@ static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool 
select_phy)
dwc2_hs_phy_init(hsotg, select_phy);
}
 
-   hs_phy_type = hsotg-hwcfg2  GHWCFG2_HS_PHY_TYPE_MASK;
-   fs_phy_type = hsotg-hwcfg2  GHWCFG2_FS_PHY_TYPE_MASK;
-
-   if (hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI 
-   fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED 
+   if (hsotg-hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI 
+   hsotg-hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED 
hsotg-core_params-ulpi_fs_ls  0) {
dev_dbg(hsotg-dev, Setting ULPI FSLS\n);
usbcfg = readl(hsotg-regs + GUSBCFG);
@@ -279,7 +274,7 @@ static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
 {
u32 ahbcfg = 0;
 
-   switch (hsotg-hwcfg2  GHWCFG2_ARCHITECTURE_MASK) {
+   switch (hsotg-hw_params.arch) {
case GHWCFG2_EXT_DMA_ARCH:
dev_err(hsotg-dev, External DMA Mode not supported\n);
return -EINVAL;
@@ -331,7 +326,7 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
usbcfg = readl(hsotg-regs + GUSBCFG);
usbcfg = ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP);
 
-   switch (hsotg-hwcfg2  GHWCFG2_OP_MODE_MASK) {
+   switch (hsotg-hw_params.op_mode) {
case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE:
if (hsotg-core_params-otg_cap ==
DWC2_CAP_PARAM_HNP_SRP_CAPABLE)
@@ -392,12 +387,9 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool 
select_phy)
dwc2_core_reset(hsotg);
 
dev_dbg(hsotg-dev, num_dev_perio_in_ep=%d\n,
-   hsotg-hwcfg4  GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT 
-   GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK 
-   GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT);
+   hsotg-hw_params.num_dev_perio_in_ep);
 
-   hsotg-total_fifo_size = hsotg-hwcfg3  GHWCFG3_DFIFO_DEPTH_SHIFT 
-   GHWCFG3_DFIFO_DEPTH_MASK  

usb video capture issue due to uvc_complete callback spends more time

2013-03-27 Thread B, Ravi
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.

Appreciate your help.

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 v3] USB: PHY: Palmas USB Transceiver Driver

2013-03-27 Thread Graeme Gregory
On 26/03/13 20:23, Stephen Warren wrote:
 On 03/26/2013 10:57 AM, Graeme Gregory wrote:
 On 26/03/13 16:22, Stephen Warren wrote:
 On 03/26/2013 03:27 AM, Graeme Gregory wrote:
 ...
 If we are tightly coupling as above then using platform_irq is an extra
 inefficiency. You both have to populate this then parse it afterwards.
 Why not just use the regmap helper? Ill admit this code is like this as
 there was a period where platform irqs in DT just was not working right!

 We should really agree now if we are going for loose or tight coupling
 now rather than keep switching?
 Yes, this is something that I think needs to be fully resolved before
 any more Palmas patches are discussed.

 So you can have the MFD components fully coupled or completely
 standalone. We should fully pick one or the other, not some mish-mash of
 the two.

 In practical terms, this means that:

 a) Tightly coupled

 The top-level MFD device knows exactly which child devices are present.
 It has an internal table to defined the set of child devices, and
 instantiate them. It provides them with IRQs, I2C addresses and register
 base addresses (or regmaps), etc. etc., using purely Palmas-internal
 APIs. If using device tree, the DT won't include any representation of
 which child devices are present, nor their I2C addresses, nor their
 register addresses, nor their IRQs, etc. That's all inside the driver.

 b) Completely decoupled.

 The top-level MFD device knows nothing about its children. It simply
 provides a bus into which they can be instantiated, and a generic IRQ
 controller into which they can hook.

 Platform data or device tree is solely used to define which children
 exist, which of the top-level MFD's I2C addresses is used for each
 child, the base register address for each child device, the IRQs used by
 each child device, etc.


 Which of those two models are different people expecting?

 (b) appears to be the most flexible, and since you have defined the DT
 bindings to contain a separate node for each MFD child device, each with
 its own compatible value, seems to be what you're aiming for. In this
 scenario, there should be no private APIs between the top-level MFD
 device and the children though; everything should be using standard bus
 APIs.
 I was aiming towards (b) which would allow drivers for IP blocks that I
 know are shared between multiple devices such as RTC which is shared by
 twl6030, twl6032, tps80032, tps65910, tps65911, tps65912, tps80035,
 tps80036 and probably many more.

 Finishing (b) implementation is currently beyond the time I have
 available I think.

 If we choose to ignore path (b) and ignore the code duplication of half
 a dozen RTC drivers for the same IP than the path to (a) is much quicker
 and easier. Can just rip out a lot of the DT stuff, use mfd_add_devices.
 Makes the binding much simpler. Means we don't have to use platform
 resources for IRQs. Makes palmas and palmas-charger just 2 black boxes
 which is in line with other MFDs.

 So I think I have come around to just do it the easy way and select (a)

 I shall work on the main palmas series to implement (a).

 This will obviously invalidate some comments on this patch and the main
 series.
 Well, my question was more directed at which way we want to model the HW
 in the device tree, rather than how we want to implement the driver. The
 driver implementation style might end up being derived from the DT
 structure, but it shouldn't be the other way around.

 I think if people think (b) is the right way to go, we should just do
 it, and ignore any time issues; if it takes a while to get it right
 upstream, what is the issue with that?
I'm going to take a timeout now, I have to travel on an emergency
tonight and not sure when I will be back.

Graeme

--
To unsubscribe from this list: send the line unsubscribe 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] support other fsl SoCs with usbmisc + small fixes

2013-03-27 Thread Michael Grzeschik
On Tue, Mar 26, 2013 at 02:08:05PM +0200, Alexander Shishkin wrote:
 Michael Grzeschik m...@pengutronix.de writes:
  Hi Alexander, Fabio, Greg,

[...]

  618bfec Revert USB: chipidea: add vbus detect for udc
 
 This one, though, I will pick.

I miss that patch in your posted series.

-- 
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 4/4] usb: gadget: mv_u3d: drop ARCH dependency

2013-03-27 Thread Felipe Balbi
Hi,

On Fri, Mar 22, 2013 at 09:30:17AM +0200, Felipe Balbi wrote:
 this driver compiles fine everywhere which
 means we can use linux-next to compile it
 for us frequently.
 
 By dropping the arch dependency, we also
 ensure driver writers don't add virtual
 arch-depdencies to the driver by e.g. using
 the wrong headers.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/gadget/Kconfig | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
 index af5cc35..261b1e3 100644
 --- a/drivers/usb/gadget/Kconfig
 +++ b/drivers/usb/gadget/Kconfig
 @@ -324,7 +324,6 @@ config USB_MV_UDC
  
  config USB_MV_U3D
   tristate MARVELL PXA2128 USB 3.0 controller
 - depends on CPU_MMP3

there's one piece missing here. Marvell's PHY dependency is bogus.
Currently PHY depends on UDC, which is nonsense. This means that by
dropping ARCH dependency on this driver, which builds fine everywhere,
we can could try to build PHY in Arches which don't provide
writel_relaxed and readl_relaxed.

I fixed the original commit since it wasn't in my next branch yet, see
below:

commit 60630c2eabd40fb119a1b88af364003d2915b370
Author: Felipe Balbi ba...@ti.com
Date:   Fri Mar 22 09:15:45 2013 +0200

usb: gadget: mv_u3d: drop ARCH dependency

this driver compiles fine everywhere which
means we can use linux-next to compile it
for us frequently.

By dropping the arch dependency, we also
ensure driver writers don't add virtual
arch-depdencies to the driver by e.g. using
the wrong headers.

While at that, fix Marvell's USB3 PHY dependency,
that's the driver which depends on CPU_MM3, not
mv_u3d_core.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index af5cc35..261b1e3 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -324,7 +324,6 @@ config USB_MV_UDC
 
 config USB_MV_U3D
tristate MARVELL PXA2128 USB 3.0 controller
-   depends on CPU_MMP3
help
  MARVELL PXA2128 Processor series include a super speed USB3.0 device
  controller, which support super speed USB peripheral.
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7e8fe0f..372db48 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -45,7 +45,7 @@ config ISP1301_OMAP
 
 config MV_U3D_PHY
bool Marvell USB 3.0 PHY controller Driver
-   depends on USB_MV_U3D
+   depends on CPU_MMP3
help
  Enable this to support Marvell USB 3.0 phy controller for Marvell
  SoC.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH RESEND v2 1/1] usb: musb: implement (un)map_urb_for_dma hooks

2013-03-27 Thread Felipe Balbi
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 ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-03-27 Thread Felipe Balbi
Hi,

On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote:
 @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device 
 *pdev)
  static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
   .cpu_type   = TYPE_EXYNOS5250,
   .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
 + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,

why isn't this just a clk_get_rate() ???

-- 
balbi


signature.asc
Description: Digital signature


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

2013-03-27 Thread Yann Sionneau

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

[ 1322.011656] xhci_hcd :00:14.0: Poll event ring: 256448
[ 1322.011671] xhci_hcd :00:14.0: op reg status = 0x0
[ 1322.011677] xhci_hcd :00:14.0: ir_set 0 pending = 0x2
[ 1322.011681] xhci_hcd :00:14.0: HC error bitmask = 0x4
[ 1322.011685] xhci_hcd :00:14.0: Event ring:
[ 1322.011691] xhci_hcd :00:14.0: @3057e400 29ce4040
 0100 19028000
[ 1322.011696] xhci_hcd :00:14.0: @3057e410 29ce4050
 0100 19028000
[ 1322.011701] xhci_hcd :00:14.0: @3057e420 29ce4060
 0100 19028000
[ 1322.011706] xhci_hcd :00:14.0: @3057e430 29ce4070
 0100 19028000
[ 1322.011711] xhci_hcd :00:14.0: @3057e440 29ce4080
 0100 19028000
[ 1322.011716] xhci_hcd :00:14.0: @3057e450 29ce4090
 0100 19028000
[ 1322.011722] xhci_hcd :00:14.0: @3057e460 29ce40a0
 0100 19028000
[ 1322.011726] xhci_hcd :00:14.0: @3057e470 29ce40b0
 0100 19028000
[ 1322.011731] xhci_hcd :00:14.0: @3057e480 29ce40c0
 0100 19028000
[ 1322.011736] xhci_hcd :00:14.0: @3057e490 29ce40d0
 0100 19028000
[ 1322.011741] xhci_hcd :00:14.0: @3057e4a0 29ce40e0
 0100 19028000
[ 1322.011745] xhci_hcd :00:14.0: @3057e4b0 29ce40f0
 0100 19028000
[ 1322.011751] xhci_hcd :00:14.0: @3057e4c0 29ce4100
 0100 19028000
[ 1322.011756] xhci_hcd :00:14.0: @3057e4d0 29ce4110
 0100 19028000
[ 1322.011761] xhci_hcd :00:14.0: @3057e4e0 29ce4120
 0100 19028000
[ 1322.011766] xhci_hcd :00:14.0: @3057e4f0 29ce4130
 0100 19028000
[ 1322.011771] xhci_hcd :00:14.0: @3057e500 29ce4140
 0100 19028000
[ 1322.011776] xhci_hcd :00:14.0: @3057e510 29ce4150
 0100 19028000
[ 1322.011781] xhci_hcd :00:14.0: @3057e520 29ce4160
 0100 19028000
[ 1322.011785] xhci_hcd :00:14.0: @3057e530 29ce4170
 0100 19028000
[ 1322.011790] xhci_hcd :00:14.0: @3057e540 29ce4180
 0100 19028000
[ 1322.011795] xhci_hcd :00:14.0: @3057e550 29ce4190
 0100 19028000
[ 1322.011800] xhci_hcd :00:14.0: @3057e560 29ce41a0
 0100 19028000
[ 1322.011805] xhci_hcd :00:14.0: @3057e570 29ce41b0
 0100 19028000
[ 1322.011809] xhci_hcd :00:14.0: @3057e580 29ce41c0
 0100 19028000
[ 1322.011814] xhci_hcd :00:14.0: @3057e590 29ce41d0
 0100 19028000
[ 1322.011819] xhci_hcd :00:14.0: @3057e5a0 29ce41e0
 0100 19028000
[ 1322.011823] xhci_hcd :00:14.0: @3057e5b0 29ce41f0
 0100 19028000
[ 1322.011829] xhci_hcd :00:14.0: @3057e5c0 29ce4200
 0100 19028000
[ 1322.011834] xhci_hcd :00:14.0: @3057e5d0 29ce4210
 0100 19028000
[ 1322.011838] xhci_hcd :00:14.0: @3057e5e0 29ce4220
 0100 19028000
[ 1322.011843] xhci_hcd :00:14.0: @3057e5f0 29ce4230
 0100 19028000
[ 1322.011848] xhci_hcd :00:14.0: @3057e600 29ce4240
 0100 19028000
[ 1322.011853] xhci_hcd :00:14.0: @3057e610 29ce4250
 0100 19028000
[ 1322.011858] xhci_hcd :00:14.0: @3057e620 29ce4260
 0100 19028000
[ 1322.011863] xhci_hcd :00:14.0: @3057e630 29ce4270
 0100 19028000
[ 1322.011868] xhci_hcd :00:14.0: @3057e640 29ce4280
 0100 19028000
[ 1322.011873] xhci_hcd :00:14.0: @3057e650 29ce4290
 0100 19028000
[ 1322.011878] xhci_hcd :00:14.0: @3057e660 29ce42a0
 0100 

Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-03-27 Thread Tomasz Figa
Hi Felipe,

On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote:
 Hi,
 
 On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote:
  @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct
  platform_device *pdev) 
   static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
   
  .cpu_type   = TYPE_EXYNOS5250,
  .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
  
  +   .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
 
 why isn't this just a clk_get_rate() ???

As the name suggests, this is a function to get appropriate CLKSEL value to 
write into PHYCLK register for reference clock rate on particular platform 
(clk_get_rate is used inside to get the rate).

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center
SW Solution Development, Kernel and System Framework

--
To unsubscribe from this list: send the line unsubscribe 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/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-03-27 Thread Felipe Balbi
Hi,

On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote:
 Hi Felipe,
 
 On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote:
  Hi,
  
  On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote:
   @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct
   platform_device *pdev) 
static struct samsung_usbphy_drvdata usb3phy_exynos5 = {

 .cpu_type   = TYPE_EXYNOS5250,
 .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
   
   + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
  
  why isn't this just a clk_get_rate() ???
 
 As the name suggests, this is a function to get appropriate CLKSEL value to 
 write into PHYCLK register for reference clock rate on particular platform 
 (clk_get_rate is used inside to get the rate).

alright, then you don't need this function pointer at all, look at both
your rate_to_clksel functions (quoted below):

| +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy,
| +   unsigned long rate)
| +{
| +   unsigned int clksel;
| +
| +   switch (rate) {
| +   case 12 * MHZ:
| +   clksel = PHYCLK_CLKSEL_12M;
| +   break;
| +   case 24 * MHZ:
| +   clksel = PHYCLK_CLKSEL_24M;
| +   break;
| +   case 48 * MHZ:
| +   clksel = PHYCLK_CLKSEL_48M;
| +   break;
| +   default:
| +   dev_err(sphy-dev,
| +   Invalid reference clock frequency: %lu\n, rate);
| +   return -EINVAL;
| +   }
| +
| +   return clksel;
| +}
| +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx);
| +
| +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy,
| +   unsigned long rate)
| +{
| +   unsigned int clksel;
| +
| +   switch (rate) {
| +   case 9600 * KHZ:
| +   clksel = FSEL_CLKSEL_9600K;
| +   break;
| +   case 10 * MHZ:
| +   clksel = FSEL_CLKSEL_10M;
| +   break;
| +   case 12 * MHZ:
| +   clksel = FSEL_CLKSEL_12M;
| +   break;
| +   case 19200 * KHZ:
| +   clksel = FSEL_CLKSEL_19200K;
| +   break;
| +   case 20 * MHZ:
| +   clksel = FSEL_CLKSEL_20M;
| +   break;
| +   case 24 * MHZ:
| +   clksel = FSEL_CLKSEL_24M;
| +   break;
| +   case 50 * MHZ:
| +   clksel = FSEL_CLKSEL_50M;
| +   break;
| +   default:
| +   dev_err(sphy-dev,
| +   Invalid reference clock frequency: %lu\n, rate);
| +   return -EINVAL;
| +   }
| +
| +   return clksel;
| +}
| +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_4x12);

They both return the same thing and test the same thing. You clearly
don't need this function pointer. The only thing you need to be careful
is that different platforms will have different clock rates, but that
can just as easily be turned into a generic check.

I don't see the need for $SUBJECT.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH V4 0/2] UVC webcam gadget related changes

2013-03-27 Thread Felipe Balbi
Hi,

On Thu, Mar 21, 2013 at 01:56:07PM +0530, Bhupesh Sharma wrote:
 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 patches 1/2 and 2/2 in this patchset try to handle all the review comments
 received on the following UVC gadget related patches:
 
 [PATCH V3 3/5] usb: gadget/uvc: Port UVC webcam gadget to use
   videobuf2 framework
 [PATCH V3 5/5] usb: gadget/uvc: Add support for 'get_unmapped_area'
   for MMUless architectures
 
 which can be viewed here:
 [1] http://comments.gmane.org/gmane.linux.usb.general/2
 [2] http://www.spinics.net/lists/linux-usb/msg77400.html
 
 The rest of the patches in the V3 patchset have already been accepted
 into Laurent's git tree.
 
 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
 patch.
 
 Changes since V3:
 - Addresses the review comments received on V3 of this patchset from Laurent
   and Michael Grzeschik.
 
 - 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.

Laurent, are you giving your Ack here ? It has been almost a week since
these were sent to linux-usb.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-03-27 Thread Tomasz Figa
On Wednesday 27 of March 2013 15:31:49 Felipe Balbi wrote:
 Hi,
 
 On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote:
  Hi Felipe,
  
  On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote:
   Hi,
   
   On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote:
@@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct
platform_device *pdev)

 static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
 
.cpu_type   = TYPE_EXYNOS5250,
.devphy_en_mask = EXYNOS_USBPHY_ENABLE,

+   .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
   
   why isn't this just a clk_get_rate() ???
  
  As the name suggests, this is a function to get appropriate CLKSEL value
  to
  write into PHYCLK register for reference clock rate on particular platform
  (clk_get_rate is used inside to get the rate).
 
 alright, then you don't need this function pointer at all, look at both
 
 your rate_to_clksel functions (quoted below):
 | +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy,
 | +   unsigned long
 | rate)
 | +{
 | +   unsigned int clksel;
 | +
 | +   switch (rate) {
 | +   case 12 * MHZ:
 | +   clksel = PHYCLK_CLKSEL_12M;

Please note the PHYCLK_CLKSEL_ prefix here...

 | +   break;
 | +   case 24 * MHZ:
 | +   clksel = PHYCLK_CLKSEL_24M;
 | +   break;
 | +   case 48 * MHZ:
 | +   clksel = PHYCLK_CLKSEL_48M;
 | +   break;
 | +   default:
 | +   dev_err(sphy-dev,
 | +   Invalid reference clock frequency: %lu\n, rate);
 | +   return -EINVAL;
 | +   }
 | +
 | +   return clksel;
 | +}
 | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx);
 | +
 | +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy,
 | +   unsigned long
 | rate)
 | +{
 | +   unsigned int clksel;
 | +
 | +   switch (rate) {
 | +   case 9600 * KHZ:
 | +   clksel = FSEL_CLKSEL_9600K;
 | +   break;
 | +   case 10 * MHZ:
 | +   clksel = FSEL_CLKSEL_10M;
 | +   break;
 | +   case 12 * MHZ:
 | +   clksel = FSEL_CLKSEL_12M;

..and then FSEL_CLKSEL_ here. They have different values. (Their names are a 
bit unfortunate, though...)

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center
SW Solution Development, Kernel and System Framework

 | +   break;
 | +   case 19200 * KHZ:
 | +   clksel = FSEL_CLKSEL_19200K;
 | +   break;
 | +   case 20 * MHZ:
 | +   clksel = FSEL_CLKSEL_20M;
 | +   break;
 | +   case 24 * MHZ:
 | +   clksel = FSEL_CLKSEL_24M;
 | +   break;
 | +   case 50 * MHZ:
 | +   clksel = FSEL_CLKSEL_50M;
 | +   break;
 | +   default:
 | +   dev_err(sphy-dev,
 | +   Invalid reference clock frequency: %lu\n, rate);
 | +   return -EINVAL;
 | +   }
 | +
 | +   return clksel;
 | +}
 | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_4x12);
 
 They both return the same thing and test the same thing. You clearly
 don't need this function pointer. The only thing you need to be careful
 is that different platforms will have different clock rates, but that
 can just as easily be turned into a generic check.
 
 I don't see the need for $SUBJECT.

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


Renesas sparse errors

2013-03-27 Thread Felipe Balbi
Hi Kuninori,

Renesas USB driver constantly gives me sparse errors:

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

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling

2013-03-27 Thread Felipe Balbi
Hi,

On Wed, Mar 27, 2013 at 02:39:08PM +0100, Tomasz Figa wrote:
 On Wednesday 27 of March 2013 15:31:49 Felipe Balbi wrote:
  Hi,
  
  On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote:
   Hi Felipe,
   
   On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote:
Hi,

On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote:
 @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct
 platform_device *pdev)
 
  static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
  
   .cpu_type   = TYPE_EXYNOS5250,
   .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
 
 + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,

why isn't this just a clk_get_rate() ???
   
   As the name suggests, this is a function to get appropriate CLKSEL value
   to
   write into PHYCLK register for reference clock rate on particular platform
   (clk_get_rate is used inside to get the rate).
  
  alright, then you don't need this function pointer at all, look at both
  
  your rate_to_clksel functions (quoted below):
  | +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy,
  | +   unsigned long
  | rate)
  | +{
  | +   unsigned int clksel;
  | +
  | +   switch (rate) {
  | +   case 12 * MHZ:
  | +   clksel = PHYCLK_CLKSEL_12M;
 
 Please note the PHYCLK_CLKSEL_ prefix here...
 
  | +   break;
  | +   case 24 * MHZ:
  | +   clksel = PHYCLK_CLKSEL_24M;
  | +   break;
  | +   case 48 * MHZ:
  | +   clksel = PHYCLK_CLKSEL_48M;
  | +   break;
  | +   default:
  | +   dev_err(sphy-dev,
  | +   Invalid reference clock frequency: %lu\n, rate);
  | +   return -EINVAL;
  | +   }
  | +
  | +   return clksel;
  | +}
  | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx);
  | +
  | +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy,
  | +   unsigned long
  | rate)
  | +{
  | +   unsigned int clksel;
  | +
  | +   switch (rate) {
  | +   case 9600 * KHZ:
  | +   clksel = FSEL_CLKSEL_9600K;
  | +   break;
  | +   case 10 * MHZ:
  | +   clksel = FSEL_CLKSEL_10M;
  | +   break;
  | +   case 12 * MHZ:
  | +   clksel = FSEL_CLKSEL_12M;
 
 ..and then FSEL_CLKSEL_ here. They have different values. (Their names are a 
 bit unfortunate, though...)

indeed, my eyes failed there. So I agree with the patch :-)

sorry for the noise.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v4 0/6] support other fsl SoCs with usbmisc + small fixes

2013-03-27 Thread Alexander Shishkin
Michael Grzeschik m...@pengutronix.de writes:

 On Tue, Mar 26, 2013 at 02:08:05PM +0200, Alexander Shishkin wrote:
 Michael Grzeschik m...@pengutronix.de writes:
  Hi Alexander, Fabio, Greg,

 [...]

  618bfec Revert USB: chipidea: add vbus detect for udc
 
 This one, though, I will pick.

 I miss that patch in your posted series.

I haven't decided yet if it is better to apply a revert or to fix the
existing patch for the time being. If you have any opinions on this,
please share.

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

2013-03-27 Thread Alan Stern
On Wed, 27 Mar 2013, victor yeo wrote:

 Thanks, the halt feature is ok now. The SCSI_READ_10 command has a
 problem. The reply value from do_read is -5, which means -EIO. The
 for(;;) loop in do_read() was probably broken at if (amount_left ==
 0). Is this if-statement valid?

Yes, it is.  do_read() is supposed to return -EIO.  This tells the code
at the end of do_scsi_command() that the final reply buffer has already
been set up.  The final buffer is sent to the host by the
finish_reply() routine.

 Here is the gadget log when receiving SCSI_READ_10 from Linux host.
 
 g_file_storage gadget: bulk-out, length 31:
 : 55 53 42 43 0f 00 00 00 00 10 00 00 80 00 0a 28
 0010: 00 00 00 00 00 00 00 08 00 00 00 00 e0 f8 02
 SCSI CDB: 28 00 00 00 00 00 00 00 08 00
 g_file_storage gadget: SCSI command: READ(10);  Dc=10, Di=4096;  Hc=10, 
 Hi=4096
  driver read from SD card .
 g_file_storage gadget-lun0: file read 4096 @ 0 - 4096
 READ_10 reply -5   *** printk added by me***
 g_file_storage gadget: bulk-in, length 13:
 : 55 53 42 53 0f 00 00 00 00 00 00 00 00

This all looks right.

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 V4 0/2] UVC webcam gadget related changes

2013-03-27 Thread Laurent Pinchart
Hi Felipe,

On Wednesday 27 March 2013 15:33:15 Felipe Balbi wrote:
 On Thu, Mar 21, 2013 at 01:56:07PM +0530, Bhupesh Sharma wrote:
  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 patches 1/2 and 2/2 in this patchset try to handle all the review
  comments received on the following UVC gadget related patches:
  
  [PATCH V3 3/5] usb: gadget/uvc: Port UVC webcam gadget to use
  
  videobuf2 framework
  
  [PATCH V3 5/5] usb: gadget/uvc: Add support for 'get_unmapped_area'
  
  for MMUless architectures
  
  which can be viewed here:
  [1] http://comments.gmane.org/gmane.linux.usb.general/2
  [2] http://www.spinics.net/lists/linux-usb/msg77400.html
  
  The rest of the patches in the V3 patchset have already been accepted
  into Laurent's git tree.
  
  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 patch.
  
  Changes since V3:
  - Addresses the review comments received on V3 of this patchset from
Laurent and Michael Grzeschik.
  
  - 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.
 
 Laurent, are you giving your Ack here ? It has been almost a week since
 these were sent to linux-usb.

I'm reviewing the patches.

-- 
Regards,

Laurent Pinchart


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place

2013-03-27 Thread Greg KH
On Wed, Mar 27, 2013 at 10:48:17AM +0100, Matthijs Kooijman wrote:
 Hi folks,
 
 while going through the dwc2 driver, I've found that there is a lot of
 places that need info from the hwcfg registers. Currently, each of these
 places do their own bitshifting and masking, but it occurs to me that
 the code would become cleaner if all this happened in single place, so
 all the other spots can just refer to a proper variable instead.
 
 I've made a start at implementing this, but before I finish up this
 patch, I'd like to know what other people think. Is this a good idea, or
 would you prefer to keep it like this?
 
 The advantage of this patch would be that the code is more clear. The
 disadvantage is a couple of dozen bytes of extra memory used (though
 this could be reduced to just a few to no bytes by using bitfields to
 store the values instead, if preferred). Another disadvantage could be
 that it would be a pretty big, but simple, patch to review.

Then do it one bit at a time (sorry, bad pun...)

But yes, it does seem to be a good idea, as long as these options can't
change over time.

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

2013-03-27 Thread Matthijs Kooijman
Hi Greg,

 But yes, it does seem to be a good idea,

Ok, thanks.

 as long as these options can't change over time.
It's only read-only registers, or registers whose power-on values
determine their maximum possible value, so they indeed won't change over
time.

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 v6 3/6] usb: chipidea: udc: move ZLT flag change to ep_enable

2013-03-27 Thread Michael Grzeschik
Its not necessary and also not specified in the datasheet to change the
ZLT flag before every ep_prime. This patch moves this to the ep_enable
and applies it only for non configuration endpoints.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 4bbbe62..a8528ad 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
-   mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
 
@@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep,
mEp-qh.ptr-cap = ~QH_MULT;
else
mEp-qh.ptr-cap = ~QH_ZLT;
+   if (mEp-num)
+   mEp-qh.ptr-cap |= QH_ZLT;
 
mEp-qh.ptr-cap |=
(mEp-ep.maxpacket  ffs_nr(QH_MAX_PKT))  QH_MAX_PKT;
-- 
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 v6 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Michael Grzeschik
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

 drivers/usb/chipidea/udc.c | 133 ++---
 drivers/usb/chipidea/udc.h |   4 +-
 2 files changed, 80 insertions(+), 57 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 v6 5/6] usb: chipidea: udc: don't truncate requests to single tds

2013-03-27 Thread Michael Grzeschik
It is not safe to truncate requests to the maximum possible size the
controller can handle with one td and to keep working. That patch fixes
that with proper error handling instead.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d4db3ac..e7c84ab 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
}
 
if (req-length  4 * CI13XXX_PAGE_SIZE) {
-   req-length = 4 * CI13XXX_PAGE_SIZE;
retval = -EMSGSIZE;
-   dev_warn(mEp-ci-dev, request length truncated\n);
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
}
 
dbg_queue(_usb_addr(mEp), req, retval);
-- 
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 v6 4/6] usb: chipidea: udc: read status of td only once in hardware_dequeue

2013-03-27 Thread Michael Grzeschik
This patch changes the read of the td status to one atomic operation to
analyse coherent bits.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index a8528ad..d4db3ac 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -507,10 +507,12 @@ done:
  */
 static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 {
+   u32 tmptoken = mReq-ptr-token;
+
if (mReq-req.status != -EALREADY)
return -EINVAL;
 
-   if ((TD_STATUS_ACTIVE  mReq-ptr-token) != 0)
+   if ((TD_STATUS_ACTIVE  tmptoken) != 0)
return -EBUSY;
 
if (mReq-zptr) {
@@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir);
 
-   mReq-req.status = mReq-ptr-token  TD_STATUS;
+   mReq-req.status = tmptoken  TD_STATUS;
if ((TD_STATUS_HALTED  mReq-req.status) != 0)
mReq-req.status = -1;
else if ((TD_STATUS_DT_ERR  mReq-req.status) != 0)
@@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
else if ((TD_STATUS_TR_ERR  mReq-req.status) != 0)
mReq-req.status = -1;
 
-   mReq-req.actual   = mReq-ptr-token  TD_TOTAL_BYTES;
+   mReq-req.actual   = tmptoken  TD_TOTAL_BYTES;
mReq-req.actual = ffs_nr(TD_TOTAL_BYTES);
mReq-req.actual   = mReq-req.length - mReq-req.actual;
mReq-req.actual   = mReq-req.status ? 0 : mReq-req.actual;
-- 
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 v6 2/6] usb: chipidea: udc: only clear active and halted bits in qhead

2013-03-27 Thread Michael Grzeschik
The datasheet of the synopsys core describes only to overwrite the
active and halted bits in the qhead before priming any endpoint.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 77fb66f..4bbbe62 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
-   mEp-qh.ptr-td.token = ~TD_STATUS;   /* clear status */
+   mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
-- 
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 v6 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

2013-03-27 Thread Michael Grzeschik
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 could
read wrong data and totally stuck.

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


[PATCH v6 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function

2013-03-27 Thread Michael Grzeschik
There is no need to call ep_queue unlocked inside the own driver. We
move its functionionality into an unlocked version.

This patch removes potential unlocked timeslots inside
isr_setup_status_phase and isr_get_status_response, in which the lock
got released just before acquired again inside usb_ep_queue.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index e7c84ab..61afd71 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, 
struct usb_request *req)
 }
 
 /**
+ * _ep_queue: queues (submits) an I/O request to an endpoint
+ *
+ * Caller must hold lock
+ */
+static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
+   gfp_t __maybe_unused gfp_flags)
+{
+   struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
+   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
+   struct ci13xxx *ci = mEp-ci;
+   int retval = 0;
+
+   if (ep == NULL || req == NULL || mEp-ep.desc == NULL)
+   return -EINVAL;
+
+   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
+   if (req-length)
+   mEp = (ci-ep0_dir == RX) ?
+  ci-ep0out : ci-ep0in;
+   if (!list_empty(mEp-qh.queue)) {
+   _ep_nuke(mEp);
+   retval = -EOVERFLOW;
+   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
+_usb_addr(mEp));
+   }
+   }
+
+   if (usb_endpoint_xfer_isoc(mEp-ep.desc)) {
+   if (mReq-req.length  mEp-ep.maxpacket)
+   return -EMSGSIZE;
+   }
+
+   /* first nuke then test link, e.g. previous status has not sent */
+   if (!list_empty(mReq-queue)) {
+   retval = -EBUSY;
+   dev_err(mEp-ci-dev, request already in queue\n);
+   goto done;
+   }
+
+   if (req-length  4 * CI13XXX_PAGE_SIZE) {
+   retval = -EMSGSIZE;
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
+   }
+
+   dbg_queue(_usb_addr(mEp), req, retval);
+
+   /* push request */
+   mReq-req.status = -EINPROGRESS;
+   mReq-req.actual = 0;
+
+   retval = _hardware_enqueue(mEp, mReq);
+
+   if (retval == -EALREADY) {
+   dbg_event(_usb_addr(mEp), QUEUE, retval);
+   retval = 0;
+   }
+   if (!retval)
+   list_add_tail(mReq-queue, mEp-qh.queue);
+
+ done:
+   return retval;
+}
+
+/**
  * isr_get_status_response: get_status request response
  * @ci: ci struct
  * @setup: setup request packet
@@ -717,9 +782,7 @@ __acquires(mEp-lock)
}
/* else do nothing; reserved for future use */
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, req, gfp_flags);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, req, gfp_flags);
if (retval)
goto err_free_buf;
 
@@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct 
usb_request *req)
  * This function returns an error code
  */
 static int isr_setup_status_phase(struct ci13xxx *ci)
-__releases(mEp-lock)
-__acquires(mEp-lock)
 {
int retval;
struct ci13xxx_ep *mEp;
@@ -776,9 +837,7 @@ __acquires(mEp-lock)
ci-status-context = ci;
ci-status-complete = isr_setup_status_complete;
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
 
return retval;
 }
@@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
gfp_t __maybe_unused gfp_flags)
 {
struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
-   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
-   struct ci13xxx *ci = mEp-ci;
int retval = 0;
unsigned long flags;
 
@@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct 
usb_request *req,
 
spin_lock_irqsave(mEp-lock, flags);
 
-   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
-   if (req-length)
-   mEp = (ci-ep0_dir == RX) ?
-  ci-ep0out : ci-ep0in;
-   if (!list_empty(mEp-qh.queue)) {
-   _ep_nuke(mEp);
-   retval = -EOVERFLOW;
-   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
-_usb_addr(mEp));
-   }
-   }
-
-  

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

2013-03-27 Thread Alexander Shishkin
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

Didn't you forget Felipe's reviewed-bys?

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 v6 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Michael Grzeschik
On Wed, Mar 27, 2013 at 06:06:51PM +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
 
 Didn't you forget Felipe's reviewed-bys?

Yes, i will repost v7 in a minute.

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


[PATCH v7 3/6] usb: chipidea: udc: move ZLT flag change to ep_enable

2013-03-27 Thread Michael Grzeschik
Its not necessary and also not specified in the datasheet to change the
ZLT flag before every ep_prime. This patch moves this to the ep_enable
and applies it only for non configuration endpoints.

Cc: stable sta...@vger.kernel.org
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/chipidea/udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 4bbbe62..a8528ad 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
-   mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
 
@@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep,
mEp-qh.ptr-cap = ~QH_MULT;
else
mEp-qh.ptr-cap = ~QH_ZLT;
+   if (mEp-num)
+   mEp-qh.ptr-cap |= QH_ZLT;
 
mEp-qh.ptr-cap |=
(mEp-ep.maxpacket  ffs_nr(QH_MAX_PKT))  QH_MAX_PKT;
-- 
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 v7 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function

2013-03-27 Thread Michael Grzeschik
There is no need to call ep_queue unlocked inside the own driver. We
move its functionionality into an unlocked version.

This patch removes potential unlocked timeslots inside
isr_setup_status_phase and isr_get_status_response, in which the lock
got released just before acquired again inside usb_ep_queue.

Cc: stable sta...@vger.kernel.org
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
Reviewed-by: Peter Chen peter.c...@freescale.com
---
 drivers/usb/chipidea/udc.c | 118 ++---
 1 file changed, 68 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index e7c84ab..61afd71 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, 
struct usb_request *req)
 }
 
 /**
+ * _ep_queue: queues (submits) an I/O request to an endpoint
+ *
+ * Caller must hold lock
+ */
+static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
+   gfp_t __maybe_unused gfp_flags)
+{
+   struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
+   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
+   struct ci13xxx *ci = mEp-ci;
+   int retval = 0;
+
+   if (ep == NULL || req == NULL || mEp-ep.desc == NULL)
+   return -EINVAL;
+
+   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
+   if (req-length)
+   mEp = (ci-ep0_dir == RX) ?
+  ci-ep0out : ci-ep0in;
+   if (!list_empty(mEp-qh.queue)) {
+   _ep_nuke(mEp);
+   retval = -EOVERFLOW;
+   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
+_usb_addr(mEp));
+   }
+   }
+
+   if (usb_endpoint_xfer_isoc(mEp-ep.desc)) {
+   if (mReq-req.length  mEp-ep.maxpacket)
+   return -EMSGSIZE;
+   }
+
+   /* first nuke then test link, e.g. previous status has not sent */
+   if (!list_empty(mReq-queue)) {
+   retval = -EBUSY;
+   dev_err(mEp-ci-dev, request already in queue\n);
+   goto done;
+   }
+
+   if (req-length  4 * CI13XXX_PAGE_SIZE) {
+   retval = -EMSGSIZE;
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
+   }
+
+   dbg_queue(_usb_addr(mEp), req, retval);
+
+   /* push request */
+   mReq-req.status = -EINPROGRESS;
+   mReq-req.actual = 0;
+
+   retval = _hardware_enqueue(mEp, mReq);
+
+   if (retval == -EALREADY) {
+   dbg_event(_usb_addr(mEp), QUEUE, retval);
+   retval = 0;
+   }
+   if (!retval)
+   list_add_tail(mReq-queue, mEp-qh.queue);
+
+ done:
+   return retval;
+}
+
+/**
  * isr_get_status_response: get_status request response
  * @ci: ci struct
  * @setup: setup request packet
@@ -717,9 +782,7 @@ __acquires(mEp-lock)
}
/* else do nothing; reserved for future use */
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, req, gfp_flags);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, req, gfp_flags);
if (retval)
goto err_free_buf;
 
@@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct 
usb_request *req)
  * This function returns an error code
  */
 static int isr_setup_status_phase(struct ci13xxx *ci)
-__releases(mEp-lock)
-__acquires(mEp-lock)
 {
int retval;
struct ci13xxx_ep *mEp;
@@ -776,9 +837,7 @@ __acquires(mEp-lock)
ci-status-context = ci;
ci-status-complete = isr_setup_status_complete;
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
 
return retval;
 }
@@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
gfp_t __maybe_unused gfp_flags)
 {
struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
-   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
-   struct ci13xxx *ci = mEp-ci;
int retval = 0;
unsigned long flags;
 
@@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct 
usb_request *req,
 
spin_lock_irqsave(mEp-lock, flags);
 
-   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
-   if (req-length)
-   mEp = (ci-ep0_dir == RX) ?
-  ci-ep0out : ci-ep0in;
-   if (!list_empty(mEp-qh.queue)) {
-   _ep_nuke(mEp);
-   retval = -EOVERFLOW;
-   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
-

[PATCH v7 5/6] usb: chipidea: udc: don't truncate requests to single tds

2013-03-27 Thread Michael Grzeschik
It is not safe to truncate requests to the maximum possible size the
controller can handle with one td and to keep working. That patch fixes
that with proper error handling instead.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d4db3ac..e7c84ab 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
}
 
if (req-length  4 * CI13XXX_PAGE_SIZE) {
-   req-length = 4 * CI13XXX_PAGE_SIZE;
retval = -EMSGSIZE;
-   dev_warn(mEp-ci-dev, request length truncated\n);
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
}
 
dbg_queue(_usb_addr(mEp), req, retval);
-- 
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 v7 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Michael Grzeschik
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

 drivers/usb/chipidea/udc.c | 133 ++---
 drivers/usb/chipidea/udc.h |   4 +-
 2 files changed, 80 insertions(+), 57 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 v7 2/6] usb: chipidea: udc: only clear active and halted bits in qhead

2013-03-27 Thread Michael Grzeschik
The datasheet of the synopsys core describes only to overwrite the
active and halted bits in the qhead before priming any endpoint.

Cc: stable sta...@vger.kernel.org
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/chipidea/udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 77fb66f..4bbbe62 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
-   mEp-qh.ptr-td.token = ~TD_STATUS;   /* clear status */
+   mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
-- 
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 v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

2013-03-27 Thread Michael Grzeschik
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 could
read wrong data and totally stuck.

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


[PATCH v7 4/6] usb: chipidea: udc: read status of td only once in hardware_dequeue

2013-03-27 Thread Michael Grzeschik
This patch changes the read of the td status to one atomic operation to
analyse coherent bits.

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

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index a8528ad..d4db3ac 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -507,10 +507,12 @@ done:
  */
 static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 {
+   u32 tmptoken = mReq-ptr-token;
+
if (mReq-req.status != -EALREADY)
return -EINVAL;
 
-   if ((TD_STATUS_ACTIVE  mReq-ptr-token) != 0)
+   if ((TD_STATUS_ACTIVE  tmptoken) != 0)
return -EBUSY;
 
if (mReq-zptr) {
@@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir);
 
-   mReq-req.status = mReq-ptr-token  TD_STATUS;
+   mReq-req.status = tmptoken  TD_STATUS;
if ((TD_STATUS_HALTED  mReq-req.status) != 0)
mReq-req.status = -1;
else if ((TD_STATUS_DT_ERR  mReq-req.status) != 0)
@@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
else if ((TD_STATUS_TR_ERR  mReq-req.status) != 0)
mReq-req.status = -1;
 
-   mReq-req.actual   = mReq-ptr-token  TD_TOTAL_BYTES;
+   mReq-req.actual   = tmptoken  TD_TOTAL_BYTES;
mReq-req.actual = ffs_nr(TD_TOTAL_BYTES);
mReq-req.actual   = mReq-req.length - mReq-req.actual;
mReq-req.actual   = mReq-req.status ? 0 : mReq-req.actual;
-- 
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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)

2013-03-27 Thread David Linares
On 26 March 2013 18:50, Alan Stern st...@rowland.harvard.edu wrote:
 On Tue, 26 Mar 2013, David Linares wrote:

  The device I am working on is an embedded device, therefore the wireless
  adapter has to be connected to the internal hub. This hub and the whole 
  device
  are powered via an usb cable.
 
  Oh.  Then maybe the USB cable doesn't provide enough power to run both
  the embedded device and the wireless adapter.

 There are 2 ports, so does the hub provide 500mA per port or in total?
 If that's 500mA for each port, that's fine.

 You seem to be missing the point.  You said above that your entire
 device is powered via a USB cable.  That means the entire device gets
 only 500 mA, total.  If the hub sends 500 mA out one of its ports then
 there will be nothing left for the other port and nothing left for the
 rest of your device.


Thanks Alan. I understand, but the USB cable is plugged into an AC/DC
adaptor which can provide an output of 1A. After breaking out an USB
cable, I measured that the consumption of my board is around 130mA
(nothing connected to the hub). Then I measured with the dongle
plugged in. The real consumption of the wireless adapter is around
210mA. The whole thing draws around 340mA

 Of course, the way it works in reality is that the device uses a
 significant fraction of the total current, leaving a lot less than 500
 mA available for the hub.  For example, the hub may get only 200 mA.
 There's no way it can take that input and magically convert it to 500
 mA for each of two ports.

 I have done some progress on my side. I have noticed that in a failing case,
 the 2-port hub is actually detected as a standalone hub but when it
 works fine,
 it is detected as a compound device.
 Also printed the hubstatus that I get from here
 http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1513
 and it is really weird:

 - when it works, the value returned is 1, and therefore, I fall into the
 Self-powered external hub case

 - when it doesn't work, the value seems quite random (ex: 0xe6c0) and I fall
 into the 100mA limit = my Ralink wireless adapter gets a rejected config.

 That's to be expected.  When there isn't enough power, you start
 getting random data and things stop working.


Today, I did some further tests *without* any wireless adapter. So, I
have got the device plugged in into a main power socket via USB
(delivering up to 1A), the root hub, and the 2-port hub and nothing
else USB-wise.
The problem still happens from time to time. The 2-port hub sometimes
says it will give 500mA to its children and sometimes it gives only
100mA (because of a corrupted hubstatus mentioned earlier).
Now that there is nothing connected on the hub, I am just wondering
how come the hub is not always detected in the same way at each boot.
Also, should it be detected as self powered (
http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or
not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538
)?

I forgot to mention that this issue where the hub is not detected as
self-powered external hub is quite hard to reproduce. It happens 1
out of 20 tries.
Thanks a lot Alan and everybody else. I really appreciate the support
and understanding you are providing.

 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 v6 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function

2013-03-27 Thread Alexander Shishkin
Michael Grzeschik m.grzesc...@pengutronix.de writes:

 There is no need to call ep_queue unlocked inside the own driver. We
 move its functionionality into an unlocked version.

 This patch removes potential unlocked timeslots inside
 isr_setup_status_phase and isr_get_status_response, in which the lock
 got released just before acquired again inside usb_ep_queue.

 Cc: stable sta...@vger.kernel.org

Other patches in this set are less questionable, but this is certainly
not stable material. See Documentation/stable_kernel_rules.txt

 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 ---
  drivers/usb/chipidea/udc.c | 118 
 ++---
  1 file changed, 68 insertions(+), 50 deletions(-)

 diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
 index e7c84ab..61afd71 100644
 --- a/drivers/usb/chipidea/udc.c
 +++ b/drivers/usb/chipidea/udc.c
 @@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, 
 struct usb_request *req)
  }
  
  /**
 + * _ep_queue: queues (submits) an I/O request to an endpoint
 + *
 + * Caller must hold lock
 + */
 +static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
 + gfp_t __maybe_unused gfp_flags)
 +{
 + struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
 + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
 + struct ci13xxx *ci = mEp-ci;
 + int retval = 0;
 +
 + if (ep == NULL || req == NULL || mEp-ep.desc == NULL)
 + return -EINVAL;
 +
 + if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
 + if (req-length)
 + mEp = (ci-ep0_dir == RX) ?
 +ci-ep0out : ci-ep0in;
 + if (!list_empty(mEp-qh.queue)) {
 + _ep_nuke(mEp);
 + retval = -EOVERFLOW;
 + dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
 +  _usb_addr(mEp));
 + }
 + }
 +
 + if (usb_endpoint_xfer_isoc(mEp-ep.desc)) {
 + if (mReq-req.length  mEp-ep.maxpacket)
 + return -EMSGSIZE;
 + }
 +
 + /* first nuke then test link, e.g. previous status has not sent */
 + if (!list_empty(mReq-queue)) {
 + retval = -EBUSY;
 + dev_err(mEp-ci-dev, request already in queue\n);
 + goto done;
 + }
 +
 + if (req-length  4 * CI13XXX_PAGE_SIZE) {
 + retval = -EMSGSIZE;
 + dev_err(mEp-ci-dev, request bigger than one td\n);
 + goto done;
 + }
 +
 + dbg_queue(_usb_addr(mEp), req, retval);
 +
 + /* push request */
 + mReq-req.status = -EINPROGRESS;
 + mReq-req.actual = 0;
 +
 + retval = _hardware_enqueue(mEp, mReq);
 +
 + if (retval == -EALREADY) {
 + dbg_event(_usb_addr(mEp), QUEUE, retval);
 + retval = 0;
 + }
 + if (!retval)
 + list_add_tail(mReq-queue, mEp-qh.queue);
 +
 + done:
 + return retval;
 +}
 +
 +/**
   * isr_get_status_response: get_status request response
   * @ci: ci struct
   * @setup: setup request packet
 @@ -717,9 +782,7 @@ __acquires(mEp-lock)
   }
   /* else do nothing; reserved for future use */
  
 - spin_unlock(mEp-lock);
 - retval = usb_ep_queue(mEp-ep, req, gfp_flags);
 - spin_lock(mEp-lock);
 + retval = _ep_queue(mEp-ep, req, gfp_flags);
   if (retval)
   goto err_free_buf;
  
 @@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct 
 usb_request *req)
   * This function returns an error code
   */
  static int isr_setup_status_phase(struct ci13xxx *ci)
 -__releases(mEp-lock)
 -__acquires(mEp-lock)
  {
   int retval;
   struct ci13xxx_ep *mEp;
 @@ -776,9 +837,7 @@ __acquires(mEp-lock)
   ci-status-context = ci;
   ci-status-complete = isr_setup_status_complete;
  
 - spin_unlock(mEp-lock);
 - retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
 - spin_lock(mEp-lock);
 + retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
  
   return retval;
  }
 @@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct 
 usb_request *req,
   gfp_t __maybe_unused gfp_flags)
  {
   struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
 - struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
 - struct ci13xxx *ci = mEp-ci;
   int retval = 0;
   unsigned long flags;
  
 @@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct 
 usb_request *req,
  
   spin_lock_irqsave(mEp-lock, flags);
  
 - if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
 - if (req-length)
 - mEp = (ci-ep0_dir == RX) ?
 -ci-ep0out : ci-ep0in;
 - if (!list_empty(mEp-qh.queue)) {
 - _ep_nuke(mEp);
 -  

Re: [PATCH v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

2013-03-27 Thread Greg KH
On Wed, Mar 27, 2013 at 05:21:13PM +0100, Michael Grzeschik wrote:
 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 could
 read wrong data and totally stuck.
 
 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

How does this (and the other patches in this series) meet the
stable_kernel_rules.txt requirements?

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 v7 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Alexander Shishkin
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?

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 v7 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Marc Kleine-Budde
On 03/27/2013 05:35 PM, 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?

Patch 1/7 is essential to make the udc stable on armv5, should be added
to the patch description, though.

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: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Marc Kleine-Budde
On 03/27/2013 05:37 PM, Marc Kleine-Budde wrote:
 On 03/27/2013 05:35 PM, 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?
 
 Patch 1/7 is essential to make the udc stable on armv5, should be added
 to the patch description, though.

I mean 1/6.

usb: chipidea: udc: add attribute aligned(4) to shared memory structs

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: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

2013-03-27 Thread Michael Grzeschik
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.

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

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 0/4] usb: remote_wakeup code clean up and usb port related feature

2013-03-27 Thread Lan Tianyu
PATCH 1 is for code clean up.
PATCH 2~3 is to introduce usb port power off new feature
PATCH 4 is to fix usb port doesn't register to any bus_type 

usb: add usb_enable/disable_remote_wakeup()
usb: introduce usb force power off mechanism
usb: Add usb port system pm support
usb: register usb port to usb_bus_type


--
To unsubscribe from this list: send the line unsubscribe 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/4] usb: add usb_enable/disable_remote_wakeup()

2013-03-27 Thread Lan Tianyu
usb2.0 and usb3.0 devices have different ways to enalbe/disable
remote wakeup. This patch is to put both their operations into
the seperate functions. Otherwise, usb_control_msg() has some
long arguments and are usually nested some indentations. So
encapsulating it into seperate functions would be convienient
to use and more readable.

Signed-off-by: Lan Tianyu tianyu@intel.com
---
 drivers/usb/core/hub.c |  136 
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 7815462..6b7f5b9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2826,22 +2826,72 @@ void usb_enable_ltm(struct usb_device *udev)
 EXPORT_SYMBOL_GPL(usb_enable_ltm);
 
 #ifdef CONFIG_USB_SUSPEND
-/*
- * usb_disable_function_remotewakeup - disable usb3.0
- * device's function remote wakeup
- * @udev: target device
- *
- * Assume there's only one function on the USB 3.0
- * device and disable remote wake for the first
- * interface. FIXME if the interface association
- * descriptor shows there's more than one function.
- */
-static int usb_disable_function_remotewakeup(struct usb_device *udev)
+static int usb_disable_remote_wakeup(struct usb_device *udev)
 {
-   return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
-   USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
+   int status = 0;
+   u16 devstatus;
+
+   if (udev-speed != USB_SPEED_SUPER) {
+   status = usb_get_status(udev, USB_RECIP_DEVICE, 0, devstatus);
+   le16_to_cpus(devstatus);
+   if (!status  devstatus  (1  USB_DEVICE_REMOTE_WAKEUP))
+   status = usb_control_msg(udev,
+   usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_DEVICE,
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else {
+   /*
+* Assume there's only one function on the USB 3.0
+* device and disable remote wake for the first
+* interface. FIXME if the interface association
+* descriptor shows there's more than one function.
+*/
+   status = usb_get_status(udev, USB_RECIP_INTERFACE, 0,
+   devstatus);
+   le16_to_cpus(devstatus);
+   if (!status  devstatus 
+ (USB_INTRF_STAT_FUNC_RW_CAP | USB_INTRF_STAT_FUNC_RW))
+   status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_INTERFACE,
+   USB_INTRF_FUNC_SUSPEND, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   }
+
+   return status;
+}
+
+static int usb_enable_remote_wakeup(struct usb_device *udev)
+{
+   int status;
+
+   if (udev-speed != USB_SPEED_SUPER) {
+   status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+   USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else {
+   /* Assume there's only one function on the USB 3.0
+* device and enable remote wake for the first
+* interface. FIXME if the interface association
+* descriptor shows there's more than one function.
+*/
+   status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+   USB_REQ_SET_FEATURE,
+   USB_RECIP_INTERFACE,
+   USB_INTRF_FUNC_SUSPEND,
+   USB_INTRF_FUNC_SUSPEND_RW |
+   USB_INTRF_FUNC_SUSPEND_LP,
+   NULL, 0,
USB_CTRL_SET_TIMEOUT);
+   }
+
+   return status;
 }
 
 /*
@@ -2905,27 +2955,7 @@ int usb_port_suspend(struct usb_device *udev, 
pm_message_t msg)
 * we don't explicitly enable it here.
 */
if (udev-do_remote_wakeup) {
-   if (!hub_is_superspeed(hub-hdev)) {
-   status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
-   USB_DEVICE_REMOTE_WAKEUP, 0,
-   NULL, 0,
-   

[PATCH 3/4] usb: Add usb port system pm support

2013-03-27 Thread Lan Tianyu
This patch is to add usb port system pm support. Add
usb port's system suspend/resume callbacks and call
usb_port_runtime_resume/suspend() to power off these
ports whose pm qos NO_POWER_OFF flag is not set, system
wakeup is disabled and persist is enalbed.

During system pm, usb port should be powered off after
dev being suspended and powered on before dev being
resumed. Since usb ports and devs enable async suspend,
call device_pm_wait_for_dev() in the usb_port_system_suspend()
and usb_port_resume() to keeping the order.

If usb port was already powered off by runtime pm with
port_dev-power_is_on being false, usb_port_system_suspend()
returns directly.

If usb port was not powered off during system suspend with
port_dev-power_is_on being true, usb_port_system_resume()
returns directly.

Signed-off-by: Lan Tianyu tianyu@intel.com
---
 drivers/usb/core/hub.c  |4 
 drivers/usb/core/port.c |   49 ---
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c228e69..8c2562e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3166,6 +3166,10 @@ int usb_port_resume(struct usb_device *udev, 
pm_message_t msg)
int status;
u16 portchange, portstatus;
 
+   /* Wait for usb port system resume finishing */
+   if (!PMSG_IS_AUTO(msg))
+   device_pm_wait_for_dev(udev-dev, port_dev-dev);
+
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
port_dev-did_runtime_put = false;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 797f9d5..f14fc72 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -71,7 +71,7 @@ static void usb_port_device_release(struct device *dev)
kfree(port_dev);
 }
 
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM
 static int usb_port_runtime_resume(struct device *dev)
 {
struct usb_port *port_dev = to_usb_port(dev);
@@ -131,25 +131,68 @@ static int usb_port_runtime_suspend(struct device *dev)
set_bit(port1, hub-busy_bits);
retval = usb_hub_set_port_power(hdev, port1, false);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
-   usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
+   usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
clear_bit(port1, hub-busy_bits);
usb_autopm_put_interface(intf);
return retval;
 }
-#endif
+
+static int usb_port_system_suspend(struct device *dev)
+{
+   struct usb_port *port_dev = to_usb_port(dev);
+
+   if (!port_dev-power_is_on)
+   return 0;
+
+   if (port_dev-child) {
+   struct usb_device *udev = port_dev-child;
+
+   /*
+* usb port can't be powered off when dev's system
+* wakeup is enabled or persist is disabled.
+*/
+   if (device_may_wakeup(udev-dev)
+   || !udev-persist_enabled)
+   return 0;
+
+   /*
+* usb port should be powered off after usb dev
+* being suspended.
+*/
+   device_pm_wait_for_dev(dev, port_dev-child-dev);
+   }
+
+   usb_port_runtime_suspend(dev);
+   return 0;
+}
+
+static int usb_port_system_resume(struct device *dev)
+{
+   struct usb_port *port_dev = to_usb_port(dev);
+
+   if (port_dev-power_is_on)
+   return 0;
+
+   return usb_port_runtime_resume(dev);
+}
 
 static const struct dev_pm_ops usb_port_pm_ops = {
+   .suspend =  usb_port_system_suspend,
+   .resume =   usb_port_system_resume,
 #ifdef CONFIG_USB_SUSPEND
.runtime_suspend =  usb_port_runtime_suspend,
.runtime_resume =   usb_port_runtime_resume,
.runtime_idle = pm_generic_runtime_idle,
 #endif
 };
+#endif
 
 struct device_type usb_port_device_type = {
.name = usb_port,
.release =  usb_port_device_release,
+#if CONFIG_PM
.pm =   usb_port_pm_ops,
+#endif
 };
 
 int usb_hub_create_port_device(struct usb_hub *hub, int port1)
-- 
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


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

2013-03-27 Thread Lan Tianyu
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

Signed-off-by: Lan Tianyu tianyu@intel.com
---
 Documentation/ABI/testing/sysfs-bus-usb |6 +++---
 drivers/usb/core/port.c |   10 +-
 drivers/usb/core/usb-acpi.c |7 +--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index c8baaf5..842e8b8 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -221,14 +221,14 @@ Description:
The file will be present for all speeds of USB devices, and will
always read no for USB 1.1 and USB 2.0 devices.
 
-What:  /sys/bus/usb/devices/.../(hub interface)/portX
+What:  /sys/bus/usb/devices/.../(hub interface)/portX-X
 Date:  August 2012
 Contact:   Lan Tianyu tianyu@intel.com
 Description:
-   The /sys/bus/usb/devices/.../(hub interface)/portX
+   The /sys/bus/usb/devices/.../(hub interface)/portX-X
is usb port device's sysfs directory.
 
-What:  /sys/bus/usb/devices/.../(hub interface)/portX/connect_type
+What:  /sys/bus/usb/devices/.../(hub interface)/portX-X/connect_type
 Date:  January 2013
 Contact:   Lan Tianyu tianyu@intel.com
 Description:
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index f14fc72..2c086dc 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -198,6 +198,7 @@ struct device_type usb_port_device_type = {
 int usb_hub_create_port_device(struct usb_hub *hub, int port1)
 {
struct usb_port *port_dev = NULL;
+   struct usb_device *hdev = hub-hdev;
int retval;
 
port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
@@ -212,7 +213,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int 
port1)
port_dev-dev.parent = hub-intfdev;
port_dev-dev.groups = port_dev_group;
port_dev-dev.type = usb_port_device_type;
-   dev_set_name(port_dev-dev, port%d, port1);
+   port_dev-dev.bus = usb_bus_type;
+
+   if (!hdev-parent)
+   dev_set_name(port_dev-dev, port%d-%d,
+   hdev-bus-busnum, port1);
+   else
+   dev_set_name(port_dev-dev, port%s.%d,
+   dev_name(hdev-dev), port1);
 
retval = device_register(port_dev-dev);
if (retval)
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index b6f4bad..1cc55c9 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -17,7 +17,7 @@
 #include linux/pci.h
 #include acpi/acpi_bus.h
 
-#include usb.h
+#include hub.h
 
 /**
  * usb_acpi_power_manageable - check whether usb port has
@@ -178,7 +178,10 @@ static int usb_acpi_find_device(struct device *dev, 
acpi_handle *handle)
return -ENODEV;
return 0;
} else if (is_usb_port(dev)) {
-   sscanf(dev_name(dev), port%d, port_num);
+   struct usb_port *port_dev = to_usb_port(dev);
+
+   port_num = port_dev-portnum;
+
/* Get the struct usb_device point of port's hub */
udev = to_usb_device(dev-parent-parent);
 
-- 
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


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

2013-03-27 Thread Lan Tianyu
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  |   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, __u32)
 #define USBDEVFS_DISCONNECT_CLAIM  _IOR('U', 27, struct 
usbdevfs_disconnect_claim)
+#define USBDEVFS_POWER_RESET  _IO('U', 28)
 
 #endif /* _UAPI_LINUX_USBDEVICE_FS_H */
-- 
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: [PATCH v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

2013-03-27 Thread Michael Grzeschik
On Wed, Mar 27, 2013 at 09:31:33AM -0700, Greg KH wrote:
 On Wed, Mar 27, 2013 at 05:21:13PM +0100, Michael Grzeschik wrote:
  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 could
  this should be will ---^

  read wrong data and totally stuck.
  
  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
 
 How does this (and the other patches in this series) meet the
 stable_kernel_rules.txt requirements?

Beside the other patches, this one meets the requirements. I will reword
that patch, leave the Cc: stable in it, remove it from the others and
repost the series.

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

2013-03-27 Thread Lan, Tianyu
A small tool to test this patch.

#include stdio.h
#include unistd.h
#include fcntl.h
#include errno.h
#include sys/ioctl.h
#include linux/usbdevice_fs.h

#define USBDEVFS_POWER_RESET   _IO('U', 28)

int main(int argc, char **argv)
{
const char *filename;
int fd;
int rc;

if (argc != 2) {
fprintf(stderr, Usage: usbreset device-filename\n);
return 1;
}
filename = argv[1];

fd = open(filename, O_WRONLY);
if (fd  0) {
perror(Error opening output file);
return 1;
}

printf(Repowering USB device %s\n, filename);
rc = ioctl(fd, USBDEVFS_POWER_RESET, 0);
if (rc  0) {
perror(Error in ioctl);
return 1;
}
printf(Repower successful\n);

close(fd);
return 0;
}


Best Regards 
Tianyu Lan 


-Original Message-
From: Lan, Tianyu 
Sent: Thursday, March 28, 2013 1:11 AM
To: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; 
st...@rowland.harvard.edu
Cc: linux-usb@vger.kernel.org; Lan, Tianyu
Subject: [PATCH 2/4] 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  |   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 @@ 

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

2013-03-27 Thread Laurent Pinchart
Hi Bhupesh,

Thanks for the patch. Please see below for a couple of small comments.

On Thursday 21 March 2013 13:56:08 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
 ---
  drivers/usb/gadget/Kconfig |1 +
  drivers/usb/gadget/uvc_queue.c |  533 -
  drivers/usb/gadget/uvc_queue.h |   25 +--
  drivers/usb/gadget/uvc_v4l2.c  |   37 +--
  drivers/usb/gadget/uvc_video.c |   17 +-
  5 files changed, 188 insertions(+), 425 deletions(-)

[snip]

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

[snip]

 +static int uvc_queue_init(struct uvc_video_queue *queue,
 +   enum v4l2_buf_type type)
  {
 + int ret;
 
 + queue-queue.type = type;
 + queue-queue.io_modes = VB2_MMAP | VB2_USERPTR;
 + queue-queue.drv_priv = queue;
 + queue-queue.buf_struct_size = sizeof(struct uvc_buffer);
 + queue-queue.ops = uvc_queue_qops;
 + queue-queue.mem_ops = vb2_vmalloc_memops;
 + ret = vb2_queue_init(queue-queue);
 + if (ret)
 + return ret;
 
 + mutex_init(queue-mutex);
 + spin_lock_init(queue-irqlock);
 + INIT_LIST_HEAD(queue-irqqueue);

I think you should initialize queue-flags to 0 here.

 
 + return 0;
  }

[snip]

 +static int uvc_queue_buffer(struct uvc_video_queue *queue,
 + struct v4l2_buffer *buf)
  {
 + int ret;
 +
 + mutex_lock(queue-mutex);
 + ret = vb2_qbuf(queue-queue, buf);
 + ret = (queue-flags  UVC_QUEUE_PAUSED) != 0;
 + queue-flags = ~UVC_QUEUE_PAUSED;

Access to the queue flags need to be protected by the irqlock spinlock here 
(in addition to the queue mutex).

 + mutex_unlock(queue-mutex);
 
 + return ret;
  }

[snip]

 @@ -516,26 +283,32 @@ static void uvc_queue_cancel(struct uvc_video_queue
 *queue, int disconnect) */
  static int uvc_queue_enable(struct uvc_video_queue *queue, int enable)
  {
 - unsigned int i;
 + unsigned long flags;
   int ret = 0;
 
   mutex_lock(queue-mutex);
   if (enable) {
 - if (uvc_queue_streaming(queue)) {
 - ret = -EBUSY;
 + ret = vb2_streamon(queue-queue, queue-queue.type);
 + if (ret  0)
   goto done;
 - }
 - queue-sequence = 0;

Shouldn't you keep this line ?

 - queue-flags |= UVC_QUEUE_STREAMING;
 +
   queue-buf_used = 0;
   } else {
 - uvc_queue_cancel(queue, 0);
 - INIT_LIST_HEAD(queue-mainqueue);
 + ret = vb2_streamoff(queue-queue, queue-queue.type);
 + if (ret  0)
 + goto done;
 
 - for (i = 0; i  queue-count; ++i)
 - queue-buffer[i].state = UVC_BUF_STATE_IDLE;
 + spin_lock_irqsave(queue-irqlock, flags);
 + INIT_LIST_HEAD(queue-irqqueue);
 
 - queue-flags = ~UVC_QUEUE_STREAMING;
 + /*
 +  * FIXME: We need to clear the DISCONNECTED flag to ensure that
 +  * applications will be able to queue buffers for the next
 +  * streaming run. However, clearing it here doesn't guarantee
 +  * that the device will be reconnected in the meantime.
 +  */
 + queue-flags = ~UVC_QUEUE_DISCONNECTED;
 + spin_unlock_irqrestore(queue-irqlock, flags);
   }

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

2013-03-27 Thread Laurent Pinchart
Hi Bhupesh,

Thanks for the patch.

On Thursday 21 March 2013 13:56:09 Bhupesh Sharma wrote:
 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 3f5eeae..d0b8e38 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c
 @@ -228,6 +228,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
  };
-- 
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-27 Thread Laurent Pinchart
Hi Ravi,

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) 
doesn't show any significant source of delay. You will likely need to profile 
the code to find out where the problem comes from. You should also make sure 
that no other IRQ handler on the system keeps IRQs disabled for a long time.

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

2013-03-27 Thread Alan Stern
On Wed, 27 Mar 2013, David Linares wrote:

 Thanks Alan. I understand, but the USB cable is plugged into an AC/DC
 adaptor which can provide an output of 1A. After breaking out an USB
 cable, I measured that the consumption of my board is around 130mA
 (nothing connected to the hub). Then I measured with the dongle
 plugged in. The real consumption of the wireless adapter is around
 210mA. The whole thing draws around 340mA

Ah, okay.  Then maybe this isn't a power issue.

 Today, I did some further tests *without* any wireless adapter. So, I
 have got the device plugged in into a main power socket via USB
 (delivering up to 1A), the root hub, and the 2-port hub and nothing
 else USB-wise.
 The problem still happens from time to time. The 2-port hub sometimes
 says it will give 500mA to its children and sometimes it gives only
 100mA (because of a corrupted hubstatus mentioned earlier).
 Now that there is nothing connected on the hub, I am just wondering
 how come the hub is not always detected in the same way at each boot.
 Also, should it be detected as self powered (
 http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or
 not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538
 )?

It sounds like the hub is rather flaky.  Also, it should always
describe itself as bus-powered, never as self-powered.  Maybe you 
should try using a different kind of hub.

 I forgot to mention that this issue where the hub is not detected as
 self-powered external hub is quite hard to reproduce. It happens 1
 out of 20 tries.

This could even be a loose electrical connection, silly as that seems.

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.

 Thanks a lot Alan and everybody else. I really appreciate the support
 and understanding you are providing.

You're welcome.

Alan Stern

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


Re: [PATCH 1/4] usb: add usb_enable/disable_remote_wakeup()

2013-03-27 Thread Alan Stern
On Thu, 28 Mar 2013, Lan Tianyu wrote:

 usb2.0 and usb3.0 devices have different ways to enalbe/disable
 remote wakeup. This patch is to put both their operations into
 the seperate functions. Otherwise, usb_control_msg() has some
 long arguments and are usually nested some indentations. So
 encapsulating it into seperate functions would be convienient
 to use and more readable.
 
 Signed-off-by: Lan Tianyu tianyu@intel.com

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

2013-03-27 Thread Alan Stern
On Thu, 28 Mar 2013, 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.
 
 This patch is also helpful fo some QAs who want to do hcd's memleak
 test(Plug and unplug device thousand times.)

This is not a bad idea, but it needs a little more work...

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

What happens if hdev is NULL?

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

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

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

 + 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().

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

2013-03-27 Thread Alan Stern
On Thu, 28 Mar 2013, Lan Tianyu wrote:

 This patch is to add usb port system pm support. Add
 usb port's system suspend/resume callbacks and call
 usb_port_runtime_resume/suspend() to power off these
 ports whose pm qos NO_POWER_OFF flag is not set, system
 wakeup is disabled and persist is enalbed.
 
 During system pm, usb port should be powered off after
 dev being suspended and powered on before dev being
 resumed. Since usb ports and devs enable async suspend,
 call device_pm_wait_for_dev() in the usb_port_system_suspend()
 and usb_port_resume() to keeping the order.
 
 If usb port was already powered off by runtime pm with
 port_dev-power_is_on being false, usb_port_system_suspend()
 returns directly.
 
 If usb port was not powered off during system suspend with
 port_dev-power_is_on being true, usb_port_system_resume()
 returns directly.
 
 Signed-off-by: Lan Tianyu tianyu@intel.com

...

 +static int usb_port_system_suspend(struct device *dev)
 +{
 + struct usb_port *port_dev = to_usb_port(dev);
 +
 + if (!port_dev-power_is_on)
 + return 0;
 +
 + if (port_dev-child) {
 + struct usb_device *udev = port_dev-child;
 +
 + /*
 +  * usb port can't be powered off when dev's system
 +  * wakeup is enabled or persist is disabled.
 +  */
 + if (device_may_wakeup(udev-dev)
 + || !udev-persist_enabled)
 + return 0;
 +
 + /*
 +  * usb port should be powered off after usb dev
 +  * being suspended.
 +  */
 + device_pm_wait_for_dev(dev, port_dev-child-dev);
 + }
 +
 + usb_port_runtime_suspend(dev);
 + return 0;
 +}

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

--
To unsubscribe from this list: send the line unsubscribe 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

2013-03-27 Thread Alan Stern
On Thu, 28 Mar 2013, 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
 
 Signed-off-by: Lan Tianyu tianyu@intel.com

This ends up looking like a mess, but I guess there's no way around it.

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

2013-03-27 Thread Alan Stern
On Wed, 27 Mar 2013, Lan, Tianyu wrote:

 A small tool to test this patch.
 
 #include stdio.h
 #include unistd.h
 #include fcntl.h
 #include errno.h
 #include sys/ioctl.h
 #include linux/usbdevice_fs.h
 
 #define USBDEVFS_POWER_RESET _IO('U', 28)
 
 int main(int argc, char **argv)
 {
   const char *filename;
   int fd;
   int rc;
 
   if (argc != 2) {
   fprintf(stderr, Usage: usbreset device-filename\n);

The name should be usbrepower (or usb-repower, which is more readable).  
Not usbreset -- there's already a different program using that name.

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-27 Thread Paul Zimmerman
 From: Matthijs Kooijman [mailto:matth...@stdin.nl]
 Sent: Wednesday, March 27, 2013 8:42 AM
 To: Greg KH
 
 Hi Greg,
 
  But yes, it does seem to be a good idea,
 
 Ok, thanks.
 
  as long as these options can't change over time.
 It's only read-only registers, or registers whose power-on values
 determine their maximum possible value, so they indeed won't change over
 time.

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

But since the driver is still in staging and has no real users yet, that's
not a big deal I guess.

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.

-- 
Paul

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


Re: [PATCH] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Peter Vasil
Hi again,
the patch works fine on my WM8850 netbook. I didn't get any
unused-function warning on build, most likely because I have CONFIG_PM
set in kernel config.
Regards,
Peter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)

2013-03-27 Thread David Linares
On 27 March 2013 18:35, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 27 Mar 2013, David Linares wrote:

 Thanks Alan. I understand, but the USB cable is plugged into an AC/DC
 adaptor which can provide an output of 1A. After breaking out an USB
 cable, I measured that the consumption of my board is around 130mA
 (nothing connected to the hub). Then I measured with the dongle
 plugged in. The real consumption of the wireless adapter is around
 210mA. The whole thing draws around 340mA

 Ah, okay.  Then maybe this isn't a power issue.

 Today, I did some further tests *without* any wireless adapter. So, I
 have got the device plugged in into a main power socket via USB
 (delivering up to 1A), the root hub, and the 2-port hub and nothing
 else USB-wise.
 The problem still happens from time to time. The 2-port hub sometimes
 says it will give 500mA to its children and sometimes it gives only
 100mA (because of a corrupted hubstatus mentioned earlier).
 Now that there is nothing connected on the hub, I am just wondering
 how come the hub is not always detected in the same way at each boot.
 Also, should it be detected as self powered (
 http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or
 not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538
 )?

 It sounds like the hub is rather flaky.  Also, it should always
 describe itself as bus-powered, never as self-powered.  Maybe you
 should try using a different kind of hub.


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

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.

 I forgot to mention that this issue where the hub is not detected as
 self-powered external hub is quite hard to reproduce. It happens 1
 out of 20 tries.

 This could even be a loose electrical connection, silly as that seems.

Yes, that could very well be. I will inspect that tomorrow


 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.
Thanks again.


 Thanks a lot Alan and everybody else. I really appreciate the support
 and understanding you are providing.

 You're welcome.

 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 v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Arnd Bergmann
This lets us use the ehci-generic driver on platforms without special
requirements for their ehci controllers. In particular, this is true
for the vt8500/wm8x50 platforms, which currently have a separate
driver that causes problems with multiplatform configurations.

Tested-by: Tony Prisk li...@prisktech.co.nz
Tested-by: Peter Vasil peterva...@gmail.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Arnd Bergmann a...@arndb.de
---
v3: always set dma_mask and coherent_dma_mask when not already set,
rather than overriding them for any device without platform data
v2: reset the platform data pointer on unload


 drivers/usb/host/ehci-hcd.c  |   5 --
 drivers/usb/host/ehci-platform.c |  28 ++--
 drivers/usb/host/ehci-vt8500.c   | 150 -
 3 files changed, 22 insertions(+), 161 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 0c3314c..960e7cf 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1286,11 +1286,6 @@ MODULE_LICENSE (GPL);
 #define PLATFORM_DRIVERehci_octeon_driver
 #endif
 
-#ifdef CONFIG_ARCH_VT8500
-#include ehci-vt8500.c
-#definePLATFORM_DRIVER vt8500_ehci_driver
-#endif
-
 #ifdef CONFIG_PLAT_SPEAR
 #include ehci-spear.c
 #define PLATFORM_DRIVERspear_ehci_hcd_driver
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index ca75063..cda0fa9 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -18,11 +18,13 @@
  *
  * Licensed under the GNU/GPL. See COPYING for details.
  */
+#include linux/dma-mapping.h
 #include linux/err.h
 #include linux/kernel.h
 #include linux/hrtimer.h
 #include linux/io.h
 #include linux/module.h
+#include linux/of.h
 #include linux/platform_device.h
 #include linux/usb.h
 #include linux/usb/hcd.h
@@ -62,22 +64,32 @@ static const struct ehci_driver_overrides 
platform_overrides __initdata = {
.reset =ehci_platform_reset,
 };
 
+static struct usb_ehci_pdata ehci_platform_defaults;
+
 static int ehci_platform_probe(struct platform_device *dev)
 {
struct usb_hcd *hcd;
struct resource *res_mem;
-   struct usb_ehci_pdata *pdata = dev-dev.platform_data;
+   struct usb_ehci_pdata *pdata;
int irq;
int err = -ENOMEM;
 
-   if (!pdata) {
-   WARN_ON(1);
-   return -ENODEV;
-   }
-
if (usb_disabled())
return -ENODEV;
 
+   /*
+* use reasonable defaults so platforms don't have to provide these.
+* with DT probing on ARM, none of these are set.
+*/
+   if (!dev-dev.platform_data)
+   dev-dev.platform_data = ehci_platform_defaults;
+   if (!dev-dev.dma_mask)
+   dev-dev.dma_mask = dev-dev.coherent_dma_mask;
+   if (!dev-dev.coherent_dma_mask)
+   dev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+   pdata = dev-dev.platform_data;
+
irq = platform_get_irq(dev, 0);
if (irq  0) {
dev_err(dev-dev, no irq provided);
@@ -139,6 +151,9 @@ static int ehci_platform_remove(struct platform_device *dev)
if (pdata-power_off)
pdata-power_off(dev);
 
+   if (pdata == ehci_platform_defaults)
+   dev-dev.platform_data = NULL;
+
return 0;
 }
 
@@ -183,6 +198,12 @@ static int ehci_platform_resume(struct device *dev)
 #define ehci_platform_resume   NULL
 #endif /* CONFIG_PM */
 
+static const struct of_device_id vt8500_ehci_ids[] = {
+   { .compatible = via,vt8500-ehci, },
+   { .compatible = wm,prizm-ehci, },
+   {}
+};
+
 static const struct platform_device_id ehci_platform_table[] = {
{ ehci-platform, 0 },
{ }
@@ -203,6 +224,7 @@ static struct platform_driver ehci_platform_driver = {
.owner  = THIS_MODULE,
.name   = ehci-platform,
.pm = ehci_platform_pm_ops,
+   .of_match_table = of_match_ptr(vt8500_ehci_ids),
}
 };
 
diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c
deleted file mode 100644
index 7ecf709..000
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Piggy-backing new hardware using old usb-serial

2013-03-27 Thread Wesley W. Terpstra
Good evening,

We are developing a custom piece of hardware that provides the
equivalent of two serial console interfaces. They do not speak the AT
command set. One port provides an interactive console to the user for
configuration purposes (via minicom/etc). The other speaks a proprietary
protocol used by purpose-built userspace tools. Neither port has an
inherent baud rate limit; both can fully utilize USB2 bandwidth.

The closest standard USB-IF class I could find is the cdc-acm device
class. However, this seems to be very much targeted at modems with ATx
commands. The linux cdc-acm drivers would put the hardware
at /dev/ttyACM and software seems to treat these like modems. The closer
matching usb-serial dongles all appear to have unstandardized drivers.

We would like our hardware to work out of the box on released Linux
versions (and to a lesser extent on windows). Our current prototypes
borrow the Sierra VID to trick the kernel into loading the sierra
driver. This works well for the interactive console. However, I assume
that distributing the device like this will have legal consequences.

I could write a custom driver, but then end users need to compile
+install it. Our device will never be widely available and thus our
driver will never be included in linux, ie: even future users will have
to compile+install for themselves. Because the USB logic is inside an
FPGA, I can readily change the hardware to suite any existing driver.

What driver should I target?
Is there a way to AUTOload an older driver despite the new VID:PID?
Should I give up and require a custom driver?

Thank you for any and all feedback/advice.


--
To unsubscribe from this list: send the line unsubscribe 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-27 Thread Alan Stern
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.

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-27 Thread Greg KH
On Wed, Mar 27, 2013 at 08:45:38PM +0100, Wesley W. Terpstra wrote:
 Good evening,
 
 We are developing a custom piece of hardware that provides the
 equivalent of two serial console interfaces. They do not speak the AT
 command set. One port provides an interactive console to the user for
 configuration purposes (via minicom/etc). The other speaks a proprietary
 protocol used by purpose-built userspace tools. Neither port has an
 inherent baud rate limit; both can fully utilize USB2 bandwidth.
 
 The closest standard USB-IF class I could find is the cdc-acm device
 class. However, this seems to be very much targeted at modems with ATx
 commands. The linux cdc-acm drivers would put the hardware
 at /dev/ttyACM and software seems to treat these like modems. The closer
 matching usb-serial dongles all appear to have unstandardized drivers.
 
 We would like our hardware to work out of the box on released Linux
 versions (and to a lesser extent on windows). Our current prototypes
 borrow the Sierra VID to trick the kernel into loading the sierra
 driver. This works well for the interactive console. However, I assume
 that distributing the device like this will have legal consequences.

Yes, Sierra will get mad at you :)

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.

 I could write a custom driver, but then end users need to compile
 +install it. Our device will never be widely available and thus our
 driver will never be included in linux, ie: even future users will have
 to compile+install for themselves. Because the USB logic is inside an
 FPGA, I can readily change the hardware to suite any existing driver.

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.

 What driver should I target?

What chip do you use for your device?

If you just want raw data, then do something like the
drivers/usb/serial/zio.c driver, tiny, simple, and trivial.

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


Re: [PATCH v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Alan Stern
On Wed, 27 Mar 2013, Arnd Bergmann wrote:

 This lets us use the ehci-generic driver on platforms without special
 requirements for their ehci controllers.

Well, actually the driver's name is ehci-platform.  And the patch
lets us use it on platforms where the DMA mask hasn't been set up, as 
often happens with DT.

  In particular, this is true
 for the vt8500/wm8x50 platforms, which currently have a separate
 driver that causes problems with multiplatform configurations.
 
 Tested-by: Tony Prisk li...@prisktech.co.nz
 Tested-by: Peter Vasil peterva...@gmail.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Arnd Bergmann a...@arndb.de
 ---
 v3: always set dma_mask and coherent_dma_mask when not already set,
 rather than overriding them for any device without platform data
 v2: reset the platform data pointer on unload

The patch all looks good to me.

Alan Stern

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


[PATCH] USB: avoid error messages when a device is disconnected

2013-03-27 Thread Alan Stern
This patch (as1673) reduces the amount of log spew from the hub driver
by removing a bunch of error messages in the case where the device in
question is already known to have been disconnected.  Since the
disconnect event itself appears in the log, there's no need for other
error messages.

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

---

 drivers/usb/core/generic.c |2 -
 drivers/usb/core/hub.c |   65 -
 2 files changed, 37 insertions(+), 30 deletions(-)

Index: usb-3.9/drivers/usb/core/hub.c
===
--- usb-3.9.orig/drivers/usb/core/hub.c
+++ usb-3.9/drivers/usb/core/hub.c
@@ -555,8 +555,9 @@ static int hub_port_status(struct usb_hu
mutex_lock(hub-status_mutex);
ret = get_port_status(hub-hdev, port1, hub-status-port);
if (ret  4) {
-   dev_err(hub-intfdev,
-   %s failed (err = %d)\n, __func__, ret);
+   if (ret != -ENODEV)
+   dev_err(hub-intfdev,
+   %s failed (err = %d)\n, __func__, ret);
if (ret = 0)
ret = -EIO;
} else {
@@ -699,7 +700,7 @@ static void hub_tt_work(struct work_stru
/* drop lock so HCD can concurrently report other TT errors */
spin_unlock_irqrestore (hub-tt.lock, flags);
status = hub_clear_tt_buffer (hdev, clear-devinfo, clear-tt);
-   if (status)
+   if (status  status != -ENODEV)
dev_err (hdev-dev,
clear tt %d (%04x) error %d\n,
clear-tt, clear-devinfo, status);
@@ -837,10 +838,11 @@ static int hub_hub_status(struct usb_hub
 
mutex_lock(hub-status_mutex);
ret = get_hub_status(hub-hdev, hub-status-hub);
-   if (ret  0)
-   dev_err (hub-intfdev,
-   %s failed (err = %d)\n, __func__, ret);
-   else {
+   if (ret  0) {
+   if (ret != -ENODEV)
+   dev_err(hub-intfdev,
+   %s failed (err = %d)\n, __func__, ret);
+   } else {
*status = le16_to_cpu(hub-status-hub.wHubStatus);
*change = le16_to_cpu(hub-status-hub.wHubChange); 
ret = 0;
@@ -877,11 +879,8 @@ static int hub_usb3_port_disable(struct
return -EINVAL;
 
ret = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_SS_DISABLED);
-   if (ret) {
-   dev_err(hub-intfdev, cannot disable port %d (err = %d)\n,
-   port1, ret);
+   if (ret)
return ret;
-   }
 
/* Wait for the link to enter the disabled state. */
for (total_time = 0; ; total_time += HUB_DEBOUNCE_STEP) {
@@ -918,7 +917,7 @@ static int hub_port_disable(struct usb_h
ret = usb_clear_port_feature(hdev, port1,
USB_PORT_FEAT_ENABLE);
}
-   if (ret)
+   if (ret  ret != -ENODEV)
dev_err(hub-intfdev, cannot disable port %d (err = %d)\n,
port1, ret);
return ret;
@@ -2192,8 +2191,9 @@ static int usb_enumerate_device(struct u
if (udev-config == NULL) {
err = usb_get_configuration(udev);
if (err  0) {
-   dev_err(udev-dev, can't read configurations, error 
%d\n,
-   err);
+   if (err != -ENODEV)
+   dev_err(udev-dev, can't read configurations, 
error %d\n,
+   err);
return err;
}
}
@@ -2640,14 +2640,16 @@ static int hub_port_reset(struct usb_hub
status = set_port_feature(hub-hdev, port1, (warm ?
USB_PORT_FEAT_BH_PORT_RESET :
USB_PORT_FEAT_RESET));
-   if (status) {
+   if (status == -ENODEV) {
+   ;   /* The hub is gone */
+   } else if (status) {
dev_err(hub-intfdev,
cannot %sreset port %d (err = %d)\n,
warm ? warm  : , port1, status);
} else {
status = hub_port_wait_reset(hub, port1, udev, delay,
warm);
-   if (status  status != -ENOTCONN)
+   if (status  status != -ENOTCONN  status != -ENODEV)
dev_dbg(hub-intfdev,
port_wait_reset: err = %d\n,
status);
@@ 

[PATCH 1/2] USB: use global suspend for system sleep on USB-2 buses

2013-03-27 Thread Alan Stern
This patch (as1674) speeds up system sleep transitions by not
suspending each individual device on a USB-1.1 or USB-2 bus.  The
devices will automatically go into suspend when their root hubs are
suspended (i.e., stop sending out Start-Of-Frame packets) -- this is
what the USB spec calls global suspend.

Since this is what we do already when CONFIG_USB_SUSPEND isn't
enabled, it shouldn't cause any problems.

Signed-off-by: Alan Stern st...@rowland.harvard.edu
CC: Peter Chen peter.c...@freescale.com

---

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

Index: usb-3.9/drivers/usb/core/hub.c
===
--- usb-3.9.orig/drivers/usb/core/hub.c
+++ usb-3.9/drivers/usb/core/hub.c
@@ -2882,9 +2882,11 @@ static int usb_disable_function_remotewa
  * Linux (2.6) currently has NO mechanisms to initiate that:  no khubd
  * timer, no SRP, no requests through sysfs.
  *
- * If CONFIG_USB_SUSPEND isn't enabled, devices only really suspend when
- * the root hub for their bus goes into global suspend ... so we don't
- * (falsely) update the device power state to say it suspended.
+ * If CONFIG_USB_SUSPEND isn't enabled, non-SuperSpeed devices really get
+ * suspended only when their bus goes into global suspend (i.e., the root
+ * hub is suspended).  Nevertheless, we change @udev-state to
+ * USB_STATE_SUSPENDED as this is the device's logical state.  The actual
+ * upstream port setting is stored in @udev-port_is_suspended.
  *
  * Returns 0 on success, else negative errno.
  */
@@ -2895,6 +2897,7 @@ int usb_port_suspend(struct usb_device *
enum pm_qos_flags_status pm_qos_stat;
int port1 = udev-portnum;
int status;
+   boolreally_suspend = true;
 
/* enable remote wakeup when appropriate; this lets the device
 * wake up the upstream hub (including maybe the root hub).
@@ -2951,9 +2954,19 @@ int usb_port_suspend(struct usb_device *
/* see 7.1.7.6 */
if (hub_is_superspeed(hub-hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
-   else
+   else if (PMSG_IS_AUTO(msg))
status = set_port_feature(hub-hdev, port1,
USB_PORT_FEAT_SUSPEND);
+   /*
+* For system suspend, we do not need to enable the suspend feature
+* on individual USB-2 ports.  The devices will automatically go
+* into suspend a few ms after the root hub stops sending packets.
+* The USB 2.0 spec calls this global suspend.
+*/
+   else {
+   really_suspend = false;
+   status = 0;
+   }
if (status) {
dev_dbg(hub-intfdev, can't suspend port %d, status %d\n,
port1, status);
@@ -2989,8 +3002,10 @@ int usb_port_suspend(struct usb_device *
(PMSG_IS_AUTO(msg) ? auto- : ),
udev-do_remote_wakeup);
usb_set_device_state(udev, USB_STATE_SUSPENDED);
-   udev-port_is_suspended = 1;
-   msleep(10);
+   if (really_suspend) {
+   udev-port_is_suspended = 1;
+   msleep(10);
+   }
}
 
/*

--
To unsubscribe from this list: send the line unsubscribe 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] USB: remove CONFIG_USB_SUSPEND option

2013-03-27 Thread Alan Stern
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

---

 drivers/usb/core/Kconfig |   16 
 drivers/usb/core/driver.c|4 ++--
 drivers/usb/core/hcd.c   |   10 +-
 drivers/usb/core/hub.c   |   42 +++---
 drivers/usb/core/port.c  |4 ++--
 drivers/usb/core/sysfs.c |4 ++--
 drivers/usb/core/usb.c   |4 ++--
 drivers/usb/core/usb.h   |2 +-
 drivers/usb/host/ehci-pci.c  |   12 +---
 drivers/usb/host/ohci-hub.c  |6 --
 drivers/usb/host/sl811-hcd.c |2 +-
 drivers/usb/host/u132-hcd.c  |9 +
 drivers/usb/host/xhci-hub.c  |2 +-
 drivers/usb/host/xhci.c  |4 ++--
 include/linux/usb.h  |2 +-
 include/linux/usb/hcd.h  |6 +++---
 16 files changed, 35 insertions(+), 94 deletions(-)

Index: usb-3.9/drivers/usb/core/Kconfig
===
--- usb-3.9.orig/drivers/usb/core/Kconfig
+++ usb-3.9/drivers/usb/core/Kconfig
@@ -38,22 +38,6 @@ config USB_DYNAMIC_MINORS
 
  If you are unsure about this, say N here.
 
-config USB_SUSPEND
-   bool USB runtime power management (autosuspend) and wakeup
-   depends on USB  PM_RUNTIME
-   help
- If you say Y here, you can use driver calls or the sysfs
- power/control file to enable or disable autosuspend for
- individual USB peripherals (see
- Documentation/usb/power-management.txt for more details).
-
- Also, USB remote wakeup signaling is supported, whereby some
- USB devices (like keyboards and network adapters) can wake up
- their parent hub.  That wakeup cascades up the USB tree, and
- could wake the system from states like suspend-to-RAM.
-
- If you are unsure about this, say N here.
-
 config USB_OTG
bool OTG support
depends on USB
Index: usb-3.9/drivers/usb/core/driver.c
===
--- usb-3.9.orig/drivers/usb/core/driver.c
+++ usb-3.9/drivers/usb/core/driver.c
@@ -1407,7 +1407,7 @@ int usb_resume(struct device *dev, pm_me
 
 #endif /* CONFIG_PM */
 
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM_RUNTIME
 
 /**
  * usb_enable_autosuspend - allow a USB device to be autosuspended
@@ -1775,7 +1775,7 @@ int usb_set_usb2_hardware_lpm(struct usb
return ret;
 }
 
-#endif /* CONFIG_USB_SUSPEND */
+#endif /* CONFIG_PM_RUNTIME */
 
 struct bus_type usb_bus_type = {
.name = usb,
Index: usb-3.9/drivers/usb/core/hcd.c
===
--- usb-3.9.orig/drivers/usb/core/hcd.c
+++ usb-3.9/drivers/usb/core/hcd.c
@@ -2125,7 +2125,7 @@ int hcd_bus_resume(struct usb_device *rh
 
 #endif /* CONFIG_PM */
 
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM_RUNTIME
 
 /* Workqueue routine for root-hub remote wakeup */
 static void hcd_resume_work(struct work_struct *work)
@@ -2160,7 +2160,7 @@ void usb_hcd_resume_root_hub (struct usb
 }
 EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
 
-#endif /* CONFIG_USB_SUSPEND */
+#endif /* CONFIG_PM_RUNTIME */
 
 /*-*/
 
@@ -2336,7 +2336,7 @@ struct usb_hcd *usb_create_shared_hcd(co
init_timer(hcd-rh_timer);
hcd-rh_timer.function = rh_timer_func;
hcd-rh_timer.data = (unsigned long) hcd;
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM_RUNTIME
INIT_WORK(hcd-wakeup_work, hcd_resume_work);
 #endif
 
@@ -2582,7 +2582,7 @@ error_create_attr_group:
hcd-rh_registered = 0;
spin_unlock_irq(hcd_root_hub_lock);
 
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM_RUNTIME
cancel_work_sync(hcd-wakeup_work);
 #endif
mutex_lock(usb_bus_list_lock);
@@ -2637,7 +2637,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
hcd-rh_registered = 0;
spin_unlock_irq (hcd_root_hub_lock);
 
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM_RUNTIME
cancel_work_sync(hcd-wakeup_work);
 #endif
 
Index: usb-3.9/drivers/usb/core/hub.c
===
--- usb-3.9.orig/drivers/usb/core/hub.c
+++ usb-3.9/drivers/usb/core/hub.c
@@ -2823,7 +2823,7 @@ void usb_enable_ltm(struct usb_device *u
 }
 EXPORT_SYMBOL_GPL(usb_enable_ltm);
 
-#ifdef CONFIG_USB_SUSPEND

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

2013-03-27 Thread Alexander Shishkin
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?
At least the xscale version seems to be fine.

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

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

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

2013-03-27 Thread Wesley W. Terpstra
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. I take it then, that just adding
support to an existing driver is acceptable?

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.

  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 just want raw data, then do something like the
 drivers/usb/serial/zio.c driver, tiny, simple, and trivial.

Yes, if I make source-level changes to the kernel I have many options.

PS. Thank you for the very prompt reply!


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


Re: [PATCH v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Peter Vasil
On Wed, Mar 27, 2013 at 9:09 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 27 Mar 2013, Arnd Bergmann wrote:

 This lets us use the ehci-generic driver on platforms without special
 requirements for their ehci controllers.

 Well, actually the driver's name is ehci-platform.  And the patch
 lets us use it on platforms where the DMA mask hasn't been set up, as
 often happens with DT.

  In particular, this is true
 for the vt8500/wm8x50 platforms, which currently have a separate
 driver that causes problems with multiplatform configurations.

 Tested-by: Tony Prisk li...@prisktech.co.nz
 Tested-by: Peter Vasil peterva...@gmail.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Arnd Bergmann a...@arndb.de
 ---
 v3: always set dma_mask and coherent_dma_mask when not already set,
 rather than overriding them for any device without platform data
 v2: reset the platform data pointer on unload

 The patch all looks good to me.

 Alan Stern

 --
 You received this message because you are subscribed to the Google Groups 
 VT8500/WM8505 Linux Kernel group.
 To unsubscribe from this group and stop receiving emails from it, send an 
 email to vt8500-wm8505-linux-kernel+unsubscr...@googlegroups.com.
 For more options, visit https://groups.google.com/groups/opt_out.



Hi, unfortunately I still can't apply it on 3.9-rc4 :-(

$ git apply --check ehci-dt-bus-glue.patch
error: removal patch leaves file contents
error: drivers/usb/host/ehci-vt8500.c: patch does not apply

Looks git really doesn't like ehci-vt8500.c removal. If I delete the
file manually, git diff actually outputs full file content prefixed
with -.
Regards,
Peter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-03-27 Thread Michael Grzeschik
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?

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

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 v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Arnd Bergmann
On Wednesday 27 March 2013, Peter Vasil wrote:
 Hi, unfortunately I still can't apply it on 3.9-rc4 :-(
 
 $ git apply --check ehci-dt-bus-glue.patch
 error: removal patch leaves file contents
 error: drivers/usb/host/ehci-vt8500.c: patch does not apply
 
 Looks git really doesn't like ehci-vt8500.c removal. If I delete the
 file manually, git diff actually outputs full file content prefixed
 with -.

Are you using an old git version? I normally use 'git format-patch -M -D'
to produce patches in the reduced format that handles moves and deletions
nicer but is incompatible with older versions of 'patch'.

I'll fix up the changelog to mention the correct driver and resend without -D.

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


[PATCH v4] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Arnd Bergmann
This lets us use the ehci-platform driver on platforms without special
requirements for their ehci controllers. In particular, this is true
for the vt8500/wm8x50 platforms, which currently have a separate
driver that causes problems with multiplatform configurations.

Tested-by: Tony Prisk li...@prisktech.co.nz
Tested-by: Peter Vasil peterva...@gmail.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Arnd Bergmann a...@arndb.de
---
v4: fix the name of the driver in the changelog
submit the patch with the full file deletion.
v3: always set dma_mask and coherent_dma_mask when not already set,
rather than overriding them for any device without platform data
v2: reset the platform data pointer on unload

 drivers/usb/host/ehci-hcd.c  |   5 --
 drivers/usb/host/ehci-platform.c |  34 +++--
 drivers/usb/host/ehci-vt8500.c   | 150 ---
 3 files changed, 28 insertions(+), 161 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 0c3314c..960e7cf 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1286,11 +1286,6 @@ MODULE_LICENSE (GPL);
 #define PLATFORM_DRIVERehci_octeon_driver
 #endif
 
-#ifdef CONFIG_ARCH_VT8500
-#include ehci-vt8500.c
-#definePLATFORM_DRIVER vt8500_ehci_driver
-#endif
-
 #ifdef CONFIG_PLAT_SPEAR
 #include ehci-spear.c
 #define PLATFORM_DRIVERspear_ehci_hcd_driver
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index ca75063..cda0fa9 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -18,11 +18,13 @@
  *
  * Licensed under the GNU/GPL. See COPYING for details.
  */
+#include linux/dma-mapping.h
 #include linux/err.h
 #include linux/kernel.h
 #include linux/hrtimer.h
 #include linux/io.h
 #include linux/module.h
+#include linux/of.h
 #include linux/platform_device.h
 #include linux/usb.h
 #include linux/usb/hcd.h
@@ -62,22 +64,32 @@ static const struct ehci_driver_overrides 
platform_overrides __initdata = {
.reset =ehci_platform_reset,
 };
 
+static struct usb_ehci_pdata ehci_platform_defaults;
+
 static int ehci_platform_probe(struct platform_device *dev)
 {
struct usb_hcd *hcd;
struct resource *res_mem;
-   struct usb_ehci_pdata *pdata = dev-dev.platform_data;
+   struct usb_ehci_pdata *pdata;
int irq;
int err = -ENOMEM;
 
-   if (!pdata) {
-   WARN_ON(1);
-   return -ENODEV;
-   }
-
if (usb_disabled())
return -ENODEV;
 
+   /*
+* use reasonable defaults so platforms don't have to provide these.
+* with DT probing on ARM, none of these are set.
+*/
+   if (!dev-dev.platform_data)
+   dev-dev.platform_data = ehci_platform_defaults;
+   if (!dev-dev.dma_mask)
+   dev-dev.dma_mask = dev-dev.coherent_dma_mask;
+   if (!dev-dev.coherent_dma_mask)
+   dev-dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+   pdata = dev-dev.platform_data;
+
irq = platform_get_irq(dev, 0);
if (irq  0) {
dev_err(dev-dev, no irq provided);
@@ -139,6 +151,9 @@ static int ehci_platform_remove(struct platform_device *dev)
if (pdata-power_off)
pdata-power_off(dev);
 
+   if (pdata == ehci_platform_defaults)
+   dev-dev.platform_data = NULL;
+
return 0;
 }
 
@@ -183,6 +198,12 @@ static int ehci_platform_resume(struct device *dev)
 #define ehci_platform_resume   NULL
 #endif /* CONFIG_PM */
 
+static const struct of_device_id vt8500_ehci_ids[] = {
+   { .compatible = via,vt8500-ehci, },
+   { .compatible = wm,prizm-ehci, },
+   {}
+};
+
 static const struct platform_device_id ehci_platform_table[] = {
{ ehci-platform, 0 },
{ }
@@ -203,6 +224,7 @@ static struct platform_driver ehci_platform_driver = {
.owner  = THIS_MODULE,
.name   = ehci-platform,
.pm = ehci_platform_pm_ops,
+   .of_match_table = of_match_ptr(vt8500_ehci_ids),
}
 };
 
diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c
deleted file mode 100644
index 7ecf709..000
--- a/drivers/usb/host/ehci-vt8500.c
+++ /dev/null
@@ -1,150 +0,0 @@
-/*
- * drivers/usb/host/ehci-vt8500.c
- *
- * Copyright (C) 2010 Alexey Charkov alch...@gmail.com
- *
- * Based on ehci-au1xxx.c
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-

Re: 3.8.4: ohci question

2013-03-27 Thread Bjorn Helgaas
[+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!

 Kind regards,
 Udo
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Peter Vasil
On Wed, Mar 27, 2013 at 10:41 PM, Arnd Bergmann a...@arndb.de wrote:
 On Wednesday 27 March 2013, Peter Vasil wrote:
 Hi, unfortunately I still can't apply it on 3.9-rc4 :-(

 $ git apply --check ehci-dt-bus-glue.patch
 error: removal patch leaves file contents
 error: drivers/usb/host/ehci-vt8500.c: patch does not apply

 Looks git really doesn't like ehci-vt8500.c removal. If I delete the
 file manually, git diff actually outputs full file content prefixed
 with -.

 Are you using an old git version? I normally use 'git format-patch -M -D'
 to produce patches in the reduced format that handles moves and deletions
 nicer but is incompatible with older versions of 'patch'.

 I'll fix up the changelog to mention the correct driver and resend without -D.

 Arnd

Hm, no idea what the latest versions are :-) I use archlinux, with
latest updates installed.
Here are my git and patch versions:

$ git --version
git version 1.8.2
$ patch --version
GNU patch 2.7.1

Btw. my git understands format-patch -D, but its help looks a bit suspicous:

   -D, --irreversible-delete
   Omit the preimage for deletes, i.e. print only the header
but not the diff between the preimage and /dev/null. The
   resulting patch is not meant to be applied with patch nor
git apply; this is solely for people who want to just
   concentrate on reviewing the text after the change. In
addition, the output obviously lack enough information to
   apply such a patch in reverse, even manually, hence the
name of the option.

   When used together with -B, omit also the preimage in the
deletion part of a delete/create pair.

Perhaps there is a newer version that can also apply such a reduced patch?
Regards,
Peter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] USB: EHCI: DT support for generic bus glue

2013-03-27 Thread Arnd Bergmann
On Wednesday 27 March 2013, Peter Vasil wrote:
 Hm, no idea what the latest versions are :-) I use archlinux, with
 latest updates installed.
 Here are my git and patch versions:
 
 $ git --version
 git version 1.8.2
 $ patch --version
 GNU patch 2.7.1
 
 Btw. my git understands format-patch -D, but its help looks a bit suspicous:
 
-D, --irreversible-delete
Omit the preimage for deletes, i.e. print only the header
 but not the diff between the preimage and /dev/null. The
resulting patch is not meant to be applied with patch nor
 git apply; this is solely for people who want to just
concentrate on reviewing the text after the change. In
 addition, the output obviously lack enough information to
apply such a patch in reverse, even manually, hence the
 name of the option.
 
When used together with -B, omit also the preimage in the
 deletion part of a delete/create pair.
 
 Perhaps there is a newer version that can also apply such a reduced patch?

Hmm, my version is slightly older than yours. I also find the same thing
here:

$ git format-patch -M -D HEAD^
0001-USB-EHCI-DT-support-for-generic-bus-glue.patch
$ git reset --hard HEAD^
HEAD is now at 417c765 USB: EHCI: fix up incorrect merge resolution
$ git am 0001-USB-EHCI-DT-support-for-generic-bus-glue.patch
Applying: USB: EHCI: DT support for generic bus glue
error: removal patch leaves file contents
error: drivers/usb/host/ehci-vt8500.c: patch does not apply
Patch failed at 0001 USB: EHCI: DT support for generic bus glue
The copy of the patch that failed is found in:
   /home/arnd/arm-soc/.git/rebase-apply/patch
When you have resolved this problem, run git am --resolved.
If you prefer to skip this patch, run git am --skip instead.
To restore the original branch and stop patching, run git am --abort.

I guess I won't be using the -D switch again then. Thanks for pointing
this out!

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


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

2013-03-27 Thread Sarah Sharp
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
--
To unsubscribe from this list: send the line unsubscribe 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 v8 4/8] usb: chipidea: udc: read status of td only once in hardware_dequeue

2013-03-27 Thread Michael Grzeschik
This patch changes the read of the td status to one atomic operation to
analyse coherent bits.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
 drivers/usb/chipidea/udc.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index a8528ad..d4db3ac 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -507,10 +507,12 @@ done:
  */
 static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 {
+   u32 tmptoken = mReq-ptr-token;
+
if (mReq-req.status != -EALREADY)
return -EINVAL;
 
-   if ((TD_STATUS_ACTIVE  mReq-ptr-token) != 0)
+   if ((TD_STATUS_ACTIVE  tmptoken) != 0)
return -EBUSY;
 
if (mReq-zptr) {
@@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir);
 
-   mReq-req.status = mReq-ptr-token  TD_STATUS;
+   mReq-req.status = tmptoken  TD_STATUS;
if ((TD_STATUS_HALTED  mReq-req.status) != 0)
mReq-req.status = -1;
else if ((TD_STATUS_DT_ERR  mReq-req.status) != 0)
@@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
else if ((TD_STATUS_TR_ERR  mReq-req.status) != 0)
mReq-req.status = -1;
 
-   mReq-req.actual   = mReq-ptr-token  TD_TOTAL_BYTES;
+   mReq-req.actual   = tmptoken  TD_TOTAL_BYTES;
mReq-req.actual = ffs_nr(TD_TOTAL_BYTES);
mReq-req.actual   = mReq-req.length - mReq-req.actual;
mReq-req.actual   = mReq-req.status ? 0 : mReq-req.actual;
-- 
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 v8 8/8] usb: chipidea: udc: prepare qhead with dma_alloc_coherent

2013-03-27 Thread Michael Grzeschik
The prepared memory for the qhead needs to be contiguos and 2K aligned.
We change the code from allocating extra buffer for every ep qhead to
one big area. This patch lowers the amount of code to prepare the
memory.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
 drivers/usb/chipidea/ci.h  |  9 +++--
 drivers/usb/chipidea/udc.c | 31 ++-
 drivers/usb/chipidea/udc.h |  2 +-
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index e25d126..12eca83 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -110,7 +110,7 @@ struct hw_bank {
  * @is_otg: if the device is otg-capable
  * @work: work for role changing
  * @wq: workqueue thread
- * @qh_pool: allocation pool for queue heads
+ * @qh: allocation struct for queue heads
  * @td_pool: allocation pool for transfer descriptors
  * @gadget: device side representation for peripheral controller
  * @driver: gadget driver
@@ -142,7 +142,12 @@ struct ci13xxx {
struct work_struct  vbus_work;
struct workqueue_struct *wq;
 
-   struct dma_pool *qh_pool;
+   struct {
+   struct ci13xxx_qh   *ptr;
+   dma_addr_t  dma;
+   size_t  size;
+   }   qh;
+
struct dma_pool *td_pool;
 
struct usb_gadget   gadget;
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index a0a0a64..c9f0a1a 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -23,6 +23,7 @@
 #include linux/irq.h
 #include linux/kernel.h
 #include linux/slab.h
+#include linux/sizes.h
 #include linux/pm_runtime.h
 #include linux/usb/ch9.h
 #include linux/usb/gadget.h
@@ -1417,7 +1418,7 @@ static int ci13xxx_vbus_session(struct usb_gadget 
*_gadget, int is_active)
pm_runtime_get_sync(_gadget-dev);
hw_device_reset(ci, USBMODE_CM_DC);
hw_enable_vbus_intr(ci);
-   hw_device_state(ci, ci-ep0out-qh.dma);
+   hw_device_state(ci, ci-qh.dma);
} else {
hw_device_state(ci, 0);
if (ci-platdata-notify_event)
@@ -1520,8 +1521,8 @@ static int init_eps(struct ci13xxx *ci)
mEp-ep.maxpacket = (unsigned short)~0;
 
INIT_LIST_HEAD(mEp-qh.queue);
-   mEp-qh.ptr = dma_pool_alloc(ci-qh_pool, GFP_KERNEL,
-mEp-qh.dma);
+   mEp-qh.ptr = ci-qh.ptr[i * 2 + j];
+
if (mEp-qh.ptr == NULL)
retval = -ENOMEM;
else
@@ -1549,13 +1550,8 @@ static int init_eps(struct ci13xxx *ci)
 
 static void destroy_eps(struct ci13xxx *ci)
 {
-   int i;
-
-   for (i = 0; i  ci-hw_ep_max; i++) {
-   struct ci13xxx_ep *mEp = ci-ci13xxx_ep[i];
-
-   dma_pool_free(ci-qh_pool, mEp-qh.ptr, mEp-qh.dma);
-   }
+   dma_free_coherent(ci-dev, ci-qh.size,
+   ci-qh.ptr, ci-qh.dma);
 }
 
 /**
@@ -1601,7 +1597,7 @@ static int ci13xxx_start(struct usb_gadget *gadget,
}
}
 
-   retval = hw_device_state(ci, ci-ep0out-qh.dma);
+   retval = hw_device_state(ci, ci-qh.dma);
if (retval)
pm_runtime_put_sync(ci-gadget.dev);
 
@@ -1747,11 +1743,14 @@ static int udc_start(struct ci13xxx *ci)
ci-gadget.dev.parent   = dev;
ci-gadget.dev.release  = udc_release;
 
+   ci-qh.size = ci-hw_ep_max * sizeof(struct ci13xxx_qh);
+   ci-qh.size = ALIGN(ci-qh.size, SZ_2K);
+
/* alloc resources */
-   ci-qh_pool = dma_pool_create(ci13xxx_qh, dev,
-  sizeof(struct ci13xxx_qh),
-  64, CI13XXX_PAGE_SIZE);
-   if (ci-qh_pool == NULL)
+   ci-qh.ptr = dma_alloc_coherent(dev, ci-qh.size,
+   ci-qh.dma, GFP_ATOMIC);
+
+   if (ci-qh.ptr == NULL)
return -ENOMEM;
 
ci-td_pool = dma_pool_create(ci13xxx_td, dev,
@@ -1831,7 +1830,6 @@ destroy_eps:
 free_pools:
dma_pool_destroy(ci-td_pool);
 free_qh_pool:
-   dma_pool_destroy(ci-qh_pool);
return retval;
 }
 
@@ -1853,7 +1851,6 @@ static void udc_stop(struct ci13xxx *ci)
destroy_eps(ci);
 
dma_pool_destroy(ci-td_pool);
-   dma_pool_destroy(ci-qh_pool);
 
if (!IS_ERR_OR_NULL(ci-transceiver)) {
otg_set_peripheral(ci-transceiver-otg, NULL);
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index d12e8b5..59c091b 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -57,7 +57,7 @@ struct ci13xxx_qh {
/* 9 */
u32 

[PATCH v8 2/8] usb: chipidea: udc: only clear active and halted bits in qhead

2013-03-27 Thread Michael Grzeschik
The datasheet of the synopsys core describes only to overwrite the
active and halted bits in the qhead before priming any endpoint.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/chipidea/udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 77fb66f..4bbbe62 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
 
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
-   mEp-qh.ptr-td.token = ~TD_STATUS;   /* clear status */
+   mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
-- 
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 v8 7/8] usb: chipidea: udc: fix possible memory leak in _ep_nuke

2013-03-27 Thread Michael Grzeschik
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
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


[PATCH v8 6/8] usb: chipidea: udc: move _ep_queue into an unlocked function

2013-03-27 Thread Michael Grzeschik
There is no need to call ep_queue unlocked inside the own driver. We
move its functionionality into an unlocked version.

This patch removes potential unlocked timeslots inside
isr_setup_status_phase and isr_get_status_response, in which the lock
got released just before acquired again inside usb_ep_queue.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/udc.c | 113 +
 1 file changed, 63 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index e7c84ab..b40c259 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -671,6 +671,66 @@ static void isr_get_status_complete(struct usb_ep *ep, 
struct usb_request *req)
 }
 
 /**
+ * _ep_queue: queues (submits) an I/O request to an endpoint
+ *
+ * Caller must hold lock
+ */
+static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
+   gfp_t __maybe_unused gfp_flags)
+{
+   struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
+   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
+   struct ci13xxx *ci = mEp-ci;
+   int retval = 0;
+
+   if (ep == NULL || req == NULL || mEp-ep.desc == NULL)
+   return -EINVAL;
+
+   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
+   if (req-length)
+   mEp = (ci-ep0_dir == RX) ?
+  ci-ep0out : ci-ep0in;
+   if (!list_empty(mEp-qh.queue)) {
+   _ep_nuke(mEp);
+   retval = -EOVERFLOW;
+   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
+_usb_addr(mEp));
+   }
+   }
+
+   /* first nuke then test link, e.g. previous status has not sent */
+   if (!list_empty(mReq-queue)) {
+   retval = -EBUSY;
+   dev_err(mEp-ci-dev, request already in queue\n);
+   goto done;
+   }
+
+   if (req-length  4 * CI13XXX_PAGE_SIZE) {
+   retval = -EMSGSIZE;
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
+   }
+
+   dbg_queue(_usb_addr(mEp), req, retval);
+
+   /* push request */
+   mReq-req.status = -EINPROGRESS;
+   mReq-req.actual = 0;
+
+   retval = _hardware_enqueue(mEp, mReq);
+
+   if (retval == -EALREADY) {
+   dbg_event(_usb_addr(mEp), QUEUE, retval);
+   retval = 0;
+   }
+   if (!retval)
+   list_add_tail(mReq-queue, mEp-qh.queue);
+
+ done:
+   return retval;
+}
+
+/**
  * isr_get_status_response: get_status request response
  * @ci: ci struct
  * @setup: setup request packet
@@ -717,9 +777,7 @@ __acquires(mEp-lock)
}
/* else do nothing; reserved for future use */
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, req, gfp_flags);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, req, gfp_flags);
if (retval)
goto err_free_buf;
 
@@ -766,8 +824,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct 
usb_request *req)
  * This function returns an error code
  */
 static int isr_setup_status_phase(struct ci13xxx *ci)
-__releases(mEp-lock)
-__acquires(mEp-lock)
 {
int retval;
struct ci13xxx_ep *mEp;
@@ -776,9 +832,7 @@ __acquires(mEp-lock)
ci-status-context = ci;
ci-status-complete = isr_setup_status_complete;
 
-   spin_unlock(mEp-lock);
-   retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
-   spin_lock(mEp-lock);
+   retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC);
 
return retval;
 }
@@ -1177,8 +1231,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
gfp_t __maybe_unused gfp_flags)
 {
struct ci13xxx_ep  *mEp  = container_of(ep,  struct ci13xxx_ep, ep);
-   struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
-   struct ci13xxx *ci = mEp-ci;
int retval = 0;
unsigned long flags;
 
@@ -1187,47 +1239,8 @@ static int ep_queue(struct usb_ep *ep, struct 
usb_request *req,
 
spin_lock_irqsave(mEp-lock, flags);
 
-   if (mEp-type == USB_ENDPOINT_XFER_CONTROL) {
-   if (req-length)
-   mEp = (ci-ep0_dir == RX) ?
-  ci-ep0out : ci-ep0in;
-   if (!list_empty(mEp-qh.queue)) {
-   _ep_nuke(mEp);
-   retval = -EOVERFLOW;
-   dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n,
-_usb_addr(mEp));
-   }
-   }
+   retval = _ep_queue(ep, req, gfp_flags);
 
-   /* first nuke then test link, e.g. previous status has not sent */
-   if (!list_empty(mReq-queue)) {
-   retval = -EBUSY;
- 

[PATCH v8 1/8] usb: chipidea: udc: add attribute aligned(4) to shared memory structs

2013-03-27 Thread Michael Grzeschik
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
---
 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


[PATCH v8 5/8] usb: chipidea: udc: don't truncate requests to single tds

2013-03-27 Thread Michael Grzeschik
It is not safe to truncate requests to the maximum possible size the
controller can handle with one td and to keep working. That patch fixes
that with proper error handling instead.

Reported-by: Felipe Balbi ba...@ti.com
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
 drivers/usb/chipidea/udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d4db3ac..e7c84ab 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request 
*req,
}
 
if (req-length  4 * CI13XXX_PAGE_SIZE) {
-   req-length = 4 * CI13XXX_PAGE_SIZE;
retval = -EMSGSIZE;
-   dev_warn(mEp-ci-dev, request length truncated\n);
+   dev_err(mEp-ci-dev, request bigger than one td\n);
+   goto done;
}
 
dbg_queue(_usb_addr(mEp), req, retval);
-- 
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 v8 3/8] usb: chipidea: udc: move ZLT flag change to ep_enable

2013-03-27 Thread Michael Grzeschik
Its not necessary and also not specified in the datasheet to change the
ZLT flag before every ep_prime. This patch moves this to the ep_enable
and applies it only for non configuration endpoints.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/chipidea/udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 4bbbe62..a8528ad 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct 
ci13xxx_req *mReq)
/*  QH configuration */
mEp-qh.ptr-td.next   = mReq-dma;/* TERMINATE = 0 */
mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE);
-   mEp-qh.ptr-cap |=  QH_ZLT;
 
wmb();   /* synchronize before ep prime */
 
@@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep,
mEp-qh.ptr-cap = ~QH_MULT;
else
mEp-qh.ptr-cap = ~QH_ZLT;
+   if (mEp-num)
+   mEp-qh.ptr-cap |= QH_ZLT;
 
mEp-qh.ptr-cap |=
(mEp-ep.maxpacket  ffs_nr(QH_MAX_PKT))  QH_MAX_PKT;
-- 
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: Piggy-backing new hardware using old usb-serial

2013-03-27 Thread Greg KH
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.

   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


Re: [PATCH 3/4] usb: Add usb port system pm support

2013-03-27 Thread Lan Tianyu

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.

--
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 V2 0/6] remove the clock name from pdata

2013-03-27 Thread Chao Xie
On Wed, Mar 27, 2013 at 9:14 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Mon, Mar 25, 2013 at 03:06:51AM -0400, Chao Xie wrote:
 The clock is defined by device, so the driver knows how many
 clocks needed by the device.
 The orignal way that passing the clock name by pdata is not correct.
 The following patches fix it.

 V2-V1:
typo fix
rebased on latest usb-next

 Chao Xie (6):
   usb: gadget: mv_udc_core: remove unused clock
   usb: otg: mv_otg: remove unused clock
   usb: ehci: mv_ehci: remove unused clock
   arm: mmp: remove clock from usb pdata for aspenite
   arm: mmp: remove clock name from usb pdata for ttc
   usb: mv_usb: remove clock name from pdata

 how should this series be handled ?

 If you want me to carry all patches, I need Acks from other maintainers
 for ehci and arch/arm/ parts.

 Until then, I'm dropping this from my TODO list.

 --
 balbi

hi, Alan

Can you help me to review the EHCI parts?
It changes the way of getting clock. The orginal way uses the pdata to
get the clock name, and it is not correct, and the patch fix it.
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: [PATCH V2 3/6] usb: ehci: mv_ehci: remove unused clock

2013-03-27 Thread Chao Xie
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);
 --
 1.7.4.1

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


Re: [PATCH V2 4/6] arm: mmp: remove clock from usb pdata for aspenite

2013-03-27 Thread Chao Xie
hi, Haojian
The patches correct the wrong usage of clock. It finally do not need
pass clock by pdata. Can you help to review the patches relates to
arch-mmp, and ack it?Thanks.

On Mon, Mar 25, 2013 at 3:06 PM, Chao Xie chao@marvell.com wrote:
 The clock name will directly get by driver. Removing
 the name from pdata.

 Signed-off-by: Chao Xie chao@marvell.com
 ---
  arch/arm/mach-mmp/aspenite.c |6 --
  1 files changed, 0 insertions(+), 6 deletions(-)

 diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c
 index 9f64d56..76901f4 100644
 --- a/arch/arm/mach-mmp/aspenite.c
 +++ b/arch/arm/mach-mmp/aspenite.c
 @@ -223,13 +223,7 @@ static struct pxa27x_keypad_platform_data 
 aspenite_keypad_info __initdata = {
  };

  #if defined(CONFIG_USB_EHCI_MV)
 -static char *pxa168_sph_clock_name[] = {
 -   [0] = PXA168-USBCLK,
 -};
 -
  static struct mv_usb_platform_data pxa168_sph_pdata = {
 -   .clknum = 1,
 -   .clkname= pxa168_sph_clock_name,
 .mode   = MV_USB_MODE_HOST,
 .phy_init   = pxa_usb_phy_init,
 .phy_deinit = pxa_usb_phy_deinit,
 --
 1.7.4.1

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


  1   2   >