On Wed, Mar 02, 2022 at 08:00:14PM +0900, �念��/TV S/W Lab(VD)/Staff Engineer/�Z���� wrote: > > -----Original Message----- > > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Sent: Wednesday, March 2, 2022 4:58 PM > > To: Youngjin Jang <yj84.jang@xxxxxxxxxxx> > > Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>; Pavel Machek <pavel@xxxxxx>; > > Len Brown <len.brown@xxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; > > linux-pm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > > pci@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; linux- > usb@xxxxxxxxxxxxxxx; > > js07.lee@xxxxxxxxxxx > > Subject: Re: [PATCH] PM: Add device name to suspend_report_result() > > > > On Wed, Mar 02, 2022 at 03:49:17PM +0900, Youngjin Jang wrote: > > > From: "yj84.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 information 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, > > > PM: dpm_run_callback(): pm_generic_suspend+0x0/0x48 (amba) returns 0 > > > PM: dpm_run_callback(): platform_pm_suspend+0x0/0x68 (armv7-pmu) > > > returns 0 > > > > > > Signed-off-by: yj84.jang <yj84.jang@xxxxxxxxxxx> > > > --- > > > 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 04ea92c..a762fe8 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); > > > + pr_err("%s(): %pS (%s) returns %d\n", function, fn, > > > +dev_driver_string(dev), ret); > > > > If you have a struct device, please use dev_err(). > > > > thanks, > > > > greg k-h > > Hello, > Thanks for your review. > > I think dev_err() is nice option, but we can see a minor issue. > Prefix log "PM: " would be lost, If I use dev_err() in this context. > As you know, all logs in power management include "PM :" prefix. Why does that matter? Fix them all to use the struct device pointer and then they will be properly unified with the rest of the kernel log infrastructure. thanks, greg k-h