On 07/03/2013 05:20 AM, Wolfgang Denk wrote:
> Dear Jim Lin,
>
> In message <1372847667-31928-1-git-send-email-ji...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
