* Re: [PATCH v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-05-18 15:18 ` [PATCH v5] powerpc/powernv: Add poweroff " Vipin K Parashar
@ 2015-05-19 13:15 ` trigg
2015-06-02 9:16 ` Michael Ellerman
2015-06-02 5:20 ` Vipin K Parashar
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: trigg @ 2015-05-19 13:15 UTC (permalink / raw)
To: Vipin K Parashar
Cc: stewart@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
joel@jms.id.au
> On 18-May-2015, at 20:48, Vipin K Parashar <vipin@linux.vnet.ibm.com> wrot=
e:
>=20
> This patch adds support for FSP EPOW (Early Power Off Warning) and
> DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
> generated by SPCN/FSP due to various critical system conditions that
> need system shutdown. Few examples of these conditions are high
> ambient temperature or system running on UPS power with low UPS battery.
> DPO event is generated in response to admin initiated system request.
> Upon receipt of EPOW and DPO events host kernel invokes
> orderly_poweroff for performing graceful system shutdown. System admin
> can also add systemd service shutdown scripts to perform any specific
> actions like graceful guest shutdown upon system poweroff. libvirt-guests
> is systemd service available on recent distros for management of guests
> at system start/shutdown time.
>=20
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
Acked-by: trigg <mr.triggtrigg@gmail.com>=
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-05-19 13:15 ` trigg
@ 2015-06-02 9:16 ` Michael Ellerman
0 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2015-06-02 9:16 UTC (permalink / raw)
To: trigg
Cc: Vipin K Parashar, stewart@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org, joel@jms.id.au
On Tue, 2015-05-19 at 18:45 +0530, trigg wrote:
>
> > On 18-May-2015, at 20:48, Vipin K Parashar <vipin@linux.vnet.ibm.com> wrote:
> >
> > This patch adds support for FSP EPOW (Early Power Off Warning) and
> > DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
> > generated by SPCN/FSP due to various critical system conditions that
> > need system shutdown. Few examples of these conditions are high
> > ambient temperature or system running on UPS power with low UPS battery.
> > DPO event is generated in response to admin initiated system request.
> > Upon receipt of EPOW and DPO events host kernel invokes
> > orderly_poweroff for performing graceful system shutdown. System admin
> > can also add systemd service shutdown scripts to perform any specific
> > actions like graceful guest shutdown upon system poweroff. libvirt-guests
> > is systemd service available on recent distros for management of guests
> > at system start/shutdown time.
> >
> > Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
>
> Acked-by: trigg <mr.triggtrigg@gmail.com>
Please use your real name.
See: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n446
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-05-18 15:18 ` [PATCH v5] powerpc/powernv: Add poweroff " Vipin K Parashar
2015-05-19 13:15 ` trigg
@ 2015-06-02 5:20 ` Vipin K Parashar
2015-06-03 3:41 ` Vaibhav Jain
2015-06-03 5:13 ` [v5] " Michael Ellerman
3 siblings, 0 replies; 8+ messages in thread
From: Vipin K Parashar @ 2015-06-02 5:20 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: joel, stewart, vaibhav
Hi Michael,
Please add below minor additions in commit log once u
accept this patch. Thanks for your help with this. Let me know,
if anything else is needed from me on this.
Regards,
Vipin
On 05/18/2015 08:48 PM, Vipin K Parashar wrote:
> This patch adds support for FSP EPOW (Early Power Off Warning) and
> DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
> generated by SPCN/FSP due to various critical system conditions that
> need system shutdown. Few examples of these conditions are high
> ambient temperature or system running on UPS power with low UPS battery.
> DPO event is generated in response to admin initiated system request.
s/system request/shutdown request/
> Upon receipt of EPOW and DPO events host kernel invokes
> orderly_poweroff for performing graceful system shutdown. System admin
> can also add systemd service shutdown scripts to perform any specific
> actions like graceful guest shutdown upon system poweroff. libvirt-guests
> is systemd service available on recent distros for management of guests
> at system start/shutdown time.
>
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/opal-api.h | 44 ++++++++
> arch/powerpc/include/asm/opal.h | 3 +-
> arch/powerpc/platforms/powernv/opal-power.c | 147 ++++++++++++++++++++++---
> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
> 4 files changed, 179 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 0321a90..90fa364 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -355,6 +355,10 @@ enum opal_msg_type {
> OPAL_MSG_TYPE_MAX,
> };
>
> +/* OPAL_MSG_SHUTDOWN parameter values */
> +#define SOFT_OFF 0x00
> +#define SOFT_REBOOT 0x01
> +
> struct opal_msg {
> __be32 msg_type;
> __be32 reserved;
> @@ -730,6 +734,46 @@ struct opal_i2c_request {
> __be64 buffer_ra; /* Buffer real address */
> };
>
> +/*
> + * EPOW status sharing (OPAL and the host)
> + *
> + * The host will pass on OPAL, a buffer of length OPAL_SYSEPOW_MAX
> + * with individual elements being 16 bits wide to fetch the system
> + * wide EPOW status. Each element in the buffer will contain the
> + * EPOW status in it's bit representation for a particular EPOW sub
> + * class as defiend here. So multiple detailed EPOW status bits
> + * specific for any sub class can be represented in a single buffer
> + * element as it's bit representation.
> + */
> +
> +/* System EPOW type */
> +enum OpalSysEpow {
> + OPAL_SYSEPOW_POWER = 0, /* Power EPOW */
> + OPAL_SYSEPOW_TEMP = 1, /* Temperature EPOW */
> + OPAL_SYSEPOW_COOLING = 2, /* Cooling EPOW */
> + OPAL_SYSEPOW_MAX = 3, /* Max EPOW categories */
> +};
> +
> +/* Power EPOW */
> +enum OpalSysPower {
> + OPAL_SYSPOWER_UPS = 0x0001, /* System on UPS power */
> + OPAL_SYSPOWER_CHNG = 0x0002, /* System power config change */
> + OPAL_SYSPOWER_FAIL = 0x0004, /* System impending power failure */
> + OPAL_SYSPOWER_INCL = 0x0008, /* System incomplete power */
> +};
> +
> +/* Temperature EPOW */
> +enum OpalSysTemp {
> + OPAL_SYSTEMP_AMB = 0x0001, /* System over ambient temperature */
> + OPAL_SYSTEMP_INT = 0x0002, /* System over internal temperature */
> + OPAL_SYSTEMP_HMD = 0x0004, /* System over ambient humidity */
> +};
> +
> +/* Cooling EPOW */
> +enum OpalSysCooling {
> + OPAL_SYSCOOL_INSF = 0x0001, /* System insufficient cooling */
> +};
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 042af1a..d30766f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -141,7 +141,8 @@ int64_t opal_pci_fence_phb(uint64_t phb_id);
> int64_t opal_pci_reinit(uint64_t phb_id, uint64_t reinit_scope, uint64_t data);
> int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
> int64_t opal_set_slot_led_status(uint64_t phb_id, uint64_t slot_id, uint8_t led_type, uint8_t led_action);
> -int64_t opal_get_epow_status(__be64 *status);
> +int64_t opal_get_epow_status(uint16_t *status, uint16_t *length);
> +int64_t opal_get_dpo_status(int64_t *dpo_timeout);
> int64_t opal_set_system_attention_led(uint8_t led_action);
> int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
> __be16 *pci_error_type, __be16 *severity);
> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
> index ac46c2c..581bbd8 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 support for OPAL power-control, poweroff events
> *
> * Copyright 2015 IBM Corp.
> *
> @@ -9,18 +9,87 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#define pr_fmt(fmt) "OPAL-POWER: " fmt
> +
> #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
> +/* Get EPOW status */
> +static bool get_epow_status(void)
> +{
> + int i;
> + u16 num_classes;
> + __be16 epow_classes;
> + __be16 opal_epow_status[OPAL_SYSEPOW_MAX] = {0};
> +
> + /* Send kernel EPOW classes supported info to OPAL */
> + epow_classes = cpu_to_be16(OPAL_SYSEPOW_MAX);
> +
> + /* Get EPOW events information from OPAL */
> + opal_get_epow_status(opal_epow_status, &epow_classes);
> +
> + /* Look for EPOW events present */
> + num_classes = be16_to_cpu(epow_classes);
> + for (i = 0; i < num_classes; i++) {
> + if (be16_to_cpu(opal_epow_status[i]))
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/* Process existing EPOW, DPO events */
> +static void process_existing_poweroff_events(void)
> +{
> + int rc;
> + __be64 opal_dpo_timeout;
>
> + /* Check for DPO event */
> + rc = opal_get_dpo_status(&opal_dpo_timeout);
> + if (rc != OPAL_WRONG_STATE) {
> + pr_info("Existing DPO event detected. Powering off system\n");
> + goto poweroff;
> + }
> +
> + /* Check for EPOW event */
> + if (get_epow_status()) {
> + pr_info("Existing EPOW event detected. Powering off system");
> + goto poweroff;
> + }
> + return;
> +
> +poweroff:
> + orderly_poweroff(true);
> +}
> +
> +/* OPAL EPOW, DPO event notifier */
> +static int opal_epow_dpo_event(struct notifier_block *nb,
> + unsigned long msg_type, void *msg)
> +{
> + switch (msg_type) {
> + case OPAL_MSG_EPOW:
> + pr_info("EPOW msg received. Powering off system\n");
> + break;
> + case OPAL_MSG_DPO:
> + pr_info("DPO msg received. Powering off system\n");
> + break;
> + default:
> + pr_err("Unknown message type %lu\n", msg_type);
> + return 0;
> + }
> +
> + orderly_poweroff(true);
> + return 0;
> +}
> +
> +/* OPAL 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;
> @@ -29,20 +98,35 @@ static int opal_power_control_event(struct notifier_block *nb,
>
> switch (type) {
> case SOFT_REBOOT:
> - pr_info("OPAL: reboot requested\n");
> + pr_info("Reboot requested\n");
> orderly_reboot();
> break;
> case SOFT_OFF:
> - pr_info("OPAL: poweroff requested\n");
> + pr_info("Poweroff requested\n");
> orderly_poweroff(true);
> break;
> default:
> - pr_err("OPAL: power control type unexpected %016llx\n", type);
> + pr_err("Unknown power-control type %llu\n", type);
> }
>
> return 0;
> }
>
> +/* OPAL EPOW event notifier block */
> +static struct notifier_block opal_epow_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL DPO event notifier block */
> +static struct notifier_block opal_dpo_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL power-control event notifier block */
> static struct notifier_block opal_power_control_nb = {
> .notifier_call = opal_power_control_event,
> .next = NULL,
> @@ -51,16 +135,49 @@ static struct notifier_block opal_power_control_nb = {
>
> static int __init opal_power_control_init(void)
> {
> - int ret;
> + int ret, epow_dpo_supported = 0;
> + struct device_node *node_epow;
>
> + /* Register OPAL power-control events notifier */
> 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;
> + &opal_power_control_nb);
> + if (ret)
> + pr_err("Power-control events notifier registration "
> + "failed, ret = %d\n", ret);
> +
> + /* Determine EPOW, DPO support in hardware. */
> + node_epow = of_find_node_by_path("/ibm,opal/epow");
> + if (node_epow) {
> + epow_dpo_supported = of_device_is_compatible(node_epow,
> + "ibm,opal-v3-epow");
> + of_node_put(node_epow);
> }
>
> + if (epow_dpo_supported)
> + pr_info("OPAL EPOW, DPO support detected.\n");
> + else
> + return 0;
> +
> + /* 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);
> +
> + /* 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();
> +
> + pr_info("Poweroff events support initialized\n");
> +
> return 0;
> }
> +
> machine_subsys_initcall(powernv, opal_power_control_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);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-05-18 15:18 ` [PATCH v5] powerpc/powernv: Add poweroff " Vipin K Parashar
2015-05-19 13:15 ` trigg
2015-06-02 5:20 ` Vipin K Parashar
@ 2015-06-03 3:41 ` Vaibhav Jain
2015-06-03 5:13 ` [v5] " Michael Ellerman
3 siblings, 0 replies; 8+ messages in thread
From: Vaibhav Jain @ 2015-06-03 3:41 UTC (permalink / raw)
To: Vipin K Parashar; +Cc: linuxppc-dev, Stewart Smith, joel, Vaibhav Jain
Acked-by: Vaibhav Jain <mr.triggtrigg@gmail.com>
Cc: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
On Mon, May 18, 2015 at 8:48 PM, Vipin K Parashar
<vipin@linux.vnet.ibm.com> wrote:
> This patch adds support for FSP EPOW (Early Power Off Warning) and
> DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
> generated by SPCN/FSP due to various critical system conditions that
> need system shutdown. Few examples of these conditions are high
> ambient temperature or system running on UPS power with low UPS battery.
> DPO event is generated in response to admin initiated system request.
> Upon receipt of EPOW and DPO events host kernel invokes
> orderly_poweroff for performing graceful system shutdown. System admin
> can also add systemd service shutdown scripts to perform any specific
> actions like graceful guest shutdown upon system poweroff. libvirt-guests
> is systemd service available on recent distros for management of guests
> at system start/shutdown time.
>
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/opal-api.h | 44 ++++++++
> arch/powerpc/include/asm/opal.h | 3 +-
> arch/powerpc/platforms/powernv/opal-power.c | 147 ++++++++++++++++++++++---
> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
> 4 files changed, 179 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 0321a90..90fa364 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -355,6 +355,10 @@ enum opal_msg_type {
> OPAL_MSG_TYPE_MAX,
> };
>
> +/* OPAL_MSG_SHUTDOWN parameter values */
> +#define SOFT_OFF 0x00
> +#define SOFT_REBOOT 0x01
> +
> struct opal_msg {
> __be32 msg_type;
> __be32 reserved;
> @@ -730,6 +734,46 @@ struct opal_i2c_request {
> __be64 buffer_ra; /* Buffer real address */
> };
>
> +/*
> + * EPOW status sharing (OPAL and the host)
> + *
> + * The host will pass on OPAL, a buffer of length OPAL_SYSEPOW_MAX
> + * with individual elements being 16 bits wide to fetch the system
> + * wide EPOW status. Each element in the buffer will contain the
> + * EPOW status in it's bit representation for a particular EPOW sub
> + * class as defiend here. So multiple detailed EPOW status bits
> + * specific for any sub class can be represented in a single buffer
> + * element as it's bit representation.
> + */
> +
> +/* System EPOW type */
> +enum OpalSysEpow {
> + OPAL_SYSEPOW_POWER = 0, /* Power EPOW */
> + OPAL_SYSEPOW_TEMP = 1, /* Temperature EPOW */
> + OPAL_SYSEPOW_COOLING = 2, /* Cooling EPOW */
> + OPAL_SYSEPOW_MAX = 3, /* Max EPOW categories */
> +};
> +
> +/* Power EPOW */
> +enum OpalSysPower {
> + OPAL_SYSPOWER_UPS = 0x0001, /* System on UPS power */
> + OPAL_SYSPOWER_CHNG = 0x0002, /* System power config change */
> + OPAL_SYSPOWER_FAIL = 0x0004, /* System impending power failure */
> + OPAL_SYSPOWER_INCL = 0x0008, /* System incomplete power */
> +};
> +
> +/* Temperature EPOW */
> +enum OpalSysTemp {
> + OPAL_SYSTEMP_AMB = 0x0001, /* System over ambient temperature */
> + OPAL_SYSTEMP_INT = 0x0002, /* System over internal temperature */
> + OPAL_SYSTEMP_HMD = 0x0004, /* System over ambient humidity */
> +};
> +
> +/* Cooling EPOW */
> +enum OpalSysCooling {
> + OPAL_SYSCOOL_INSF = 0x0001, /* System insufficient cooling */
> +};
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 042af1a..d30766f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -141,7 +141,8 @@ int64_t opal_pci_fence_phb(uint64_t phb_id);
> int64_t opal_pci_reinit(uint64_t phb_id, uint64_t reinit_scope, uint64_t data);
> int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
> int64_t opal_set_slot_led_status(uint64_t phb_id, uint64_t slot_id, uint8_t led_type, uint8_t led_action);
> -int64_t opal_get_epow_status(__be64 *status);
> +int64_t opal_get_epow_status(uint16_t *status, uint16_t *length);
> +int64_t opal_get_dpo_status(int64_t *dpo_timeout);
> int64_t opal_set_system_attention_led(uint8_t led_action);
> int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
> __be16 *pci_error_type, __be16 *severity);
> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
> index ac46c2c..581bbd8 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 support for OPAL power-control, poweroff events
> *
> * Copyright 2015 IBM Corp.
> *
> @@ -9,18 +9,87 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#define pr_fmt(fmt) "OPAL-POWER: " fmt
> +
> #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
> +/* Get EPOW status */
> +static bool get_epow_status(void)
> +{
> + int i;
> + u16 num_classes;
> + __be16 epow_classes;
> + __be16 opal_epow_status[OPAL_SYSEPOW_MAX] = {0};
> +
> + /* Send kernel EPOW classes supported info to OPAL */
> + epow_classes = cpu_to_be16(OPAL_SYSEPOW_MAX);
> +
> + /* Get EPOW events information from OPAL */
> + opal_get_epow_status(opal_epow_status, &epow_classes);
> +
> + /* Look for EPOW events present */
> + num_classes = be16_to_cpu(epow_classes);
> + for (i = 0; i < num_classes; i++) {
> + if (be16_to_cpu(opal_epow_status[i]))
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/* Process existing EPOW, DPO events */
> +static void process_existing_poweroff_events(void)
> +{
> + int rc;
> + __be64 opal_dpo_timeout;
>
> + /* Check for DPO event */
> + rc = opal_get_dpo_status(&opal_dpo_timeout);
> + if (rc != OPAL_WRONG_STATE) {
> + pr_info("Existing DPO event detected. Powering off system\n");
> + goto poweroff;
> + }
> +
> + /* Check for EPOW event */
> + if (get_epow_status()) {
> + pr_info("Existing EPOW event detected. Powering off system");
> + goto poweroff;
> + }
> + return;
> +
> +poweroff:
> + orderly_poweroff(true);
> +}
> +
> +/* OPAL EPOW, DPO event notifier */
> +static int opal_epow_dpo_event(struct notifier_block *nb,
> + unsigned long msg_type, void *msg)
> +{
> + switch (msg_type) {
> + case OPAL_MSG_EPOW:
> + pr_info("EPOW msg received. Powering off system\n");
> + break;
> + case OPAL_MSG_DPO:
> + pr_info("DPO msg received. Powering off system\n");
> + break;
> + default:
> + pr_err("Unknown message type %lu\n", msg_type);
> + return 0;
> + }
> +
> + orderly_poweroff(true);
> + return 0;
> +}
> +
> +/* OPAL 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;
> @@ -29,20 +98,35 @@ static int opal_power_control_event(struct notifier_block *nb,
>
> switch (type) {
> case SOFT_REBOOT:
> - pr_info("OPAL: reboot requested\n");
> + pr_info("Reboot requested\n");
> orderly_reboot();
> break;
> case SOFT_OFF:
> - pr_info("OPAL: poweroff requested\n");
> + pr_info("Poweroff requested\n");
> orderly_poweroff(true);
> break;
> default:
> - pr_err("OPAL: power control type unexpected %016llx\n", type);
> + pr_err("Unknown power-control type %llu\n", type);
> }
>
> return 0;
> }
>
> +/* OPAL EPOW event notifier block */
> +static struct notifier_block opal_epow_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL DPO event notifier block */
> +static struct notifier_block opal_dpo_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL power-control event notifier block */
> static struct notifier_block opal_power_control_nb = {
> .notifier_call = opal_power_control_event,
> .next = NULL,
> @@ -51,16 +135,49 @@ static struct notifier_block opal_power_control_nb = {
>
> static int __init opal_power_control_init(void)
> {
> - int ret;
> + int ret, epow_dpo_supported = 0;
> + struct device_node *node_epow;
>
> + /* Register OPAL power-control events notifier */
> 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;
> + &opal_power_control_nb);
> + if (ret)
> + pr_err("Power-control events notifier registration "
> + "failed, ret = %d\n", ret);
> +
> + /* Determine EPOW, DPO support in hardware. */
> + node_epow = of_find_node_by_path("/ibm,opal/epow");
> + if (node_epow) {
> + epow_dpo_supported = of_device_is_compatible(node_epow,
> + "ibm,opal-v3-epow");
> + of_node_put(node_epow);
> }
>
> + if (epow_dpo_supported)
> + pr_info("OPAL EPOW, DPO support detected.\n");
> + else
> + return 0;
> +
> + /* 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);
> +
> + /* 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();
> +
> + pr_info("Poweroff events support initialized\n");
> +
> return 0;
> }
> +
> machine_subsys_initcall(powernv, opal_power_control_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
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-05-18 15:18 ` [PATCH v5] powerpc/powernv: Add poweroff " Vipin K Parashar
` (2 preceding siblings ...)
2015-06-03 3:41 ` Vaibhav Jain
@ 2015-06-03 5:13 ` Michael Ellerman
2015-06-03 19:12 ` Vipin K Parashar
3 siblings, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2015-06-03 5:13 UTC (permalink / raw)
To: Vipin K Parashar, linuxppc-dev; +Cc: stewart, Vipin K Parashar, joel
On Mon, 2015-18-05 at 15:18:04 UTC, Vipin K Parashar wrote:
> This patch adds support for FSP EPOW (Early Power Off Warning) and
Please spell out the acronyms the first time you use them, including FSP.
> DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
^
the
> generated by SPCN/FSP due to various critical system conditions that
SPCN?
> need system shutdown. Few examples of these conditions are high
^
s/need/require/ ? A few
> ambient temperature or system running on UPS power with low UPS battery.
> DPO event is generated in response to admin initiated system request.
Blank line between paragraphs please.
> Upon receipt of EPOW and DPO events host kernel invokes
^
the host kernel
> orderly_poweroff for performing graceful system shutdown. System admin
I like it if you spell functions with a trailing () to make it clear they are
functions, so this would be "orderly_powerof()".
> can also add systemd service shutdown scripts to perform any specific
> actions like graceful guest shutdown upon system poweroff. libvirt-guests
> is systemd service available on recent distros for management of guests
> at system start/shutdown time.
This last part about the scripts is not relevant to the kernel patch so just
leave it out please.
>
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/opal-api.h | 44 ++++++++
> arch/powerpc/include/asm/opal.h | 3 +-
> arch/powerpc/platforms/powernv/opal-power.c | 147 ++++++++++++++++++++++---
> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
> 4 files changed, 179 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 0321a90..90fa364 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -355,6 +355,10 @@ enum opal_msg_type {
> OPAL_MSG_TYPE_MAX,
> };
>
> +/* OPAL_MSG_SHUTDOWN parameter values */
> +#define SOFT_OFF 0x00
> +#define SOFT_REBOOT 0x01
I don't see this in the skiboot version of opal-api.h ?
They should be kept in sync.
If it's a Linux only define it should go in opal.h
> struct opal_msg {
> __be32 msg_type;
> __be32 reserved;
> @@ -730,6 +734,46 @@ struct opal_i2c_request {
> __be64 buffer_ra; /* Buffer real address */
> };
>
> +/*
> + * EPOW status sharing (OPAL and the host)
> + *
> + * The host will pass on OPAL, a buffer of length OPAL_SYSEPOW_MAX
> + * with individual elements being 16 bits wide to fetch the system
> + * wide EPOW status. Each element in the buffer will contain the
> + * EPOW status in it's bit representation for a particular EPOW sub
> + * class as defiend here. So multiple detailed EPOW status bits
> + * specific for any sub class can be represented in a single buffer
> + * element as it's bit representation.
> + */
> +
> +/* System EPOW type */
> +enum OpalSysEpow {
> + OPAL_SYSEPOW_POWER = 0, /* Power EPOW */
> + OPAL_SYSEPOW_TEMP = 1, /* Temperature EPOW */
> + OPAL_SYSEPOW_COOLING = 2, /* Cooling EPOW */
> + OPAL_SYSEPOW_MAX = 3, /* Max EPOW categories */
> +};
> +
> +/* Power EPOW */
> +enum OpalSysPower {
> + OPAL_SYSPOWER_UPS = 0x0001, /* System on UPS power */
> + OPAL_SYSPOWER_CHNG = 0x0002, /* System power config change */
> + OPAL_SYSPOWER_FAIL = 0x0004, /* System impending power failure */
> + OPAL_SYSPOWER_INCL = 0x0008, /* System incomplete power */
> +};
> +
> +/* Temperature EPOW */
> +enum OpalSysTemp {
> + OPAL_SYSTEMP_AMB = 0x0001, /* System over ambient temperature */
> + OPAL_SYSTEMP_INT = 0x0002, /* System over internal temperature */
> + OPAL_SYSTEMP_HMD = 0x0004, /* System over ambient humidity */
> +};
> +
> +/* Cooling EPOW */
> +enum OpalSysCooling {
> + OPAL_SYSCOOL_INSF = 0x0001, /* System insufficient cooling */
> +};
I don't see the last three of these enums used at all, so please drop them.
> #endif /* __ASSEMBLY__ */
>
> #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 042af1a..d30766f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -141,7 +141,8 @@ int64_t opal_pci_fence_phb(uint64_t phb_id);
> int64_t opal_pci_reinit(uint64_t phb_id, uint64_t reinit_scope, uint64_t data);
> int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
> int64_t opal_set_slot_led_status(uint64_t phb_id, uint64_t slot_id, uint8_t led_type, uint8_t led_action);
> -int64_t opal_get_epow_status(__be64 *status);
> +int64_t opal_get_epow_status(uint16_t *status, uint16_t *length);
Has the signature of this function really changed or was it just wrong before?
If it's changed how do we know we're running on a version of OPAL that supports
the two argument version?
The parameter names don't seem very clear either. status is actually a pointer
to an array of "statuses", and length is the number of entries in that array.
Also you removed the endian annotations but then you pass it __be16 values, so
that looks incorrect, you should be using __be16 here.
> +int64_t opal_get_dpo_status(int64_t *dpo_timeout);
Similarly this should be __be64 AFAICS.
> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
> index ac46c2c..581bbd8 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 support for OPAL power-control, poweroff events
> *
> * Copyright 2015 IBM Corp.
> *
> @@ -9,18 +9,87 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#define pr_fmt(fmt) "OPAL-POWER: " fmt
Please don't shout, "opal-power" is fine.
> #include <linux/kernel.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
Don't think you need those?
> #include <linux/reboot.h>
> -#include <linux/notifier.h>
> -
I think you DO need notifier.h.
> +#include <linux/of.h>
> #include <asm/opal.h>
> #include <asm/machdep.h>
>
> -#define SOFT_OFF 0x00
> -#define SOFT_REBOOT 0x01
> +/* Get EPOW status */
> +static bool get_epow_status(void)
This is not a great name, "get" implies it gives you something back, but it
doesn't it just tells you true or false.
So maybe epow_event_pending() ?
> +{
> + int i;
> + u16 num_classes;
> + __be16 epow_classes;
I think this would be cleaner if you just had a single num_classes and you
endian swap in the one place you use it.
> + __be16 opal_epow_status[OPAL_SYSEPOW_MAX] = {0};
> +
> + /* Send kernel EPOW classes supported info to OPAL */
> + epow_classes = cpu_to_be16(OPAL_SYSEPOW_MAX);
> +
> + /* Get EPOW events information from OPAL */
> + opal_get_epow_status(opal_epow_status, &epow_classes);
This could fail.
> +
> + /* Look for EPOW events present */
> + num_classes = be16_to_cpu(epow_classes);
> + for (i = 0; i < num_classes; i++) {
> + if (be16_to_cpu(opal_epow_status[i]))
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/* Process existing EPOW, DPO events */
> +static void process_existing_poweroff_events(void)
> +{
> + int rc;
> + __be64 opal_dpo_timeout;
>
> + /* Check for DPO event */
> + rc = opal_get_dpo_status(&opal_dpo_timeout);
> + if (rc != OPAL_WRONG_STATE) {
> + pr_info("Existing DPO event detected. Powering off system\n");
> + goto poweroff;
> + }
> +
> + /* Check for EPOW event */
> + if (get_epow_status()) {
> + pr_info("Existing EPOW event detected. Powering off system");
> + goto poweroff;
> + }
> + return;
> +
> +poweroff:
> + orderly_poweroff(true);
I don't like that much, you shouldn't need to use goto for such simple logic.
Can you create a single function, maybe called event_pending(), and have it
check both EPOW and DPO and return a bool if there's any kind of event pending.
Then this can just become:
if (event_pending())
orderly_poweroff(true);
> +}
> +
> +/* OPAL EPOW, DPO event notifier */
> +static int opal_epow_dpo_event(struct notifier_block *nb,
> + unsigned long msg_type, void *msg)
> +{
> + switch (msg_type) {
> + case OPAL_MSG_EPOW:
> + pr_info("EPOW msg received. Powering off system\n");
> + break;
> + case OPAL_MSG_DPO:
> + pr_info("DPO msg received. Powering off system\n");
> + break;
> + default:
> + pr_err("Unknown message type %lu\n", msg_type);
> + return 0;
> + }
> +
> + orderly_poweroff(true);
> + return 0;
> +}
Why do we need a separate notifier function? Can't this just be folded into
opal_power_control_event() ?
> +
> +/* OPAL 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;
> @@ -29,20 +98,35 @@ static int opal_power_control_event(struct notifier_block *nb,
>
> switch (type) {
> case SOFT_REBOOT:
> - pr_info("OPAL: reboot requested\n");
> + pr_info("Reboot requested\n");
> orderly_reboot();
> break;
> case SOFT_OFF:
> - pr_info("OPAL: poweroff requested\n");
> + pr_info("Poweroff requested\n");
> orderly_poweroff(true);
> break;
> default:
> - pr_err("OPAL: power control type unexpected %016llx\n", type);
> + pr_err("Unknown power-control type %llu\n", type);
> }
>
> return 0;
> }
>
> +/* OPAL EPOW event notifier block */
> +static struct notifier_block opal_epow_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL DPO event notifier block */
> +static struct notifier_block opal_dpo_nb = {
> + .notifier_call = opal_epow_dpo_event,
> + .next = NULL,
> + .priority = 0,
> +};
> +
> +/* OPAL power-control event notifier block */
> static struct notifier_block opal_power_control_nb = {
> .notifier_call = opal_power_control_event,
> .next = NULL,
> @@ -51,16 +135,49 @@ static struct notifier_block opal_power_control_nb = {
>
> static int __init opal_power_control_init(void)
> {
> - int ret;
> + int ret, epow_dpo_supported = 0;
Can you make that a bool and call it "supported".
> + struct device_node *node_epow;
It's typical to just call it "np".
>
> + /* Register OPAL power-control events notifier */
> 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;
> + &opal_power_control_nb);
> + if (ret)
> + pr_err("Power-control events notifier registration "
> + "failed, ret = %d\n", ret);
Please don't split the string, and similarly below.
> +
> + /* Determine EPOW, DPO support in hardware. */
> + node_epow = of_find_node_by_path("/ibm,opal/epow");
> + if (node_epow) {
> + epow_dpo_supported = of_device_is_compatible(node_epow,
> + "ibm,opal-v3-epow");
> + of_node_put(node_epow);
> }
>
> + if (epow_dpo_supported)
> + pr_info("OPAL EPOW, DPO support detected.\n");
> + else
> + return 0;
Clearer as:
if (!supported)
return 0;
pr_info("OPAL EPOW, DPO support detected.\n");
> +
> + /* 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);
> +
> + /* 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();
> +
> + pr_info("Poweroff events support initialized\n");
> +
> return 0;
> }
> +
No extra blank line thanks.
> machine_subsys_initcall(powernv, opal_power_control_init);
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [v5] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform
2015-06-03 5:13 ` [v5] " Michael Ellerman
@ 2015-06-03 19:12 ` Vipin K Parashar
0 siblings, 0 replies; 8+ messages in thread
From: Vipin K Parashar @ 2015-06-03 19:12 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: stewart, joel
Hi Michael,
Thanks for review. Responses below
On 06/03/2015 10:43 AM, Michael Ellerman wrote:
> On Mon, 2015-18-05 at 15:18:04 UTC, Vipin K Parashar wrote:
>> This patch adds support for FSP EPOW (Early Power Off Warning) and
> Please spell out the acronyms the first time you use them, including FSP.
Will do.
>
>> DPO (Delayed Power Off) events for PowerNV platform. EPOW events are
> ^
> the
the PowerNV platform. Will edit.
>
>> generated by SPCN/FSP due to various critical system conditions that
> SPCN?
Will remove SPCN. FSP should be sufficient.
>
>> need system shutdown. Few examples of these conditions are high
> ^
> s/need/require/ ? A few
Agreed.
>
>> ambient temperature or system running on UPS power with low UPS battery.
>> DPO event is generated in response to admin initiated system request.
> Blank line between paragraphs please.
Sure
>
>> Upon receipt of EPOW and DPO events host kernel invokes
> ^
> the host kernel
will edit
>> orderly_poweroff for performing graceful system shutdown. System admin
> I like it if you spell functions with a trailing () to make it clear they are
> functions, so this would be "orderly_powerof()".
Agreed.
>
>> can also add systemd service shutdown scripts to perform any specific
>> actions like graceful guest shutdown upon system poweroff. libvirt-guests
>> is systemd service available on recent distros for management of guests
>> at system start/shutdown time.
> This last part about the scripts is not relevant to the kernel patch so just
> leave it out please.
Agreed.
>
>> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/opal-api.h | 44 ++++++++
>> arch/powerpc/include/asm/opal.h | 3 +-
>> arch/powerpc/platforms/powernv/opal-power.c | 147 ++++++++++++++++++++++---
>> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
>> 4 files changed, 179 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
>> index 0321a90..90fa364 100644
>> --- a/arch/powerpc/include/asm/opal-api.h
>> +++ b/arch/powerpc/include/asm/opal-api.h
>> @@ -355,6 +355,10 @@ enum opal_msg_type {
>> OPAL_MSG_TYPE_MAX,
>> };
>>
>> +/* OPAL_MSG_SHUTDOWN parameter values */
>> +#define SOFT_OFF 0x00
>> +#define SOFT_REBOOT 0x01
> I don't see this in the skiboot version of opal-api.h ?
>
> They should be kept in sync.
>
> If it's a Linux only define it should go in opal.h
Agreed. Won't add these definitions to opal-api.h as its not present in
skiboot version of opal-api.h.
>> struct opal_msg {
>> __be32 msg_type;
>> __be32 reserved;
>> @@ -730,6 +734,46 @@ struct opal_i2c_request {
>> __be64 buffer_ra; /* Buffer real address */
>> };
>>
>> +/*
>> + * EPOW status sharing (OPAL and the host)
>> + *
>> + * The host will pass on OPAL, a buffer of length OPAL_SYSEPOW_MAX
>> + * with individual elements being 16 bits wide to fetch the system
>> + * wide EPOW status. Each element in the buffer will contain the
>> + * EPOW status in it's bit representation for a particular EPOW sub
>> + * class as defiend here. So multiple detailed EPOW status bits
>> + * specific for any sub class can be represented in a single buffer
>> + * element as it's bit representation.
>> + */
>> +
>> +/* System EPOW type */
>> +enum OpalSysEpow {
>> + OPAL_SYSEPOW_POWER = 0, /* Power EPOW */
>> + OPAL_SYSEPOW_TEMP = 1, /* Temperature EPOW */
>> + OPAL_SYSEPOW_COOLING = 2, /* Cooling EPOW */
>> + OPAL_SYSEPOW_MAX = 3, /* Max EPOW categories */
>> +};
>> +
>> +/* Power EPOW */
>> +enum OpalSysPower {
>> + OPAL_SYSPOWER_UPS = 0x0001, /* System on UPS power */
>> + OPAL_SYSPOWER_CHNG = 0x0002, /* System power config change */
>> + OPAL_SYSPOWER_FAIL = 0x0004, /* System impending power failure */
>> + OPAL_SYSPOWER_INCL = 0x0008, /* System incomplete power */
>> +};
>> +
>> +/* Temperature EPOW */
>> +enum OpalSysTemp {
>> + OPAL_SYSTEMP_AMB = 0x0001, /* System over ambient temperature */
>> + OPAL_SYSTEMP_INT = 0x0002, /* System over internal temperature */
>> + OPAL_SYSTEMP_HMD = 0x0004, /* System over ambient humidity */
>> +};
>> +
>> +/* Cooling EPOW */
>> +enum OpalSysCooling {
>> + OPAL_SYSCOOL_INSF = 0x0001, /* System insufficient cooling */
>> +};
> I don't see the last three of these enums used at all, so please drop them.
OPAL_SYSPOWER_CHNG / FAIL / INCL, OPAL_SYSTEMP_HMD and OPAL_SYSCOOL_INSF
enums aren't used here but they are part of skiboot version of
opal-api.h and
thus need to be retained.
PKVM2.1 uses these enums and thus can't be removed from
skiboot opal-api.h
>
>> #endif /* __ASSEMBLY__ */
>>
>> #endif /* __OPAL_API_H */
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 042af1a..d30766f 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -141,7 +141,8 @@ int64_t opal_pci_fence_phb(uint64_t phb_id);
>> int64_t opal_pci_reinit(uint64_t phb_id, uint64_t reinit_scope, uint64_t data);
>> int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
>> int64_t opal_set_slot_led_status(uint64_t phb_id, uint64_t slot_id, uint8_t led_type, uint8_t led_action);
>> -int64_t opal_get_epow_status(__be64 *status);
>> +int64_t opal_get_epow_status(uint16_t *status, uint16_t *length);
> Has the signature of this function really changed or was it just wrong before?
>
> If it's changed how do we know we're running on a version of OPAL that supports
> the two argument version?
__be64 * status is wrong here. OPAL expects two arguments of type __be16 *
and has been unchanged.
>
> The parameter names don't seem very clear either. status is actually a pointer
> to an array of "statuses", and length is the number of entries in that array.
>
> Also you removed the endian annotations but then you pass it __be16 values, so
> that looks incorrect, you should be using __be16 here.
Will correct endian notation and use below names for epow, dpo api
int64_t opal_get_epow_status(__be16 *epow_status, __be16 *num_epow_classes);
int64_t opal_get_dpo_status(__be64 *dpo_timeout);
>
>> +int64_t opal_get_dpo_status(int64_t *dpo_timeout);
> Similarly this should be __be64 AFAICS.
Agreed. Changed api above.
>
>> diff --git a/arch/powerpc/platforms/powernv/opal-power.c b/arch/powerpc/platforms/powernv/opal-power.c
>> index ac46c2c..581bbd8 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 support for OPAL power-control, poweroff events
>> *
>> * Copyright 2015 IBM Corp.
>> *
>> @@ -9,18 +9,87 @@
>> * 2 of the License, or (at your option) any later version.
>> */
>>
>> +#define pr_fmt(fmt) "OPAL-POWER: " fmt
> Please don't shout, "opal-power" is fine.
Will keep in small letters.
>
>> #include <linux/kernel.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/timer.h>
> Don't think you need those?
yes, not needed now.
>
>> #include <linux/reboot.h>
>> -#include <linux/notifier.h>
>> -
> I think you DO need notifier.h.
Agreed. Won't remove.
>
>> +#include <linux/of.h>
>> #include <asm/opal.h>
>> #include <asm/machdep.h>
>>
>> -#define SOFT_OFF 0x00
>> -#define SOFT_REBOOT 0x01
>> +/* Get EPOW status */
>> +static bool get_epow_status(void)
> This is not a great name, "get" implies it gives you something back, but it
> doesn't it just tells you true or false.
>
> So maybe epow_event_pending() ?
Agreed. Will use better naming.
>
>> +{
>> + int i;
>> + u16 num_classes;
>> + __be16 epow_classes;
> I think this would be cleaner if you just had a single num_classes and you
> endian swap in the one place you use it.
Agreed.
>
>> + __be16 opal_epow_status[OPAL_SYSEPOW_MAX] = {0};
>> +
>> + /* Send kernel EPOW classes supported info to OPAL */
>> + epow_classes = cpu_to_be16(OPAL_SYSEPOW_MAX);
>> +
>> + /* Get EPOW events information from OPAL */
>> + opal_get_epow_status(opal_epow_status, &epow_classes);
> This could fail.
Will add a check for API failure.
Though skiboot implementation doesn't have a fail case for this.
>
>> +
>> + /* Look for EPOW events present */
>> + num_classes = be16_to_cpu(epow_classes);
>> + for (i = 0; i < num_classes; i++) {
>> + if (be16_to_cpu(opal_epow_status[i]))
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +/* Process existing EPOW, DPO events */
>> +static void process_existing_poweroff_events(void)
>> +{
>> + int rc;
>> + __be64 opal_dpo_timeout;
>>
>> + /* Check for DPO event */
>> + rc = opal_get_dpo_status(&opal_dpo_timeout);
>> + if (rc != OPAL_WRONG_STATE) {
>> + pr_info("Existing DPO event detected. Powering off system\n");
>> + goto poweroff;
>> + }
>> +
>> + /* Check for EPOW event */
>> + if (get_epow_status()) {
>> + pr_info("Existing EPOW event detected. Powering off system");
>> + goto poweroff;
>> + }
>> + return;
>> +
>> +poweroff:
>> + orderly_poweroff(true);
> I don't like that much, you shouldn't need to use goto for such simple logic.
>
> Can you create a single function, maybe called event_pending(), and have it
> check both EPOW and DPO and return a bool if there's any kind of event pending.
>
> Then this can just become:
>
> if (event_pending())
> orderly_poweroff(true);
Agreed. Will reorganize this code.
>
>> +}
>> +
>> +/* OPAL EPOW, DPO event notifier */
>> +static int opal_epow_dpo_event(struct notifier_block *nb,
>> + unsigned long msg_type, void *msg)
>> +{
>> + switch (msg_type) {
>> + case OPAL_MSG_EPOW:
>> + pr_info("EPOW msg received. Powering off system\n");
>> + break;
>> + case OPAL_MSG_DPO:
>> + pr_info("DPO msg received. Powering off system\n");
>> + break;
>> + default:
>> + pr_err("Unknown message type %lu\n", msg_type);
>> + return 0;
>> + }
>> +
>> + orderly_poweroff(true);
>> + return 0;
>> +}
> Why do we need a separate notifier function? Can't this just be folded into
> opal_power_control_event() ?
Will merge all of them into opal_power_control_event.
>
>> +
>> +/* OPAL 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;
>> @@ -29,20 +98,35 @@ static int opal_power_control_event(struct notifier_block *nb,
>>
>> switch (type) {
>> case SOFT_REBOOT:
>> - pr_info("OPAL: reboot requested\n");
>> + pr_info("Reboot requested\n");
>> orderly_reboot();
>> break;
>> case SOFT_OFF:
>> - pr_info("OPAL: poweroff requested\n");
>> + pr_info("Poweroff requested\n");
>> orderly_poweroff(true);
>> break;
>> default:
>> - pr_err("OPAL: power control type unexpected %016llx\n", type);
>> + pr_err("Unknown power-control type %llu\n", type);
>> }
>>
>> return 0;
>> }
>>
>> +/* OPAL EPOW event notifier block */
>> +static struct notifier_block opal_epow_nb = {
>> + .notifier_call = opal_epow_dpo_event,
>> + .next = NULL,
>> + .priority = 0,
>> +};
>> +
>> +/* OPAL DPO event notifier block */
>> +static struct notifier_block opal_dpo_nb = {
>> + .notifier_call = opal_epow_dpo_event,
>> + .next = NULL,
>> + .priority = 0,
>> +};
>> +
>> +/* OPAL power-control event notifier block */
>> static struct notifier_block opal_power_control_nb = {
>> .notifier_call = opal_power_control_event,
>> .next = NULL,
>> @@ -51,16 +135,49 @@ static struct notifier_block opal_power_control_nb = {
>>
>> static int __init opal_power_control_init(void)
>> {
>> - int ret;
>> + int ret, epow_dpo_supported = 0;
> Can you make that a bool and call it "supported".
Will change name to supported. Pasted reorganized code below.
>
>> + struct device_node *node_epow;
> It's typical to just call it "np".
Agreed. will use "np"
>
>>
>> + /* Register OPAL power-control events notifier */
>> 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;
>> + &opal_power_control_nb);
>> + if (ret)
>> + pr_err("Power-control events notifier registration "
>> + "failed, ret = %d\n", ret);
> Please don't split the string, and similarly below.
sure
>
>> +
>> + /* Determine EPOW, DPO support in hardware. */
>> + node_epow = of_find_node_by_path("/ibm,opal/epow");
>> + if (node_epow) {
>> + epow_dpo_supported = of_device_is_compatible(node_epow,
>> + "ibm,opal-v3-epow");
>> + of_node_put(node_epow);
>> }
>>
>> + if (epow_dpo_supported)
>> + pr_info("OPAL EPOW, DPO support detected.\n");
>> + else
>> + return 0;
> Clearer as:
>
> if (!supported)
> return 0;
>
> pr_info("OPAL EPOW, DPO support detected.\n");
Reorganized code as below
/* Determine OPAL EPOW, DPO support */
np = of_find_node_by_path("/ibm,opal/epow");
if (np) {
int supported;
supported = of_device_is_compatible(np,
"ibm,opal-v3-epow");
of_node_put(np);
if (!supported)
return 0;
pr_info("OPAL EPOW, DPO support detected.\n");
}
>
>> +
>> + /* 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);
>> +
>> + /* 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();
>> +
>> + pr_info("Poweroff events support initialized\n");
>> +
>> return 0;
>> }
>> +
> No extra blank line thanks.
sure
>
>> machine_subsys_initcall(powernv, opal_power_control_init);
>
> cheers
>
^ permalink raw reply [flat|nested] 8+ messages in thread