[PATCH v2 1/2] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform

Joel Stanley joel at jms.id.au
Fri May 8 11:26:36 AEST 2015


Hello Vipin,

On Thu, May 7, 2015 at 7:00 PM, Vipin K Parashar
<vipin at linux.vnet.ibm.com> wrote:
> This patch adds support for FSP EPOW (Early Power Off Warning) and
> DPO (Delayed Power Off) events support for PowerNV platform.

I reviewed this patch for the changes it made to the existing poweroff
code, you still need someone to look at the EPOW code itself.

> Signed-off-by: Vipin K Parashar <vipin at linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/opal-api.h            |  30 ++
>  arch/powerpc/include/asm/opal.h                |   3 +-
>  arch/powerpc/platforms/powernv/opal-power.c    | 379 +++++++++++++++++++++++--
>  arch/powerpc/platforms/powernv/opal-wrappers.S |   1 +
>  4 files changed, 391 insertions(+), 22 deletions(-)
>

>  /* Internal functions */
>  extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
> index ac46c2c..7c1b2f8 100644
> --- a/arch/powerpc/platforms/powernv/opal-power.c
> +++ b/arch/powerpc/platforms/powernv/opal-power.c
> @@ -1,5 +1,5 @@
>  /*
> - * PowerNV OPAL power control for graceful shutdown handling
> + * PowerNV poweroff events support
>   *
>   * Copyright 2015 IBM Corp.
>   *
> @@ -9,58 +9,395 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>
> +#define pr_fmt(fmt)    "POWEROFF_EVENT: "    fmt

OPAL_POWER?

> +
>  #include <linux/kernel.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
>  #include <linux/reboot.h>
> -#include <linux/notifier.h>
> -
> +#include <linux/of.h>
>  #include <asm/opal.h>
>  #include <asm/machdep.h>
>
> -#define SOFT_OFF 0x00
> -#define SOFT_REBOOT 0x01
> +/* Power control event types */
> +#define SOFT_OFF       0x00
> +#define SOFT_REBOOT    0x01

While you're touching this code, I think these should be moved to opal-api.h

> +
> +/* Max time for graceful system shutdown including guests. */
> +#define MAX_POWEROFF_SYS_TIME  600
> +

> +/* IPMI power-control events notifier */
>  static int opal_power_control_event(struct notifier_block *nb,
> -                                   unsigned long msg_type, void *msg)
> +                               unsigned long msg_type, void *msg)
>  {
> -       struct opal_msg *power_msg = msg;
>         uint64_t type;
> +       struct opal_msg *power_msg = msg;
>
>         type = be64_to_cpu(power_msg->params[0]);
>
>         switch (type) {
>         case SOFT_REBOOT:
> -               pr_info("OPAL: reboot requested\n");
> +               pr_info("Reboot requested\n");

I prefer the OPAL prefix.

>                 orderly_reboot();
>                 break;
>         case SOFT_OFF:
> -               pr_info("OPAL: poweroff requested\n");
> +               pr_info("Poweroff requested\n");

Ditto.

>                 orderly_poweroff(true);
>                 break;
>         default:
> -               pr_err("OPAL: power control type unexpected %016llx\n", type);
> +               pr_err("Unknown event %llu\n", type);

Ditto.

>         }
>
>         return 0;
>  }
>
> +/* OPAL EPOW event notifier block */
> +static struct notifier_block opal_epow_nb = {
> +       .notifier_call  = opal_epow_event,
> +       .next           = NULL,
> +       .priority       = 0,
> +};
> +
> +/* OPAL DPO event notifier block */
> +static struct notifier_block opal_dpo_nb = {
> +       .notifier_call  = opal_dpo_event,
> +       .next           = NULL,
> +       .priority       = 0,
> +};
> +
> +/* OPAL Power control events */
>  static struct notifier_block opal_power_control_nb = {
> -       .notifier_call  = opal_power_control_event,
> -       .next           = NULL,
> -       .priority       = 0,
> +       .notifier_call  = opal_power_control_event,
> +       .next           = NULL,
> +       .priority       = 0,
>  };

