[PATCH] ARM: stm32: Increase USB power-good delay on DHSOM

[Marek Vasut]

On 11/15/21 12:18 PM, Patrick DELAUNAY wrote:

Hi,

[...]

>> diff --git a/include/configs/stm32mp15_common.h 
>> b/include/configs/stm32mp15_common.h
>> index 4e2cabff2e7..be22d3ea7f1 100644
>> --- a/include/configs/stm32mp15_common.h
>> +++ b/include/configs/stm32mp15_common.h
>> @@ -169,7 +169,8 @@
>>       STM32MP_BOOTCMD \
>>       STM32MP_PARTS_DEFAULT \
>>       BOOTENV \
>> -    STM32MP_EXTRA
>> +    STM32MP_EXTRA \
>> +    STM32MP_BOARD_EXTRA_ENV
> 
> I think you that patch failed for the board which include 
> "stm32mp15_common.h" only
> 
> STM32MP_BOARD_EXTRA_ENV is not define
> 
> => board/engicam/stm32mp1/Kconfig:10:    default "stm32mp15_common"
> 
> at minimal, you need to add:
> 
> #ifndef STM32MP_BOARD_EXTRA_ENV
> 
> #defineSTM32MP_BOARD_EXTRA_ENV
> #endif
> 
> 
> But for my point of view CONFIG_EXTRA_ENV_SETTINGS is already a 
> "default" value
> 
> => it can be override in board (after #undef)
> 
> or the overidde should be also managed by  STM32MP_EXTRA, if we add in 
> the common file:
> 
> 
> #ifndef STM32MP_EXTRA
> 
> #defineSTM32MP_EXTRA \
> "altbootcmd=run bootcmd\0"\
> "env_check=if env info -p -d -q; then env save; fi\0"\
> "boot_net_usb_start=true\0"
> 
> #endif

Except in either case, you end up with a lot of duplication.

>>   #endif /* ifndef CONFIG_SPL_BUILD */
>>   #endif /* ifdef CONFIG_DISTRO_DEFAULTS*/
>> diff --git a/include/configs/stm32mp15_dh_dhsom.h 
>> b/include/configs/stm32mp15_dh_dhsom.h
>> index c559cd72da7..bac9e8388a7 100644
>> --- a/include/configs/stm32mp15_dh_dhsom.h
>> +++ b/include/configs/stm32mp15_dh_dhsom.h
>> @@ -8,6 +8,9 @@
>>   #ifndef __CONFIG_STM32MP15_DH_DHSOM_H__
>>   #define __CONFIG_STM32MP15_DH_DHSOM_H__
>> +#define STM32MP_BOARD_EXTRA_ENV \
>> +    "usb_pgood_delay=1000\0"
>> +
>>   #include <configs/stm32mp15_common.h>
> 
> proposal 1: overiddeCONFIG_EXTRA_ENV_SETTINGS as it is done in 
> stm32mp15_st_common.h
> 
> #include <configs/stm32mp15_common.h>
> 
> #define STM32MP_DH_EXTRA_ENV \
>      "usb_pgood_delay=1000\0"
> 
> #undefCONFIG_EXTRA_ENV_SETTINGS
> #defineCONFIG_EXTRA_ENV_SETTINGS \
> STM32MP_MEM_LAYOUT \
> ST_STM32MP1_BOOTCMD \
> STM32MP_PARTS_DEFAULT \
> BOOTENV \
> STM32MP_EXTRA \
> 
>          STM32MP_DH_EXTRA_ENV
> 
> or
> 
> #include <configs/stm32mp15_common.h>
> 
> #undefCONFIG_EXTRA_ENV_SETTINGS
> 
> #defineCONFIG_EXTRA_ENV_SETTINGS \
> STM32MP_MEM_LAYOUT \
> ST_STM32MP1_BOOTCMD \
> STM32MP_PARTS_DEFAULT \
> BOOTENV \
> STM32MP_EXTRA \
> "usb_pgood_delay=1000\0"
> 
> 
> proposal 2: you can overrideSTM32MP_EXTRA
> #defineSTM32MP_EXTRA \
> 
> "altbootcmd=run bootcmd\0"\
> "env_check=if env info -p -d -q; then env save; fi\0"\
> "boot_net_usb_start=true\0" \
> 
>      "usb_pgood_delay=1000\0"
> 
> #include <configs/stm32mp15_common.h>
> open point : do you really need "altbootcmd" ? "boot_net_usb_start" 
> ?"env_check" ?

altbootcmd is for when bootcounter reaches bootlimit, so yes.
boot_net_usb_start, depends on board.

[...]