Re: [PATCH 2/2] cpufreq: powernv: Register for OCC related opal_message notification

[Viresh Kumar]

On 22 April 2015 at 22:34, Shilpasri G Bhat
<shilpa.bhat@xxxxxxxxxxxxxxxxxx> wrote:
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c

> +static char throttle_reason[6][50] = { "No throttling",

Don't need to mention 6 here.

And the max length you need right now is 27, so maybe s/50/30 ?

Also, start 'No Throttling' in a new line, like below.

> +                                       "Power Cap",
> +                                       "Processor Over Temperature",
> +                                       "Power Supply Failure",
> +                                       "OverCurrent",

s/OverCurrent/Over Current/ ?

> +                                       "OCC Reset"
> +                                    };
> +
> +static int powernv_cpufreq_occ_msg(struct notifier_block *nb,
> +               unsigned long msg_type, void *msg)
> +{
> +       struct opal_msg *occ_msg = msg;
> +       uint64_t token;
> +       uint64_t chip_id, reason;
> +
> +       if (msg_type != OPAL_MSG_OCC)
> +               return 0;

Blank line here.

> +       token = be64_to_cpu(occ_msg->params[0]);

Here as well..

> +       switch (token) {
> +       case 0:
> +               occ_reset = true;
> +               /*
> +                * powernv_cpufreq_throttle_check() is called in
> +                * target() callback which can detect the throttle state
> +                * for governors like ondemand.
> +                * But static governors will not call target() often thus
> +                * report throttling here.
> +                */

Now, do I understand correctly that this notifier will be called as
soon as we switch throttling state ?

If yes, then do we still need the throttle_check() routine you added
earlier ? Maybe not.

> +               if (!throttled) {
> +                       throttled = true;
> +                       pr_crit("CPU Frequency is throttled\n");
> +               }
> +               pr_info("OCC in Reset\n");
> +               break;
> +       case 1:
> +               pr_info("OCC is Loaded\n");
> +               break;
> +       case 2:
> +               chip_id = be64_to_cpu(occ_msg->params[1]);
> +               reason = be64_to_cpu(occ_msg->params[2]);

Blank line here.

> +               if (occ_reset) {
> +                       occ_reset = false;
> +                       throttled = false;
> +                       pr_info("OCC is Active\n");
> +                       /* Sanity check for static governors */
> +                       powernv_cpufreq_throttle_check(smp_processor_id());
> +               } else if (reason) {
> +                       throttled = true;
> +                       pr_info("Pmax reduced due to %s on chip %x\n",
> +                                       throttle_reason[reason], (int)chip_id);
> +               } else {
> +                       throttled = false;
> +                       pr_info("%s on chip %x\n",
> +                                       throttle_reason[reason], (int)chip_id);
> +               }

Run checkpatch with --strict option, and you will see some warnings.

> +               break;
> +       }
> +       return 0;
> +}
> +
> +static struct notifier_block powernv_cpufreq_opal_nb = {
> +       .notifier_call  = powernv_cpufreq_occ_msg,
> +       .next           = NULL,
> +       .priority       = 0,
> +};
> +
>  static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
>  {
>         struct powernv_smp_call_data freq_data;
> @@ -430,6 +497,7 @@ static int __init powernv_cpufreq_init(void)
>         }
>
>         register_reboot_notifier(&powernv_cpufreq_reboot_nb);
> +       opal_message_notifier_register(OPAL_MSG_OCC, &powernv_cpufreq_opal_nb);
>         return cpufreq_register_driver(&powernv_cpufreq_driver);
>  }
>  module_init(powernv_cpufreq_init);
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/