Re: [RFC 00/24] OMAP serial driver flow control fixes, and preparation for DMA engine conversion

[Sourav <sourav.poddar@ti.com>]

Hi,

On Saturday 06 October 2012 06:08 PM, Russell King - ARM Linux wrote:
> 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.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda 
board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.

Note, I also tested the patches against the current master but only 
after rebasing, since the current master includes serial patches from 
Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139

Tested-by: Sourav Poddar <sourav.poddar@ti.com>

Thanks,
Sourav