[PATCH v3 5/6] ARM: davinci: Add support for PRUSS on DA850

On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote:

On 10/3/2012 8:25 PM, Matt Porter wrote:

quoted

Adds PRUSS clock, registers the L3RAM pool, and registers the
platform device for uio_pruss on DA850.

Signed-off-by: Matt Porter <redacted>

I am interested in knowing how this patch was tested.

I'll add similar test information in the commit that I mention in
my other reply. Basically, I use the examples in the PRU package
downloadable from ti.com. There's a PRU_memAccessL3andDDR example
which will fail miserably if the uio_pruss driver SRAM resource
is not configured properly. With the complete series in place, it's
able to complete execution successfully, communicating via the
shared sram pool.

quoted

---
 arch/arm/mach-davinci/board-da850-evm.c    |   12 +++++
 arch/arm/mach-davinci/da850.c              |    7 +++
 arch/arm/mach-davinci/devices-da8xx.c      |   66 ++++++++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/da8xx.h |    2 +
 4 files changed, 87 insertions(+)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 1295e61..acc6e84 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c

@@ -29,6 +29,7 @@
 #include <linux/regulator/machine.h>
 #include <linux/regulator/tps6507x.h>
 #include <linux/input/tps6507x-ts.h>
+#include <linux/platform_data/uio_pruss.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
 #include <linux/delay.h>

@@ -42,6 +43,7 @@
 #include <mach/da8xx.h>
 #include <linux/platform_data/mtd-davinci.h>
 #include <mach/mux.h>
+#include <mach/sram.h>
 #include <linux/platform_data/mtd-davinci-aemif.h>
 #include <linux/platform_data/spi-davinci.h>

I know you did not introduce the mess, but can you include a patch to
fix the mixture of mach/ and linux/ includes here? mach/ includes should
follow the linux/ includes.

Ok. Yeah, I agree it's a mess there.

quoted

@@ -1253,6 +1255,10 @@ static __init int da850_wl12xx_init(void)
 
 #endif /* CONFIG_DA850_WL12XX */
 
+struct uio_pruss_pdata da8xx_pruss_uio_pdata = {
+	.pintc_base	= 0x4000,
+};
+
 #define DA850EVM_SATA_REFCLKPN_RATE	(100 * 1000 * 1000)
 
 static __init void da850_evm_init(void)

@@ -1339,6 +1345,12 @@ static __init void da850_evm_init(void)
 		pr_warning("da850_evm_init: lcdcntl mux setup failed: %d\n",
 				ret);
 
+	da8xx_pruss_uio_pdata.l3ram_pool = sram_get_gen_pool();

I wonder if this platform data should still be called l3ram pool. L3RAM
is OMAP terminology. May be just call it sram_pool? Also this patch
should follow 6/6 to prevent build breakage.

I agree, I'll change to sram_pool. I went back and forth on this because
I found even the userspace PRU tools had it called L3...I think it's
wise just say sram and avoid confusion. I'll move the patch too.

quoted

+	ret = da8xx_register_pruss_uio(&da8xx_pruss_uio_pdata);
+	if (ret)
+		pr_warning("pruss_uio initialization failed: %d\n",
+				ret);
+
 	/* Handle board specific muxing for LCD here */
 	ret = davinci_cfg_reg_list(da850_evm_lcdc_pins);
 	if (ret)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index d8d69de..7c01d31 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c

Can you separate out board and SoC changes into different patches?

Ok, yes.

quoted

@@ -212,6 +212,12 @@ static struct clk tptc2_clk = {
 	.flags		= ALWAYS_ENABLED,
 };
 
+static struct clk pruss_clk = {
+	.name		= "pruss",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC0_PRUSS,
+};
+
 static struct clk uart0_clk = {
 	.name		= "uart0",
 	.parent		= &pll0_sysclk2,

@@ -378,6 +384,7 @@ static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"tptc1",	&tptc1_clk),
 	CLK(NULL,		"tpcc1",	&tpcc1_clk),
 	CLK(NULL,		"tptc2",	&tptc2_clk),
+	CLK(NULL,		"pruss",	&pruss_clk),

This is actually incorrect since we should use device name rather than
con_id for matching the clock. If there is just one clock that the
driver needs, connection id should be NULL. Looking at the driver now,
the clk_get() call seems to pass a valid device pointer. So, I wonder
how you are able to look up the clock even with a NULL device name.

Hrm, I'll look into this. This part was a not looked at closely, just a
quick copy/paste/modify and worked as tested with the PRU examples so
I didn't look further. I'll look again let you know why it actually works.
I'm guessing I got lucky here, and should be a simple fix.

quoted

 	CLK(NULL,		"uart0",	&uart0_clk),
 	CLK(NULL,		"uart1",	&uart1_clk),
 	CLK(NULL,		"uart2",	&uart2_clk),

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index bd2f72b..7c2e0d2 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c

@@ -518,6 +518,72 @@ void __init da8xx_register_mcasp(int id, struct snd_platform_data *pdata)
 	}
 }
 
+#define DA8XX_PRUSS_MEM_BASE		0x01C30000

In this file all base addresses are added at the top of the file. The
defines are sorted in ascending order of address. If that's broken, its
all my fault, but please don't add to the breakage when adding this entry :)

Ok :) Will fix.

quoted

+
+static struct resource da8xx_pruss_resources[] = {
+	{
+		.start	= DA8XX_PRUSS_MEM_BASE,
+		.end	= DA8XX_PRUSS_MEM_BASE + 0xFFFF,
+		.flags	= IORESOURCE_MEM,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT0,
+		.end	= IRQ_DA8XX_EVTOUT0,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT1,
+		.end	= IRQ_DA8XX_EVTOUT1,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT2,
+		.end	= IRQ_DA8XX_EVTOUT2,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT3,
+		.end	= IRQ_DA8XX_EVTOUT3,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT4,
+		.end	= IRQ_DA8XX_EVTOUT4,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT5,
+		.end	= IRQ_DA8XX_EVTOUT5,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT6,
+		.end	= IRQ_DA8XX_EVTOUT6,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.start	= IRQ_DA8XX_EVTOUT7,
+		.end	= IRQ_DA8XX_EVTOUT7,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device da8xx_pruss_uio_dev = {
+	.name		= "pruss_uio",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(da8xx_pruss_resources),
+	.resource	= da8xx_pruss_resources,
+	.dev	 =	{
+		.coherent_dma_mask = 0xffffffff,

DMA_BIT_MASK(32)?

Yeah, more copy/paste/modify stuff. I'll fix that.

-Matt