omap-serial: transmission of x-char with DMA (and other issues)

omap-serial: transmission of x-char with DMA (and other issues)

[Russell King - ARM Linux]

Can someone tell me how this works with the current omap-serial driver
please?  It looks to me like this has been broken when DMA support was
added to the driver.

Moreover, please look at the probe function error paths.  They seem to
be lacking any kind of realistic cleanup, so are a potential memory leak.

Then there's the issue of fiddling with the xmit buffer so that it's
using coherent memory in the startup and shutdown functions (why?  when
other serial drivers cope just fine without doing this).  If we want to
use DMA coherent memory there, there should be a clean way to do this,
rather than going behind the upper layers.

Re: omap-serial: transmission of x-char with DMA (and other issues)

[Raja, Govindraj]

On Fri, Apr 13, 2012 at 4:11 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Can someone tell me how this works with the current omap-serial driver
> please?  It looks to me like this has been broken when DMA support was
> added to the driver.

Yes x-char transmission in dma case is broken will post a patch to fix this.

>
> Moreover, please look at the probe function error paths.  They seem to
> be lacking any kind of realistic cleanup, so are a potential memory leak.

I thought this is fixed in 3.4-rc3 with this commit:

commit 388bc26226807fbcf4c626b81bb17a2e74aa4b1b
Author: Shubhrajyoti D <shubhrajyoti@ti.com>
Date:   Wed Mar 21 17:22:22 2012 +0530

    omap-serial: Fix the error handling in the omap_serial probe


>
> Then there's the issue of fiddling with the xmit buffer so that it's
> using coherent memory in the startup and shutdown functions (why?  when
> other serial drivers cope just fine without doing this).  If we want to
> use DMA coherent memory there, there should be a clean way to do this,
> rather than going behind the upper layers.

Okay I will have a look into this.

--
Thanks,
Govindraj.R
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: omap-serial: transmission of x-char with DMA (and other issues)

[Russell King - ARM Linux]

On Mon, Apr 16, 2012 at 04:39:09PM +0530, Raja, Govindraj wrote:
> On Fri, Apr 13, 2012 at 4:11 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Can someone tell me how this works with the current omap-serial driver
> > please?  It looks to me like this has been broken when DMA support was
> > added to the driver.
> 
> Yes x-char transmission in dma case is broken will post a patch to fix this.

Looking at the facilities of the OMAP UART hardware and the behaviour of
the DMA hardware, I don't think we have an option to manually insert an
x-char into the data stream.  We certainly can't pause TX DMA at the
DMA hardware without the DMA hardware aborting and flushing its FIFO,
and the UART has no way to temporarily suspend requesting DMA service.

So, I think we actually need a pair of new hooks from uart_throttle()
and uart_unthrottle() for these automatic flow control UARTs which
disable the RX FIFO being read (iow, disable RX interrupts if using PIO
or RX DMA if using DMA.)  The RX FIFO will then fill, and if the watermarks
are set correctly along with hardware-assisted flow control, the UART
will send the XOFF or deassert RTS for us automatically.

At least with RX DMA (which is hardware source synchronised), the DMA
hardware will apparantly drain its FIFOs to memory when disabled, rather
than aborting and discarding its FIFO contents.

> > Moreover, please look at the probe function error paths.  They seem to
> > be lacking any kind of realistic cleanup, so are a potential memory leak.
> 
> I thought this is fixed in 3.4-rc3 with this commit:
> 
> commit 388bc26226807fbcf4c626b81bb17a2e74aa4b1b
> Author: Shubhrajyoti D <shubhrajyoti@ti.com>
> Date:   Wed Mar 21 17:22:22 2012 +0530
> 
>     omap-serial: Fix the error handling in the omap_serial probe

Well, it also depends on where I base my branches for development - and
if fixes go in after I've started then these things get missed until I
rebase those branches.

If it's already fixed, then great.

> > Then there's the issue of fiddling with the xmit buffer so that it's
> > using coherent memory in the startup and shutdown functions (why?  when
> > other serial drivers cope just fine without doing this).  If we want to
> > use DMA coherent memory there, there should be a clean way to do this,
> > rather than going behind the upper layers.
> 
> Okay I will have a look into this.

I've already cooked up a patch to handle this which adds new hooks from
serial_core into the low level driver for xmit buffer allocation/freeing.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html