On Thu, Oct 04, 2012 at 05:52:45PM +0530, Sekhar Nori wrote: > On 10/3/2012 8:25 PM, Matt Porter wrote: > > Adds PRUSS clock, registers the L3RAM pool, and registers the > > platform device for uio_pruss on DA850. > > > > Signed-off-by: Matt Porter <mpor...@ti.com> > > 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. > > --- > > 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. > > @@ -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. > > + 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. > > @@ -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. > > 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. > > + > > +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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/