From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752838AbeDJLKy (ORCPT ); Tue, 10 Apr 2018 07:10:54 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:36720 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751774AbeDJLKv (ORCPT ); Tue, 10 Apr 2018 07:10:51 -0400 Date: Tue, 10 Apr 2018 12:10:43 +0100 From: Mark Rutland To: Ulf Hansson Cc: "Rafael J . Wysocki" , Sudeep Holla , Lorenzo Pieralisi , Linux PM , Kevin Hilman , Lina Iyer , Lina Iyer , Rob Herring , Daniel Lezcano , Thomas Gleixner , Vincent Guittot , Stephen Boyd , Juri Lelli , Geert Uytterhoeven , Linux ARM , linux-arm-msm@vger.kernel.org, Linux Kernel Mailing List Subject: Re: [PATCH v6 20/25] drivers: firmware: psci: Introduce psci_dt_topology_init() Message-ID: <20180410111043.t577a43a3bvjpkc7@lakrids.cambridge.arm.com> References: <1521046715-30683-1-git-send-email-ulf.hansson@linaro.org> <1521046715-30683-21-git-send-email-ulf.hansson@linaro.org> <20180314173833.dyu7jtweu233rfi4@lakrids.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 10, 2018 at 09:19:26AM +0200, Ulf Hansson wrote: > On 14 March 2018 at 18:38, Mark Rutland wrote: > > On Wed, Mar 14, 2018 at 05:58:30PM +0100, Ulf Hansson wrote: > >> In case the hierarchical layout is used in DT, we want to initialize the > >> corresponding PM domain topology for the CPUs, by using the generic PM > >> domain (aka genpd) infrastructure. > >> > >> At first glance, it may seem feasible to hook into the existing > >> psci_dt_init() function, although because it's called quite early in the > >> boot sequence, allocating the dynamic data structure for a genpd doesn't > >> work. > >> > >> Therefore, let's export a new init function for PSCI, > >> psci_dt_topology_init(), which the ARM machine code should call from a > >> suitable initcall. > >> > >> Succeeding to initialize the PM domain topology, which means at least one > >> instance of a genpd becomes created, allows us to continue to enable the > >> PSCI OS initiated mode for the platform. If everything turns out fine, > >> let's print a message in log to inform the user about the changed mode. > >> > >> In case of any failures, we stick to the default PSCI Platform Coordinated > >> mode. > > > > For kexec/kdump we'll need to explicitly set the suspend mode to > > platform coordinated if for whatever reason we choose not to use OSI. > > Could you please elaborate on this? I am not really understanding what > you are suggesting me to do and why. Sorry for not being clear. What I'd like to see is that we always call SET_SUSPEND_MODE before invoking CPU_SUSPEND. Either deciding early if we can use OSI, or always setting it to platform co-ordinated early on in boot and later switching it over. That way we can be sure of the suspend mode, even if we've kexec'd from a kernel that had fiddled with it. > >> Cc: Lina Iyer > >> Co-developed-by: Lina Iyer > >> Signed-off-by: Ulf Hansson > >> --- > >> drivers/firmware/psci.c | 31 +++++++++++++++++++++++++++++++ > >> include/linux/psci.h | 2 ++ > >> 2 files changed, 33 insertions(+) > >> > >> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c > >> index 463f78c..45d55fc 100644 > >> --- a/drivers/firmware/psci.c > >> +++ b/drivers/firmware/psci.c > >> @@ -723,6 +723,37 @@ int __init psci_dt_init(void) > >> return ret; > >> } > >> > >> +int __init psci_dt_topology_init(void) > >> +{ > >> + struct device_node *np; > >> + int ret; > >> + > >> + if (!psci_has_osi_support()) > >> + return 0; > >> + > >> + np = of_find_matching_node_and_match(NULL, psci_of_match, NULL); > >> + if (!np) > >> + return -ENODEV; > >> + > >> + /* Initialize the CPU PM domains based on topology described in DT. */ > >> + ret = psci_dt_init_pm_domains(np); > >> + if (ret <= 0) > >> + goto out; > >> + > >> + /* Enable OSI mode. */ > >> + ret = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, > >> + PSCI_1_0_SUSPEND_MODE_OSI, 0, 0); > > > > IIUC, this patch will need to be moved after the subsequent two patches > > to ensure that this series bisects cleanly. > > Actually not. > > $subject patch adds a new init function, which purpose is to > initialize the PSCI PM domains. At this point none is calling it. Ah. I missed the fact there were no callers. Sorry for the noise! Thanks, Mark.