[PATCH] mach-snapdragon: handle platforms without PSCI support

[Sam Day]

Hello Mark,

On Monday, 27 January 2025 at 17:36, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:

> 
> 
> > Date: Mon, 27 Jan 2025 14:48:55 +0000
> 
> > From: Sam Day me at samcday.com
> 
> 
> Hi Sam,
> 
> > Most MSM8916 devices shipped without PSCI support. The history is quite
> > nuanced (a good overview can be found in 1), but the end result is
> > that the upstream DTs for this SoC pretend that PSCI exists, and it's
> > expected that the bootloader handles the case where it doesn't. This is
> > codified by the de-facto bootloader for MSM8916 devices, lk2nd 2.
> > 
> > So we handle it here by deleting the /psci node if we detect the absence
> > of PSCI. We need to do this early to ensure sysreset works correctly,
> > since the PSCI firmware driver is PRE_RELOC and binds the PSCI sysreset
> > driver.
> 
> 
> But deleting the /psci node isn't enough is it? At least on systems
> with more than a single core the OS will need a way to spin up the
> additonal cores. So you'll need to modify the CPU nodes as well and
> change the "enable-method" property to "spin-table" and add the
> necessary infrastructure for that.
> 

Yes, you're absolutely right. I'm preparing to submit a patch series that
does exactly that. The spin-table setup bits already exists in U-Boot, the
MSM8916 specific bits (qcom-scm SMC call + poking APCS registers) will be
the bulk of the work I submit later tonight.

I opted to submit this patch alone without that work for a few reasons:

 * I've already submitted some patches to enable sysreset support for
   MSM8916 [1] and with this patch, sysreset will then work properly on
   this platform.
 * As it currently stands, the secondary cores can't be booted on this
   platform, regardless of whether the /psci node exists or not. Deleting
   the psci node doesn't make matters any *worse* when booting on devices
   that have no PSCI implementation.
 * The SMP bring-up code is a lot meatier (I had to pull in a bunch of qcom
   SCM calling code from the kernel) and might need a few rounds before it
   can land.
 * U-Boot can be experimentally chained from lk2nd (which itself is chained
   from aboot, which itself is chained from ... well, you get the idea!),
   and so in that case the SMP spin-table setup + DT patching has already
   happened in that previous bootloader. But that BL only *disables* the
   /psci node, which doesn't prevent the U-Boot PSCI firmware driver from
   binding and causing the (non-functional) PSCI sysreset driver to also be
   bound.
 * And so considering the previous points, my hope is that this modest
   patch will land in time for v2025.04 release and make U-Boot a bit more
   functional on (non-PSCI) MSM8916 devices.

[1]: https://patchwork.ozlabs.org/project/uboot/list/?series=441769

Warm Regards,
-Sam


> > Additionally, show_psci_version is updated to check that PSCI exists.
> > Currently this banner outputs "PSCI: 65535.65535" on devices without
> > PSCI support, which isn't very useful :)
> > 
> > Signed-off-by: Sam Day me at samcday.com
> > ---
> > arch/arm/mach-snapdragon/board.c | 43 ++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 39 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> > index e87551784b8d7afb4186533d43b8bddacba9faec..75b9cf1a8a1b3383665eb2d4e6725efe59718de0 100644
> > --- a/arch/arm/mach-snapdragon/board.c
> > +++ b/arch/arm/mach-snapdragon/board.c
> > @@ -162,11 +162,42 @@ static void show_psci_version(void)
> > 
> > arm_smccc_smc(ARM_PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
> > 
> > + /* Some older SoCs like MSM8916 don't always support PSCI */
> > + if ((int)res.a0 == PSCI_RET_NOT_SUPPORTED)
> > + return;
> > +
> > debug("PSCI: v%ld.%ld\n",
> > PSCI_VERSION_MAJOR(res.a0),
> > PSCI_VERSION_MINOR(res.a0));
> > }
> > 
> > +/**
> > + * Most MSM8916 devices in the wild shipped without PSCI support, but the
> > + * upstream DTs pretend that PSCI exists. If that situation is detected here,
> > + * the /psci node is deleted. This is done very early to ensure the PSCI
> > + * firmware driver doesn't bind (which then binds a sysreset driver that won't
> > + * work).
> > + */
> > +static void qcom_psci_fixup(void fdt)
> > +{
> > + int offset, ret;
> > + struct arm_smccc_res res;
> > +
> > + arm_smccc_smc(ARM_PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
> > +
> > + if ((int)res.a0 != PSCI_RET_NOT_SUPPORTED)
> > + return;
> > +
> > + offset = fdt_path_offset(fdt, "/psci");
> > + if (offset < 0)
> > + return;
> > +
> > + debug("Found /psci DT node on device with no PSCI. Deleting.\n");
> > + ret = fdt_del_node(fdt, offset);
> > + if (ret)
> > + log_err("Failed to delete /psci node: %d\n", ret);
> > +}
> > +
> > / We support booting U-Boot with an internal DT when running as a first-stage bootloader
> > * or for supporting quirky devices where it's easier to leave the downstream DT in place
> > * to improve ABL compatibility. Otherwise, we use the DT provided by ABL.
> > @@ -212,12 +243,16 @@ int board_fdt_blob_setup(void **fdtp)
> > 
> > if (internal_valid) {
> > debug("Using built in FDT\n");
> > - return -EEXIST;
> > + ret = -EEXIST;
> > + } else {
> > + debug("Using external FDT\n");
> > + *fdtp = external_fdt;
> > + ret = 0;
> > }
> > 
> > - debug("Using external FDT\n");
> > - *fdtp = external_fdt;
> > - return 0;
> > + qcom_psci_fixup(*fdtp);
> > +
> > + return ret;
> > }
> > 
> > void reset_cpu(void)
> > 
> > ---
> > base-commit: 2eed5a1ff36217372e19f7513bd07077fc76718a
> > change-id: 20250127-qcom-handle-absent-psci-6e8e8af7bcd7
> > prerequisite-change-id: 20250120-qcom-parse-memory-updates-96ffe248cdf1:v3
> > prerequisite-patch-id: 65448b5a93120d2117c1361035afc9bd911d005f
> > 
> > Best regards,
> > --
> > Sam Day me at samcday.com