Bug 88151 - PCI device intel xhci with 64-bit bar address can NOT work on x86_32 PAE kernel

2014-11-13 Thread Aaron Ma
x86_32 kernel config

This is associated with [Bug 88131]
The bootlog is already in Bug 88131.

Kernel enabled PAE and HIGHMEM64G.
PCI bar0 base address is above 4G, xhci driver failed to work.
Yinghai Lu has already made some changes, please check it on bug 88131.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net-next 2/2] r8152: adjust rtl_start_rx

2014-11-13 Thread Hayes Wang
David Miller [mailto:da...@davemloft.net] 
> Sent: Friday, November 14, 2014 5:23 AM
[...]
> What if even the first r8152_submit_rx() fails?  What ever will cause
> any of these retries to trigger at all?

According to the patch #1 "adjust r8152_submit_rx", the
r8152_submit_rx() would add the rx to the list and schedule
the tasklet, when the error occurs. Each time the tasklet is
called, the rx_bottom() would deal with all the rx in the
list. If the actual_length isn't vaild, the rx buffer would be
submitted directly. By this way, the retries would be done.
That is, the retries would be triggered when the tasklet
is called. Therefore, any tx, rx, and tasklet scheduling
would result in the retries.

> Second, why does your patch increment 'i' with 'i++;' in the error
> break path?  You should mark the first failed entry as unallocated
> with actual_length == 0 and place it on the rx_done queue.

Because the r8152_submit_rx() would add the failed rx to
the list, I only have to deal with the remaining ones. That
is why I increase the "i", otherwise the failed one would
be added twice.

I remember the usb_submit_urb() would set actual_length
to 0, so I skip the step. I would check it again.
 
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] usb: dwc2: add bus suspend/resume for dwc2

2014-11-13 Thread Julius Werner
> I will figure out how to make dwc2 detect the device connect after auto
> suspend,
> or disable the auto suspend feature for the dwc2 hcd.

I think auto-suspend of the root hub device (which is what calls
bus_suspend, but is not the host controller device itself) is expected
to always happen and not really meant to be disabled. I'm surprised
that the controller would fail to come back up, though. Does removing
the PCGCTL part make it work? That's the only thing I can think of
(but then again the function should immediately return if the port is
not in L0, so if there is nothing plugged in the suspend shouldn't
really do anything).

Another thing might be that the port connect interrupt does not
correctly resume the root hub. I don't really know many details about
how that works, and it seems pretty complicated. But I can see that
all other HCDs seem to call usb_hcd_resume_root_hub() from their
interrupt handlers, which we don't. There's also a
usb_hcd_start_port_resume() in EHCI which I'm not familiar with, that
seems to have been added recently. And finally, it seems that all
normal host controllers (UHCI/OHCI/EHCI/XHCI) now do the
usb_hcd->uses_new_polling thing (where you're supposed to call
hcd_poll_rh_status() from the HCD code)... the old polling code still
seems to be in place, but without any relevant driver using I wouldn't
be so sure if it's still functional.

+Alan who should know HCD/core interactions much better and might have
some ideas what the DWC2 driver is still missing right now.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] USB: host: Introduce flag to enable use of 64-bit dma_mask for ehci-platform

2014-11-13 Thread Florian Fainelli
2014-11-13 13:36 GMT-08:00 Andreas Herrmann
:
> ehci-octeon driver used a 64-bit dma_mask. With removal of ehci-octeon
> and usage of ehci-platform ehci dma_mask is now limited to 32 bits
> (coerced in ehci_platform_probe).
>
> Provide a flag in ehci platform data to allow use of 64 bits for
> dma_mask.

Why not just allow enforcing an arbitrary DMA mask?

