[PATCH v2 5/6] mach-snapdragon: MSM8916 spin-table CPU boot support
Sam Day
me at samcday.com
Thu Jun 4 01:25:31 CEST 2026
Hey Stephan,
On Thursday, 4 June 2026 at 12:12 AM, Stephan Gerhold <stephan.gerhold at linaro.org> wrote:
> On Tue, Jun 02, 2026 at 05:19:01PM +1000, Sam Day via B4 Relay wrote:
> > From: Sam Day <me at samcday.com>
> >
> > Most MSM8916 devices lack PSCI support, instead they're brought online
> > with a qcom SCM call to set the boot address, and some register poking
> > of the APCS register block.
> >
> > Signed-off-by: Sam Day <me at samcday.com>
> > ---
> > arch/arm/mach-snapdragon/Makefile | 1 +
> > arch/arm/mach-snapdragon/msm8916-smp.c | 138 +++++++++++++++++++++++++++++++++
> > 2 files changed, 139 insertions(+)
> >
> > diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile
> > index 343e825c6fd..584b8af055a 100644
> > --- a/arch/arm/mach-snapdragon/Makefile
> > +++ b/arch/arm/mach-snapdragon/Makefile
> > @@ -5,3 +5,4 @@
> > obj-y += board.o
> > obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += capsule_update.o
> > obj-$(CONFIG_OF_LIVE) += of_fixup.o
> > +obj-$(CONFIG_ARMV8_SPIN_TABLE) += msm8916-smp.o
> > diff --git a/arch/arm/mach-snapdragon/msm8916-smp.c b/arch/arm/mach-snapdragon/msm8916-smp.c
> > new file mode 100644
> > index 00000000000..24f5590ccc3
> > --- /dev/null
> > +++ b/arch/arm/mach-snapdragon/msm8916-smp.c
> > @@ -0,0 +1,138 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * On MSM8916 devices that lack a PSCI implementation, firing up the secondary
> > + * cores requires a call to TZ to set the boot address, and some poking of ACPS
> > + * register block.
> > + *
> > + * Copyright (c) 2025 Linaro Ltd.
> > + */
>
> Was Linaro involved in this in 2025? Did you write the code yourself
> (the init sequence, especially) or take it over from somewhere else?
> Would be good to be precise here.
Uh - I don't remember why Jan-2025 Sam thought to place Linaro copyrights here.
I think the logic was that, since I don't care about owning the copyright, I
figured I'd just assign it to the entity that owns a lot of the code in
mach-snapdragon already.
Anyway, since most of the code here came from the kernel, and I used lk2nd as
a reference, I'll just preserve those copyrights in v3 instead.
>
> > +
> > +#include <asm/io.h>
> > +#include <asm/spin_table.h>
> > +#include <asm/system.h>
> > +#include <cpu_func.h>
> > +#include <dm/ofnode.h>
> > +#include <firmware/qcom/scm.h>
> > +#include <linux/arm-smccc.h>
> > +#include <linux/delay.h>
> > +
> > +#define APCS_CPU_PWR_CTL 0x04
> > +#define CORE_PWRD_UP BIT(7)
> > +#define COREPOR_RST BIT(5)
> > +#define CORE_RST BIT(4)
> > +#define CORE_MEM_HS BIT(3)
> > +#define CORE_MEM_CLAMP BIT(1)
> > +#define CLAMP BIT(0)
> > +
> > +#define APC_PWR_GATE_CTL 0x14
> > +#define GDHS_CNT_SHIFT 24
> > +#define GDHS_EN BIT(0)
> > +
> > +static void qcom_boot_cortex_a53(phys_addr_t acc_base)
> > +{
> > + u32 reg_val;
> > +
> > + /* Put the CPU into reset. */
> > + reg_val = CORE_RST | COREPOR_RST | CLAMP | CORE_MEM_CLAMP;
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
> > +
> > + /* Turn on the GDHS and set the GDHS_CNT to 16 XO clock cycles */
> > + writel(GDHS_EN | (0x10 << GDHS_CNT_SHIFT), acc_base + APC_PWR_GATE_CTL);
> > + /* Wait for the GDHS to settle */
> > + udelay(2);
> > +
> > + reg_val &= ~CORE_MEM_CLAMP;
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
> > + reg_val |= CORE_MEM_HS;
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
> > + udelay(2);
> > +
> > + reg_val &= ~CLAMP;
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
> > + udelay(2);
> > +
> > + /* Release CPU out of reset and bring it to life. */
> > + reg_val &= ~(CORE_RST | COREPOR_RST);
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
> > + reg_val |= CORE_PWRD_UP;
> > + writel(reg_val, acc_base + APCS_CPU_PWR_CTL);
>
> Nitpick (might be fine as-is, since Linux seems to have the same):
> Strictly speaking, I think we need some memory barriers / DSBs in here
> to make sure the write is complete before the udelay() starts (although
> this is notoriously hard. If you're interested in this, take a look at
> https://youtu.be/i6DayghhA8Q?t=1677, although I think it doesn't
> translate exactly to U-Boot).
Hmm yeah, it's odd because kernel is *not* issuing barriers in the
cortex_a7 startup sequence, but *is* for kpss v1/v2. Since lk2nd
already has prior art for this I'll just follow that lead in v3.
>
> > +}
> > +
> > +static int qcom_scm_set_boot_addr_mc(void *entry, unsigned int flags)
> > +{
> > + struct qcom_scm_desc desc = {
> > + .svc = QCOM_SCM_SVC_BOOT,
> > + .cmd = QCOM_SCM_BOOT_SET_ADDR_MC,
> > + .owner = ARM_SMCCC_OWNER_SIP,
> > + .arginfo = QCOM_SCM_ARGS(6),
> > + .args = {
> > + (u64)entry,
> > + /* Apply to all CPUs in all affinity levels */
> > + ~0ULL, ~0ULL, ~0ULL, ~0ULL,
> > + flags,
> > + },
> > + };
> > +
> > + if (!qcom_scm_is_call_available(desc.svc, desc.cmd))
> > + return -EOPNOTSUPP;
> > +
> > + return qcom_scm_call(&desc, NULL);
> > +}
> > +
> > +static bool boot_addr_set;
> > +
> > +int spin_table_boot_cpu(void *fdt, int cpu_offset)
> > +{
> > + struct fdtdec_phandle_args acc;
> > + u32 mpidr_aff, acc_base, reg;
> > + int ret;
> > +
> > + if (fdt_node_check_compatible(fdt, 0, "qcom,msm8916"))
> > + return 0;
>
> What if we have a qcom,apq8016? :)
Wellll, in that case there's a functional TZ that can handle the PSCI call,
right? ;) I suppose there's probably situations where that's not the case.
Rabble rabble rabble.
Anyway, I added this check in the eleventh hour right before submitting,
because I was paranoid that it might impact some other platform (big-tent board
code is great but also hellish). In retrospect this is unnecessary:
spin_table_boot_cpu() is kicked by the generic code for each spin-table CPU
it finds in the DT. We only force cores to spin-table enablement when we detect
the busted-PSCI situation.
I've removed this check entirely in v3.
>
> > +
> > + reg = fdtdec_get_uint(fdt, cpu_offset, "reg", 0);
> > +
> > + if (fdt_node_check_compatible(fdt, cpu_offset, "arm,cortex-a53")) {
> > + log_warning("CPU%d is not arm,cortex-a53 compatible\n", reg);
> > + return -EINVAL;
> > + }
> > +
> > + if (!boot_addr_set) {
> > + debug("Setting CPU boot address to 0x%llx\n",
> > + (phys_addr_t)&spin_table_reserve_begin);
> > + ret = qcom_scm_set_boot_addr_mc(&spin_table_reserve_begin,
> > + QCOM_SCM_BOOT_MC_FLAG_AARCH64 |
> > + QCOM_SCM_BOOT_MC_FLAG_COLDBOOT);
> > + if (ret) {
> > + log_err("Failed to set CPU boot addr: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + boot_addr_set = true;
> > + }
>
> I would move this below log_info("Booting CPU"), so you don't waste
> doing this for nothing if one of the other check fails.
Good catch, fixed in v3.
Thanks for the thorough review Stephan!
-Sam
>
> > +
> > + mpidr_aff = read_mpidr() & 0xffffff;
> > +
> > + if (reg == mpidr_aff) {
> > + debug("Skipping boot of current CPU%d\n", reg);
> > + return 0;
> > + }
> > +
> > + ret = fdtdec_parse_phandle_with_args(fdt, cpu_offset, "qcom,acc",
> > + NULL, 0, 0, &acc);
> > + if (ret) {
> > + log_err("Failed to parse qcom,acc phandle: %d\n", reg);
> > + return ret;
> > + }
> > +
> > + acc_base = fdtdec_get_addr_size_auto_noparent(fdt, acc.node, "reg", 0,
> > + NULL, true);
> > + if (!acc_base) {
> > + log_err("Failed to parse qcom,acc regbase\n");
> > + return -EINVAL;
> > + }
> > +
> > + log_info("Booting CPU%d @ 0x%x\n", reg, acc_base);
> > + qcom_boot_cortex_a53(acc_base);
> > + return 0;
> > +}
>
> Thanks,
> Stephan
>
More information about the U-Boot
mailing list