All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Marshall <mark.marshall60@ntlworld.com>
To: kvm@vger.kernel.org
Cc: qemu-devel@nongnu.org
Subject: Re: more about serial ports: do they even work?
Date: Tue, 03 Feb 2009 08:41:14 +0000	[thread overview]
Message-ID: <4988032A.5010905@ntlworld.com> (raw)
In-Reply-To: <4987FCBA.7020104@msgid.tls.msk.ru>

Michael Tokarev wrote:
> David S. Ahern wrote:
>> Michael Tokarev wrote:
>>> David S. Ahern wrote:
>>>> I don't recall seeing a followup to this post.
>>>>
>>>> To put Michael's second suggestion into patch form, the following fixes
>>>> the problem for me:
>>>>
>>>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>>>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>>>> @@ -1078,20 +1078,21 @@
>>>>              if (sarg | TIOCM_DTR)
>>>>                  *targ |= CHR_TIOCM_DTR;
>>>>              if (sarg | TIOCM_RTS)
>>>>                  *targ |= CHR_TIOCM_RTS;
>>>>          }
>>>>          break;
>>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>>          {
>>>>              int sarg = *(int *)arg;
>>>>              int targ = 0;
>>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>>> here, one more operation is necessary:
>>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
>> Interesting. that change was not needed to fix my problem.
> 
> It just means you (or, rather, your guests) never really needed to
> DROP those signal lines, only to raise them.
> 
>>>>              if (sarg | CHR_TIOCM_DTR)
>>>>                  targ |= TIOCM_DTR;
>>>>              if (sarg | CHR_TIOCM_RTS)
>>>>                  targ |= TIOCM_RTS;
Is this code really correct.  If it is is there a comment somewhere 
describing why it's correct?  I would have expected the lines above to be:

               if (sarg & CHR_TIOCM_DTR)
                   targ |= TIOCM_DTR;
               if (sarg & CHR_TIOCM_RTS)
                   targ |= TIOCM_RTS;

(Just an observation as this went past).

MM

> 
> Without that line above, the code never drops the two bits, once
> set they can't be "removed" anymore.
> 
> By the way, this is upstream qemu issue, not kvm one, and has to be
> pushed as such.  Good you CC'd qemu list.
> 
> /mjt
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


WARNING: multiple messages have this Message-ID (diff)
From: Mark Marshall <mark.marshall60@ntlworld.com>
To: Michael Tokarev <mjt@tls.msk.ru>
Cc: KVM list <kvm@vger.kernel.org>,
	"David S. Ahern" <daahern@cisco.com>,
	qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: more about serial ports: do they even work?
Date: Tue, 03 Feb 2009 08:41:14 +0000	[thread overview]
Message-ID: <4988032A.5010905@ntlworld.com> (raw)
In-Reply-To: <4987FCBA.7020104@msgid.tls.msk.ru>

Michael Tokarev wrote:
> David S. Ahern wrote:
>> Michael Tokarev wrote:
>>> David S. Ahern wrote:
>>>> I don't recall seeing a followup to this post.
>>>>
>>>> To put Michael's second suggestion into patch form, the following fixes
>>>> the problem for me:
>>>>
>>>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>>>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>>>> @@ -1078,20 +1078,21 @@
>>>>              if (sarg | TIOCM_DTR)
>>>>                  *targ |= CHR_TIOCM_DTR;
>>>>              if (sarg | TIOCM_RTS)
>>>>                  *targ |= CHR_TIOCM_RTS;
>>>>          }
>>>>          break;
>>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>>          {
>>>>              int sarg = *(int *)arg;
>>>>              int targ = 0;
>>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>>> here, one more operation is necessary:
>>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
>> Interesting. that change was not needed to fix my problem.
> 
> It just means you (or, rather, your guests) never really needed to
> DROP those signal lines, only to raise them.
> 
>>>>              if (sarg | CHR_TIOCM_DTR)
>>>>                  targ |= TIOCM_DTR;
>>>>              if (sarg | CHR_TIOCM_RTS)
>>>>                  targ |= TIOCM_RTS;
Is this code really correct.  If it is is there a comment somewhere 
describing why it's correct?  I would have expected the lines above to be:

               if (sarg & CHR_TIOCM_DTR)
                   targ |= TIOCM_DTR;
               if (sarg & CHR_TIOCM_RTS)
                   targ |= TIOCM_RTS;

(Just an observation as this went past).

MM

> 
> Without that line above, the code never drops the two bits, once
> set they can't be "removed" anymore.
> 
> By the way, this is upstream qemu issue, not kvm one, and has to be
> pushed as such.  Good you CC'd qemu list.
> 
> /mjt
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  reply	other threads:[~2009-02-03  8:55 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-26 20:20 more about serial ports: do they even work? Michael Tokarev
2009-01-26 20:39 ` Michael Tokarev
2009-02-02 21:27   ` David S. Ahern
2009-02-02 21:27     ` [Qemu-devel] " David S. Ahern
2009-02-02 21:32     ` Michael Tokarev
2009-02-02 21:32       ` [Qemu-devel] " Michael Tokarev
2009-02-02 21:36       ` David S. Ahern
2009-02-02 21:36         ` [Qemu-devel] " David S. Ahern
2009-02-03  8:13         ` Michael Tokarev
2009-02-03  8:13           ` [Qemu-devel] " Michael Tokarev
2009-02-03  8:41           ` Mark Marshall [this message]
2009-02-03  8:41             ` Mark Marshall
2009-02-04  8:09             ` Marcelo Tosatti
2009-02-04  8:09               ` [Qemu-devel] " Marcelo Tosatti
2009-02-04  8:37               ` Michael Tokarev
2009-02-04  8:37                 ` [Qemu-devel] " Michael Tokarev
2009-02-04  8:52                 ` Marcelo Tosatti
2009-02-04  8:52                   ` [Qemu-devel] " Marcelo Tosatti
2009-02-04  8:56                   ` Michael Tokarev
2009-02-04  8:56                     ` [Qemu-devel] " Michael Tokarev
2009-02-04 11:09                 ` Stefano Stabellini
2009-02-04 11:09                   ` [Qemu-devel] " Stefano Stabellini
2009-02-05 18:36                   ` David S. Ahern
2009-02-05 18:36                     ` [Qemu-devel] " David S. Ahern
2009-02-05 20:50                     ` Stefano Stabellini
2009-02-05 20:50                       ` [Qemu-devel] " Stefano Stabellini
2009-02-04 11:17           ` Stefano Stabellini
2009-02-04 11:17             ` Stefano Stabellini
2009-02-05 18:33 ` [PATCH, finally]: " Michael Tokarev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4988032A.5010905@ntlworld.com \
    --to=mark.marshall60@ntlworld.com \
    --cc=kvm@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.