>
> Cc: David Daney 
> Cc: Alex Smith 
> Cc: Alan Stern 
> Signed-off-by: Andreas Herrmann 
> ---
>  arch/mips/cavium-octeon/octeon-platform.c |4 +---
>  drivers/usb/host/ehci-platform.c  |3 ++-
>  include/linux/usb/ehci_pdriver.h  |1 +
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
> b/arch/mips/cavium-octeon/octeon-platform.c
> index eea60b6..12410a2 100644
> --- a/arch/mips/cavium-octeon/octeon-platform.c
> +++ b/arch/mips/cavium-octeon/octeon-platform.c
> @@ -310,6 +310,7 @@ static struct usb_ehci_pdata octeon_ehci_pdata = {
>  #ifdef __BIG_ENDIAN
> .big_endian_mmio= 1,
>  #endif
> +   .dma_mask_64= 1,
> .power_on   = octeon_ehci_power_on,
> .power_off  = octeon_ehci_power_off,
>  };
> @@ -331,8 +332,6 @@ static void __init octeon_ehci_hw_start(struct device 
> *dev)
> octeon2_usb_clocks_stop();
>  }
>
> -static u64 octeon_ehci_dma_mask = DMA_BIT_MASK(64);
> -
>  static int __init octeon_ehci_device_init(void)
>  {
> struct platform_device *pd;
> @@ -347,7 +346,6 @@ static int __init octeon_ehci_device_init(void)
> if (!pd)
> return 0;
>
> -   pd->dev.dma_mask = &octeon_ehci_dma_mask;
> pd->dev.platform_data = &octeon_ehci_pdata;
> octeon_ehci_hw_start(&pd->dev);
>
> diff --git a/drivers/usb/host/ehci-platform.c 
> b/drivers/usb/host/ehci-platform.c
> index 2da18ea..6df808b 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -155,7 +155,8 @@ static int ehci_platform_probe(struct platform_device 
> *dev)
> if (!pdata)
> pdata = &ehci_platform_defaults;
>
> -   err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
> +   err = dma_coerce_mask_and_coherent(&dev->dev,
> +   pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
> if (err)
> return err;
>
> diff --git a/include/linux/usb/ehci_pdriver.h 
> b/include/linux/usb/ehci_pdriver.h
> index 7eb4dcd..f69529e 100644
> --- a/include/linux/usb/ehci_pdriver.h
> +++ b/include/linux/usb/ehci_pdriver.h
> @@ -45,6 +45,7 @@ struct usb_ehci_pdata {
> unsignedbig_endian_desc:1;
> unsignedbig_endian_mmio:1;
> unsignedno_io_watchdog:1;
> +   unsigneddma_mask_64:1;
>
> /* Turn on all power and clocks */
> int (*power_on)(struct platform_device *pdev);
> --
> 1.7.9.5
>
>



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe 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: chipidea: parameter 'mode' isn't needed for hw_device_reset

2014-11-13 Thread Felipe Balbi
On Fri, Nov 14, 2014 at 08:03:16AM +0800, Peter Chen wrote:
> The hw_device_reset is dedicated to be used at device mode initializaiton,
> so delete the parameter 'mode'. For host driver, the ehci driver will
> all things.

this last sentence doesn't parse very well.

> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/chipidea/ci.h  | 2 +-
>  drivers/usb/chipidea/core.c| 8 
>  drivers/usb/chipidea/otg_fsm.c | 2 +-
>  drivers/usb/chipidea/udc.c | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 5bbfcc7..65913d4 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -352,7 +352,7 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci);
>  
>  u32 hw_read_intr_status(struct ci_hdrc *ci);
>  
> -int hw_device_reset(struct ci_hdrc *ci, u32 mode);
> +int hw_device_reset(struct ci_hdrc *ci);
>  
>  int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index dffd89b..053ab4f 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -410,7 +410,7 @@ static int hw_controller_reset(struct ci_hdrc *ci)
>*
>   * This function returns an error code
>   */
> -int hw_device_reset(struct ci_hdrc *ci, u32 mode)
> +int hw_device_reset(struct ci_hdrc *ci)
>  {
>   int ret;
>  
> @@ -440,12 +440,12 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
>  
>   /* USBMODE should be configured step by step */
>   hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
> - hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
> + hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_DC);
>   /* HW >= 2.3 */
>   hw_write(ci, OP_USBMODE, USBMODE_SLOM, USBMODE_SLOM);
>  
> - if (hw_read(ci, OP_USBMODE, USBMODE_CM) != mode) {
> - pr_err("cannot enter in %s mode", ci_role(ci)->name);
> + if (hw_read(ci, OP_USBMODE, USBMODE_CM) != USBMODE_CM_DC) {
> + pr_err("cannot enter in %s device mode", ci_role(ci)->name);
>   pr_err("lpm = %i", ci->hw_bank.lpm);
>   return -ENODEV;
>   }
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 21058ae..e8c936d 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -543,7 +543,7 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int on)
>   ci_role_start(ci, CI_ROLE_HOST);
>   } else {
>   ci_role_stop(ci);
> - hw_device_reset(ci, USBMODE_CM_DC);
> + hw_device_reset(ci);
>   ci_role_start(ci, CI_ROLE_GADGET);
>   }
>   mutex_lock(&fsm->lock);
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index bdaa7ba..4fe18ce 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1471,7 +1471,7 @@ static int ci_udc_vbus_session(struct usb_gadget 
> *_gadget, int is_active)
>   if (gadget_ready) {
>   if (is_active) {
>   pm_runtime_get_sync(&_gadget->dev);
> - hw_device_reset(ci, USBMODE_CM_DC);
> + hw_device_reset(ci);
>   hw_device_state(ci, ci->ep0out->qh.dma);
>   usb_gadget_set_state(_gadget, USB_STATE_POWERED);
>   } else {
> @@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
>   pm_runtime_get_sync(&ci->gadget.dev);
>   if (ci->vbus_active) {
>   spin_lock_irqsave(&ci->lock, flags);
> - hw_device_reset(ci, USBMODE_CM_DC);
> + hw_device_reset(ci);

wow, you guys reset this IP all the time... that's scary, could be
hiding bugs easily.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v2 1/3] usb: chipidea: add controller reset API

2014-11-13 Thread Felipe Balbi
Hi,

On Fri, Nov 14, 2014 at 08:03:15AM +0800, Peter Chen wrote:
> Add controller reset API, it may be used for host/otg driver in future.

you need a better commit log here. How would it be used ? when ? And,
more importantly, why ?

Why would we need to reset the IP outside of ->probe() ?

I don't oppose to the refactoring, but why would we ever need to reset
this IP ?

> Signed-off-by: Peter Chen 
> ---
> 
> Changes for v2:
> - Add return value check for controller reset at hw_device_reset
> 
>  drivers/usb/chipidea/core.c | 30 +++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index bd74f27..dffd89b 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -385,6 +385,26 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
>  }
>  
>  /**
> + * hw_controller_reset: do controller reset
> + * @ci: the controller
> +  *

minor nit: indentation

> + * This function returns an error code
> + */
> +static int hw_controller_reset(struct ci_hdrc *ci)
> +{
> + int count = 0;
> +
> + hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
> + while (hw_read(ci, OP_USBCMD, USBCMD_RST)) {
> + udelay(10);
> + if (count++ > 1000)
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +/**
>   * hw_device_reset: resets chip (execute without interruption)
>   * @ci: the controller
>*
> @@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
>   */
>  int hw_device_reset(struct ci_hdrc *ci, u32 mode)
>  {
> + int ret;
> +
>   /* should flush & stop before reset */
>   hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
>   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
>  
> - hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
> - while (hw_read(ci, OP_USBCMD, USBCMD_RST))
> - udelay(10); /* not RTOS friendly */
> + ret = hw_controller_reset(ci);
> + if (ret) {
> + dev_err(ci->dev, "error for reset, ret=%d\n", ret);
> + return ret;
> + }
>  
>   if (ci->platdata->notify_event)
>   ci->platdata->notify_event(ci,
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v4 0/3] Rework "xhci: clear root port wake on bits if controller isn't wake-up capable"

2014-11-13 Thread Lu, Baolu

Hi Mathias,

This patch series has been acked by Alan Stern. There seems no further 
comments from others. Can you please pull in it?


Thanks,
-baolu

On 2014年11月06日 10:50, Lu Baolu wrote:

This serie of patch reworks commit ff8cbf250b448aac35589f6075082c3fcad8a8fe.
This has been discussed at http://www.spinics.net/lists/linux-usb/msg114986.html
It also includes a patch to fix a comment in drivers/usb/host/xhci.h.

Changes in v4:
- Refine xhci_disable_port_wake_on_bits().
- Add "Acked-by: Alan Stern"

Changes in v3:
- Need to consider run-time suspend as well.
- Change "wake-up capable" to "allowed to do wakeup"
in both comments and patch description.
- Add "Suggested-by: Alan Stern"

Changes in v2:
- Should not be a quirk.
- Should be applied to all xhci controllers.

Lu Baolu (3):
   usb: xhci: Revert "xhci: clear root port wake on bits if controller
 isn't wake-up capable"
   usb: xhci: This reworks ff8cbf250b448aac35589f6075082c3fcad8a8fe
   usb: xhci: fix comment for PORT_DEV_REMOVE

  drivers/usb/host/xhci-hub.c  |  5 +
  drivers/usb/host/xhci-pci.c  |  2 +-
  drivers/usb/host/xhci-plat.c | 10 +-
  drivers/usb/host/xhci.c  | 42 +-
  drivers/usb/host/xhci.h  |  4 ++--
  5 files changed, 54 insertions(+), 9 deletions(-)



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


Re: [PATCH 1/6] usb: chipidea: add a core function to setup ci_hdrc_platform_data

2014-11-13 Thread Peter Chen
On Thu, Oct 30, 2014 at 12:36:42PM +0100, Antoine Tenart wrote:
> Add a function into the chipidea core to help drivers setup the internal
> ci_hdrc_platform_data structure. This helps not duplicating common code.
> 
> The ci_hdrc_get_platdata function only setup non filled members of the
> structure so that is is possible to give an already filled one. This is
> what the ci_pdata_default parameter is for.
> 
> Signed-off-by: Antoine Tenart 
> ---
>  drivers/usb/chipidea/core.c  | 129 
> +++
>  include/linux/usb/chipidea.h |   2 +
>  2 files changed, 131 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index ba0ac2723098..0ad55c10a903 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -535,6 +535,135 @@ static int ci_get_platdata(struct device *dev,
>   return 0;
>  }
>  
> +/*
> + * Getting a PHY or an USB PHY is optional:
> + * If no PHY or USB PHY is found, or if their subsystems aren't enabled,
> + * PHY and/or USB PHY will be set to NULL. Otherwise returns an error.
> + */
> +static int ci_hdrc_get_phy(struct device *dev,
> +struct ci_hdrc_platform_data *ci_pdata)
> +{
> + ci_pdata->phy = devm_phy_get(dev, "usb");
> + ci_pdata->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +
> + if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER ||
> + PTR_ERR(ci_pdata->usb_phy) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (IS_ERR(ci_pdata->phy)) {
> + if (PTR_ERR(ci_pdata->phy) == -ENOSYS ||
> + PTR_ERR(ci_pdata->phy) == -ENODEV) {
> + ci_pdata->phy = NULL;
> + } else {
> + dev_err(dev, "Could not get PHY: %ld\n",
> + PTR_ERR(ci_pdata->phy));
> + return PTR_ERR(ci_pdata->phy);
> + }
> + }
> +
> + if (IS_ERR(ci_pdata->usb_phy)) {
> + if (PTR_ERR(ci_pdata->usb_phy) == -ENXIO ||
> + PTR_ERR(ci_pdata->usb_phy) == -ENODEV) {
> + ci_pdata->usb_phy = NULL;
> + } else {
> + dev_err(dev, "Could not get USB PHY: %ld\n",
> + PTR_ERR(ci_pdata->usb_phy));
> + return PTR_ERR(ci_pdata->usb_phy);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ci_hdrc_get_usb_phy_mode(struct device *dev,
> + struct ci_hdrc_platform_data *ci_pdata)
> +{
> + if (!ci_pdata->phy_mode)
> + ci_pdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
> +
> + if (!ci_pdata->dr_mode)
> + ci_pdata->dr_mode = of_usb_get_dr_mode(dev->of_node);
> +
> + if (of_usb_get_maximum_speed(dev->of_node) == USB_SPEED_FULL)
> + ci_pdata->flags |= CI_HDRC_FORCE_FULLSPEED;
> +
> + return 0;
> +}
> +

The things in above function are no relationship.

> +/*
> + * Getting a regulator is optional:
> + * If no regulator is found, or if the regulator subsystem isn't enabled,
> + * the regulator will be set to NULL. Otherwise returns an error.
> + */
> +static int ci_hdrc_get_regulator(struct device *dev,
> +  struct ci_hdrc_platform_data *ci_pdata)
> +{
> + ci_pdata->reg_vbus = devm_regulator_get(dev, "vbus");
> +
> + if (IS_ERR(ci_pdata->reg_vbus)) {
> + if (PTR_ERR(ci_pdata->reg_vbus) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (PTR_ERR(ci_pdata->reg_vbus) == -ENODEV) {
> + ci_pdata->reg_vbus = NULL;
> + } else {
> + dev_err(dev, "Could not get regulator for vbus: %ld\n",
> + PTR_ERR(ci_pdata->reg_vbus));
> + return PTR_ERR(ci_pdata->reg_vbus);
> + }
> + }
> +
> + return 0;
> +}
> +
> +struct ci_hdrc_platform_data *ci_hdrc_get_platdata(struct device *dev,
> + struct ci_hdrc_platform_data *ci_pdata_default)
> +{
> + struct ci_hdrc_platform_data *ci_pdata;
> + int ret;
> +
> + if (!ci_pdata_default) {
> + ci_pdata = devm_kzalloc(dev, sizeof(*ci_pdata), GFP_KERNEL);
> + if (!ci_pdata)
> + return ERR_PTR(-ENOMEM);
> + } else {
> + ci_pdata = ci_pdata_default;
> + }
> +
> + if (!ci_pdata->name)
> + ci_pdata->name = dev_name(dev);
> +
> + if (!ci_pdata->phy && !ci_pdata->usb_phy) {
> + ret = ci_hdrc_get_phy(dev, ci_pdata);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + if (ci_pdata->usb_phy) {
> + ret = ci_hdrc_get_usb_phy_mode(dev, ci_pdata);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + if (ci_pdata->dr_mode == USB_DR_MODE_UNKNOWN)
> + 

Re: [RESUBMIT] [PATCH] Replace mentions of "list_struct" to "list_head"

2014-11-13 Thread Steven Rostedt
On Fri, 14 Nov 2014 05:09:55 +0400
Andrey Utkin  wrote:

> There's no such thing as "list_struct".

I guess there isn't.

> 
> Signed-off-by: Andrey Utkin 

Acked-by: Steven Rostedt 

-- Steve

> ---
>  drivers/gpu/drm/radeon/mkregtable.c  | 24 
>  drivers/media/pci/cx18/cx18-driver.h |  2 +-
>  include/linux/list.h | 34 +-
>  include/linux/plist.h| 10 +-
>  include/linux/rculist.h  |  8 
>  scripts/kconfig/list.h   |  6 +++---
>  tools/usb/usbip/libsrc/list.h|  2 +-
>  7 files changed, 43 insertions(+), 43 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 2/3] usb: chipidea: parameter 'mode' isn't needed for hw_device_reset

2014-11-13 Thread Peter Chen
The hw_device_reset is dedicated to be used at device mode initializaiton,
so delete the parameter 'mode'. For host driver, the ehci driver will
all things.

Signed-off-by: Peter Chen 
---
 drivers/usb/chipidea/ci.h  | 2 +-
 drivers/usb/chipidea/core.c| 8 
 drivers/usb/chipidea/otg_fsm.c | 2 +-
 drivers/usb/chipidea/udc.c | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 5bbfcc7..65913d4 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -352,7 +352,7 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci);
 
 u32 hw_read_intr_status(struct ci_hdrc *ci);
 
-int hw_device_reset(struct ci_hdrc *ci, u32 mode);
+int hw_device_reset(struct ci_hdrc *ci);
 
 int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
 
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index dffd89b..053ab4f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -410,7 +410,7 @@ static int hw_controller_reset(struct ci_hdrc *ci)
   *
  * This function returns an error code
  */
-int hw_device_reset(struct ci_hdrc *ci, u32 mode)
+int hw_device_reset(struct ci_hdrc *ci)
 {
int ret;
 
@@ -440,12 +440,12 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
 
/* USBMODE should be configured step by step */
hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_IDLE);
-   hw_write(ci, OP_USBMODE, USBMODE_CM, mode);
+   hw_write(ci, OP_USBMODE, USBMODE_CM, USBMODE_CM_DC);
/* HW >= 2.3 */
hw_write(ci, OP_USBMODE, USBMODE_SLOM, USBMODE_SLOM);
 
-   if (hw_read(ci, OP_USBMODE, USBMODE_CM) != mode) {
-   pr_err("cannot enter in %s mode", ci_role(ci)->name);
+   if (hw_read(ci, OP_USBMODE, USBMODE_CM) != USBMODE_CM_DC) {
+   pr_err("cannot enter in %s device mode", ci_role(ci)->name);
pr_err("lpm = %i", ci->hw_bank.lpm);
return -ENODEV;
}
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 21058ae..e8c936d 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -543,7 +543,7 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int on)
ci_role_start(ci, CI_ROLE_HOST);
} else {
ci_role_stop(ci);
-   hw_device_reset(ci, USBMODE_CM_DC);
+   hw_device_reset(ci);
ci_role_start(ci, CI_ROLE_GADGET);
}
mutex_lock(&fsm->lock);
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index bdaa7ba..4fe18ce 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1471,7 +1471,7 @@ static int ci_udc_vbus_session(struct usb_gadget 
*_gadget, int is_active)
if (gadget_ready) {
if (is_active) {
pm_runtime_get_sync(&_gadget->dev);
-   hw_device_reset(ci, USBMODE_CM_DC);
+   hw_device_reset(ci);
hw_device_state(ci, ci->ep0out->qh.dma);
usb_gadget_set_state(_gadget, USB_STATE_POWERED);
} else {
@@ -1660,7 +1660,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
pm_runtime_get_sync(&ci->gadget.dev);
if (ci->vbus_active) {
spin_lock_irqsave(&ci->lock, flags);
-   hw_device_reset(ci, USBMODE_CM_DC);
+   hw_device_reset(ci);
} else {
pm_runtime_put_sync(&ci->gadget.dev);
return retval;
-- 
1.9.1

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


[PATCH 3/3] usb: chipidea: remove duplicate dev_set_drvdata for host_start

2014-11-13 Thread Peter Chen
The core driver has already done it, besides, move set driver data
operation just after ci has allocated successfully in case some
code (like ci_role_start) want to access this driver data.

Signed-off-by: Peter Chen 
---
 drivers/usb/chipidea/core.c | 2 +-
 drivers/usb/chipidea/host.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 053ab4f..fe930bb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -669,6 +669,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (!ci)
return -ENOMEM;
 
+   platform_set_drvdata(pdev, ci);
ci->dev = dev;
ci->platdata = dev_get_platdata(dev);
ci->imx28_write_fix = !!(ci->platdata->flags &
@@ -782,7 +783,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
}
 
-   platform_set_drvdata(pdev, ci);
ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
ci->platdata->name, ci);
if (ret)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 48731d0..c1694cf 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -91,7 +91,6 @@ static int host_start(struct ci_hdrc *ci)
if (!hcd)
return -ENOMEM;
 
-   dev_set_drvdata(ci->dev, ci);
hcd->rsrc_start = ci->hw_bank.phys;
hcd->rsrc_len = ci->hw_bank.size;
hcd->regs = ci->hw_bank.abs;
-- 
1.9.1

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


[RESUBMIT] [PATCH] Replace mentions of "list_struct" to "list_head"

2014-11-13 Thread Andrey Utkin
There's no such thing as "list_struct".

Signed-off-by: Andrey Utkin 
---
 drivers/gpu/drm/radeon/mkregtable.c  | 24 
 drivers/media/pci/cx18/cx18-driver.h |  2 +-
 include/linux/list.h | 34 +-
 include/linux/plist.h| 10 +-
 include/linux/rculist.h  |  8 
 scripts/kconfig/list.h   |  6 +++---
 tools/usb/usbip/libsrc/list.h|  2 +-
 7 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/radeon/mkregtable.c 
b/drivers/gpu/drm/radeon/mkregtable.c
index 4a85bb6..b928c17 100644
--- a/drivers/gpu/drm/radeon/mkregtable.c
+++ b/drivers/gpu/drm/radeon/mkregtable.c
@@ -347,7 +347,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_entry - get the struct for this entry
  * @ptr:   the &struct list_head pointer.
  * @type:  the type of the struct this is embedded in.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  */
 #define list_entry(ptr, type, member) \
container_of(ptr, type, member)
@@ -356,7 +356,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_first_entry - get the first element from a list
  * @ptr:   the list head to take the element from.
  * @type:  the type of the struct this is embedded in.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  *
  * Note, that list is expected to be not empty.
  */
@@ -406,7 +406,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_for_each_entry -   iterate over list of given type
  * @pos:   the type * to use as a loop cursor.
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  */
 #define list_for_each_entry(pos, head, member) \
for (pos = list_entry((head)->next, typeof(*pos), member);  \
@@ -417,7 +417,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_for_each_entry_reverse - iterate backwards over list of given type.
  * @pos:   the type * to use as a loop cursor.
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  */
 #define list_for_each_entry_reverse(pos, head, member) \
for (pos = list_entry((head)->prev, typeof(*pos), member);  \
@@ -428,7 +428,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_prepare_entry - prepare a pos entry for use in 
list_for_each_entry_continue()
  * @pos:   the type * to use as a start point
  * @head:  the head of the list
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  *
  * Prepares a pos entry for use as a start point in 
list_for_each_entry_continue().
  */
@@ -439,7 +439,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_for_each_entry_continue - continue iteration over list of given type
  * @pos:   the type * to use as a loop cursor.
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  *
  * Continue to iterate over list of given type, continuing after
  * the current position.
@@ -453,7 +453,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_for_each_entry_continue_reverse - iterate backwards from the given 
point
  * @pos:   the type * to use as a loop cursor.
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  *
  * Start to iterate over list of given type backwards, continuing after
  * the current position.
@@ -467,7 +467,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * list_for_each_entry_from - iterate over list of given type from the current 
point
  * @pos:   the type * to use as a loop cursor.
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  *
  * Iterate over list of given type, continuing from current position.
  */
@@ -480,7 +480,7 @@ static inline void list_splice_tail_init(struct list_head 
*list,
  * @pos:   the type * to use as a loop cursor.
  * @n: another type * to use as temporary storage
  * @head:  the head for your list.
- * @member:the name of the list_struct within the struct.
+ * @member:the name of the list_head within the struct.
  */
 #define list_for_each_entry_safe(pos, n, head, member

[PATCH v2 1/3] usb: chipidea: add controller reset API

2014-11-13 Thread Peter Chen
Add controller reset API, it may be used for host/otg driver in future.

Signed-off-by: Peter Chen 
---

Changes for v2:
- Add return value check for controller reset at hw_device_reset

 drivers/usb/chipidea/core.c | 30 +++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bd74f27..dffd89b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -385,6 +385,26 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
 }
 
 /**
+ * hw_controller_reset: do controller reset
+ * @ci: the controller
+  *
+ * This function returns an error code
+ */
+static int hw_controller_reset(struct ci_hdrc *ci)
+{
+   int count = 0;
+
+   hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
+   while (hw_read(ci, OP_USBCMD, USBCMD_RST)) {
+   udelay(10);
+   if (count++ > 1000)
+   return -ETIMEDOUT;
+   }
+
+   return 0;
+}
+
+/**
  * hw_device_reset: resets chip (execute without interruption)
  * @ci: the controller
   *
@@ -392,13 +412,17 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
  */
 int hw_device_reset(struct ci_hdrc *ci, u32 mode)
 {
+   int ret;
+
/* should flush & stop before reset */
hw_write(ci, OP_ENDPTFLUSH, ~0, ~0);
hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
 
-   hw_write(ci, OP_USBCMD, USBCMD_RST, USBCMD_RST);
-   while (hw_read(ci, OP_USBCMD, USBCMD_RST))
-   udelay(10); /* not RTOS friendly */
+   ret = hw_controller_reset(ci);
+   if (ret) {
+   dev_err(ci->dev, "error for reset, ret=%d\n", ret);
+   return ret;
+   }
 
if (ci->platdata->notify_event)
ci->platdata->notify_event(ci,
-- 
1.9.1

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


Re: [PATCH V2] usb:host:xhci-plat: 64-bit dma addressing support

2014-11-13 Thread Feng Kan
Just want to ping this again to see if there is any comments, thanks.

On Tue, Nov 4, 2014 at 11:06 AM, Feng Kan  wrote:
> Use dma_addr_t to support 64-bit plaforms, which access beyond the default
> 32 bit address range.
>
> Signed-off-by: Bao Truong 
> Signed-off-by: Feng Kan 
> ---
>  Changes:
> V2: fixed GKH's comment regarding not mark up the comment after
> code change.
>
>  drivers/usb/host/xhci-plat.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3d78b0c..f75764f 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -96,14 +96,18 @@ static int xhci_plat_probe(struct platform_device *pdev)
> return ret;
> }
>
> -   /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> -   ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +   /*
> +* Initialize dma_mask and coherent_dma_mask to valid DMA or bus
> +* address for the platform.
> +*/
> +   ret = dma_set_coherent_mask(&pdev->dev,
> +   DMA_BIT_MASK(sizeof(dma_addr_t)*8));
> if (ret)
> return ret;
> if (!pdev->dev.dma_mask)
> pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> else
> -   dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> +   dma_set_mask(&pdev->dev, DMA_BIT_MASK(sizeof(dma_addr_t)*8));
>
> hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
> if (!hcd)
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] USB: host: Introduce flag to enable use of 64-bit dma_mask for ehci-platform

2014-11-13 Thread Andreas Herrmann
ehci-octeon driver used a 64-bit dma_mask. With removal of ehci-octeon
and usage of ehci-platform ehci dma_mask is now limited to 32 bits
(coerced in ehci_platform_probe).

Provide a flag in ehci platform data to allow use of 64 bits for
dma_mask.

Cc: David Daney 
Cc: Alex Smith 
Cc: Alan Stern 
Signed-off-by: Andreas Herrmann 
---
 arch/mips/cavium-octeon/octeon-platform.c |4 +---
 drivers/usb/host/ehci-platform.c  |3 ++-
 include/linux/usb/ehci_pdriver.h  |1 +
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index eea60b6..12410a2 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -310,6 +310,7 @@ static struct usb_ehci_pdata octeon_ehci_pdata = {
 #ifdef __BIG_ENDIAN
.big_endian_mmio= 1,
 #endif
+   .dma_mask_64= 1,
.power_on   = octeon_ehci_power_on,
.power_off  = octeon_ehci_power_off,
 };
@@ -331,8 +332,6 @@ static void __init octeon_ehci_hw_start(struct device *dev)
octeon2_usb_clocks_stop();
 }
 
-static u64 octeon_ehci_dma_mask = DMA_BIT_MASK(64);
-
 static int __init octeon_ehci_device_init(void)
 {
struct platform_device *pd;
@@ -347,7 +346,6 @@ static int __init octeon_ehci_device_init(void)
if (!pd)
return 0;
 
-   pd->dev.dma_mask = &octeon_ehci_dma_mask;
pd->dev.platform_data = &octeon_ehci_pdata;
octeon_ehci_hw_start(&pd->dev);
 
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 2da18ea..6df808b 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -155,7 +155,8 @@ static int ehci_platform_probe(struct platform_device *dev)
if (!pdata)
pdata = &ehci_platform_defaults;
 
-   err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+   err = dma_coerce_mask_and_coherent(&dev->dev,
+   pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
if (err)
return err;
 
diff --git a/include/linux/usb/ehci_pdriver.h b/include/linux/usb/ehci_pdriver.h
index 7eb4dcd..f69529e 100644
--- a/include/linux/usb/ehci_pdriver.h
+++ b/include/linux/usb/ehci_pdriver.h
@@ -45,6 +45,7 @@ struct usb_ehci_pdata {
unsignedbig_endian_desc:1;
unsignedbig_endian_mmio:1;
unsignedno_io_watchdog:1;
+   unsigneddma_mask_64:1;
 
/* Turn on all power and clocks */
int (*power_on)(struct platform_device *pdev);
-- 
1.7.9.5

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


Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Benson Leung
Hi Alan,

On Thu, Nov 13, 2014 at 2:11 PM, Alan Stern  wrote:
> Wait a minute -- in your previous email you said this approach didn't
> work.  So does it work or doesn't it?

Sorry for the confusion. The approach *does* work.

That was actually my original idea to fix the problem, but I saw other
places in the kernel where it was done with a get/put.


-- 
Benson Leung
Software Engineer, Chrom* OS
ble...@chromium.org
--
To unsubscribe from this list: send the line "unsubscribe 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 0/3] USB: host: Misc patches to remove hard-coded octeon platform information

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Andreas Herrmann wrote:

> Hi Alan,
> 
> With following patches I want to base octeon ehci/ohci device
> configuration on device tree information.
> 
> I picked up patches that were submitted in May. See
> http://marc.info/?l=linux-usb&m=140135823325811&w=2
> and http://marc.info/?l=linux-mips&m=140139694721623&w=2
> 
> Patch #1 is your "untested preliminary pass" to remove
>  [oe]hci-octeon drivers.
> Patch #2 is the removal of hard-coded platform information (but now
>  rebased on your patch)
> Patch #3 adapts dma_mask for ehci (as used in ehci-octeon)
> 
> Overall diffstat is
> 
>  arch/mips/cavium-octeon/octeon-platform.c |  380 
> +++--
>  arch/mips/configs/cavium_octeon_defconfig |3 +
>  drivers/usb/host/Kconfig  |   18 +-
>  drivers/usb/host/Makefile |1 -
>  drivers/usb/host/ehci-hcd.c   |5 -
>  drivers/usb/host/ehci-octeon.c|  188 --
>  drivers/usb/host/ehci-platform.c  |4 +-
>  drivers/usb/host/octeon2-common.c |  200 ---
>  drivers/usb/host/ohci-hcd.c   |5 -
>  drivers/usb/host/ohci-octeon.c|  202 ---
>  drivers/usb/host/ohci-platform.c  |1 +
>  include/linux/usb/ehci_pdriver.h  |1 +
>  12 files changed, 330 insertions(+), 678 deletions(-)
> 
> Patches are based on v3.18-rc4-65-g2c54396
> 
> Comments welcome.

At a very quick first glance, it looks great.  Have you tested it 
thoroughly?

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] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Benson Leung wrote:

> On Thu, Nov 13, 2014 at 1:18 PM, Alan Stern  wrote:
> > If you interchange the two lines then the flag _will_ be cleared before
> > usb_kill_urb() and autosuspend_check() can run.  This means your patch
> > description is bogus.
> 
> Gotcha. v2 posted. Thanks for the direction.

Wait a minute -- in your previous email you said this approach didn't 
work.  So does it work or doesn't it?

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 0/3] USB: host: Misc patches to remove hard-coded octeon platform information

2014-11-13 Thread Andreas Herrmann
Hi Alan,

With following patches I want to base octeon ehci/ohci device
configuration on device tree information.

I picked up patches that were submitted in May. See
http://marc.info/?l=linux-usb&m=140135823325811&w=2
and http://marc.info/?l=linux-mips&m=140139694721623&w=2

Patch #1 is your "untested preliminary pass" to remove
 [oe]hci-octeon drivers.
Patch #2 is the removal of hard-coded platform information (but now
 rebased on your patch)
Patch #3 adapts dma_mask for ehci (as used in ehci-octeon)

Overall diffstat is

 arch/mips/cavium-octeon/octeon-platform.c |  380 +++--
 arch/mips/configs/cavium_octeon_defconfig |3 +
 drivers/usb/host/Kconfig  |   18 +-
 drivers/usb/host/Makefile |1 -
 drivers/usb/host/ehci-hcd.c   |5 -
 drivers/usb/host/ehci-octeon.c|  188 --
 drivers/usb/host/ehci-platform.c  |4 +-
 drivers/usb/host/octeon2-common.c |  200 ---
 drivers/usb/host/ohci-hcd.c   |5 -
 drivers/usb/host/ohci-octeon.c|  202 ---
 drivers/usb/host/ohci-platform.c  |1 +
 include/linux/usb/ehci_pdriver.h  |1 +
 12 files changed, 330 insertions(+), 678 deletions(-)

Patches are based on v3.18-rc4-65-g2c54396

Comments welcome.


Thanks,

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


[PATCH 2/3] USB: host: Remove hard-coded octeon platform information for ehci/ohci

2014-11-13 Thread Andreas Herrmann
Instead rely on device tree information for ehci and ohci.

This was suggested with
http://www.linux-mips.org/archives/linux-mips/2014-05/msg00307.html

  "The device tree will *always* have correct ehci/ohci clock
  configuration, so use it.  This allows us to remove a big chunk of
  platform configuration code from octeon-platform.c."

More or less I rebased that patch on Alan's work to remove ehci-octeon
and ohci-octeon drivers.

Cc: David Daney 
Cc: Alex Smith 
Cc: Alan Stern 
Signed-off-by: Andreas Herrmann 
---
 arch/mips/cavium-octeon/octeon-platform.c |  148 -
 drivers/usb/host/ehci-platform.c  |1 +
 drivers/usb/host/ohci-platform.c  |1 +
 3 files changed, 64 insertions(+), 86 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index b67ddf0..eea60b6 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(octeon2_usb_clocks_mutex);
 
 static int octeon2_usb_clock_start_cnt;
 
-static void octeon2_usb_clocks_start(void)
+static void octeon2_usb_clocks_start(struct device *dev)
 {
u64 div;
union cvmx_uctlx_if_ena if_ena;
@@ -86,6 +86,8 @@ static void octeon2_usb_clocks_start(void)
union cvmx_uctlx_uphy_portx_ctl_status port_ctl_status;
int i;
unsigned long io_clk_64_to_ns;
+   u32 clock_rate = 1200;
+   bool is_crystal_clock = false;
 
 
mutex_lock(&octeon2_usb_clocks_mutex);
@@ -96,6 +98,28 @@ static void octeon2_usb_clocks_start(void)
 
io_clk_64_to_ns = 640ull / octeon_get_io_clock_rate();
 
+   if (dev->of_node) {
+   struct device_node *uctl_node;
+   const char *clock_type;
+
+   uctl_node = of_get_parent(dev->of_node);
+   if (!uctl_node) {
+   dev_err(dev, "No UCTL device node\n");
+   goto exit;
+   }
+   i = of_property_read_u32(uctl_node,
+"refclk-frequency", &clock_rate);
+   if (i) {
+   dev_err(dev, "No UCTL \"refclk-frequency\"\n");
+   goto exit;
+   }
+   i = of_property_read_string(uctl_node,
+   "refclk-type", &clock_type);
+
+   if (!i && strcmp("crystal", clock_type) == 0)
+   is_crystal_clock = true;
+   }
+
/*
 * Step 1: Wait for voltages stable.  That surely happened
 * before starting the kernel.
@@ -126,9 +150,22 @@ static void octeon2_usb_clocks_start(void)
cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
 
/* 3b */
-   /* 12MHz crystal. */
-   clk_rst_ctl.s.p_refclk_sel = 0;
-   clk_rst_ctl.s.p_refclk_div = 0;
+   clk_rst_ctl.s.p_refclk_sel = is_crystal_clock ? 0 : 1;
+   switch (clock_rate) {
+   default:
+   pr_err("Invalid UCTL clock rate of %u, using 1200 
instead\n",
+   clock_rate);
+   /* Fall through */
+   case 1200:
+   clk_rst_ctl.s.p_refclk_div = 0;
+   break;
+   case 2400:
+   clk_rst_ctl.s.p_refclk_div = 1;
+   break;
+   case 4800:
+   clk_rst_ctl.s.p_refclk_div = 2;
+   break;
+   }
cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
 
/* 3c */
@@ -259,7 +296,7 @@ static void octeon2_usb_clocks_stop(void)
 
 static int octeon_ehci_power_on(struct platform_device *pdev)
 {
-   octeon2_usb_clocks_start();
+   octeon2_usb_clocks_start(&pdev->dev);
return 0;
 }
 
@@ -277,11 +314,11 @@ static struct usb_ehci_pdata octeon_ehci_pdata = {
.power_off  = octeon_ehci_power_off,
 };
 
-static void __init octeon_ehci_hw_start(void)
+static void __init octeon_ehci_hw_start(struct device *dev)
 {
union cvmx_uctlx_ehci_ctl ehci_ctl;
 
-   octeon2_usb_clocks_start();
+   octeon2_usb_clocks_start(dev);
 
ehci_ctl.u64 = cvmx_read_csr(CVMX_UCTLX_EHCI_CTL(0));
/* Use 64-bit addressing. */
@@ -299,59 +336,28 @@ static u64 octeon_ehci_dma_mask = DMA_BIT_MASK(64);
 static int __init octeon_ehci_device_init(void)
 {
struct platform_device *pd;
+   struct device_node *ehci_node;
int ret = 0;
 
-   struct resource usb_resources[] = {
-   {
-   .flags  = IORESOURCE_MEM,
-   }, {
-   .flags  = IORESOURCE_IRQ,
-   }
-   };
-
-   /* Only Octeon2 has ehci/ohci */
-   if (!OCTEON_IS_MODEL(OCTEON_CN63XX))
+   ehci_node = of_find_node_by_name(NULL, "ehci");
+   if (!ehci_node)
return 0;
 
-   if (octeon_is_simulation() || usb_disabled())
-   return 0; /* No USB in the 

[PATCH 1/3] USB: host: Remove ehci-octeon and ohci-octeon drivers

2014-11-13 Thread Andreas Herrmann
From: Alan Stern 

From: Alan Stern 

Remove special-purpose octeon drivers and instead use ehci-platform
and ohci-platform as suggested with
http://marc.info/?l=linux-mips&m=140139694721623&w=2

[andreas.herrmann:
fixed compile error]

Cc: David Daney 
Cc: Alex Smith 
Cc: Alan Stern 
Signed-off-by: Alan Stern 
Signed-off-by: Andreas Herrmann 
---
 arch/mips/cavium-octeon/octeon-platform.c |  274 -
 arch/mips/configs/cavium_octeon_defconfig |3 +
 drivers/usb/host/Kconfig  |   18 +-
 drivers/usb/host/Makefile |1 -
 drivers/usb/host/ehci-hcd.c   |5 -
 drivers/usb/host/ehci-octeon.c|  188 
 drivers/usb/host/octeon2-common.c |  200 -
 drivers/usb/host/ohci-hcd.c   |5 -
 drivers/usb/host/ohci-octeon.c|  202 -
 9 files changed, 285 insertions(+), 611 deletions(-)
 delete mode 100644 drivers/usb/host/ehci-octeon.c
 delete mode 100644 drivers/usb/host/octeon2-common.c
 delete mode 100644 drivers/usb/host/ohci-octeon.c

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index 6df0f4d..b67ddf0 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -7,22 +7,27 @@
  * Copyright (C) 2008 Wind River Systems
  */
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 /* Octeon Random Number Generator.  */
 static int __init octeon_rng_device_init(void)
@@ -68,6 +73,229 @@ device_initcall(octeon_rng_device_init);
 
 #ifdef CONFIG_USB
 
+static DEFINE_MUTEX(octeon2_usb_clocks_mutex);
+
+static int octeon2_usb_clock_start_cnt;
+
+static void octeon2_usb_clocks_start(void)
+{
+   u64 div;
+   union cvmx_uctlx_if_ena if_ena;
+   union cvmx_uctlx_clk_rst_ctl clk_rst_ctl;
+   union cvmx_uctlx_uphy_ctl_status uphy_ctl_status;
+   union cvmx_uctlx_uphy_portx_ctl_status port_ctl_status;
+   int i;
+   unsigned long io_clk_64_to_ns;
+
+
+   mutex_lock(&octeon2_usb_clocks_mutex);
+
+   octeon2_usb_clock_start_cnt++;
+   if (octeon2_usb_clock_start_cnt != 1)
+   goto exit;
+
+   io_clk_64_to_ns = 640ull / octeon_get_io_clock_rate();
+
+   /*
+* Step 1: Wait for voltages stable.  That surely happened
+* before starting the kernel.
+*
+* Step 2: Enable  SCLK of UCTL by writing UCTL0_IF_ENA[EN] = 1
+*/
+   if_ena.u64 = 0;
+   if_ena.s.en = 1;
+   cvmx_write_csr(CVMX_UCTLX_IF_ENA(0), if_ena.u64);
+
+   /* Step 3: Configure the reference clock, PHY, and HCLK */
+   clk_rst_ctl.u64 = cvmx_read_csr(CVMX_UCTLX_CLK_RST_CTL(0));
+
+   /*
+* If the UCTL looks like it has already been started, skip
+* the initialization, otherwise bus errors are obtained.
+*/
+   if (clk_rst_ctl.s.hrst)
+   goto end_clock;
+   /* 3a */
+   clk_rst_ctl.s.p_por = 1;
+   clk_rst_ctl.s.hrst = 0;
+   clk_rst_ctl.s.p_prst = 0;
+   clk_rst_ctl.s.h_clkdiv_rst = 0;
+   clk_rst_ctl.s.o_clkdiv_rst = 0;
+   clk_rst_ctl.s.h_clkdiv_en = 0;
+   clk_rst_ctl.s.o_clkdiv_en = 0;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
+
+   /* 3b */
+   /* 12MHz crystal. */
+   clk_rst_ctl.s.p_refclk_sel = 0;
+   clk_rst_ctl.s.p_refclk_div = 0;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
+
+   /* 3c */
+   div = octeon_get_io_clock_rate() / 13000ull;
+
+   switch (div) {
+   case 0:
+   div = 1;
+   break;
+   case 1:
+   case 2:
+   case 3:
+   case 4:
+   break;
+   case 5:
+   div = 4;
+   break;
+   case 6:
+   case 7:
+   div = 6;
+   break;
+   case 8:
+   case 9:
+   case 10:
+   case 11:
+   div = 8;
+   break;
+   default:
+   div = 12;
+   break;
+   }
+   clk_rst_ctl.s.h_div = div;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
+   /* Read it back, */
+   clk_rst_ctl.u64 = cvmx_read_csr(CVMX_UCTLX_CLK_RST_CTL(0));
+   clk_rst_ctl.s.h_clkdiv_en = 1;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
+   /* 3d */
+   clk_rst_ctl.s.h_clkdiv_rst = 1;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk_rst_ctl.u64);
+
+   /* 3e: delay 64 io clocks */
+   ndelay(io_clk_64_to_ns);
+
+   /*
+* Step 4: Program the power-on reset field in the UCTL
+* clock-reset-control register.
+*/
+   clk_rst_ctl.s.p_por = 0;
+   cvmx_write_csr(CVMX_UCTLX_CLK_RST_CTL(0), clk

Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Benson Leung
On Thu, Nov 13, 2014 at 1:18 PM, Alan Stern  wrote:
> If you interchange the two lines then the flag _will_ be cleared before
> usb_kill_urb() and autosuspend_check() can run.  This means your patch
> description is bogus.

Gotcha. v2 posted. Thanks for the direction.

-- 
Benson Leung
Software Engineer, Chrom* OS
ble...@chromium.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] HID: usbhid: clear needs_remote_wakeup before usb_kill_urb

2014-11-13 Thread Benson Leung
usbhid->intf->needs_remote_wakeup is set when a device is
opened, and is cleared when a device is closed.

When a usbhid device that does not support remote wake
( i.e. !device_can_wakeup() ) is closed, we fail out of
autosuspend_check() because the autosuspend check is called
before the flag is cleared as a result of usb_kill_urb(usbhid->urbin);

The result is that a device that may otherwise autosuspend will
fail to enter suspend again after all handles to it are closed.

Clear the flag first before usb_kill_urb, which may result in a
autosuspend_check().

Signed-off-by: Benson Leung 
---
 drivers/hid/usbhid/hid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 04e34b9..ca66ad7 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -734,8 +734,8 @@ void usbhid_close(struct hid_device *hid)
spin_unlock_irq(&usbhid->lock);
hid_cancel_delayed_stuff(usbhid);
if (!(hid->quirks & HID_QUIRK_ALWAYS_POLL)) {
-   usb_kill_urb(usbhid->urbin);
usbhid->intf->needs_remote_wakeup = 0;
+   usb_kill_urb(usbhid->urbin);
}
} else {
spin_unlock_irq(&usbhid->lock);
-- 
2.1.0.rc2.206.gedb03e5

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


Re: [PATCH net-next 2/2] r8152: adjust rtl_start_rx

2014-11-13 Thread David Miller
From: David Miller 
Date: Wed, 12 Nov 2014 22:31:46 -0500 (EST)

> From: Hayes Wang 
> Date: Thu, 13 Nov 2014 02:31:14 +
> 
>> My last method which I mentioned yesterday is similar to
>> this one. The difference is that I would re-use the rx
>> buffers, so I have to add them to the list for re-submitting,
>> not alwayes allocate new one.
>> 
>> Although one rx buffer could contain many packets, I don't
>> think the whole size of the rx buffer is alwayes used.
>> Therefore, I re-use the rx buffers to avoid allocating
>> the 16K bytes rx buffer alwayes. This also makes sure that
>> I always have the buffers to submit without allocating new
>> one.
>> 
>> If you could accept this, I would modify this patch by
>> this way.
> 
> I'll reread your original patch and think some more about this.

What if even the first r8152_submit_rx() fails?  What ever will cause
any of these retries to trigger at all?

Second, why does your patch increment 'i' with 'i++;' in the error
break path?  You should mark the first failed entry as unallocated
with actual_length == 0 and place it on the rx_done queue.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Benson Leung wrote:

> On Thu, Nov 13, 2014 at 12:41 PM, Alan Stern  
> wrote:
> > usbhid_stop probably doesn't need it.  And it should be possible to fix
> > usbhid_close more easily just by interchanging the two lines:
> >
> > -   usb_kill_urb(usbhid->urbin);
> > usbhid->intf->needs_remote_wakeup = 0;
> > +   usb_kill_urb(usbhid->urbin);
> >
> > Have you tried this?
> 
> Yes, I tried that as well, and that does work.
> 
> I used the get/put because that's the way it was done in other
> drivers, for example in synusb_close() in
> drivers/input/mouse/synaptics_usb.c

In the patch description, you wrote:

> When a usbhid device that does not support remote wake
> ( i.e. !device_can_wakeup() ) is closed, we fail out of
> autosuspend_check() because the autosuspend check is called
> before the flag is cleared as a result of usb_kill_urb(usbhid->urbin);

If you interchange the two lines then the flag _will_ be cleared before
usb_kill_urb() and autosuspend_check() can run.  This means your patch
description is bogus.

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 v3 1/2] usb: dwc2/gadget: add mutex to serialize init/deinit calls

2014-11-13 Thread Paul Zimmerman
> From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
> Sent: Thursday, November 13, 2014 7:18 AM
> 
> On 2014-10-31 19:46, Paul Zimmerman wrote:
> >> From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
> >> Sent: Friday, October 31, 2014 3:13 AM
> >>
> >> This patch adds mutex, which protects initialization and
> >> deinitialization procedures against suspend/resume methods.
> >>
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >>   drivers/usb/dwc2/core.h   |  1 +
> >>   drivers/usb/dwc2/gadget.c | 20 
> >>   2 files changed, 21 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> >> index 9f77b4d1c5ff..58732a9a0019 100644
> >> --- a/drivers/usb/dwc2/core.h
> >> +++ b/drivers/usb/dwc2/core.h
> >> @@ -187,6 +187,7 @@ struct s3c_hsotg {
> >>struct s3c_hsotg_plat*plat;
> >>
> >>spinlock_t  lock;
> >> +  struct mutexinit_mutex;
> >>
> >>void __iomem*regs;
> >>int irq;
> >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> >> index d8dda39c9e16..a2e4272a904e 100644
> >> --- a/drivers/usb/dwc2/gadget.c
> >> +++ b/drivers/usb/dwc2/gadget.c
> >> @@ -21,6 +21,7 @@
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >>   #include 
> >>   #include 
> >>   #include 
> >> @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget 
> >> *gadget,
> >>return -EINVAL;
> >>}
> >>
> >> +  mutex_lock(&hsotg->init_mutex);
> >>WARN_ON(hsotg->driver);
> >>
> >>driver->driver.bus = NULL;
> >> @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget 
> >> *gadget,
> >>
> >>dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
> >>
> >> +  mutex_unlock(&hsotg->init_mutex);
> >> +
> >>return 0;
> >>
> >>   err:
> >> +  mutex_unlock(&hsotg->init_mutex);
> >>hsotg->driver = NULL;
> >>return ret;
> >>   }
> >> @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget 
> >> *gadget,
> >>if (!hsotg)
> >>return -ENODEV;
> >>
> >> +  mutex_lock(&hsotg->init_mutex);
> >> +
> >>/* all endpoints should be shutdown */
> >>for (ep = 1; ep < hsotg->num_of_eps; ep++)
> >>s3c_hsotg_ep_disable(&hsotg->eps[ep].ep);
> >> @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget 
> >> *gadget,
> >>
> >>clk_disable(hsotg->clk);
> >>
> >> +  mutex_unlock(&hsotg->init_mutex);
> >> +
> >>return 0;
> >>   }
> >>
> >> @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget 
> >> *gadget, int is_on)
> >>
> >>dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);
> >>
> >> +  mutex_lock(&hsotg->init_mutex);
> >>spin_lock_irqsave(&hsotg->lock, flags);
> >>if (is_on) {
> >>clk_enable(hsotg->clk);
> >> @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget 
> >> *gadget, int is_on)
> >>
> >>hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> >>spin_unlock_irqrestore(&hsotg->lock, flags);
> >> +  mutex_unlock(&hsotg->init_mutex);
> >>
> >>return 0;
> >>   }
> >> @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device 
> >> *pdev)
> >>}
> >>
> >>spin_lock_init(&hsotg->lock);
> >> +  mutex_init(&hsotg->init_mutex);
> >>
> >>hsotg->irq = ret;
> >>
> >> @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device 
> >> *pdev, pm_message_t
> state)
> >>unsigned long flags;
> >>int ret = 0;
> >>
> >> +  mutex_lock(&hsotg->init_mutex);
> >> +
> >>if (hsotg->driver)
> >>dev_info(hsotg->dev, "suspending usb gadget %s\n",
> >> hsotg->driver->driver.name);
> >> @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device 
> >> *pdev, pm_message_t
> state)
> >>clk_disable(hsotg->clk);
> >>}
> >>
> >> +  mutex_unlock(&hsotg->init_mutex);
> >> +
> >>return ret;
> >>   }
> >>
> >> @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device 
> >> *pdev)
> >>unsigned long flags;
> >>int ret = 0;
> >>
> >> +  mutex_lock(&hsotg->init_mutex);
> >>if (hsotg->driver) {
> >> +
> >>dev_info(hsotg->dev, "resuming usb gadget %s\n",
> >> hsotg->driver->driver.name);
> >>
> >> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device 
> >> *pdev)
> >>s3c_hsotg_core_connect(hsotg);
> >>spin_unlock_irqrestore(&hsotg->lock, flags);
> >>
> >> +  mutex_unlock(&hsotg->init_mutex);
> >> +
> >>return ret;
> >>   }
> >>
> > Hmm. I can't find any other UDC driver that uses a mutex in its
> > suspend/resume functions. Can you explain why this is needed only
> > for dwc2?
> 
> I've posted this version because I thought you were not convinced that
> the patch
> "usb: dwc2/gadget: rework suspend/resume code to correctly restore
> gadget state"
> can add code for initialization and deinitialization in suspend/resume
> paths.

