[PATCH/RFC 2/2] usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume
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
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&r=1&w=2 http://marc.info/?t=14871253435&r=1&w=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
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 ? &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
> -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
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(&pdev->dev); > pm_runtime_enable(&pdev->dev); > + device_enable_async_suspend(&pdev->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
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(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->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
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