[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Hans de Goede]

Hi Marek & Stephen,

As discussed before we've a problem where our standard bootcmds sometimes
scan usb more then once, causing a large boot delay.

Marek, as discussed with you before, this patch-set tackles this differently
then previous sets, by simply making "usb start" a oneshot command (atleast
until "usb stop" is called).

Stephen the second patch then modifies config_distro_bootcmd.h to simply call
"usb start" unconditionally, since calling it repeatedly now can be done
without side-effects.

Assuming you both like these patches, that leaves the question of how to
merge them, they can be merged individually each through your own trees, or
one of you can pick up both of them, I'll leave that to you.

Regards,

Hans

[U-Boot] [PATCH 1/2] USB: make "usb start" start usb only once

[Hans de Goede]

Currently we've this magic in include/config_distro_bootcmd.h to avoid
scanning the usb bus multiple times.

And it does not work when also using an usb keyboard because then the
preboot command has already scanned the bus, so we're still scanning it
twice.

This commit makes "usb start" only start usb if it is no already started,
allowing us to remove all the magic for it from include/config_distro_bootcmd.h
and just call it unconditionally.

This also causes "usb start" and "usb reset" to actually do what their
different names suggest, rather then both of them doing exactly the same.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/cmd_usb.c | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index c192498..27813f0 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -441,6 +441,26 @@ static int do_usb_stop_keyboard(int force)
 	return 0;
 }
 
+static void do_usb_start(void)
+{
+	bootstage_mark_name(BOOTSTAGE_ID_USB_START, "usb_start");
+
+	if (usb_init() < 0)
+		return;
+
+#ifdef CONFIG_USB_STORAGE
+	/* try to recognize storage devices immediately */
+	usb_stor_curr_dev = usb_stor_scan(1);
+#endif
+#ifdef CONFIG_USB_HOST_ETHER
+	/* try to recognize ethernet devices immediately */
+	usb_ether_curr_dev = usb_host_eth_scan(1);
+#endif
+#ifdef CONFIG_USB_KEYBOARD
+	drv_usb_kbd_init();
+#endif
+}
+
 /******************************************************************************
  * usb command intepreter
  */
@@ -457,26 +477,20 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	if ((strncmp(argv[1], "reset", 5) == 0) ||
-		 (strncmp(argv[1], "start", 5) == 0)) {
-		bootstage_mark_name(BOOTSTAGE_ID_USB_START, "usb_start");
+	if (strncmp(argv[1], "start", 5) == 0) {
+		if (usb_started)
+			return 0; /* Already started */
+		printf("starting USB...\n");
+		do_usb_start();
+		return 0;
+	}
+
+	if (strncmp(argv[1], "reset", 5) == 0) {
+		printf("resetting USB...\n");
 		if (do_usb_stop_keyboard(1) != 0)
 			return 1;
 		usb_stop();
-		printf("(Re)start USB...\n");
-		if (usb_init() >= 0) {
-#ifdef CONFIG_USB_STORAGE
-			/* try to recognize storage devices immediately */
-			usb_stor_curr_dev = usb_stor_scan(1);
-#endif
-#ifdef CONFIG_USB_HOST_ETHER
-			/* try to recognize ethernet devices immediately */
-			usb_ether_curr_dev = usb_host_eth_scan(1);
-#endif
-#ifdef CONFIG_USB_KEYBOARD
-			drv_usb_kbd_init();
-#endif
-		}
+		do_usb_start();
 		return 0;
 	}
 	if (strncmp(argv[1], "stop", 4) == 0) {
-- 
2.1.0

[U-Boot] [PATCH 2/2] config_distro_bootcmd.h: Remove unnecessary magic to avoid repeated USB scans

[Hans de Goede]

Now that "usb start" will only start usb if not already started, we can simply
call "usb start" whenever we (may) need access to usb devices, and it will only
actually scan the bus at the first call.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/config_distro_bootcmd.h | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index be616e8..becbe3f 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -90,15 +90,8 @@
 #endif
 
 #ifdef CONFIG_CMD_USB
-#define BOOTENV_RUN_USB_INIT "run usb_init; "
-#define BOOTENV_SET_USB_NEED_INIT "setenv usb_need_init; "
+#define BOOTENV_RUN_USB_INIT "usb start; "
 #define BOOTENV_SHARED_USB \
-	"usb_init=" \
-		"if ${usb_need_init}; then " \
-			"setenv usb_need_init false; " \
-			"usb start 0; " \
-		"fi\0" \
-	\
 	"usb_boot=" \
 		BOOTENV_RUN_USB_INIT \
 		BOOTENV_SHARED_BLKDEV_BODY(usb)
@@ -106,7 +99,6 @@
 #define BOOTENV_DEV_NAME_USB	BOOTENV_DEV_NAME_BLKDEV
 #else
 #define BOOTENV_RUN_USB_INIT
-#define BOOTENV_SET_USB_NEED_INIT
 #define BOOTENV_SHARED_USB
 #define BOOTENV_DEV_USB \
 	BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB
@@ -202,7 +194,7 @@
 	\
 	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
 	\
-	"bootcmd=" BOOTENV_SET_USB_NEED_INIT BOOTENV_SET_SCSI_NEED_INIT   \
+	"bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                             \
 		"for target in ${boot_targets}; do "                      \
 			"run bootcmd_${target}; "                         \
 		"done\0"
-- 
2.1.0

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Stephen Warren]

On 01/06/2015 06:27 AM, Hans de Goede wrote:
> Hi Marek & Stephen,
>
> As discussed before we've a problem where our standard bootcmds sometimes
> scan usb more then once, causing a large boot delay.
>
> Marek, as discussed with you before, this patch-set tackles this differently
> then previous sets, by simply making "usb start" a oneshot command (atleast
> until "usb stop" is called).
>
> Stephen the second patch then modifies config_distro_bootcmd.h to simply call
> "usb start" unconditionally, since calling it repeatedly now can be done
> without side-effects.
>
> Assuming you both like these patches, that leaves the question of how to
> merge them, they can be merged individually each through your own trees, or
> one of you can pick up both of them, I'll leave that to you.

Users will have to get used to running "usb reset" rather than "usb 
start", but I guess that's fine.

Should we rename "usb reset" to "usb restart"?

Either way, the series,
Acked-by: Stephen Warren <swarren@nvidia.com>

Will you do something similar for SCSI?

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Marek Vasut]

