[U-Boot] [PATCH v7 1/2] NET: Add net_busy_flag if CONFIG_USB_KEYBOARD is defined

[U-Boot] [PATCH v7 1/2] NET: Add net_busy_flag if CONFIG_USB_KEYBOARD is defined

[Jim Lin]

This flag is to have console aware that NET transfer is running or not.

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. Add CONFIG_USB_KEYBOARD

 net/net.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/net/net.c b/net/net.c
index df94789..38f7bf7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -206,6 +206,9 @@ uchar *NetTxPacket;
 static int net_check_prereq(enum proto_t protocol);
 
 static int NetTryCount;
+#ifdef CONFIG_USB_KEYBOARD
+int net_busy_flag;
+#endif
 
 /**********************************************************************/
 
@@ -341,6 +344,9 @@ int NetLoop(enum proto_t protocol)
 		eth_init_state_only(bd);
 
 restart:
+#ifdef CONFIG_USB_KEYBOARD
+	net_busy_flag = 0;
+#endif
 	net_set_state(NETLOOP_CONTINUE);
 
 	/*
@@ -453,6 +459,9 @@ restart:
 		status_led_set(STATUS_LED_RED, STATUS_LED_ON);
 #endif /* CONFIG_SYS_FAULT_ECHO_LINK_DOWN, ... */
 #endif /* CONFIG_MII, ... */
+#ifdef CONFIG_USB_KEYBOARD
+	net_busy_flag = 1;
+#endif
 
 	/*
 	 *	Main packet reception loop.  Loop receiving packets until
@@ -558,6 +567,9 @@ restart:
 	}
 
 done:
+#ifdef CONFIG_USB_KEYBOARD
+	net_busy_flag = 0;
+#endif
 #ifdef CONFIG_CMD_TFTPPUT
 	/* Clear out the handlers */
 	net_set_udp_handler(NULL);
-- 
1.7.7

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[Jim Lin]

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(-)

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 */
-- 
1.7.7

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[Wolfgang Denk]

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

-- 
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
A weak mind is like a microscope, which magnifies trifling things,
but cannot receive great ones.      -- Philip Earl of Chesterfield

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[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.




-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[Marek Vasut]

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

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[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.
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

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[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
> > > > 
> > > >  /* 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

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[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
> > > > 
> > > >  /* 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

[U-Boot] [PATCH v7 2/2] console: usb: kbd: To improve TFTP booting performance

[Marek Vasut]

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