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!