Re: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Peter Stuge
Xiaofan Chen wrote:
> On Thu, Oct 11, 2012 at 2:50 PM, Peter Stuge  wrote:
> > But it isn't certain that is is involved at all. If your impact
> > version uses libusb.so like in ISE 11.1 then it will not be. One way
> > to test is by trying to generate a libusb debug log. See
> > https://libusb.org/wiki/debug for instructions which are for libusb,
> > but which work also for libusbx since the code is nearly the same.
> > (Just replace the repository to clone from.)
> 
> Actually this is not that correct. For libusbx, you do not need
> to rebuild libusbx to get the debug log at all, just need to
> set environment variable LIBUSB_DEBUG=4 to get the
> debug log.

The configure option to completely disable logging is still present
in libusbx, and I do not know which distributions use that, if any,
so I prefer to recommend the method that is guaranteed to work.


//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 1/3] USB: EHCI: move logging macros to ehci.h

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 03:07:30PM -0400, Alan Stern wrote:
> In preparation for splitting the ehci-hcd driver into a core library
> and separate platform-specific driver modules, this patch (as1616)
> moves the console logging macros from ehci-dbg.c to ehci.h, where they
> will be available to the platform drivers.
> 
> Signed-off-by: Alan Stern 

Looks good:

Acked-by: Felipe Balbi 

> 
> ---
> 
>  drivers/usb/host/ehci-dbg.c |   15 ---
>  drivers/usb/host/ehci.h |   15 +++
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> Index: usb-3.6/drivers/usb/host/ehci-dbg.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-dbg.c
> +++ usb-3.6/drivers/usb/host/ehci-dbg.c
> @@ -18,21 +18,6 @@
>  
>  /* this file is part of ehci-hcd.c */
>  
> -#define ehci_dbg(ehci, fmt, args...) \
> - dev_dbg (ehci_to_hcd(ehci)->self.controller , fmt , ## args )
> -#define ehci_err(ehci, fmt, args...) \
> - dev_err (ehci_to_hcd(ehci)->self.controller , fmt , ## args )
> -#define ehci_info(ehci, fmt, args...) \
> - dev_info (ehci_to_hcd(ehci)->self.controller , fmt , ## args )
> -#define ehci_warn(ehci, fmt, args...) \
> - dev_warn (ehci_to_hcd(ehci)->self.controller , fmt , ## args )
> -
> -#ifdef VERBOSE_DEBUG
> -#define ehci_vdbg ehci_dbg
> -#else
> - static inline void ehci_vdbg(struct ehci_hcd *ehci, ...) {}
> -#endif
> -
>  #ifdef   DEBUG
>  
>  /* check the values in the HCSPARAMS register
> Index: usb-3.6/drivers/usb/host/ehci.h
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci.h
> +++ usb-3.6/drivers/usb/host/ehci.h
> @@ -762,6 +762,21 @@ static inline u32 hc32_to_cpup (const st
>  
>  /*-*/
>  
> +#define ehci_dbg(ehci, fmt, args...) \
> + dev_dbg(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +#define ehci_err(ehci, fmt, args...) \
> + dev_err(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +#define ehci_info(ehci, fmt, args...) \
> + dev_info(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +#define ehci_warn(ehci, fmt, args...) \
> + dev_warn(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +
> +#ifdef VERBOSE_DEBUG
> +#define ehci_vdbg ehci_dbg
> +#else
> + static inline void ehci_vdbg(struct ehci_hcd *ehci, ...) {}
> +#endif
> +
>  #ifdef CONFIG_PCI
>  
>  /* For working around the MosChip frame-index-register bug */
> 

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 2/3] USB: EHCI: make ehci_read_frame_index platform independent

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 03:07:39PM -0400, Alan Stern wrote:
> In preparation for splitting the ehci-hcd driver into a core library
> and separate platform-specific driver modules, this patch (as1617)
> changes the way ehci_read_frame_index() is handled.
> 
> Since the same core library will have to work with both PCI and
> non-PCI platforms, the quirk handler routine will be compiled
> unconditionally.  The decision about whether to call it or simply to
> read the frame index register is made at run time, based on whether
> the frame_index_bug quirk flag is set.
> 
> Signed-off-by: Alan Stern 

Acked-by: Felipe Balbi 

> 
> ---
> 
>  drivers/usb/host/ehci-hcd.c   |   27 ++-
>  drivers/usb/host/ehci-sched.c |   23 ---
>  drivers/usb/host/ehci.h   |   16 
>  3 files changed, 26 insertions(+), 40 deletions(-)
> 
> Index: usb-3.6/drivers/usb/host/ehci-hcd.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-hcd.c
> +++ usb-3.6/drivers/usb/host/ehci-hcd.c
> @@ -118,9 +118,34 @@ MODULE_PARM_DESC(hird, "host initiated r
>  /*-*/
>  
>  #include "ehci.h"
> -#include "ehci-dbg.c"
>  #include "pci-quirks.h"
>  
> +/*
> + * The MosChip MCS9990 controller updates its microframe counter
> + * a little before the frame counter, and occasionally we will read
> + * the invalid intermediate value.  Avoid problems by checking the
> + * microframe number (the low-order 3 bits); if they are 0 then
> + * re-read the register to get the correct value.
> + */
> +static unsigned ehci_moschip_read_frame_index(struct ehci_hcd *ehci)
> +{
> + unsigned uf;
> +
> + uf = ehci_readl(ehci, &ehci->regs->frame_index);
> + if (unlikely((uf & 7) == 0))
> + uf = ehci_readl(ehci, &ehci->regs->frame_index);
> + return uf;
> +}
> +
> +static inline unsigned ehci_read_frame_index(struct ehci_hcd *ehci)
> +{
> + if (ehci->frame_index_bug)
> + return ehci_moschip_read_frame_index(ehci);
> + return ehci_readl(ehci, &ehci->regs->frame_index);
> +}
> +
> +#include "ehci-dbg.c"
> +
>  /*-*/
>  
>  /*
> Index: usb-3.6/drivers/usb/host/ehci-sched.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-sched.c
> +++ usb-3.6/drivers/usb/host/ehci-sched.c
> @@ -36,29 +36,6 @@
>  
>  static int ehci_get_frame (struct usb_hcd *hcd);
>  
> -#ifdef CONFIG_PCI
> -
> -static unsigned ehci_read_frame_index(struct ehci_hcd *ehci)
> -{
> - unsigned uf;
> -
> - /*
> -  * The MosChip MCS9990 controller updates its microframe counter
> -  * a little before the frame counter, and occasionally we will read
> -  * the invalid intermediate value.  Avoid problems by checking the
> -  * microframe number (the low-order 3 bits); if they are 0 then
> -  * re-read the register to get the correct value.
> -  */
> - uf = ehci_readl(ehci, &ehci->regs->frame_index);
> - if (unlikely(ehci->frame_index_bug && ((uf & 7) == 0)))
> - uf = ehci_readl(ehci, &ehci->regs->frame_index);
> - return uf;
> -}
> -
> -#endif
> -
> -/*-*/
> -
>  /*
>   * periodic_next_shadow - return "next" pointer on shadow list
>   * @periodic: host pointer to qh/itd/sitd
> Index: usb-3.6/drivers/usb/host/ehci.h
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci.h
> +++ usb-3.6/drivers/usb/host/ehci.h
> @@ -777,22 +777,6 @@ static inline u32 hc32_to_cpup (const st
>   static inline void ehci_vdbg(struct ehci_hcd *ehci, ...) {}
>  #endif
>  
> -#ifdef CONFIG_PCI
> -
> -/* For working around the MosChip frame-index-register bug */
> -static unsigned ehci_read_frame_index(struct ehci_hcd *ehci);
> -
> -#else
> -
> -static inline unsigned ehci_read_frame_index(struct ehci_hcd *ehci)
> -{
> - return ehci_readl(ehci, &ehci->regs->frame_index);
> -}
> -
> -#endif
> -
> -/*-*/
> -
>  #ifndef DEBUG
>  #define STUB_DEBUG_FILES
>  #endif   /* DEBUG */
> 

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 3/3] USB: EHCI: move ehci_update_device() to ehci-lpm.c

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 03:07:46PM -0400, Alan Stern wrote:
> In preparation for splitting the ehci-hcd driver into a core library
> and separate platform-specific driver modules, this patch (as1618)
> moves ehci_update_device() from a couple of platform-specific source
> files into ehci-lpm.c.  This is where it should have been all along,
> since all it does is call a couple of other functions that are already
> in ehci-lpm.c.
> 
> Signed-off-by: Alan Stern 

Acked-by: Felipe Balbi 

> 
> ---
> 
>  drivers/usb/host/ehci-lpm.c|   23 ---
>  drivers/usb/host/ehci-pci.c|   16 
>  drivers/usb/host/ehci-vt8500.c |   16 
>  3 files changed, 20 insertions(+), 35 deletions(-)
> 
> Index: usb-3.6/drivers/usb/host/ehci-lpm.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-lpm.c
> +++ usb-3.6/drivers/usb/host/ehci-lpm.c
> @@ -17,8 +17,8 @@
>  */
>  
>  /* this file is part of ehci-hcd.c */
> -static int __maybe_unused ehci_lpm_set_da(struct ehci_hcd *ehci,
> - int dev_addr, int port_num)
> +
> +static int ehci_lpm_set_da(struct ehci_hcd *ehci, int dev_addr, int port_num)
>  {
>   u32 __iomem portsc;
>  
> @@ -38,7 +38,7 @@ static int __maybe_unused ehci_lpm_set_d
>   * this function is used to check if the device support LPM
>   * if yes, mark the PORTSC register with PORT_LPM bit
>   */
> -static int __maybe_unused ehci_lpm_check(struct ehci_hcd *ehci, int port)
> +static int ehci_lpm_check(struct ehci_hcd *ehci, int port)
>  {
>   u32 __iomem *portsc ;
>   u32 val32;
> @@ -82,3 +82,20 @@ static int __maybe_unused ehci_lpm_check
>  
>   return retval;
>  }
> +
> +static int __maybe_unused ehci_update_device(struct usb_hcd *hcd,
> + struct usb_device *udev)
> +{
> + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> + int rc = 0;
> +
> + if (!udev->parent) /* udev is root hub itself, impossible */
> + rc = -1;
> + /* we only support lpm device connected to root hub yet */
> + if (ehci->has_lpm && !udev->parent->parent) {
> + rc = ehci_lpm_set_da(ehci, udev->devnum, udev->portnum);
> + if (!rc)
> + rc = ehci_lpm_check(ehci, udev->portnum);
> + }
> + return rc;
> +}
> Index: usb-3.6/drivers/usb/host/ehci-pci.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-pci.c
> +++ usb-3.6/drivers/usb/host/ehci-pci.c
> @@ -379,22 +379,6 @@ static int ehci_pci_resume(struct usb_hc
>  }
>  #endif
>  
> -static int ehci_update_device(struct usb_hcd *hcd, struct usb_device *udev)
> -{
> - struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> - int rc = 0;
> -
> - if (!udev->parent) /* udev is root hub itself, impossible */
> - rc = -1;
> - /* we only support lpm device connected to root hub yet */
> - if (ehci->has_lpm && !udev->parent->parent) {
> - rc = ehci_lpm_set_da(ehci, udev->devnum, udev->portnum);
> - if (!rc)
> - rc = ehci_lpm_check(ehci, udev->portnum);
> - }
> - return rc;
> -}
> -
>  static const struct hc_driver ehci_pci_hc_driver = {
>   .description =  hcd_name,
>   .product_desc = "EHCI Host Controller",
> Index: usb-3.6/drivers/usb/host/ehci-vt8500.c
> ===
> --- usb-3.6.orig/drivers/usb/host/ehci-vt8500.c
> +++ usb-3.6/drivers/usb/host/ehci-vt8500.c
> @@ -19,22 +19,6 @@
>  #include 
>  #include 
>  
> -static int ehci_update_device(struct usb_hcd *hcd, struct usb_device *udev)
> -{
> - struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> - int rc = 0;
> -
> - if (!udev->parent) /* udev is root hub itself, impossible */
> - rc = -1;
> - /* we only support lpm device connected to root hub yet */
> - if (ehci->has_lpm && !udev->parent->parent) {
> - rc = ehci_lpm_set_da(ehci, udev->devnum, udev->portnum);
> - if (!rc)
> - rc = ehci_lpm_check(ehci, udev->portnum);
> - }
> - return rc;
> -}
> -
>  static const struct hc_driver vt8500_ehci_hc_driver = {
>   .description= hcd_name,
>   .product_desc   = "VT8500 EHCI",
> 

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 2/3] usb: xhci: Add the suspend/resume functionality

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 10:58:16AM -0700, Sarah Sharp wrote:
> On Wed, Oct 10, 2012 at 07:35:48PM +0530, Vikas C Sajjan wrote:
> > From: Vikas Sajjan 
> > 
> > Adding the suspend and resume functionality for the XHCI driver
> > 
> > Signed-off-by: Abhilash Kesavan 
> > Signed-off-by: Vikas C Sajjan 
> > CC: Doug Anderson 
> 
> Acked-by: Sarah Sharp 
> 
> Felipe, you want to take this?

I can, will get to these patches in a while.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Oliver Neukum
On Thursday 11 October 2012 11:18:22 Ming Lei wrote:
> On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum  wrote:
> >
> > No, the problem is autoresume.
> >
> > Suppose we have a device with two interface. Interface A be usbnet; 
> > interface B
> > something you page on. Now consider that you can only resume both interfaces
> > and this is (and needs to be) done synchronously.
> >
> > Now we can have this code path:
> >
> > autoresume of device -> resume() -> kmalloc(..., GFP_KERNEL) ->
> > VM layer decides to start paging out -> IO to interface B -> autoresume of 
> > device
> > --> DEADLOCK
> 
> Currently scsi disk can only be runtime suspended when the device is not
> opened, so are you sure that the paging out above can cause IO on a suspend
> usb mass storage disk which is not mounted or opened by utility now?

We definitely do not wish to keep it that way. People at Intel are currently 
working
on better power management for sd, which would allow full autosuspend.

Regards
Oliver

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


Re: [PATCH 3/3] exynos5: usb: dwc3: Add the suspend/resume functionality

2012-10-11 Thread Felipe Balbi
Hi,

On Wed, Oct 10, 2012 at 07:35:49PM +0530, Vikas C Sajjan wrote:
> From: Vikas Sajjan 
> 
> Adding the suspend and resume functionality to exynos dwc3 driver
> 
> Signed-off-by: Abhilash Kesavan 
> Signed-off-by: Vikas C Sajjan 
> CC: Doug Anderson 
> ---
>  drivers/usb/dwc3/dwc3-exynos.c |   60 
> 
>  1 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index ca65978..1154cee 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -200,11 +201,70 @@ static int __devexit dwc3_exynos_remove(struct 
> platform_device *pdev)
>   return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int dwc3_exynos_suspend(struct device *dev)
> +{
> + struct dwc3_exynos_data *pdata = dev->platform_data;
> + struct dwc3_exynos  *exynos;
> + struct platform_device *pdev = to_platform_device(dev);

you shouldn't have to access platform device here.

> +
> + exynos = dev_get_drvdata(dev);
> +
> + if (!exynos)
> + return -EINVAL;

this shouldn't ever be the case and if it is, it deserves the Oops.

> +
> + if (pdata && pdata->phy_exit)
> + pdata->phy_exit(pdev, pdata->phy_type);

NAK, you should introduce a proper PHY driver and handle PHY suspend
generically in dwc3/core.c

> + clk_disable(exynos->clk);
> +
> + return 0;
> +}
> +
> +static int dwc3_exynos_resume(struct device *dev)
> +{
> + struct dwc3_exynos_data *pdata = dev->platform_data;
> + struct dwc3_exynos  *exynos;
> + struct platform_device *pdev = to_platform_device(dev);

ditto

> + exynos = dev_get_drvdata(dev);
> +
> + if (!exynos)
> + return -EINVAL;

ditto

> + clk_enable(exynos->clk);
> +
> + /* PHY initialization */
> + if (!pdata) {
> + dev_dbg(&pdev->dev, "missing platform data\n");
> + } else {
> + if (pdata->phy_init)
> + pdata->phy_init(pdev, pdata->phy_type);
> + }

missing PHY driver

> + /* runtime set active to reflect active state. */
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_exynos_pm_ops = {
> + .suspend= dwc3_exynos_suspend,
> + .resume = dwc3_exynos_resume,
> +};

right here you should:

#define DWC3_EXYNOS_DEV_PM_OPS  &(dwc3_exynos_pm_ops)
#else
#define DWC3_EXYNOS_DEV_PM_OPS  NULL

> +#endif /* CONFIG_PM */
> +
>  static struct platform_driver dwc3_exynos_driver = {
>   .probe  = dwc3_exynos_probe,
>   .remove = __devexit_p(dwc3_exynos_remove),
>   .driver = {
>   .name   = "exynos-dwc3",
> +#ifdef CONFIG_PM
> + .pm = &dwc3_exynos_pm_ops,

.pm = DWC3_EXYNOS_DEV_PM_OPS,

> +#endif

avoid the ifdeferry all over, see above.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 0/3] USB: dwc3: Add suspend/resume support

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 07:35:46PM +0530, Vikas C Sajjan wrote:
> From: Vikas Sajjan 
> 
> This patchset adds suspend/resume functionality to dwc3-core layer
> and xhci-platform driver. It also adds S2R support to dwc3-exynos
> glue layer.

how has this been tested ? For how long ?

> Based on 'usb-next' branch.
> 
> Vikas Sajjan (3):
>   usb: dwc3: Add the suspend/resume functionality
>   usb: xhci: Add the suspend/resume functionality
>   exynos5: usb: dwc3: Add the suspend/resume functionality
> 
>  drivers/usb/dwc3/core.c|  268 
> +---
>  drivers/usb/dwc3/dwc3-exynos.c |   60 +
>  drivers/usb/host/xhci-plat.c   |   44 +++
>  3 files changed, 273 insertions(+), 99 deletions(-)
> 
> -- 
> 1.7.6.5
> 

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 1/3] usb: dwc3: Add the suspend/resume functionality

2012-10-11 Thread Felipe Balbi
Hi,

On Wed, Oct 10, 2012 at 07:35:47PM +0530, Vikas C Sajjan wrote:
> From: Vikas Sajjan 
> 
> Adding the suspend and resume funtionality to DWC3 core.
> 
> Signed-off-by: Abhilash Kesavan 
> Signed-off-by: Vikas C Sajjan 
> CC: Doug Anderson 
> ---
>  drivers/usb/dwc3/core.c |  268 +-
>  1 files changed, 169 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index b415c0c..58b51e1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -70,51 +70,20 @@ MODULE_PARM_DESC(maximum_speed, "Maximum supported 
> speed.");
>  
>  static DECLARE_BITMAP(dwc3_devs, DWC3_DEVS_POSSIBLE);
>  
> -int dwc3_get_device_id(void)
> -{
> - int id;
> -
> -again:
> - id = find_first_zero_bit(dwc3_devs, DWC3_DEVS_POSSIBLE);
> - if (id < DWC3_DEVS_POSSIBLE) {
> - int old;
> -
> - old = test_and_set_bit(id, dwc3_devs);
> - if (old)
> - goto again;
> - } else {
> - pr_err("dwc3: no space for new device\n");
> - id = -ENOMEM;
> - }
> -
> - return id;
> -}
> -EXPORT_SYMBOL_GPL(dwc3_get_device_id);
> -
> -void dwc3_put_device_id(int id)
> -{
> - int ret;
> -
> - if (id < 0)
> - return;
> -
> - ret = test_bit(id, dwc3_devs);
> - WARN(!ret, "dwc3: ID %d not in use\n", id);
> - smp_mb__before_clear_bit();
> - clear_bit(id, dwc3_devs);
> -}
> -EXPORT_SYMBOL_GPL(dwc3_put_device_id);
> -
> -void dwc3_set_mode(struct dwc3 *dwc, u32 mode)

why are you even moving all this code around ? Doesn't look necessary.

> +static void __devinit dwc3_cache_hwparams(struct dwc3 *dwc)
>  {
> - u32 reg;
> + struct dwc3_hwparams*parms = &dwc->hwparams;
>  
> - reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> - reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
> - reg |= DWC3_GCTL_PRTCAPDIR(mode);
> - dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> + parms->hwparams0 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS0);
> + parms->hwparams1 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS1);
> + parms->hwparams2 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS2);
> + parms->hwparams3 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
> + parms->hwparams4 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS4);
> + parms->hwparams5 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS5);
> + parms->hwparams6 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS6);
> + parms->hwparams7 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS7);
> + parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8);
>  }
> -
>  /**
>   * dwc3_core_soft_reset - Issues core soft reset and PHY reset
>   * @dwc: pointer to our context structure
> @@ -160,6 +129,102 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>   dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>  }
>  
> +static int dwc3_core_reset(struct dwc3 *dwc)

this looks unnecessary.

> +{
> + unsigned long   timeout;
> + u32 reg;
> +
> + /* issue device SoftReset too */
> + timeout = jiffies + msecs_to_jiffies(500);
> + dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
> + do {
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + if (!(reg & DWC3_DCTL_CSFTRST))
> + break;
> +
> + if (time_after(jiffies, timeout)) {
> + dev_err(dwc->dev, "Reset Timed Out\n");
> + return -ETIMEDOUT;
> + }
> +
> + cpu_relax();
> + } while (true);
> +
> + dwc3_core_soft_reset(dwc);
> +
> + dwc3_cache_hwparams(dwc);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> + reg &= ~DWC3_GCTL_DISSCRAMBLE;
> +
> + switch (DWC3_GHWPARAMS1_EN_PWROPT(dwc->hwparams.hwparams1)) {
> + case DWC3_GHWPARAMS1_EN_PWROPT_CLK:
> + reg &= ~DWC3_GCTL_DSBLCLKGTNG;
> + break;
> + default:
> + dev_dbg(dwc->dev, "No power optimization available\n");
> + }
> +
> + /*
> +  * WORKAROUND: DWC3 revisions <1.90a have a bug
> +  * where the device can fail to connect at SuperSpeed
> +  * and falls back to high-speed mode which causes
> +  * the device to enter a Connect/Disconnect loop
> +  */
> + if (dwc->revision < DWC3_REVISION_190A)
> + reg |= DWC3_GCTL_U2RSTECN;
> +
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +
> + return 0;
> +}
> +
> +int dwc3_get_device_id(void)
> +{
> + int id;
> +
> +again:
> + id = find_first_zero_bit(dwc3_devs, DWC3_DEVS_POSSIBLE);
> + if (id < DWC3_DEVS_POSSIBLE) {
> + int old;
> +
> + old = test_and_set_bit(id, dwc3_devs);
> + if (old)
> + goto again;
> + } else {
> + pr_err("dwc3: no space for new device\n");
> + id = -ENOMEM;
> + }
> +
> + return id;
> +}
> +EXPORT_SYMBOL_GPL(dwc3_ge

Re: [PATCH 2/3] usb: xhci: Add the suspend/resume functionality

2012-10-11 Thread Felipe Balbi
On Wed, Oct 10, 2012 at 07:35:48PM +0530, Vikas C Sajjan wrote:
> From: Vikas Sajjan 
> 
> Adding the suspend and resume functionality for the XHCI driver
> 
> Signed-off-by: Abhilash Kesavan 
> Signed-off-by: Vikas C Sajjan 
> CC: Doug Anderson 
> ---
>  drivers/usb/host/xhci-plat.c |   44 
> ++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index df90fe5..384cf4d 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -185,11 +185,55 @@ static int xhci_plat_remove(struct platform_device *dev)
>   return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int xhci_plat_suspend(struct device *dev)
> +{
> + struct usb_hcd  *hcd;
> + struct xhci_hcd *xhci;
> +
> + hcd = dev_get_drvdata(dev);

make it a single line, together with declaration.

> + if (!hcd)
> + return -EINVAL;

remove this.

> + xhci = hcd_to_xhci(hcd);

make it a single line, together with declaration.

> + /* Make sure that the HCD Core as set state HC_STATE_SUSPENDED */

HCD Core *HAS* set ??? I think this also deserves a "to" after "state":

/* Make sure HCD Core has set state to HC_STATE_SUSPENDED */

> + if (hcd->state != HC_STATE_SUSPENDED ||
> + xhci->shared_hcd->state != HC_STATE_SUSPENDED)
> + return -EINVAL;
> +
> + return xhci_suspend(xhci);
> +}
> +
> +static int xhci_plat_resume(struct device *dev)
> +{
> + struct usb_hcd  *hcd;
> + struct xhci_hcd *xhci;
> +
> + hcd = dev_get_drvdata(dev);

make it a single line, together with declaration.


> + if (!hcd)
> + return -EINVAL;

remove this.

> +
> + xhci = hcd_to_xhci(hcd);

make it a single line, together with declaration.

> + return xhci_resume(xhci, 0);
> +}

this should look like:

static int xhci_plat_resume(struct device *dev)
{
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);

return xhci_resume(xhci, 0);
}

> +static const struct dev_pm_ops xhci_plat_pm_ops = {
> + .suspend= xhci_plat_suspend,
> + .resume = xhci_plat_resume,
> +};

#define XHCI_PLAT_PM_OPS(&xhci_plat_pm_ops)
#else
#define XHCI_PLAT_PM_OPSNULL
#endif

> +#endif
> +
>  static struct platform_driver usb_xhci_driver = {
>   .probe  = xhci_plat_probe,
>   .remove = xhci_plat_remove,
>   .driver = {
>   .name = "xhci-hcd",
> +#ifdef CONFIG_PM
> + .pm = &xhci_plat_pm_ops,
> +#endif

.pm = XHCI_PLAT_PM_OPS,

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Ming Lei
On Thu, Oct 11, 2012 at 12:11 PM, Oliver Neukum  wrote:
> On Thursday 11 October 2012 11:18:22 Ming Lei wrote:

>> Currently scsi disk can only be runtime suspended when the device is not
>> opened, so are you sure that the paging out above can cause IO on a suspend
>> usb mass storage disk which is not mounted or opened by utility now?
>
> We definitely do not wish to keep it that way. People at Intel are currently 
> working
> on better power management for sd, which would allow full autosuspend.

OK, got it.

For auto-resume situation, it can be solved with switching the gpf_t flag
runtime inside helper, but I think it is better to do it after the sd's
full autosuspend is seen in -next tree.

For error handling case, it is inevitably for usbnet to allocate memory
with GFP_KERNEL because no usbnet drivers have implemented
.pre_reset and .post_reset callback, and no such actual problems
have been reported until now, so it should be OK to not consider the
case now.

So could we merge the patch set[1-11] first?

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


Re: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Hans de Goede

Hi,

On 10/10/2012 10:31 PM, Henrik Rydberg wrote:

Hi Hans, Alan, Greg,

commit 3d97ff63f8997761f12c8fbe8082996c6eeaba1a
Author: Hans de Goede 
Date:   Wed Jul 4 09:18:03 2012 +0200

 usbdevfs: Use scatter-gather lists for large bulk transfers

breaks an usb programming cable over here. The problem is reported as
"bulk tranfer failed" [sic] by the tool, and bisection leads to this
commit. Reverting on top of 3.6 solves it for me.


Oh what fun (not). The best way to figure out what really is going
on is to get some usb level traces. Note my first hunch is that what
you're seeing is a device firmware bug, as this patch together with
a new libusb (which you seem to also have) will make bulk transfers
run slightly faster, which might be just enough to overwhelm your
device ...

Can you please do the following:
1) unplug as many usb devices as possible
2) plug in the programmer
3) run lsusb, note the programmer bus number
4) if the programmer is one the same bus as another device (other then
   a hub), try a different port
