[PATCH v4 0/9] ARM: davinci: remoteproc support

[PATCH v4 0/9] ARM: davinci: remoteproc support

[Tivy, Robert]

I have uploaded the commits corresponding to this patch set to a public repo on GitHub, forked from Linus' v3.7-rc2 tag on his "linux" repo.  These commits are on a branch named "davinci-remoteproc", here is the URL: https://github.com/RTivy/linux/tree/davinci-remoteproc

This should make for an easier review of the changes.

Regards,

- Rob

> -----Original Message-----
> From: Tivy, Robert
> Sent: Wednesday, December 19, 2012 5:34 PM
> To: davinci-linux-open-source at linux.davincidsp.com; linux-arm-
> kernel at lists.infradead.org; Nori, Sekhar; ohad at wizery.com;
> rob at landley.net; linux-doc at vger.kernel.org; Ring, Chris; Grosen, Mark
> Cc: Tivy, Robert
> Subject: [PATCH v4 0/9] ARM: davinci: remoteproc support
> 
> This patch series adds remoteproc support for OMAP-L138, along with
> needed
> supporting mach-davinci infrastructure.
> 
> Some notes for reviewers...
> 
> DOCUMENTATION maintainers: patch 6/9 in this series contains a change
> to
> kernel-parameters.txt that adds a description for a new kernel command-
> line
> parameter, along with the code that defines the new kernel command-line
> parameter.  You are, of course, free to look at the whole series, but
> only
> patch 6/9 is of particular interest.
> 
> Robert Tivy (9):
>   ARM: davinci: da850 board: change pr_warning() to pr_warn()
>   ARM: davinci: devices-da8xx.c: change pr_warning() to pr_warn()
>   ARM: davinci: psc.c: change pr_warning() to pr_warn()
>   ARM: davinci: da850: added pll0_sysclk1 for DSP usage
>   New reset assert/deassert functionality/API provided for Davinci DSP
>   Remoteproc platform device creation data/code for DA8xx devices
>   Added .reserve function and rproc platform registration
>   Added dsp clock definition, keyed to "davinci-rproc.0"
>   Remoteproc driver support for OMAP-L138 DSP
> 
>  Documentation/kernel-parameters.txt            |    7 +
>  arch/arm/mach-davinci/board-da850-evm.c        |  110 ++++-----
>  arch/arm/mach-davinci/board-omapl138-hawk.c    |   38 +--
>  arch/arm/mach-davinci/clock.c                  |   31 +++
>  arch/arm/mach-davinci/clock.h                  |    3 +
>  arch/arm/mach-davinci/da850.c                  |   18 ++
>  arch/arm/mach-davinci/devices-da8xx.c          |   93 +++++++-
>  arch/arm/mach-davinci/include/mach/clock.h     |    3 +
>  arch/arm/mach-davinci/include/mach/da8xx.h     |    6 +
>  arch/arm/mach-davinci/include/mach/psc.h       |    3 +
>  arch/arm/mach-davinci/psc.c                    |   32 ++-
>  drivers/remoteproc/Kconfig                     |   20 ++
>  drivers/remoteproc/Makefile                    |    1 +
>  drivers/remoteproc/davinci_remoteproc.c        |  303
> ++++++++++++++++++++++++
>  include/linux/platform_data/da8xx-remoteproc.h |   33 +++
>  15 files changed, 617 insertions(+), 84 deletions(-)
>  create mode 100644 drivers/remoteproc/davinci_remoteproc.c
>  create mode 100644 include/linux/platform_data/da8xx-remoteproc.h
> 
> --
> 1.7.9.4

[PATCH v4 0/9] ARM: davinci: remoteproc support

[Sekhar Nori]

On 12/22/2012 3:39 AM, Tivy, Robert wrote:
> I have uploaded the commits corresponding to this patch set to a public repo on GitHub, forked from Linus' v3.7-rc2 tag on his "linux" repo.  These commits are on a branch named "davinci-remoteproc", here is the URL: https://github.com/RTivy/linux/tree/davinci-remoteproc
> 
> This should make for an easier review of the changes.

