[U-Boot] [PATCH 1/2] NS16550: buffer reads

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[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 mw.l lines can
be pasted.  With this patch, it handles around 70 lines before lossage,
long enough for most things you'd paste.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 README                   |    8 ++++
 drivers/serial/ns16550.c |   96 +++++++++++++++++++++++++++++++++++++++++++--
 drivers/serial/serial.c  |    4 +-
 include/ns16550.h        |    4 +-
 4 files changed, 103 insertions(+), 9 deletions(-)

diff --git a/README b/README
index bd03523..d21ea9c 100644
--- a/README
+++ b/README
@@ -2682,6 +2682,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 8eeb48f..ed3428d 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 <common.h>
 #include <config.h>
 #include <ns16550.h>
 #include <watchdog.h>
 #include <linux/types.h>
 #include <asm/io.h>
 
+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 */
@@ -86,21 +89,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(&regs->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(&regs->rbr);
+}
+
+static int NS16550_raw_tstc(NS16550_t regs)
+{
+	return ((serial_in(&regs->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(&regs->lsr) & UART_LSR_DR) != 0)
+		enqueue(port, serial_in(&regs->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 /* CONFIG_NS16550_BUFFER_READS */
 #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index 4032dfd..3fc80b1 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -219,13 +219,13 @@ _serial_puts (const char *s,const int port)
 int
 _serial_getc(const int port)
 {
-	return NS16550_getc(PORT);
+	return NS16550_getc(PORT, port);
 }
 
 int
 _serial_tstc(const int port)
 {
-	return NS16550_tstc(PORT);
+	return NS16550_tstc(PORT, port);
 }
 
 void
diff --git a/include/ns16550.h b/include/ns16550.h
index 9ea81e9..fa3e62e 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -160,6 +160,6 @@ typedef volatile struct NS16550 *NS16550_t;
 
 void	NS16550_init   (NS16550_t com_port, int baud_divisor);
 void	NS16550_putc   (NS16550_t com_port, char c);
-char	NS16550_getc   (NS16550_t com_port);
-int	NS16550_tstc   (NS16550_t com_port);
+char	NS16550_getc   (NS16550_t regs, unsigned int port);
+int	NS16550_tstc   (NS16550_t regs, unsigned int port);
 void	NS16550_reinit (NS16550_t com_port, int baud_divisor);
-- 
1.7.1

[U-Boot] [PATCH 2/2] corenet_ds: enable buffered reads

[Scott Wood]

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 include/configs/corenet_ds.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/configs/corenet_ds.h b/include/configs/corenet_ds.h
index 7bafa05..47edabd 100644
--- a/include/configs/corenet_ds.h
+++ b/include/configs/corenet_ds.h
@@ -236,6 +236,8 @@
 #define CONFIG_SYS_NS16550_REG_SIZE	1
 #define CONFIG_SYS_NS16550_CLK		(get_bus_freq(0)/2)
 
+#define CONFIG_NS16550_BUFFER_READS
+
 #define CONFIG_SYS_BAUDRATE_TABLE	\
 	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200}
 
-- 
1.7.1

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Wolfgang Denk]

Dear Scott Wood,

In message <20110406203012.GA30167@schlenkerla.am.freescale.net> you wrote:
> This improves the performance of U-Boot when accepting rapid input,
> such as pasting a sequence of commands.
...
> +static struct ns16550_priv rxstate[NUM_PORTS];

Do we really need to statically allocate these buffers for all
configured serial ports?  Actual I/O will always be done to a single
port only, so eventually only one such buffer will ever be used?

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

Is it really wise to silentrly drop characters here?  Maybe we should
stop reading from the device, and/or issue some error message?



What is the increase of the memory footprint (flash and RAM) with
this patch, with CONFIG_NS16550_BUFFER_READS enabled and not enabled?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It usually takes more than three weeks to prepare  a  good  impromptu
speech.                                                  - Mark Twain

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Scott Wood]

On Wed, 6 Apr 2011 22:42:12 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Scott Wood,
> 
> In message <20110406203012.GA30167@schlenkerla.am.freescale.net> you wrote:
> > This improves the performance of U-Boot when accepting rapid input,
> > such as pasting a sequence of commands.
> ...
> > +static struct ns16550_priv rxstate[NUM_PORTS];
> 
> Do we really need to statically allocate these buffers for all
> configured serial ports?  Actual I/O will always be done to a single
> port only, so eventually only one such buffer will ever be used?

If we use just one buffer, then switching ports could have some latency, in
terms of some queued data after the switch command being used from the old
port -- unless we add code to flush the queue when switching.  It probably
won't matter much in practice, though, unless you do something like:

"setenv stdout=...; something else" all at once.

And it wouldn't matter at all for boards without CONFIG_SERIAL_MULTI --
except for linkstation, which does some access to a secondary serial port
(see board/linkstation/avr.c).

> > +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;
> 
> Is it really wise to silentrly drop characters here?

It's what currently happens, except they're silently dropped by the
hardware instead (and that's still possible with this patch, it just
requires a longer time between getc/tstc calls).

It's also what happens with usbtty (in lib/circbuf.c), though it drops old
characters rather than new (both options seem about equally damaging).

In case you're wondering, I didn't use lib/circbuf.c here as it would have
been larger and more complicated (requiring dynamic port initialization).

> Maybe we should stop reading from the device, 

That would hang the U-Boot console, for what is usually a temporary
problem.

> and/or issue some error message?

Would probably exacerbate the problem by slowing things down further,
would need to be ratelimited to avoid scrolling all useful stuff off the
screen, and it would behave inconsistently (only happenning if the dropping
doesn't happen in hardware).

> What is the increase of the memory footprint (flash and RAM) with
> this patch, with CONFIG_NS16550_BUFFER_READS enabled and not enabled?

Currently, looks like up to 16 bytes with it not enabled -- due to not
ifdeffing the change to pass an extra argument to the public NS16550
functions.  This would go away if we just use a single queue.  If we don't
do that, I think we should ifdef the function signature change as well --
not so much for the size savings, as that it appears that some boards use
or define these functions themselves (alternatively, I could try to fix up
those boards).

With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
bytes of BSS.

-Scott

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Simon Glass]

On Wed, Apr 6, 2011 at 2:28 PM, Scott Wood <scottwood@freescale.com> wrote:

> On Wed, 6 Apr 2011 22:42:12 +0200
> Wolfgang Denk <wd@denx.de> wrote:
>
> > Dear Scott Wood,
> >
> > In message <20110406203012.GA30167@schlenkerla.am.freescale.net> you
> wrote:
> > > This improves the performance of U-Boot when accepting rapid input,
> > > such as pasting a sequence of commands.
> > ...
> > > +static struct ns16550_priv rxstate[NUM_PORTS];
> >
> > Do we really need to statically allocate these buffers for all
> > configured serial ports?  Actual I/O will always be done to a single
> > port only, so eventually only one such buffer will ever be used?
>
> If we use just one buffer, then switching ports could have some latency, in
> terms of some queued data after the switch command being used from the old
> port -- unless we add code to flush the queue when switching.  It probably
> won't matter much in practice, though, unless you do something like:
>
> "setenv stdout=...; something else" all at once.
>
> And it wouldn't matter at all for boards without CONFIG_SERIAL_MULTI --
> except for linkstation, which does some access to a secondary serial port
> (see board/linkstation/avr.c).
>
> > > +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;
> >
> > Is it really wise to silentrly drop characters here?
>
> It's what currently happens, except they're silently dropped by the
> hardware instead (and that's still possible with this patch, it just
> requires a longer time between getc/tstc calls).
>
> It's also what happens with usbtty (in lib/circbuf.c), though it drops old
> characters rather than new (both options seem about equally damaging).
>
> In case you're wondering, I didn't use lib/circbuf.c here as it would have
> been larger and more complicated (requiring dynamic port initialization).
>
> > Maybe we should stop reading from the device,
>
> That would hang the U-Boot console, for what is usually a temporary
> problem.
>
> > and/or issue some error message?
>
> Would probably exacerbate the problem by slowing things down further,
> would need to be ratelimited to avoid scrolling all useful stuff off the
> screen, and it would behave inconsistently (only happenning if the dropping
> doesn't happen in hardware).
>
> > What is the increase of the memory footprint (flash and RAM) with
> > this patch, with CONFIG_NS16550_BUFFER_READS enabled and not enabled?
>
> Currently, looks like up to 16 bytes with it not enabled -- due to not
> ifdeffing the change to pass an extra argument to the public NS16550
> functions.  This would go away if we just use a single queue.  If we don't
> do that, I think we should ifdef the function signature change as well --
> not so much for the size savings, as that it appears that some boards use
> or define these functions themselves (alternatively, I could try to fix up
> those boards).
>
> With it enabled, on ppc it's an extra 396 bytes of image size, and 1056
> bytes of BSS.
>
> -Scott
>

Hi Scott,

This is a very useful patch and it works well. I have taken the liberty of
modifying it slightly, because I think you should subtract 1 from the port
number that you pass to NS16550. For some reason the 'COM' ports are
numbered from 1 instead of 0. Please see below, and sorry if the patch
doesn't come through cleanly.

Regards,
Simon


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 mw.l lines can
be pasted.  With this patch, it handles around 70 lines before lossage,
long enough for most things you'd paste.

Signed-off-by: Scott Wood <scottwood@freescale.com>
Tested-by: Simon Glass <sjg@chromium.org>
---
 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 ba01c52..5f6044f 100644
--- a/README
+++ b/README
@@ -2665,6 +2665,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 8eeb48f..ed3428d 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 <common.h>
 #include <config.h>
 #include <ns16550.h>
 #include <watchdog.h>
 #include <linux/types.h>
 #include <asm/io.h>

+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 */
@@ -86,21 +89,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(&regs->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(&regs->rbr);
+}
+
+static int NS16550_raw_tstc(NS16550_t regs)
+{
+ return ((serial_in(&regs->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(&regs->lsr) & UART_LSR_DR) != 0)
+ enqueue(port, serial_in(&regs->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 /* CONFIG_NS16550_BUFFER_READS */
 #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index 4032dfd..49b2591 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -92,6 +92,7 @@ static NS16550_t serial_ports[4] = {
 };

 #define PORT serial_ports[port-1]
+#define PORTNR (port-1)
 #if defined(CONFIG_CONS_INDEX)
 #define CONSOLE (serial_ports[CONFIG_CONS_INDEX-1])
 #endif
@@ -219,13 +220,13 @@ _serial_puts (const char *s,const int port)
 int
 _serial_getc(const int port)
 {
- return NS16550_getc(PORT);
+ return NS16550_getc(PORT, PORTNR);
 }

 int
 _serial_tstc(const int port)
 {
- return NS16550_tstc(PORT);
+ return NS16550_tstc(PORT, PORTNR);
 }

 void
diff --git a/include/ns16550.h b/include/ns16550.h
index 9ea81e9..fa3e62e 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -160,6 +160,6 @@ typedef volatile struct NS16550 *NS16550_t;

 void NS16550_init   (NS16550_t com_port, int baud_divisor);
 void NS16550_putc   (NS16550_t com_port, char c);
-char NS16550_getc   (NS16550_t com_port);
-int NS16550_tstc   (NS16550_t com_port);
+char NS16550_getc   (NS16550_t regs, unsigned int port);
+int NS16550_tstc   (NS16550_t regs, unsigned int port);
 void NS16550_reinit (NS16550_t com_port, int baud_divisor);
--
1.7.3.1



>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Scott Wood]

On Wed, 4 May 2011 16:30:00 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi Scott,
> 
> This is a very useful patch and it works well. I have taken the liberty of
> modifying it slightly, because I think you should subtract 1 from the port
> number that you pass to NS16550. For some reason the 'COM' ports are
> numbered from 1 instead of 0.

Thanks for spotting!

>Please see below, and sorry if the patch doesn't come through cleanly.

The whitespace got mangled.

There's another thing that needs to be fixed -- the signatures
of the 16550 functions should not change when the feature is disabled (see
the discussion on http://patchwork.ozlabs.org/patch/90066/), or it'll
break boards like linkstation.

I'll include both fixes in my respin.

-Scott

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Simon Glass]

Hi Scott,

On Fri, May 6, 2011 at 1:28 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Wed, 4 May 2011 16:30:00 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi Scott,
>>
>> This is a very useful patch and it works well. I have taken the liberty of
>> modifying it slightly, because I think you should subtract 1 from the port
>> number that you pass to NS16550. For some reason the 'COM' ports are
>> numbered from 1 instead of 0.
>
> Thanks for spotting!
>
>>Please see below, and sorry if the patch doesn't come through cleanly.
>
> The whitespace got mangled.
>
> There's another thing that needs to be fixed -- the signatures
> of the 16550 functions should not change when the feature is disabled (see
> the discussion on http://patchwork.ozlabs.org/patch/90066/), or it'll
> break boards like linkstation.
>
> I'll include both fixes in my respin.

Are you planning a respin of this? If not I will take this up as it is
pretty important for Tegra.

Regards,
Simon


>
> -Scott
>
>

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Kumar Gala]

On Oct 12, 2011, at 4:23 PM, Simon Glass wrote:

> Hi Scott,
> 
> On Fri, May 6, 2011 at 1:28 PM, Scott Wood <scottwood@freescale.com> wrote:
>> On Wed, 4 May 2011 16:30:00 -0700
>> Simon Glass <sjg@chromium.org> wrote:
>> 
>>> Hi Scott,
>>> 
>>> This is a very useful patch and it works well. I have taken the liberty of
>>> modifying it slightly, because I think you should subtract 1 from the port
>>> number that you pass to NS16550. For some reason the 'COM' ports are
>>> numbered from 1 instead of 0.
>> 
>> Thanks for spotting!
>> 
>>> Please see below, and sorry if the patch doesn't come through cleanly.
>> 
>> The whitespace got mangled.
>> 
>> There's another thing that needs to be fixed -- the signatures
>> of the 16550 functions should not change when the feature is disabled (see
>> the discussion on http://patchwork.ozlabs.org/patch/90066/), or it'll
>> break boards like linkstation.
>> 
>> I'll include both fixes in my respin.
> 
> Are you planning a respin of this? If not I will take this up as it is
> pretty important for Tegra.

Scott's on vacation for next 2 weeks so guessing he'd be fine w/you respining.

- k

[U-Boot] [PATCH 1/2] NS16550: buffer reads

[Simon Glass]

Hi Kumar,

On Wed, Oct 12, 2011 at 6:08 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Oct 12, 2011, at 4:23 PM, Simon Glass wrote:
>
>> Hi Scott,
>>
>> On Fri, May 6, 2011 at 1:28 PM, Scott Wood <scottwood@freescale.com> wrote:
>>> On Wed, 4 May 2011 16:30:00 -0700
>>> Simon Glass <sjg@chromium.org> wrote:
>>>
>>>> Hi Scott,
>>>>
>>>> This is a very useful patch and it works well. I have taken the liberty of
>>>> modifying it slightly, because I think you should subtract 1 from the port
>>>> number that you pass to NS16550. For some reason the 'COM' ports are
>>>> numbered from 1 instead of 0.
>>>
>>> Thanks for spotting!
>>>
>>>> Please see below, and sorry if the patch doesn't come through cleanly.
>>>
>>> The whitespace got mangled.
>>>
>>> There's another thing that needs to be fixed -- the signatures
>>> of the 16550 functions should not change when the feature is disabled (see
>>> the discussion on http://patchwork.ozlabs.org/patch/90066/), or it'll
>>> break boards like linkstation.
>>>
>>> I'll include both fixes in my respin.
>>
>> Are you planning a respin of this? If not I will take this up as it is
>> pretty important for Tegra.
>
> Scott's on vacation for next 2 weeks so guessing he'd be fine w/you respining.

OK I have sent it to the list, just fixing two checkpatch warnings.

It is pretty-much essential on Tegra since otherwise pasting stuff
into the terminal hardly works at all.

Regards,
Simon

>
> - k