My proble

RE: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq

2014-11-13 Thread Paul Zimmerman
> From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
> Sent: Thursday, November 13, 2014 5:40 AM
> 
> On 2014-10-31 19:15, Paul Zimmerman wrote:
> >> From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
> >> Sent: Friday, October 31, 2014 1:04 AM
> >> To: linux-usb@vger.kernel.org; linux-samsung-...@vger.kernel.org
> >> Cc: Marek Szyprowski; Kyungmin Park; Robert Baldyga; Paul Zimmerman; 
> >> Krzysztof Kozlowski; Felipe
> Balbi
> >> Subject: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end 
> >> session' irq
> >>
> >> This patch adds a call to s3c_hsotg_disconnect() from 'end session'
> >> interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
> >> about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
> >> look a bit more suitable for this event, but it is asserted only in
> >> host mode, so in device mode we need to use something else.
> >>
> >> Additional check has been added in s3c_hsotg_disconnect() function
> >> to ensure that the event is reported only once after successful device
> >> enumeration.
> >>
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >>   drivers/usb/dwc2/core.h   |  1 +
> >>   drivers/usb/dwc2/gadget.c | 10 ++
> >>   2 files changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> >> index 55c90c53f2d6..b42df32e7737 100644
> >> --- a/drivers/usb/dwc2/core.h
> >> +++ b/drivers/usb/dwc2/core.h
> >> @@ -212,6 +212,7 @@ struct s3c_hsotg {
> >>struct usb_gadget   gadget;
> >>unsigned intsetup;
> >>unsigned long   last_rst;
> >> +  unsigned intaddress;
> >>struct s3c_hsotg_ep *eps;
> >>   };
> >>
> >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> >> index fcd2bb55ccca..6304efba11aa 100644
> >> --- a/drivers/usb/dwc2/gadget.c
> >> +++ b/drivers/usb/dwc2/gadget.c
> >> @@ -1114,6 +1114,7 @@ static void s3c_hsotg_process_control(struct 
> >> s3c_hsotg *hsotg,
> >> DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK;
> >>writel(dcfg, hsotg->regs + DCFG);
> >>
> >> +  hsotg->address = ctrl->wValue;
> >>dev_info(hsotg->dev, "new address %d\n", ctrl->wValue);
> >>
> >>ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
> >> @@ -2031,6 +2032,10 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg 
> >> *hsotg)
> >>   {
> >>unsigned ep;
> >>
> >> +  if (!hsotg->address)
> >> +  return;
> >> +
> >> +  hsotg->address = 0;
> >>for (ep = 0; ep < hsotg->num_of_eps; ep++)
> >>kill_all_requests(hsotg, &hsotg->eps[ep], -ESHUTDOWN, true);
> >>
> >> @@ -2290,6 +2295,11 @@ irq_retry:
> >>dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);
> >>
> >>writel(otgint, hsotg->regs + GOTGINT);
> >> +
> >> +  if (otgint & GOTGINT_SES_END_DET) {
> >> +  s3c_hsotg_disconnect(hsotg);
> >> +  hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> >> +  }
> >>}
> >>
> >>if (gintsts & GINTSTS_SESSREQINT) {
> > I don't think this is right. The host can send control requests to
> > the device before it sends a SetAddress to change from the default
> > address of 0. So if a GOTGINT_SES_END_DET happens before the
> > SetAddress, it will be ignored.
> >
> > Or am I missing something?
> 
> Well, right. However before finishing enumeration (setting the address)
> host usually
> only retrieves some usb descriptors what doesn't change the state of the
> gadget.
> Right now we always reported 'disconnected' event before setting the new
> address,
> what is a bit overkill (in some cases gadget driver got this even more
> than once).
> The above code handles all cases correctly and reports disconnect event
> only once.

Well, if the disconnect happens before the SetAddress, the disconnect
won't be reported at all. Unless I'm reading the code wrong.

-- 
Paul



Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Benson Leung
On Thu, Nov 13, 2014 at 12:41 PM, Alan Stern  wrote:
> usbhid_stop probably doesn't need it.  And it should be possible to fix
> usbhid_close more easily just by interchanging the two lines:
>
> -   usb_kill_urb(usbhid->urbin);
> usbhid->intf->needs_remote_wakeup = 0;
> +   usb_kill_urb(usbhid->urbin);
>
> Have you tried this?

Yes, I tried that as well, and that does work.

I used the get/put because that's the way it was done in other
drivers, for example in synusb_close() in
drivers/input/mouse/synaptics_usb.c

-- 
Benson Leung
Software Engineer, Chrom* OS
ble...@chromium.org
--
To unsubscribe from this list: send the line "unsubscribe 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] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Benson Leung wrote:

> usbhid->intf->needs_remote_wakeup is set when a device is
> opened, and is cleared when a device is closed.
> 
> When a usbhid device that does not support remote wake
> ( i.e. !device_can_wakeup() ) is closed, we fail out of
> autosuspend_check() because the autosuspend check is called
> before the flag is cleared as a result of usb_kill_urb(usbhid->urbin);
> 
> The result is that a device that may otherwise autosuspend will
> fail to enter suspend again after all handles to it are closed.
> 
> In usbhid_open, usb_autopm_get_interface is called
> before setting the needs_remote_wakeup flag, and
> usb_autopm_put_interface is called after hid_start_in.
> 
> However, when the device is closed in usbhid_close, the same
> protection isn't there when clearing needs_remote_wakeup. This will
> add that to usbhid_close as well as usbhid_stop.

usbhid_stop probably doesn't need it.  And it should be possible to fix 
usbhid_close more easily just by interchanging the two lines:

-   usb_kill_urb(usbhid->urbin);
usbhid->intf->needs_remote_wakeup = 0;
+   usb_kill_urb(usbhid->urbin);

Have you tried this?

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] HID: usbhid: get/put around clearing needs_remote_wakeup