Thanks. Is there a sample application we could use to exercise the
driver? Looks like we need some firmware to load on the DSP too.

Thanks,
Sekhar

[PATCH v4 0/9] ARM: davinci: remoteproc support

[Tivy, Robert]

Sekhar,

At this point we have some examples/tests that exercise the davinci remoteproc framework using a socket layer that exists only in personal repos.  Ohad created rpmsg-based socket support for the kernel using vrings, and along with SYS/BIOS-based content developed by my TI team and others, it allows a SysLink-like MessageQ implementation to be used between the DSP and Linux user apps.  These apps consist of a Linux app communicating with the DSP firmware file loaded by remoteproc.  They rely on the rpmsg-based socket support, so they're of no use without that.

What I can do, for now, is provide a simple DSP firmware file that prints a "Hello, world" to a SYS/BIOS trace buffer that is known to remoteproc, and the remoteproc driver can be used to display the trace buffer contents through debugfs.  However, this won't exercise the run-time component of davinci remoteproc (which is really just interrupt support between the DSP and ARM).  I will do this and upload the firmware file and a README to my GitHub repo.  I will provide just the firmware binary since the capability to build it from source relies on SYS/BIOS, TI IPC, and XDC packages/installations.

Regards,

- Rob


> -----Original Message-----
> From: Nori, Sekhar
> Sent: Friday, January 04, 2013 4:11 AM
> To: Tivy, Robert
> Cc: davinci-linux-open-source at linux.davincidsp.com; linux-arm-
> kernel at lists.infradead.org; ohad at wizery.com; rob at landley.net; linux-
> doc at vger.kernel.org; Ring, Chris; Grosen, Mark
> Subject: Re: [PATCH v4 0/9] ARM: davinci: remoteproc support
> 
> On 12/22/2012 3:39 AM, Tivy, Robert wrote:
> > I have uploaded the commits corresponding to this patch set to a
> public repo on GitHub, forked from Linus' v3.7-rc2 tag on his "linux"
> repo.  These commits are on a branch named "davinci-remoteproc", here
> is the URL: https://github.com/RTivy/linux/tree/davinci-remoteproc
> >
> > This should make for an easier review of the changes.
> 
> Thanks. Is there a sample application we could use to exercise the
> driver? Looks like we need some firmware to load on the DSP too.
> 
> Thanks,
> Sekhar

[PATCH v4 1/9] ARM: davinci: da850 board: change pr_warning() to pr_warn()

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Also, while modifying those pr_warning() calls I changed hardcoded
> function names to use '"%s:", __func__' instead, and converted acronym
> usage to upper case
> 
> Signed-off-by: Robert Tivy <rtivy@ti.com>
> ---
> Clean up files that will be otherwise modified in subsequent patch.
> 
> Applies to v3.7-rc2 tag (commit 6f0c0580b70c89094b3422ba81118c7b959c7556) of
> Linus' mainline kernel at git.kernel.org.

This doesnt apply to the latest (v3.8-rc2) kernel anymore. Can you
please rebase and resend?

Nit: Patch description should be complete even when read without the
subject line. Please fix this as well when reposting.

Looks fine otherwise.

Thanks,
Sekhar

[PATCH v4 2/9] ARM: davinci: devices-da8xx.c: change pr_warning() to pr_warn()

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Signed-off-by: Robert Tivy <rtivy@ti.com>

Need a description even for trivial patches and even if its mostly a
reworded subject line.

Thanks,
Sekhar

[PATCH v4 5/9] ARM: davinci: New reset functionality/API provided for Davinci DSP

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Since there is no general "reset" support for SoC devices, and since the
> remoteproc driver needs explicit control of the DSP's reset line, a new
> Davinci-specific API is added.

You should probably note here that the private API will disappear with
DT migration and provide a link to the latest discussions on reset
handling in DT case.