On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
> On 01/06/2015 06:27 AM, Hans de Goede wrote:
> > Hi Marek & Stephen,
> > 
> > As discussed before we've a problem where our standard bootcmds sometimes
> > scan usb more then once, causing a large boot delay.
> > 
> > Marek, as discussed with you before, this patch-set tackles this
> > differently then previous sets, by simply making "usb start" a oneshot
> > command (atleast until "usb stop" is called).
> > 
> > Stephen the second patch then modifies config_distro_bootcmd.h to simply
> > call "usb start" unconditionally, since calling it repeatedly now can be
> > done without side-effects.
> > 
> > Assuming you both like these patches, that leaves the question of how to
> > merge them, they can be merged individually each through your own trees,
> > or one of you can pick up both of them, I'll leave that to you.
> 
> Users will have to get used to running "usb reset" rather than "usb
> start", but I guess that's fine.

Hi!

So why do we not have "usb reset" only, why can we not discard the "usb start" 
altogether?

> Should we rename "usb reset" to "usb restart"?

No, let's not mess with the UI any more than we already did.

> Either way, the series,
> Acked-by: Stephen Warren <swarren@nvidia.com>
> 
> Will you do something similar for SCSI?

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Stephen Warren]

On 01/07/2015 04:35 PM, Marek Vasut wrote:
> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
>> On 01/06/2015 06:27 AM, Hans de Goede wrote:
>>> Hi Marek & Stephen,
>>>
>>> As discussed before we've a problem where our standard bootcmds sometimes
>>> scan usb more then once, causing a large boot delay.
>>>
>>> Marek, as discussed with you before, this patch-set tackles this
>>> differently then previous sets, by simply making "usb start" a oneshot
>>> command (atleast until "usb stop" is called).
>>>
>>> Stephen the second patch then modifies config_distro_bootcmd.h to simply
>>> call "usb start" unconditionally, since calling it repeatedly now can be
>>> done without side-effects.
>>>
>>> Assuming you both like these patches, that leaves the question of how to
>>> merge them, they can be merged individually each through your own trees,
>>> or one of you can pick up both of them, I'll leave that to you.
>>
>> Users will have to get used to running "usb reset" rather than "usb
>> start", but I guess that's fine.
>
> Hi!
>
> So why do we not have "usb reset" only, why can we not discard the "usb start"
> altogether?

The whole point is that we need separate commands for:

* usb start: If USB isn't started, start it and scan the bus, else do 
nothing.

This is used by automatic scripts that want to ensure that USB is 
available, but not force bus rescans over and over.

* usb reset: If USB isn't started, start it. Always rescan the bus.

This is used by the user to force a rescan of the USB bus, if they know 
they've plugged in a new device.

>> Should we rename "usb reset" to "usb restart"?
>
> No, let's not mess with the UI any more than we already did.

