From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e28smtp06.in.ibm.com (e28smtp06.in.ibm.com [122.248.162.6]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id A79111A0008 for ; Fri, 15 May 2015 20:56:09 +1000 (AEST) Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 15 May 2015 16:26:06 +0530 Received: from d28relay05.in.ibm.com (d28relay05.in.ibm.com [9.184.220.62]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 372653940048 for ; Fri, 15 May 2015 16:26:02 +0530 (IST) Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay05.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t4FAu0IW44695734 for ; Fri, 15 May 2015 16:26:01 +0530 Received: from d28av05.in.ibm.com (localhost [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t4FAu0wc024412 for ; Fri, 15 May 2015 16:26:00 +0530 Message-ID: <5555D0BE.2090906@linux.vnet.ibm.com> Date: Fri, 15 May 2015 16:25:58 +0530 From: Vipin K Parashar MIME-Version: 1.0 To: trigg Subject: Re: [PATCH v3] powerpc/powernv: Add poweroff (EPOW, DPO) events support for PowerNV platform References: <1431600419-6544-1-git-send-email-vipin@linux.vnet.ibm.com> <1431600419-6544-2-git-send-email-vipin@linux.vnet.ibm.com> <3BA04CFA-6498-4154-B784-5C22E4B9F747@gmail.com> In-Reply-To: <3BA04CFA-6498-4154-B784-5C22E4B9F747@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Cc: "stewart@linux.vnet.ibm.com" , "linuxppc-dev@lists.ozlabs.org" , "joel@jms.id.au" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Thanks for review. Made changes as suggested. On 05/14/2015 08:51 PM, trigg wrote: > >> On 14-May-2015, at 16:16, Vipin K Parashar wrote: >> >> This patch adds support for FSP EPOW (Early Power Off Warning) and >> DPO (Delayed Power Off) events support 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 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 >> --- >> arch/powerpc/include/asm/opal-api.h | 44 +++++++ >> arch/powerpc/include/asm/opal.h | 3 +- >> arch/powerpc/platforms/powernv/opal-power.c | 167 ++++++++++++++++++++++--- >> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 + >> 4 files changed, 197 insertions(+), 18 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..0a1e07b 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,40 +9,137 @@ >> * 2 of the License, or (at your option) any later version. >> */ >> >> +#define pr_fmt(fmt) "OPAL-POWER: " fmt >> + >> #include >> +#include >> +#include >> #include >> -#include >> - >> +#include >> #include >> #include >> >> -#define SOFT_OFF 0x00 >> -#define SOFT_REBOOT 0x01 >> +/* System EPOW, DPO support variable */ >> +static bool epow_dpo_supported; > Make this local to init function. >> + >> +/* System EPOW status */ >> +static u16 epow_status[OPAL_SYSEPOW_MAX]; >> +static u16 num_epow_classes; >> + >> +/* Get DPO status */ >> +static bool get_dpo_status(void) >> +{ >> + int rc; >> + __be64 opal_dpo_timeout; >> + >> + rc = opal_get_dpo_status(&opal_dpo_timeout); >> + if (rc == OPAL_WRONG_STATE) >> + return false; >> + >> + return true; >> +} >> + >> +/* Get EPOW status */ >> +static bool get_epow_status(void) >> +{ >> + int i; >> + >> + __be16 opal_epow_status[OPAL_SYSEPOW_MAX] = {0}; >> + __be16 epow_classes; >> + >> + /* 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); >> >> + /* Copy EPOW status obtained from OPAL */ >> + memset(epow_status, 0, sizeof(epow_status)); >> + num_epow_classes = be16_to_cpu(epow_classes); >> + for (i = 0; i < num_epow_classes; i++) { >> + epow_status[i] = be16_to_cpu(opal_epow_status[i]); >> + if (epow_status[i]) >> + return true; > Bug? You may want to set a flag here instead of returning. >> + } >> + >> + return false; >> +} >> + >> +/* Process existing EPOW, DPO events */ >> +static void process_existing_poweroff_events(void) >> +{ >> + >> + /* Check for any existing DPO, EPOW event */ >> + if (epow_dpo_supported) { >> + > Instead of checking for dpo support here just call this function > From init only when dpo is supported. >> + /* Check for DPO event first */ >> + if (get_dpo_status()) { > get_dpo_status function is very small and its functionality > can be incorporated here itself. >> + pr_info("Existing DPO event detected. " >> + "Powering off system\n"); >> + orderly_poweroff(true); >> + return; >> + } >> + >> + /* EPOW event check */ >> + if (get_epow_status()) { >> + pr_info("Existing EPOW event detected. " >> + "Powering off system"); >> + orderly_poweroff(true); >> + return; >> + } >> + } >> +} >> + >> +/* 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"); >> + orderly_poweroff(true); >> + break; >> + >> + case OPAL_MSG_DPO: >> + pr_info("DPO msg received. Powering off system\n"); >> + orderly_poweroff(true); > Missing a 'break' here. Also better if you have some warning > or BUG() for default case. >> + } >> + >> + 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; >> + 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"); >> 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 event %llu\n", type); >> } >> >> return 0; >> } >> >> +/* OPAL EPOW, DPO event notifier block */ >> +static struct notifier_block opal_epow_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, >> @@ -52,15 +149,51 @@ static struct notifier_block opal_power_control_nb = { >> static int __init opal_power_control_init(void) >> { >> 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. >> + */ > Single line comment using multi line syntax. >> + node_epow = of_find_node_by_path("/ibm,opal/epow"); >> + if (node_epow) { >> + if (of_device_is_compatible(node_epow, "ibm,opal-v3-epow")) { >> + epow_dpo_supported = true; >> + pr_info("OPAL EPOW, DPO support detected.\n"); >> + } >> + of_node_put(node_epow); >> + } >> + >> + /* Prepare to handle EPOW, DPO events */ >> + if (epow_dpo_supported) { >> + >> + /* Register EPOW event notifier */ >> + ret = opal_message_notifier_register(OPAL_MSG_EPOW, >> + &opal_epow_dpo_nb); >> + if (ret) >> + pr_err("EPOW event notifier registration failed, " >> + "ret = %d\n", ret); >> + > This is a bug. You can't use same struct notifierblock to register to > two different chains. >> + /* Register DPO event notifier */ >> + ret = opal_message_notifier_register(OPAL_MSG_DPO, >> + &opal_epow_dpo_nb); >> + if (ret) >> + pr_err("DPO event notifier registration failed, " >> + "ret = %d\n", ret); >> } >> >> + /* Register OPAL power-control events notifier */ >> + ret = opal_message_notifier_register(OPAL_MSG_SHUTDOWN, >> + &opal_power_control_nb); >> + if (ret) >> + pr_err("Power-control events notifier registration " >> + "failed, ret = %d\n", ret); >> + >> + /* Check for any existing EPOW or DPO events. */ >> + process_existing_poweroff_events(); >> + >> + pr_info("Power-control, 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