All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v6 1/2] console: usb: kbd: To improve TFTP booting performance
Date: Fri, 19 Jul 2013 08:43:27 +0200	[thread overview]
Message-ID: <20130719064327.0CF07380ADF@gemini.denx.de> (raw)
In-Reply-To: <1374156931-1718-1-git-send-email-jilin@nvidia.com>

Dear Jim Lin,

In message <1374156931-1718-1-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.
> 
> Signed-off-by: Jim Lin <jilin@nvidia.com>

The sequence of your patches is wrong; you must reverse it: as is,
with only patch 1/2 applied, you will get compile errors because
net_busy_flag is undefined.  This breaks bisectability.

Please switch the order of your patches.


> +	/*
> +	 * 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) {
> +		if (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ)
> +			return 0;
> +		else
> +			kbd_testc_tms = get_timer(0);
> +	}

When first entering this code, the variable kbd_testc_tms is
(implicitly) initialized as zero;  later, for example when running
multiple network commands, the last used value (i. e. a random number)
is used.  So strictly speaking the comment above is incorrect, as you
don't test for key presses "every second" - the first test may happen
much earlier (even immediately).  I think this should be explained in
the comment to prevent incorrect expectations on the behaviour.



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Eureka!                                                 -- Archimedes

      parent reply	other threads:[~2013-07-19  6:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-18 14:15 [U-Boot] [PATCH v6 1/2] console: usb: kbd: To improve TFTP booting performance Jim Lin
2013-07-18 14:15 ` [U-Boot] [PATCH v6 2/2] NET: Add net_busy_flag Jim Lin
2013-07-18 17:32   ` Stephen Warren
2013-07-18 23:37     ` Marek Vasut
2013-07-18 14:24 ` [U-Boot] [PATCH v6 1/2] console: usb: kbd: To improve TFTP booting performance Marek Vasut
2013-07-18 17:29 ` Stephen Warren
2013-07-19  6:43 ` Wolfgang Denk [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130719064327.0CF07380ADF@gemini.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.