Re: [PATCH v2] PM: Add device name to suspend_report_result()

From: Rafael J. Wysocki
Date: Thu Mar 03 2022 - 06:48:48 EST

Next message: Miaohe Lin: "Re: [PATCH 4/8] mm: thp: only regular file could be THP eligible"
Previous message: Vinod Polimera: "RE: [v1] drm/msm/disp/dpu1: add inline rotation support for sc7280 target"
In reply to: Youngjin Jang: "[PATCH v2] PM: Add device name to suspend_report_result()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

+Todd

On Thu, Mar 3, 2022 at 10:43 AM Youngjin Jang <yj84.jang@xxxxxxxxxxx> wrote:
>
> From: "Youngjin Jang" <yj84.jang@xxxxxxxxxxx>
>
> 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(): pm_generic_suspend+0x0/0x48 returns 0
> PM: dpm_run_callback(): platform_pm_suspend+0x0/0x68 returns 0
> after patch,
> amba 1740000.etm: dpm_run_callback(): pm_generic_suspend+0x0/0x48
> returns 0
> armv7-pmu soc:pmu: dpm_run_callback(): platform_pm_suspend+0x0/0x68
> returns 0

The PM prefix would still be useful IMO.

@Todd: Would this change affect pm-graph in any way?

>
> Signed-off-by: Youngjin Jang <yj84.jang@xxxxxxxxxxx>
> ---
> 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 588588cfda48..415f7664b010 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -596,7 +596,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;
>
> @@ -775,7 +775,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;
>
> @@ -821,7 +821,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;
>
> @@ -1010,7 +1010,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;
> }
> @@ -1030,7 +1030,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;
> }
> @@ -1116,7 +1116,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;
> }
> @@ -1154,7 +1154,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 d630cccd2e6e..dd44e37a454a 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 e1e9402180b9..cdccbb9cef2c 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -745,11 +745,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);
> @@ -789,7 +789,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
>

Next message: Miaohe Lin: "Re: [PATCH 4/8] mm: thp: only regular file could be THP eligible"
Previous message: Vinod Polimera: "RE: [v1] drm/msm/disp/dpu1: add inline rotation support for sc7280 target"
In reply to: Youngjin Jang: "[PATCH v2] PM: Add device name to suspend_report_result()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]