Re: [RFC PATCH 10/17] clk: sunxi: Add support for the D1 CCU

Andre Przywara
2023-05-26T22:08:32Z

On Fri, 26 May 2023 14:22:29 -0600
Sam Edwards <cfsworks@gmail.com> wrote:

Hi Sam,

> On 5/26/23 13:27, Maksim Kiselev wrote:
> >
> > There is a Linux patch that allows to bring up the second core.
> > https://github.com/szemzoa/awboot/blob/6ea4ae4ad7a558ad952fefee1942e260aea1a69f/linux/second_core_support_in_platsmp.patch#L10
> >
> > I think this could be useful for adding PSCI support for T113.
>
> I'm a little surprised that apparently all that is necessary is to set
> the entry point and deassert reset. I've been trying essentially that so
> far with no success (entirely possible I made a mistake somewhere).

So with "no success" you are referring to the patch below? Which is the
Linux patch ported to U-Boot? And does that mean that the Linux patch
works, but the U-Boot version doesn't?

> Perhaps this patch assumes that power to the core is enabled before
> kernel boot? I haven't yet figured out what in the PRCM I need to poke
> to get power to the core, and the PRCM for this chip is not well-documented.

The PRCM is routinely not documented in the user manuals, IIUC
Allwinner provides separate documents for that (but not to us).
So far this was all reverse engineered from either BSP code or
some disassembly. Is there any indication in the BSP source code as to
how this is supposed to work? And maybe there is no separate power
control for the second core?

And have you checked that the PSCI runtime code is correctly hooked up
in U-Boot? I have to check what's really needed, but the whole code
needs to be linked to secure SRAM, IIRC.

> On 5/26/23 04:50, Andre Przywara wrote:
> > I checked the manuals, and it seems the required bits are documented,
> > but IIRC they differ from the other (much older) 32-bit parts. So it
> > would require some refactoring of the existing sunxi PSCI code to
> > accommodate the T113.
>
> Yeah, I decided to factor out the register-manipulating bits to a
> handful of sunxi_cpu_set_* functions, which can be switched out to suit
> the specific SoC in use. psci.c is growing way too many #ifdef branches,
> so I might refactor this yet again to the "weak symbols with strong
> overrides in e.g. psci-r528.c" pattern, unless you find that pattern
> particularly distasteful.

We don't need #ifdef's, you can use normal C "if" statements:

