diff mbox

cpufreq: s5pv210: remove unused call of pr_err()

Message ID 1400702036.30334.6.camel@x220 (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Paul Bolle May 21, 2014, 7:53 p.m. UTC
A call of pr_err() was added in v3.1. It was guarded by a check for
CONFIG_PM_VERBOSE. The Kconfig symbol PM_VERBOSE was removed in v3.0. So
this call of pr_err() has never been used. Remove it.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Untested.

 drivers/cpufreq/s5pv210-cpufreq.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Sachin Kamat May 22, 2014, 3:22 a.m. UTC | #1
On 22 May 2014 01:23, Paul Bolle <pebolle@tiscali.nl> wrote:
> A call of pr_err() was added in v3.1. It was guarded by a check for
> CONFIG_PM_VERBOSE. The Kconfig symbol PM_VERBOSE was removed in v3.0. So
> this call of pr_err() has never been used. Remove it.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> Untested.
>
>  drivers/cpufreq/s5pv210-cpufreq.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
> index ab2c1a40d437..0873729135df 100644
> --- a/drivers/cpufreq/s5pv210-cpufreq.c
> +++ b/drivers/cpufreq/s5pv210-cpufreq.c
> @@ -175,10 +175,6 @@ static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
>         mutex_lock(&set_freq_lock);
>
>         if (no_cpufreq_access) {
> -#ifdef CONFIG_PM_VERBOSE
> -               pr_err("%s:%d denied access to %s as it is disabled"
> -                               "temporarily\n", __FILE__, __LINE__, __func__);
> -#endif

The config symbol itself is not defined and hence could be removed.
However I feel, the
error message could be retained after trimming down the arguments a bit.
Viresh Kumar May 22, 2014, 4:14 a.m. UTC | #2
On 22 May 2014 08:52, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> On 22 May 2014 01:23, Paul Bolle <pebolle@tiscali.nl> wrote:
>> A call of pr_err() was added in v3.1. It was guarded by a check for
>> CONFIG_PM_VERBOSE. The Kconfig symbol PM_VERBOSE was removed in v3.0. So
>> this call of pr_err() has never been used. Remove it.
>>
>> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
>> ---
>> Untested.
>>
>>  drivers/cpufreq/s5pv210-cpufreq.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
>> index ab2c1a40d437..0873729135df 100644
>> --- a/drivers/cpufreq/s5pv210-cpufreq.c
>> +++ b/drivers/cpufreq/s5pv210-cpufreq.c
>> @@ -175,10 +175,6 @@ static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
>>         mutex_lock(&set_freq_lock);
>>
>>         if (no_cpufreq_access) {
>> -#ifdef CONFIG_PM_VERBOSE
>> -               pr_err("%s:%d denied access to %s as it is disabled"
>> -                               "temporarily\n", __FILE__, __LINE__, __func__);
>> -#endif
>
> The config symbol itself is not defined and hence could be removed.

What do you meant by this? He already got rid of CONFIG_PM_VERBOSE..

> However I feel, the
> error message could be retained after trimming down the arguments a bit.

I agree..
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sachin Kamat May 22, 2014, 4:19 a.m. UTC | #3
On 22 May 2014 09:44, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 22 May 2014 08:52, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> On 22 May 2014 01:23, Paul Bolle <pebolle@tiscali.nl> wrote:
>>> A call of pr_err() was added in v3.1. It was guarded by a check for
>>> CONFIG_PM_VERBOSE. The Kconfig symbol PM_VERBOSE was removed in v3.0. So
>>> this call of pr_err() has never been used. Remove it.
>>>
>>> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
>>> ---
>>> Untested.
>>>
>>>  drivers/cpufreq/s5pv210-cpufreq.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
>>> index ab2c1a40d437..0873729135df 100644
>>> --- a/drivers/cpufreq/s5pv210-cpufreq.c
>>> +++ b/drivers/cpufreq/s5pv210-cpufreq.c
>>> @@ -175,10 +175,6 @@ static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
>>>         mutex_lock(&set_freq_lock);
>>>
>>>         if (no_cpufreq_access) {
>>> -#ifdef CONFIG_PM_VERBOSE
>>> -               pr_err("%s:%d denied access to %s as it is disabled"
>>> -                               "temporarily\n", __FILE__, __LINE__, __func__);
>>> -#endif
>>
>> The config symbol itself is not defined and hence could be removed.
>
> What do you meant by this? He already got rid of CONFIG_PM_VERBOSE..

Yes, my ack for that change :)
Paul Bolle May 22, 2014, 6:41 a.m. UTC | #4
Sachin,

On Thu, 2014-05-22 at 08:52 +0530, Sachin Kamat wrote:
> The config symbol itself is not defined and hence could be removed.
> However I feel, the
> error message could be retained after trimming down the arguments a bit.

My reasoning is, of course, that never used code can simply be dropped.

But you want to print an error here. That's fine with me. I prefer if
you'd submit a patch that does that, as I have no idea whatsoever why an
error is needed here nor what that error should say.

I do hope you don't mind me prodding you if that takes too long, and
this pointless check of CONFIG_PM_VERBOSE is still here after, say,
another development cycle.


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sachin Kamat May 22, 2014, 8:23 a.m. UTC | #5
Hi Paul,

On 22 May 2014 12:11, Paul Bolle <pebolle@tiscali.nl> wrote:
> Sachin,
>
> On Thu, 2014-05-22 at 08:52 +0530, Sachin Kamat wrote:
>> The config symbol itself is not defined and hence could be removed.
>> However I feel, the
>> error message could be retained after trimming down the arguments a bit.
>
> My reasoning is, of course, that never used code can simply be dropped.

Yes, I agree. However the error message in this case should have been outside
the ifdefs to begin with as it is not an extra information but an
essential message
reporting error condition.

>
> But you want to print an error here. That's fine with me. I prefer if
> you'd submit a patch that does that, as I have no idea whatsoever why an
> error is needed here nor what that error should say.

You may just remove the __FILE__ and __LINE__ thing and leave the message as is.

pr_err("Denied access to %s as it is disabled temporarily\n", __func__);
diff mbox

Patch

diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
index ab2c1a40d437..0873729135df 100644
--- a/drivers/cpufreq/s5pv210-cpufreq.c
+++ b/drivers/cpufreq/s5pv210-cpufreq.c
@@ -175,10 +175,6 @@  static int s5pv210_target(struct cpufreq_policy *policy, unsigned int index)
 	mutex_lock(&set_freq_lock);
 
 	if (no_cpufreq_access) {
-#ifdef CONFIG_PM_VERBOSE
-		pr_err("%s:%d denied access to %s as it is disabled"
-				"temporarily\n", __FILE__, __LINE__, __func__);
-#endif
 		ret = -EINVAL;
 		goto exit;
 	}