2019-09-19 00:19:17

by Matthias Kaehlcke

Subject: [PATCH] devfreq: Make log message more explicit when devfreq device already exists

Before creating a new devfreq device devfreq_add_device() checks
if there is already a devfreq dev associated with the requesting
device (parent). If that's the case the function rejects to create
another devfreq dev for that parent and logs an error. The error
message is very unspecific, make it a bit more explicit.

Signed-off-by: Matthias Kaehlcke <[email protected]>
---
drivers/devfreq/devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ab22bf8a12d6..0e2dd734ab58 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -625,7 +625,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq = find_device_devfreq(dev);
mutex_unlock(&devfreq_list_lock);
if (!IS_ERR(devfreq)) {
- dev_err(dev, "%s: Unable to create devfreq for the device.\n",
+ dev_err(dev, "%s: devfreq device already exists!\n",
__func__);
err = -EINVAL;
goto err_out;
--
2.23.0.237.gc6a4ce50a0-goog

2019-09-19 06:56:32

by MyungJoo Ham

Subject: RE: [PATCH] devfreq: Make log message more explicit when devfreq device already exists

>Before creating a new devfreq device devfreq_add_device() checks
>if there is already a devfreq dev associated with the requesting
>device (parent). If that's the case the function rejects to create
>another devfreq dev for that parent and logs an error. The error
>message is very unspecific, make it a bit more explicit.
>
>Signed-off-by: Matthias Kaehlcke <[email protected]>
>---
> drivers/devfreq/devfreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>index ab22bf8a12d6..0e2dd734ab58 100644
>--- a/drivers/devfreq/devfreq.c
>+++ b/drivers/devfreq/devfreq.c
>@@ -625,7 +625,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> devfreq = find_device_devfreq(dev);
> mutex_unlock(&devfreq_list_lock);
> if (!IS_ERR(devfreq)) {
>- dev_err(dev, "%s: Unable to create devfreq for the device.\n",
>+ dev_err(dev, "%s: devfreq device already exists!\n",

Yes, this is more helpful! Thanks!

Acked-by: MyungJoo Ham <[email protected]>

2019-09-20 21:54:24

by Chanwoo Choi

Subject: Re: [PATCH] devfreq: Make log message more explicit when devfreq device already exists

Hi,

On 19. 9. 19. 오전 9:09, Matthias Kaehlcke wrote:
> Before creating a new devfreq device devfreq_add_device() checks
> if there is already a devfreq dev associated with the requesting
> device (parent). If that's the case the function rejects to create
> another devfreq dev for that parent and logs an error. The error
> message is very unspecific, make it a bit more explicit.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/devfreq/devfreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index ab22bf8a12d6..0e2dd734ab58 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -625,7 +625,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> devfreq = find_device_devfreq(dev);
> mutex_unlock(&devfreq_list_lock);
> if (!IS_ERR(devfreq)) {
> - dev_err(dev, "%s: Unable to create devfreq for the device.\n",
> + dev_err(dev, "%s: devfreq device already exists!\n",

Looks good to me. But After edited the log message, you can make
it on one line because the length is not over 80 char.
You can change it for the readability as following:

if (!IS_ERR(devfreq)) {
- dev_err(dev, "%s: Unable to create devfreq for the device.\n",
- __func__);
+ dev_err(dev, "%s: devfreq device already exists.\n", __func__);

If you agree my comment, you can feel free to add my tag:
Reviewed-by: Chanwoo Choi <[email protected]>


> __func__);
> err = -EINVAL;
> goto err_out;
>

--
Best Regards,
Chanwoo Choi
Samsung Electronics