[PATCH v1 0/4] davinci: Add SPI support for da8xx platforms

[PATCH v1 0/4] davinci: Add SPI support for da8xx platforms

[Michael Williamson]

This patch series adds the necessary SPI resources and registration
routines for da850/OMAP-L138/AM18x and da830/OMAP-L137/AM17x devices.
It also adds on-board SPI FLASH devices for the da830 evm, the da850
evm, and the MityDSP-L138/MityARM-1808 platforms.

These patches are based on work done during testing of davinci SPI driver 
submissions incorporated in version 2.6.38 of the kernel, at [1].

The da850 and da830 EVM portions of this patch need verification / ack.

The MityDSP-L138 platform patch has been tested.

The patch series is against commit 8fb837c158e342413d08d6f211b5b8f67d5adc2f
of kevin's tree.

[1] http://arago-project.org/git/projects/?p=linux-davinci.git;a=shortlog;h=refs/heads/davinci-spi-rewrite

---
Changes since v0:
   - fixed up issues in device stuctures pointed out by Sergei
   - moved spi platform data structures to common array in devices-da8xx.c
   - Added sign-off information for Sekhar's patches per comments.

Michael Williamson (2):
  davinci: da8xx/omap-l1: add support for SPI
  davinci: add spi devices support for MityDSP-L138/MityARM-1808
    platform

Sekhar Nori (2):
  davinci: add spi devices support for da850/omap-l138/am18x evm
  davinci: add spi devices support for da830/omap-l137/am17x evm

 arch/arm/mach-davinci/board-da830-evm.c    |   78 ++++++++++++++++++++++
 arch/arm/mach-davinci/board-da850-evm.c    |   84 +++++++++++++++++++++++
 arch/arm/mach-davinci/board-mityomapl138.c |  100 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/da850.c              |   16 +++++
 arch/arm/mach-davinci/devices-da8xx.c      |   96 ++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |    3 +
 6 files changed, 377 insertions(+), 0 deletions(-)

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

Add SPI registration routines, clocks, and driver resources for
DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
---
 arch/arm/mach-davinci/da850.c              |   16 +++++
 arch/arm/mach-davinci/devices-da8xx.c      |   96 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |    3 +
 3 files changed, 115 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 3443d97..68fe4c2 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -359,6 +359,20 @@ static struct clk usb20_clk = {
 	.gpsc		= 1,
 };
 
+static struct clk spi0_clk = {
+	.name		= "spi0",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC0_SPI0,
+};
+
+static struct clk spi1_clk = {
+	.name		= "spi1",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC1_SPI1,
+	.gpsc		= 1,
+	.flags		= DA850_CLK_ASYNC3,
+};
+
 static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"ref",		&ref_clk),
 	CLK(NULL,		"pll0",		&pll0_clk),
@@ -403,6 +417,8 @@ static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"aemif",	&aemif_clk),
 	CLK(NULL,		"usb11",	&usb11_clk),
 	CLK(NULL,		"usb20",	&usb20_clk),
+	CLK("spi_davinci.0",	NULL,		&spi0_clk),
+	CLK("spi_davinci.1",	NULL,		&spi1_clk),
 	CLK(NULL,		NULL,		NULL),
 };
 
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index beda8a4..f421f97 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
 
 	return platform_device_register(&da8xx_cpuidle_device);
 }
+
+static struct resource da8xx_spi0_resources[] = {
+	[0] = {
+		.start	= 0x01c41000,
+		.end	= 0x01c41fff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= IRQ_DA8XX_SPINT0,
+		.end	= IRQ_DA8XX_SPINT0,
+		.flags	= IORESOURCE_IRQ,
+	},
+	[2] = {
+		.start	= EDMA_CTLR_CHAN(0, 14),
+		.end	= EDMA_CTLR_CHAN(0, 14),
+		.flags	= IORESOURCE_DMA,
+	},
+	[3] = {
+		.start	= EDMA_CTLR_CHAN(0, 15),
+		.end	= EDMA_CTLR_CHAN(0, 15),
+		.flags	= IORESOURCE_DMA,
+	},
+	[4] = {
+		.flags	= IORESOURCE_DMA,
+	},
+};
+
+static struct resource da8xx_spi1_resources[] = {
+	[0] = {
+		.start	= 0x01f0e000,
+		.end	= 0x01f0efff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= IRQ_DA8XX_SPINT1,
+		.end	= IRQ_DA8XX_SPINT1,
+		.flags	= IORESOURCE_IRQ,
+	},
+	[2] = {
+		.start	= EDMA_CTLR_CHAN(0, 18),
+		.end	= EDMA_CTLR_CHAN(0, 18),
+		.flags	= IORESOURCE_DMA,
+	},
+	[3] = {
+		.start	= EDMA_CTLR_CHAN(0, 19),
+		.end	= EDMA_CTLR_CHAN(0, 19),
+		.flags	= IORESOURCE_DMA,
+	},
+	[4] = {
+		.flags	= IORESOURCE_DMA,
+	},
+};
+
+struct davinci_spi_platform_data da8xx_spi_pdata[] = {
+	[0] = {
+		.version	= SPI_VERSION_2,
+		.intr_line	= 1,
+	},
+	[1] = {
+		.version	= SPI_VERSION_2,
+		.intr_line	= 1,
+	},
+};
+
+static struct platform_device da8xx_spi_device[] = {
+	[0] = {
+		.name		= "spi_davinci",
+		.id		= 0,
+		.num_resources	= ARRAY_SIZE(da8xx_spi0_resources),
+		.resource	= da8xx_spi0_resources,
+		.dev		= {
+			.platform_data = &da8xx_spi_pdata[0],
+		},
+	},
+	[1] = {
+		.name		= "spi_davinci",
+		.id		= 1,
+		.num_resources	= ARRAY_SIZE(da8xx_spi1_resources),
+		.resource	= da8xx_spi1_resources,
+		.dev		= {
+			.platform_data = &da8xx_spi_pdata[1],
+		},
+	},
+};
+
+int __init da8xx_register_spi(int instance)
+{
+	struct platform_device *pdev;
+
+	if (instance == 0 || instance == 1)
+		pdev = &da8xx_spi_device[instance];
+	else
+		return -EINVAL;
+
+	return platform_device_register(pdev);
+}
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index cfcb223..0c5fa01 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -23,6 +23,7 @@
 #include <mach/mmc.h>
 #include <mach/usb.h>
 #include <mach/pm.h>
