mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Remy Bohmer" <linux@bohmer.net>
To: "Atsushi Nemoto" <anemo@mba.ocn.ne.jp>
Cc: hskinnemoen@atmel.com, marc.pignat@hevs.ch, linux-kernel@vger.kernel.org
Subject: Re: atmel_serial might lose modem status change
Date: Wed, 12 Nov 2008 20:04:24 +0100	[thread overview]
Message-ID: <3efb10970811121104o7497d354n6730a23ce5756093@mail.gmail.com> (raw)
In-Reply-To: <20081113.021601.30439144.anemo@mba.ocn.ne.jp>

Hello Atsushi,

2008/11/12 Atsushi Nemoto <anemo@mba.ocn.ne.jp>:
> I found a problem of handling of modem status of atmel_serial driver.
>
> With the commit 1ecc26 ("atmel_serial: split the interrupt handler"),
> handling of modem status signal was splitted into two parts.  The
> atmel_tasklet_func() compares new status with irq_status_prev, but
> irq_status_prev is not correct if signal status was changed while the
> port is closed.
>
> Here is a sequence to cause problem:
>
> 1. Remote side sets CTS (and DSR).
> 2. Local side close the port.
> 3. Local side clears RTS and DTR.
> 4. Remote side clears CTS and DSR.
> 5. Local side reopen the port.  hw_stopped becomes 1.
> 6. Local side sets RTS and DTR.
> 7. Remote side sets CTS and DSR.
>
> Then CTS change interrupt can be received, but since CTS bit in
> irq_status_prev and new status is same, uart_handle_cts_change() will
> not be called (so hw_stopped will not be cleared, i.e. cannot send any
> data).
>
> I suppose irq_status_prev should be initialized at somewhere in open
> sequence.  Is this fix correct?

I cannot shoot a hole in it... So, to me it appears okay.

Kind Regards,

Remy


> diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
> index 4c535f7..9f3b697 100644
> --- a/drivers/serial/atmel_serial.c
> +++ b/drivers/serial/atmel_serial.c
> @@ -877,6 +877,9 @@ static int atmel_startup(struct uart_port *port)
>                }
>        }
>
> +       /* Save current CSR for comparison in atmel_tasklet_func() */
> +       atmel_port->irq_status_prev = UART_GET_CSR(port);
> +
>        /*
>         * Finally, enable the serial port
>         */
>
> ---
> Atsushi Nemoto
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

  reply	other threads:[~2008-11-12 19:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-12 17:16 Atsushi Nemoto
2008-11-12 19:04 ` Remy Bohmer [this message]
2009-01-05 15:17   ` Haavard Skinnemoen

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=3efb10970811121104o7497d354n6730a23ce5756093@mail.gmail.com \
    --to=linux@bohmer.net \
    --cc=anemo@mba.ocn.ne.jp \
    --cc=hskinnemoen@atmel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.pignat@hevs.ch \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®