5) ideally rinse repeat until it is on a bus of its own, or atleast
   a bus with a device which generate little trafic
6) Writedown the bus number, and keep using the same port for
   all further tests

Then:
1) start wireshark as root, tell it to start capturing on the USB bus
you've ended up using
2) Do what you always do with the programmer
3) When finished, or when things have failed stop wireshark capture and
save the capture to a file

Repeat 2 times once with a kernel with the problematic commit, once without.

Then send me the 2 wireshark captures. Note:

1) Privacy warning: In theory I might be able to reconstruct what you're sending
over the captured USB bus when you send me these captures
2) Less is more, so if you can find some mini mini program to send to the
device you're programming that makes live easier (and should make 1 less of
an issue too).

Regards,

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


usb audio device can't work when it is attached to host via usb 2.0 hub

2012-10-11 Thread Elric Fu
Hi all,

I recently found a strange issue on a embedded platform and don't know
how to fix it. Please give me some suggestions.

The platform is ar9331. The issue is if we attach a usb audio device to
the host on ar9331 directly or via a usb 1.1 hub the usb audio works fine,
but if we attach it via usb 2.0 hub the undermentioned message will be
reported. Moreover, when the usb audio was attached to the host on X86
via usb 2.0 hub it worked fine too.

~ # usb 1-1.3: new full speed USB device using ar7240-ehci and address 3
usb 1-1.3: unable to read config index 0 descriptor/start: -145
usb 1-1.3: chopping to 0 config(s)
usb 1-1.3: string descriptor 0 read error: -145
usb 1-1.3: New USB device found, idVendor=040d, idProduct=3400
usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 1-1.3: no configuration chosen from 0 choices

I tracked the issue and found the usb audio had a strange issue. It is a
usb 1.1 device and works at full speed. But the interesting thing is the
bcdUSB field of device descriptor is 0x0200. So the
hub_port_connect_change() will involke the check_highspeed() which
would get device qualifier descriptor.

The correct case is the device doesn't have the device qualifier descriptor
and usb_get_descriptor() will retry 3 times and return -32. But now
usb_get_descriptor() will return -145 at the second time. Then when the
usb core send a get configuration descriptor request, but the control transfer
will be timeout again.

I used the usb analyzer to capture the trace and found host just sent one
get-device-qualifier-descriptor control transfer and the device return a
STALL. But at the second time, actually, the host even didn't send the setup
packet. I don't know why.

My opinion is the split transactions to the device which has a 0x0200 bcdUSB
but is actually a full speed device have some issues. I would
appreciate any ideas.

In addition, the entire descriptors of the usb audio device are :

Bus 002 Device 002: ID 040d:3400 VIA Technologies, Inc.
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0
  bDeviceProtocol 0
  bMaxPacketSize064
  idVendor   0x040d VIA Technologies, Inc.
  idProduct  0x3400
  bcdDevice0.90
  iManufacturer   1 VIA Technologies Inc.
  iProduct2 VIA USB Dongle
  iSerial 0
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  518
bNumInterfaces  4
bConfigurationValue 1
iConfiguration  0
bmAttributes 0x80
  (Bus Powered)
MaxPower  100mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   0
  bInterfaceClass 1 Audio
  bInterfaceSubClass  1 Control Device
  bInterfaceProtocol  0
  iInterface  0
  AudioControl Interface Descriptor:
bLength10
bDescriptorType36
bDescriptorSubtype  1 (HEADER)
bcdADC   1.00
wTotalLength  117
bInCollection   2
baInterfaceNr( 0)   1
baInterfaceNr( 1)   2
  AudioControl Interface Descriptor:
bLength12
bDescriptorType36
bDescriptorSubtype  2 (INPUT_TERMINAL)
bTerminalID 1
wTerminalType  0x0101 USB Streaming
bAssocTerminal  0
bNrChannels 2
wChannelConfig 0x0003
  Left Front (L)
  Right Front (R)
iChannelNames   0
iTerminal   0
  AudioControl Interface Descriptor:
bLength12
bDescriptorType36
bDescriptorSubtype  2 (INPUT_TERMINAL)
bTerminalID 2
wTerminalType  0x0201 Microphone
bAssocTerminal  0
bNrChannels 2
wChannelConfig 0x0003
  Left Front (L)
  Right Front (R)
iChannelNames   0
iTerminal   0
  AudioControl Interface Descriptor:
bLength 9
bDescriptorType36
bDescriptorSubtype  3 (OUTPUT_TERMINAL)
bTerminalID 3
wTerminalType  0x0301 Speaker
bAssocTerminal  0
bSourceID   6
iTerminal   0
  AudioControl Interface Descriptor:
bLength 9
bDescriptorType36
bDescriptorSubtype  3 (OUTPUT_TERMINAL)
bTerminalID 4
wTerminalType  0x0101 USB Streaming
bAssocTerminal

Re: [PATCH] usb: remove CONFIG_USB_MUSB_HOST etc

2012-10-11 Thread Sekhar Nori
On 10/11/2012 12:05 PM, Heiko Schocher wrote:
> Hello Manjunathappa
> 
> On 11.10.2012 07:42, Manjunathappa, Prakash wrote:
>> Hi,
>> On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
>>> From: Constantine Shulyupin
>>>
>>> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
>>> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
>>> and set MUSB_OTG configuration by default
>>> because this configuration options are removed from Kconfig.
>>>
>>> Signed-off-by: Constantine Shulyupin
>>>
>>> ---
>>>   arch/arm/mach-davinci/usb.c |6 --
>>>   1 file changed, 6 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
>>> index f77b953..34509ff 100644
>>> --- a/arch/arm/mach-davinci/usb.c
>>> +++ b/arch/arm/mach-davinci/usb.c
>>> @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
>>>   };
>>>
>>>   static struct musb_hdrc_platform_data usb_data = {
>>> -#if defined(CONFIG_USB_MUSB_OTG)
>>>   /* OTG requires a Mini-AB connector */
>>>   .mode   = MUSB_OTG,
>>> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
>>> -.mode   = MUSB_PERIPHERAL,
>>> -#elif defined(CONFIG_USB_MUSB_HOST)
>>> -.mode   = MUSB_HOST,
>>> -#endif
>>>   .clock= "usb",
>>>   .config=&musb_config,
>>>   };
>>
>> Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
>> mode with g-ether. It works.
>>
>> Acked-by: Manjunathappa, Prakash
> 
> I sent a similiar patch here:
> 
> http://comments.gmane.org/gmane.linux.usb.general/54512
> 
> If the issues, mentioned from Sergei for my patch, nullified I add my:

The last outstanding issue from Sergei seems to be additional comments
describing why MUSB_OTG is OK to use.

Prakash/Constantine,

Did you have to make any hardware changes when testing host/gadget on
DM6446 EVM? Or change in kernel configuration?

It appears that there is no way to choose any of the config option
affecting the mode setting. Right now it seems to be just defaulting to
MUSB_UNDEFINED. Setting it to MUSB_OTG would be better than that.

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


Re: [RFC] net: usbnet: add pre and post_reset support

2012-10-11 Thread Bjørn Mork
Bjørn Mork  writes:
> Oliver Neukum  writes:
>
>> On Wednesday 10 October 2012 18:34:52 Bjørn Mork wrote:
>>> Signed-off-by: Bjørn Mork 
>>> ---
>>> What do you think about something like this? Does it have to
>>> be more complicated?  Somewhat tested, and seems to do the
>>> job for me.
>>
>> Looks good. Which minidriver did you test with?
>
> Thanks for reviewing it.  I'll test a bit more with other drivers and
> submit it then.

I have now tested it with
  qmi_wwan, using Sierra Wireless MC7710 and Huawei E367u-2
  rndis_host, using a Samsung Galaxy 8.9 LTE with Android 3.2
  cdc_ncm, using an Ericsson F5521gw

The qmi_wwan and rndis_host tests were OK, keeping an active connection
up and running over the USB reset.  Even the mode switching Huawei
device survived this for some reason.  I guess that depends on how the
firmware interpretes "reset".

The cdc_ncm test was not entirely successful.  The Ericsson device
disconnects the active session on reset:

Oct 11 10:25:31 nemi kernel: [42965.212151] usb 2-1: reset high-speed USB 
device number 13 using ehci_hcd
Oct 11 10:25:31 nemi kernel: [42965.382001] cdc_ncm 2-1:1.6: wwan2: rxqlen 0 
--> 10
Oct 11 10:25:31 nemi kernel: [42965.382017] cdc_ncm 2-1:1.6: wwan2: rxqlen 10 
--> 11
Oct 11 10:25:31 nemi kernel: [42965.392019] cdc_ncm: wwan2: network connection: 
connected
Oct 11 10:25:31 nemi kernel: [42965.400037] cdc_ncm: wwan2: network connection: 
disconnected

And then the driver is out of sequence if you bring up the connection
again:

Oct 11 10:26:48 nemi kernel: [43042.144886] cdc_ncm: wwan2: 21 mbit/s downlink 
5 mbit/s uplink
Oct 11 10:26:48 nemi kernel: [43042.152877] cdc_ncm: wwan2: network connection: 
connected
Oct 11 10:26:50 nemi kernel: [43044.112325] sequence number glitch prev=267 
curr=0

This is harmless because the driver ignores the sequence numbers anyway.
But if the disconnect on reset is typical for NCM devices, then I guess
it does not help much adding pre and post_reset in cdc_ncm.

So I guess that demonstrates the issue with pre and post_reset on
devices keeping internal state pretty well: The effect is entirely
device dependent. The device reset behaviour is not known, and we risk
that the device changes some of its internal variables on reset.  So
adding reset support to class minidrivers is risky.  Although my
rndis_host test worked, there is no guarantee that other rndis devices
will work the same way.

What do we do? Drop it?  Add the support to usbnet and use it only where
"safe"? Add it and implement it in any minidriver using plain usbnet
suspend and resume, possibly adding device quirks later?

I believe qmi_wwan is a candidate for unconditional reset support
because all supported devices are based on the same chipset family using
the same chipset vendor firmware as a basis.  And there is some benefit,
as these devices often expose a usb-storage interface in modem mode as
well.


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


Re: [PATCH 2/3] usb: xhci: Add the suspend/resume functionality

2012-10-11 Thread Felipe Balbi
Hi,

On Thu, Oct 11, 2012 at 02:22:42PM +0530, Vikas Sajjan wrote:
> Hi Felipe,
> 
> Thanks for the comments, will get back with modifications.

cool, but please avoid top posting ;-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Oliver Neukum
On Thursday 11 October 2012 16:14:02 Ming Lei wrote:
> On Thu, Oct 11, 2012 at 12:11 PM, Oliver Neukum  wrote:
> > On Thursday 11 October 2012 11:18:22 Ming Lei wrote:
> 
> >> Currently scsi disk can only be runtime suspended when the device is not
> >> opened, so are you sure that the paging out above can cause IO on a suspend
> >> usb mass storage disk which is not mounted or opened by utility now?
> >
> > We definitely do not wish to keep it that way. People at Intel are 
> > currently working
> > on better power management for sd, which would allow full autosuspend.
> 
> OK, got it.
> 
> For auto-resume situation, it can be solved with switching the gpf_t flag
> runtime inside helper, but I think it is better to do it after the sd's
> full autosuspend is seen in -next tree.

That depends on whether an API change would be necessary.
Changing the code only when necessary is no problem. But the
API I want to do right from the beginning if that is possible.

This depends on whether you get in your extension to task_struct.
If you do, we can certainly use it also for the suspend case.

> For error handling case, it is inevitably for usbnet to allocate memory
> with GFP_KERNEL because no usbnet drivers have implemented
> .pre_reset and .post_reset callback, and no such actual problems
> have been reported until now, so it should be OK to not consider the
> case now.
> 
> So could we merge the patch set[1-11] first?

Given the solution for error handling you came up with, yes, we can.
Would you resubmit?

Regards
Oliver

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


Re: FTDI USB-to-UART converters and tcdrain()

2012-10-11 Thread Jarkko Huijts
On Thu, 2012-10-11 at 04:57 +0900, Greg KH wrote:
> The patch looks great, thanks so much for doing this.  I'll queue it up
> after 3.7-rc1 is released by Linus (which should be in a few days).

No problem. It's pretty cool to be able to contribute.

--
To unsubscribe from this list: send the line "unsubscribe 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 audio device can't work when it is attached to host via usb 2.0 hub

2012-10-11 Thread Peter Stuge
Elric Fu wrote:
> It is a usb 1.1 device and works at full speed.

It is a frequent misunderstanding that a device must be 1.1 only
because it does not support high speed. This is not the case.


> But the interesting thing is the bcdUSB field of device descriptor
> is 0x0200.

It is perfectly legal for a USB 2.0 device to only support low or
full speed.


I'm not sure what the problem is with this host or device, just
mentioning that a device can be USB 2.0 and only support lower
speeds.

It amazes me how many people reduce USB 2.0 to mean high speed.
The standard took several years to develop, and it contains a lot
more than the added high speed communication.


//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: [RFC] net: usbnet: add pre and post_reset support

2012-10-11 Thread Oliver Neukum
On Thursday 11 October 2012 11:05:05 Bjørn Mork wrote:
> Bjørn Mork  writes:

> This is harmless because the driver ignores the sequence numbers anyway.
> But if the disconnect on reset is typical for NCM devices, then I guess
> it does not help much adding pre and post_reset in cdc_ncm.

Nor does it hurt.
 
> So I guess that demonstrates the issue with pre and post_reset on
> devices keeping internal state pretty well: The effect is entirely
> device dependent. The device reset behaviour is not known, and we risk
> that the device changes some of its internal variables on reset.  So
> adding reset support to class minidrivers is risky.  Although my
> rndis_host test worked, there is no guarantee that other rndis devices
> will work the same way.
> 
> What do we do? Drop it?  Add the support to usbnet and use it only where

We want to do error handling well. That means we don't diminish
support for good hardware because crap hardware can't do it.
We don't break normal operation, but we are talking about error
handling. Something is wrong away when we go down this code path.

> "safe"? Add it and implement it in any minidriver using plain usbnet
> suspend and resume, possibly adding device quirks later?

We can add quirks later.
 
> I believe qmi_wwan is a candidate for unconditional reset support
> because all supported devices are based on the same chipset family using
> the same chipset vendor firmware as a basis.  And there is some benefit,
> as these devices often expose a usb-storage interface in modem mode as
> well.

Go for it.

Regards
Oliver

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


Re: [RFC] net: usbnet: add pre and post_reset support

2012-10-11 Thread Oliver Neukum
On Thursday 11 October 2012 11:05:05 Bjørn Mork wrote:
> So I guess that demonstrates the issue with pre and post_reset on
> devices keeping internal state pretty well: The effect is entirely
> device dependent. The device reset behaviour is not known, and we risk
> that the device changes some of its internal variables on reset.  So
> adding reset support to class minidrivers is risky.  Although my
> rndis_host test worked, there is no guarantee that other rndis devices
> will work the same way.

On second thought we'd better tell user space that a reset has happened.
Any ideas on how?

Regards
Oliver

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


Re: [PATCH] usb: remove CONFIG_USB_MUSB_HOST etc

2012-10-11 Thread Constantine Shulyupin
I've tested gadget mode on DM365 without HW changes.