static void __secure sunxi_set_entry_address(void *entry)
{
if (IS_ENABLED(CONFIG_MACH_SUN8I_R528))
writel((u32)entry,
SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
else
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
}

"Dead code elimination" in modern compilers will remove everything
that's not needed, if this can be figured out at compile time (try it!).

This just requires to have all symbols defined all the time, but there
is no reason to protect #define's with #ifdef's either, as it doesn't
hurt to have unused preprocessor symbols. Or you define shared names
(SUNXI_ENTRY_ADDRESS_BASE), depending on Kconfig symbols.
arch/arm/mach-sunxi/spl_spi_sunxi.c makes use of those techniques
extensively, have a look there for inspiration (like static functions
to return base addresses: spi0_base_address(), this will be optimised
away completely).

> I have my WIP diff below. Before inclusion, I'll split it into multiple
> patches (refactoring -> adding R528 support) but it's definitely not
> ready for that yet. However if you could, it would help if you checked
> my changes to cpu_sunxi_ncat2.h and squashed them into your own series
> if they look good to you.

Sure. I actually removed most of the symbols there, as we don't need
them, but can surely add the ones you found for PSCI. Do you need
I2C in the SPL (looking at SUNXI_R_TWI_BASE)?

Thanks,
Andre

> ---
> diff --git a/arch/arm/cpu/armv7/sunxi/psci.c
> b/arch/arm/cpu/armv7/sunxi/psci.c
> index e1d3638b5c..02c5c56c86 100644
> --- a/arch/arm/cpu/armv7/sunxi/psci.c
> +++ b/arch/arm/cpu/armv7/sunxi/psci.c
> @@ -38,6 +38,8 @@
> #define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4)
> #define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0 (0xbc)
>
> +#define SUN8I_R528_SOFT_ENTRY (0x1c8)
> +
> static void __secure cp15_write_cntp_tval(u32 tval)
> {
> asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
> @@ -125,6 +127,13 @@ static void __secure sunxi_set_entry_address(void
> *entry)
> writel((u32)entry,
> SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
> }
> +#elif defined CONFIG_MACH_SUN8I_R528
> +/* secondary core entry address is programmed differently on R528 */
> +static void __secure sunxi_set_entry_address(void *entry)
> +{
> + writel((u32)entry,
> + SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
> +}
> #else
> static void __secure sunxi_set_entry_address(void *entry)
> {
> @@ -155,6 +164,10 @@ static void __secure sunxi_cpu_set_power(int cpu,
> bool on)
> (void *)cpucfg + SUN8I_R40_PWROFF,
> on, cpu);
> }
> +#elif defined CONFIG_MACH_SUN8I_R528
> +static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool
> __always_unused on)
> +{
> +}
> #else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
> static void __secure sunxi_cpu_set_power(int cpu, bool on)
> {
> @@ -166,30 +179,91 @@ static void __secure sunxi_cpu_set_power(int cpu,
> bool on)
> }
> #endif /* CONFIG_MACH_SUN7I */
>
> -void __secure sunxi_cpu_power_off(u32 cpuid)
> +#ifdef CONFIG_MACH_SUN8I_R528
> +#define C0_RST_CTRL 0x0000
> +#define C0_CTRL_REG0 0x0010
> +#define C0_CPU_STATUS 0x0080
> +
> +#define C0_BIT_WFI 16
> +
> +static void __secure sunxi_cpu_set_reset(int cpu, bool active)
> +{
> + if (active)
> + clrbits_le32(SUNXI_CPUX_BASE + C0_RST_CTRL, BIT(cpu));
> + else
> + setbits_le32(SUNXI_CPUX_BASE + C0_RST_CTRL, BIT(cpu));
> +}
> +
> +static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
> +{
> + /* TODO: I don't know what this is or how to do it */
> +}
> +
> +static bool __secure sunxi_cpu_poll_wfi(int cpu)
> +{
> + return !!(readl(SUNXI_CPUX_BASE + C0_CPU_STATUS) & BIT(C0_BIT_WFI + cpu));
> +}
> +
> +static void __secure sunxi_cpu_invalidate_cache(int cpu)
> +{
> + clrbits_le32(SUNXI_CPUX_BASE + C0_CTRL_REG0, BIT(cpu));
> +}
> +#else /* ! CONFIG_MACH_SUN8I_R528 */
> +static void __secure sunxi_cpu_set_reset(int cpu, bool active)
> +{
> + struct sunxi_cpucfg_reg *cpucfg =
> + (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> +
> + writel(active ? 0b00 : 0b11, &cpucfg->cpu[cpu].rst);
> +}
> +
> +static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
> {
> struct sunxi_cpucfg_reg *cpucfg =
> (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> +
> + if (lock)
> + clrbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> + else
> + setbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> +}
> +
> +static bool __secure sunxi_cpu_poll_wfi(int cpu)
> +{
> + struct sunxi_cpucfg_reg *cpucfg =
> + (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> +
> + return !!(readl(&cpucfg->cpu[cpu].status) & BIT(2));
> +}
> +
> +static void __secure sunxi_cpu_invalidate_cache(int cpu)
> +{
> + struct sunxi_cpucfg_reg *cpucfg =
> + (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> +
> + clrbits_le32(&cpucfg->gen_ctrl, BIT(cpu));
> +}
> +#endif
> +
> +void __secure sunxi_cpu_power_off(u32 cpuid)
> +{
> u32 cpu = cpuid & 0x3;
>
> /* Wait for the core to enter WFI */
> - while (1) {
> - if (readl(&cpucfg->cpu[cpu].status) & BIT(2))
> - break;
> + while (!sunxi_cpu_poll_wfi(cpu))
> __mdelay(1);
> - }
>
> /* Assert reset on target CPU */
> - writel(0, &cpucfg->cpu[cpu].rst);
> + sunxi_cpu_set_reset(cpu, true);
>
> /* Lock CPU (Disable external debug access) */
> - clrbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> + sunxi_cpu_set_locking(cpu, true);
>
> /* Power down CPU */
> sunxi_cpu_set_power(cpuid, false);
>
> - /* Unlock CPU (Disable external debug access) */
> - setbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> + /* Unlock CPU (Reenable external debug access) */
> + sunxi_cpu_set_locking(cpu, false);
> }
>
> static u32 __secure cp15_read_scr(void)
> @@ -246,8 +320,6 @@ out:
> int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
> u32 context_id)
> {
> - struct sunxi_cpucfg_reg *cpucfg =
> - (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> u32 cpu = (mpidr & 0x3);
>
> /* store target PC and context id */
> @@ -257,22 +329,22 @@ int __secure psci_cpu_on(u32 __always_unused
> unused, u32 mpidr, u32 pc,
> sunxi_set_entry_address(&psci_cpu_entry);
>
> /* Assert reset on target CPU */
> - writel(0, &cpucfg->cpu[cpu].rst);
> + sunxi_cpu_set_reset(cpu, true);
>
> /* Invalidate L1 cache */
> - clrbits_le32(&cpucfg->gen_ctrl, BIT(cpu));
> + sunxi_cpu_invalidate_cache(cpu);
>
> /* Lock CPU (Disable external debug access) */
> - clrbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> + sunxi_cpu_set_locking(cpu, true);
>
> /* Power up target CPU */
> sunxi_cpu_set_power(cpu, true);
>
> /* De-assert reset on target CPU */
> - writel(BIT(1) | BIT(0), &cpucfg->cpu[cpu].rst);
> + sunxi_cpu_set_reset(cpu, false);
>
> - /* Unlock CPU (Disable external debug access) */
> - setbits_le32(&cpucfg->dbg_ctrl1, BIT(cpu));
> + /* Unlock CPU (Reenable external debug access) */
> + sunxi_cpu_set_locking(cpu, false);
>
> return ARM_PSCI_RET_SUCCESS;
> }
> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> b/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> index d01508517c..629d761aa6 100644
> --- a/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> @@ -17,6 +17,8 @@
> #define SUNXI_SIDC_BASE 0x03006000
> #define SUNXI_SID_BASE 0x03006200
> #define SUNXI_TIMER_BASE 0x02050000
> +#define SUNXI_GIC400_BASE 0x03020000
> +#define SUNXI_CPUX_BASE 0x09010000
>
> #ifdef CONFIG_MACH_SUN50I_H6
> #define SUNXI_DRAM_COM_BASE 0x04002000
> @@ -34,11 +36,11 @@
> #define SUNXI_SPI0_BASE 0x04025000
> #define SUNXI_SPI1_BASE 0x04026000
>
> -#define SUNXI_RTC_BASE 0x07000000
> #define SUNXI_R_CPUCFG_BASE 0x07000400
> #define SUNXI_PRCM_BASE 0x07010000
> #define SUNXI_R_WDOG_BASE 0x07020400
> -#define SUNXI_R_TWI_BASE 0x07081400
> +#define SUNXI_R_TWI_BASE 0x07020800
> +#define SUNXI_RTC_BASE 0x07090000
>
> #ifndef __ASSEMBLY__
> void sunxi_board_init(void);
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index bb9b863d2c..a5d312d377 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -366,6 +366,8 @@ config MACH_SUN8I_R40
> config MACH_SUN8I_R528
> bool "sun8i (Allwinner R528)"
> select CPU_V7A
> + select CPU_V7_HAS_NONSEC
> + select ARCH_SUPPORT_PSCI
> select SUNXI_GEN_NCAT2
> select SUNXI_NEW_PINCTRL
> select MMC_SUNXI_HAS_NEW_MODE