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

[Stephen Warren]

On 07/03/2013 05:20 AM, Wolfgang Denk wrote:
> Dear Jim Lin,
> 
> In message <1372847667-31928-1-git-send-email-jilin at nvidia.com> you wrote:
>> TFTP booting is slow when a USB keyboard is installed and
>> CONFIG_USB_KEYBOARD is defined.
>> The fix is to change Ctrl-C polling to every second when NET transfer
>> is running.
> 
> I'm not sure if we can accept this implementation.
> 
>> +#ifdef CONFIG_USB_KEYBOARD
>> +		/*
>> +		 * Reduce ctrl-c checking to 1 second once
>> +		 * to improve TFTP boot performance.
>> +		 */
>> +		ctrlc_t = get_timer(kbd_ctrlc_tms);
>> +		if (ctrlc_t > CONFIG_SYS_HZ) {
>> +			ctrlc_result = ctrlc();
>> +			kbd_ctrlc_tms = get_timer(0);
>> +		} else {
>> +			ctrlc_result = 0;
>> +		}
>> +		if (ctrlc_result) {
>> +#else
> 
> get_timer() is used by a number of network related services.  For
> information, just grep for it in the net/ and drivers/net/
> directories.  The "get_timer(0)" used in your code resets a global
> resource, and has thus the potential of messing up a number of
> timeouts running elsewhere in the network code.  I wonder to which
> extend this has actually been considered (and tested) ?

I recall you mentioning this before, but can you expand on this a bit
please?

For the two platforms I'm familiar with (Tegra and BCM2835), the
implementation of get_timer() is simply:

unlong get_timer(ulong base)
{
    ulong time = read_hw_register()
    time -= base;
    return time;
}

There's no global state involved. Is this implementation of get_timer()
wrong somehow? I'm having a hard time envisaging what kind of global
state it's supposed to maintain.

I always thought that every user of get_timer() was supposed to do
something like:

ulong base = get_timer(0);
... work to be timed
ulong time_diff = get_timer(base);

in other words, every user maintains their own base variable, and hence
get_timer(0) doesn't affect any other users.