On Thu, Oct 11, 2012 at 10:50 AM, Sekhar Nori  wrote:
> On 10/11/2012 12:05 PM, Heiko Schocher wrote:
>> Hello Manjunathappa
>>
>> On 11.10.2012 07:42, Manjunathappa, Prakash wrote:
>>> Hi,
>>> On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
 From: Constantine Shulyupin

 Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
 CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
 and set MUSB_OTG configuration by default
 because this configuration options are removed from Kconfig.

 Signed-off-by: Constantine Shulyupin

 ---
   arch/arm/mach-davinci/usb.c |6 --
   1 file changed, 6 deletions(-)

 diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
 index f77b953..34509ff 100644
 --- a/arch/arm/mach-davinci/usb.c
 +++ b/arch/arm/mach-davinci/usb.c
 @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
   };

   static struct musb_hdrc_platform_data usb_data = {
 -#if defined(CONFIG_USB_MUSB_OTG)
   /* OTG requires a Mini-AB connector */
   .mode   = MUSB_OTG,
 -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
 -.mode   = MUSB_PERIPHERAL,
 -#elif defined(CONFIG_USB_MUSB_HOST)
 -.mode   = MUSB_HOST,
 -#endif
   .clock= "usb",
   .config=&musb_config,
   };
>>>
>>> Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
>>> mode with g-ether. It works.
>>>
>>> Acked-by: Manjunathappa, Prakash
>>
>> I sent a similiar patch here:
>>
>> http://comments.gmane.org/gmane.linux.usb.general/54512
>>
>> If the issues, mentioned from Sergei for my patch, nullified I add my:
>
> The last outstanding issue from Sergei seems to be additional comments
> describing why MUSB_OTG is OK to use.
>
> Prakash/Constantine,
>
> Did you have to make any hardware changes when testing host/gadget on
> DM6446 EVM? Or change in kernel configuration?
>
> It appears that there is no way to choose any of the config option
> affecting the mode setting. Right now it seems to be just defaulting to
> MUSB_UNDEFINED. Setting it to MUSB_OTG would be better than that.
>
> Thanks,
> Sekhar



-- 
Constantine Shulyupin
http://www.MakeLinux.com/
Embedded Linux Systems,
Device Drivers, TI DaVinci
--
To unsubscribe from this list: send the line "unsubscribe 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: removing the timer from cdc-ncm

2012-10-11 Thread Bjørn Mork
Oliver Neukum  writes:
> On Tuesday 25 September 2012 13:18:10 Alexey ORISHKO wrote:
>
>> Linux host got panic right after data path has been established
>> (i.e. connected to mobile network). 
>
> Thank you. Worse than I hoped, but not unexpected. I'll stare at the
> code a bit.

I had the same experience, but thought I was going to look a bit more at
it before sending anything and forgot all about it.  So this time I am
just sending you my preliminary results instead of waiting.

I am running with your initial patch, Alexeys suggested cleanup, and
some additional debug printks.  The driver receives a few ARPs from the
device, but crashes on the very first outgoing dhcp packet (which is
dumped with a ">>" prefix from cdc_ncm_tx_bundle just before calling
cdc_ncm_fill_tx_frame):

[48880.037638] cdc_ncm: wwan0: network connection: connected
[48880.044038] IPv6: ADDRCONF(NETDEV_CHANGE): wwan0: link becomes ready
[48880.048351] >> : ff ff ff ff ff ff 02 80 37 ec 02 00 08 00 45 10  
7.E.
[48880.048361] >> 0010: 01 48 00 00 00 00 80 11 39 96 00 00 00 00 ff ff  
.H..9...
[48880.048365] >> 0020: ff ff 00 44 00 43 01 34 9e fd 01 01 06 00 9e 31  
...D.C.4...1
[48880.048370] >> 0030: 64 26 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
d&..
[48880.048374] >> 0040: 00 00 00 00 00 00 02 80 37 ec 02 00 00 00 00 00  
7...
[48880.048378] >> 0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048382] >> 0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048386] >> 0070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048390] >> 0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048395] >> 0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048398] >> 00a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048406] >> 00b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048411] >> 00c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048414] >> 00d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048418] >> 00e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048422] >> 00f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048427] >> 0100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048431] >> 0110: 00 00 00 00 00 00 63 82 53 63 35 01 01 37 07 01  
..c.Sc5..7..
[48880.048435] >> 0120: 1c 02 03 0f 06 0c ff 00 00 00 00 00 00 00 00 00  

[48880.048439] >> 0130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048444] >> 0140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  

[48880.048448] >> 0150: 00 00 00 00 00 00
..
[48880.048453] cdc_ncm_fill_tx_frame: ctx=880162c97600, skb=88018237bac0
[48880.048494] BUG: unable to handle kernel NULL pointer dereference at 
0068
[48880.048573] IP: [] cdc_ncm_tx_bundle+0x168/0x43b [cdc_ncm]
[48880.048638] PGD 0 
[48880.048663] Oops:  [#1] SMP 
[48880.048702] Modules linked in: cdc_wdm cdc_ncm(O) netconsole configfs 
usbnet(O) mii cdc_acm usbhid hid option usb_storage uas nfsv3 nfsv4 auth_rpcgss 
udf crc_itu_t xt_multiport iptable_filter ip_tables cpufreq_userspace 
cpufreq_stats cpufreq_conservative cpufreq_powersave rfcomm bnep xt_hl 
binfmt_misc ip6table_filter ip6_tables x_tables fuse nfsd nfs_acl nfs lockd 
fscache sunrpc 8021q garp stp llc tun ext2 loop snd_hda_codec_conexant 
snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss iTCO_wdt 
snd_pcm iTCO_vendor_support thinkpad_acpi nvram snd_page_alloc snd_seq_midi 
snd_seq_midi_event snd_rawmidi arc4 iwldvm mac80211 snd_seq snd_timer 
snd_seq_device qcserial usb_wwan coretemp kvm_intel usbserial uvcvideo 
videobuf2_vmalloc btusb kvm videobuf2_memops videobuf2_core bluetooth psmouse 
i2c_i801 serio_raw videodev evdev crc16 lpc_ich acpi_cpufreq mfd_core ac 
battery snd iwlwifi mperf wmi i915 cfg80211 rfkill video processor button 
drm_kms_helper drm soundcore mei i2c_algo_bit i2c_core ext3 mbcache jbd 
sha256_generic ablk_helper cryptd aes_x86_64 aes_generic cbc dm_crypt dm_mod 
nbd sg sd_mod sr_mod crc_t10dif cdrom microcode thermal thermal_sys uhci_hcd 
ahci ehci_hcd libahci libata e1000e scsi_mod usbcore usb_common [last unloaded: 
cdc_ncm]
[48880.050129] CPU 1 
[48880.050151] Pid: 5467, comm: dhclient Tainted: GW  O 3.6.0 #36 
LENOVO 2776LEG/2776LEG
[48880.050218] RIP: 0010:[]  [] 
cdc_ncm_tx_bundle+0x168/0x43b [cdc_ncm]
[48880.050297] RSP: 0018:880232189ab8  EFLAGS: 00010287
[48880.050340] RAX:  RBX: 880162c97600 RCX: 
[48880.050395] RDX: 0800 RSI: 8802310bac00 RDI: 0246
[

Re: [RFC] net: usbnet: add pre and post_reset support

2012-10-11 Thread Bjørn Mork
Oliver Neukum  writes:
> On Thursday 11 October 2012 11:05:05 Bjørn Mork wrote:
>> So I guess that demonstrates the issue with pre and post_reset on
>> devices keeping internal state pretty well: The effect is entirely
>> device dependent. The device reset behaviour is not known, and we risk
>> that the device changes some of its internal variables on reset.  So
>> adding reset support to class minidrivers is risky.  Although my
>> rndis_host test worked, there is no guarantee that other rndis devices
>> will work the same way.
>
> On second thought we'd better tell user space that a reset has happened.
> Any ideas on how?

No, not really. I guess we need to trigger a rtnetlink message of some
sort?  Flash the carrier status maybe?  But I don't know how to do that
in a generic way.


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


Re: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-10-11 Thread Gian-Carlo Pascutto
On 21/09/12 22:44, Alan Stern wrote:

> What is the Razer DeathAdder?

A mouse. (With some extra buttons)

> I can't tell much from these logs.  A usbmon trace might help more,
> although I don't know whether it would be better to trace the bus for
> the headset or the bus for the camera.  See the instructions in
> Documentation/usb/usbmon.txt.  Start the trace after the skype
> connection is established, and leave it running until after you drop
> the connection and start getting all those errors.
> 
> Also, please send the contents of /sys/kernel/debug/usb/devices with 
> everything plugged in and skype running.

Will do.

I noticed that sometimes when booting the USB subsystem already behaves
erratically. For example this morning I'm greeted by:

[  234.455604] ohci_hcd :00:04.0: urb 8801296fac80 path 4 ep1in
9312 cc 9 --> status -121
[  234.466567] ohci_hcd :00:04.0: urb 88010ce362c0 path 4 ep1in
9212 cc 9 --> status -121

which keeps repeating infinitely.

I've also seen:

Oct  8 08:52:37 denebix kernel: [  358.482780] ohci_hcd :00:04.0:
GetStatus roothub.portstatus [3] = 0x00130301 PRSC PESC CSC LSDA PPS CCS
Oct  8 08:52:37 denebix kernel: [  358.482808] hub 4-0:1.0: unable to
enumerate USB device on port 4
Oct  8 08:52:37 denebix kernel: [  358.482823] hub 4-0:1.0: state 7
ports 6 chg  evt 0010
Oct  8 08:52:37 denebix kernel: [  358.482837] ohci_hcd :00:04.0:
GetStatus roothub.portstatus [3] = 0x00030301 PESC CSC LSDA PPS CCS
Oct  8 08:52:37 denebix kernel: [  358.482851] hub 4-0:1.0: port 4,
status 0301, change 0003, 1.5 Mb/s

which also repeats infinitely.

Maybe my hardware/chipset is just crap?

-- 
GCP

--
To unsubscribe from this list: send the line "unsubscribe 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 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Ming Lei
On Thu, Oct 11, 2012 at 5:05 PM, Oliver Neukum  wrote:
> That depends on whether an API change would be necessary.
> Changing the code only when necessary is no problem. But the
> API I want to do right from the beginning if that is possible.

For the auto-resume situation, the current API is OK even without
changes to task_struct.

>
> This depends on whether you get in your extension to task_struct.
> If you do, we can certainly use it also for the suspend case.

I will do it.

>
>> For error handling case, it is inevitably for usbnet to allocate memory
>> with GFP_KERNEL because no usbnet drivers have implemented
>> .pre_reset and .post_reset callback, and no such actual problems
>> have been reported until now, so it should be OK to not consider the
>> case now.
>>
>> So could we merge the patch set[1-11] first?
>
> Given the solution for error handling you came up with, yes, we can.
> Would you resubmit?

OK, will do it.

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


USB errors, ehci testing modes

2012-10-11 Thread Tim Sander
Hi

I have some stange errors on a arm pcm043 with internal usb phy. The log is 
attached at the end of the mail. It would be nice if someone could give me a 
pointer whats going wrong?. It would also be nice to know where the status 
flags which are output by the usb debug messages are documented?

As it might be a hw problem i would like to activate the testmodes of the ehci 
controller. The old Freescale BSP driver had a file in debugfs where enabling 
this stuff was easy. Is there also an easy way to enable the ehci debug 
facility from linux usermode? 

Thanks
Tim

The very seldom error i have seen:
usb-storage: Bulk data transfer result 0x0
usb-storage: Attempting to get CSW...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 13/13
usb-storage: -- transfer complete
usb-storage: Bulk status result = 0
usb-storage: Bulk Status S 0x53425355 T 0x148af R 0 Stat 0x0
usb-storage: scsi cmd done, result=0x0
usb-storage: *** thread sleeping.
hub 1-0:1.0: state 7 ports 1 chg  evt 0002
mxc-ehci mxc-ehci.0: GetStatus port:1 status c00100a 6  ACK POWER sig=se0 PEC 
CSC
hub 1-0:1.0: port 1, status 0100, change 0003, 12 Mb/s
usb 1-1: USB disconnect, device number 23
usb 1-1: unregistering device
usb 1-1: unregistering interface 1-1:1.0
usb-storage: storage_disconnect() called
usb-storage: -- usb_stor_release_resources
usb-storage: -- sending exit command to thread
usb-storage: *** thread awakened.
usb-storage: -- exiting
usb-storage: -- dissociate_dev
usb 1-1: usb_disable_device nuking all URBs
mxc-ehci mxc-ehci.0: GetStatus port:1 status 001803 0  ACK POWER sig=j CSC 
CONNECT
hub 1-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101
mxc-ehci mxc-ehci.0: port 1 high speed
mxc-ehci mxc-ehci.0: GetStatus port:1 status 8001205 4  ACK POWER sig=se0 LPM 
PE CONNECT
usb 1-1: new high-speed USB device number 24 using mxc-ehci
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 1
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 2
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 3
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 4
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 5
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 6
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 7
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 8
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 9
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 10
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 11
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 12
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 13
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 14
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 15
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 16
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 17
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 18
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 19
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 20
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 21
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 22
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 23
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 24
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 25
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 26
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 27
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 28
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 29
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 30
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 31
mxc-ehci mxc-ehci.0: devpath 1 ep0in 3strikes
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 1
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 2
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 3
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 4
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 5
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 6
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 7
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 8
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 9
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 10
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 11
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 12
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 13
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 14
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 15
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 16
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 17
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 18
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 19
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 20
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 21
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 22
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 23
mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 24
mxc-ehci mxc-ehci.0: detected XactErr len 0/

Re: removing the timer from cdc-ncm

2012-10-11 Thread Bjørn Mork
Bjørn Mork  writes:

> [48880.048494] BUG: unable to handle kernel NULL pointer dereference at 
> 0068
> [48880.048573] IP: [] cdc_ncm_tx_bundle+0x168/0x43b 
> [cdc_ncm]

This one is because you removed the "if (skb == NULL)" from the for
loop, but left the "skb = NULL;" at the end:


@@ -719,28 +707,15 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb)
/* compute maximum buffer size */
rem = ctx->tx_max - offset;
 
-   if (skb == NULL) {
-   skb = ctx->tx_rem_skb;
-   ctx->tx_rem_skb = NULL;
-
-   /* check for end of skb */
-   if (skb == NULL)
-   break;
-   }
-
if (skb->len > rem) {
if (n == 0) {
/* won't fit, MTU problem? */
dev_kfree_skb_any(skb);
skb = NULL;
ctx->netdev->stats.tx_dropped++;
+   error = 1;
} else {
-   /* no room for skb - store for later */
-   if (ctx->tx_rem_skb != NULL) {
-   dev_kfree_skb_any(ctx->tx_rem_skb);
-   ctx->netdev->stats.tx_dropped++;
-   }
-   ctx->tx_rem_skb = skb;
+
skb = NULL;
ready2send = 1;
}
@@ -768,13 +743,6 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct 
sk_buff *skb)
skb = NULL;
}


If I understand your intentions with this code, then the for loop should
probably just go away completely?

Anyway, after avoiding that one, I ended up with

[  857.112692] cdc_ncm_fill_tx_frame: skb_out=880218624cc0, length=2048
[  857.112696] cdc_ncm_tx_fixup: returning   (null)
[  857.112731] BUG: unable to handle kernel NULL pointer dereference at 
0068
[  857.112807] IP: [] usbnet_start_xmit+0x128/0x4e9 [usbnet]

No need for the reset of the trace here. Removing the "goto not_drop"
makes usbnet_start_xmit continue after tx_fixup returns NULL:


-   if (info->tx_fixup) {
+   if (transmit_now && info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) {
if (netif_msg_tx_err(dev)) {
netif_dbg(dev, tx_err, dev->net, "can't 
tx_fixup skb\n");
goto drop;
-   } else {
-   /* cdc_ncm collected packet; waits for more */
-   goto not_drop;
}
}
}


// some devices want funky USB-level framing, for
// win32 driver (usually) and/or hardware quirks
if (transmit_now && info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) {
if (netif_msg_tx_err(dev)) {
netif_dbg(dev, tx_err, dev->net, "can't 
tx_fixup skb\n");
goto drop;
}
}
}
length = skb->len;



I stopped there.  I am not exactly sure where you were going with this
anymore.  tx_fixup will return the tx_curr_skb being currently built
every time it is called, without anything resetting it at that point.
Then when finally cdc_ncm_fill_tx_frame has been called enough times to
fill it completely, you end up just dropping it on the floor.  The
result is that nothing is ever transmitted even after fixing the above
errors because the tx_curr_skb has an incomplete header every time
tx_fixup returns it.

I do like your idea, but if you don't mind I think we'll go ahead and
base the upcoming cdc_mbim driver on the current cdc_ncm *with* the
timer.  The plan is to reuse as much of cdc_ncm as possible, so in
theory it should be easy to update both drivers when the timer goes away
(there is no need for cdc_mbim be aware of the timer at all).  But
unfortunately it seems that cdc_ncm_fill_tx_frame must be changed a lot
to support the concept of multiple NDPs in a single NTB.  This means
that the timer removal patches will conflict heavily with the
preparations for MBIM support.  With some luck and acceptance from all
contributors, I hope to be able to post the first version of that as the
merge window closes.



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


[PATCH v1 00/11] usbnet: usb_control_msg cleanup

2012-10-11 Thread Ming Lei
Hi,

This patch set introduces 3 helpers for handling usb read, write
and write_async command, and replaces the low level's implemention
with the generic ones.

This patchset is a cleanup and about 300 lines code can be saved.

Also, the patchset fixes problem of DMA on buffer embedded inside
one dynamic allocated buffer, and such usages can be found
in cdc-ncm, sierra_net, mcs7830 and asix drivers.

v1:
- drop previous patch 12, and let net/core handle
runtime PM in ioctl path

 drivers/net/usb/asix_common.c |  117 
 drivers/net/usb/cdc_ncm.c |   73 +++---
 drivers/net/usb/dm9601.c  |  107 +
 drivers/net/usb/int51x1.c |   52 +---
 drivers/net/usb/mcs7830.c |   85 ++
 drivers/net/usb/net1080.c |  110 ++
 drivers/net/usb/plusb.c   |   11 ++--
 drivers/net/usb/sierra_net.c  |   45 ++
 drivers/net/usb/smsc75xx.c|   39 +---
 drivers/net/usb/smsc95xx.c|  115 +--
 drivers/net/usb/usbnet.c  |  133 +
 include/linux/usb/usbnet.h|6 ++
 12 files changed, 291 insertions(+), 602 deletions(-)

Thanks,
--
Ming Lei

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


[PATCH v1 01/11] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Ming Lei
This patch introduces the below 3 usb command helpers:

usbnet_read_cmd / usbnet_write_cmd / usbnet_write_cmd_async

so that each low level driver doesn't need to implement them
by itself, and the dma buffer allocation for usb transfer and
runtime PM things can be handled just in one place.

Signed-off-by: Ming Lei 
---
 drivers/net/usb/usbnet.c   |  133 
 include/linux/usb/usbnet.h |6 ++
 2 files changed, 139 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index fc9f578..3b51554 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1592,6 +1592,139 @@ int usbnet_resume (struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usbnet_resume);
 
+/*-*/
+int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+   u16 value, u16 index, void *data, u16 size)
+{
+   void *buf = NULL;
+   int err = -ENOMEM;
+
+   netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   if (data) {
+   buf = kmalloc(size, GFP_KERNEL);
+   if (!buf)
+   goto out;
+   }
+
+   err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_GET_TIMEOUT);
+   if (err > 0 && err <= size)
+   memcpy(data, buf, err);
+   kfree(buf);
+out:
+   return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd);
+
+int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+u16 value, u16 index, const void *data, u16 size)
+{
+   void *buf = NULL;
+   int err = -ENOMEM;
+
+   netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   if (data) {
+   buf = kmemdup(data, size, GFP_KERNEL);
+   if (!buf)
+   goto out;
+   }
+
+   err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_SET_TIMEOUT);
+   kfree(buf);
+
+out:
+   return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd);
+
+static void usbnet_async_cmd_cb(struct urb *urb)
+{
+   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
+   int status = urb->status;
+
+   if (status < 0)
+   dev_dbg(&urb->dev->dev, "%s failed with %d",
+   __func__, status);
+
+   kfree(req);
+   usb_free_urb(urb);
+}
+
+int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+  u16 value, u16 index, const void *data, u16 size)
+{
+   struct usb_ctrlrequest *req = NULL;
+   struct urb *urb;
+   int err = -ENOMEM;
+   void *buf = NULL;
+
+   netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   urb = usb_alloc_urb(0, GFP_ATOMIC);
+   if (!urb) {
+   netdev_err(dev->net, "Error allocating URB in"
+  " %s!\n", __func__);
+   goto fail;
+   }
+
+   if (data) {
+   buf = kmemdup(data, size, GFP_ATOMIC);
+   if (!buf) {
+   netdev_err(dev->net, "Error allocating buffer"
+  " in %s!\n", __func__);
+   goto fail_free;
+   }
+   }
+
+   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+   if (!req) {
+   netdev_err(dev->net, "Failed to allocate memory for %s\n",
+  __func__);
+   goto fail_free_buf;
+   }
+
+   req->bRequestType = reqtype;
+   req->bRequest = cmd;
+   req->wValue = cpu_to_le16(value);
+   req->wIndex = cpu_to_le16(index);
+   req->wLength = cpu_to_le16(size);
+
+   usb_fill_control_urb(urb, dev->udev,
+usb_sndctrlpipe(dev->udev, 0),
+(void *)req, buf, size,
+usbnet_async_cmd_cb, req);
+   urb->transfer_flags |= URB_FREE_BUFFER;
+
+   err = usb_submit_urb(urb, GFP_ATOMIC);
+   if (err < 0) {
+   netdev_err(dev->net, "Error submitting the control"
+  " message: status=%d\n", err);
+   goto fail_free;
+   }
+   return 0;
+
+fail_free_buf:
+   kfree(buf);
+fail_free:
+   kfree(req);
+   usb_free_urb(urb);
+fail:
+   return err;
+
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd_async);
+
 
 /*--

[PATCH v1 02/11] usbnet: asix: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/asix_common.c |  117 +
 1 file changed, 13 insertions(+), 104 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 774d9ce..50d1673 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -25,121 +25,30 @@
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data)
 {
-   void *buf;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   buf = kmalloc(size, GFP_KERNEL);
-   if (!buf)
-   goto out;
-
-   err = usb_control_msg(
-   dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   cmd,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value,
-   index,
-   buf,
-   size,
-   USB_CTRL_GET_TIMEOUT);
-   if (err == size)
-   memcpy(data, buf, size);
-   else if (err >= 0)
-   err = -EINVAL;
-   kfree(buf);
+   int ret;
+   ret = usbnet_read_cmd(dev, cmd,
+  USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  value, index, data, size);
 
-out:
-   return err;
+   if (ret != size && ret >= 0)
+   return -EINVAL;
+   return ret;
 }
 
 int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
   u16 size, void *data)
 {
-   void *buf = NULL;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   if (data) {
-   buf = kmemdup(data, size, GFP_KERNEL);
-   if (!buf)
-   goto out;
-   }
-
-   err = usb_control_msg(
-   dev->udev,
-   usb_sndctrlpipe(dev->udev, 0),
-   cmd,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value,
-   index,
-   buf,
-   size,
-   USB_CTRL_SET_TIMEOUT);
-   kfree(buf);
-
-out:
-   return err;
-}
-
-static void asix_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
-   status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, cmd,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   value, index, data, size);
 }
 
 void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data)
 {
-   struct usb_ctrlrequest *req;
-   int status;
-   struct urb *urb;
-
-   netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_err(dev->net, "Error allocating URB in 
write_cmd_async!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-   if (!req) {
-   netdev_err(dev->net, "Failed to allocate memory for control 
request\n");
-   usb_free_urb(urb);
-   return;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-   req->bRequest = cmd;
-   req->wValue = cpu_to_le16(value);
-   req->wIndex = cpu_to_le16(index);
-   req->wLength = cpu_to_le16(size);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, size,
-asix_async_cmd_callback, req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_err(dev->net, "Error submitting the control message: 
status=%d\n",
-  status);
-   kfree(req);
-   usb_free_urb(urb);
-   }
+   usbnet_write_cmd_async(dev, cmd,
+  USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  value, index, data, size);
 }
 
 int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-- 
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 v1 03/11] usbnet: cdc-ncm: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/cdc_ncm.c |   73 ++---
 1 file changed, 23 insertions(+), 50 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..429a2ad 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -159,16 +159,16 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
u8 iface_no;
int err;
u16 ntb_fmt_supported;
+   struct usbnet *dev = netdev_priv(ctx->netdev);
 
iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
-   err = usb_control_msg(ctx->udev,
-   usb_rcvctrlpipe(ctx->udev, 0),
+   err = usbnet_read_cmd(dev,
USB_CDC_GET_NTB_PARAMETERS,
USB_TYPE_CLASS | USB_DIR_IN
 | USB_RECIP_INTERFACE,
0, iface_no, &ctx->ncm_parm,
-   sizeof(ctx->ncm_parm), 1);
+   sizeof(ctx->ncm_parm));
if (err < 0) {
pr_debug("failed GET_NTB_PARAMETERS\n");
return 1;
@@ -217,40 +217,23 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
 
if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
-   struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
+   struct usb_cdc_ncm_ndp_input_size ndp_in_sz;
 
-   ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
-   if (!ndp_in_sz) {
-   err = -ENOMEM;
-   goto size_err;
-   }
-
-   err = usb_control_msg(ctx->udev,
-   usb_sndctrlpipe(ctx->udev, 0),
+   memset(&ndp_in_sz, 0, sizeof(ndp_in_sz));
+   err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_INPUT_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
 | USB_RECIP_INTERFACE,
-   0, iface_no, ndp_in_sz, 8, 1000);
-   kfree(ndp_in_sz);
+   0, iface_no, &ndp_in_sz, 8);
} else {
-   __le32 *dwNtbInMaxSize;
-   dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize),
-   GFP_KERNEL);
-   if (!dwNtbInMaxSize) {
-   err = -ENOMEM;
-   goto size_err;
-   }
-   *dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
+   __le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
 
-   err = usb_control_msg(ctx->udev,
-   usb_sndctrlpipe(ctx->udev, 0),
+   err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_INPUT_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
 | USB_RECIP_INTERFACE,
-   0, iface_no, dwNtbInMaxSize, 4, 1000);
-   kfree(dwNtbInMaxSize);
+   0, iface_no, &dwNtbInMaxSize, 4);
}
-size_err:
if (err < 0)
pr_debug("Setting NTB Input Size failed\n");
}
@@ -306,23 +289,23 @@ size_err:
 
/* set CRC Mode */
if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
-   err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+   err = usbnet_write_cmd(dev,
USB_CDC_SET_CRC_MODE,
USB_TYPE_CLASS | USB_DIR_OUT
 | USB_RECIP_INTERFACE,
USB_CDC_NCM_CRC_NOT_APPENDED,
-   iface_no, NULL, 0, 1000);
+   iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting CRC mode off failed\n");
}
 
