PATCH: TIOCMBIS/TIOCMBIC not working when using usbser.sys

PATCH: TIOCMBIS/TIOCMBIC not working when using usbser.sys

[Carlo B.]

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

Hello,
into winsup/cygwin/fhandler/serial.cc, the function
fhandler_serial::switch_modem_lines() is called when TIOCMBIS/TIOCMBIC
are used into an ioctl() call.
This function uses EscapeCommFunction() for setting and resetting RTS
and DTR signals of a serial port.
Unfortunately, this function does not work on USB CDC devices.
This is not a true bug of a CYGWIN but an issue of the usbser.sys
driver from Microsoft, from Windows 2000 to the latest Windows 11.
Both 32bit and 64bit versions of the operating system are affected.
Actually, I tested EscapeCommFunction() also when using a real UART,
based on the traditional 16550 driver and it works fine.
Using thirdy party CDC drivers, like the one provided by FTDI for
their USB bridge chips, probably also works.
However, it is also possible to drive the RTS/DTR signals by writing
their state with SetCommState(), which proved to be working fine all
types of connection.

Here, I attached a patch that I made for fixing this issue.
Anyways, in my opinion this is also a better solution for handling
these signals since RTS/DTR can be set at the same time rather than
having two separate calls with a visible delay between them.

I was wondering if this fix could be imported into the core of CYGWIN
and if it could be possible to include it also into the v3.3.x branch,
since I have seen that 32bit support has been removed from Master
branch recently.

Thank you very much for your time.

Sincerely,

Carlo Bramini.

