[U-Boot] [PATCH v5 1/1] NET: Improve TFTP booting performance when CONFIG_USB_KEYBOARD

[Joe Hershberger]

Hi Jim and Stephen,

On Wed, Jul 3, 2013 at 11:01 PM, Jim Lin <jilin at nvidia.com> wrote:
> TFTP booting is slow when a USB keyboard is installed and
> CONFIG_USB_KEYBOARD is defined.
> This fix is to change Ctrl-C polling to every second when NET transfer
> is running.
>
> Signed-off-by: Jim Lin <jilin at nvidia.com>
> ---
> Changes in v2:
>  1. Change configuration name from CONFIG_CTRLC_POLL_MS to CONFIG_CTRLC_POLL_S.
>  2. New code will be executed only when CONFIG_CTRLC_POLL_S is defined in
>     configuration header file.
>  3. Add description in README.console.
> Changes in v3:
>  1. Move changes to common/usb_kbd.c and doc/README.usb
>  2. Rename config setting to CONFIG_USBKB_TESTC_PERIOD.
>  3. Remove slow response on USB-keyboard input when TFTP boot is not running.
> Changes in v4:
>  1. Remove changes in doc/README.usb, common/usb_kbd.c and
>     CONFIG_USBKB_TESTC_PERIOD
>  2. Modify net/net.c
> Changes in v5:
>  1. Change variable name to ctrlc_t_start.
>  2. Use two calls of get_timer(0) to get time gap.
>
>  net/net.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index df94789..ec88b02 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -322,6 +322,11 @@ int NetLoop(enum proto_t protocol)
>  {
>         bd_t *bd = gd->bd;
>         int ret = -1;
> +#ifdef CONFIG_USB_KEYBOARD
> +       unsigned long ctrlc_t_start;
> +       unsigned long ctrlc_t;
> +       int ctrlc_result;
> +#endif
>
>         NetRestarted = 0;
>         NetDevExists = 0;
> @@ -472,7 +477,24 @@ restart:
>                 /*
>                  *      Abort if ctrl-c was pressed.
>                  */
> +#ifdef CONFIG_USB_KEYBOARD

It seems this is the result of the USB Keyboard behavior.  Why is it a
good idea to litter the TFTP code with this unrelated code?  It seems
the very same check could be down inside of ctrlc() somewhere that is
at least console I/O related.  Besides, having it in a common place
will allow any operation that accesses the keyboard to benefit from
not hanging up on slow USB stuff.

It also seems that it should depend on what the actual source of the
stdin is, not just if you compiled in CONFIG_USB_KEYBOARD support.
Again, something that belongs in the console source.

> +               /*
> +                * Reduce ctrl-c checking to 1 second once
> +                * to improve TFTP boot performance.
> +                */
> +               if (ctrlc_t_start > get_timer(0))
> +                       ctrlc_t_start = get_timer(0);
> +               ctrlc_t = get_timer(0) - ctrlc_t_start;

Why is it preferable to do the subtraction yourself instead of letting
get_timer() do it?  I.e. what compelled did you change this from v4?

> +               if (ctrlc_t > CONFIG_SYS_HZ) {

Why is hard-coding it to 1 second a good idea?  Is that really how
unresponsive it has to be to not significantly impact TFTP boot time?

> +                       ctrlc_result = ctrlc();
> +                       ctrlc_t_start = get_timer(0);
> +               } else {
> +                       ctrlc_result = 0;
> +               }
> +               if (ctrlc_result) {
> +#else
>                 if (ctrlc()) {
> +#endif
>                         /* cancel any ARP that may not have completed */
>                         NetArpWaitPacketIP = 0;
>
> --
> 1.7.7
>