/* set NTB format, if both formats are supported */
if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
-   err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+   err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
 | USB_DIR_OUT | USB_RECIP_INTERFACE,
USB_CDC_NCM_NTB16_FORMAT,
-   iface_no, NULL, 0, 1000);
+   iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting NTB format to 16-bit failed\n");
}
@@ 

[PATCH v1 04/11] usbnet: dm9601: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/dm9601.c |  107 +++---
 1 file changed, 15 insertions(+), 92 deletions(-)

diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index e0433ce..3f554c1 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -56,27 +56,12 @@
 
 static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-   void *buf;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "dm_read() reg=0x%02x length=%d\n", reg, length);
-
-   buf = kmalloc(length, GFP_KERNEL);
-   if (!buf)
-   goto out;
-
-   err = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- DM_READ_REGS,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-   if (err == length)
-   memcpy(data, buf, length);
-   else if (err >= 0)
+   int err;
+   err = usbnet_read_cmd(dev, DM_READ_REGS,
+  USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  0, reg, data, length);
+   if(err != length && err >= 0)
err = -EINVAL;
-   kfree(buf);
-
- out:
return err;
 }
 
@@ -87,91 +72,29 @@ static int dm_read_reg(struct usbnet *dev, u8 reg, u8 
*value)
 
 static int dm_write(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-   void *buf = NULL;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "dm_write() reg=0x%02x, length=%d\n", reg, length);
+   int err;
+   err = usbnet_write_cmd(dev, DM_WRITE_REGS,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   0, reg, data, length);
 
