linux-kernel - Re: [v16 5/5] usb: dwc3: qcom: Keep power domain on to retain controller status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yn2OTOF07A5jhYSF@google.com>
Date:   Thu, 12 May 2022 15:46:36 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Krishna Kurapati <quic_kriskura@...cinc.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Felipe Balbi <balbi@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Doug Anderson <dianders@...omium.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org, quic_pkondeti@...cinc.com,
        quic_ppratap@...cinc.com, quic_vpulyala@...cinc.com,
        Sandeep Maheswaram <quic_c_sanm@...cinc.com>
Subject: Re: [v16 5/5] usb: dwc3: qcom: Keep power domain on to retain
 controller status

On Thu, May 12, 2022 at 11:53:22PM +0530, Krishna Kurapati wrote:
> From: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
> 
> Keep the power domain always on during runtime suspend or if the
> controller supports wakeup in order to retain controller status
> and to support wakeup from devices.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 9804a19..1f9589a 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>
> @@ -718,12 +719,13 @@ dwc3_qcom_create_urs_usb_platdev(struct device *dev)
>  
>  static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
> -	struct device_node	*np = pdev->dev.of_node;
> -	struct device		*dev = &pdev->dev;
> -	struct dwc3_qcom	*qcom;
> -	struct resource		*res, *parent_res = NULL;
> -	int			ret, i;
> -	bool			ignore_pipe_clk;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct dwc3_qcom *qcom;
> +	struct resource	*res, *parent_res = NULL;
> +	int ret, i;
> +	bool ignore_pipe_clk;
> +	struct generic_pm_domain *genpd;
>  
>  	qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
>  	if (!qcom)
> @@ -732,6 +734,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, qcom);
>  	qcom->dev = &pdev->dev;
>  
> +	genpd = pd_to_genpd(qcom->dev->pm_domain);
> +
>  	if (has_acpi_companion(dev)) {
>  		qcom->acpi_pdata = acpi_device_get_match_data(dev);
>  		if (!qcom->acpi_pdata) {
> @@ -839,7 +843,12 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto interconnect_exit;
>  
> -	device_init_wakeup(&pdev->dev, 1);
> +	genpd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
> +
> +	if (device_may_wakeup(&qcom->dwc3->dev)) {

This should probably be device_can_wakeup(), otherwise you'll have to set/clear
the flag before suspending (the wakeup policy might change after the device is
probed). Also I'm not sure if switching the domain off can impact the power
consumption of some connected peripherals, as is observed when the PHYs are
powered off (I could confirm this if needed).

> +		genpd->flags |= GENPD_FLAG_ALWAYS_ON;
> +		device_init_wakeup(&pdev->dev, true);
> +	}
>  	qcom->is_suspended = false;
>  	pm_runtime_set_active(dev);
>  	pm_runtime_enable(dev);
> -- 
> 2.7.4
>