2014-11-13 Thread Benson Leung
usbhid->intf->needs_remote_wakeup is set when a device is
opened, and is cleared when a device is closed.

When a usbhid device that does not support remote wake
( i.e. !device_can_wakeup() ) is closed, we fail out of
autosuspend_check() because the autosuspend check is called
before the flag is cleared as a result of usb_kill_urb(usbhid->urbin);

The result is that a device that may otherwise autosuspend will
fail to enter suspend again after all handles to it are closed.

In usbhid_open, usb_autopm_get_interface is called
before setting the needs_remote_wakeup flag, and
usb_autopm_put_interface is called after hid_start_in.

However, when the device is closed in usbhid_close, the same
protection isn't there when clearing needs_remote_wakeup. This will
add that to usbhid_close as well as usbhid_stop.

Signed-off-by: Benson Leung 
---
 drivers/hid/usbhid/hid-core.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 04e34b9..2a0b91d 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -734,8 +734,15 @@ void usbhid_close(struct hid_device *hid)
spin_unlock_irq(&usbhid->lock);
hid_cancel_delayed_stuff(usbhid);
if (!(hid->quirks & HID_QUIRK_ALWAYS_POLL)) {
+   int autopm_error;
+
+   autopm_error = usb_autopm_get_interface(usbhid->intf);
+
usb_kill_urb(usbhid->urbin);
usbhid->intf->needs_remote_wakeup = 0;
+
+   if (!autopm_error)
+   usb_autopm_put_interface(usbhid->intf);
}
} else {
spin_unlock_irq(&usbhid->lock);
@@ -1179,9 +1186,17 @@ static void usbhid_stop(struct hid_device *hid)
if (WARN_ON(!usbhid))
return;
 
-   if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
+   if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
+   int autopm_error;
+
+   autopm_error = usb_autopm_get_interface(usbhid->intf);
+
usbhid->intf->needs_remote_wakeup = 0;
 
+   if (!autopm_error)
+   usb_autopm_put_interface(usbhid->intf);
+   }
+
clear_bit(HID_STARTED, &usbhid->iofl);
spin_lock_irq(&usbhid->lock);   /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
-- 
2.1.0.rc2.206.gedb03e5

--
To unsubscribe from this list: send the line "unsubscribe 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: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Felipe Balbi
Hi,

On Thu, Nov 13, 2014 at 11:18:39AM -0800, Greg KH wrote:
> On Thu, Nov 13, 2014 at 01:13:44PM -0600, Felipe Balbi wrote:
> > > I just got back from Korea, sorry for the delay, will get to this
> > > today...
> > 
> > no problem, let me add your G+ to my 'following' circle :-p
> 
> I was only there for 48 hours, not even long enough to post anything
> there...

oh alright :-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Greg KH
On Thu, Nov 13, 2014 at 01:13:44PM -0600, Felipe Balbi wrote:
> > I just got back from Korea, sorry for the delay, will get to this
> > today...
> 
> no problem, let me add your G+ to my 'following' circle :-p

I was only there for 48 hours, not even long enough to post anything
there...

greg "I fly too much" 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: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Greg KH
On Mon, Nov 10, 2014 at 02:52:59PM -0600, Felipe Balbi wrote:
> Hi Greg,
> 
> Here's another minor for v3.18-rc5. Please consider
> merging to your usb-linus branch.
> 
> Tested with AM437x SK and testusb.
> 
> cheers
> 
> The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:
> 
>   Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> tags/fixes-for-v3.18-rc5

Pulled and pushed out.

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: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Felipe Balbi
On Thu, Nov 13, 2014 at 11:11:59AM -0800, Greg KH wrote:
> On Thu, Nov 13, 2014 at 12:05:17PM -0600, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Nov 10, 2014 at 02:52:59PM -0600, Felipe Balbi wrote:
> > > Hi Greg,
> > > 
> > > Here's another minor for v3.18-rc5. Please consider
> > > merging to your usb-linus branch.
> > > 
> > > Tested with AM437x SK and testusb.
> > > 
> > > cheers
> > > 
> > > The following changes since commit 
> > > 206c5f60a3d902bc4b56dab2de3e88de5eb06108:
> > > 
> > >   Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> > > tags/fixes-for-v3.18-rc5
> > > 
> > > for you to fetch changes up to 520fe7633692181bb6d1560d655fbdfbb7c05aaa:
> > > 
> > >   usb: dwc3: ep0: fix for dead code (2014-11-10 14:39:44 -0600)
> > > 
> > > 
> > > usb: fixes for v3.18-rc5
> > > 
> > > Just one fix here on dwc3 which fixes a minor
> > > bug caused by a fix that went in this v3.18-rc
> > > cycle.
> > > 
> > > The corner case is minimal as it can only be
> > > reproduced with back-to-back Setup transfers
> > > (without starting data or status phase) by
> > > means of a LeCroy USB Trainer, where we can
> > > generate USB packets any way we like.
> > > 
> > > Signed-off-by: Felipe Balbi 
> > > 
> > > 
> > > Felipe Balbi (1):
> > >   usb: dwc3: ep0: fix for dead code
> > > 
> > >  drivers/usb/dwc3/ep0.c | 8 
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > Gentle reminder on this pull request.
> 
> I just got back from Korea, sorry for the delay, will get to this
> today...

no problem, let me add your G+ to my 'following' circle :-p

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH RESEND v2] usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC

2014-11-13 Thread Felipe Balbi
Hi,

(please don't top-post)

On Thu, Nov 13, 2014 at 11:26:50AM -0700, Ashwini Pahuja wrote:
> Hi Felipe,
> 
> I Just sent you the V3 PATCH, I think it should be good for your next
> submission to Greg for 3.18-rc6, I guess it's too late for 3.18-rc5. I
> really appreciate all your feedback from v1/v2.

huh ? new drivers only go into the merge window which is still a few
weeks ahead of us. You driver will (assuming it's good enough to be
merged to my 'next' now) hit v3.19 only.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Greg KH
On Thu, Nov 13, 2014 at 12:05:17PM -0600, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Nov 10, 2014 at 02:52:59PM -0600, Felipe Balbi wrote:
> > Hi Greg,
> > 
> > Here's another minor for v3.18-rc5. Please consider
> > merging to your usb-linus branch.
> > 
> > Tested with AM437x SK and testusb.
> > 
> > cheers
> > 
> > The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:
> > 
> >   Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> > tags/fixes-for-v3.18-rc5
> > 
> > for you to fetch changes up to 520fe7633692181bb6d1560d655fbdfbb7c05aaa:
> > 
> >   usb: dwc3: ep0: fix for dead code (2014-11-10 14:39:44 -0600)
> > 
> > 
> > usb: fixes for v3.18-rc5
> > 
> > Just one fix here on dwc3 which fixes a minor
> > bug caused by a fix that went in this v3.18-rc
> > cycle.
> > 
> > The corner case is minimal as it can only be
> > reproduced with back-to-back Setup transfers
> > (without starting data or status phase) by
> > means of a LeCroy USB Trainer, where we can
> > generate USB packets any way we like.
> > 
> > Signed-off-by: Felipe Balbi 
> > 
> > 
> > Felipe Balbi (1):
> >   usb: dwc3: ep0: fix for dead code
> > 
> >  drivers/usb/dwc3/ep0.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Gentle reminder on this pull request.

I just got back from Korea, sorry for the delay, will get to this
today...
--
To unsubscribe from this list: send the line "unsubscribe 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: driver pl2303 doesn't detect device after upgrade

2014-11-13 Thread Milan Kocian
On Thu, Nov 13, 2014 at 08:06:05AM -0800, Greg KH wrote:
> On Thu, Nov 13, 2014 at 04:59:32PM +0100, Milan Kocian wrote:
> > On Thu, Nov 13, 2014 at 07:44:20AM -0800, Greg KH wrote:
> > > On Thu, Nov 13, 2014 at 10:17:07AM +0100, Milan Kocian wrote:
> > > > Hi,
> > > > 
> > > > after upgrade from 3.16 to 3.17.2 this device isn't in lsusb output
> > > > after plugging in :
> > > > 
> > > > Bus 004 Device 002: ID 067b:2303 Prolific Technology, Inc. PL2303 
> > > > Serial Port
> > > > (driver pl2303, usb to serial converter)
> > > > 
> > > > The only way how to make it functional is to boot machine with 
> > > > connected device.
> > > > But after unlug/plug the device disappeared and isn't detected again.
> > > > 
> > > > 
> > > > (Probably a found once more non-functional device (USB keyboard) that 
> > > > wasn't
> > > > detected but I thinked that problem is in keyboard...)
> > > > 
> > > > Some other devices work fine (USB modem, flashdisk).
> > > > 
> > > > Workaround is to downgrade to the kernel 3.16.
> > > > 
> > > > System: debian unstable, selfcompiled vanilla, Thinkpad X61x
> > > 
> > > Are there any kernel log messages when you plug the device in?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > 
> > 
> > No.
> 
> Nothing at all?  That indicates that the hub isn't powered on at all, so
> it's an electrical issue?  Very confused...
> 
> greg k-h
> 

Yes, nothing at all. But when I connect other device to the same port 
(e.g USB gsm modem) it's detected, so I don't think its electrical issue.

Yes, it's very confused. Initially I thinked that USB-serial convertor is
dead so I tried it for sure in another machine but it works. Then I remembered
non-functional USB keyboard and began to look at the problem :-).
And result is that problem is in new kernel. 