+#include <mach/spi.h>
 
 extern void __iomem *da8xx_syscfg0_base;
 extern void __iomem *da8xx_syscfg1_base;
@@ -77,6 +78,7 @@ void __init da850_init(void);
 int da830_register_edma(struct edma_rsv_info *rsv);
 int da850_register_edma(struct edma_rsv_info *rsv[2]);
 int da8xx_register_i2c(int instance, struct davinci_i2c_platform_data *pdata);
+int da8xx_register_spi(int instance);
 int da8xx_register_watchdog(void);
 int da8xx_register_usb20(unsigned mA, unsigned potpgt);
 int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
@@ -95,6 +97,7 @@ extern struct platform_device da8xx_serial_device;
 extern struct emac_platform_data da8xx_emac_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lcd035q3dg01_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lk043t1dg01_pdata;
+extern struct davinci_spi_platform_data da8xx_spi_pdata[];
 
 extern struct platform_device da8xx_wdt_device;
 
-- 
1.7.0.4

[PATCH v1 2/4] davinci: add spi devices support for MityDSP-L138/MityARM-1808 platform

[Michael Williamson]

This patch adds support for accessing the on board SPI NOR FLASH
device for MityDSP-L138 and MityARM-1808 SoMs.

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
Tested-by: Michael Williamson <michael.williamson@criticallink.com>
---
 arch/arm/mach-davinci/board-mityomapl138.c |  100 ++++++++++++++++++++++++++++
 1 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 0ea5932..dc4c3f1 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -17,6 +17,8 @@
 #include <linux/i2c.h>
 #include <linux/i2c/at24.h>
 #include <linux/etherdevice.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/flash.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -25,6 +27,7 @@
 #include <mach/da8xx.h>
 #include <mach/nand.h>
 #include <mach/mux.h>
+#include <mach/spi.h>
 
 #define MITYOMAPL138_PHY_ID		"0:03"
 
@@ -294,6 +297,100 @@ static int __init pmic_tps65023_init(void)
 }
 
 /*
+ * SPI Devices:
+ *	SPI1_CS0: 8M Flash ST-M25P64-VME6G
+ */
+static struct mtd_partition spi_flash_partitions[] = {
+	[0] = {
+		.name		= "ubl",
+		.offset		= 0,
+		.size		= SZ_64K,
+		.mask_flags	= MTD_WRITEABLE,
+	},
+	[1] = {
+		.name		= "u-boot",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_512K,
+		.mask_flags	= MTD_WRITEABLE,
+	},
+	[2] = {
+		.name		= "u-boot-env",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_64K,
+		.mask_flags	= MTD_WRITEABLE,
+	},
+	[3] = {
+		.name		= "periph-config",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_64K,
+		.mask_flags	= MTD_WRITEABLE,
+	},
+	[4] = {
+		.name		= "reserved",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_256K + SZ_64K,
+	},
+	[5] = {
+		.name		= "kernel",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_2M + SZ_1M,
+	},
+	[6] = {
+		.name		= "fpga",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= SZ_2M,
+	},
+	[7] = {
+		.name		= "spare",
+		.offset		= MTDPART_OFS_APPEND,
+		.size		= MTDPART_SIZ_FULL,
+	},
+};
+
+static struct flash_platform_data mityomapl138_spi_flash_data = {
+	.name		= "m25p80",
+	.parts		= spi_flash_partitions,
+	.nr_parts	= ARRAY_SIZE(spi_flash_partitions),
+	.type		= "m24p64",
+};
+
+static struct davinci_spi_config spi_eprom_config = {
+	.io_type	= SPI_IO_TYPE_DMA,
+	.c2tdelay	= 8,
+	.t2cdelay	= 8,
+};
+
+static struct spi_board_info mityomapl138_spi_flash_info[] = {
+	{
+		.modalias		= "m25p80",
+		.platform_data		= &mityomapl138_spi_flash_data,
+		.controller_data	= &spi_eprom_config,
+		.mode			= SPI_MODE_0,
+		.max_speed_hz		= 30000000,
+		.bus_num		= 1,
+		.chip_select		= 0,
+	},
+};
+
+static void __init mityomapl138_init_spi1(struct spi_board_info *info,
+					  unsigned len)
+{
+	int ret;
+
+	ret = spi_register_board_info(info, len);
+	if (ret)
+		pr_warning("%s: failed to register board info : %d\n",
+				__func__, ret);
+
+	da8xx_spi_pdata[1].num_chipselect = len;
+
+	ret = da8xx_register_spi(1);
+	if (ret)
+		pr_warning("%s: failed to register spi 1 device : %d\n",
+				__func__, ret);
+}
+
+/*
  * MityDSP-L138 includes a 256 MByte large-page NAND flash
  * (128K blocks).
  */
@@ -448,6 +545,9 @@ static void __init mityomapl138_init(void)
 
 	mityomapl138_setup_nand();
 
+	mityomapl138_init_spi1(mityomapl138_spi_flash_info,
+			       ARRAY_SIZE(mityomapl138_spi_flash_info));
+
 	mityomapl138_config_emac();
 
 	ret = da8xx_register_rtc();
-- 
1.7.0.4

[PATCH v1 3/4] davinci: add spi devices support for da850/omap-l138/am18x evm

[Michael Williamson]

From: Sekhar Nori <nsekhar@ti.com>

This patch adds the on-board SPI flash device to the
DA850/OMAP-L138/AM18x EVM. It also registers the SPI flash
device to the MTD subsystem.

Based on SPI flash device support for MityDSP-L138F platform.

Signed-off-by: Sekhar Nori <nsekhar@ti.com>
[michael.williamson at criticallink.com: moved da850_evm_spi1_pdata to devices-da8xx.c]
Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
---
 arch/arm/mach-davinci/board-da850-evm.c |   84 +++++++++++++++++++++++++++++++
 1 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 11f986b..487bd3a 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -29,6 +29,8 @@
 #include <linux/regulator/machine.h>
 #include <linux/regulator/tps6507x.h>
 #include <linux/input/tps6507x-ts.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/flash.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -38,6 +40,7 @@
 #include <mach/nand.h>
 #include <mach/mux.h>
 #include <mach/aemif.h>
