more about serial ports: do they even work?

Commit Message

David S. Ahern Feb. 5, 2009, 6:36 p.m. UTC

Stefano Stabellini wrote:
> Michael Tokarev wrote:
> 
>> Other than that, an.. excellent idea, I wanted to propose
>> just that when I first saw all this stuff, but was somewhat
>> afraid.  And I *think* there's at least *some* sense.  Qemu
>> is a CPU emulator and may work on another arch where those
>> bits are defined differently.  Maybe that was the reason for
>> all this converting - to be safe than sorry, so to say.  No?
>>
> 
> Yes, this is exactly the reason why they were introduced in the first place.
> Let's suppose that the guest defines those constants differently: we
> need to parse them and covert them appropriately to the host format.
> CHR_IOCTL_SERIAL_SET_TIOCM and CHR_TIOCM_DTR correspond to the guest
> version of TIOCMSET and TIOCM_DTR and can be defined differently
> depending on the particular guest arch.

The following works for me.  It fixes the existing checks in place for
the GET and replicates that for the SET. The ioctl initialization is
needed in the SET is needed.




david
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Stefano Stabellini Feb. 5, 2009, 8:50 p.m. UTC | #1

David S. Ahern wrote:

> 
> Stefano Stabellini wrote:
>> Michael Tokarev wrote:
>>
>>> Other than that, an.. excellent idea, I wanted to propose
>>> just that when I first saw all this stuff, but was somewhat
>>> afraid.  And I *think* there's at least *some* sense.  Qemu
>>> is a CPU emulator and may work on another arch where those
>>> bits are defined differently.  Maybe that was the reason for
>>> all this converting - to be safe than sorry, so to say.  No?
>>>
>> Yes, this is exactly the reason why they were introduced in the first place.
>> Let's suppose that the guest defines those constants differently: we
>> need to parse them and covert them appropriately to the host format.
>> CHR_IOCTL_SERIAL_SET_TIOCM and CHR_TIOCM_DTR correspond to the guest
>> version of TIOCMSET and TIOCM_DTR and can be defined differently
>> depending on the particular guest arch.
> 
> The following works for me.  It fixes the existing checks in place for
> the GET and replicates that for the SET. The ioctl initialization is
> needed in the SET is needed.

It looks pretty good to me.

Acked-by: [e-mail]

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: trunk/qemu-char.c
===================================================================
--- trunk/qemu-char.c   (revision 6519)
+++ trunk/qemu-char.c   (working copy)
@@ -1067,17 +1067,17 @@ 
             int *targ = (int *)arg;
             ioctl(s->fd_in, TIOCMGET, &sarg);
             *targ = 0;
-            if (sarg | TIOCM_CTS)
+            if (sarg & TIOCM_CTS)
                 *targ |= CHR_TIOCM_CTS;
-            if (sarg | TIOCM_CAR)
+            if (sarg & TIOCM_CAR)
                 *targ |= CHR_TIOCM_CAR;
-            if (sarg | TIOCM_DSR)
+            if (sarg & TIOCM_DSR)
                 *targ |= CHR_TIOCM_DSR;
-            if (sarg | TIOCM_RI)
+            if (sarg & TIOCM_RI)
                 *targ |= CHR_TIOCM_RI;
-            if (sarg | TIOCM_DTR)
+            if (sarg & TIOCM_DTR)
                 *targ |= CHR_TIOCM_DTR;
-            if (sarg | TIOCM_RTS)
+            if (sarg & TIOCM_RTS)
                 *targ |= CHR_TIOCM_RTS;
         }
         break;
@@ -1085,9 +1085,20 @@ 
         {
             int sarg = *(int *)arg;
             int targ = 0;
-            if (sarg | CHR_TIOCM_DTR)
+            ioctl(s->fd_in, TIOCMGET, &targ);
+            targ &= ~(CHR_TIOCM_CTS | CHR_TIOCM_CAR | CHR_TIOCM_DSR
+                     | CHR_TIOCM_RI | CHR_TIOCM_DTR | CHR_TIOCM_RTS);
+            if (sarg & CHR_TIOCM_CTS)
+                targ |= TIOCM_CTS;
+            if (sarg & CHR_TIOCM_CAR)
+                targ |= TIOCM_CAR;
+            if (sarg & CHR_TIOCM_DSR)
+                targ |= TIOCM_DSR;
+            if (sarg & CHR_TIOCM_RI)
+                targ |= TIOCM_RI;
+            if (sarg & CHR_TIOCM_DTR)
                 targ |= TIOCM_DTR;
-            if (sarg | CHR_TIOCM_RTS)
+            if (sarg & CHR_TIOCM_RTS)
                 targ |= TIOCM_RTS;
             ioctl(s->fd_in, TIOCMSET, &targ);
         }