[PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"

[Marek Vasut]

On 10/4/21 6:34 PM, Patrick DELAUNAY wrote:
> Hi,

Hi,

> On 10/4/21 1:48 PM, Marek Vasut wrote:
>> This reverts commit d5d726d3cc47691ace3c68fa31147ad104aaf579,
>> which breaks boards which ship with multiple SD/eMMC sockets.
>>
>> This stm32mp1.h config is not used only by the ST reference
>> boards, but all the other STM32MP1 based boards in U-Boot, so
>> changes to this stm32mp1.h cannot break the other boards.
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
>> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> ---
>> NOTE: I think we might need to split out the env for different
>>        boards into different headers instead. Thoughts ?
>> ---
>>   include/configs/stm32mp1.h | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
>> index 973a4f1d4b8..a75ed693f57 100644
>> --- a/include/configs/stm32mp1.h
>> +++ b/include/configs/stm32mp1.h
>> @@ -120,7 +120,7 @@
>>    * for serial/usb: execute the stm32prog command
>>    * for mmc boot (eMMC, SD card), boot only on the same device
>>    * for nand or spi-nand boot, boot with on ubifs partition on UBI 
>> partition
>> - * for nor boot, use SD card = mmc0
>> + * for nor boot, use the default order
>>    */
>>   #define STM32MP_BOOTCMD "bootcmd_stm32mp=" \
>>       "echo \"Boot over ${boot_device}${boot_instance}!\";" \
>> @@ -133,8 +133,6 @@
>>           "if test ${boot_device} = nand ||" \
>>             " test ${boot_device} = spi-nand ;" \
>>           "then env set boot_targets ubifs0; fi;" \
>> -        "if test ${boot_device} = nor;" \
>> -        "then env set boot_targets mmc0; fi;" \
>>           "run distro_bootcmd;" \
>>       "fi;\0"
> 
> 
> Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
> 
> 
> Sorry to break your board, but I assumed the "stm32mp1.h" was only the 
> default ST configuration

No worries really. No, it isn't only for the ST evalboards, and I don't 
know whether this is the right approach.

> for ST boards and other boards need to be overridden it if it is not 
> align with their needs
> 
> as I don't know the expected boot sequence.
> 
> for example with:
> 
>      CONFIG_BOOTCOMMAND="run bootcmd_dh_stm32mp"
> 
> with bootcmd_dh_stm32mp to be defined
> 
> 
> but today this file is a mix between SOC configuration (generic) and ST 
> boards needs.
> 
> 
> So I will merge your revert and I will push a other solution to only 
> support SD card
> 
> after NOR in bootcmd_stm32mp but only for STMicroelectronics boards
> 
> (because the revert now break the EV1 boot from NOR).
> 
> 1) stm32mp1.h = common for SOC STMP15x (as today)
> 
> 2) st_stm32mp1.h = ST boards configuration (override common)

Maybe the naming should be the other way around, so we have some sort of 
namespacing, like this:

stm32mp1_common.h
... common stuff for all boards and SoM and all ...

stm32mp1_st_evalboard.h (or whatever you want to call it)
#include <configs/stm32mp1_common.h>
#define custom ST env ...

stm32mp1_dh_dhsom.h
#include <configs/stm32mp1_common.h>
#define custom DH env ...

That's what imx does , except for the namespacing, so the file names are 
a mess. I think we can do better there. So anything related to stm32mp1 
should have stm32mp1_* filename prefix, and then some vendor_ or board_ 
suffix.