| Message ID | 1359168322-17733-2-git-send-email-rtivy@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello. On 26-01-2013 6:45, Robert Tivy wrote: > Adding a remoteproc driver implementation for OMAP-L138 DSP You forgot to sign off the patch. > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index 96ce101..e923599 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig [...] > @@ -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. > + depends on ARCH_DAVINCI_DA850 It's also not clear why you limit the driver d\to DA850 while you call it da8xx_remoteproc.c. There's at least one more member to DA8xx familiy (at least supported in the community): DA830/OMAP-L137. > + select REMOTEPROC > + select RPMSG > + select CMA > + default n > + help > + Say y here to support DaVinci DA850/OMAPL138 remote processors Same here. > diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c > new file mode 100644 > index 0000000..c6eb6bf > --- /dev/null > +++ b/drivers/remoteproc/da8xx_remoteproc.c > @@ -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; > +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? > +#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. > +static int da8xx_rproc_probe(struct platform_device *pdev) [...] > + 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. > + } > + > + bootreg = devm_request_and_ioremap(dev, bootreg_res); > + if (!bootreg) { > + dev_err(dev, "unable to map boot register\n"); > + return -EINVAL; Same here. > +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? > +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... > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index dd3bfaf..ac4449a 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -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. > + /* Indent with tabs please. > + * 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. > rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL); > if (!rproc) { > dev_err(dev, "%s: kzalloc failed\n", __func__); > return NULL; > } > > + if (!firmware) { > + p = (char *)rproc + sizeof(struct rproc) + len; > + sprintf(p, template, name); > + } > + else > + p = (char *)firmware; > + > + rproc->firmware = p; Same here. WBR, Sergei
Hello. On 26-01-2013 6:45, Robert Tivy wrote: > Adding a remoteproc driver implementation for OMAP-L138 DSP > diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c > new file mode 100644 > index 0000000..c6eb6bf > --- /dev/null > +++ b/drivers/remoteproc/da8xx_remoteproc.c > @@ -0,0 +1,327 @@ [...] > +#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
> -----Original Message----- > From: Sergei Shtylyov [mailto:sshtylyov@mvista.com] > Sent: Saturday, January 26, 2013 7:22 AM > > Hello. > > On 26-01-2013 6:45, Robert Tivy wrote: > > > Adding a remoteproc driver implementation for OMAP-L138 DSP > > > diff --git a/drivers/remoteproc/da8xx_remoteproc.c > b/drivers/remoteproc/da8xx_remoteproc.c > > new file mode 100644 > > index 0000000..c6eb6bf > > --- /dev/null > > +++ b/drivers/remoteproc/da8xx_remoteproc.c > > @@ -0,0 +1,327 @@ > [...] > > +#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 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
Hi Sergei, > -----Original Message----- > From: Sergei Shtylyov [mailto:sshtylyov@mvista.com] > Sent: Saturday, January 26, 2013 6:33 AM > > Hello. > > On 26-01-2013 6:45, Robert Tivy wrote: > > > 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. > > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > > index 96ce101..e923599 100644 > > --- a/drivers/remoteproc/Kconfig > > +++ b/drivers/remoteproc/Kconfig > [...] > > @@ -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. Ok, although DaVinci is used extensively in other places that support DA830/DA850. > > > + depends on ARCH_DAVINCI_DA850 > > It's also not clear why you limit the driver d\to DA850 while you > call it > da8xx_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. > > > + select REMOTEPROC > > + select RPMSG > > + select CMA > > + default n > > + help > > + Say y here to support DaVinci DA850/OMAPL138 remote processors > > Same here. > > > diff --git a/drivers/remoteproc/da8xx_remoteproc.c > b/drivers/remoteproc/da8xx_remoteproc.c > > new file mode 100644 > > index 0000000..c6eb6bf > > --- /dev/null > > +++ b/drivers/remoteproc/da8xx_remoteproc.c > > @@ -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; > > +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? #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. > > > +#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). > > > +static int da8xx_rproc_probe(struct platform_device *pdev) > [...] > > + 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. OK, sounds better, I will change it. > > > + } > > + > > + bootreg = devm_request_and_ioremap(dev, bootreg_res); > > + if (!bootreg) { > > + dev_err(dev, "unable to map boot register\n"); > > + return -EINVAL; > > Same here. > > > +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? 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. > > > +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... It appears that they're being deprecated for now, being #defined to empty. I will remove the macro usage and associated __dev* keywords. > > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > > index dd3bfaf..ac4449a 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -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. According to Documentation/CodingStyle: Do not unnecessarily use braces where a single statement will do. if (condition) action(); > > > + /* > > Indent with tabs please. My bad, I will fix this (although I blame cut-and-paste, as well as it being a Friday night :) > > > + * 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. > > > rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL); > > if (!rproc) { > > dev_err(dev, "%s: kzalloc failed\n", __func__); > > return NULL; > > } > > > > + if (!firmware) { > > + p = (char *)rproc + sizeof(struct rproc) + len; > > + sprintf(p, template, name); > > + } > > + else > > + p = (char *)firmware; > > + > > + rproc->firmware = p; > > Same here. This one does violate Documentation/CodingStyle, as it says: if (condition) { do_this(); do_that(); } else { otherwise(); } I will correct this. > > WBR, Sergei >
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 96ce101..e923599 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -5,7 +5,7 @@ config REMOTEPROC tristate depends on EXPERIMENTAL depends on HAS_DMA - select FW_CONFIG + select FW_LOADER select VIRTIO config OMAP_REMOTEPROC @@ -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)" + depends on ARCH_DAVINCI_DA850 + select REMOTEPROC + select RPMSG + select CMA + default n + help + Say y here to support DaVinci DA850/OMAPL138 remote processors + via the remote processor framework. + + You want to say y here in order to enable AMP + use-cases to run on your platform (multimedia codecs are + offloaded to remote DSP processors using this framework). + + This module controls the name of the firmware file that gets + loaded on the DSP. This file must reside in the /lib/firmware + directory. It can be specified via the module parameter + da8xx_fw_name=<filename>, and if not specified will default to + "rproc-dsp-fw". + + It's safe to say n here if you're not interested in multimedia + offloading or just want a bare minimum kernel. + endmenu diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 391b651..ac2ff75 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -9,3 +9,4 @@ remoteproc-y += remoteproc_virtio.o remoteproc-y += remoteproc_elf_loader.o obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o obj-$(CONFIG_STE_MODEM_RPROC) += ste_modem_rproc.o +obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c new file mode 100644 index 0000000..c6eb6bf --- /dev/null +++ b/drivers/remoteproc/da8xx_remoteproc.c @@ -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; +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 +#define SYSCFG_CHIPINT0 BIT(0) +#define SYSCFG_CHIPINT1 BIT(1) +#define SYSCFG_CHIPINT2 BIT(2) +#define SYSCFG_CHIPINT3 BIT(3) + +/** + * struct da8xx_rproc - da8xx remote processor instance state + * @rproc: rproc handle + * @dsp_clk: placeholder for platform's DSP clk + * @ack_fxn: chip-specific ack function for ack'ing irq + * @irq_data: ack_fxn function parameter + * @chipsig: virt ptr to DSP interrupt registers (CHIPSIG & CHIPSIG_CLR) + * @bootreg: virt ptr to DSP boot address register (HOST1CFG) + * @irq: irq # used by this instance + */ +struct da8xx_rproc { + struct rproc *rproc; + struct clk *dsp_clk; + void (*ack_fxn)(struct irq_data *data); + struct irq_data *irq_data; + void __iomem *chipsig; + void __iomem *bootreg; + int irq; +}; + +/** + * handle_event() - inbound virtqueue message workqueue function + * + * This function is registered as a kernel thread and is scheduled by the + * kernel handler. + */ +static irqreturn_t handle_event(int irq, void *p) +{ + struct rproc *rproc = (struct rproc *)p; + + /* Process incoming buffers on our vring */ + while (IRQ_HANDLED == rproc_vq_interrupt(rproc, 0)) + ; + + /* Must allow wakeup of potenitally blocking senders */ + rproc_vq_interrupt(rproc, 1); + + return IRQ_HANDLED; +} + +/** + * da8xx_rproc_callback() - inbound virtqueue message handler + * + * This handler is invoked directly by the kernel whenever the remote + * core (DSP) has modified the state of a virtqueue. There is no + * "payload" message indicating the virtqueue index as is the case with + * mailbox-based implementations on OMAP4. As such, this handler "polls" + * each known virtqueue index for every invocation. + */ +static irqreturn_t da8xx_rproc_callback(int irq, void *p) +{ + struct rproc *rproc = (struct rproc *)p; + struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv; + u32 chipsig; + + chipsig = readl(drproc->chipsig); + if (chipsig & SYSCFG_CHIPINT0) { + /* Clear interrupt level source */ + writel(SYSCFG_CHIPINT0, drproc->chipsig + 4); + + /* + * ACK intr to AINTC. + * + * It has already been ack'ed by the kernel before calling + * this function, but since the ARM<->DSP interrupts in the + * CHIPSIG register are "level" instead of "pulse" variety, + * we need to ack it after taking down the level else we'll + * be called again immediately after returning. + */ + drproc->ack_fxn(drproc->irq_data); + + return IRQ_WAKE_THREAD; + } + + return IRQ_HANDLED; +} + +static int da8xx_rproc_start(struct rproc *rproc) +{ + struct device *dev = rproc->dev.parent; + struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv; + struct clk *dsp_clk; + + /* hw requires the start (boot) address be on 1KB boundary */ + if (rproc->bootaddr & 0x3ff) { + dev_err(dev, "invalid boot address: must be aligned to 1KB\n"); + + return -EINVAL; + } + writel(rproc->bootaddr, drproc->bootreg); + + dsp_clk = devm_clk_get(dev, NULL); + if (IS_ERR(dsp_clk)) { + dev_err(dev, "clk_get error: %ld\n", PTR_ERR(dsp_clk)); + + return PTR_RET(dsp_clk); + } + drproc->dsp_clk = dsp_clk; + + clk_enable(dsp_clk); + davinci_clk_reset_deassert(dsp_clk); + + return 0; +} + +static int da8xx_rproc_stop(struct rproc *rproc) +{ + struct da8xx_rproc *drproc = rproc->priv; + struct clk *dsp_clk = drproc->dsp_clk; + + clk_disable(dsp_clk); + devm_clk_put(rproc->dev.parent, dsp_clk); + + return 0; +} + +/* kick a virtqueue */ +static void da8xx_rproc_kick(struct rproc *rproc, int vqid) +{ + struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv; + int timed_out; + unsigned long timeout; + + timed_out = 0; + timeout = jiffies + HZ/100; + + /* Poll for ack from other side first */ + while (readl(drproc->chipsig) & SYSCFG_CHIPINT2) + if (time_after(jiffies, timeout)) { + if (readl(drproc->chipsig) & SYSCFG_CHIPINT2) { + dev_err(rproc->dev.parent, + "failed to receive ack\n"); + timed_out = 1; + } + + break; + } + + if (!timed_out) + /* Interupt remote proc */ + writel(SYSCFG_CHIPINT2, drproc->chipsig); +} + +static struct rproc_ops da8xx_rproc_ops = { + .start = da8xx_rproc_start, + .stop = da8xx_rproc_stop, + .kick = da8xx_rproc_kick, +}; + +static int da8xx_rproc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct da8xx_rproc *drproc; + struct rproc *rproc; + struct clk *dsp_clk; + struct irq_data *irq_data; + struct resource *bootreg_res; + struct resource *chipsig_res; + void __iomem *chipsig; + void __iomem *bootreg; + int irq; + int ret; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_err(dev, "platform_get_irq(pdev, 0) error: %d\n", irq); + return irq; + } + + irq_data = irq_get_irq_data(irq); + if (!irq_data) { + dev_err(dev, "irq_get_irq_data(%d): NULL\n", irq); + return -EINVAL; + } + + bootreg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!bootreg_res) { + dev_err(dev, + "platform_get_resource(IORESOURCE_MEM, 0): NULL\n"); + return -EINVAL; + } + + chipsig_res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (!chipsig_res) { + dev_err(dev, + "platform_get_resource(IORESOURCE_MEM, 1): NULL\n"); + return -EINVAL; + } + + chipsig = devm_request_and_ioremap(dev, chipsig_res); + if (!chipsig) { + dev_err(dev, "unable to map CHIPSIG register\n"); + return -EINVAL; + } + + bootreg = devm_request_and_ioremap(dev, bootreg_res); + if (!bootreg) { + dev_err(dev, "unable to map boot register\n"); + return -EINVAL; + } + + rproc = rproc_alloc(dev, "dsp", &da8xx_rproc_ops, da8xx_fw_name, + sizeof(*drproc)); + if (!rproc) + return -ENOMEM; + + drproc = rproc->priv; + drproc->rproc = rproc; + + platform_set_drvdata(pdev, rproc); + + /* + * rproc_add() can end up enabling the DSP's clk with the DSP + * *not* in reset, but da8xx_rproc_start() needs the DSP to be + * held in reset at the time it is called. + */ + dsp_clk = clk_get(dev, NULL); + if (IS_ERR(dsp_clk)) { + dev_err(dev, "clk_get error: %ld\n", PTR_ERR(dsp_clk)); + ret = PTR_RET(dsp_clk); + goto free_rproc; + } + davinci_clk_reset_assert(dsp_clk); + clk_put(dsp_clk); + + ret = rproc_add(rproc); + if (ret) { + dev_err(dev, "rproc_add failed: %d\n", ret); + goto free_rproc; + } + + drproc->chipsig = chipsig; + drproc->bootreg = bootreg; + drproc->ack_fxn = irq_data->chip->irq_ack; + drproc->irq_data = irq_data; + drproc->irq = irq; + + /* everything the ISR needs is now setup, so hook it up */ + ret = devm_request_threaded_irq(dev, irq, da8xx_rproc_callback, + handle_event, 0, "da8xx-remoteproc", rproc); + if (ret) { + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret); + rproc_del(rproc); + goto free_rproc; + } + + return 0; + +free_rproc: + rproc_put(rproc); + + return ret; +} + +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); + + ret = rproc_del(rproc); + rproc_put(rproc); + + return ret; +} + +static struct platform_driver da8xx_rproc_driver = { + .probe = da8xx_rproc_probe, + .remove = __devexit_p(da8xx_rproc_remove), + .driver = { + .name = "davinci-rproc", + .owner = THIS_MODULE, + }, +}; + +module_platform_driver(da8xx_rproc_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Davinci DA850 Remote Processor control driver"); diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index dd3bfaf..ac4449a 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -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) + /* + * 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; + rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL); if (!rproc) { dev_err(dev, "%s: kzalloc failed\n", __func__); return NULL; } + if (!firmware) { + p = (char *)rproc + sizeof(struct rproc) + len; + sprintf(p, template, name); + } + else + p = (char *)firmware; + + rproc->firmware = p; rproc->name = name; rproc->ops = ops; - rproc->firmware = firmware; rproc->priv = &rproc[1]; device_initialize(&rproc->dev);