Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752310AbcDWEqz (ORCPT ); Sat, 23 Apr 2016 00:46:55 -0400 Received: from mail-oi0-f45.google.com ([209.85.218.45]:36037 "EHLO mail-oi0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752040AbcDWEqx (ORCPT ); Sat, 23 Apr 2016 00:46:53 -0400 Date: Fri, 22 Apr 2016 23:46:51 -0500 From: Andy Gross To: Bjorn Andersson Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Stephen Boyd , devicetree@vger.kernel.org, Bjorn Andersson , jilai wang Subject: Re: [PATCH 5/8] firmware: qcom: scm: Use atomic SCM for cold boot Message-ID: <20160423044651.GC6968@hector.attlocal.net> References: <1461363432-5730-1-git-send-email-andy.gross@linaro.org> <1461363432-5730-6-git-send-email-andy.gross@linaro.org> <20160422235005.GI3202@tuxbot> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160422235005.GI3202@tuxbot> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1709 Lines: 48 On Fri, Apr 22, 2016 at 04:50:05PM -0700, Bjorn Andersson wrote: > On Fri 22 Apr 15:17 PDT 2016, Andy Gross wrote: > > > This patch changes the cold_set_boot_addr function to use atomic SCM > > calls. This removes the need for memory allocation and instead places > > all arguments in registers. > > > > Signed-off-by: Andy Gross > > --- > > drivers/firmware/qcom_scm-32.c | 40 ++++++++++++++++++++++++++-------------- > > 1 file changed, 26 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c > [..] > > /* > > * Set the cold/warm boot address for one of the CPU cores. > > */ > > -static int qcom_scm_set_boot_addr(u32 addr, int flags) > > +static int qcom_scm_set_boot_addr(u32 addr, int flags, bool do_atomic) > > { > > struct { > > __le32 flags; > > __le32 addr; > > } cmd; > > > > - cmd.addr = cpu_to_le32(addr); > > - cmd.flags = cpu_to_le32(flags); > > - return qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR, > > - &cmd, sizeof(cmd), NULL, 0); > > + if (do_atomic) { > > + return qcom_scm_call_atomic(QCOM_SCM_SVC_BOOT, > > + QCOM_SCM_BOOT_ADDR, 2, flags, addr); > > + } else { > > + > > + cmd.addr = cpu_to_le32(addr); > > + cmd.flags = cpu_to_le32(flags); > > + > > + return qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR, > > + &cmd, sizeof(cmd), NULL, 0); > > + } > > I would prefer that you split this into two functions, rather than > hiding two functions bodies in one function. > > Perhaps qcom_scm_set_boot_addr and qcom_scm_set_boot_addr_atomic? Fair enough. It does get a little contrived when you start throwing the extra options in there.