linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Sandeep Maheswaram <sanm@codeaurora.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@kernel.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Pratham Pratap <prathampratap@codeaurora.org>
Subject: Re: [PATCH v8 6/6] usb: dwc3: qcom: Keep power domain on to support wakeup
Date: Mon, 28 Jun 2021 14:23:56 -0700	[thread overview]
Message-ID: <YNo97HQXmYjUNz/C@google.com> (raw)
In-Reply-To: <1624882097-23265-7-git-send-email-sanm@codeaurora.org>

On Mon, Jun 28, 2021 at 05:38:17PM +0530, Sandeep Maheswaram wrote:
> If wakeup capable devices are connected to the controller (directly
> or through hubs) at suspend time keep the power domain on in order
> to support wakeup from these devices.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> Checking phy_power_off flag instead of usb_wakeup_enabled_descendants 
> to keep gdsc active.
> 
>  drivers/usb/dwc3/dwc3-qcom.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 82125bc..ba31aa3 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -17,6 +17,7 @@
>  #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>
> @@ -355,9 +356,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>  	u32 val;
>  	int i, ret;
>  
> +	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->phy_power_off && dwc->xhci)
> +		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");
> @@ -382,9 +389,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);
>  

This is essentially the same as v7, which Felipe NAKed
(https://patchwork.kernel.org/project/linux-arm-msm/patch/1619586716-8687-6-git-send-email-sanm@codeaurora.org/)

I think Felipe wants to see the handling of the power domain in the
xhci-plat driver. One problem here is that the power domain is owned
by the glue driver. For dwc3 the glue device is the parent of the xHCI
device, this is also the case for some other drivers like histb or
cdns3, but I'm not sure if it is universally true. If it isn't
xhci-plat could only make use of dev->parent->pm_domain for certain
compatible strings.

One could argue that it isn't very clean either if xhci-plat manipulates
a resource of it's parent. At the same time the glue driver isn't
supposed to check for the wakeup capable devices, so I guess some kind
of trade-off needs to be made.

  reply	other threads:[~2021-06-28 21:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-28 12:08 [PATCH v8 0/6] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 1/6] usb: dwc3: core: Add HS phy mode variable and phy poweroff flag Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 2/6] usb: host: xhci: plat: Add suspend quirk for dwc3 controller Sandeep Maheswaram
2021-07-12  9:31   ` Felipe Balbi
2021-09-28 23:08   ` Brian Norris
2021-06-28 12:08 ` [PATCH v8 3/6] usb: dwc3: core: Host wake up support from system suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 4/6] usb: dwc3: qcom: Add helper functions to enable,disable wake irqs Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 5/6] usb: dwc3: qcom: Configure wakeup interrupts during suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 6/6] usb: dwc3: qcom: Keep power domain on to support wakeup Sandeep Maheswaram
2021-06-28 21:23   ` Matthias Kaehlcke [this message]
2021-07-12  9:42     ` Felipe Balbi
2021-08-18  9:14       ` Sandeep Maheswaram
2021-08-18  9:56         ` Felipe Balbi
2021-09-15 14:05       ` Pavan Kondeti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YNo97HQXmYjUNz/C@google.com \
    --to=mka@chromium.org \
    --cc=agross@kernel.org \
    --cc=balbi@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=prathampratap@codeaurora.org \
    --cc=sanm@codeaurora.org \
    --cc=swboyd@chromium.org \
    --subject='Re: [PATCH v8 6/6] usb: dwc3: qcom: Keep power domain on to support wakeup' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).