linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] PM: Add device name to suspend_report_result()
       [not found] <CGME20220308104013epcas1p4c096ed0065700f2f1f8b5ea1ae4ed994@epcas1p4.samsung.com>
@ 2022-03-07 19:07 ` Youngjin Jang
  2022-03-08 19:00   ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Youngjin Jang @ 2022-03-07 19:07 UTC (permalink / raw)
  To: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
	Bjorn Helgaas, linux-pm, linux-kernel, linux-pci, linux-acpi,
	linux-usb, todd.e.brandt
  Cc: Youngjin Jang, js07.lee

currently, suspend_report_result() prints only function information.
If any driver uses common pm function, nobody knows who called
failed function exactly.

So, device is needed to recognize specific wrong driver.

e.g.)
PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0
after patch,
serial 00:05: PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
pci 0000:00:01.3: PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0

Signed-off-by: Youngjin Jang <yj84.jang@samsung.com>
---
Changes since v2:
 - Update commit message, example logs
Changes since v1:
 - Use dev_err() to print out device name
 - Use real name on email sender
 
 drivers/base/power/main.c  | 10 +++++-----
 drivers/pci/pci-driver.c   | 14 +++++++-------
 drivers/pnp/driver.c       |  2 +-
 drivers/usb/core/hcd-pci.c |  4 ++--
 include/linux/pm.h         |  8 ++++----
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 04ea92cbd9cf..41e17b8c2c20 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -485,7 +485,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev,
 	trace_device_pm_callback_start(dev, info, state.event);
 	error = cb(dev);
 	trace_device_pm_callback_end(dev, error);
-	suspend_report_result(cb, error);
+	suspend_report_result(dev, cb, error);
 
 	initcall_debug_report(dev, calltime, cb, error);
 
@@ -1568,7 +1568,7 @@ static int legacy_suspend(struct device *dev, pm_message_t state,
 	trace_device_pm_callback_start(dev, info, state.event);
 	error = cb(dev, state);
 	trace_device_pm_callback_end(dev, error);
-	suspend_report_result(cb, error);
+	suspend_report_result(dev, cb, error);
 
 	initcall_debug_report(dev, calltime, cb, error);
 
@@ -1855,7 +1855,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
 	device_unlock(dev);
 
 	if (ret < 0) {
-		suspend_report_result(callback, ret);
+		suspend_report_result(dev, callback, ret);
 		pm_runtime_put(dev);
 		return ret;
 	}
@@ -1960,10 +1960,10 @@ int dpm_suspend_start(pm_message_t state)
 }
 EXPORT_SYMBOL_GPL(dpm_suspend_start);
 
-void __suspend_report_result(const char *function, void *fn, int ret)
+void __suspend_report_result(const char *function, struct device *dev, void *fn, int ret)
 {
 	if (ret)
-		pr_err("%s(): %pS returns %d\n", function, fn, ret);
+		dev_err(dev, "%s(): %pS returns %d\n", function, fn, ret);
 }
 EXPORT_SYMBOL_GPL(__suspend_report_result);
 
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f61c40a47891..4ceeb75fc899 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -607,7 +607,7 @@ static int pci_legacy_suspend(struct device *dev, pm_message_t state)
 		int error;
 
 		error = drv->suspend(pci_dev, state);
-		suspend_report_result(drv->suspend, error);
+		suspend_report_result(dev, drv->suspend, error);
 		if (error)
 			return error;
 
@@ -786,7 +786,7 @@ static int pci_pm_suspend(struct device *dev)
 		int error;
 
 		error = pm->suspend(dev);
-		suspend_report_result(pm->suspend, error);
+		suspend_report_result(dev, pm->suspend, error);
 		if (error)
 			return error;
 
@@ -832,7 +832,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 		int error;
 
 		error = pm->suspend_noirq(dev);
-		suspend_report_result(pm->suspend_noirq, error);
+		suspend_report_result(dev, pm->suspend_noirq, error);
 		if (error)
 			return error;
 
@@ -1021,7 +1021,7 @@ static int pci_pm_freeze(struct device *dev)
 		int error;
 
 		error = pm->freeze(dev);
-		suspend_report_result(pm->freeze, error);
+		suspend_report_result(dev, pm->freeze, error);
 		if (error)
 			return error;
 	}
@@ -1041,7 +1041,7 @@ static int pci_pm_freeze_noirq(struct device *dev)
 		int error;
 
 		error = pm->freeze_noirq(dev);
-		suspend_report_result(pm->freeze_noirq, error);
+		suspend_report_result(dev, pm->freeze_noirq, error);
 		if (error)
 			return error;
 	}
@@ -1127,7 +1127,7 @@ static int pci_pm_poweroff(struct device *dev)
 		int error;
 
 		error = pm->poweroff(dev);
-		suspend_report_result(pm->poweroff, error);
+		suspend_report_result(dev, pm->poweroff, error);
 		if (error)
 			return error;
 	}
@@ -1165,7 +1165,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 		int error;
 
 		error = pm->poweroff_noirq(dev);
-		suspend_report_result(pm->poweroff_noirq, error);
+		suspend_report_result(dev, pm->poweroff_noirq, error);
 		if (error)
 			return error;
 	}
diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index cc6757dfa3f1..c02e7bf643a6 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -171,7 +171,7 @@ static int __pnp_bus_suspend(struct device *dev, pm_message_t state)
 
 	if (pnp_drv->driver.pm && pnp_drv->driver.pm->suspend) {
 		error = pnp_drv->driver.pm->suspend(dev);
-		suspend_report_result(pnp_drv->driver.pm->suspend, error);
+		suspend_report_result(dev, pnp_drv->driver.pm->suspend, error);
 		if (error)
 			return error;
 	}
diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 784466117c92..8176bc81a635 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -446,7 +446,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
 				HCD_WAKEUP_PENDING(hcd->shared_hcd))
 			return -EBUSY;
 		retval = hcd->driver->pci_suspend(hcd, do_wakeup);
-		suspend_report_result(hcd->driver->pci_suspend, retval);
+		suspend_report_result(dev, hcd->driver->pci_suspend, retval);
 
 		/* Check again in case wakeup raced with pci_suspend */
 		if ((retval == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) ||
@@ -556,7 +556,7 @@ static int hcd_pci_suspend_noirq(struct device *dev)
 		dev_dbg(dev, "--> PCI %s\n",
 				pci_power_name(pci_dev->current_state));
 	} else {
-		suspend_report_result(pci_prepare_to_sleep, retval);
+		suspend_report_result(dev, pci_prepare_to_sleep, retval);
 		return retval;
 	}
 
diff --git a/include/linux/pm.h b/include/linux/pm.h
index f7d2be686359..e65b3ab28377 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -770,11 +770,11 @@ extern int dpm_suspend_late(pm_message_t state);
 extern int dpm_suspend(pm_message_t state);
 extern int dpm_prepare(pm_message_t state);
 
-extern void __suspend_report_result(const char *function, void *fn, int ret);
+extern void __suspend_report_result(const char *function, struct device *dev, void *fn, int ret);
 
-#define suspend_report_result(fn, ret)					\
+#define suspend_report_result(dev, fn, ret)				\
 	do {								\
-		__suspend_report_result(__func__, fn, ret);		\
+		__suspend_report_result(__func__, dev, fn, ret);	\
 	} while (0)
 
 extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
@@ -814,7 +814,7 @@ static inline int dpm_suspend_start(pm_message_t state)
 	return 0;
 }
 
-#define suspend_report_result(fn, ret)		do {} while (0)
+#define suspend_report_result(dev, fn, ret)	do {} while (0)
 
 static inline int device_pm_wait_for_dev(struct device *a, struct device *b)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH v3] PM: Add device name to suspend_report_result()
  2022-03-07 19:07 ` [PATCH v3] PM: Add device name to suspend_report_result() Youngjin Jang
