NET: Add net_busy_flag if CONFIG_USB_KEYBOARD is defined

PATCH Superseded Jim Lin 3 participants 9 messages Jul 19, 2013 – Jul 31, 2013 Networking

lore

Share & tools

lore

Jim Lin

author

2013-07-19 09:36 UTC #0 lore raw

PATCH v7 1/2 NET: Add net_busy_flag if CONFIG_USB_KEYBOARD is defined

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

Signed-off-by Jim Lin <[email protected]>

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

net/net.c +12 -0

index df94789..38f7bf7 100644
@@ -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);

Signature · 1 line

1.7.7

Jim Lin

author

2013-07-19 09:36 UTC #1 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

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 <[email protected]>

 common/usb_kbd.c |   14 ++++++++++++++
 include/usb.h    |    4 +++-
 2 files changed, 17 insertions(+), 1 deletions(-)

common/usb_kbd.c +14 -0

index 3174b5e..05e0c7e 100644
@@ -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;

include/usb.h +3 -1

index d7b082d..e1901d1 100644
@@ -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 */

Signature · 1 line

1.7.7

Wolfgang Denk

2013-07-19 11:17 UTC #2 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

Dear Jim Lin,

Quoted 4 lines · In message <[email protected]> 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.

...

Quoted 11 lines

common/usb_kbd.c +4 -0

+#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;

common/usb_kbd.c static int usb_kbd_testc(void) · line 382

Quoted 10 lines

common/usb_kbd.c +10 -0

+#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

Signature · 5 lines

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

Jim Lin

author

2013-07-19 12:06 UTC #3 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

Quoted 19 lines · Wolfgang Denk

Dear Jim Lin,
Quoted 4 lines · In message <[email protected]> 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.
...
Quoted 11 lines
common/usb_kbd.c +4 -0
+#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;
 

Quoted 15 lines

common/usb_kbd.c static int usb_kbd_testc(void) · line 382
Quoted 10 lines
common/usb_kbd.c +10 -0
+#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.
-----------------------------------------------------------------------------------

Marek Vasut

2013-07-19 13:53 UTC #4 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

Dear Jim Lin,

Quoted 40 lines · Wolfgang Denk

Quoted 23 lines
Dear Jim Lin,
Quoted 4 lines · In message <[email protected]> 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.
...
Quoted 13 lines
common/usb_kbd.c +4 -0
+#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;
 
Quoted 15 lines
common/usb_kbd.c static int usb_kbd_testc(void) · line 382
Quoted 10 lines
common/usb_kbd.c +10 -0
+#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.

Quoted 2 lines

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

Jim Lin

author

2013-07-31 10:21 UTC #5 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

Quoted 42 lines · Marek Vasut

Dear Jim Lin,
Quoted 40 lines · Wolfgang Denk
Quoted 38 lines
Dear Jim Lin,
Quoted 4 lines · In message <[email protected]> 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.
...
Quoted 13 lines
common/usb_kbd.c +4 -0
+#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;
 
common/usb_kbd.c static int usb_kbd_testc(void) · line 383
Quoted 10 lines
common/usb_kbd.c +10 -0
+#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.

Quoted 5 lines

Quoted 2 lines
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

Marek Vasut

2013-07-31 10:36 UTC #6 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

Dear Jim Lin,

Quoted 7 lines · Marek Vasut

Quoted 6 lines
Dear Jim Lin,
Quoted 4 lines · Wolfgang Denk
Quoted 3 lines
Dear Jim Lin,
In message <[email protected]> you

Quoted 11 lines · wrote:

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 4 lines
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.
...
common/usb_kbd.c struct usb_kbd_pdata { · line 126
Quoted 4 lines
common/usb_kbd.c +4 -0
+#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 ;-)

[...]

Quoted 9 lines

Quoted 3 lines
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

Jim Lin

author

2013-07-31 10:30 UTC #7 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

In reply to Marek Vasut

Quoted 42 lines · Marek Vasut

Dear Jim Lin,
Quoted 40 lines · Wolfgang Denk
Quoted 38 lines
Dear Jim Lin,
Quoted 4 lines · In message <[email protected]> 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.
...
Quoted 13 lines
common/usb_kbd.c +4 -0
+#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;
 
common/usb_kbd.c static int usb_kbd_testc(void) · line 383
Quoted 10 lines
common/usb_kbd.c +10 -0
+#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.

Quoted 5 lines

Quoted 2 lines
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

Jim Lin

author

2013-07-31 09:48 UTC #8 lore raw

PATCH v7 2/2 console: usb: kbd: To improve TFTP booting performance

In reply to Wolfgang Denk

Quoted 19 lines · Wolfgang Denk

Dear Jim Lin,
Quoted 4 lines · In message <[email protected]> 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.
...
Quoted 11 lines
common/usb_kbd.c +4 -0
+#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;
 

Quoted 15 lines

common/usb_kbd.c static int usb_kbd_testc(void) · line 382
Quoted 10 lines
common/usb_kbd.c +10 -0
+#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