Re: [PATCH RESEND] intel_mid_ssp_spi: Moorestown and Medfield SPI for SSP devices
Hi Grant, On Thu, 9 Feb 2012 07:31:21 -0800 Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Feb 08, 2012 at 10:41:10AM +, Alan Cox wrote: From: Mathieu SOULARD mathieux.soul...@intel.com This driver is a fusion of various internal drivers into a single driver for the SPI slave/master on the Intel Moorestown and Medfield SSP devices. Signed-off-by: Mathieu SOULARD mathieux.soul...@intel.com [Queueing and runtime pm added] Signed-off-by: Kristen Carlson Accardi kris...@linux.intel.com [Ported to the -next tree DMA engine] Signed-off-by: Alan Cox a...@linux.intel.com --- drivers/spi/Kconfig |8 drivers/spi/Makefile|2 drivers/spi/spi-intel-mid-ssp.c | 1426 +++ drivers/spi/spi-intel-mid-ssp.h | 308 If this is merging several of the drivers, what is the plan for the existing SPI_DESIGNWARE and SPI_TOPCLIFF_PCH drivers? Or are those for different devices? The DESIGNWARE controller has a different HW IP core, so the 2 drivers can't be merged. And for the TOPCLIFF one, seems it also use a different HW IP than this one, so I guess it can't be merged either. Thanks, Feng -- Virtualization Cloud Management Using Capacity Planning Cloud computing makes use of virtualization - but cloud computing also focuses on allowing computing to be delivered as a service. http://www.accelacomm.com/jaw/sfnl/114/51521223/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: spi-dw: fix all sparse warnings
On Wed, 21 Sep 2011 04:50:00 +0800
Grant Likely grant.lik...@secretlab.ca wrote:
On Tue, Sep 20, 2011 at 11:06:17AM -0700, H Hartley Sweeten wrote:
The dw_{read,write}[lw] macros produce sparse warnings everytime
they are used. The read ones cause:
warning: cast removes address space of expression
warning: incorrect type in argument 1 (different address spaces)
expected void const volatile [noderef] asn:2*addr
got unsigned int *noident
And the write ones:
warning: cast removes address space of expression
warning: incorrect type in argument 2 (different address spaces)
expected void volatile [noderef] asn:2*addr
got unsigned int *noident
Fix this by removing struct dw_spi_reg and converting all the
register offsets to #defines. Then convert the macros into inlined
functions so that proper type checking can occur.
While here, also fix the three sparse warnings in spi-dw-mid.c due
to the return value of ioremap_nocache being stored in a u32 * not a
void __iomem *.
With these changes the spi-dw* files all build with no sparse
warnings.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Grant Likely grant.lik...@secretlab.ca
Looks good to me. Feng, does this look okay to you?
Yes, the patch looks good to me, thanks
- Feng
g.
---
diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index 130e555..e743a45 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -116,13 +116,13 @@ static int mid_spi_dma_transfer(struct dw_spi
*dws, int cs_change) /* 1. setup DMA related registers */
if (cs_change) {
spi_enable_chip(dws, 0);
- dw_writew(dws, dmardlr, 0xf);
- dw_writew(dws, dmatdlr, 0x10);
+ dw_writew(dws, DW_SPI_DMARDLR, 0xf);
+ dw_writew(dws, DW_SPI_DMATDLR, 0x10);
if (dws-tx_dma)
dma_ctrl |= 0x2;
if (dws-rx_dma)
dma_ctrl |= 0x1;
- dw_writew(dws, dmacr, dma_ctrl);
+ dw_writew(dws, DW_SPI_DMACR, dma_ctrl);
spi_enable_chip(dws, 1);
}
@@ -200,7 +200,8 @@ static struct dw_spi_dma_ops mid_dma_ops = {
int dw_spi_mid_init(struct dw_spi *dws)
{
- u32 *clk_reg, clk_cdiv;
+ void __iomem *clk_reg;
+ u32 clk_cdiv;
clk_reg = ioremap_nocache(MRST_CLK_SPI0_REG, 16);
if (!clk_reg)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 0b99320..082458d 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -89,35 +89,35 @@ static ssize_t spi_show_regs(struct file
*file, char __user *user_buf, len += snprintf(buf + len,
SPI_REGS_BUFSIZE - len, =\n);
len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- CTRL0: \t\t0x%08x\n, dw_readl(dws,
ctrl0));
+ CTRL0: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_CTRL0)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- CTRL1: \t\t0x%08x\n, dw_readl(dws,
ctrl1));
+ CTRL1: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_CTRL1)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- SSIENR: \t0x%08x\n, dw_readl(dws,
ssienr));
+ SSIENR: \t0x%08x\n, dw_readl(dws,
DW_SPI_SSIENR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- SER: \t\t0x%08x\n, dw_readl(dws, ser));
+ SER: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_SER)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- BAUDR: \t\t0x%08x\n, dw_readl(dws,
baudr));
+ BAUDR: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_BAUDR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- TXFTLR: \t0x%08x\n, dw_readl(dws,
txfltr));
+ TXFTLR: \t0x%08x\n, dw_readl(dws,
DW_SPI_TXFLTR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- RXFTLR: \t0x%08x\n, dw_readl(dws,
rxfltr));
+ RXFTLR: \t0x%08x\n, dw_readl(dws,
DW_SPI_RXFLTR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- TXFLR: \t\t0x%08x\n, dw_readl(dws,
txflr));
+ TXFLR: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_TXFLR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- RXFLR: \t\t0x%08x\n, dw_readl(dws,
rxflr));
+ RXFLR: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_RXFLR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- SR: \t\t0x%08x\n, dw_readl(dws, sr));
+ SR: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_SR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- IMR: \t\t0x%08x\n, dw_readl(dws, imr));
+ IMR: \t\t0x%08x\n, dw_readl(dws,
DW_SPI_IMR)); len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
- ISR:
[PATCH 1/2] spi_dw: move spi_dw_chip definition to a header file in include/linux/spi/
So that it could be used to setup slave device's platform data, when platform
boot code enumerates and registers those spi slave devices.
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/spi-dw.h| 20 +---
include/linux/spi/spi-dw-platform.h | 23 +++
2 files changed, 24 insertions(+), 19 deletions(-)
create mode 100644 include/linux/spi/spi-dw-platform.h
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 8b7b07b..290688f 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -3,6 +3,7 @@
#include linux/io.h
#include linux/scatterlist.h
+#include linux/spi/spi-dw-platform.h
/* Bit fields in CTRLR0 */
#define SPI_DFS_OFFSET 0
@@ -49,12 +50,6 @@
/* TX RX interrupt level threshold, max can be 256 */
#define SPI_INT_THRESHOLD 32
-enum dw_ssi_type {
- SSI_MOTO_SPI = 0,
- SSI_TI_SSP,
- SSI_NS_MICROWIRE,
-};
-
struct dw_spi_reg {
u32 ctrl0;
u32 ctrl1;
@@ -209,19 +204,6 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32
mask)
dw_writel(dws, imr, new_mask);
}
-/*
- * Each SPI slave device to work with dw_api controller should
- * has such a structure claiming its working mode (PIO/DMA etc),
- * which can be save in the controller_data member of the
- * struct spi_device
- */
-struct dw_spi_chip {
- u8 poll_mode; /* 0 for contoller polling mode */
- u8 type;/* SPI/SSP/Micrwire */
- u8 enable_dma;
- void (*cs_control)(u32 command);
-};
-
extern int dw_spi_add_host(struct dw_spi *dws);
extern void dw_spi_remove_host(struct dw_spi *dws);
extern int dw_spi_suspend_host(struct dw_spi *dws);
diff --git a/include/linux/spi/spi-dw-platform.h
b/include/linux/spi/spi-dw-platform.h
new file mode 100644
index 000..7c283c6
--- /dev/null
+++ b/include/linux/spi/spi-dw-platform.h
@@ -0,0 +1,23 @@
+#ifndef SPI_DW_PLATFORM_HEADER_H
+#define SPI_DW_PLATFORM_HEADER_H
+
+enum dw_ssi_type {
+ SSI_MOTO_SPI = 0,
+ SSI_TI_SSP,
+ SSI_NS_MICROWIRE,
+};
+
+/*
+ * Each SPI slave device to work with dw_api controller should
+ * has such a structure claiming its working mode (PIO/DMA etc),
+ * which can be save in the controller_data member of the
+ * struct spi_device
+ */
+struct dw_spi_chip {
+ u8 poll_mode; /* 0 for contoller polling mode */
+ u8 type;/* SPI/SSP/Micrwire */
+ u8 enable_dma;
+ void (*cs_control)(u32 command);
+};
+
+#endif /* SPI_DW_PLATFORM_HEADER_H */
--
1.7.1
--
Doing More with Less: The Next Generation Virtual Desktop
What are the key obstacles that have prevented many mid-market businesses
from deploying virtual desktops? How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] dw_spi: Add spi number into spi irq desc
Hi Shuo,
Looks ok to me, and thanks for the fix. One nit is do we have to use a char[48]
for the name?
Thanks,
Feng
On Fri, 8 Jul 2011 11:51:05 +0800
Liu, ShuoX shuox@intel.com wrote:
From 2efa9dbb5c4b1d8fa798d1792498ae21fc3a9d04 Mon Sep 17 00:00:00 2001
From: ShuoX Liu shuox@intel.com
Date: Thu, 7 Jul 2011 16:09:41 +0800
Subject: [PATCH] dw_spi: Add spi number into spi irq desc
Signed-off-by: ShuoX Liu shuox@intel.com
---
drivers/spi/dw_spi.c |4 +++-
drivers/spi/dw_spi.h |1 +
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 919fa9d..68a3026 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -818,9 +818,11 @@ int __devinit dw_spi_add_host(struct dw_spi *dws)
dws-prev_chip = NULL;
dws-dma_inited = 0;
dws-dma_addr = (dma_addr_t)(dws-paddr + 0x60);
+ snprintf(dws-name, sizeof(dws-name), dw_spi%d,
+ dws-bus_num);
ret = request_irq(dws-irq, dw_spi_irq, IRQF_SHARED,
- dw_spi, dws);
+ dws-name, dws);
if (ret 0) {
dev_err(master-dev, can not get IRQ\n);
goto err_free_master;
diff --git a/drivers/spi/dw_spi.h b/drivers/spi/dw_spi.h
index 7a5e78d..c7b9165 100644
--- a/drivers/spi/dw_spi.h
+++ b/drivers/spi/dw_spi.h
@@ -96,6 +96,7 @@ struct dw_spi {
struct spi_device *cur_dev;
struct device *parent_dev;
enum dw_ssi_typetype;
+ charname[48];
void __iomem*regs;
unsigned long paddr;
--
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 00/11] RFC spi-dw updates
Hi Dirk,
On Thu, 23 Jun 2011 10:00:02 +0800
dirk.brande...@gmail.com dirk.brande...@gmail.com wrote:
From: Dirk Brandewie dirk.brande...@gmail.com
This patch set contains a number of changes were buried inside of one
large patch in my last patch set. The rework of the message/transfer
handling changes are not part of this patch set.
This patch set applies after commit ca632f5 on spi/next
Dirk Brandewie (11):
spi-dw: expose platform data stucture.
spi-dw: update function naming convention to match file naming
spi-dw: change MRST prefix to generic prefix
spi-dw: remove unused definition
spi-dw: split spi_dw_enable_chip() into
spi_dw_enable()/spi_dw_disable() spi-dw: Force error on out of range
chip select. spi-dw: Set number of available chip selects correctly
spi-dw: Ensure fifo lenght is set.
spi-dw: Fix condition in spi_dw_{writer/reader}
spi-dw: Move checking of max_speed_hz value to be a prerequisite in
spi_dw_setup spi-dw: remove noop else clause
Most of the patches looks good to me, I'll reply to the rest separately.
Thanks,
Feng
drivers/spi/spi-dw-mid.c | 30 +++---
drivers/spi/spi-dw-mmio.c | 38
drivers/spi/spi-dw-pci.c | 34 +++---
drivers/spi/spi-dw.c | 247
drivers/spi/spi-dw.h | 82 ++-
include/linux/spi/spi-dw.h | 42 6 files changed, 241
insertions(+), 232 deletions(-) create mode 100644
include/linux/spi/spi-dw.h
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 08/11] spi-dw: Ensure fifo lenght is set.
On Thu, 23 Jun 2011 10:00:10 +0800
dirk.brande...@gmail.com dirk.brande...@gmail.com wrote:
From: Dirk Brandewie dirk.brande...@gmail.com
Bug on fifo_len not being set. The fifo sizing routine does not work
since the txfltr register can not be written while the controller is
enabled. The max value of txfltr can be larger than the fifo. The
register allows values upto 0x3f (63) the fifo depth on the Intel
SOC's if 40
Signed-off-by: Dirk Brandewie dirk.brande...@gmail.com
---
drivers/spi/spi-dw.c | 18 ++
1 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index ad92826..cc38aa0 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -776,25 +776,11 @@ static int destroy_queue(struct spi_dw *dws)
/* Restart the controller, disable all interrupts, clean rx fifo */
static void spi_dw_hw_init(struct spi_dw *dws)
{
+ BUG_ON(!dws-fifo_len);
+
spi_dw_disable(dws);
spi_dw_mask_intr(dws, 0xff);
spi_dw_enable(dws);
-
- /*
- * Try to detect the FIFO depth if not set by interface
driver,
- * the depth could be from 2 to 256 from HW spec
- */
- if (!dws-fifo_len) {
- u32 fifo;
- for (fifo = 2; fifo = 257; fifo++) {
- dw_writew(dws, txfltr, fifo);
- if (fifo != dw_readw(dws, txfltr))
- break;
- }
-
- dws-fifo_len = (fifo == 257) ? 0 : fifo;
- dw_writew(dws, txfltr, 0);
- }
}
This code is requested by other community developers, the FIFO
length is adjustable for the dw_spi core when deployed on
different platforms, for those who are not certain about the
FIFO len, they can use this code to probe the FIFO len.
So we need to keep it.
int __devinit spi_dw_add_host(struct spi_dw *dws)
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 09/11] spi-dw: Fix condition in spi_dw_{writer/reader}
On Thu, 23 Jun 2011 10:00:11 +0800
dirk.brande...@gmail.com dirk.brande...@gmail.com wrote:
From: Dirk Brandewie dirk.brande...@gmail.com
Fix the condition based on whether the current transfer has a tx/rx
buffer.
Signed-off-by: Dirk Brandewie dirk.brande...@gmail.com
---
drivers/spi/spi-dw.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index cc38aa0..35b952b 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -193,8 +193,8 @@ static void spi_dw_writer(struct spi_dw *dws)
u16 txw = 0;
while (max--) {
- /* Set the tx word if the transfer's original tx
is not null */
- if (dws-tx_end - dws-len) {
+ /* Set the tx word if the transfer's tx is not
null */
+ if (dws-tx) {
No, in current mainstream code, the dws-tx is changing, see code:
dws-tx += dws-n_bytes;
so we have to use if (dws-tx_end - dws-len) for now, maybe
we can use some bit to indicate whether the original tx is null
if (dws-n_bytes == 1)
txw = *(u8 *)(dws-tx);
else
@@ -213,7 +213,7 @@ static void spi_dw_reader(struct spi_dw *dws)
while (max--) {
rxw = dw_readw(dws, dr);
/* Care rx only if the transfer's original rx is
not null */
- if (dws-rx_end - dws-len) {
+ if (dws-rx) {
if (dws-n_bytes == 1)
*(u8 *)(dws-rx) = rxw;
else
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 11/11] spi-dw: remove noop else clause
On Thu, 23 Jun 2011 10:00:13 +0800
dirk.brande...@gmail.com dirk.brande...@gmail.com wrote:
From: Dirk Brandewie dirk.brande...@gmail.com
The value of spi-bits_per_word is checked on function entry to be 8
or 16. The else clause has no meaning since it can never be reached.
Signed-off-by: Dirk Brandewie dirk.brande...@gmail.com
---
drivers/spi/spi-dw.c |9 +++--
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 7b3f607..5ddd45f 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -667,17 +667,14 @@ static int spi_dw_setup(struct spi_device *spi)
chip-enable_dma = chip_info-enable_dma;
}
- if (spi-bits_per_word = 8) {
+ if (spi-bits_per_word == 8) {
chip-n_bytes = 1;
chip-dma_width = 1;
- } else if (spi-bits_per_word = 16) {
+ } else if (spi-bits_per_word == 16) {
chip-n_bytes = 2;
chip-dma_width = 2;
- } else {
- /* Never take 16b case for DW SPIC */
- dev_err(spi-dev, invalid wordsize\n);
- return -EINVAL;
}
These else case is used to ban the slave spi devices which
try to use 32 bits per word mode. This is like to tell those
spi devices with 32 bpw capability we don't support 32b, pls
change
+
chip-bits_per_word = spi-bits_per_word;
chip-speed_hz = spi-max_speed_hz;
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 08/11] spi-dw: Ensure fifo lenght is set.
On Thu, 23 Jun 2011 11:01:25 +0800
Dirk Brandewie dirk.brande...@gmail.com wrote:
- /*
- * Try to detect the FIFO depth if not set by interface
driver,
- * the depth could be from 2 to 256 from HW spec
- */
- if (!dws-fifo_len) {
- u32 fifo;
- for (fifo = 2; fifo= 257; fifo++) {
- dw_writew(dws, txfltr, fifo);
- if (fifo != dw_readw(dws, txfltr))
- break;
- }
-
- dws-fifo_len = (fifo == 257) ? 0 : fifo;
- dw_writew(dws, txfltr, 0);
- }
}
This code is requested by other community developers, the FIFO
length is adjustable for the dw_spi core when deployed on
different platforms, for those who are not certain about the
FIFO len, they can use this code to probe the FIFO len.
So we need to keep it.
Then I have bad documentation because document I have says that you
can NOt write to txfltr while the controller is enabled. The sizing
routine as it stands can't work for the implementation in Moorsetown
and Medfield.
If there are other SOC's that are implementing different semantics
for txfltr we will need to handle this differently and do some
runtime detection of how the fifo should be sized.
Can you give ne a reference to the person that requiested the sizing
code so I can follow-up
Git-show c587b6fa0510
commit c587b6fa05106606053fc5e8e344f07cd34ace23
Author: Feng Tang feng.t...@intel.com
Date: Thu Jan 21 10:41:10 2010 +0800
spi/dw_spi: add a FIFO depth detection
FIFO depth is configurable for each implementation of DW core,
so add a depth detection for those interface drivers who don't set
the fifo_len explicitly
Signed-off-by: Feng Tang feng.t...@intel.com
Acked-by: Jean-Hugues Deschenes jean-hugues.desche...@octasic.com
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
I added the code per request from Jean-Hugues Deschenes.
Recently Alek Du has tested this code on his Medfield platform.
Thanks,
Feng
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 09/11] spi-dw: Fix condition in spi_dw_{writer/reader}
On Thu, 23 Jun 2011 11:09:34 +0800
Dirk Brandewie dirk.brande...@gmail.com wrote:
On 06/22/2011 07:45 PM, Feng Tang wrote:
On Thu, 23 Jun 2011 10:00:11 +0800
dirk.brande...@gmail.comdirk.brande...@gmail.com wrote:
From: Dirk Brandewiedirk.brande...@gmail.com
Fix the condition based on whether the current transfer has a tx/rx
buffer.
Signed-off-by: Dirk Brandewiedirk.brande...@gmail.com
---
drivers/spi/spi-dw.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index cc38aa0..35b952b 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -193,8 +193,8 @@ static void spi_dw_writer(struct spi_dw *dws)
u16 txw = 0;
while (max--) {
- /* Set the tx word if the transfer's original tx
is not null */
- if (dws-tx_end - dws-len) {
+ /* Set the tx word if the transfer's tx is not
null */
+ if (dws-tx) {
No, in current mainstream code, the dws-tx is changing, see code:
dws-tx += dws-n_bytes;
so we have to use if (dws-tx_end - dws-len) for now, maybe
we can use some bit to indicate whether the original tx is null
If dws-tx is non-null then it points to a valid buffer, it is
unconditionally set with dws-tx = (void *)transfer-tx_buf when the
transfer is setup. if the original transfer-tx_buf is non-null then
this change works.
dws-tx is changing as I said, non-null doesn't mean the value is valid,
when the original tx is null, we don't access it at all, it's just a
indicator of whether the dws-len of zeor has been filled to FIFO.
This logic may looks confusing, but it's correct after years of test.
if (dws-n_bytes == 1)
txw = *(u8 *)(dws-tx);
else
@@ -213,7 +213,7 @@ static void spi_dw_reader(struct spi_dw *dws)
while (max--) {
rxw = dw_readw(dws, dr);
/* Care rx only if the transfer's original rx
is not null */
- if (dws-rx_end - dws-len) {
+ if (dws-rx) {
if (dws-n_bytes == 1)
*(u8 *)(dws-rx) = rxw;
else
--
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 3/5] dw_spi: rework message processing
On Fri, 17 Jun 2011 01:28:16 +0800 Dirk Brandewie dirk.brande...@gmail.com wrote: On 06/16/2011 07:00 AM, Feng Tang wrote: Hi Dirk, IMHO, the patch is too big, it contains too many changes to the original drivers, and we can't see clearly what you've changed to each logical code part or section, If possible, could you separate this patch to several small ones in a logical way. First, I have some questions, what devices have you tested with this patch? high speed, low speed? Do you have any performance data to show the benefit of this change? Current dw_spi driver has been tested with many devices, so to not break them or cause obvious regression, we have to be cautious. See Thread with grant for list of environments where it has been tested. The boot time of the platforms it is being used on decreased 2-5 seconds with no regressions reported. It has been in use/under test for ~3 months in various Meego and Android builds. It clears all the bugs reported against the driver that I am aware of. If you can give me pointers to teams/projects that are using v2.6.37+ kernels I would be more that happy to provide them with patches for their kernel to ensure we get the most comprehensive test possible. I was not asking about the environments, but the actual devices connect. to and work with the dw_spi controller, here is a quote from one of my previous email of devices listing: I don't know all the devices and users, but here is what I know: I've tested Max3110 spi-uart (in-tree), Option GTM501L high-speed 3G modem (out of tree), ektf1236 spi touch screen (out of tree). Alek Du (Cced) should have tested current dw_spi driver with some spi bluetooth device and modem device. Also the original author fordw_spi_mmio.c Jean-Hugues Deschense should have some experience too. - Meego or Android doesn't mean much for a device driver, as all these OSes are using the same Linux kernel. Is 2-5 seconds boot time cuts the only performance data you got? The current driver has 2 phases, first is the original one from me, the second is the optimization by Alek Du which introduce the batch operation for TX/RX FIFO, Alek mentioned the boot time cut too, so I guess the boot time gain is tested against the original driver, not the current mainstream driver, right? For the kernel version you mentioned, my thought is the kernel version diff between 2.6.35 and 2.6.37 doesn't mean much to a simple dw_spi device driver's performance? some users of dw_spi driver are still using 2.6.35 kernel with all new optimizations back ported, I have no power to force them upgrade the kernel, but if the dw_spi driver is the same, then the performance data is mostly trustable. Here are some general comments according to the commit logs: 1. I think the threaded irq handling is a good idea. And let driver chose to use poll or interrupt is good, some other spi controller driver has used that way for a long time 2. Why you remove the cs_change code, in some case, the controller is only be used by one device, we don't need do the config for every single spi_transfer There is no guarantee that all the transfers in a given spi_message have the same values for speed_hz, bits_per_word, cs_change and delay_usecs atleast nothing I could find put that restriction in place. Since we need to deal with possible changes (although unlikely) it gets rid some state we need to maintain and makes the code path common for all transfers. right, if you look at the driver, cs_change is only judge condition for judge, speed_hz, bits_per_word's change are counted in too. Otherwise the driver should fail years ago. 3. Why do you remove the chip select control code, dw_spi controller hw has some problem in chip select controller by SER, and thus many devices has to use external GPIO has their chip select, this is real world usages! Which devices/platforms are you referring to? I was unable to find any platforms or client drivers using this functionality. If they are not public please respond internally. In any case it is mute since I already agreed to put is back in in my response to Grant. Some intel platforms used/are using that, I got requests to help them on it. Also some community guys are using that (though I don't know the detail), as this piece of code is introduced by Shore George once made some change according to their specific change, see commit 052dc7c4 4. I saw you enable both TX/RX interrupt when doing interrupt transfer, spi devices' TX/RX are born to be simutaneous, when one word is sent over TX line, a RX word will be received from RX line, so both the orignal interrupt transfer handling written by me and the later optimization from Alek Du only enable TX interrupt, which will only generate half of IRQs
[PATCH v2 2/4] spi/dw_spi: remove the un-necessary flush()
From: Alek Du alek...@intel.com
The flush() is used to drain all the left data in rx fifo, currently
is is always called together with disabling hw. But from spec, disabling
hw will also reset all the fifo, so flush() is not needed.
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 15 +--
1 files changed, 1 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index c4fca3d..d3aaf8d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -173,17 +173,6 @@ static void wait_till_not_busy(struct dw_spi *dws)
DW SPI: Status keeps busy for 5000us after a read/write!\n);
}
-static void flush(struct dw_spi *dws)
-{
- while (dw_readw(dws, sr) SR_RF_NOT_EMPT) {
- dw_readw(dws, dr);
- cpu_relax();
- }
-
- wait_till_not_busy(dws);
-}
-
-
static int dw_writer(struct dw_spi *dws)
{
u16 txw = 0;
@@ -297,8 +286,7 @@ static void giveback(struct dw_spi *dws)
static void int_error_stop(struct dw_spi *dws, const char *msg)
{
- /* Stop and reset hw */
- flush(dws);
+ /* Stop the hw */
spi_enable_chip(dws, 0);
dev_err(dws-master-dev, %s\n, msg);
@@ -800,7 +788,6 @@ static void spi_hw_init(struct dw_spi *dws)
spi_enable_chip(dws, 0);
spi_mask_intr(dws, 0xff);
spi_enable_chip(dws, 1);
- flush(dws);
/*
* Try to detect the FIFO depth if not set by interface driver,
--
1.7.0.4
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH v2 1/4] spi/dw_spi: unify the low level read/write routines
The original version has many duplicated codes for null/u8/u16 case,
so unify them to make it cleaner
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 103 +-
drivers/spi/dw_spi.h |2 -
2 files changed, 26 insertions(+), 79 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 9a61964..c4fca3d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -58,8 +58,6 @@ struct chip_data {
u8 bits_per_word;
u16 clk_div;/* baud rate divider */
u32 speed_hz; /* baud rate */
- int (*write)(struct dw_spi *dws);
- int (*read)(struct dw_spi *dws);
void (*cs_control)(u32 command);
};
@@ -185,80 +183,45 @@ static void flush(struct dw_spi *dws)
wait_till_not_busy(dws);
}
-static int null_writer(struct dw_spi *dws)
-{
- u8 n_bytes = dws-n_bytes;
-
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
- dw_writew(dws, dr, 0);
- dws-tx += n_bytes;
- wait_till_not_busy(dws);
- return 1;
-}
-
-static int null_reader(struct dw_spi *dws)
+static int dw_writer(struct dw_spi *dws)
{
- u8 n_bytes = dws-n_bytes;
+ u16 txw = 0;
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
- dw_readw(dws, dr);
- dws-rx += n_bytes;
- }
- wait_till_not_busy(dws);
- return dws-rx == dws-rx_end;
-}
-
-static int u8_writer(struct dw_spi *dws)
-{
if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
|| (dws-tx == dws-tx_end))
return 0;
- dw_writew(dws, dr, *(u8 *)(dws-tx));
- ++dws-tx;
-
- wait_till_not_busy(dws);
- return 1;
-}
-
-static int u8_reader(struct dw_spi *dws)
-{
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
- *(u8 *)(dws-rx) = dw_readw(dws, dr);
- ++dws-rx;
+ /* Set the tx word if the transfer's original tx is not null */
+ if (dws-tx_end - dws-len) {
+ if (dws-n_bytes == 1)
+ txw = *(u8 *)(dws-tx);
+ else
+ txw = *(u16 *)(dws-tx);
}
- wait_till_not_busy(dws);
- return dws-rx == dws-rx_end;
-}
-
-static int u16_writer(struct dw_spi *dws)
-{
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
-
- dw_writew(dws, dr, *(u16 *)(dws-tx));
- dws-tx += 2;
+ dw_writew(dws, dr, txw);
+ dws-tx += dws-n_bytes;
wait_till_not_busy(dws);
return 1;
}
-static int u16_reader(struct dw_spi *dws)
+static int dw_reader(struct dw_spi *dws)
{
- u16 temp;
+ u16 rxw;
while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
(dws-rx dws-rx_end)) {
- temp = dw_readw(dws, dr);
- *(u16 *)(dws-rx) = temp;
- dws-rx += 2;
+ rxw = dw_readw(dws, dr);
+ /* Care rx only if the transfer's original rx is not null */
+ if (dws-rx_end - dws-len) {
+ if (dws-n_bytes == 1)
+ *(u8 *)(dws-rx) = rxw;
+ else
+ *(u16 *)(dws-rx) = rxw;
+ }
+ dws-rx += dws-n_bytes;
}
wait_till_not_busy(dws);
@@ -383,8 +346,8 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
left = (left int_level) ? int_level : left;
while (left--)
- dws-write(dws);
- dws-read(dws);
+ dw_writer(dws);
+ dw_reader(dws);
/* Re-enable the IRQ if there is still data left to tx */
if (dws-tx_end dws-tx)
@@ -417,13 +380,13 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
/* Must be called inside pump_transfers() */
static void poll_transfer(struct dw_spi *dws)
{
- while (dws-write(dws))
- dws-read(dws);
+ while (dw_writer(dws))
+ dw_reader(dws);
/*
* There is a possibility that the last word of a transaction
* will be lost if data is not ready. Re-read to solve this issue.
*/
- dws-read(dws);
+ dw_reader(dws);
dw_spi_xfer_done(dws);
}
@@ -483,8 +446,6 @@ static void pump_transfers(unsigned long data)
dws-tx_end = dws-tx + transfer-len;
dws-rx = transfer-rx_buf;
dws-rx_end = dws-rx + transfer-len;
- dws-write = dws-tx ? chip-write : null_writer;
- dws-read = dws-rx ? chip-read : null_reader;
dws-cs_change = transfer-cs_change;
dws-len = dws-cur_transfer-len;
if (chip != dws-prev_chip)
@@ -520,18 +481,10 @@ static void pump_transfers(unsigned
[PATCH v2 0/4] patch serie for dw_spi driver
Hi all, Alek Du made some optimization for the dw_spi driver, the original driver will wait for non-busy state after every work of read/write. With these patches, driver will read/write as much as possible data from/to the HW FIFO. The DMA mode transfer is not affected at all. There is also some minor code cleanup. Pls help to review, thanks! v2 change: * fix a typo about bits_per_word checking * re-generate the patches against v2.6.39-rc1 Thanks, Feng - Alek Du (3): spi/dw_spi: remove the un-necessary flush() spi/dw_spi: change poll mode transfer from byte ops to batch ops spi/dw_spi: improve the interrupt mode with the batch ops Feng Tang (1): spi/dw_spi: unify the low level read/write routines drivers/spi/dw_spi.c | 202 - drivers/spi/dw_spi.h |2 - 2 files changed, 66 insertions(+), 138 deletions(-) -- Create and publish websites with WebMatrix Use the most popular FREE web apps or write code yourself; WebMatrix provides all the features you need to develop and publish your website. http://p.sf.net/sfu/ms-webmatrix-sf ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops
From: Alek Du alek...@intel.com
Current poll transfer will read/write one word, then wait till the
hw is non-busy, it's not efficient. This patch will try to read/write
as many words as permitted by hardware FIFO depth.
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 71 +
1 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index d3aaf8d..7a2a722 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -160,6 +160,37 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi
*dws)
}
#endif /* CONFIG_DEBUG_FS */
+/* Return the max entries we can fill into tx fifo */
+static inline u32 tx_max(struct dw_spi *dws)
+{
+ u32 tx_left, tx_room, rxtx_gap;
+
+ tx_left = (dws-tx_end - dws-tx) / dws-n_bytes;
+ tx_room = dws-fifo_len - dw_readw(dws, txflr);
+
+ /*
+* Another concern is about the tx/rx mismatch, we
+* though to use (dws-fifo_len - rxflr - txflr) as
+* one maximum value for tx, but it doesn't cover the
+* data which is out of tx/rx fifo and inside the
+* shift registers. So a control from sw point of
+* view is taken.
+*/
+ rxtx_gap = ((dws-rx_end - dws-rx) - (dws-tx_end - dws-tx))
+ / dws-n_bytes;
+
+ return min3(tx_left, tx_room, (u32) (dws-fifo_len - rxtx_gap));
+}
+
+/* Return the max entries we should read out of rx fifo */
+static inline u32 rx_max(struct dw_spi *dws)
+{
+ u32 rx_left = (dws-rx_end - dws-rx) / dws-n_bytes;
+
+ return min(rx_left, (u32)dw_readw(dws, rxflr));
+}
+
+
static void wait_till_not_busy(struct dw_spi *dws)
{
unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
@@ -175,33 +206,30 @@ static void wait_till_not_busy(struct dw_spi *dws)
static int dw_writer(struct dw_spi *dws)
{
+ u32 max = tx_max(dws);
u16 txw = 0;
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
-
- /* Set the tx word if the transfer's original tx is not null */
- if (dws-tx_end - dws-len) {
- if (dws-n_bytes == 1)
- txw = *(u8 *)(dws-tx);
- else
- txw = *(u16 *)(dws-tx);
+ while (max--) {
+ /* Set the tx word if the transfer's original tx is not null
*/
+ if (dws-tx_end - dws-len) {
+ if (dws-n_bytes == 1)
+ txw = *(u8 *)(dws-tx);
+ else
+ txw = *(u16 *)(dws-tx);
+ }
+ dw_writew(dws, dr, txw);
+ dws-tx += dws-n_bytes;
}
- dw_writew(dws, dr, txw);
- dws-tx += dws-n_bytes;
-
- wait_till_not_busy(dws);
return 1;
}
static int dw_reader(struct dw_spi *dws)
{
+ u32 max = rx_max(dws);
u16 rxw;
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
+ while (max--) {
rxw = dw_readw(dws, dr);
/* Care rx only if the transfer's original rx is not null */
if (dws-rx_end - dws-len) {
@@ -213,7 +241,6 @@ static int dw_reader(struct dw_spi *dws)
dws-rx += dws-n_bytes;
}
- wait_till_not_busy(dws);
return dws-rx == dws-rx_end;
}
@@ -368,13 +395,11 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
/* Must be called inside pump_transfers() */
static void poll_transfer(struct dw_spi *dws)
{
- while (dw_writer(dws))
+ do {
+ dw_writer(dws);
dw_reader(dws);
- /*
-* There is a possibility that the last word of a transaction
-* will be lost if data is not ready. Re-read to solve this issue.
-*/
- dw_reader(dws);
+ cpu_relax();
+ } while (dws-rx_end dws-rx);
dw_spi_xfer_done(dws);
}
--
1.7.0.4
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH v2 4/4] spi/dw_spi: improve the interrupt mode with the batch ops
From: Alek Du alek...@intel.com
leverage the performance gain by change in low level
read/write batch operations
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 63 -
1 files changed, 16 insertions(+), 47 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 7a2a722..855ac4a 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -190,21 +190,7 @@ static inline u32 rx_max(struct dw_spi *dws)
return min(rx_left, (u32)dw_readw(dws, rxflr));
}
-
-static void wait_till_not_busy(struct dw_spi *dws)
-{
- unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
-
- while (time_before(jiffies, end)) {
- if (!(dw_readw(dws, sr) SR_BUSY))
- return;
- cpu_relax();
- }
- dev_err(dws-master-dev,
- DW SPI: Status keeps busy for 5000us after a read/write!\n);
-}
-
-static int dw_writer(struct dw_spi *dws)
+static void dw_writer(struct dw_spi *dws)
{
u32 max = tx_max(dws);
u16 txw = 0;
@@ -220,11 +206,9 @@ static int dw_writer(struct dw_spi *dws)
dw_writew(dws, dr, txw);
dws-tx += dws-n_bytes;
}
-
- return 1;
}
-static int dw_reader(struct dw_spi *dws)
+static void dw_reader(struct dw_spi *dws)
{
u32 max = rx_max(dws);
u16 rxw;
@@ -240,8 +224,6 @@ static int dw_reader(struct dw_spi *dws)
}
dws-rx += dws-n_bytes;
}
-
- return dws-rx == dws-rx_end;
}
static void *next_transfer(struct dw_spi *dws)
@@ -340,35 +322,28 @@ EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
static irqreturn_t interrupt_transfer(struct dw_spi *dws)
{
- u16 irq_status, irq_mask = 0x3f;
- u32 int_level = dws-fifo_len / 2;
- u32 left;
+ u16 irq_status = dw_readw(dws, isr);
- irq_status = dw_readw(dws, isr) irq_mask;
/* Error handling */
if (irq_status (SPI_INT_TXOI | SPI_INT_RXOI | SPI_INT_RXUI)) {
dw_readw(dws, txoicr);
dw_readw(dws, rxoicr);
dw_readw(dws, rxuicr);
- int_error_stop(dws, interrupt_transfer: fifo overrun);
+ int_error_stop(dws, interrupt_transfer: fifo
overrun/underrun);
return IRQ_HANDLED;
}
+ dw_reader(dws);
+ if (dws-rx_end == dws-rx) {
+ spi_mask_intr(dws, SPI_INT_TXEI);
+ dw_spi_xfer_done(dws);
+ return IRQ_HANDLED;
+ }
if (irq_status SPI_INT_TXEI) {
spi_mask_intr(dws, SPI_INT_TXEI);
-
- left = (dws-tx_end - dws-tx) / dws-n_bytes;
- left = (left int_level) ? int_level : left;
-
- while (left--)
- dw_writer(dws);
- dw_reader(dws);
-
- /* Re-enable the IRQ if there is still data left to tx */
- if (dws-tx_end dws-tx)
- spi_umask_intr(dws, SPI_INT_TXEI);
- else
- dw_spi_xfer_done(dws);
+ dw_writer(dws);
+ /* Enable TX irq always, it will be disabled when RX finished */
+ spi_umask_intr(dws, SPI_INT_TXEI);
}
return IRQ_HANDLED;
@@ -377,15 +352,13 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
static irqreturn_t dw_spi_irq(int irq, void *dev_id)
{
struct dw_spi *dws = dev_id;
- u16 irq_status, irq_mask = 0x3f;
+ u16 irq_status = dw_readw(dws, isr) 0x3f;
- irq_status = dw_readw(dws, isr) irq_mask;
if (!irq_status)
return IRQ_NONE;
if (!dws-cur_msg) {
spi_mask_intr(dws, SPI_INT_TXEI);
- /* Never fail */
return IRQ_HANDLED;
}
@@ -492,12 +465,8 @@ static void pump_transfers(unsigned long data)
switch (bits) {
case 8:
- dws-n_bytes = 1;
- dws-dma_width = 1;
- break;
case 16:
- dws-n_bytes = 2;
- dws-dma_width = 2;
+ dws-n_bytes = dws-dma_width = bits 3;
break;
default:
printk(KERN_ERR MRST SPI0: unsupported bits:
@@ -541,7 +510,7 @@ static void pump_transfers(unsigned long data)
txint_level = dws-fifo_len / 2;
txint_level = (templen txint_level) ? txint_level : templen;
- imask |= SPI_INT_TXEI;
+ imask |= SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI |
SPI_INT_RXOI;
dws-transfer_handler = interrupt_transfer;
}
--
1.7.0.4
--
Create and publish websites with WebMatrix
Use the most
Re: dw_spi CS control/transfer modes
Hi Jamie, On Sat, 19 Mar 2011 04:11:35 +0800 Jamie Iles ja...@jamieiles.com wrote: Hi, I have a platform with a DesignWare SSI and I'm trying to use the dw_spi block using a GPIO for the chip select to workaround the problem of CS changing and the device losing state (we have a SPI flash attached), and I've found that using interrupt driven transfers, reading the SPI flash results in very few interrupts from the controller and a sluggish system (and no data from the flash). However, I've found that removing the conditional transfer mode setting (effectively reverting commit 052dc7c45 [spi/dw_spi: conditional transfer mode changes]) allows everything to work fine by keeping the transfer mode to transmit+receive. What version of code are you using? The cs_control will be default to NULL if dw_spi_chip as the controller data doesn't explicitly set it, and then that piece of code won't be called. Thanks, Feng Does anyone have any ideas on the best way to fix this? That code must have been added for a reason so perhaps it's just a quirk on our platform, but I'd be keen to know in what ways it's been tested before - afaict, no other in-tree platforms are using dw_spi with their own cs_control so it's difficult to know if perhaps these are all doing polled mode transfers. Thanks, Jamie -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH 2/4] spi/dw_spi: remove the un-necessary flush()
The flush() is used to drain all the left data in rx fifo, currently
is is always called together with disabling hw. But from spec, disabling
hw will also reset all the fifo, so flush() is not needed.
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 15 +--
1 files changed, 1 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index c83f6b1..35ac2b9 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -173,17 +173,6 @@ static void wait_till_not_busy(struct dw_spi *dws)
DW SPI: Status keeps busy for 5000us after a read/write!\n);
}
-static void flush(struct dw_spi *dws)
-{
- while (dw_readw(dws, sr) SR_RF_NOT_EMPT) {
- dw_readw(dws, dr);
- cpu_relax();
- }
-
- wait_till_not_busy(dws);
-}
-
-
static int dw_writer(struct dw_spi *dws)
{
u16 txw = 0;
@@ -297,8 +286,7 @@ static void giveback(struct dw_spi *dws)
static void int_error_stop(struct dw_spi *dws, const char *msg)
{
- /* Stop and reset hw */
- flush(dws);
+ /* Stop the hw */
spi_enable_chip(dws, 0);
dev_err(dws-master-dev, %s\n, msg);
@@ -800,7 +788,6 @@ static void spi_hw_init(struct dw_spi *dws)
spi_enable_chip(dws, 0);
spi_mask_intr(dws, 0xff);
spi_enable_chip(dws, 1);
- flush(dws);
/*
* Try to detect the FIFO depth if not set by interface driver,
--
1.7.0.4
--
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH 1/4] spi/dw_spi: unify the low level read/write routines
The original version has many duplicated codes for null/u8/u16 case,
so unify them to make it cleaner
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 103 +++-
include/linux/spi/dw_spi.h |2 -
2 files changed, 26 insertions(+), 79 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 22af77f..c83f6b1 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -58,8 +58,6 @@ struct chip_data {
u8 bits_per_word;
u16 clk_div;/* baud rate divider */
u32 speed_hz; /* baud rate */
- int (*write)(struct dw_spi *dws);
- int (*read)(struct dw_spi *dws);
void (*cs_control)(u32 command);
};
@@ -185,80 +183,45 @@ static void flush(struct dw_spi *dws)
wait_till_not_busy(dws);
}
-static int null_writer(struct dw_spi *dws)
-{
- u8 n_bytes = dws-n_bytes;
-
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
- dw_writew(dws, dr, 0);
- dws-tx += n_bytes;
- wait_till_not_busy(dws);
- return 1;
-}
-
-static int null_reader(struct dw_spi *dws)
+static int dw_writer(struct dw_spi *dws)
{
- u8 n_bytes = dws-n_bytes;
+ u16 txw = 0;
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
- dw_readw(dws, dr);
- dws-rx += n_bytes;
- }
- wait_till_not_busy(dws);
- return dws-rx == dws-rx_end;
-}
-
-static int u8_writer(struct dw_spi *dws)
-{
if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
|| (dws-tx == dws-tx_end))
return 0;
- dw_writew(dws, dr, *(u8 *)(dws-tx));
- ++dws-tx;
-
- wait_till_not_busy(dws);
- return 1;
-}
-
-static int u8_reader(struct dw_spi *dws)
-{
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
- *(u8 *)(dws-rx) = dw_readw(dws, dr);
- ++dws-rx;
+ /* Set the tx word if the transfer's original tx is not null */
+ if (dws-tx_end - dws-len) {
+ if (dws-n_bytes == 8)
+ txw = *(u8 *)(dws-tx);
+ else
+ txw = *(u16 *)(dws-tx);
}
- wait_till_not_busy(dws);
- return dws-rx == dws-rx_end;
-}
-
-static int u16_writer(struct dw_spi *dws)
-{
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
-
- dw_writew(dws, dr, *(u16 *)(dws-tx));
- dws-tx += 2;
+ dw_writew(dws, dr, txw);
+ dws-tx += dws-n_bytes;
wait_till_not_busy(dws);
return 1;
}
-static int u16_reader(struct dw_spi *dws)
+static int dw_reader(struct dw_spi *dws)
{
- u16 temp;
+ u16 rxw;
while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
(dws-rx dws-rx_end)) {
- temp = dw_readw(dws, dr);
- *(u16 *)(dws-rx) = temp;
- dws-rx += 2;
+ rxw = dw_readw(dws, dr);
+ /* Care rx only if the transfer's original rx is not null */
+ if (dws-rx_end - dws-len) {
+ if (dws-n_bytes == 8)
+ *(u8 *)(dws-rx) = rxw;
+ else
+ *(u16 *)(dws-rx) = rxw;
+ }
+ dws-rx += dws-n_bytes;
}
wait_till_not_busy(dws);
@@ -383,8 +346,8 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
left = (left int_level) ? int_level : left;
while (left--)
- dws-write(dws);
- dws-read(dws);
+ dw_writer(dws);
+ dw_reader(dws);
/* Re-enable the IRQ if there is still data left to tx */
if (dws-tx_end dws-tx)
@@ -417,13 +380,13 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
/* Must be called inside pump_transfers() */
static void poll_transfer(struct dw_spi *dws)
{
- while (dws-write(dws))
- dws-read(dws);
+ while (dw_writer(dws))
+ dw_reader(dws);
/*
* There is a possibility that the last word of a transaction
* will be lost if data is not ready. Re-read to solve this issue.
*/
- dws-read(dws);
+ dw_reader(dws);
dw_spi_xfer_done(dws);
}
@@ -483,8 +446,6 @@ static void pump_transfers(unsigned long data)
dws-tx_end = dws-tx + transfer-len;
dws-rx = transfer-rx_buf;
dws-rx_end = dws-rx + transfer-len;
- dws-write = dws-tx ? chip-write : null_writer;
- dws-read = dws-rx ? chip-read : null_reader;
dws-cs_change = transfer-cs_change;
dws-len = dws-cur_transfer-len;
if (chip != dws-prev_chip)
@@ -520,18 +481,10 @@ static void pump_transfers
[PATCH 0/4] patch serie for dw_spi driver
Hi all, Alek Du made some optimization for the dw_spi driver, the original driver will wait for non-busy state after every work of read/write. With these patches, driver will read/write as much as possible data from/to the HW FIFO. The DMA mode transfer is not affected at all. There is also some minor code cleanup. The patch serie is generated against 2.6.38 kernel. Pls help to review, thanks! - Feng - Feng Tang (4): spi/dw_spi: unify the low level read/write routines spi/dw_spi: remove the un-necessary flush() spi/dw_spi: change poll mode transfer from byte ops to batch ops spi/dw_spi: improve the interrupt mode with the batch ops drivers/spi/dw_spi.c | 202 ++- include/linux/spi/dw_spi.h |2 - 2 files changed, 66 insertions(+), 138 deletions(-) -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH 4/4] spi/dw_spi: improve the interrupt mode with the batch ops
leverage the performance gain by change in low level
read/write batch operations
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 63 -
1 files changed, 16 insertions(+), 47 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index ba8b394..a063019 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -190,21 +190,7 @@ static inline u32 rx_max(struct dw_spi *dws)
return min(rx_left, (u32)dw_readw(dws, rxflr));
}
-
-static void wait_till_not_busy(struct dw_spi *dws)
-{
- unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
-
- while (time_before(jiffies, end)) {
- if (!(dw_readw(dws, sr) SR_BUSY))
- return;
- cpu_relax();
- }
- dev_err(dws-master-dev,
- DW SPI: Status keeps busy for 5000us after a read/write!\n);
-}
-
-static int dw_writer(struct dw_spi *dws)
+static void dw_writer(struct dw_spi *dws)
{
u32 max = tx_max(dws);
u16 txw = 0;
@@ -220,11 +206,9 @@ static int dw_writer(struct dw_spi *dws)
dw_writew(dws, dr, txw);
dws-tx += dws-n_bytes;
}
-
- return 1;
}
-static int dw_reader(struct dw_spi *dws)
+static void dw_reader(struct dw_spi *dws)
{
u32 max = rx_max(dws);
u16 rxw;
@@ -240,8 +224,6 @@ static int dw_reader(struct dw_spi *dws)
}
dws-rx += dws-n_bytes;
}
-
- return dws-rx == dws-rx_end;
}
static void *next_transfer(struct dw_spi *dws)
@@ -340,35 +322,28 @@ EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
static irqreturn_t interrupt_transfer(struct dw_spi *dws)
{
- u16 irq_status, irq_mask = 0x3f;
- u32 int_level = dws-fifo_len / 2;
- u32 left;
+ u16 irq_status = dw_readw(dws, isr);
- irq_status = dw_readw(dws, isr) irq_mask;
/* Error handling */
if (irq_status (SPI_INT_TXOI | SPI_INT_RXOI | SPI_INT_RXUI)) {
dw_readw(dws, txoicr);
dw_readw(dws, rxoicr);
dw_readw(dws, rxuicr);
- int_error_stop(dws, interrupt_transfer: fifo overrun);
+ int_error_stop(dws, interrupt_transfer: fifo
overrun/underrun);
return IRQ_HANDLED;
}
+ dw_reader(dws);
+ if (dws-rx_end == dws-rx) {
+ spi_mask_intr(dws, SPI_INT_TXEI);
+ dw_spi_xfer_done(dws);
+ return IRQ_HANDLED;
+ }
if (irq_status SPI_INT_TXEI) {
spi_mask_intr(dws, SPI_INT_TXEI);
-
- left = (dws-tx_end - dws-tx) / dws-n_bytes;
- left = (left int_level) ? int_level : left;
-
- while (left--)
- dw_writer(dws);
- dw_reader(dws);
-
- /* Re-enable the IRQ if there is still data left to tx */
- if (dws-tx_end dws-tx)
- spi_umask_intr(dws, SPI_INT_TXEI);
- else
- dw_spi_xfer_done(dws);
+ dw_writer(dws);
+ /* Enable TX irq always, it will be disabled when RX finished */
+ spi_umask_intr(dws, SPI_INT_TXEI);
}
return IRQ_HANDLED;
@@ -377,15 +352,13 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
static irqreturn_t dw_spi_irq(int irq, void *dev_id)
{
struct dw_spi *dws = dev_id;
- u16 irq_status, irq_mask = 0x3f;
+ u16 irq_status = dw_readw(dws, isr) 0x3f;
- irq_status = dw_readw(dws, isr) irq_mask;
if (!irq_status)
return IRQ_NONE;
if (!dws-cur_msg) {
spi_mask_intr(dws, SPI_INT_TXEI);
- /* Never fail */
return IRQ_HANDLED;
}
@@ -492,12 +465,8 @@ static void pump_transfers(unsigned long data)
switch (bits) {
case 8:
- dws-n_bytes = 1;
- dws-dma_width = 1;
- break;
case 16:
- dws-n_bytes = 2;
- dws-dma_width = 2;
+ dws-n_bytes = dws-dma_width = bits 3;
break;
default:
printk(KERN_ERR MRST SPI0: unsupported bits:
@@ -541,7 +510,7 @@ static void pump_transfers(unsigned long data)
txint_level = dws-fifo_len / 2;
txint_level = (templen txint_level) ? txint_level : templen;
- imask |= SPI_INT_TXEI;
+ imask |= SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI |
SPI_INT_RXOI;
dws-transfer_handler = interrupt_transfer;
}
--
1.7.0.4
--
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
[PATCH 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops
Current poll transfer will read/write one word, then wait till the
hw is non-busy, it's not efficient. This patch will try to read/write
as many words as permitted by hardware FIFO depth.
Signed-off-by: Alek Du alek...@intel.com
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 71 +
1 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 35ac2b9..ba8b394 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -160,6 +160,37 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi
*dws)
}
#endif /* CONFIG_DEBUG_FS */
+/* Return the max entries we can fill into tx fifo */
+static inline u32 tx_max(struct dw_spi *dws)
+{
+ u32 tx_left, tx_room, rxtx_gap;
+
+ tx_left = (dws-tx_end - dws-tx) / dws-n_bytes;
+ tx_room = dws-fifo_len - dw_readw(dws, txflr);
+
+ /*
+* Another concern is about the tx/rx mismatch, we
+* though to use (dws-fifo_len - rxflr - txflr) as
+* one maximum value for tx, but it doesn't cover the
+* data which is out of tx/rx fifo and inside the
+* shift registers. So a control from sw point of
+* view is taken.
+*/
+ rxtx_gap = ((dws-rx_end - dws-rx) - (dws-tx_end - dws-tx))
+ / dws-n_bytes;
+
+ return min3(tx_left, tx_room, (u32) (dws-fifo_len - rxtx_gap));
+}
+
+/* Return the max entries we should read out of rx fifo */
+static inline u32 rx_max(struct dw_spi *dws)
+{
+ u32 rx_left = (dws-rx_end - dws-rx) / dws-n_bytes;
+
+ return min(rx_left, (u32)dw_readw(dws, rxflr));
+}
+
+
static void wait_till_not_busy(struct dw_spi *dws)
{
unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
@@ -175,33 +206,30 @@ static void wait_till_not_busy(struct dw_spi *dws)
static int dw_writer(struct dw_spi *dws)
{
+ u32 max = tx_max(dws);
u16 txw = 0;
- if (!(dw_readw(dws, sr) SR_TF_NOT_FULL)
- || (dws-tx == dws-tx_end))
- return 0;
-
- /* Set the tx word if the transfer's original tx is not null */
- if (dws-tx_end - dws-len) {
- if (dws-n_bytes == 8)
- txw = *(u8 *)(dws-tx);
- else
- txw = *(u16 *)(dws-tx);
+ while (max--) {
+ /* Set the tx word if the transfer's original tx is not null
*/
+ if (dws-tx_end - dws-len) {
+ if (dws-n_bytes == 8)
+ txw = *(u8 *)(dws-tx);
+ else
+ txw = *(u16 *)(dws-tx);
+ }
+ dw_writew(dws, dr, txw);
+ dws-tx += dws-n_bytes;
}
- dw_writew(dws, dr, txw);
- dws-tx += dws-n_bytes;
-
- wait_till_not_busy(dws);
return 1;
}
static int dw_reader(struct dw_spi *dws)
{
+ u32 max = rx_max(dws);
u16 rxw;
- while ((dw_readw(dws, sr) SR_RF_NOT_EMPT)
-(dws-rx dws-rx_end)) {
+ while (max--) {
rxw = dw_readw(dws, dr);
/* Care rx only if the transfer's original rx is not null */
if (dws-rx_end - dws-len) {
@@ -213,7 +241,6 @@ static int dw_reader(struct dw_spi *dws)
dws-rx += dws-n_bytes;
}
- wait_till_not_busy(dws);
return dws-rx == dws-rx_end;
}
@@ -368,13 +395,11 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
/* Must be called inside pump_transfers() */
static void poll_transfer(struct dw_spi *dws)
{
- while (dw_writer(dws))
+ do {
+ dw_writer(dws);
dw_reader(dws);
- /*
-* There is a possibility that the last word of a transaction
-* will be lost if data is not ready. Re-read to solve this issue.
-*/
- dw_reader(dws);
+ cpu_relax();
+ } while (dws-rx_end dws-rx);
dw_spi_xfer_done(dws);
}
--
1.7.0.4
--
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 4/4] spi/dw_spi: improve the interrupt mode with the batch ops
Hi Alan, On Fri, 18 Mar 2011 18:13:10 +0800 Alan Cox a...@linux.intel.com wrote: On Fri, 18 Mar 2011 15:50:56 +0800 Feng Tang feng.t...@intel.com wrote: spi/dw_spi: improve the interrupt mode with the batch ops Look sane to me - only question I have is should the patches have a From: Alek Du ... on them ? I've strongly requested Alek to make the patches himself, but he is too busy with his project and just give me a .35 version dw_spi.c. So to get it quickly upstream, I manually separate his change, modify a little, combined with mine, test, add some comments and his Signed-off. I really don't know the From: matters so much, and thought the signed-off and the cover-letter already claim the right credit :). Is is possible for Grant to change the From: to Alek when he would like to merge these patches? Thanks, Feng -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/dw_spi: move dw_spi.h into drivers/spi
Looks good to me, thanks for the move.
- Feng
On Tue, 1 Mar 2011 04:50:55 +0800
Grant Likely grant.lik...@secretlab.ca wrote:
include/linux/dw_spi.h only includes driver internal data. It doesn't
expose a platform_data configuration structure or similar (at least
nothing in-tree). This patch moves the header into drivers/spi so
that the scope is limited to only the dw_spi_*.c driver files
This patch also adds a missing #include linux/scatterlist.h that
prevented the driver from building.
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Cc: Feng Tang feng.t...@intel.com
Cc: spi-devel-general@lists.sourceforge.net
---
drivers/spi/dw_spi.c |4 -
drivers/spi/dw_spi.h | 233
drivers/spi/dw_spi_mid.c |3 - drivers/spi/dw_spi_mmio.c |4
+ drivers/spi/dw_spi_pci.c |3 -
include/linux/spi/dw_spi.h | 233
6 files changed, 242
insertions(+), 238 deletions(-) create mode 100644
drivers/spi/dw_spi.h delete mode 100644 include/linux/spi/dw_spi.h
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 22af77f..9a61964 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -22,10 +22,10 @@
#include linux/highmem.h
#include linux/delay.h
#include linux/slab.h
-
-#include linux/spi/dw_spi.h
#include linux/spi/spi.h
+#include dw_spi.h
+
#ifdef CONFIG_DEBUG_FS
#include linux/debugfs.h
#endif
diff --git a/drivers/spi/dw_spi.h b/drivers/spi/dw_spi.h
new file mode 100644
index 000..6cd10f6
--- /dev/null
+++ b/drivers/spi/dw_spi.h
@@ -0,0 +1,233 @@
+#ifndef DW_SPI_HEADER_H
+#define DW_SPI_HEADER_H
+
+#include linux/io.h
+
+/* Bit fields in CTRLR0 */
+#define SPI_DFS_OFFSET 0
+
+#define SPI_FRF_OFFSET 4
+#define SPI_FRF_SPI0x0
+#define SPI_FRF_SSP0x1
+#define SPI_FRF_MICROWIRE 0x2
+#define SPI_FRF_RESV 0x3
+
+#define SPI_MODE_OFFSET6
+#define SPI_SCPH_OFFSET6
+#define SPI_SCOL_OFFSET7
+
+#define SPI_TMOD_OFFSET8
+#define SPI_TMOD_MASK (0x3 SPI_TMOD_OFFSET)
+#defineSPI_TMOD_TR 0x0 /*
xmit recv */ +#define SPI_TMOD_TO
0x1 /* xmit only */ +#define
SPI_TMOD_RO0x2 /* recv only */
+#define SPI_TMOD_EPROMREAD 0x3 /* eeprom
read mode */ + +#define SPI_SLVOE_OFFSET 10
+#define SPI_SRL_OFFSET 11
+#define SPI_CFS_OFFSET 12
+
+/* Bit fields in SR, 7 bits */
+#define SR_MASK0x7f/*
cover 7 bits */ +#define SR_BUSY(1
0) +#define SR_TF_NOT_FULL (1 1)
+#define SR_TF_EMPT (1 2)
+#define SR_RF_NOT_EMPT (1 3)
+#define SR_RF_FULL (1 4)
+#define SR_TX_ERR (1 5)
+#define SR_DCOL(1 6)
+
+/* Bit fields in ISR, IMR, RISR, 7 bits */
+#define SPI_INT_TXEI (1 0)
+#define SPI_INT_TXOI (1 1)
+#define SPI_INT_RXUI (1 2)
+#define SPI_INT_RXOI (1 3)
+#define SPI_INT_RXFI (1 4)
+#define SPI_INT_MSTI (1 5)
+
+/* TX RX interrupt level threshhold, max can be 256 */
+#define SPI_INT_THRESHOLD 32
+
+enum dw_ssi_type {
+ SSI_MOTO_SPI = 0,
+ SSI_TI_SSP,
+ SSI_NS_MICROWIRE,
+};
+
+struct dw_spi_reg {
+ u32 ctrl0;
+ u32 ctrl1;
+ u32 ssienr;
+ u32 mwcr;
+ u32 ser;
+ u32 baudr;
+ u32 txfltr;
+ u32 rxfltr;
+ u32 txflr;
+ u32 rxflr;
+ u32 sr;
+ u32 imr;
+ u32 isr;
+ u32 risr;
+ u32 txoicr;
+ u32 rxoicr;
+ u32 rxuicr;
+ u32 msticr;
+ u32 icr;
+ u32 dmacr;
+ u32 dmatdlr;
+ u32 dmardlr;
+ u32 idr;
+ u32 version;
+ u32 dr; /* Currently oper as 32 bits,
+ though only low 16 bits matters */
+} __packed;
+
+struct dw_spi;
+struct dw_spi_dma_ops {
+ int (*dma_init)(struct dw_spi *dws);
+ void (*dma_exit)(struct dw_spi *dws);
+ int (*dma_transfer)(struct dw_spi *dws, int cs_change);
+};
+
+struct dw_spi {
+ struct spi_master *master;
+ struct spi_device *cur_dev;
+ struct device *parent_dev;
+ enum dw_ssi_typetype;
+
+ void __iomem*regs;
+ unsigned long paddr;
+ u32
Re: MRST SPI
Hi Tom,
On Thu, 10 Feb 2011 18:48:00 +0800
Tom tom.spi.de...@gmail.com wrote:
Hi All,
I have a SPI device problem on MRST CDK.
I use a GPIO as SS because my slave device expects the SS to be low
between byte transfers.
The DW spi controller has a slave select register for that purpose, and
myself don't have experience of using an external GPIO lines for SS, so
no comment here.
My probe function does a read as:
int read_val;
---
---
read_val = spi_w8r8(spi_dev,0x20);
if ( read_val 0 )
printk ( spi_read err\n);
else
printk ( spi_read %d\n,read_val );
--
The MRST SFI interface passes the device board info. Because, my
device is not listed in the IA32 f/w I modified the MRST.c code
sfi_handle_spi_dev function as:
while(dev-name[0])
{
if (dev-type == SFI_DEV_TYPE_SPI
!strncmp(dev-name,
spi_info-modalias, 16)) {
if (!strncmp(spi_old_device,
spi_info-modalias, 16)){
strncpy (spi_info-modalias,
spi_my_dev,16);
pdata =
dev-get_platform_data(spi_info); pr_info ( spi_old_device modalias
to spi_my_dev );
}
else
{
pdata = dev-get_platform_data(spi_info);
}
break;
}
dev++;
}
i also modified the devs_id array to change the platform_data
{spi_old_dev, SFI_DEV_TYPE_SPI, 0, no_platform_data}
The issue is that for the spi_w8r8 call, i get multiple read
(multiple * (8 clock cycles/SS toggle))on the SPI bus.
Also regarding the spi-irq, is it the responsibility of the protocol
driver to register for this irq and handle it or does the spi
framework or master driver take care of this ?
Any pointers would be really helpful. ( attached is the test code)
Suggest you refer the arch/platform/mrst/mrst.c about setting up
the max3110 spi device's platform and controller data, you can
find it in Alan Cox's MID reference tree:
git://git.kernel.org/pub/scm/linux/kernel/git/alan/linux-2.6-mid-ref.git
Thanks,
Feng
Regards,
Tom
--
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[Patch v2 2/3] spi/dw_spi: change to EXPORT_SYMBOL_GPL for exported APIs
Acked-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 25238a8..b50bf5b 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -941,7 +941,7 @@ err_free_master:
exit:
return ret;
}
-EXPORT_SYMBOL(dw_spi_add_host);
+EXPORT_SYMBOL_GPL(dw_spi_add_host);
void __devexit dw_spi_remove_host(struct dw_spi *dws)
{
@@ -965,7 +965,7 @@ void __devexit dw_spi_remove_host(struct dw_spi *dws)
/* Disconnect from the SPI framework */
spi_unregister_master(dws-master);
}
-EXPORT_SYMBOL(dw_spi_remove_host);
+EXPORT_SYMBOL_GPL(dw_spi_remove_host);
int dw_spi_suspend_host(struct dw_spi *dws)
{
@@ -978,7 +978,7 @@ int dw_spi_suspend_host(struct dw_spi *dws)
spi_set_clk(dws, 0);
return ret;
}
-EXPORT_SYMBOL(dw_spi_suspend_host);
+EXPORT_SYMBOL_GPL(dw_spi_suspend_host);
int dw_spi_resume_host(struct dw_spi *dws)
{
@@ -990,7 +990,7 @@ int dw_spi_resume_host(struct dw_spi *dws)
dev_err(dws-master-dev, fail to start queue (%d)\n, ret);
return ret;
}
-EXPORT_SYMBOL(dw_spi_resume_host);
+EXPORT_SYMBOL_GPL(dw_spi_resume_host);
MODULE_AUTHOR(Feng Tang feng.t...@intel.com);
MODULE_DESCRIPTION(Driver for DesignWare SPI controller core);
--
1.7.0.4
--
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and,
should the need arise, upgrade to a full multi-node Oracle RAC database
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[Patch v2 1/3] spi/dw_spi: Fix too short timeout in spi polling loop
The SPI polling loop timeout only works with HZ=100 as the loop was
actually too short.
Also add appropriate cpu_relax() in the busy wait loops...
Acked-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Arjan van de Ven ar...@linux.intel.com
Signed-off-by: Alan Cox a...@linux.intel.com
---
drivers/spi/dw_spi.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 0838c79..25238a8 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -164,20 +164,23 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi
*dws)
static void wait_till_not_busy(struct dw_spi *dws)
{
- unsigned long end = jiffies + 1 + usecs_to_jiffies(1000);
+ unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
while (time_before(jiffies, end)) {
if (!(dw_readw(dws, sr) SR_BUSY))
return;
+ cpu_relax();
}
dev_err(dws-master-dev,
- DW SPI: Status keeps busy for 1000us after a read/write!\n);
+ DW SPI: Status keeps busy for 5000us after a read/write!\n);
}
static void flush(struct dw_spi *dws)
{
- while (dw_readw(dws, sr) SR_RF_NOT_EMPT)
+ while (dw_readw(dws, sr) SR_RF_NOT_EMPT) {
dw_readw(dws, dr);
+ cpu_relax();
+ }
wait_till_not_busy(dws);
}
--
1.7.0.4
--
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and,
should the need arise, upgrade to a full multi-node Oracle RAC database
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[Patch v2 3/3] spi/dw_spi: add DMA support
dw_spi driver in upstream only supports PIO mode, and this patch
will support it to cowork with the Designware dma controller used
on Intel Moorestown platform, at the same time it provides a general
framework to support dw_spi core to cowork with dma controllers on
other platforms
It has been tested with a Option GTM501L 3G modem and Infenion 60x60
modem. To use DMA mode, DMA controller 2 of Moorestown has to be enabled
Also change the dma interface suggested by Linus Walleij.
Acked-by: Linus Walleij linus.wall...@stericsson.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Feng Tang feng.t...@intel.com
[Typo fix and renames to match intel_mid_dma renaming]
Signed-off-by: Vinod Koul vinod.k...@intel.com
Signed-off-by: Alan Cox a...@linux.intel.com
---
drivers/spi/Kconfig|4 +
drivers/spi/Makefile |3 +-
drivers/spi/dw_spi.c | 33 ---
drivers/spi/dw_spi_mid.c | 224
drivers/spi/dw_spi_pci.c | 20 +++-
include/linux/spi/dw_spi.h | 24 -
6 files changed, 284 insertions(+), 24 deletions(-)
create mode 100644 drivers/spi/dw_spi_mid.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 78f9fd0..d53c830 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -396,6 +396,10 @@ config SPI_DW_PCI
tristate PCI interface driver for DW SPI core
depends on SPI_DESIGNWARE PCI
+config SPI_DW_MID_DMA
+ bool DMA support for DW SPI controller on Intel Moorestown platform
+ depends on SPI_DW_PCI INTEL_MID_DMAC
+
config SPI_DW_MMIO
tristate Memory-mapped io interface driver for DW SPI core
depends on SPI_DESIGNWARE HAVE_CLK
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 8bc1a5a..5e6e812 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -17,7 +17,8 @@ obj-$(CONFIG_SPI_BUTTERFLY) += spi_butterfly.o
obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= coldfire_qspi.o
obj-$(CONFIG_SPI_DAVINCI) += davinci_spi.o
obj-$(CONFIG_SPI_DESIGNWARE) += dw_spi.o
-obj-$(CONFIG_SPI_DW_PCI) += dw_spi_pci.o
+obj-$(CONFIG_SPI_DW_PCI) += dw_spi_midpci.o
+dw_spi_midpci-objs := dw_spi_pci.o dw_spi_mid.o
obj-$(CONFIG_SPI_DW_MMIO) += dw_spi_mmio.o
obj-$(CONFIG_SPI_EP93XX) += ep93xx_spi.o
obj-$(CONFIG_SPI_GPIO) += spi_gpio.o
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index b50bf5b..497ecb3 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -288,8 +288,10 @@ static void *next_transfer(struct dw_spi *dws)
*/
static int map_dma_buffers(struct dw_spi *dws)
{
- if (!dws-cur_msg-is_dma_mapped || !dws-dma_inited
- || !dws-cur_chip-enable_dma)
+ if (!dws-cur_msg-is_dma_mapped
+ || !dws-dma_inited
+ || !dws-cur_chip-enable_dma
+ || !dws-dma_ops)
return 0;
if (dws-cur_transfer-tx_dma)
@@ -341,7 +343,7 @@ static void int_error_stop(struct dw_spi *dws, const char
*msg)
tasklet_schedule(dws-pump_transfers);
}
-static void transfer_complete(struct dw_spi *dws)
+void dw_spi_xfer_done(struct dw_spi *dws)
{
/* Update total byte transfered return count actual bytes read */
dws-cur_msg-actual_length += dws-len;
@@ -356,6 +358,7 @@ static void transfer_complete(struct dw_spi *dws)
} else
tasklet_schedule(dws-pump_transfers);
}
+EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
static irqreturn_t interrupt_transfer(struct dw_spi *dws)
{
@@ -387,7 +390,7 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
if (dws-tx_end dws-tx)
spi_umask_intr(dws, SPI_INT_TXEI);
else
- transfer_complete(dws);
+ dw_spi_xfer_done(dws);
}
return IRQ_HANDLED;
@@ -422,11 +425,7 @@ static void poll_transfer(struct dw_spi *dws)
*/
dws-read(dws);
- transfer_complete(dws);
-}
-
-static void dma_transfer(struct dw_spi *dws, int cs_change)
-{
+ dw_spi_xfer_done(dws);
}
static void pump_transfers(unsigned long data)
@@ -608,7 +607,7 @@ static void pump_transfers(unsigned long data)
}
if (dws-dma_mapped)
- dma_transfer(dws, cs_change);
+ dws-dma_ops-dma_transfer(dws, cs_change);
if (chip-poll_mode)
poll_transfer(dws);
@@ -904,11 +903,17 @@ int __devinit dw_spi_add_host(struct dw_spi *dws)
master-setup = dw_spi_setup;
master-transfer = dw_spi_transfer;
- dws-dma_inited = 0;
-
/* Basic HW init */
spi_hw_init(dws);
+ if (dws-dma_ops dws-dma_ops-dma_init) {
+ ret = dws-dma_ops-dma_init(dws);
+ if (ret) {
+ dev_warn(master-dev, DMA init failed\n);
+ dws
Re: [PATCH] dw_spi: add DMA support
On Tue, 30 Nov 2010 23:13:22 +0800 Linus Walleij linus.wall...@stericsson.com wrote: 2010/11/23 Feng Tang feng.t...@intel.com: Thanks for the reviews, I made some dma change as you suggested, pls help to review. This is looking much better. I guess there is a corresponding patch to drivers/dma/* to provide the proper interfaces for this to work? Anyway, Acked-by: Linus Walleij linus.wall...@stericsson.com Yes, there will be some changed to the corresponding dma driver. Thanks again for the review - Feng Yours, Linus Walleij -- Increase Visibility of Your 3D Game App Earn a Chance To Win $500! Tap into the largest installed PC base get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] dw_spi: add DMA support
On Tue, 23 Nov 2010 14:48:39 +0800
Linus Walleij linus.ml.wall...@gmail.com wrote:
This is much better than last time but I still have questions...
2010/11/18 Alan Cox a...@lxorguk.ukuu.org.uk:
+ Â Â Â /* 1. Init rx channel */
+ Â Â Â rxs = dw_dma-dmas_rx;
+ Â Â Â ds = rxs-dma_slave;
+ Â Â Â ds-direction = DMA_FROM_DEVICE;
+ Â Â Â rxs-hs_mode = LNW_DMA_HW_HS;
+ Â Â Â rxs-cfg_mode = LNW_DMA_PER_TO_MEM;
+ Â Â Â ds-src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+ Â Â Â ds-dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ Â Â Â ds-src_maxburst = 16;
+ Â Â Â ds-dst_maxburst = 16;
This is great stuff! That is exactly how ds is to be set up.
I would prefer that you don't dereference the this rxs thing here
but whatever.
+ Â Â Â dma_cap_zero(mask);
+ Â Â Â dma_cap_set(DMA_MEMCPY, mask);
+ Â Â Â dma_cap_set(DMA_SLAVE, mask);
This is not elegant. Are you going to do memcpy() or slave
transfers? What you want to do is fix your DMA engine so that
just asking for DMA_SLAVE works.
+ Â Â Â dma_cap_set(DMA_SLAVE, mask);
+ Â Â Â dma_cap_set(DMA_MEMCPY, mask);
Here again...
+static int mid_spi_dma_transfer(struct dw_spi *dws, int cs_change)
+{
(...)
+ Â Â Â flag = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
+ Â Â Â if (dws-tx_dma) {
+ Â Â Â Â Â Â Â txdesc =
txchan-device-device_prep_dma_memcpy(txchan,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dws-dma_addr, dws-tx_dma,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dws-len, flag);
+ Â Â Â Â Â Â Â txdesc-callback = dw_spi_dma_done;
+ Â Â Â Â Â Â Â txdesc-callback_param = dws-tx_param;
+ Â Â Â }
+
+ Â Â Â /* 3. start the RX dma transfer */
+ Â Â Â if (dws-rx_dma) {
+ Â Â Â Â Â Â Â rxdesc =
rxchan-device-device_prep_dma_memcpy(rxchan,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dws-rx_dma, dws-dma_addr,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dws-len, flag);
+ Â Â Â Â Â Â Â rxdesc-callback = dw_spi_dma_done;
+ Â Â Â Â Â Â Â rxdesc-callback_param = dws-rx_param;
+ Â Â Â }
Using device_prep_dma_memcpy() for this is still nonsense, it should
be device_prep_slave_sg().
I know the DMA driver needs fixing in order for this to work properly,
so why not fix it?
These are the most important concerns I raised last iteration, so
I challenge you to fix drivers/dma/dw_dmac.c or wherever the real
problem sits.
Can you describe where the problem with fixing this to use real
slave sglists is?
Yours,
Linus Walleij
Hi Linus,
Thanks for the reviews, I made some dma change as you suggested, pls
help to review.
Thanks,
Feng
From b77efc5945e31442b53b285d6a3f77def376ebb3 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 23 Nov 2010 17:34:28 +0800
Subject: [PATCH] spi/dw_spi: add DMA support
dw_spi driver in upstream only supports PIO mode, and this patch
will support it to cowork with the Designware DMA controller used
on Intel Moorestown platform
It has been tested with a Option GTM501L 3G modem, to use DMA mode,
DMA controller 2 of Moorestown has to be enabled
Signed-off-by: Feng Tang feng.t...@intel.com
[Typo fix and renames to match intel_mid_dma renaming]
Signed-off-by: Vinod Koul vinod.k...@intel.com
[Clean up, change dma interface suggested by Linus Walleij]
Signed-off-by: Feng Tang feng.t...@intel.com
[Fix timing delay, add cpu_relax]
Signed-off-by: Arjan van de Ven ar...@linux.intel.com
Signed-off-by: Alan Cox a...@linux.intel.com
---
drivers/spi/Kconfig|4 +
drivers/spi/Makefile |3 +-
drivers/spi/dw_spi.c | 48 ++
drivers/spi/dw_spi_mid.c | 225
drivers/spi/dw_spi_pci.c | 14 ++-
include/linux/spi/dw_spi.h | 24 -
6 files changed, 288 insertions(+), 30 deletions(-)
create mode 100644 drivers/spi/dw_spi_mid.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 78f9fd0..d53c830 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -396,6 +396,10 @@ config SPI_DW_PCI
tristate PCI interface driver for DW SPI core
depends on SPI_DESIGNWARE PCI
+config SPI_DW_MID_DMA
+ bool DMA support for DW SPI controller on Intel Moorestown platform
+ depends on SPI_DW_PCI INTEL_MID_DMAC
+
config SPI_DW_MMIO
tristate Memory-mapped io interface driver for DW SPI core
depends on SPI_DESIGNWARE HAVE_CLK
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 8bc1a5a..5e6e812 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -17,7 +17,8 @@ obj-$(CONFIG_SPI_BUTTERFLY) += spi_butterfly.o
obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= coldfire_qspi.o
obj-$(CONFIG_SPI_DAVINCI) += davinci_spi.o
obj-$(CONFIG_SPI_DESIGNWARE) += dw_spi.o
-obj-$(CONFIG_SPI_DW_PCI) += dw_spi_pci.o
+obj-$(CONFIG_SPI_DW_PCI) += dw_spi_midpci.o
+dw_spi_midpci-objs := dw_spi_pci.o dw_spi_mid.o
obj-$(CONFIG_SPI_DW_MMIO
Re: spi/spi: don't release the spi device twice
Hi Sebastian,
On Tue, 23 Nov 2010 18:33:03 +0800
Sebastian Andrzej Siewior bige...@linutronix.de wrote:
David Lamparter wrote:
This code is the old code, before patch 3486008 which you're citing.
3486008 does:
- dummy = device_for_each_child(master-dev.parent,
master-dev,
- __unregister);
+ dummy = device_for_each_child(master-dev, NULL,
__unregister);
Considering that this patch is in 2.6.36 (and 36.1), you seem to
have mixed up your sources. Please make sure your checkout is
current and unbroken...
Hmmm.
# git describe --long
v2.6.37-rc3-0-g3561d43
After looking at spi_unregister_master() in drivers/spi/spi.c, I see:
dummy = device_for_each_child(master-dev.parent, master-dev,
__unregister);
device_unregister(master-dev);
}
This change got back in by:
commit 2b9603a0d7e395fb844af90fba71448bc8019077
Author: Feng Tang feng.t...@intel.com
Date: Mon Aug 2 15:52:15 2010 +0800
spi: enable spi_board_info to be registered after spi_master
which is v2.6.37-rc1~2^2~4. So I probably mixed up you with Feng.
This thread starts at
http://www.mail-archive.com/spi-devel-general@lists.sourceforge.net/msg05437.html
I checked my original patch which didn't touch the logic of
spi_unregister_master() as
-
@@ -568,6 +592,10 @@ void spi_unregister_master(struct spi_master *master)
{
int dummy;
+ mutex_lock(board_lock);
+ list_del(master-list);
+ mutex_unlock(board_lock);
+
dummy = device_for_each_child(master-dev.parent, master-dev,
__unregister);
device_unregister(master-dev);
-
So this should be a merge problem, which corrupt the commit from David's commit
3486008 spi: free children in spi_unregister_master, not siblings
Thanks,
Feng
--
Increase Visibility of Your 3D Game App Earn a Chance To Win $500!
Tap into the largest installed PC base get more eyes on your game by
optimizing for Intel(R) Graphics Technology. Get started today with the
Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v2] spi/spi: don't release the spi device twice
On Wed, 24 Nov 2010 00:45:20 +0800 David Lamparter equi...@diac24.net wrote: This was fixed by David Lamparter in v2.6.36-rc5 3486008 (spi: free children in spi_unregister_master, not siblings) and broken again in v2.6.37-rc1~2^2~4 during the merge of 2b9603a0 (spi: enable spi_board_info to be registered after spi_master). Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- - dummy = device_for_each_child(master-dev.parent, master-dev, - __unregister); + dummy = device_for_each_child(master-dev, NULL, __unregister); Signed-off-by: David Lamparter equi...@diac24.net very simple merge/rebase/forward-port breakage... Feng, can you check 2b9603a0 for whether anything else got broken? I just checked, all other parts should be ok. Thanks, Feng -- Increase Visibility of Your 3D Game App Earn a Chance To Win $500! Tap into the largest installed PC base get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: add a bits_per_word to struct spi_board_info
Hi Grant, On Thu, 9 Sep 2010 04:21:59 +0800 Grant Likely grant.lik...@secretlab.ca wrote: Yes, driver itself know the bits_per_word info best in most case, but there is some device (Option GTM501L spi modem) which supports multiple bits_per_mode option, and here platform code is the good place to set it. Not unless it knows which mode the driver uses... a kind of layering violation. spi_board_info has a member of mode, so for my platform, I prefer to set all these infos in platform code, while protocol driver can check if the preset infos like speed_hz/mode/bits_per_word are supported. The real problem is that spi_drivers are normally allowed to change bits_per_word as needed. For drivers who don't accept bits_per_word comes from platform code, it could just ignores it and set the value it wants. If the spi_driver gets unbound and rebound, then the original platform-set value for bits_per_word is lost and the behaviour changes. Um, if you are talking about driver load/unload, then the bits_per_word info won't get lost as it is set to spi_device from spi_board_info when spi_new_device get called. If you really want to give the driver hints about the best setting for bits_per_word or similar, then you should probably use a driver-specific pdata structure. Good point, really a solution to the GTM501L modem driver. But still, adding the bits_per_word to spi_board_info will be more generic when more devices which support multiple bits models show up. Thanks, Feng -- This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: add a bits_per_word to struct spi_board_info
On Thu, 9 Sep 2010 11:48:22 +0800 David Brownell davi...@pacbell.net wrote: You know, in retrospect, I shouldn't have put most of those SPI device setup params into the board setup data. There's one which MUST be there: polarity of the chip select line. The rest seem like they could (and arguably should) all be handled by driver-specific params. (Possible exception: clock rate, which sometimes matters even when chips are not selected). At any rate, adding MORE driver-specific params (like bits-per-word) to board setup data seems like the wrong direction to go.. Agree. Thank you two for helping me get an overall picture of configuring these parameters. -Feng -- This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH v2] spi/dw_spi: clean the cs_control code
commit 052dc7c45i spi/dw_spi: conditional transfer mode change
introduced cs_control code, which has a bug by using bit offset
for spi mode to set transfer mode in control register. Also it
forces devices who don't need cs_control to re-configure the
control registers for each spi transfer. This patch will fix them
Signed-off-by: Feng Tang feng.t...@intel.com
Cc: David Brownell dbrown...@users.sourceforge.net
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: George Shore geo...@georgeshore.com
---
drivers/spi/dw_spi.c | 17 +
include/linux/spi/dw_spi.h |2 ++
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index d256cb0..4b75a81 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -181,10 +181,6 @@ static void flush(struct dw_spi *dws)
wait_till_not_busy(dws);
}
-static void null_cs_control(u32 command)
-{
-}
-
static int null_writer(struct dw_spi *dws)
{
u8 n_bytes = dws-n_bytes;
@@ -322,7 +318,7 @@ static void giveback(struct dw_spi *dws)
struct spi_transfer,
transfer_list);
- if (!last_transfer-cs_change)
+ if (!last_transfer-cs_change dws-cs_control)
dws-cs_control(MRST_SPI_DEASSERT);
msg-state = NULL;
@@ -544,13 +540,13 @@ static void pump_transfers(unsigned long data)
*/
if (dws-cs_control) {
if (dws-rx dws-tx)
- chip-tmode = 0x00;
+ chip-tmode = SPI_TMOD_TR;
else if (dws-rx)
- chip-tmode = 0x02;
+ chip-tmode = SPI_TMOD_RO;
else
- chip-tmode = 0x01;
+ chip-tmode = SPI_TMOD_TO;
- cr0 = ~(0x3 SPI_MODE_OFFSET);
+ cr0 = ~SPI_TMOD_MASK;
cr0 |= (chip-tmode SPI_TMOD_OFFSET);
}
@@ -699,9 +695,6 @@ static int dw_spi_setup(struct spi_device *spi)
chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
if (!chip)
return -ENOMEM;
-
- chip-cs_control = null_cs_control;
- chip-enable_dma = 0;
}
/*
diff --git a/include/linux/spi/dw_spi.h b/include/linux/spi/dw_spi.h
index cc813f9..c91302f 100644
--- a/include/linux/spi/dw_spi.h
+++ b/include/linux/spi/dw_spi.h
@@ -14,7 +14,9 @@
#define SPI_MODE_OFFSET6
#define SPI_SCPH_OFFSET6
#define SPI_SCOL_OFFSET7
+
#define SPI_TMOD_OFFSET8
+#define SPI_TMOD_MASK (0x3 SPI_TMOD_OFFSET)
#defineSPI_TMOD_TR 0x0 /* xmit recv
*/
#define SPI_TMOD_TO0x1 /* xmit only */
#define SPI_TMOD_RO0x2 /* recv only */
--
1.7.0.4
--
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: add a bits_per_word to struct spi_board_info
On Wed, 1 Sep 2010 23:46:18 +0800 David Brownell davi...@pacbell.net wrote: Re $SUBJECT ... consider it a quirk of how SPI devices get setup that this field isn't one of the ones the platform code can set up. I've thought about fixing this before, and decided against it: What is the use case that you would want to do this. Knowing what the bits_per_word should be is the responsibility of the spi device driver. What is the use-case where the driver wouldn't know what bits_per_word should be? What's the case where platform code could know better than the driver? Grant also asked me the same question. Yes, driver itself know the bits_per_word info best in most case, but there is some device (Option GTM501L spi modem) which supports multiple bits_per_mode option, and here platform code is the good place to set it. One of my intension of this patch is to save some extra spi_setup. For many spi protocol drivers in kernel, spi_setup() will be called twice for them, first time is inside spi_new_device(it calls spi_add_device), the second time will be inside driver's probe func, after setting bits_per_word info. So if we can set it before registering board info, then the second spi_setup can be removed for many drivers. Thanks, Feng -- This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/dw_spi: clean the cs_control code
On Wed, 1 Sep 2010 23:07:57 +0800
Grant Likely grant.lik...@secretlab.ca wrote:
Hi Feng,
On Wed, Sep 01, 2010 at 01:35:59PM +0800, Feng Tang wrote:
commit 052dc7c45 introduced cs_control code,
Be friendly to mere-mortals. You should quote the patch title
spi/dw_spi: conditional transfer mode changes in addition to the
sha1 id.
Got it, thanks, will add it in V2
@@ -544,13 +540,13 @@ static void pump_transfers(unsigned long data)
*/
if (dws-cs_control) {
if (dws-rx dws-tx)
- chip-tmode = 0x00;
+ chip-tmode = SPI_TMOD_TR;
else if (dws-rx)
- chip-tmode = 0x02;
+ chip-tmode = SPI_TMOD_RO;
else
- chip-tmode = 0x01;
+ chip-tmode = SPI_TMOD_TO;
- cr0 = ~(0x3 SPI_MODE_OFFSET);
+ cr0 = ~SPI_TMOD_MASK;
cr0 |= (chip-tmode SPI_TMOD_OFFSET);
Changing these values isn't mentioned in the patch description. I
assume this is not the bug fix because the #defines are the same
values.
OK, it's a little confusing, there are 2 OFFSET here:
SPI_MODE_OFFSET (for spi mode 0/1/2 etc setting)
SPI_TMOD_OFFSET (for tx only/rx only/duplex)
So simple fix should be
- cr0 = ~(0x3 SPI_MODE_OFFSET);
+ cr0 = ~(0x3 SPI_TMOD_OFFSET);
while my new introduced SPI_TMOD_MASK make things complex :)
Thanks,
Feng
--
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: add a bits_per_word to struct spi_board_info
Hi David,
Grant also asked me the same question.
I saw him ask a different question (which
I quoted) ...
My mistake, I tried to answer Grant's and your questions in one email to
keep the discussion in one thread.
Yes, driver itself
know the bits_per_word info best in most case, but there
is
some device (Option GTM501L spi modem) which supports
multiple
bits_per_mode option, and here platform code
is the good place to set it.
Not unless it knows which mode the driver uses...
a kind of layering violation.
spi_board_info has a member of mode, so for my platform, I prefer to
set all these infos in platform code, while protocol driver can
check if the preset infos like speed_hz/mode/bits_per_word are supported.
I thought platform code have to know the detail of the spi device
when setting spi_board_info for it.
Also the following code quoted from spi_setup() indicates it supports
setting mode for spi_board_info
bad_bits = spi-mode ~spi-master-mode_bits;
if (bad_bits) {
dev_dbg(spi-dev, setup: unsupported mode bits %x\n,
bad_bits);
return -EINVAL;
}
Thanks,
Feng
--
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/dw_spi: Enable Intel Moorestown SPI controller 1
NAK.
We should not export SPI1 has x86 core has no way to access its register.
though the device physically exists.
Thanks,
Feng
On Tue, 31 Aug 2010 14:14:16 +0800
Yong Wang yong.y.w...@linux.intel.com wrote:
Enable Intel Moorestown SPI controller 1
Signed-off-by: Yong Wang yong.y.w...@intel.com
---
drivers/spi/dw_spi_pci.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/spi/dw_spi_pci.c b/drivers/spi/dw_spi_pci.c
index 1f52755..eec9d0b 100644
--- a/drivers/spi/dw_spi_pci.c
+++ b/drivers/spi/dw_spi_pci.c
@@ -142,6 +142,8 @@ static int spi_resume(struct pci_dev *pdev)
static const struct pci_device_id pci_ids[] __devinitdata = {
/* Intel Moorestown platform SPI controller 0 */
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0800) },
+ /* Intel Moorestown platform SPI controller 1 */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0801) },
{},
};
--
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: controller_data handling through spidev.c
It's a sibling to the platform_data field, and should be set up with that. Neither one should be assigned or understood by protocol drivers. Historically, controller_data was used ISTR to cope with tuning that the PXA SPI controller required (DMA etc). Most controllers should have no need for it. DW spi driver is also using the controller data, but as you said protocol driver itself doesn't need set it. Platform code where that spi device is registered (by spi_register_board_info) should set up the controller data. Thanks, Feng -- This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH] spi: add a bits_per_word to struct spi_board_info
Currently a spi device's bits_per_word can only be inited
inside protocol driver's probe func, this patch will enable
platform code to specify this info when registering the
spi_board_info
Signed-off-by: Feng Tang feng.t...@intel.com
Cc: David Brownell dbrown...@users.sourceforge.net
Cc: Grant Likely grant.lik...@secretlab.ca
---
drivers/spi/spi.c |1 +
include/linux/spi/spi.h |3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a9e5c79..a8690e7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -350,6 +350,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
proxy-chip_select = chip-chip_select;
proxy-max_speed_hz = chip-max_speed_hz;
proxy-mode = chip-mode;
+ proxy-bits_per_word = chip-bits_per_word;
proxy-irq = chip-irq;
strlcpy(proxy-modalias, chip-modalias, sizeof(proxy-modalias));
proxy-dev.platform_data = (void *) chip-platform_data;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 92e52a1..f36d0d5 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -698,6 +698,8 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8
cmd)
* @mode: Initializes spi_device.mode; based on the chip datasheet, board
* wiring (some devices support both 3WIRE and standard modes), and
* possibly presence of an inverter in the chipselect path.
+ * @bits_per_word: Initializes spi_device.bits_per_word, usually it will
+ * be from 8 to 32 bits
*
* When adding new SPI devices to the device tree, these structures serve
* as a partial device template. They hold information which can't always
@@ -742,6 +744,7 @@ struct spi_board_info {
* where the default of SPI_CS_HIGH = 0 is wrong.
*/
u8 mode;
+ u8 bits_per_word;
/* ... may need additional spi_device chip config data here.
* avoid stuff protocol drivers can set; but include stuff
--
1.7.0.4
--
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
___
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 v3] spi: enable spi_board_info to be registered after spi_master
On Mon, 2 Aug 2010 13:53:25 +0800
Grant Likely grant.lik...@secretlab.ca wrote:
 /**
@@ -365,6 +370,24 @@ struct spi_device *spi_new_device(struct
spi_master *master, }
 EXPORT_SYMBOL_GPL(spi_new_device);
+/* Has to be called when board_lock is acquired */
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+ Â Â Â struct spi_master *master;
+ Â Â Â struct spi_device *dev;
+
+ Â Â Â list_for_each_entry(master, spi_master_list, list) {
+ Â Â Â Â Â Â Â if (master-bus_num != bi-bus_num)
+ Â Â Â Â Â Â Â Â Â Â Â continue;
+
+ Â Â Â Â Â Â Â dev = spi_new_device(master, bi);
+ Â Â Â Â Â Â Â if (!dev)
+ Â Â Â Â Â Â Â Â Â Â Â dev_err(master-dev.parent,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â can't create new device for %s\n,
+ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â bi-modalias);
+ Â Â Â }
+}
+
The abstraction isn't quite at the right place because the same code
block is now duplicated twice. Consider the following instead:
static void spi_match_master_to_boardinfo(struct spi_master *master,
struct spi_board_info *bi)
{
struct spi_device *dev;
if (master-bus_num != bi-bus_num)
return;
dev = spi_new_device(master, bi);
if (!dev)
dev_err(master-dev.parent, can't create new device
for %s\n, bi-modalias);
}
And then from spi_register_board_info, do:
list_for_each_entry(master, spi_master_list, list)
spi_match_master_to_boardinfo(master, bi-boardinfo);
And in spi_register_master, do:
list_for_each_entry(bi, board_list, list)
spi_match_master_to_boardinfo(master, bi-boardinfo);
It will be less code that way.
Otherwise, this patch looks good to me. Unfortunately, it's too late
for 2.6.36, but I'll pick it up into my test branch in prep for the
2.6.37 cycle.
Cheers,
g.
Yeah, make much sense! Thanks for the thorough reviews.
- Feng
--
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://p.sf.net/sfu/dev2dev-palm
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH v4] spi: enable spi_board_info to be registered after spi_master
Hi All,
Pls review this patch. thanks,
-Feng
Changelog:
v4:
* Abstract a spi_match_master_to_boardinfo() which will
be called in scanning master/boardinfo list, suggested
by Grant also.
v3:
* As suggested by Grant, use one lock to protect the match
between spi_master and spi board info
v2:
* fix a typo in looping scan_masterlist()
* fix one race condition found by Grant
From 799ff94edc90117afdbba223fb35cb36a1d2b4b4 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 27 Jul 2010 17:15:05 +0800
Subject: [PATCH] spi: enable spi_board_info to be registered after spi_master
Currently spi_register_board_info() has to be called before its related
spi_master be registered, otherwise these board info will be just ignored.
This patch will remove this order limit, it adds a global spi master list
like the existing global board info listr. Whenever a board info or a
spi_master is registered, the spi master list or board info list
will be scanned, and a new spi device will be created if there is a
master-board info match.
Cc: David Brownell dbrown...@users.sourceforge.net
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/spi.c | 86 +++
include/linux/spi/spi.h |3 ++
2 files changed, 60 insertions(+), 29 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b3a1f92..70fb141 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -196,11 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
struct boardinfo {
struct list_headlist;
- unsignedn_board_info;
- struct spi_board_info board_info[0];
+ struct spi_board_info board_info;
};
static LIST_HEAD(board_list);
+static LIST_HEAD(spi_master_list);
+
+/*
+ * Used to protect add/del opertion for board_info list and
+ * spi_master list, and their matching process
+ */
static DEFINE_MUTEX(board_lock);
/**
@@ -365,6 +370,30 @@ struct spi_device *spi_new_device(struct spi_master
*master,
}
EXPORT_SYMBOL_GPL(spi_new_device);
+static void spi_match_master_to_boardinfo(struct spi_master *master,
+ struct spi_board_info *bi)
+{
+ struct spi_device *dev;
+
+ if (master-bus_num != bi-bus_num)
+ return;
+
+ dev = spi_new_device(master, bi);
+ if (!dev)
+ dev_err(master-dev.parent, can't create new device for %s\n,
+ bi-modalias);
+}
+
+
+/* Has to be called when board_lock is acquired */
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+ struct spi_master *master;
+
+ list_for_each_entry(master, spi_master_list, list)
+ spi_match_master_to_boardinfo(master, bi);
+}
+
/**
* spi_register_board_info - register SPI devices for a given board
* @info: array of chip descriptors
@@ -387,43 +416,36 @@ EXPORT_SYMBOL_GPL(spi_new_device);
int __init
spi_register_board_info(struct spi_board_info const *info, unsigned n)
{
- struct boardinfo*bi;
+ struct boardinfo *bi;
+ int i;
- bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+ bi = kzalloc(n * sizeof(*bi), GFP_KERNEL);
if (!bi)
return -ENOMEM;
- bi-n_board_info = n;
- memcpy(bi-board_info, info, n * sizeof *info);
- mutex_lock(board_lock);
- list_add_tail(bi-list, board_list);
- mutex_unlock(board_lock);
+ for (i = 0; i n; i++, bi++, info++) {
+ memcpy(bi-board_info, info, sizeof(*info));
+ mutex_lock(board_lock);
+ list_add_tail(bi-list, board_list);
+ spi_scan_masterlist(bi-board_info);
+ mutex_unlock(board_lock);
+ }
+
return 0;
}
-/* FIXME someone should add support for a __setup(spi, ...) that
+/*
+ * FIXME someone should add support for a __setup(spi, ...) that
* creates board info from kernel command lines
+ *
+ * Should be called when board_lock is acquired
*/
-
static void scan_boardinfo(struct spi_master *master)
{
- struct boardinfo*bi;
+ struct boardinfo *bi;
- mutex_lock(board_lock);
- list_for_each_entry(bi, board_list, list) {
- struct spi_board_info *chip = bi-board_info;
- unsignedn;
-
- for (n = bi-n_board_info; n 0; n--, chip++) {
- if (chip-bus_num != master-bus_num)
- continue;
- /* NOTE: this relies on spi_new_device to
-* issue diagnostics when given bogus inputs
-*/
- (void) spi_new_device(master, chip);
- }
- }
- mutex_unlock(board_lock);
+ list_for_each_entry(bi, board_list
[spi-devel-general] [PATCH v3] spi: enable spi_board_info to be registered after spi_master
Hi All,
Pls review this patch. thanks,
-Feng
Changelog:
v3:
* As suggested by Grant, use one lock to protect the match
between spi_master and spi board info
v2:
* fix a typo in looping scan_masterlist()
* fix one race condition found by Grant
From 28af337ec89225a0dfb7ca516545b7eff112e3b2 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 27 Jul 2010 17:15:05 +0800
Subject: [PATCH] spi: enable spi_board_info to be registered after spi_master
Currently spi_register_board_info() has to be called before its related
spi_master be registered, otherwise these board info will be just ignored.
This patch will remove this order limit, it adds a global spi master list
like the existing global board info listr. Whenever a board info or a
spi_master is registered, the spi master list or board info list
will be scanned, and a new spi device will be created if there is a
master-board info match.
Cc: David Brownell dbrown...@users.sourceforge.net
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/spi.c | 84 --
include/linux/spi/spi.h |3 ++
2 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b3a1f92..8fb2337 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -196,11 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
struct boardinfo {
struct list_headlist;
- unsignedn_board_info;
- struct spi_board_info board_info[0];
+ struct spi_board_info board_info;
};
static LIST_HEAD(board_list);
+static LIST_HEAD(spi_master_list);
+
+/*
+ * Used to protect add/del opertion for board_info list and
+ * spi_master list, and their matching process
+ */
static DEFINE_MUTEX(board_lock);
/**
@@ -365,6 +370,24 @@ struct spi_device *spi_new_device(struct spi_master
*master,
}
EXPORT_SYMBOL_GPL(spi_new_device);
+/* Has to be called when board_lock is acquired */
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+ struct spi_master *master;
+ struct spi_device *dev;
+
+ list_for_each_entry(master, spi_master_list, list) {
+ if (master-bus_num != bi-bus_num)
+ continue;
+
+ dev = spi_new_device(master, bi);
+ if (!dev)
+ dev_err(master-dev.parent,
+ can't create new device for %s\n,
+ bi-modalias);
+ }
+}
+
/**
* spi_register_board_info - register SPI devices for a given board
* @info: array of chip descriptors
@@ -387,43 +410,46 @@ EXPORT_SYMBOL_GPL(spi_new_device);
int __init
spi_register_board_info(struct spi_board_info const *info, unsigned n)
{
- struct boardinfo*bi;
+ struct boardinfo *bi;
+ int i;
- bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+ bi = kzalloc(n * sizeof(*bi), GFP_KERNEL);
if (!bi)
return -ENOMEM;
- bi-n_board_info = n;
- memcpy(bi-board_info, info, n * sizeof *info);
- mutex_lock(board_lock);
- list_add_tail(bi-list, board_list);
- mutex_unlock(board_lock);
+ for (i = 0; i n; i++, bi++, info++) {
+ memcpy(bi-board_info, info, sizeof(*info));
+ mutex_lock(board_lock);
+ list_add_tail(bi-list, board_list);
+ spi_scan_masterlist(bi-board_info);
+ mutex_unlock(board_lock);
+ }
+
return 0;
}
-/* FIXME someone should add support for a __setup(spi, ...) that
+/*
+ * FIXME someone should add support for a __setup(spi, ...) that
* creates board info from kernel command lines
+ *
+ * Should be called when board_lock is acquired
*/
-
static void scan_boardinfo(struct spi_master *master)
{
struct boardinfo*bi;
+ struct spi_device *dev;
- mutex_lock(board_lock);
list_for_each_entry(bi, board_list, list) {
- struct spi_board_info *chip = bi-board_info;
- unsignedn;
-
- for (n = bi-n_board_info; n 0; n--, chip++) {
- if (chip-bus_num != master-bus_num)
- continue;
- /* NOTE: this relies on spi_new_device to
-* issue diagnostics when given bogus inputs
-*/
- (void) spi_new_device(master, chip);
- }
+ struct spi_board_info *chip = bi-board_info;
+
+ if (chip-bus_num != master-bus_num)
+ continue;
+ dev = spi_new_device(master, chip);
+ if (!dev)
+ dev_err(master-dev.parent,
+ can't create new device for %s\n
Re: [spi-devel-general] [PATCH] spi/mpc5121: register spi child devices of spi node
On Tue, 27 Jul 2010 14:28:57 +0800 David Brownell davi...@pacbell.net wrote: There are things wrong with the concept of this particular patch. First, that it's mpc5121-only. Second, that it's not already handled as part of registering the platform's SPI devices. Hi David, Grant, This actually brings up another topic: currently spi_register_board_info() has to be called before spi controller(master) get inited. But in some case, to solve some device dependency issue, some spi slave device need be delayed registering, can we adjust it to make register_board_info() be callable after spi controller is inited? Which makes spi bus like general pci/usb bus, where devices/drivers have no registering order limit. Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ 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] spi/mpc5121: register spi child devices of spi node
On Tue, 27 Jul 2010 15:00:29 +0800 Grant Likely grant.lik...@secretlab.ca wrote: On Tue, Jul 27, 2010 at 1:14 AM, Feng Tang feng.t...@intel.com wrote: On Tue, 27 Jul 2010 14:28:57 +0800 David Brownell davi...@pacbell.net wrote: There are things wrong with the concept of this particular patch. Â First, that it's mpc5121-only. Second, that it's not already handled as part of registering the platform's SPI devices. Hi David, Grant, This actually brings up another topic: currently spi_register_board_info() has to be called before spi controller(master) get inited. But in some case, to solve some device dependency issue, some spi slave device need be delayed registering, can we adjust it to make register_board_info() be callable after spi controller is inited? Which makes spi bus like general pci/usb bus, where devices/drivers have no registering order limit. Is spi_alloc_device()/spi_add_device() not suitable for your use-case? g. Yes, I've thought about that, but spi_alloc_device() is called when you know which spi_master it is connecting to, while our platform need kind of blind registering (just register something to spi-bus). Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ 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] spi/mpc5121: register spi child devices of spi node
On Tue, 27 Jul 2010 15:07:28 +0800 David Brownell davi...@pacbell.net wrote: --- On Tue, 7/27/10, Feng Tang feng.t...@intel.com wrote: be delayed registering, can we adjust it to make register_board_info() be callable after spi controller is inited? Which makes spi bus like general pci/usb bus, where devices/drivers have no registering order limit. They require parents to be registered before children. In particular: bus segments before devices on them. SPI is no different (nor should it be). Sorry, I made a wrong example. What I want to say is the matching mechanism between spi board info and spi master, currently they use bus number to match, but there is a ordering limit now. For general device/driver matching in linux device model, there is no such limit. Is it possible to remove the order limit? Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ 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] spi/mpc5121: register spi child devices of spi node
On Tue, 27 Jul 2010 15:27:50 +0800 David Brownell davi...@pacbell.net wrote: --- On Tue, 7/27/10, Feng Tang feng.t...@intel.com wrote: What I want to say is the matching mechanism between spi board info and spi master, currently they use bus number to match, but there is a ordering limit now. For general device/driver matching in linux device model, there is no such limit. Is it possible to remove the order limit? I have no idea what you mean. Are you proposing to change how SPI busses are identified? Lots of busses are numbered, SPI isn't unique; a device on PCI bus #2 is different from one on bus #1, etc. What do you mean by order limit In current spi core, spi_register_info() has to be called before the spi controller driver call spi_register_master(), then these board info will be matched and used to create a new spi device. But if spi_register_info() are called after its related spi master is registered, then these info will be ignored and never get to create a new spi device. This is the order limit I want to describe :) Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ 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] spi/mpc5121: register spi child devices of spi node
On Tue, 27 Jul 2010 15:44:48 +0800 David Brownell davi...@pacbell.net wrote: --- On Tue, 7/27/10, Feng Tang feng.t...@intel.com wrote: In current spi core, spi_register_info() has to be called before the spi controller driver call spi_register_master(), then these board info will be matched and used to create a new spi device. But if spi_register_info() are called after its related spi master is registered, then these info will be ignored and never get to create a new spi device. That doesn't seem necessary. Got patch? I would first discuss it on mail list and get bless from you two. I will try to make one patch. Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH] spi: enable spi_board_info to be registered after spi_master
Currently spi_register_board_info() has to be called before its related
spi_master be registered, otherwise these board info will be just ignored.
This patch will remove this order limit, it adds a global spi master list
like the existing global board info listr. Whenever a board info is
registered, the spi master list will be scanned, and a new spi device will
be created if they match.
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/spi.c | 66 +-
include/linux/spi/spi.h |3 ++
2 files changed, 50 insertions(+), 19 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b3a1f92..f4a5221 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -196,13 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
struct boardinfo {
struct list_headlist;
- unsignedn_board_info;
- struct spi_board_info board_info[0];
+ struct spi_board_info board_info;
};
static LIST_HEAD(board_list);
static DEFINE_MUTEX(board_lock);
+
+static LIST_HEAD(master_list);
+static DEFINE_MUTEX(master_lock);
+
/**
* spi_alloc_device - Allocate a new SPI device
* @master: Controller to which device is connected
@@ -365,6 +368,19 @@ struct spi_device *spi_new_device(struct spi_master
*master,
}
EXPORT_SYMBOL_GPL(spi_new_device);
+static void scan_masterlist(struct spi_board_info *bi)
+{
+ struct spi_master *master;
+
+ mutex_lock(master_lock);
+ list_for_each_entry(master, master_list, list) {
+ if (master-bus_num != bi-bus_num)
+ continue;
+ (void) spi_new_device(master, bi);
+ }
+ mutex_unlock(master_lock);
+}
+
/**
* spi_register_board_info - register SPI devices for a given board
* @info: array of chip descriptors
@@ -387,41 +403,45 @@ EXPORT_SYMBOL_GPL(spi_new_device);
int __init
spi_register_board_info(struct spi_board_info const *info, unsigned n)
{
- struct boardinfo*bi;
+ struct boardinfo*bi, *tmp_bi;
+ int i;
- bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+ bi = kzalloc(n * sizeof *bi, GFP_KERNEL);
if (!bi)
return -ENOMEM;
- bi-n_board_info = n;
- memcpy(bi-board_info, info, n * sizeof *info);
+ tmp_bi = bi;
mutex_lock(board_lock);
- list_add_tail(bi-list, board_list);
+ for (i = 0; i n; i++, bi++, info++) {
+ memcpy(bi-board_info, info, sizeof *info);
+ list_add_tail(bi-list, board_list);
+ }
mutex_unlock(board_lock);
+
+ bi = tmp_bi;
+ for (i = 0; i n; i++)
+ scan_masterlist(bi-board_info);
+
return 0;
}
/* FIXME someone should add support for a __setup(spi, ...) that
* creates board info from kernel command lines
*/
-
static void scan_boardinfo(struct spi_master *master)
{
struct boardinfo*bi;
mutex_lock(board_lock);
list_for_each_entry(bi, board_list, list) {
- struct spi_board_info *chip = bi-board_info;
- unsignedn;
-
- for (n = bi-n_board_info; n 0; n--, chip++) {
- if (chip-bus_num != master-bus_num)
- continue;
- /* NOTE: this relies on spi_new_device to
-* issue diagnostics when given bogus inputs
-*/
- (void) spi_new_device(master, chip);
- }
+ struct spi_board_info *chip = bi-board_info;
+
+ if (chip-bus_num != master-bus_num)
+ continue;
+ /* NOTE: this relies on spi_new_device to
+* issue diagnostics when given bogus inputs
+*/
+ (void) spi_new_device(master, chip);
}
mutex_unlock(board_lock);
}
@@ -537,6 +557,10 @@ int spi_register_master(struct spi_master *master)
dev_dbg(dev, registered master %s%s\n, dev_name(master-dev),
dynamic ? (dynamic) : );
+ mutex_lock(master_lock);
+ list_add_tail(master-list, master_list);
+ mutex_unlock(master_lock);
+
/* populate children from any spi device tables */
scan_boardinfo(master);
status = 0;
@@ -568,6 +592,10 @@ void spi_unregister_master(struct spi_master *master)
{
int dummy;
+ mutex_lock(master_lock);
+ list_del(master-list);
+ mutex_unlock(master_lock);
+
dummy = device_for_each_child(master-dev.parent, master-dev,
__unregister);
device_unregister(master-dev);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index af56071..f4a29b6 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -204,6 +204,7 @@ static inline void spi_unregister_driver(struct
Re: [spi-devel-general] [PATCH] spi: enable spi_board_info to be registered after spi_master
Hi Grant, Thanks for your thorough reviews! Will address most of the comments. On Wed, 28 Jul 2010 01:27:16 +0800 Grant Likely grant.lik...@secretlab.ca wrote: for (i = 0, tmp_bi = bi; i n; i++, tmp_bi++, info++) + Â Â Â Â Â Â Â memcpy(bi-board_info, info, sizeof *info); + Â Â Â Â Â Â Â list_add_tail(bi-list, board_list); + Â Â Â } Â Â Â Â mutex_unlock(board_lock); + + Â Â Â bi = tmp_bi; + Â Â Â for (i = 0; i n; i++) + Â Â Â Â Â Â Â scan_masterlist(bi-board_info); How does this work? The same board_info struct is passed to scan_masterlist() multiple times. My bad, I tested 2 cases by calling register_board_info before and after spi_register_master(), but only with one spi device. Will fix it @@ -537,6 +557,10 @@ int spi_register_master(struct spi_master *master) dev_dbg(dev, registered master %s%s\n, dev_name(master-dev), dynamic ? (dynamic) : ); + Â Â Â mutex_lock(master_lock); + Â Â Â list_add_tail(master-list, master_list); + Â Â Â mutex_unlock(master_lock); + There seems to be a race condition here. If a boardinfo is registered after this list add, but before scan_boardinfo(master), then a boardinfo can get parsed twice for a single master. I think the proper thing to do is to hold the mutex over the whole operation; from registering the list or master, until after the scan is complete. Good point. Maybe moving the list_add(master) after scan_boardinfo() can fix this race condition. Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ 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] spi: enable spi_board_info to be registered after spi_master
On Wed, 28 Jul 2010 09:23:06 +0800 Feng Tang feng.t...@intel.com wrote: @@ -537,6 +557,10 @@ int spi_register_master(struct spi_master *master) dev_dbg(dev, registered master %s%s\n, dev_name(master-dev), dynamic ? (dynamic) : ); + Â Â Â mutex_lock(master_lock); + Â Â Â list_add_tail(master-list, master_list); + Â Â Â mutex_unlock(master_lock); + There seems to be a race condition here. If a boardinfo is registered after this list add, but before scan_boardinfo(master), then a boardinfo can get parsed twice for a single master. I think the proper thing to do is to hold the mutex over the whole operation; from registering the list or master, until after the scan is complete. Good point. Maybe moving the list_add(master) after scan_boardinfo() can fix this race condition. My solution still has problem, it was: scan_boardinfo(master); mutex_lock(spi_master_lock); list_add_tail(master-list, spi_master_list); mutex_unlock(spi_master_lock); If another boardinfo registering just after scan_boardinfo() but before list_add(), it will still get ignored. I will just follow Grant's suggestion. Thanks, Feng -- The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH v2] spi: enable spi_board_info to be registered after spi_master
Currently spi_register_board_info() has to be called before its related
spi_master be registered, otherwise these board info will be just ignored.
This patch will remove this order limit, it adds a global spi master list
like the existing global board info listr. Whenever a board info is
registered, the spi master list will be scanned, and a new spi device will
be created if master and boardinfo match.
Cc: David Brownell dbrown...@users.sourceforge.net
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/spi.c | 73 ++-
include/linux/spi/spi.h |3 ++
2 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b3a1f92..1266fb2 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -196,13 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
struct boardinfo {
struct list_headlist;
- unsignedn_board_info;
- struct spi_board_info board_info[0];
+ struct spi_board_info board_info;
};
static LIST_HEAD(board_list);
static DEFINE_MUTEX(board_lock);
+
+static LIST_HEAD(spi_master_list);
+static DEFINE_MUTEX(spi_master_lock);
+
/**
* spi_alloc_device - Allocate a new SPI device
* @master: Controller to which device is connected
@@ -365,6 +368,25 @@ struct spi_device *spi_new_device(struct spi_master
*master,
}
EXPORT_SYMBOL_GPL(spi_new_device);
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+ struct spi_master *master;
+ struct spi_device *dev;
+
+ mutex_lock(spi_master_lock);
+ list_for_each_entry(master, spi_master_list, list) {
+ if (master-bus_num != bi-bus_num)
+ continue;
+
+ dev = spi_new_device(master, bi);
+ if (!dev)
+ dev_err(master-dev.parent,
+ can't create new device for %s\n,
+ bi-modalias);
+ }
+ mutex_unlock(spi_master_lock);
+}
+
/**
* spi_register_board_info - register SPI devices for a given board
* @info: array of chip descriptors
@@ -387,41 +409,45 @@ EXPORT_SYMBOL_GPL(spi_new_device);
int __init
spi_register_board_info(struct spi_board_info const *info, unsigned n)
{
- struct boardinfo*bi;
+ struct boardinfo *bi, *tmp_bi;
+ int i;
- bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+ bi = kzalloc(n * sizeof(*bi), GFP_KERNEL);
if (!bi)
return -ENOMEM;
- bi-n_board_info = n;
- memcpy(bi-board_info, info, n * sizeof *info);
mutex_lock(board_lock);
- list_add_tail(bi-list, board_list);
+ for (i = 0, tmp_bi = bi; i n; i++, tmp_bi++, info++) {
+ memcpy(tmp_bi-board_info, info, sizeof(*info));
+ list_add_tail(tmp_bi-list, board_list);
+ }
mutex_unlock(board_lock);
+
+ for (i = 0, tmp_bi = bi; i n; i++, tmp_bi++)
+ spi_scan_masterlist(tmp_bi-board_info);
+
return 0;
}
/* FIXME someone should add support for a __setup(spi, ...) that
* creates board info from kernel command lines
*/
-
static void scan_boardinfo(struct spi_master *master)
{
struct boardinfo*bi;
+ struct spi_device *dev;
mutex_lock(board_lock);
list_for_each_entry(bi, board_list, list) {
- struct spi_board_info *chip = bi-board_info;
- unsignedn;
-
- for (n = bi-n_board_info; n 0; n--, chip++) {
- if (chip-bus_num != master-bus_num)
- continue;
- /* NOTE: this relies on spi_new_device to
-* issue diagnostics when given bogus inputs
-*/
- (void) spi_new_device(master, chip);
- }
+ struct spi_board_info *chip = bi-board_info;
+
+ if (chip-bus_num != master-bus_num)
+ continue;
+ dev = spi_new_device(master, chip);
+ if (!dev)
+ dev_err(master-dev.parent,
+ can't create new device for %s\n,
+ chip-modalias);
}
mutex_unlock(board_lock);
}
@@ -537,15 +563,18 @@ int spi_register_master(struct spi_master *master)
dev_dbg(dev, registered master %s%s\n, dev_name(master-dev),
dynamic ? (dynamic) : );
+ mutex_lock(spi_master_lock);
+ list_add_tail(master-list, spi_master_list);
/* populate children from any spi device tables */
scan_boardinfo(master);
+ mutex_unlock(spi_master_lock);
+
status = 0;
done:
return status;
}
EXPORT_SYMBOL_GPL(spi_register_master);
-
static int __unregister(struct device *dev
Re: [spi-devel-general] [PATCH v1 3/4] max3100: adds console support for MAX3100
On Tue, 30 Mar 2010 14:49:57 +0800 christian pellegrin chrip...@fsfe.org wrote: On Tue, Mar 30, 2010 at 4:14 AM, Feng Tang feng.t...@intel.com wrote: On Mon, 29 Mar 2010 20:55:51 +0800 When the HW works at 230400 bps, when we copy bulk data to the console, max3110 will probably receive about 20K characters, so the time for handling each char is 50us. If you use one char per spi transfer, it will have to execute 20K spi_transfer in one second, and each need be done in 50us, in this 50us you'll have to deal with controller IRQ handler + spi system overhead + tty receiving overhead. Is it a little over-killing to use one char per spi transfer? while the HW does have a 8 words RX FIFO this is too hackish, max3100 wasn't conceived this way. Let me point some problems of such an approach: 1) latency on rx chars becomes very high because we can process incoming transfers only after a full 8 byte (or whatever the spi transfer dimension is). For a 9600 this is too much. 2) even worse is that we can do flow control decision only on such boundary. 3) this is not reliable: think of what happens if the actual SPI transfer speed we get will be slower that the one we requested. We won't be emptying the RX buffer fastly enough even if could. 4) for low speeds we are going to monopolize the SPI bus for ages. No words in my last email suggested to use dynamic SPI clock speed, I admited that method is brutal and not mature when my driver was reviewed. In early version of our 3110 driver, we don't use dynamic clock speed, but the maxim clock supports by 3110, while we still use 8 words buffer read for RX. Let me cut my question clear: why not use the 8 words RX HW fifo? For bulk RX in 230400 case, is 50us enough to handle controller IRQ + spi subsystem overhead + tty receive overhead for one char per spi transfer model? So I'm rather convinced that the SPI transfer has to happen as soon as possible at a SPI device driver level without any guess on how the data will be clocked out. I suggest you to improve the dw_spi driver for better performance. Will consider that, thanks [[   ..00  iiuu  eessoo  44rr11((eeggffnn--77  ggccvvrriinn11((bbnnuu1144bbnnuu [[   ..00  BBOO--8800  1100--8800  uuaall)) huh, here I'm seeing another kind of problem: the same char is repeated two times (for example BBOO is written instead of BIOS). I'm quite sure (but I will triple check shortly) the the console driver doesn't do this (it's quite an easy loop in max3100_console_work, I will check wit a printascii to another serial port what is sent). So I guess the problem can be in the SPI master driver: is it correctly handling the CS changes? Isn't it having problems with DMA for example (I always found DMA for small data transfer (such 2 bytes in this case) problematic (for example many platforms just do it on a 4 byte boundary))? Have you tested it with other devices? Yes, the controller HW has a register to chose slave devices and thus the HW CS handling is transparent to software. The controller supports several slave devices connecting to it simultaneously, one is a 3G modem whose throughput is about several Mega bits. The current in-tree controller driver lacks DMA part, which is rejected as the related DMA controller driver never show up publicly. -- 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 v1 3/4] max3100: adds console support for MAX3100
On Mon, 29 Mar 2010 14:11:15 +0800
christian pellegrin chrip...@fsfe.org wrote:
I modified the code a little and run it on our HW platform, it
really show some sigh of life: it can boots to console (the print
format is not so good), I can input command and it execute
correctly, but very slow, I type 3 characters and it takes about 2
seconds to echo back on screen and start the execution, and after
about 1 minute, the console hang there and input stopped to work.
never seen such a behavior. Which platform are you using? Which SPI
driver? Do you have a low level printk (printascii) that puts output
somewhere else so I can send you a patch with some debugging output?
Can you log in some other way (like via network) and see if the CPU
load is at 100% for some reason?
Hi,
Our platform is Intel Moorestown platform, and use a spi controller core
from Designware (drivers/spi/dw_*.c). I know the problem may probably be
caused by my setting, but the dw_spi driver works fine with our own
3110 driver.
For debug method, sadly I don't get another output port yet, but if you
have some debug patch, that's great, it will help when I find another debug
output than max3110.
+ Â Â Â Â Â Â max3100_sr(s, tx, rx);
It doesn't handle received characters here? If the console is
printing out a bulk of message while user input some command, the
command may be ignored. Myself have met the same problem in our
driver.
yes but I think it's quite difficult to solve this problem in every
case. Console output is massively used only on boot when the user is
not supposed to type a lot.
It's difficult but not impossible, actually our driver checks every word
read back and handle it if it contains a valid data
+ Â Â if (next != s-console_tail) {
+ Â Â Â Â Â Â s-console_buf[next] = ch;
+ Â Â Â Â Â Â s-console_head = next;
+ Â Â }
Also I saw max3100_sr() uses cpu_to_be16() and be16_to_cpu(), is it
really necessary, our platform is little-endian(x86), and I have to
disable them to make the code work. Is your test platform
big-endian?
Have you configured your SPI controller as LSB first somehow, haven't
you? BTW my platform is a quite usual ARM9 S3C2440 which is little
endian.
yeah, you hit the point that our spi controller is LSB naturally (not
configured to), here may need a check for whether to do a swap
--
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 v1 3/4] max3100: adds console support for MAX3100
On Mon, 29 Mar 2010 20:55:51 +0800 christian pellegrin chrip...@fsfe.org wrote: On Mon, Mar 29, 2010 at 9:06 AM, Feng Tang feng.t...@intel.com wrote: Our platform is Intel Moorestown platform, and use a spi controller core from Designware (drivers/spi/dw_*.c). I know the problem may probably be caused by my setting, but the dw_spi driver works fine with our own 3110 driver. I had a look at the dw_spi driver. The spi_transfer path queues some work to a worqueue that itself schedules a tasklet. I don't think this is good for latency, I won't bet that such an architecture could deliver good performance. Now I see why you needed to do only big fat SPI transfers. Anyway this doesn't justify the 2 seconds delay between Hi DW controller driver don't need max3110 driver to use big spi transfer, the early version of our max3110 is also one word per spi transfer mode, and the 2 drivers work fine, though the rx performance is not good (copy paste a bulk message to console). Let me use some example to explain why I use big spi transfer for 3110: When the HW works at 230400 bps, when we copy bulk data to the console, max3110 will probably receive about 20K characters, so the time for handling each char is 50us. If you use one char per spi transfer, it will have to execute 20K spi_transfer in one second, and each need be done in 50us, in this 50us you'll have to deal with controller IRQ handler + spi system overhead + tty receiving overhead. Is it a little over-killing to use one char per spi transfer? while the HW does have a 8 words RX FIFO chars coming in and going out through the max31x0 you are seeing. I will try to analyze what's going on. BTW is only input slow or output is sluggish too? The initial messages from the console are coming out fast? If you disable the MAX3110 for console but you use just as a normal terminal (set console=/dev/null in the kernel command line and add getty 115200 /dev/ttyMAX0 in iniitab) is the interaction with the system fine? Thanks for helping sorting out this. The output is not so slow as input, if we set the console=/dev/ttyMAX0, the output is sluggish, looks like below, but when enter command line console the output is smooth, while the input is very slow. -- kernel print log of max3100 over Moorestown platform -- 0.00] Moorestown CPU Lincroft identified [[..00 iiuu eessoo 44rr11((eeggffnn--77 ggccvvrriinn11((bbnnuu1144bbnnuu [[..00 BBOO--8800 1100--8800 uuaall)) ]] IISSee22::00 11 - 0f200 (usabl)) [ .000 BIOS-e822:: 022000110 00((eserved) [0.00] BIISSe820:: ee0 --0fec01000 (reseree)) ]] IISSee200 [[..00 BBOO--8800 ff00-- rrssrree)) ]]NNttcc::NN [[..00 8800uuddtt aagg::((ssbbee == rrssrree)) 00 00]]ee22 eeooeerrnnee 00aa--0011 uuaall)) ]]llss__ffxx 2200mmxxaacc__ffxx00 ]]MMRR eeaall yyee nnaahhbbee ]]MMRR iiee aa [[..00-- rrtt--aakk [[..00-- rrtt--aakk]] AABBuuaall ]]MMRR aaiibbeerrnnee nnbbee:: 1) the console code has to check if the serial port associated to the same physical max3100 is up (console driver start their life much before serial ones). 2) if yes send data to the tty associated to the serial driver. Locking is needed here. I will implement this ASAP. cool! -- 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 3/3] max3100: adds console support for MAX3100
On Fri, 19 Mar 2010 16:39:57 +0800
Christian Pellegrin chrip...@fsfe.org wrote:
+#ifdef CONFIG_SERIAL_MAX3100_CONSOLE
+
+static void max3100_console_putchar(struct uart_port *port, int ch)
+{
+ struct max3100_port *s = container_of(port, struct
max3100_port, port);
+ unsigned long tout = 10 * HZ / 1000; /* 10ms */
+ unsigned long start;
+ u16 tx, rx;
+
+ if (tout == 0)
+ tout = 1;
+ start = jiffies;
+ do {
+ tx = MAX3100_RC;
+ max3100_sr(s, tx, rx);
Hi Christian,
You'd better not call max3110_sr (which calls spi_sync) here, a console's
putchar() func may be called in some atomic context.
I used similar method when I started to implement a console, but then I switch
to current method in my 3110 driver: save the printk buffer inside putchar,
and wake up a thread to really handle the spi transfer later.
Thanks,
Feng
+ } while ((rx MAX3100_T) == 0
+ !time_after(jiffies, start + tout));
+ tx = ch;
+ max3100_calc_parity(s, tx);
+ tx |= MAX3100_WD | MAX3100_RTS;
+ max3100_sr(s, tx, rx);
+}
+
+static void max3100_console_write(struct console *co,
+ const char *str, unsigned int
count) +{
+ struct max3100_port *s = max3100s[co-index];;
+
+ uart_console_write(s-port, str, count,
max3100_console_putchar); +}
--
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 v6] serial: spi: add spi-uart driver for Maxim 3110
On Wed, 17 Mar 2010 15:39:23 +0800 christian pellegrin chrip...@gmail.com wrote: On Wed, Mar 17, 2010 at 3:35 AM, Feng Tang feng.t...@intel.com wrote: Hi Christian, Hi, your driver is posted on public. And the reasons I still posted driver here are: 1. It provides a console I'll provide a patch to the mainline driver for the console. Let me just finish my daily job duties ;-) Cool!! I can test that on my platform. 2. It use buffer read/write to meet our performance goal, max3110 doesn't have good FIFO, but spi controller have and we can leverage that for performance I think this line of thinking is wrong. SPI generic layer is well generic. For example the platform I use is the S3C24xx which has a SPI shift register depth of just one byte. And using DMA is not an option because the setup of a DMA transfer takes tens of microseconds. Additionally the SPI driver in mainline is based on bit-banging which uses workqueues and so is limited by the fact that it fights with other sched_other processes. Unfortunately an alternative driver which was posted never got mainlined and we, who need it, have to maintain it ourself. But this is another story, the bottom line is that we have to cope with the most basic SPI support we can meet. I'm no your side suggesting not to use DMA for 3110/3100, but David and others have a valid point, we don't know what kind of SPI controller that this driver will work with, and the driver has to be as general and compatible as possible. this is exactly the problem I was facing: loosing chars on RX. I tracked the problem to a too long latency in awakening of the workqueue (the RX FIFO of the MAX3100 overflows). I will post shortly a patch that moves this to threaded interrupts which solved the problem on the S3C platform I mentioned above working at 115200. A quick and dirty fix for workqueues is posted here: http://www.evolware.org/chri/paciugo/index.html. Anyway it has to do with scheduling of processes so many more factors are concerned, like HZ and preemption being enabled. I will post some tests together with the patch, if you have some benchmarks you like to be run don't hesitate to send them. Thanks in advance! heh, I don't have benchmarks but simply the copy/paste stuff and using zmodem sometimes. I don't think this is a good idea to change speed. Just check the T bit to see when the TX buffer is empty and send at the maximum available speed. So the usage of the SPI bus is better. Yes, checking T bit is a good idea for single word transfer model, but it doesn't help when using buffer read/write I really don't understand why, can you elaborate on this? Checking T bit has to be done in word level, send one word out, read one back and check T bit inside one spi xfer. Why not use the buffer model, the HW has a 8 words RX buffer, why not read 8 words back in one spi xfer, saving a lot of system cost. And if we make the spi clk slightly slower than the baud rate, there will be no TX overflow problem. I didn't use kmalloc/kfree in my earlier submission, but other developers mentioned the buffer allocated here need be DMA safe, as 3110 may work with many kinds of SPI controller, some of them only supports DMA mode. why you don't preallocate buffer for SPI transfers that are DMA-safe? For example the mcp251x driver (http://lxr.free-electrons.com/source/drivers/net/can/mcp251x.c?v=2.6.34-rc1) does this. Some function will be called in several places, say re-entered, hard to use a preallocate buffer. I guess you are using mthread_up to avoid awakening the main thread too many times. But this makes sense? Anyway because the awakening and the setting of the bit is not atomic it won't work anyway. Why this can't work? the test/set_bit are atomic operations. This bit check is not exclusive, it just try to save some waking up, though waking a already running thread doesn't cost too much as mentioned by Andrew First of all I think that the cost of re-awakening an already running thread is lower than the logic of testing bits. My note was just nit-picking: there is a window between when the task was woken-up and where you set the bit, so it doesn't guarantee you that you aren't awakening the thread more than once anyway. OK, got your point now. test/set_bit's cost depends on the platform, and my simple test using perf tool showing it cost 10us level to wake a running thread. Anyway, re-wake it does no harm :) Thanks, Feng -- 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
Re: [spi-devel-general] [PATCH v6] serial: spi: add spi-uart driver for Maxim 3110
Hi Christian, Thanks for taking time to review the driver. Please see my answers inline - Feng On Wed, 17 Mar 2010 01:22:46 +0800 christian pellegrin chrip...@gmail.com wrote: On Thu, Mar 4, 2010 at 8:22 AM, spi-devel-general-requ...@lists.sourceforge.net wrote: Hi All, Hi, first of all let me apologize that I haven't see your previous versions of the driver. I completely missed them. I am the author of the max3100 driver. As far as I can see the max3110 is just a max3100 with the TTL to EIA level converter. So, first of all, may I ask why you did start a new driver from scratch? I'm quite happy with the I don't want to reinvent the wheel, as I have many more tasks to handle :) My 3110 driver started to work in 2008(surely very basic at that time) before your driver is posted on public. And the reasons I still posted driver here are: 1. It provides a console 2. It use buffer read/write to meet our performance goal, max3110 doesn't have good FIFO, but spi controller have and we can leverage that for performance If we can cooperate to make current 3100 driver meet our expectation, I'm more than happy to drop my 3110 one. I'd rather submit some bug fix for it than posting 6 or more versions of code :) current one, the only problem I see is that it uses worqueues which are scheduled like normal processes and so under load their latency is *big*. I wanted to move to threaded interrupts (otherwise you have to use some ugly tricks to give workqueue threads real-time scheduling class) but was waiting for my beta testers^H^H^H^H^H^H^H^H^H^H^H^H^H customers to move to modern kernel that support them. Anyway the patch is trivial and I can provide it if there is some interest. A quick glance at your patch shows you are using the same sched_other threads so you will face the same problems. I saw you added console capability which isn't present in current driver, but it's easy to add (I didn't implement it initially because I don't see a good idea using the max3100 for initial bring-up of an embedded system because it relies on the SPI subsystem, worqueues, scheduler and such complicated things). Anyway keep in mind that the MAX3100 is rather low-end UART so don't push it too much. It's not me push it too much, but I was pushed too much :) We can't make the decision how the HW is used on all kinds of platforms. For ours, we heavily rely on it as a sole console, and basic requirement is: the datasheet say it supports 230400~300 baud rate, then driver guy need make it work as it claims, like copying 500 bytes string somewhere and paste it on this console in 230400/600 rate. For the kernel early boot phase debug, I actually had another 3110 early_printk patch which can work right after the kernel starts I'm more than happy to work with you to have just one perfect (or almost perfect) driver instead of two of them half-done. See some comments inline. Can't agree more, all I want is just a upstream driver which meets the basic requirement. + * 1. From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has + * Â Â 1 word. If SPI master controller doesn't support sclk frequency change, + * Â Â then the char need be sent out one by one with some delay I don't think this is a good idea to change speed. Just check the T bit to see when the TX buffer is empty and send at the maximum available speed. So the usage of the SPI bus is better. Yes, checking T bit is a good idea for single word transfer model, but it doesn't help when using buffer read/write + * + * 2. Currently only RX availabe interrrupt is used yeah, this is one of the reasons why MAX31x0 sucks: when you change configuration of interrupts the RX buffer is cleared. So you cannot activate/deactivate TX empty interrupt as needed. +static int use_irq = 1; +module_param(use_irq, int, 0444); +MODULE_PARM_DESC(use_irq, Whether using Max3110's IRQ capability); I don't think it's a good idea using an UART in polling mode, it just kills the system. Just pretend this is not possible otherwise some hardware guys will pester us to do this just to save an electric wire. Yes, like Grant and others have mentioned, I plan to use spi-irq as the condition, platform code which initialize the board_info should clear the irq to 0 if they won't use IRQ for 3110 if (spi-irq) request_irq(spi-irq,...); else start_read_thread; + Â Â Â buf = kmalloc(8, GFP_KERNEL | GFP_DMA); you do kmalloc and kfree in routines that are called in quite tight loops: I guess it's an overkill for performance. Overkill is just the word I have used when replying comments on my earlier version :) I didn't use kmalloc/kfree in my earlier submission, but other developers mentioned the buffer allocated here need be DMA safe, as 3110 may work with many kinds of SPI controller, some of them only supports DMA mode. are you sure it's sane to just ignore these
Re: [spi-devel-general] [PATCH v6] serial: spi: add spi-uart driver for Maxim 3110
On Mon, 8 Mar 2010 12:30:13 +0800 Grant Likely grant.lik...@secretlab.ca wrote: Hi Feng, I'm really concerned with this driver. I think it needs a lot more work before it is ready for prime-time. Again, you might want to consider asking Greg to add it to the staging tree while you clean stuff up and coordinate with the max3100 driver author. Comments below. Hi Grant, Thanks for the comments, will address them in future submission. For your suggestion about merging with max3100.c, I did tried but failed as the basic working model differ a lot, anyway I will recheck that. - Feng -- 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 v6] serial: spi: add spi-uart driver for Maxim 3110
On Mon, 8 Mar 2010 12:12:34 +0800 Grant Likely grant.lik...@secretlab.ca wrote: Yep, here you touched a key point of the driver. In the early version of our driver, we used a fixed spi rate which is a little owner than its own working rate (3.684MHz) as a spi controller can hardly get a divided rate exactly same as 3110's clock , with that we can only handle one word per spi transfer, and have to add a udelay function for each transfer, and we even have to calculate the delay time for all kinds of the UART baud rate it's working with. struct spi_transfer has a member deay_usecs just for this case. One key point for SPI devices is they can't be accessed by CPU directly and has to go though tasklet or workqueue for each spi transfer. To improve performance, we chose to use buffer read/write to let one transfer contain more data, and use dynamic spi rate by setting spi_transfer-speed_hz for each baud rate accordingly. spi rate is set by spi controller drivers based on the speed_hz number and they should chose a lower spi rate than 3110's baud rate if can't find a same rate. For a spi controller support multiple devices including 3110, the delay from 3110 is inevitable no matter we use a explicit udelay or the dynamic spi rate. Till here, I have to say the spi_transfer of spi core is well designed, which provides bits_per_word/delay_usecs/speed_hz of great flexibility for developing spi device/controller drivers You're abusing the speed_hz setting of the SPI controller. By clocking down the SPI bus, you're effectively using the SPI completion interrupt as a high resolution timer to schedule your character transmissions so that you don't overrun the max3110 which has no tx fifo. Using a udelay is also the wrong thing to do because it chews up CPU cycles to achieve the same result. The correct thing to do is to either use a high resolution timer, or the IRQ from the MAX3110 itself to schedule your spi transfers, and set the SPI rate to the highest frequency that the MAX3110 can handle so that your driver neither hogs the SPI bus, nor wastes CPU cycles. You can improve the performance of the driver by using spi_async() instead of spi_sync() it enqueue the transfers so that you can handle everything at IRQ context and you don't need to drop into a workqueue to process the data. Yes, using a high resolution timer instead of udelay is definitely better. The reason we didn't use it is the delay for 115200 bps is about 90us, which is shorter than the time of executing an hr_timer(IRQ handling and precess waking stuff), but it's our HW's problem anyway. But this doesn't help on buffer read/write, and only works for one word per spi_transfer model as 3110 only has 1 word TX buffer and 8 word RX buffer. We normally use such a test case, Ctrl+C a big string (500 Bytes), and Ctrl+V it on the 3110 console, check if they are correctly received and echoed on screen, one word per transfer can hardly make that, due to the cost in spi layer and tty layer, while buffer read/write model can. If speed_hz are not for this case, then where should it be used for, spi core already has max_speed_hz specifying the maxim clock permitted by HW. Using speed_hz does affect other devices connecting to the same controller, but can we provide an option for the platform code to chose whether dynamic lower spi clk is allowed, in case the spi controller has only 3110 as its slave device. Thanks, Feng g. -- 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 v6] serial: spi: add spi-uart driver for Maxim 3110
On Fri, 5 Mar 2010 15:44:50 +0800 Erwin Authried ea...@softsys.co.at wrote: The spi rate and the uart clock aren't synchronized, what happens if the spi rate is slightly higher than the MAX3100's baud rate clock? In addition, if there are other devices on the spi bus, the bus will be occupied unnecessarily long, especially when low baudrates are used. Hi Erwin, Yep, here you touched a key point of the driver. In the early version of our driver, we used a fixed spi rate which is a little owner than its own working rate (3.684MHz) as a spi controller can hardly get a divided rate exactly same as 3110's clock , with that we can only handle one word per spi transfer, and have to add a udelay function for each transfer, and we even have to calculate the delay time for all kinds of the UART baud rate it's working with. struct spi_transfer has a member deay_usecs just for this case. One key point for SPI devices is they can't be accessed by CPU directly and has to go though tasklet or workqueue for each spi transfer. To improve performance, we chose to use buffer read/write to let one transfer contain more data, and use dynamic spi rate by setting spi_transfer-speed_hz for each baud rate accordingly. spi rate is set by spi controller drivers based on the speed_hz number and they should chose a lower spi rate than 3110's baud rate if can't find a same rate. For a spi controller support multiple devices including 3110, the delay from 3110 is inevitable no matter we use a explicit udelay or the dynamic spi rate. Till here, I have to say the spi_transfer of spi core is well designed, which provides bits_per_word/delay_usecs/speed_hz of great flexibility for developing spi device/controller drivers Thanks, Feng One other small cosmetic thing: in serial_m3110_tx_empty(), TIOCSER_TEMT should be returned instead of 1. -Erwin -- 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 v6] serial: spi: add spi-uart driver for Maxim 3110
Hi Mokuno,
Thanks for the thorough review and good comments!
Some of your comments are answered inline, others will be addressed in future
patch submission.
I would give v6 of this patch more time to be reviewed by community, so that
more comments can come and I can handle them altogether.
Thanks,
Feng
On Fri, 5 Mar 2010 02:46:07 +0800
Masakazu Mokuno mok...@sm.sony.co.jp wrote:
Hi Feng,
My comments inlined.
On Thu, 4 Mar 2010 15:25:24 +0800
Feng Tang feng.t...@intel.com wrote:
+ wait_queue_head_t wq;
+ struct task_struct *main_thread;
+ struct task_struct *read_thread;
+ spinlock_t lock;
checkpatch.pl complained:
CHECK: spinlock_t definition without comment
#119: FILE: drivers/serial/max3110.c:53:
+ spinlock_t lock;
A little strange, I'm using checkpatch.pl in kernel source, which doesn't
generate any warning. Which version are you using?
+
+ /* If caller doesn't provide a buffer, then handle received
char */
+ pbuf = rxbuf ? rxbuf : 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;
else
break;
Can be added in order to optimize a bit :)
There are other similar places where search valid received chars.
Reading Max3110 buffer is asynchronous operation, like for the reader thread,
spi controller will write 8 words of 0 to max3110, and read 8 words back, the
first word doesn't have a valid data doesn't mean the following don't, and we
need to check them all
+static int max3110_main_thread(void *_max)
+{
+ struct uart_max3110 *max = _max;
+ wait_queue_head_t *wq = max-wq;
+ int ret = 0;
+ struct circ_buf *xmit = max-con_xmit;
+
+ init_waitqueue_head(wq);
+ pr_info(PR_FMT start main thread\n);
+
+ do {
+ wait_event_interruptible(*wq,
+ max-flags || kthread_should_stop());
+ test_and_set_bit(0, max-mthread_up);
The result of testing ignored. Why testing?
Andrew mentioned a race condition for using set/test_bit for mthread_up
flag, which I don't quiet follow due to my lack of SMP race experience,
mthread_up is not designed to make some code exclusive. And whether the main
thread can be woken up to run depends on the max-flags setting.
+
+ if (use_irq test_bit(M3110_IRQ_PENDING,
max-flags)) {
+ max3110_con_receive(max);
+ clear_bit(M3110_IRQ_PENDING, max-flags);
+ }
test_and_clear_bit()?
You mean
if (use_irq test_and_clear_bit(..))
max3110_con_receive(max);
?
Yes, it's better. I put clear_bit() after max3110_con_receive() because
I test that bit to decide whether we need wakeup the main thread again
in the ISR in old version submission.
+
+ /* As we use thread to handle tx/rx, need set low latency */
+ port-state-port.tty-low_latency = 1;
+
+ if (use_irq) {
+ ret = request_irq(max-irq, serial_m3110_irq,
+ IRQ_TYPE_EDGE_FALLING,
max3110, max);
According to the manufacturer's datasheet, it looks like MAX3110'irq
is level interrupt. Refer Figure 6 of the datasheet.
Yep, good catch here, if there is RX data in buffer, the IRQ pin will
be asserted low always. But the IRQ line will usually be connected to
system through a GPIO pin, we use falling edge for GPIO pin IRQ trigger,
when 3110's IRQ is asserted, that GPIO pin will be changed from high to low,
the ISR and following handler will try to read all the RX data out to
make 3110's IRQ go back to high, waiting for another IRQ.
+ if (baud == 230400)
+ clk_div = WC_BAUD_DR1;
This if statement can be omitted as WC_BAUD_DR1 is 0 and clk_div
becomes (-1 + 1).
Nice trick, but it may confuse readers without any explanation :)
+ max-port.type = PORT_MAX3110;
+ max-port.fifosize = 2; /* Only have 16b buffer */
I guess MAX3110 has 8 chars RX FIFO and no TX FIFO. If so, value 2 is
OK here?
If you check the Figure 1 of the data sheet, there is still a TX buffer
there tough only one word :) or should I change it to 1?
+
+ max-main_thread = kthread_run(max3110_main_thread,
+ max, max3110_main);
+ if (IS_ERR(max-main_thread)) {
+ ret = PTR_ERR(max-main_thread);
+ goto err_kthread;
+ }
+
+ pmax = max;
If this driver supports only one instance of devices, how about
declaring a global struct uart_m3100 instead of kmallc()?
If using global struct, the space will be allocated no matter the system
really have a Max3110, putting the allocation in probe() is flexible
--
Download Intel#174
[spi-devel-general] [PATCH v6] serial: spi: add spi-uart driver for Maxim 3110
Hi All, Here is a driver for Maxim 3110 SPI-UART device, please help to review. It has been validated with Designware SPI controller (drivers/spi: dw_spi.c dw_spi_pci.c). It supports polling and IRQ mode, supports batch read, and provides a console. change log: since v5: * Make the spi data buffer DMA safe, thanks to Mssakazu Mokuno, David Brownell and Grant Likely for pointing the bug out since v4: * Add explanation why not using DMA * Fix a bug in max3110_read_multi() since v3: * Remove the Designware controller related stuff * Remove some initialization code which should be done in the platform setup code * Add a missing change for drivers/serial/Makefile since v2: * Address comments from Andrew Morton * Use test/set_bit to avoid race condition for SMP/preempt case * Fix bug related with endian order since v1: * Address comments from Alan Cox * Use a use_irq module parameter to runtime check whether to use IRQ mode * Add the suspend/resume implementation Thanks! Feng From 6ff1d75c34d9465d4ffce076350473bfaf5404a5 Mon Sep 17 00:00:00 2001 From: Feng Tang feng.t...@intel.com Date: Fri, 26 Feb 2010 11:37:29 +0800 Subject: [PATCH] serial: spi: add spi-uart driver for Maxim 3110 This is the driver for Max3110 SPI-UART device, which connect to host with SPI interface. It supports baud rates from 300 to 230400, and supports both polling and IRQ mode, as well as providing a console for system use Its datasheet could be found here: http://datasheets.maxim-ic.com/en/ds/MAX3110E-MAX3111E.pdf Signed-off-by: Feng Tang feng.t...@intel.com Cc: Andrew Morton a...@linux-foundation.org Cc: David Brownell davi...@pacbell.net Cc: Greg KH g...@kroah.com Cc: Grant Likely grant.lik...@secretlab.ca Acked-by: Alan Cox a...@linux.intel.com --- drivers/serial/Kconfig |8 + drivers/serial/Makefile |1 + drivers/serial/max3110.c| 892 +++ drivers/serial/max3110.h| 61 +++ include/linux/serial_core.h |3 + 5 files changed, 965 insertions(+), 0 deletions(-) create mode 100644 drivers/serial/max3110.c create mode 100644 drivers/serial/max3110.h diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 9ff47db..94aa282 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -688,6 +688,14 @@ config SERIAL_SA1100_CONSOLE your boot loader (lilo or loadlin) about how to pass options to the kernel at boot time.) +config SERIAL_MAX3110 + tristate SPI UART driver for Max3110 + depends on SPI_MASTER + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + help + This is the UART protocol driver for MAX3110 device + config SERIAL_BFIN tristate Blackfin serial port support depends on BLACKFIN diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index 5548fe7..b93d8a0 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -46,6 +46,7 @@ obj-$(CONFIG_SERIAL_S3C24A0) += s3c24a0.o obj-$(CONFIG_SERIAL_S3C6400) += s3c6400.o obj-$(CONFIG_SERIAL_S5PC100) += s3c6400.o obj-$(CONFIG_SERIAL_MAX3100) += max3100.o +obj-$(CONFIG_SERIAL_MAX3110) += max3110.o obj-$(CONFIG_SERIAL_IP22_ZILOG) += ip22zilog.o obj-$(CONFIG_SERIAL_MUX) += mux.o obj-$(CONFIG_SERIAL_68328) += 68328serial.o diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c new file mode 100644 index 000..9b39914 --- /dev/null +++ b/drivers/serial/max3110.c @@ -0,0 +1,892 @@ +/* + * max3110.c - spi uart protocol driver for Maxim 3110 + * + * Copyright (c) 2009, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/* + * Note: + * 1. From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has + *1 word. If SPI master controller doesn't support sclk frequency change, + *then the char need be sent out one by one with some delay + * + * 2. Currently only RX availabe interrrupt is used + */ + +#include linux/module.h +#include linux/ioport.h +#include linux/init.h +#include linux/console.h +#include linux/tty.h +#include linux/tty_flip.h +#include linux/serial_core.h +#include linux/serial_reg.h +#include linux/kthread.h +#include linux/delay.h +#include linux/spi/spi.h + +#include max3110.h
[spi-devel-general] [PATCH v5] serial: spi: add spi-uart driver for Maxim 3110
Hi All, Here is a driver for Maxim 3110 SPI-UART device, please help to review. It has been validated with Designware SPI controller (drivers/spi: dw_spi.c dw_spi_pci.c). It supports polling and IRQ mode, supports batch read, and provides a console. change log: since v4: * Add explanation why not using DMA * Fix a bug in max3110_read_multi() since v3: * Remove the Designware controller related stuff * Remove some initialization code which should be done in the platform setup code * Add a missing change for drivers/serial/Makefile since v2: * Address comments from Andrew Morton * Use test/set_bit to avoid race condition for SMP/preempt case * Fix bug related with endian order since v1: * Address comments from Alan Cox * Use a use_irq module parameter to runtime check whether to use IRQ mode * Add the suspend/resume implementation Thanks! Feng From 6ea4918365f5eed04f82544f0da3efe93cb2cc20 Mon Sep 17 00:00:00 2001 From: Feng Tang feng.t...@intel.com Date: Fri, 26 Feb 2010 11:37:29 +0800 Subject: [PATCH] serial: spi: add spi-uart driver for Maxim 3110 This is the driver for Max3110 SPI-UART device, which connect to host with SPI interface. It supports baud rates from 300 to 230400, and supports both polling and IRQ mode, as well as providing a console for system use Its datasheet could be found here: http://datasheets.maxim-ic.com/en/ds/MAX3110E-MAX3111E.pdf Signed-off-by: Feng Tang feng.t...@intel.com Cc: David Brownell davi...@pacbell.net Cc: Greg KH g...@kroah.com Acked-by: Alan Cox a...@linux.intel.com Acked-by: Grant Likely grant.lik...@secretlab.ca --- drivers/serial/Kconfig |8 + drivers/serial/Makefile |1 + drivers/serial/max3110.c| 875 +++ drivers/serial/max3110.h| 61 +++ include/linux/serial_core.h |3 + 5 files changed, 948 insertions(+), 0 deletions(-) create mode 100644 drivers/serial/max3110.c create mode 100644 drivers/serial/max3110.h diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 9ff47db..94aa282 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -688,6 +688,14 @@ config SERIAL_SA1100_CONSOLE your boot loader (lilo or loadlin) about how to pass options to the kernel at boot time.) +config SERIAL_MAX3110 + tristate SPI UART driver for Max3110 + depends on SPI_MASTER + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + help + This is the UART protocol driver for MAX3110 device + config SERIAL_BFIN tristate Blackfin serial port support depends on BLACKFIN diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index 5548fe7..b93d8a0 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -46,6 +46,7 @@ obj-$(CONFIG_SERIAL_S3C24A0) += s3c24a0.o obj-$(CONFIG_SERIAL_S3C6400) += s3c6400.o obj-$(CONFIG_SERIAL_S5PC100) += s3c6400.o obj-$(CONFIG_SERIAL_MAX3100) += max3100.o +obj-$(CONFIG_SERIAL_MAX3110) += max3110.o obj-$(CONFIG_SERIAL_IP22_ZILOG) += ip22zilog.o obj-$(CONFIG_SERIAL_MUX) += mux.o obj-$(CONFIG_SERIAL_68328) += 68328serial.o diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c new file mode 100644 index 000..aff0465 --- /dev/null +++ b/drivers/serial/max3110.c @@ -0,0 +1,875 @@ +/* + * max3110.c - spi uart protocol driver for Maxim 3110 + * + * Copyright (c) 2009, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/* + * Note: + * 1. From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has + *1 word. If SPI master controller doesn't support sclk frequency change, + *then the char need be sent out one by one with some delay + * + * 2. Currently only RX availabe interrrupt is used + * + * 3. This driver doesn't support work with a spi cotnroller in DMA mode, and + *the reason for not using DMA is: + *a) Maxim 3110 is a low speed UART device, whose tx/rx buffer are very few, + *and using DMA may be over-killing when working as a system console(imaging + *we edit a text file on it) + *b) Handling only one 16b word transfer will be very common, but non-32b + *aligned DMA operation is not supported by all kinds of DMA controllers + * + *Some spi
Re: [spi-devel-general] [PATCH v5] serial: spi: add spi-uart driver for Maxim 3110
On Wed, 3 Mar 2010 12:51:58 +0800 David Brownell davi...@pacbell.net wrote: On Tuesday 02 March 2010, Feng Tang wrote: + * 3. This driver doesn't support work with a spi cotnroller in DMA mode, As Grant said: That's a bug ... one that will randomly kick in based on whether the underlying SPI controller driver happens to use DMA for a given transaction. From this perspective, yes, it's a bug that this driver use non DMA-safe IO buffer, which prevents it to be portable for all kinds of controllers, and I can make it DMA safe. But I still don't think it's a good idea to use DMA for Max3110 for performance reason, I know there is some spi controller who only works in DMA mode. Here comes another idea, can we add a capability flag in struct spi_master indicating the master supporting poll or dma or both. Also we add similar bits in struct spi_transfer indicating the this transfer wants to be handled in poll or dma mode. For spi controller driver, it can claim its capability when registering as a spi_master, for a spi device driver, it can select the working mode based on spi_master's capability and set the working mode in struct spi_transfer. Then controller and device don't need to guess what the other is capable, and make the info public. Any thoughts? Thanks, Feng -- 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 v5] serial: spi: add spi-uart driver for Maxim 3110
Here comes another idea, can we add a capability flag in struct spi_master indicating the master supporting poll or dma or both. Also we add similar bits in struct spi_transfer indicating the this transfer wants to be handled in poll or dma mode. Let's not do either of those. There's no need to introduce such complexity, or to enable the associated new failure modes and bugs. This idea has nothing to do with the dma-safe problem you pointed out, I will make the buffer dma safe anyway. What I proposed just wants to provide some flexibility for protocol device drivers, it will use dma-safe buffer always, but it prefer not to use DMA for its one-word transfers, and hope to have a choice to do that. For current existing controller and device drivers, they can simply ignore the new working mode setting in struct spi_master and spi_transfer and leave them as 0. Thanks, Feng Much simpler to just use DMA-safe I/O buffers in the first place ... which is what pretty much every driver stack in Linux already expects or requires. - 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
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] [RFC][PATCH v3] serial: spi: add spi-uart driver for Maxim 3110
On Thu, 25 Feb 2010 07:18:32 +0800
Andrew Morton a...@linux-foundation.org wrote:
...
+static int max3110_main_thread(void *_max)
+{
+ struct uart_max3110 *max = _max;
+ wait_queue_head_t *wq = max-wq;
+ int ret = 0;
+ struct circ_buf *xmit = max-con_xmit;
+
+ init_waitqueue_head(wq);
+ pr_info(PR_FMT start main thread\n);
+
+ do {
+ wait_event_interruptible(*wq,
+ max-flags ||
kthread_should_stop());
+ set_bit(0, max-mthread_up);
+
+ if (use_irq test_bit(M3110_IRQ_PENDING,
max-flags)) {
+ max3110_con_receive(max);
+ clear_bit(M3110_IRQ_PENDING, max-flags);
+ }
+
+ /* First handle console output */
+ if (test_bit(M3110_CON_TX_NEED, max-flags)) {
+ send_circ_buf(max, xmit);
+ clear_bit(M3110_CON_TX_NEED, max-flags);
+ }
+
+ /* Handle uart output */
+ if (test_bit(M3110_UART_TX_NEED, max-flags)) {
+ transmit_char(max);
+ clear_bit(M3110_UART_TX_NEED, max-flags);
+ }
+ clear_bit(0, max-mthread_up);
+ } while (!kthread_should_stop());
+
+ return ret;
+}
+
+static irqreturn_t serial_m3110_irq(int irq, void *dev_id)
+{
+ struct uart_max3110 *max = dev_id;
+
+ /* max3110's irq is a falling edge, not level triggered,
+* so no need to disable the irq */
+ set_bit(M3110_IRQ_PENDING, max-flags);
+
+ if (!test_bit(0, max-mthread_up))
+ wake_up_process(max-main_thread);
+
+ return IRQ_HANDLED;
+}
The manipulation of mthread_up here (and in several other places) is
clearly quite racy. If you hit that race, the thread will not wake up
and the driver will sit there not doing anything until some other
event happens.
This is perhaps fixable with test_and_set_bit() and
test_and_clear_bit() (need to think about that) or, of course, by
adding locking.
But a simpler fix is just to delete mthread_up altogether.
wake_up_process() on a running process is an OK thing to do and
hopefully isn't terribly slow.
Yes, wake_up_process won't harm a running process, our driver's case
is a little special, the console's write() func may call
wake_up_process() for every character in the buffer, thus we will
try to avoid to call it. mthread_up can't be removed as it is also
referenced in read_thread.
I prefer to use the test_and_set/clear_bit for mthread_up.
Thanks,
Feng
diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c
index e8c44fa..d5bd71f 100644
--- a/drivers/serial/max3110.c
+++ b/drivers/serial/max3110.c
@@ -400,7 +400,7 @@ static int max3110_main_thread(void *_max)
do {
wait_event_interruptible(*wq,
max-flags || kthread_should_stop());
- set_bit(0, max-mthread_up);
+ test_and_set_bit(0, max-mthread_up);
if (use_irq test_bit(M3110_IRQ_PENDING, max-flags)) {
max3110_con_receive(max);
@@ -418,7 +418,7 @@ static int max3110_main_thread(void *_max)
transmit_char(max);
clear_bit(M3110_UART_TX_NEED, max-flags);
}
- clear_bit(0, max-mthread_up);
+ test_and_clear_bit(0, max-mthread_up);
} while (!kthread_should_stop());
return ret;
--
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] [RFC][PATCH v3] serial: spi: add spi-uart driver for Maxim 3110
On Thu, 25 Feb 2010 12:43:14 +0800
David Brownell davi...@pacbell.net wrote:
On Tuesday 23 February 2010, Feng Tang wrote:
+config SERIAL_MAX3110
+Â Â Â tristate SPI UART driver for Max3110
+Â Â Â select SERIAL_CORE
+Â Â Â select SERIAL_CORE_CONSOLE
Shouldn't that depend on SPI_MASTER? As it stands, you're
permitting it to build on systems that you *know* don't have
a prayer of running this code ... or often, even finishing
the build.
+Â Â Â help
+Â Â Â Â This is the UART protocol driver for MAX3110 device
+
+config MAX3110_DESIGNWARE
Having this depend on a specific underlying SPI master controller
is not a good thing. It should instead be written so that it
runs correctly on *any* controller which exports the standard SPI
programming interface.
+Â Â Â boolean Enable Max3110 to work with Designware controller
+Â Â Â default y
+Â Â Â depends on SERIAL_MAX3110
+
That is, stuff like this, from your max3110 driver:
+
+#ifdef CONFIG_MAX3110_DESIGNWARE
+static struct dw_spi_chip spi_uart = {
+Â Â Â .poll_mode = 1,
+Â Â Â .enable_dma = 0,
+Â Â Â .type = SPI_FRF_SPI,
+};
+#endif
Is completely irrelevant for other hardware ... or else
(as with DMA, which should be enabled by default) is
relevant, but shouldn't be parameterized.
+Â Â Â spi-mode = SPI_MODE_0;
+Â Â Â spi-bits_per_word = 16;
+#ifdef CONFIG_MAX3110_DESIGNWARE
+Â Â Â spi-controller_data = spi_uart;
+#endif
That all looks fishy too. SPI_MODE should have
been set up as part of device creation. Ditto
any spi-controller_data ... normally, that all
gets set up as part of board-specific setup.
Normally one goes to a lot of effort to keep
such board-specific code out of drivers.
Good point about the DW controller specific data, I'll remove them.
For those bits_per_word setting, I think we can put it here instead of
the board initialization code, as many types of boards can leverage the
setting here as it only works in 16b mode.
Thanks,
Feng
- 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] [RFC][PATCH] serial: spi: add spi-uart driver for Maxim 3110
On Thu, 25 Feb 2010 12:47:13 +0800 David Brownell davi...@pacbell.net wrote: On Tuesday 29 December 2009, Erwin Authried wrote: I think there's no need for a MAX3100 **and** a MAX3110 driver, this is just confusing. The MAX3110 driver is identical to the MAX3100 from the software view, it is simply a MAX3100 with transceivers added to the chip. If there's any improvement, that should be merged into the existing MAX3100 driver. Assuming that's true ... who will resolve the issue? Hi David, I've answered Erwin's comments before in v1 submission cycle, following is the quote: I agree there should not be 2 drivers cover 1 family of HW, so this is a RFC. I've thinked about merge with current 3100 code, but it depends on one char per spi_transfer, while my driver relys on batch data transfer for efficiency. Another key point is the console, SPI UART can't be directly accessed by CPU, so every spi_transfer will go through tasklet/workqueue, which makes supporting printk a big part of my driver. I really did consider about that, but has no good clue, so I think better to shape my driver first. Thanks, Feng -- 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
[spi-devel-general] [RFC][PATCH v3] serial: spi: add spi-uart driver for Maxim 3110
Hi All,
Here is a driver for Maxim 3110 SPI-UART device, please help to review.
It has been validated with Designware SPI controller (drivers/spi: dw_spi.c
dw_spi_pci.c). It supports polling and IRQ mode, supports batch read, and
provides a console.
change log:
since v2:
* Address comments from Andrew Morton
* Use test/set_bit to avoid race condition for SMP/preempt case
* Fix bug related with endian order
since v1:
* Address comments from Alan Cox
* Use a use_irq module parameter to runtime check whether
to use IRQ mode
* Add the suspend/resume implementation
Thanks!
Feng
From 93455ea6fbf0bfb6494b88ea83296f26f29f7eee Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 23 Feb 2010 17:52:00 +0800
Subject: [PATCH] serial: spi: add spi-uart driver for Maxim 3110
This is the driver for Max3110 SPI-UART device, which connect
to host with SPI interface. It supports baud rates from 300 to
230400, and supports both polling and IRQ mode, as well as
providing a console for system use
Its datasheet could be found here:
http://datasheets.maxim-ic.com/en/ds/MAX3110E-MAX3111E.pdf
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/serial/Kconfig | 12 +
drivers/serial/max3110.c| 879 +++
drivers/serial/max3110.h| 60 +++
include/linux/serial_core.h |3 +
4 files changed, 954 insertions(+), 0 deletions(-)
create mode 100644 drivers/serial/max3110.c
create mode 100644 drivers/serial/max3110.h
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 9ff47db..f7daf2f 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -688,6 +688,18 @@ config SERIAL_SA1100_CONSOLE
your boot loader (lilo or loadlin) about how to pass options to the
kernel at boot time.)
+config SERIAL_MAX3110
+ tristate SPI UART driver for Max3110
+ select SERIAL_CORE
+ select SERIAL_CORE_CONSOLE
+ help
+ This is the UART protocol driver for MAX3110 device
+
+config MAX3110_DESIGNWARE
+ boolean Enable Max3110 to work with Designware controller
+ default y
+ depends on SERIAL_MAX3110
+
config SERIAL_BFIN
tristate Blackfin serial port support
depends on BLACKFIN
diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c
new file mode 100644
index 000..e8c44fa
--- /dev/null
+++ b/drivers/serial/max3110.c
@@ -0,0 +1,879 @@
+/*
+ * max3110.c - spi uart protocol driver for Maxim 3110
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/*
+ * Note:
+ * * From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has
+ * 1 word. If SPI master controller doesn't support sclk frequency change,
+ * then the char need be sent out one by one with some delay
+ *
+ * * Currently only RX availabe interrrupt is used
+ */
+
+#include linux/module.h
+#include linux/ioport.h
+#include linux/init.h
+#include linux/console.h
+#include linux/platform_device.h
+#include linux/tty.h
+#include linux/tty_flip.h
+#include linux/serial_core.h
+#include linux/serial_reg.h
+
+#include linux/kthread.h
+#include linux/delay.h
+#include linux/spi/spi.h
+#include linux/spi/dw_spi.h
+
+#include max3110.h
+
+#define PR_FMT max3110:
+
+struct uart_max3110 {
+ struct uart_port port;
+ struct spi_device *spi;
+ char *name;
+
+ wait_queue_head_t wq;
+ struct task_struct *main_thread;
+ struct task_struct *read_thread;
+ spinlock_t lock;
+
+ u32 baud;
+ u16 cur_conf;
+ u8 clock;
+ u8 parity, word_7bits;
+ u16 irq;
+
+ /* bit map for UART status */
+ unsigned long flags;
+#define M3110_CON_TX_NEED 0
+#define M3110_UART_TX_NEED 1
+#define M3110_IRQ_PENDING 2
+ unsigned long mthread_up;
+
+ /* console buffer */
+ struct circ_buf con_xmit;
+};
+
+static struct uart_max3110 *pmax;
+
+static int use_irq = 1;
+module_param(use_irq, int, 0444);
+MODULE_PARM_DESC(use_irq, Whether using Max3110's IRQ capability);
+
+static void receive_chars(struct uart_max3110 *max,
+ unsigned char *str, int len);
+static int max3110_read_multi(struct uart_max3110 *max, u8 *buf);
+static void max3110_con_receive(struct
[spi-devel-general] [RFC][PATCH v2] serial: spi: add spi-uart driver for Maxim 3110
Hi All,
Here is a driver for Maxim 3110 SPI-UART device, please help to review.
It has been validated with Designware SPI controller (drivers/spi: dw_spi.c
dw_spi_pci.c). It supports polling and IRQ mode, supports batch read, and
provides a console.
change since v1:
* Address comments from Alan Cox
* Use a use_irq module parameter to runtime check whether
to use IRQ mode
* Add the suspend/resume implementation
Thanks!
Feng
From 6d14c5d68cdae8d48b6d8a00b6653022f2b100d0 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Mon, 8 Feb 2010 16:02:59 +0800
Subject: [PATCH] serial: spi: add spi-uart driver for Maxim 3110
This is the driver for Max3110 SPI-UART device, which connect
to host with SPI interface. It supports baud rates from 300 to
230400, and supports both polling and IRQ mode, as well as
providing a console for system use
Its datasheet could be found here:
http://datasheets.maxim-ic.com/en/ds/MAX3110E-MAX3111E.pdf
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/serial/Kconfig | 12 +
drivers/serial/max3110.c| 848 +++
drivers/serial/max3110.h| 59 +++
include/linux/serial_core.h |3 +
4 files changed, 922 insertions(+), 0 deletions(-)
create mode 100644 drivers/serial/max3110.c
create mode 100644 drivers/serial/max3110.h
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 9ff47db..f7daf2f 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -688,6 +688,18 @@ config SERIAL_SA1100_CONSOLE
your boot loader (lilo or loadlin) about how to pass options to the
kernel at boot time.)
+config SERIAL_MAX3110
+ tristate SPI UART driver for Max3110
+ select SERIAL_CORE
+ select SERIAL_CORE_CONSOLE
+ help
+ This is the UART protocol driver for MAX3110 device
+
+config MAX3110_DESIGNWARE
+ boolean Enable Max3110 to work with Designware controller
+ default y
+ depends on SERIAL_MAX3110
+
config SERIAL_BFIN
tristate Blackfin serial port support
depends on BLACKFIN
diff --git a/drivers/serial/max3110.c b/drivers/serial/max3110.c
new file mode 100644
index 000..c7c012b
--- /dev/null
+++ b/drivers/serial/max3110.c
@@ -0,0 +1,848 @@
+/*
+ * max3110.c - spi uart protocol driver for Maxim 3110
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/*
+ * Note:
+ * * From Max3110 spec, the Rx FIFO has 8 words, while the Tx FIFO only has
+ * 1 word. If SPI master controller doesn't support sclk frequency change,
+ * then the char need be sent out one by one with some delay
+ *
+ * * Currently only RX availabe interrrupt is used
+ */
+
+#include linux/module.h
+#include linux/ioport.h
+#include linux/init.h
+#include linux/console.h
+#include linux/platform_device.h
+#include linux/tty.h
+#include linux/tty_flip.h
+#include linux/serial_core.h
+#include linux/serial_reg.h
+
+#include linux/kthread.h
+#include linux/delay.h
+#include asm/atomic.h
+#include linux/spi/spi.h
+#include linux/spi/dw_spi.h
+
+#include max3110.h
+
+#define PR_FMT max3110:
+
+struct uart_max3110 {
+ struct uart_port port;
+ struct spi_device *spi;
+ char *name;
+
+ wait_queue_head_t wq;
+ struct task_struct *main_thread;
+ struct task_struct *read_thread;
+ int mthread_up;
+ spinlock_t lock;
+
+ u32 baud;
+ u16 cur_conf;
+ u8 clock;
+ u8 parity, word_7bits;
+
+ atomic_t uart_tx_need;
+
+ /* console related */
+ struct circ_buf con_xmit;
+ atomic_t con_tx_need;
+
+ /* irq related */
+ u16 irq;
+ atomic_t irq_pending;
+};
+
+static struct uart_max3110 *pmax;
+
+static int use_irq = 1;
+module_param(use_irq, int, 0444);
+MODULE_PARM_DESC(use_irq, Whether using Max3110's IRQ capability);
+
+static void receive_chars(struct uart_max3110 *max,
+ unsigned char *str, int len);
+static int max3110_read_multi(struct uart_max3110 *max, int len, u8 *buf);
+static void max3110_con_receive(struct uart_max3110 *max);
+
+int max3110_write_then_read(struct uart_max3110 *max,
+ const u8 *txbuf, u8 *rxbuf, unsigned len, int always_fast)
+{
+ struct spi_device *spi = max-spi
Re: [spi-devel-general] [PATCH 3/3] spi: dw_spi: refine the IRQ mode working flow
On Wed, 20 Jan 2010 20:44:03 +0800 Jean-Hugues Deschenes jean-hugues.desche...@octasic.com wrote: For FIFO depth, it depends on each specific implementation based on DW core, and interface driver would better set it. If fifo_len is not set in IRQ mode, core driver will set 0 as the TX interrupt threshold, which will only trigger the TXE IRQ when the TX FIFO is fully empty. This is my design thought. I'm sorry; At first, I was under the impression that it referred to a software FIFO. ...So could this be auto-detected from the [RT]XFTLR registers, then? Yep, really a good idea, from the HW spec, the depth could be detected from [RT}XTLR, will submit a patch for this. Thanks, Feng -- Throughout its 18-year history, RSA Conference consistently attracts the world's best and brightest in the field, creating opportunities for Conference attendees to learn about information security's most important issues through interactions with peers, luminaries and emerging and established companies. http://p.sf.net/sfu/rsaconf-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 1/3] spi: dw_spi: fix a bug in wait_till_not_busy()
From 124fda393891d31debc2a3b760627edb18422203 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Fri, 25 Dec 2009 11:18:32 +0800
Subject: [PATCH 1/3] spi: dw_spi: fix a bug in wait_till_not_busy()
Make the driver wait at least for 1 jiffie before issuing the
warning, no matter what HZ is set to
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 31620fa..521d680 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -161,7 +161,7 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi
*dws)
static void wait_till_not_busy(struct dw_spi *dws)
{
- unsigned long end = jiffies + usecs_to_jiffies(1000);
+ unsigned long end = jiffies + 1 + usecs_to_jiffies(1000);
while (time_before(jiffies, end)) {
if (!(dw_readw(dws, sr) SR_BUSY))
--
1.6.0.4
--
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 2/3] spi0: dw_spi: add a missed dw_spi_remove_host() in exit sequence
From 476cd29cdde8764b8491128f15c7739547af514f Mon Sep 17 00:00:00 2001 From: Feng Tang feng.t...@intel.com Date: Mon, 28 Dec 2009 11:35:03 +0800 Subject: [PATCH 2/3] spi0: dw_spi: add a missed dw_spi_remove_host() in exit sequence Signed-off-by: Feng Tang feng.t...@intel.com --- drivers/spi/dw_spi_pci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/spi/dw_spi_pci.c b/drivers/spi/dw_spi_pci.c index 34ba691..7980f14 100644 --- a/drivers/spi/dw_spi_pci.c +++ b/drivers/spi/dw_spi_pci.c @@ -98,6 +98,7 @@ static void __devexit spi_pci_remove(struct pci_dev *pdev) struct dw_spi_pci *dwpci = pci_get_drvdata(pdev); pci_set_drvdata(pdev, NULL); + dw_spi_remove_host(dwpci-dws); iounmap(dwpci-dws.regs); pci_release_region(pdev, 0); kfree(dwpci); -- 1.6.0.4 -- Throughout its 18-year history, RSA Conference consistently attracts the world's best and brightest in the field, creating opportunities for Conference attendees to learn about information security's most important issues through interactions with peers, luminaries and emerging and established companies. http://p.sf.net/sfu/rsaconf-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 3/3] spi: dw_spi: refine the IRQ mode working flow
From e1a3ba6f5605e7d0ae568b3d747b5ff03f7e202c Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 19 Jan 2010 10:22:45 +0800
Subject: [PATCH 3/3] spi: dw_spi: refine the IRQ mode working flow
Now dw_spi core fully supports 3 transfer modes: pure polling,
DMA and IRQ mode. IRQ mode will use the FIFO half empty as
the IRQ trigger, so each interface driver need set the fifo_len,
so that core driver can handle it properly
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/dw_spi.c | 64 ++-
drivers/spi/dw_spi_pci.c |1 +
include/linux/spi/dw_spi.h |1 +
3 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 521d680..1bb709b 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -358,6 +358,8 @@ static void transfer_complete(struct dw_spi *dws)
static irqreturn_t interrupt_transfer(struct dw_spi *dws)
{
u16 irq_status, irq_mask = 0x3f;
+ u32 int_level = dws-fifo_len / 2;
+ u32 left;
irq_status = dw_readw(dws, isr) irq_mask;
/* Error handling */
@@ -369,22 +371,23 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
return IRQ_HANDLED;
}
- /* INT comes from tx */
- if (dws-tx (irq_status SPI_INT_TXEI)) {
- while (dws-tx dws-tx_end)
+ if (irq_status SPI_INT_TXEI) {
+ spi_mask_intr(dws, SPI_INT_TXEI);
+
+ left = (dws-tx_end - dws-tx) / dws-n_bytes;
+ left = (left int_level) ? int_level : left;
+
+ while (left--)
dws-write(dws);
+ dws-read(dws);
- if (dws-tx == dws-tx_end) {
- spi_mask_intr(dws, SPI_INT_TXEI);
+ /* Re-enable the IRQ if there is still data left to tx */
+ if (dws-tx_end dws-tx)
+ spi_umask_intr(dws, SPI_INT_TXEI);
+ else
transfer_complete(dws);
- }
}
- /* INT comes from rx */
- if (dws-rx (irq_status SPI_INT_RXFI)) {
- if (dws-read(dws))
- transfer_complete(dws);
- }
return IRQ_HANDLED;
}
@@ -428,6 +431,7 @@ static void pump_transfers(unsigned long data)
u8 bits = 0;
u8 imask = 0;
u8 cs_change = 0;
+ u16 txint_level = 0;
u16 clk_div = 0;
u32 speed = 0;
u32 cr0 = 0;
@@ -438,6 +442,9 @@ static void pump_transfers(unsigned long data)
chip = dws-cur_chip;
spi = message-spi;
+ if (unlikely(!chip-clk_div))
+ chip-clk_div = dws-max_freq / chip-speed_hz;
+
if (message-state == ERROR_STATE) {
message-status = -EIO;
goto early_exit;
@@ -492,7 +499,7 @@ static void pump_transfers(unsigned long data)
/* clk_div doesn't support odd number */
clk_div = dws-max_freq / speed;
- clk_div = (clk_div 1) 1;
+ clk_div = (clk_div + 1) 0xfffe;
chip-speed_hz = speed;
chip-clk_div = clk_div;
@@ -535,11 +542,16 @@ static void pump_transfers(unsigned long data)
/* Check if current transfer is a DMA transaction */
dws-dma_mapped = map_dma_buffers(dws);
+ /*
+* Interrupt mode
+* we only need set the TXEI IRQ, as TX/RX always happen syncronizely
+*/
if (!dws-dma_mapped !chip-poll_mode) {
- if (dws-rx)
- imask |= SPI_INT_RXFI;
- if (dws-tx)
- imask |= SPI_INT_TXEI;
+ int templen = dws-len / dws-n_bytes;
+ txint_level = dws-fifo_len / 2;
+ txint_level = (templen txint_level) ? txint_level : templen;
+
+ imask |= SPI_INT_TXEI;
dws-transfer_handler = interrupt_transfer;
}
@@ -549,21 +561,23 @@ static void pump_transfers(unsigned long data)
* 2. clk_div is changed
* 3. control value changes
*/
- if (dw_readw(dws, ctrl0) != cr0 || cs_change || clk_div) {
+ if (dw_readw(dws, ctrl0) != cr0 || cs_change || clk_div || imask) {
spi_enable_chip(dws, 0);
if (dw_readw(dws, ctrl0) != cr0)
dw_writew(dws, ctrl0, cr0);
+ spi_set_clk(dws, clk_div ? clk_div : chip-clk_div);
+ spi_chip_sel(dws, spi-chip_select);
+
/* Set the interrupt mask, for poll mode just diable all int */
spi_mask_intr(dws, 0xff);
- if (!chip-poll_mode)
+ if (imask)
spi_umask_intr(dws, imask);
+ if (txint_level)
+ dw_writew(dws, txfltr, txint_level
Re: [spi-devel-general] [PATCH] Memory-mapped Dwsignware SPI driver
You may need modify the code a little, I just submit some code change :) mainly about a dw_spi_remove_host() and fifo_len setting Thanks, Feng On Tue, 19 Jan 2010 00:50:01 +0800 Jean-Hugues Deschenes jean-hugues.desche...@octasic.com wrote: Adds a memory-mapped I/O dw_spi platform device. Signed-off-by: Jean-Hugues Deschenes jean-hugues.desche...@octasic.com --- drivers/spi/Kconfig | 16 drivers/spi/Makefile |1 drivers/spi/dw_spi_mmio.c | 164 + 3 files changed, 178 insertions(+), 3 deletions(-) create mode 100644 drivers/spi/dw_spi_mmio.c -- Throughout its 18-year history, RSA Conference consistently attracts the world's best and brightest in the field, creating opportunities for Conference attendees to learn about information security's most important issues through interactions with peers, luminaries and emerging and established companies. http://p.sf.net/sfu/rsaconf-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 0/3 v3] spi: controller driver for Designware SPI core and PCI interface
Hi all, This patch series adds driver for Designware SPI core + PCI interface, and they are generated against kernel 2.6.32-rc4 0001: driver for the DW SPI core 0002: driver for PCI interface 0003: add DMA support This driver has been verified on Intel Moorestown platform, with Maxim's Max3110 UART device and Option's 3G modem GTM501L, both PIO and DMA works fine, and these 2 slave devcies can work simultaneously. 0003 is RFC only, as it has dependency over DMA controller driver's acceptance to mainline. User can use dw_apb_ssi_db.pdf from Synopsys as HW datasheet please help to review them. --- change history: v3 * address some review comments from Andrew Morton v2 * divide the original one driver to core+interface framework according to David's comments v1 * initial submission Thanks, Feng -- Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 2/3 v3] spi: add PCI interface driver for Designware SPI core
From 08d9f64c47e2c76c6f417a6eb7f8f7f2ce45cb47 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Sat, 10 Oct 2009 14:57:33 +0800
Subject: [PATCH 2/3] spi: add PCI interface driver for Designware SPI core
This add the PCI interface driver for DW SPI core, it has
been tested on Intel Moorestown platform
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig |4 +
drivers/spi/Makefile |1 +
drivers/spi/dw_spi_pci.c | 169 ++
3 files changed, 174 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/dw_spi_pci.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6eb9621..bd39e80 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -250,6 +250,10 @@ config SPI_DESIGNWARE
help
general driver for SPI controller core from DesignWare
+config SPI_DW_PCI
+ tristate PCI interface driver for DW SPI core
+ depends on SPI_DESIGNWARE PCI
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 5cc4e19..51ebff0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
obj-$(CONFIG_SPI_SH_SCI) += spi_sh_sci.o
obj-$(CONFIG_SPI_STMP3XXX) += spi_stmp.o
obj-$(CONFIG_SPI_DESIGNWARE) += dw_spi.o
+obj-$(CONFIG_SPI_DW_PCI) += dw_spi_pci.o
# ... add above this line ...
# SPI protocol drivers (device/link on bus)
diff --git a/drivers/spi/dw_spi_pci.c b/drivers/spi/dw_spi_pci.c
new file mode 100644
index 000..24809c9
--- /dev/null
+++ b/drivers/spi/dw_spi_pci.c
@@ -0,0 +1,169 @@
+/*
+ * mrst_spi_pci.c - PCI interface driver for DW SPI Core
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include linux/interrupt.h
+#include linux/pci.h
+#include linux/spi/dw_spi.h
+#include linux/spi/spi.h
+
+#define DRIVER_NAME dw_spi_pci
+
+struct dw_spi_pci {
+ struct pci_dev *pdev;
+ struct dw_spi dws;
+};
+
+static int __devinit spi_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct dw_spi_pci *dwpci;
+ struct dw_spi *dws;
+ int pci_bar = 0;
+ int ret;
+
+ printk(KERN_INFO DW: found PCI SPI controller(ID: %04x:%04x)\n,
+ pdev-vendor, pdev-device);
+
+ ret = pci_enable_device(pdev);
+ if (ret)
+ return ret;
+
+ dwpci = kzalloc(sizeof(struct dw_spi_pci), GFP_KERNEL);
+ if (!dwpci) {
+ ret = -ENOMEM;
+ goto err_disable;
+ }
+
+ dwpci-pdev = pdev;
+ dws = dwpci-dws;
+
+ /* Get basic io resource and map it */
+ dws-paddr = pci_resource_start(pdev, pci_bar);
+ dws-iolen = pci_resource_len(pdev, pci_bar);
+
+ ret = pci_request_region(pdev, pci_bar, dev_name(pdev-dev));
+ if (ret)
+ goto err_kfree;
+
+ dws-regs = ioremap_nocache((unsigned long)dws-paddr,
+ pci_resource_len(pdev, pci_bar));
+ if (!dws-regs) {
+ ret = -ENOMEM;
+ goto err_release_reg;
+ }
+
+ dws-parent_dev = pdev-dev;
+ dws-bus_num = 0;
+ dws-num_cs = 4;
+ dws-max_freq = 2500; /* for Moorestwon */
+ dws-irq = pdev-irq;
+
+ ret = dw_spi_add_host(dws);
+ if (ret)
+ goto err_unmap;
+
+ /* PCI hook and SPI hook use the same drv data */
+ pci_set_drvdata(pdev, dwpci);
+ return 0;
+
+err_unmap:
+ iounmap(dws-regs);
+err_release_reg:
+ pci_release_region(pdev, pci_bar);
+err_kfree:
+ kfree(dwpci);
+err_disable:
+ pci_disable_device(pdev);
+ return ret;
+}
+
+static void __devexit spi_pci_remove(struct pci_dev *pdev)
+{
+ struct dw_spi_pci *dwpci = pci_get_drvdata(pdev);
+
+ pci_set_drvdata(pdev, NULL);
+ iounmap(dwpci-dws.regs);
+ pci_release_region(pdev, 0);
+ kfree(dwpci);
+ pci_disable_device(pdev);
+}
+
+#ifdef CONFIG_PM
+static int spi_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+ struct dw_spi_pci *dwpci = pci_get_drvdata(pdev);
+ int ret;
+
+ ret
[spi-devel-general] [PATCH 1/3 v3]spi: controller driver for Designware SPI core
From 2945f4eab344742d878ea10a774285fe770e6b10 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Sat, 10 Oct 2009 14:55:48 +0800
Subject: [PATCH 1/3] spi: controller driver for Designware SPI core
Driver for the Designware SPI core, it supports multipul interfaces
like PCI/APB etc. User can use dw_apb_ssi_db.pdf from Synopsys as
HW datasheet.
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig|6 +
drivers/spi/Makefile |1 +
drivers/spi/dw_spi.c | 941
include/linux/spi/dw_spi.h | 212 ++
4 files changed, 1160 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/dw_spi.c
create mode 100644 include/linux/spi/dw_spi.h
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 4b6f7cb..6eb9621 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -244,6 +244,12 @@ config SPI_XILINX
See the OPB Serial Peripheral Interface (SPI) (v1.00e)
Product Specification document (DS464) for hardware details.
+config SPI_DESIGNWARE
+ bool DesignWare SPI controller core support
+ depends on SPI_MASTER
+ help
+ general driver for SPI controller core from DesignWare
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 21a1182..5cc4e19 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_SPI_TXX9)+= spi_txx9.o
obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
obj-$(CONFIG_SPI_SH_SCI) += spi_sh_sci.o
obj-$(CONFIG_SPI_STMP3XXX) += spi_stmp.o
+obj-$(CONFIG_SPI_DESIGNWARE) += dw_spi.o
# ... add above this line ...
# SPI protocol drivers (device/link on bus)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
new file mode 100644
index 000..3fd90a6
--- /dev/null
+++ b/drivers/spi/dw_spi.c
@@ -0,0 +1,941 @@
+/*
+ * dw_spi.c - Designware SPI core controller driver (refer pxa2xx_spi.c)
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include linux/dma-mapping.h
+#include linux/interrupt.h
+#include linux/highmem.h
+#include linux/delay.h
+
+#include linux/spi/dw_spi.h
+#include linux/spi/spi.h
+
+#ifdef CONFIG_DEBUG_FS
+#include linux/debugfs.h
+#endif
+
+#define START_STATE((void *)0)
+#define RUNNING_STATE ((void *)1)
+#define DONE_STATE ((void *)2)
+#define ERROR_STATE((void *)-1)
+
+#define QUEUE_RUNNING 0
+#define QUEUE_STOPPED 1
+
+#define MRST_SPI_DEASSERT 0
+#define MRST_SPI_ASSERT1
+
+/* Slave spi_dev related */
+struct chip_data {
+ u16 cr0;
+ u8 cs; /* chip select pin */
+ u8 n_bytes; /* current is a 1/2/4 byte op */
+ u8 tmode; /* TR/TO/RO/EEPROM */
+ u8 type;/* SPI/SSP/MicroWire */
+
+ u8 poll_mode; /* 1 means use poll mode */
+
+ u32 dma_width;
+ u32 rx_threshold;
+ u32 tx_threshold;
+ u8 enable_dma;
+ u8 bits_per_word;
+ u16 clk_div;/* baud rate divider */
+ u32 speed_hz; /* baud rate */
+ int (*write)(struct dw_spi *dws);
+ int (*read)(struct dw_spi *dws);
+ void (*cs_control)(u32 command);
+};
+
+#ifdef CONFIG_DEBUG_FS
+static int spi_show_regs_open(struct inode *inode, struct file *file)
+{
+ file-private_data = inode-i_private;
+ return 0;
+}
+
+#define SPI_REGS_BUFSIZE 1024
+static ssize_t spi_show_regs(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct dw_spi *dws;
+ char *buf;
+ u32 len = 0;
+ ssize_t ret;
+
+ dws = file-private_data;
+
+ buf = kzalloc(SPI_REGS_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return 0;
+
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ MRST SPI0 registers:\n);
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ =\n);
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ CTRL0: \t\t0x%08x\n, dw_readl(dws, ctrl0));
+ len += snprintf(buf + len
[spi-devel-general] [PATCH 3/3 v3][RFC] spi: DMA support for Designware core on Moorestown platform
From 347b16b69df7d75cc5ab7060a7c5311e2856b932 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 4 Aug 2009 16:42:31 +0800
Subject: [PATCH 3/3] spi: DMA support for Designware core on Moorestown platform
Designware core can work with multiple DMA controllers for DMA
operation, and this patch supports it to cowork with the Designware
DMA controller used on Intel Moorestown platform
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig|4 +
drivers/spi/dw_spi.c | 171 ++-
include/linux/spi/dw_spi.h |6 ++
3 files changed, 177 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index bd39e80..8132272 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -250,6 +250,10 @@ config SPI_DESIGNWARE
help
general driver for SPI controller core from DesignWare
+config SPI_DW_MRST_DMA
+ bool DMA support for DW SPI controller on Intel Moorestown platform
+ depends on SPI_DESGINWARE MRST_DMA
+
config SPI_DW_PCI
tristate PCI interface driver for DW SPI core
depends on SPI_DESIGNWARE PCI
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 3fd90a6..e9d3db4 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -29,6 +29,10 @@
#include linux/debugfs.h
#endif
+#ifdef CONFIG_SPI_MRST_DMA
+#include linux/lnw_dma.h
+#endif
+
#define START_STATE((void *)0)
#define RUNNING_STATE ((void *)1)
#define DONE_STATE ((void *)2)
@@ -62,6 +66,166 @@ struct chip_data {
void (*cs_control)(u32 command);
};
+#ifdef CONFIG_SPI_MRST_DMA
+static void dw_spi_dma_init(struct dw_spi *dws)
+{
+ struct lnw_dma_slave *rxs, *txs;
+ dma_cap_mask_t mask;
+
+ dws-txchan = NULL;
+ dws-rxchan = NULL;
+
+ /* 1. Init rx channel */
+ rxs = dws-dmas_rx;
+
+ rxs-dirn = DMA_FROM_DEVICE;
+ rxs-hs_mode = LNW_DMA_HW_HS;
+ rxs-cfg_mode = LNW_DMA_PER_TO_MEM;
+ rxs-src_width = LNW_DMA_WIDTH_16BIT;
+ rxs-dst_width = LNW_DMA_WIDTH_32BIT;
+ rxs-src_msize = LNW_DMA_MSIZE_16;
+ rxs-dst_msize = LNW_DMA_MSIZE_16;
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_MEMCPY, mask);
+ dma_cap_set(DMA_SLAVE, mask);
+
+ dws-rxchan = dma_request_channel(mask, NULL, NULL);
+ if (!dws-rxchan)
+ goto err_exit;
+ dws-rxchan-private = rxs;
+
+ /* 2. Init tx channel */
+ txs = dws-dmas_tx;
+
+ txs-dirn = DMA_TO_DEVICE;
+ txs-hs_mode = LNW_DMA_HW_HS;
+ txs-cfg_mode = LNW_DMA_MEM_TO_PER;
+ txs-src_width = LNW_DMA_WIDTH_32BIT;
+ txs-dst_width = LNW_DMA_WIDTH_16BIT;
+ txs-src_msize = LNW_DMA_MSIZE_16;
+ txs-dst_msize = LNW_DMA_MSIZE_16;
+
+ dma_cap_set(DMA_SLAVE, mask);
+ dma_cap_set(DMA_MEMCPY, mask);
+
+ dws-txchan = dma_request_channel(mask, NULL, NULL);
+ if (!dws-txchan)
+ goto free_rxchan;
+ dws-txchan-private = txs;
+
+ /* Set the dma done bit to 1 */
+ dws-dma_inited = 1;
+ dws-txdma_done = 1;
+ dws-rxdma_done = 1;
+
+ dws-tx_param = ((u64)(unsigned long)dws 32)
+ | (unsigned long)(dws-txdma_done);
+ dws-rx_param = ((u64)(unsigned long)dws 32)
+ | (unsigned long)(dws-rxdma_done);
+ return;
+
+free_rxchan:
+ dma_release_channel(dws-rxchan);
+err_exit:
+ return;
+}
+
+static void dw_spi_dma_exit(struct dw_spi *dws)
+{
+ dma_release_channel(dws-txchan);
+ dma_release_channel(dws-rxchan);
+}
+
+static void transfer_complete(struct dw_spi *dws);
+
+static void dw_spi_dma_done(void *arg)
+{
+ u64 *param = arg;
+ struct dw_spi *dws;
+ int *done;
+
+ dws = (struct dw_spi *)(unsigned long)(*param 32);
+ done = (int *)(unsigned long)(*param 0x);
+
+ *done = 1;
+ /* wait till both tx/rx channels are done */
+ if (!dws-txdma_done || !dws-rxdma_done)
+ return;
+
+ transfer_complete(dws);
+}
+
+static void dma_transfer(struct dw_spi *dws, int cs_change)
+{
+ struct dma_async_tx_descriptor *txdesc = NULL, *rxdesc = NULL;
+ struct dma_chan *txchan, *rxchan;
+ enum dma_ctrl_flags flag;
+ u16 dma_ctrl = 0;
+
+ /* 1. setup DMA related registers */
+ if (cs_change) {
+ spi_enable_chip(dws, 0);
+ dw_writew(dws, dmardlr, 0xf);
+ dw_writew(dws, dmatdlr, 0x10);
+ if (dws-tx_dma)
+ dma_ctrl |= 0x2;
+ if (dws-rx_dma)
+ dma_ctrl |= 0x1;
+ dw_writew(dws, dmacr, dma_ctrl);
+ spi_enable_chip(dws, 1);
+ }
+
+ if (dws-tx_dma)
+ dws-txdma_done = 0;
+ if (dws-rx_dma)
+ dws-rxdma_done = 0;
+
+ /* 2. start the TX dma transfer */
+ txchan
Re: [spi-devel-general] [PATCH 0/3 v3] spi: controller driver for Designware SPI core and PCI interface
On Tue, 13 Oct 2009 06:25:53 +0800 Andrew Morton a...@linux-foundation.org wrote: On Mon, 12 Oct 2009 14:17:16 +0800 Feng Tang feng.t...@intel.com wrote: This patch series adds driver for Designware SPI core + PCI interface, and they are generated against kernel 2.6.32-rc4 0001: driver for the DW SPI core 0002: driver for PCI interface 0003: add DMA support This driver has been verified on Intel Moorestown platform, with Maxim's Max3110 UART device and Option's 3G modem GTM501L, both PIO and DMA works fine, and these 2 slave devcies can work simultaneously. 0003 is RFC only, as it has dependency over DMA controller driver's acceptance to mainline. What is DMA controller driver? Some other patch? What is the status of that patch and where is it? Yes, it's another driver patch for DMA controller of Moorestown, which is developed by another guy. I don't know if he has submitted the patch to community (I guess not), though we did test the SPIC+DMAC+3G-Modem. Thanks, Feng -- Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ___ 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 0/3 v3] spi: controller driver for Designware SPI core and PCI interface
On Tue, 13 Oct 2009 09:53:27 +0800 Andrew Morton a...@linux-foundation.org wrote: 0003 is RFC only, as it has dependency over DMA controller driver's acceptance to mainline. What is DMA controller driver? Some other patch? What is the status of that patch and where is it? Yes, it's another driver patch for DMA controller of Moorestown, which is developed by another guy. I don't know if he has submitted the patch to community (I guess not), though we did test the SPIC+DMAC+3G-Modem. So am I correct in believing that your 0003: add DMA support doesn't actually work? That it depends on some other patch which is unavailable to testers and reviewers and patch integrators? Yes, it's fair enough to not trust the 0003 patch and remove it from mm tree :) That's why I added a [RFC] for it. Also many thanks for reviewing the patches, they have been not receiving any comments for a while. Thanks, Feng Strange. -- Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 0/3 v2] [RESEND]spi: controller driver for Designware SPI core and PCI interface
Hi all, This patch series adds driver for Designware SPI core + PCI interface, and they are generated against kernel 2.6.31 0001: driver for the DW SPI core 0002: driver for PCI interface 0003: add DMA support This driver has been verified on Intel Moorestown platform, with Maxim's Max3110 UART device and Option's 3G modem GTM501L, both PIO and DMA works fine, and these 2 slave devcies can work simultaneously. User can use dw_apb_ssi_db.pdf from Synopsys as HW datasheet please help to review them. Thanks, Feng -- Come build with us! The BlackBerryreg; Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9#45;12, 2009. Register now#33; http://p.sf.net/sfu/devconf ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 1/3 v2] [RESEND]spi: controller driver for Designware SPI core
From 5d87c87105aba522468cabb6fe206a4515dc85e8 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Tue, 4 Aug 2009 16:23:47 +0800
Subject: [PATCH 1/3] spi: controller driver for Designware SPI core
Driver for the Designware SPI core, it supports multipul interfaces
like PCI/APB etc. User can use dw_apb_ssi_db.pdf from Synopsys as
HW datasheet.
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig|6 +
drivers/spi/Makefile |1 +
drivers/spi/dw_spi.c | 941
include/linux/spi/dw_spi.h | 212 ++
4 files changed, 1160 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/dw_spi.c
create mode 100644 include/linux/spi/dw_spi.h
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 2c733c2..80b1017 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -227,6 +227,12 @@ config SPI_XILINX
See the OPB Serial Peripheral Interface (SPI) (v1.00e)
Product Specification document (DS464) for hardware details.
+config SPI_DESIGNWARE
+ bool DesignWare SPI controller core support
+ depends on SPI_MASTER
+ help
+ general driver for SPI controller core from DesignWare
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 3de408d..f3f36e0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SPI_S3C24XX) += spi_s3c24xx.o
obj-$(CONFIG_SPI_TXX9) += spi_txx9.o
obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
obj-$(CONFIG_SPI_SH_SCI) += spi_sh_sci.o
+obj-$(CONFIG_SPI_DESIGNWARE) += dw_spi.o
# ... add above this line ...
# SPI protocol drivers (device/link on bus)
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
new file mode 100644
index 000..261c27c
--- /dev/null
+++ b/drivers/spi/dw_spi.c
@@ -0,0 +1,941 @@
+/*
+ * dw_spi.c - Designware SPI core controller driver (refer pxa2xx_spi.c)
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include linux/dma-mapping.h
+#include linux/interrupt.h
+#include linux/highmem.h
+#include linux/delay.h
+
+#include linux/spi/dw_spi.h
+#include linux/spi/spi.h
+
+#ifdef CONFIG_DEBUG_FS
+#include linux/debugfs.h
+#endif
+
+#define START_STATE((void *)0)
+#define RUNNING_STATE ((void *)1)
+#define DONE_STATE ((void *)2)
+#define ERROR_STATE((void *)-1)
+
+#define QUEUE_RUNNING 0
+#define QUEUE_STOPPED 1
+
+#define MRST_SPI_DEASSERT 0
+#define MRST_SPI_ASSERT1
+
+/* Slave spi_dev related */
+struct chip_data {
+ u16 cr0;
+ u8 cs; /* chip select pin */
+ u8 n_bytes; /* current is a 1/2/4 byte op */
+ u8 tmode; /* TR/TO/RO/EEPROM */
+ u8 type;/* SPI/SSP/MicroWire */
+
+ u8 poll_mode; /* 1 means use poll mode */
+
+ u32 dma_width;
+ u32 rx_threshold;
+ u32 tx_threshold;
+ u8 enable_dma;
+ u8 bits_per_word;
+ u16 clk_div;/* baud rate divider */
+ u32 speed_hz; /* baud rate */
+ int (*write)(struct dw_spi *dws);
+ int (*read)(struct dw_spi *dws);
+ void (*cs_control)(u32 command);
+};
+
+#ifdef CONFIG_DEBUG_FS
+static int spi_show_regs_open(struct inode *inode, struct file *file)
+{
+ file-private_data = inode-i_private;
+ return 0;
+}
+
+#define SPI_REGS_BUFSIZE 1024
+static ssize_t spi_show_regs(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct dw_spi *dws;
+ char *buf;
+ u32 len = 0;
+ ssize_t ret;
+
+ dws = (struct dw_spi *)file-private_data;
+
+ buf = kzalloc(SPI_REGS_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return 0;
+
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ MRST SPI0 registers:\n);
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ =\n);
+ len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
+ CTRL0: \t\t0x%08x\n, dw_readl(dws, ctrl0));
+ len += snprintf
[spi-devel-general] [PATCH 1/2] spi: driver for SPI controller of Intel Moorestown platform
From 5aeff10330cef37792857c5f6977503607deb3b0 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Mon, 15 Jun 2009 16:50:44 +0800
Subject: [PATCH 1/2] spi: driver for SPI controller of Intel Moorestown platform
The controller is a PCI device, and supports both PIO and DMA mode.
This driver has been tested with Maxim's Max3110 UART device and
Option's 3G modem GTM501L, both PIO and DMA works fine, and these
2 slave devcies can work simultaneously
User can use dw_apb_ssi_db.pdf from Synopsys as HW datasheet
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig |9 +
drivers/spi/Makefile |1 +
drivers/spi/mrst_spi.c | 1153 ++
include/linux/spi/mrst_spi.h | 143 ++
4 files changed, 1306 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/mrst_spi.c
create mode 100644 include/linux/spi/mrst_spi.h
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 83a185d..e1cba00 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -220,6 +220,15 @@ config SPI_XILINX
See the OPB Serial Peripheral Interface (SPI) (v1.00e)
Product Specification document (DS464) for hardware details.
+config SPI_MRST
+ tristate SPI controller driver for Intel Moorestown platform
+ depends on SPI_MASTER PCI
+ help
+ Driver for SPI controller on Intel Moorestown platform.
+
+ The controller is a PCI device, and its core is a Synopsis
+ DesignWare SPI controller.
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 5d04519..d07825d 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_SPI_S3C24XX) += spi_s3c24xx.o
obj-$(CONFIG_SPI_TXX9) += spi_txx9.o
obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
obj-$(CONFIG_SPI_SH_SCI) += spi_sh_sci.o
+obj-$(CONFIG_SPI_MRST) += mrst_spi.o
# ... add above this line ...
# SPI protocol drivers (device/link on bus)
diff --git a/drivers/spi/mrst_spi.c b/drivers/spi/mrst_spi.c
new file mode 100644
index 000..99a1129
--- /dev/null
+++ b/drivers/spi/mrst_spi.c
@@ -0,0 +1,1153 @@
+/*
+ * mrst_spi.c
+ *
+ * SPI controller driver for Intel Moorestown platform (refer pxa2xx_spi.c)
+ *
+ * Copyright (C) Intel 2008 Feng Tang feng.t...@intel.com
+ *
+ * ~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * ~~
+ *
+ */
+
+#include linux/delay.h
+#include linux/highmem.h
+#include linux/pci.h
+#include linux/dma-mapping.h
+#include linux/interrupt.h
+
+#include linux/spi/spi.h
+#include linux/spi/mrst_spi.h
+
+#ifdef CONFIG_DEBUG_FS
+#include linux/debugfs.h
+#endif
+
+#define DRIVER_NAME mrst_spi
+
+#define START_STATE((void *)0)
+#define RUNNING_STATE ((void *)1)
+#define DONE_STATE ((void *)2)
+#define ERROR_STATE((void *)-1)
+
+#define QUEUE_RUNNING 0
+#define QUEUE_STOPPED 1
+
+#define MRST_SPI_DEASSERT 0
+#define MRST_SPI_ASSERT1
+
+#define MRST_SPIC_MAX_FREQ 2500
+
+/* per controller struct */
+struct driver_data {
+ /* Driver model hookup */
+ struct pci_dev *pdev;
+ struct spi_master *master;
+
+ struct spi_device *devices;
+ struct spi_device *cur_dev;
+ enum mrst_ssi_type type;
+
+ /* phy and virtual register addresses */
+ void *paddr;
+ void *vaddr;
+ u32 iolen;
+ int irq;
+
+ /* Driver message queue */
+ struct workqueue_struct *workqueue;
+ struct work_struct pump_messages;
+ spinlock_t lock;
+ struct list_head queue;
+ int busy;
+ int run;
+
+ /* Message Transfer pump */
+ struct tasklet_struct pump_transfers;
+
+ /* Current message transfer state info */
+ struct spi_message *cur_msg;
+ struct spi_transfer *cur_transfer;
+ struct chip_data *cur_chip;
+ struct chip_data *prev_chip;
+ size_t len;
+ void *tx;
+ void *tx_end;
+ void *rx;
+ void *rx_end
[spi-devel-general] [PATCH 0/2] spi: add driver for SPI controller of Intel Moorestown platform
Hi David, This patch series adds driver for SPI controller of Intel Moorestown platform. This driver has been tested with Maxim's Max3110 UART device and Option's 3G modem GTM501L, both PIO and DMA works fine, and these 2 slave devcies can work simultaneously User can use dw_apb_ssi_db.pdf from Synopsys as HW datasheet please help to review them. I haven't seen your mail on this list for a while, so I cc' Andrew and Alan for their reviews as well. Thanks, Feng -- Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[spi-devel-general] [PATCH 2/2] spi: add DMA support for Intel Moorestown SPI controller
From 6dbcddf7fefbef8b8828ecb358dc177f37685c46 Mon Sep 17 00:00:00 2001
From: Feng Tang feng.t...@intel.com
Date: Mon, 15 Jun 2009 17:00:11 +0800
Subject: [PATCH 2/2] spi: add DMA support for Intel Moorestown SPI controller
The DMA has been only tested with the DMA controller 2 on the
Moorestown platform
Signed-off-by: Feng Tang feng.t...@intel.com
---
drivers/spi/Kconfig|7 ++
drivers/spi/mrst_spi.c | 208
2 files changed, 215 insertions(+), 0 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index e1cba00..f7d82ea 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -229,6 +229,13 @@ config SPI_MRST
The controller is a PCI device, and its core is a Synopsis
DesignWare SPI controller.
+config SPI_MRST_DMA
+ boolean Enable DMA for MRST SPI0 controller
+ default y
+ depends on SPI_MRST INTEL_LNW_DMAC2
+ help
+ This has to be enabled after Moorestown DMAC2 driver is enabled
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/mrst_spi.c b/drivers/spi/mrst_spi.c
index 99a1129..11fae26 100644
--- a/drivers/spi/mrst_spi.c
+++ b/drivers/spi/mrst_spi.c
@@ -34,6 +34,10 @@
#include linux/spi/spi.h
#include linux/spi/mrst_spi.h
+#ifdef CONFIG_SPI_MRST_DMA
+#include linux/lnw_dma.h
+#endif
+
#ifdef CONFIG_DEBUG_FS
#include linux/debugfs.h
#endif
@@ -109,6 +113,19 @@ struct driver_data {
#endif
int dma_inited;
+
+#ifdef CONFIG_SPI_MRST_DMA
+ struct lnw_dma_slavedmas_tx;
+ struct lnw_dma_slavedmas_rx;
+ struct dma_chan *txchan;
+ struct dma_chan *rxchan;
+ int txdma_done;
+ int rxdma_done;
+
+ u64 tx_param;
+ u64 rx_param;
+ struct pci_dev *dma_dev;
+#endif
};
/* slave spi_dev related */
@@ -136,6 +153,128 @@ struct chip_data {
void (*cs_control)(u32 command);
};
+#ifdef CONFIG_SPI_MRST_DMA
+static bool chan_filter(struct dma_chan *chan, void *param)
+{
+ struct driver_data *drv_data = param;
+ bool ret = false;
+
+ if (chan-device-dev == drv_data-dma_dev-dev)
+ ret = true;
+ return ret;
+}
+
+static void mrst_spi_dma_init(struct driver_data *drv_data)
+{
+ struct lnw_dma_slave *rxs, *txs;
+ dma_cap_mask_t mask;
+ struct pci_dev *dmac2;
+
+ drv_data-txchan = NULL;
+ drv_data-rxchan = NULL;
+
+ /* mrst spi0 controller only work with mrst dma contrller 2 */
+ dmac2 = pci_get_device(PCI_VENDOR_ID_INTEL, 0x0813, NULL);
+ if (!dmac2) {
+ printk(KERN_WARNING
+ MRST SPI0: can't find DMAC2, dma init failed\n);
+ return;
+ } else
+ drv_data-dma_dev = dmac2;
+
+ /* 1. init rx channel */
+ rxs = drv_data-dmas_rx;
+
+ rxs-dirn = DMA_FROM_DEVICE;
+ rxs-hs_mode = LNW_DMA_HW_HS;
+ rxs-cfg_mode = LNW_DMA_PER_TO_MEM;
+ rxs-src_width = LNW_DMA_WIDTH_16BIT;
+ rxs-dst_width = LNW_DMA_WIDTH_32BIT;
+ rxs-src_msize = LNW_DMA_MSIZE_16;
+ rxs-dst_msize = LNW_DMA_MSIZE_16;
+
+ rxs-rx_reg = (dma_addr_t)(drv_data-paddr + 0x60);
+ rxs-tx_reg = (dma_addr_t)0;
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_MEMCPY, mask);
+ dma_cap_set(DMA_SLAVE, mask);
+
+ drv_data-rxchan = dma_request_channel(mask, chan_filter,
+ drv_data);
+ if (!drv_data-rxchan)
+ goto err_exit;
+ drv_data-rxchan-private = rxs;
+
+ /* 2. init tx channel */
+ txs = drv_data-dmas_tx;
+
+ txs-dirn = DMA_TO_DEVICE;
+ txs-hs_mode = LNW_DMA_HW_HS;
+ txs-cfg_mode = LNW_DMA_MEM_TO_PER;
+ txs-src_width = LNW_DMA_WIDTH_32BIT;
+ txs-dst_width = LNW_DMA_WIDTH_16BIT;
+ txs-src_msize = LNW_DMA_MSIZE_16;
+ txs-dst_msize = LNW_DMA_MSIZE_16;
+
+ txs-tx_reg = (dma_addr_t)(drv_data-paddr + 0x60);
+ txs-rx_reg = (dma_addr_t)0;
+ dma_cap_set(DMA_SLAVE, mask);
+ dma_cap_set(DMA_MEMCPY, mask);
+
+ drv_data-txchan = dma_request_channel(mask, chan_filter,
+ drv_data);
+ if (!drv_data-txchan)
+ goto free_rxchan;
+ drv_data-txchan-private = txs;
+
+ /* set the dma done bit to 1 */
+ drv_data-dma_inited = 1;
+ drv_data-txdma_done = 1;
+ drv_data-rxdma_done = 1;
+
+ drv_data-tx_param = ((u64)(u32)drv_data 32)
+ | (u32)(drv_data-txdma_done);
+ drv_data-rx_param = ((u64)(u32)drv_data 32)
+ | (u32)(drv_data-rxdma_done);
+ return;
+
+free_rxchan:
+ dma_release_channel(drv_data-rxchan);
+err_exit:
+ pci_dev_put(dmac2);
+ return;
+}
+
+static void mrst_spi_dma_exit(struct driver_data *drv_data)
+{
+ dma_release_channel(drv_data
