SPI: DaVinci: Adding SPI driver for DaVinci

PATCH In review Pablo Bitton 3 participants 4 messages Aug 17, 2009 – Aug 25, 2009 1 message not in the archive

Subsystems (4)

Arm (32-bit) Arm sub-architectures SPI TI DaVinci machine support

lore

Share & tools

lore

Pablo Bitton

author

2009-08-17 14:27 UTC #0 lore raw

PATCH v2 1/1 SPI: DaVinci: Adding SPI driver for DaVinci

This patch adds support for SPI in DaVinci DM6446
(and tries to keep support for DM355/DM365/DM6467 and DA8xx).
Mostly the same as the patch by Sandeep Paulraj.

This has been tested on the DM6446 by defining a spidev device and using a scope (to check correctness) and a hardware loopback.
This was NOT tested on DM355, DM365 and DM6467 - in fact, it will probably not work "as is" because its default mode is CS low-inactive (default mode of SPI in kernel) - need to set CS_HIGH mode to work as in the previous patch.

Changes from the patch by Sandeep Paulraj:

Bug fixes:
* Additional word written with chip select up after each transfer. Particulary problematic with NO_CS mode where this word can't be distiguished from correct words. Problem was in davinci_chip_select.
* setup() for one chip select may interfere with transfer for another
* Small nitpicks (bits that can be changed only on VERSION_2)

Features added:
* Support DM6446
* Support CS_HIGH mode (using SPIDEF register). Note that CS low is default.

Other:
* Less accesses to registers used.
* Once-per-device configuration is done only in probe(), not each transfer.

Uglyness still there:
* VERSION_X definitions for different SPI controllers - added VERSION_3 for the dm6446, which is ugly.

NOTE:
This patch is based on following patches:

SPI: DaVinci: Adding SPI driver for DM3xx/DM6467/DA8xx

The patch adds support for SPI in DaVinci
DM355/DM365/DM6467 and DA8xx.

This has been tested on the DM355, DM365 and DM6467 EVMs using the EEPROM connected to SPI0

Signed-off-by Sandeep Paulraj <[email protected]>

DaVinci: DM646x: Adding Support for SPI

The patch does the following

1) Adds a clock for SPI
2) Defines resources specific to DM646x SOC

Signed-off-by Sandeep Paulraj <[email protected]>

Signed-off-by Pablo Bitton <[email protected]>

---
 arch/arm/mach-davinci/dm644x.c              |   50 ++-
 arch/arm/mach-davinci/dm646x.c              |   53 ++
 arch/arm/mach-davinci/include/mach/dm644x.h |    3 +
 arch/arm/mach-davinci/include/mach/dm646x.h |    2 +
 arch/arm/mach-davinci/include/mach/spi.h    |   46 ++
 drivers/spi/Kconfig                         |    7 +
 drivers/spi/Makefile                        |    1 +
 drivers/spi/davinci_spi.c                   |  771 +++++++++++++++++++++++++++
 drivers/spi/davinci_spi.h                   |  178 ++++++
 12 files changed, 1149 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-davinci/include/mach/spi.h
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

arch/arm/mach-davinci/dm644x.c +49 -1

index 55317b1..d395354 100644
@@ -14,6 +14,7 @@
 #include <linux/serial_8250.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
+#include <linux/spi/spi.h>
 
 #include <asm/mach/map.h>

@@ -28,6 +29,7 @@
 #include <mach/serial.h>
 #include <mach/common.h>
 #include <mach/asp.h>
+#include <mach/spi.h>
 
 #include "clock.h"
 #include "mux.h"

