[U-Boot] [PATCH v3 1/2] console: usbkbd: Improve TFTP booting performance
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.
Signed-off-by: Jim Lin jilin@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.
common/usb_kbd.c | 20 ++++++++++++++++++++ doc/README.usb | 15 ++++++++++++++- 2 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 3174b5e..25bf677 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -121,6 +121,11 @@ struct usb_kbd_pdata { uint8_t flags; };
+#ifdef CONFIG_USBKB_TESTC_PERIOD +/* The period of time between two calls of usb_kbd_testc(). */ +static unsigned long kbd_testc_tms; +#endif + /* Generic keyboard event polling. */ void usb_kbd_generic_poll(void) { @@ -366,6 +371,21 @@ static int usb_kbd_testc(void) struct usb_device *usb_kbd_dev; struct usb_kbd_pdata *data;
+#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 dev = stdio_get_by_name(DEVNAME); usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr; diff --git a/doc/README.usb b/doc/README.usb index b4c3ef5..ff4ee6f 100644 --- a/doc/README.usb +++ b/doc/README.usb @@ -80,7 +80,20 @@ CONFIG_USB_UHCI defines the lowlevel part.A lowlevel part must be defined CONFIG_USB_KEYBOARD enables the USB Keyboard CONFIG_USB_STORAGE enables the USB storage devices CONFIG_USB_HOST_ETHER enables USB ethernet adapter support - +CONFIG_USBKB_TESTC_PERIOD defines the average time (in ms) between two calls + of usb_kbd_testc() when TFTP boot is not running + In u-boot console, usb_kbd_testc() will be called + periodically. + When this configuration is defined and other task like + TFTP boot is running, USB keyboard will be polled less + frequently and will be polled every second. + This is to improve TFTP booting performance when USB + keyboard is installed. + In u-boot console, no impact on keyboard input if TFTP + boot is not running. + Example: + Define the following in configuration header file + #define CONFIG_USBKB_TESTC_PERIOD 100
USB Host Networking ===================
Add CONFIG_USBKB_TESTC_PERIOD configuration if CONFIG_USB_KEYBOARD is defined in platform configuration file.
Signed-off-by: Jim Lin jilin@nvidia.com --- Changes in v2: - None Changes in v3: - Add CONFIG_USBKB_TESTC_PERIOD in include/configs/tegra-common-post.h
include/configs/tegra-common-post.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index 6ed2fde..ee341ef 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -128,6 +128,7 @@ #ifdef CONFIG_USB_KEYBOARD #define STDIN_KBD_USB ",usbkbd" #define CONFIG_SYS_USB_EVENT_POLL +#define CONFIG_USBKB_TESTC_PERIOD 100 #define CONFIG_PREBOOT "usb start" #else #define STDIN_KBD_USB ""
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.
It's quite possible that "some other task" is extremely fine-grained, and calls usb_kbd_testc() every 0.1ms, and would be severely negatively affected by usb_kbd_testc() taking a long time to execute.
Conversly, it's quite possible that "some other task" is quite granular, and calls usb_kbd_testc() a wide intervals, say every 200ms.
So, I think this change keys of entirely the wrong thing.
Shouldn't the TFTP process (or use of USB networking?) or other long-running tasks that do check for keyboard IO simply set some flag to indicate to usb_kbd_testc() that it should run at a reduced rate, or even just have those long-running processses call usb_kbd_testc() at a reduced rate themselves?
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.
It's quite possible that "some other task" is extremely fine-grained, and calls usb_kbd_testc() every 0.1ms, and would be severely negatively affected by usb_kbd_testc() taking a long time to execute.
Conversly, it's quite possible that "some other task" is quite granular, and calls usb_kbd_testc() a wide intervals, say every 200ms.
So, I think this change keys of entirely the wrong thing.
Shouldn't the TFTP process (or use of USB networking?) or other long-running tasks that do check for keyboard IO simply set some flag to indicate to usb_kbd_testc() that it should run at a reduced rate, or even just have those long-running processses call usb_kbd_testc() at a reduced rate themselves?
To fix in usb_kbd_testc() is easier because this issue happens only when USB keyboard is installed and CONFIG_USB_KEYBOARD is defined.
-- nvpublic
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?
On Fri, 2013-06-28 at 13:09 +0800, Stephen Warren wrote:
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
Good idea and will be introduced in next revision.
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?
They should react at same rate.
-- nvpublic
participants (2)
-
Jim Lin -
Stephen Warren