| Message ID | 20191223191923.10450-1-tiny.windzz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PM / devfreq: exynos-bus: Add error log when get event fails | expand |
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@gmail.com> > --- > 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); 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; > } >
On Tue, Dec 24, 2019 at 12:00 PM Chanwoo Choi <cw00.choi@samsung.com> 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@gmail.com> > > --- > > 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... 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
On 12/24/19 11:51 PM, Frank Lee wrote: > On Tue, Dec 24, 2019 at 12:00 PM Chanwoo Choi <cw00.choi@samsung.com> 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@gmail.com> >>> --- >>> 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 > >
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); stat->total_time = stat->busy_time = 0; goto err; }
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@gmail.com> --- drivers/devfreq/exynos-bus.c | 2 ++ 1 file changed, 2 insertions(+)