[-- Attachment #2: winsup_cygwin_fhandler_serial.cc.patch --]
[-- Type: application/octet-stream, Size: 1743 bytes --]

diff --git a/winsup/cygwin/fhandler/serial.cc b/winsup/cygwin/fhandler/serial.cc
index 174a57a43..58a8df677 100644
--- a/winsup/cygwin/fhandler/serial.cc
+++ b/winsup/cygwin/fhandler/serial.cc
@@ -391,50 +391,39 @@ fhandler_serial::tcflow (int action)
 int
 fhandler_serial::switch_modem_lines (int set, int clr)
 {
-  int res = 0;
-
-  if (set & TIOCM_RTS)
-    {
-      if (EscapeCommFunction (get_handle (), SETRTS))
-	rts = TIOCM_RTS;
-      else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  else if (clr & TIOCM_RTS)
-    {
-      if (EscapeCommFunction (get_handle (), CLRRTS))
-	rts = 0;
-      else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  if (set & TIOCM_DTR)
+  if ((set & (TIOCM_RTS | TIOCM_DTR)) || (clr & (TIOCM_RTS | TIOCM_DTR)))
     {
-      if (EscapeCommFunction (get_handle (), SETDTR))
-	rts = TIOCM_DTR;
+      DCB dcb;
+
+      memset(&dcb,0,sizeof(dcb));
+      dcb.DCBlength = sizeof(dcb);
+
+      if (!GetCommState(get_handle(), &dcb))
+        {
+          __seterrno ();
+          return -1;
+        }
+
+      if (set & TIOCM_RTS)
+        dcb.fRtsControl = RTS_CONTROL_ENABLE;
       else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  else if (clr & TIOCM_DTR)
-    {
-      if (EscapeCommFunction (get_handle (), CLRDTR))
-	rts = 0;
+      if (clr & TIOCM_RTS)
+        dcb.fRtsControl = RTS_CONTROL_DISABLE;
+
+      if (set & TIOCM_DTR)
+        dcb.fRtsControl = DTR_CONTROL_ENABLE;
       else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
+      if (clr & TIOCM_DTR)
+        dcb.fRtsControl = DTR_CONTROL_DISABLE;
+
+      if (!SetCommState(get_handle(), &dcb))
+        {
+          __seterrno ();
+          return -1;
+        }
     }
 
-  return res;
+  return 0;
 }
 
 /* ioctl: */

Re: PATCH: TIOCMBIS/TIOCMBIC not working when using usbser.sys

[Corinna Vinschen]

Hi Carlo,

On Nov 10 10:07, Carlo B. wrote:
> Hello,
> into winsup/cygwin/fhandler/serial.cc, the function
> fhandler_serial::switch_modem_lines() is called when TIOCMBIS/TIOCMBIC
> are used into an ioctl() call.
> This function uses EscapeCommFunction() for setting and resetting RTS
> and DTR signals of a serial port.
> Unfortunately, this function does not work on USB CDC devices.
> This is not a true bug of a CYGWIN but an issue of the usbser.sys
> driver from Microsoft, from Windows 2000 to the latest Windows 11.
> Both 32bit and 64bit versions of the operating system are affected.
> Actually, I tested EscapeCommFunction() also when using a real UART,
> based on the traditional 16550 driver and it works fine.
> Using thirdy party CDC drivers, like the one provided by FTDI for
> their USB bridge chips, probably also works.
> However, it is also possible to drive the RTS/DTR signals by writing
> their state with SetCommState(), which proved to be working fine all
> types of connection.
> 
> Here, I attached a patch that I made for fixing this issue.

Thanks for your patch, but would you be so kind to create a complete git
patch, created by committing your patch locally on top of git master,
with a fine commit message outlining the technical details as described
above?  Then create a patch file using `git format-patch -1' and send it
here or to the cygwin-patches mailing list, please?

> I was wondering if this fix could be imported into the core of CYGWIN
> and if it could be possible to include it also into the v3.3.x branch,
> since I have seen that 32bit support has been removed from Master
> branch recently.

There will be no more 32 bit release. 3.3.6 is the last one.


Thanks,
Corinna

Re: PATCH: TIOCMBIS/TIOCMBIC not working when using usbser.sys

[Corinna Vinschen]

Hi Carlo,

On Nov 11 11:30, Carlo B. wrote:
> I'm sending here the patch as you requested.
> I hope that you will find it useful.

Thanks for the patch, it looks good to me.  I just reworked the 
commit message slightly (formatting, mainly) and pushed it
to the master branch.


Thanks,
Corinna

PATCH: TIOCMBIS/TIOCMBIC not working when using usbser.sys

[Carlo B.]

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

I'm sending here the patch as you requested.
I hope that you will find it useful.

Sincerely,

Carlo Bramini.

[-- Attachment #2: 0001-PATCH-Cygwin-fix-TIOCMBIS-TIOCMBIC-not-working-when-.patch --]
[-- Type: application/octet-stream, Size: 3254 bytes --]

From 753b491fd70d337b6f21de577e833a85b19a8e7c Mon Sep 17 00:00:00 2001
From: Carlo Bramini <30959007+carlo-bramini@users.noreply.github.com>
Date: Fri, 11 Nov 2022 11:15:27 +0100
Subject: [PATCH] [PATCH] Cygwin: fix TIOCMBIS/TIOCMBIC not working when using
 usbser.sys

Into winsup/cygwin/fhandler/serial.cc, the function fhandler_serial::switch_modem_lines() is called when TIOCMBIS/TIOCMBIC are used into an ioctl() call.
This function uses EscapeCommFunction() for setting and resetting RTS and DTR signals of a serial port.
Unfortunately, this function does not work on USB CDC devices.
This is not a true bug of a CYGWIN but an issue of the usbser.sys driver from Microsoft, from Windows 2000 to the latest Windows 11.
Both 32bit and 64bit versions of the operating system are affected.
Actually, I tested EscapeCommFunction() also when using a real UART, based on the traditional 16550 driver and it works fine.
Using thirdy party CDC drivers, like the one provided by FTDI for their USB bridge chips, probably also works.
However, it is also possible to drive the RTS/DTR signals by writing their state with SetCommState(), which proved to be working fine all types of connection.
This is also a better solution for handling these signals since RTS/DTR can be set at the same time rather than having two separate calls with a visible delay between them.
---
 winsup/cygwin/fhandler/serial.cc | 67 +++++++++++++-------------------
 1 file changed, 28 insertions(+), 39 deletions(-)

diff --git a/winsup/cygwin/fhandler/serial.cc b/winsup/cygwin/fhandler/serial.cc
index 174a57a43..58a8df677 100644
--- a/winsup/cygwin/fhandler/serial.cc
+++ b/winsup/cygwin/fhandler/serial.cc
@@ -391,50 +391,39 @@ fhandler_serial::tcflow (int action)
 int
 fhandler_serial::switch_modem_lines (int set, int clr)
 {
-  int res = 0;
-
-  if (set & TIOCM_RTS)
-    {
-      if (EscapeCommFunction (get_handle (), SETRTS))
-	rts = TIOCM_RTS;
-      else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  else if (clr & TIOCM_RTS)
-    {
-      if (EscapeCommFunction (get_handle (), CLRRTS))
-	rts = 0;
-      else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  if (set & TIOCM_DTR)
+  if ((set & (TIOCM_RTS | TIOCM_DTR)) || (clr & (TIOCM_RTS | TIOCM_DTR)))
     {
-      if (EscapeCommFunction (get_handle (), SETDTR))
-	rts = TIOCM_DTR;
+      DCB dcb;
+
+      memset(&dcb,0,sizeof(dcb));
+      dcb.DCBlength = sizeof(dcb);
+
+      if (!GetCommState(get_handle(), &dcb))
+        {
+          __seterrno ();
+          return -1;
+        }
+
+      if (set & TIOCM_RTS)
+        dcb.fRtsControl = RTS_CONTROL_ENABLE;
       else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
-    }
-  else if (clr & TIOCM_DTR)
-    {
-      if (EscapeCommFunction (get_handle (), CLRDTR))
-	rts = 0;
+      if (clr & TIOCM_RTS)
+        dcb.fRtsControl = RTS_CONTROL_DISABLE;
+
+      if (set & TIOCM_DTR)
+        dcb.fRtsControl = DTR_CONTROL_ENABLE;
       else
-	{
-	  __seterrno ();
-	  res = -1;
-	}
+      if (clr & TIOCM_DTR)
+        dcb.fRtsControl = DTR_CONTROL_DISABLE;
+
+      if (!SetCommState(get_handle(), &dcb))
+        {
+          __seterrno ();
+          return -1;
+        }
     }
 
-  return res;
+  return 0;
 }
 
 /* ioctl: */
-- 
2.38.1.windows.1