All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@hp.com>
To: Thomas Koeller <thomas@koeller.dyndns.org>
Cc: linux-serial@vger.kernel.org,
	Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
Subject: Re: RM9000 code broken in 8250.c
Date: Fri, 21 Dec 2007 07:23:16 -0700	[thread overview]
Message-ID: <1198246996.7091.17.camel@bling> (raw)
In-Reply-To: <200712211250.18002.thomas@koeller.dyndns.org>

Hi Thomas,

   Sorry this code is causing you trouble.  I tried the existing TXEN
bug code, it seems to be fighting a similar, but still quite different
bug.  The UART bug the backup timer code is trying to fix is more
asynchronous.  It might need a kick in the middle of a transmit, not
just at the beginning.  Can we re-arrange the tests such that they both
work?  It would be fine with me if we don't run the test for the backup
timer on ports with UART_BUG_TXEN already enabled.  The easiest
mechanism might be to test port.type for PORT_RM9000, and not test those
for the backup timer.  Then we don't need to re-order the code too much.
Thanks,

  Alex

On Fri, 2007-12-21 at 12:50 +0100, Thomas Koeller wrote:
> Hi,
> 
> while investigating breakage of the RM9000 code in
> drivers/serial/8250.c, I found that my problems are
> caused by commit 40b36daad0ac704e6d5c1b75789f371ef5b053c1.
> My device has the characteristic of not generating a
> 'transmitter empty' interrupt immediately if interrupts
> are enabled while the transmitter is empty, it needs to
> be kicked by transmitting a character first. Before
> commit 40b36daad0ac704e6d5c1b75789f371ef5b053c1 this was
> taken care of by code in serial8250_startup() that probed
> the hardware for this bug an set the UART_BUG_TXEN flag,
> which then modified the behavior of serial8250_start_tx()
> to take the bug into account. This used to work well for
> my device.
> 
> The commit introduced another check for the same condition
> which is done earlier in serial8250_startup(), and causes
> the driver to use a periodic timer instead of the hardware
> interrupt. Now my device uses that timer, while it was
> perfectly capable of using the serial port transmit
> interrupt before.
> 
> I wonder why a new workaround has been introduced for a
> problem for which there had already been a (far superior)
> solution. Didn't this existing solution solve the problem
> for the hardware in question? If so, wouldn't it have been
> possible to modify the existing solution instead of adding
> another one that conflicts with existing code that depends
> on the old code?
> 
> regards,
> Thomas
-- 
Alex Williamson                             HP Open Source & Linux Org.


  reply	other threads:[~2007-12-21 14:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-21 11:50 RM9000 code broken in 8250.c Thomas Koeller
2007-12-21 14:23 ` Alex Williamson [this message]
2008-02-16 17:18   ` Thomas Koeller
2008-02-17  6:09     ` Alex Williamson
2008-02-17 12:47       ` Thomas Koeller
2008-02-20 17:56         ` Alex Williamson
2008-02-26  0:59           ` Thomas Koeller
2008-02-26 15:37             ` Alex Williamson
2008-03-02 16:03               ` Thomas Koeller

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=1198246996.7091.17.camel@bling \
    --to=alex.williamson@hp.com \
    --cc=aris@cathedrallabs.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=thomas@koeller.dyndns.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.