[PATCH 07/10] stm32mp1: spl: Configure MAC address when booting OP-TEE

[Alex G.]

On 8/31/21 12:10 PM, Patrick DELAUNAY wrote:
> Hi,
> 
> 
> On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:
>> When OP-TEE is booted as the SPL payload, the stage after OP-TEE is
>> not guaranteed to be u-boot. Thus the FDT patching in u-boot is not
>> guaranteed to occur. Add this step to SPL.
>>
>> The patching by stm32_fdt_setup_mac_addr() is done in SPL, and patches
>> the target FDT directly. This differs is different from
>> setup_mac_address(), which sets the "ethaddr" env variable, and does
>> not work in SPL.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
>> ---
>>   arch/arm/mach-stm32mp/cpu.c                   | 22 +++++++++++++++++++
>>   .../arm/mach-stm32mp/include/mach/sys_proto.h |  3 +++
>>   arch/arm/mach-stm32mp/spl.c                   |  1 +
>>   3 files changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
>> index 8727de513c..2b8b67bb40 100644
>> --- a/arch/arm/mach-stm32mp/cpu.c
>> +++ b/arch/arm/mach-stm32mp/cpu.c
>> @@ -10,6 +10,7 @@
>>   #include <cpu_func.h>
>>   #include <debug_uart.h>
>>   #include <env.h>
>> +#include <fdt_support.h>
>>   #include <init.h>
>>   #include <log.h>
>>   #include <lmb.h>
>> @@ -646,6 +647,27 @@ __weak int setup_mac_address(void)
>>       return 0;
>>   }
>> +int stm32_fdt_setup_mac_addr(void *fdt)
>> +{
>> +    int ret;
>> +    uchar enetaddr[ARP_HLEN];
>> +
>> +    ret = stm32_read_otp_mac(enetaddr);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (!is_valid_ethaddr(enetaddr)) {
>> +        printf("invalid MAC address in OTP\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    ret = fdt_ethernet_set_macaddr(fdt, 0, enetaddr);
>> +    if (ret)
>> +        debug("Failed to set mac address from OTP: %d\n", ret);
>> +
>> +    return ret;
>> +}
>> +
>>   static int setup_serial_number(void)
>>   {
>>       char serial_string[25];
>> diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h 
>> b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
>> index 4149d3a133..2d24cfee3f 100644
>> --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
>> +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
>> @@ -47,7 +47,10 @@ void get_soc_name(char name[SOC_NAME_SIZE]);
>>   /* return boot mode */
>>   u32 get_bootmode(void);
>> +/* Set 'ethaddr' env variable with MAC from OTP (useful for u-boot 
>> proper) */
>>   int setup_mac_address(void);
>> +/* Patch the first 'ethernet' node of FDT with MAC from OTP (useful 
>> for SPL) */
>> +int stm32_fdt_setup_mac_addr(void *fdt);
>>   /* board power management : configure vddcore according OPP */
>>   void board_vddcore_init(u32 voltage_mv);
>> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
>> index 405eff68a3..d9fdc5926c 100644
>> --- a/arch/arm/mach-stm32mp/spl.c
>> +++ b/arch/arm/mach-stm32mp/spl.c
>> @@ -181,6 +181,7 @@ void stm32_init_tzc_for_optee(void)
>>   void spl_board_prepare_for_optee(void *fdt)
>>   {
>> +    stm32_fdt_setup_mac_addr(fdt);
>>       stm32_init_tzc_for_optee();
>>   }
> 
> 
> I think that all this part is not required for falcon mode:
> 
> let the Linux driver fallback to nvmem access to read the MAC address in 
> OTP when the DT is not updated by boot loader
> 
> remove also the patch 06/10 and 04/40
> 
> PS: it is already working in OpenSTLinux based on the device tree 
> modification for your board
> 
> &bsec{
>              ethernet_mac_address: mac at e4 {
>              reg = <0xe4 0x6>;
> };
> 
> &ethernet0 {
>      nvmem-cells = <&ethernet_mac_address>;
>      nvmem-cell-names = "mac-address";
> };

I was looking at this, following Sean's suggestion from last week. The 
linux BSEC driver backends on stm32_bsec_smc(). This requires a 
corresponding secure-world callout. which is not currently implemented 
for SPL. We'd just get "Can't read data57 (-5)" error from linux, and a 
random MAC address.

Is this change as proposed ideal? Probably not. It works with SPL and 
the current LTS linux (v5.10), and is likely the least intensive 
solution in terms of lines of code.

Alex

> PS: this part is not yet upstreamed in Linux

That's not problematic. I can accommodate devicetree changes by using 
overlays in the FIT image. This flexibility is also a huge reason why 
I've chosen to go with FIT versus other image formats.