* Re: [PATCH 2/2] usb: dwc3: Host wake up support from system suspend
2020-06-11 14:28 ` [PATCH 2/2] usb: dwc3: Host wake up support from system suspend Sandeep Maheswaram
@ 2020-06-11 23:21 ` Stephen Boyd
2020-06-15 1:43 ` Peter Chen
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2020-06-11 23:21 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Doug Anderson, Felipe Balbi,
Greg Kroah-Hartman, Mark Rutland, Matthias Kaehlcke,
Michael Turquette, Rob Herring, Sandeep Maheswaram
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
linux-clk, Taniya Das, Sandeep Maheswaram
Quoting Sandeep Maheswaram (2020-06-11 07:28:03)
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..8370350 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,15 +31,19 @@
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> #include <linux/usb/otg.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
> #include "gadget.h"
> #include "io.h"
>
> #include "debug.h"
> +#include "../host/xhci.h"
>
> #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
>
> +bool need_phy_for_wakeup;
static? But why isn't it part of 'struct dwc3'? There could be multiple
dwc3 instances that may or may not be wakeup capable.
> +
> /**
> * dwc3_get_dr_mode - Validates and sets dr_mode
> * @dwc: pointer to our context structure
> @@ -1627,10 +1631,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> return ret;
> }
>
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> + int i, num_ports;
> + u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
> + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> + dwc->hs_phy_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
> +
> + reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +
> + num_ports = HCS_MAX_PORTS(reg);
> + for (i = 0; i < num_ports; i++) {
> + reg = readl(&xhci_hcd->op_regs->port_status_base + i*0x10);
Please format this as 'port_status_base + i * 0x10'
> + if (reg & PORT_PE) {
> + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
> + else if (DEV_LOWSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;
> + }
> + }
> + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
> +}
> +
> static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> {
> unsigned long flags;
> u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
>
> switch (dwc->current_dr_role) {
> case DWC3_GCTL_PRTCAP_DEVICE:
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 013f42a..ff02d41 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1094,6 +1094,7 @@ struct dwc3 {
> struct phy *usb3_generic_phy;
>
> bool phys_ready;
> + int hs_phy_flags;
Does it need to be signed? Why not unsigned int or unsigned long?
>
> struct ulpi *ulpi;
> bool ulpi_ready;
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..ec183646 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -19,6 +19,7 @@
> #include <linux/usb/of.h>
> #include <linux/reset.h>
> #include <linux/iopoll.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
>
> @@ -192,21 +193,34 @@ static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
>
> static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
> {
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> +
> if (qcom->hs_phy_irq) {
> disable_irq_wake(qcom->hs_phy_irq);
> disable_irq_nosync(qcom->hs_phy_irq);
> }
> + if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
> + if (qcom->dp_hs_phy_irq) {
> + disable_irq_wake(qcom->dp_hs_phy_irq);
> + disable_irq_nosync(qcom->dp_hs_phy_irq);
> + }
> + } else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
> + if (qcom->dm_hs_phy_irq) {
> + disable_irq_wake(qcom->dm_hs_phy_irq);
> + disable_irq_nosync(qcom->dm_hs_phy_irq);
> + }
> + } else {
>
> - if (qcom->dp_hs_phy_irq) {
> - disable_irq_wake(qcom->dp_hs_phy_irq);
> - disable_irq_nosync(qcom->dp_hs_phy_irq);
> - }
> + if (qcom->dp_hs_phy_irq) {
> + disable_irq_wake(qcom->dp_hs_phy_irq);
> + disable_irq_nosync(qcom->dp_hs_phy_irq);
> + }
>
> - if (qcom->dm_hs_phy_irq) {
> - disable_irq_wake(qcom->dm_hs_phy_irq);
> - disable_irq_nosync(qcom->dm_hs_phy_irq);
> + if (qcom->dm_hs_phy_irq) {
> + disable_irq_wake(qcom->dm_hs_phy_irq);
> + disable_irq_nosync(qcom->dm_hs_phy_irq);
> + }
> }
> -
> if (qcom->ss_phy_irq) {
> disable_irq_wake(qcom->ss_phy_irq);
> disable_irq_nosync(qcom->ss_phy_irq);
> @@ -215,21 +229,34 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
>
> static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> {
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> +
> if (qcom->hs_phy_irq) {
> enable_irq(qcom->hs_phy_irq);
> enable_irq_wake(qcom->hs_phy_irq);
> }
> + if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
> + if (qcom->dp_hs_phy_irq) {
> + enable_irq(qcom->dp_hs_phy_irq);
> + enable_irq_wake(qcom->dp_hs_phy_irq);
> + }
> + } else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
> + if (qcom->dm_hs_phy_irq) {
> + enable_irq(qcom->dm_hs_phy_irq);
> + enable_irq_wake(qcom->dm_hs_phy_irq);
> + }
> + } else {
>
> - if (qcom->dp_hs_phy_irq) {
> - enable_irq(qcom->dp_hs_phy_irq);
> - enable_irq_wake(qcom->dp_hs_phy_irq);
> - }
> + if (qcom->dp_hs_phy_irq) {
> + enable_irq(qcom->dp_hs_phy_irq);
> + enable_irq_wake(qcom->dp_hs_phy_irq);
> + }
>
> - if (qcom->dm_hs_phy_irq) {
> - enable_irq(qcom->dm_hs_phy_irq);
> - enable_irq_wake(qcom->dm_hs_phy_irq);
> + if (qcom->dm_hs_phy_irq) {
> + enable_irq(qcom->dm_hs_phy_irq);
> + enable_irq_wake(qcom->dm_hs_phy_irq);
> + }
> }
> -
> if (qcom->ss_phy_irq) {
> enable_irq(qcom->ss_phy_irq);
> enable_irq_wake(qcom->ss_phy_irq);
Is it possible to move this code to use the wakeirq library? I believe
only one irq can be the "wakeup" irq in that case but maybe that is
possible if we know what mode that phy is in? Or does the superspeed and
some sort of high speed irq need to be enabled for wakeup in case a usb2
or usb3 device wants to wakeup?
> @@ -240,6 +267,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> {
> u32 val;
> int i;
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
Weird spacing here ---^
> +
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + device_init_wakeup(qcom->dev, 1);
>
> if (qcom->is_suspended)
> return 0;
> @@ -262,6 +294,8 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
> int ret;
> int i;
>
> + device_init_wakeup(qcom->dev, 0);
Usually device_init_wakeup() is called once during probe and then the
wakeup enable state for a device is controlled from userspace. Calling
this here will be semi-disastrous in the sense that we're going to be
creating and destroying a wakeup sysfs object each time we suspend. I
see that dwc3 core code has this pattern of calling device_init_wakeup()
from the suspend/resume path too, which looks wrong.
Shouldn't we be forwarding the wakeup request from the root hub to the
controller? I'm not super clear on how USB PM is supposed to work but
this doesn't look right.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/2] usb: dwc3: Host wake up support from system suspend
2020-06-11 14:28 ` [PATCH 2/2] usb: dwc3: Host wake up support from system suspend Sandeep Maheswaram
2020-06-11 23:21 ` Stephen Boyd
@ 2020-06-15 1:43 ` Peter Chen
2020-08-11 8:50 ` Manu Gautam
2020-08-11 9:12 ` Felipe Balbi
3 siblings, 0 replies; 11+ messages in thread
From: Peter Chen @ 2020-06-15 1:43 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Mark Rutland, Felipe Balbi, Stephen Boyd, Doug Anderson,
Matthias Kaehlcke, Michael Turquette, linux-arm-msm, linux-usb,
devicetree, linux-kernel, Manu Gautam, linux-clk, Taniya Das
On 20-06-11 19:58:03, Sandeep Maheswaram wrote:
> Avoiding phy powerdown in host mode so that it can be wake up by devices.
> Set usb controller wakeup capable when wakeup capable devices are
> connected to the host.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++-----
> drivers/usb/dwc3/core.h | 1 +
> drivers/usb/dwc3/dwc3-qcom.c | 66 +++++++++++++++++++++++++++++++++-----------
> 3 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..8370350 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,15 +31,19 @@
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> #include <linux/usb/otg.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
> #include "gadget.h"
> #include "io.h"
>
> #include "debug.h"
> +#include "../host/xhci.h"
>
> #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
>
> +bool need_phy_for_wakeup;
Global variable may not suitable.
> +
> /**
> * dwc3_get_dr_mode - Validates and sets dr_mode
> * @dwc: pointer to our context structure
> @@ -1627,10 +1631,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> return ret;
> }
>
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> + int i, num_ports;
> + u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
> + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> + dwc->hs_phy_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
> +
> + reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +
> + num_ports = HCS_MAX_PORTS(reg);
> + for (i = 0; i < num_ports; i++) {
> + reg = readl(&xhci_hcd->op_regs->port_status_base + i*0x10);
> + if (reg & PORT_PE) {
> + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
> + else if (DEV_LOWSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;
> + }
> + }
> + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
> +}
> +
> static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> {
> unsigned long flags;
> u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
>
> switch (dwc->current_dr_role) {
> case DWC3_GCTL_PRTCAP_DEVICE:
> @@ -1643,9 +1673,10 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> dwc3_core_exit(dwc);
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> + dwc3_set_phy_speed_flags(dwc);
> if (!PMSG_IS_AUTO(msg)) {
> - dwc3_core_exit(dwc);
> - break;
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + need_phy_for_wakeup = true;
You may only need to check device_may_wakeup(dwc->dev) for wakeup
enabled since it is at controller driver, and we only need to let
controller's wakeup feature be ready if needed.
It is not easy for driver to judge if all /sys/../wakeup/enabled are
true, consider the use case: there is a HUB with USB mouse on the
controller, if the wakeup is not enabled for USB mouse, the controller
will not be waken up through click the mouse.
> }
>
> /* Let controller to suspend HSPHY before PHY driver suspends */
> @@ -1705,11 +1736,13 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> if (!PMSG_IS_AUTO(msg)) {
> - ret = dwc3_core_init_for_resume(dwc);
> - if (ret)
> - return ret;
> - dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> - break;
> + if (!need_phy_for_wakeup) {
> + ret = dwc3_core_init_for_resume(dwc);
> + if (ret)
> + return ret;
> + dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> + break;
> + }
> }
> /* Restore GUSB2PHYCFG bits that were modified in suspend */
> reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 013f42a..ff02d41 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1094,6 +1094,7 @@ struct dwc3 {
> struct phy *usb3_generic_phy;
>
> bool phys_ready;
> + int hs_phy_flags;
>
> struct ulpi *ulpi;
> bool ulpi_ready;
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..ec183646 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
You may split core and glue changes as two patches.
> @@ -19,6 +19,7 @@
> #include <linux/usb/of.h>
> #include <linux/reset.h>
> #include <linux/iopoll.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
>
> @@ -192,21 +193,34 @@ static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
>
> if (qcom->ss_phy_irq) {
> enable_irq(qcom->ss_phy_irq);
> enable_irq_wake(qcom->ss_phy_irq);
> @@ -240,6 +267,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> {
> u32 val;
> int i;
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
> +
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + device_init_wakeup(qcom->dev, 1);
You could let the glue layer wakeup entry always there through:
device_set_wakeup_capable(dev, true), and the user could choose
if the wakeup is enabled or not.
Peter
>
> if (qcom->is_suspended)
> return 0;
> @@ -262,6 +294,8 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
> int ret;
> int i;
>
> + device_init_wakeup(qcom->dev, 0);
> +
> if (!qcom->is_suspended)
> return 0;
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/2] usb: dwc3: Host wake up support from system suspend
2020-06-11 14:28 ` [PATCH 2/2] usb: dwc3: Host wake up support from system suspend Sandeep Maheswaram
2020-06-11 23:21 ` Stephen Boyd
2020-06-15 1:43 ` Peter Chen
@ 2020-08-11 8:50 ` Manu Gautam
2020-08-11 9:12 ` Felipe Balbi
3 siblings, 0 replies; 11+ messages in thread
From: Manu Gautam @ 2020-08-11 8:50 UTC (permalink / raw)
To: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
Stephen Boyd, Doug Anderson, Matthias Kaehlcke,
Michael Turquette
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, linux-clk,
Taniya Das
Hi,
On 6/11/2020 7:58 PM, Sandeep Maheswaram wrote:
> Avoiding phy powerdown in host mode so that it can be wake up by devices.
> Set usb controller wakeup capable when wakeup capable devices are
> connected to the host.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++-----
> drivers/usb/dwc3/core.h | 1 +
> drivers/usb/dwc3/dwc3-qcom.c | 66 +++++++++++++++++++++++++++++++++-----------
> 3 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..8370350 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,15 +31,19 @@
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> #include <linux/usb/otg.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
> #include "gadget.h"
> #include "io.h"
>
> #include "debug.h"
> +#include "../host/xhci.h"
>
> #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
>
> +bool need_phy_for_wakeup;
> +
> /**
> * dwc3_get_dr_mode - Validates and sets dr_mode
> * @dwc: pointer to our context structure
> @@ -1627,10 +1631,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> return ret;
> }
>
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> + int i, num_ports;
> + u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
dwc->xhci could be NULL for DR_MODE = PERIPHERAL.
> + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> + dwc->hs_phy_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
> +
> + reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +
> + num_ports = HCS_MAX_PORTS(reg);
> + for (i = 0; i < num_ports; i++) {
> + reg = readl(&xhci_hcd->op_regs->port_status_base + i*0x10);
> + if (reg & PORT_PE) {
> + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
> + else if (DEV_LOWSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;
> + }
> + }
> + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
Can we move this logic to xhci driver instead?
> +}
> +
..
> @@ -240,6 +267,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> {
> u32 val;
> int i;
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
dwc->xhci could be NULL
> +
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + device_init_wakeup(qcom->dev, 1);
>
> if (qcom->is_suspended)
> return 0;
> @@ -262,6 +294,8 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
> int ret;
> int i;
>
> + device_init_wakeup(qcom->dev, 0);
> +
> if (!qcom->is_suspended)
> return 0;
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/2] usb: dwc3: Host wake up support from system suspend
2020-06-11 14:28 ` [PATCH 2/2] usb: dwc3: Host wake up support from system suspend Sandeep Maheswaram
` (2 preceding siblings ...)
2020-08-11 8:50 ` Manu Gautam
@ 2020-08-11 9:12 ` Felipe Balbi
3 siblings, 0 replies; 11+ messages in thread
From: Felipe Balbi @ 2020-08-11 9:12 UTC (permalink / raw)
To: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, Matthias Kaehlcke, Michael Turquette
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
linux-clk, Taniya Das, Sandeep Maheswaram
[-- Attachment #1: Type: text/plain, Size: 3353 bytes --]
Sandeep Maheswaram <sanm@codeaurora.org> writes:
> Avoiding phy powerdown in host mode so that it can be wake up by devices.
> Set usb controller wakeup capable when wakeup capable devices are
> connected to the host.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++-----
> drivers/usb/dwc3/core.h | 1 +
> drivers/usb/dwc3/dwc3-qcom.c | 66 +++++++++++++++++++++++++++++++++-----------
> 3 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..8370350 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,15 +31,19 @@
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> #include <linux/usb/otg.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
> #include "gadget.h"
> #include "io.h"
>
> #include "debug.h"
> +#include "../host/xhci.h"
nope
> #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
>
> +bool need_phy_for_wakeup;
nope
> +
> /**
> * dwc3_get_dr_mode - Validates and sets dr_mode
> * @dwc: pointer to our context structure
> @@ -1627,10 +1631,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> return ret;
> }
>
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> + int i, num_ports;
> + u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
> + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> + dwc->hs_phy_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
> +
> + reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +
> + num_ports = HCS_MAX_PORTS(reg);
> + for (i = 0; i < num_ports; i++) {
> + reg = readl(&xhci_hcd->op_regs->port_status_base + i*0x10);
> + if (reg & PORT_PE) {
> + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
> + else if (DEV_LOWSPEED(reg))
> + dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;
> + }
> + }
> + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
XHCI already supports PHY framework, no?
> static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> {
> unsigned long flags;
> u32 reg;
> + struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
>
> switch (dwc->current_dr_role) {
> case DWC3_GCTL_PRTCAP_DEVICE:
> @@ -1643,9 +1673,10 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> dwc3_core_exit(dwc);
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> + dwc3_set_phy_speed_flags(dwc);
> if (!PMSG_IS_AUTO(msg)) {
> - dwc3_core_exit(dwc);
> - break;
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + need_phy_for_wakeup = true;
should be done in xhci-plat
> @@ -1705,11 +1736,13 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> if (!PMSG_IS_AUTO(msg)) {
> - ret = dwc3_core_init_for_resume(dwc);
> - if (ret)
> - return ret;
> - dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> - break;
> + if (!need_phy_for_wakeup) {
> + ret = dwc3_core_init_for_resume(dwc);
> + if (ret)
> + return ret;
> + dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> + break;
why?
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread