NS16550: trivial code clean for checkpatch
PATCH v4 1/2 NS16550: trivial code clean for checkpatch
This removes most checkpatch warnings from the ns16550 driver and its header.
--- drivers/serial/ns16550.c | 37 ++++++++++++++++++++----------------- include/ns16550.h | 16 ++++++++-------- 2 files changed, 28 insertions(+), 25 deletions(-)
drivers/serial/ns16550.c +20 -17
@@ -17,24 +17,24 @@ UART_FCR_RXSR | \ UART_FCR_TXSR) /* Clear & enable FIFOs */#ifdef CONFIG_SYS_NS16550_PORT_MAPPED -#define serial_out (x,y) outb(x,(ulong)y) -#define serial_in (y) inb((ulong)y) +#define serial_out (x, y) outb(x, (ulong)y) +#define serial_in (y) inb((ulong)y)#elif defined(CONFIG_SYS_NS16550_MEM32 ) && (CONFIG_SYS_NS16550_REG_SIZE >0 ) -#define serial_out (x,y) out_be32(y,x) -#define serial_in (y) in_be32(y) +#define serial_out (x, y) out_be32(y, x) +#define serial_in (y) in_be32(y)#elif defined(CONFIG_SYS_NS16550_MEM32 ) && (CONFIG_SYS_NS16550_REG_SIZE <0 ) -#define serial_out (x,y) out_le32(y,x) -#define serial_in (y) in_le32(y) +#define serial_out (x, y) out_le32(y, x) +#define serial_in (y) in_le32(y)#else -#define serial_out (x,y) writeb(x,y) -#define serial_in (y) readb(y) +#define serial_out (x, y) writeb(x, y) +#define serial_in (y) readb(y)#endif #ifndef CONFIG_SYS_NS16550_IER #define CONFIG_SYS_NS16550_IER 0x00#endif /* CONFIG_SYS_NS16550_IER */ -void NS16550_init (NS16550_t com_port,int baud_divisor) +void NS16550_init (NS16550_t com_port,int baud_divisor) {serial_out (CONFIG_SYS_NS16550_IER , &com_port->ier);#if defined(CONFIG_OMAP ) && !defined(CONFIG_OMAP3_ZOOM2 )
@@ -52,15 +52,17 @@ void NS16550_init (NS16550_t com_port, int baud_divisor)serial_out (UART_LCRVAL , &com_port->lcr);#if defined(CONFIG_OMAP ) && !defined(CONFIG_OMAP3_ZOOM2 )#if defined(CONFIG_APTIX ) -serial_out (3 , &com_port->mdr1);/* /13 mode so Aptix 6MHz can hit 115200 */ +/* /13 mode so Aptix 6MHz can hit 115200 */ +serial_out (3 , &com_port->mdr1);#else -serial_out (0 , &com_port->mdr1);/* /16 is proper to hit 115200 with 48MHz */ +/* /16 is proper to hit 115200 with 48MHz */ +serial_out (0 , &com_port->mdr1);#endif #endif /* CONFIG_OMAP */ }#ifndef CONFIG_NS16550_MIN_FUNCTIONS -void NS16550_reinit (NS16550_t com_port,int baud_divisor) +void NS16550_reinit (NS16550_t com_port,int baud_divisor) {serial_out (CONFIG_SYS_NS16550_IER , &com_port->ier);serial_out (UART_LCR_BKSE |UART_LCRVAL , &com_port->lcr);
@@ -76,9 +78,10 @@ void NS16550_reinit (NS16550_t com_port, int baud_divisor) }#endif /* CONFIG_NS16550_MIN_FUNCTIONS */ -void NS16550_putc (NS16550_t com_port,char c) +void NS16550_putc (NS16550_t com_port,char c) { -while ((serial_in (&com_port->lsr) &UART_LSR_THRE ) ==0 ); +while ((serial_in (&com_port->lsr) &UART_LSR_THRE ) ==0 ) + ;serial_out (c, &com_port->thr);/*
@@ -92,7 +95,7 @@ void NS16550_putc (NS16550_t com_port, char c) }#ifndef CONFIG_NS16550_MIN_FUNCTIONS -char NS16550_getc (NS16550_t com_port) +char NS16550_getc (NS16550_t com_port) {while ((serial_in (&com_port->lsr) &UART_LSR_DR ) ==0 ) {#ifdef CONFIG_USB_TTY
@@ -104,9 +107,9 @@ char NS16550_getc (NS16550_t com_port) return serial_in(&com_port->rbr); } -int NS16550_tstc (NS16550_t com_port) +int NS16550_tstc (NS16550_t com_port) { -return ((serial_in (&com_port->lsr) &UART_LSR_DR ) !=0 ); +return (serial_in (&com_port->lsr) &UART_LSR_DR ) !=0 ; }#endif /* CONFIG_NS16550_MIN_FUNCTIONS */
include/ns16550.h +8 -8
@@ -65,12 +65,12 @@ struct NS16550 {#define dll rbr#define dlm ier -typedef volatile struct NS16550 *NS16550_t ; +typedef struct NS16550 *NS16550_t ;/* * These are the definitions for the FIFO Control Register */ -#define UART_FCR_FIFO_EN 0x01/* Fifo enable */ +#define UART_FCR_FIFO_EN 0x01/* Fifo enable */ #define UART_FCR_CLEAR_RCVR 0x02/* Clear the RCVR FIFO */ #define UART_FCR_CLEAR_XMIT 0x04/* Clear the XMIT FIFO */ #define UART_FCR_DMA_SELECT 0x08/* For DMA applications */
@@ -106,7 +106,7 @@ typedef volatile struct NS16550 *NS16550_t;#define UART_LCR_WLS_6 0x01/* 6 bit character length */ #define UART_LCR_WLS_7 0x02/* 7 bit character length */ #define UART_LCR_WLS_8 0x03/* 8 bit character length */ -#define UART_LCR_STB 0x04/* Number of stop Bits, off = 1, on = 1.5 or 2) */ +#define UART_LCR_STB 0x04/* # stop Bits, off=1, on=1.5 or 2) */ #define UART_LCR_PEN 0x08/* Parity eneble */ #define UART_LCR_EPS 0x10/* Even Parity Select */ #define UART_LCR_STKP 0x20/* Stick Parity */
@@ -162,8 +162,8 @@ typedef volatile struct NS16550 *NS16550_t;/* useful defaults for LCR */ #define UART_LCR_8N1 0x03 -void NS16550_init (NS16550_t com_port,int baud_divisor); -void NS16550_putc (NS16550_t com_port,char c); -char NS16550_getc (NS16550_t com_port); -int NS16550_tstc (NS16550_t com_port); -void NS16550_reinit (NS16550_t com_port,int baud_divisor); +void NS16550_init (NS16550_t com_port,int baud_divisor); +void NS16550_putc (NS16550_t com_port,char c); +char NS16550_getc (NS16550_t com_port); +int NS16550_tstc (NS16550_t com_port); +void NS16550_reinit (NS16550_t com_port,int baud_divisor);
Signature · 1 line
1.7.3.1
PATCH v4 2/2 NS16550: buffer reads
From: Scott Wood <[email protected]>
From: Scott Wood <[email protected]>
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass <[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
---
Changes in v2:
- Fix checkpatch warnings, the other one was already there
Changes in v3:
- Use CONFIG_NS16550_BUFFER_READS to set the buffer size also
Changes in v4:
- Change config option to CONFIG_NS16550_RBUF_SIZE
- Add additional checkpatch-cleanup patch
README | 12 ++++++ drivers/serial/ns16550.c | 97 +++++++++++++++++++++++++++++++++++++++++++-- drivers/serial/serial.c | 5 +- include/ns16550.h | 4 +- 4 files changed, 109 insertions(+), 9 deletions(-)
README +12 -0
@@ -2862,6 +2862,18 @@ use the "saveenv" command to store a valid environment. space for already greatly restricted images, including but not limited to NAND_SPL configurations. +- CONFIG_NS16550_RBUF_SIZE: + Instead of reading directly from the receive register + every time U-Boot is ready for another byte, keep a + buffer and fill it from the hardware fifo every time + U-Boot reads a character. This helps U-Boot keep up with + a larger amount of rapid input, such as happens when + pasting text into the terminal. + + To use this option, define CONFIG_NS16550_RBUF_SIZE to + the size of the buffer you want (256 is a reasonable value). + + Low Level (hardware related) configuration options: ---------------------------------------------------
drivers/serial/ns16550.c +92 -5
125 lines not on this page · raw message
drivers/serial/serial.c +3 -2
@@ -93,6 +93,7 @@ static NS16550_t serial_ports[4] = { };#define PORT serial_ports[port-1] +#define PORTNR (port-1)#if defined(CONFIG_CONS_INDEX )#define CONSOLE (serial_ports[CONFIG_CONS_INDEX-1])#endif
@@ -219,13 +220,13 @@ _serial_puts (const char *s,const int port)int _serial_getc (const int port) { -return NS16550_getc (PORT ); +return NS16550_getc (PORT ,PORTNR ); }int _serial_tstc (const int port) { -return NS16550_tstc (PORT ); +return NS16550_tstc (PORT ,PORTNR ); }void
include/ns16550.h +2 -2
@@ -164,6 +164,6 @@ typedef struct NS16550 *NS16550_t;void NS16550_init (NS16550_t com_port,int baud_divisor);void NS16550_putc (NS16550_t com_port,char c); -char NS16550_getc (NS16550_t com_port); -int NS16550_tstc (NS16550_t com_port); +char NS16550_getc (NS16550_t regs,unsigned int port); +int NS16550_tstc (NS16550_t regs,unsigned int port);void NS16550_reinit (NS16550_t com_port,int baud_divisor);
Signature · 1 line
1.7.3.1
PATCH v4 2/2 NS16550: buffer reads
Hi Simon,
Le 16/10/2011 07:14, Simon Glass a ?crit :
Quoted 22 lines
From: Scott Wood<[email protected]>
From: Scott Wood<[email protected]>
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass<[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
Signed-off-by Scott Wood<[email protected]>---
Changes in v2:
- Fix checkpatch warnings, the other one was already there
Quoted 6 lines
Changes in v3:
- UseCONFIG_NS16550_BUFFER_READSto set the buffer size alsoChanges in v4:
- Change config option toCONFIG_NS16550_RBUF_SIZE
- Add additional checkpatch-cleanup patch
Hmm... What about the conversation around V2 of the patch re: using XOFF/XON to control input flow? IIUC, even this V4 patch would not help much if there is a command within the pasted code that sends a lot of output, right?
Amicalement,
Signature · 1 line
Albert.
PATCH v4 2/2 NS16550: buffer reads
Hi Albert,
Quoted 29 lines · Albert ARIBAUD
Hi Simon,
Le 16/10/2011 07:14, Simon Glass a ?crit :
Quoted 26 lines
From: Scott Wood<[email protected]>
From: Scott Wood<[email protected]>
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. ?With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass<[email protected]> ?- ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
Signed-off-by Scott Wood<[email protected]>---
Changes in v2:
- Fix checkpatch warnings, the other one was already thereChanges in v3:
- UseCONFIG_NS16550_BUFFER_READSto set the buffer size also
Quoted 7 lines
Quoted 3 lines
Changes in v4:
- Change config option toCONFIG_NS16550_RBUF_SIZE
- Add additional checkpatch-cleanup patchHmm... What about the conversation around V2 of the patch re: using XOFF/XON to control input flow? IIUC, even this V4 patch would not help much if there is a command within the pasted code that sends a lot of output, right?
Well it will help so long as there isn't much more input. Perhaps the way to think of it is that every character you output that isn't just an echo, puts you one more character behind the input. This adds one character to the buffer in this patch. When you have fallen (say) 256 characters behind the input, then you will start losing characters even with this patch. However it is rare to paste in commands which send lots of output - and 3 lines of full output is pretty uncommon in my experience.
I feel that relying on the first level UART FIFO as the only buffering is brave, and cannot be justified by lack of interrupts alone. There seems to me to be no fundamental reason why U-Boot should not buffer its input for when it is latest ready to process it. Granted this should perhaps be higher up the software stack, but until someone tidies up serial a little I suspect that would be a pain to implement. With a more generic serial layer (a device pointer and a ->priv pointer across all drivers for example) we could attach this buffering as a separate file to be enabled / disabled by a Makefile.
To address your question, I think we did discuss it. Wolfgang felt that it should be as simple as sending XOFF when receiving the end of a line and XON when waiting for characters on the new line. It certainly sounds plausible.
I have other reasons for favouring this patch. On the main hardware platform I currently use, SPI and the console UART are multiplexed, so before using SPI (e.g. saveenv, reading the environment) we must read in any pending characters to avoid corruption. After re-enabling the UART later we must clear out any junk that has arrived. This patch provides a way around that which XON/XOFF does not (since when you send XOFF you may already have input characters outstanding - junk will later be added and you can't tell the difference).
I am unsure whether to bring this up or not - clearly this platform is not ideal, but it is in very common use and we need to support it with U-Boot.
Regards,
Simon
Quoted 5 lines
Amicalement,
--
Albert.
PATCH v4 2/2 NS16550: buffer reads
Dear Simon Glass,
Quoted 6 lines · In message <CAPnjgZ2Q3XtGK=5HhF0KhQo4XQDA6CAH3QFKfmgCwzNV_y0mUA@mail.gmail.com>…
Quoted 4 lines
Hmm... What about the conversation around V2 of the patch re: using XOFF/XON to control input flow? IIUC, even this V4 patch would not help much if there is a command within the pasted code that sends a lot of output, right?
Well it will help so long as there isn't much more input. Perhaps the
Don't you see that this "as long as there isn't much more input" is the fatal flaw of your patch? You are not fixing the problem, you are just extending the point wher eit hits to some extend - it may work for a few more situations, but it's still guaranteed to fail for others.
Quoted 4 lines
To address your question, I think we did discuss it. Wolfgang felt that it should be as simple as sending XOFF when receiving the end of a line and XON when waiting for characters on the new line. It certainly sounds plausible.
Then please let's take this approach instead, if you really need to support such a mode of operation.
Quoted 8 lines
I have other reasons for favouring this patch. On the main hardware platform I currently use, SPI and the console UART are multiplexed, so before using SPI (e.g. saveenv, reading the environment) we must read in any pending characters to avoid corruption. After re-enabling the UART later we must clear out any junk that has arrived. This patch provides a way around that which XON/XOFF does not (since when you send XOFF you may already have input characters outstanding - junk will later be added and you can't tell the difference).
That's an unrelated hardware issue that needs to be addressed independently.
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 Most legends have their basis in facts. -- Kirk, "And The Children Shall Lead", stardate 5029.5
PATCH v4 2/2 NS16550: buffer reads
Hi Wolfgang,
Quoted 8 lines · Wolfgang Denk
Dear Simon Glass,
Quoted 5 lines · In message <CAPnjgZ2Q3XtGK=5HhF0KhQo4XQDA6CAH3QFKfmgCwzNV_y0mUA@mail.gmail.com>…
Quoted 4 lines
Hmm... What about the conversation around V2 of the patch re: using XOFF/XON to control input flow? IIUC, even this V4 patch would not help much if there is a command within the pasted code that sends a lot of output, right?
Quoted 7 lines
Quoted 1 line
Well it will help so long as there isn't much more input. Perhaps the
Don't you see that this "as long as there isn't much more input" is the fatal flaw of your patch? ?You are not fixing the problem, you are just extending the point wher eit hits to some extend - it may work for a few more situations, but it's still guaranteed to fail for others.
In a similar way, the Linux kernel has a fatal flaw. Serial data coming into the flip buffers under extreme interrupt load can be lost, or the secondary buffers can become exhausted, or user space cannot keep up with input under heavy load, etc., etc. This is the reality of the world. As each problem presents itself we direct our attention to it.
Quoted 8 lines
Quoted 5 lines
To address your question, I think we did discuss it. Wolfgang felt that it should be as simple as sending XOFF when receiving the end of a line and XON when waiting for characters on the new line. It certainly sounds plausible.
Then please let's take this approach instead, if you really need to support such a mode of operation.
As I said I will look at this.
Quoted 10 lines
Quoted 9 lines
I have other reasons for favouring this patch. On the main hardware platform I currently use, SPI and the console UART are multiplexed, so before using SPI (e.g. saveenv, reading the environment) we must read in any pending characters to avoid corruption. After re-enabling the UART later we must clear out any junk that has arrived. This patch provides a way around that which XON/XOFF does not (since when you send XOFF you may already have input characters outstanding - junk will later be added and you can't tell the difference).
Quoted 2 lines
That's an unrelated hardware issue that needs to be addressed independently.
Yes it is. I can't help wondering what the solution might be though, if we are not allowed to buffer things.
Regards,
Simon
Quoted 12 lines
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 Most legends have their basis in facts.
? ? ? ?-- Kirk, "And The Children Shall Lead", stardate 5029.5
PATCH v4 2/2 NS16550: buffer reads
Dear Simon Glass,
Quoted 7 lines · In message <CAPnjgZ1VUF-HFbDX9byuV32sJfEE3SKRiT124gfgQOm0f+QSEw@mail.gmail.com>…
In a similar way, the Linux kernel has a fatal flaw. Serial data coming into the flip buffers under extreme interrupt load can be lost, or the secondary buffers can become exhausted, or user space cannot keep up with input under heavy load, etc., etc. This is the reality of the world. As each problem presents itself we direct our attention to it.
This is not a fatal flaw. There is nothing in the definition of RS232 or the serialinterface that guarantees you an error free data stream - not on that protocol level. You are facing loss of Ethernet packages on the network as well.
Problems start only when you make wrong assumptions.
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 girl with a future avoids the man with a past.
-- Evan Esar, "The Humor of Humor"PATCH v4 2/2 NS16550: buffer reads
In reply to Simon GlassDear Simon Glass,
In message <[email protected]> you wrote:
...
Quoted 5 lines
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass <[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
One problem I see is that the code size on some boards grows even if the new option is not enabled. Here a compare before and after applying your patches (without any changes to the respective board configurations):
Configuring for AR405 board...
text data bss dec hex filename
- 246058 12972 14636 273666 42d02 /work/wd/tmp-ppc/u-boot
+ 246062 12972 14636 273670 42d06 /work/wd/tmp-ppc/u-boot
Configuring for CANBT board...
text data bss dec hex filename
- 120710 8604 3876 133190 20846 /work/wd/tmp-ppc/u-boot
+ 120714 8604 3876 133194 2084a /work/wd/tmp-ppc/u-boot
Configuring for pcs440ep board...
text data bss dec hex filename
- 296191 19636 345088 660915 a15b3 /work/wd/tmp-ppc/u-boot
+ 296195 19636 345088 660919 a15b7 /work/wd/tmp-ppc/u-boot...
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 "Out of register space (ugh)" - vi
PATCH v4 2/2 NS16550: buffer reads
Hi Wolfgang,
Quoted 15 lines · Wolfgang Denk
Dear Simon Glass,
In message <[email protected]> you wrote:
...Quoted 5 lines
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass <[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
One problem I see is that the code size on some boards grows even if the new option is not enabled. ?Here a compare before and after applying your patches (without any changes to the respective board configurations):
Quoted 14 lines
?Configuring for AR405 board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 246058 ? 12972 ? 14636 ?273666 ? 42d02 /work/wd/tmp-ppc/u-boot
+ 246062 ? 12972 ? 14636 ?273670 ? 42d06 /work/wd/tmp-ppc/u-boot ?Configuring for CANBT board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 120710 ? ?8604 ? ?3876 ?133190 ? 20846 /work/wd/tmp-ppc/u-boot
+ 120714 ? ?8604 ? ?3876 ?133194 ? 2084a /work/wd/tmp-ppc/u-boot ?Configuring for pcs440ep board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 296191 ? 19636 ?345088 ?660915 ? a15b3 /work/wd/tmp-ppc/u-boot
+ 296195 ? 19636 ?345088 ?660919 ? a15b7 /work/wd/tmp-ppc/u-boot
...
This doesn't happen for me on ARM - the compiler just inlines the new static raw functions into the public functions. I will see if I can figure out what is happening on PPC. It is 4 bytes, or one instruction, right?
The fix might be to duplicate more code, or perhaps add an inline keyword to the two functions.
Regards,
Simon
Quoted 11 lines
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 "Out of register space (ugh)"
- vi
PATCH v4 2/2 NS16550: buffer reads
In reply to Wolfgang DenkQuoted 15 lines · Wolfgang Denk
Dear Simon Glass,
In message <[email protected]> you wrote:
...Quoted 5 lines
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass <[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
One problem I see is that the code size on some boards grows even if the new option is not enabled. ?Here a compare before and after applying your patches (without any changes to the respective board configurations):
Quoted 13 lines
?Configuring for AR405 board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 246058 ? 12972 ? 14636 ?273666 ? 42d02 /work/wd/tmp-ppc/u-boot
+ 246062 ? 12972 ? 14636 ?273670 ? 42d06 /work/wd/tmp-ppc/u-boot ?Configuring for CANBT board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 120710 ? ?8604 ? ?3876 ?133190 ? 20846 /work/wd/tmp-ppc/u-boot
+ 120714 ? ?8604 ? ?3876 ?133194 ? 2084a /work/wd/tmp-ppc/u-boot ?Configuring for pcs440ep board...
? ?text ? ?data ? ? bss ? ? dec ? ? hex filename
- 296191 ? 19636 ?345088 ?660915 ? a15b3 /work/wd/tmp-ppc/u-boot
+ 296195 ? 19636 ?345088 ?660919 ? a15b7 /work/wd/tmp-ppc/u-boot
...
Hi Wolfgang,
Can you please tell me which ELDK version this is using? I see that the 405 board seems to need ppc_4xx which suggests 4.2 rather than 5, since in 5 the compiler is called powerpc-
But I might be confused.
Regards,
Simon
Quoted 12 lines
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 "Out of register space (ugh)"
- vi
PATCH v4 2/2 NS16550: buffer reads
Dear Simon Glass,
Quoted 4 lines · In message <CAPnjgZ1412ezJh3f4Ww16k4Z55kJdgGWj7+vMyK_ot-MzVd70g@mail.gmail.com>…
Can you please tell me which ELDK version this is using? I see that the 405 board seems to need
ppc_4xxwhich suggests 4.2 rather than 5, since in 5 the compiler is called powerpc-
Right, I was testing this with ELDK 4.2.
Best regards,
Wolfgang Denk
Signature · 4 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 You can observe a lot just by watchin'. - Yogi Berra
PATCH v4 2/2 NS16550: buffer reads
Hi Wolfgang,
Quoted 9 lines · Wolfgang Denk
Dear Simon Glass,
Quoted 4 lines · In message <CAPnjgZ1412ezJh3f4Ww16k4Z55kJdgGWj7+vMyK_ot-MzVd70g@mail.gmail.com>…
Can you please tell me which ELDK version this is using? I see that the 405 board seems to need
ppc_4xxwhich suggests 4.2 rather than 5, since in 5 the compiler is called powerpc-Right, I was testing this with ELDK 4.2.
I am struggling to repeat this and don't even get the same numbers...
For your AR405 board, you saw:
Quoted 2 lines
- 246058 12972 14636 273666 42d02 /work/wd/tmp-ppc/u-boot + 246062 12972 14636 273670 42d06 /work/wd/tmp-ppc/u-boot
For me:
ppc_4xx-gcc -v
Reading specs from/opt/eldk-4.2.4xx/usr/bin/../lib/gcc/powerpc-linux/4.2.2/specs
Target: powerpc-linux
Configured with:/opt/eldk/build/ppc-2008-04-01/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/gcc-4.2.2/configure --target=powerpc-linux --host=i686-host_pc-linux-gnu --prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux
--disable-hosted-libstdcxx --with-headers=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux/include --with-local-prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux --disable-nls --enable-threads=posix --enable-symvers=gnu
--enable-__cxa_atexit --enable-languages=c,c++,java --enable-shared
--enable-c99 --enable-long-long --without-x
Thread model: posix
gcc version 4.2.2
ARCH=powerpc make O=ppc AR405_config
ARCH=powerpc make O=ppc
without feature (checkout of upstream/master d8fffa05):
$ ppc_4xx-objdump -h ppc/drivers/serial/ns16550.o
...
7 .text.NS16550_tstc 00000020 00000000 00000000 00000bf4 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
8 .text.NS16550_putc 0000002c 00000000 00000000 00000c14 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
9 .text.NS16550_getc 00000038 00000000 00000000 00000c40 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
10 .text.NS16550_init 0000007c 00000000 00000000 00000c78 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
11 .text.NS16550_reinit 00000080 00000000 00000000 00000cf4 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE text data bss dec hex filename
245942 12964 14632 273538 42c82 ppc/u-boot
with feature but not enabled (with Scott's two patches):
7 .text.NS16550_putc 0000002c 00000000 00000000 00000fec 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
8 .text.NS16550_getc 00000038 00000000 00000000 00001018 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
9 .text.NS16550_tstc 00000020 00000000 00000000 00001050 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
10 .text.NS16550_init 0000007c 00000000 00000000 00001070 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
11 .text.NS16550_reinit 00000080 00000000 00000000 000010ec 2**2
CONTENTS, ALLOC, LOAD, READONLY, CODE
size ppc/u-boot
text data bss dec hex filename
245942 12964 14632 273538 42c82 ppc/u-boot
Do you have any suggestions please?
Regards,
Simon
Quoted 11 lines
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 You can observe a lot just by watchin'. ? ? ? ? ? ? ? ? ?- Yogi Berra
PATCH v4 2/2 NS16550: buffer reads
Le 17/10/2011 22:58, Simon Glass a ?crit :
Quoted 7 lines
Hi Wolfgang,
Quoted 4 lines · Wolfgang Denk
Dear Simon Glass,
Quoted 1 line · In message<CAPnjgZ1412ezJh3f4Ww16k4Z55kJdgGWj7+vMyK_ot-MzVd70g@mail.gmail.com>…
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Can you please tell me which ELDK version this is using? I see that the 405 board seems to need
ppc_4xxwhich suggests 4.2 rather than 5, since in 5 the compiler is called powerpc-Right, I was testing this with ELDK 4.2.
I am struggling to repeat this and don't even get the same numbers...
Could be due to two things:
1) both of you do not work from the same exact tree commit, or
2) both of you work in a different base directory
Quoted 7 lines
For your AR405 board, you saw:
Quoted 2 lines
- 246058 12972 14636 273666 42d02 /work/wd/tmp-ppc/u-boot + 246062 12972 14636 273670 42d06 /work/wd/tmp-ppc/u-bootFor me:
245942 12964 14632 273538 42c82 ppc/u-boot
I suggest Wolfgang and you compare your trees to eliminate risk 1, and make sure you compare .bin files to reduce risk 2.
Amicalement,
Signature · 1 line
Albert.
PATCH v4 2/2 NS16550: buffer reads
In reply to Simon GlassDear Simon,
Quoted 7 lines · In message <[email protected]> you wrote:
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
Let me try to summarize my thinking:
- It is a fundamental design decision that U-Boot is single tasking. This implies that while a command is running, no other things get done in parallel. This includes communication over the serial line, USB, Ethernet, ...
- That means that by design U-Boot does not support multi-line input:
as soon as you hit "enter" U-Boot will start executing your command, and will only become ready for new input when it has completed the execution of the command(s), i. e. after printing the next prompt.
- For this suported mode of operation your patch is not needed. It just adds code size and complexity.
- Your description "rapid input" is actually wrong. The speed of input over the serial line is limited by the baud rate settings, and even if you transfer at maximum speed all supported systems are fast enough to receive the data without loss.
- The use case you are trying to support is actually multi-line input, so you should describe it as such. I agree that this would be an interesting feature for some use cases, but if we go on and implement it, we should (1) document it properly and (2) implement it in a way that works reliably.
- However, your implementation does not result in reliable multi-line input. It works only in a few specific use cases, and will fail silently for others. I don't see a way ho you would announce this new feature. The numbers you mention in the commit message ("it handles around 70 lines before lossage") apply for this specific board only, and for your use case only (pasting of short lines that produce no output).
So essentially you are saying: hey, we now have multi-line input, but it works only a bit. It will fail sooner or later, without error messages, and I cannot even tell you what the limits for your system are. And it depends on which input you send.
This does not exactly sound promising.
On the other hand, we also have the rule that things that are useful to some and that don't hurt others should be allowed to go in.
What makes me hesitate are two things:
- The patch promises a feature (multi-line input), but fails to provide a reliably working implementation.
- As it turns out, the patch increases code size even for boards that do not activate this feature.
I have to admit that I'm at a loss with a decision here.
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 "We don't have to protect the environment -- the Second Coming is at hand." - James Watt
PATCH v4 2/2 NS16550: buffer reads
Hi Wolfgang,
Quoted 31 lines · Wolfgang Denk
Dear Simon,
Quoted 7 lines · In message <[email protected]> you wrote:
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. ?With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
Let me try to summarize my thinking:
- It is a fundamental design decision that U-Boot is single tasking. ?This implies that while a command is running, no other things get ?done in parallel. This includes communication over the serial line, ?USB, Ethernet, ...
- That means that by design U-Boot does not support multi-line input:
?as soon as you hit "enter" U-Boot will start executing your command, ?and will only become ready for new input when it has completed the ?execution of the command(s), i. e. after printing the next prompt.- For this suported mode of operation your patch is not needed. It ?just adds code size and complexity.
- Your description "rapid input" is actually wrong. ?The speed of ?input over the serial line is limited by the baud rate settings, ?and even if you transfer at maximum speed all supported systems ?are fast enough to receive the data without loss.
Quoted 5 lines
- The use case you are trying to support is actually multi-line ?input, so you should describe it as such. ?I agree that this would ?be an interesting feature for some use cases, but if we go on and ?implement it, we should (1) document it properly and (2) implement ?it in a way that works reliably.
I can certainly improve the documentation and commit message.
Quoted 9 lines
- However, your implementation does not result in reliable multi-line ?input. ?It works only in a few specific use cases, and will fail ?silently for others. ?I don't see a way ho you would announce this ?new feature. ?The numbers you mention in the commit message ("it ?handles around 70 lines before lossage") apply for this specific ?board only, and for your use case only (pasting of short lines that ?produce no output).
Quoted 6 lines
?So essentially you are saying: hey, we now have multi-line input, ?but it works only a bit. ?It will fail sooner or later, without ?error messages, and I cannot even tell you what the limits for your ?system are. ?And it depends on which input you send.
?This does not exactly sound promising.
That all sounds reasonable.
I think it is good enough, but not perfect. Due to the fundamental design decision you mention at the top, we cannot squirrel away serial input in the background. The best we can do is squirrel it away in the foreground, as we output new serial data (I suppose we could squirrel it away in net loops and other places but I don't want to go there!). This turns out to be mostly good enough, because it is rare for people to want to paste 'sleep 10' and the like into their terminal (your other message).
I would like to spit out a warning when serial input is lost - as I mentioned that could be combined with a serial overhaul at some point.
Quoted 9 lines
On the other hand, we also have the rule that things that are useful to some and that don't hurt others should be allowed to go in.
What makes me hesitate are two things:
- The patch promises a feature (multi-line input), but fails to ?provide a reliably working implementation.
I *think* it does the best possible within the fundamental design decision constraint. If there is more it can do, please let me have your ideas. I don't believe buffering conflicts with the constraint - they are separate things. But yes in systems with interrupts normally the input buffer is filled in the background and drained in the foreground.
Quoted 3 lines
- As it turns out, the patch increases code size even for boards that ?do not activate this feature.
Yes, I will take a look at this problem.
Quoted 3 lines
I have to admit that I'm at a loss with a decision here.
Well it's not easy being a maintainer :-) In any case this patch is not the end of the story as serial needs some work - another objection you didn't mention above is that this function lives in only one driver. Is that a good thing (hide it away) or a bad thing (all drivers should support it and the implementation should move up a level)?
Regards,
Simon
Quoted 12 lines
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 "We don't have to protect the environment -- the Second Coming is ?at hand." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - James Watt
PATCH v4 2/2 NS16550: buffer reads
Le 17/10/2011 18:40, Simon Glass a ?crit :
Quoted 9 lines
Quoted 6 lines
So essentially you are saying: hey, we now have multi-line input, but it works only a bit. It will fail sooner or later, without error messages, and I cannot even tell you what the limits for your system are. And it depends on which input you send.
This does not exactly sound promising.
That all sounds reasonable.
Quoted 8 lines
I think it is good enough, but not perfect. Due to the fundamental design decision you mention at the top, we cannot squirrel away serial input in the background. The best we can do is squirrel it away in the foreground, as we output new serial data (I suppose we could squirrel it away in net loops and other places but I don't want to go there!). This turns out to be mostly good enough, because it is rare for people to want to paste 'sleep 10' and the like into their terminal (your other message).
I think you're not seeing the point that while your patch is good enough for 'making multiline input less impractical in some cases', it is not good enough for 'making multiline input reliable': it adds marginal benefits and we have clear evidence that it will never guarantee a correct result however you tweak it, because the only way it could would be by multi-tasking in some way; otherwise, there can always be some U-Boot activity in between input readings that will be long enough to cause character loss.
Quoted 2 lines
I would like to spit out a warning when serial input is lost - as I mentioned that could be combined with a serial overhaul at some point.
I don't think the warning would make the functionality better.
Quoted 8 lines
Quoted 7 lines
On the other hand, we also have the rule that things that are useful to some and that don't hurt others should be allowed to go in.
What makes me hesitate are two things:
- The patch promises a feature (multi-line input), but fails to provide a reliably working implementation.
Quoted 6 lines
I *think* it does the best possible within the fundamental design decision constraint. If there is more it can do, please let me have your ideas. I don't believe buffering conflicts with the constraint - they are separate things. But yes in systems with interrupts normally the input buffer is filled in the background and drained in the foreground.
There *is* a much better solution within the fundamental single tasking design constraint, and it is the one that was invented precisely for
this: flow control. Granted, hardware flow control may be impractical, and that's why software flow control was conceived as early as 1963 and is still widely used today and supported by any serial software or driver worth its salt.
Quoted 10 lines
Quoted 2 lines
- As it turns out, the patch increases code size even for boards that do not activate this feature.
Yes, I will take a look at this problem.
Quoted 3 lines
I have to admit that I'm at a loss with a decision here.
Well it's not easy being a maintainer :-)
Well, FWIW, were I the serial code maintainer, I'd NAK this and ask for a XON/XOFF implementation.
Quoted 6 lines
In any case this patch is
not the end of the story as serial needs some work - another objection you didn't mention above is that this function lives in only one driver. Is that a good thing (hide it away) or a bad thing (all drivers should support it and the implementation should move up a level)?
Software flow control, whatever way it is implemented, is pure SW and hardware independent, so it should be a generic thing. If XON/XOFF input flow control gets implemented (and I believe it should), then it should be done above hardware serial drivers.
Quoted 2 lines
Regards,
Simon
Amicalement,
Signature · 1 line
Albert.
PATCH v4 2/2 NS16550: buffer reads
Hi Simon, Albert,
Quoted 1 line · Albert ARIBAUD
Le 17/10/2011 18:40, Simon Glass a ?crit :
[snip]
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
On the other hand, we also have the rule that things that are useful to some and that don't hurt others should be allowed to go in.
What makes me hesitate are two things:
- The patch promises a feature (multi-line input), but fails to provide a reliably working implementation.
I fully agree with this - We should strive to eliminate bugs, not cover them with more dirt :)
Quoted 7 lines
Quoted 6 lines
I *think* it does the best possible within the fundamental design decision constraint. If there is more it can do, please let me have your ideas. I don't believe buffering conflicts with the constraint - they are separate things. But yes in systems with interrupts normally the input buffer is filled in the background and drained in the foreground.
Quoted 6 lines
There *is* a much better solution within the fundamental single tasking design constraint, and it is the one that was invented precisely for
this: flow control. Granted, hardware flow control may be impractical, and that's why software flow control was conceived as early as 1963 and is still widely used today and supported by any serial software or driver worth its salt.
Agree - flow control is the way to go
Quoted 10 lines
Well, FWIW, were I the serial code maintainer, I'd NAK this and ask for a XON/XOFF implementation.
Quoted 6 lines
In any case this patch is
not the end of the story as serial needs some work - another objection you didn't mention above is that this function lives in only one driver. Is that a good thing (hide it away) or a bad thing (all drivers should support it and the implementation should move up a level)?
Quoted 4 lines
Software flow control, whatever way it is implemented, is pure SW and hardware independent, so it should be a generic thing. If XON/XOFF input flow control gets implemented (and I believe it should), then it should be done above hardware serial drivers.
One problem I see with XON/XOFF is that if we don't send XOFF at the right time, we run the risk of entering a busy loop (any reasonable timeout delay for example) and loosing input. So in theory, we would need to send XOFF after every getc() and XON before each getc() and then need to delay a bit to wait for the character (unless tstc returns true in which case we can just grab the char in the buffer) which seems a bit excessive.
Maybe we need disable/enable flow control functions for when we know we will be entering a busy loop the consumes serial input (ymodem and kermit transfers and readline for example)
Regards,
Graeme
PATCH v4 2/2 NS16550: buffer reads
Dear Graeme Russ,
Quoted 5 lines · In message <[email protected]> you wrote:
One problem I see with XON/XOFF is that if we don't send XOFF at the right time, we run the risk of entering a busy loop (any reasonable timeout delay for example) and loosing input. So in theory, we would need to send XOFF after every
getc()...
That's not true. I am not aware of any significant delays that take place while receiving characters that belong to a single line. If we had any of these, we would lose characters all the time - but we don't.
It should be sufficient to send XOFF after receiving a newline character.
Quoted 3 lines
Maybe we need disable/enable flow control functions for when we know we will be entering a busy loop the consumes serial input (ymodem and kermit transfers and readline for example)
This should not be necessary. Actually the implementation should not need to know about such special cases.
Best regards,
Wolfgang Denk
Signature · 6 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 Compassion -- that's the one things no machine ever had. Maybe it's the one thing that keeps men ahead of them. -- McCoy, "The Ultimate Computer", stardate 4731.3
PATCH v4 2/2 NS16550: buffer reads
Hi Wolgang,
Quoted 6 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 2 lines · In message <[email protected]> you wrote:
One problem I see with XON/XOFF is that if we don't send XOFF at the
right
Quoted 1 line
Quoted 1 line
time, we run the risk of entering a busy loop (any reasonable timeout
delay
Quoted 10 lines
Quoted 2 lines
for example) and loosing input. So in theory, we would need to send XOFF after every
getc()...That's not true. I am not aware of any significant delays that take place while receiving characters that belong to a single line. If we had any of these, we would lose characters all the time - but we don't.
It should be sufficient to send XOFF after receiving a newline character.
And, ergo, we send an XON when entering the readline function
Hmm, should we move readline() into console.c
Quoted 2 lines
Quoted 2 lines
Maybe we need disable/enable flow control functions for when we know we will be entering a busy loop the consumes serial input (ymodem and
kermit
Quoted 4 lines
Quoted 1 line
transfers and readline for example)
This should not be necessary. Actually the implementation should not need to know about such special cases.
So how does kermit/ymodem send the XON after the user has entered the receive command and we have sent the XOFF after the newline?
Regards,
Graeme
PATCH v4 2/2 NS16550: buffer reads
Dear Graeme Russ,
Quoted 5 lines · In message <CALButCLEg6c30En3N4LjPv1woJjFxwkEkHvQYMoJy8+MgSzqJw@mail.gmail.com>…
Quoted 3 lines
It should be sufficient to send XOFF after receiving a newline character.
And, ergo, we send an XON when entering the readline function
This is probably not sufficient, as some commands take direct input. I think both getc() and tstc() should check the XON/XOFF state and send a XON if XOFF was sent before.
Quoted 1 line
Hmm, should we move
readline()intoconsole.c
Makes sense.
Quoted 5 lines
Quoted 2 lines
This should not be necessary. Actually the implementation should not need to know about such special cases.
So how does kermit/ymodem send the XON after the user has entered the receive command and we have sent the XOFF after the newline?
Upon the first getc() that follows?
Best regards,
Wolfgang Denk
Signature · 4 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 Marriage is the sole cause of divorce.
PATCH v4 2/2 NS16550: buffer reads
Hi Wolfgang,
Quoted 4 lines · Wolfgang Denk
Dear Graeme Russ,
In message <
CALButCLEg6c30En3N4LjPv1woJjFxwkEkHvQYMoJy8+MgSzqJw@mail.gmail.com> you
Quoted 1 line · wrote:
Quoted 1 line
[snip]
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
This should not be necessary. Actually the implementation should not need to know about such special cases.
So how does kermit/ymodem send the XON after the user has entered the receive command and we have sent the XOFF after the newline?
Upon the first
getc()that follows?
And as there will be no corresponding newline, when do we send XOFF?
Regards,
Graeme
PATCH v4 2/2 NS16550: buffer reads
Dear Graeme Russ,
Quoted 7 lines · In message <CALButCK0oVLTbYxbTF=y-vXL+3+=09VEJpzUnX1+-0SYJQEQJA@mail.gmail.com>…
Quoted 5 lines
Quoted 3 lines
So how does kermit/ymodem send the XON after the user has entered the receive command and we have sent the XOFF after the newline?
Upon the first
getc()that follows?And as there will be no corresponding newline, when do we send XOFF?
Never? Note that kermit / ymodem / S-record download etc. all don't have any issues with sendign characters back-to-back at line speed.
Problems happen only with multi-line input, so it is perfectly fine to handle just that - at the root cause, i. e. when input turns into multi-line input.
Best regards,
Wolfgang Denk
Signature · 4 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 Include the success of others in your dreams for your own success.
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 3 lines · Wolfgang Denk
Dear Graeme Russ,
In message <CALButCK0oVLTbYxbTF=y-vXL+3+=
Quoted 6 lines · [email protected]> you wrote:
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
So how does kermit/ymodem send the XON after the user has entered the receive command and we have sent the XOFF after the newline?
Upon the first
getc()that follows?
Quoted 8 lines
Quoted 1 line
And as there will be no corresponding newline, when do we send XOFF?
Never? Note that kermit / ymodem / S-record download etc. all don't have any issues with sendign characters back-to-back at line speed.
Problems happen only with multi-line input, so it is perfectly fine to handle just that - at the root cause, i. e. when input turns into multi-line input.
Can the U-Boot command line handle multiple commands per line (delimited by ; for example)
If so, could it not be possible that a Kermit/ymodem command followed by a time consuming command on the same line cause lost input?
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Dear Graeme Russ,
Quoted 7 lines · In message <CALButCJH8BVzfVh14D83WR2JOV89O9jvJO9VzzB7r_ZgKzZqeg@mail.gmail.com>…
Quoted 4 lines
Problems happen only with multi-line input, so it is perfectly fine to handle just that - at the root cause, i. e. when input turns into multi-line input.
Can the U-Boot command line handle multiple commands per line (delimited by ; for example)
Yes, it can.
Quoted 2 lines
If so, could it not be possible that a Kermit/ymodem command followed by a time consuming command on the same line cause lost input?
I don't think so. All serial transfers use a protocol - and when the transfer is complete, it does not matter any more, because no more data are flowing.
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 History tends to exaggerate. -- Col. Green, "The Savage Curtain", stardate 5906.4
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 3 lines · Wolfgang Denk
Dear Graeme Russ,
In message <
CALButCJH8BVzfVh14D83WR2JOV89O9jvJO9VzzB7r_ZgKzZqeg@mail.gmail.com> you
Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 4 lines
Problems happen only with multi-line input, so it is perfectly fine to handle just that - at the root cause, i. e. when input turns into multi-line input.
Can the U-Boot command line handle multiple commands per line (delimited
by
Quoted 5 lines
Quoted 1 line
; for example)
Yes, it can.
Quoted 1 line
If so, could it not be possible that a Kermit/ymodem command followed by
a
Quoted 5 lines
Quoted 1 line
time consuming command on the same line cause lost input?
I don't think so. All serial transfers use a protocol - and when the transfer is complete, it does not matter any more, because no more data are flowing.
My point is that the transfer turns off flow control - When the transfer completes, flow control will be off when the next command begins to run. If the next command is one which takes a long time to execute and it is on the same line as the transfer command (i.e. no \r to send XOFF) and the user types something then that input can be lost.
I think the solution is fairly trivial though - During the processing of commands entered via readline(), cause an XOFF to be sent each time (i.e. immediately before) the command string is dispatched a to the command processor just in case the previous command called getc() (and thus caused an XON to be sent)
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Hi,
Quoted 33 lines · Graeme Russ
Hi Wolfgang,
Quoted 3 lines · Wolfgang Denk
Dear Graeme Russ,
In message <
CALButCJH8BVzfVh14D83WR2JOV89O9jvJO9VzzB7r_ZgKzZqegat mail.gmail.com> youQuoted 6 lines · wrote:
Quoted 6 lines
Quoted 4 lines
Problems happen only with multi-line input, so it is perfectly fine to handle just that - at the root cause, i. e. when input turns into multi-line input.
Can the U-Boot command line handle multiple commands per line (delimited
by
Quoted 5 lines
Quoted 1 line
; for example)
Yes, it can.
Quoted 1 line
If so, could it not be possible that a Kermit/ymodem command followed by
a
Quoted 5 lines
Quoted 1 line
time consuming command on the same line cause lost input?
I don't think so. ?All serial transfers use a protocol - and when the transfer is complete, it does not matter any more, because no more data are flowing.
My point is that the transfer turns off flow control - When the transfer completes, flow control will be off when the next command begins to run. If the next command is one which takes a long time to execute and it is on the same line as the transfer command (i.e. no \r to send XOFF) and the user types something then that input can be lost.
Quoted 5 lines
I think the solution is fairly trivial though - During the processing of commands entered via
readline(), cause an XOFF to be sent each time (i.e. immediately before) the command string is dispatched a to the command processor just in case the previous command calledgetc()(and thus caused an XON to be sent)
I had a go at a patch for this, will send out tomorrow. It will need fixing up for xmodem etc. So far it works ok with minicom but not ser2net. It is pretty simple.
I think Albert mentioned that XON/XOFF was invented in 1962. Maybe it will come back into fashion? Not at all sure it will...
Regards,
Simon
Quoted 11 lines
Regards,
Graeme
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH v2 1/1 NS16550: buffer reads
In reply to Graeme RussDear Graeme Russ,
Quoted 10 lines · In message <CALButCKD2ucJ0ZUQJpLCP2ABYcCzO-mACa=FPWczCTVeHeoTKg@mail.gmail.com>…
Quoted 7 lines
Quoted 3 lines
If so, could it not be possible that a Kermit/ymodem command followed by a time consuming command on the same line cause lost input?
I don't think so. All serial transfers use a protocol - and when the transfer is complete, it does not matter any more, because no more data are flowing.
My point is that the transfer turns off flow control - When the transfer completes, flow control will be off when the next command begins to run.
Why would any of the transfer commands actually turn off flow control? There is no need to do that so far. And even if they do - that's no fundamental difference to now, where we are not reading the input then, either.
Quoted 3 lines
If the next command is one which takes a long time to execute and it is on the same line as the transfer command (i.e. no \r to send XOFF) and the user types something then that input can be lost.
I don't understand what you mean. We're talking about a single line of input here, right? Re-enabling XON is not needed before we're ready to read the next line. And during that, no characters would be lost because none are sent due to flow control being shut off.
Quoted 5 lines
I think the solution is fairly trivial though - During the processing of commands entered via
readline(), cause an XOFF to be sent each time (i.e. immediately before) the command string is dispatched a to the command processor just in case the previous command calledgetc()(and thus caused an XON to be sent)
This sounds like unneeded overhead to me.
Best regards,
Wolfgang Denk
Signature · 6 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 Teenagers are people who express a burning desire to be different by dressing exactly alike. There are some strings. They're just not attached.
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 7 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 4 lines · In message <CALButCKD2ucJ0ZUQJpLCP2ABYcCzO-mACa=FPWczCTVeHeoTKg@mail.gmail.com>…
Quoted 4 lines
Quoted 3 lines
If so, could it not be possible that a Kermit/ymodem command followed by a time consuming command on the same line cause lost input?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I don't think so. All serial transfers use a protocol - and when the transfer is complete, it does not matter any more, because no more data are flowing.
My point is that the transfer turns off flow control - When the transfer completes, flow control will be off when the next command begins to run.
Why would any of the transfer commands actually turn off flow control?
getc() sends an XOFF
Quoted 13 lines
There is no need to do that so far. And even if they do - that's no fundamental difference to now, where we are not reading the input then, either.
Quoted 3 lines
If the next command is one which takes a long time to execute and it is on the same line as the transfer command (i.e. no \r to send XOFF) and the user types something then that input can be lost.
I don't understand what you mean. We're talking about a single line of input here, right? Re-enabling XON is not needed before we're ready to read the next line. And during that, no characters would be lost because none are sent due to flow control being shut off.
Quoted 7 lines
Quoted 5 lines
I think the solution is fairly trivial though - During the processing of commands entered via
readline(), cause an XOFF to be sent each time (i.e. immediately before) the command string is dispatched a to the command processor just in case the previous command calledgetc()(and thus caused an XON to be sent)This sounds like unneeded overhead to me.
consider the follow (admittedly canned) example:
loadb ; sleep 20
An XOFF will be sent when the user hits 'enter' but loadb will send an XON when it calls getc(). Now after the transfer is complete, there will have been no XOFF before the sleep command is run so if the user enters anything during the sleep command, those characters can be lost
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Why would any of the transfer commands actually turn off flow control?
getc()sends an XOFF
Oops? You got something wrong, I think.
Receiving a '\n' would cause sending XOFF, and getc() would, if state is XOFF, send an XON.
Quoted 8 lines
consider the follow (admittedly canned) example:
loadb ; sleep 20
An XOFF will be sent when the user hits 'enter' but loadb will send an XON when it calls
getc(). Now after the transfer is complete, there will have been no XOFF before the sleep command is run so if the user enters anything during the sleep command, those characters can be lost
Agreed. You can relax situation if functions that use the serial line for their own purposes (like running some serial transfer protocol) would have some initialization part where they remember the line state (and, if it was XOFF, send XON), plus a termination part where they restore the old line state (i. e. if it was XOFF initially, send XOFF again).
But note that such software flow control is again just a little improvement, it is definitely not a guaranteed way to prevent any character losses. Assume the situation where the user copy & pastes some multi-line command sequence. We will receive a character sequence like this:
a b c d e '\n' f g h i j k ...
We will send XOFF after receiving the '\n' - guess when XOFF will be loaded into our transmitter? When the last bit and stop bits have been sent? When the receiver on the other size will actually read this from his FIFO? And How long it then will take until his Tx fifo empties, and he stops sending more characters?
Assume we have a simple device with a small Rx FIFO - say, 8 bytes only. Guess what the chances are that we will overrun this FIFO (and then lose characters) before the other side stops sending?
Best regards,
Wolfgang Denk
Signature · 6 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 As far as the laws of mathematics refer to reality, they are not certain; and as far as they are certain, they do not refer to reality. -- Albert Einstein
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 4 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 1 line · In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Why would any of the transfer commands actually turn off flow control?
getc()sends an XOFFOops? You got something wrong, I think.
Receiving a '\n' would cause sending XOFF, and
getc()would, if state is XOFF, send an XON.
Oops, yes you are right
Quoted 9 lines
Quoted 8 lines
consider the follow (admittedly canned) example:
loadb ; sleep 20
An XOFF will be sent when the user hits 'enter' but loadb will send an XON when it calls
getc(). Now after the transfer is complete, there will have been no XOFF before the sleep command is run so if the user enters anything during the sleep command, those characters can be lost
Quoted 6 lines
Agreed. You can relax situation if functions that use the serial line for their own purposes (like running some serial transfer protocol) would have some initialization part where they remember the line state (and, if it was XOFF, send XON), plus a termination part where they restore the old line state (i. e. if it was XOFF initially, send XOFF again).
I think it is sufficient to send an XOFF just before starting to process a command and XON after command processing is complete. I would think it safe to assume any command that consumes characters either does so fast enough, or does flow control internally. And while not processing a command, we are in a busy loop processing terminal input waiting for a command.
Quoted 3 lines
But note that such software flow control is again just a little improvement, it is definitely not a guaranteed way to prevent any character losses. Assume the situation where the user copy & pastes
Actually, I think we can guarantee no loss provided both ends of the serial link are configured correctly.
Quoted 10 lines
some multi-line command sequence. We will receive a character sequence like this:
a b c d e '\n' f g h i j k ...
We will send XOFF after receiving the '\n' - guess when XOFF will be loaded into our transmitter? When the last bit and stop bits have been sent? When the receiver on the other size will actually read this from his FIFO? And How long it then will take until his Tx fifo empties, and he stops sending more characters?
Provided the transmitter and receiver code are running in busy loops, at most a few characters. This is why 16550 et. al. have hardware Rx buffers, to account for the delay in receiving the XOFF. The hardware Tx should never be used unless you have hardware flow control which is controlled directly by the UART (and I think the original implementation of the 16550 only had Rx buffers for this very reason).
Quoted 3 lines
Assume we have a simple device with a small Rx FIFO - say, 8 bytes only. Guess what the chances are that we will overrun this FIFO (and then lose characters) before the other side stops sending?
Should be zero (if you disable the hardware Tx buffer) unless you have an incredibly slow Rx routine (in which case, your baud rate is too high). And even with a Tx buffer in play the solution is fairly simple - After sending an XOFF, flush the Rx buffer (and remote Tx buffer) into an internal buffer until tstc() returns false - use this buffer for getc() until it is empty. Of course, this fails if the remote end does not honour XOFF, but that's the users own fault for turning software flow control on at one end only and/or using a broken serial emulator.
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Dear Graeme Russ,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Assume we have a simple device with a small Rx FIFO - say, 8 bytes only. Guess what the chances are that we will overrun this FIFO (and then lose characters) before the other side stops sending?
Should be zero (if you disable the hardware Tx buffer) unless you have an incredibly slow Rx routine (in which case, your baud rate is too high). And even with a Tx buffer in play the solution is fairly simple - After sending an XOFF, flush the Rx buffer (and remote Tx buffer) into an internal buffer until
tstc()returns false - use this buffer forgetc()until it is empty.
Things are becoming more and more complicated, it seems.
If you ask me: forget about all this, and stick with single line input. Do the processing on the sender's side (always wait for a prompt before sending the next line). This is MUCH easier.
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 "Never face facts; if you do, you'll never get up in the morning." - Marlo Thomas
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 14 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Assume we have a simple device with a small Rx FIFO - say, 8 bytes only. Guess what the chances are that we will overrun this FIFO (and then lose characters) before the other side stops sending?
Should be zero (if you disable the hardware Tx buffer) unless you have an incredibly slow Rx routine (in which case, your baud rate is too high). And even with a Tx buffer in play the solution is fairly simple - After sending an XOFF, flush the Rx buffer (and remote Tx buffer) into an internal buffer until
tstc()returns false - use this buffer forgetc()until it is empty.
Quoted 5 lines
Things are becoming more and more complicated, it seems.
If you ask me: forget about all this, and stick with single line input. Do the processing on the sender's side (always wait for a prompt before sending the next line). This is MUCH easier.
Oh, I wholehearedly agree that if we impose such a limitation, life will be much easier for us all ;)
But realistically, the solution is actually very trivial and comprises just two parts:
1a) When the command line interpreter (which is just the really simple
function detecting new-line or command delimiter) dispatches a command string to the command interpreter (which looks up the command table and if it finds a command runs it) send an XOFF
1b) When the command interpreter returns control back to the command
line interpreter, send an XON
1c) If an XOFF has been recently sent (i.e. no XON since) send an XON
in getc()
2) After sending XOFF, flush the transmitter's buffer into a small (16 bytes chould sufffice) buffer. Grab characters from this buffer before grabing from the UART when getc() is called
The second part should not even be necessary if you have a half-sane serial port with a half-decent hardware buffer.
It really would not be that hard (maybe a few dozen lines of code at most), would work for all UARTS and should (hopefully) resolve all 'dropped character' issues.
Now 1c) makes the bold assumption that any command which calls getc() 'knows what it is doing' and will consume all input characters at a fast enough rate AND will not invoke a delay between receiving the last character and returning back to the command line interpreter. If the command knows it received characters and knows it will invoke a delay (some kind of post-processing) then in order to not lose characters, the command MUST issue it's own XOFF - nothing else aside from an interrupt driven serial driver will solve the problem in this case.
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Quoted 34 lines · Graeme Russ
Hi Wolfgang,
Quoted 11 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 8 lines · In message<[email protected]> you wrote:
Quoted 3 lines
Assume we have a simple device with a small Rx FIFO - say, 8 bytes only. Guess what the chances are that we will overrun this FIFO (and then lose characters) before the other side stops sending?
Should be zero (if you disable the hardware Tx buffer) unless you have an incredibly slow Rx routine (in which case, your baud rate is too high). And even with a Tx buffer in play the solution is fairly simple - After sending an XOFF, flush the Rx buffer (and remote Tx buffer) into an internal buffer until
tstc()returns false - use this buffer forgetc()until it is empty.Quoted 5 lines
Things are becoming more and more complicated, it seems.
If you ask me: forget about all this, and stick with single line input. Do the processing on the sender's side (always wait for a prompt before sending the next line). This is MUCH easier.
Oh, I wholehearedly agree that if we impose such a limitation, life will be much easier for us all ;)
But realistically, the solution is actually very trivial and comprises just two parts:
1a) When the command line interpreter (which is just the really simple
function detecting new-line or command delimiter) dispatches a command string to the command interpreter (which looks up the command table and if it finds a command runs it) send an XOFF
1b) When the command interpreter returns control back to the command
line interpreter, send an XON
1c) If an XOFF has been recently sent (i.e. no XON since) send an XON
ingetc()
Quoted 3 lines
2) After sending XOFF, flush the transmitter's buffer into a small (16 bytes chould sufffice) buffer. Grab characters from this buffer before grabing from the UART when
getc()is called
Hi All,
The problem with all this is that the transmitting CPU may send
"several" characters after the XOFF is transmitted, where several will be at least two if the transmitting CPU has no output fifo, up to possibly the output fifo length + 2. Furthermore, this "overage" can theoretically increase with each x-on x-off sequence, as each command may be, in theory, shorter than the output fifo length. So the input buffering is needed in all cases, i.e. the receive fifo must be "run dry" before executing a command. If the receive fifo is shorter than the transmit fifo +2, you are doomed. If the receive fifo is larger than this, you are ok, the transmitting CPU will stop before overrunning the input fifo and data being lost. However, a 16 byte buffer will not be enough if there are lots of "short" commands in the input stream.
Best Regards,
Bill Campbell
Quoted 26 lines
The second part should not even be necessary if you have a half-sane serial port with a half-decent hardware buffer.
It really would not be that hard (maybe a few dozen lines of code at most), would work for all UARTS and should (hopefully) resolve all 'dropped character' issues.
Now 1c) makes the bold assumption that any command which calls
getc()'knows what it is doing' and will consume all input characters at a fast enough rate AND will not invoke a delay between receiving the last character and returning back to the command line interpreter. If the command knows it received characters and knows it will invoke a delay (some kind of post-processing) then in order to not lose characters, the command MUST issue it's own XOFF - nothing else aside from an interrupt driven serial driver will solve the problem in this case.Regards,
Graeme
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH v2 1/1 NS16550: buffer reads
In reply to Graeme RussDear Graeme Russ,
Quoted 9 lines · In message <CALButCLNEjjEB=1zvvikrVo5-X9vyYOECU1-ZrpXf2myavM+Pg@mail.gmail.com>…
Now 1c) makes the bold assumption that any command which calls
getc()'knows what it is doing' and will consume all input characters at a fast enough rate AND will not invoke a delay between receiving the last character and returning back to the command line interpreter. If the command knows it received characters and knows it will invoke a delay (some kind of post-processing) then in order to not lose characters, the command MUST issue it's own XOFF - nothing else aside from an interrupt driven serial driver will solve the problem in this case.
This is a fundamentally broken design. No command should ever need to have any such knowledge, not to mention that we must not littering potentially all code with serial flow control calls.
You will have really hard times to get anything like that into mainline.
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
The optimum committee has no members.
- Norman AugustinePATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 13 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 9 lines · In message <CALButCLNEjjEB=1zvvikrVo5-X9vyYOECU1-ZrpXf2myavM+Pg@mail.gmail.com>…
Now 1c) makes the bold assumption that any command which calls
getc()'knows what it is doing' and will consume all input characters at a fast enough rate AND will not invoke a delay between receiving the last character and returning back to the command line interpreter. If the command knows it received characters and knows it will invoke a delay (some kind of post-processing) then in order to not lose characters, the command MUST issue it's own XOFF - nothing else aside from an interrupt driven serial driver will solve the problem in this case.
Quoted 3 lines
This is a fundamentally broken design. No command should ever need to have any such knowledge, not to mention that we must not littering potentially all code with serial flow control calls.
Well, if a command reads from the console (a transfer command for example) and then has a long delay (busy processing loop) before returning back to the command line processor then that command must be fundamentally broken - there are five ways I see to fix it:
a) Fix the command so it isn't broken
b) Have the command tell the console it has finished with the console before it starts the busy loop
c) Use UART managed hardware flow control
d) Implement interrupt based serial drivers
e) Prohibit multi-line input
Quoted 1 line
You will have really hard times to get anything like that into mainline.
So let's assume for the meantime that there are no 'broken' commands we can simply issue an XOFF before running each command - That should eliminate most dropped character issues - This is Simon's latest 2-patch series.
Adding rx_flush() and a small local buffer will fix dropped characters due to insufficiently small local UART Rx buffers (and brain-dead remote Tx buffers) - That's a separate small patch
So any remaining dropped character issues are reduced to 'broken' commands that use console I/O and then enter a busy loop which does not process console I/O - Now we're into hack territory
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> you wrote:
Well, if a command reads from the console (a transfer command for example) and then has a long delay (busy processing loop) before returning back to the command line processor then that command must be fundamentally broken -
???? Could you please explain what makes you think so?
What the command does, and when it performs I/O or when it spens time for other tasks is only up to the command.
If anything is broken then it is a design that puts restrictions on such behaviour.
Quoted 6 lines
a) Fix the command so it isn't broken
b) Have the command tell the console it has finished with the console before it starts the busy loop
c) Use UART managed hardware flow control
d) Implement interrupt based serial drivers
e) Prohibit multi-line input
Prohibit is a bit of a strong word here. "Not support" is more like it.
Quoted 1 line
So let's assume for the meantime that there are no 'broken' commands we can
This is not an assumption. No implementation must put any restrictions on the timing behaviour of any commands.
Quoted 1 line
simply issue an XOFF before running each command - That should eliminate
That's brainded overhead. In 99.999% of all cases it's not needed. And it happens at completely the wrong place.
Serial flow control is something we should deal with in the serial driver code. Nowhere else.
I will not accept such a design nor such an implementation.
Best regards,
Wolfgang Denk
Signature · 4 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 "Have you lived in this village all your life?" "No, not yet."
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 31 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> you wrote:
Well, if a command reads from the console (a transfer command for example) and then has a long delay (busy processing loop) before returning back to the command line processor then that command must be fundamentally broken -
???? Could you please explain what makes you think so?
What the command does, and when it performs I/O or when it spens time for other tasks is only up to the command.
If anything is broken then it is a design that puts restrictions on such behaviour.
Quoted 6 lines
a) Fix the command so it isn't broken
b) Have the command tell the console it has finished with the console before it starts the busy loop
c) Use UART managed hardware flow control
d) Implement interrupt based serial drivers
e) Prohibit multi-line inputProhibit is a bit of a strong word here. "Not support" is more like it.
Quoted 1 line
So let's assume for the meantime that there are no 'broken' commands we can
This is not an assumption. No implementation must put any restrictions on the timing behaviour of any commands.
Quoted 7 lines
Quoted 1 line
simply issue an XOFF before running each command - That should eliminate
That's brainded overhead. In 99.999% of all cases it's not needed. And it happens at completely the wrong place.
Serial flow control is something we should deal with in the serial driver code. Nowhere else.
This problem comes down to managing two asynchronous tasks - Command Processing and Serial Input Processing. Any solution to the problem is going to involve task switching. The way this is done in U-Boot currently is:
- When in the main() loop, U-Boot is effectively running a Command Processing Taks (monitor inbound characters, determine when a command has been entered) which continually switches over to Serial Input Processing Task. When a valid command is detected, U-Boot switches to the corresponding Command Processing Task
- Some 'commands' (file transfers in particular) have their own Serial Input Processing Sub-Tasks - The Command Processing Task may run for a 'significant' amount of time after these Sub-Tasks are finished with and no longer processing serial input
When you are dealing with asynchronous tasks, you can task switch by either:
1) Using a hardware interrupt (either a tick-timer or interrupt line from the serial UART)
2) Littering the main taks with co-operative interrupt calls
3) Task switch at clearly defined locations in the code
Option 1 is not supported in U-Boot
Option 2 is used by U-Boot for triggering the watchdog - It's not pretty, but in the absence of #1, we have no other option
Option 3 is all we are left with...
And U-Boot does actually do #3, but the 'clearly defined locations' are not obvious, and the switch is done _without_ telling the remote serial transmitter that U-Boot is busy and will not be able to process any more inbound characters for the time being.
Note that Serial Input Processing Task does not need to run after we have sent an XOFF and flushed the remote Tx buffer and local Rx buffer as the remote end _should_ have stopped sending characters. So in order to absolutely prevent dropped characters, it is a simple matter of sending an XOFF and flushing the buffers when we switch out of the Serial Input Processing Task and into the Command Processing Task. Sounds easy enough, BUT, the Serial Input Processing Task is a dumb task (it simply reads single characters from the UART) and has no idea what triggers the switch to the Command Processing Task so there is no way for it to know when to sent the XOFF - The Command Processing Task needs to tell the Serial Input Processing Task
I suggested a solution whereby any task which _knows_ it switches to the Serial Input Processing Taks (i.e. calls getc()) can:
- Tell the Serial Input Processing Task 'I will need serial input from the remote end, can you please arrange it so that I recieve those characters'
- Process the incoming stream (it is the responsibility of the task to make sure the stream is processed without dropping characters, or use a protocol which allows for resending of dropped charaters)
- Tell the Serial Input Processing Task 'OK, I've recieved all the data I need to and I will not be processing any more - You have been warned, so if you want don't want to loose input, you'll need to do something about it'
I'm at a loss to think of any other solution - Can you?
Quoted 1 line
I will not accept such a design nor such an implementation.
Then we live with dropped characters
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Hi,
Quoted 102 lines · Graeme Russ
Hi Wolfgang,
Quoted 31 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> you wrote:
Well, if a command reads from the console (a transfer command for example) and then has a long delay (busy processing loop) before returning back to the command line processor then that command must be fundamentally broken -
???? Could you please explain what makes you think so?
What the command does, and when it performs I/O or when it spens time for other tasks is only up to the command.
If anything is broken then it is a design that puts restrictions on such behaviour.
Quoted 6 lines
?a) Fix the command so it isn't broken
?b) Have the command tell the console it has finished with the console ? ? before it starts the busy loop
?c) Use UART managed hardware flow control
?d) Implement interrupt based serial drivers
?e) Prohibit multi-line inputProhibit is a bit of a strong word here. ?"Not support" is more like it.
Quoted 1 line
So let's assume for the meantime that there are no 'broken' commands we can
This is not an assumption. ?No implementation must put any restrictions on the timing behaviour of any commands.
Quoted 7 lines
Quoted 1 line
simply issue an XOFF before running each command - That should eliminate
That's brainded overhead. In 99.999% of all cases it's not needed. And it happens at completely the wrong place.
Serial flow control is something we should deal with in the serial driver code. ?Nowhere else.
This problem comes down to managing two asynchronous tasks - Command Processing and Serial Input Processing. Any solution to the problem is going to involve task switching. The way this is done in U-Boot currently is:
?- When in the
main()loop, U-Boot is effectively running a Command ? Processing Taks (monitor inbound characters, determine when a ? command has been entered) which continually switches over to Serial ? Input Processing Task. When a valid command is detected, U-Boot ? switches to the corresponding Command Processing Task
?- Some 'commands' (file transfers in particular) have their own Serial ? Input Processing Sub-Tasks - The Command Processing Task may run for a ? 'significant' amount of time after these Sub-Tasks are finished with ? and no longer processing serial inputWhen you are dealing with asynchronous tasks, you can task switch by either:
?1) Using a hardware interrupt (either a tick-timer or interrupt line from ? ?the serial UART)
?2) Littering the main taks with co-operative interrupt calls ?3) Task switch at clearly defined locations in the codeOption 1 is not supported in U-Boot
Option 2 is used by U-Boot for triggering the watchdog - It's not pretty, but in the absence of #1, we have no other option
Option 3 is all we are left with...And U-Boot does actually do #3, but the 'clearly defined locations' are not obvious, and the switch is done _without_ telling the remote serial transmitter that U-Boot is busy and will not be able to process any more inbound characters for the time being.
Note that Serial Input Processing Task does not need to run after we have sent an XOFF and flushed the remote Tx buffer and local Rx buffer as the remote end _should_ have stopped sending characters. So in order to absolutely prevent dropped characters, it is a simple matter of sending an XOFF and flushing the buffers when we switch out of the Serial Input Processing Task and into the Command Processing Task. Sounds easy enough, BUT, the Serial Input Processing Task is a dumb task (it simply reads single characters from the UART) and has no idea what triggers the switch to the Command Processing Task so there is no way for it to know when to sent the XOFF - The Command Processing Task needs to tell the Serial Input Processing Task
I suggested a solution whereby any task which _knows_ it switches to the Serial Input Processing Taks (i.e. calls
getc()) can:
?- Tell the Serial Input Processing Task 'I will need serial input from ? the remote end, can you please arrange it so that I recieve those ? characters'
?- Process the incoming stream (it is the responsibility of the task to ? make sure the stream is processed without dropping characters, or ? use a protocol which allows for resending of dropped charaters) ?- Tell the Serial Input Processing Task 'OK, I've recieved all the data ? I need to and I will not be processing any more - You have been warned, ? so if you want don't want to loose input, you'll need to do something ? about it'I'm at a loss to think of any other solution - Can you?
Quoted 8 lines
Quoted 1 line
I will not accept such a design nor such an implementation.
Then we live with dropped characters
Regards,
Graeme
Did I mention a can of worms? After 65 messages on this topic Scott's patch seems pretty appealing right now! We can even move it up a level in the s/w stack if that helps.
But to continue this a little, and donning my asbestos suit, should U-Boot have a CONFIG to enable an event loop called in delay functions, network functions, read/write operations, etc.? It would permit us to solve this problem properly I think, if we think it is worth solving. Not saying it is a good idea...
Regards,
Simon
PATCH v2 1/1 NS16550: buffer reads
Hi Simon,
On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass <[email protected]> wrote:
[big snip]
Quoted 3 lines
Did I mention a can of worms? After 65 messages on this topic Scott's patch seems pretty appealing right now! We can even move it up a level in the s/w stack if that helps.
But I agree with Wolfgang that Scott's proposal only hides the real problem even deeper and will make a real solution more difficult next time around
Quoted 5 lines
But to continue this a little, and donning my asbestos suit, should U-Boot have a CONFIG to enable an event loop called in delay functions, network functions, read/write operations, etc.? It would permit us to solve this problem properly I think, if we think it is worth solving. Not saying it is a good idea...
That is what the watchdog code does - It is a horrible hack, and every now and again, another patch comes through which adds another call to the watchdog trigger in some obscure loop in some obscure code location. Or someone submits new code for a board that does not have a watchdog that somebody else then uses on a board that does have one and the watchdog suddenly triggers resetting the board. It's messy and ugly.
My biggest point about the 'dropped character' issue is that the fix is simple provided the 'tasks' that use the serial console behave and inform the serial console driver that they have finished receiving characters - not a big ask in my opinion. Way smaller that spattering 'Check the serial port' all over the code
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Hi Graeme,
Quoted 6 lines · Graeme Russ
Hi Simon,
On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass <[email protected]> wrote:
[big snip]
Quoted 7 lines
Quoted 3 lines
Did I mention a can of worms? After 65 messages on this topic Scott's patch seems pretty appealing right now! We can even move it up a level in the s/w stack if that helps.
But I agree with Wolfgang that Scott's proposal only hides the real problem even deeper and will make a real solution more difficult next time around
I feel that the 'real' solution is flow control or buffering and probably both.
Quoted 14 lines
Quoted 6 lines
But to continue this a little, and donning my asbestos suit, should U-Boot have a CONFIG to enable an event loop called in delay functions, network functions, read/write operations, etc.? It would permit us to solve this problem properly I think, if we think it is worth solving. Not saying it is a good idea...
That is what the watchdog code does - It is a horrible hack, and every now and again, another patch comes through which adds another call to the watchdog trigger in some obscure loop in some obscure code location. Or someone submits new code for a board that does not have a watchdog that somebody else then uses on a board that does have one and the watchdog suddenly triggers resetting the board. It's messy and ugly.
Quoted 5 lines
My biggest point about the 'dropped character' issue is that the fix is simple provided the 'tasks' that use the serial console behave and inform the serial console driver that they have finished receiving characters - not a big ask in my opinion. Way smaller that spattering 'Check the serial port' all over the code
I kind-of agree so long as it is more of an event loop and less of something specific to serial (of course serial might be the only customer but then we could display a spinning thing on the LCD...). Funnily enough that problem (one bit of code wanting to tell serial something) exists on Seaboard, where the SPI driver has to tell the UART to flush or invalidate since they cannot co-exist. And that is why I was so interested in this patch.
Regards,
Simon
Quoted 5 lines
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Hi Simon,
Quoted 39 lines · Simon Glass
Hi Graeme,
Quoted 6 lines · Graeme Russ
Hi Simon,
On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass <[email protected]> wrote:
[big snip]
Quoted 7 lines
Quoted 3 lines
Did I mention a can of worms? After 65 messages on this topic Scott's patch seems pretty appealing right now! We can even move it up a level in the s/w stack if that helps.
But I agree with Wolfgang that Scott's proposal only hides the real problem even deeper and will make a real solution more difficult next time around
I feel that the 'real' solution is flow control or buffering and probably both.
Quoted 14 lines
Quoted 6 lines
But to continue this a little, and donning my asbestos suit, should U-Boot have a CONFIG to enable an event loop called in delay functions, network functions, read/write operations, etc.? It would permit us to solve this problem properly I think, if we think it is worth solving. Not saying it is a good idea...
That is what the watchdog code does - It is a horrible hack, and every now and again, another patch comes through which adds another call to the watchdog trigger in some obscure loop in some obscure code location. Or someone submits new code for a board that does not have a watchdog that somebody else then uses on a board that does have one and the watchdog suddenly triggers resetting the board. It's messy and ugly.
Quoted 5 lines
My biggest point about the 'dropped character' issue is that the fix is simple provided the 'tasks' that use the serial console behave and inform the serial console driver that they have finished receiving characters - not a big ask in my opinion. Way smaller that spattering 'Check the serial port' all over the code
Quoted 3 lines
I kind-of agree so long as it is more of an event loop and less of something specific to serial (of course serial might be the only customer but then we could display a spinning thing on the LCD...)
I don't think we need to get too fancy - U-Boot is not an OS :)
Quoted 4 lines
Funnily enough that problem (one bit of code wanting to tell serial something) exists on Seaboard, where the SPI driver has to tell the UART to flush or invalidate since they cannot co-exist. And that is why I was so interested in this patch.
Well, I have the feeling than an console API might be in order. The way U-Boot is structured at the moment, serial I/O is kind of taken for granted to 'just exist' and nobody needs to really be self-aware that they use it - The same cannot be said of users of USB and Network - Net is a good example - all 'users' of net call NetLoop() which does an eth_init()...do stuff...eth_halt()
So maybe a serial API which 'users' must 'wake up' and 'sleep'...
- console_configure() - Set parameters (port, baud rate etc)
- console_wake() - Prepare the console (could be serial, USB, netconsole)
- console_flush() - Throw away any buffered characters
- console_getc(), console_putc(), console_tstc() - Self explainatory
- console_sleep() - Shut the console down (send XOFF, turn of USB etc)
This would resolve the issue were devices are multiplexed with the serial UART like your SPI example. The command line interpreter calls console_sleep() to release the serial UART resource before the command which uses the multiplexed device wakes that device, does it's thing and shuts the device down before returning to the command line interpreter which wakes up the serial UART...
Regards,
Graeme
Regards,
Graeme
PATCH v2 1/1 NS16550: buffer reads
Dear Graeme,
Quoted 5 lines · In message <CALButCKQ_bgpOZquozzQnU01v41Y+uWo8ELLNqSE15D30p52ug@mail.gmail.com>…
Well, I have the feeling than an console API might be in order. The way U-Boot is structured at the moment, serial I/O is kind of taken for granted to 'just exist' and nobody needs to really be self-aware that they use it...
Indeed. All we need is a working set of srdin/stdout/stderr. Keep in mind that anything couldbe attached - not only a serial line.
Quoted 3 lines
... The same cannot be said of users of USB and Network -
Net is a good example - all 'users' of net callNetLoop()which does an eth_init()...do stuff...eth_halt()
Driver API...
Quoted 1 line
So maybe a serial API which 'users' must 'wake up' and 'sleep'...
Arghhh...
Quoted 5 lines
-
console_configure()- Set parameters (port, baud rate etc)
-console_wake()- Prepare the console (could be serial, USB, netconsole)
-console_flush()- Throw away any buffered characters
-console_getc(),console_putc(),console_tstc()- Self explainatory
-console_sleep()- Shut the console down (send XOFF, turn of USB etc)
Forget it. We will not have a separate and completely non-standard "serial API". If anything gets changed here, then in the context of a general driver API cleanup, or at least based on a design for such a generic driver API.
Quoted 5 lines
UART like your SPI example. The command line interpreter calls
console_sleep()to release the serial UART resource before the command which uses the multiplexed device wakes that device, does it's thing and shuts the device down before returning to the command line interpreter which wakes up the serial UART...
Sorry, but that's crappy by design.
Best regards,
Wolfgang Denk
Signature · 4 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 Marriage is the sole cause of divorce.
PATCH v2 1/1 NS16550: buffer reads
Hi Wolfgang,
Quoted 28 lines · Wolfgang Denk
Dear Graeme,
Quoted 5 lines · In message <CALButCKQ_bgpOZquozzQnU01v41Y+uWo8ELLNqSE15D30p52ug@mail.gmail.com>…
Well, I have the feeling than an console API might be in order. The way U-Boot is structured at the moment, serial I/O is kind of taken for granted to 'just exist' and nobody needs to really be self-aware that they use it...
Indeed. All we need is a working set of srdin/stdout/stderr. Keep in mind that anything couldbe attached - not only a serial line.
Quoted 3 lines
... The same cannot be said of users of USB and Network -
Net is a good example - all 'users' of net callNetLoop()which does an eth_init()...do stuff...eth_halt()Driver API...
Quoted 1 line
So maybe a serial API which 'users' must 'wake up' and 'sleep'...
Arghhh...
Quoted 5 lines
-
console_configure()- Set parameters (port, baud rate etc)
-console_wake()- Prepare the console (could be serial, USB, netconsole)
-console_flush()- Throw away any buffered characters
-console_getc(),console_putc(),console_tstc()- Self explainatory
-console_sleep()- Shut the console down (send XOFF, turn of USB etc)
Quoted 4 lines
Forget it. We will not have a separate and completely non-standard "serial API". If anything gets changed here, then in the context of a general driver API cleanup, or at least based on a design for such a generic driver API.
Funny, I wrote that without the code in front of me. But look at this...
struct serial_device {
char name[NAMESIZE];
int (*init) (void);
int (*uninit) (void);
void (*setbrg) (void);
int (*getc) (void);
int (*tstc) (void);
void (*putc) (const char c);
void (*puts) (const char *s);
#if CONFIG_POST & CONFIG_SYS_POST_UART
void (*loop) (int);#endif
struct serial_device *next; };
and
struct stdio_dev {
int flags; /* Device flags: input/output/system */
int ext; /* Supported extensions */
char name[16] /* Device name *//* GENERAL functions */
int (*start) (void); /* To start the device */ int (*stop) (void); /* To stop the device */
/* OUTPUT functions */
void (*putc) (const char c); /* To put a char */ void (*puts) (const char *s); /* To put a string */
/* INPUT functions */
int (*tstc) (void); /* To test if a char is ready...*/ int (*getc) (void); /* To get that char */
/* Other functions */
void *priv; /* Private extensions */ struct list_head list; };
and
void serial_stdio_init (void)
{
struct stdio_dev dev;
struct serial_device *s = serial_devices;
while (s) {
memset (&dev, 0, sizeof (dev));
strcpy (dev.name, s->name);
dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
dev.start = s->init;
dev.stop = s->uninit;
dev.putc = s->putc;
dev.puts = s->puts;
dev.getc = s->getc;
dev.tstc = s->tstc;
stdio_register (&dev);
s = s->next;
}
}and mpc512x has this gem
int close_port(int num)
{
struct stdio_dev *port;
int ret;
char name[7];
if (num < 0 || num > 11)
return -1;
sprintf(name, "psc%d", num);
port = stdio_get_by_name(name);
if (!port)
return -1;
ret = port->stop();
clear_bit(num, &initialized);
return ret;
}stdio_dev->start() is called in console_setfile() which is called by console_init_r(). stdio_dev->stop() is never called apart from in the above mpc512x example.
As I said, stdio is, unlike other devices, taken for granted - It gets setup during console_init_r() and is assumed to be in-volatile from then on.
So the hooks are already there - All that would be needed is a hook to allow console users to ultimately call stdio_dev->start when they want to start using stdio and stdio_dev->stop when they are finished. Of course the physical device itself is hidden and could be serial, USB, Ethernet, VGA, Braille display - whatever.
Hmmm, stdio_open() and stdio_close()
Quoted 7 lines
Quoted 5 lines
UART like your SPI example. The command line interpreter calls
console_sleep()to release the serial UART resource before the command which uses the multiplexed device wakes that device, does it's thing and shuts the device down before returning to the command line interpreter which wakes up the serial UART...Sorry, but that's crappy by design.
When resources are tight - The eNET board has 8-bit CF ports instead of 16 bit because of board space and FPGA limitation which necessitated a slight mod to the Linux CF driver. Crappy yes, but hey, when given lemons
Regards,
Graeme
Wolfgang Denk
NAKPATCH v2 1/1 NS16550: buffer reads
Dear Graeme Russ,
Quoted 6 lines · In message <[email protected]> you wrote:
So the hooks are already there - All that would be needed is a hook to allow console users to ultimately call
stdio_dev->startwhen they want to start using stdio andstdio_dev->stopwhen they are finished. Of course the physical device itself is hidden and could be serial, USB, Ethernet, VGA, Braille display - whatever.
And I say No. No application whatever should have to care about sarting or stopping any drivers that provide stdio functionality.
Quoted 1 line
Hmmm,
stdio_open()andstdio_close()
NAK.
Sorry, we are wasting time here. Can we please put this to an end now?
Best regards,
Wolfgang Denk
Signature · 4 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 Above all else -- sky.
PATCH v2 1/1 NS16550: buffer reads
In reply to Graeme RussQuoted 6 lines · Graeme Russ
Hi Simon,
On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass <[email protected]> wrote:
[big snip]
Quoted 7 lines
Quoted 3 lines
Did I mention a can of worms? After 65 messages on this topic Scott's patch seems pretty appealing right now! We can even move it up a level in the s/w stack if that helps.
But I agree with Wolfgang that Scott's proposal only hides the real problem even deeper and will make a real solution more difficult next time around
Even in Linux you can get loss if you overflow the kernel's buffer.
Flow control is just not that commonly used in this sort of application.
If we insist on software flow control for U-Boot to get any usable
multi-line pasting to work, the user has to reconfigure their terminal software for this -- it would take some effort for me to figure out how to do that on our board farm serial server. And maybe that's not the right setting for what the user wants to do with the port later on.
Someone also raised the issue of TX buffering -- and you replied that it should be turned off if hardware flow control is not used. This is not what happens in Linux, at least, and we should not be depending in U-Boot on the other end not using TX buffers.
The point here is to make an incremental improvement that solves a problem for some people (even if not "solving" the general problem), without significant impact elsewhere. The minor size increase when the option is not enabled can be fixed. See previous discussion:
http://lists.denx.de/pipermail/u-boot/2011-April/089878.html
If someone wants to further improve things by adding optional interrupt support or flow control, I don't see how this patch makes that more difficult.
-Scott
PATCH v2 1/1 NS16550: buffer reads
Dear Scott Wood,
Quoted 3 lines · In message <[email protected]> you wrote:
The point here is to make an incremental improvement that solves a problem for some people (even if not "solving" the general problem),
Sorry, but this is not an improment in any way, even if it looks so at first glance. Actually it's papering over the bugs that result from mis-use of the given user interface, without actually providing a fix.
Instead of making the users aware of correct use, and providing help to do so, this makes people believe that serial input was a reliable transfer medium and will accept any amount of data dumped to it.
I can only repeat (and I think I'll do this for the last time in this thread):
- Patches are welcome that allow to overcome this existing limitation are welcome - butt hese must provide a reliablke solution that works in (at least nearly) all test cases.
- Failure to process multi-line input is a restriction, and if you like you may even consider it a bug. But papering over that bug and suggesting to the end users that they are allowed to use multi-line input while we all know very well that it actually works only under certain (somewhar fragile) assumptions is a fraudulent misrepresen- tation.
The approaches discussed so far all have issues. So far I don't see an approach that appears to be generally usable, clean in design and reliable in operation.
Regarding this specific patch you submitted: I made my mind up, and I reject it for the resons given above and in previous messages.
Regarding approaches to lift this restriction: I insist on a clean design. It would be helpful if we could base this on a clean driver model, but we are not there yet - nevertheless, we should keep that in mind. Iam also pretty much convinced that the implementation must be contained within the serial input driver layer, without need to change "application" code or to put restrictions on the behaviour of any code that reads or writes from resp. to the console in genral, and the serial port in particular.
Any code to test one approach or another is welcome, even if it's half-baked. But I will not accept such prelimiary code for mainline that "solves a problem for some people" without actually fixing it.
Thanks.
Best regards,
Wolfgang Denk
Signature · 6 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 At the source of every error which is blamed on the computer you will find at least two human errors, including the error of blaming it on the computer.
PATCH v2 1/1 NS16550: buffer reads
Quoted 2 lines · Wolfgang Denk
- Failure to process multi-line input is a restriction, and if you like you may even consider it a bug.
Is this restriction documented anywhere?
Quoted 5 lines
But papering over that bug and
suggesting to the end users that they are allowed to use multi-line input while we all know very well that it actually works only under certain (somewhar fragile) assumptions is a fraudulent misrepresen- tation.
Well then I guess you'd better forcibly dump any input received after a newline, before the next prompt is displayed -- because the current U-Boot implementation "fraudulently" made me think such a thing was possible but resource-constrained when it worked for up to 5 lines for the particular thing I was trying to paste. :-)
Quoted 3 lines
The approaches discussed so far all have issues. So far I don't see an approach that appears to be generally usable, clean in design and reliable in operation.
That seems like it would be interrupts, but there's a conflict here between "solve it perfectly (at least up to the limit of the internal buffer)" and "this is just a bootloader, keep it simple". And it seems there's no room for a pragmatic middle ground.
Quoted 2 lines
Regarding this specific patch you submitted: I made my mind up, and I reject it for the resons given above and in previous messages.
Oh well, I guess we'll just maintain it locally, or find some alternative to pasting things.
-Scott
PATCH v2 1/1 NS16550: buffer reads
Dear Scott Wood,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines
- Failure to process multi-line input is a restriction, and if you like you may even consider it a bug.
Is this restriction documented anywhere?
Only indirectly - it's a consequence of the "strictly single-tasking" statement.
This is not very explicit, agreed. Please feel free to improve the documentation - you are already registered as wiki user, so you just have to edit the text.
Quoted 8 lines
Quoted 3 lines
The approaches discussed so far all have issues. So far I don't see an approach that appears to be generally usable, clean in design and reliable in operation.
That seems like it would be interrupts, but there's a conflict here between "solve it perfectly (at least up to the limit of the internal buffer)" and "this is just a bootloader, keep it simple". And it seems there's no room for a pragmatic middle ground.
I also think an interrupt driven driver would be the best approach. If the interrupt part can be configured out so users who don't need/want this feature are unaffected I see no reason why such a driver would not be accepted.
Quoted 2 lines
Oh well, I guess we'll just maintain it locally, or find some alternative to pasting things.
Some terminal emulations allow for special configurations, like inter-character delays, end-of-line delays; maybe there are even ones that can wait for a prompt? If not - expect is your friend ...
Best regards,
Wolfgang Denk
Signature · 6 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 On our campus the UNIX system has proved to be not only an effective software tool, but an agent of technical and social change within the University. - John Lions (U. of Toronto (?))
PATCH v2 1/1 NS16550: buffer reads
In reply to Graeme RussDear Graeme,
Quoted 5 lines · In message <CALButCKRs7kNa086V-jUC0h-yjA7jfNo+2C9ifKpoREsdmWZQA@mail.gmail.com>…
This problem comes down to managing two asynchronous tasks - Command Processing and Serial Input Processing. Any solution to the problem is going to involve task switching. The way this is done in U-Boot currently is:
I wonder how this idea of "tasks" sneaked into your brain.
I agree that one thing we suffer from is a clean device model. And I will not hesitate to agree that things would be way easier if the serial driver was interrupt based.
But tasks in the classical sense have nothing to do with it (I don't consider an ISR a "task").
Quoted 9 lines
- When in the
main()loop, U-Boot is effectively running a Command Processing Taks (monitor inbound characters, determine when a command has been entered) which continually switches over to Serial Input Processing Task. When a valid command is detected, U-Boot switches to the corresponding Command Processing Task
- Some 'commands' (file transfers in particular) have their own Serial Input Processing Sub-Tasks - The Command Processing Task may run for a 'significant' amount of time after these Sub-Tasks are finished with and no longer processing serial input
I disagree completely. There is no such thing as "Command Processing Tasks" or "Serial Input Processing Tasks" or Subtasks or anything like that. There are no tasks at all.
Quoted 5 lines
When you are dealing with asynchronous tasks, you can task switch by either:
1) Using a hardware interrupt (either a tick-timer or interrupt line from the serial UART)
Yes, an interrupt driven driver would be one valid approach to adddress this issue. It brings additional complexity in other areas, though - things like code size, need for dual mode (polled before relocation, interrupt driven later?) etc. come to mind.
Quoted 4 lines
2) Littering the main taks with co-operative interrupt calls
3) Task switch at clearly defined locations in the codeOption 1 is not supported in U-Boot
??? What makes you think so? It's frowned upon, because the disadvantages are way bigger than the advantages, but that's all.
Quoted 2 lines
Option 2 is used by U-Boot for triggering the watchdog - It's not pretty, but in the absence of #1, we have no other option
This is another misconception. We do have interrupts on some architectures, but if you trigger a watchdog from a timer interrupt or similar you can as well turn it off. That's not how a watchdog is supposed to work.
Quoted 1 line
Option 3 is all we are left with...
I disagree.
Quoted 2 lines
I suggested a solution whereby any task which _knows_ it switches to the Serial Input Processing Taks (i.e. calls
getc()) can:
Please stop talking about a "Serial Input Processing Task". What you mean is a serial driver. This driver is supposed to provide nothing else to the "application code" than the functionality we can attach to the stdin and stdout/stderr I/O channels.
All things like flow control (be it hard or soft), buffering, etc. etc. are internal to the serial driver only.
Actually please stop talking about tasks at all. U-Boot is single-tasked. Period.
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 Cogito cogito ergo cogito sum - "I think that I think, therefore I think that I am." - Ambrose Bierce, "The Devil's Dictionary"
PATCH v4 2/2 NS16550: buffer reads
In reply to Simon GlassDear Simon Glass,
Quoted 11 lines · In message <[email protected]> you wrote:
From: Scott Wood <[email protected]>
From: Scott Wood <[email protected]>
This improves the performance of U-Boot when accepting rapid input, such as pasting a sequence of commands.
Without this patch, on P4080DS I see a maximum of around 5 lines can be pasted. With this patch, it handles around 70 lines before lossage, long enough for most things you'd paste.
Quoted 7 lines
With it enabled, on ppc it's an extra 396 bytes of image size, and 1056 bytes of BSS.
ARM note from Simon Glass <[email protected]> - ARM code size goes from 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
Signed-off-by Scott Wood <[email protected]>
I've made up my mind. The reasons why you want to add such code are well understood, but the unsolved and unsolvable issues with this approach are so severe that I don't want to raise false hopes in innocent users.
Sorry, but I reject this patch.
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 "Nature is very un-American. Nature never hurries." - William George Jordan
Wolfgang Denk
AppliedPATCH v4 1/2 NS16550: trivial code clean for checkpatch
In reply to Simon GlassDear Simon Glass,
Quoted 8 lines · In message <[email protected]> you wrote:
This removes most checkpatch warnings from the ns16550 driver and its header.
Signed-off-by Simon Glass <[email protected]>--- drivers/serial/ns16550.c | 37 ++++++++++++++++++++----------------- include/ns16550.h | 16 ++++++++-------- 2 files changed, 28 insertions(+), 25 deletions(-)
Applied, thanks.
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 Don't put off for tomorrow what you can do today, because if you enjoy it today you can do it again tomorrow.