I can try patches, enable some debug options but you must say what to try :-).
(I am not USB expert).

Best regards and thanks for your answer,

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


Re: [PATCH v2 2/2] [usb] add support for ACPI identification to xhci-platform

2014-11-13 Thread Greg KH
On Thu, Nov 13, 2014 at 12:36:09PM -0600, Mark Langsdorf wrote:
> On 11/04/2014 11:12 AM, Greg KH wrote:
> >On Tue, Nov 04, 2014 at 10:50:33AM -0600, Mark Langsdorf wrote:
>   #endif
> >>
> >>+#ifdef CONFIG_ACPI
> >>+static const struct acpi_device_id usb_xhci_acpi_match[] = {
> >>+   /* APM X-Gene USB Controller */
> >>+   { "PNP0D10", },
> >>+   { }
> >>+};
> >>+MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
> >>+#endif
> >
> >That looks like a very "generic" PNP value, are you sure it is assigned
> >only to this specific device?
> 
> Although this is a generic PNP device, the specific implementation
> I'm testing has issues with USB3. Is there a flag or function
> call that will disable the USB3 host while keeping the USB2
> host?? My naive attempts in finding one mostly hung the machine.

If this is an xhci chip, there is no stand-along USB 2 controller
anymore, sorry.

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: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Felipe Balbi
On Thu, Nov 13, 2014 at 12:31:28PM -0600, Felipe Balbi wrote:
> Hi,
> 
> (your mailing is un-wrapping emails, I always end up with pretty long
> lines and have to rewrap them)
> 
> On Thu, Nov 13, 2014 at 07:58:28PM +0200, Mathias Nyman wrote:
> > > The algorithm described in the DVB tuner bug is clearly wrong, since
> > > it doesn't move the dequeue pointer until usb_clear_halt() is
> > > called, which is far too late.  The right approach is to fix up the
> > > dequeue pointer before giving back the URB (so there's no need to
> > > save a "stopped TD" value).  Otherwise there will be TDs in the
> > > endpoint ring containing stale DMA pointers to buffers that have
> > > already been unmapped.
> > 
> > Thats right, I cleaned up the patch and removed resetting the endpoint
> > from the .endpoint_reset() callback which was called as a result of
> > usb_clear_halt(). Now we queue a reset endpoint and set dequeue
> > pointer before giving back the URB.
> > 
> > I still set a "stopped td" value, but could as well just pass it as
> > function parameter.  Actually I'll do that in later cleanup patch.
> > 
> > Latest version of the patch is now in my tree in a reset-rework-v2
> > branch, with fixes comments and removed The brach includes the other
> > dorbell ringing patch as well.  both are on top of 3.18-rc4.
> > 
> > I still need to test it before sending it further, the tree is here:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git  
> > reset-rework-v2
> 
> I'll test this one.

for both commits on that branch, you can add my:

Tested-by: Felipe Balbi 

But please also add proper fixes and Cc: stable, so older kernels can
use those.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v2 2/2] [usb] add support for ACPI identification to xhci-platform

2014-11-13 Thread Mark Langsdorf

On 11/04/2014 11:12 AM, Greg KH wrote:

On Tue, Nov 04, 2014 at 10:50:33AM -0600, Mark Langsdorf wrote:

  #endif


+#ifdef CONFIG_ACPI
+static const struct acpi_device_id usb_xhci_acpi_match[] = {
+   /* APM X-Gene USB Controller */
+   { "PNP0D10", },
+   { }
+};
+MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
+#endif


That looks like a very "generic" PNP value, are you sure it is assigned
only to this specific device?


Although this is a generic PNP device, the specific implementation
I'm testing has issues with USB3. Is there a flag or function
call that will disable the USB3 host while keeping the USB2
host?? My naive attempts in finding one mostly hung the machine.

--Mark Langsdorf

--
To unsubscribe from this list: send the line "unsubscribe 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/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Felipe Balbi
Hi,

(your mailing is un-wrapping emails, I always end up with pretty long
lines and have to rewrap them)

On Thu, Nov 13, 2014 at 07:58:28PM +0200, Mathias Nyman wrote:
> > The algorithm described in the DVB tuner bug is clearly wrong, since
> > it doesn't move the dequeue pointer until usb_clear_halt() is
> > called, which is far too late.  The right approach is to fix up the
> > dequeue pointer before giving back the URB (so there's no need to
> > save a "stopped TD" value).  Otherwise there will be TDs in the
> > endpoint ring containing stale DMA pointers to buffers that have
> > already been unmapped.
> 
> Thats right, I cleaned up the patch and removed resetting the endpoint
> from the .endpoint_reset() callback which was called as a result of
> usb_clear_halt(). Now we queue a reset endpoint and set dequeue
> pointer before giving back the URB.
> 
> I still set a "stopped td" value, but could as well just pass it as
> function parameter.  Actually I'll do that in later cleanup patch.
> 
> Latest version of the patch is now in my tree in a reset-rework-v2
> branch, with fixes comments and removed The brach includes the other
> dorbell ringing patch as well.  both are on top of 3.18-rc4.
> 
> I still need to test it before sending it further, the tree is here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git  reset-rework-v2

I'll test this one.

> latest reset stall patch:
> https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=reset-rework-v2&id=263ae54010ffadec17741f7215de64ad40a4bf5e
>
> fix doorbell ring patch (already tested by Felipe):
> https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=reset-rework-v2&id=c96885c658294fef593f2109d194fa07d140c384

Looks like both patches need to have a Fixes and Cc: stable added to
them.

Now let me build and run this branch.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH RESEND v2] usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC

2014-11-13 Thread Ashwini Pahuja
Hi Felipe,

I Just sent you the V3 PATCH, I think it should be good for your next
submission to Greg for 3.18-rc6, I guess it's too late for 3.18-rc5. I
really appreciate all your feedback from v1/v2.

Thanks
Ashwini

On Wed, Nov 12, 2014 at 12:35 PM, Felipe Balbi  wrote:
> Hi,
>
> On Wed, Nov 12, 2014 at 12:24:11PM -0700, Ashwini Pahuja wrote:
>> On Wed, Nov 5, 2014 at 12:18 PM, Felipe Balbi  wrote:
>> > On Fri, Oct 31, 2014 at 01:14:33PM -0700, Ashwini Pahuja wrote:
>> >> This patch adds a UDC driver for Broadcom's USB3.0 Peripheral core
>> >> named BDC. BDC is capable of supporting all transfer types control,
>> >> bulk, Int and isoch on each endpoint.
>> >>
>> >> Signed-off-by: Ashwini Pahuja 
>> >> ---
>> >> Changes for v2:
>> >> - Includes all the feedback provided by Felipe on the v1.
>> >> - Removed unnecessary out of memory messages.
>> >> - Added usb_gadget_giveback_request.
>> >> - udc_stop: removed unnecessary driver argument.
>> >> - Removed unused defines.
>> >> - Renamed upsc to uspc.
>> >
>> > which platform uses this ? Is the platform working in mainline today ?
>>
>> Currently our "brcmstb" platform in arch/arm/mach-bcm/brcmstb.c, which
>> is in mainline and which supports (to some degree) BCM7439 does have
>> this HW block. BDC is a generic IP that will go inside various kinds
>> of application and there is no dependency on one particular chip. BDC
>> driver is developed/tested on our FPGA-PCIe based platform that
>> connects to x86 machine. BDC IP is also USB-IF certified, The 3.6
>> kernel version of this driver was used during USB-IF certification in
>> the Mass storage mode.
>>
>> > Can you show me boot up logs and logs of this driver working with g_zero
>> > and g_mass_storage ? Also make sure to run USB20CV chapter 9 with both
>> > g_zero and mass_storage and post the logs somewhere I can access, or
>> > just attach everything to this mail as a tarball or something.
>> >
>> Sure, I always run the USB30CV, USB20CV before sending out the code.
>> Also testusb has been running for several days. I am attaching the
>> logs with CONFIG_USB_GADGET_DEBUG enabled. Let me know if you need the
>> logs with CONFIG_USB_GADGET_VERBOSE as well.
>
> cool, I see you've been testing on v3.16-rc6, which should be fine, a
> little bit on the old side, but still pretty recent :-)
>
> [snip]
>
>> >> + gbdi = 0;
>> >> + dev_vdbg(bdc->dev,
>> >> + "Dump bd list for %s epnum:%d\n",
>> >> + ep->name, ep->ep_num);
>> >> +
>> >> + dev_vdbg(bdc->dev,
>> >> + "tabs:%d max_bdi:%d eqp_bdi:%d hwd_bdi:%d 
>> >> num_bds_table:%d\n",
>> >> + bd_list->num_tabs, bd_list->max_bdi, bd_list->eqp_bdi,
>> >> + bd_list->hwd_bdi, bd_list->num_bds_table);
>> >> +
>> >> + for (tbi = 0; tbi < bd_list->num_tabs; tbi++) {
>> >> + bd_table = bd_list->bd_table_array[tbi];
>> >> + for (bdi = 0; bdi < bd_list->num_bds_table; bdi++) {
>> >> + bd =  bd_table->start_bd + bdi;
>> >> + dma = bd_table->dma + (sizeof(struct bdc_bd) * bdi);
>> >> + dev_vdbg(bdc->dev,
>> >> + "tbi:%2d bdi:%2d gbdi:%2d virt:%p phys:%llx 
>> >> %08x %08x %08x %08x\n",
>> >> + tbi, bdi, gbdi++, bd, (unsigned long 
>> >> long)dma,
>> >> + le32_to_cpu(bd->offset[0]),
>> >> + le32_to_cpu(bd->offset[1]),
>> >> + le32_to_cpu(bd->offset[2]),
>> >> + le32_to_cpu(bd->offset[3]));
>> >> + }
>> >> + dev_vdbg(bdc->dev, "\n\n");
>> >> + }
>> >> +}
>> >
>> > all of these debugging features should be done either using tracepoints
>> > or debugfs. At a minimum you should stub them out when building without
>> > debug to avoid the extra overhead.
>> >
>> OK, I will stub these debug functions in v3. I will consider adding
>> support for tracepoints/debugfs in near future.
>
> cool, thanks.
>
>> >> diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c 
>> >> b/drivers/usb/gadget/udc/bdc/bdc_ep.c
>> >> new file mode 100644
>> >> index 000..f5adcb4
>> >> --- /dev/null
>> >> +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
>> >> @@ -0,0 +1,2019 @@
>> >> +/*
>> >> + * bdc_ep.c - BRCM BDC USB3.0 device controller endpoint related 
>> >> functions
>> >> + *
>> >> + * Copyright (C) 2014 Broadcom Corporation
>> >> + *
>> >> + * Author: Ashwini Pahuja
>> >> + *
>> >> + * Based on drivers under drivers/usb/
>> >> + *
>> >> + * This program is free software; you can redistribute it and/or modify 
>> >> it
>> >> + * under the terms of the GNU General Public License as published by the
>> >> + * Free Software Foundation; either version 2 of the License, or (at your
>> >> + * option) any later version.
>> >> + *
>> >> + */
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#inclu

[PATCH v3] usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC

2014-11-13 Thread Ashwini Pahuja
This patch adds a UDC driver for Broadcom's USB3.0 Peripheral core named BDC.
BDC supports control traffic on ep0 and bulk/Int/Isoch traffic on all other
endpoints.

Signed-off-by: Ashwini Pahuja 
---
Changes for v3:
 - Incorporated feeback by Felipe on the v2
 - Made stubs for debug functions
 - removed not that useful bdc_dbg_uspc function
 - Fixed couple of sparse warnings

Changes for v2:
- Includes all the feedback provided by Felipe on the v1.
- Removed unnecessary out of memory messages.
- Added usb_gadget_giveback_request.
- udc_stop: removed unnecessary driver argument.
- Removed unused defines.
- Renamed upsc to uspc.
---
 drivers/usb/gadget/udc/Kconfig|2 +
 drivers/usb/gadget/udc/Makefile   |1 +
 drivers/usb/gadget/udc/bdc/Kconfig|   21 +
 drivers/usb/gadget/udc/bdc/Makefile   |8 +
 drivers/usb/gadget/udc/bdc/bdc.h  |  490 
 drivers/usb/gadget/udc/bdc/bdc_cmd.c  |  376 ++
 drivers/usb/gadget/udc/bdc/bdc_cmd.h  |   29 +
 drivers/usb/gadget/udc/bdc/bdc_core.c |  533 +
 drivers/usb/gadget/udc/bdc/bdc_dbg.c  |  123 ++
 drivers/usb/gadget/udc/bdc/bdc_dbg.h  |   37 +
 drivers/usb/gadget/udc/bdc/bdc_ep.c   | 2022 +
 drivers/usb/gadget/udc/bdc/bdc_ep.h   |   22 +
 drivers/usb/gadget/udc/bdc/bdc_pci.c  |  132 +++
 drivers/usb/gadget/udc/bdc/bdc_udc.c  |  587 ++
 14 files changed, 4383 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/gadget/udc/bdc/Kconfig
 create mode 100644 drivers/usb/gadget/udc/bdc/Makefile
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc.h
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_cmd.c
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_cmd.h
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_core.c
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_dbg.c
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_dbg.h
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_ep.c
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_ep.h
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_pci.c
 create mode 100644 drivers/usb/gadget/udc/bdc/bdc_udc.c

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 217365d..b8e213e 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -241,6 +241,8 @@ config USB_M66592
   dynamically linked module called "m66592_udc" and force all
   gadget drivers to also be dynamically linked.
 
+source "drivers/usb/gadget/udc/bdc/Kconfig"
+
 #
 # Controllers available only in discrete form (and all PCI controllers)
 #
diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
index a7f4491..fba2049 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_USB_FOTG210_UDC) += fotg210-udc.o
 obj-$(CONFIG_USB_MV_U3D)   += mv_u3d_core.o
 obj-$(CONFIG_USB_GR_UDC)   += gr_udc.o
 obj-$(CONFIG_USB_GADGET_XILINX)+= udc-xilinx.o
