Re: [spi-devel-general] [PATCH v4] serial: spi: add spi-uart driver for Maxim 3110
On Fri, 26 Feb 2010 17:59:38 +0800 Masakazu Mokuno mok...@sm.sony.co.jp wrote: Hi, + u8 *pbuf, valid_str[M3110_RX_FIFO_DEPTH]; + int i, j; + + memset(obuf, 0, sizeof(obuf)); + memset(obuf, 0, sizeof(ibuf)); memset(ibuf, 0, sizeof(ibuf))? thanks for the catch, here is a follow on patch diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c index 3283ba6..3dd2082 100644 --- a/drivers/serial/max3110.c +++ b/drivers/serial/max3110.c @@ -142,7 +142,7 @@ static int max3110_read_multi(struct uart_max3110 *max, u8 *buf) int i, j; memset(obuf, 0, sizeof(obuf)); - memset(obuf, 0, sizeof(ibuf)); + memset(ibuf, 0, sizeof(ibuf)); if (max3110_write_then_read(max, obuf, ibuf, M3110_RX_FIFO_DEPTH * 2, 1)) -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v4] serial: spi: add spi-uart driver for Maxim 3110
On Sat, 27 Feb 2010 03:41:42 +0800
David Brownell davi...@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
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v4] serial: spi: add spi-uart driver for Maxim 3110
Hi,
On Fri, 26 Feb 2010 11:47:29 +0800
Feng Tang feng.t...@intel.com wrote:
Hi All,
Here is a driver for Maxim 3110 SPI-UART device, please help to review.
snip
+/*
+ * This is usually used to read data from SPIC RX FIFO, which doesn't
+ * need any delay like flushing character out.
+ *
+ * Return how many valide bytes are read back
+ */
+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?
+ u8 *pbuf, valid_str[M3110_RX_FIFO_DEPTH];
+ int i, j;
+
+ memset(obuf, 0, sizeof(obuf));
+ memset(obuf, 0, sizeof(ibuf));
memset(ibuf, 0, sizeof(ibuf))?
+
+ if (max3110_write_then_read(max, obuf, ibuf,
+ M3110_RX_FIFO_DEPTH * 2, 1))
+ return 0;
+
+ /* If caller doesn't provide a buffer, then handle received char */
+ pbuf = buf ? buf : valid_str;
+
+ for (i = 0, j = 0; i M3110_RX_FIFO_DEPTH; i++) {
+ if (ibuf[i] MAX3110_READ_DATA_AVAILABLE)
+ pbuf[j++] = ibuf[i] 0xff;
+ }
+
+ if (j (pbuf == valid_str))
+ receive_chars(max, valid_str, j);
+
+ return j;
--
Masakazu Mokuno
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v4] serial: spi: add spi-uart driver for Maxim 3110
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.
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
--
Download Intel#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
