ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v6 1/2 ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hello.
Quoted 1 line · Robert Tivy
Adding a remoteproc driver implementation for OMAP-L138 DSP
You forgot to sign off the patch.
Quoted 4 lines
drivers/remoteproc/Kconfig +0
-0
[...]
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 45
Quoted 6 lines
drivers/remoteproc/Kconfig +2
-0@@ -41,4 +41,28 @@ config STE_MODEM_RPROC This can be either built-in or a loadable module. If unsure say N. +config DA8XX_REMOTEPROC + tristate "DaVinci DA850/OMAPL138 remoteproc support (EXPERIMENTAL)"
Neither DA850 nor OMAP-L138 are true DaVinci processors. Please drop the
"DaVinci" word.
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 46
Quoted 1 line
drivers/remoteproc/Kconfig +1
-0+ depends on ARCH_DAVINCI_DA850
It's also not clear why you limit the driver d\to DA850 while you call itda8xx_remoteproc.c. There's at least one more member to DA8xx familiy (at least supported in the community): DA830/OMAP-L137.
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 52
Quoted 6 lines
drivers/remoteproc/Kconfig +6
-0+ select REMOTEPROC + select RPMSG + select CMA + default n + help + Say y here to support DaVinci DA850/OMAPL138 remote processors
Same here.
Quoted 32 lines
drivers/remoteproc/da8xx_remoteproc.c new file +26
-0@@ -0,0 +1,327 @@ +/* +* Remote processor machine-specific module for DA8XX +* +* Copyright (C) 2013 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. +*/ + +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/remoteproc.h> + +#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */ + +#include "remoteproc_internal.h" + +static char *da8xx_fw_name;
drivers/remoteproc/da8xx_remoteproc.c line 36
Quoted 10 lines
drivers/remoteproc/da8xx_remoteproc.c new file +10
-0+module_param (da8xx_fw_name, charp,S_IRUGO ); +MODULE_PARM_DESC (da8xx_fw_name, +"\n\t\tName of DSP firmware file in /lib/firmware" ); + +/* +* OMAP-L138 Technical References: +* http://www.ti.com/product/omap-l138 +*/ +#define SYSCFG_CHIPSIG_OFFSET 0x174 +#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178
<mach/da8xx.h> has SYSCFG register #define's ending with '_REG', not
'_OFFSET' -- I'd like this tradition to be kept. And perhaps we should #define these registers there instead of the driver?
drivers/remoteproc/da8xx_remoteproc.c line 40
Quoted 4 lines
drivers/remoteproc/da8xx_remoteproc.c new file +4
-0+#define SYSCFG_CHIPINT0 BIT(0) +#define SYSCFG_CHIPINT1 BIT(1) +#define SYSCFG_CHIPINT2 BIT(2) +#define SYSCFG_CHIPINT3 BIT(3)
DA830/OMAP-l137 has the same registers. Only the datasheet calls the bits
CHIPSIGn there. Bit 4 also exists and means DSP NMI.
Quoted 1 line
drivers/remoteproc/da8xx_remoteproc.c new file +1
-0+static int da8xx_rproc_probe (struct platform_device *pdev)
[...]
drivers/remoteproc/da8xx_remoteproc.c
Quoted 4 lines
drivers/remoteproc/da8xx_remoteproc.c new file +4
-0+ chipsig =devm_request_and_ioremap (dev, chipsig_res); +if (!chipsig) { +dev_err (dev,"unable to map CHIPSIG register\n" ); +return -EINVAL ;
Why -EINVAL? Comment to devm_request_and_ioremap() suggests returning-EADDRNOTAVAIL.
drivers/remoteproc/da8xx_remoteproc.c
Quoted 6 lines
drivers/remoteproc/da8xx_remoteproc.c new file +6
-0+ } + + bootreg =devm_request_and_ioremap (dev, bootreg_res); +if (!bootreg) { +dev_err (dev,"unable to map boot register\n" ); +return -EINVAL ;
Same here.
drivers/remoteproc/da8xx_remoteproc.c
Quoted 12 lines
drivers/remoteproc/da8xx_remoteproc.c new file +12
-0+static int __devexit da8xx_rproc_remove(struct platform_device *pdev) +{ +struct rproc *rproc =platform_get_drvdata (pdev); +struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv; +int ret; + +/* +* The devm subsystem might end up releasing things before +* freeing the irq, thus allowing an interrupt to sneak in while +* the device is being removed. This should prevent that. +*/ +disable_irq (drproc->irq);
Will the IRQ be enabled properly upon reloading the driver? You're
effectively disabling it twice, once here and once in devm_free_irq(), aren't you?
drivers/remoteproc/da8xx_remoteproc.c
Quoted 3 lines
drivers/remoteproc/da8xx_remoteproc.c new file +3
-0+static struct platform_driver da8xx_rproc_driver = { + .probe = da8xx_rproc_probe, + .remove =__devexit_p (da8xx_rproc_remove),
Isn't _devexit_p() removed now? I thought __devinit and friends have all
been removed in 3.8-rc1...
Quoted 5 lines
drivers/remoteproc/remoteproc_core.c +0
-0
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, const char *name, · line 1231
Quoted 10 lines
drivers/remoteproc/remoteproc_core.c +3
-0@@ -1222,19 +1222,39 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, const char *firmware, int len) {struct rproc *rproc; +char *template ="rproc-%s-fw" ; +char *p;if (!dev || !name || !ops)return NULL ; +if (!firmware)
It makes sense to use {} despite singkle-statement branch.
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, const char *name, · line 1232
Quoted 1 line
drivers/remoteproc/remoteproc_core.c +1
-0+/*
Indent with tabs please.
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, const char *name, · line 1240
Quoted 8 lines
drivers/remoteproc/remoteproc_core.c +8
-0+* Make room for default firmware name (minus %s plus '\0'). +* If the caller didn't pass in a firmware name then +* construct a default name. We're already glomming 'len' +* bytes onto the end of the struct rproc allocation, so do +* a few more for the default firmware name (but only if +* the caller doesn't pass one). +*/ + len +=strlen (name) +strlen (template) -2 +1 ;
Same here.
Quoted 4 lines
drivers/remoteproc/remoteproc_core.c +0
-0rproc =kzalloc (sizeof (struct rproc ) + len,GFP_KERNEL );if (!rproc) {dev_err (dev,"%s: kzalloc failed\n" , __func__);return NULL ;
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, const char *name, · line 1254
Quoted 10 lines
drivers/remoteproc/remoteproc_core.c +8
-0} +if (!firmware) { + p = (char *)rproc +sizeof (struct rproc ) + len; +sprintf (p, template, name); + } +else + p = (char *)firmware; + + rproc->firmware = p;
Same here.
WBR, Sergei
PATCH v6 1/2 ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hi Sergei,
Quoted 11 lines
-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov at mvista.com]
Sent: Saturday, January 26, 2013 6:33 AMHello.
Quoted 1 line · Robert Tivy
Adding a remoteproc driver implementation for OMAP-L138 DSP
You forgot to sign off the patch.
Ugh, thanks for pointing that out, I will correct this in the next submission.
Quoted 10 lines
Quoted 5 lines
drivers/remoteproc/Kconfig +0
-0[...]
Quoted 4 lines
drivers/remoteproc/Kconfig +0
-0@@ -41,4 +41,28 @@ config STE_MODEM_RPROC This can be either built-in or a loadable module. If unsure say N.
Quoted 7 lines
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 45
Quoted 2 lines
drivers/remoteproc/Kconfig +2
-0+config DA8XX_REMOTEPROC + tristate "DaVinci DA850/OMAPL138 remoteproc support(EXPERIMENTAL)"
Neither DA850 nor OMAP-L138 are true DaVinci processors. Please
drop the
"DaVinci" word.
Ok, although DaVinci is used extensively in other places that support DA830/DA850.
Quoted 8 lines
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 46
Quoted 2 lines
drivers/remoteproc/Kconfig +1
-0+ depends on ARCH_DAVINCI_DA850It's also not clear why you limit the driver d\to DA850 while you
call itda8xx_remoteproc.c. There's at least one more member to DA8xx familiy (at
least supported in the community): DA830/OMAP-L137.
I'll change this to:
Depends on ARCH_DAVINCI_DA8XX
since that gets selected for either ARCH_DAVINCI_DA830 or ARCH_DAVINCI_DA850.
Quoted 52 lines
drivers/remoteproc/Kconfig config STE_MODEM_RPROC · line 52
Quoted 7 lines
drivers/remoteproc/Kconfig +6
-0+ select REMOTEPROC + select RPMSG + select CMA + default n + help + Say y here to support DaVinci DA850/OMAPL138 remote processorsSame here.
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/remoteproc/da8xx_remoteproc.cQuoted 17 lines
new file mode 100644
index 0000000..c6eb6bfdrivers/remoteproc/da8xx_remoteproc.c new file +12
-0@@ -0,0 +1,327 @@ +/* +* Remote processor machine-specific module for DA8XX +* +* Copyright (C) 2013 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. +*/ + +#include <linux/bitops.h> +#include <linux/clk.h> drivers/remoteproc/da8xx_remoteproc.c line 22
Quoted 10 lines
drivers/remoteproc/da8xx_remoteproc.c new file +10
-0+#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/remoteproc.h> + +#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
Quoted 12 lines
drivers/remoteproc/da8xx_remoteproc.c new file +12
-0+ +#include "remoteproc_internal.h" + +static char *da8xx_fw_name; +module_param (da8xx_fw_name, charp,S_IRUGO ); +MODULE_PARM_DESC (da8xx_fw_name, +"\n\t\tName of DSP firmware file in /lib/firmware" ); + +/* +* OMAP-L138 Technical References: +* http://www.ti.com/product/omap-l138 +*/
Quoted 8 lines
drivers/remoteproc/da8xx_remoteproc.c line 36
Quoted 2 lines
drivers/remoteproc/da8xx_remoteproc.c new file +2
-0+#define SYSCFG_CHIPSIG_OFFSET 0x174 +#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178<
mach/da8xx.h> has SYSCFG register #define's ending with '_REG',
not
'_OFFSET' -- I'd like this tradition to be kept. And perhaps we should #define
these registers there instead of the driver?
#include'ing header files in 'mach' subdirectories is frowned upon due to such header files being machine-specific, so I'm trying to avoid that. But as you noted in your subsequent email, these register offsets aren't used so I'll delete them altogether.
Quoted 9 lines
drivers/remoteproc/da8xx_remoteproc.c line 40
Quoted 5 lines
drivers/remoteproc/da8xx_remoteproc.c new file +4
-0+#define SYSCFG_CHIPINT0 BIT(0) +#define SYSCFG_CHIPINT1 BIT(1) +#define SYSCFG_CHIPINT2 BIT(2) +#define SYSCFG_CHIPINT3 BIT(3)DA830/OMAP-l137 has the same registers. Only the datasheet calls
the bits
CHIPSIGn there. Bit 4 also exists and means DSP NMI.
Since the register bits are termed CHIPSIGn I will change them to that name (I believe the "lines" or signals are called CHIPINTn).
Quoted 11 lines
Quoted 2 lines
drivers/remoteproc/da8xx_remoteproc.c new file +1
-0+static int da8xx_rproc_probe (struct platform_device *pdev)[...]
drivers/remoteproc/da8xx_remoteproc.c
Quoted 4 lines
drivers/remoteproc/da8xx_remoteproc.c new file +4
-0+ chipsig =devm_request_and_ioremap (dev, chipsig_res); +if (!chipsig) { +dev_err (dev,"unable to map CHIPSIG register\n" ); +return -EINVAL ;Why
-EINVAL? Comment todevm_request_and_ioremap()suggests
returning-EADDRNOTAVAIL.
OK, sounds better, I will change it.
Quoted 12 lines
drivers/remoteproc/da8xx_remoteproc.c
Quoted 7 lines
drivers/remoteproc/da8xx_remoteproc.c new file +6
-0+ } + + bootreg =devm_request_and_ioremap (dev, bootreg_res); +if (!bootreg) { +dev_err (dev,"unable to map boot register\n" ); +return -EINVAL ;Same here.
Quoted 1 line
+static int
__devexitda8xx_rproc_remove(structplatform_device*pdev)
Quoted 15 lines
drivers/remoteproc/da8xx_remoteproc.c
Quoted 11 lines
drivers/remoteproc/da8xx_remoteproc.c new file +11
-0+{ +struct rproc *rproc =platform_get_drvdata (pdev); +struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv; +int ret; + +/* +* The devm subsystem might end up releasing things before +* freeing the irq, thus allowing an interrupt to sneak in while +* the device is being removed. This should prevent that. +*/ +disable_irq (drproc->irq);Will the IRQ be enabled properly upon reloading the driver? You're
effectively disabling it twice, once here and once indevm_free_irq(), aren't you?
I suppose I am. I recall reading somewhere (can't find it now) that when using devm_request_threaded_irq(), when you remove the driver the irq doesn't actually get auto-freed until after some driver resources have been freed, thus rendering the ISR unusable were it to fire, so it suggested disabling the irq in the driver "remove" function.
Regardless, the irq appears to get enabled properly on the next module insertion.
Quoted 8 lines
drivers/remoteproc/da8xx_remoteproc.c
Quoted 4 lines
drivers/remoteproc/da8xx_remoteproc.c new file +3
-0+static struct platform_driver da8xx_rproc_driver = { + .probe = da8xx_rproc_probe, + .remove =__devexit_p (da8xx_rproc_remove),Isn't
_devexit_p()removed now? I thought__devinitand friends
have all
been removed in 3.8-rc1...
It appears that they're being deprecated for now, being #defined to empty. I will remove the macro usage and associated __dev* keywords.
Quoted 14 lines
Review
Quoted 2 lines
(quoted diff) +0
-0
b/drivers/remoteproc/remoteproc_core.cdrivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev,
Quoted 4 lines
index dd3bfaf..ac4449a 100644
drivers/remoteproc/remoteproc_core.c +0
-0@@ -1222,19 +1222,39 @@ struct rproc *rproc_alloc(struct device *dev,const char *name,
Quoted 6 lines
drivers/remoteproc/remoteproc_core.c +2
-0const char *firmware, int len) {struct rproc *rproc; +char *template ="rproc-%s-fw" ; +char *p;
Quoted 6 lines
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, · line 1231
Quoted 4 lines
drivers/remoteproc/remoteproc_core.c +1
-0if (!dev || !name || !ops)return NULL ; +if (!firmware)It makes sense to use {} despite singkle-statement branch.
According to Documentation/CodingStyle:
Do not unnecessarily use braces where a single statement will do. if (condition) action();
Quoted 4 lines
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, · line 1232
Quoted 2 lines
drivers/remoteproc/remoteproc_core.c +1
-0+/* Indent with tabs please.
My bad, I will fix this (although I blame cut-and-paste, as well as it being a Friday night :)
Quoted 28 lines
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, · line 1233
Quoted 2 lines
drivers/remoteproc/remoteproc_core.c +1
-0+* Make room for default firmware name (minus %s plus '\0').
drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, · line 1240
Quoted 7 lines
drivers/remoteproc/remoteproc_core.c +7
-0+* If the caller didn't pass in a firmware name then +* construct a default name. We're already glomming 'len' +* bytes onto the end of the struct rproc allocation, so do +* a few more for the default firmware name (but only if +* the caller doesn't pass one). +*/ + len +=strlen (name) +strlen (template) -2 +1 ;Same here.
Quoted 4 lines
drivers/remoteproc/remoteproc_core.c +0
-0rproc =kzalloc (sizeof (struct rproc ) + len,GFP_KERNEL );if (!rproc) {dev_err (dev,"%s: kzalloc failed\n" , __func__);return NULL ;drivers/remoteproc/remoteproc_core.c struct rproc *rproc_alloc(struct device *dev, · line 1254
Quoted 10 lines
drivers/remoteproc/remoteproc_core.c +8
-0} +if (!firmware) { + p = (char *)rproc +sizeof (struct rproc ) + len; +sprintf (p, template, name); + } +else + p = (char *)firmware; + + rproc->firmware = p;
Quoted 1 line
Same here.
This one does violate Documentation/CodingStyle, as it says:
if (condition) {
do_this();
do_that();
} else {
otherwise();
}I will correct this.
Quoted 3 lines
WBR, Sergei
PATCH v6 1/2 ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
In reply to a message not in the archiveHello.
Quoted 8 lines · Robert Tivy
Adding a remoteproc driver implementation for OMAP-L138 DSP
drivers/remoteproc/da8xx_remoteproc.c new file +0
-0@@ -0,0 +1,327 @@
[...]
drivers/remoteproc/da8xx_remoteproc.c
Quoted 2 lines
drivers/remoteproc/da8xx_remoteproc.c new file +2
-0+#define SYSCFG_CHIPSIG_OFFSET 0x174 +#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178
Wait, you don't even use these #define's -- why they're here at all?
WBR, Sergei
PATCH v6 1/2 ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Quoted 18 lines
-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov at mvista.com]
Sent: Saturday, January 26, 2013 7:22 AMHello.
Review
Quoted 3 lines · Robert Tivy
Adding a remoteproc driver implementation for OMAP-L138 DSP
(quoted diff) +0
-0
b/drivers/remoteproc/da8xx_remoteproc.cQuoted 5 lines
new file mode 100644
index 0000000..c6eb6bfdrivers/remoteproc/da8xx_remoteproc.c new file +0
-0@@ -0,0 +1,327 @@[...]
Quoted 7 lines
drivers/remoteproc/da8xx_remoteproc.c
Quoted 2 lines
drivers/remoteproc/da8xx_remoteproc.c new file +2
-0+#define SYSCFG_CHIPSIG_OFFSET 0x174 +#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178Wait, you don't even use these #define's -- why they're here at
all?WBR, Sergei
I will remove those. They were inadvertently left over from when the driver was mapping SYSCFG0's base address and using those offsets to get to the CHIPSIG registers.
Regards,
- Rob
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v6 2/2 ARM: davinci: Remoteproc platform device creation data/code
Hello.
Quoted 4 lines · Robert Tivy
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.
Quoted 4 lines
arch/arm/mach-davinci/devices-da8xx.c +0
-0
[...]
Quoted 4 lines
arch/arm/mach-davinci/devices-da8xx.c +0
-0@@ -706,6 +706,96 @@ int __init da850_register_mmcsd1(struct davinci_mmc_config *config) }#endif
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct davinci_mmc_config *config) · line 718
Quoted 10 lines
arch/arm/mach-davinci/devices-da8xx.c +10
-0+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?
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct davinci_mmc_config *config) · line 725
Quoted 7 lines
arch/arm/mach-davinci/devices-da8xx.c +7
-0+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".
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct davinci_mmc_config *config) · line 726
Quoted 1 line
arch/arm/mach-davinci/devices-da8xx.c +1
-0+
Empty line hardly needed here.
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct davinci_mmc_config *config) · line 727
Quoted 1 line
arch/arm/mach-davinci/devices-da8xx.c +1
-0+return ret;
Not needed here, just move it outside the {} to replace 'return 0'.
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct davinci_mmc_config *config) · line 732
Quoted 5 lines
arch/arm/mach-davinci/devices-da8xx.c +5
-0+ } + +return 0 ; +}; +
WBR, Sergei
PATCH v6 2/2 ARM: davinci: Remoteproc platform device creation data/code
Hi Sergei,
Quoted 6 lines
-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov at mvista.com]
Sent: Saturday, January 26, 2013 6:43 AMHello.
Quoted 8 lines
Quoted 4 lines · Robert Tivy
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.
Quoted 16 lines
arch/arm/mach-
Quoted 2 lines
arch/arm/mach- renamed from arch/arm/mach-davinci/devices-da8xx.c +0
-0
davinci/devices-da8xx.cQuoted 3 lines
index fb2f51b..a455e5c 100644
arch/arm/mach-davinci/devices-da8xx.c +0
-0[...]
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct
Quoted 1 line
arch/arm/mach-davinci/devices-da8xx.c +0
-0@@ -706,6 +706,96 @@ int __init da850_register_mmcsd1(struct
davinci_mmc_config*config)arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 711
Quoted 6 lines
arch/arm/mach-davinci/devices-da8xx.c +3
-0}#endif +static struct resource da8xx_rproc_resources[] = { + {/* DSP boot address */ + .start = DA8XX_SYSCFG0_BASE +
DA8XX_HOST1CFG_REG,
Quoted 11 lines
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 718
Quoted 7 lines
arch/arm/mach-davinci/devices-da8xx.c +7
-0+ .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.
Quoted 11 lines
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 725
Quoted 8 lines
arch/arm/mach-davinci/devices-da8xx.c +7
-0+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.
Quoted 8 lines
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 726
Quoted 2 lines
arch/arm/mach-davinci/devices-da8xx.c +1
-0+Empty line hardly needed here.
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 727
Quoted 1 line
arch/arm/mach-davinci/devices-da8xx.c +1
-0+return ret;Not needed here, just move it outside the {} to replace 'return 0'.
Thanks, I will change this.
Regards,
- Rob
Quoted 8 lines
arch/arm/mach-davinci/devices-da8xx.c int __init da850_register_mmcsd1(struct · line 732
Quoted 6 lines
arch/arm/mach-davinci/devices-da8xx.c +5
-0+ } + +return 0 ; +}; +WBR, Sergei