Re: [PATCH 2/2] usb: dwc3: pci: Runtime resume child device from wq

[Manu Gautam]

Hi,


On 7/3/2017 4:55 PM, Manu Gautam wrote:
> Driver currently resumes and increments pm usage_count
> of its child device (dwc3 main) from its runtime_resume
> handler. This requires dwc3 runtime_resume to perform
> pm_runtime_put to decrement the pm usage_count. However
> runtime_put from dwc3 happens for non pci drivers
> (e.g. dwc3-if-simple.c) as well which results in dwc3
> pm usage_count becoming negative after couple of
> runtime suspend resume iterations. Fix this by
> performing runtime_get/put from dwc3-pci driver only
> using workqueue.

Felipe, I don't have any platform that uses dwc3-pci, hence couldn't
test. However, with this change I don't see issue of pm usage_count
becoming -ve on my platform that uses dwc3-of-simple.c.
Let me know if this looks fine.

>
> Signed-off-by: Manu Gautam 
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fc556a4..87dedb9 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1378,7 +1378,6 @@ static int dwc3_runtime_resume(struct device *dev)
>   }
>  
>   pm_runtime_mark_last_busy(dev);
> - pm_runtime_put(dev);
>  
>   return 0;
>  }
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 84a2ceb..e354acc 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -61,6 +62,7 @@ struct dwc3_pci {
>   u8 uuid[16];
>  
>   unsigned int has_dsm_for_pm:1;
> + struct work_struct wakeup_work;
>  };
>  
>  static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> @@ -174,6 +176,22 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
>   return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static void dwc3_pci_resume_work(struct work_struct *work)
> +{
> + struct dwc3_pci *dwc = container_of(work, struct dwc3_pci, wakeup_work);
> + struct platform_device *dwc3 = dwc->dwc3;
> + int ret;
> +
> + ret = pm_runtime_get_sync(>dev);
> + if (ret)
> + return;
> +
> + pm_runtime_mark_last_busy(>dev);
> + pm_runtime_put_sync_autosuspend(>dev);
> +}
> +#endif
> +
>  static int dwc3_pci_probe(struct pci_dev *pci,
>   const struct pci_device_id *id)
>  {
> @@ -233,6 +251,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
>   device_set_run_wake(dev, true);
>   pci_set_drvdata(pci, dwc);
>   pm_runtime_put(dev);
> +#ifdef CONFIG_PM
> + INIT_WORK(>wakeup_work, dwc3_pci_resume_work);
> +#endif
>  
>   return 0;
>  err:
> @@ -244,6 +265,9 @@ static void dwc3_pci_remove(struct pci_dev *pci)
>  {
>   struct dwc3_pci *dwc = pci_get_drvdata(pci);
>  
> +#ifdef CONFIG_PM
> + cancel_work_sync(>wakeup_work);
> +#endif
>   device_init_wakeup(>dev, false);
>   pm_runtime_get(>dev);
>   platform_device_unregister(dwc->dwc3);
> @@ -319,14 +343,15 @@ static int dwc3_pci_runtime_suspend(struct device *dev)
>  static int dwc3_pci_runtime_resume(struct device *dev)
>  {
>   struct dwc3_pci *dwc = dev_get_drvdata(dev);
> - struct platform_device  *dwc3 = dwc->dwc3;
>   int ret;
>  
>   ret = dwc3_pci_dsm(dwc, PCI_INTEL_BXT_STATE_D0);
>   if (ret)
>   return ret;
>  
> - return pm_runtime_get(>dev);
> + queue_work(pm_wq, >wakeup_work);
> +
> + return 0;
>  }
>  #endif /* CONFIG_PM */
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] usb: dwc3: pci: Runtime resume child device from wq

[Manu Gautam]

Driver currently resumes and increments pm usage_count
of its child device (dwc3 main) from its runtime_resume
handler. This requires dwc3 runtime_resume to perform
pm_runtime_put to decrement the pm usage_count. However
runtime_put from dwc3 happens for non pci drivers
(e.g. dwc3-if-simple.c) as well which results in dwc3
pm usage_count becoming negative after couple of
runtime suspend resume iterations. Fix this by
performing runtime_get/put from dwc3-pci driver only
using workqueue.

Signed-off-by: Manu Gautam 

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fc556a4..87dedb9 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1378,7 +1378,6 @@ static int dwc3_runtime_resume(struct device *dev)
}
 
pm_runtime_mark_last_busy(dev);
-   pm_runtime_put(dev);
 
return 0;
 }
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 84a2ceb..e354acc 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -61,6 +62,7 @@ struct dwc3_pci {
u8 uuid[16];
 
unsigned int has_dsm_for_pm:1;
+   struct work_struct wakeup_work;
 };
 
 static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
@@ -174,6 +176,22 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
return 0;
 }
 
+#ifdef CONFIG_PM
+static void dwc3_pci_resume_work(struct work_struct *work)
+{
+   struct dwc3_pci *dwc = container_of(work, struct dwc3_pci, wakeup_work);
+   struct platform_device *dwc3 = dwc->dwc3;
+   int ret;
+
+   ret = pm_runtime_get_sync(>dev);
+   if (ret)
+   return;
+
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_put_sync_autosuspend(>dev);
+}
+#endif
+
 static int dwc3_pci_probe(struct pci_dev *pci,
const struct pci_device_id *id)
 {
@@ -233,6 +251,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
device_set_run_wake(dev, true);
pci_set_drvdata(pci, dwc);
pm_runtime_put(dev);
+#ifdef CONFIG_PM
+   INIT_WORK(>wakeup_work, dwc3_pci_resume_work);
+#endif
 
return 0;
 err:
@@ -244,6 +265,9 @@ static void dwc3_pci_remove(struct pci_dev *pci)
 {
struct dwc3_pci *dwc = pci_get_drvdata(pci);
 
+#ifdef CONFIG_PM
+   cancel_work_sync(>wakeup_work);
+#endif
device_init_wakeup(>dev, false);
pm_runtime_get(>dev);
platform_device_unregister(dwc->dwc3);
@@ -319,14 +343,15 @@ static int dwc3_pci_runtime_suspend(struct device *dev)
 static int dwc3_pci_runtime_resume(struct device *dev)
 {
struct dwc3_pci *dwc = dev_get_drvdata(dev);
-   struct platform_device  *dwc3 = dwc->dwc3;
int ret;
 
ret = dwc3_pci_dsm(dwc, PCI_INTEL_BXT_STATE_D0);
if (ret)
return ret;
 
-   return pm_runtime_get(>dev);
+   queue_work(pm_wq, >wakeup_work);
+
+   return 0;
 }
 #endif /* CONFIG_PM */
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na 
Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html