Well, it might not be such a bad idea if the semantics are changing, to 
give people a heads-up?

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Hans de Goede]

Hi,

On 08-01-15 01:19, Stephen Warren wrote:
> On 01/07/2015 04:35 PM, Marek Vasut wrote:
>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:

<snip>

>>> Should we rename "usb reset" to "usb restart"?
>>
>> No, let's not mess with the UI any more than we already did.
>
> Well, it might not be such a bad idea if the semantics are changing, to give people a heads-up?

Only the semantics of "usb start" are changing, and you are suggesting
renaming "usb reset" which remains unchanged ...

Regards,

Hans

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Hans de Goede]

Hi,

On 06-01-15 18:02, Stephen Warren wrote:
> On 01/06/2015 06:27 AM, Hans de Goede wrote:
>> Hi Marek & Stephen,
>>
>> As discussed before we've a problem where our standard bootcmds sometimes
>> scan usb more then once, causing a large boot delay.
>>
>> Marek, as discussed with you before, this patch-set tackles this differently
>> then previous sets, by simply making "usb start" a oneshot command (atleast
>> until "usb stop" is called).
>>
>> Stephen the second patch then modifies config_distro_bootcmd.h to simply call
>> "usb start" unconditionally, since calling it repeatedly now can be done
>> without side-effects.
>>
>> Assuming you both like these patches, that leaves the question of how to
>> merge them, they can be merged individually each through your own trees, or
>> one of you can pick up both of them, I'll leave that to you.
>
> Users will have to get used to running "usb reset" rather than "usb start", but I guess that's fine.
>
> Should we rename "usb reset" to "usb restart"?
>
> Either way, the series,
> Acked-by: Stephen Warren <swarren@nvidia.com>
>
> Will you do something similar for SCSI?

I think it would probably make sense to do something similar for SCSI,
but I'm not all that familiar with the SCSI code, and I really don't
have time to work on this for SCSI, so atm I do not plan to work that.

Regards,

Hans

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Stephen Warren]

On 01/08/2015 01:34 AM, Hans de Goede wrote:
> Hi,
>
> On 08-01-15 01:19, Stephen Warren wrote:
>> On 01/07/2015 04:35 PM, Marek Vasut wrote:
>>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
>
> <snip>
>
>>>> Should we rename "usb reset" to "usb restart"?
>>>
>>> No, let's not mess with the UI any more than we already did.
>>
>> Well, it might not be such a bad idea if the semantics are changing,
>> to give people a heads-up?
>
> Only the semantics of "usb start" are changing, and you are suggesting
> renaming "usb reset" which remains unchanged ...

Yes, that's true. So I suppose we should indeed not rename anything.

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Marek Vasut]

On Thursday, January 08, 2015 at 05:16:53 PM, Stephen Warren wrote:
> On 01/08/2015 01:34 AM, Hans de Goede wrote:
> > Hi,

Hi!

> > On 08-01-15 01:19, Stephen Warren wrote:
> >> On 01/07/2015 04:35 PM, Marek Vasut wrote:
> >>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
> > <snip>
> > 
> >>>> Should we rename "usb reset" to "usb restart"?
> >>> 
> >>> No, let's not mess with the UI any more than we already did.
> >> 
> >> Well, it might not be such a bad idea if the semantics are changing,
> >> to give people a heads-up?
> > 
> > Only the semantics of "usb start" are changing, and you are suggesting
> > renaming "usb reset" which remains unchanged ...
> 
> Yes, that's true. So I suppose we should indeed not rename anything.

OK, let's go with this approach now.

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Hans de Goede]

Hi,

On 08-01-15 18:06, Marek Vasut wrote:
> On Thursday, January 08, 2015 at 05:16:53 PM, Stephen Warren wrote:
>> On 01/08/2015 01:34 AM, Hans de Goede wrote:
>>> Hi,
>
> Hi!
>
>>> On 08-01-15 01:19, Stephen Warren wrote:
>>>> On 01/07/2015 04:35 PM, Marek Vasut wrote:
>>>>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
>>> <snip>
>>>
>>>>>> Should we rename "usb reset" to "usb restart"?
>>>>>
>>>>> No, let's not mess with the UI any more than we already did.
>>>>
>>>> Well, it might not be such a bad idea if the semantics are changing,
>>>> to give people a heads-up?
>>>
>>> Only the semantics of "usb start" are changing, and you are suggesting
>>> renaming "usb reset" which remains unchanged ...
>>
>> Yes, that's true. So I suppose we should indeed not rename anything.
>
> OK, let's go with this approach now.

