| Message ID | 1374226576-13401-2-git-send-email-jilin@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Jim Lin, In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > TFTP booting is slow when a USB keyboard is installed and > stdin has usbkbd added. > This fix is to change Ctrl-C polling for USB keyboard to every second > when NET transfer is running. ... > +#ifdef CONFIG_CMD_NET > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > struct usb_device *usb_kbd_dev; > struct usb_kbd_pdata *data; > > +#ifdef CONFIG_CMD_NET > + /* > + * If net_busy_flag is 1, NET transfer is running, > + * then we check key pressed every second to improve > + * TFTP booting performance. > + */ > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > + return 0; > + kbd_testc_tms = get_timer(0); > +#endif You did not comment on my remark about kbd_testc_tms being used basically with a random start value for each invocation of a network command. The "every second" above is wrong. The actual interval may be much shorter (even nearly zero, at least once), or longer. BTW: Please fix the list of mail addresses you're sending to, and remove non-existing ones. Best regards, Wolfgang Denk
On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > Dear Jim Lin, > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > TFTP booting is slow when a USB keyboard is installed and > > stdin has usbkbd added. > > This fix is to change Ctrl-C polling for USB keyboard to every second > > when NET transfer is running. > ... > > +#ifdef CONFIG_CMD_NET > > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > > struct usb_device *usb_kbd_dev; > > struct usb_kbd_pdata *data; > > > > +#ifdef CONFIG_CMD_NET > > + /* > > + * If net_busy_flag is 1, NET transfer is running, > > + * then we check key pressed every second to improve > > + * TFTP booting performance. > > + */ > > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > > + return 0; > > + kbd_testc_tms = get_timer(0); > > +#endif > > You did not comment on my remark about kbd_testc_tms being used > basically with a random start value for each invocation of a network > command. The "every second" above is wrong. The actual interval may > be much shorter (even nearly zero, at least once), or longer. Okay, I will check again. ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
Dear Jim Lin, > On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > > Dear Jim Lin, > > > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > > TFTP booting is slow when a USB keyboard is installed and > > > stdin has usbkbd added. > > > This fix is to change Ctrl-C polling for USB keyboard to every second > > > when NET transfer is running. > > > > ... > > > > > +#ifdef CONFIG_CMD_NET > > > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > > > > > > struct usb_device *usb_kbd_dev; > > > struct usb_kbd_pdata *data; > > > > > > +#ifdef CONFIG_CMD_NET > > > + /* > > > + * If net_busy_flag is 1, NET transfer is running, > > > + * then we check key pressed every second to improve > > > + * TFTP booting performance. > > > + */ > > > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > > > + return 0; > > > + kbd_testc_tms = get_timer(0); > > > +#endif > > > > You did not comment on my remark about kbd_testc_tms being used > > basically with a random start value for each invocation of a network > > command. The "every second" above is wrong. The actual interval may > > be much shorter (even nearly zero, at least once), or longer. > > Okay, I will check again. > I'll wait for v3. In the meantime, I homestly dont like having such hacks on both sides, but I dont think much can be done about it. Maybe use __maybe_unused for the net_busy_flag to drop the ifdef ? Best regards, Marek Vasut
On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > Dear Jim Lin, > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > TFTP booting is slow when a USB keyboard is installed and > > stdin has usbkbd added. > > This fix is to change Ctrl-C polling for USB keyboard to every second > > when NET transfer is running. > ... > > +#ifdef CONFIG_CMD_NET > > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > > struct usb_device *usb_kbd_dev; > > struct usb_kbd_pdata *data; > > > > +#ifdef CONFIG_CMD_NET > > + /* > > + * If net_busy_flag is 1, NET transfer is running, > > + * then we check key pressed every second to improve > > + * TFTP booting performance. > > + */ > > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > > + return 0; > > + kbd_testc_tms = get_timer(0); > > +#endif > > You did not comment on my remark about kbd_testc_tms being used > basically with a random start value for each invocation of a network > command. The "every second" above is wrong. The actual interval may > be much shorter (even nearly zero, at least once), or longer. I will change the comment as the following. /* * If net_busy_flag is 1, NET transfer is running, * then we check key pressed every second (first check * may be less than 1 second) * to improve TFTP booting performance. */ The behavior you mentioned is not that critical. --nvpublic
On Fri, 2013-07-19 at 21:53 +0800, Marek Vasut wrote: > Dear Jim Lin, > > > On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > > > Dear Jim Lin, > > > > > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > > > TFTP booting is slow when a USB keyboard is installed and > > > > stdin has usbkbd added. > > > > This fix is to change Ctrl-C polling for USB keyboard to every second > > > > when NET transfer is running. > > > > > > ... > > > > > > > +#ifdef CONFIG_CMD_NET > > > > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > > > > > > > > struct usb_device *usb_kbd_dev; > > > > struct usb_kbd_pdata *data; > > > > > > > > +#ifdef CONFIG_CMD_NET > > > > + /* > > > > + * If net_busy_flag is 1, NET transfer is running, > > > > + * then we check key pressed every second to improve > > > > + * TFTP booting performance. > > > > + */ > > > > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > > > > + return 0; > > > > + kbd_testc_tms = get_timer(0); > > > > +#endif > > > > > > You did not comment on my remark about kbd_testc_tms being used > > > basically with a random start value for each invocation of a network > > > command. The "every second" above is wrong. The actual interval may > > > be much shorter (even nearly zero, at least once), or longer. > > > > Okay, I will check again. > > > I'll wait for v3. In the meantime, I homestly dont like having such hacks on > both sides, but I dont think much can be done about it. Maybe use __maybe_unused > for the net_busy_flag to drop the ifdef ? I don't quite understand what you wanted me to do. Could you give me an example? What to add and what to remove. Thanks. --nvpublic
On Fri, 2013-07-19 at 21:53 +0800, Marek Vasut wrote: > Dear Jim Lin, > > > On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > > > Dear Jim Lin, > > > > > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > > > TFTP booting is slow when a USB keyboard is installed and > > > > stdin has usbkbd added. > > > > This fix is to change Ctrl-C polling for USB keyboard to every second > > > > when NET transfer is running. > > > > > > ... > > > > > > > +#ifdef CONFIG_CMD_NET > > > > +/* 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 +370,16 @@ static int usb_kbd_testc(void) > > > > > > > > struct usb_device *usb_kbd_dev; > > > > struct usb_kbd_pdata *data; > > > > > > > > +#ifdef CONFIG_CMD_NET > > > > + /* > > > > + * If net_busy_flag is 1, NET transfer is running, > > > > + * then we check key pressed every second to improve > > > > + * TFTP booting performance. > > > > + */ > > > > + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) > > > > + return 0; > > > > + kbd_testc_tms = get_timer(0); > > > > +#endif > > > > > > You did not comment on my remark about kbd_testc_tms being used > > > basically with a random start value for each invocation of a network > > > command. The "every second" above is wrong. The actual interval may > > > be much shorter (even nearly zero, at least once), or longer. > > > > Okay, I will check again. > > > I'll wait for v3. In the meantime, I homestly dont like having such hacks on > both sides, but I dont think much can be done about it. Maybe use __maybe_unused > for the net_busy_flag to drop the ifdef ? I don't quite understand what you wanted me to do. Could you give me an example? What to add and what to remove. Thanks. --nvpublic
Dear Jim Lin, > On Fri, 2013-07-19 at 21:53 +0800, Marek Vasut wrote: > > Dear Jim Lin, > > > > > On Fri, 2013-07-19 at 19:17 +0800, Wolfgang Denk wrote: > > > > Dear Jim Lin, > > > > > > > > In message <1374226576-13401-2-git-send-email-jilin@nvidia.com> you wrote: > > > > > TFTP booting is slow when a USB keyboard is installed and > > > > > stdin has usbkbd added. > > > > > This fix is to change Ctrl-C polling for USB keyboard to every > > > > > second when NET transfer is running. > > > > > > > > ... > > > > > > > > > +#ifdef CONFIG_CMD_NET > > > > > +/* The period of time between two calls of usb_kbd_testc(). */ > > > > > +static unsigned long kbd_testc_tms; > > > > > +#endif Just use __maybe_unused here to squash the ifdef ;-) [...] > > I'll wait for v3. In the meantime, I homestly dont like having such hacks > > on both sides, but I dont think much can be done about it. Maybe use > > __maybe_unused for the net_busy_flag to drop the ifdef ? > > I don't quite understand what you wanted me to do. > Could you give me an example? What to add and what to remove. > Thanks. > > --nvpublic Best regards, Marek Vasut
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 3174b5e..05e0c7e 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -120,6 +120,10 @@ struct usb_kbd_pdata { uint8_t flags; }; +#ifdef CONFIG_CMD_NET +/* 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 +370,16 @@ static int usb_kbd_testc(void) struct usb_device *usb_kbd_dev; struct usb_kbd_pdata *data; +#ifdef CONFIG_CMD_NET + /* + * If net_busy_flag is 1, NET transfer is running, + * then we check key pressed every second to improve + * TFTP booting performance. + */ + if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)) + return 0; + 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/include/usb.h b/include/usb.h index d7b082d..e1901d1 100644 --- a/include/usb.h +++ b/include/usb.h @@ -206,7 +206,9 @@ int usb_host_eth_scan(int mode); int drv_usb_kbd_init(void); int usb_kbd_deregister(void); - +#ifdef CONFIG_CMD_NET +extern int net_busy_flag; +#endif #endif /* routines */ int usb_init(void); /* initialize the USB Controller */
TFTP booting is slow when a USB keyboard is installed and stdin has usbkbd added. This fix is to change Ctrl-C polling for USB keyboard to every second when NET transfer 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. 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. Changes in v6: 1. In common/usb_kbd.c, check net_busy_flag to determine whether we poll USB keyboard status. 2. In include/usb.h, add external variable declaration net_busy_flag Changes in v7: 1. In common/usb_kbd.c and include/usb.h, add #ifdef CONFIG_CMD_NET. 2. In common/usb_kbd.c, modify code to get correct time gap. common/usb_kbd.c | 14 ++++++++++++++ include/usb.h | 4 +++- 2 files changed, 17 insertions(+), 1 deletions(-)