problem with TIOCSSERIAL setting on linux #12

Closed
mklight opened this Issue Oct 5, 2012 · 11 comments

Projects

None yet

4 participants

@mklight
mklight commented Oct 5, 2012

I am using a open bench logic sniffer that uses the cdc_acm driver in linux.
I was able to run fine with the 3.3 kernel, but switching to 3.5 I see this problem:
java.lang.IllegalStateException: JTermios call returned -1 at class jtermios.JTermios$JTermiosLogging line 489
at purejavacomm.PureJavaSerialPort.checkReturnCode(PureJavaSerialPort.java:906)
at purejavacomm.PureJavaSerialPort.setSerialPortParams(PureJavaSerialPort.java:478)
at purejavacomm.PureJavaSerialPort.(PureJavaSerialPort.java:722)
at purejavacomm.CommPortIdentifier.open(CommPortIdentifier.java:138)
...
I was able to trace it down to this failing:
ioctl(fd, TIOCSSERIAL, ss)) in src/jtermios/linux/JTermiosImpl.java
Changing the code to ignore the error was an okay workaround for me.

diff --git a/src/jtermios/linux/JTermiosImpl.java b/src/jtermios/linux/JTermiosImpl.java
index 9d6479f..085c284 100755
--- a/src/jtermios/linux/JTermiosImpl.java
+++ b/src/jtermios/linux/JTermiosImpl.java
@@ -498,7 +498,7 @@ public class JTermiosImpl implements jtermios.JTermios.JTermiosInterface {
                // Do the logging here as this does not go through the JTermios which normally does the logging
                log = log && log(5, "> ioctl(%d,%d,%s)\n", fd, cmd, data);
                int ret = m_Clib.ioctl(fd, cmd, data);
-               log = log && log(3, "< tcsetattr(%d,%d,%s) => %d\n", fd, cmd, data, ret);
+               log = log && log(3, "< ioctl(%d,%d,%s) => %d\n", fd, cmd, data, ret);
                return ret;
        }

@@ -544,8 +544,7 @@ public class JTermiosImpl implements jtermios.JTermios.JTermiosInterface {
                                // not every driver supports TIOCGSERIAL, so if it fails, just ignore it
                                if ((r = ioctl(fd, TIOCGSERIAL, ss)) == 0) {
                                        ss.flags &= ~ASYNC_SPD_MASK;
-                                       if ((r = ioctl(fd, TIOCSSERIAL, ss)) != 0)
-                                               return r;
+                                       r = ioctl(fd, TIOCSSERIAL, ss); // ignore this if it fails as well
                                }

                                // now set the speed with the constant from the table
@nyholku
Owner
nyholku commented Oct 6, 2012

On 5.10.2012 22.36, mklight wrote:

I am using a open bench logic sniffer that uses the cdc_acm driver in linux.
I was able to run fine with the 3.2 kernel, but switching to 3.4 I see this problem:
java.lang.IllegalStateException: JTermios call returned -1 at class jtermios.JTermios$JTermiosLogging line 489
at purejavacomm.PureJavaSerialPort.checkReturnCode(PureJavaSerialPort.java:906)
at purejavacomm.PureJavaSerialPort.setSerialPortParams(PureJavaSerialPort.java:478)
at purejavacomm.PureJavaSerialPort.(PureJavaSerialPort.java:722)
at purejavacomm.CommPortIdentifier.open(CommPortIdentifier.java:138)
...
I was able to trace it down to this failing:
ioctl(fd, TIOCSSERIAL, ss)) in src/jtermios/linux/JTermiosImpl.java
Changing the code to ignore the error was an okay workaround for me.

I noticed that someone had forked PureJavaComm and ignored most of the
return codes for ioctl() calls. That's a work around but of course
we should find out why it 'fails' and try to figure out how to make
it not to fail. I'm not happy to just ignore error codes, that is
bad practice, though maybe necessary at some instances.

br Kusti

@mklight
mklight commented Oct 8, 2012

The problem with ioctl(fd, TIOCSSERIAL, ss)) with the cdc_acm driver is that according to this thread the part or all of the ASYNC_SPD_MASK setting does not make sense for this device.
http://permalink.gmane.org/gmane.linux.usb.general/59285

So here is an alternate patch checking for invalid arguments.

