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

Sam Edwards
2023-05-27T02:15:27Z

Hey Andre,

On 5/26/23 16:07, Andre Przywara wrote:
> 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?

Yes, the diff below is what I've been trying to use. It's not the Linux
patch ported to U-Boot; I was trying to get it to work (wondering if
there was something I was missing, like power control) when Maksim
pointed out the Linux patch. I haven't tried the latter at all.

My "no success" is Linux stalling indefinitely at:
[ 0.123090] smp: Bringing up secondary CPUs ...

> 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?

The user manual strongly suggests that power is enabled by default, so I
may not need to touch PRCM after all. I also checked (and
double-checked) the raw disassembly of my resulting `psci_cpu_on` and it
does look like all of the correct registers are being hit.

I don't have any BSP source code suggesting anything otherwise. I do
have an OP-TEE binary, which doesn't appear to try anything more than
the kernel patch (and my current PSCI implementation) does.

> 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.

I'm about 80% sure the PSCI code is properly callable. I've toyed with
the kernel a little bit to test this. I get a "bad mode" panic if U-Boot
doesn't register the SMC vector, and PSCI_RET_NOT_SUPPORTED if I call
unused function IDs, so I think U-Boot is able to get called and return
just fine.

The OP-TEE binary also doesn't appear to try to relocate itself into any
kind of particular RAM location. It just runs where it loads from DRAM.
I take this to mean U-Boot's "in-situ" secure segment case is
appropriate here, and no specific linking is needed.

The one thing I'm not very sure about at the moment is whether
`psci_cpu_entry` is properly handling the newly-awakened second core.
I'll keep debugging.

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

I'll try to follow that approach then.

> This just requires to have all symbols defined all the time

Egh, gross, this means some code that shouldn't be used for a particular
target can slip by the compiler's diagnostics unnoticed (such as when
this happened with SUNXI_R_PIO_BASE). I wish there was a better way.

> 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)?

I'm not sure yet: my target does have an EEPROM on TWI2, which might
eventually be used to store a board revision (and therefore which
determines the FDT that the SPL should load), but I'm also considering
overlays for other versions that are applied within main U-Boot.

But I can't fit much more in SPL anyway. The text section there is
already up to 0x6ec0 (without SPI or NAND yet), and with third-party
tooling assuming U-Boot at offset 0x8000, I can't very easily allocate
more room to it. I might have to figure out what more to clear out here.

But anyway I only filled in that address because I came across the
correct value in Allwinner's fork of U-Boot and noted it down before I
forgot.

Cheers,
Sam