On Wed, Oct 07, 2020 at 08:37:57AM +0000, Patrick DELAUNAY wrote:
> Hi,
> 
> > From: U-Boot <[email protected]> On Behalf Of Peter Robinson
> > Sent: mardi 29 septembre 2020 11:48
> > 
> > The 44758771ee commit removes CONFIG_PREBOOT but actually sets the
> > USE_PREBOOT Kconfig option which isn't CONFIG_PREBOOT and is also a bool
> > option which means we regress because 'usb start' isn't run when expected, 
> > it
> > should also be run for devices that have USB storage because keyboards 
> > aren't
> > the only thing we might need the USB bus for.
> > 
> > Fixes: 44758771ee ("arm: move CONFIG_PREBOOT="usb start" to KConfig")
> > Signed-off-by: Peter Robinson <[email protected]>
> > Cc: Jonas Smedegaard <[email protected]>
> > Cc: Neil Armstrong <[email protected]>
> > ---
> >  common/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/Kconfig b/common/Kconfig index b1934b3a9c..9c20a9738e
> > 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -403,7 +403,6 @@ config BOOTCOMMAND
> > 
> >  config USE_PREBOOT
> >     bool "Enable preboot"
> > -   default "usb start" if USB_KEYBOARD
> >     help
> >       When this option is enabled, the existence of the environment
> >       variable "preboot" will be checked immediately before starting the @@ 
> > -
> > 417,6 +416,7 @@ config USE_PREBOOT  config PREBOOT
> >     string "preboot default value"
> >     depends on USE_PREBOOT && !USE_DEFAULT_ENV_FILE
> > +   default "usb start" if USB_KEYBOARD || USB_STORAGE
> >     default ""
> >     help
> >       This is the default of "preboot" environment variable.
> > --
> > 2.26.2
> 
> For information, this patch cause unexpected 'usb start' on STM32MP15x boards
> and slow down the start-up in realease v2020.10.
> 
> For me it is unexpected because
> - USB keyboard is not activated
> - USB storage is activated but USB boot is not supported (not managed by 
> distro boot command)
> 
> I sent a patch [1] for the associated defconfig but I'm afraid that other 
> boards are impacted.
> 
> As the USB storage boot initialization is correctly managed by distro boot 
> command 'usb_boot' 
> (defined in include/config_distro_bootcmd.h, it already include 'usb start'), 
> I think that the 
> USB_STORAGE test should be removed or limited by !DISTRO_DEFAULTS.
> 
> [1] = "configs: stm32mp: force empty PREBOOT"
> http://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

Re-re-reading everything and this is a helpful explanation.  This commit
is wrong as it did more than just fix 44758771ee, which put the default
in the wrong place, but added new logic that shouldn't be required.

Patrick, can you please send a new patch to fix this commit and in turn
NOT also default usb start on USB_STORAGE, only USB_KEYBOARD?  Thanks!

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to