> 
> Signed-off-by: Robert Tivy <rtivy@ti.com>
> ---
>  arch/arm/mach-davinci/clock.c              |   31 ++++++++++++++++++++++++++++
>  arch/arm/mach-davinci/clock.h              |    3 +++
>  arch/arm/mach-davinci/include/mach/clock.h |    3 +++
>  arch/arm/mach-davinci/include/mach/psc.h   |    3 +++
>  arch/arm/mach-davinci/psc.c                |   28 +++++++++++++++++++++++++
>  5 files changed, 68 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index 34668ea..d50664f 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -52,6 +52,37 @@ static void __clk_disable(struct clk *clk)
>  		__clk_disable(clk->parent);
>  }
>  
> +int davinci_clk_reset(struct clk *clk, bool reset)
> +{
> +	unsigned long flags;
> +
> +	if (clk == NULL || IS_ERR(clk))
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&clockfw_lock, flags);
> +	if (clk->flags & CLK_PSC)
> +		davinci_psc_reset_config(clk->domain, clk->gpsc, clk->lpsc,
> +				reset, clk->flags);

Not sure if you really need to pass flags down to this function. You
could straightaway check for (clk->flags & PSC_LRST) here.

> +	spin_unlock_irqrestore(&clockfw_lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(davinci_clk_reset);
> +
> +int davinci_reset_assert(struct clk *clk)

davinci_clk_reset_assert for consistency?

> +{
> +	BUG_ON(!clk->reset);

Triggering a bug is too harsh. No need to crash without even allowing to
save data. Just return error.

> +	return clk->reset(clk, true);
> +}
> +EXPORT_SYMBOL(davinci_reset_assert);
> +
> +int davinci_reset_deassert(struct clk *clk)
> +{
> +	BUG_ON(!clk->reset);
> +	return clk->reset(clk, false);
> +}
> +EXPORT_SYMBOL(davinci_reset_deassert);
> +
>  int clk_enable(struct clk *clk)
>  {
>  	unsigned long flags;
> diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
> index 46f0f1b..8694b39 100644
> --- a/arch/arm/mach-davinci/clock.h
> +++ b/arch/arm/mach-davinci/clock.h
> @@ -103,6 +103,7 @@ struct clk {
>  	unsigned long (*recalc) (struct clk *);
>  	int (*set_rate) (struct clk *clk, unsigned long rate);
>  	int (*round_rate) (struct clk *clk, unsigned long rate);
> +	int (*reset) (struct clk *clk, bool reset);
>  };
>  
>  /* Clock flags: SoC-specific flags start at BIT(16) */
> @@ -112,6 +113,7 @@ struct clk {
>  #define PRE_PLL			BIT(4) /* source is before PLL mult/div */
>  #define PSC_SWRSTDISABLE	BIT(5) /* Disable state is SwRstDisable */
>  #define PSC_FORCE		BIT(6) /* Force module state transtition */
> +#define PSC_LRST		BIT(8) /* Use local reset on enable/disable */
>  
>  #define CLK(dev, con, ck) 	\
>  	{			\
> @@ -126,6 +128,7 @@ int davinci_set_pllrate(struct pll_data *pll, unsigned int prediv,
>  int davinci_set_sysclk_rate(struct clk *clk, unsigned long rate);
>  int davinci_set_refclk_rate(unsigned long rate);
>  int davinci_simple_set_rate(struct clk *clk, unsigned long rate);
> +int davinci_clk_reset(struct clk *clk, bool reset);
>  
>  extern struct platform_device davinci_wdt_device;
>  extern void davinci_watchdog_reset(struct platform_device *);
> diff --git a/arch/arm/mach-davinci/include/mach/clock.h b/arch/arm/mach-davinci/include/mach/clock.h
> index a3b0402..cdb8f2f 100644
> --- a/arch/arm/mach-davinci/include/mach/clock.h
> +++ b/arch/arm/mach-davinci/include/mach/clock.h
> @@ -18,4 +18,7 @@ struct clk;
>  extern int clk_register(struct clk *clk);
>  extern void clk_unregister(struct clk *clk);
>  
> +int davinci_reset_assert(struct clk *c);
> +int davinci_reset_deassert(struct clk *c);
> +
>  #endif
> diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach-davinci/include/mach/psc.h
> index 40a0027..21746bd 100644
> --- a/arch/arm/mach-davinci/include/mach/psc.h
> +++ b/arch/arm/mach-davinci/include/mach/psc.h
> @@ -246,6 +246,7 @@
>  
>  #define MDSTAT_STATE_MASK	0x3f
>  #define PDSTAT_STATE_MASK	0x1f
> +#define MDCTL_LRST		BIT(8)
>  #define MDCTL_FORCE		BIT(31)
>  #define PDCTL_NEXT		BIT(0)
>  #define PDCTL_EPCGOOD		BIT(8)
> @@ -253,6 +254,8 @@
>  #ifndef __ASSEMBLER__
>  
>  extern int davinci_psc_is_clk_active(unsigned int ctlr, unsigned int id);
> +extern void davinci_psc_reset_config(unsigned int domain, unsigned int ctlr,
> +		unsigned int id, bool reset, u32 flags);
>  extern void davinci_psc_config(unsigned int domain, unsigned int ctlr,
>  		unsigned int id, bool enable, u32 flags);
>  
> diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c
> index bddaba9..a2a33d4 100644
> --- a/arch/arm/mach-davinci/psc.c
> +++ b/arch/arm/mach-davinci/psc.c
> @@ -48,6 +48,34 @@ int __init davinci_psc_is_clk_active(unsigned int ctlr, unsigned int id)
>  	return mdstat & BIT(12);
>  }
>  
> +/* Control "reset" line associated with PSC domain */
> +void davinci_psc_reset_config(unsigned int domain, unsigned int ctlr,
> +		unsigned int id, bool reset, u32 flags)

You don't use domain in this function. No need to pass it.

Thanks,
Sekhar

[PATCH v4 6/9] ARM: davinci: Remoteproc platform device creation data/code

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Contains CMA-based reservation of physical memory block.  A new kernel
> command-line parameter has been added to allow boot-time specification of
> physical memory block.
> 
> Signed-off-by: Robert Tivy <rtivy@ti.com>

The driver patch should precede platform data creation in the patch
series since it is more logical to review that way.

> ---
>  Documentation/kernel-parameters.txt            |    7 ++
>  arch/arm/mach-davinci/devices-da8xx.c          |   87 +++++++++++++++++++++++-
>  arch/arm/mach-davinci/include/mach/da8xx.h     |    6 ++
>  include/linux/platform_data/da8xx-remoteproc.h |   33 +++++++++
>  4 files changed, 132 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/platform_data/da8xx-remoteproc.h
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 9776f06..87efcc6 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -44,6 +44,7 @@ parameter is applicable:
>  	AVR32	AVR32 architecture is enabled.
>  	AX25	Appropriate AX.25 support is enabled.
>  	BLACKFIN Blackfin architecture is enabled.
> +	CMA	Contiguous Memory Area support is enabled.
>  	DRM	Direct Rendering Management support is enabled.
>  	DYNAMIC_DEBUG Build in debug messages and enable them at runtime
>  	EDD	BIOS Enhanced Disk Drive Services (EDD) is enabled
> @@ -2579,6 +2580,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			Useful for devices that are detected asynchronously
>  			(e.g. USB and MMC devices).
>  
> +	rproc_mem=nn[KMG][@address]
> +			[KNL,ARM,CMA] Remoteproc physical memory block.
> +			Memory area to be used by remote processor image,
> +			managed by CMA.  Suitable defaults exist if not
> +			specified.
> +
>  	rw		[KNL] Mount root device read-write on boot
>  
>  	S		[KNL] Run init in single mode
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index 466b70c..aa151df 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -12,10 +12,13 @@
>   */
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
> -#include <linux/dma-mapping.h>
> +#ifdef CONFIG_CMA
> +#include <linux/dma-contiguous.h>
> +#endif

Ifdefs around includes are typically not required. Do you hit any issue
if you don't have them?

>  #include <linux/serial_8250.h>
>  #include <linux/ahci_platform.h>
>  #include <linux/clk.h>
> +#include <linux/platform_data/da8xx-remoteproc.h>
>  
>  #include <mach/cputype.h>
>  #include <mach/common.h>
> @@ -655,6 +658,88 @@ int __init da850_register_mmcsd1(struct davinci_mmc_config *config)
>  }
>  #endif
>  
> +static struct resource da8xx_rproc_resources[] = {
> +	{ /* DSP boot address */
> +		.start		= DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG,
> +		.end		= DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG,

DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG + 3?

> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{ /* dsp irq */
> +		.start		= IRQ_DA8XX_CHIPINT0,
> +		.end		= IRQ_DA8XX_CHIPINT0,
> +		.flags		= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct da8xx_rproc_pdata rproc_pdata = {
> +	.name		= "dsp",
> +	.firmware	= "da8xx-dsp.xe674",

I thought we agreed not have firmware name in platform data.

> +};
> +
> +static struct platform_device da8xx_dsp = {
> +	.name	= "davinci-rproc",
> +	.id	= 0,
> +	.dev	= {
> +		.platform_data		= &rproc_pdata,
> +		.coherent_dma_mask	= DMA_BIT_MASK(32),
> +	},
> +	.num_resources	= ARRAY_SIZE(da8xx_rproc_resources),
> +	.resource	= da8xx_rproc_resources,
> +};
> +
> +#ifdef CONFIG_CMA

#if IS_ENABLED(DAVINCI_PROC) (or the appropriate config symbol) is more
correct here. CMA could be enabled for other reasons.

> +
> +#define DAVINCI_DSP_RPROC_CMA_BASE 0xc3000000
> +#define DAVINCI_DSP_RPROC_CMA_SIZE 0x01000000
> +
> +static phys_addr_t rproc_base __initdata = DAVINCI_DSP_RPROC_CMA_BASE;
> +static unsigned long rproc_size __initdata = DAVINCI_DSP_RPROC_CMA_SIZE;

Please get rid of these hardcoded defines. Dont want to have them even
as back-up if kernel parameters are not passed. There is absolutely no
way this will work for all users.

> +
> +static int __init early_rproc_mem(char *p)
> +{
> +	char *endp;
> +
> +	if (p == NULL)
> +		return 0;
> +
> +	rproc_size = memparse(p, &endp);
> +	if (*endp == '@')
> +		rproc_base = memparse(endp + 1, NULL);
> +
> +	return 0;
> +}
> +early_param("rproc_mem", early_rproc_mem);
> +
> +void __init da8xx_rproc_reserve_cma(void)
> +{
> +	int ret;
> +
> +	pr_info("%s: reserving 0x%lx @ 0x%lx...\n",
> +		__func__, rproc_size, (unsigned long)rproc_base);
> +
> +	ret = dma_declare_contiguous(&da8xx_dsp.dev, rproc_size, rproc_base, 0);
> +	if (ret)
> +		pr_err("%s: dma_declare_contiguous failed %d\n", __func__, ret);
> +}
> +
> +#endif
> +
> +int __init da8xx_register_rproc(void)
> +{
> +	int ret;
> +
> +	ret = platform_device_register(&da8xx_dsp);
> +	if (ret) {
> +		pr_err("%s: platform_device_register: %d\n", __func__, ret);
> +
> +		return ret;
> +	}
> +
> +	dev_set_name(&da8xx_dsp.dev, "%s.%d", da8xx_dsp.name, da8xx_dsp.id);

Why is this needed? This should have already been done by
platform_device_register() call above.

> +
> +	return 0;
> +};
> +
>  static struct resource da8xx_rtc_resources[] = {
>  	{
>  		.start		= DA8XX_RTC_BASE,
> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
> index aaccdc4..331fc28 100644
> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
> @@ -53,6 +53,7 @@ extern unsigned int da850_max_speed;
>  #define DA8XX_SYSCFG0_BASE	(IO_PHYS + 0x14000)
>  #define DA8XX_SYSCFG0_VIRT(x)	(da8xx_syscfg0_base + (x))
>  #define DA8XX_JTAG_ID_REG	0x18
> +#define DA8XX_HOST1CFG_REG	0x44
>  #define DA8XX_CFGCHIP0_REG	0x17c
>  #define DA8XX_CFGCHIP2_REG	0x184
>  #define DA8XX_CFGCHIP3_REG	0x188
> @@ -102,6 +103,11 @@ int __init da850_register_vpif_display
>  int __init da850_register_vpif_capture
>  			(struct vpif_capture_config *capture_config);
>  void da8xx_restart(char mode, const char *cmd);
> +#ifdef CONFIG_CMA
> +void __init da8xx_rproc_reserve_cma(void);
> +#endif

You dont typically use ifdefs around function declaration. You can
define this to be a function with empty implementation if remote proc is
not used.

> +int da8xx_register_rproc(void);
> +
>  
>  extern struct platform_device da8xx_serial_device;
>  extern struct emac_platform_data da8xx_emac_pdata;
> diff --git a/include/linux/platform_data/da8xx-remoteproc.h b/include/linux/platform_data/da8xx-remoteproc.h
> new file mode 100644
> index 0000000..50e8c55
> --- /dev/null
> +++ b/include/linux/platform_data/da8xx-remoteproc.h
> @@ -0,0 +1,33 @@
> +/*
> + * Remote Processor
> + *
> + * Copyright (C) 2011-2012 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __DA8XX_REMOTEPROC_H__
> +#define __DA8XX_REMOTEPROC_H__
> +
> +#include <linux/remoteproc.h>
> +
> +/**
> + * struct da8xx_rproc_pdata - da8xx remoteproc's platform data
> + * @name: the remoteproc's name
> + * @firmware: name of firmware file to load
> + * @ops: start/stop rproc handlers
> + */
> +struct da8xx_rproc_pdata {
> +	const char *name;
> +	const char *firmware;
> +	const struct rproc_ops *ops;
> +};

This file should be added a part of driver patch.

Thanks,
Sekhar

[PATCH v4 7/9] ARM: davinci: da850 board: Added .reserve function and rproc platform registration

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Signed-off-by: Robert Tivy <rtivy@ti.com>
> ---
>  arch/arm/mach-davinci/board-da850-evm.c     |    8 ++++++++
>  arch/arm/mach-davinci/board-omapl138-hawk.c |    8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> index b7cfb3e..0c959e9 100644
> --- a/arch/arm/mach-davinci/board-da850-evm.c
> +++ b/arch/arm/mach-davinci/board-da850-evm.c
> @@ -1550,6 +1550,11 @@ static __init void da850_evm_init(void)
>  		pr_warn("%s: SATA registration failed: %d\n", __func__, ret);
>  
>  	da850_evm_setup_mac_addr();
> +
> +	ret = da8xx_register_rproc();
> +	if (ret)
> +		pr_warn("%s: dsp/rproc registration failed: %d\n",
> +			__func__, ret);
>  }
>  
>  #ifdef CONFIG_SERIAL_8250_CONSOLE
> @@ -1577,4 +1582,7 @@ MACHINE_START(DAVINCI_DA850_EVM, "DaVinci DA850/OMAP-L138/AM18x EVM")
>  	.init_late	= davinci_init_late,
>  	.dma_zone_size	= SZ_128M,
>  	.restart	= da8xx_restart,
> +#ifdef CONFIG_CMA
> +	.reserve	= da8xx_rproc_reserve_cma,
> +#endif

Please get rid of this ifdef as well. You can have an empty function if
remote proc is not enabled in the build.

Thanks,
Sekhar

[PATCH v4 8/9] ARM: davinci: da850: Added dsp clock definition, keyed to "davinci-rproc.0"

[Sekhar Nori]

On 12/20/2012 7:04 AM, Robert Tivy wrote:
> Signed-off-by: Robert Tivy <rtivy@ti.com>
> ---
>  arch/arm/mach-davinci/da850.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 31ff65a..afc814f 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -369,6 +369,15 @@ static struct clk sata_clk = {
>  	.flags		= PSC_FORCE,
>  };
>  
> +static struct clk dsp_clk = {
> +	.name		= "dsp",
> +	.parent		= &pll0_sysclk1,
> +	.domain		= DAVINCI_GPSC_DSPDOMAIN,
> +	.lpsc		= DA8XX_LPSC0_GEM,
> +	.flags		= PSC_LRST,
> +	.reset		= davinci_clk_reset,

Instead of each clock node initializing the reset function, this can be
once in davinci_clk_init() whenever PSC_LRST is set in flags.

Thanks,
Sekhar