[PATCH/RFC 2/2] usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume

[Yoshihiro Shimoda]

This patch fixes an issue that a usb 1.1 device is not connected in
system resume and then the following message appeared if debug messages
are enabled:
usb 2-1: Waited 2000ms for CONNECT

To resolve this issue, the EHCI controller must be resumed after its
companion controllers. So, this patch adds such code on the driver.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/ehci-platform.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index a268d9e..65a7725 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ehci.h"
 
@@ -297,6 +298,8 @@ static int ehci_platform_probe(struct platform_device *dev)
goto err_power;
 
device_wakeup_enable(hcd->self.controller);
+   if (usb_of_has_companion(hcd->self.controller))
+   device_enable_async_suspend(hcd->self.controller);
platform_set_drvdata(dev, hcd);
 
return err;
@@ -370,6 +373,7 @@ static int ehci_platform_resume(struct device *dev)
struct usb_ehci_pdata *pdata = dev_get_platdata(dev);
struct platform_device *pdev = to_platform_device(dev);
struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   struct device *companion_dev;
 
if (pdata->power_on) {
int err = pdata->power_on(pdev);
@@ -377,6 +381,10 @@ static int ehci_platform_resume(struct device *dev)
return err;
}
 
+   companion_dev = usb_of_get_companion_dev(hcd->self.controller);
+   if (companion_dev)
+   device_pm_wait_for_dev(hcd->self.controller, companion_dev);
+
ehci_resume(hcd, priv->reset_on_resume);
return 0;
 }
-- 
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/RFC 0/2] usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume

[Yoshihiro Shimoda]

This patch set is based on Greg's usb.git / usb-next branch
(the commit id = 0df8a3dbacb585bb9c8b2e55de43c6aac9d86488).

This patch set is related to the following email threads:
 http://marc.info/?t=14865351421=1=2
 http://marc.info/?t=14871253435=1=2

Since I'm not sure the helper functions can be in core/of.c,
I sent the patch set as RFC. I guess if these functions are not needed
to other codes, the functions should be into ehci-platform.c.

Yoshihiro Shimoda (2):
  usb: of: add functions to bind a companion controller
  usb: host: ehci-platform: fix usb 1.1 device is not connected in
system resume

 Documentation/devicetree/bindings/usb/generic.txt |  1 +
 drivers/usb/core/of.c | 36 +++
 drivers/usb/host/ehci-platform.c  |  8 +
 include/linux/usb/of.h| 10 +++
 4 files changed, 55 insertions(+)

-- 
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/RFC 1/2] usb: of: add functions to bind a companion controller

[Yoshihiro Shimoda]

EHCI controllers will have a companion controller. However, on platform
bus, there was difficult to bind them in previous code. So, this
patch adds helper functions to bind them using a "companion" property.

Signed-off-by: Yoshihiro Shimoda 
---
 Documentation/devicetree/bindings/usb/generic.txt |  1 +
 drivers/usb/core/of.c | 36 +++
 include/linux/usb/of.h| 10 +++
 3 files changed, 47 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index bfadeb1..0a74ab8 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -22,6 +22,7 @@ Optional properties:
property is used if any real OTG features(HNP/SRP/ADP)
is enabled, if ADP is required, otg-rev should be
0x0200 or above.
+ - companion: phandle of a companion
  - hnp-disable: tells OTG controllers we want to disable OTG HNP, normally HNP
is the basic function of real OTG except you want it
to be a srp-capable only B device.
diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
index 3de4f88..033530b 100644
--- a/drivers/usb/core/of.c
+++ b/drivers/usb/core/of.c
@@ -18,6 +18,7 @@
  */
 
 #include 
+#include 
 #include 
 
 /**
@@ -46,3 +47,38 @@ struct device_node *usb_of_get_child_node(struct device_node 
*parent,
 }
 EXPORT_SYMBOL_GPL(usb_of_get_child_node);
 
+/**
+ * usb_of_has_companion - To get if the device has a companion device
+ * @dev: the device pointer to get if it has a companion
+ *
+ * This function gets if the device has a companion property.
+ *
+ */
+bool usb_of_has_companion(struct device *dev)
+{
+   return !!of_find_property(dev->of_node, "companion", NULL);
+}
+EXPORT_SYMBOL_GPL(usb_of_has_companion);
+
+/**
+ * usb_of_get_companion_dev - Find the companion device
+ * @dev: the device pointer to find a companion
+ *
+ * Find the companion device from platform bus.
+ *
+ * Return: On success, a pointer to the companion device, %NULL on failure.
+ */
+struct device *usb_of_get_companion_dev(struct device *dev)
+{
+   struct device_node *node;
+   struct platform_device *pdev = NULL;
+
+   node = of_parse_phandle(dev->of_node, "companion", 0);
+   if (node)
+   pdev = of_find_device_by_node(node);
+
+   of_node_put(node);
+
+   return pdev ? >dev : NULL;
+}
+EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index 5ff9032..a5c7885 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -18,6 +18,8 @@ int of_usb_update_otg_caps(struct device_node *np,
struct usb_otg_caps *otg_caps);
 struct device_node *usb_of_get_child_node(struct device_node *parent,
int portnum);
+bool usb_of_has_companion(struct device *dev);
+struct device *usb_of_get_companion_dev(struct device *dev);
 #else
 static inline enum usb_dr_mode
 of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
