* [PATCH v5 1/4] usb: dwc3: core: Host wake up support from system suspend
2021-03-22 17:31 [PATCH v5 0/4] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
@ 2021-03-22 17:31 ` Sandeep Maheswaram
2021-03-23 12:10 ` Greg Kroah-Hartman
2021-03-24 0:27 ` Matthias Kaehlcke
2021-03-22 17:31 ` [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host Sandeep Maheswaram
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Sandeep Maheswaram @ 2021-03-22 17:31 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Sandeep Maheswaram
Avoiding phy powerdown when wakeup capable devices are connected.
Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
drivers/usb/dwc3/core.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 94fdbe5..9ecd7ac 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1702,7 +1702,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
dwc3_core_exit(dwc);
break;
case DWC3_GCTL_PRTCAP_HOST:
- if (!PMSG_IS_AUTO(msg)) {
+ if (!PMSG_IS_AUTO(msg) && dwc->phy_power_off) {
dwc3_core_exit(dwc);
break;
}
@@ -1763,13 +1763,15 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
spin_unlock_irqrestore(&dwc->lock, flags);
break;
case DWC3_GCTL_PRTCAP_HOST:
- if (!PMSG_IS_AUTO(msg)) {
+ if (!PMSG_IS_AUTO(msg) && dwc->phy_power_off) {
ret = dwc3_core_init_for_resume(dwc);
if (ret)
return ret;
dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
break;
- }
+ } else
+ dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
+
/* Restore GUSB2PHYCFG bits that were modified in suspend */
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
if (dwc->dis_u2_susphy_quirk)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/4] usb: dwc3: core: Host wake up support from system suspend
2021-03-22 17:31 ` [PATCH v5 1/4] usb: dwc3: core: Host " Sandeep Maheswaram
@ 2021-03-23 12:10 ` Greg Kroah-Hartman
2021-03-24 0:27 ` Matthias Kaehlcke
1 sibling, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-23 12:10 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Felipe Balbi,
Stephen Boyd, Doug Anderson, Matthias Kaehlcke, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:17PM +0530, Sandeep Maheswaram wrote:
> Avoiding phy powerdown when wakeup capable devices are connected.
That says "what" (in a very abbreviated way), but not _WHY_ you want to
do this. Please fix this up.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/4] usb: dwc3: core: Host wake up support from system suspend
2021-03-22 17:31 ` [PATCH v5 1/4] usb: dwc3: core: Host " Sandeep Maheswaram
2021-03-23 12:10 ` Greg Kroah-Hartman
@ 2021-03-24 0:27 ` Matthias Kaehlcke
1 sibling, 0 replies; 13+ messages in thread
From: Matthias Kaehlcke @ 2021-03-24 0:27 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:17PM +0530, Sandeep Maheswaram wrote:
> Avoiding phy powerdown when wakeup capable devices are connected.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 94fdbe5..9ecd7ac 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1702,7 +1702,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> dwc3_core_exit(dwc);
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> - if (!PMSG_IS_AUTO(msg)) {
> + if (!PMSG_IS_AUTO(msg) && dwc->phy_power_off) {
This is the first patch of the series, but the 'phy_power_off' flag is
only added by '[2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host'.
Patches should not rely on later patches in the series in order to build
error/warning free. It seems you need to swap the order of patch 1 and 2.
> dwc3_core_exit(dwc);
> break;
> }
> @@ -1763,13 +1763,15 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> spin_unlock_irqrestore(&dwc->lock, flags);
> break;
> case DWC3_GCTL_PRTCAP_HOST:
> - if (!PMSG_IS_AUTO(msg)) {
> + if (!PMSG_IS_AUTO(msg) && dwc->phy_power_off) {
> ret = dwc3_core_init_for_resume(dwc);
> if (ret)
> return ret;
> dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> break;
> - }
> + } else
> + dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> +
nit: use curly braces since the 'if' block has them.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host
2021-03-22 17:31 [PATCH v5 0/4] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
2021-03-22 17:31 ` [PATCH v5 1/4] usb: dwc3: core: Host " Sandeep Maheswaram
@ 2021-03-22 17:31 ` Sandeep Maheswaram
2021-03-23 12:09 ` Greg Kroah-Hartman
` (2 more replies)
2021-03-22 17:31 ` [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag Sandeep Maheswaram
2021-03-22 17:31 ` [PATCH v5 4/4] arm64: dts: qcom: sc7180: Add wakeup-source property for USB node in IDP and trogdor Sandeep Maheswaram
3 siblings, 3 replies; 13+ messages in thread
From: Sandeep Maheswaram @ 2021-03-22 17:31 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Sandeep Maheswaram
Adding suspend quirk function for dwc3 host which will be called
during xhci suspend.
Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode
during host suspend.
Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
drivers/usb/dwc3/core.h | 3 +++
drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index ce6bd84..f05546c 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1113,6 +1113,9 @@ struct dwc3 {
bool phys_ready;
+ unsigned int hs_phy_mode;
+ bool phy_power_off;
+
struct ulpi *ulpi;
bool ulpi_ready;
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index f29a264..3db042c 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -11,6 +11,13 @@
#include <linux/platform_device.h>
#include "core.h"
+#include "../host/xhci.h"
+#include "../host/xhci-plat.h"
+int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
+
+static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
+ .suspend_quirk = xhci_dwc3_suspend_quirk,
+};
static int dwc3_host_get_irq(struct dwc3 *dwc)
{
@@ -115,6 +122,13 @@ int dwc3_host_init(struct dwc3 *dwc)
}
}
+ ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci,
+ sizeof(struct xhci_plat_priv));
+ if (ret) {
+ dev_err(dwc->dev, "failed to add data to xHCI\n");
+ goto err;
+ }
+
ret = platform_device_add(xhci);
if (ret) {
dev_err(dwc->dev, "failed to register xHCI device\n");
@@ -127,6 +141,50 @@ int dwc3_host_init(struct dwc3 *dwc)
return ret;
}
+static void dwc3_set_phy_mode(struct usb_hcd *hcd)
+{
+
+ int i, num_ports;
+ u32 reg;
+ unsigned int ss_phy_mode = 0;
+ struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
+ struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
+
+ dwc->hs_phy_mode = 0;
+
+ 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 * 0x04);
+ if (reg & PORT_PE) {
+ if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
+ dwc->hs_phy_mode |= PHY_MODE_USB_HOST_HS;
+ else if (DEV_LOWSPEED(reg))
+ dwc->hs_phy_mode |= PHY_MODE_USB_HOST_LS;
+
+ if (DEV_SUPERSPEED(reg))
+ ss_phy_mode |= PHY_MODE_USB_HOST_SS;
+ }
+ }
+ phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_mode);
+ phy_set_mode(dwc->usb3_generic_phy, ss_phy_mode);
+}
+
+int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd)
+{
+ struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
+
+ dwc3_set_phy_mode(hcd);
+
+ dwc->phy_power_off = true;
+
+ if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
+ dwc->phy_power_off = false;
+
+ return 0;
+}
+
void dwc3_host_exit(struct dwc3 *dwc)
{
platform_device_unregister(dwc->xhci);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host
2021-03-22 17:31 ` [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host Sandeep Maheswaram
@ 2021-03-23 12:09 ` Greg Kroah-Hartman
2021-03-23 12:09 ` Greg Kroah-Hartman
2021-03-24 0:38 ` Matthias Kaehlcke
2 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-23 12:09 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Felipe Balbi,
Stephen Boyd, Doug Anderson, Matthias Kaehlcke, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote:
> Adding suspend quirk function for dwc3 host which will be called
> during xhci suspend.
What does xhci have to do with this?
And where is the user of this quirk function in this series?
> Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode
> during host suspend.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ce6bd84..f05546c 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1113,6 +1113,9 @@ struct dwc3 {
>
> bool phys_ready;
>
> + unsigned int hs_phy_mode;
> + bool phy_power_off;
> +
> struct ulpi *ulpi;
> bool ulpi_ready;
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index f29a264..3db042c 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -11,6 +11,13 @@
> #include <linux/platform_device.h>
>
> #include "core.h"
> +#include "../host/xhci.h"
> +#include "../host/xhci-plat.h"
Hah, really?
> +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
Didn't checkpatch complain about this?
Put function prototypes in .h files please, this isn't the 1980's...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host
2021-03-22 17:31 ` [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host Sandeep Maheswaram
2021-03-23 12:09 ` Greg Kroah-Hartman
@ 2021-03-23 12:09 ` Greg Kroah-Hartman
2021-03-24 0:38 ` Matthias Kaehlcke
2 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-23 12:09 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Felipe Balbi,
Stephen Boyd, Doug Anderson, Matthias Kaehlcke, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote:
> Adding suspend quirk function for dwc3 host which will be called
> during xhci suspend.
> Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode
> during host suspend.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ce6bd84..f05546c 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1113,6 +1113,9 @@ struct dwc3 {
>
> bool phys_ready;
>
> + unsigned int hs_phy_mode;
> + bool phy_power_off;
> +
> struct ulpi *ulpi;
> bool ulpi_ready;
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index f29a264..3db042c 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -11,6 +11,13 @@
> #include <linux/platform_device.h>
>
> #include "core.h"
> +#include "../host/xhci.h"
> +#include "../host/xhci-plat.h"
> +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
Wait, this is right below down in this file, shouldn't it be static?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host
2021-03-22 17:31 ` [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host Sandeep Maheswaram
2021-03-23 12:09 ` Greg Kroah-Hartman
2021-03-23 12:09 ` Greg Kroah-Hartman
@ 2021-03-24 0:38 ` Matthias Kaehlcke
2 siblings, 0 replies; 13+ messages in thread
From: Matthias Kaehlcke @ 2021-03-24 0:38 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:18PM +0530, Sandeep Maheswaram wrote:
> Adding suspend quirk function for dwc3 host which will be called
> during xhci suspend.
> Setting hs_phy_mode, ss_phy_mode , phy_power_off flags and phy mode
> during host suspend.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ce6bd84..f05546c 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1113,6 +1113,9 @@ struct dwc3 {
>
> bool phys_ready;
>
> + unsigned int hs_phy_mode;
> + bool phy_power_off;
> +
> struct ulpi *ulpi;
> bool ulpi_ready;
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index f29a264..3db042c 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -11,6 +11,13 @@
> #include <linux/platform_device.h>
>
> #include "core.h"
> +#include "../host/xhci.h"
> +#include "../host/xhci-plat.h"
> +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
> +
> +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
> + .suspend_quirk = xhci_dwc3_suspend_quirk,
> +};
>
> static int dwc3_host_get_irq(struct dwc3 *dwc)
> {
> @@ -115,6 +122,13 @@ int dwc3_host_init(struct dwc3 *dwc)
> }
> }
>
> + ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci,
> + sizeof(struct xhci_plat_priv));
> + if (ret) {
> + dev_err(dwc->dev, "failed to add data to xHCI\n");
> + goto err;
> + }
> +
> ret = platform_device_add(xhci);
> if (ret) {
> dev_err(dwc->dev, "failed to register xHCI device\n");
> @@ -127,6 +141,50 @@ int dwc3_host_init(struct dwc3 *dwc)
> return ret;
> }
>
> +static void dwc3_set_phy_mode(struct usb_hcd *hcd)
> +{
> +
> + int i, num_ports;
> + u32 reg;
> + unsigned int ss_phy_mode = 0;
> + struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
> + struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
> +
> + dwc->hs_phy_mode = 0;
> +
> + reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +
> + num_ports = HCS_MAX_PORTS(reg);
nit: remove empty line between the two lines above. 'reg' is read to
determine the number of ports and discarded afterwards, so it makes
sense to establish that relationship visually. You could (optionally)
add an empty line after assigning 'num_ports'.
> + for (i = 0; i < num_ports; i++) {
> + reg = readl(&xhci_hcd->op_regs->port_status_base + i * 0x04);
> + if (reg & PORT_PE) {
> + if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> + dwc->hs_phy_mode |= PHY_MODE_USB_HOST_HS;
> + else if (DEV_LOWSPEED(reg))
> + dwc->hs_phy_mode |= PHY_MODE_USB_HOST_LS;
> +
> + if (DEV_SUPERSPEED(reg))
> + ss_phy_mode |= PHY_MODE_USB_HOST_SS;
> + }
> + }
> + phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_mode);
> + phy_set_mode(dwc->usb3_generic_phy, ss_phy_mode);
> +}
> +
> +int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
> +
> + dwc3_set_phy_mode(hcd);
> +
> + dwc->phy_power_off = true;
it seems silly to assign this field only to overwrite it two lines later
if wakeup capable descendants are connected. Making the assignment in an
'else' branch would be clearer IMO.
> +
> + if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> + dwc->phy_power_off = false;
> +
> + return 0;
> +}
> +
> void dwc3_host_exit(struct dwc3 *dwc)
> {
> platform_device_unregister(dwc->xhci);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag
2021-03-22 17:31 [PATCH v5 0/4] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
2021-03-22 17:31 ` [PATCH v5 1/4] usb: dwc3: core: Host " Sandeep Maheswaram
2021-03-22 17:31 ` [PATCH v5 2/4] usb: dwc3: host: Add suspend_quirk for dwc3 host Sandeep Maheswaram
@ 2021-03-22 17:31 ` Sandeep Maheswaram
2021-03-23 12:11 ` Greg Kroah-Hartman
2021-03-22 17:31 ` [PATCH v5 4/4] arm64: dts: qcom: sc7180: Add wakeup-source property for USB node in IDP and trogdor Sandeep Maheswaram
3 siblings, 1 reply; 13+ messages in thread
From: Sandeep Maheswaram @ 2021-03-22 17:31 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Sandeep Maheswaram
Configure interrupts based on hs_phy_mode to avoid triggering of
interrupts during system suspend and suspends successfully.
Set genpd active wakeup flag for usb gdsc if wakeup capable devices
are connected so that wake up happens without reenumeration.
Add helper functions to enable,disable wake irqs.
Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
drivers/usb/dwc3/dwc3-qcom.c | 87 ++++++++++++++++++++++++++++----------------
1 file changed, 56 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 5149dea..e53228e 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -17,9 +17,11 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
+#include <linux/pm_domain.h>
#include <linux/usb/of.h>
#include <linux/reset.h>
#include <linux/iopoll.h>
+#include <linux/usb/hcd.h>
#include "core.h"
@@ -293,60 +295,77 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
icc_put(qcom->icc_path_apps);
}
+static void dwc3_qcom_enable_wakeup_irq(int irq)
+{
+ if (!irq)
+ return;
+
+ enable_irq(irq);
+ enable_irq_wake(irq);
+}
+
+static void dwc3_qcom_disable_wakeup_irq(int irq)
+{
+ if (!irq)
+ return;
+
+ disable_irq_wake(irq);
+ disable_irq_nosync(irq);
+}
+
static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
{
- if (qcom->hs_phy_irq) {
- disable_irq_wake(qcom->hs_phy_irq);
- disable_irq_nosync(qcom->hs_phy_irq);
- }
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
- if (qcom->dp_hs_phy_irq) {
- disable_irq_wake(qcom->dp_hs_phy_irq);
- disable_irq_nosync(qcom->dp_hs_phy_irq);
- }
+ dwc3_qcom_disable_wakeup_irq(qcom->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 (dwc->hs_phy_mode & PHY_MODE_USB_HOST_LS)
+ dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq);
+ else if (dwc->hs_phy_mode & PHY_MODE_USB_HOST_HS)
+ dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
+ else {
+ dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
}
- if (qcom->ss_phy_irq) {
- disable_irq_wake(qcom->ss_phy_irq);
- disable_irq_nosync(qcom->ss_phy_irq);
- }
+ dwc3_qcom_disable_wakeup_irq(qcom->ss_phy_irq);
}
static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
{
- if (qcom->hs_phy_irq) {
- enable_irq(qcom->hs_phy_irq);
- enable_irq_wake(qcom->hs_phy_irq);
- }
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
- if (qcom->dp_hs_phy_irq) {
- enable_irq(qcom->dp_hs_phy_irq);
- enable_irq_wake(qcom->dp_hs_phy_irq);
- }
+ dwc3_qcom_enable_wakeup_irq(qcom->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 (dwc->hs_phy_mode & PHY_MODE_USB_HOST_LS)
+ dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq);
+ else if (dwc->hs_phy_mode & PHY_MODE_USB_HOST_HS)
+ dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq);
+ else {
+ dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq);
+ dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq);
}
- if (qcom->ss_phy_irq) {
- enable_irq(qcom->ss_phy_irq);
- enable_irq_wake(qcom->ss_phy_irq);
- }
+ dwc3_qcom_enable_wakeup_irq(qcom->ss_phy_irq);
}
static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
{
u32 val;
int i, ret;
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ struct usb_hcd *hcd;
+ struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
if (qcom->is_suspended)
return 0;
+ if (dwc->xhci) {
+ hcd = platform_get_drvdata(dwc->xhci);
+ if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
+ genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+ }
+
val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
dev_err(qcom->dev, "HS-PHY not in L2\n");
@@ -371,9 +390,15 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
int ret;
int i;
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
if (!qcom->is_suspended)
return 0;
+ if (dwc->xhci)
+ genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;
+
if (device_may_wakeup(qcom->dev))
dwc3_qcom_disable_interrupts(qcom);
@@ -816,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
if (ret)
goto interconnect_exit;
- device_init_wakeup(&pdev->dev, 1);
+ device_init_wakeup(&pdev->dev, of_property_read_bool(np, "wakeup-source"));
qcom->is_suspended = false;
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag
2021-03-22 17:31 ` [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag Sandeep Maheswaram
@ 2021-03-23 12:11 ` Greg Kroah-Hartman
2021-03-24 0:49 ` Matthias Kaehlcke
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-23 12:11 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Felipe Balbi,
Stephen Boyd, Doug Anderson, Matthias Kaehlcke, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Mon, Mar 22, 2021 at 11:01:19PM +0530, Sandeep Maheswaram wrote:
> Configure interrupts based on hs_phy_mode to avoid triggering of
> interrupts during system suspend and suspends successfully.
> Set genpd active wakeup flag for usb gdsc if wakeup capable devices
> are connected so that wake up happens without reenumeration.
> Add helper functions to enable,disable wake irqs.
That feels like a lot of different things all in one patch.
What are you trying to fix here? I can't figure it out at all...
Please work on your changelog text for these patches when you resend
them.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag
2021-03-23 12:11 ` Greg Kroah-Hartman
@ 2021-03-24 0:49 ` Matthias Kaehlcke
2021-03-24 0:56 ` Matthias Kaehlcke
0 siblings, 1 reply; 13+ messages in thread
From: Matthias Kaehlcke @ 2021-03-24 0:49 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sandeep Maheswaram, Andy Gross, Bjorn Andersson, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Tue, Mar 23, 2021 at 01:11:18PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 22, 2021 at 11:01:19PM +0530, Sandeep Maheswaram wrote:
> > Configure interrupts based on hs_phy_mode to avoid triggering of
> > interrupts during system suspend and suspends successfully.
> > Set genpd active wakeup flag for usb gdsc if wakeup capable devices
> > are connected so that wake up happens without reenumeration.
> > Add helper functions to enable,disable wake irqs.
>
> That feels like a lot of different things all in one patch.
Sandeep: one thing you could do to reduce the churn is to add
dwc3_qcom_enable/disable_wakeup_irq() in a separate patch, without
any functional changes. Then this patch would only add the different
branches based on the PHY mode.
The handling of the power domain could probably also be done in a
separate patch, if I recall correctly it is only an optimization.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag
2021-03-24 0:49 ` Matthias Kaehlcke
@ 2021-03-24 0:56 ` Matthias Kaehlcke
0 siblings, 0 replies; 13+ messages in thread
From: Matthias Kaehlcke @ 2021-03-24 0:56 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sandeep Maheswaram, Andy Gross, Bjorn Andersson, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam
On Tue, Mar 23, 2021 at 05:49:14PM -0700, Matthias Kaehlcke wrote:
> On Tue, Mar 23, 2021 at 01:11:18PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Mar 22, 2021 at 11:01:19PM +0530, Sandeep Maheswaram wrote:
> > > Configure interrupts based on hs_phy_mode to avoid triggering of
> > > interrupts during system suspend and suspends successfully.
> > > Set genpd active wakeup flag for usb gdsc if wakeup capable devices
> > > are connected so that wake up happens without reenumeration.
> > > Add helper functions to enable,disable wake irqs.
> >
> > That feels like a lot of different things all in one patch.
>
> Sandeep: one thing you could do to reduce the churn is to add
> dwc3_qcom_enable/disable_wakeup_irq() in a separate patch, without
> any functional changes. Then this patch would only add the different
> branches based on the PHY mode.
>
> The handling of the power domain could probably also be done in a
> separate patch, if I recall correctly it is only an optimization.
Actually another thing that could be in a separate patch is enabling
wakeup support based on 'wakeup-source'. That's not even directly
related with this series.
With all that you'd have fairly atomic patches and it should be easy to
write meaningful commit messages.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 4/4] arm64: dts: qcom: sc7180: Add wakeup-source property for USB node in IDP and trogdor
2021-03-22 17:31 [PATCH v5 0/4] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
` (2 preceding siblings ...)
2021-03-22 17:31 ` [PATCH v5 3/4] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag Sandeep Maheswaram
@ 2021-03-22 17:31 ` Sandeep Maheswaram
3 siblings, 0 replies; 13+ messages in thread
From: Sandeep Maheswaram @ 2021-03-22 17:31 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Felipe Balbi, Stephen Boyd, Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Sandeep Maheswaram
Adding wakeup-source property for USB controller in
SC7180 IDP and trogdor boards.
Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index e77a792..fe1366a 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -369,6 +369,7 @@
&usb_1 {
status = "okay";
+ wakeup-source;
};
&usb_1_dwc3 {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 192e2e4..78e44c8 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -844,6 +844,7 @@ ap_spi_fp: &spi10 {
&usb_1 {
status = "okay";
+ wakeup-source;
};
&usb_1_dwc3 {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread