Re: [PATCH RFC 5/9] arm: meson: initial u-boot SPL support for GX SoCs

On 08/09/2025 18:53, Ferass El Hafidi wrote:
> On Mon Sep 8, 2025 at 8:24 AM UTC, Neil Armstrong via groups.io wrote:
>> On 07/09/2025 16:36, Ferass El Hafidi wrote:
>>> Add initial boilerplate for U-Boot SPL support on Amlogic.
>>>
>>> Signed-off-by: Ferass El Hafidi <[email protected]>
>>> ---
>>>    arch/arm/include/asm/arch-meson/clock-gx.h |   1 +
>>>    arch/arm/include/asm/arch-meson/gx.h       |  36 ++++
>>>    arch/arm/mach-meson/Kconfig                |  42 ++++-
>>>    arch/arm/mach-meson/Makefile               |   5 +
>>>    arch/arm/mach-meson/board-common.c         |  11 ++
>>>    arch/arm/mach-meson/spl-gx.c               | 278 +++++++++++++++++++++++++++++
>>>    arch/arm/mach-meson/spl.c                  | 123 +++++++++++++
>>>    7 files changed, 495 insertions(+), 1 deletion(-)
>>>
>> <...>
>>> diff --git a/arch/arm/mach-meson/spl-gx.c b/arch/arm/mach-meson/spl-gx.c
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..65b301bc9ed89b7ca2156b7fa672ed4ef3f8633b
>>> --- /dev/null
>>> +++ b/arch/arm/mach-meson/spl-gx.c
>>> @@ -0,0 +1,278 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Portions Copyright (C) 2015, Amlogic, Inc. All rights reserved.
>>> + * Copyright (C) 2023, Ferass El Hafidi <[email protected]>
>>> + */
>>> +#include <hang.h>
>>> +#include <image.h>
>>> +#include <spl.h>
>>> +#include <vsprintf.h>
>>> +#include <asm/io.h>
>>> +#include <asm/arch/boot.h>
>>> +#include <asm/arch/clock-gx.h>
>>> +#include <asm/arch/gx.h>
>>> +#include <linux/delay.h>
>>> +
>>> +/* Meson GX SPL code */
>>> +
>>> +#if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
>>> +#if defined(CONFIG_MESON_GXBB)
>>
>> I think you should indicate why it's only needed for GXBB
>>
> 
> 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!

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

Neil

> 
>> <...>
>>
>> Anyway it's really clean !
>>
>> Thanks!
>> Neil
>>
> 
> Thank you!
> 
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#2757): https://groups.io/g/u-boot-amlogic/message/2757
>> Mute This Topic: https://groups.io/mt/115113902/8399868
>> Group Owner: [email protected]
>> Unsubscribe: https://groups.io/g/u-boot-amlogic/unsub [[email protected]]
>> -=-=-=-=-=-=-=-=-=-=-=-
>