Looks like you changed the whitespace?

>
> -static int __init opal_power_control_init(void)
> +/* Poweroff events init */
> +static int __init opal_poweroff_events_init(void)

This comment does not add any value.

Renaming the function doesn't add much either.

>  {
>         int ret;
> +       struct device_node *node_epow;
>
> -       ret = opal_message_notifier_register(OPAL_MSG_SHUTDOWN,
> -                                            &opal_power_control_nb);
> -       if (ret) {
> -               pr_err("%s: Can't register OPAL event notifier (%d)\n",
> -                               __func__, ret);
> -               return ret;
> +       /*
> +       * Determine EPOW, DPO support in hardware.
> +       */
> +       node_epow = of_find_node_by_path("/ibm,opal/epow");
> +       if (node_epow) {
> +               if (of_device_is_compatible(node_epow, "ibm,opal-epow")) {
> +                       epow_supported = true;
> +                       dpo_supported = true;

Why are these separate flags? Do we have any systems that will support
EPOW but not DPO, or DPO without EPOW?

I suggest merging them into the one flag.

> +                       pr_info("OPAL EPOW, DPO support detected.\n");
> +               }
> +               of_node_put(node_epow);
> +       }
> +
> +       /* Prepare to handle EPOW events */
> +       if (epow_supported) {
> +               /* Initialize poweroff timer */
> +               init_timer(&epow_timer);
> +               epow_timer.function = epow_poweroff;
> +
> +               /* Register EPOW event notifier */
> +               ret = opal_message_notifier_register(OPAL_MSG_EPOW,
> +                               &opal_epow_nb);
> +               if (ret) {
> +                       pr_err("EPOW event notifier registration failed, "
> +                                       "ret = %d\n", ret);
> +               }
> +       }
> +
> +       if (dpo_supported) {
> +               /* Register DPO event notifier */
> +               ret = opal_message_notifier_register(OPAL_MSG_DPO,
> +                               &opal_dpo_nb);
> +               if (ret)
> +                       pr_err("DPO event notifier registration failed, "
> +                                       "ret = %d\n", ret);
>         }
>
> +       /* Check for any existing EPOW or DPO events. */
> +       process_existing_poweroff_events();
> +
> +       /* Register IPMI poweroff events notifier */
> +       ret = opal_message_notifier_register(OPAL_MSG_SHUTDOWN,
> +                               &opal_power_control_nb);
> +       if (ret)
> +               pr_err("IPMI power-control events notifier registration "
> +                               "failed, ret = %d\n", ret);

The comment and the pr_err message are both incorrect; this is not an
IPMI event but an OPAL event.

> +
> +
> +       pr_info("Poweroff events support initialized\n");

This is unnecessary.

>         return 0;
>  }
> -machine_subsys_initcall(powernv, opal_power_control_init);
> +
> +machine_subsys_initcall(powernv, opal_poweroff_events_init);
> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
> index a7ade94..5d3c8e3 100644
> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
> @@ -249,6 +249,7 @@ OPAL_CALL(opal_pci_reinit,                  OPAL_PCI_REINIT);
>  OPAL_CALL(opal_pci_mask_pe_error,              OPAL_PCI_MASK_PE_ERROR);
>  OPAL_CALL(opal_set_slot_led_status,            OPAL_SET_SLOT_LED_STATUS);
>  OPAL_CALL(opal_get_epow_status,                        OPAL_GET_EPOW_STATUS);
> +OPAL_CALL(opal_get_dpo_status,                 OPAL_GET_DPO_STATUS);
>  OPAL_CALL(opal_set_system_attention_led,       OPAL_SET_SYSTEM_ATTENTION_LED);
>  OPAL_CALL(opal_pci_next_error,                 OPAL_PCI_NEXT_ERROR);
>  OPAL_CALL(opal_pci_poll,                       OPAL_PCI_POLL);
> --
> 1.9.3
>


More information about the Linuxppc-dev mailing list