On Tue, 2013-07-09 at 06:17 +0800, Joe Hershberger wrote: > Hi Jim and Stephen, > > On Wed, Jul 3, 2013 at 11:01 PM, Jim Lin <ji...@nvidia.com> wrote: > > TFTP booting is slow when a USB keyboard is installed and > > CONFIG_USB_KEYBOARD is defined. > > This fix is to change Ctrl-C polling to every second when NET transfer > > is running. > > > > Signed-off-by: Jim Lin <ji...@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. > > > > net/net.c | 22 ++++++++++++++++++++++ > > 1 files changed, 22 insertions(+), 0 deletions(-) > > > > diff --git a/net/net.c b/net/net.c > > index df94789..ec88b02 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -322,6 +322,11 @@ int NetLoop(enum proto_t protocol) > > { > > bd_t *bd = gd->bd; > > int ret = -1; > > +#ifdef CONFIG_USB_KEYBOARD > > + unsigned long ctrlc_t_start; > > + unsigned long ctrlc_t; > > + int ctrlc_result; > > +#endif > > > > NetRestarted = 0; > > NetDevExists = 0; > > @@ -472,7 +477,24 @@ restart: > > /* > > * Abort if ctrl-c was pressed. > > */ > > +#ifdef CONFIG_USB_KEYBOARD > > It seems this is the result of the USB Keyboard behavior. Why is it a > good idea to litter the TFTP code with this unrelated code? It seems
So far this is the best place to resolve this issue. > the very same check could be down inside of ctrlc() somewhere that is > at least console I/O related. Besides, having it in a common place > will allow any operation that accesses the keyboard to benefit from > not hanging up on slow USB stuff. > > It also seems that it should depend on what the actual source of the > stdin is, not just if you compiled in CONFIG_USB_KEYBOARD support. This issue only goes with USB keyboard installed and CONFIG_USB_KEYBOARD defined. Therefore compiled in CONFIG_USB_KEYBOARD support. Non-usb-keyboard doesn't have such issue. > Again, something that belongs in the console source. > > > + /* > > + * Reduce ctrl-c checking to 1 second once > > + * to improve TFTP boot performance. > > + */ > > + if (ctrlc_t_start > get_timer(0)) > > + ctrlc_t_start = get_timer(0); > > + ctrlc_t = get_timer(0) - ctrlc_t_start; > > Why is it preferable to do the subtraction yourself instead of letting > get_timer() do it? I.e. what compelled did you change this from v4? As Wolfgang Denk said in another mail, "An exception is "arch/arm/cpu/sa1100/timer.c" which does not respect the "base" argument at all, i. e. which is broken. ". So this v5 patch uses get_timer(0), like other code did in this file. > > > + if (ctrlc_t > CONFIG_SYS_HZ) { > > Why is hard-coding it to 1 second a good idea? > Is that really how unresponsive it has to be to not > significantly impact TFTP boot time? Do you want me to add a CONFIG setting to have this time adjustable? I was thinking "1 second" checking on Ctrl-C should be fine while TFT boot is running. -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot