RE: [PATCH v6 2/2] ARM: davinci: Remoteproc platform device creation data/code

2013-01-28 Thread Tivy, Robert
Hi Sergei,

 -Original Message-
 From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
 Sent: Saturday, January 26, 2013 6:43 AM
 
 Hello.
 
 On 26-01-2013 6:45, Robert Tivy wrote:
 
  Added a new remoteproc platform device for DA8XX.  Contains CMA-based
  reservation of physical memory block.  A new kernel command-line
  parameter has been added to allow boot-time specification of the
  physical memory block.
 
 No signoff again.

Thanks, I will fix this.

 
  diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-
 davinci/devices-da8xx.c
  index fb2f51b..a455e5c 100644
  --- a/arch/arm/mach-davinci/devices-da8xx.c
  +++ b/arch/arm/mach-davinci/devices-da8xx.c
 [...]
  @@ -706,6 +706,96 @@ 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 + 3,
  +   .flags  = IORESOURCE_MEM,
  +   },
  +   { /* DSP interrupt registers */
  +   .start  = DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG,
  +   .end= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG + 7,
  +   .flags  = IORESOURCE_MEM,
 
 Does it really make sense to pass these as 2 resources -- they have
 the
 same base address?

I didn't want to impart that knowledge on the remoteproc driver.  As far as the 
driver is concerned, there is a boot address register and some 
interrupt-related registers.  That the boot address and interrupt-related 
registers share the same base address is a device-specific fact.

 
  +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);
 
 Better message would be can't register DSP device.

Agreed, I will change this.

 
  +
 
 Empty line hardly needed here.
 
  +   return ret;
 
 Not needed here, just move it outside the {} to replace 'return 0'.

Thanks, I will change this.

Regards,

- Rob

 
  +   }
  +
  +   return 0;
  +};
  +
 
 WBR, Sergei

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v6 2/2] ARM: davinci: Remoteproc platform device creation data/code

2013-01-26 Thread Sergei Shtylyov

Hello.

On 26-01-2013 6:45, Robert Tivy wrote:


Added a new remoteproc platform device for DA8XX.  Contains CMA-based
reservation of physical memory block.  A new kernel command-line
parameter has been added to allow boot-time specification of the
physical memory block.


   No signoff again.


diff --git a/arch/arm/mach-davinci/devices-da8xx.c 
b/arch/arm/mach-davinci/devices-da8xx.c
index fb2f51b..a455e5c 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c

[...]

@@ -706,6 +706,96 @@ 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 + 3,
+   .flags  = IORESOURCE_MEM,
+   },
+   { /* DSP interrupt registers */
+   .start  = DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG,
+   .end= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG + 7,
+   .flags  = IORESOURCE_MEM,


   Does it really make sense to pass these as 2 resources -- they have the 
same base address?



+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);


   Better message would be can't register DSP device.


+


   Empty line hardly needed here.


+   return ret;


   Not needed here, just move it outside the {} to replace 'return 0'.


+   }
+
+   return 0;
+};
+


WBR, Sergei

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source