So we are all in agreement, good, so through who's tree are these 2
patches going to go upstream ?  Note I can take them upstream through
the sunxi tree, but the usb tree seems better to me ...

Regards,

Hans

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Marek Vasut]

On Thursday, January 08, 2015 at 06:23:55 PM, Hans de Goede wrote:
> Hi,
> 
> On 08-01-15 18:06, Marek Vasut wrote:
> > On Thursday, January 08, 2015 at 05:16:53 PM, Stephen Warren wrote:
> >> On 01/08/2015 01:34 AM, Hans de Goede wrote:
> >>> Hi,
> > 
> > Hi!
> > 
> >>> On 08-01-15 01:19, Stephen Warren wrote:
> >>>> On 01/07/2015 04:35 PM, Marek Vasut wrote:
> >>>>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
> >>> <snip>
> >>> 
> >>>>>> Should we rename "usb reset" to "usb restart"?
> >>>>> 
> >>>>> No, let's not mess with the UI any more than we already did.
> >>>> 
> >>>> Well, it might not be such a bad idea if the semantics are changing,
> >>>> to give people a heads-up?
> >>> 
> >>> Only the semantics of "usb start" are changing, and you are suggesting
> >>> renaming "usb reset" which remains unchanged ...
> >> 
> >> Yes, that's true. So I suppose we should indeed not rename anything.
> > 
> > OK, let's go with this approach now.
> 
> So we are all in agreement, good, so through who's tree are these 2
> patches going to go upstream ?  Note I can take them upstream through
> the sunxi tree, but the usb tree seems better to me ...

I can pick them, shall I pick them as they are ? Are they for -next or current ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Hans de Goede]

Hi,

On 08-01-15 18:36, Marek Vasut wrote:
> On Thursday, January 08, 2015 at 06:23:55 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 08-01-15 18:06, Marek Vasut wrote:
>>> On Thursday, January 08, 2015 at 05:16:53 PM, Stephen Warren wrote:
>>>> On 01/08/2015 01:34 AM, Hans de Goede wrote:
>>>>> Hi,
>>>
>>> Hi!
>>>
>>>>> On 08-01-15 01:19, Stephen Warren wrote:
>>>>>> On 01/07/2015 04:35 PM, Marek Vasut wrote:
>>>>>>> On Tuesday, January 06, 2015 at 06:02:57 PM, Stephen Warren wrote:
>>>>> <snip>
>>>>>
>>>>>>>> Should we rename "usb reset" to "usb restart"?
>>>>>>>
>>>>>>> No, let's not mess with the UI any more than we already did.
>>>>>>
>>>>>> Well, it might not be such a bad idea if the semantics are changing,
>>>>>> to give people a heads-up?
>>>>>
>>>>> Only the semantics of "usb start" are changing, and you are suggesting
>>>>> renaming "usb reset" which remains unchanged ...
>>>>
>>>> Yes, that's true. So I suppose we should indeed not rename anything.
>>>
>>> OK, let's go with this approach now.
>>
>> So we are all in agreement, good, so through who's tree are these 2
>> patches going to go upstream ?  Note I can take them upstream through
>> the sunxi tree, but the usb tree seems better to me ...
>
> I can pick them, shall I pick them as they are ?

Yes, AFAIK there were no requests for changes.

> Are they for -next or current ?

They are intended for -next, the double usb scan when using a usb keyboard *and*
booting from usb is a nuisance, but not fatal, and very few people actually use
the combo, so given that current is about to be released in a couple of days
lets play it safe :)

Regards,

Hans

[U-Boot] [PATCH 0/2] Fix repeated USB scans problems

[Marek Vasut]

On Thursday, January 08, 2015 at 06:40:19 PM, Hans de Goede wrote:
> Hi,

Hi!

[...]

> >>>>> Only the semantics of "usb start" are changing, and you are
> >>>>> suggesting renaming "usb reset" which remains unchanged ...
> >>>> 
> >>>> Yes, that's true. So I suppose we should indeed not rename anything.
> >>> 
> >>> OK, let's go with this approach now.
> >> 
> >> So we are all in agreement, good, so through who's tree are these 2
> >> patches going to go upstream ?  Note I can take them upstream through
> >> the sunxi tree, but the usb tree seems better to me ...
> > 
> > I can pick them, shall I pick them as they are ?
> 
> Yes, AFAIK there were no requests for changes.
> 
> > Are they for -next or current ?
> 
> They are intended for -next, the double usb scan when using a usb keyboard
> *and* booting from usb is a nuisance, but not fatal, and very few people
> actually use the combo, so given that current is about to be released in a
> couple of days lets play it safe :)

OK, applied both to next. Thanks!

Best regards,
Marek Vasut