On 12/24/19 11:51 PM, Frank Lee wrote: > On Tue, Dec 24, 2019 at 12:00 PM Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote: >> >> Hi, >> >> I think that you better to use 'devfreq-event' instead of just 'event' >> as following: >> >> PM / devfreq: exynos-bus: Add error log when fail to get devfreq-event >> >> On 12/24/19 4:19 AM, Yangtao Li wrote: >>> Adding an error log makes it easier to trace the function's error path. >>> Because the error code may be rewritten on return, print error code here. >>> >>> Signed-off-by: Yangtao Li <tiny.windzz@xxxxxxxxx> >>> --- >>> drivers/devfreq/exynos-bus.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>> index 948e9340f91c..634d63fd00ea 100644 >>> --- a/drivers/devfreq/exynos-bus.c >>> +++ b/drivers/devfreq/exynos-bus.c >>> @@ -126,6 +126,8 @@ static int exynos_bus_get_dev_status(struct device *dev, >>> >>> ret = exynos_bus_get_event(bus, &edata); >>> if (ret < 0) { >>> + dev_err(dev, "failed to get event from devfreq-event devices %d\n", >>> + ret); > > Emmm, it looks a bit strange to me... If don't show the error value, it is possible to make it until 81 char. I edit it as following and applied it with modified patch title. diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 1259a0da7db7..8fa8eb541373 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -126,6 +126,7 @@ static int exynos_bus_get_dev_status(struct device *dev, ret = exynos_bus_get_event(bus, &edata); if (ret < 0) { + dev_err(dev, "failed to get event from devfreq-event devices\n"); stat->total_time = stat->busy_time = 0; goto err; } > V2 has been sent. > > Yours, > Yangtao > > >> >> Better to make it under 80 char as following: >> >> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >> index f5d4c369c7fb..10f4fa1a0363 100644 >> --- a/drivers/devfreq/exynos-bus.c >> +++ b/drivers/devfreq/exynos-bus.c >> @@ -126,7 +126,8 @@ static int exynos_bus_get_dev_status(struct device *dev, >> >> ret = exynos_bus_get_event(bus, &edata); >> if (ret < 0) { >> - dev_err(dev, "failed to get event from devfreq-event devices %d\n", >> + dev_err(dev, >> + "failed to get event from devfreq-event devices %d\n", >> ret); >> stat->total_time = stat->busy_time = 0; >> goto err; >> >> >>> stat->total_time = stat->busy_time = 0; >>> goto err; >>> } >>> >> >> >> -- >> Best Regards, >> Chanwoo Choi >> Samsung Electronics > > -- Best Regards, Chanwoo Choi Samsung Electronics