+#include <mach/spi.h>
 
 #define DA850_EVM_PHY_ID		"0:00"
 #define DA850_LCD_PWR_PIN		GPIO_TO_PIN(2, 8)
@@ -48,6 +51,85 @@
 
 #define DA850_MII_MDIO_CLKEN_PIN	GPIO_TO_PIN(2, 6)
 
+static struct mtd_partition da850evm_spiflash_part[] = {
+	[0] = {
+		.name = "UBL",
+		.offset = 0,
+		.size = SZ_64K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[1] = {
+		.name = "U-Boot",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_512K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[2] = {
+		.name = "U-Boot-Env",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_64K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[3] = {
+		.name = "Kernel",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_2M + SZ_512K,
+		.mask_flags = 0,
+	},
+	[4] = {
+		.name = "Filesystem",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_4M,
+		.mask_flags = 0,
+	},
+	[5] = {
+		.name = "MAC-Address",
+		.offset = SZ_8M - SZ_64K,
+		.size = SZ_64K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+};
+
+static struct flash_platform_data da850evm_spiflash_data = {
+	.name		= "m25p80",
+	.parts		= da850evm_spiflash_part,
+	.nr_parts	= ARRAY_SIZE(da850evm_spiflash_part),
+	.type		= "m25p64",
+};
+
+static struct davinci_spi_config da850evm_spiflash_cfg = {
+	.io_type	= SPI_IO_TYPE_DMA,
+	.c2tdelay	= 8,
+	.t2cdelay	= 8,
+};
+
+static struct spi_board_info da850evm_spi_info[] = {
+	{
+		.modalias		= "m25p80",
+		.platform_data		= &da850evm_spiflash_data,
+		.controller_data	= &da850evm_spiflash_cfg,
+		.mode			= SPI_MODE_0,
+		.max_speed_hz		= 30000000,
+		.bus_num		= 1,
+		.chip_select		= 0,
+	},
+};
+
+static void __init da850evm_init_spi1(struct spi_board_info *info, unsigned len)
+{
+	int ret;
+
+	ret = spi_register_board_info(info, len);
+	if (ret)
+		pr_warning("failed to register board info : %d\n", ret);
+
+	da8xx_spi_pdata[1].num_chipselect = len;
+
+	ret = da8xx_register_spi(1);
+	if (ret)
+		pr_warning("failed to register spi 1 device : %d\n", ret);
+}
+
 static struct mtd_partition da850_evm_norflash_partition[] = {
 	{
 		.name           = "bootloaders + env",
@@ -1167,6 +1249,8 @@ static __init void da850_evm_init(void)
 	if (ret)
 		pr_warning("da850_evm_init: suspend registration failed: %d\n",
 				ret);
+
+	da850evm_init_spi1(da850evm_spi_info, ARRAY_SIZE(da850evm_spi_info));
 }
 
 #ifdef CONFIG_SERIAL_8250_CONSOLE
-- 
1.7.0.4

[PATCH v1 4/4] davinci: add spi devices support for da830/omap-l137/am17x evm

[Michael Williamson]

From: Sekhar Nori <nsekhar@ti.com>

This patch adds the on-board SPI flash device to the
DA830/OMAP-L137/AM17x EVM. It also registers the SPI flash
device to the MTD subsystem.

Based on SPI flash device support for MityDSP-L138F platform.

Signed-off-by: Sekhar Nori <nsekhar@ti.com>
[michael.williamson at criticallink.com: moved da830evm_spi0_pdata to devices-da8xx.c]
Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
---
 arch/arm/mach-davinci/board-da830-evm.c |   78 +++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index b52a3a1..01319bd 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -20,6 +20,8 @@
 #include <linux/i2c/at24.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/flash.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -30,6 +32,7 @@
 #include <mach/da8xx.h>
 #include <mach/usb.h>
 #include <mach/aemif.h>
+#include <mach/spi.h>
 
 #define DA830_EVM_PHY_ID		""
 /*
@@ -534,6 +537,79 @@ static struct edma_rsv_info da830_edma_rsv[] = {
 	},
 };
 
+static struct mtd_partition da830evm_spiflash_part[] = {
+	[0] = {
+		.name = "DSP-UBL",
+		.offset = 0,
+		.size = SZ_8K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[1] = {
+		.name = "ARM-UBL",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_16K + SZ_8K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[2] = {
+		.name = "U-Boot",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_256K - SZ_32K,
+		.mask_flags = MTD_WRITEABLE,
+	},
+	[3] = {
+		.name = "U-Boot-Environment",
+		.offset = MTDPART_OFS_APPEND,
+		.size = SZ_16K,
+		.mask_flags = 0,
+	},
+	[4] = {
+		.name = "Kernel",
+		.offset = MTDPART_OFS_APPEND,
+		.size = MTDPART_SIZ_FULL,
+		.mask_flags = 0,
+	},
+};
+
+static struct flash_platform_data da830evm_spiflash_data = {
+	.name		= "m25p80",
+	.parts		= da830evm_spiflash_part,
+	.nr_parts	= ARRAY_SIZE(da830evm_spiflash_part),
+	.type		= "w25x32",
+};
+
+static struct davinci_spi_config da830evm_spiflash_cfg = {
+	.io_type	= SPI_IO_TYPE_DMA,
+	.c2tdelay	= 8,
+	.t2cdelay	= 8,
+};
+
+static struct spi_board_info da830evm_spi_info[] = {
+	{
+		.modalias		= "m25p80",
+		.platform_data		= &da830evm_spiflash_data,
+		.controller_data	= &da830evm_spiflash_cfg,
+		.mode			= SPI_MODE_0,
+		.max_speed_hz		= 30000000,
+		.bus_num		= 0,
+		.chip_select		= 0,
+	},
+};
+
+static void __init da830evm_init_spi0(struct spi_board_info *info, unsigned len)
+{
+	int ret;
+
+	ret = spi_register_board_info(info, len);
+	if (ret)
+		pr_warning("failed to register board info : %d\n", ret);
+
+	da8xx_spi_pdata[0].num_chipselect = len;
+
+	ret = da8xx_register_spi(0);
+	if (ret)
+		pr_warning("failed to register spi 0 device : %d\n", ret);
+}
+
 static __init void da830_evm_init(void)
 {
 	struct davinci_soc_info *soc_info = &davinci_soc_info;
@@ -590,6 +666,8 @@ static __init void da830_evm_init(void)
 	ret = da8xx_register_rtc();
 	if (ret)
 		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
+
+	da830evm_init_spi0(da830evm_spi_info, ARRAY_SIZE(da830evm_spi_info));
 }
 
 #ifdef CONFIG_SERIAL_8250_CONSOLE
-- 
1.7.0.4

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Sergei Shtylyov]

Hello.

On 02-02-2011 0:49, Michael Williamson wrote:

> Add SPI registration routines, clocks, and driver resources for
> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

    You're only adding clocks for the former platform -- the ones for the 
latter platfrom are already there...

> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
[...]

> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 3443d97..68fe4c2 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -359,6 +359,20 @@ static struct clk usb20_clk = {
>   	.gpsc		= 1,
>   };
>
> +static struct clk spi0_clk = {
> +	.name		= "spi0",
> +	.parent		= &pll0_sysclk2,
> +	.lpsc		= DA8XX_LPSC0_SPI0,
> +};
> +
> +static struct clk spi1_clk = {
> +	.name		= "spi1",
> +	.parent		= &pll0_sysclk2,
> +	.lpsc		= DA8XX_LPSC1_SPI1,
> +	.gpsc		= 1,
> +	.flags		= DA850_CLK_ASYNC3,
> +};
> +
>   static struct clk_lookup da850_clks[] = {
>   	CLK(NULL,		"ref",		&ref_clk),
>   	CLK(NULL,		"pll0",		&pll0_clk),
> @@ -403,6 +417,8 @@ static struct clk_lookup da850_clks[] = {
>   	CLK(NULL,		"aemif",	&aemif_clk),
>   	CLK(NULL,		"usb11",	&usb11_clk),
>   	CLK(NULL,		"usb20",	&usb20_clk),
> +	CLK("spi_davinci.0",	NULL,		&spi0_clk),
> +	CLK("spi_davinci.1",	NULL,		&spi1_clk),
>   	CLK(NULL,		NULL,		NULL),
>   };

    It's worth separating this into a patch of its own...

> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index beda8a4..f421f97 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>
>   	return platform_device_register(&da8xx_cpuidle_device);
>   }
> +
> +static struct resource da8xx_spi0_resources[] = {
> +	[0] = {
> +		.start	= 0x01c41000,
> +		.end	= 0x01c41fff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= IRQ_DA8XX_SPINT0,
> +		.end	= IRQ_DA8XX_SPINT0,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +	[2] = {
> +		.start	= EDMA_CTLR_CHAN(0, 14),
> +		.end	= EDMA_CTLR_CHAN(0, 14),
> +		.flags	= IORESOURCE_DMA,
> +	},
> +	[3] = {
> +		.start	= EDMA_CTLR_CHAN(0, 15),
> +		.end	= EDMA_CTLR_CHAN(0, 15),
> +		.flags	= IORESOURCE_DMA,
> +	},

    We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...

> +	[4] = {
> +		.flags	= IORESOURCE_DMA,
> +	},

    BTW, why do you need this placeholder?

> +};
> +
> +static struct resource da8xx_spi1_resources[] = {
> +	[0] = {
> +		.start	= 0x01f0e000,
> +		.end	= 0x01f0efff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= IRQ_DA8XX_SPINT1,
> +		.end	= IRQ_DA8XX_SPINT1,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +	[2] = {
> +		.start	= EDMA_CTLR_CHAN(0, 18),
> +		.end	= EDMA_CTLR_CHAN(0, 18),
> +		.flags	= IORESOURCE_DMA,
> +	},
> +	[3] = {
> +		.start	= EDMA_CTLR_CHAN(0, 19),
> +		.end	= EDMA_CTLR_CHAN(0, 19),
> +		.flags	= IORESOURCE_DMA,
> +	},

    We have DA830_DMACH_SPI1_[RT]X defined for SPI1 DMA channels...

> +	[4] = {
> +		.flags	= IORESOURCE_DMA,

     ... and this one?

> +	},
> +};
> +

WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Sergei Shtylyov]

Hello.

On 02-02-2011 0:49, Michael Williamson wrote:

> Add SPI registration routines, clocks, and driver resources for
> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
[...]

> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 3443d97..68fe4c2 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
[...]
> @@ -403,6 +417,8 @@ static struct clk_lookup da850_clks[] = {
>   	CLK(NULL,		"aemif",	&aemif_clk),
>   	CLK(NULL,		"usb11",	&usb11_clk),
>   	CLK(NULL,		"usb20",	&usb20_clk),
> +	CLK("spi_davinci.0",	NULL,		&spi0_clk),
> +	CLK("spi_davinci.1",	NULL,		&spi1_clk),

    BTW, da830.c has "dm_spi.[01]" instead -- which should be fixed (by yet 
another patch).

>   	CLK(NULL,		NULL,		NULL),
>   };
>

WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Sergei Shtylyov]

Hello.

On 02-02-2011 0:49, Michael Williamson wrote:

> Add SPI registration routines, clocks, and driver resources for
> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
[...]

> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index beda8a4..f421f97 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>
>   	return platform_device_register(&da8xx_cpuidle_device);
>   }
> +
> +static struct resource da8xx_spi0_resources[] = {
> +	[0] = {
> +		.start	= 0x01c41000,
> +		.end	= 0x01c41fff,

    BTW, I suggest to declare DA8XX_SPI0_BASE, not to deviate from the 
tradition developed in this file...

> +static struct resource da8xx_spi1_resources[] = {
> +	[0] = {
> +		.start	= 0x01f0e000,
> +		.end	= 0x01f0efff,

    The same about DA8XX_SPI1_BASE...

WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

Hi Sergei,

On 2/2/2011 7:22 AM, Sergei Shtylyov wrote:

> Hello.
> 
> On 02-02-2011 0:49, Michael Williamson wrote:
> 
>> Add SPI registration routines, clocks, and driver resources for
>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.
> 
>    You're only adding clocks for the former platform -- the ones for the latter platfrom are already there...
> 
>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
> [...]
> 
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index 3443d97..68fe4c2 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -359,6 +359,20 @@ static struct clk usb20_clk = {
>>       .gpsc        = 1,
>>   };
>>
>> +static struct clk spi0_clk = {
>> +    .name        = "spi0",
>> +    .parent        = &pll0_sysclk2,
>> +    .lpsc        = DA8XX_LPSC0_SPI0,
>> +};
>> +
>> +static struct clk spi1_clk = {
>> +    .name        = "spi1",
>> +    .parent        = &pll0_sysclk2,
>> +    .lpsc        = DA8XX_LPSC1_SPI1,
>> +    .gpsc        = 1,
>> +    .flags        = DA850_CLK_ASYNC3,
>> +};
>> +
>>   static struct clk_lookup da850_clks[] = {
>>       CLK(NULL,        "ref",        &ref_clk),
>>       CLK(NULL,        "pll0",        &pll0_clk),
>> @@ -403,6 +417,8 @@ static struct clk_lookup da850_clks[] = {
>>       CLK(NULL,        "aemif",    &aemif_clk),
>>       CLK(NULL,        "usb11",    &usb11_clk),
>>       CLK(NULL,        "usb20",    &usb20_clk),
>> +    CLK("spi_davinci.0",    NULL,        &spi0_clk),
>> +    CLK("spi_davinci.1",    NULL,        &spi1_clk),
>>       CLK(NULL,        NULL,        NULL),
>>   };
> 
>    It's worth separating this into a patch of its own...
> 


OK, I will pull this into a separate patch.

>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index beda8a4..f421f97 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>>
>>       return platform_device_register(&da8xx_cpuidle_device);
>>   }
>> +
>> +static struct resource da8xx_spi0_resources[] = {
>> +    [0] = {
>> +        .start    = 0x01c41000,
>> +        .end    = 0x01c41fff,
>> +        .flags    = IORESOURCE_MEM,
>> +    },
>> +    [1] = {
>> +        .start    = IRQ_DA8XX_SPINT0,
>> +        .end    = IRQ_DA8XX_SPINT0,
>> +        .flags    = IORESOURCE_IRQ,
>> +    },
>> +    [2] = {
>> +        .start    = EDMA_CTLR_CHAN(0, 14),
>> +        .end    = EDMA_CTLR_CHAN(0, 14),
>> +        .flags    = IORESOURCE_DMA,
>> +    },
>> +    [3] = {
>> +        .start    = EDMA_CTLR_CHAN(0, 15),
>> +        .end    = EDMA_CTLR_CHAN(0, 15),
>> +        .flags    = IORESOURCE_DMA,
>> +    },
> 
>    We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...
> 


Right.  The current routines/structures were designed to support either da830 or
da850.  They work because the interrupts and the DMA channels are fortunately
the same for the SPI devices between the platforms.  I can use the DA830 enums, 
but I had preferred the clarity of the EDMA_CTRL_CHAN macro, and it would 
remove confusion that this might only apply for DA830 and not DA850.  It would
be nice to rename that DA8XX_DMAC_SPI_[RT]X if the enums were used...

Can I leave this, or do you really want me to use the enums?

>> +    [4] = {
>> +        .flags    = IORESOURCE_DMA,
>> +    },
> 
>    BTW, why do you need this placeholder?
> 


This is for the eventq selection in the edma allocation by the spi driver.  When I 
previously had 0, I should have used EVENTQ_0 (how it was tested).  I will add it 
back in to clarification.

>> +};
>> +
>> +static struct resource da8xx_spi1_resources[] = {
>> +    [0] = {
>> +        .start    = 0x01f0e000,
>> +        .end    = 0x01f0efff,
>> +        .flags    = IORESOURCE_MEM,
>> +    },
>> +    [1] = {
>> +        .start    = IRQ_DA8XX_SPINT1,
>> +        .end    = IRQ_DA8XX_SPINT1,
>> +        .flags    = IORESOURCE_IRQ,
>> +    },
>> +    [2] = {
>> +        .start    = EDMA_CTLR_CHAN(0, 18),
>> +        .end    = EDMA_CTLR_CHAN(0, 18),
>> +        .flags    = IORESOURCE_DMA,
>> +    },
>> +    [3] = {
>> +        .start    = EDMA_CTLR_CHAN(0, 19),
>> +        .end    = EDMA_CTLR_CHAN(0, 19),
>> +        .flags    = IORESOURCE_DMA,
>> +    },
> 
>    We have DA830_DMACH_SPI1_[RT]X defined for SPI1 DMA channels...
> 


I'd really prefer this way, but will change it to DA830_DMAC_SPI1* enum
if you require it.

>> +    [4] = {
>> +        .flags    = IORESOURCE_DMA,
> 
>     ... and this one?
> 


See above.

>> +    },
>> +};
>> +
> 
> WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

On 2/2/2011 7:53 AM, Sergei Shtylyov wrote:

> Hello.
> 
> On 02-02-2011 0:49, Michael Williamson wrote:
> 
>> Add SPI registration routines, clocks, and driver resources for
>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.
> 
>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
> [...]
> 
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index beda8a4..f421f97 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>>
>>       return platform_device_register(&da8xx_cpuidle_device);
>>   }
>> +
>> +static struct resource da8xx_spi0_resources[] = {
>> +    [0] = {
>> +        .start    = 0x01c41000,
>> +        .end    = 0x01c41fff,
> 
>    BTW, I suggest to declare DA8XX_SPI0_BASE, not to deviate from the tradition developed in this file...
> 


OK. Thanks.

>> +static struct resource da8xx_spi1_resources[] = {
>> +    [0] = {
>> +        .start    = 0x01f0e000,
>> +        .end    = 0x01f0efff,
> 
>    The same about DA8XX_SPI1_BASE...
> 

Thanks.

> WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

On 2/2/2011 7:29 AM, Sergei Shtylyov wrote:

> Hello.
> 
> On 02-02-2011 0:49, Michael Williamson wrote:
> 
>> Add SPI registration routines, clocks, and driver resources for
>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.
> 
>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
> [...]
> 
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index 3443d97..68fe4c2 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
> [...]
>> @@ -403,6 +417,8 @@ static struct clk_lookup da850_clks[] = {
>>       CLK(NULL,        "aemif",    &aemif_clk),
>>       CLK(NULL,        "usb11",    &usb11_clk),
>>       CLK(NULL,        "usb20",    &usb20_clk),
>> +    CLK("spi_davinci.0",    NULL,        &spi0_clk),
>> +    CLK("spi_davinci.1",    NULL,        &spi1_clk),
> 
>    BTW, da830.c has "dm_spi.[01]" instead -- which should be fixed (by yet another patch).
> 


As I am going around the loop again, I will add a separate patch to the series to rename
this clock to spi_davinci.[01].  Thanks for the close look.  I clearly missed some of the
da830 details.

>>       CLK(NULL,        NULL,        NULL),
>>   };
>>
> 
> WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Sergei Shtylyov]

Hello.

On 02-02-2011 15:55, Michael Williamson wrote:

>>> Add SPI registration routines, clocks, and driver resources for
>>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

>>     You're only adding clocks for the former platform -- the ones for the latter platfrom are already there...

>>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
>> [...]

>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>> index beda8a4..f421f97 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>>>
>>>        return platform_device_register(&da8xx_cpuidle_device);
>>>    }
>>> +
>>> +static struct resource da8xx_spi0_resources[] = {
>>> +    [0] = {
>>> +        .start    = 0x01c41000,
>>> +        .end    = 0x01c41fff,
>>> +        .flags    = IORESOURCE_MEM,
>>> +    },
>>> +    [1] = {
>>> +        .start    = IRQ_DA8XX_SPINT0,
>>> +        .end    = IRQ_DA8XX_SPINT0,
>>> +        .flags    = IORESOURCE_IRQ,
>>> +    },
>>> +    [2] = {
>>> +        .start    = EDMA_CTLR_CHAN(0, 14),
>>> +        .end    = EDMA_CTLR_CHAN(0, 14),
>>> +        .flags    = IORESOURCE_DMA,
>>> +    },
>>> +    [3] = {
>>> +        .start    = EDMA_CTLR_CHAN(0, 15),
>>> +        .end    = EDMA_CTLR_CHAN(0, 15),
>>> +        .flags    = IORESOURCE_DMA,
>>> +    },

>>     We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...

> Right.  The current routines/structures were designed to support either da830 or
> da850.  They work because the interrupts and the DMA channels are fortunately
> the same for the SPI devices between the platforms.  I can use the DA830 enums,
> but I had preferred the clarity of the EDMA_CTRL_CHAN macro, and it would
> remove confusion that this might only apply for DA830 and not DA850.  It would
> be nice to rename that DA8XX_DMAC_SPI_[RT]X if the enums were used...

> Can I leave this, or do you really want me to use the enums?

    OK, you can leave this as is...

>>> +    [4] = {
>>> +        .flags    = IORESOURCE_DMA,
>>> +    },

>>     BTW, why do you need this placeholder?

> This is for the eventq selection in the edma allocation by the spi driver.  When I
> previously had 0, I should have used EVENTQ_0 (how it was tested).  I will add it
> back in to clarification.

    But event queue is not a DMA channel, so can't have just IORESOURCE_DMA in 
'flags'. IIUC, there was a patch to add some clarifying flag(s) to 
IORESOURCE_DMA to support the event queue resource (which I never approved :-).

WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

On 2/2/2011 8:04 AM, Sergei Shtylyov wrote:

> Hello.
> 
> On 02-02-2011 15:55, Michael Williamson wrote:
> 
>>>> Add SPI registration routines, clocks, and driver resources for
>>>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.
> 
>>>     You're only adding clocks for the former platform -- the ones for the latter platfrom are already there...
> 
>>>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
>>> [...]
> 
>>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>>> index beda8a4..f421f97 100644
>>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>>> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>>>>
>>>>        return platform_device_register(&da8xx_cpuidle_device);
>>>>    }
>>>> +
>>>> +static struct resource da8xx_spi0_resources[] = {
>>>> +    [0] = {
>>>> +        .start    = 0x01c41000,
>>>> +        .end    = 0x01c41fff,
>>>> +        .flags    = IORESOURCE_MEM,
>>>> +    },
>>>> +    [1] = {
>>>> +        .start    = IRQ_DA8XX_SPINT0,
>>>> +        .end    = IRQ_DA8XX_SPINT0,
>>>> +        .flags    = IORESOURCE_IRQ,
>>>> +    },
>>>> +    [2] = {
>>>> +        .start    = EDMA_CTLR_CHAN(0, 14),
>>>> +        .end    = EDMA_CTLR_CHAN(0, 14),
>>>> +        .flags    = IORESOURCE_DMA,
>>>> +    },
>>>> +    [3] = {
>>>> +        .start    = EDMA_CTLR_CHAN(0, 15),
>>>> +        .end    = EDMA_CTLR_CHAN(0, 15),
>>>> +        .flags    = IORESOURCE_DMA,
>>>> +    },
> 
>>>     We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...
> 
>> Right.  The current routines/structures were designed to support either da830 or
>> da850.  They work because the interrupts and the DMA channels are fortunately
>> the same for the SPI devices between the platforms.  I can use the DA830 enums,
>> but I had preferred the clarity of the EDMA_CTRL_CHAN macro, and it would
>> remove confusion that this might only apply for DA830 and not DA850.  It would
>> be nice to rename that DA8XX_DMAC_SPI_[RT]X if the enums were used...
> 
>> Can I leave this, or do you really want me to use the enums?
> 
>    OK, you can leave this as is...
> 
>>>> +    [4] = {
>>>> +        .flags    = IORESOURCE_DMA,
>>>> +    },
> 
>>>     BTW, why do you need this placeholder?
> 
>> This is for the eventq selection in the edma allocation by the spi driver.  When I
>> previously had 0, I should have used EVENTQ_0 (how it was tested).  I will add it
>> back in to clarification.
> 
>    But event queue is not a DMA channel, so can't have just IORESOURCE_DMA in 'flags'. IIUC, there was a patch to add some clarifying flag(s) to IORESOURCE_DMA to support the event queue resource (which I never approved :-).
> 



I'd like to get the support in there for the driver that's currently in 
the mainline, which needs this resource as it is.  I had essentially copied
the setup approach from the dm365.c board file, which is doing the same thing.

It looks like other devices take EVENTQ information as platform data (e.g., 
snd_platform_data takes .asp_chan_q).  Would it make sense to update the
spi platform data to accept an EVENTQ parameter as well?  I would be willing
to push a follow on patch to sort this out, if it is required.

Thanks.

> WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Nori, Sekhar]

On Wed, Feb 02, 2011 at 18:25:22, Michael Williamson wrote:

> >> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> >> index beda8a4..f421f97 100644
> >> --- a/arch/arm/mach-davinci/devices-da8xx.c
> >> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> >> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
> >>
> >>       return platform_device_register(&da8xx_cpuidle_device);
> >>   }
> >> +
> >> +static struct resource da8xx_spi0_resources[] = {
> >> +    [0] = {
> >> +        .start    = 0x01c41000,
> >> +        .end    = 0x01c41fff,
> >> +        .flags    = IORESOURCE_MEM,
> >> +    },
> >> +    [1] = {
> >> +        .start    = IRQ_DA8XX_SPINT0,
> >> +        .end    = IRQ_DA8XX_SPINT0,
> >> +        .flags    = IORESOURCE_IRQ,
> >> +    },
> >> +    [2] = {
> >> +        .start    = EDMA_CTLR_CHAN(0, 14),
> >> +        .end    = EDMA_CTLR_CHAN(0, 14),
> >> +        .flags    = IORESOURCE_DMA,
> >> +    },
> >> +    [3] = {
> >> +        .start    = EDMA_CTLR_CHAN(0, 15),
> >> +        .end    = EDMA_CTLR_CHAN(0, 15),
> >> +        .flags    = IORESOURCE_DMA,
> >> +    },
> > 
> >    We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...
> > 
> 
> 
> Right.  The current routines/structures were designed to support either da830 or
> da850.  They work because the interrupts and the DMA channels are fortunately
> the same for the SPI devices between the platforms.  I can use the DA830 enums, 
> but I had preferred the clarity of the EDMA_CTRL_CHAN macro, and it would 
> remove confusion that this might only apply for DA830 and not DA850.  It would
> be nice to rename that DA8XX_DMAC_SPI_[RT]X if the enums were used...
> 
> Can I leave this, or do you really want me to use the enums?
> 

Looks like a bunch of cleaning-up needs to happen in this area.
The DA830_DMACH_* are not used anywhere at all. Instead most of
devices-da8xx.c uses DMA channel numbers directly (except for ASP,
which has them defined in asp.h).

Since all the platform device definitions for DA8XX devices happens
in devices-da8xx.c, it makes sense to get rid of the DA830_DMACH_*
enum altogether and instead just define the channel numbers used in
devices-da8xx.c above the actual usage.

Mike, is that something you are willing take-up?

Thanks,
Sekhar

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Sergei Shtylyov]

Hello.

On 02-02-2011 16:21, Michael Williamson wrote:

>>>>> Add SPI registration routines, clocks, and driver resources for
>>>>> DA850/OMAP-L138/AM18x and DA830/OMAP-L137/AM17x platforms.

>>>>      You're only adding clocks for the former platform -- the ones for the latter platfrom are already there...

>>>>> Signed-off-by: Michael Williamson<michael.williamson@criticallink.com>
>>>> [...]

>>>>> +    [4] = {
>>>>> +        .flags    = IORESOURCE_DMA,
>>>>> +    },

>>>>      BTW, why do you need this placeholder?

>>> This is for the eventq selection in the edma allocation by the spi driver.  When I
>>> previously had 0, I should have used EVENTQ_0 (how it was tested).  I will add it
>>> back in to clarification.

>>     But event queue is not a DMA channel, so can't have just IORESOURCE_DMA in 'flags'. IIUC,

    IIRC, I meant to type.

> there was a patch to add some clarifying flag(s) to IORESOURCE_DMA to support the event queue resource (which I never approved :-).


> I'd like to get the support in there for the driver that's currently in
> the mainline, which needs this resource as it is.  I had essentially copied
> the setup approach from the dm365.c board file, which is doing the same thing.

> It looks like other devices take EVENTQ information as platform data (e.g.,
> snd_platform_data takes .asp_chan_q).  Would it make sense to update the
> spi platform data to accept an EVENTQ parameter as well?

    Yes, as the event queue is not really a DMA resource (DMA channels are).

> Thanks.

WBR, Sergei

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

On 2/2/2011 8:33 AM, Nori, Sekhar wrote:

> On Wed, Feb 02, 2011 at 18:25:22, Michael Williamson wrote:
> 
>>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>>> index beda8a4..f421f97 100644
>>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>>> @@ -725,3 +725,99 @@ int __init da8xx_register_cpuidle(void)
>>>>
>>>>       return platform_device_register(&da8xx_cpuidle_device);
>>>>   }
>>>> +
>>>> +static struct resource da8xx_spi0_resources[] = {
>>>> +    [0] = {
>>>> +        .start    = 0x01c41000,
>>>> +        .end    = 0x01c41fff,
>>>> +        .flags    = IORESOURCE_MEM,
>>>> +    },
>>>> +    [1] = {
>>>> +        .start    = IRQ_DA8XX_SPINT0,
>>>> +        .end    = IRQ_DA8XX_SPINT0,
>>>> +        .flags    = IORESOURCE_IRQ,
>>>> +    },
>>>> +    [2] = {
>>>> +        .start    = EDMA_CTLR_CHAN(0, 14),
>>>> +        .end    = EDMA_CTLR_CHAN(0, 14),
>>>> +        .flags    = IORESOURCE_DMA,
>>>> +    },
>>>> +    [3] = {
>>>> +        .start    = EDMA_CTLR_CHAN(0, 15),
>>>> +        .end    = EDMA_CTLR_CHAN(0, 15),
>>>> +        .flags    = IORESOURCE_DMA,
>>>> +    },
>>>
>>>    We have DA830_DMACH_SPI0_[RT]X defined for SPI0 DMA channels...
>>>
>>
>>
>> Right.  The current routines/structures were designed to support either da830 or
>> da850.  They work because the interrupts and the DMA channels are fortunately
>> the same for the SPI devices between the platforms.  I can use the DA830 enums, 
>> but I had preferred the clarity of the EDMA_CTRL_CHAN macro, and it would 
>> remove confusion that this might only apply for DA830 and not DA850.  It would
>> be nice to rename that DA8XX_DMAC_SPI_[RT]X if the enums were used...
>>
>> Can I leave this, or do you really want me to use the enums?
>>
> 
> Looks like a bunch of cleaning-up needs to happen in this area.
> The DA830_DMACH_* are not used anywhere at all. Instead most of
> devices-da8xx.c uses DMA channel numbers directly (except for ASP,
> which has them defined in asp.h).
> 


I did a quick check, a large volume of the #defines in the asp.h file
are only used in the platform setup.  Wondering if they should really
be in that file at all...

> Since all the platform device definitions for DA8XX devices happens
> in devices-da8xx.c, it makes sense to get rid of the DA830_DMACH_*
> enum altogether and instead just define the channel numbers used in
> devices-da8xx.c above the actual usage.
> 
> Mike, is that something you are willing take-up?
> 


I can take this up as a separate patch series. I'd like to keep the 
spi platform support series isolated to that subject matter, if that's
OK.

> Thanks,
> Sekhar
> 

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Nori, Sekhar]

Hi Mike,

On Wed, Feb 02, 2011 at 20:18:39, Michael Williamson wrote:
 
> > Since all the platform device definitions for DA8XX devices happens
> > in devices-da8xx.c, it makes sense to get rid of the DA830_DMACH_*
> > enum altogether and instead just define the channel numbers used in
> > devices-da8xx.c above the actual usage.
> > 
> > Mike, is that something you are willing take-up?
> > 
> 
> 
> I can take this up as a separate patch series. I'd like to keep the 
> spi platform support series isolated to that subject matter, if that's
> OK.

Yes, the clean-up can be a separate series - but it will have to
precede the SPI series.

Thanks,
Sekhar

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Michael Williamson]

Hi Sekhar,

On 2/2/2011 10:37 AM, Nori, Sekhar wrote:

> Hi Mike,
> 
> On Wed, Feb 02, 2011 at 20:18:39, Michael Williamson wrote:
>  
>>> Since all the platform device definitions for DA8XX devices happens
>>> in devices-da8xx.c, it makes sense to get rid of the DA830_DMACH_*
>>> enum altogether and instead just define the channel numbers used in
>>> devices-da8xx.c above the actual usage.
>>>
>>> Mike, is that something you are willing take-up?
>>>
>>
>>
>> I can take this up as a separate patch series. I'd like to keep the 
>> spi platform support series isolated to that subject matter, if that's
>> OK.
> 
> Yes, the clean-up can be a separate series - but it will have to
> precede the SPI series.

> 


Quick question:

In addition to cleaning up the enums in edma.h, are you also asking to 
pull the channel / base resources for ASP currently defined in asp.h 
into devices-da8xx.c as well?  Or should that be left alone? E.G., 
these defines look like they should go in devices-da8xx and come out 
of asp.h:

DAVINCI_DA8XX_MCASP0_REG_BASE
DAVINCI_DA830_MCASP1_REG_BASE
DAVINCI_DA8XX_DMA_MCASP0_AREVT
DAVINCI_DA8XX_DMA_MCASP0_AXEVT
DAVINCI_DA830_DMA_MCASP1_AREVT
DAVINCI_DA830_DMA_MCASP1_AXEVT

If moved, there are other #defines that should then be shuffled out of asp.h
to other files for other chips/platforms for completeness, E.G.

DAVINCI_DM646X_DMA_MCASP0_AXEVT0 should go to dm646x.c.

I'm not too keen on shuffling stuff around like that if I can't test it beyond
compiling.  Especially if it's not adding any functionality.  But, I will submit
it if it's necessary to get the SPI support in.

Thanks.

-Mike

[PATCH v1 1/4] davinci: da8xx/omap-l1: add support for SPI

[Nori, Sekhar]

Hi Mike,

On Thu, Feb 03, 2011 at 17:53:37, Michael Williamson wrote:
> Hi Sekhar,
> 
> On 2/2/2011 10:37 AM, Nori, Sekhar wrote:
> 
> > Hi Mike,
> > 
> > On Wed, Feb 02, 2011 at 20:18:39, Michael Williamson wrote:
> >  
> >>> Since all the platform device definitions for DA8XX devices happens
> >>> in devices-da8xx.c, it makes sense to get rid of the DA830_DMACH_*
> >>> enum altogether and instead just define the channel numbers used in
> >>> devices-da8xx.c above the actual usage.
> >>>
> >>> Mike, is that something you are willing take-up?
> >>>
> >>
> >>
> >> I can take this up as a separate patch series. I'd like to keep the 
> >> spi platform support series isolated to that subject matter, if that's
> >> OK.
> > 
> > Yes, the clean-up can be a separate series - but it will have to
> > precede the SPI series.
> 
> > 
> 
> 
> Quick question:
> 
> In addition to cleaning up the enums in edma.h, are you also asking to 
> pull the channel / base resources for ASP currently defined in asp.h 
> into devices-da8xx.c as well?  Or should that be left alone? E.G., 
> these defines look like they should go in devices-da8xx and come out 
> of asp.h:
> 
> DAVINCI_DA8XX_MCASP0_REG_BASE
> DAVINCI_DA830_MCASP1_REG_BASE
> DAVINCI_DA8XX_DMA_MCASP0_AREVT
> DAVINCI_DA8XX_DMA_MCASP0_AXEVT
> DAVINCI_DA830_DMA_MCASP1_AREVT
> DAVINCI_DA830_DMA_MCASP1_AXEVT
> 
> If moved, there are other #defines that should then be shuffled out of asp.h
> to other files for other chips/platforms for completeness, E.G.
> 
> DAVINCI_DM646X_DMA_MCASP0_AXEVT0 should go to dm646x.c.
> 
> I'm not too keen on shuffling stuff around like that if I can't test it beyond
> compiling.  Especially if it's not adding any functionality.  But, I will submit
> it if it's necessary to get the SPI support in.

I don't think cleaning up all this ASP stuff is necessary to get SPI in.
Just do:

1) Get rid of the DA830 DMA event enum in edma.h
2) define local macros in devices-da8xx.c for the DMA events used in that 
   file.

The ASP clean-up can be taken-up at a later time.

Thanks for your efforts on this.

Best Regards,
Sekhar