@@ -38,6 +40,14 @@ static inline int of_usb_update_otg_caps(struct device_node 
*np,
 {
return NULL;
 }
+static inline bool usb_of_has_companion(struct device *dev)
+{
+   return false;
+}
+static inline struct device *usb_of_get_companion_dev(struct device *dev)
+{
+   return NULL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT)
-- 
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] usb: ohci-at91: revert patch 2e2aa1bc7eff90ec on cpu without SFR register

[Wenyou.Yang]

> -Original Message-
> From: Jelle Martijn Kok [mailto:jm...@youcom.nl]
> Sent: 2017年2月16日 23:20
> To: linux-usb@vger.kernel.org
> Cc: Wenyou Yang - A41535 ; Alan Stern
> 
> Subject: [PATCH] usb: ohci-at91: revert patch 2e2aa1bc7eff90ec on cpu without
> SFR register
> 
> External USB hubs seems to go into suspend, but never wakeup again.
> Tested on an AT91SAM9G20
> 
> Signed-off-by: Jelle Martijn Kok 

Thank you for your discovery and fixed.

Tested-by: Wenyou Yang 


> ---
>   drivers/usb/host/ohci-at91.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index
> b38a228..af0566d 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -361,7 +361,7 @@ static int ohci_at91_hub_control(struct usb_hcd *hcd, u16
> typeReq, u16 wValue,
>   case USB_PORT_FEAT_SUSPEND:
>   dev_dbg(hcd->self.controller, "SetPortFeat: SUSPEND\n");
> - if (valid_port(wIndex)) {
> + if (valid_port(wIndex) && ohci_at91->sfr_regmap) {
>   ohci_at91_port_suspend(ohci_at91->sfr_regmap,
>  1);
>   return 0;
> @@ -404,7 +404,7 @@ static int ohci_at91_hub_control(struct usb_hcd *hcd, u16
> typeReq, u16 wValue,
>   case USB_PORT_FEAT_SUSPEND:
>   dev_dbg(hcd->self.controller, "ClearPortFeature:
> SUSPEND\n");
> - if (valid_port(wIndex)) {
> + if (valid_port(wIndex) && ohci_at91->sfr_regmap) {
>   ohci_at91_port_suspend(ohci_at91->sfr_regmap,
>  0);
>   return 0;
> --
> 2.1.4


Best Regards,
Wenyou Yang

Re: [PATCH RESEND v7 1/1] usb: xhci: plat: Enable async suspend/resume

[Baolin Wang]

On 10 February 2017 at 22:56, Robert Foss  wrote:
> From: Andrew Bresticker 
>
> USB host controllers can take a significant amount of time to suspend
> and resume, adding several hundred miliseconds to the kernel resume
> time. Since the XHCI controller has no outside dependencies (other than
> clocks, which are suspended late/resumed early), allow it to suspend and
> resume asynchronously.
>
> Signed-off-by: Andrew Bresticker 
> Tested-by: Andrew Bresticker 
> Tested-by: Robert Foss 
> Signed-off-by: Robert Foss 
> ---
>  drivers/usb/host/xhci-plat.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3cf8e120c620..4d6741a0d8f8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -257,6 +257,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>
> pm_runtime_set_active(>dev);
> pm_runtime_enable(>dev);
> +   device_enable_async_suspend(>dev);
>
> return 0;
>
> --
> 2.11.0.453.g787f75f05
>

Reviewed-by: Baolin Wang 

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

Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM

[Baolin Wang]

Hi Mathias,

On 6 February 2017 at 13:26, Baolin Wang  wrote:
> Hi Mathias,
>
> On 31 January 2017 at 21:14, Mathias Nyman
>  wrote:
>> On 16.01.2017 12:56, Baolin Wang wrote:
>>>
>>> Hi Mathias,
>>
>>
>> Hi
>>
>> Sorry about the long review delay
>> CC Alan in case my pm assumptions need to be corrected
>>
>>
>>>
>>> On 13 December 2016 at 15:49, Baolin Wang  wrote:

 Enable the xhci plat runtime PM for parent device to suspend/resume xhci.
 Also call pm_runtime_get_noresume() in probe() function in case the
 parent
 device doesn't call suspend/resume callback by runtime PM now.

 Signed-off-by: Baolin Wang 
 ---
 Changes since v4:
   - No updates.

 Changes since v3:
   - Fix kbuild error.

 Changes since v2:
   - Add pm_runtime_get_noresume() in probe() function.
   - Add pm_runtime_set_suspended()/pm_runtime_put_noidle() in remove()
 function.

 Changes since v1:
   - No updates.
 ---
>>>
>>>
>>> Do you have any comments about this patch? Thanks.
>>>
   drivers/usb/host/xhci-plat.c |   41
 -
   1 file changed, 36 insertions(+), 5 deletions(-)

 diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
 index ed56bf9..5805c6a 100644
 --- a/drivers/usb/host/xhci-plat.c
 +++ b/drivers/usb/host/xhci-plat.c
 @@ -246,6 +246,10 @@ static int xhci_plat_probe(struct platform_device
 *pdev)
  if (ret)
  goto dealloc_usb2_hcd;

 +   pm_runtime_get_noresume(>dev);
 +   pm_runtime_set_active(>dev);
 +   pm_runtime_enable(>dev);
 +
  return 0;


>>
>> To me this looks like we are not really enabling runtime pm for xhci, we
>> increment the
>> usage count in probe, and keep it until remove is called.
>>
>> This would normally prevent parent dwc3 from runtime suspending, but I see
>> that in patch 2/2 adds
>> pm_suspend_ignore_children(dev, true) to dwc3, and, that dwc3 actually
>> controls xhci runtime
>> pm by calling pm_runtime_get/put for xhci in its own dwc3
>> runtime_suspend/resume callbacks.
>>
>> Looks a bit upside down to me, what's the reason for this?
>>
>> what prevents calling pm_runtime_put_* before leaving probe in xhci and let
>> pm code handle
>> the runtime suspend and parent/child relationships?
>
> When dwc3 controller is working on host mode, which will create and
> add the USB hcd for host side. Then if we want to suspend dwc3
> controller as host mode, the USB device (bus) of host side will
> runtime suspend automatically if there are no slave attached (by
> usb_runtime_suspend()--->usb_suspend_both()--->usb_suspend_interface()--->usb_suspend_device()--->generic_suspend()--->hcd_bus_suspend()--->xhci_bus_suspend()),
> but we should not suspend xHCI device (issuing xhci_suspend()) now,
> since some other  controllers did not implement the runtime PM to
> control xHCI device's runtime state, which will cause other
> controllers can not resume xHCI device (issuing xhci_resume()) if the
> xHCI device suspend automatically by its child device. Thus we should
> get the runtime count for xHCI device in xhci_plat_probe() in case
> xHCI device will also suspend automatically by its child device.
> According to that, for xHCI device's parent: dwc3 device, we should
> put the xHCI device's runtime count to suspend xHCI device manually.
>

Any more comments?

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

Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase

[Baolin Wang]

On 17 February 2017 at 16:04, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
 (One possible approach would be to have the setup routine return
 different values for explicit and implicit status stages -- for
 example, return 1 if it wants to submit an explicit status request.
 That wouldn't be very different from the current
 USB_GADGET_DELAYED_STATUS approach.)
>>>
>>> not really, no. The idea was for composite.c and/or functions to support
>>> both methods (temporarily) and use "gadget->wants_explicit_stages" to
>>> explicitly queue DATA and STATUS. That would mean that f_mass_storage
>>> wouldn't have to return DELAYED_STATUS if
>>> (gadget->wants_explicit_stages).
>>>
>>> After all UDCs are converted over and set wants_explicit_stages (which
>>> should all be done in a single series), then we get rid of the flag and
>>> the older method of DELAYED_STATUS.
>>
>> (Sorry for late reply due to my holiday)
>> I also met the problem pointed by Alan, from my test, I still want to
>> need one return value to indicate if it wants to submit an explicit
>> status request. Think about the Control-IN with a data stage, we can
>> not get the STATUS phase request from usb_ep_queue() call, and we need
>
> why not? wLength tells you that this is a 3-stage transfer. Gadget
> driver should be able to figure out that it needs to usb_ep_queue()
> another request for status stage.
>
>> to handle this STATUS phase request in dwc3_ep0_xfernotready(). But
>> Control-OUT will get one 0-length IN request for the status stage from
>> usb_ep_queue(), so we need one return value from setup routine to
>
> no we don't :-)
>
>> distinguish these in dwc3_ep0_xfernotready(), or we can not handle
>> status request correctly. Maybe I missed something else.
>>>
 On the other hand, I am very doubtful about requiring explicit setup
 requests.
>>>
>>> right, me too ;-)
>>
>> So do you suggest me continue to try to do this? Thanks.
>
> explicit setup? no
> explicit status? yes
>
> If you don't wanna do it, it's fine :-) I'll just add to my TODO
> list. It just depends on how much other tasks you have on your end ;-)

OK, I will take some time to check and test again. It will be better
if I send out one RFC patch to review.

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