[U-Boot] [PATCH v2] NS16550: buffer reads
From: Scott Wood
From: Scott Wood
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 - ARM code size goes from
212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
Signed-off-by: Scott Wood
---
Changes in v2:
- Fix checkpatch warnings, the other one was already there
README |8
drivers/serial/ns16550.c | 96 +++--
drivers/serial/serial.c |5 +-
include/ns16550.h|4 +-
4 files changed, 104 insertions(+), 9 deletions(-)
diff --git a/README b/README
index 7e032a9..28d2e8a 100644
--- a/README
+++ b/README
@@ -2862,6 +2862,14 @@ 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_BUFFER_READS:
+ 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.
+
Low Level (hardware related) configuration options:
---
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 0174744..a012e5d 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -4,12 +4,15 @@
* modified to use CONFIG_SYS_ISA_MEM and new defines
*/
+#include
#include
#include
#include
#include
#include
+DECLARE_GLOBAL_DATA_PTR;
+
#define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no parity */
#define UART_MCRVAL (UART_MCR_DTR | \
UART_MCR_RTS) /* RTS/DTR */
@@ -92,21 +95,104 @@ void NS16550_putc (NS16550_t com_port, char c)
}
#ifndef CONFIG_NS16550_MIN_FUNCTIONS
-char NS16550_getc (NS16550_t com_port)
+
+static char NS16550_raw_getc(NS16550_t regs)
{
- while ((serial_in(&com_port->lsr) & UART_LSR_DR) == 0) {
+ while ((serial_in(®s->lsr) & UART_LSR_DR) == 0) {
#ifdef CONFIG_USB_TTY
extern void usbtty_poll(void);
usbtty_poll();
#endif
WATCHDOG_RESET();
}
- return serial_in(&com_port->rbr);
+ return serial_in(®s->rbr);
+}
+
+static int NS16550_raw_tstc(NS16550_t regs)
+{
+ return ((serial_in(®s->lsr) & UART_LSR_DR) != 0);
+}
+
+
+#ifdef CONFIG_NS16550_BUFFER_READS
+
+#define BUF_SIZE 256
+#define NUM_PORTS 4
+
+struct ns16550_priv {
+ char buf[BUF_SIZE];
+ unsigned int head, tail;
+};
+
+static struct ns16550_priv rxstate[NUM_PORTS];
+
+static void enqueue(unsigned int port, char ch)
+{
+ /* If queue is full, drop the character. */
+ if ((rxstate[port].head - rxstate[port].tail - 1) % BUF_SIZE == 0)
+ return;
+
+ rxstate[port].buf[rxstate[port].tail] = ch;
+ rxstate[port].tail = (rxstate[port].tail + 1) % BUF_SIZE;
+}
+
+static int dequeue(unsigned int port, char *ch)
+{
+ /* Empty queue? */
+ if (rxstate[port].head == rxstate[port].tail)
+ return 0;
+
+ *ch = rxstate[port].buf[rxstate[port].head];
+ rxstate[port].head = (rxstate[port].head + 1) % BUF_SIZE;
+ return 1;
+}
+
+static void fill_rx_buf(NS16550_t regs, unsigned int port)
+{
+ while ((serial_in(®s->lsr) & UART_LSR_DR) != 0)
+ enqueue(port, serial_in(®s->rbr));
+}
+
+char NS16550_getc(NS16550_t regs, unsigned int port)
+{
+ char ch;
+
+ if (port >= NUM_PORTS || !(gd->flags & GD_FLG_RELOC))
+ return NS16550_raw_getc(regs);
+
+ do {
+#ifdef CONFIG_USB_TTY
+ extern void usbtty_poll(void);
+ usbtty_poll();
+#endif
+ fill_rx_buf(regs, port);
+ WATCHDOG_RESET();
+ } while (!dequeue(port, &ch));
+
+ return ch;
+}
+
+int NS16550_tstc(NS16550_t regs, unsigned int port)
+{
+ if (port >= NUM_PORTS || !(gd->flags & GD_FLG_RELOC))
+ return NS16550_raw_tstc(regs);
+
+ fill_rx_buf(regs, port);
+
+ return rxstate[port].head != rxstate[port].tail;
+}
+
+#else /* CONFIG_NS16550_BUFFER_READS */
+
+char NS16550_getc(NS16550_t regs, unsigned int port)
+{
+ return NS16550_raw_getc(regs);
}
-int NS16550_tstc (NS16550_t com_port)
+int NS16550_tstc(NS16550_t regs, unsigned int port)
{
- return ((serial_in(&com_port->lsr) & UART_LSR_DR) != 0);
+ return NS16550_raw_tstc(regs);
}
+#endif /*
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Simon,
Le 15/10/2011 02:03, Simon Glass a écrit :
> From: Scott Wood
>
> From: Scott Wood
>
> 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 - ARM code size goes from
> 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
>
> Signed-off-by: Scott Wood
> ---
> Changes in v2:
> - Fix checkpatch warnings, the other one was already there
A general question: is the issue with input as such, or with echoing
this input?
> README |8
> drivers/serial/ns16550.c | 96
> +++--
> drivers/serial/serial.c |5 +-
> include/ns16550.h|4 +-
> 4 files changed, 104 insertions(+), 9 deletions(-)
>
> diff --git a/README b/README
> index 7e032a9..28d2e8a 100644
> --- a/README
> +++ b/README
> @@ -2862,6 +2862,14 @@ 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_BUFFER_READS:
> + 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.
> +
> Low Level (hardware related) configuration options:
> ---
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 0174744..a012e5d 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -4,12 +4,15 @@
>* modified to use CONFIG_SYS_ISA_MEM and new defines
>*/
>
> +#include
> #include
> #include
> #include
> #include
> #include
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> #define UART_LCRVAL UART_LCR_8N1/* 8 data, 1 stop, no parity */
> #define UART_MCRVAL (UART_MCR_DTR | \
>UART_MCR_RTS) /* RTS/DTR */
> @@ -92,21 +95,104 @@ void NS16550_putc (NS16550_t com_port, char c)
> }
>
> #ifndef CONFIG_NS16550_MIN_FUNCTIONS
> -char NS16550_getc (NS16550_t com_port)
> +
> +static char NS16550_raw_getc(NS16550_t regs)
> {
> - while ((serial_in(&com_port->lsr)& UART_LSR_DR) == 0) {
> + while ((serial_in(®s->lsr)& UART_LSR_DR) == 0) {
> #ifdef CONFIG_USB_TTY
> extern void usbtty_poll(void);
> usbtty_poll();
> #endif
> WATCHDOG_RESET();
> }
> - return serial_in(&com_port->rbr);
> + return serial_in(®s->rbr);
> +}
> +
> +static int NS16550_raw_tstc(NS16550_t regs)
> +{
> + return ((serial_in(®s->lsr)& UART_LSR_DR) != 0);
> +}
> +
> +
> +#ifdef CONFIG_NS16550_BUFFER_READS
> +
> +#define BUF_SIZE 256
I guess this define conditions how well U-Boot will cope with steady
high-rate input, correct? If so, I'd like it to be a configuration
option -- actually, one could combine enabling the option *and* setting
the expected buffer size.
> +#define NUM_PORTS 4
> +
> +struct ns16550_priv {
> + char buf[BUF_SIZE];
> + unsigned int head, tail;
> +};
> +
> +static struct ns16550_priv rxstate[NUM_PORTS];
> +
> +static void enqueue(unsigned int port, char ch)
> +{
> + /* If queue is full, drop the character. */
> + if ((rxstate[port].head - rxstate[port].tail - 1) % BUF_SIZE == 0)
> + return;
> +
> + rxstate[port].buf[rxstate[port].tail] = ch;
> + rxstate[port].tail = (rxstate[port].tail + 1) % BUF_SIZE;
> +}
> +
> +static int dequeue(unsigned int port, char *ch)
> +{
> + /* Empty queue? */
> + if (rxstate[port].head == rxstate[port].tail)
> + return 0;
> +
> + *ch = rxstate[port].buf[rxstate[port].head];
> + rxstate[port].head = (rxstate[port].head + 1) % BUF_SIZE;
> + return 1;
> +}
> +
> +static void fill_rx_buf(NS16550_t regs, unsigned int port)
> +{
> + while ((serial_in(®s->lsr)& UART_LSR_DR) != 0)
> + enqueue(port, serial_in(®s->rbr));
> +}
> +
> +char NS16550_getc(NS16550_t regs, unsigned int port)
> +{
> + char ch;
> +
> + if (port>= NUM_PORTS || !(gd->flags& GD_FLG_RELOC))
> + return NS16550_raw_getc(regs);
> +
> + do {
> +#ifdef CONFIG_USB_TTY
> + extern void usbtty_poll(void);
> + usbtty_poll();
> +#endif
> + fill_rx_buf(regs, port);
> + WATCHDOG_RESET();
> + } while (!dequeue(port,&ch));
> +
> + return ch;
> +}
> +
> +int NS16550_tstc(NS16550_t regs, uns
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Albert,
On Sat, Oct 15, 2011 at 3:43 AM, Albert ARIBAUD
wrote:
> Hi Simon,
>
> Le 15/10/2011 02:03, Simon Glass a écrit :
>>
>> From: Scott Wood
>>
>> From: Scott Wood
>>
>> 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 - ARM code size goes from
>> 212 to 484 bytes (extra 272 bytes), BSS to 1056 bytes.
>>
>> Signed-off-by: Scott Wood
>> ---
>> Changes in v2:
>> - Fix checkpatch warnings, the other one was already there
>
> A general question: is the issue with input as such, or with echoing this
> input?
The problem is in echoing the input, since this requires the uart to
wait once its transmit FIFO is full. While it is waiting (and not
checking the receive side), it misses characters coming from the other
end. Basically we need to constantly check the receiving side while we
are transmitting.
>
>> README | 8
>> drivers/serial/ns16550.c | 96
>> +++--
>> drivers/serial/serial.c | 5 +-
>> include/ns16550.h | 4 +-
>> 4 files changed, 104 insertions(+), 9 deletions(-)
>>
>> diff --git a/README b/README
>> index 7e032a9..28d2e8a 100644
>> --- a/README
>> +++ b/README
>> @@ -2862,6 +2862,14 @@ 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_BUFFER_READS:
>> + 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.
>> +
>> Low Level (hardware related) configuration options:
>> ---
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index 0174744..a012e5d 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -4,12 +4,15 @@
>> * modified to use CONFIG_SYS_ISA_MEM and new defines
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> #include
>> #include
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> #define UART_LCRVAL UART_LCR_8N1 /* 8 data, 1 stop, no
>> parity */
>> #define UART_MCRVAL (UART_MCR_DTR | \
>> UART_MCR_RTS) /* RTS/DTR */
>> @@ -92,21 +95,104 @@ void NS16550_putc (NS16550_t com_port, char c)
>> }
>>
>> #ifndef CONFIG_NS16550_MIN_FUNCTIONS
>> -char NS16550_getc (NS16550_t com_port)
>> +
>> +static char NS16550_raw_getc(NS16550_t regs)
>> {
>> - while ((serial_in(&com_port->lsr)& UART_LSR_DR) == 0) {
>> + while ((serial_in(®s->lsr)& UART_LSR_DR) == 0) {
>> #ifdef CONFIG_USB_TTY
>> extern void usbtty_poll(void);
>> usbtty_poll();
>> #endif
>> WATCHDOG_RESET();
>> }
>> - return serial_in(&com_port->rbr);
>> + return serial_in(®s->rbr);
>> +}
>> +
>> +static int NS16550_raw_tstc(NS16550_t regs)
>> +{
>> + return ((serial_in(®s->lsr)& UART_LSR_DR) != 0);
>> +}
>> +
>> +
>> +#ifdef CONFIG_NS16550_BUFFER_READS
>> +
>> +#define BUF_SIZE 256
>
> I guess this define conditions how well U-Boot will cope with steady
> high-rate input, correct? If so, I'd like it to be a configuration option --
> actually, one could combine enabling the option *and* setting the expected
> buffer size.
Yes so you could have something like:
#define CONFIG_NS16550_BUFFER_READS 256
>
>> +#define NUM_PORTS 4
>> +
>> +struct ns16550_priv {
>> + char buf[BUF_SIZE];
>> + unsigned int head, tail;
>> +};
>> +
>> +static struct ns16550_priv rxstate[NUM_PORTS];
>> +
>> +static void enqueue(unsigned int port, char ch)
>> +{
>> + /* If queue is full, drop the character. */
>> + if ((rxstate[port].head - rxstate[port].tail - 1) % BUF_SIZE == 0)
>> + return;
>> +
>> + rxstate[port].buf[rxstate[port].tail] = ch;
>> + rxstate[port].tail = (rxstate[port].tail + 1) % BUF_SIZE;
>> +}
>> +
>> +static int dequeue(unsigned int port, char *ch)
>> +{
>> + /* Empty queue? */
>> + if (rxstate[port].head == rxstate[port].tail)
>> + return 0;
>> +
>> + *ch = rxstate[port].buf[rxstate[port].head];
>> + rxstate[port].head = (rxstate[port].head + 1) % BUF_SIZE;
>> + return 1;
>> +}
>> +
>> +static void fill_rx_buf(NS16550_t regs, unsigned
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > > A general question: is the issue with input as such, or with echoing this > > input? > > The problem is in echoing the input, since this requires the uart to > wait once its transmit FIFO is full. While it is waiting (and not > checking the receive side), it misses characters coming from the other > end. Basically we need to constantly check the receiving side while we > are transmitting. No, we do not. U-Boot is strictly single tasking. This is one of the fundamental design principles. We do not receive network packets while sending ourself, or when doing other things. We do not receive characters on the serial line while sending characters, or when doing other things. This is known behaviour, and you should just adapt to it. I don't know what exactly is your situation when you run into such problems - the recommended approach for example when remote-controlling U-Boot is to wait for the next input prompt before sending the next line of characters - and stop after the newline, until you receive the next prompt. 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: w...@denx.de I believe you find life such a problem because you think there are the good people and the bad people. You're wrong, of course. There are, always and only, the bad people, but some of them are on oppo- site sides. - Terry Pratchett, _Guards! Guards!_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Sat, Oct 15, 2011 at 9:02 AM, Wolfgang Denk wrote: > Dear Simon Glass, > > In message > you > wrote: >> >> > A general question: is the issue with input as such, or with echoing this >> > input? >> >> The problem is in echoing the input, since this requires the uart to >> wait once its transmit FIFO is full. While it is waiting (and not >> checking the receive side), it misses characters coming from the other >> end. Basically we need to constantly check the receiving side while we >> are transmitting. > > No, we do not. > > U-Boot is strictly single tasking. This is one of the fundamental > design principles. We do not receive network packets while sending > ourself, or when doing other things. We do not receive characters on > the serial line while sending characters, or when doing other things. > > This is known behaviour, and you should just adapt to it. I don't know > what exactly is your situation when you run into such problems - the > recommended approach for example when remote-controlling U-Boot is to > wait for the next input prompt before sending the next line of > characters - and stop after the newline, until you receive the next > prompt. I certainly don't want to break a fundamental design principle. . The situation this occurs is when you paste characters into a serial terminal connected to U-Boot. This is a pretty common requirement. Yes you can manually select each line and paste it but that is a pain. With this patch it is possible to paste a series of 'setenv' commands, for example. This caused an awful lot of confusion here because characters would be silently dropped within the command line (normally about 16 chars from the end). Then the system would not boot. It is really not a friendly experience. Regards, Simon > > 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: w...@denx.de > I believe you find life such a problem because you think there are > the good people and the bad people. You're wrong, of course. There > are, always and only, the bad people, but some of them are on oppo- > site sides. - Terry Pratchett, _Guards! Guards!_ > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Le 15/10/2011 18:12, Simon Glass a écrit : > Hi Wolfgang, > > On Sat, Oct 15, 2011 at 9:02 AM, Wolfgang Denk wrote: >> Dear Simon Glass, >> >> In >> message >> you wrote: >>> A general question: is the issue with input as such, or with echoing this input? >>> >>> The problem is in echoing the input, since this requires the uart to >>> wait once its transmit FIFO is full. While it is waiting (and not >>> checking the receive side), it misses characters coming from the other >>> end. Basically we need to constantly check the receiving side while we >>> are transmitting. >> >> No, we do not. >> >> U-Boot is strictly single tasking. This is one of the fundamental >> design principles. We do not receive network packets while sending >> ourself, or when doing other things. We do not receive characters on >> the serial line while sending characters, or when doing other things. >> >> This is known behaviour, and you should just adapt to it. I don't know >> what exactly is your situation when you run into such problems - the >> recommended approach for example when remote-controlling U-Boot is to >> wait for the next input prompt before sending the next line of >> characters - and stop after the newline, until you receive the next >> prompt. > > I certainly don't want to break a fundamental design principle. > . > The situation this occurs is when you paste characters into a serial > terminal connected to U-Boot. This is a pretty common requirement. Yes > you can manually select each line and paste it but that is a pain. > With this patch it is possible to paste a series of 'setenv' commands, > for example. > > This caused an awful lot of confusion here because characters would be > silently dropped within the command line (normally about 16 chars from > the end). Then the system would not boot. It is really not a friendly > experience. > > Regards, > Simon How about using the time-honored flow-control mechanism? If the serial port supports hardware CTS and RTS signals, you can use them to sync with the sender. If not, you can send XOFF and XON to respectively ask the sender to pause and resume sending. Of course, the sender must support hardware or software flow control. Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Albert, On Sat, Oct 15, 2011 at 9:21 AM, Albert ARIBAUD wrote: > Le 15/10/2011 18:12, Simon Glass a écrit : >> >> Hi Wolfgang, >> >> On Sat, Oct 15, 2011 at 9:02 AM, Wolfgang Denk wrote: >>> >>> Dear Simon Glass, >>> >>> In >>> message >>> you wrote: > A general question: is the issue with input as such, or with echoing > this > input? The problem is in echoing the input, since this requires the uart to wait once its transmit FIFO is full. While it is waiting (and not checking the receive side), it misses characters coming from the other end. Basically we need to constantly check the receiving side while we are transmitting. >>> >>> No, we do not. >>> >>> U-Boot is strictly single tasking. This is one of the fundamental >>> design principles. We do not receive network packets while sending >>> ourself, or when doing other things. We do not receive characters on >>> the serial line while sending characters, or when doing other things. >>> >>> This is known behaviour, and you should just adapt to it. I don't know >>> what exactly is your situation when you run into such problems - the >>> recommended approach for example when remote-controlling U-Boot is to >>> wait for the next input prompt before sending the next line of >>> characters - and stop after the newline, until you receive the next >>> prompt. >> >> I certainly don't want to break a fundamental design principle. >> . >> The situation this occurs is when you paste characters into a serial >> terminal connected to U-Boot. This is a pretty common requirement. Yes >> you can manually select each line and paste it but that is a pain. >> With this patch it is possible to paste a series of 'setenv' commands, >> for example. >> >> This caused an awful lot of confusion here because characters would be >> silently dropped within the command line (normally about 16 chars from >> the end). Then the system would not boot. It is really not a friendly >> experience. >> >> Regards, >> Simon > > How about using the time-honored flow-control mechanism? If the serial port > supports hardware CTS and RTS signals, you can use them to sync with the > sender. If not, you can send XOFF and XON to respectively ask the sender to > pause and resume sending. Of course, the sender must support hardware or > software flow control. It is common to have just a 2-wire UART for the console. Perhaps this is unfortunate. Are you suggesting that U-Boot should send XOFF sometimes, or does it support this already? (I haven't seen it). When would U-Boot send XOFF? Perhaps it could send XOFF at the end of each line that it receives. Then XON when it starts reading the next line or maybe when it runs out of characters and wants more. Most terminal problems will support software flow control. But are you sure this is better? Regards, Simon > > Amicalement, > -- > Albert. > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
On Sat, Oct 15, 2011 at 9:50 AM, Simon Glass wrote: > Hi Albert, > > On Sat, Oct 15, 2011 at 9:21 AM, Albert ARIBAUD > wrote: >> Le 15/10/2011 18:12, Simon Glass a écrit : >>> >>> Hi Wolfgang, >>> >>> On Sat, Oct 15, 2011 at 9:02 AM, Wolfgang Denk wrote: Dear Simon Glass, In message you wrote: > >> A general question: is the issue with input as such, or with echoing >> this >> input? > > The problem is in echoing the input, since this requires the uart to > wait once its transmit FIFO is full. While it is waiting (and not > checking the receive side), it misses characters coming from the other > end. Basically we need to constantly check the receiving side while we > are transmitting. No, we do not. U-Boot is strictly single tasking. This is one of the fundamental design principles. We do not receive network packets while sending ourself, or when doing other things. We do not receive characters on the serial line while sending characters, or when doing other things. This is known behaviour, and you should just adapt to it. I don't know what exactly is your situation when you run into such problems - the recommended approach for example when remote-controlling U-Boot is to wait for the next input prompt before sending the next line of characters - and stop after the newline, until you receive the next prompt. >>> >>> I certainly don't want to break a fundamental design principle. >>> . >>> The situation this occurs is when you paste characters into a serial >>> terminal connected to U-Boot. This is a pretty common requirement. Yes >>> you can manually select each line and paste it but that is a pain. >>> With this patch it is possible to paste a series of 'setenv' commands, >>> for example. >>> >>> This caused an awful lot of confusion here because characters would be >>> silently dropped within the command line (normally about 16 chars from >>> the end). Then the system would not boot. It is really not a friendly >>> experience. >>> >>> Regards, >>> Simon >> >> How about using the time-honored flow-control mechanism? If the serial port >> supports hardware CTS and RTS signals, you can use them to sync with the >> sender. If not, you can send XOFF and XON to respectively ask the sender to >> pause and resume sending. Of course, the sender must support hardware or >> software flow control. > > It is common to have just a 2-wire UART for the console. Perhaps this > is unfortunate. > > Are you suggesting that U-Boot should send XOFF sometimes, or does it > support this already? (I haven't seen it). When would U-Boot send > XOFF? Perhaps it could send XOFF at the end of each line that it > receives. Then XON when it starts reading the next line or maybe when > it runs out of characters and wants more. > > Most terminal problems will support software flow control. But are you > sure this is better? > > Regards, > Simon Another thought...the problem is that every character we receive we echo, so we can of course never get ahead of the workload. But we can get behind, since while we are doing something other than receiving serial characters (e.g. processing a command, outputing a prompt, updating the LCD) we are not transmitting. Then when we next go to receive, we find (say) 10 characters built up, and start echoing them all. We are now 10 characters behind the receive and will stay that way until there is a let-up. Even worse if one of the commands outputs something, since then we might be 20 or 30 characters behind, and start dropping things. Even if no commands output anything, delays and the U-Boot prompt will create a problem. (As an aside, I note the call to the USB code to deal with transmission/reception so clearly there is some interrupt handling going on...) So another approach would be to buffer all output at the console layer and send it out in the fullness of time using a function similar to tstc() that tests whether or not there is space in the output fifo. This seems a more invasive but more general fix. I'm not that keen on it since it changes something that people might rely on - that printf() waits if the serial fifo is full. It might mean that prior to running bootm U-Boot would need to wait for the buffer to flush. Ick. But I am happy to look at this if you think it isn't even worse. Regards, Simon > >> >> Amicalement, >> -- >> Albert. >> > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > The situation this occurs is when you paste characters into a serial > terminal connected to U-Boot. This is a pretty common requirement. Yes > you can manually select each line and paste it but that is a pain. But that's exactly how it's supposed to work. Depending on hardware that may or may not be FIFOs available to buffer such asynchronous input, and I rather see limited, but consistent behaviour, instead of fancy stuff that works oin one board but not on another. > With this patch it is possible to paste a series of 'setenv' commands, > for example. If you want to run multi-line command sequences, then load and run a script. In the special case of setting environment variables, consider using featurs like "env import" instead. > This caused an awful lot of confusion here because characters would be > silently dropped within the command line (normally about 16 chars from > the end). Then the system would not boot. It is really not a friendly > experience. There should never be character loss when sending data on a line by line base. Even the slowest processors we support are fast enough to deal with that. Multi-line input has never been supported. 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: w...@denx.de Alan Turing thought about criteria to settle the question of whether machines can think, a question of which we now know that it is about as relevant as the question of whether submarines can swim. -- Edsger Dijkstra ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > > How about using the time-honored flow-control mechanism? If the serial port > > supports hardware CTS and RTS signals, you can use them to sync with the > > sender. If not, you can send XOFF and XON to respectively ask the sender to > > pause and resume sending. Of course, the sender must support hardware or > > software flow control. > > It is common to have just a 2-wire UART for the console. Perhaps this > is unfortunate. Indeed that's a pretty common configuration - but soft-flow would still work fine. > Are you suggesting that U-Boot should send XOFF sometimes, or does it > support this already? (I haven't seen it). When would U-Boot send > XOFF? Perhaps it could send XOFF at the end of each line that it > receives. Then XON when it starts reading the next line or maybe when > it runs out of characters and wants more. It should be sufficient to send XOFF when receiving a '\n' character (and storing that state) and XON when expecting inout (if state says XOFF had been sent before). > Most terminal problems will support software flow control. But are you > sure this is better? Definitely. for example, because it is hardware independent and will work on all architectures, all SoCs, all UARTs. 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: w...@denx.de "We don't care. We don't have to. We're the Phone Company." ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > Another thought...the problem is that every character we receive we > echo, so we can of course never get ahead of the workload. But we can Even worse, if your input contains any commands that prodce output, mayeb even lots of output (say "md $addr 1") then even your increased buffer size is not going to help you. > way until there is a let-up. Even worse if one of the commands outputs > something, since then we might be 20 or 30 characters behind, and > start dropping things. Even if no commands output anything, delays and > the U-Boot prompt will create a problem. Indeed. And this is why the simple rule applies: always wait for the next command prompt before sending the next line of input. 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: w...@denx.de To understand a program you must become both the machine and the program. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Sat, Oct 15, 2011 at 12:00 PM, Wolfgang Denk wrote: > Dear Simon Glass, > > In message > you > wrote: >> >> The situation this occurs is when you paste characters into a serial >> terminal connected to U-Boot. This is a pretty common requirement. Yes >> you can manually select each line and paste it but that is a pain. > > But that's exactly how it's supposed to work. Depending on hardware > that may or may not be FIFOs available to buffer such asynchronous > input, and I rather see limited, but consistent behaviour, instead of > fancy stuff that works oin one board but not on another. Well this is particular serial driver is in pretty widespread use. We could of course introduce a serial buffer scheme that extends across all drivers but I don't feel this is the right time, given that there are other priorities in serial. > >> With this patch it is possible to paste a series of 'setenv' commands, >> for example. > > If you want to run multi-line command sequences, then load and run a > script. If the setenv commands are configuration commands (such as network config, or setting up a new boot path) then they may not already exist in the machine. It is much easier to paste commands from a text editor / notepad than create a script, put it onto the tftp server, tftp the script and run it. > > In the special case of setting environment variables, consider using > featurs like "env import" instead. s/and run it/and "env import" it/ > >> This caused an awful lot of confusion here because characters would be >> silently dropped within the command line (normally about 16 chars from >> the end). Then the system would not boot. It is really not a friendly >> experience. > > There should never be character loss when sending data on a line by > line base. Even the slowest processors we support are fast enough to > deal with that. That's right. > > Multi-line input has never been supported. Hence this patch, I feel. Pasting a number of lines into the terminal is convenient, natural, useful, and when it fails it is unexpected and feels like a bug to many. I would certainly like to fix it. Regards, Simon > > 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: w...@denx.de > Alan Turing thought about criteria to settle the question of whether > machines can think, a question of which we now know that it is about > as relevant as the question of whether submarines can swim. > -- Edsger Dijkstra > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Sat, Oct 15, 2011 at 12:14 PM, Wolfgang Denk wrote: > Dear Simon Glass, > > In message > you > wrote: >> >> Another thought...the problem is that every character we receive we >> echo, so we can of course never get ahead of the workload. But we can > > Even worse, if your input contains any commands that prodce output, > mayeb even lots of output (say "md $addr 1") then even your > increased buffer size is not going to help you. It helps a little - provided there are no more than 250 characters more 'serial input' after the md command, all is well. But I think this example is a little contrived. > >> way until there is a let-up. Even worse if one of the commands outputs >> something, since then we might be 20 or 30 characters behind, and >> start dropping things. Even if no commands output anything, delays and >> the U-Boot prompt will create a problem. > > Indeed. And this is why the simple rule applies: always wait for the > next command prompt before sending the next line of input. I feel this is a painful restriction. I see no indication of this limitation anywhere on the command line, no error or warning, etc. It is common to want to paste in new bootargs, new network parameters, new environment variables. U-Boot has a command line which purports to support this, and with a fairly straightforward patch and in the majority of cases, it can. Yes this is a minor point, but it does bite people, and usability is important. Regards, Simon > > 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: w...@denx.de > To understand a program you must become both the machine and the > program. > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > > But that's exactly how it's supposed to work. =A0Depending on hardware > > that may or may not be FIFOs available to buffer such asynchronous > > input, and I rather see limited, but consistent behaviour, instead of > > fancy stuff that works oin one board but not on another. > > Well this is particular serial driver is in pretty widespread use. We Widespread, yes. But there is still a huge number of boards where it is not being used. > > If you want to run multi-line command sequences, then load and run a > > script. > > If the setenv commands are configuration commands (such as network > config, or setting up a new boot path) then they may not already exist > in the machine. It is much easier to paste commands from a text editor > / notepad than create a script, put it onto the tftp server, tftp the > script and run it. Well, there is two cases: - You are doing this just once or twice: then it does not make much sense to create scripts or use tools; but then it is not too much of an issue to copy & paste line by line either. - You are doing this more frequently. Then copy & paste is a cumbersome and error prone procedure. You have to be careful about quoting etc., and errors are not easy to detect. In such a situation it always makes sense to think about more efficient ways: 1) Use tools. For example, use something like "expect" or similar to send your inout line by line, always waiting for the next command prompt. 2) Use a U-Boot script image which you can download and run. 3) Use a plain text file which you can "env import". 4) ... > > In the special case of setting environment variables, consider using > > featurs like "env import" instead. > > s/and run it/and "env import" it/ No. Running a script image is one solution; using "env import" is another, independent one. > > Multi-line input has never been supported. > > Hence this patch, I feel. Pasting a number of lines into the terminal > is convenient, natural, useful, and when it fails it is unexpected and > feels like a bug to many. I would certainly like to fix it. But your patch does not _solve_ the problem. It just extends the limits a bit - to som degree that seems to be acceptable to you, but the next person might paste a few lines more, and it's failing again. I think it's much better to state clearly that multi-line input is not supported than to claim it is, without being able to give a precise limit where it will fail, or without being able to detect error situations clearly. 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: w...@denx.de Little known fact about Middle Earth: The Hobbits had a very sophi- sticated computer network! It was a Tolkien Ring... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > > Even worse, if your input contains any commands that prodce output, > > mayeb even lots of output (say "md $addr 1") then even your > > increased buffer size is not going to help you. > > It helps a little - provided there are no more than 250 characters > more 'serial input' after the md command, all is well. But I think > this example is a little contrived. 250 characters? That's 4, maybe 5 lines of text. Not exactly much... > I feel this is a painful restriction. I see no indication of this > limitation anywhere on the command line, no error or warning, etc. It > is common to want to paste in new bootargs, new network parameters, > new environment variables. U-Boot has a command line which purports to > support this, and with a fairly straightforward patch and in the > majority of cases, it can. Pasting commands is OK for occasional use. If you use this more frequently you're doing something wrong. > Yes this is a minor point, but it does bite people, and usability is > important. Agreed. But consistent behaviour has it's charme as well: it's the principle of least surprise (POLS) - see http://en.wikipedia.org/wiki/Principle_of_least_surprise As mentioned before, I'd rather have a clear restriction that is consistent on all systems, instead of some "works for me" implementation on a few systems that will break under just slightly modified usage modes. 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: w...@denx.de Uncertain fortune is thoroughly mastered by the equity of the calcu- lation. - Blaise Pascal ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Sun, Oct 16, 2011 at 12:47 PM, Wolfgang Denk wrote: > Dear Simon Glass, > > In message > you > wrote: >> >> > But that's exactly how it's supposed to work. =A0Depending on hardware >> > that may or may not be FIFOs available to buffer such asynchronous >> > input, and I rather see limited, but consistent behaviour, instead of >> > fancy stuff that works oin one board but not on another. >> >> Well this is particular serial driver is in pretty widespread use. We > > Widespread, yes. But there is still a huge number of boards where it > is not being used. Yes. > >> > If you want to run multi-line command sequences, then load and run a >> > script. >> >> If the setenv commands are configuration commands (such as network >> config, or setting up a new boot path) then they may not already exist >> in the machine. It is much easier to paste commands from a text editor >> / notepad than create a script, put it onto the tftp server, tftp the >> script and run it. > > Well, there is two cases: > > - You are doing this just once or twice: then it does not make much > sense to create scripts or use tools; but then it is not too much of > an issue to copy & paste line by line either. > > - You are doing this more frequently. Then copy & paste is a > cumbersome and error prone procedure. You have to be careful about > quoting etc., and errors are not easy to detect. In such a > situation it always makes sense to think about more efficient ways: > > 1) Use tools. For example, use something like "expect" or similar > to send your inout line by line, always waiting for the next > command prompt. > > 2) Use a U-Boot script image which you can download and run. > > 3) Use a plain text file which you can "env import". > > 4) ... I think this is what said before. In the case of expect, I must put the board into the right state, disconnect my terminal from the board, run the expect script and re-connect. In the case of 2 and 3, they are essentially the same. Both require a file transfer to the board (perhaps tftp) which involves editing and putting the file somewhere. This is not friendly behaviour. > >> > In the special case of setting environment variables, consider using >> > featurs like "env import" instead. >> >> s/and run it/and "env import" it/ > > No. Running a script image is one solution; using "env import" is > another, independent one. To repeat the bit of my message that you omitted: > If the setenv commands are configuration commands (such as network > config, or setting up a new boot path) then they may not already exist > in the machine. It is much easier to paste commands from a text editor > / notepad than create a script, put it onto the tftp server, tftp the > script and run it. My point was that they both require putting the file somewhere and using the network to get the file. > >> > Multi-line input has never been supported. >> >> Hence this patch, I feel. Pasting a number of lines into the terminal >> is convenient, natural, useful, and when it fails it is unexpected and >> feels like a bug to many. I would certainly like to fix it. > > But your patch does not _solve_ the problem. It just extends the > limits a bit - to som degree that seems to be acceptable to you, but > the next person might paste a few lines more, and it's failing again. Well I would like to have a message that a serial overflow happened - we don't have the means to check this at present but perhaps one day... With this patch, using commands like setenv that don't emit output, I can paste at least 50 lines of text. The problem over buffer overflow is not soluble in the limit. I could paste the works of Shakespeare into U-Boot but it would run out of buffer memory. There is always a buffer limit - at present it is about 16 characters which means within a few lines you run of space. This patch increases that to 256 characters. I have already granted that the patch should perhaps move up a level, to encompass serial, but the serial layer is not ready for that yet. > > I think it's much better to state clearly that multi-line input is not > supported than to claim it is, without being able to give a precise > limit where it will fail, or without being able to detect error > situations clearly. I would agree with this if there were an error message or warning that serial input overflow occurred. At the moment it is just confusing or buggy. Regards, SImon > > 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: w...@denx.de > Little known fact about Middle Earth: The Hobbits had a very sophi- > sticated computer network! It was a Tolkien Ring... > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Simon Glass, In message you wrote: > > I think this is what said before. In the case of expect, I must put > the board into the right state, disconnect my terminal from the board, > run the expect script and re-connect. You can run it under expect all the time when you are planning to perform such actions. > In the case of 2 and 3, they are essentially the same. Both require a > file transfer to the board (perhaps tftp) which involves editing and > putting the file somewhere. > > This is not friendly behaviour. Come on. It's even more friendly than what you are doing now. With "env import" you can just use the output from "printenv" on a working, tested board, copy and paste this into a file, and then download nd import that (through network or serial download - serial download is not slower than pasting it over the serial line - just way more reliable). For copy and paste, you have to manuallay insert "setenv " in front of all lines, and add appropriate quoting. And the BIG benfit is: it just works. > > If the setenv commands are configuration commands (such as network > > config, or setting up a new boot path) then they may not already exist > > in the machine. It is much easier to paste commands from a text editor > > / notepad than create a script, put it onto the tftp server, tftp the > > script and run it. > > My point was that they both require putting the file somewhere and > using the network to get the file. Put the file somehwere, yes. Some file anywhere on your system. Not a big problem, or is it? Using the network: this is one option; it's the most convenient one if network is already configured. Otherwise just use serial download. > I would agree with this if there were an error message or warning that > serial input overflow occurred. At the moment it is just confusing or > buggy. But your patch is NOT fixing that. It just pushes the limits a bit. 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: w...@denx.de It is clear that the individual who persecutes a man, his brother, because he is not of the same opinion, is a monster. - Voltaire ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Sun, Oct 16, 2011 at 12:52 PM, Wolfgang Denk wrote: > Dear Simon Glass, > > In message > you > wrote: >> >> > Even worse, if your input contains any commands that prodce output, >> > mayeb even lots of output (say "md $addr 1") then even your >> > increased buffer size is not going to help you. >> >> It helps a little - provided there are no more than 250 characters >> more 'serial input' after the md command, all is well. But I think >> this example is a little contrived. > > 250 characters? That's 4, maybe 5 lines of text. Not exactly much... It is 250 characters *behind* the serial input. If the only output on each line is the U-Boot prompt, then this is perhaps 50 lines of input, which is plenty for most people. > >> I feel this is a painful restriction. I see no indication of this >> limitation anywhere on the command line, no error or warning, etc. It >> is common to want to paste in new bootargs, new network parameters, >> new environment variables. U-Boot has a command line which purports to >> support this, and with a fairly straightforward patch and in the >> majority of cases, it can. > > Pasting commands is OK for occasional use. If you use this more > frequently you're doing something wrong. I don't understand this comment - it fails every time. > >> Yes this is a minor point, but it does bite people, and usability is >> important. > > Agreed. But consistent behaviour has it's charme as well: it's the > principle of least surprise (POLS) - see > http://en.wikipedia.org/wiki/Principle_of_least_surprise That seems like an argument for this patch. When pasting lines of text into U-Boot now, the bahaviour is not consistent. Sometimes it drops a character or two and the system won't boot. Sometimes it fails to complete and it is clear that something is wrong. > > As mentioned before, I'd rather have a clear restriction that is > consistent on all systems, instead of some "works for me" > implementation on a few systems that will break under just slightly > modified usage modes. Yes, so in summary, we have: 1. The XON/XOFF option, which I will look at. This requires everyone affected to switch over to XON/XOFF in whatever terminal/ser2net program they use. 2. Putting a buffer features at the serial port level (one level up from this patch) to augment the 16-byte FIFO. This would be a pain to do until someone tidies up serial. We might find a way of emitting a warning when input overflow occurs. I could look at this but not in the immediate future. Of course even this option cannot provide infinite buffer space, that would need to be clearly understood. 3. This patch, which is a point solution for NS16550 only. It is up to you what you want to select. The patch is up if you want it, and I should be able to look at XON/XOFF this week. My preference: option 3 now, option 1 later for those who can use XON/XOFF, option 2 much much later. Regards, Simon > > 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: w...@denx.de > Uncertain fortune is thoroughly mastered by the equity of the calcu- > lation. - Blaise Pascal > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Monday, October 24, 2011, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message you wrote: >> >> > > 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. 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message you wrote: > > > 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. > 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 -- 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: w...@denx.de History tends to exaggerate. -- Col. Green, "The Savage Curtain", stardate 5906.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Monday, October 24, 2011, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message < calbutcjh8bvzfvh14d83wr2jov89o9jvjo9vzzb7r_zgkzz...@mail.gmail.com> you wrote: >> >> > 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. > >> 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. 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi, On Sun, Oct 23, 2011 at 4:30 PM, Graeme Russ wrote: > Hi Wolfgang, > > On Monday, October 24, 2011, Wolfgang Denk wrote: >> Dear Graeme Russ, >> >> In message < > calbutcjh8bvzfvh14d83wr2jov89o9jvjo9vzzb7r_zgkzz...@mail.gmail.com> you > wrote: >>> >>> > 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. >> >>> 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. > 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) 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 > > Regards, > > Graeme > > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message you wrote: > > >> 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. > 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. > 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) This sounds like unneeded overhead to me. 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: w...@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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On 25/10/11 05:46, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message > you > wrote: >> 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? getc() sends an XOFF > 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. > >> 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. > >> 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) > > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message <4ea5bbef.1050...@gmail.com> you wrote: > > > 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. > 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 -- 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: w...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On 25/10/11 07:00, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message <4ea5bbef.1050...@gmail.com> you wrote: >> >>> 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. Oops, yes you are right >> 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). 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. > 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. > 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). > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message <4ea5cd39.2070...@gmail.com> you wrote: > > > 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. 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 -- 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: w...@denx.de "Never face facts; if you do, you'll never get up in the morning." - Marlo Thomas ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Tue, Oct 25, 2011 at 8:59 AM, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message <4ea5cd39.2070...@gmail.com> you wrote: >> >> > 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. > > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
On 10/24/2011 3:22 PM, Graeme Russ wrote: > Hi Wolfgang, > > On Tue, Oct 25, 2011 at 8:59 AM, Wolfgang Denk wrote: >> Dear Graeme Russ, >> >> In message<4ea5cd39.2070...@gmail.com> you wrote: 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. >> 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 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 > > 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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message you wrote: > > 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 -- 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: w...@denx.de The optimum committee has no members. - Norman Augustine ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On 25/10/11 18:31, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message > you > wrote: >> >> 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. 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 > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message <4ea67491.5090...@gmail.com> 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. > 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. > 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. > 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 -- 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: w...@denx.de "Have you lived in this village all your life?""No, not yet." ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang, On Wed, Oct 26, 2011 at 5:41 AM, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message <4ea67491.5090...@gmail.com> 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. > >> 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. > >> 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. > >> 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? > I will not accept such a design nor such an implementation. Then we live with dropped characters Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi, On Tue, Oct 25, 2011 at 3:37 PM, Graeme Russ wrote: > Hi Wolfgang, > > On Wed, Oct 26, 2011 at 5:41 AM, Wolfgang Denk wrote: >> Dear Graeme Russ, >> >> In message <4ea67491.5090...@gmail.com> 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. >> >>> 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. >> >>> 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. >> >>> 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? > >> 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 seem
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Simon, On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass wrote: [big snip] > 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 > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Graeme, On Tue, Oct 25, 2011 at 4:37 PM, Graeme Russ wrote: > Hi Simon, > > On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass wrote: > > [big snip] > >> 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. > >> 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 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 > > Regards, > > Graeme > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Simon, On Wed, Oct 26, 2011 at 10:48 AM, Simon Glass wrote: > Hi Graeme, > > On Tue, Oct 25, 2011 at 4:37 PM, Graeme Russ wrote: >> Hi Simon, >> >> On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass wrote: >> >> [big snip] >> >>> 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. > >> >>> 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 > > 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 :) > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme, In message you wrote: > > 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"). > - 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. > 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. > 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 ??? What makes you think so? It's frowned upon, because the disadvantages are way bigger than the advantages, but that's all. > 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. > Option 3 is all we are left with... I disagree. > 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 -- 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: w...@denx.de Cogito cogito ergo cogito sum - "I think that I think, therefore I think that I am." - Ambrose Bierce, "The Devil's Dictionary" ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme, In message you wrote: > > 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. > ... 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() Driver API... > So maybe a serial API which 'users' must 'wake up' and 'sleep'... Arghhh... > - 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. > 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 -- 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: w...@denx.de Marriage is the sole cause of divorce. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Hi Wolfgang,
On 26/10/11 18:00, Wolfgang Denk wrote:
> Dear Graeme,
>
> In message
> you
> wrote:
>>
>> 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.
>
>> ... 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()
>
> Driver API...
>
>> So maybe a serial API which 'users' must 'wake up' and 'sleep'...
>
> Arghhh...
>
>> - 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.
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 */
charname[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()
>> 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
__
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Graeme Russ, In message <4ea7d071.9010...@gmail.com> 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->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. And I say No. No application whatever should have to care about sarting or stopping any drivers that provide stdio functionality. > Hmmm, stdio_open() and stdio_close() NAK. Sorry, we are wasting time here. Can we please put this to an end now? 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: w...@denx.de Above all else -- sky. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
On 10/25/2011 06:37 PM, Graeme Russ wrote: > Hi Simon, > > On Wed, Oct 26, 2011 at 10:17 AM, Simon Glass wrote: > > [big snip] > >> 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Scott Wood, In message <4ea83b88.8040...@freescale.com> 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 -- 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: w...@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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
On 10/26/2011 01:17 PM, Wolfgang Denk wrote: > - 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? > 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. :-) > 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. > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] NS16550: buffer reads
Dear Scott Wood, In message <4ea8566a.1050...@freescale.com> you wrote: > > > - 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. > > 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. > 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 -- 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: w...@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 (?)) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