diff --git a/src/jtermios/linux/JTermiosImpl.java b/src/jtermios/linux/JTermiosImpl.java
index 9d6479f..5300c02 100755
--- a/src/jtermios/linux/JTermiosImpl.java
+++ b/src/jtermios/linux/JTermiosImpl.java
@@ -498,7 +498,10 @@ public class JTermiosImpl implements jtermios.JTermios.JTermiosInterface {
                // Do the logging here as this does not go through the JTermios which normally does the logging
                log = log && log(5, "> ioctl(%d,%d,%s)\n", fd, cmd, data);
                int ret = m_Clib.ioctl(fd, cmd, data);
-               log = log && log(3, "< tcsetattr(%d,%d,%s) => %d\n", fd, cmd, data, ret);
+               if (ret == 0)
+                       log = log && log(3, "< ioctl(%d,%d,%s) => %d\n", fd, cmd, data, ret);
+               else
+                       log = log && log(3, "< ioctl(%d,%d,%s) => %d, errno => %d\n", fd, cmd, data, ret, errno());
                return ret;
        }

@@ -541,11 +544,17 @@ public class JTermiosImpl implements jtermios.JTermios.JTermiosInterface {
                                // in case custom divisor was in use, turn it off first
                                serial_struct ss = new serial_struct();

-                               // not every driver supports TIOCGSERIAL, so if it fails, just ignore it
-                               if ((r = ioctl(fd, TIOCGSERIAL, ss)) == 0) {
+                               // not every driver supports TIOCGSERIAL, so if it fails as invalid, just ignore it
+                               r = ioctl(fd, TIOCGSERIAL, ss);
+                               if (r == 0) {
                                        ss.flags &= ~ASYNC_SPD_MASK;
-                                       if ((r = ioctl(fd, TIOCSSERIAL, ss)) != 0)
+                                       r = ioctl(fd, TIOCSSERIAL, ss); // ignore this if it fails as invalid as well
+                                       if ((r != 0) && (errno() != EINVAL)) {
                                                return r;
+                                       }
+                               }
+                               else if ((r != 0) && (errno() != EINVAL)) {
+                                       return r;
                                }

                                // now set the speed with the constant from the table

Here is log output with this patch running on Linux3.3 showing the failed ioctl(TIOCGSERIAL)

log: < ioctl(56,21534,JTermiosImpl$Linux_C_lib$serial_struct(allocated@0x7f64602c6230 (72 bytes)) {
  int type@0=0
  int line@4=0
  int port@8=0
  int irq@c=0
  int flags@10=0
  int xmit_fifo_size@14=0
  int custom_divisor@18=0
  int baud_base@1c=0
  short close_delay@20=0
  short io_type@22=0
  int hub6@24=0
  short closing_wait@28=0
  short closing_wait2@2a=0
  Pointer iomem_base@30=null
  short iomem_reg_shift@38=0
  int port_high@3c=0
  NativeLong iomap_base@40=0
}) => -1, errno => 22

Here is log output with this patch running on Linux3.5 showing the failed ioctl(TIOCSSERIAL)

log: < ioctl(54,21535,JTermiosImpl$Linux_C_lib$serial_struct(allocated@0x7fc8a44d7b50 (72 bytes)) {
  int type@0=0
  int line@4=0
  int port@8=0
  int irq@c=0
  int flags@10=2000
  int xmit_fifo_size@14=500
  int custom_divisor@18=0
  int baud_base@1c=2580
  short close_delay@20=0
  short io_type@22=0
  int hub6@24=0
  short closing_wait@28=0
  short closing_wait2@2a=0
  Pointer iomem_base@30=null
  short iomem_reg_shift@38=0
  int port_high@3c=0
  NativeLong iomap_base@40=0
}) => -1, errno => 22
@nyholku
Owner
nyholku commented Oct 8, 2012

On 8.10.2012 17.21, mklight wrote:

The problem with ioctl(fd, TIOCSSERIAL, ss)) with the cdc_acm driver is that according to this thread the part or all of the ASYNC_SPD_MASK setting does not make sense for this device.
http://permalink.gmane.org/gmane.linux.usb.general/59285

Hmm I read through that by failed to spot where it said
specifically about ASYNC_SPD_MASK but I guess that is
implied, however since the code you patched (below)
is about UNsetting that mask I would have thought
that this should always 'make sense' to the device?

I maybe not following properly what is going on in here.

Apart from non standard baudrates I think the USB CDC ACM
spec (nearly) mandates support for setting baud rates and
control line states so it looks like a bigotism of the worst
kind if Linux kernel starts to fail for these ioctls for
serial ports ... after all they are serial ports and
the behavior of those has been pretty much standard
from umpteen years ... why break it if it ain't in the
need of fix.

Not saying that a we could not work around it like that,
just wondering out aloud.

br Kusti

@@ -541,11 +544,17 @@ public class JTermiosImpl implements jtermios.JTermios.JTermiosInterface {
// in case custom divisor was in use, turn it off first
serial_struct ss = new serial_struct();

  •                           // not every driver supports TIOCGSERIAL, so if it fails, just ignore it
    
  •                           if ((r = ioctl(fd, TIOCGSERIAL, ss)) == 0) {
    
  •                           // not every driver supports TIOCGSERIAL, so if it fails as invalid, just ignore it
    
  •                           r = ioctl(fd, TIOCGSERIAL, ss);
    
  •                           if (r == 0) {
                                     ss.flags&= ~ASYNC_SPD_MASK;
    
  •                                   if ((r = ioctl(fd, TIOCSSERIAL, ss)) != 0)
    
  •                                   r = ioctl(fd, TIOCSSERIAL, ss); // ignore this if it fails as invalid as well
    
  •                                   if ((r != 0)&&  (errno() != EINVAL)) {
                                             return r;
    
  •                                   }
    
  •                           }
    
  •                           else if ((r != 0)&&  (errno() != EINVAL)) {
    
  •                                   return r;
                             }
    
                             // now set the speed with the constant from the table
    

Here is log output with this patch running on Linux3.3 showing the failed ioctl(TIOCGSERIAL)

log:< ioctl(56,21534,JTermiosImpl$Linux_C_lib$serial_struct(allocated@0x7f64602c6230 (72 bytes)) {
int type@0=0
int line@4=0
int port@8=0
int irq@c=0
int flags@10=0
int xmit_fifo_size@14=0
int custom_divisor@18=0
int baud_base@1c=0
short close_delay@20=0
short io_type@22=0
int hub6@24=0
short closing_wait@28=0
short closing_wait2@2a=0
Pointer iomem_base@30=null
short iomem_reg_shift@38=0
int port_high@3c=0
NativeLong iomap_base@40=0
}) => -1, errno => 22


Here is log output with this patch running on Linux3.5 showing the failed ioctl(TIOCSSERIAL)

log:< ioctl(54,21535,JTermiosImpl$Linux_C_lib$serial_struct(allocated@0x7fc8a44d7b50 (72 bytes)) {
int type@0=0
int line@4=0
int port@8=0
int irq@c=0
int flags@10=2000
int xmit_fifo_size@14=500
int custom_divisor@18=0
int baud_base@1c=2580
short close_delay@20=0
short io_type@22=0
int hub6@24=0
short closing_wait@28=0
short closing_wait2@2a=0
Pointer iomem_base@30=null
short iomem_reg_shift@38=0
int port_high@3c=0
NativeLong iomap_base@40=0
}) => -1, errno => 22



---
Reply to this email directly or view it on GitHub:
https://github.com/nyholku/purejavacomm/issues/12#issuecomment-9227261
@jawi
jawi commented Oct 15, 2012

Digging through the kernel sources of Linux, the support for TIOCGSERIAL only exists since kernel 3.3 for the cdc_acm driver. As it is not apparent that support for TIOCSSERIAL will arrive shortly, and to support all current kernels, it might be the best idea to simply ignore the whole TIOCSSERIAL error codes in case it returns EINVAL.

I've patched my own fork of PureJavacomm and did some tests with it: it works ok for recent Linux kernels now...

@nyholku
Owner
nyholku commented Oct 16, 2012

On 15.10.2012 18.36, Jan Willem Janssen wrote:

Digging through the kernel sources of Linux, the support for TIOCGSERIAL only exists since kernel 3.3 for the cdc_acm driver. As it is not apparent that support for TIOCSSERIAL will arrive shortly, and to support all current kernels, it might be the best idea to simply ignore the whole TIOCSSERIAL error codes in case it returns EINVAL.

I've patched my own fork of PureJavacomm and did some tests with it: it works ok for recent Linux kernels now...


Reply to this email directly or view it on GitHub:
#12 (comment)

I guess you are right, though it this pretty annoying having
to ignore error codes because of what appears to be a
regression on the Linux kernel ... oh well that seems to
be the nature of progress in the Free world.

I'll try to get around to implementing this is the near future.

br Kusti

@nyholku
Owner
nyholku commented Oct 28, 2012

I just pushed version 0.0.9 that hopefully fixes this issue. Please try and report back.

@jawi
jawi commented Oct 28, 2012

Almost; I had to add a missing c_line entry to the getFieldOrder() method in order to get things running. Aside that, everything runs just fine under Linux...

See jawi@f64bd32#diff-0 (you can ignore the wrong date of the commit, that's a mistake made by me).

@nyholku
Owner
nyholku commented Oct 28, 2012

Thanks for testing, I added that and bumped the version number to 0.0.10. Hopefully this works for most people now.

@jawi
jawi commented Oct 28, 2012

Thanks for the quick response. I'll give it a closer look soon; also test in some various "real-life" applications...

@kbabioch

ols is now working for me (using Linux 3.6.3), so it seems to do the trick. Thanks for this quick response.

@nyholku
Owner
nyholku commented Oct 29, 2012

Ok, thanks for contributing, I'll close this issue then.

@nyholku nyholku closed this Oct 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment