From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Thu, 27 Jun 2013 23:09:51 -0600
Subject: [U-Boot] [PATCH v3 1/2] console: usbkbd: Improve TFTP booting
	performance
In-Reply-To: <1372391947.2491.31.camel@jilin-desktop>
References: <1372326323-8661-1-git-send-email-jilin@nvidia.com>
	<51CC8254.2060409@wwwdotorg.org>
	<1372391947.2491.31.camel@jilin-desktop>
Message-ID: <51CD1A9F.5060900@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 06/27/2013 09:59 PM, Jim Lin wrote:
> On Fri, 2013-06-28 at 02:20 +0800, Stephen Warren wrote:
>> On 06/27/2013 03:45 AM, Jim Lin wrote:
>>> TFTP booting is observed a little bit slow, especially when a USB
>>> keyboard is installed.
>>> The fix is to move polling to every second if we sense that other task
>>> like TFTP boot is running.
>>>
>>
>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>
>>> +#ifdef CONFIG_USBKB_TESTC_PERIOD
>>> +	/*
>>> +	 * T is the time between two calls of usb_kbd_testc().
>>> +	 * If CONFIG_USBKB_TESTC_PERIOD ms < T < 1000 ms,
>>> +	 * it implies other task like TFTP boot is running,
>>> +	 * then we reduce polling to every second
>>> +	 * to improve TFTP booting performance.
>>> +	 */
>>> +	if ((get_timer(kbd_testc_tms) >=
>>> +	    (CONFIG_USBKB_TESTC_PERIOD * CONFIG_SYS_HZ / 1000)) &&
>>> +	    (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
>>> +		return 0;
>>> +	else
>>> +		kbd_testc_tms = get_timer(0);
>>> +#endif
>>
>> I have a hard time understanding why the fact that "some other task is
>> running" implies anything at all re: how often usb_kbd_testc() would be
>> called.
> In my case it takes about 95 ms on Tegra20 and Tegra114 for
> usb_kbd_testc() to be called periodically.
> So I set CONFIG_USBKB_TESTC_PERIOD to 100.
> Like I said, if CONFIG_USBKB_TESTC_PERIOD ms < T < 1000 ms
> we reduce polling (send command to USB keyboard to check is there
> any key pressed) to every second.

OK, so I think how this works is: If nothing is happening, then
usb_kbd_testc() is repeatedly called back-to-back with no delay between.
So, if the time between two calls to usb_kbd_testc() is much longer than
the time it takes to execute it once, then something else is going on,
and hence the code should skip some calls to usb_kbd_testc().

If that's how this works, then why require CONFIG_USBKBD_TESTC_PERIOD to
be set? Why not simply measure the time between when usb_kbd_testc()
returns, and when it is re-entered? If it's very short, nothing else is
happening. If it's very long, something else is happening. That is a far
more direct measurement, and is immune to e.g. CPU frequency differences
in a way that a static value for CONFIG_USBKBD_TESTC_PERIOD is not.

Also, any kind of time measurement doesn't solve the issue I mentioned
re: how granular the other task is.

Finally, if you're sitting at the command-prompt, is usb_kbd_testc()
used at all? How does regular typing using a USB keyboard interact with
this code; will typing react fast, but CTRL-C react slowly?