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

[Patrice CHOTARD]

Hi Marek

On 11/15/21 1:59 PM, Marek Vasut wrote:
> 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.
> 
> [...]
Applied to u-boot-stm32/next

Thanks
Patrice