-   if (data) {
-   buf = kmemdup(data, length, GFP_KERNEL);
-   if (!buf)
-   goto out;
-   }
-
-   err = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- DM_WRITE_REGS,
- USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-   kfree(buf);
if (err >= 0 && err < length)
err = -EINVAL;
- out:
return err;
 }
 
 static int dm_write_reg(struct usbnet *dev, u8 reg, u8 value)
 {
-   netdev_dbg(dev->net, "dm_write_reg() reg=0x%02x, value=0x%02x\n",
-  reg, value);
-   return usb_control_msg(dev->udev,
-  usb_sndctrlpipe(dev->udev, 0),
-  DM_WRITE_REG,
-  USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
-  value, reg, NULL, 0, USB_CTRL_SET_TIMEOUT);
-}
-
-static void dm_write_async_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "dm_write_async_callback() failed with %d\n",
-  status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, DM_WRITE_REGS,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   value, reg, NULL, 0);
 }
 
 static void dm_write_async_helper(struct usbnet *dev, u8 reg, u8 value,
  u16 length, void *data)
 {
-   struct usb_ctrlrequest *req;
-   struct urb *urb;
-   int status;
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_err(dev->net, "Error allocating URB in 
dm_write_async_helper!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-   if (!req) {
-   netdev_err(dev->net, "Failed to allocate memory for control 
request\n");
-   usb_free_urb(urb);
-   return;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-   req->bRequest = length ? DM_WRITE_REGS : DM_WRITE_REG;
-   req->wValue = cpu_to_le16(value);
-   req->wIndex = cpu_to_le16(reg);
-   req->wLength = cpu_to_le16(length);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, length,
-dm_write_async_callback, req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_err(dev->net, "Error submitting the control message: 
status=%d\n",
-  status);
-   kfree(req);
-   usb_free_urb(urb);
-   }
+   usbnet_write_cmd_async(dev, DM_WRITE_REGS,
+  USB_DIR_OUT | USB_TYPE_VENDOR | U

[PATCH v1 05/11] usbnet: int51x1: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/int51x1.c |   52 +++--
 1 file changed, 3 insertions(+), 49 deletions(-)

diff --git a/drivers/net/usb/int51x1.c b/drivers/net/usb/int51x1.c
index 8de6417..ace9e74 100644
--- a/drivers/net/usb/int51x1.c
+++ b/drivers/net/usb/int51x1.c
@@ -116,23 +116,8 @@ static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev,
return skb;
 }
 
-static void int51x1_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   dev_warn(&urb->dev->dev, "async callback failed with %d\n", 
status);
-
-   kfree(req);
-   usb_free_urb(urb);
-}
-
 static void int51x1_set_multicast(struct net_device *netdev)
 {
-   struct usb_ctrlrequest *req;
-   int status;
-   struct urb *urb;
struct usbnet *dev = netdev_priv(netdev);
u16 filter = PACKET_TYPE_DIRECTED | PACKET_TYPE_BROADCAST;
 
@@ -149,40 +134,9 @@ static void int51x1_set_multicast(struct net_device 
*netdev)
netdev_dbg(dev->net, "receive own packets only\n");
}
 
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_warn(dev->net, "Error allocating URB\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(*req), GFP_ATOMIC);
-   if (!req) {
-   netdev_warn(dev->net, "Error allocating control msg\n");
-   goto out;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
-   req->bRequest = SET_ETHERNET_PACKET_FILTER;
-   req->wValue = cpu_to_le16(filter);
-   req->wIndex = 0;
-   req->wLength = 0;
-
-   usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   (void *)req, NULL, 0,
-   int51x1_async_cmd_callback,
-   (void *)req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_warn(dev->net, "Error submitting control msg, sts=%d\n",
-   status);
-   goto out1;
-   }
-   return;
-out1:
-   kfree(req);
-out:
-   usb_free_urb(urb);
+   usbnet_write_cmd_async(dev, SET_ETHERNET_PACKET_FILTER,
+  USB_DIR_OUT | USB_TYPE_CLASS | 
USB_RECIP_INTERFACE,
+  filter, 0, NULL, 0);
 }
 
 static const struct net_device_ops int51x1_netdev_ops = {
-- 
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 v1 06/11] usbnet: mcs7830: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/mcs7830.c |   85 -
 1 file changed, 6 insertions(+), 79 deletions(-)

diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index 03c2d8d..db46a68 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -123,93 +123,20 @@ static const char driver_name[] = "MOSCHIP usb-ethernet 
driver";
 
 static int mcs7830_get_reg(struct usbnet *dev, u16 index, u16 size, void *data)
 {
-   struct usb_device *xdev = dev->udev;
-   int ret;
-   void *buffer;
-
-   buffer = kmalloc(size, GFP_NOIO);
-   if (buffer == NULL)
-   return -ENOMEM;
-
-   ret = usb_control_msg(xdev, usb_rcvctrlpipe(xdev, 0), MCS7830_RD_BREQ,
- MCS7830_RD_BMREQ, 0x, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
-   memcpy(data, buffer, size);
-   kfree(buffer);
-
-   return ret;
+   return usbnet_read_cmd(dev, MCS7830_RD_BREQ, MCS7830_RD_BMREQ,
+   0x, index, data, size);
 }
 
 static int mcs7830_set_reg(struct usbnet *dev, u16 index, u16 size, const void 
*data)
 {
-   struct usb_device *xdev = dev->udev;
-   int ret;
-   void *buffer;
-
-   buffer = kmemdup(data, size, GFP_NOIO);
-   if (buffer == NULL)
-   return -ENOMEM;
-
-   ret = usb_control_msg(xdev, usb_sndctrlpipe(xdev, 0), MCS7830_WR_BREQ,
- MCS7830_WR_BMREQ, 0x, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
-   kfree(buffer);
-   return ret;
-}
-
-static void mcs7830_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "%s() failed with %d\n",
-  __func__, status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+   0x, index, data, size);
 }
 
 static void mcs7830_set_reg_async(struct usbnet *dev, u16 index, u16 size, 
void *data)
 {
-   struct usb_ctrlrequest *req;
-   int ret;
-   struct urb *urb;
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   dev_dbg(&dev->udev->dev,
-   "Error allocating URB in write_cmd_async!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof *req, GFP_ATOMIC);
-   if (!req) {
-   dev_err(&dev->udev->dev,
-   "Failed to allocate memory for control request\n");
-   goto out;
-   }
-   req->bRequestType = MCS7830_WR_BMREQ;
-   req->bRequest = MCS7830_WR_BREQ;
-   req->wValue = 0;
-   req->wIndex = cpu_to_le16(index);
-   req->wLength = cpu_to_le16(size);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, size,
-mcs7830_async_cmd_callback, req);
-
-   ret = usb_submit_urb(urb, GFP_ATOMIC);
-   if (ret < 0) {
-   dev_err(&dev->udev->dev,
-   "Error submitting the control message: ret=%d\n", ret);
-   goto out;
-   }
-   return;
-out:
-   kfree(req);
-   usb_free_urb(urb);
+   usbnet_write_cmd_async(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+   0x, index, data, size);
 }
 
 static int mcs7830_hif_get_mac_address(struct usbnet *dev, unsigned char *addr)
-- 
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 v1 07/11] usbnet: net1080: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/net1080.c |  110 +
 1 file changed, 30 insertions(+), 80 deletions(-)

diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index c062a3e..93e0716 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -109,13 +109,11 @@ struct nc_trailer {
 static int
 nc_vendor_read(struct usbnet *dev, u8 req, u8 regnum, u16 *retval_ptr)
 {
-   int status = usb_control_msg(dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   0, regnum,
-   retval_ptr, sizeof *retval_ptr,
-   USB_CTRL_GET_TIMEOUT);
+   int status = usbnet_read_cmd(dev, req,
+USB_DIR_IN | USB_TYPE_VENDOR |
+USB_RECIP_DEVICE,
+0, regnum, retval_ptr,
+sizeof *retval_ptr);
if (status > 0)
status = 0;
if (!status)
@@ -133,13 +131,9 @@ nc_register_read(struct usbnet *dev, u8 regnum, u16 
*retval_ptr)
 static void
 nc_vendor_write(struct usbnet *dev, u8 req, u8 regnum, u16 value)
 {
-   usb_control_msg(dev->udev,
-   usb_sndctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value, regnum,
-   NULL, 0,// data is in setup packet
-   USB_CTRL_SET_TIMEOUT);
+   usbnet_write_cmd(dev, req,
+USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+value, regnum, NULL, 0);
 }
 
 static inline void
@@ -288,37 +282,34 @@ static inline void nc_dump_ttl(struct usbnet *dev, u16 
ttl)
 static int net1080_reset(struct usbnet *dev)
 {
u16 usbctl, status, ttl;
-   u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+   u16 vp;
int retval;
 
-   if (!vp)
-   return -ENOMEM;
-
// nc_dump_registers(dev);
 
-   if ((retval = nc_register_read(dev, REG_STATUS, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_STATUS, &vp)) < 0) {
netdev_dbg(dev->net, "can't read %s-%s status: %d\n",
   dev->udev->bus->bus_name, dev->udev->devpath, 
retval);
goto done;
}
-   status = *vp;
+   status = vp;
nc_dump_status(dev, status);
 
-   if ((retval = nc_register_read(dev, REG_USBCTL, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_USBCTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read USBCTL, %d\n", retval);
goto done;
}
-   usbctl = *vp;
+   usbctl = vp;
nc_dump_usbctl(dev, usbctl);
 
nc_register_write(dev, REG_USBCTL,
USBCTL_FLUSH_THIS | USBCTL_FLUSH_OTHER);
 
-   if ((retval = nc_register_read(dev, REG_TTL, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_TTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read TTL, %d\n", retval);
goto done;
}
-   ttl = *vp;
+   ttl = vp;
// nc_dump_ttl(dev, ttl);
 
nc_register_write(dev, REG_TTL,
@@ -331,7 +322,6 @@ static int net1080_reset(struct usbnet *dev)
retval = 0;
 
 done:
-   kfree(vp);
return retval;
 }
 
@@ -339,13 +329,10 @@ static int net1080_check_connect(struct usbnet *dev)
 {
int retval;
u16 status;
-   u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+   u16 vp;
 
-   if (!vp)
-   return -ENOMEM;
-   retval = nc_register_read(dev, REG_STATUS, vp);
-   status = *vp;
-   kfree(vp);
+   retval = nc_register_read(dev, REG_STATUS, &vp);
+   status = vp;
if (retval != 0) {
netdev_dbg(dev->net, "net1080_check_conn read - %d\n", retval);
return retval;
@@ -355,59 +342,22 @@ static int net1080_check_connect(struct usbnet *dev)
return 0;
 }
 
-static void nc_flush_complete(struct urb *urb)
-{
-   kfree(urb->context);
-   usb_free_urb(urb);
-}
-
 static void nc_ensure_sync(struct usbnet *dev)
 {
-   dev->frame_errors++;
-   if (dev->frame_errors > 5) {
-   struct urb  *urb;
-   struct usb_ctrlrequest  *req;
-   int status;
-
-   /* Send a flush */
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb)
-   return;
-
-   req = kmalloc(sizeof *req, GFP_ATOMIC);
-   if (!req) {
-   usb_free_urb(urb);
-   return;
-   }
+   if (++dev->frame_errors <= 5)
+   return;
 
- 

[PATCH v1 08/11] usbnet: plusb: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/plusb.c |   11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 4584b9a..0fcc8e6 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -71,13 +71,10 @@
 static inline int
 pl_vendor_req(struct usbnet *dev, u8 req, u8 val, u8 index)
 {
-   return usb_control_msg(dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   val, index,
-   NULL, 0,
-   USB_CTRL_GET_TIMEOUT);
+   return usbnet_read_cmd(dev, req,
+   USB_DIR_IN | USB_TYPE_VENDOR |
+   USB_RECIP_DEVICE,
+   val, index, NULL, 0);
 }
 
 static inline int
-- 
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 v1 09/11] usbnet: sierra_net: apply introduced usb command APIs

2012-10-11 Thread Ming Lei

Signed-off-by: Ming Lei 
---
 drivers/net/usb/sierra_net.c |   45 --
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index c27d277..eb5c7a8 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -311,10 +311,9 @@ static int sierra_net_send_cmd(struct usbnet *dev,
struct sierra_net_data *priv = sierra_net_get_private(dev);
int  status;
 
-   status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_CDC_SEND_ENCAPSULATED_COMMAND,
-   USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE, 0,
-   priv->ifnum, cmd, cmdlen, USB_CTRL_SET_TIMEOUT);
+   status = usbnet_write_cmd(dev, USB_CDC_SEND_ENCAPSULATED_COMMAND,
+ 
USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
+ 0, priv->ifnum, cmd, cmdlen);
 
if (status != cmdlen && status != -ENODEV)
netdev_err(dev->net, "Submit %s failed %d\n", cmd_name, status);
@@ -632,32 +631,22 @@ static int sierra_net_change_mtu(struct net_device *net, 
int new_mtu)
 static int sierra_net_get_fw_attr(struct usbnet *dev, u16 *datap)
 {
int result = 0;
-   u16 *attrdata;
-
-   attrdata = kmalloc(sizeof(*attrdata), GFP_KERNEL);
-   if (!attrdata)
-   return -ENOMEM;
-
-   result = usb_control_msg(
-   dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   /* _u8 vendor specific request */
-   SWI_USB_REQUEST_GET_FW_ATTR,
-   USB_DIR_IN | USB_TYPE_VENDOR,   /* __u8 request type */
-   0x, /* __u16 value not used */
-   0x, /* __u16 index  not used */
-   attrdata,   /* char *data */
-   sizeof(*attrdata),  /* __u16 size */
-   USB_CTRL_SET_TIMEOUT);  /* int timeout */
-
-   if (result < 0) {
-   kfree(attrdata);
+   u16 attrdata;
+
+   result = usbnet_read_cmd(dev,
+   /* _u8 vendor specific request */
+   SWI_USB_REQUEST_GET_FW_ATTR,
+   USB_DIR_IN | USB_TYPE_VENDOR,   /* __u8 request 
type */
+   0x, /* __u16 value not used */
+   0x, /* __u16 index  not used */
+   &attrdata,  /* char *data */
+   sizeof(attrdata)/* __u16 size */
+   );
+
+   if (result < 0)
return -EIO;
-   }
-
-   *datap = le16_to_cpu(*attrdata);
 
-   kfree(attrdata);
+   *datap = le16_to_cpu(attrdata);
return result;
 }
 
-- 
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 v1 10/11] usbnet: smsc75xx: apply introduced usb command APIs

2012-10-11 Thread Ming Lei
Signed-off-by: Ming Lei 
---
 drivers/net/usb/smsc75xx.c |   39 ++-
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index b77ae76..1baa53a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -85,26 +85,21 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx 
transaction");
 static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
  u32 *data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_READ_REGISTER,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+   ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to read reg index 0x%08x: %d", index, ret);
 
-   le32_to_cpus(buf);
-   *data = *buf;
-   kfree(buf);
+   le32_to_cpus(&buf);
+   *data = buf;
 
return ret;
 }
@@ -112,28 +107,22 @@ static int __must_check smsc75xx_read_reg(struct usbnet 
*dev, u32 index,
 static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
   u32 data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   *buf = data;
-   cpu_to_le32s(buf);
-
-   ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_WRITE_REGISTER,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+   buf = data;
+   cpu_to_le32s(&buf);
 
+   ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+  USB_DIR_OUT | USB_TYPE_VENDOR |
+  USB_RECIP_DEVICE,
+  0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to write reg index 0x%08x: %d", index, ret);
 
-   kfree(buf);
-
return ret;
 }
 
-- 
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 v1 11/11] usbnet: smsc95xx: apply introduced usb command APIs

2012-10-11 Thread Ming Lei
Signed-off-by: Ming Lei 
---
 drivers/net/usb/smsc95xx.c |  115 +++-
 1 file changed, 27 insertions(+), 88 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 7479a57..1730f75 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -65,11 +65,6 @@ struct smsc95xx_priv {
spinlock_t mac_cr_lock;
 };
 
-struct usb_context {
-   struct usb_ctrlrequest req;
-   struct usbnet *dev;
-};
-
 static bool turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -77,25 +72,20 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx 
transaction");
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
  u32 *data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_READ_REGISTER,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+   ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to read register index 0x%08x\n", 
index);
 
-   le32_to_cpus(buf);
-   *data = *buf;
-   kfree(buf);
+   le32_to_cpus(&buf);
+   *data = buf;
 
return ret;
 }
@@ -103,27 +93,22 @@ static int __must_check smsc95xx_read_reg(struct usbnet 
*dev, u32 index,
 static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
   u32 data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   *buf = data;
-   cpu_to_le32s(buf);
+   buf = data;
+   cpu_to_le32s(&buf);
 
-   ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_WRITE_REGISTER,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
 
+   ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+  USB_DIR_OUT | USB_TYPE_VENDOR |
+  USB_RECIP_DEVICE,
+  0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to write register index 
0x%08x\n", index);
 
-   kfree(buf);
-
return ret;
 }
 
@@ -132,11 +117,8 @@ static int smsc95xx_set_feature(struct usbnet *dev, u32 
feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
 
-   cpu_to_le32s(&feature);
-
-   return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   return usbnet_write_cmd(dev, USB_REQ_SET_FEATURE,
+   USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -144,11 +126,8 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 
feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
 
-   cpu_to_le32s(&feature);
-
-   return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   return usbnet_write_cmd(dev, USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 /* Loop until the read is completed with timeout
@@ -350,60 +329,20 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 
offset, u32 length,
return 0;
 }
 
-static void smsc95xx_async_cmd_callback(struct urb *urb)
-{
-   struct usb_context *usb_context = urb->context;
-   struct usbnet *dev = usb_context->dev;
-   int status = urb->status;
-
-   check_warn(status, "async callback failed with %d\n", status);
-
-   kfree(usb_context);
-   usb_free_urb(urb);
-}
-
 static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
 u32 *data)
 {
-   struct usb_context *usb_context;
-   int status;
-   struct urb *urb;
const u16 size = 4;
+   int ret;
 
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_warn(dev->net, "Error allocating URB\n");
-   return -ENOMEM;
-   }
-
-   usb_context = kmalloc(sizeof(struct usb_con

Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Oliver Neukum wrote:

> On Thursday 11 October 2012 11:18:22 Ming Lei wrote:
> > On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum  wrote:
> > >
> > > No, the problem is autoresume.
> > >
> > > Suppose we have a device with two interface. Interface A be usbnet; 
> > > interface B
> > > something you page on. Now consider that you can only resume both 
> > > interfaces
> > > and this is (and needs to be) done synchronously.
> > >
> > > Now we can have this code path:
> > >
> > > autoresume of device -> resume() -> kmalloc(..., GFP_KERNEL) ->
> > > VM layer decides to start paging out -> IO to interface B -> autoresume 
> > > of device
> > > --> DEADLOCK
> > 
> > Currently scsi disk can only be runtime suspended when the device is not
> > opened, so are you sure that the paging out above can cause IO on a suspend
> > usb mass storage disk which is not mounted or opened by utility now?
> 
> We definitely do not wish to keep it that way. People at Intel are currently 
> working
> on better power management for sd, which would allow full autosuspend.

It's worse than you may realize.  When a SCSI disk is suspended, all of
its ancestor devices may be suspended too.  Pages can't be read in from
the drive until all those ancestors are resumed.  This means that all
runtime resume code paths for all drivers that could be bound to an
ancestor of a block device must avoid GFP_KERNEL.  In practice it's
probably easiest for the runtime PM core to use tsk_set_allowd_gfp()
before calling any runtime_resume method.

Or at least, this will be true when sd supports nontrivial autosuspend.

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: Linux Kernel: NP dereference on removing usb device

2012-10-11 Thread Alan Stern
On Fri, 12 Oct 2012, L N Wirz wrote:

> Dear Alan,
> 
> I found your address on the lkml when I looked for someone who recently
> worked on the usb subsystem.

Messages like this should always be CC'ed to the linux-usb mailing 
list.

> I've encountered a reproducible kernel oops, when I remove a (Garmin)
> GPS-device that is bound to /dev/ttyUSB0.  Removing can either be
> physical unplugging or telling the gps-device to switch to be a mass
> storage device.  This oops doesn't crash the whole machine, but
> /dev/ttyUSB) becomes unavailable, and suspending to ram leads to a crash.

Your problem is fixed by this patch, which was submitted yesterday:

http://marc.info/?l=linux-usb&m=134989262623097&w=2

Alan Stern

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


Re: usb audio device can't work when it is attached to host via usb 2.0 hub

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Elric Fu wrote:

> Hi all,
> 
> I recently found a strange issue on a embedded platform and don't know
> how to fix it. Please give me some suggestions.
> 
> The platform is ar9331. The issue is if we attach a usb audio device to
> the host on ar9331 directly or via a usb 1.1 hub the usb audio works fine,
> but if we attach it via usb 2.0 hub the undermentioned message will be
> reported. Moreover, when the usb audio was attached to the host on X86
> via usb 2.0 hub it worked fine too.
> 
> ~ # usb 1-1.3: new full speed USB device using ar7240-ehci and address 3
> usb 1-1.3: unable to read config index 0 descriptor/start: -145
> usb 1-1.3: chopping to 0 config(s)
> usb 1-1.3: string descriptor 0 read error: -145
> usb 1-1.3: New USB device found, idVendor=040d, idProduct=3400
> usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 1-1.3: no configuration chosen from 0 choices
> 
> I tracked the issue and found the usb audio had a strange issue. It is a
> usb 1.1 device and works at full speed. But the interesting thing is the
> bcdUSB field of device descriptor is 0x0200.

That means it is a USB-2.0 device, not USB-1.1.

>  So the
> hub_port_connect_change() will involke the check_highspeed() which
> would get device qualifier descriptor.
> 
> The correct case is the device doesn't have the device qualifier descriptor

That's okay.  It means the device doesn't support high-speed operation, 
only full speed.

> and usb_get_descriptor() will retry 3 times and return -32. But now
> usb_get_descriptor() will return -145 at the second time. Then when the

What is error -145 on your platform?  Is it -ETIMEDOUT?

> usb core send a get configuration descriptor request, but the control transfer
> will be timeout again.
> 
> I used the usb analyzer to capture the trace and found host just sent one
> get-device-qualifier-descriptor control transfer and the device return a
> STALL. But at the second time, actually, the host even didn't send the setup
> packet. I don't know why.

Since the transfer uses aplit transactions, the host doesn't send any 
SETUP packets at all.  The intervening hub sends them.  Maybe there's 
something wrong with the hub.

Can you use your analyzer to capture the traffic between the computer 
and the hub, rather than between the hub and the device?

> My opinion is the split transactions to the device which has a 0x0200 bcdUSB
> but is actually a full speed device have some issues. I would
> appreciate any ideas.

What happens if you use both the device and the same hub with a regular 
PC?

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


usb audio race at disconnect time

2012-10-11 Thread Matthieu CASTET
Hi,

while doing some monkey tests on a product we found races in usb audio code when
the device in unplugged from usb (on linus master tree).

This can be reproduced with usb_audio_show_race.diff and CONFIG_DEBUG_SLAB.
With this patch, start a stream :
# arecord -D hw:0 -r 44100 -c 2  -f S16_LE > /dev/null
you will see the kernel log : "in snd_usb_hw_params sleeping"
Unplug the device before "in snd_usb_hw_params sleeping exit", and you will see
an oops in snd_pcm_hw_params


Instead of using CONFIG_DEBUG_SLAB, usb_audio_show_use_after_free.diff can show
use after free by setting usb_device pointer to NULL in
snd_usb_audio_disconnect. [1]


In order to protect from all the races, before using any usb_device we need to
check if it is not freed.

What's the best way to do that ?

A trival fix would be to take a mutex in all snd_pcm_ops callback that access 
usb :

{
mutex_lock(&chip->shutdown_mutex);
if (!chip->dev) {
mutex_unlock(&chip->shutdown_mutex);
return -ENODEV;
}
[...]
mutex_unlock(&chip->shutdown_mutex);
}


But that will serialize all snd_pcm_ops callbacks.

Another solution could be to use refcounting or rwlock patern :
- snd_pcm_ops callbacks call rdlock_trylock/rdlock_unlock
- usb_driver disconnect callback take rwlock_lock and never release it

This will make "usb_driver disconnect" wait that all "snd_pcm_ops callback" are
finished and abort all future call to "snd_pcm_ops callback".

I believe similar problems exist in other drivers, (we saw another one  in
hidraw [2]), so it could be nice to find a generic pattern to help driver to
manage correctly disconnection.

Are there any recommended pattern ?


Do you have any comment ?

Thanks,

Matthieu


[1]

[ 1366.315307] usb 1-2.1.3: new full-speed USB device number 4 using ehci-omap
[ 1366.556579] input: USB Audio as /devices/platform/usbhs_omap/ehci-omap.0/usb1
/1-2/1-2.1/1-2.1.3/1-2.1.3:1.3/input/input1
[ 1366.604553] hid-generic 0003:06F8:C000.0001: input,hidraw0: USB HID v1.00 Dev
ice [USB Audio] on usb-ehci-omap.0-2.1.3/input3

#
#
# arecord -D hw:0 -r 44100 -c 2  -f S16_LE > /dev/null
Recording WAVE '[ 1368.937774] in snd_usb_hw_params sleeping
stdin' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo
Warning: rate is not accurate (requested = 44100Hz, got = 48000Hz)
 please, try the plug plugin
[ 1372.385375] hid-generic 0003:06F8:C000.0001: can't reset device, ehci-omap.0-
2.1.3/input3, status -71
[ 1372.439514] usb 1-2.1.3: USB disconnect, device number 4
[ 1372.449005] hid-generic 0003:06F8:C000.0001: can't reset device, ehci-omap.0-
2.1.3/input3, status -71
[ 1378.947235] in snd_usb_hw_params sleeping exit
[ 1378.952301] Unable to handle kernel NULL pointer dereference at virtual addre
ss 029c
[ 1378.961029] pgd = c31e8000
[ 1378.964019] [029c] *pgd=8e885831, *pte=, *ppte=
[ 1378.970825] Internal error: Oops: 17 [#1] SMP ARM
[ 1378.975799] Modules linked in:
[ 1378.979064] CPU: 0Not tainted  (3.6.0-03889-ge8dc7a6-dirty #5)
[ 1378.985595] PC is at usb_ifnum_to_if+0xc/0xd4
[ 1378.990234] LR is at snd_usb_hw_params+0x1bc/0x764
[ 1378.995300] pc : []lr : []psr: 0013
[ 1378.995300] sp : cf3fddf0  ip : cf3fde08  fp : cf3fde04
[ 1379.007415] r10: bb80  r9 : 0001  r8 : 
[ 1379.012908] r7 : cf972f40  r6 : cf3e1f14  r5 : ce8ad000  r4 : cf3e1eb0
[ 1379.019805] r3 : 0004  r2 : 0005  r1 : 0002  r0 : 
[ 1379.026702] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[ 1379.034240] Control: 10c5387d  Table: 831e8019  DAC: 0015
[ 1379.040313] Process arecord (pid: 636, stack limit = 0xcf3fc2f8)
[ 1379.046661] Stack: (0xcf3fddf0 to 0xcf3fe000)
[ 1379.051269] dde0: cf3e1eb0 ce8ad000 cf3fd
e5c cf3fde08
[ 1379.059906] de00: c030816c c0275338 cf3fde3c cf3fde18 c02f5d68 c02f59e4 c05b4
c60 ce8ad000
[ 1379.068542] de20: cf3e1800 0001 0004 0001 cf3fde5c cf3e1800 ce8ad
800 c05b8310
[ 1379.077178] de40: ce8ad000 c0014728 cf3fc000  cf3fde7c cf3fde60 c02f0
b8c c0307fbc
[ 1379.085815] de60: cf3e1800 be91b890 be91b890 ce8ad000 cf3fdec4 cf3fde80 c02f1
778 c02f0ad4
[ 1379.094451] de80: cf3fc000 6013 cf97b540 0002 c0126484  cf3fd
ed4 cf3e1800
[ 1379.103088] dea0: be91b890 cf3e5078 c0045877 c0014728 cf3fc000  cf3fd
efc cf3fdec8
[ 1379.111755] dec0: c02f2150 c02f1228 cf3fdf3c cf3fded8 c0126634 c0068ef0 0
001 
[ 1379.120391] dee0: cf87f280 be91b890 cf3e5078 c0045877 cf3fdf0c cf3fdf00 c02f2
5dc c02f2124
[ 1379.129028] df00: cf3fdf7c cf3fdf10 c00fd6b4 c02f25ac c03d7328 cf8b49c8 0
026 
[ 1379.137664] df20: cd673738 0002 cf3fc000  cf3fdf6c cf3fdf40 c00ec
138 c0126420
[ 1379.146301] df40:   0006 cf8b49c0 be919140  cf87f
280 be91b890
[ 1379.154907] df60: c25c4111 0004 c0014728 cf3fc000 cf3fdfa4 cf3fdf80 c00fd
c6c c00fd638
[ 1379.163543] df80: c00183f0  00028288 b6fc68a0 00028238 0036 0
000 cf3fd

Re: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Gian-Carlo Pascutto wrote:

> I noticed that sometimes when booting the USB subsystem already behaves
> erratically. For example this morning I'm greeted by:
> 
> [  234.455604] ohci_hcd :00:04.0: urb 8801296fac80 path 4 ep1in
> 9312 cc 9 --> status -121
> [  234.466567] ohci_hcd :00:04.0: urb 88010ce362c0 path 4 ep1in
> 9212 cc 9 --> status -121
> 
> which keeps repeating infinitely.
> 
> I've also seen:
> 
> Oct  8 08:52:37 denebix kernel: [  358.482780] ohci_hcd :00:04.0:
> GetStatus roothub.portstatus [3] = 0x00130301 PRSC PESC CSC LSDA PPS CCS
> Oct  8 08:52:37 denebix kernel: [  358.482808] hub 4-0:1.0: unable to
> enumerate USB device on port 4
> Oct  8 08:52:37 denebix kernel: [  358.482823] hub 4-0:1.0: state 7
> ports 6 chg  evt 0010
> Oct  8 08:52:37 denebix kernel: [  358.482837] ohci_hcd :00:04.0:
> GetStatus roothub.portstatus [3] = 0x00030301 PESC CSC LSDA PPS CCS
> Oct  8 08:52:37 denebix kernel: [  358.482851] hub 4-0:1.0: port 4,
> status 0301, change 0003, 1.5 Mb/s
> 
> which also repeats infinitely.
> 
> Maybe my hardware/chipset is just crap?

I don't know, maybe.  Maybe that's just a bad cable connection (this 
includes the possibility of bad connections inside the PC case).

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: USB errors, ehci testing modes

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Tim Sander wrote:

> Hi
> 
> I have some stange errors on a arm pcm043 with internal usb phy. The log is 
> attached at the end of the mail. It would be nice if someone could give me a 
> pointer whats going wrong?. It would also be nice to know where the status 
> flags which are output by the usb debug messages are documented?

Which status flags are you referring to?  I'll explain the ones at the 
end of the log.

> As it might be a hw problem i would like to activate the testmodes of the 
> ehci 
> controller. The old Freescale BSP driver had a file in debugfs where enabling 
> this stuff was easy. Is there also an easy way to enable the ehci debug 
> facility from linux usermode? 

No.  You will have to write a program to send a 
Set-Port-Feature(PORT_TEST) request to the root hub.  Or patch the 
kernel to add a debugfs file like the BSP driver had.

> hub 1-0:1.0: state 7 ports 1 chg  evt 0002
> mxc-ehci mxc-ehci.0: GetStatus port:1 status c00100a 6  ACK POWER sig=se0 PEC
> CSC
> hub 1-0:1.0: port 1, status 0100, change 0003, 12 Mb/s
> usb 1-1: USB disconnect, device number 23
> usb 1-1: unregistering device
> usb 1-1: unregistering interface 1-1:1.0
> usb-storage: storage_disconnect() called
> usb-storage: -- usb_stor_release_resources
> usb-storage: -- sending exit command to thread
> usb-storage: *** thread awakened.
> usb-storage: -- exiting
> usb-storage: -- dissociate_dev
> usb 1-1: usb_disable_device nuking all URBs

These messages mean there was a disconnect.  Assuming you didn't unplug 
the cable, some signal made the controller or phy think it was 
unplugged.

Possibly the event was connected with Link Power Management.  You might
want to try disabling LPM for that port (write "disable 1" to the "lpm"
file in the EHCI debugging directory).

> mxc-ehci mxc-ehci.0: GetStatus port:1 status 001803 0  ACK POWER sig=j CSC 
> CONNECT
> hub 1-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101
> mxc-ehci mxc-ehci.0: port 1 high speed
> mxc-ehci mxc-ehci.0: GetStatus port:1 status 8001205 4  ACK POWER sig=se0 LPM 
> PE CONNECT
> usb 1-1: new high-speed USB device number 24 using mxc-ehci
> mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 1
...
> mxc-ehci mxc-ehci.0: devpath 1 ep0in 3strikes

These messages mean that there was a low-level communication error.  
The controller did not receive a response from the device.

> mxc-ehci mxc-ehci.0: GetStatus port:1 status c00100a 6  ACK POWER sig=se0 PEC 
> CSC

The "c00100a" bits are explained in Table 2-16 of the EHCI spec and the
EHCI-1.1 addendum.  The other fields are merely symbolic names for the bits
in the status value.

> hub 1-0:1.0: unable to enumerate USB device on port 1
> hub 1-0:1.0: state 7 ports 1 chg  evt 0002

"state 7" means the root hub is in the Configured state.  "evt 0002" 
means an event has occurred on port 1.  These values are not documented 
anywhere because they are used only for debugging the kernel.

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: usb audio race at disconnect time

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Matthieu CASTET wrote:

> Hi,
> 
> while doing some monkey tests on a product we found races in usb audio code 
> when
> the device in unplugged from usb (on linus master tree).
> 
> This can be reproduced with usb_audio_show_race.diff and CONFIG_DEBUG_SLAB.
> With this patch, start a stream :
> # arecord -D hw:0 -r 44100 -c 2  -f S16_LE > /dev/null
> you will see the kernel log : "in snd_usb_hw_params sleeping"
> Unplug the device before "in snd_usb_hw_params sleeping exit", and you will 
> see
> an oops in snd_pcm_hw_params
> 
> 
> Instead of using CONFIG_DEBUG_SLAB, usb_audio_show_use_after_free.diff can 
> show
> use after free by setting usb_device pointer to NULL in
> snd_usb_audio_disconnect. [1]
> 
> 
> In order to protect from all the races, before using any usb_device we need to
> check if it is not freed.

This should never be a problem.  Take a reference to the interface 
(usb_get_intf) and drop the reference (usb_put_intf) when you are 
finished accessing the usb_device structure.  This will prevent the 
usb_device from being deallocated while it is in use.

More important is a restriction you did not mention: Drivers are not 
allowed to communicate with a USB device after their disconnect routine 
has returned.

> What's the best way to do that ?

You could protect all occurrences of usb_submit_urb() with a spinlock.  
While holding the lock, check a flag to see if the disconnect routine
has been called.

In the disconnect routine, set this flag while holding the spinlock and 
then kill all the outstanding URBs.

I'm not very familiar with the snd-usb-audio driver; maybe it already 
does some of these things.

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: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-10-11 Thread Gian-Carlo Pascutto
On 11/10/12 17:19, Alan Stern wrote:

>> Maybe my hardware/chipset is just crap?
> 
> I don't know, maybe.  Maybe that's just a bad cable connection (this 
> includes the possibility of bad connections inside the PC case).

Ouch. I can check a few things, like the USB card reader. It should be
possible to correlate the ohci_hcd :00:04.0 with a header?

I just got the following error, with nothing connected (well, at least
not the headset or camera). So maybe they're not directly responsible,
except for putting more stress on the USB system.

[ 6036.188360] ohci_hcd :00:04.0: urb 88012567be00 path 4 ep1in
9212 cc 9 --> status -121
[ 6221.192777] INFO: task scsi_eh_8:675 blocked for more than 120 seconds.
[ 6221.192786] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 6221.192791] scsi_eh_8   D 88012fc92600 0   675  2
0x
[ 6221.192802]  88012881d4b0 0046 88012aeca790

[ 6221.192813]  00012600 880128951fd8 880128951fd8
88012881d4b0
[ 6221.192821]   7fff 7fff
8801279f7800
[ 6221.192829] Call Trace:
[ 6221.192845]  [] ? schedule_timeout+0x1c/0xdb
[ 6221.192882]  [] ? start_ed_unlink+0x13/0x54 [ohci_hcd]
[ 6221.192898]  [] ? ohci_urb_dequeue+0x85/0x95 [ohci_hcd]
[ 6221.192908]  [] ? should_resched+0x5/0x23
[ 6221.192916]  [] ? _cond_resched+0x6/0x1b
[ 6221.192924]  [] ? wait_for_common+0x9e/0x117
[ 6221.192931]  [] ? try_to_wake_up+0x199/0x199
[ 6221.192942]  [] ? command_abort+0x6b/0x74 [usb_storage]
[ 6221.192964]  [] ? scsi_error_handler+0x2d5/0x5c5
[scsi_mod]
[ 6221.192983]  [] ? scsi_eh_get_sense+0x172/0x172
[scsi_mod]
[ 6221.193001]  [] ? scsi_eh_get_sense+0x172/0x172
[scsi_mod]
[ 6221.193008]  [] ? kthread+0x67/0x6f
[ 6221.193018]  [] ? kernel_thread_helper+0x4/0x10
[ 6221.193025]  [] ?
kthread_freezable_should_stop+0x3c/0x3c
[ 6221.193033]  [] ? gs_change+0xb/0xb
[ 6221.193071] INFO: task udisks-daemon:3773 blocked for more than 120
seconds.
[ 6221.193075] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 6221.193078] udisks-daemon   D 88012fc12600 0  3773   3758
0x
[ 6221.193087]  880128bacde0 0082 88012954e8d0
810547ec
[ 6221.193095]  00012600 8801269f7fd8 8801269f7fd8
880128bacde0
[ 6221.193103]  88012fc12670 7fff 7fff
8801269f7b40
[ 6221.193110] Call Trace:
[ 6221.193120]  [] ?
wakeup_preempt_entity.isra.40+0x18/0x28
[ 6221.193128]  [] ? schedule_timeout+0x1c/0xdb
[ 6221.193136]  [] ? ttwu_do_wakeup+0x24/0xc8
[ 6221.193143]  [] ? should_resched+0x5/0x23
[ 6221.193150]  [] ? _cond_resched+0x6/0x1b
[ 6221.193157]  [] ? wait_for_common+0x9e/0x117
[ 6221.193163]  [] ? try_to_wake_up+0x199/0x199
[ 6221.193172]  [] ? flush_work+0x21/0x2a
[ 6221.193178]  [] ? hweight_long+0x6/0x6
[ 6221.193187]  [] ? disk_clear_events+0x82/0xd9
[ 6221.193200]  [] ? check_disk_change+0x1f/0x4f
[ 6221.193214]  [] ? sd_open+0xe5/0x18e [sd_mod]
[ 6221.193220]  [] ? __blkdev_get+0xf1/0x3cd
[ 6221.193229]  [] ? blkdev_get+0x2d3/0x2d3
[ 6221.193235]  [] ? blkdev_get+0x18b/0x2d3
[ 6221.193242]  [] ? __d_lookup_rcu+0x99/0xc0
[ 6221.193248]  [] ? blkdev_get+0x2d3/0x2d3
[ 6221.193255]  [] ? do_dentry_open+0x172/0x20e
[ 6221.193261]  [] ? finish_open+0x2a/0x33
[ 6221.193268]  [] ? do_last+0x85b/0xa03
[ 6221.193275]  [] ? __inode_permission+0x57/0x95
[ 6221.193283]  [] ? path_openat+0xc0/0x33a
[ 6221.193291]  [] ? do_filp_open+0x2a/0x6e
[ 6221.193299]  [] ? alloc_fd+0xda/0xec
[ 6221.193306]  [] ? do_sys_open+0x5e/0xe2
[ 6221.193314]  [] ? system_call_fastpath+0x16/0x1b
[ 6340.817581] INFO: task scsi_eh_8:675 blocked for more than 120 seconds.
[ 6340.817590] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 6340.817596] scsi_eh_8   D 88012fc92600 0   675  2
0x
[ 6340.817606]  88012881d4b0 0046 88012aeca790

[ 6340.817617]  00012600 880128951fd8 880128951fd8
88012881d4b0
[ 6340.817625]   7fff 7fff
8801279f7800
[ 6340.817633] Call Trace:
[ 6340.817649]  [] ? schedule_timeout+0x1c/0xdb
[ 6340.817686]  [] ? start_ed_unlink+0x13/0x54 [ohci_hcd]
[ 6340.817701]  [] ? ohci_urb_dequeue+0x85/0x95 [ohci_hcd]
[ 6340.817711]  [] ? should_resched+0x5/0x23
[ 6340.817719]  [] ? _cond_resched+0x6/0x1b
[ 6340.817726]  [] ? wait_for_common+0x9e/0x117
[ 6340.817734]  [] ? try_to_wake_up+0x199/0x199
[ 6340.817745]  [] ? command_abort+0x6b/0x74 [usb_storage]
[ 6340.817766]  [] ? scsi_error_handler+0x2d5/0x5c5
[scsi_mod]
[ 6340.817786]  [] ? scsi_eh_get_sense+0x172/0x172
[scsi_mod]
[ 6340.817804]  [] ? scsi_eh_get_sense+0x172/0x172
[scsi_mod]
[ 6340.817811]  [] ? kthread+0x67/0x6f
[ 6340.817820]  [] ? kernel_thread_helper+0x4/0x10
[ 6340.817828]  [] ?
kthread_freezable_should_stop+0x3c/0x3c
[ 6340.817836]  [] ? gs_change+0xb/0xb

[PATCH] kaweth: print correct debug ptr

2012-10-11 Thread Alan Cox
From: Alan Cox 

We nowdays copy the buffer and free fw->data, so make the debug printk use
the right thing.

Signed-off-by: Alan Cox 
---

 drivers/net/usb/kaweth.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index c75e11e..afb117c 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -424,7 +424,7 @@ static int kaweth_download_firmware(struct kaweth_device 
*kaweth,
 
netdev_dbg(kaweth->net,
   "Downloading firmware at %p to kaweth device at %p\n",
-  fw->data, kaweth);
+  kaweth->firmware_buf, kaweth);
netdev_dbg(kaweth->net, "Firmware length: %d\n", data_len);
 
return kaweth_control(kaweth,

--
To unsubscribe from this list: send the line "unsubscribe 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] Enable USB peripheral mode on dm365 EVM

2012-10-11 Thread Constantine Shulyupin
From: Constantine Shulyupin 

Sets USB PHY clock source to 24 MHz clock.

Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC.

To active the patch need to call davinci_setup_usb from dm365_evm_init

References:

Definition of USB_PHY_CTRL and PHYCLKFREQ:
- http://www.makelinux.com/lib/ti/DM36x_ARM/doc-141

Original patch by miguel.agui...@ridgerun.com three years ago:
- 
http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html

Signed-off-by: Constantine Shulyupin 
---

Note:

Changelog

Changes since v6
- moved call to davinci_setup_usb from dm365_evm_init to another patch 
accordinly request of Sergei
 
Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html
accordingy feedback of nsek...@ti.com 
http://www.spinics.net/lists/kernel/msg1414914.html
- phy configuration moved to drivers/usb/musb/davinci.c
- USB_OTG configuration is submitted in separated patch: 
http://www.spinics.net/lists/kernel/msg1414964.html
- Setting current limit to 1000 mA. Any way the current is limited to 510 mA in 
davinci_setup_usb.

Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html
- removed fix of dev_info in musb_init_controller

Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
- removed optional altering of pr_info

Changes since v1  http://marc.info/?l=linux-kernel&m=130894150803661&w=2:
- removed optional code and reordered
- removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is 
not need for peripheral USB

This patch is based on code from projects Arago, Angstom and RidgeRun.

---
 drivers/usb/musb/davinci.c  |3 +++
 drivers/usb/musb/davinci.h  |1 +

diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 472c8b4..af09ebf 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -428,6 +428,9 @@ static int davinci_musb_init(struct musb *musb)
__raw_writel(deepsleep, DM355_DEEPSLEEP);
}
 
+   if (machine_is_davinci_dm365_evm())
+   writel(readl(USB_PHY_CTRL) | USBPHY_CLKFREQ_24MHZ, 
USB_PHY_CTRL);
+
/* reset the controller */
musb_writel(tibase, DAVINCI_USB_CTRL_REG, 0x1);
 
diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
index 371baa0..e737d97 100644
--- a/drivers/usb/musb/davinci.h
+++ b/drivers/usb/musb/davinci.h
@@ -16,6 +16,7 @@
 
 /* Integrated highspeed/otg PHY */
 #define USBPHY_CTL_PADDR   0x01c40034
+#define USBPHY_CLKFREQ_24MHZ   BIT(13)
 #define USBPHY_DATAPOL BIT(11) /* (dm355) switch D+/D- */
 #define USBPHY_PHYCLKGDBIT(8)
 #define USBPHY_SESNDEN BIT(7)  /* v(sess_end) comparator */
-- 
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: usb audio race at disconnect time

2012-10-11 Thread Takashi Iwai
[Added Daniel and Clemens in the loop]

At Thu, 11 Oct 2012 17:17:59 +0200,
Matthieu CASTET wrote:
> 
> Hi,
> 
> while doing some monkey tests on a product we found races in usb audio code 
> when
> the device in unplugged from usb (on linus master tree).
> 
> This can be reproduced with usb_audio_show_race.diff and CONFIG_DEBUG_SLAB.
> With this patch, start a stream :
> # arecord -D hw:0 -r 44100 -c 2  -f S16_LE > /dev/null
> you will see the kernel log : "in snd_usb_hw_params sleeping"
> Unplug the device before "in snd_usb_hw_params sleeping exit", and you will 
> see
> an oops in snd_pcm_hw_params
> 
> 
> Instead of using CONFIG_DEBUG_SLAB, usb_audio_show_use_after_free.diff can 
> show
> use after free by setting usb_device pointer to NULL in
> snd_usb_audio_disconnect. [1]
> 
> 
> In order to protect from all the races, before using any usb_device we need to
> check if it is not freed.
> 
> What's the best way to do that ?
> 
> A trival fix would be to take a mutex in all snd_pcm_ops callback that access 
> usb :
> 
> {
> mutex_lock(&chip->shutdown_mutex);
> if (!chip->dev) {
>   mutex_unlock(&chip->shutdown_mutex);
>   return -ENODEV;
> }
> [...]
> mutex_unlock(&chip->shutdown_mutex);
> }
> 
> 
> But that will serialize all snd_pcm_ops callbacks.

We had already some protections but they don't cover the recent code
rewrites at all, so this bad result for now.

Yes, the easiest and maybe sane way would be to put the mutex in each
ops except for trigger.  open, close, hw_params, hw_free and prepare
are ops that don't need much parallelism, so this can be protected
well.  Or, we may introduce a new mutex per stream if we really want
parallel operations, but I don't think it's worth.

The trigger callback is an atomic ops, so it won't need the check.
Maybe the spinlock is needed to avoid the race in
snd_usb_stream_disconnect().

> Another solution could be to use refcounting or rwlock patern :
> - snd_pcm_ops callbacks call rdlock_trylock/rdlock_unlock
> - usb_driver disconnect callback take rwlock_lock and never release it

I don't think this is needed.

So... the below is a quick hack I did without testing at all.
Hopefully this can give some advance.


Takashi

---
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 561bb74..115484e 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -131,9 +131,13 @@ static void snd_usb_stream_disconnect(struct list_head 
*head)
subs = &as->substream[idx];
if (!subs->num_formats)
continue;
+   if (subs->pcm_substream)
+   snd_pcm_stream_lock_irq(subs->pcm_substream);
subs->interface = -1;
subs->data_endpoint = NULL;
subs->sync_endpoint = NULL;
+   if (subs->pcm_substream)
+   snd_pcm_stream_unlock_irq(subs->pcm_substream);
}
 }
 
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 55e19e1..01e82ac 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -444,7 +444,6 @@ static int configure_endpoint(struct snd_usb_substream 
*subs)
 {
int ret;
 
-   mutex_lock(&subs->stream->chip->shutdown_mutex);
/* format changed */
stop_endpoints(subs, 0, 0, 0);
ret = snd_usb_endpoint_set_params(subs->data_endpoint,
@@ -455,7 +454,7 @@ static int configure_endpoint(struct snd_usb_substream 
*subs)
  subs->cur_audiofmt,
  subs->sync_endpoint);
if (ret < 0)
-   goto unlock;
+   return ret;
 
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->data_endpoint,
@@ -466,8 +465,6 @@ static int configure_endpoint(struct snd_usb_substream 
*subs)
  subs->cur_audiofmt,
  NULL);
 
-unlock:
-   mutex_unlock(&subs->stream->chip->shutdown_mutex);
return ret;
 }
 
@@ -488,10 +485,15 @@ static int snd_usb_hw_params(struct snd_pcm_substream 
*substream,
struct audioformat *fmt;
int ret;
 
+   mutex_lock(&subs->stream->chip->shutdown_mutex);
+   if (subs->stream->chip->shutdown) {
+   ret = -ENODEV;
+   goto unlock;
+   }
ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
   params_buffer_bytes(hw_params));
if (ret < 0)
-   return ret;
+   goto unlock;
 
subs->pcm_format = params_format(hw_params);
subs->period_bytes = params_period_bytes(hw_params);
@@ -502,17 +504,20 @@ static int snd_usb_hw_params(struct snd_pcm_substream 
*substream,
if (!fmt) {
snd_printd(KERN_DEBUG "cannot set format: format = %#x, rate = 
%d, channels = %d\n",
   subs->pcm_format, subs->cur_rate, subs->channels);
-   return -EINVAL;
+  

[PATCH v7] Initialize USB on dm365 EVM

2012-10-11 Thread Constantine Shulyupin
From: Constantine Shulyupin 

Call USB initialization davinci_setup_usb from board initialization 
dm365_evm_init.

Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC.

Note: register USB_PHY_CTRL must have flag USBPHY_CLKFREQ_24MHZ

References:

Original patch by miguel.agui...@ridgerun.com three years ago:
- 
http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html

Signed-off-by: Constantine Shulyupin 
---

Changelog

Changes since v6
- patch splitted accordinly request of Sergei
- this split contains call to davinci_setup_usb from dm365_evm_init

Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html
accordingy feedback of nsek...@ti.com 
http://www.spinics.net/lists/kernel/msg1414914.html
- phy configuration moved to drivers/usb/musb/davinci.c
- USB_OTG configuration is submitted in separated patch: 
http://www.spinics.net/lists/kernel/msg1414964.html
- Setting current limit to 1000 mA. Any way the current is limited to 510 mA in 
davinci_setup_usb.

Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html
- removed fix of dev_info in musb_init_controller

Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
- removed optional altering of pr_info

Changes since v1  http://marc.info/?l=linux-kernel&m=130894150803661&w=2:
- removed optional code and reordered
- removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is 
not need for peripheral USB

This patch is based on code from projects Arago, Angstom and RidgeRun.

---
 arch/arm/mach-davinci/board-dm365-evm.c |2 ++

diff --git a/arch/arm/mach-davinci/board-dm365-evm.c 
b/arch/arm/mach-davinci/board-dm365-evm.c
index 688a9c5..ba5ffc1 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -610,6 +611,7 @@ static __init void dm365_evm_init(void)
 
dm365_init_spi0(BIT(0), dm365_evm_spi_info,
ARRAY_SIZE(dm365_evm_spi_info));
+   davinci_setup_usb(1000, 8);
 }
 
 MACHINE_START(DAVINCI_DM365_EVM, "DaVinci DM365 EVM")
-- 
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: cannot submit urb 0, error -22: internal error followed by USB hung tasks

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Gian-Carlo Pascutto wrote:

> On 11/10/12 17:19, Alan Stern wrote:
> 
> >> Maybe my hardware/chipset is just crap?
> > 
> > I don't know, maybe.  Maybe that's just a bad cable connection (this 
> > includes the possibility of bad connections inside the PC case).
> 
> Ouch. I can check a few things, like the USB card reader. It should be
> possible to correlate the ohci_hcd :00:04.0 with a header?

Yes.

> I just got the following error, with nothing connected (well, at least
> not the headset or camera). So maybe they're not directly responsible,
> except for putting more stress on the USB system.

My suggestion is to forget all about these problems and buy an add-on 
PCI card with a USB controller.  It seems pretty clear that whatever 
the cause is, it's a hardware issue, not a software issue.

Alan Stern

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


Hibernation with LPM, was: Re: xhci: LPM issues using Western Digital harddrive

2012-10-11 Thread Sarah Sharp
On Mon, Oct 01, 2012 at 03:53:03PM -0400, Alan Stern wrote:
> On Mon, 1 Oct 2012, Sarah Sharp wrote:
> 
> > > I don't remember the details.  Doesn't usb_unlocked_disable_lpm() force 
> > > the device back into U0?  You call that routine before suspending the 
> > > port.
> > 
> > It sends the control transfer to bring the port from U1/U2 into U0 and
> > disables any further device or hub initiated U1/U2 transitions.  But it
> > doesn't actually _wait_ for the port to enter U0.
> 
> Would you have to poll for it in a loop?

Actually, looking at the code, after we disable the parent U1/U2
timeouts, we send a control transfer to disable device-initiated U1/U2.
The device must be in U0 to receive the control transfer, so the device
is in U0 when usb_disable_lpm() returns.

> > > But we don't when going into the FREEZE or PRETHAW stages of 
> > > hibernation -- all we do is call the bus_suspend routines for root 
> > > hubs.  That may need to be changed for USB-3 buses.
> > 
> > I think so.  Where is the code that only calls bus_suspend?  I'll take a
> > stab at fixing it.
> 
> generic.c:generic_suspend().  Note that nowadays, PM_EVENT_PRETHAW is 
> merely an alias for PM_EVENT_QUIESCE.

Thinking about this further, USB 2.1 devices need to be brought out of
their low power link state (L1) before they are suspended.  Some xHCI
host controllers have hardware-driven USB 2.1 LPM, so the device could
have been put in L1 before hibernation.  Therefore we need to suspend
USB 2.1 and USB 3.0 devices before we hibernate.

Should we suspend all USB 2.0 devices, or should we only suspend devices
with bcdUSB 2.1 and higher?  If we only suspend the USB 2.1 devices,
will that work if there is a USB 2.1 hub with USB 2.0 children?

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


Re: Link PM policy advice?

2012-10-11 Thread Sarah Sharp
On Tue, Oct 09, 2012 at 03:36:59PM -0400, Alan Stern wrote:
> On Tue, 9 Oct 2012, Sarah Sharp wrote:
> 
> > Hmm, so I've tested the VIA hubs under a different Intel chipset (Lynx
> > Point), and they exhibit the same issues as when a bus analyzer is
> > connected on Panther Point.  Only the issues show up without the
> > analyzer in between.
> 
> Come to think of it, you might want to report the Panther Point results 
> to the company that made the bus analyzer.  It's the sort of thing they 
> should be interested in fixing.

Yeah, I should send them an email.  They also display the Set SEL
command wrong, so I don't think the LPM portion of their
firmware/software is very well tested.

> > I'll do some more triaging, but it looks like some devices may work with
> > LPM enabled on some hosts and not others.
> 
> Have you spoken to the hardware people at Intel about this?  They 
> should be interested in hearing about failures in their host 
> controllers.

It could be that the device is marginal electrically, and putting the
analyzer in between just pushes it over the edge.  Maybe the new host
controller isn't as tolerant of the signaling as the old host
controller.  I'm not a hardware expert, so yes, I'm talking to Intel's
chipset team.

> >  What do you think about
> > dynamically adding device VID:PIDs to a blacklist?
> 
> How safe would it be to populate this list?  That is, how sure can you 
> be that once a device fails with LPM enabled, you can get it to work 
> again simply by turning off LPM?

When LPM fails, the device often goes into the Inactive state, which
also marks the port as not connected.  The USB core will issue a warm
reset to the device and then do a logical disconnect.  VIA hubs come
back fine after the warm reset, but that does mean the user will see a
disconnect and reconnect of devices beneath the tree.

> Also, how annoying will it be for users if their device fails and has 
> to be reset (or replugged) every time they boot?

Yep, you're right that it would be pretty annoying.  I'm leaning towards
the static blacklist in the kernel.

One of the things I've found out is that VIA hubs keep their firmware
revision in the bcdDevice field of their device descriptor.  If I find
one version of the VIA firmware works with LPM, can the quirks table
create a rule that would effectively be something like "bcdDevice <
0x9a90"?

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


Re: Etron USB 3.0 xHCI

2012-10-11 Thread Sarah Sharp
On Wed, Oct 10, 2012 at 02:15:33PM -0700, Matthew Hall wrote:
> On Mon, Oct 08, 2012 at 02:34:24PM -0700, Gary E. Miller wrote:
> > > The error you're getting seems to me to indicate a
> > > hardware issue, and I'm really not sure how to solve it.
> > 
> > Could be, I find many people have problems with the chip.
> > 
> > > > Cards with the Etron chip on them are $14 on newegg.  I'd be happy
> > > > to buy one for anyone that wants to see the problem up close and
> > > > personal.
> > > 
> > > Money's not really the issue for me, it's time...
> > 
> > I can see it not being of major interest to you.  But the offer stands
> > for anyone that wants to take a crack at it.
> > 
> > I guess I'll go back to kurking.
> 
> Hi all,
> 
> I'm in kind of the same sinking boat as Gary here.
> 
> It's unfortunate that somebody soldered this chip into my motherboard with 
> all 
> the issues that have come up.
> 
> I'm wondering, what are my other options? How can I go about acquiring a PCIe 
> card that uses a known-good working USB 3.0 chip instead? Or what else can I 
> do to sort this issue out?

You can buy an xHCI host controller with the NEC (Rensas) chipset it in.
You may be able to read the data sheets for the various cards and see
which chipset they're using.  For example, doing a search for "nec usb
3.0 card" on newegg.com brought up this:

http://www.newegg.com/Product/Product.aspx?Item=N82E16815150161

There is a couple cheaper cards available with the NEC chipset, but the
reviews are full of people receiving dead cards, so I'd go with the more
expensive brand.

Sorry about this!  The NEC is one of the most stable hosts out there, so
I hope you can get one and enjoy a less buggy USB 3.0 experience. :-/

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


Re: Hibernation with LPM, was: Re: xhci: LPM issues using Western Digital harddrive

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Sarah Sharp wrote:

> Thinking about this further, USB 2.1 devices need to be brought out of
> their low power link state (L1) before they are suspended.  Some xHCI
> host controllers have hardware-driven USB 2.1 LPM, so the device could
> have been put in L1 before hibernation.  Therefore we need to suspend
> USB 2.1 and USB 3.0 devices before we hibernate.

I need to go through the LPM document more carefully before I can 
answer fully.  However, what about disabling LPM instead of going all 
the way into suspend?  For USB 2.1 devices at least?

> Should we suspend all USB 2.0 devices, or should we only suspend devices
> with bcdUSB 2.1 and higher?  If we only suspend the USB 2.1 devices,
> will that work if there is a USB 2.1 hub with USB 2.0 children?

Here's more or less what I'd like to do (I don't know which parts will
work with USB-3 hubs and devices, though):

In generic_suspend, skip calling usb_port_suspend for
FREEZE and PRETHAW events (just as we do now).

In usb_port_suspend, set the wakeup feature as we do now.
But if the event is for a system sleep, don't set the
port-suspend feature but do disable LPM.

In hub_suspend, if the event is a runtime suspend,
clear the port-suspend feature for any children that
have wakeup enabled.  Keep track of which ports they
were on for later use.

In hub_resume, if the event is a runtime resume, queue
a runtime-resume request for the devices attached to
those ports.  Also, prevent the hub from going back
into runtime suspend until all those resumes have
completed.

Partly this is to optimize system suspend, and partly it is to work 
around the child-wakeup-request vs. port-suspend race.  At this time 
I'm not sure if it will interact correctly with USB >=2.1 LPM.

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: Link PM policy advice?

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Sarah Sharp wrote:

> When LPM fails, the device often goes into the Inactive state, which
> also marks the port as not connected.  The USB core will issue a warm
> reset to the device and then do a logical disconnect.  VIA hubs come
> back fine after the warm reset, but that does mean the user will see a
> disconnect and reconnect of devices beneath the tree.

Kind of a bummer if this happens while you've got a filesystem mounted 
on the device...

> > Also, how annoying will it be for users if their device fails and has 
> > to be reset (or replugged) every time they boot?
> 
> Yep, you're right that it would be pretty annoying.  I'm leaning towards
> the static blacklist in the kernel.
> 
> One of the things I've found out is that VIA hubs keep their firmware
> revision in the bcdDevice field of their device descriptor.  If I find
> one version of the VIA firmware works with LPM, can the quirks table
> create a rule that would effectively be something like "bcdDevice <
> 0x9a90"?

Yes.  A usb_device_id can specify upper and lower limits for a
bcdDevice range.

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] kaweth: print correct debug ptr

2012-10-11 Thread David Miller
From: Alan Cox 
Date: Thu, 11 Oct 2012 17:22:03 +0100

> From: Alan Cox 
> 
> We nowdays copy the buffer and free fw->data, so make the debug printk use
> the right thing.
> 
> Signed-off-by: Alan Cox 

Applied and queued up for -stable.
--
To unsubscribe from this list: send the line "unsubscribe 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: About resume time optimization for bus resume routine

2012-10-11 Thread Alan Stern
On Thu, 11 Oct 2012, Peter Chen wrote:

> > You could test all the port status registers.  If any of them have the
> > PORT_PE bit set and the PORT_SUSPEND bit clear then the delay is
> > needed; otherwise it can be skipped.
> 
> I think the condition should like below, as we need to consider remote wakeup.
> When remote wakeup occurs, it doesn't need this delay either.
> 
> if (portsc & PORT_PE) && ~(portsc & (PORT_SUSPEND | PORT_RESUME)))
>   do_delay;

It shouldn't make any difference.  Whenever PORT_RESUME is set, 
PORT_SUSPEND has to be set too (the hardware turns both bits off at the 
same time, when the resume sequence is finished).

> I mean the port status before roothub sends resume, only at remote wakeup
> situation, that port status will be unsuspended enabled.

Yes, because of your buggy EHCI hardware, right?  According to the
EHCI spec, when a device sends a remote wakeup signal, the port status
should continue to have PORT_SUSPEND set until after the driver clears
PORT_RESUME.  But your hardware doesn't wait.

In the end I think it won't matter.  Even if a device does send a 
wakeup request, it still needs the TRSMRCY delay.

> > It is needed under these conditions:
> > 
> > The HCD is not ehci-hcd.
> If you have only delay above 20ms, and then clear PORT_RESUME, you
> still need delay TRSMRCY, as TRSMRCY is the time after PORT_RESUME is
> cleared.

Whoops, you're right.  I got the two delays mixed up.  Okay, forget 
about this condition.

> > There is a port on the root hub with suspend status clear
> > and enabled status set (which implies there must be a device
> > attached to the port, because disconnected ports can't be
> > enabled).
> > 
> > Obviously if a port isn't enabled then it doesn't need any delay.
> Is it ok to set one hcd flag, like hcd->unsuspended_device_on_port
> to get condition at xxx_bus_suspend?

Only if you're willing to change all the host controller drivers to 
make them set that flag!

usb_bus_resume() can poll the port statuses to see if there are any 
enabled, unsuspended ports.  If there aren't any, the delay can be 
skipped.

How does that sound?

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 1/4] USB: Enable LPM after a failed probe.

2012-10-11 Thread Sarah Sharp
Before a driver is probed, we want to disable USB 3.0 Link Power
Management (LPM), in case the driver needs hub-initiated LPM disabled.
After the probe finishes, we want to attempt to re-enable LPM, order to
balance the LPM ref count.

When a probe fails (such as when libusual doesn't want to bind to a USB
3.0 mass storage device), make sure to balance the LPM ref counts by
re-enabling LPM.

This patch should be backported to kernels as old as 3.5, that contain
the commit 8306095fd2c1100e8244c09bf560f97aca5a311d "USB: Disable USB
3.0 LPM in critical sections."

Signed-off-by: Sarah Sharp 
Cc: sta...@vger.kernel.org
---
 drivers/usb/core/driver.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index ddd820d..6056db7 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -367,6 +367,10 @@ static int usb_probe_interface(struct device *dev)
intf->condition = USB_INTERFACE_UNBOUND;
usb_cancel_queued_reset(intf);
 
+   /* If the LPM disable succeeded, balance the ref counts. */
+   if (!lpm_disable_error)
+   usb_unlocked_enable_lpm(udev);
+
/* Unbound interfaces are always runtime-PM-disabled and -suspended */
if (driver->supports_autosuspend)
pm_runtime_disable(dev);
-- 
1.7.9

--
To unsubscribe from this list: send the line "unsubscribe 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: Don't enable LPM if the exit latency is zero.

2012-10-11 Thread Sarah Sharp
Some USB 3.0 devices signal that they don't implement Link PM by having
all zeroes in the U1/U2 exit latencies in their SuperSpeed BOS
descriptor.  Don found that a Western Digital device he has experiences
transfer errors when LPM is enabled.  The lsusb shows the U1/U2 exit
latencies are set to zero:

Binary Object Store Descriptor:
  bLength 5
  bDescriptorType15
  wTotalLength   22
  bNumDeviceCaps  2
  SuperSpeed USB Device Capability:
bLength10
bDescriptorType16
bDevCapabilityType  3
bmAttributes 0x00
  Latency Tolerance Messages (LTM) Supported
wSpeedsSupported   0x000e
  Device can operate at Full Speed (12Mbps)
  Device can operate at High Speed (480Mbps)
  Device can operate at SuperSpeed (5Gbps)
bFunctionalitySupport   1
  Lowest fully-functional device speed is Full Speed (12Mbps)
bU1DevExitLat   0 micro seconds
bU2DevExitLat   0 micro seconds

The fix is to not enable LPM for a particular link state if we find its
corresponding exit latency is zero.

This patch should be backported to kernels as old as 3.5, that contain
the commit 1ea7e0e8e3d0f50901d335ea4178ab2aa8c88201 "USB: Add support to
enable/disable USB3 link states."

Signed-off-by: Sarah Sharp 
Reported-by: Don Zickus 
Tested-by: Don Zickus 
Cc: sta...@vger.kernel.org
---
 drivers/usb/core/hub.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 673ee46..8f04787 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3415,6 +3415,16 @@ static void usb_enable_link_state(struct usb_hcd *hcd, 
struct usb_device *udev,
enum usb3_link_state state)
 {
int timeout;
+   __u8 u1_mel = udev->bos->ss_cap->bU1devExitLat;
+   __le16 u2_mel = udev->bos->ss_cap->bU2DevExitLat;
+
+   /* If the device says it doesn't have *any* exit latency to come out of
+* U1 or U2, it's probably lying.  Assume it doesn't implement that link
+* state.
+*/
+   if ((state == USB3_LPM_U1 && u1_mel == 0) ||
+   (state == USB3_LPM_U2 && u2_mel == 0))
+   return;
 
/* We allow the host controller to set the U1/U2 timeout internally
 * first, so that it can change its schedule to account for the
-- 
1.7.9

--
To unsubscribe from this list: send the line "unsubscribe 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: trival: Fix debugging units mistake.

2012-10-11 Thread Sarah Sharp
SEL and PEL are in microseconds, not milliseconds.  Also, fix a split
string that will trigger checkpatch warnings.

Signed-off-by: Sarah Sharp 
---
 drivers/usb/core/hub.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b8e11ff..64854d7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3241,8 +3241,7 @@ static int usb_req_set_sel(struct usb_device *udev, enum 
usb3_link_state state)
(state == USB3_LPM_U2 &&
 (u2_sel > USB3_LPM_MAX_U2_SEL_PEL ||
  u2_pel > USB3_LPM_MAX_U2_SEL_PEL))) {
-   dev_dbg(&udev->dev, "Device-initiated %s disabled due "
-   "to long SEL %llu ms or PEL %llu ms\n",
+   dev_dbg(&udev->dev, "Device-initiated %s disabled due to long 
SEL %llu us or PEL %llu us\n",
usb3_lpm_names[state], u1_sel, u1_pel);
return -EINVAL;
}
-- 
1.7.9

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


[Pull Request] Link PM patches for 3.7

2012-10-11 Thread Sarah Sharp
The following changes since commit ecefbd94b834fa32559d854646d777c56749ef1c:

  Merge tag 'kvm-3.7-1' of git://git.kernel.org/pub/scm/virt/kvm/kvm 
(2012-10-04 09:30:33 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git 
for-usb-linus-2012-10-11

for you to fetch changes up to 1510a1a2d0946b34422fe8816187f274a5086904:

  usb: trival: Fix debugging units mistake. (2012-10-08 11:48:41 -0700)


USB 3.0 Link PM fixes.

Hi Greg,

Here's four fixes for the USB 3.0 Link Power Management code.  They
work around some USB 3.0 devices that don't support LPM, like the
Western Digital My Passport hard drive.  They also fix an LPM disable
ref count bug that would cause LPM to remain disabled for a device
after a failed probe.

Please queue for 3.7 after the merge window closes.  Several patches are
marked for stable.

Sarah Sharp


Sarah Sharp (4):
  USB: Enable LPM after a failed probe.
  usb: Don't enable LPM if the exit latency is zero.
  usb: Send Set SEL before enabling parent U1/U2 timeout.
  usb: trival: Fix debugging units mistake.

 drivers/usb/core/driver.c |4 
 drivers/usb/core/hub.c|   36 +++-
 2 files changed, 27 insertions(+), 13 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] usb: Send Set SEL before enabling parent U1/U2 timeout.

2012-10-11 Thread Sarah Sharp
The Set SEL control transfer tells a device the exit latencies
associated with a device-initated U1 or U2 exit.  Since a parent hub may
initiate a transition to U1 soon after a downstream port's U1 timeout is
set, we need to make sure the device receives the Set SEL transfer
before the parent hub timeout is set.

This patch should be backported to kernels as old as 3.5, that contain
the commit 1ea7e0e8e3d0f50901d335ea4178ab2aa8c88201 "USB: Add support to
enable/disable USB3 link states."

Signed-off-by: Sarah Sharp 
Cc: sta...@vger.kernel.org
---
 drivers/usb/core/hub.c |   23 ---
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8f04787..b8e11ff 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3320,16 +3320,6 @@ static int usb_set_device_initiated_lpm(struct 
usb_device *udev,
 
if (enable) {
/*
-* First, let the device know about the exit latencies
-* associated with the link state we're about to enable.
-*/
-   ret = usb_req_set_sel(udev, state);
-   if (ret < 0) {
-   dev_warn(&udev->dev, "Set SEL for device-initiated "
-   "%s failed.\n", usb3_lpm_names[state]);
-   return -EBUSY;
-   }
-   /*
 * Now send the control transfer to enable device-initiated LPM
 * for either U1 or U2.
 */
@@ -3414,7 +3404,7 @@ static int usb_set_lpm_timeout(struct usb_device *udev,
 static void usb_enable_link_state(struct usb_hcd *hcd, struct usb_device *udev,
enum usb3_link_state state)
 {
-   int timeout;
+   int timeout, ret;
__u8 u1_mel = udev->bos->ss_cap->bU1devExitLat;
__le16 u2_mel = udev->bos->ss_cap->bU2DevExitLat;
 
@@ -3426,6 +3416,17 @@ static void usb_enable_link_state(struct usb_hcd *hcd, 
struct usb_device *udev,
(state == USB3_LPM_U2 && u2_mel == 0))
return;
 
+   /*
+* First, let the device know about the exit latencies
+* associated with the link state we're about to enable.
+*/
+   ret = usb_req_set_sel(udev, state);
+   if (ret < 0) {
+   dev_warn(&udev->dev, "Set SEL for device-initiated %s 
failed.\n",
+   usb3_lpm_names[state]);
+   return;
+   }
+
/* We allow the host controller to set the U1/U2 timeout internally
 * first, so that it can change its schedule to account for the
 * additional latency to send data to a device in a lower power
-- 
1.7.9

--
To unsubscribe from this list: send the line "unsubscribe 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: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Henrik Rydberg
Hi Hans,

> Oh what fun (not). The best way to figure out what really is going
> on is to get some usb level traces. Note my first hunch is that what
> you're seeing is a device firmware bug, as this patch together with
> a new libusb (which you seem to also have) will make bulk transfers
> run slightly faster, which might be just enough to overwhelm your
> device ...

Or, the large bulk transfer actually never worked in the first place.
The list you gave me seemed boringly long, so I read the patch more
closely instead. The fix below is the result. Greg, will you please
take it through your tree?

Thanks,
Henrik

>From 40b70394747eea51fdd07cc8213dd6afd24b1b30 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg 
Date: Thu, 11 Oct 2012 23:27:04 +0200
Subject: [PATCH] usbdevfs: Fix broken scatter-gather transfer

The recently introduced handling of large bulk transfers is completely
broken; the same user page is read over and over again. Fixed with
this patch.

Cc: sta...@kernel.org
Signed-off-by: Henrik Rydberg 
---
 drivers/usb/core/devio.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index ebb8a9d..6e58b59 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1172,6 +1172,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
struct usb_ctrlrequest *dr = NULL;
unsigned int u, totlen, isofrmlen;
int i, ret, is_in, num_sgs = 0, ifnum = -1;
+   void __user *userbuf;
void *buf;
 
if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
@@ -1333,6 +1334,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
as->urb->num_sgs = num_sgs;
sg_init_table(as->urb->sg, as->urb->num_sgs);
 
+   userbuf = uurb->buffer;
totlen = uurb->buffer_length;
for (i = 0; i < as->urb->num_sgs; i++) {
u = (totlen > USB_SG_SIZE) ? USB_SG_SIZE : totlen;
@@ -1344,11 +1346,12 @@ static int proc_do_submiturb(struct dev_state *ps, 
struct usbdevfs_urb *uurb,
sg_set_buf(&as->urb->sg[i], buf, u);
 
if (!is_in) {
-   if (copy_from_user(buf, uurb->buffer, u)) {
+   if (copy_from_user(buf, userbuf, u)) {
ret = -EFAULT;
goto error;
}
}
+   userbuf += u;
totlen -= u;
}
} else if (uurb->buffer_length > 0) {
-- 
1.7.12.2

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


Re: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Peter Stuge
Henrik Rydberg wrote:
> Hi Hans,
> 
> > Oh what fun (not). The best way to figure out what really is going
> > on is to get some usb level traces. Note my first hunch is that what
> > you're seeing is a device firmware bug, as this patch together with
> > a new libusb (which you seem to also have) will make bulk transfers
> > run slightly faster, which might be just enough to overwhelm your
> > device ...
> 
> Or, the large bulk transfer actually never worked in the first place.
> The list you gave me seemed boringly long, so I read the patch more
> closely instead. The fix below is the result. Greg, will you please
> take it through your tree?
> 
> Thanks,
> Henrik
> 
> From 40b70394747eea51fdd07cc8213dd6afd24b1b30 Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg 
> Date: Thu, 11 Oct 2012 23:27:04 +0200
> Subject: [PATCH] usbdevfs: Fix broken scatter-gather transfer
> 
> The recently introduced handling of large bulk transfers is completely
> broken; the same user page is read over and over again. Fixed with
> this patch.
> 
> Cc: sta...@kernel.org
> Signed-off-by: Henrik Rydberg 

Acked-by: Peter Stuge 
--
To unsubscribe from this list: send the line "unsubscribe 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: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Henrik Rydberg
On Thu, Oct 11, 2012 at 08:50:33AM +0200, Peter Stuge wrote:
> Henrik Rydberg wrote:
> > > What is the programming cable and software that uses it?
> > 
> > The programmer is impact, using libusbx-1.0.14-1.
> 
> Do you know for a fact that your version calls libusb-1.0? Did you
> establish this with e.g. strace? ISE 11.1 impact uses only libusb.so,
> ie. libusb-0.1.

I modified libusbx during testing, and yes, it is really used.

> > The patch is pretty generic, so I am suprised the problem has not
> > shown up earlier.
> 
> There are several explanations. There is clearly a problem with Hans'
> patch(es) for some cases, but those are perhaps not so common.

As it turned out, all cases of large bulk transfers, so I guess 3.6
has not been that widely tested yet. ;-)

> I've reviewed Hans' patch that he added to libusbx and which is in
> libusbx-1.0.14, but I am not sure that it is correct, which is why
> I haven't included it in libusb yet.

I looked at it for a while, it does seem correct.

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe 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: REGRESSION: usbdevfs: Use-scatter-gather-lists-for-large-bulk-transfers

2012-10-11 Thread Greg Kroah-Hartman
On Thu, Oct 11, 2012 at 11:37:07PM +0200, Henrik Rydberg wrote:
> Hi Hans,
> 
> > Oh what fun (not). The best way to figure out what really is going
> > on is to get some usb level traces. Note my first hunch is that what
> > you're seeing is a device firmware bug, as this patch together with
> > a new libusb (which you seem to also have) will make bulk transfers
> > run slightly faster, which might be just enough to overwhelm your
> > device ...
> 
> Or, the large bulk transfer actually never worked in the first place.
> The list you gave me seemed boringly long, so I read the patch more
> closely instead. The fix below is the result. Greg, will you please
> take it through your tree?

Henrik, Very nice fix, thanks for debugging this.

Hans, any objection to me taking this?

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: [Pull Request] Link PM patches for 3.7

2012-10-11 Thread Greg Kroah-Hartman
On Thu, Oct 11, 2012 at 02:28:06PM -0700, Sarah Sharp wrote:
> The following changes since commit ecefbd94b834fa32559d854646d777c56749ef1c:
> 
>   Merge tag 'kvm-3.7-1' of git://git.kernel.org/pub/scm/virt/kvm/kvm 
> (2012-10-04 09:30:33 -0700)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git 
> for-usb-linus-2012-10-11
> 
> for you to fetch changes up to 1510a1a2d0946b34422fe8816187f274a5086904:
> 
>   usb: trival: Fix debugging units mistake. (2012-10-08 11:48:41 -0700)
> 
> 
> USB 3.0 Link PM fixes.
> 
> Hi Greg,
> 
> Here's four fixes for the USB 3.0 Link Power Management code.  They
> work around some USB 3.0 devices that don't support LPM, like the
> Western Digital My Passport hard drive.  They also fix an LPM disable
> ref count bug that would cause LPM to remain disabled for a device
> after a failed probe.
> 
> Please queue for 3.7 after the merge window closes.  Several patches are
> marked for stable.

Great, I'll queue this up after 3.7-rc1 is out, or if I get bored today
while sitting in conference talks :)

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] usbhid: use GFP_NOIO in reset code path

2012-10-11 Thread Jiri Kosina
On Thu, 11 Oct 2012, Ming Lei wrote:

> Keeping allowed gfp_flag inside task_struct should be one solution, and
> let that teach mm to allocate memory, see the draft idea below:

Hmm, I actually like that idea.

I guess you are going to propose this to a wider audience, right? So I am 
now putting Oliver's patch on hold, and please CC me on your further 
efforts with this patch.

Thanks,

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


Re: locking, reset, memory allocations and deadlocks

2012-10-11 Thread Greg KH
On Wed, Oct 10, 2012 at 11:08:11AM -0400, Alan Stern wrote:
> On Wed, 10 Oct 2012, Oliver Neukum wrote:
> 
> > Hi,
> > 
> > Ming Lei made me look. I found a scenario I don't like.
> > Suppose we have a device with two interfaces #1 and #0
> > Let #1 be storage and #0 be vendor specific and the skeleton driver bound 
> > to it.
> > 
> > 
> > CPU A   
> > CPU B
> > 
> > skel_read()
> > mutex_lock_interruptible(&dev->io_mutex)
> > copy_to_user(...)
> > page fault
> > schedule reading a page from interface #0
> 
> Looks like the copy_to_user() needs to go outside the scope of the 
> io_mutex.  In fact, io_mutex needs to protect only the device I/O.  And 
> not even all of that; just the places where URBs are submitted.
> 
> > 
> > 
> > usb_stor_port_reset()
> > 
> > 
> > usb_reset_device()
> > 
> > 
> > drv->pre_reset()
> > 
> > 
> > skel_pre_reset()
> > 
> > 
> > mutex_lock(&dev->io_mutex)
> > 
> > Contrary to the usual scenario you don't need writable mmap. In fact it 
> > could happen with a scratched CD.
> > What do you think?
> 
> usb-skeleton is way, way too complicated to serve as a good programming 
> example.  :-)

I agree, it's grown way to large and complex, which makes me wonder if a
"simple" USB driver is too large and complex these days and we are doing
something wrong...

It's on my TODO list to clean up and look at closer one of these days,
luckily no one is writing new USB drivers anymore, they all just end up
being some type of a class device.

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] usbhid: use GFP_NOIO in reset code path

2012-10-11 Thread Ming Lei
On Fri, Oct 12, 2012 at 5:55 AM, Jiri Kosina  wrote:
> On Thu, 11 Oct 2012, Ming Lei wrote:
>
>> Keeping allowed gfp_flag inside task_struct should be one solution, and
>> let that teach mm to allocate memory, see the draft idea below:
>
> Hmm, I actually like that idea.
>
> I guess you are going to propose this to a wider audience, right? So I am

Yes, I am working on it.

> now putting Oliver's patch on hold, and please CC me on your further
> efforts with this patch.

No problem, :-)


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


Re: USB errors, ehci testing modes

2012-10-11 Thread Tim Sander
Hi Alan

Thanks for your reply!

> > I have some stange errors on a arm pcm043 with internal usb phy. The log
> > is attached at the end of the mail. It would be nice if someone could
> > give me a pointer whats going wrong?. It would also be nice to know
> > where the status flags which are output by the usb debug messages are
> > documented?



> > As it might be a hw problem i would like to activate the testmodes of the
> > ehci controller. The old Freescale BSP driver had a file in debugfs
> > where enabling this stuff was easy. Is there also an easy way to enable
> > the ehci debug facility from linux usermode?
> 
> No.  You will have to write a program to send a
> Set-Port-Feature(PORT_TEST) request to the root hub.  Or patch the
> kernel to add a debugfs file like the BSP driver had.
Ok, so i can instruct the root hub via usermode to go into test mode. So its 
possible from usermode as far as i see but not from shell directly?

> > hub 1-0:1.0: state 7 ports 1 chg  evt 0002
> > mxc-ehci mxc-ehci.0: GetStatus port:1 status c00100a 6  ACK POWER sig=se0
Is there anything strang with this status code   ^^^ ?
This is where the trouble begins.
> > PEC CSC
> > hub 1-0:1.0: port 1, status 0100, change 0003, 12 Mb/s
> > usb 1-1: USB disconnect, device number 23
> > usb 1-1: unregistering device
> > usb 1-1: unregistering interface 1-1:1.0
> > usb-storage: storage_disconnect() called
> > usb-storage: -- usb_stor_release_resources
> > usb-storage: -- sending exit command to thread
> > usb-storage: *** thread awakened.
> > usb-storage: -- exiting
> > usb-storage: -- dissociate_dev
> > usb 1-1: usb_disable_device nuking all URBs
> 
> These messages mean there was a disconnect.  Assuming you didn't unplug
> the cable, some signal made the controller or phy think it was
> unplugged.
Well, it is unconnecting then out of the blue, randomly very seldom without 
any good reason...

> Possibly the event was connected with Link Power Management.  You might
> want to try disabling LPM for that port (write "disable 1" to the "lpm"
> file in the EHCI debugging directory).
> 
> > mxc-ehci mxc-ehci.0: GetStatus port:1 status 001803 0  ACK POWER sig=j
> > CSC CONNECT
> > hub 1-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101
> > mxc-ehci mxc-ehci.0: port 1 high speed
> > mxc-ehci mxc-ehci.0: GetStatus port:1 status 8001205 4  ACK POWER sig=se0
> > LPM PE CONNECT
> > usb 1-1: new high-speed USB device number 24 using mxc-ehci
> > mxc-ehci mxc-ehci.0: detected XactErr len 0/8 retry 1
> 
> ...
> 
> > mxc-ehci mxc-ehci.0: devpath 1 ep0in 3strikes
> 
> These messages mean that there was a low-level communication error.
> The controller did not receive a response from the device.
> 
> > mxc-ehci mxc-ehci.0: GetStatus port:1 status c00100a 6  ACK POWER sig=se0
> > PEC CSC
> 
> The "c00100a" bits are explained in Table 2-16 of the EHCI spec and the
> EHCI-1.1 addendum.  The other fields are merely symbolic names for the bits
> in the status value.
Ah ok this is the status field i was talking about. The device is an USB stick 
doing file operations. So right after the disconnect the stick is not 
answering, but later it is working again. So after the disconnect the stick 
probably needs some time to reset?

> > hub 1-0:1.0: unable to enumerate USB device on port 1
> > hub 1-0:1.0: state 7 ports 1 chg  evt 0002
> 
> "state 7" means the root hub is in the Configured state.  "evt 0002"
> means an event has occurred on port 1.  These values are not documented
> anywhere because they are used only for debugging the kernel.
Ok but they are deferred from the flags if i understand the sourcecode 
correctly?

Best regards
Tim
--
To unsubscribe from this list: send the line "unsubscribe 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] usbhid: use GFP_NOIO in reset code path

2012-10-11 Thread Ming Lei
On Thu, Oct 11, 2012 at 12:00 AM, Ming Lei  wrote:
>
> Keeping allowed gfp_flag inside task_struct should be one solution, and
> let that teach mm to allocate memory, see the draft idea below:
>
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index c0543c8..781447f 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -1329,11 +1329,13 @@ EXPORT_SYMBOL_GPL(usb_stor_Bulk_reset);
>  int usb_stor_port_reset(struct us_data *us)
>  {
> int result;
> +   gfp_t orig_gfp = tsk_get_allowd_gfp(current);
>
> /*for these devices we must use the class specific method */
> if (us->pusb_dev->quirks & USB_QUIRK_RESET)
> return -EPERM;
>
> +   tsk_set_allowd_gfp(current, orig_gfp & ~GFP_IOFS);
> result = usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf);
> if (result < 0)
> US_DEBUGP("unable to lock device for reset: %d\n", result);
> @@ -1349,5 +1351,6 @@ int usb_stor_port_reset(struct us_data *us)
> }
> usb_unlock_device(us->pusb_dev);
> }
> +   tsk_set_allowd_gfp(current, orig_gfp);
> return result;

In fact, the error handling case may be generalized to the context
which is doing the actual IO transfer, and the change on mass storage
should be as below. We can find other similar situations too, such as
mmc thread(mmc_queue_thread).

I will study the problem further so that more block drivers or
subsystem can benefit from the idea, which may help to persuade
LKML people to accept it.

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 12aa726..29ad4ac 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -297,7 +297,9 @@ static int usb_stor_control_thread(void * __us)
 {
struct us_data *us = (struct us_data *)__us;
struct Scsi_Host *host = us_to_host(us);
+   gfp_t orig_gfp = tsk_get_allowd_gfp(current);

+   tsk_set_allowd_gfp(current, orig_gfp & ~GFP_IOFS);
for (;;) {
US_DEBUGP("*** thread sleeping.\n");
if (wait_for_completion_interruptible(&us->cmnd_ready))
@@ -404,6 +406,7 @@ SkipForAbort:
/* unlock the device pointers */
mutex_unlock(&us->dev_mutex);
} /* for (;;) */
+   tsk_set_allowd_gfp(current, orig_gfp);

/* Wait until we are told to stop */
for (;;) {


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


Re: usb audio device can't work when it is attached to host via usb 2.0 hub

2012-10-11 Thread Elric Fu
2012/10/11 Alan Stern :
> On Thu, 11 Oct 2012, Elric Fu wrote:
>
>> Hi all,
>>
>> I recently found a strange issue on a embedded platform and don't know
>> how to fix it. Please give me some suggestions.
>>
>> The platform is ar9331. The issue is if we attach a usb audio device to
>> the host on ar9331 directly or via a usb 1.1 hub the usb audio works fine,
>> but if we attach it via usb 2.0 hub the undermentioned message will be
>> reported. Moreover, when the usb audio was attached to the host on X86
>> via usb 2.0 hub it worked fine too.
>>
>> ~ # usb 1-1.3: new full speed USB device using ar7240-ehci and address 3
>> usb 1-1.3: unable to read config index 0 descriptor/start: -145
>> usb 1-1.3: chopping to 0 config(s)
>> usb 1-1.3: string descriptor 0 read error: -145
>> usb 1-1.3: New USB device found, idVendor=040d, idProduct=3400
>> usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=0
>> usb 1-1.3: no configuration chosen from 0 choices
>>
>> I tracked the issue and found the usb audio had a strange issue. It is a
>> usb 1.1 device and works at full speed. But the interesting thing is the
>> bcdUSB field of device descriptor is 0x0200.
>
> That means it is a USB-2.0 device, not USB-1.1.

Well. I see.

>
>>  So the
>> hub_port_connect_change() will involke the check_highspeed() which
>> would get device qualifier descriptor.
>>
>> The correct case is the device doesn't have the device qualifier descriptor
>
> That's okay.  It means the device doesn't support high-speed operation,
> only full speed.
>
>> and usb_get_descriptor() will retry 3 times and return -32. But now
>> usb_get_descriptor() will return -145 at the second time. Then when the
>
> What is error -145 on your platform?  Is it -ETIMEDOUT?

Yes. -145 means -ETIMEOUT.

>
>> usb core send a get configuration descriptor request, but the control 
>> transfer
>> will be timeout again.
>>
>> I used the usb analyzer to capture the trace and found host just sent one
>> get-device-qualifier-descriptor control transfer and the device return a
>> STALL. But at the second time, actually, the host even didn't send the setup
>> packet. I don't know why.
>
> Since the transfer uses aplit transactions, the host doesn't send any
> SETUP packets at all.  The intervening hub sends them.  Maybe there's
> something wrong with the hub.
>
> Can you use your analyzer to capture the traffic between the computer
> and the hub, rather than between the hub and the device?

Yes. I have already capture the traffic between the host and the hub.Same
situation would happened. I can't found any packets belong to the second
get-device-qualifier-descriptor control transfer.

>
>> My opinion is the split transactions to the device which has a 0x0200 bcdUSB
>> but is actually a full speed device have some issues. I would
>> appreciate any ideas.
>
> What happens if you use both the device and the same hub with a regular
> PC?

If I use the same device and the same hub with a regular pc, everything is
normal. The issue just occurred on Atheros ar9331 platform.

>
> 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: usb audio device can't work when it is attached to host via usb 2.0 hub

2012-10-11 Thread Elric Fu
2012/10/11 Peter Stuge :
> Elric Fu wrote:
>> It is a usb 1.1 device and works at full speed.
>
> It is a frequent misunderstanding that a device must be 1.1 only
> because it does not support high speed. This is not the case.
>
>
>> But the interesting thing is the bcdUSB field of device descriptor
>> is 0x0200.
>
> It is perfectly legal for a USB 2.0 device to only support low or
> full speed.
>
>
> I'm not sure what the problem is with this host or device, just
> mentioning that a device can be USB 2.0 and only support lower
> speeds.
>
> It amazes me how many people reduce USB 2.0 to mean high speed.
> The standard took several years to develop, and it contains a lot
> more than the added high speed communication.
>

I see. Thanks a lot.

>
> //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 01/12] usbnet: introduce usbnet 3 command helpers

2012-10-11 Thread Ming Lei
On Thu, Oct 11, 2012 at 10:36 PM, Alan Stern  wrote:

>
> It's worse than you may realize.  When a SCSI disk is suspended, all of
> its ancestor devices may be suspended too.  Pages can't be read in from
> the drive until all those ancestors are resumed.  This means that all
> runtime resume code paths for all drivers that could be bound to an
> ancestor of a block device must avoid GFP_KERNEL.  In practice it's

Exactly, so several subsystems(for example, pci, usb, scsi) will be involved,
and converting GFP_KERNEL in runtime PM path to GFP_NOIO becomes
more difficult.

> probably easiest for the runtime PM core to use tsk_set_allowd_gfp()
> before calling any runtime_resume method.

Yes, it might be done in usb runtime resume context because all
usb device might include a mass storage interface. But, in fact,
we can find if there is one mass storage interface on the current
configuration easily inside usb_runtime_resume().

Also, we can loose the constraint in runtime PM core, before calling
runtime_resume callback for one device, the current context is marked
as ~GFP_IOFS only if it is a block device or there is one block device
descendant. But the approach becomes a bit complicated because
device tree traversing is involved.


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


[PATCH] usb: renesas_usbhs: fixup __usbhs_for_each_pipe 1st pos

2012-10-11 Thread Kuninori Morimoto
1st pos of __usbhs_for_each_pipe() was wrong.
Each pipe were pipe0, pipe2, pipe3 ...
This patch modifies it.

Signed-off-by: Kuninori Morimoto 
---
 drivers/usb/renesas_usbhs/pipe.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/pipe.h b/drivers/usb/renesas_usbhs/pipe.h
index 08786c0..3d80c7b 100644
--- a/drivers/usb/renesas_usbhs/pipe.h
+++ b/drivers/usb/renesas_usbhs/pipe.h
@@ -54,7 +54,7 @@ struct usbhs_pipe_info {
  * pipe list
  */
 #define __usbhs_for_each_pipe(start, pos, info, i) \
-   for (i = start, pos = (info)->pipe; \
+   for (i = start, pos = (info)->pipe + i; \
 i < (info)->size;  \
 i++, pos = (info)->pipe + i)
 
-- 
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: About resume time optimization for bus resume routine

2012-10-11 Thread Peter Chen
On Thu, Oct 11, 2012 at 03:20:51PM -0400, Alan Stern wrote:
> On Thu, 11 Oct 2012, Peter Chen wrote:
> 
> 
> > >   There is a port on the root hub with suspend status clear
> > >   and enabled status set (which implies there must be a device
> > >   attached to the port, because disconnected ports can't be
> > >   enabled).
> > > 
> > > Obviously if a port isn't enabled then it doesn't need any delay.
> > Is it ok to set one hcd flag, like hcd->unsuspended_device_on_port
> > to get condition at xxx_bus_suspend?
> 
> Only if you're willing to change all the host controller drivers to 
> make them set that flag!

Taking EHCI controller as an example, it just needs to change ehci_bus_resume,
if there is any enabled, unsuspended port, set hcd->unsuspended_device_on_port
> 
> usb_bus_resume() can poll the port statuses to see if there are any 
> enabled, unsuspended ports.  If there aren't any, the delay can be 
> skipped.
Taking EHCI controller as an example, you mean:

- Change hcd_bus_resume like below:

status = hcd->driver->bus_resume(hcd);
clear_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
if (status == 0) {
charbuffer[6];

status = hcd->driver->hub_status_data(hcd, buffer);
if (status != 0) {
/* There are any enabled unsuspended ports */
/* TRSMRCY = 10 msec */
msleep(10);
spin_lock_irq(&hcd_root_hub_lock);
- Add get any enabled, unsuspended port as port change condition at
ehci_hub_status_data.

> 
> How does that sound?

So, you prefer the latter one?

-- 

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] usb: renesas_usbhs: gadget: add usb_gadget_ops :: pullup support

2012-10-11 Thread Kuninori Morimoto
Signed-off-by: Kuninori Morimoto 
---
 drivers/usb/renesas_usbhs/common.c |5 +
 drivers/usb/renesas_usbhs/common.h |1 +
 drivers/usb/renesas_usbhs/mod_gadget.c |   11 +++
 3 files changed, 17 insertions(+)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index 072edc1..3bf922ab 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -132,6 +132,11 @@ void usbhs_sys_function_ctrl(struct usbhs_priv *priv, int 
enable)
usbhs_bset(priv, SYSCFG, mask, enable ? val : 0);
 }
 
+void usbhs_sys_function_pullup(struct usbhs_priv *priv, int enable)
+{
+   usbhs_bset(priv, SYSCFG, DPRPU, enable ? DPRPU : 0);
+}
+
 void usbhs_sys_set_test_mode(struct usbhs_priv *priv, u16 mode)
 {
usbhs_write(priv, TESTMODE, mode);
diff --git a/drivers/usb/renesas_usbhs/common.h 
b/drivers/usb/renesas_usbhs/common.h
index dddf40a..c69dd2f 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -285,6 +285,7 @@ void usbhs_bset(struct usbhs_priv *priv, u32 reg, u16 mask, 
u16 data);
  */
 void usbhs_sys_host_ctrl(struct usbhs_priv *priv, int enable);
 void usbhs_sys_function_ctrl(struct usbhs_priv *priv, int enable);
+void usbhs_sys_function_pullup(struct usbhs_priv *priv, int enable);
 void usbhs_sys_set_test_mode(struct usbhs_priv *priv, u16 mode);
 
 /*
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c 
b/drivers/usb/renesas_usbhs/mod_gadget.c
index 28478ce..dd41f61 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -883,6 +883,16 @@ static int usbhsg_get_frame(struct usb_gadget *gadget)
return usbhs_frame_get_num(priv);
 }
 
+static int usbhsg_pullup(struct usb_gadget *gadget, int is_on)
+{
+   struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+   struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+
+   usbhs_sys_function_pullup(priv, is_on);
+
+   return 0;
+}
+
 static int usbhsg_set_selfpowered(struct usb_gadget *gadget, int is_self)
 {
struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
@@ -900,6 +910,7 @@ static struct usb_gadget_ops usbhsg_gadget_ops = {
.set_selfpowered= usbhsg_set_selfpowered,
.udc_start  = usbhsg_gadget_start,
.udc_stop   = usbhsg_gadget_stop,
+   .pullup = usbhsg_pullup,
 };
 
 static int usbhsg_start(struct usbhs_priv *priv)
-- 
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] usb: renesas_usbhs: fixup interrupt status clear method

2012-10-11 Thread Kuninori Morimoto
When interrupt happened, renesas_usbhs driver gets irq status
by usbhs_status_get_each_irq(), and cleared all status by using 0.
But, this method is incorrect,
since extra interrupt might occur between them.
This patch cleared corresponding bits only

Signed-off-by: Kuninori Morimoto 
---
 drivers/usb/renesas_usbhs/mod.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
index 35c5208..61933a9 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -273,9 +273,9 @@ static irqreturn_t usbhs_interrupt(int irq, void *data)
usbhs_write(priv, INTSTS0, ~irq_state.intsts0 & INTSTS0_MAGIC);
usbhs_write(priv, INTSTS1, ~irq_state.intsts1 & INTSTS1_MAGIC);
 
-   usbhs_write(priv, BRDYSTS, 0);
-   usbhs_write(priv, NRDYSTS, 0);
-   usbhs_write(priv, BEMPSTS, 0);
+   usbhs_write(priv, BRDYSTS, ~irq_state.brdysts);
+   usbhs_write(priv, NRDYSTS, ~irq_state.nrdysts);
+   usbhs_write(priv, BEMPSTS, ~irq_state.bempsts);
 
/*
 * call irq callback functions
-- 
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