@ 2022-03-08 19:00   ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2022-03-08 19:00 UTC (permalink / raw)
  To: Youngjin Jang
  Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
	Bjorn Helgaas, Linux PM, Linux Kernel Mailing List, Linux PCI,
	ACPI Devel Maling List, open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
	Todd Brandt, js07.lee

On Tue, Mar 8, 2022 at 11:40 AM Youngjin Jang <yj84.jang@samsung.com> wrote:
>
> currently, suspend_report_result() prints only function information.
> If any driver uses common pm function, nobody knows who called
> failed function exactly.
>
> So, device is needed to recognize specific wrong driver.
>
> e.g.)
> PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
> PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0
> after patch,
> serial 00:05: PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
> pci 0000:00:01.3: PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0
>
> Signed-off-by: Youngjin Jang <yj84.jang@samsung.com>
> ---
> Changes since v2:
>  - Update commit message, example logs
> Changes since v1:
>  - Use dev_err() to print out device name
>  - Use real name on email sender
>
>  drivers/base/power/main.c  | 10 +++++-----
>  drivers/pci/pci-driver.c   | 14 +++++++-------
>  drivers/pnp/driver.c       |  2 +-
>  drivers/usb/core/hcd-pci.c |  4 ++--
>  include/linux/pm.h         |  8 ++++----
>  5 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 04ea92cbd9cf..41e17b8c2c20 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -485,7 +485,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev,
>         trace_device_pm_callback_start(dev, info, state.event);
>         error = cb(dev);
>         trace_device_pm_callback_end(dev, error);
> -       suspend_report_result(cb, error);
> +       suspend_report_result(dev, cb, error);
>
>         initcall_debug_report(dev, calltime, cb, error);
>
> @@ -1568,7 +1568,7 @@ static int legacy_suspend(struct device *dev, pm_message_t state,
>         trace_device_pm_callback_start(dev, info, state.event);
>         error = cb(dev, state);
>         trace_device_pm_callback_end(dev, error);
> -       suspend_report_result(cb, error);
> +       suspend_report_result(dev, cb, error);
>
>         initcall_debug_report(dev, calltime, cb, error);
>
> @@ -1855,7 +1855,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>         device_unlock(dev);
>
>         if (ret < 0) {
> -               suspend_report_result(callback, ret);
> +               suspend_report_result(dev, callback, ret);
>                 pm_runtime_put(dev);
>                 return ret;
>         }
> @@ -1960,10 +1960,10 @@ int dpm_suspend_start(pm_message_t state)
>  }
>  EXPORT_SYMBOL_GPL(dpm_suspend_start);
>
> -void __suspend_report_result(const char *function, void *fn, int ret)
> +void __suspend_report_result(const char *function, struct device *dev, void *fn, int ret)
>  {
>         if (ret)
> -               pr_err("%s(): %pS returns %d\n", function, fn, ret);
> +               dev_err(dev, "%s(): %pS returns %d\n", function, fn, ret);
>  }
>  EXPORT_SYMBOL_GPL(__suspend_report_result);
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index f61c40a47891..4ceeb75fc899 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -607,7 +607,7 @@ static int pci_legacy_suspend(struct device *dev, pm_message_t state)
>                 int error;
>
>                 error = drv->suspend(pci_dev, state);
> -               suspend_report_result(drv->suspend, error);
> +               suspend_report_result(dev, drv->suspend, error);
>                 if (error)
>                         return error;
>
> @@ -786,7 +786,7 @@ static int pci_pm_suspend(struct device *dev)
>                 int error;
>
>                 error = pm->suspend(dev);
> -               suspend_report_result(pm->suspend, error);
> +               suspend_report_result(dev, pm->suspend, error);
>                 if (error)
>                         return error;
>
> @@ -832,7 +832,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
>                 int error;
>
>                 error = pm->suspend_noirq(dev);
> -               suspend_report_result(pm->suspend_noirq, error);
> +               suspend_report_result(dev, pm->suspend_noirq, error);
>                 if (error)
>                         return error;
>
> @@ -1021,7 +1021,7 @@ static int pci_pm_freeze(struct device *dev)
>                 int error;
>
>                 error = pm->freeze(dev);
> -               suspend_report_result(pm->freeze, error);
> +               suspend_report_result(dev, pm->freeze, error);
>                 if (error)
>                         return error;
>         }
> @@ -1041,7 +1041,7 @@ static int pci_pm_freeze_noirq(struct device *dev)
>                 int error;
>
>                 error = pm->freeze_noirq(dev);
> -               suspend_report_result(pm->freeze_noirq, error);
> +               suspend_report_result(dev, pm->freeze_noirq, error);
>                 if (error)
>                         return error;
>         }
> @@ -1127,7 +1127,7 @@ static int pci_pm_poweroff(struct device *dev)
>                 int error;
>
>                 error = pm->poweroff(dev);
> -               suspend_report_result(pm->poweroff, error);
> +               suspend_report_result(dev, pm->poweroff, error);
>                 if (error)
>                         return error;
>         }
> @@ -1165,7 +1165,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
>                 int error;
>
>                 error = pm->poweroff_noirq(dev);
> -               suspend_report_result(pm->poweroff_noirq, error);
> +               suspend_report_result(dev, pm->poweroff_noirq, error);
>                 if (error)
>                         return error;
>         }
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index cc6757dfa3f1..c02e7bf643a6 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -171,7 +171,7 @@ static int __pnp_bus_suspend(struct device *dev, pm_message_t state)
>
>         if (pnp_drv->driver.pm && pnp_drv->driver.pm->suspend) {
>                 error = pnp_drv->driver.pm->suspend(dev);
> -               suspend_report_result(pnp_drv->driver.pm->suspend, error);
> +               suspend_report_result(dev, pnp_drv->driver.pm->suspend, error);
>                 if (error)
>                         return error;
>         }
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 784466117c92..8176bc81a635 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -446,7 +446,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
>                                 HCD_WAKEUP_PENDING(hcd->shared_hcd))
>                         return -EBUSY;
>                 retval = hcd->driver->pci_suspend(hcd, do_wakeup);
> -               suspend_report_result(hcd->driver->pci_suspend, retval);
> +               suspend_report_result(dev, hcd->driver->pci_suspend, retval);
>
>                 /* Check again in case wakeup raced with pci_suspend */
>                 if ((retval == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) ||
> @@ -556,7 +556,7 @@ static int hcd_pci_suspend_noirq(struct device *dev)
>                 dev_dbg(dev, "--> PCI %s\n",
>                                 pci_power_name(pci_dev->current_state));
>         } else {
> -               suspend_report_result(pci_prepare_to_sleep, retval);
> +               suspend_report_result(dev, pci_prepare_to_sleep, retval);
>                 return retval;
>         }
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index f7d2be686359..e65b3ab28377 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -770,11 +770,11 @@ extern int dpm_suspend_late(pm_message_t state);
>  extern int dpm_suspend(pm_message_t state);
>  extern int dpm_prepare(pm_message_t state);
>
> -extern void __suspend_report_result(const char *function, void *fn, int ret);
> +extern void __suspend_report_result(const char *function, struct device *dev, void *fn, int ret);
>
> -#define suspend_report_result(fn, ret)                                 \
> +#define suspend_report_result(dev, fn, ret)                            \
>         do {                                                            \
> -               __suspend_report_result(__func__, fn, ret);             \
> +               __suspend_report_result(__func__, dev, fn, ret);        \
>         } while (0)
>
>  extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
> @@ -814,7 +814,7 @@ static inline int dpm_suspend_start(pm_message_t state)
>         return 0;
>  }
>
> -#define suspend_report_result(fn, ret)         do {} while (0)
> +#define suspend_report_result(dev, fn, ret)    do {} while (0)
>
>  static inline int device_pm_wait_for_dev(struct device *a, struct device *b)
>  {
> --

Applied as 5.18 material, thanks!

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-08 19:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20220308104013epcas1p4c096ed0065700f2f1f8b5ea1ae4ed994@epcas1p4.samsung.com>
2022-03-07 19:07 ` [PATCH v3] PM: Add device name to suspend_report_result() Youngjin Jang
2022-03-08 19:00   ` Rafael J. Wysocki

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).