Re: [PATCH RFC 5/9] arm: meson: initial u-boot SPL support for GX SoCs
"Ferass El Hafidi" <[email protected]> Wed, 10 Sep 2025 10:28:00 +0000
| Newsgroups | io.groups.u-boot-amlogic |
|---|---|
| Message-ID | <[email protected]> |
On Wed Sep 10, 2025 at 7:02 AM UTC, Neil Armstrong via groups.io wrote:
> <...>
>>
>> Sure! I will also explain here to clarify.
>> Basically this is sending the SCP firmware (aka. bl30/bl301) to the SCP.
>> Historically (in GXBB) this was done by bl2.bin, but (to save space?) it
>> was moved to BL31 in GXL:
>>
>> - https://git.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/refs/heads/master/plat/amlogic/gxl/gxl_bl31_setup.c#129
>>
>> and BL2 would give image info for bl30/bl301 to BL31. I patched this
>> upstream so we don't have to do that in SPL:
>>
>> - https://git.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/refs/heads/master/plat/amlogic/gxl/gxl_bl31_setup.c#105
>>
>> Whether we can move SCP fw loading to BL31 on GXBB is .. an interesting
>> thing that could be considered. That way we could possibly also save
>> some space in U-Boot SPL.
>>
>
> Would be nice, but keep it as-is for now!
>
Yeah, something for later :)
>>>> +void meson_power_init(void)
>>>> +{
>>>> + /* TODO: Support more voltages */
>>>> +
>>>> + /* Init PWM B */
>>>> + clrsetbits_32(GX_PWM_MISC_REG_AB, 0x7f << 16, (1 << 23) | (1 << 1));
>>>> +
>>>> + /* Set voltage */
>>>> + if (CONFIG_IS_ENABLED(MESON_GX_VCCK_1120MV))
>>>> + writel(0x02001a, GX_PWM_PWM_B);
>>>> + else if (CONFIG_IS_ENABLED(MESON_GX_VCCK_1100MV))
>>>> + writel(0x040018, GX_PWM_PWM_B);
>>>> + else if (CONFIG_IS_ENABLED(MESON_GX_VCCK_1000MV))
>>>> + writel(0x0e000e, GX_PWM_PWM_B);
>>>> +
>>>> + if (IS_ENABLED(CONFIG_MESON_GXBB)) {
>>>> + clrbits_32(GX_PIN_MUX_REG7, 1 << 22);
>>>> + clrsetbits_32(GX_PIN_MUX_REG3, 1 << 22, 1 << 21);
>>>> + } else {
>>>> + clrbits_32(GX_PIN_MUX_REG1, 1 << 10);
>>>> + clrsetbits_32(GX_PIN_MUX_REG2, 1 << 5, 1 << 11);
>>>> + }
>>>> +
>>>> + /* Init PWM D */
>>>> + clrsetbits_32(GX_PWM_MISC_REG_CD, 0x7f << 16, (1 << 23) | (1 << 1));
>>>> +
>>>> + /* Set voltage */
>>>> + if (CONFIG_IS_ENABLED(MESON_GX_VDDEE_1100MV))
>>>> + writel(0x040018, GX_PWM_PWM_B);
>>>> + else if (CONFIG_IS_ENABLED(MESON_GX_VDDEE_1000MV))
>>>> + writel(0x0e000e, GX_PWM_PWM_B);
>>>> +
>>>> + if (IS_ENABLED(CONFIG_MESON_GXBB)) {
>>>> + clrbits_32(GX_PIN_MUX_REG7, 1 << 23);
>>>> + setbits_32(GX_PIN_MUX_REG3, 1 << 20);
>>>> + } else {
>>>> + clrbits_32(GX_PIN_MUX_REG1, (1 << 9) | (1 << 11));
>>>> + setbits_32(GX_PIN_MUX_REG2, 1 << 12);
>>>> + }
>>>> +}
>>>
>>> Seems much of the code is duplicated between GXBB and GXL, so I wonder if it would'nt
>>> be cleaner to have spl-gxbb an spl-gxl ?
>>>
>>
>> You mean separate power init functions (meson_power_init_gxbb/gxl)? Should be doable.
>
>
> I mean even separate files, it would avoid any #ifdef stuff
>
Got it.
Thanks!