+obj-$(CONFIG_USB_BDC_UDC)  += bdc/
diff --git a/drivers/usb/gadget/udc/bdc/Kconfig 
b/drivers/usb/gadget/udc/bdc/Kconfig
new file mode 100644
index 000..0d7b8c9
--- /dev/null
+++ b/drivers/usb/gadget/udc/bdc/Kconfig
@@ -0,0 +1,21 @@
+config USB_BDC_UDC
+   tristate "Broadcom USB3.0 device controller IP driver(BDC)"
+   depends on USB_GADGET && HAS_DMA
+
+   help
+   BDC is Broadcom's USB3.0 device controller IP. If your SOC has a BDC IP
+   then select this driver.
+
+   Say "y" here to link the driver statically, or "m" to build a 
dynamically
+   linked module called "bdc".
+
+if USB_BDC_UDC
+
+comment "Platform Support"
+config USB_BDC_PCI
+   tristate "BDC support for PCIe based platforms"
+   depends on PCI
+   default USB_BDC_UDC
+   help
+   Enable support for platforms which have BDC connected through 
PCIe, such as Lego3 FPGA platform.
+endif
diff --git a/drivers/usb/gadget/udc/bdc/Makefile 
b/drivers/usb/gadget/udc/bdc/Makefile
new file mode 100644
index 000..5cf6a3b
--- /dev/null
+++ b/drivers/usb/gadget/udc/bdc/Makefile
@@ -0,0 +1,8 @@
+obj-$(CONFIG_USB_BDC_UDC)  += bdc.o
+bdc-y  := bdc_core.o bdc_cmd.o bdc_ep.o bdc_udc.o
+
+ifneq ($(CONFIG_USB_GADGET_VERBOSE),)
+   bdc-y   += bdc_dbg.o
+endif
+
+obj-$(CONFIG_USB_BDC_PCI)  += bdc_pci.o
diff --git a/drivers/usb/gadget/udc/bdc/bdc.h b/drivers/usb/gadget/udc/bdc/bdc.h
new file mode 100644
index 000..dc18a20
--- /dev/null
+++ b/drivers/usb/gadget/udc/bdc/bdc.h
@@ -0,0 +1,490 @@
+/*
+ * bdc.h - header for the BRCM BDC USB3.0 device controller
+ *
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * Author: Ashwini Pahuja
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef__LINUX_BDC_H__
+#define  

Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Mathias Nyman
> 
> Latest version of the patch is now in my tree in a reset-rework-v2 branch, 
> with fixes comments and removed The brach includes the other dorbell ringing 
> patch as well.
> both are on top of 3.18-rc4.
> 

..with fixed function comments. The branch incudes the other doorbell patch as 
well.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe 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: [GIT PULL] USB fixes for v3.18-rc5

2014-11-13 Thread Felipe Balbi
Hi,

On Mon, Nov 10, 2014 at 02:52:59PM -0600, Felipe Balbi wrote:
> Hi Greg,
> 
> Here's another minor for v3.18-rc5. Please consider
> merging to your usb-linus branch.
> 
> Tested with AM437x SK and testusb.
> 
> cheers
> 
> The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:
> 
>   Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> tags/fixes-for-v3.18-rc5
> 
> for you to fetch changes up to 520fe7633692181bb6d1560d655fbdfbb7c05aaa:
> 
>   usb: dwc3: ep0: fix for dead code (2014-11-10 14:39:44 -0600)
> 
> 
> usb: fixes for v3.18-rc5
> 
> Just one fix here on dwc3 which fixes a minor
> bug caused by a fix that went in this v3.18-rc
> cycle.
> 
> The corner case is minimal as it can only be
> reproduced with back-to-back Setup transfers
> (without starting data or status phase) by
> means of a LeCroy USB Trainer, where we can
> generate USB packets any way we like.
> 
> Signed-off-by: Felipe Balbi 
> 
> 
> Felipe Balbi (1):
>   usb: dwc3: ep0: fix for dead code
> 
>  drivers/usb/dwc3/ep0.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Gentle reminder on this pull request.

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: your mail

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Sebastian Andrzej Siewior wrote:

> On 11/07/2014 07:53 PM, Alan Stern wrote:
> >> If I put pm_runtime_get_sync() + put in musb_resume() then the problem
> >> is gone. I don't see ehci-hcd doing this in resume, in fact I don't see
> >> ehci doing pm_runtime_* at all. So for ehci the device is probably
> >> always RPM_ACTIVE.
> > 
> > No, it doesn't always have to be RPM_ACTIVE.
> > 
> > Remember, the most common implementation of EHCI is the PCI version.  
> > In drivers/pci/pci-driver.c, the pci_pm_suspend() routine calls
> > pm_runtime_resume(). 
> 
> PCI? hehe. This is funny: I assumed that PCI is the most common and
> well tested version so I looked at the hooks of usb_hcd_pci_pm_ops.
> Didn't think to look at the bus ops / pci_dev_pm_ops.
> 
> This resume function is only invoked since  v3.15-rc1 via 7cd0602d7836
> ("PCI / PM: Resume runtime-suspended devices later during system
> suspend"). Before that it was in pci_pm_prepare() which probably did
> the same job.
> 
> > EHCI controllers on other bus types might not behave so well.  That 
> > probably should be fixed in ehci_resume().
> 
> Do you have the same thing in mind as for musb?

Yes.

> >>  And still, if the HCD has a short suspend
> >> delay it might go into suspend before the khubd is invoked (but then it
> >> probably kills the status URB).
> > 
> > We need the musb device to go to the RPM_ACTIVE state at some point
> > during the system suspend/resume, and to remain that way until after
> > the root hub has been resumed.  Putting the
> > 
> > pm_runtime_disable(dev);
> > pm_runtime_set_active(dev);
> > pm_runtime_enable(dev);
> > 
> > calls in musb_resume() is one way to accomplish this.
> 
> Yes, this what I am doing…
> 
> Thanks you for explaining things :)

You're welcome.

Alan Stern

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


Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Mathias Nyman
On 13.11.2014 17:45, Alan Stern wrote:
> On Thu, 13 Nov 2014, Mathias Nyman wrote:
> 
>> Currently we queue a reset endpoint command from the .endpoint_reset 
>> callback in host, this is far too late and should be moved
>> to when we get a STALL event.  
>>
>> xhci needs to reset control endpoints on stall as well [1]
>>
>> I got a testpatch for this, but the more I look into how we handle reset 
>> endpoint for clearing halts, stop endpoint for urb dequeue, and reset device,
>> the more I notice that there are several other cases that needs fixing. 
>> testpatch for the halted ep is here:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=ep_reset_halt_test&id=fe43d559e0816f65e5373e863a7da8062d311cd7
>>
>> It's hard to see from patch diff itself what it does, but basically we call 
>> xhci_cleanup_halted_endpoint() in finish_td() if the transfer event status
>> is STALL, or a TX error that requires resetting the endpoint.
> 
> That sounds right.  You also need to ring the doorbell if the endpoint 
> queue is non-empty.
> 

Right, we'll ring the doorbell when the set dq pointer command completes.

Previously the doorbell was rung also on completed reset endpoint commans, 
which caused issues Felipe ran into.
That is fixed in an earlier patch.

>> There are still issues with setting the dequeue pointer correctly after stop 
>> or reset endpoint, I think this
>> is because we try to find the next TD based on a saved "stopped TD" value 
>> that might not valid anymore (i.e. a reset device in between reset endpoint 
>> and set dq pointer)
>> this issue is seen with DVB tuners when changing channels:
> 
> I'm not aware of the details.  Don't you always want to move to the
> start of the first TRB following the TD that got the error?

Yes, thats how I also understood it.

> 
> The algorithm described in the DVB tuner bug is clearly wrong, since it
> doesn't move the dequeue pointer until usb_clear_halt() is called,
> which is far too late.  The right approach is to fix up the dequeue
> pointer before giving back the URB (so there's no need to save a
> "stopped TD" value).  Otherwise there will be TDs in the endpoint ring
> containing stale DMA pointers to buffers that have already been
> unmapped.

Thats right, I cleaned up the patch and removed resetting the endpoint from the 
.endpoint_reset() callback which
was called as a result of usb_clear_halt(). Now we queue a reset endpoint and 
set dequeue pointer before giving back the URB.

I still set a "stopped td" value, but could as well just pass it as function 
parameter.
Actually I'll do that in later cleanup patch.

Latest version of the patch is now in my tree in a reset-rework-v2 branch, with 
fixes comments and removed The brach includes the other dorbell ringing patch 
as well.
both are on top of 3.18-rc4.

I still need to test it before sending it further, the tree is here:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git  reset-rework-v2

latest reset stall patch:
https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=reset-rework-v2&id=263ae54010ffadec17741f7215de64ad40a4bf5e

fix doorbell ring patch (already tested by Felipe):
https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=reset-rework-v2&id=c96885c658294fef593f2109d194fa07d140c384

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe 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: musb: core: make sure musb is in RPM_ACTIVE on resume

2014-11-13 Thread Sebastian Andrzej Siewior
On am335x-evm with musb in host mode and using it as a wakeup source the
following happens once the CPU comes out of suspend to ram:
|PM: Wakeup source MPU_WAKE
|PM: noirq resume of devices complete after 15.453 msecs
|PM: early resume of devices complete after 2.222 msecs
|PM: resume of devices complete after 507.351 msecs
|Restarting tasks ...
|[ cut here ]
|WARNING: CPU: 0 PID: 322 at drivers/usb/core/urb.c:339 
usb_submit_urb+0x494/0x4c8()
|URB cc0db380 submitted while active
|[] (usb_submit_urb) from [] (hub_activate+0x2b8/0x49c)
|[] (hub_activate) from [] (hub_resume+0x14/0x1c)
|[] (hub_resume) from [] 
(usb_resume_interface.isra.4+0xdc/0x110)
|[] (usb_resume_interface.isra.4) from [] 
(usb_resume_both+0x6c/0x13c)
|[] (usb_resume_both) from [] (usb_runtime_resume+0x10/0x14)
|[] (usb_runtime_resume) from [] (__rpm_callback+0x2c/0x60)
|[] (__rpm_callback) from [] (rpm_callback+0x20/0x74)
|[] (rpm_callback) from [] (rpm_resume+0x380/0x548)
|[] (rpm_resume) from [] (rpm_resume+0x238/0x548)
|[] (rpm_resume) from [] (__pm_runtime_resume+0x64/0x94)
|[] (__pm_runtime_resume) from [] 
(usb_autopm_get_interface+0x18/0x5c)
|[] (usb_autopm_get_interface) from [] 
(hub_thread+0x10c/0x115c)
|[] (hub_thread) from [] (kthread+0xbc/0xd8)
|---[ end trace 036aa5fe78203142 ]---
|hub 1-0:1.0: activate --> -16
|hub 2-0:1.0: activate --> -16

The reason for this backtrace is the attempt of the USB code to resume
the HUB twice and thus enqueue the status URB twice.
Alan Stern was a great help by explaining how the USB code supposed to
work and what is most likely the problem. The root problem is that after
resume the musb runtime-suspend state remains RPM_SUSPENDED.
According to git log it RPM was added for the omap2430 platform. If I
understand it correct the omap2430 invokes a get on musb once a cable is
connected and a put once the cable is gone. In between the device could
go auto-idle/off. Not sure what happens when the device goes into suspend
but then I guess it was gadget only.
On DSPS I see only a get in probe and put in remove function. This would
forbid RPM from working but then the devices enterns suspended state
anyway :)

To get rid of this warning, I set the device state to RPM_ACTIVE which
the expected state.

Cc: Alan Stern 
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/usb/musb/musb_core.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 3345c945262f..f1dfe53515fb 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2321,6 +2321,14 @@ static int musb_resume(struct device *dev)
schedule_delayed_work(&musb->finish_resume_work,
  msecs_to_jiffies(20));
}
+
+   /*
+* The USB HUB code expects the device to be in RPM_ACTIVE once it came
+* out of suspend
+*/
+   pm_runtime_disable(dev);
+   pm_runtime_set_active(dev);
+   pm_runtime_enable(dev);
return 0;
 }
 
-- 
2.1.3

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


[PATCH v2] usb: musb: musb_cppi41: recognize HS devices in hostmode

2014-11-13 Thread Sebastian Andrzej Siewior
There is a poll loop for max 25us for HS devices. Now guess what, I
tested it in gadget mode and forgot about the little detail. Nobody seem
to have it noticed…
This patch adds the missing logic for hostmode so it is recognized in
host and device mode properly.

Fixes: 50aea6fca771 ("usb: musb: cppi41: fire hrtimer according to
programmed channel length")
Signed-off-by: Sebastian Andrzej Siewior 
---
v1…v2: proper ident.

Felipe, I've been told that his is no risk while patch two makes things
wrose for ISO. I added a fixes tag since this should improve performance
for HDD behind an USB bridge. For a proper/better #2 I need to take of
ISO first…

 drivers/usb/musb/musb_cppi41.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 5a9b977fbc19..f64fd964dc6d 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -253,6 +253,7 @@ static void cppi41_dma_callback(void *private_data)
cppi41_trans_done(cppi41_channel);
} else {
struct cppi41_dma_controller *controller;
+   int is_hs = 0;
/*
 * On AM335x it has been observed that the TX interrupt fires
 * too early that means the TXFIFO is not yet empty but the DMA
@@ -265,7 +266,14 @@ static void cppi41_dma_callback(void *private_data)
 */
controller = cppi41_channel->controller;
 
-   if (musb->g.speed == USB_SPEED_HIGH) {
+   if (is_host_active(musb)) {
+   if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
+   is_hs = 1;
+   } else {
+   if (musb->g.speed == USB_SPEED_HIGH)
+   is_hs = 1;
+   }
+   if (is_hs) {
unsigned wait = 25;
 
do {
-- 
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe 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: your mail

2014-11-13 Thread Sebastian Andrzej Siewior
On 11/07/2014 07:53 PM, Alan Stern wrote:
>> If I put pm_runtime_get_sync() + put in musb_resume() then the problem
>> is gone. I don't see ehci-hcd doing this in resume, in fact I don't see
>> ehci doing pm_runtime_* at all. So for ehci the device is probably
>> always RPM_ACTIVE.
> 
> No, it doesn't always have to be RPM_ACTIVE.
> 
> Remember, the most common implementation of EHCI is the PCI version.  
> In drivers/pci/pci-driver.c, the pci_pm_suspend() routine calls
> pm_runtime_resume(). 

PCI? hehe. This is funny: I assumed that PCI is the most common and
well tested version so I looked at the hooks of usb_hcd_pci_pm_ops.
Didn't think to look at the bus ops / pci_dev_pm_ops.

This resume function is only invoked since  v3.15-rc1 via 7cd0602d7836
("PCI / PM: Resume runtime-suspended devices later during system
suspend"). Before that it was in pci_pm_prepare() which probably did
the same job.

> EHCI controllers on other bus types might not behave so well.  That 
> probably should be fixed in ehci_resume().

Do you have the same thing in mind as for musb?

>>  And still, if the HCD has a short suspend
>> delay it might go into suspend before the khubd is invoked (but then it
>> probably kills the status URB).
> 
> We need the musb device to go to the RPM_ACTIVE state at some point
> during the system suspend/resume, and to remain that way until after
> the root hub has been resumed.  Putting the
> 
>   pm_runtime_disable(dev);
>   pm_runtime_set_active(dev);
>   pm_runtime_enable(dev);
> 
> calls in musb_resume() is one way to accomplish this.

Yes, this what I am doing…

Thanks you for explaining things :)

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


RE: [PATCH v4] usb: gadget: f_fs: add "no_disconnect" mode

2014-11-13 Thread Krzysztof Opasiak
Hello,

> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Thursday, November 13, 2014 3:44 AM
> To: Felipe Balbi
> Cc: Robert Baldyga; David Cohen; gre...@linuxfoundation.org; linux-
> u...@vger.kernel.org; linux-ker...@vger.kernel.org;
> min...@mina86.com; andrze...@samsung.com; m.szyprow...@samsung.com;
> k.opas...@samsung.com
> Subject: Re: [PATCH v4] usb: gadget: f_fs: add "no_disconnect" mode

(...)

> > > David, you have been messing with ffs lately, can I get a
> Tested-by
> > > from you on this patch ?
> >
> > ping. David ? Any chance you can test this one out ?
> 
> a reminder on this request, it's getting close to tree lockdown
> (probably another 2 to 3 weeks).

I have tested this patch today but I'm afraid that I can't give
tested-by tag:

When I kill the daemon and then reconnect my device I get on my host:

[776092.877175] usb 1-1.1: can't set config #1, error -32

While on device there is only:

[ 2147.035340] configfs-gadget gadget: high-speed config #1: The only
one

Moreover, I'm not very enthusiastic about the convention in which we
allow
to reconnect the device without any changes.

In my opinion when daemon has been killed we should allow user to finish
communication with other functions and then simply unbind our gadget.
This should happen not only on reconnection but also when host is trying
to enable configuration which contains this function "zombie".
This function is not fully operational so it is very good idea to
allow user to finish his job with other functions but when he reconnects
the
device we should unbind gadget to notify userspace that something is
wrong.
The same when we switch configuration.

What do you think about this Michal and Felipe?

--
Krzysiek

--
To unsubscribe from this list: send the line "unsubscribe 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: driver pl2303 doesn't detect device after upgrade

2014-11-13 Thread Greg KH
On Thu, Nov 13, 2014 at 04:59:32PM +0100, Milan Kocian wrote:
> On Thu, Nov 13, 2014 at 07:44:20AM -0800, Greg KH wrote:
> > On Thu, Nov 13, 2014 at 10:17:07AM +0100, Milan Kocian wrote:
> > > Hi,
> > > 
> > > after upgrade from 3.16 to 3.17.2 this device isn't in lsusb output
> > > after plugging in :
> > > 
> > > Bus 004 Device 002: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial 
> > > Port
> > > (driver pl2303, usb to serial converter)
> > > 
> > > The only way how to make it functional is to boot machine with connected 
> > > device.
> > > But after unlug/plug the device disappeared and isn't detected again.
> > > 
> > > 
> > > (Probably a found once more non-functional device (USB keyboard) that 
> > > wasn't
> > > detected but I thinked that problem is in keyboard...)
> > > 
> > > Some other devices work fine (USB modem, flashdisk).
> > > 
> > > Workaround is to downgrade to the kernel 3.16.
> > > 
> > > System: debian unstable, selfcompiled vanilla, Thinkpad X61x
> > 
> > Are there any kernel log messages when you plug the device in?
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> No.

Nothing at all?  That indicates that the hub isn't powered on at all, so
it's an electrical issue?  Very confused...

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


Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Mathias Nyman wrote:

> The comment for xhci_endpoint_reset says its called in_interrupt context?
> Does the usb core really call the .endpoint_reset in interrupt
> context? I tried to follow the codepaths in the usb core but couln't figure 
> it out  

It doesn't really say anywhere.  I'm pretty sure the intention was that
the endpoint_reset callback is always called in process context.  It
wouldn't hurt to add a comment to usb_hcd_reset_endpoint() documenting
this.

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 v1] chipidea: issue message when some calls fail in ->probe()

2014-11-13 Thread Andy Shevchenko
There is no message when PHY is not enabled, IRQ line can't be acquired, or
debugfs registration fails. This patch adds the messages.

Signed-off-by: Andy Shevchenko 
---
 drivers/usb/chipidea/core.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index c57448a..ffd2457 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -659,8 +659,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
/* if both generic PHY and USB PHY layers aren't enabled */
if (PTR_ERR(ci->phy) == -ENOSYS &&
-   PTR_ERR(ci->usb_phy) == -ENXIO)
+   PTR_ERR(ci->usb_phy) == -ENXIO) {
+   dev_err(dev, "PHY and USB PHY layers aren't enabled\n");
return -ENXIO;
+   }
 
if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy))
return -EPROBE_DEFER;
@@ -761,16 +763,22 @@ static int ci_hdrc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ci);
ret = request_irq(ci->irq, ci_irq, IRQF_SHARED, ci->platdata->name,
  ci);
-   if (ret)
+   if (ret) {
+   dev_err(dev, "can't acquire IRQ line %d\n", ci->irq);
goto stop;
+   }
 
