Patchwork [U-Boot,1/1] console: Improve TFTP booting performance

login
register
mail settings
Submitter Jim Lin
Date Jan. 25, 2013, 7:03 a.m.
Message ID <1359097425-20517-1-git-send-email-jilin@nvidia.com>
Download mbox | patch
Permalink /patch/215564/
State Changes Requested
Delegated to: Wolfgang Denk
Headers show

Comments

Jim Lin - Jan. 25, 2013, 7:03 a.m.
TFTP booting is observed a little bit slow, especially when a USB
keyboard is installed.
The fix is to check whether Ctrl-C key is pressed every
CONFIG_CTRLC_POLL_MS ms.
If CONFIG_CTRLC_POLL_MS is not defined in configuration file, we
define it as 1000.

Signed-off-by: Jim Lin <jilin@nvidia.com>
---
 common/console.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
Wolfgang Denk - Jan. 25, 2013, 9:18 a.m.
Dear Jim Lin,

In message <1359097425-20517-1-git-send-email-jilin@nvidia.com> you wrote:
> TFTP booting is observed a little bit slow, especially when a USB
> keyboard is installed.
> The fix is to check whether Ctrl-C key is pressed every
> CONFIG_CTRLC_POLL_MS ms.
> If CONFIG_CTRLC_POLL_MS is not defined in configuration file, we
> define it as 1000.

CONFIG_* variables MUST be documented in the README.

Best regards,

Wolfgang Denk
Wolfgang Denk - Jan. 25, 2013, 9:20 a.m.
Dear Jim Lin,

In message <1359097425-20517-1-git-send-email-jilin@nvidia.com> you wrote:
> TFTP booting is observed a little bit slow, especially when a USB
> keyboard is installed.
> The fix is to check whether Ctrl-C key is pressed every
> CONFIG_CTRLC_POLL_MS ms.
> If CONFIG_CTRLC_POLL_MS is not defined in configuration file, we
> define it as 1000.

...also:

> +#ifndef CONFIG_CTRLC_POLL_MS
> +/*
> + * Process Ctrl-C every 1000 ms by default to improve performance
> + * (like TFTP boot) when interlaced with other tasks.
> + */
> +#define CONFIG_CTRLC_POLL_MS 1000
> +#endif
> +static unsigned long ctrlc_ts = CONFIG_CTRLC_POLL_MS;

Don't set such a default.  If this is good for you, OK.  But for all
others, the behaviour should not change at all.


Also, are you positively sure that your callto get_timer() does not
mess up with other timers in the network subsystem?

Best regards,

Wolfgang Denk
Jim Lin - Jan. 29, 2013, 6:35 a.m.
On Fri, 2013-01-25 at 17:20 +0800, Wolfgang Denk wrote:
> Dear Jim Lin,
> 
> In message <1359097425-20517-1-git-send-email-jilin@nvidia.com> you wrote:
> > TFTP booting is observed a little bit slow, especially when a USB
> > keyboard is installed.
> > The fix is to check whether Ctrl-C key is pressed every
> > CONFIG_CTRLC_POLL_MS ms.
> > If CONFIG_CTRLC_POLL_MS is not defined in configuration file, we
> > define it as 1000.
> 
> ...also:
> 
> > +#ifndef CONFIG_CTRLC_POLL_MS
> > +/*
> > + * Process Ctrl-C every 1000 ms by default to improve performance
> > + * (like TFTP boot) when interlaced with other tasks.
> > + */
> > +#define CONFIG_CTRLC_POLL_MS 1000
> > +#endif
> > +static unsigned long ctrlc_ts = CONFIG_CTRLC_POLL_MS;
> 
> Don't set such a default.  If this is good for you, OK.  But for all
> others, the behaviour should not change at all.
> 
> 
> Also, are you positively sure that your callto get_timer() does not
> mess up with other timers in the network subsystem?
> 
OK. I will rewrite and check again.
Thanks.

--nvpublic

Patch

diff --git a/common/console.c b/common/console.c
index bf73178..3339a93 100644
--- a/common/console.c
+++ b/common/console.c
@@ -524,9 +524,21 @@  int vprintf(const char *fmt, va_list args)
 /* test if ctrl-c was pressed */
 static int ctrlc_disabled = 0;	/* see disable_ctrl() */
 static int ctrlc_was_pressed = 0;
+#ifndef CONFIG_CTRLC_POLL_MS
+/*
+ * Process Ctrl-C every 1000 ms by default to improve performance
+ * (like TFTP boot) when interlaced with other tasks.
+ */
+#define CONFIG_CTRLC_POLL_MS 1000
+#endif
+static unsigned long ctrlc_ts = CONFIG_CTRLC_POLL_MS;
 int ctrlc(void)
 {
 	if (!ctrlc_disabled && gd->have_console) {
+		if (get_timer(ctrlc_ts) < CONFIG_CTRLC_POLL_MS)
+			return 0;
+		else
+			ctrlc_ts = get_timer(0);
 		if (tstc()) {
 			switch (getc()) {
 			case 0x03:		/* ^C - Control C */