diff mbox series

cpufreq: Improve code around unlisted freq check

Message ID 37c3f1f76c055b305d1bba2c2001ac5b1d7a9b5f.1602565964.git.viresh.kumar@linaro.org
State Accepted
Commit 97148d0ae5303bcc18fcd1c9b968a9485292f32a
Headers show
Series cpufreq: Improve code around unlisted freq check | expand

Commit Message

Viresh Kumar Oct. 13, 2020, 5:12 a.m. UTC
The cpufreq core checks if the frequency programmed by the bootloaders
is not listed in the freq table and programs one from the table in such
a case. This is done only if the driver has set the
CPUFREQ_NEED_INITIAL_FREQ_CHECK flag.

Currently we print two separate messages, with almost the same content,
and do this with a pr_warn() which may be a bit too much as the driver
only asked us to check this as it expected this to be the case. Lower
down the severity of the print message by switching to pr_info() instead
and print a single message only.

Reported-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Sumit Gupta Oct. 16, 2020, 11:17 a.m. UTC | #1
On 13/10/20 10:42 AM, Viresh Kumar wrote:
> External email: Use caution opening links or attachments
> 
> 
> The cpufreq core checks if the frequency programmed by the bootloaders
> is not listed in the freq table and programs one from the table in such
> a case. This is done only if the driver has set the
> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag.
> 
> Currently we print two separate messages, with almost the same content,
> and do this with a pr_warn() which may be a bit too much as the driver
> only asked us to check this as it expected this to be the case. Lower
> down the severity of the print message by switching to pr_info() instead
> and print a single message only.
> 

Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
Tested-by: Sumit Gupta <sumitg@nvidia.com>

> Reported-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/cpufreq/cpufreq.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2ea245a6c0c0..99864afac272 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1461,14 +1461,13 @@ static int cpufreq_online(unsigned int cpu)
>           */
>          if ((cpufreq_driver->flags & CPUFREQ_NEED_INITIAL_FREQ_CHECK)
>              && has_target()) {
> +               unsigned int old_freq = policy->cur;
> +
>                  /* Are we running at unknown frequency ? */
> -               ret = cpufreq_frequency_table_get_index(policy, policy->cur);
> +               ret = cpufreq_frequency_table_get_index(policy, old_freq);
>                  if (ret == -EINVAL) {
> -                       /* Warn user and fix it */
> -                       pr_warn("%s: CPU%d: Running at unlisted freq: %u KHz\n",
> -                               __func__, policy->cpu, policy->cur);
> -                       ret = __cpufreq_driver_target(policy, policy->cur - 1,
> -                               CPUFREQ_RELATION_L);
> +                       ret = __cpufreq_driver_target(policy, old_freq - 1,
> +                                                     CPUFREQ_RELATION_L);
> 
>                          /*
>                           * Reaching here after boot in a few seconds may not
> @@ -1476,8 +1475,8 @@ static int cpufreq_online(unsigned int cpu)
>                           * frequency for longer duration. Hence, a BUG_ON().
>                           */
>                          BUG_ON(ret);
> -                       pr_warn("%s: CPU%d: Unlisted initial frequency changed to: %u KHz\n",
> -                               __func__, policy->cpu, policy->cur);
> +                       pr_info("%s: CPU%d: Running at unlisted initial frequency: %u KHz, changing to: %u KHz\n",
> +                               __func__, policy->cpu, old_freq, policy->cur);
>                  }
>          }
> 
> --
> 2.25.0.rc1.19.g042ed3e048af
>
Rafael J. Wysocki Oct. 16, 2020, 2:21 p.m. UTC | #2
On Tue, Oct 13, 2020 at 7:12 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> The cpufreq core checks if the frequency programmed by the bootloaders
> is not listed in the freq table and programs one from the table in such
> a case. This is done only if the driver has set the
> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag.
>
> Currently we print two separate messages, with almost the same content,
> and do this with a pr_warn() which may be a bit too much as the driver
> only asked us to check this as it expected this to be the case. Lower
> down the severity of the print message by switching to pr_info() instead
> and print a single message only.
>
> Reported-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2ea245a6c0c0..99864afac272 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1461,14 +1461,13 @@ static int cpufreq_online(unsigned int cpu)
>          */
>         if ((cpufreq_driver->flags & CPUFREQ_NEED_INITIAL_FREQ_CHECK)
>             && has_target()) {
> +               unsigned int old_freq = policy->cur;
> +
>                 /* Are we running at unknown frequency ? */
> -               ret = cpufreq_frequency_table_get_index(policy, policy->cur);
> +               ret = cpufreq_frequency_table_get_index(policy, old_freq);
>                 if (ret == -EINVAL) {
> -                       /* Warn user and fix it */
> -                       pr_warn("%s: CPU%d: Running at unlisted freq: %u KHz\n",
> -                               __func__, policy->cpu, policy->cur);
> -                       ret = __cpufreq_driver_target(policy, policy->cur - 1,
> -                               CPUFREQ_RELATION_L);
> +                       ret = __cpufreq_driver_target(policy, old_freq - 1,
> +                                                     CPUFREQ_RELATION_L);
>
>                         /*
>                          * Reaching here after boot in a few seconds may not
> @@ -1476,8 +1475,8 @@ static int cpufreq_online(unsigned int cpu)
>                          * frequency for longer duration. Hence, a BUG_ON().
>                          */
>                         BUG_ON(ret);
> -                       pr_warn("%s: CPU%d: Unlisted initial frequency changed to: %u KHz\n",
> -                               __func__, policy->cpu, policy->cur);
> +                       pr_info("%s: CPU%d: Running at unlisted initial frequency: %u KHz, changing to: %u KHz\n",
> +                               __func__, policy->cpu, old_freq, policy->cur);
>                 }
>         }
>
> --

Applied as 5.10-rc material, thanks!
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2ea245a6c0c0..99864afac272 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1461,14 +1461,13 @@  static int cpufreq_online(unsigned int cpu)
 	 */
 	if ((cpufreq_driver->flags & CPUFREQ_NEED_INITIAL_FREQ_CHECK)
 	    && has_target()) {
+		unsigned int old_freq = policy->cur;
+
 		/* Are we running at unknown frequency ? */
-		ret = cpufreq_frequency_table_get_index(policy, policy->cur);
+		ret = cpufreq_frequency_table_get_index(policy, old_freq);
 		if (ret == -EINVAL) {
-			/* Warn user and fix it */
-			pr_warn("%s: CPU%d: Running at unlisted freq: %u KHz\n",
-				__func__, policy->cpu, policy->cur);
-			ret = __cpufreq_driver_target(policy, policy->cur - 1,
-				CPUFREQ_RELATION_L);
+			ret = __cpufreq_driver_target(policy, old_freq - 1,
+						      CPUFREQ_RELATION_L);
 
 			/*
 			 * Reaching here after boot in a few seconds may not
@@ -1476,8 +1475,8 @@  static int cpufreq_online(unsigned int cpu)
 			 * frequency for longer duration. Hence, a BUG_ON().
 			 */
 			BUG_ON(ret);
-			pr_warn("%s: CPU%d: Unlisted initial frequency changed to: %u KHz\n",
-				__func__, policy->cpu, policy->cur);
+			pr_info("%s: CPU%d: Running at unlisted initial frequency: %u KHz, changing to: %u KHz\n",
+				__func__, policy->cpu, old_freq, policy->cur);
 		}
 	}