if (ci_otg_is_fsm_mode(ci))
ci_hdrc_otg_fsm_start(ci);
 
ret = dbg_create_files(ci);
-   if (!ret)
-   return 0;
+   if (ret) {
+   dev_err(dev, "can't register debugfs files\n");
+   goto release_irq;
+   }
+   return 0;
 
+release_irq:
free_irq(ci->irq, ci);
 stop:
ci_role_destroy(ci);
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe 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: driver pl2303 doesn't detect device after upgrade

2014-11-13 Thread Milan Kocian
On Thu, Nov 13, 2014 at 07:44:20AM -0800, Greg KH wrote:
> On Thu, Nov 13, 2014 at 10:17:07AM +0100, Milan Kocian wrote:
> > Hi,
> > 
> > after upgrade from 3.16 to 3.17.2 this device isn't in lsusb output
> > after plugging in :
> > 
> > Bus 004 Device 002: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial 
> > Port
> > (driver pl2303, usb to serial converter)
> > 
> > The only way how to make it functional is to boot machine with connected 
> > device.
> > But after unlug/plug the device disappeared and isn't detected again.
> > 
> > 
> > (Probably a found once more non-functional device (USB keyboard) that wasn't
> > detected but I thinked that problem is in keyboard...)
> > 
> > Some other devices work fine (USB modem, flashdisk).
> > 
> > Workaround is to downgrade to the kernel 3.16.
> > 
> > System: debian unstable, selfcompiled vanilla, Thinkpad X61x
> 
> Are there any kernel log messages when you plug the device in?
> 
> thanks,
> 
> greg k-h
> 

No.

-- 
Milan Kocian
--
To unsubscribe from this list: send the line "unsubscribe 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/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Felipe Balbi
On Thu, Nov 13, 2014 at 10:31:55AM -0500, Alan Stern wrote:
> On Wed, 12 Nov 2014, Felipe Balbi wrote:
> 
> > > By the way, does the same sort of thing happen after a transfer
> > > error (such as a CRC mismatch)?  Does the xHCI controller change the 
> > > state to EP_STATE_HALTED?  Or does it instead go directly to 
> > 
> > There are a few conditions in which XHCI will change EP state to
> > EP_STATE_HALTED, one of them is a STALL token from the peripheral and
> > the others would be really error conditions: Babble, CRC error, etc.
> > 
> > The spec even has a note about it, which I quote:
> > 
> > "
> > A Halt condition or USB Transaction error detected on a USB pipe
> > shall cause a Running Endpoint to transition to the Halted
> > state. A Reset Endpoint Command shall be used to clear the Halt
> > condition on the endpoint and transition the endpoint to the
> > Stopped state. A Stop Endpoint Command received while an
> > endpoint is in the Halted state shall have no effect and shall
> > generate a Command Completion Event with the Completion Code set
> > to Context State Error.
> > "
> > 
> > > EP_STATE_STOPPED?  You probably want to treat that case and the STALL 
> > > case as similarly as possible.
> > 
> > There's already code to deal with that, take a look at
> > xhci_requires_manual_halt_cleanup() and its callers:
> 
> Then shouldn't the recovery from a STALL be exactly the same as the 
> recovery from any other sort of transfer error?

and it is :-) 

if (requres_manual_halt_cleanup())
xhci_endpoint_cleanup_halt();

that's basically what happens.

> > > Right.  In theory you could do it any time up until the completion
> > > routine returns.  Doing it when you process the failed TD seems like a
> > > good choice -- advance the dequeue pointer and issue the command at the
> > > same time.
> > 
> > My concern here is that this will happen when the first usb_submit_urb()
> > completes. I wonder if moving this to when the following
> > usb_submit_urb() is about to start would be better ?
> 
> No, because there may be some other URBs already on the endpoint queue.  
> If no further URBs are submitted then the queue won't get cleaned up.

oh, ok... makes sense.

> > I think this has a higher probability of being correct. Class driver
> > might not queue any URB to a particular after the first Stall, so why
> > would be move the endpoint away from EP_STATE_HALTED prematurely ?
> 
> In order to handle the queue.  Of course, if the queue is empty then 
> there's no harm in leaving the ep in EP_STATE_HALTED until another URB 
> is submitted.
> 
> By the same reasoning, when the state is changed to EP_STATE_STOPPED, 
> the doorbell should be rung.

right.

-- 
balbi


signature.asc
Description: Digital signature


Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Alan Stern
On Thu, 13 Nov 2014, Mathias Nyman wrote:

> Currently we queue a reset endpoint command from the .endpoint_reset callback 
> in host, this is far too late and should be moved
> to when we get a STALL event.  
> 
> xhci needs to reset control endpoints on stall as well [1]
> 
> I got a testpatch for this, but the more I look into how we handle reset 
> endpoint for clearing halts, stop endpoint for urb dequeue, and reset device,
> the more I notice that there are several other cases that needs fixing. 
> testpatch for the halted ep is here:
> 
> https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=ep_reset_halt_test&id=fe43d559e0816f65e5373e863a7da8062d311cd7
> 
> It's hard to see from patch diff itself what it does, but basically we call 
> xhci_cleanup_halted_endpoint() in finish_td() if the transfer event status
> is STALL, or a TX error that requires resetting the endpoint.

That sounds right.  You also need to ring the doorbell if the endpoint 
queue is non-empty.

> There are still issues with setting the dequeue pointer correctly after stop 
> or reset endpoint, I think this
> is because we try to find the next TD based on a saved "stopped TD" value 
> that might not valid anymore (i.e. a reset device in between reset endpoint 
> and set dq pointer)
> this issue is seen with DVB tuners when changing channels:

I'm not aware of the details.  Don't you always want to move to the
start of the first TRB following the TD that got the error?

The algorithm described in the DVB tuner bug is clearly wrong, since it
doesn't move the dequeue pointer until usb_clear_halt() is called,
which is far too late.  The right approach is to fix up the dequeue
pointer before giving back the URB (so there's no need to save a
"stopped TD" value).  Otherwise there will be TDs in the endpoint ring
containing stale DMA pointers to buffers that have already been
unmapped.

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: driver pl2303 doesn't detect device after upgrade

2014-11-13 Thread Greg KH
On Thu, Nov 13, 2014 at 10:17:07AM +0100, Milan Kocian wrote:
> Hi,
> 
> after upgrade from 3.16 to 3.17.2 this device isn't in lsusb output
> after plugging in :
> 
> Bus 004 Device 002: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port
> (driver pl2303, usb to serial converter)
> 
> The only way how to make it functional is to boot machine with connected 
> device.
> But after unlug/plug the device disappeared and isn't detected again.
> 
> 
> (Probably a found once more non-functional device (USB keyboard) that wasn't
> detected but I thinked that problem is in keyboard...)
> 
> Some other devices work fine (USB modem, flashdisk).
> 
> Workaround is to downgrade to the kernel 3.16.
> 
> System: debian unstable, selfcompiled vanilla, Thinkpad X61x

Are there any kernel log messages when you plug the device in?

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: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Alan Stern
On Wed, 12 Nov 2014, Felipe Balbi wrote:

> > By the way, does the same sort of thing happen after a transfer
> > error (such as a CRC mismatch)?  Does the xHCI controller change the 
> > state to EP_STATE_HALTED?  Or does it instead go directly to 
> 
> There are a few conditions in which XHCI will change EP state to
> EP_STATE_HALTED, one of them is a STALL token from the peripheral and
> the others would be really error conditions: Babble, CRC error, etc.
> 
> The spec even has a note about it, which I quote:
> 
>   "
>   A Halt condition or USB Transaction error detected on a USB pipe
>   shall cause a Running Endpoint to transition to the Halted
>   state. A Reset Endpoint Command shall be used to clear the Halt
>   condition on the endpoint and transition the endpoint to the
>   Stopped state. A Stop Endpoint Command received while an
>   endpoint is in the Halted state shall have no effect and shall
>   generate a Command Completion Event with the Completion Code set
>   to Context State Error.
>   "
> 
> > EP_STATE_STOPPED?  You probably want to treat that case and the STALL 
> > case as similarly as possible.
> 
> There's already code to deal with that, take a look at
> xhci_requires_manual_halt_cleanup() and its callers:

Then shouldn't the recovery from a STALL be exactly the same as the 
recovery from any other sort of transfer error?


> > Right.  In theory you could do it any time up until the completion
> > routine returns.  Doing it when you process the failed TD seems like a
> > good choice -- advance the dequeue pointer and issue the command at the
> > same time.
> 
> My concern here is that this will happen when the first usb_submit_urb()
> completes. I wonder if moving this to when the following
> usb_submit_urb() is about to start would be better ?

No, because there may be some other URBs already on the endpoint queue.  
If no further URBs are submitted then the queue won't get cleaned up.

> I think this has a higher probability of being correct. Class driver
> might not queue any URB to a particular after the first Stall, so why
> would be move the endpoint away from EP_STATE_HALTED prematurely ?

In order to handle the queue.  Of course, if the queue is empty then 
there's no harm in leaving the ep in EP_STATE_HALTED until another URB 
is submitted.

By the same reasoning, when the state is changed to EP_STATE_STOPPED, 
the doorbell should be rung.

Alan Stern

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


Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Mathias Nyman
Hi

On 13.11.2014 17:11, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Nov 13, 2014 at 01:16:34PM +0200, Mathias Nyman wrote:
>> On 13.11.2014 00:28, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Wed, Nov 12, 2014 at 04:54:06PM -0500, Alan Stern wrote:
>>>
>>> [...]
>>>
> and the doorbell will never rung. But even if I drop EP_HALTED from the
> check below and let EP doorbell be rung, nothing will happen because,
> according to XHCI spec 1.0, we *must* first issue a Reset Endpoint
> command to get Endpoint to move to EP_STATE_STOPPED, in which case,
> ringing that EP's doorbell will actually cause a transfer to happen.
>
> Right now, what happens is that second usb_submit_urb() does nothing and
> the 10 second timer expires, causing the URB to be canceled and test
> failing with -ETIMEDOUT.

 Okay, I see.  What usbcore and usbtest expect to happen is this:

 (1)An URB fails because the endpoint sent a STALL.  The completion
routine is called with status -EPIPE.

 (2)When the completion routine returns, the next URB in the
endpoint's queue should get handled by the hardware.  If the
endpoint is still halted, this URB should fail with -EPIPE
just like the first.

 (3)Etc.  Eventually the endpoint queue empties out or the class
driver calls usb_clear_halt().
>>>
>>> perfect :-)
>>>
 So (1) works as desired, but (2) doesn't work because the doorbell
 never gets rung.
>>>
>>> right.
>>>
 And the easiest way to make (2) work is to issue a Reset Endpoint
 command.
>>>
>>> There's one extra bit of information here, see below.
>>>
 (There are other, more complicated ways to get the same result.  For 
 instance, you could loop through the remaining queued URBs, giving them 
 back one by one with -EPIPE.  And each time an URB is submitted, you 
 could give it back right away.  But Reset Endpoint is simpler.)
>>>
>>> IMO issuing a Reset Endpoint is the only correct way of implementing
>>> this. See, there are two sides to usbtest + g_zero love relationship:
>>>
>>> (a) it will help you test your UDC driver.
>>> (b) it will help you test your HCD driver.
>>>
>>> Currently, we have a bug with (b), but if iterate over the list of
>>> submitted URBs we can create two problems:
>>>
>>> (i) There's no way to synchronize class driver's usb_submit_urb() with
>>> the exact time when we iterate over the list of pending URBs in
>>> xhci-hcd. Which means that we could very well iterate over an empty list
>>> and think everything was fine. Heck, this would even happen with
>>> usbtest, note that simple_io() always waits 10 seconds for a transfer
>>> completion before returning control to the caller.
>>>
>>> (ii) We would fall into the possibility of not catching bugs with UDCs
>>> running g_zero because HCD would just return early -EPIPE for us without
>>> asking UDC to handle another transfer :-)
>>>
>>> Because of these two cases, I think the *only* way to solve this is by
>>> issuing a Reset Endpoint cmd so another token can be shifted onto the
>>> data lines.
>>>
 In the patch, you talked about clearing the endpoint halt.  But that's 
 not what you want to do; you want to issue a Reset Endpoint command, 
 which affects only the host controller.  The endpoint's status in the 
>>>
>>> this is exactly what xhci_cleanup_halted_endpoint() does. Sure, it's a
>>> misnamer and should probably be renamed to either
>>> xhci_reset_endpoint_cmd() or xhci_clear_endpoint_state_halted() to make
>>> it clearer as to what's happening. But that can happen later, we don't
>>> need to clean that up in order to fix the bug :-)
>>>
 peripheral device will remain unchanged -- no halt will be cleared.  
 That contributed to my confusion on reading the patch.
>>>
>>> yeah, that got me for a while too. I had to keep reminding myself what
>>> xhci_cleanup_halted_endpoint() was doing ;-)
>>>
 By the way, does the same sort of thing happen after a transfer
 error (such as a CRC mismatch)?  Does the xHCI controller change the 
 state to EP_STATE_HALTED?  Or does it instead go directly to 
>>>
>>> There are a few conditions in which XHCI will change EP state to
>>> EP_STATE_HALTED, one of them is a STALL token from the peripheral and
>>> the others would be really error conditions: Babble, CRC error, etc.
>>>
>>> The spec even has a note about it, which I quote:
>>>
>>> "
>>> A Halt condition or USB Transaction error detected on a USB pipe
>>> shall cause a Running Endpoint to transition to the Halted
>>> state. A Reset Endpoint Command shall be used to clear the Halt
>>> condition on the endpoint and transition the endpoint to the
>>> Stopped state. A Stop Endpoint Command received while an
>>> endpoint is in the Halted state shall have no effect and shall
>>> generate a Command Completion Event with the Completion Code set
>>> to Context Sta

Re: [PATCH v3 1/2] usb: dwc2/gadget: add mutex to serialize init/deinit calls

2014-11-13 Thread Marek Szyprowski

Hello,

On 2014-10-31 19:46, Paul Zimmerman wrote:

From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Friday, October 31, 2014 3:13 AM

This patch adds mutex, which protects initialization and
deinitialization procedures against suspend/resume methods.

Signed-off-by: Marek Szyprowski 
---
  drivers/usb/dwc2/core.h   |  1 +
  drivers/usb/dwc2/gadget.c | 20 
  2 files changed, 21 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9f77b4d1c5ff..58732a9a0019 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -187,6 +187,7 @@ struct s3c_hsotg {
struct s3c_hsotg_plat*plat;

spinlock_t  lock;
+   struct mutexinit_mutex;

void __iomem*regs;
int irq;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d8dda39c9e16..a2e4272a904e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -21,6 +21,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
return -EINVAL;
}

+   mutex_lock(&hsotg->init_mutex);
WARN_ON(hsotg->driver);

driver->driver.bus = NULL;
@@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,

dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);

+   mutex_unlock(&hsotg->init_mutex);
+
return 0;

  err:
+   mutex_unlock(&hsotg->init_mutex);
hsotg->driver = NULL;
return ret;
  }
@@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
if (!hsotg)
return -ENODEV;

+   mutex_lock(&hsotg->init_mutex);
+
/* all endpoints should be shutdown */
for (ep = 1; ep < hsotg->num_of_eps; ep++)
s3c_hsotg_ep_disable(&hsotg->eps[ep].ep);
@@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,

clk_disable(hsotg->clk);

+   mutex_unlock(&hsotg->init_mutex);
+
return 0;
  }

@@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, 
int is_on)

dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);

+   mutex_lock(&hsotg->init_mutex);
spin_lock_irqsave(&hsotg->lock, flags);
if (is_on) {
clk_enable(hsotg->clk);
@@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, 
int is_on)

hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(&hsotg->lock, flags);
+   mutex_unlock(&hsotg->init_mutex);

return 0;
  }
@@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
}

spin_lock_init(&hsotg->lock);
+   mutex_init(&hsotg->init_mutex);

hsotg->irq = ret;

@@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device 
*pdev, pm_message_t state)
unsigned long flags;
int ret = 0;

+   mutex_lock(&hsotg->init_mutex);
+
if (hsotg->driver)
dev_info(hsotg->dev, "suspending usb gadget %s\n",
 hsotg->driver->driver.name);
@@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device 
*pdev, pm_message_t state)
clk_disable(hsotg->clk);
}

+   mutex_unlock(&hsotg->init_mutex);
+
return ret;
  }

@@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
unsigned long flags;
int ret = 0;

+   mutex_lock(&hsotg->init_mutex);
if (hsotg->driver) {
+
dev_info(hsotg->dev, "resuming usb gadget %s\n",
 hsotg->driver->driver.name);

@@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
s3c_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(&hsotg->lock, flags);

+   mutex_unlock(&hsotg->init_mutex);
+
return ret;
  }


Hmm. I can't find any other UDC driver that uses a mutex in its
suspend/resume functions. Can you explain why this is needed only
for dwc2?


I've posted this version because I thought you were not convinced that 
the patch
"usb: dwc2/gadget: rework suspend/resume code to correctly restore 
gadget state"
can add code for initialization and deinitialization in suspend/resume 
paths.


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe 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/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Felipe Balbi
Hi,

