DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version
         :content-type:content-transfer-encoding:content-disposition
         :references:x-google-sender-auth;
        b=eF7KbWg2n7I7KiQ8GUcmSyMnJE7ldM7pWzZX/R2DgXUQ7PWQMgj8WtHGwfasv8mETW
         WEexn5jtHqP93dmLjwS82VCedHYkeDUWBHXZrj+EZo2QkItC0D3boICdhBEG6M6dwxFF
         57ERsiZvlbDyZKS3pswgsdP6UZdRC0KI9nxH0=
Message-ID: <3efb10970811121104o7497d354n6730a23ce5756093@mail.gmail.com>
Date: Wed, 12 Nov 2008 20:04:24 +0100
From: "Remy Bohmer" <linux@bohmer.net>
To: "Atsushi Nemoto" <anemo@mba.ocn.ne.jp>
Subject: Re: atmel_serial might lose modem status change
Cc: hskinnemoen@atmel.com, marc.pignat@hevs.ch, linux-kernel@vger.kernel.org
In-Reply-To: <20081113.021601.30439144.anemo@mba.ocn.ne.jp>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <20081113.021601.30439144.anemo@mba.ocn.ne.jp>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2179
Lines: 64

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/
>
--
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/