[U-Boot] [PATCH v6 1/2] console: usb: kbd: To improve TFTP booting performance

[Stephen Warren]

On 07/18/2013 08:15 AM, Jim Lin wrote:
> TFTP booting is slow when a USB keyboard is installed and
> stdin has usbkbd added.
> This fix is to change Ctrl-C polling for USB keyboard to every second
> when NET transfer is running.

I think this general approach is a reasonable compromise to the problem.

Some issues need to be considered:

1) git bisect is broken as Marek pointed out.

2) Does the code still compile/link if USB keyboard and/or networking
support is not enabled? I suspect it won't in the case where USB
keyboard is enabled, yet networking support isn't. This probably needs
to be solved by putting "int net_busy_flag;" into some common
non-optional file so that all the users of that variable don't have to
be chronically ifdef'd for all the possible feature combinations.

3) Is "net_busy_flag" the best name for the variable? Perhaps the flag
could be used to disable other time-consuming polling beyond just USB
keyboard CTRL-C handling. Perhaps other operations besides networking
transfers could benefit from setting this flag. At the risk of
bike-shedding, how about renaming this one of:

reduce_non_critical_polling_frequency
non_interactive_operation_in_progress

?

A comment on the code below:

> diff --git a/common/usb_kbd.c b/common/usb_kbd.c

> @@ -366,6 +369,18 @@ static int usb_kbd_testc(void)
>  	struct usb_device *usb_kbd_dev;
>  	struct usb_kbd_pdata *data;
>  
> +	/*
> +	 * If net_busy_flag is 1, NET transfer is running,
> +	 * then we check key pressed every second to improve
> +	 * TFTP booting performance.
> +	 */
> +	if (net_busy_flag) {
> +		if (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)
> +			return 0;
> +		else
> +			kbd_testc_tms = get_timer(0);
> +	}

I think you always want to assign to kbd_testc_tms so it always records
the most recent time of the most recent USB keyboard activity, not the
most recent USB keyboard activity while a network operation was in
progress. So,

if (net_busy_flag && get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)
    return 0;
kbd_testc_tms = get_timer(0);