On Thu, Nov 13, 2014 at 01:16:34PM +0200, Mathias Nyman wrote:
> On 13.11.2014 00:28, Felipe Balbi wrote:
> > Hi,
> > 
> > On Wed, Nov 12, 2014 at 04:54:06PM -0500, Alan Stern wrote:
> > 
> > [...]
> > 
> >>> and the doorbell will never rung. But even if I drop EP_HALTED from the
> >>> check below and let EP doorbell be rung, nothing will happen because,
> >>> according to XHCI spec 1.0, we *must* first issue a Reset Endpoint
> >>> command to get Endpoint to move to EP_STATE_STOPPED, in which case,
> >>> ringing that EP's doorbell will actually cause a transfer to happen.
> >>>
> >>> Right now, what happens is that second usb_submit_urb() does nothing and
> >>> the 10 second timer expires, causing the URB to be canceled and test
> >>> failing with -ETIMEDOUT.
> >>
> >> Okay, I see.  What usbcore and usbtest expect to happen is this:
> >>
> >> (1)An URB fails because the endpoint sent a STALL.  The completion
> >>routine is called with status -EPIPE.
> >>
> >> (2)When the completion routine returns, the next URB in the
> >>endpoint's queue should get handled by the hardware.  If the
> >>endpoint is still halted, this URB should fail with -EPIPE
> >>just like the first.
> >>
> >> (3)Etc.  Eventually the endpoint queue empties out or the class
> >>driver calls usb_clear_halt().
> > 
> > perfect :-)
> > 
> >> So (1) works as desired, but (2) doesn't work because the doorbell
> >> never gets rung.
> > 
> > right.
> > 
> >> And the easiest way to make (2) work is to issue a Reset Endpoint
> >> command.
> > 
> > There's one extra bit of information here, see below.
> > 
> >> (There are other, more complicated ways to get the same result.  For 
> >> instance, you could loop through the remaining queued URBs, giving them 
> >> back one by one with -EPIPE.  And each time an URB is submitted, you 
> >> could give it back right away.  But Reset Endpoint is simpler.)
> > 
> > IMO issuing a Reset Endpoint is the only correct way of implementing
> > this. See, there are two sides to usbtest + g_zero love relationship:
> > 
> > (a) it will help you test your UDC driver.
> > (b) it will help you test your HCD driver.
> > 
> > Currently, we have a bug with (b), but if iterate over the list of
> > submitted URBs we can create two problems:
> > 
> > (i) There's no way to synchronize class driver's usb_submit_urb() with
> > the exact time when we iterate over the list of pending URBs in
> > xhci-hcd. Which means that we could very well iterate over an empty list
> > and think everything was fine. Heck, this would even happen with
> > usbtest, note that simple_io() always waits 10 seconds for a transfer
> > completion before returning control to the caller.
> > 
> > (ii) We would fall into the possibility of not catching bugs with UDCs
> > running g_zero because HCD would just return early -EPIPE for us without
> > asking UDC to handle another transfer :-)
> > 
> > Because of these two cases, I think the *only* way to solve this is by
> > issuing a Reset Endpoint cmd so another token can be shifted onto the
> > data lines.
> > 
> >> In the patch, you talked about clearing the endpoint halt.  But that's 
> >> not what you want to do; you want to issue a Reset Endpoint command, 
> >> which affects only the host controller.  The endpoint's status in the 
> > 
> > this is exactly what xhci_cleanup_halted_endpoint() does. Sure, it's a
> > misnamer and should probably be renamed to either
> > xhci_reset_endpoint_cmd() or xhci_clear_endpoint_state_halted() to make
> > it clearer as to what's happening. But that can happen later, we don't
> > need to clean that up in order to fix the bug :-)
> > 
> >> peripheral device will remain unchanged -- no halt will be cleared.  
> >> That contributed to my confusion on reading the patch.
> > 
> > yeah, that got me for a while too. I had to keep reminding myself what
> > xhci_cleanup_halted_endpoint() was doing ;-)
> > 
> >> By the way, does the same sort of thing happen after a transfer
> >> error (such as a CRC mismatch)?  Does the xHCI controller change the 
> >> state to EP_STATE_HALTED?  Or does it instead go directly to 
> > 
> > There are a few conditions in which XHCI will change EP state to
> > EP_STATE_HALTED, one of them is a STALL token from the peripheral and
> > the others would be really error conditions: Babble, CRC error, etc.
> > 
> > The spec even has a note about it, which I quote:
> > 
> > "
> > A Halt condition or USB Transaction error detected on a USB pipe
> > shall cause a Running Endpoint to transition to the Halted
> > state. A Reset Endpoint Command shall be used to clear the Halt
> > condition on the endpoint and transition the endpoint to the
> > Stopped state. A Stop Endpoint Command received while an
> > endpoint is in the Halted state shall have no effect and shall
> > generate a Command Completion Event with the Completion Code set
> > to Context State Error.
> > "
> > 
> >>

Re: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq

2014-11-13 Thread Marek Szyprowski

Hello,

On 2014-10-31 19:15, Paul Zimmerman wrote:

From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Friday, October 31, 2014 1:04 AM
To: linux-usb@vger.kernel.org; linux-samsung-...@vger.kernel.org
Cc: Marek Szyprowski; Kyungmin Park; Robert Baldyga; Paul Zimmerman; Krzysztof 
Kozlowski; Felipe Balbi
Subject: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end 
session' irq

This patch adds a call to s3c_hsotg_disconnect() from 'end session'
interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
look a bit more suitable for this event, but it is asserted only in
host mode, so in device mode we need to use something else.

Additional check has been added in s3c_hsotg_disconnect() function
to ensure that the event is reported only once after successful device
enumeration.

Signed-off-by: Marek Szyprowski 
---
  drivers/usb/dwc2/core.h   |  1 +
  drivers/usb/dwc2/gadget.c | 10 ++
  2 files changed, 11 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 55c90c53f2d6..b42df32e7737 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -212,6 +212,7 @@ struct s3c_hsotg {
struct usb_gadget   gadget;
unsigned intsetup;
unsigned long   last_rst;
+   unsigned intaddress;
struct s3c_hsotg_ep *eps;
  };

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index fcd2bb55ccca..6304efba11aa 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1114,6 +1114,7 @@ static void s3c_hsotg_process_control(struct s3c_hsotg 
*hsotg,
 DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK;
writel(dcfg, hsotg->regs + DCFG);

+   hsotg->address = ctrl->wValue;
dev_info(hsotg->dev, "new address %d\n", ctrl->wValue);

ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
@@ -2031,6 +2032,10 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg)
  {
unsigned ep;

+   if (!hsotg->address)
+   return;
+
+   hsotg->address = 0;
for (ep = 0; ep < hsotg->num_of_eps; ep++)
kill_all_requests(hsotg, &hsotg->eps[ep], -ESHUTDOWN, true);

@@ -2290,6 +2295,11 @@ irq_retry:
dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);

writel(otgint, hsotg->regs + GOTGINT);
+
+   if (otgint & GOTGINT_SES_END_DET) {
+   s3c_hsotg_disconnect(hsotg);
+   hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+   }
}

if (gintsts & GINTSTS_SESSREQINT) {

I don't think this is right. The host can send control requests to
the device before it sends a SetAddress to change from the default
address of 0. So if a GOTGINT_SES_END_DET happens before the
SetAddress, it will be ignored.

Or am I missing something?


Well, right. However before finishing enumeration (setting the address) 
host usually
only retrieves some usb descriptors what doesn't change the state of the 
gadget.
Right now we always reported 'disconnected' event before setting the new 
address,
what is a bit overkill (in some cases gadget driver got this even more 
than once).
The above code handles all cases correctly and reports disconnect event 
only once.


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe 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: [PATCHv4 2/6] phy: improved lookup method

2014-11-13 Thread Vivek Gautam
Hi Heikki, Kishon,


On Tue, Nov 11, 2014 at 2:23 PM, Vivek Gautam  wrote:
> Hi Kishon,
>
>
> On Tue, Nov 11, 2014 at 2:20 PM, Kishon Vijay Abraham I  wrote:
>> Hi,
>>
>> On Tuesday 11 November 2014 02:07 PM, Vivek Gautam wrote:
>>> On Tue, Nov 11, 2014 at 12:12 PM, Kishon Vijay Abraham I  
>>> wrote:
 Hi,

 On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
> Hi Heikki,
>
>
> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
>  wrote:
>> Removes the need for the phys to be aware of their users
>> even when not using DT. The method is copied from clkdev.c.
>>
>> Signed-off-by: Heikki Krogerus 
>> Tested-by: Vivek Gautam 
>> ---
>>  Documentation/phy.txt   |  66 ---
>>  drivers/phy/phy-core.c  | 135 
>> +++-
>>  include/linux/phy/phy.h |  27 ++
>>  3 files changed, 183 insertions(+), 45 deletions(-)
>>
>> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
>> index c6594af..8add515 100644
>> --- a/Documentation/phy.txt
>> +++ b/Documentation/phy.txt
>> @@ -54,18 +54,14 @@ The PHY driver should create the PHY in order for 
>> other peripheral controllers
>>  to make use of it. The PHY framework provides 2 APIs to create the PHY.
>>
>>  struct phy *phy_create(struct device *dev, struct device_node *node,
>> -  const struct phy_ops *ops,
>> -  struct phy_init_data *init_data);
>> +  const struct phy_ops *ops);
>>  struct phy *devm_phy_create(struct device *dev, struct device_node 
>> *node,
>> -   const struct phy_ops *ops,
>> -   struct phy_init_data *init_data);
>> +   const struct phy_ops *ops);
>>
>>  The PHY drivers can use one of the above 2 APIs to create the PHY by 
>> passing
>> -the device pointer, phy ops and init_data.
>> +the device pointer and phy ops.
>>  phy_ops is a set of function pointers for performing PHY operations 
>> such as
>> -init, exit, power_on and power_off. *init_data* is mandatory to get a 
>> reference
>> -to the PHY in the case of non-dt boot. See section *Board File 
>> Initialization*
>> -on how init_data should be used.
>> +init, exit, power_on and power_off.
>>
>>  Inorder to dereference the private data (in phy_ops), the phy provider 
>> driver
>>  can use phy_set_drvdata() after creating the PHY and use 
>> phy_get_drvdata() in
>> @@ -137,42 +133,24 @@ There are exported APIs like phy_pm_runtime_get, 
>> phy_pm_runtime_get_sync,
>>  phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
>>  phy_pm_runtime_forbid for performing PM operations.
>>
>> -8. Board File Initialization
>> -
>> -Certain board file initialization is necessary in order to get a 
>> reference
>> -to the PHY in the case of non-dt boot.
>> -Say we have a single device that implements 3 PHYs that of USB, SATA 
>> and PCIe,
>> -then in the board file the following initialization should be done.
>> -
>> -struct phy_consumer consumers[] = {
>> -   PHY_CONSUMER("dwc3.0", "usb"),
>> -   PHY_CONSUMER("pcie.0", "pcie"),
>> -   PHY_CONSUMER("sata.0", "sata"),
>> -};
>> -PHY_CONSUMER takes 2 parameters, first is the device name of the 
>> controller
>> -(PHY consumer) and second is the port name.
>> -
>> -struct phy_init_data init_data = {
>> -   .consumers = consumers,
>> -   .num_consumers = ARRAY_SIZE(consumers),
>> -};
>> -
>> -static const struct platform_device pipe3_phy_dev = {
>> -   .name = "pipe3-phy",
>> -   .id = -1,
>> -   .dev = {
>> -   .platform_data = {
>> -   .init_data = &init_data,
>> -   },
>> -   },
>> -};
>> -
>> -then, while doing phy_create, the PHY driver should pass this init_data
>> -   phy_create(dev, ops, pdata->init_data);
>> -
>> -and the controller driver (phy consumer) should pass the port name 
>> along with
>> -the device to get a reference to the PHY
>> -   phy_get(dev, "pcie");
>> +8. PHY Mappings
>> +
>> +In order to get reference to a PHY without help from DeviceTree, the 
>> framework
>> +offers lookups which can be compared to clkdev that allow clk 
>> structures to be
>> +bound to devices. A lookup can be made statically by directly 
>> registering
>> +phy_lookup structure which contains the name of the PHY device, the 
>> name of the
>> +device which the PHY will be bind to and Connection ID string. 
>> Alternatively a
>> +lookup can be made during runtime when a handle to the struct phy 
>> already
>> +ex

Re: [RFC/PATCH v2] usb: host: xhci: issue 'Reset Endpoint' for !CONTROL EPs from finish_td()

2014-11-13 Thread Mathias Nyman
On 13.11.2014 00:28, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Nov 12, 2014 at 04:54:06PM -0500, Alan Stern wrote:
> 
> [...]
> 
>>> and the doorbell will never rung. But even if I drop EP_HALTED from the
>>> check below and let EP doorbell be rung, nothing will happen because,
>>> according to XHCI spec 1.0, we *must* first issue a Reset Endpoint
>>> command to get Endpoint to move to EP_STATE_STOPPED, in which case,
>>> ringing that EP's doorbell will actually cause a transfer to happen.
>>>
>>> Right now, what happens is that second usb_submit_urb() does nothing and
>>> the 10 second timer expires, causing the URB to be canceled and test
>>> failing with -ETIMEDOUT.
>>
>> Okay, I see.  What usbcore and usbtest expect to happen is this:
>>
>> (1)  An URB fails because the endpoint sent a STALL.  The completion
>>  routine is called with status -EPIPE.
>>
>> (2)  When the completion routine returns, the next URB in the
>>  endpoint's queue should get handled by the hardware.  If the
>>  endpoint is still halted, this URB should fail with -EPIPE
>>  just like the first.
>>
>> (3)  Etc.  Eventually the endpoint queue empties out or the class
>>  driver calls usb_clear_halt().
> 
> perfect :-)
> 
>> So (1) works as desired, but (2) doesn't work because the doorbell
>> never gets rung.
> 
> right.
> 
>> And the easiest way to make (2) work is to issue a Reset Endpoint
>> command.
> 
> There's one extra bit of information here, see below.
> 
>> (There are other, more complicated ways to get the same result.  For 
>> instance, you could loop through the remaining queued URBs, giving them 
>> back one by one with -EPIPE.  And each time an URB is submitted, you 
>> could give it back right away.  But Reset Endpoint is simpler.)
> 
> IMO issuing a Reset Endpoint is the only correct way of implementing
> this. See, there are two sides to usbtest + g_zero love relationship:
> 
> (a) it will help you test your UDC driver.
> (b) it will help you test your HCD driver.
> 
> Currently, we have a bug with (b), but if iterate over the list of
> submitted URBs we can create two problems:
> 
> (i) There's no way to synchronize class driver's usb_submit_urb() with
> the exact time when we iterate over the list of pending URBs in
> xhci-hcd. Which means that we could very well iterate over an empty list
> and think everything was fine. Heck, this would even happen with
> usbtest, note that simple_io() always waits 10 seconds for a transfer
> completion before returning control to the caller.
> 
> (ii) We would fall into the possibility of not catching bugs with UDCs
> running g_zero because HCD would just return early -EPIPE for us without
> asking UDC to handle another transfer :-)
> 
> Because of these two cases, I think the *only* way to solve this is by
> issuing a Reset Endpoint cmd so another token can be shifted onto the
> data lines.
> 
>> In the patch, you talked about clearing the endpoint halt.  But that's 
>> not what you want to do; you want to issue a Reset Endpoint command, 
>> which affects only the host controller.  The endpoint's status in the 
> 
> this is exactly what xhci_cleanup_halted_endpoint() does. Sure, it's a
> misnamer and should probably be renamed to either
> xhci_reset_endpoint_cmd() or xhci_clear_endpoint_state_halted() to make
> it clearer as to what's happening. But that can happen later, we don't
> need to clean that up in order to fix the bug :-)
> 
>> peripheral device will remain unchanged -- no halt will be cleared.  
>> That contributed to my confusion on reading the patch.
> 
> yeah, that got me for a while too. I had to keep reminding myself what
> xhci_cleanup_halted_endpoint() was doing ;-)
> 
>> By the way, does the same sort of thing happen after a transfer
>> error (such as a CRC mismatch)?  Does the xHCI controller change the 
>> state to EP_STATE_HALTED?  Or does it instead go directly to 
> 
> There are a few conditions in which XHCI will change EP state to
> EP_STATE_HALTED, one of them is a STALL token from the peripheral and
> the others would be really error conditions: Babble, CRC error, etc.
> 
> The spec even has a note about it, which I quote:
> 
>   "
>   A Halt condition or USB Transaction error detected on a USB pipe
>   shall cause a Running Endpoint to transition to the Halted
>   state. A Reset Endpoint Command shall be used to clear the Halt
>   condition on the endpoint and transition the endpoint to the
>   Stopped state. A Stop Endpoint Command received while an
>   endpoint is in the Halted state shall have no effect and shall
>   generate a Command Completion Event with the Completion Code set
>   to Context State Error.
>   "
> 
>> EP_STATE_STOPPED?  You probably want to treat that case and the STALL 
>> case as similarly as possible.
> 
> There's already code to deal with that, take a look at
> xhci_requires_manual_halt_cleanup() and its callers:
> 
> 1754 static int xhci_req

driver pl2303 doesn't detect device after upgrade

2014-11-13 Thread Milan Kocian
Hi,

after upgrade from 3.16 to 3.17.2 this device isn't in lsusb output
after plugging in :

Bus 004 Device 002: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port
(driver pl2303, usb to serial converter)

The only way how to make it functional is to boot machine with connected device.
But after unlug/plug the device disappeared and isn't detected again.


(Probably a found once more non-functional device (USB keyboard) that wasn't
detected but I thinked that problem is in keyboard...)

Some other devices work fine (USB modem, flashdisk).

Workaround is to downgrade to the kernel 3.16.

System: debian unstable, selfcompiled vanilla, Thinkpad X61x

Best regards,

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