Re: [PATCH 5/8] firmware: qcom: scm: Use atomic SCM for cold boot

Andy Gross
2016-04-23T04:46:55Z

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 <andy.gross@linaro.org>
> > ---
> > 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.