diff mbox

[2/5] Davinci: DM365: Adding support for SPI in DM365 SOC

Message ID 1247663826-5765-1-git-send-email-s-paulraj@ti.com (mailing list archive)
State Superseded
Headers show

Commit Message

s-paulraj@ti.com July 15, 2009, 1:17 p.m. UTC
From: Sandeep Paulraj <s-paulraj@ti.com>

The patch adds support for SPI in DM365 SOC.
A short explanation as to how to modify platform
data if GPIOs are used as chip selects is also given

Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
---
 arch/arm/mach-davinci/dm365.c              |   65 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/dm365.h |    3 +
 2 files changed, 68 insertions(+), 0 deletions(-)

Comments

David Brownell July 15, 2009, 5:38 p.m. UTC | #1
On Wednesday 15 July 2009, s-paulraj@ti.com wrote:
> +/*
> + * The davinci_spi_platform_data can have a "chip_sel" array of GPIOs
> + * For example :
> + * static u8 dm365_spi0_chip_sel[] = {23, 45, 68};

OK -- although troublesome to have *here* in two respects:

 (a) No such board-specific stuff should ever get stuck into
     these SoC-specific setup files.
 (b) The mechanism isn't SoC-specific anyway.

I suggest a better way to do this is let the board provide
GPIO config data to the init_spi0() call, with (though you
seem to disagree) doc in the driver itself.  But ...


> + * This array should then become be a part of the platform data
> + * .chip_sel = dm365_spi0_chip_sel,
> + * Appropriately change the 'num_chipselect' in the structure below
> + */
> +
> +static struct davinci_spi_platform_data dm365_spi0_pdata = {
> ...


> +void __init dm365_init_spi0(unsigned chipselect_mask,
> +               struct spi_board_info *info, unsigned len)
> +{
> +       davinci_cfg_reg(DM365_SPI0_SCLK);
> +       davinci_cfg_reg(DM365_SPI0_SDI);
> +       davinci_cfg_reg(DM365_SPI0_SDO);
> +
> +       /* not all slaves will be wired up */
> +       if (chipselect_mask & BIT(0))
> +               davinci_cfg_reg(DM365_SPI0_SDENA0);
> +       if (chipselect_mask & BIT(1))
> +               davinci_cfg_reg(DM365_SPI0_SDENA1);

... here you require the first two chip selects to be managed
using the standard chip select lines.  So it's not handling
the GPIO scenario correctly.

All things considered ... why don't you just remove the GPIO
chipselect support for the initial merge?  I agree it would
be a good thing to have-- two chipselects is weak! -- but it's
not *needed* just yet, and I'd rather not drag out this first
phase any longer.


> +
> +       spi_register_board_info(info, len);
> +
> +       platform_device_register(&dm365_spi0_device);
> +}
diff mbox

Patch

diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index f02bce8..3090cb8 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -19,6 +19,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/gpio.h>
+#include <linux/spi/spi.h>
 
 #include <asm/mach/map.h>
 
@@ -32,6 +33,7 @@ 
 #include <mach/time.h>
 #include <mach/serial.h>
 #include <mach/common.h>
+#include <mach/spi.h>
 
 #include "clock.h"
 #include "mux.h"
@@ -597,6 +599,69 @@  INT_CFG(DM365,  INT_EMAC_MISCPULSE,  17,    1,    1,     false)
 #endif
 };
 
+static u64 dm365_spi0_dma_mask = DMA_BIT_MASK(32);
+
+/*
+ * The davinci_spi_platform_data can have a "chip_sel" array of GPIOs
+ * For example :
+ * static u8 dm365_spi0_chip_sel[] = {23, 45, 68};
+ * This array should then become be a part of the platform data
+ * .chip_sel = dm365_spi0_chip_sel,
+ * Appropriately change the 'num_chipselect' in the structure below
+ */
+
+static struct davinci_spi_platform_data dm365_spi0_pdata = {
+	.version 	= DAVINCI_SPI_VERSION_1,
+	.num_chipselect = 2,
+	.clk_internal	= 1,
+	.cs_hold	= 1,
+	.intr_level	= 0,
+	.poll_mode	= 1,
+	.c2tdelay	= 8,
+	.t2cdelay	= 8,
+};
+
+static struct resource dm365_spi0_resources[] = {
+	{
+		.start = 0x01c66000,
+		.end   = 0x01c667ff,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = IRQ_DM365_SPIINT0_0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device dm365_spi0_device = {
+	.name = "spi_davinci",
+	.id = 0,
+	.dev = {
+		.dma_mask = &dm365_spi0_dma_mask,
+		.coherent_dma_mask = DMA_BIT_MASK(32),
+		.platform_data = &dm365_spi0_pdata,
+	},
+	.num_resources = ARRAY_SIZE(dm365_spi0_resources),
+	.resource = dm365_spi0_resources,
+};
+
+void __init dm365_init_spi0(unsigned chipselect_mask,
+		struct spi_board_info *info, unsigned len)
+{
+	davinci_cfg_reg(DM365_SPI0_SCLK);
+	davinci_cfg_reg(DM365_SPI0_SDI);
+	davinci_cfg_reg(DM365_SPI0_SDO);
+
+	/* not all slaves will be wired up */
+	if (chipselect_mask & BIT(0))
+		davinci_cfg_reg(DM365_SPI0_SDENA0);
+	if (chipselect_mask & BIT(1))
+		davinci_cfg_reg(DM365_SPI0_SDENA1);
+
+	spi_register_board_info(info, len);
+
+	platform_device_register(&dm365_spi0_device);
+}
 static struct emac_platform_data dm365_emac_pdata = {
 	.ctrl_reg_offset	= DM365_EMAC_CNTRL_OFFSET,
 	.ctrl_mod_reg_offset	= DM365_EMAC_CNTRL_MOD_OFFSET,
diff --git a/arch/arm/mach-davinci/include/mach/dm365.h b/arch/arm/mach-davinci/include/mach/dm365.h
index 09db434..61cfd30 100644
--- a/arch/arm/mach-davinci/include/mach/dm365.h
+++ b/arch/arm/mach-davinci/include/mach/dm365.h
@@ -25,5 +25,8 @@ 
 #define DM365_EMAC_CNTRL_RAM_SIZE	(0x2000)
 
 void __init dm365_init(void);
+struct spi_board_info;
+void dm365_init_spi0(unsigned chipselect_mask,
+		struct spi_board_info *info, unsigned len);
 
 #endif /* __ASM_ARCH_DM365_H */