All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tim Kryger <tim.kryger@linaro.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@linaro.org
Subject: Re: [PATCH] serial: 8250_dw: Improve unwritable LCR workaround
Date: Thu, 26 Sep 2013 15:46:55 -0700	[thread overview]
Message-ID: <20130926224655.GA15106@kroah.com> (raw)
In-Reply-To: <1380069549-9176-1-git-send-email-tim.kryger@linaro.org>

On Tue, Sep 24, 2013 at 05:39:09PM -0700, Tim Kryger wrote:
> The Designware UART has a limitation where it ignores writes into the
> LCR if the UART is busy.  The current workaround stashes a copy of the
> last written LCR and writes it back down to the hardware if it receives
> a special busy interrupt which is raised when a write was ignored.
> 
> Unfortunately, interrupts are typically disabled prior to performing a
> sequence of register writes that include the LCR so the point at which
> the retry occurs is too late.  An example is serial8250_do_set_termios()
> where an ignored LCR write results in the baud divisor not being set and
> instead a garbage character is sent out the transmitter.
> 
> Furthermore, since serial_port_out() offers no way to indicate failure,
> a serious effort must be made to ensure that the LCR is actually updated
> before returning back to the caller.  This is difficult, however, as a
> UART that was busy during the first attempt is likely to still be busy
> when a subsequent attempt is made unless some extra action is taken.
> 
> This updated workaround takes the extreme action of clearing the TX/RX
> FIFOs and reading the receive buffer before writing down the LCR in the
> hope that doing so will force the UART into an idle state.  While this
> may seem unnecessarily aggressive, writes to the LCR are used to change
> the baud rate, parity, stop bit, or data length so the data that may be
> lost is likely not important.  Admittedly, this is far from ideal but it
> seems to be the best that can be done given the hardware limitations.
> 
> Lastly, the revised workaround doesn't touch the LCR in the ISR, so it
> avoids the possibility of a "serial8250: too much work for irq" lock up.
> This problem is rare in real situations but can be reproduced easily by
> wiring up two UARTs and running the following commands.
> 
>   # stty -F /dev/ttyS1 echo
>   # stty -F /dev/ttyS2 echo
>   # cat /dev/ttyS1 &
>   [1] 375
>   # echo asdf > /dev/ttyS1
>   asdf
> 
>   [   27.700000] serial8250: too much work for irq96
>   [   27.700000] serial8250: too much work for irq96
>   [   27.710000] serial8250: too much work for irq96
>   [   27.710000] serial8250: too much work for irq96
>   [   27.720000] serial8250: too much work for irq96
>   [   27.720000] serial8250: too much work for irq96
>   [   27.730000] serial8250: too much work for irq96
>   [   27.730000] serial8250: too much work for irq96
>   [   27.740000] serial8250: too much work for irq96
> 
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 57 +++++++++++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 17 deletions(-)

This patch no longer applies to my tty-next tree, can you please refresh
it and resend so that I can apply it?

thanks,

greg k-h

      parent reply	other threads:[~2013-09-26 22:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-25  0:39 [PATCH] serial: 8250_dw: Improve unwritable LCR workaround Tim Kryger
2013-09-25 11:42 ` Heikki Krogerus
2013-09-25 22:47   ` Tim Kryger
2013-09-27  7:49     ` Heikki Krogerus
2013-10-01 17:13       ` Tim Kryger
2013-09-26 22:46 ` Greg Kroah-Hartman [this message]

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=20130926224655.GA15106@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=tim.kryger@linaro.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.