RE: Incomplete baud rate mapping in termiostty.c

RE: Incomplete baud rate mapping in termiostty.c

[Jay Foster]

Should the setting of priv->init = true be dependent on the return value of
real_termios_init()?

	res = real_termios_init();
	if (ENOERR == res)
		priv->init = true;

Or maybe it doesn't matter.
Jay

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Wednesday, January 30, 2008 10:41 AM
To: Rainer Arndt
Cc: ecos-patches@ecos.sourceware.org
Subject: Re: Incomplete baud rate mapping in termiostty.c


Hi Folks

Here is Rainers patch plus my patch for the init flag.

     Andrew

Re: Incomplete baud rate mapping in termiostty.c

[Jonathan Larmour]

Jay Foster wrote:
> Should the setting of priv->init = true be dependent on the return value of
> real_termios_init()?
> 
> 	res = real_termios_init();
> 	if (ENOERR == res)
> 		priv->init = true;
> 
> Or maybe it doesn't matter.

Probably better to ignore the return value so the device can be retried on
subsequent calls - the only relevant failure mode is in retrieving the
config and maybe it was a transient error.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

Incomplete baud rate mapping in termiostty.c

[Rainer Arndt]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]


The ecosbaud2posixbaud table in termiostty.c is incomplete. The baud
rate entry B1800 is missing.
I have attached a fix.

real_termios_init() is called from termios_lookup() only if the init
flag was not set before. 
I could't found where the flag is set. I think it is missing in
real_termios_init?

Because of the flag isn't set, real_termios_init() is called every time
the device is opened and requests the current device baudrate from the
ecos device.
But the value returned from map_ecosbaud_to_posixbaud() could be wrong
because of the missing B1800 entry.

One consequence is that setting a new baud rate could fail after reopen
a device (e.g. current 19200 -> new 38400), because in set_attr () the
new baud rate is only set if it is different from the current one.
nice bug ;-)

Rainer

[-- Attachment #2: termiostty.c.patch --]
[-- Type: text/plain, Size: 690 bytes --]

Index: termiostty.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/serial/current/src/common/termiostty.c,v
retrieving revision 1.9
diff -u -r1.9 termiostty.c
--- termiostty.c	3 Sep 2006 17:32:31 -0000	1.9
+++ termiostty.c	30 Jan 2008 15:25:15 -0000
@@ -190,7 +190,7 @@
 
 // map eCos bitrates to POSIX bitrates.
 static speed_t ecosbaud2posixbaud[] = {
-    0, B50, B75, B110, B134, B150, B200, B300, B600, B1200, B2400, B3600,
+    0, B50, B75, B110, B134, B150, B200, B300, B600, B1200, B1800, B2400, B3600,
     B4800, B7200, B9600, B14400, B19200, B38400, B57600, B115200, B230400 };
 
 // map POSIX bitrates to eCos bitrates.

Re: Incomplete baud rate mapping in termiostty.c

[Andrew Lunn]

On Wed, Jan 30, 2008 at 05:59:02PM +0100, Rainer Arndt wrote:
> 
> The ecosbaud2posixbaud table in termiostty.c is incomplete. The baud
> rate entry B1800 is missing.
> I have attached a fix.

Thanks. Nice catch. I will apply it now.

> real_termios_init() is called from termios_lookup() only if the init
> flag was not set before. 
> I could't found where the flag is set. I think it is missing in
> real_termios_init?

I would set it termios_lookup, inside the second if ( !priv->init ) {
code. I will fix this. Please can you test it.

      Thanks
            Andrew

Re: Incomplete baud rate mapping in termiostty.c

[Andrew Lunn]

[-- Attachment #1: Type: text/plain, Size: 78 bytes --]

Hi Folks

Here is Rainers patch plus my patch for the init flag.

     Andrew

[-- Attachment #2: termiostty.c.patch --]
[-- Type: text/x-diff, Size: 1813 bytes --]

Index: io/serial/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/serial/current/ChangeLog,v
retrieving revision 1.71
diff -u -r1.71 ChangeLog
--- io/serial/current/ChangeLog	3 Sep 2006 17:32:31 -0000	1.71
+++ io/serial/current/ChangeLog	30 Jan 2008 18:38:41 -0000
@@ -1,3 +1,14 @@
+2008-01-30  Andrew Lunn  <andrew.lunn@ascom.ch>
+
+	* src/common/termiostty.c (termios_lookup): Add missing set of
+	init flag. Pointed out by Rainer Arndt.
+
+	
+2008-01-30  Rainer Arndt <Arndt-ADT@kieback-peter.de>
+
+	* src/common/termiostty.c (ecosbaud2posixbaud[]): Add missing
+	B1800 entry.
+
 2006-08-31  Ilija Koco  <ilijak@siva.com.mk>
 	    Andrew Lunn <andrew.lunn@ascom.ch>
 	
Index: io/serial/current/src/common/termiostty.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/serial/current/src/common/termiostty.c,v
retrieving revision 1.9
diff -u -r1.9 termiostty.c
--- io/serial/current/src/common/termiostty.c	3 Sep 2006 17:32:31 -0000	1.9
+++ io/serial/current/src/common/termiostty.c	30 Jan 2008 18:38:42 -0000
@@ -190,7 +190,7 @@
 
 // map eCos bitrates to POSIX bitrates.
 static speed_t ecosbaud2posixbaud[] = {
-    0, B50, B75, B110, B134, B150, B200, B300, B600, B1200, B2400, B3600,
+    0, B50, B75, B110, B134, B150, B200, B300, B600, B1200, B1800, B2400, B3600,
     B4800, B7200, B9600, B14400, B19200, B38400, B57600, B115200, B230400 };
 
 // map POSIX bitrates to eCos bitrates.
@@ -580,6 +580,7 @@
     if ( !priv->init ) {
         cyg_drv_mutex_lock( &priv->lock );
         if ( !priv->init ) {  // retest as we may have been pre-empted
+            priv->init = true;
             priv->dev_handle = chan;
             err = real_termios_init( priv );
         }