[PATCH] rockchip: rk3328: Set efuse auto mode and timing control
Jonas Karlman
jonas at kwiboo.se
Tue Dec 5 11:37:10 CET 2023
Hi Peter,
On 2023-12-05 10:31, Peter Robinson wrote:
> On Mon, Nov 27, 2023 at 7:31 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>>
>> Hi Quentin,
>>
>> On 2023-11-27 19:42, Quentin Schulz wrote:
>>> Hi Jonas,
>>>
>>> ________________________________________
>>> From: U-Boot <u-boot-bounces at lists.denx.de> on behalf of Jonas Karlman <jonas at kwiboo.se>
>>> Sent: 26 November 2023 20:46
>>> To: Kever Yang; Simon Glass; Philipp Tomsich
>>> Cc: Jonas Karlman; u-boot at lists.denx.de
>>> Subject: [PATCH] rockchip: rk3328: Set efuse auto mode and timing control
>>>
>>> Reading from efuse return zero when mainline TF-A is used.
>>>
>>> => dump_efuse
>>> 00000000: 00 00 00 00 ....
>>> 00000004: 00 00 00 00 ....
>>> 00000008: 00 00 00 00 ....
>>> 0000000c: 00 00 00 00 ....
>>> 00000010: 00 00 00 00 ....
>>> 00000014: 00 00 00 00 ....
>>> 00000018: 00 00 00 00 ....
>>> 0000001c: 00 00 00 00 ....
>>>
>>> However, when vendor TF-A blobs is used reading from efuse works.
>>>
>>> Change to use auto mode, enable finish and auto access err interrupts
>>> and set timing control using same values that vendor TF-A blob use to
>>> fix this.
>>>
>>> With this efuse can be read when either of mainline TF-A or vendor blob
>>> is used.
>>>
>>> => dump_efuse
>>> 00000000: 52 4b 33 82 RK3.
>>> 00000004: 00 fe 21 55 ..!U
>>> 00000008: 52 4b 57 34 RKW4
>>> 0000000c: 35 30 32 39 5029
>>> 00000010: 00 00 00 00 ....
>>> 00000014: 08 25 0c 0f .%..
>>> 00000018: 02 0d 08 00 ....
>>> 0000001c: 00 00 f0 00 ....
>>>
>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>> ---
>>> arch/arm/mach-rockchip/rk3328/rk3328.c | 34 ++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-rockchip/rk3328/rk3328.c b/arch/arm/mach-rockchip/rk3328/rk3328.c
>>> index de17b8868273..edb22a58fc67 100644
>>> --- a/arch/arm/mach-rockchip/rk3328/rk3328.c
>>> +++ b/arch/arm/mach-rockchip/rk3328/rk3328.c
>>> @@ -19,6 +19,22 @@ DECLARE_GLOBAL_DATA_PTR;
>>> #define GRF_BASE 0xFF100000
>>> #define UART2_BASE 0xFF130000
>>> #define FW_DDR_CON_REG 0xFF7C0040
>>> +#define EFUSE_NS_BASE 0xFF260000
>>> +
>>> +#define EFUSE_MOD 0x0000
>>> +#define EFUSE_INT_CON 0x0014
>>> +#define EFUSE_T_CSB_P 0x0028
>>> +#define EFUSE_T_PGENB_P 0x002C
>>> +#define EFUSE_T_LOAD_P 0x0030
>>> +#define EFUSE_T_ADDR_P 0x0034
>>> +#define EFUSE_T_STROBE_P 0x0038
>>> +#define EFUSE_T_CSB_R 0x003C
>>> +#define EFUSE_T_PGENB_R 0x0040
>>> +#define EFUSE_T_LOAD_R 0x0044
>>> +#define EFUSE_T_ADDR_R 0x0048
>>> +#define EFUSE_T_STROBE_R 0x004C
>>> +
>>> +#define EFUSE_TIMING(s, l) (((s) << 16) | (l))
>>>
>>> const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>> [BROM_BOOTSOURCE_EMMC] = "/mmc at ff520000",
>>> @@ -54,6 +70,24 @@ int arch_cpu_init(void)
>>>
>>> /* Disable the ddr secure region setting to make it non-secure */
>>> rk_setreg(FW_DDR_CON_REG, 0x200);
>>> +
>>> + /* Use efuse auto mode */
>>> + writel(0x46, EFUSE_NS_BASE + EFUSE_MOD);
>>> +
>>> + /* Enable efuse finish and auto access err interrupt */
>>> + writel(0x07, EFUSE_NS_BASE + EFUSE_INT_CON);
>>> +
>>> + /* Set efuse timing control */
>>> + writel(EFUSE_TIMING(1, 241), EFUSE_NS_BASE + EFUSE_T_CSB_P);
>>> + writel(EFUSE_TIMING(1, 241), EFUSE_NS_BASE + EFUSE_T_PGENB_P);
>>> + writel(EFUSE_TIMING(1, 241), EFUSE_NS_BASE + EFUSE_T_LOAD_P);
>>> + writel(EFUSE_TIMING(1, 241), EFUSE_NS_BASE + EFUSE_T_ADDR_P);
>>> + writel(EFUSE_TIMING(2, 240), EFUSE_NS_BASE + EFUSE_T_STROBE_P);
>>> + writel(EFUSE_TIMING(1, 4), EFUSE_NS_BASE + EFUSE_T_CSB_R);
>>> + writel(EFUSE_TIMING(1, 4), EFUSE_NS_BASE + EFUSE_T_PGENB_R);
>>> + writel(EFUSE_TIMING(1, 4), EFUSE_NS_BASE + EFUSE_T_LOAD_R);
>>> + writel(EFUSE_TIMING(1, 4), EFUSE_NS_BASE + EFUSE_T_ADDR_R);
>>> + writel(EFUSE_TIMING(2, 3), EFUSE_NS_BASE + EFUSE_T_STROBE_R);
>>>
>>> Shouldn't this be done in the efuse driver instead?
>>
>> I was considering the same and was unsure if these values would need to
>> be initialized at EL3 in SPL or could be initialized at EL1 in U-Boot
>> proper (in the efuse driver).
>>
>> Did not have time to investigate further and decided to just do it at
>> EL3 same as vendor TF-A, so code ended up here.
>
> Is there a reason not to do it in the upstream TF-A like the vendor
> one does? I'm sure I've seen patches around that do this when I've dug
> into it in the past. Is there reasons where TF-A would need access to
> the information the efuses provide, I'm guessing things like serial
> numbers etc, given the vendor TF-A does this I suspect that's where
> the patch should go rather than U-Boot.
Main reason is that mainline TF-A does not seem to have a need for the
efuse values, U-Boot does however read from efuse to get serial and
generate an ethaddr. It is also possible to skip TF-A altogether.
So initializing in U-Boot SPL to be able to read values needed in U-Boot
seem like a good self-contained option in my opinion.
Regards,
Jonas
>
> Peter
More information about the U-Boot
mailing list