Re: [PATCH v13 5/6] usb: dwc3: qcom: Keep power domain on to retain controller status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 4/12/2022 2:43 AM, Matthias Kaehlcke wrote:
On Tue, Apr 12, 2022 at 12:46:53AM +0530, Sandeep Maheswaram wrote:
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@xxxxxxxxxxx>
---
  drivers/usb/dwc3/dwc3-qcom.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 9804a19..9747be6 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>
@@ -724,6 +725,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
  	struct resource		*res, *parent_res = NULL;
  	int			ret, i;
  	bool			ignore_pipe_clk;
+	struct generic_pm_domain *genpd;
nit: I'm not really a fan of gazillions of whitespaces between the type
and the variable name, but if they are kept all variable names above
should move a tab to the right. In any case the style in this file isn't
consistent, so an alternative would be to just get rid of the alignment
completely.
Okay. Will do it in next version.

  	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))
+		genpd->flags |= GENPD_FLAG_ALWAYS_ON;
In v12 only GENPD_FLAG_ALWAYS_ON was set, not GENPD_FLAG_RPM_ALWAYS_ON.
I asked a few times for a change log, in this instance it would be
useful to explain why GENPD_FLAG_RPM_ALWAYS_ON is now set.
Okay. Will add it in next version.

+
+	device_init_wakeup(&pdev->dev, device_may_wakeup(&qcom->dwc3->dev));
device_may_wakeup() isn't an expensive operation, but it's still not nice to
call it twice in three lines of code. Instead you could do this:

	if (device_may_wakeup(&qcom->dwc3->dev)) {
		genpd->flags |= GENPD_FLAG_ALWAYS_ON;
		device_init_wakeup(&pdev->dev, true);
	}
Okay. Will do it in next version.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux