Re: [PATCH v4] serial: spi: add spi-uart driver for Maxim 3110

[Feng Tang <feng.tang@intel.com>]

On Sat, 27 Feb 2010 03:41:42 +0800
David Brownell <david-b@pacbell.net> wrote:

> On Friday 26 February 2010, Masakazu Mokuno wrote:
> > > +static int max3110_read_multi(struct uart_max3110 *max, u8 *buf)
> > > +{
> > > +     u16 obuf[M3110_RX_FIFO_DEPTH], ibuf[M3110_RX_FIFO_DEPTH];
> > 
> > Doing I/O on stack is guaranteed safe for spi functions?
> 
> Good catch ...  no it's not safe.
> 
> No DMA-enabled programming interface allows it, including SPI.
Hi David,

I agree that DMA working mode must not use stack as IO buffer, and spi core
code(spi.c/spi.h) has clearly message stating that. But this driver doesn't
intend to use DMA by design. In early version of this patch there is some
controller specific data which show "enable_dma = 0".

The reason of not using DMA for Maxim 3110 is, it is a low speed SPI UART
device, which only has one word (16b) TX buffer and 8 words RX buffer, using
DMA may benefit in some use case but will be over-kill for others. This
driver implement a console,  take one example of editing a file on the console,
each char we input will be echoed back on screen, which will use a spi_message
and a spi_transfer, if using DMA, that DMA operation length will be 2 bytes,
if the file been edited has 4K characters, there will be 4K DMA transactions,
which will occupy quiet some system load. 

Also whether a 16b word could be DMAed is still a question for some types of
DMA controllers, I know some platform whose DMA controllers can only  or
configured to only work with data at least 32 bits aligned. 

So in this driver, it leverages some options provided by SPI controller drivers
(pxa2xx and Designware) to chose not using DMA. 

Thanks,
Feng

> 
> The Documentation/PCI/PCI-DMA-mapping.txt file has a section with
> the oddly punctuated title "What memory is DMA'able?".  That's
> generic to all uses of DMA.  When it was moved to the PCI area,
> that information became harder to find ... but no less true for
> non-PCI contexts (sigh).  One relevant paragraph being:
> 
>   This rule also means that you may use neither kernel image addresses
>   (items in data/text/bss segments), nor module image addresses, nor
>   stack addresses for DMA.  These could all be mapped somewhere
> entirely different than the rest of physical memory.  Even if those
> classes of memory could physically work with DMA, you'd need to
> ensure the I/O buffers were cacheline-aligned.  Without that, you'd
> see cacheline sharing problems (data corruption) on CPUs with
> DMA-incoherent caches. (The CPU could write to one word, DMA would
> write to a different one in the same cache line, and one of them
> could be overwritten.)
> 
> Those cacheline sharing problems are particularly bad for the stack,
> since the data corruption often includes return addresses.
> 
> In short, passing a stack-based I/O buffer could work on some
> machines, but cause perplexing data corruption issues on others.  So
> don't try to do it any driver that claims to be portable.
> 
> - Dave
> 
--
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