linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 00/24] OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Date: Sat, 6 Oct 2012 13:38:03 +0100	[thread overview]
Message-ID: <20121006123803.GD15246@n2100.arm.linux.org.uk> (raw)

Hi,

This series of patches fixes multiple flow control issues with the OMAP
serial driver, and prepares the driver for DMA engine conversion.  We
require hardware assisted flow control to work properly for DMA support
otherwise we have no way to properly pause the transmitter.

This is generated against v3.6, and has been developed mainly by testing
on the OMAP4430 SDP platform.

Flow control seems to be really broken in the OMAP serial driver as things
stand today.  It just about works with software flow control because the
generic serial core layer is inserting those characters, but only when the
legacy DMA support is not being used.  Otherwise, flow control is
completely non-functional.

Issues identified in the OMAP serial driver are:
- set_mctrl() can only assert modem control lines, once asserted it
  is not possible to deassert them.
- IXOFF controls sending of XON/XOFF characters, not the reception of
  these sequences.
- IXON controls the recognition of XON/XOFF characters, not the transmission
  of the same.
- Wrong bitmasks for hardware assisted software flow control.  Bit 2
  in EFR enables sending of XON2/XOFF2 which are never set.
- No point comparing received characters against XOFF2 ('special character
  detect') as XOFF2 is not set.
- Fix multiple places where bits 6 and 5 of MCR are attempted to be
  altered, but because EFR ECB is unset, these bits remain unaffected.
  This effectively prevents us accessing the right XON/XOFF/TCR/TLR
  registers.
- Remove unnecessary read-backs of EFR/MCR/LCR registers - these registers
  don't change beneath us, they are configuration registers which hold their
  values.  Not only does this simplify the code, but it makes it more
  readable, and more importantly ensures that we work from a consistent
  state where ->efr never has ECB set, and ->mcr never has the TCRTLR
  bit set.
- Fix disablement of hardware flow control and IXANY modes; once enabled
  these could never be disabled because nothing in the code ever clears
  these configuration bits.

Once that lot is fixed, these patches expand serial_core to permit hardware
assisted flow control by:
- adding throttle/unthrottle callbacks into low level serial drivers,
  which allow them to take whatever action is necessary with hardware
  assisted flow control to throttle the remote end.  In the case of
  OMAP serial, this means disabling the RX interrupts so that the FIFO
  fills to the watermark.

We then have a number of cleanups to the OMAP serial code to make the
set_termios() function clearer and less prone to the kinds of mistakes
identified above.  This results in a great simplification of the flow
control configuration code.

The OMAP serial driver hacks around with the transmit buffer allocation;
lets clean that up so that drivers can cleanly allocate their transmitter
buffer using coherent memory if that's what they desire.

Finally, the last few patches clean up the plat/omap-serial.h header file,
moving most of its contents into the OMAP serial driver itself.  Most of
this is private to the OMAP serial driver and should never have been
shared with anything else.

I have omitted to include the conversion of the transmit paths to DMA
engine.  Even with all the above fixed, it has issues when DMA transmit
is in progress, and a program issues a TCSETS call (as `less' does after
it has written its prompt.)  At the moment, this causes lots of junk to
be emitted from the serial port when issuing `dmesg | less' which sometimes
brings the port to a complete halt.

As the OMAP DMA hardware does not have a clean pause when performing a
MEM->DEV transfer (it discards its FIFO) I do not see a solution to this,
which probably means that we can _not_ ever support transmit DMA on OMAP
platforms.

This means the xmit buffer allocation patches are not that useful unless
a solution to that can be found.

Now, the remaining question is, how much of this patch set do we think
about merging, and when.  Given that flow control in this driver has been
broken for a very long time, and no one has apparantly noticed, I don't
think there's any urgency to this, so given its size, my preference would
be to queue it up for the next merge window.  The thing that would worry
me about applying some of the initial patches is that they may change
the behaviour today and make any problems here more visible.

             reply	other threads:[~2012-10-06 12:38 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-06 12:38 Russell King - ARM Linux [this message]
2012-10-06 12:39 ` [RFC 01/24] SERIAL: omap: fix set_mctrl() breakage Russell King
2012-10-06 12:39 ` [RFC 02/24] SERIAL: omap: fix bit masks for software flow control Russell King
2012-10-06 12:40 ` [RFC 03/24] SERIAL: omap: remove setting of EFR SCD bit Russell King
2012-10-06 12:40 ` [RFC 04/24] SERIAL: omap: fix MCR TCRTLR bit handling Russell King
2012-10-06 12:40 ` [RFC 05/24] SERIAL: omap: no need to re-read EFR Russell King
2012-10-06 12:41 ` [RFC 06/24] SERIAL: omap: allow hardware assisted rts/cts modes to be disabled Russell King
2012-10-06 12:41 ` [RFC 07/24] SERIAL: omap: allow hardware assisted IXANY mode " Russell King
2012-10-06 12:41 ` [RFC 08/24] SERIAL: core: use local variable uport in uart_set_termios() Russell King
2012-10-06 12:42 ` [RFC 09/24] SERIAL: core: add hardware assisted s/w flow control support Russell King
2012-10-06 12:42 ` [RFC 10/24] SERIAL: core: add hardware assisted h/w " Russell King
2012-10-06 12:42 ` [RFC 11/24] SERIAL: core: add throttle/unthrottle callbacks for hardware assisted flow control Russell King
2012-10-06 12:43 ` [RFC 12/24] SERIAL: omap: fix " Russell King
2012-10-06 12:43 ` [RFC 13/24] SERIAL: omap: configure xon/xoff before setting modem control lines Russell King
2012-10-06 12:43 ` [RFC 14/24] SERIAL: omap: serial_omap_configure_xonxoff() contents into set_termios Russell King
2012-10-06 12:44 ` [RFC 15/24] SERIAL: omap: don't read back LCR/MCR/EFR Russell King
2012-10-06 12:44 ` [RFC 16/24] SERIAL: omap: simplify Russell King
2012-10-06 12:44 ` [RFC 17/24] SERIAL: omap: always set TCR Russell King
2012-10-06 12:45 ` [RFC 18/24] SERIAL: omap: move xon/xoff setting earlier Russell King
2012-10-06 12:45 ` [RFC 19/24] SERIAL: omap: simplify (2) Russell King
2012-10-06 12:45 ` [RFC 20/24] SERIAL: core: add xmit buffer allocation callbacks Russell King
2012-10-06 15:49   ` Alan Cox
2012-10-06 16:51     ` Russell King - ARM Linux
2012-10-06 12:46 ` [RFC 21/24] SERIAL: omap: use tx buffer allocation API Russell King
2012-10-06 12:46 ` [RFC 22/24] SERIAL: omap: typesafe conversion from uart_port to uart_omap_port Russell King
2012-10-06 12:46 ` [RFC 23/24] SERIAL: omap: move driver private definitions and structures to driver Russell King
2012-10-06 12:47 ` [RFC 24/24] SERIAL: omap: remove OMAP_UART_SYSC_RESET and OMAP_UART_FIFO_CLR Russell King
2012-10-06 14:23 ` [RFC 00/24] OMAP serial driver flow control fixes, and preparation for DMA engine conversion Russell King - ARM Linux
2012-10-06 14:39 ` Russell King - ARM Linux
2012-10-06 15:35 ` Russell King - ARM Linux
2012-10-06 15:50 ` Alan Cox
2012-10-09 13:34 ` Sourav
2012-10-10 18:29   ` Kevin Hilman
2012-10-11  9:43     ` Sourav
2012-10-11  9:54       ` Russell King - ARM Linux
2012-10-11 10:21         ` Sourav
2012-10-11 11:08           ` Russell King - ARM Linux
2012-10-11 14:05           ` Jon Hunter
2012-10-12 14:51           ` Grazvydas Ignotas

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=20121006123803.GD15246@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).