@@ -306,7 +308,7 @@ struct davinci_clk dm644x_clks[] = {
 	CLK("palm_bk3710", NULL, &ide_clk),
 	CLK("davinci-asp", NULL, &asp_clk),
 	CLK("davinci_mmc.0", NULL, &mmcsd_clk),
-	CLK(NULL, "spi", &spi_clk),
+	CLK("spi_davinci.0", NULL, &spi_clk),
 	CLK(NULL, "gpio", &gpio_clk),
 	CLK(NULL, "usb", &usb_clk),
 	CLK(NULL, "vlynq", &vlynq_clk),

@@ -320,6 +322,52 @@ struct davinci_clk dm644x_clks[] = {
 	CLK(NULL, NULL, NULL),
 };
 
+
+static u64 dm644x_spi_dma_mask = DMA_BIT_MASK(32);
+
+static struct davinci_spi_platform_data dm644x_spi_pdata = {
+	.version 	= SPI_VERSION_3,
+	.num_chipselect = 2,
+	.clk_internal	= 1,
+	.cs_hold	= 0,
+	.intr_level	= 0,
+	.poll_mode	= 1,
+	.c2tdelay	= 8,
+	.t2cdelay	= 8,
+};
+
+static struct resource dm644x_spi_resources[] = {
+	{
+		.start = 0x01c66800,
+		.end   = 0x01c66fff,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = IRQ_SPINT0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device dm644x_spi_device = {
+	.name = "spi_davinci",
+	.id = 0,
+	.dev = {
+		.dma_mask = &dm644x_spi_dma_mask,
+		.coherent_dma_mask = DMA_BIT_MASK(32),
+		.platform_data = &dm644x_spi_pdata,
+	},
+	.num_resources = ARRAY_SIZE(dm644x_spi_resources),
+	.resource = dm644x_spi_resources,
+};
+
+void __init dm644x_init_spi(struct spi_board_info *info, unsigned len)
+{
+	spi_register_board_info(info, len);
+
+	platform_device_register(&dm644x_spi_device);
+}
+
+
 static struct emac_platform_data dm644x_emac_pdata = {
 	.ctrl_reg_offset	= DM644X_EMAC_CNTRL_OFFSET,
 	.ctrl_mod_reg_offset	= DM644X_EMAC_CNTRL_MOD_OFFSET,

arch/arm/mach-davinci/dm646x.c +53 -0

83 lines not on this page · raw message

arch/arm/mach-davinci/include/mach/dm644x.h +3 -0

index 8b157ce..0710596 100644
@@ -36,5 +36,8 @@
 
 void __init dm644x_init(void);
 void __init dm644x_init_asp(struct snd_platform_data *pdata);
+struct spi_board_info;
+void __init dm644x_init_spi(struct spi_board_info *info, unsigned len);
+
 
 #endif /* __ASM_ARCH_DM644X_H */

arch/arm/mach-davinci/include/mach/dm646x.h +2 -0

index 0585484..498163f 100644
@@ -25,5 +25,7 @@
 void __init dm646x_init_ide(void);
 void __init dm646x_init_mcasp0(struct snd_platform_data *pdata);
 void __init dm646x_init_mcasp1(struct snd_platform_data *pdata);
+struct spi_board_info;
+void dm646x_init_spi0(struct spi_board_info *info, unsigned len);
 
 void dm646x_video_init(void);

arch/arm/mach-davinci/include/mach/spi.h new file +46 -0

46 lines not on this page · raw message

drivers/spi/Kconfig +7 -0

index 2c733c2..7fe9211 100644
@@ -77,6 +77,13 @@ config SPI_AU1550
 	  This driver can also be built as a module.  If so, the module
 	  will be called au1550_spi.
 
+config SPI_DAVINCI
+	tristate "SPI controller driver for DaVinci/DA8xx SoC's"
+	depends on SPI_MASTER && ARCH_DAVINCI
+	select SPI_BITBANG
+	help
+	  SPI master controller for DaVinci and DA8xx SPI modules.
+
 config SPI_BITBANG
 	tristate "Utilities for Bitbanging SPI masters"
 	help

drivers/spi/Makefile +1 -0

index 3de408d..910ac6d 100644
@@ -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_DAVINCI)		+= davinci_spi.o
 # 	... add above this line ...
 
 # SPI protocol drivers (device/link on bus)

drivers/spi/davinci_spi.c new file +776 -0

776 lines not on this page · raw message

drivers/spi/davinci_spi.h new file +178 -0

178 lines not on this page · raw message

Signature · 5 lines

1.5.4.3


-- 
Pablo Bitton <[email protected]>

A message that is not in the archive (sent off-list or not yet received): [email protected]

Kevin Hilman

2009-08-25 13:20 UTC #2 lore raw

Quoted 6 lines · Pablo Bitton

The patch has received no comments so far (here and on spi-general-devel).
Can someone test it on davinci's other that the DM6446 to see that support for others has not broken?
Kevin - Is there anything that keeps it from merging upstream to this tree?

Hi Pablo,

Sorry for the delay, I've been travelling and not able to watch DaVinci closely enough...

This driver should be merged via the SPI subsystem (maintained by David Brownell), not the Davinci core code which I maintain.

That being said, in my view, here's why this driver is not ready for upstream:

1) The original driver from Sandeep that you based yours on was still going through revisions. The last review comments[1] from David Brownell had not yet been addressed by Sandeep. I hope that Sandeep will have a chance to address the existing review comments on his code, and then review yours. However, you've made it rather difficult to do that because...

2) You should have your patch apply on top of Sandeep's series, not just absorb it. That way we can clearly see what you are adding and/or changing from Sandeeps original driver. To make this part easier, I created a 'temp/spi' branch of davinci git where I've pushed the latest versions of the patches from Sandeep. Any additions/updates/fixes you have should be posted as patches against that for easier discussion and review.

3) As Sandeep did, you should keep the changes to the board/SoC code (arch/arm/mach-davinci/*) as a separate patch from the driver code (drivers/spi/*)

4) this driver needs more testing

Quoted 3 lines

So far, the patch has been added to the -mm tree - http://git.zen-sources.org/?
p=mmotm.git;a=commit;h=b693ea09ae2fb5c382ef7f2772d6115af1f9b4fc. Its filename is spi-davinci-adding-spi-driver-for-davinci.patch.

Andrew, I would recommend dropping this from -mm until the above issues are addressed.

Thanks,

Kevin (maintainer: arch/arm/mach-davinci/*)

[1] http://linux.omap.com/pipermail/davinci-linux-open-source/2009-July/014828.html

Quoted 62 lines · Pablo Bitton

This patch adds support for SPI in DaVinci DM6446 (and tries to keep support for DM355/DM365/DM6467 and DA8xx). Mostly the same as the patch by Sandeep Paulraj.
This has been tested on the DM6446 by defining a spidev device and using a scope (to check correctness) and a hardware loopback.
This was NOT tested on DM355, DM365 and DM6467 - in fact, it will probably not work "as is" because its default mode is CS low-inactive (default mode of SPI in kernel) - need to set CS_HIGH mode to work as in the previous patch.
Changes from the patch by Sandeep Paulraj:
Bug fixes:
* Additional word written with chip select up after each transfer. Particulary problematic with NO_CS mode where this word can't be distiguished from correct words. Problem was in davinci_chip_select.
* setup() for one chip select may interfere with transfer for another
* Small nitpicks (bits that can be changed only on VERSION_2)
Features added:
* Support DM6446
* Support CS_HIGH mode (using SPIDEF register). Note that CS low is default.
Other:
* Less accesses to registers used.
* Once-per-device configuration is done only in probe(), not each transfer.
Uglyness still there:
* VERSION_X definitions for different SPI controllers - added VERSION_3 for the dm6446, which is ugly.
NOTE:
This patch is based on following patches:
SPI: DaVinci: Adding SPI driver for DM3xx/DM6467/DA8xx
The patch adds support for SPI in DaVinci
DM355/DM365/DM6467 and DA8xx.
This has been tested on the DM355, DM365 and DM6467 EVMs using the EEPROM connected to SPI0
Signed-off-by Sandeep Paulraj <[email protected]>
DaVinci: DM646x: Adding Support for SPI
The patch does the following
1) Adds a clock for SPI
2) Defines resources specific to DM646x SOC
Signed-off-by Sandeep Paulraj <[email protected]>
Signed-off-by Pablo Bitton <[email protected]>

Paulraj, Sandeep

2009-08-25 13:56 UTC #3 lore raw

Kevin,

Please see inline

Quoted 22 lines

Quoted 2 lines
The patch has received no comments so far (here and on spi-general-
devel).
Quoted 2 lines
Can someone test it on davinci's other that the DM6446 to see that
support for
Quoted 3 lines
others has not broken?
Kevin - Is there anything that keeps it from merging upstream to this
tree?
Hi Pablo,
Sorry for the delay, I've been travelling and not able to watch DaVinci closely enough...
This driver should be merged via the SPI subsystem (maintained by David Brownell), not the Davinci core code which I maintain.
That being said, in my view, here's why this driver is not ready for upstream:

Quoted 6 lines

1) The original driver from Sandeep that you based yours on was still going through revisions. The last review comments[1] from David Brownell had not yet been addressed by Sandeep. I hope that Sandeep will have a chance to address the existing review comments on his code, and then review yours. However, you've made it rather difficult to do that because...

[Sandeep] There were a set of comments from David Brownell(which was actually, thanks to him, in the from of a patch).
David did say that the SPI support in that form was ready for an initial merge. I tested it on DM355/Dm365 and Dm6467 and that driver(meant for the initial merge) is in our ARAGO tree. Afcourse we all agreed that there are things to add in the SPI driver.
Also IIRC(and I am willing to be corrected) David did say that he would send it upstream when he got some time so I did not do it myself. The fact that he maintains the SPI subsystem had a part to play in my decision.

Quoted 9 lines

2) You should have your patch apply on top of Sandeep's series, not just absorb it. That way we can clearly see what you are adding and/or changing from Sandeeps original driver. To make this part easier, I created a 'temp/spi' branch of davinci git where I've pushed the latest versions of the patches from Sandeep. Any additions/updates/fixes you have should be posted as patches against that for easier discussion and review.

Quoted 5 lines

3) As Sandeep did, you should keep the changes to the board/SoC code (arch/arm/mach-davinci/*) as a separate patch from the driver code (drivers/spi/*)
4) this driver needs more testing

[Sandeep] tested by TI test team on DM355 and DM365 and I have tested on DM6467.

Quoted 6 lines

Quoted 2 lines
So far, the patch has been added to the -mm tree - http://git.zen-
sources.org/?
Quoted 2 lines
p=mmotm.git;a=commit;h=b693ea09ae2fb5c382ef7f2772d6115af1f9b4fc. Its filename is spi-davinci-adding-spi-driver-for-davinci.patch.

Quoted 6 lines

Andrew, I would recommend dropping this from -mm until the above issues are addressed.
Thanks,
Kevin (maintainer: arch/arm/mach-davinci/*)

Thanks,
Sandeep

Kevin Hilman

2009-08-25 14:29 UTC #4 lore raw

Quoted 33 lines · Sandeep

Kevin,
Please see inline
Quoted 22 lines
Quoted 2 lines
The patch has received no comments so far (here and on spi-general-
devel).
Quoted 2 lines
Can someone test it on davinci's other that the DM6446 to see that
support for
Quoted 3 lines
others has not broken?
Kevin - Is there anything that keeps it from merging upstream to this
tree?
Hi Pablo,
Sorry for the delay, I've been travelling and not able to watch DaVinci closely enough...
This driver should be merged via the SPI subsystem (maintained by David Brownell), not the Davinci core code which I maintain.
That being said, in my view, here's why this driver is not ready for upstream:
Quoted 6 lines
1) The original driver from Sandeep that you based yours on was still going through revisions. The last review comments[1] from David Brownell had not yet been addressed by Sandeep. I hope that Sandeep will have a chance to address the existing review comments on his code, and then review yours. However, you've made it rather difficult to do that because...

Quoted 10 lines

[Sandeep] There were a set of comments from David Brownell(which was actually, thanks to him, in the from of a patch). David did say that the SPI support in that form was ready for an initial merge. I tested it on DM355/Dm365 and Dm6467 and that driver(meant for the initial merge) is in our ARAGO tree. Afcourse we all agreed that there are things to add in the SPI driver. Also IIRC(and I am willing to be corrected) David did say that he would send it upstream when he got some time so I did not do it myself. The fact that he maintains the SPI subsystem had a part to play in my decision.

OK, I'll continue the original thread with some more questions on this topic.

Quoted 9 lines

Quoted 9 lines
2) You should have your patch apply on top of Sandeep's series, not just absorb it. That way we can clearly see what you are adding and/or changing from Sandeeps original driver. To make this part easier, I created a 'temp/spi' branch of davinci git where I've pushed the latest versions of the patches from Sandeep. Any additions/updates/fixes you have should be posted as patches against that for easier discussion and review.

Quoted 7 lines

Quoted 5 lines
3) As Sandeep did, you should keep the changes to the board/SoC code (arch/arm/mach-davinci/*) as a separate patch from the driver code (drivers/spi/*)
4) this driver needs more testing
[Sandeep] tested by TI test team on DM355 and DM365 and I have tested on DM6467.

Sandeep, your driver recieved sufficient testing. The driver that I was saying needs more changes was this one proposed by Pablo.

Kevin