[GIT PULL] remoteproc cleanups for 3.15
The following changes since commit cfbf8d4857c26a8a307fb7cd258074c9dcd8c691: Linux 3.14-rc4 (2014-02-23 17:40:03 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git tags/remoteproc-3.15-cleanups for you to fetch changes up to bd88acba5f9809af48f66267bb16024b9e33cf2b: remoteproc/ste_modem: staticize local symbols (2014-02-24 11:16:16 +0200) Several remoteproc cleanup patches coming from Jingoo Han, Julia Lawall and Uwe Kleine-König. Jingoo Han (1): remoteproc/ste_modem: staticize local symbols Julia Lawall (1): remoteproc/davinci: simplify use of devm_ioremap_resource Uwe Kleine-König (1): remoteproc/davinci: drop needless devm_clk_put drivers/remoteproc/da8xx_remoteproc.c | 16 +--- drivers/remoteproc/ste_modem_rproc.c | 4 ++-- 2 files changed, 3 insertions(+), 17 deletions(-) ___ 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 v10 1/1] ARM: davinci: Add a remoteproc driver implementation for OMAP-L13x DSP
Hi Rob, On Wed, Apr 10, 2013 at 12:20 AM, Robert Tivy rt...@ti.com wrote: Adding a new remoteproc driver for OMAP-L13x DSP Signed-off-by: Robert Tivy rt...@ti.com Looks good, thanks! It's now applied (with some tiny changes) to git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git#for-next. Sekhar, this for-next branch is immutable, feel free to merge it in order to apply the davinci-specific patches. Thanks, Ohad. ___ 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 v9 1/6] rpmsg: Process all available messages in virtqueue callback
On Tue, Apr 9, 2013 at 1:56 AM, Tivy, Robert rt...@ti.com wrote:
Shouldn't the virtqueue_kick() be called only when we successfully added a
buffer with virtqueue_add_buf()?
Definitely, thanks for noticing!
Take 2:
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index a59684b..4ade672 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -776,22 +776,11 @@ out:
}
EXPORT_SYMBOL(rpmsg_send_offchannel_raw);
-/* called when an rx buffer is used, and it's time to digest a message */
-static void rpmsg_recv_done(struct virtqueue *rvq)
+static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
+struct rpmsg_hdr *msg, unsigned int len)
{
- struct rpmsg_hdr *msg;
- unsigned int len;
struct rpmsg_endpoint *ept;
struct scatterlist sg;
- struct virtproc_info *vrp = rvq-vdev-priv;
- struct device *dev = rvq-vdev-dev;
- int err;
-
- msg = virtqueue_get_buf(rvq, len);
- if (!msg) {
- dev_err(dev, uhm, incoming signal, but no used buffer ?\n);
- return;
- }
dev_dbg(dev, From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n,
msg-src, msg-dst, msg-len,
@@ -806,7 +795,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
if (len RPMSG_BUF_SIZE ||
msg-len (len - sizeof(struct rpmsg_hdr))) {
dev_warn(dev, inbound msg too big: (%d, %d)\n, len, msg-len);
- return;
+ return -EINVAL;
}
/* use the dst addr to fetch the callback of the appropriate user */
@@ -842,11 +831,42 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
err = virtqueue_add_buf(vrp-rvq, sg, 0, 1, msg, GFP_KERNEL);
if (err 0) {
dev_err(dev, failed to add a virtqueue buffer: %d\n, err);
+ return err;
+ }
+
+ return 0;
+}
+
+/* called when an rx buffer is used, and it's time to digest a message */
+static void rpmsg_recv_done(struct virtqueue *rvq)
+{
+ struct virtproc_info *vrp = rvq-vdev-priv;
+ struct device *dev = rvq-vdev-dev;
+ struct rpmsg_hdr *msg;
+ unsigned int len, msgs_received = 0;
+ int err;
+
+ msg = virtqueue_get_buf(rvq, len);
+ if (!msg) {
+ dev_err(dev, uhm, incoming signal, but no used buffer ?\n);
return;
}
+ while (msg) {
+ err = rpmsg_recv_single(vrp, dev, msg, len);
+ if (err)
+ break;
+
+ msgs_received++;
+
+ msg = virtqueue_get_buf(rvq, len);
+ };
+
+ dev_dbg(dev, Received %u messages\n, msgs_received);
+
/* tell the remote processor we added another available rx buffer */
- virtqueue_kick(vrp-rvq);
+ if (msgs_received)
+ virtqueue_kick(vrp-rvq);
}
/*
Thanks for the rewrite, looks better. I'll give it a try and let you know
how it goes.
Thanks!
Ohad.
___
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 v9 4/6] ARM: davinci: Add a remoteproc driver implementation for OMAP-L13x DSP
On Tue, Apr 9, 2013 at 1:02 AM, Tivy, Robert rt...@ti.com wrote: The platform_get_irq()/irq_get_irq_data()/platform_get_resource() calls don't have put counterparts. The devm_* calls get automatically cleaned up (and I wasn't able to find any existing example of some code that actually does the cleanup explicitly). Fair enough, thanks for looking. Sergei had a comment to change devm_request_and_ioremap() to devm_ioremap_resource(), so I will submit a new patch with your and Sergei's comments applied. Thanks! ___ 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 v9 2/6] remoteproc: Change typo FW_CONFIG to FW_LOADER for REMOTEPROC
On Fri, Mar 29, 2013 at 4:41 AM, Robert Tivy rt...@ti.com wrote: Fix obvious typo introduced in commit e121aefa7d9f10eee5cf26ed47129237a05d940b (remoteproc: fix missing CONFIG_FW_LOADER configurations). Signed-off-by: Robert Tivy rt...@ti.com Applied, thanks. BTW - we don't usually want to directly cc sta...@vger.kernel.org. Instead we just add a Cc tag in the commit log (fixed before applying). ___ 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 v9 3/6] remoteproc: Add support to rproc_alloc() for a default firmware name
On Fri, Mar 29, 2013 at 4:41 AM, Robert Tivy rt...@ti.com wrote: Signed-off-by: Robert Tivy rt...@ti.com Applied with the following changes: - add commit log - document change in function description - use snprintf instead - minor style change Thanks! ___ 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 v9 1/6] rpmsg: Process all available messages in virtqueue callback
On Fri, Mar 29, 2013 at 4:41 AM, Robert Tivy rt...@ti.com wrote:
Change virtqueue callback function rpmsg_recv_done() to process all
available messages instead of just one message.
Signed-off-by: Robert Tivy rt...@ti.com
I'm thinking instead of adding an indentation level, let's split the
_recv function.
This is what I have in mind - let me know if it works for you - thanks:
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index a59684b..42b9872 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -776,22 +776,11 @@ out:
}
EXPORT_SYMBOL(rpmsg_send_offchannel_raw);
-/* called when an rx buffer is used, and it's time to digest a message */
-static void rpmsg_recv_done(struct virtqueue *rvq)
+static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
+struct rpmsg_hdr *msg, unsigned int len)
{
- struct rpmsg_hdr *msg;
- unsigned int len;
struct rpmsg_endpoint *ept;
struct scatterlist sg;
- struct virtproc_info *vrp = rvq-vdev-priv;
- struct device *dev = rvq-vdev-dev;
- int err;
-
- msg = virtqueue_get_buf(rvq, len);
- if (!msg) {
- dev_err(dev, uhm, incoming signal, but no used buffer ?\n);
- return;
- }
dev_dbg(dev, From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n,
msg-src, msg-dst, msg-len,
@@ -806,7 +795,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
if (len RPMSG_BUF_SIZE ||
msg-len (len - sizeof(struct rpmsg_hdr))) {
dev_warn(dev, inbound msg too big: (%d, %d)\n, len, msg-len);
- return;
+ return -EINVAL;
}
/* use the dst addr to fetch the callback of the appropriate user */
@@ -842,9 +831,39 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
err = virtqueue_add_buf(vrp-rvq, sg, 0, 1, msg, GFP_KERNEL);
if (err 0) {
dev_err(dev, failed to add a virtqueue buffer: %d\n, err);
+ return err;
+ }
+
+ return 0;
+}
+
+/* called when an rx buffer is used, and it's time to digest a message */
+static void rpmsg_recv_done(struct virtqueue *rvq)
+{
+ struct virtproc_info *vrp = rvq-vdev-priv;
+ struct device *dev = rvq-vdev-dev;
+ struct rpmsg_hdr *msg;
+ unsigned int len, msgs_received = 0;
+ int err;
+
+ msg = virtqueue_get_buf(rvq, len);
+ if (!msg) {
+ dev_err(dev, uhm, incoming signal, but no used buffer ?\n);
return;
}
+ while (msg) {
+ err = rpmsg_recv_single(vrp, dev, msg, len);
+ if (err)
+ break;
+
+ msgs_received++;
+
+ msg = virtqueue_get_buf(rvq, len);
+ };
+
+ dev_dbg(dev, Received %u messages\n, msgs_received);
+
/* tell the remote processor we added another available rx buffer */
virtqueue_kick(vrp-rvq);
}
___
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 v9 0/6] ARM: davinci: remoteproc support
Hi Rob, On Fri, Mar 29, 2013 at 4:41 AM, Robert Tivy rt...@ti.com wrote: This patch series adds remoteproc support for OMAP-L13x, along with needed supporting mach-davinci infrastructure. Patches look good, thanks for pushing. I'm applying 2 and 3 now. Please ack the change I've done for 1 and I'll apply it too. No. 4 just needs a small change, if you could please respin quickly I'll apply it immediately. I prefer patches 5 and 6 go through Sekhar, guess after I'll have no. 4 applied in an immutable branch Sekhar could pull it and apply 5 and 6. Thanks, Ohad. ___ 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 v7 1/2] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hi Rob,
On Tue, Feb 19, 2013 at 1:02 AM, Tivy, Robert rt...@ti.com wrote:
It looks like this patch squashes:
1. A fix to drivers/remoteproc/Kconfig
2. New functionality in remoteproc_core.c
3. A new da8xx driver
These are independent changes, so we better split them up to separate
patches.
Ok, I will submit separate patches for these. In doing so, I will have a
stand-alone patch for the drivers/remoteproc/Kconfig fix, as well as the
driver-related addition to that file in the driver patch. PCMIIW.
We should have one patch for the Kconfig fix, another one for the new
firmware-name functionality in rproc_alloc() and then a patch adding
this new driver.
+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);
IIRC we agreed on removing this part, and instead adding it to the
generic remoteproc framework (i.e. Cyril's patch).
I didn't like the idea of having extra overhead when calling the all
virtqueues version.
We know that we want to call rproc_vq_interrupt(rproc, 1) just once, and
don't care about its return value. If we did
/* Process incoming buffers on our vring */
while (IRQ_HANDLED == rproc_vq_interrupt(rproc, -1))
;
then that would essentially turn into:
do {
ret = IRQ_NONE;
if (rproc_vq_interrupt(rproc, 0) == IRQ_HANDLED)
ret = IRQ_HANDLED;
if (rproc_vq_interrupt(rproc, 1) == IRQ_HANDLED)
ret = IRQ_HANDLED;
} while (ret == IRQ_HANDLED);
which will end up calling rproc_vq_interrupt(rproc, 1) too many times.
We have a benchmark goal to keep wrt/ the whole round-trip time of messages
and we're currently not even achieving that, so more overhead than today
can't be tolerated.
Have you checked whether it really affects performance? I'd expect
this to be very negligible. We can probably also maintain a bitmap
with bits set for every IRQ_HANDLED vring, and stop processing vrings
that has this bit cleared.
But it actually looks like there's a different problem here.
rproc_vq_interrupt() may also return IRQ_HANDLED on the unlikely event
that vq-broken is set. With the current approach taken by this patch,
this may lock the system in a busy loop.
I took a brief look at the other virtio drivers, and it seems they
process all pending messages available when kicked, so we should
probably add this to rpmsg_recv_done() too. This way we don't have to
manually do it in the remoteproc layer, and all platforms will benefit
from it.
We should then only have to allow the option of asking the core to
process all available vrings, as we discussed, and we're done.
Thanks,
Ohad.
___
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 v7 1/2] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hi Rob,
On Fri, Feb 1, 2013 at 4:24 AM, Robert Tivy rt...@ti.com wrote:
drivers/remoteproc/Kconfig| 29 ++-
drivers/remoteproc/Makefile |1 +
drivers/remoteproc/da8xx_remoteproc.c | 346
+
drivers/remoteproc/remoteproc_core.c | 22 ++-
It looks like this patch squashes:
1. A fix to drivers/remoteproc/Kconfig
2. New functionality in remoteproc_core.c
3. A new da8xx driver
These are independent changes, so we better split them up to separate patches.
config OMAP_REMOTEPROC
tristate OMAP remoteproc support
- depends on EXPERIMENTAL
...
config STE_MODEM_RPROC
tristate STE-Modem remoteproc support
- depends on EXPERIMENTAL
These look unrelated to this patch, and it seems that Kees Cook
submitted these awhile ago so it should probably already be in
linux-next (haven't looked). I think we can drop these.
+/**
+ * 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);
IIRC we agreed on removing this part, and instead adding it to the
generic remoteproc framework (i.e. Cyril's patch).
+static int da8xx_rproc_start(struct rproc *rproc)
+{
...
+ 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;
Have you considered doing this in -probe() instead? Do we need to
call get/put every time we start/stop the remote processor?
+/* 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_CHIPSIG2)
+ if (time_after(jiffies, timeout)) {
+ if (readl(drproc-chipsig) SYSCFG_CHIPSIG2) {
+ dev_err(rproc-dev.parent,
+ failed to receive ack\n);
+ timed_out = 1;
+ }
+
+ break;
+ }
This still looks a bit out of place here.
Kicking should be a fast unilateral action, that doesn't depend on the
other side.
+static int da8xx_rproc_probe(struct platform_device *pdev)
+{
...
+ 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;
+ }
Shouldn't we be requesting this before we rproc_add() ?
+static int da8xx_rproc_remove(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc-priv;
+ int ret;
...
+ ret = rproc_del(rproc);
You can safely remove 'ret' altogether. We will just crash in
rproc_put if rproc is NULL.
+ rproc_put(rproc);
+
+ return ret;
+}
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index dd3bfaf..e0f703e 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'
+*
Re: [PATCH v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hi Rob, On Sat, Jan 12, 2013 at 4:26 AM, Tivy, Robert rt...@ti.com wrote: Is FW_CONFIG above supposed to be FW_LOADER? That FW_CONFIG is completely bogus of course. care to fix it in your series? We're currently handling the CHIPINT lines as levels, since they're completely controlled by SW. The interruptor raises the line and the interruptee lowers (clears) it. In a situation where every interrupt is considered to be a signal of new data arrival we need to make sure that the other side has seen the previous interrupt before we raise another one. What if we don't ? Can the DSP side just go over the vrings until no new msg is left (similar to what you do on the Linux side) ? Is this suggesting that there be separate platform device instances for each different potential fw, and that each platform device instance hardcodes the fw filename? In principle this same driver can drive many instances of remote processors you may have on your board. E.g. in OMAP the same driver controls both the M3s and the DSP. pdata is then used to hold information specific to the hw instance. I'm not sure if there's (or will be) any DaVinci platform with several remote processors but it's a better practice to write the code as if there is/will be - it's usually cleaner and will just work in case a platform with multiple rprocs does show up one day. Sekhar asked that there not be a default fw name, so there's conflicting feedback on this point. I prefer to have a default name plus the module parameter override (but don't have much opinion on whether it should be davinci-specific (and passed with davinci_remoteproc.ko) or general (and passed with remoteproc.ko), please advise). Since the fw file (i.e., DSP program) is typically paired with a particular Linux app, I like the ability to specify the fw filename at runtime, depending on the Linux app I need to run. Sure, no reason why not to allow users to override the default fw name. Thanks, Ohad. ___ 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 v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
On Tue, Jan 15, 2013 at 11:15 AM, Sekhar Nori nsek...@ti.com wrote: Right, and platform data is not the way to achieve this. How do you suggest to handle platforms with multiple different remote processors (driven by the same driver) ? Requiring the user to indicate the fw name for each processor is somewhat error prone/cumbersome. Thanks, Ohad. ___ 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 v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
On Tue, Jan 15, 2013 at 2:29 PM, Sekhar Nori nsek...@ti.com wrote: May be rproc_alloc() could auto-assign the firmware name to something like 'rproc%d-fw' if firmware name passed to it is NULL? I prefer we use name-based filenames instead to make it easier for users (and us developers). We can probably do something like rproc-%s-fw with pdata-name assuming we/you do maintain a meaningful name in the latter. Thanks, Ohad. ___ 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 v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
On Wed, Jan 16, 2013 at 1:06 AM, Tivy, Robert rt...@ti.com wrote: This sounds good, although it will introduce the need to handle dynamic storage for the generated name. I think I can jam that storage on the end of the already-dynamically-sized 'struct rproc + sizeof(pdata)' allocation in rproc_alloc(). Or you can generate it when needed, without storing the result. This rarely happens and anyway is negligible. Thanks, Ohad. ___ 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 v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hi Robert,
I'm happy to see this driver going upstream, thanks for pushing!
Please see below my few comments. PS - sorry for the belated review.
On Fri, Jan 11, 2013 at 2:23 AM, Robert Tivy rt...@ti.com wrote:
+config DAVINCI_REMOTEPROC
+ tristate DaVinci DA850/OMAPL138 remoteproc support (EXPERIMENTAL)
+ depends on ARCH_DAVINCI_DA850
+ select REMOTEPROC
+ select RPMSG
+ select FW_LOADER
This one gets selected by CONFIG_REMOTEPROC, so you don't need to do so too.
diff --git a/drivers/remoteproc/davinci_remoteproc.c
b/drivers/remoteproc/davinci_remoteproc.c
new file mode 100644
index 000..fc6fd72
--- /dev/null
+++ b/drivers/remoteproc/davinci_remoteproc.c
+static char *fw_name;
+module_param(fw_name, charp, S_IRUGO);
+MODULE_PARM_DESC(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 (1 0)
+#define SYSCFG_CHIPINT1 (1 1)
+#define SYSCFG_CHIPINT2 (1 2)
+#define SYSCFG_CHIPINT3 (1 3)
+
+/**
+ * struct davinci_rproc - davinci remote processor state
+ * @rproc: rproc handle
Add @dsp_clk ?
+ */
+struct davinci_rproc {
+ struct rproc *rproc;
+ struct clk *dsp_clk;
+};
+
+static void __iomem *syscfg0_base;
+static struct platform_device *remoteprocdev;
+static struct irq_data *irq_data;
+static void (*ack_fxn)(struct irq_data *data);
+static int irq;
Is it safe to maintain these as globals (i.e. what if we have more
than a single pdev instance) ?
+
+/**
+ * handle_event() - inbound virtqueue message workqueue function
+ *
+ * This funciton is registered as a kernel thread and is scheduled by the
typo
+ * kernel handler.
+ */
+static irqreturn_t handle_event(int irq, void *p)
+{
+ struct rproc *rproc = platform_get_drvdata(remoteprocdev);
It's probably better to pass this as an irq cookie instead of relying
on global data
+
+ /* Process incoming buffers on our vring */
+ while (IRQ_HANDLED == rproc_vq_interrupt(rproc, 0))
+ ;
This is interesting. IIUC, you want a single irq event to trigger
processing of all the available messages. It makes sense, but I'm not
sure we need this to be platform-specific.
+
+ /* Must allow wakeup of potenitally blocking senders: */
+ rproc_vq_interrupt(rproc, 1);
IIUC, you do this is because you have a single irq for all the vrings (PCMIIW).
We may want to add something in these lines to the generic remoteproc
framework, as additional platforms require it (e.g. keystone). I
remember a similar patch by Cyril was circulating internally but never
hit the mailing lists - you may want to take it even though it would
probably need to be refreshed.
+static int davinci_rproc_start(struct rproc *rproc)
+{
+ struct platform_device *pdev = to_platform_device(rproc-dev.parent);
+ struct device *dev = rproc-dev.parent;
+ struct davinci_rproc *drproc = rproc-priv;
+ struct clk *dsp_clk;
+ struct resource *r;
+ unsigned long host1cfg_physaddr;
+ unsigned int host1cfg_offset;
+ int ret;
+
+ remoteprocdev = pdev;
+
+ /* 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;
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (IS_ERR_OR_NULL(r)) {
+ dev_err(dev, platform_get_resource() error: %ld\n,
+ PTR_ERR(r));
+
+ return PTR_ERR(r);
+ }
+ host1cfg_physaddr = (unsigned long)r-start;
+
+ 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 (IS_ERR_OR_NULL(irq_data)) {
+ dev_err(dev, irq_get_irq_data(%d) error: %ld\n,
+ irq, PTR_ERR(irq_data));
+
+ return PTR_ERR(irq_data);
+ }
+ ack_fxn = irq_data-chip-irq_ack;
+
+ ret = request_threaded_irq(irq, davinci_rproc_callback, handle_event,
+ 0, davinci-remoteproc, drproc);
+ if (ret) {
+ dev_err(dev, request_threaded_irq error: %d\n, ret);
+
+ return ret;
+ }
+
+ syscfg0_base = ioremap(host1cfg_physaddr PAGE_MASK, SZ_4K);
+ host1cfg_offset = offset_in_page(host1cfg_physaddr);
+ writel(rproc-bootaddr, syscfg0_base + host1cfg_offset);
+
+ dsp_clk = clk_get(dev, NULL);
+ if (IS_ERR_OR_NULL(dsp_clk)) {
+ dev_err(dev, clk_get error: %ld\n, PTR_ERR(dsp_clk));
+ ret = PTR_ERR(dsp_clk);
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
On Thu, Sep 15, 2011 at 3:12 AM, Rusty Russell ru...@rustcorp.com.au wrote: 7... numbers are cheap :) 7 it is :) Thanks, Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
Hi Pavel, On Thu, Jun 9, 2011 at 8:12 PM, Pavel Machek pa...@ucw.cz wrote: So this is basically networking... right? Why not implement it as sockets? (accept, connect, read, write)? This patch focuses on adding the core rpmsg kernel infrastructure. The next step, after getting the basic stuff in, would be adding rpmsg drivers, and exposing user space API. For some use cases, where userland talks directly with remote entities (and otherwise requires no kernel involvement besides exposing the transport), socket API is very nice as it's flexible and prevalent. We already have several rpmsg drivers and a rpmsg-based socket API implementation too, but we'll get to it only after the core stuff gets in. Thanks, Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
On Wed, Jun 29, 2011 at 6:43 PM, Grant Likely grant.lik...@secretlab.ca wrote: So you are right that device_unregister drops the refcount to zero, but the code still needs to be fixed to not call kfree() directly. Good point, thanks ! It also looks like rpmsg_destroy_channel() needs to be fixed to remove the kfree call Yes, I need to remove this direct kfree as well, and indeed just let rpmsg_release_device do its thing when the last reference is dropped. I should also remove the direct kfree when device_register fails: in that case, I need only to put_device and let the release handler do its thing too. and an extra put_device() call. We need that extra put_device in rpmsg_destroy_channel because device_find_child() is doing get_device before returning it. Thanks, Grant! Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
Hi Randy, Thanks for your comments ! On Wed, Jun 29, 2011 at 2:44 AM, Randy Dunlap rdun...@xenotime.net wrote: +hardware accelerators, and therefore are often used to offload cpu-intensive prefer: CPU- throughout. Isn't that changing the meaning a bit ? Let's stick with the original version, I think it's more clear. +system's physical memory and/or other sensitive hardware resources (e.g. on +OMAP4, remote cores (/hardware accelerators) may have direct access to the +physical memory, gpio banks, dma controllers, i2c bus, gptimers, mailbox +devices, hwspinlocks, etc..). Moreover, those remote processors might be +running RTOS where every task can access the entire memory/devices exposed +to the processor. To minimize the risks of rogue (/buggy) userland code What is with the leading / here and above (/hardware) and below? / here means or. You can read the sentence twice, either without the (/ ..) options or with them, and then you get two (different) examples. Any idea how to make it more readable ? I prefer not to drop the second example, as it's adding information. +if RPMSG + +# OK, it's a little counter-intuitive to do this, but it puts it neatly under +# the rpmsg menu (and it's the approach preferred by the virtio folks). + +source drivers/virtio/Kconfig It seems odd to have that Kconfig file sourced in multiple places. Are the kconfig tools happy with that? They are, probably because these places are mutually exclusive today: $ git grep drivers/virtio/Kconfig arch/ia64/kvm/Kconfig:source drivers/virtio/Kconfig arch/mips/Kconfig:source drivers/virtio/Kconfig arch/powerpc/kvm/Kconfig:source drivers/virtio/Kconfig arch/s390/kvm/Kconfig:source drivers/virtio/Kconfig arch/sh/Kconfig:source drivers/virtio/Kconfig arch/tile/kvm/Kconfig:source drivers/virtio/Kconfig arch/x86/kvm/Kconfig:source drivers/virtio/Kconfig Now that we start using virtio for inter-processor communications, too, we might soon bump into a situation where virtio will be sourced twice. Probably the solution is to move 'source drivers/virtio/Kconfig' into drivers/Kconfig, and remove all other instances. Rusty, are you ok with that ? Thanks, Ohad. Sorry about the delay. I had most of this in my drafts folder and forgot about it... Np, thanks a lot ! Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
On Wed, Jun 29, 2011 at 2:59 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 29 June 2011, Ohad Ben-Cohen wrote: On Wed, Jun 29, 2011 at 2:44 AM, Randy Dunlap rdun...@xenotime.net wrote: +hardware accelerators, and therefore are often used to offload cpu-intensive prefer: CPU- throughout. Isn't that changing the meaning a bit ? Let's stick with the original version, I think it's more clear. I think you misunderstood Randy, he meant you should do 's/cpu/CPU/' globally, Oh, sorry, Randy. For some reason I thought you meant s/cpu-intensive/CPU-throughout/ which didn't make a lot of sense to me :) s/cpu/CPU/ is of course nicer. thanks ! The easiest way would be to replace it with 'or', as in ... remote cores (or hardware accelerators) may have ... yeah, i'll do it, thanks. It's a bit harder to get rid of the parentheses in the second sentence, but I'll think of something too. Probably the solution is to move 'source drivers/virtio/Kconfig' into drivers/Kconfig, and remove all other instances. I think changing that would be good. However, you need to at least restructure the contents, or they will show up in the main driver menu. I'll do that. Thanks, Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 2/8] remoteproc: add omap implementation
On Tue, Jun 28, 2011 at 12:00 AM, Grant Likely grant.lik...@secretlab.ca wrote: Very little for me to comment on here. However, something I just noticed. Why is it necessary to pass in THIS_MODULE to the rproc_register function? Having a reference to the pdev gives you the pointer to the driver, which has the THIS_MODULE value in it. That should be sufficient. Nice one, thanks ! /me also isn't sure if incrementing the refcount on the module is the best way to prevent the rproc from going away, but I haven't dug into the details in the driver code to find out. Drivers can get unbound from devices without the driver being unloaded, so I imagine there is an in-use count on the device itself that would prevent driver unbinding. Yes, increasing the module refcount is necessary to prevent the user from removing the driver when the rproc is used. If the underlying device goes away while rproc is used, then rproc_unregister should return -EBUSY, which would fail the underlying driver's -remove() handler (gpiolib is doing something very similar). I have forgotten to add this check, and will add it now. Thanks ! ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 0/8] Introducing a generic AMP/IPC framework
On Tue, Jun 28, 2011 at 12:22 AM, Grosen, Mark mgro...@ti.com wrote: 2. We added a special section (resource table) that is interpreted as part of the loading process. The tool that generates our simple format just recognizes a named section (.resource_table), so perhaps that could be done with the PIL ELF loader. I hope that DT will completely replace that resource table section eventually. We still need to try it out (as soon as DT materializes on ARM/pandaboard) and see how it all fits together, but in general it looks like DT could provide all the necessary static resource configuration, and then we just need to expose it to the remote processor. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 1/8] drivers: add generic remoteproc framework
Hi Grant, Thanks a lot for the exhaustive review and comments ! On Mon, Jun 27, 2011 at 11:49 PM, Grant Likely grant.lik...@secretlab.ca wrote: + my_rproc = rproc_get(ipu); I tend to be suspicious of apis whose primary interface is by-name lookup. It works fine when the system is small, but it can get unwieldy when the client driver doesn't have a direct relation to the setup code that chooses the name. At some point I suspect that there will need to be different lookup mechanism, such as which AMP processor is currently available (if there are multiple of the same type). Yeah, this might be too limiting on some systems. I gave this a little thought, but decided to wait until those systems show up first, so I we can better understand them/their requirements/use-cases. For now, I've just followed this simple name-based API model (which still seem a bit popular in several SoC drivers I've looked at, probably due to the general simplicity of it and its use cases). It also leaves no option for drivers to obtain a reference to the rproc instance, and bring it up/down as needed (without the name lookup every time). .. That said, it looks like only the rproc_get() api is using by-name lookup, and everything else is via the structure. Can (should) the by-name lookup part be factored out into a rproc_get_by_name() accessor? I think you are looking for a different set of API here, probably something that is better implemented using runtime PM. When a driver calls rproc_get(), not only does it power on the remote processor, but it also makes sure the underlying implementation cannot go away (i.e. platform-specific remoteproc module cannot be removed, and the rproc cannot be unregistered). After it calls rproc_put(), it cannot rely anymore on the remote processor to stick around (the rproc can be unregistered at this point), so the next time it needs it, it must go through the full get-by-name (or any other get API we will come up with eventually) getter API. If drivers need to hold onto the rproc instance, but still explicitly allow it to power off at times, they should probably call something like pm_runtime_put(rproc-dev). (remoteproc runtime PM support is not implemented yet, but is definitely planned, so we can suspend remote processors on inactivity). Since rproc_register is allocating a struct rproc instance that represent the device, shouldn't the pointer to that device be returned to the caller? Yes, it definitely should. We will have the underlying implementation remember it, and then pass it to rproc_unregister when needed. + int rproc_unregister(const char *name); I definitely would not do this by name. I think it is better to pass the actual instance pointer to rproc_unregister. Much better, yeah. Naive question: Why is bootaddr an argument? Wouldn't rproc drivers keep track of the boot address in their driver private data? Mark already got that one, but basically the boot address comes from the firmware image: we need to let the implementation know the physical address where the text section is mapped. This is ignored on implementations where that address is fixed (e.g. OMAP's M3). Other have commented on the image format, so I'll skip this bit other than saying that I agree it would be great to have a common format. We are evaluating now moving to ELF; let's see how it goes. Using a standard format is an advantage (as long as it's not overly complicated), but I wonder if achieving a common format is really feasible and whether eventually different platforms will need different binary formats anyway, and we'll have to abstract this out of remoteproc (guess that as usual, we just need to start off with something, and then evolve as requirements show up). +Most likely this kind of static allocations of hardware resources for +remote processors can also use DT, so it's interesting to see how +this all work out when DT materializes. I imagine that it will be quite straight forward. There will probably be a node in the tree to represent each slave AMP processor, and other devices attached to it could be represented using 'phandle' links between the nodes. Any configuration of the AMP process can be handled with arbitrary device-specific properties in the AMP processor's node. That sounds good. The dilemma is bigger, though. The kind of stuff we need to synchronize about are not really describing the hardware; it's more a runtime policy/configuration than a hardware description. As Brian mentioned in the other thread: The resource information is a description of what resources the firmware requires to work properly (it needs certain amounts of working memory, timers, peripheral interfaces like i2c to control camera hw, etc), which will be specific to a given firmware build. Some of those resources will be allocated dynamically using an rpmsg driver (developed by Fernando Guzman Lugo), but some must be supplied before booting the
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
Hi Grant,
On Tue, Jun 28, 2011 at 1:21 AM, Grant Likely grant.lik...@secretlab.ca wrote:
Nice patch. I'm quite thrilled to see this implemented. Some
comments below, but otherwise I think it looks pretty good.
Thanks !
+source drivers/virtio/Kconfig
What happens when kvm and rpmsg both get enabled on the same kernel.
Sounds to me that eventually we'll have to source virtio's Kconfig in
drivers/Kconfig, and remove all the other locations it is sourced at
(currently it is directly sourced by several arch Kconfigs when
VIRTUALIZATION is selected).
+ if (strncmp(id-name, rpdev-id.name, RPMSG_NAME_SIZE))
+ return 0;
+
+ return 1;
+}
or simply: 'return strncmp(id-name, rpdev-id.name, RPMSG_NAME_SIZE) == 0;'
:-)
that works too ;)
+ spin_lock(vrp-endpoints_lock);
Is a spin_lock the right choice for endpoints, or should it be a
mutex (do the endpoints operations need to be atomic)?
Today it can be a mutex: it protects idr operations invoked either by
users (creating new endpoints, definitely not requiring atomicity) or
by platform code indicating that a new message has arrived (today it's
omap's mailbox driver, which calls from a process context).
I can change that, and if someone requires atomic operations later on,
move to spinlocks again.
But it probably won't matter too much as there's no contention on that
lock today (notifications coming from omap's mailbox are serialized,
and users creating new endpoints show up very infrequently).
At put_device time, it is conceivable that the dev pointer is no
longer valid. You'll need to get do the to_rpmsg_channel() before
putting the dev.
sure.
+static void rpmsg_upref_sleepers(struct virtproc_info *vrp)
+{
+ /* support multiple concurrent senders */
+ spin_lock(vrp-tx_lock);
+
+ /* are we the first sleeping context waiting for tx buffers ? */
+ if (!vrp-sleepers++)
Maybe use a kref?
I can change it to an atomic variable, but I don't think kref's memory
barriers are needed here: we already have memory barriers induced by
the spinlock.
+static int rpmsg_remove_device(struct device *dev, void *data)
+{
+ struct rpmsg_channel *rpdev = to_rpmsg_channel(dev);
+
+ device_unregister(dev);
+
+ kfree(rpdev);
put_device() I think.
Don't think so, we get the device handle from device_for_each_child
here, which doesn't call get_device (unlike device_find_child).
+static int __init init(void)
Even for static functions, it's a really good idea to prefix all
function names and file scoped symbol with a common prefix like
rpmsg_. Doing so avoids even the outside chance of a namespace
conflict.
Sure thing.
+ ret = bus_register(rpmsg_bus);
+ if (ret) {
+ pr_err(failed to register rpmsg bus: %d\n, ret);
+ return ret;
+ }
+
+ return register_virtio_driver(virtio_ipc_driver);
And if register_virtio_driver fails?
I'll handle that, thanks.
+struct rpmsg_device_id {
+ char name[RPMSG_NAME_SIZE];
+ kernel_ulong_t driver_data /* Data private to the driver */
+ __attribute__((aligned(sizeof(kernel_ulong_t;
+};
Should this be co-located with vio_device_id?
Sure, can't hurt (I assume you meant virtio_device_id here).
It makes it easier to dereference in kernel code if you do:
#ifdef __KERNEL__
void *data;
#else
kernel_ulong_t data;
#endif
Sure thing.
Thanks!
Ohad.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
On Wed, Jun 29, 2011 at 1:51 AM, Grant Likely grant.lik...@secretlab.ca wrote: It's not the device_for_each_child() that you're 'putting' back from here. Its the original kref initialization when the device was created. device_unregister() is already calling put_device(), doesn't that deal with the original kref init for us ? ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 0/8] Introducing a generic AMP/IPC framework
Hi Stephen, On Fri, Jun 24, 2011 at 11:16 PM, Stephen Boyd sb...@codeaurora.org wrote: This sounds a lot like SMD (shared memory driver) on MSM. The main difference I see is that SMD uses the platform bus instead of the virtio bus and it has its own protocol for channel allocation. Yeah, virtio is a key factor in this work; it was suggested to us by Arnd at the AMP plumbers discussions last year, where it was apparent that many vendors have their own IPC drivers/buses/channels over shared memory with some vendor-ish binary protocol. I must say we really liked virtio: it considerably simplified the code (we're not adding any new binary protocol), it's very nicely optimized and flexible, and it comes with a set of virtio drivers (e.g. network, console, block) so we don't have to write our own. We also cared about adding this functionality as an IPC bus, so the driver core will help matching drivers to channels - it simplified the code (in both setup and tear down of channels) and kept it flexible. It will also facilitate error recovery (on remote crash, we just remove the virtio device, and then the driver core will in turn start -remove()ing the rpmsg drivers) and power management (via runtime PM). About SMD: I'm not familiar with it too much, but Brian naturally is (just for the sake of everyone who are not reading headers - Brian Swetland wrote the Linux SMD driver, and is also an author of this Google+TI joint work). Btw, I'm sure SMD is conceptually not MSM-specific, and have wondered whether you guys would like to use rpmsg/virtio (I know you have several drivers like network/tty/etc over SMD, somewhat similarly to virtio). Probably the biggest reason why not to is the pain in changing the binary protocol with the modem/dsp side. If you ever do think about it, I'd be happy to work with you to make it happen. This remote proc code is eerily similar to PIL (peripheral image loader, yes we love our acronyms) which I posted a few months back[1]. Was it inspiration for this patch series? No, we weren't (or at least I wasn't) aware of PIL. In terms of API, s/pil/rproc/ and it would be 95% identical. There are some low-level differences though (see below). Indeed, eerily similar :O I just guess the API is so simple that probably most kernel hackers would use refcounting get/put semantics here. This is an important difference between remote proc and PIL. In PIL, we do image authentication in addition to processor boot. Yes, we have this too (secure code will need to authenticate the image in some use cases) - but that's not ready yet for submission and we decided to start off with the basics first and then evolve. Instead of devising a new firmware format, we decided to just stick with elf and parse the headers in the kernel because we needed them for authentication anyway. Is this reason enough to move to an ELF format instead? I think that consolidation of code is enough reason to make an effort. I know that our firmware format was chosen for simplicity, but I'm not sure if we have the tools yet to build standard ELF files for the remote processors (IIRC it's in the works though). I'll let Mark comment this one. Another difference is inter-processor dependencies. For example, on msm8660 the modem can't boot until the dsp has been booted. I suppose we could hide this detail in the platform specific get() implementation by calling rproc_get() on the dependent processor (hopefully no locking issues arise). I'd rather have it built into the core though as it isn't really specific to the hardware. No problems, I'm sure we can solve this one easily. If we can resolve these differences I think we can easily support remote processor boot on MSM via remoteproc. That'd be very cool, I sure do hope we can work together. Thanks for your comments ! Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC 1/8] drivers: add generic remoteproc framework
Some systems have slave heterogeneous remote processor devices,
that are usually used to offload cpu-intensive computations
(e.g. multimedia codec tasks).
Booting a remote processor typically involves:
- Loading a firmware which contains the OS image (mainly text and data)
- If needed, programming an IOMMU
- Powering on the device
This patch introduces a generic remoteproc framework that allows drivers
to start and stop those remote processor devices, load up their firmware
(which might not necessarily be Linux-based), and in the future also
support power management and error recovery.
It's still not clear how much this is really reusable for other
platforms/architectures, especially the part that deals with the
firmware.
Moreover, it's not entirely clear whether this should really be an
independent layer, or if it should just be squashed with the host-specific
component of the rpmsg framework (there isn't really a remoteproc use case
that doesn't require rpmsg).
That said, it did prove useful for us on two completely different
platforms: OMAP and Davinci, each with its different remote
processor (Cortex-M3 and a C674x DSP, respectively). So to avoid
egregious duplication of code, remoteproc must not be omap-only.
Firmware loader is based on code by Mark Grosen mgro...@ti.com.
TODO:
- drop rproc_da_to_pa(), use iommu_iova_to_phys() instead
(requires completion of omap's iommu migration and some generic iommu
API work)
- instead of ioremapping reserved memory and handling IOMMUs, consider
moving to the generic DMA mapping API (with a CMA backend)
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
Documentation/remoteproc.txt| 170 +
drivers/Kconfig |2 +
drivers/Makefile|1 +
drivers/remoteproc/Kconfig |7 +
drivers/remoteproc/Makefile |5 +
drivers/remoteproc/remoteproc.c | 780 +++
include/linux/remoteproc.h | 273 ++
7 files changed, 1238 insertions(+), 0 deletions(-)
create mode 100644 Documentation/remoteproc.txt
create mode 100644 drivers/remoteproc/Kconfig
create mode 100644 drivers/remoteproc/Makefile
create mode 100644 drivers/remoteproc/remoteproc.c
create mode 100644 include/linux/remoteproc.h
diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt
new file mode 100644
index 000..3075813
--- /dev/null
+++ b/Documentation/remoteproc.txt
@@ -0,0 +1,170 @@
+Remote Processor Framework
+
+1. Introduction
+
+Modern SoCs typically have heterogeneous remote processor devices in asymmetric
+multiprocessing (AMP) configurations, which may be running different instances
+of operating system, whether it's Linux or any other flavor of real-time OS.
+
+OMAP4, for example, has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP.
+In a typical configuration, the dual cortex-A9 is running Linux in a SMP
+configuration, and each of the other three cores (two M3 cores and a DSP)
+is running its own instance of RTOS in an AMP configuration.
+
+The generic remoteproc driver allows different platforms/architectures to
+control (power on, load firmware, power off) those remote processors while
+abstracting the hardware differences, so the entire driver doesn't need to be
+duplicated.
+
+2. User API
+
+ struct rproc *rproc_get(const char *name);
+ - power up the remote processor, identified by the 'name' argument,
+ and boot it. If the remote processor is already powered on, the
+ function immediately succeeds.
+ On success, returns the rproc handle. On failure, NULL is returned.
+
+ void rproc_put(struct rproc *rproc);
+ - power off the remote processor, identified by the rproc handle.
+ Every call to rproc_get() must be (eventually) accompanied by a call
+ to rproc_put(). Calling rproc_put() redundantly is a bug.
+ Note: the remote processor will actually be powered off only when the
+ last user calls rproc_put().
+
+3. Typical usage
+
+#include linux/remoteproc.h
+
+int dummy_rproc_example(void)
+{
+ struct rproc *my_rproc;
+
+ /* let's power on and boot the image processing unit */
+ my_rproc = rproc_get(ipu);
+ if (!my_rproc) {
+ /*
+* something went wrong. handle it and leave.
+*/
+ }
+
+ /*
+* the 'ipu' remote processor is now powered on... let it work !
+*/
+
+ /* if we no longer need ipu's services, power it down */
+ rproc_put(my_rproc);
+}
+
+4. API for implementors
+
+ int rproc_register(struct device *dev, const char *name,
+ const struct rproc_ops *ops,
+ const char *firmware,
+ const struct rproc_mem_entry *memory_maps,
+ struct module *owner);
+ - should be called from the underlying platform-specific implementation, in
+ order to register a new remoteproc device. 'dev
[RFC 3/8] omap: add carveout memory support for remoteproc
This is a temporary patch to get things going, and is by no means
a suggestion for inclusion (read only if interested, but don't waste
much review energies here).
The way to go forward here is to use CMA (together with the generic
DMA API, so we also get IOMMU programming for free).
This patch also breaks tidspbridge. Use it only if you want to try out
rpmsg/remoteproc on OMAP4, and you don't care too much about multi-board
kernels.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/Kconfig|8
arch/arm/plat-omap/devices.c | 14 --
arch/arm/plat-omap/include/plat/dsp.h |6 +-
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 1c3acb5..09c91d1 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -211,6 +211,14 @@ config OMAP_SERIAL_WAKE
to data on the serial RX line. This allows you to wake the
system from serial console.
+config OMAP_CARVEOUT_MEMPOOL_SIZE
+ hex Physical carveout memory pool size (Byte)
+ depends on OMAP_REMOTE_PROC
+ default 0x330
+ help
+ Allocate specified size of memory at boot time so we can ioremap
+ it safely.
+
choice
prompt OMAP PM layer selection
depends on ARCH_OMAP
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index ea28f98..6922f3b 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -234,13 +234,16 @@ static void omap_init_uwire(void)
static inline void omap_init_uwire(void) {}
#endif
-#if defined(CONFIG_TIDSPBRIDGE) || defined(CONFIG_TIDSPBRIDGE_MODULE)
+#if defined(CONFIG_TIDSPBRIDGE) || defined(CONFIG_TIDSPBRIDGE_MODULE) || \
+ defined(CONFIG_OMAP_REMOTE_PROC)
static phys_addr_t omap_dsp_phys_mempool_base;
+static phys_addr_t omap_dsp_phys_mempool_size;
void __init omap_dsp_reserve_sdram_memblock(void)
{
- phys_addr_t size = CONFIG_TIDSPBRIDGE_MEMPOOL_SIZE;
+ /* ignoring TIDSPBRIDGE for a moment here... */
+ phys_addr_t size = CONFIG_OMAP_CARVEOUT_MEMPOOL_SIZE;
phys_addr_t paddr;
if (!size)
@@ -256,6 +259,7 @@ void __init omap_dsp_reserve_sdram_memblock(void)
memblock_remove(paddr, size);
omap_dsp_phys_mempool_base = paddr;
+ omap_dsp_phys_mempool_size = size;
}
phys_addr_t omap_dsp_get_mempool_base(void)
@@ -263,6 +267,12 @@ phys_addr_t omap_dsp_get_mempool_base(void)
return omap_dsp_phys_mempool_base;
}
EXPORT_SYMBOL(omap_dsp_get_mempool_base);
+
+phys_addr_t omap_dsp_get_mempool_size(void)
+{
+ return omap_dsp_phys_mempool_size;
+}
+EXPORT_SYMBOL(omap_dsp_get_mempool_size);
#endif
/*
diff --git a/arch/arm/plat-omap/include/plat/dsp.h
b/arch/arm/plat-omap/include/plat/dsp.h
index 9c604b3..31ee386 100644
--- a/arch/arm/plat-omap/include/plat/dsp.h
+++ b/arch/arm/plat-omap/include/plat/dsp.h
@@ -22,10 +22,14 @@ struct omap_dsp_platform_data {
phys_addr_t phys_mempool_size;
};
-#if defined(CONFIG_TIDSPBRIDGE) || defined(CONFIG_TIDSPBRIDGE_MODULE)
+#if defined(CONFIG_TIDSPBRIDGE) || defined(CONFIG_TIDSPBRIDGE_MODULE) || \
+ defined(CONFIG_OMAP_REMOTE_PROC)
extern void omap_dsp_reserve_sdram_memblock(void);
#else
static inline void omap_dsp_reserve_sdram_memblock(void) { }
#endif
+phys_addr_t omap_dsp_get_mempool_size(void);
+phys_addr_t omap_dsp_get_mempool_base(void);
+
#endif
--
1.7.1
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC 6/8] davinci: da850: add remoteproc dsp device
From: Mark Grosen mgro...@ti.com
Add davinci remoteproc device for the da850's C674x dsp remote
processor, and support it on the da850-evm and omapl138-hawk boards.
Signed-off-by: Mark Grosen mgro...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
It'd be nice to split this patch to the different components it changes,
but it's probably not so important at this early stage.
arch/arm/mach-davinci/board-da850-evm.c |4
arch/arm/mach-davinci/board-omapl138-hawk.c |4
arch/arm/mach-davinci/da850.c | 14 ++
arch/arm/mach-davinci/devices-da8xx.c | 15 +++
arch/arm/mach-davinci/include/mach/da8xx.h |1 +
5 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index a7b41bf..6b35f0a 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1176,6 +1176,10 @@ static __init void da850_evm_init(void)
i2c_register_board_info(1, da850_evm_i2c_devices,
ARRAY_SIZE(da850_evm_i2c_devices));
+ ret = da850_register_rproc();
+ if (ret)
+ pr_warning(dsp/rproc registration failed: %d\n, ret);
+
/*
* shut down uart 0 and 1; they are not used on the board and
* accessing them causes endless too much work in irq53 messages
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 67c38d0..52c8434 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -319,6 +319,10 @@ static __init void omapl138_hawk_init(void)
pr_warning(omapl138_hawk_init:
watchdog registration failed: %d\n,
ret);
+
+ ret = da850_register_rproc();
+ if (ret)
+ pr_warning(dsp/rproc registration failed: %d\n, ret);
}
#ifdef CONFIG_SERIAL_8250_CONSOLE
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 133aac4..9280b1e 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -74,6 +74,13 @@ static struct clk pll0_aux_clk = {
.flags = CLK_PLL | PRE_PLL,
};
+static struct clk pll0_sysclk1 = {
+ .name = pll0_sysclk1,
+ .parent = pll0_clk,
+ .flags = CLK_PLL,
+ .div_reg= PLLDIV1,
+};
+
static struct clk pll0_sysclk2 = {
.name = pll0_sysclk2,
.parent = pll0_clk,
@@ -373,6 +380,12 @@ static struct clk spi1_clk = {
.flags = DA850_CLK_ASYNC3,
};
+static struct clk dsp_clk = {
+ .name = dsp,
+ .parent = pll0_sysclk1,
+ .lpsc = DA8XX_LPSC0_GEM,
+};
+
static struct clk_lookup da850_clks[] = {
CLK(NULL, ref, ref_clk),
CLK(NULL, pll0, pll0_clk),
@@ -419,6 +432,7 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, usb20,usb20_clk),
CLK(spi_davinci.0,NULL, spi0_clk),
CLK(spi_davinci.1,NULL, spi1_clk),
+ CLK(NULL, dsp, dsp_clk),
CLK(NULL, NULL, NULL),
};
diff --git a/arch/arm/mach-davinci/devices-da8xx.c
b/arch/arm/mach-davinci/devices-da8xx.c
index fc4e98e..3becfd1 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -20,6 +20,7 @@
#include mach/time.h
#include mach/da8xx.h
#include mach/cpuidle.h
+#include mach/remoteproc.h
#include clock.h
@@ -647,6 +648,20 @@ int __init da850_register_mmcsd1(struct davinci_mmc_config
*config)
da850_mmcsd1_device.dev.platform_data = config;
return platform_device_register(da850_mmcsd1_device);
}
+
+int __init da850_register_rproc(void)
+{
+ struct platform_device *rproc;
+ struct davinci_rproc_pdata rproc_pdata = {
+ .name = dsp,
+ .firmware = davinci-dsp.bin,
+ .clk_name = dsp,
+ };
+
+ rproc = platform_device_register_data(NULL, davinci-rproc, 0,
+ rproc_pdata, sizeof(rproc_pdata));
+ return PTR_RET(rproc);
+};
#endif
static struct resource da8xx_rtc_resources[] = {
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h
b/arch/arm/mach-davinci/include/mach/da8xx.h
index ad64da7..a6f024f 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -74,6 +74,7 @@ void __init da850_init(void);
int da830_register_edma(struct edma_rsv_info *rsv);
int da850_register_edma(struct edma_rsv_info *rsv[2]);
+int da850_register_rproc(void);
int da8xx_register_i2c(int instance, struct davinci_i2c_platform_data *pdata);
int da8xx_register_spi(int instance, struct
[RFC 4/8] omap: add remoteproc devices
From: Guzman Lugo, Fernando fernando.l...@ti.com
Add omap remoteproc devices for the ipu and dsp remote processors.
TODO:
- rework to use CMA instead of reserving memory at boot
[o...@wizery.com: commit log, refactored and simplified, still wip]
Signed-off-by: Guzman Lugo, Fernando fernando.l...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/mach-omap2/Makefile |2 +
arch/arm/mach-omap2/remoteproc.c | 159 ++
2 files changed, 161 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/remoteproc.c
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index b148077..2b04fe6 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -270,3 +270,5 @@ disp-$(CONFIG_OMAP2_DSS):= display.o
obj-y += $(disp-m) $(disp-y)
obj-y += common-board-devices.o
+
+obj-$(CONFIG_OMAP_REMOTE_PROC) += remoteproc.o
diff --git a/arch/arm/mach-omap2/remoteproc.c b/arch/arm/mach-omap2/remoteproc.c
new file mode 100644
index 000..4f846cb
--- /dev/null
+++ b/arch/arm/mach-omap2/remoteproc.c
@@ -0,0 +1,159 @@
+/*
+ * Remote processor machine-specific module for OMAP4
+ *
+ * Copyright (C) 2011 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.
+ */
+
+#define pr_fmt(fmt)%s: fmt, __func__
+
+#include linux/kernel.h
+#include linux/err.h
+#include linux/remoteproc.h
+#include linux/memblock.h
+
+#include plat/omap_device.h
+#include plat/omap_hwmod.h
+#include plat/remoteproc.h
+#include plat/dsp.h
+#include plat/io.h
+
+#define L4_PERIPHERAL_L4CFG(L4_44XX_BASE)
+#define IPU_PERIPHERAL_L4CFG 0xAA00
+
+#define IPU_MEM_TEXT 0x0
+#define IPU_MEM_DATA 0x8000
+#define IPU_MEM_IPC0xA000
+
+/*
+ * Memory mappings for the remote M3 subsystem
+ *
+ * Don't change the device addresses (first parameter), otherwise you'd have
+ * to update the firmware (BIOS image) accordingly.
+ *
+ * A 0 physical address (second parameter) means this physical region should
+ * be dynamically carved out at boot time.
+ *
+ * Sizes should be in the form of 2 ^ n * PAGE_SIZE (where n = 0, 1, 2, ...)
+ *
+ * Alignment requirement: at least page-based
+ */
+static struct rproc_mem_entry ipu_memory_maps[] = {
+ {IPU_MEM_IPC, 0, SZ_1M}, /* keep this IPC region first */
+ {IPU_MEM_TEXT, 0, SZ_4M},
+ {IPU_MEM_DATA, 0, SZ_32M},
+ {IPU_PERIPHERAL_L4CFG, L4_PERIPHERAL_L4CFG, SZ_16M},
+ { }
+};
+
+static struct omap_rproc_pdata omap4_rproc_data[] = {
+ {
+ .name = dsp,
+ .iommu_name = tesla,
+ .firmware = tesla-dsp.bin,
+ .oh_name= dsp_c0,
+ },
+ {
+ .name = ipu,
+ .iommu_name = ducati,
+ .firmware = ducati-m3.bin,
+ .oh_name= ipu_c0,
+ .oh_name_opt= ipu_c1,
+ .memory_maps= ipu_memory_maps,
+ },
+};
+
+static struct omap_device_pm_latency omap_rproc_latency[] = {
+ {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ },
+};
+
+static int __init omap_rproc_init(void)
+{
+ const char *pdev_name = omap-rproc;
+ struct omap_hwmod *oh[2];
+ struct omap_device *od;
+ int i, ret = 0, oh_count;
+ phys_addr_t paddr, size;
+
+ /* names like ipu_cx/dsp_cx might show up on other OMAPs, too */
+ if (!cpu_is_omap44xx())
+ return 0;
+
+ paddr = omap_dsp_get_mempool_base();
+ size = omap_dsp_get_mempool_size();
+ if (!paddr || !size) {
+ pr_warn(carveout memory is unavailable: 0x%x, 0x%x\n,
+ paddr, size);
+ return -ENOMEM;
+ }
+
+ /* dynamically allocate carveout memory as required by the ipu */
+ for (i = 0; i ARRAY_SIZE(ipu_memory_maps); i++) {
+ struct rproc_mem_entry *me = ipu_memory_maps[i];
+
+ if (!me-pa me-size) {
+ if (me-size size) {
+ pr_warn(out of carveout memory\n);
+ return -ENOMEM;
+ }
+
+ me-pa = paddr;
+ paddr += me-size
[RFC 5/8] remoteproc: add davinci implementation
From: Mark Grosen mgro...@ti.com
Add remoteproc implementation for da850, so we can boot its DSP.
Signed-off-by: Mark Grosen mgro...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/mach-davinci/include/mach/remoteproc.h | 28 +
drivers/remoteproc/Kconfig | 16 +++
drivers/remoteproc/Makefile |1 +
drivers/remoteproc/davinci_remoteproc.c | 147 +++
4 files changed, 192 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-davinci/include/mach/remoteproc.h
create mode 100644 drivers/remoteproc/davinci_remoteproc.c
diff --git a/arch/arm/mach-davinci/include/mach/remoteproc.h
b/arch/arm/mach-davinci/include/mach/remoteproc.h
new file mode 100644
index 000..af6e88c
--- /dev/null
+++ b/arch/arm/mach-davinci/include/mach/remoteproc.h
@@ -0,0 +1,28 @@
+/*
+ * Remote Processor
+ *
+ * Copyright (C) 2011 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 _DAVINCI_REMOTEPROC_H
+#define _DAVINCI_REMOTEPROC_H
+
+#include linux/remoteproc.h
+
+struct davinci_rproc_pdata {
+ struct rproc_ops *ops;
+ char *name;
+ char *clk_name;
+ char *firmware;
+};
+
+#endif /* _DAVINCI_REMOTEPROC_H */
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 88421bd..1e594b5 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -26,3 +26,19 @@ config OMAP_REMOTE_PROC
It's safe to say n here if you're not interested in multimedia
offloading or just want a bare minimum kernel.
+
+config DAVINCI_REMOTE_PROC
+ tristate Davinci remoteproc support
+ depends on ARCH_DAVINCI_DA850
+ select REMOTE_PROC
+ default y
+ help
+ Say y here to support Davinci's DSP remote processor via the remote
+ processor framework (currently only da850 is supported).
+
+ Usually you want to say y here, in order to enable multimedia
+ use-cases to run on your platform (multimedia codecs are
+ offloaded to remote DSP processors using this framework).
+
+ It's safe to say n here if you're not interested in multimedia
+ offloading or just want a bare minimum kernel.
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 0198b1d..e83dac9 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_REMOTE_PROC) += remoteproc.o
obj-$(CONFIG_OMAP_REMOTE_PROC) += omap_remoteproc.o
+obj-$(CONFIG_DAVINCI_REMOTE_PROC) += davinci_remoteproc.o
diff --git a/drivers/remoteproc/davinci_remoteproc.c
b/drivers/remoteproc/davinci_remoteproc.c
new file mode 100644
index 000..0e26fe9
--- /dev/null
+++ b/drivers/remoteproc/davinci_remoteproc.c
@@ -0,0 +1,147 @@
+/*
+ * Remote processor machine-specific module for Davinci
+ *
+ * Copyright (C) 2011 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.
+ */
+
+#define pr_fmt(fmt)%s: fmt, __func__
+
+#include linux/kernel.h
+#include linux/err.h
+#include linux/printk.h
+#include linux/bitops.h
+#include linux/platform_device.h
+#include linux/remoteproc.h
+#include linux/clk.h
+#include linux/io.h
+
+#include mach/da8xx.h
+#include mach/cputype.h
+#include mach/psc.h
+#include mach/remoteproc.h
+
+/*
+ * Technical Reference:
+ * OMAP-L138 Applications Processor System Reference Guide
+ * http://www.ti.com/litv/pdf/sprugm7d
+ */
+
+/* local reset bit (0 is asserted) in MDCTL15 register (section 9.6.18) */
+#define LRST BIT(8)
+
+/* next state bits in MDCTL15 register (section 9.6.18) */
+#define NEXT_ENABLED 0x3
+
+/* register for DSP boot address in SYSCFG0 module (section 11.5.6) */
+#define HOST1CFG 0x44
+
+static inline int davinci_rproc_start(struct rproc *rproc, u64 bootaddr)
+{
+ struct device *dev = rproc-dev;
+ struct davinci_rproc_pdata *pdata = dev-platform_data;
+ struct davinci_soc_info *soc_info = davinci_soc_info;
+ void __iomem *psc_base;
+ struct clk *dsp_clk;
+
+ /* hw requires the start (boot) address be on 1KB boundary
[RFC 2/8] remoteproc: add omap implementation
From: Guzman Lugo, Fernando fernando.l...@ti.com
Add remoteproc implementation for OMAP4, so we can load the M3 and DSP
remote processors.
Based on code by Hari Kanigeri h-kanige...@ti.com
While this code is functional, and works on OMAP4 its M3's,
it is still preliminary and going to substantially change:
1. Splitting physically contiguous regions to pages should happen
inside the IOMMU API framework, and not here (so omap_rproc_map_unmap
should go away).
2. IOMMU programming in general should go away too; it should be handled by
generic code (preferably using the DMA mapping API), and not by an OMAP
specific component.
This patch depends on OMAP's IOMMU API migration, as posted here:
https://lkml.org/lkml/2011/6/2/369
[o...@wizery.com: commit log, generic iommu api migration, refactoring,
cleanups]
Signed-off-by: Guzman Lugo, Fernando fernando.l...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/include/plat/remoteproc.h | 41 +
drivers/remoteproc/Kconfig | 21 +++
drivers/remoteproc/Makefile |1 +
drivers/remoteproc/omap_remoteproc.c | 243 ++
4 files changed, 306 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/plat-omap/include/plat/remoteproc.h
create mode 100644 drivers/remoteproc/omap_remoteproc.c
diff --git a/arch/arm/plat-omap/include/plat/remoteproc.h
b/arch/arm/plat-omap/include/plat/remoteproc.h
new file mode 100644
index 000..6494570
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/remoteproc.h
@@ -0,0 +1,41 @@
+/*
+ * Remote Processor - omap-specific bits
+ *
+ * Copyright (C) 2011 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 _PLAT_REMOTEPROC_H
+#define _PLAT_REMOTEPROC_H
+
+#include linux/remoteproc.h
+
+/*
+ * struct omap_rproc_pdata - omap remoteproc's platform data
+ * @name: the remoteproc's name, cannot exceed RPROC_MAX_NAME bytes
+ * @iommu_name: iommu device we're behind of
+ * @oh_name: omap hwmod device
+ * @oh_name_opt: optional, secondary omap hwmod device
+ * @firmware: name of firmware file to load
+ * @ops: start/stop rproc handlers
+ * @memory_maps: table of da-to-pa iommu memory maps
+ */
+struct omap_rproc_pdata {
+ const char *name;
+ const char *iommu_name;
+ const char *oh_name;
+ const char *oh_name_opt;
+ const char *firmware;
+ const struct rproc_ops *ops;
+ const struct rproc_mem_entry *memory_maps;
+};
+
+#endif /* _PLAT_REMOTEPROC_H */
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index a60bb12..88421bd 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -5,3 +5,24 @@
# REMOTE_PROC gets selected by whoever wants it
config REMOTE_PROC
tristate
+
+# for tristate, either expose omap_device_* or pass it via pdata
+config OMAP_REMOTE_PROC
+ bool OMAP remoteproc support
+ depends on ARCH_OMAP4
+ select OMAP_IOMMU
+ select REMOTE_PROC
+ select OMAP_MBOX_FWK
+ default y
+ help
+ Say y here to support OMAP's remote processors (dual M3
+ and DSP on OMAP4) via the remote processor framework.
+
+ Currently only supported on OMAP4.
+
+ Usually you want to say y here, in order to enable multimedia
+ use-cases to run on your platform (multimedia codecs are
+ offloaded to remote DSP processors using this framework).
+
+ It's safe to say n here if you're not interested in multimedia
+ offloading or just want a bare minimum kernel.
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index d0f60c7..0198b1d 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_REMOTE_PROC) += remoteproc.o
+obj-$(CONFIG_OMAP_REMOTE_PROC) += omap_remoteproc.o
diff --git a/drivers/remoteproc/omap_remoteproc.c
b/drivers/remoteproc/omap_remoteproc.c
new file mode 100644
index 000..91f7f11
--- /dev/null
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -0,0 +1,243 @@
+/*
+ * Remote processor machine-specific module for OMAP4
+ *
+ * Copyright (C) 2011 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
[RFC 8/8] rpmsg: add omap host backend
Add rpmsg devices that wires virtio to the OMAP mailbox and
enabled A9-M3 communications on OMAP4.
Each M3 core gets its own virtio device, since it is running its own
independent instance of RTOS, and is therefore treated as a completely
separate processor by rpmsg.
Both M3 cores use the same mailbox device to communicate with the A9,
and the mailbox payload is used to indicate which of the virtqueues is
triggered (each M3 core gets 2 virtqueues, for rx and tx).
Map the two vrings and the IPC buffers as noncacheable; use
dynamically carved-out physical memory for that.
Put this omap implementation in a dedicated drivers/rpmsg/host folder,
with the intention of stacking different rpmsg host implementations
in one place.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
drivers/rpmsg/Kconfig |2 +
drivers/rpmsg/Makefile |2 +
drivers/rpmsg/host/Kconfig | 11 +
drivers/rpmsg/host/Makefile |1 +
drivers/rpmsg/host/omap_rpmsg.c | 540 +++
drivers/rpmsg/host/omap_rpmsg.h | 69 +
6 files changed, 625 insertions(+), 0 deletions(-)
create mode 100644 drivers/rpmsg/host/Kconfig
create mode 100644 drivers/rpmsg/host/Makefile
create mode 100644 drivers/rpmsg/host/omap_rpmsg.c
create mode 100644 drivers/rpmsg/host/omap_rpmsg.h
diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
index 41303f5..9544d62 100644
--- a/drivers/rpmsg/Kconfig
+++ b/drivers/rpmsg/Kconfig
@@ -12,3 +12,5 @@ if RPMSG
source drivers/virtio/Kconfig
endif # RPMSG
+
+source drivers/rpmsg/host/Kconfig
diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
index 7617fcb..368a526 100644
--- a/drivers/rpmsg/Makefile
+++ b/drivers/rpmsg/Makefile
@@ -1 +1,3 @@
obj-$(CONFIG_RPMSG)+= virtio_rpmsg_bus.o
+
+obj-$(CONFIG_RPMSG)+= host/
diff --git a/drivers/rpmsg/host/Kconfig b/drivers/rpmsg/host/Kconfig
new file mode 100644
index 000..fd2140c
--- /dev/null
+++ b/drivers/rpmsg/host/Kconfig
@@ -0,0 +1,11 @@
+config OMAP_RPMSG
+ tristate OMAP virtio-based remote processor messaging support
+ depends on ARCH_OMAP4 OMAP_REMOTE_PROC
+ select CONFIG_OMAP_MBOX_FWK
+ select RPMSG
+ help
+ Say Y if you want to enable OMAP's virtio-based remote-processor
+ messaging, currently only available on OMAP4. This is required
+ for offloading cpu-intensive and/or latency-sensitive tasks to
+ the remote on-chip M3s or C64x+ dsp, usually used by multimedia
+ frameworks.
diff --git a/drivers/rpmsg/host/Makefile b/drivers/rpmsg/host/Makefile
new file mode 100644
index 000..9823745
--- /dev/null
+++ b/drivers/rpmsg/host/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_OMAP_RPMSG) += omap_rpmsg.o
diff --git a/drivers/rpmsg/host/omap_rpmsg.c b/drivers/rpmsg/host/omap_rpmsg.c
new file mode 100644
index 000..b8c2305
--- /dev/null
+++ b/drivers/rpmsg/host/omap_rpmsg.c
@@ -0,0 +1,540 @@
+/*
+ * Remote processor messaging transport (OMAP platform-specific bits)
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Ohad Ben-Cohen o...@wizery.com
+ * Brian Swetland swetl...@google.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) %s: fmt, __func__
+
+#include linux/init.h
+#include linux/virtio.h
+#include linux/virtio_config.h
+#include linux/virtio_ids.h
+#include linux/virtio_ring.h
+#include linux/rpmsg.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/notifier.h
+#include linux/remoteproc.h
+
+#include plat/mailbox.h
+#include plat/dsp.h
+
+#include omap_rpmsg.h
+
+/**
+ * struct omap_rpmsg_vproc - omap's virtio remote processor state
+ * @vdev: virtio device
+ * @vring: phys address of two vrings; first one used for rx, 2nd one for tx
+ * @buf_paddr: physical address of the IPC buffer region
+ * @buf_size: size of IPC buffer region
+ * @buf_mapped: kernel (ioremap'ed) address of IPC buffer region
+ * @mbox_name: name of omap mailbox device to use with this vproc
+ * @rproc_name: name of remote proc device to use with this vproc
+ * @mbox: omap mailbox handle
+ * @rproc: remoteproc handle
+ * @nb: notifier block that will be invoked on inbound mailbox messages
+ * @vq: virtio's virtqueues
+ * @base_vq_id: index of first virtqueue that belongs to this vproc
+ * @num_of_vqs: number of virtqueues this vproc owns
+ * @static_chnls: table of static channels for this vproc
+ */
+struct omap_rpmsg_vproc {
+ struct virtio_device vdev;
+ unsigned int vring[2]; /* mpu owns first vring, ipu owns
Re: [RFC 0/8] Introducing a generic AMP/IPC framework
On Tue, Jun 21, 2011 at 10:50 AM, Ohad Ben-Cohen o...@wizery.com wrote: root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/state running (2) At this point, the two remote M3 cores also start dumping trace logs to shared memory buffers, which are exposed by debugfs entries: root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/trace0 CORE0 starting.. ... root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/trace1 CORE1 starting.. ... ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 7/8] drivers: introduce rpmsg, a remote-processor messaging bus
On Wed, Jun 22, 2011 at 5:42 AM, Rusty Russell ru...@rustcorp.com.au wrote:
On Tue, 21 Jun 2011 10:18:33 +0300, Ohad Ben-Cohen o...@wizery.com wrote:
Add a virtio-based IPC bus, which enables kernel users to communicate
with remote processors over shared memory using a simple messaging
protocol.
Wow, sometimes one writes a standard and people use it. Thanks!
And we really liked it: virtio_rpmsg_bus.c, the virtio driver which
does most of the magic here ended up pretty small thanks to virtio.
and the performance numbers are really good, too.
+ /* Platform must supply pre-allocated uncached buffers for now */
+ vdev-config-get(vdev, VPROC_BUF_ADDR, addr, sizeof(addr));
+ vdev-config-get(vdev, VPROC_BUF_NUM, num_bufs, sizeof(num_bufs));
+ vdev-config-get(vdev, VPROC_BUF_SZ, buf_size, sizeof(buf_size));
+ vdev-config-get(vdev, VPROC_BUF_PADDR, vrp-phys_base,
+ sizeof(vrp-phys_base));
The normal way is to think of the config space as a structure, and use
offsets rather than using an enum value to distinguish the fields.
Yes, I was (mis-)using the config space for now to talk with
platform-specific code (on the host), and not with the peer, so I
opted for simplicity.
But this is definitely one thing that is going away: I don't see any
reason why not just use dma_alloc_coherent (or even dma_pool_create)
directly from the driver here in order to get those buffers.
+#define RPMSG_NAME_SIZE 32
+#define RPMSG_DEVICE_MODALIAS_FMT rpmsg:%s
+
+struct rpmsg_device_id {
+ char name[RPMSG_NAME_SIZE];
+ kernel_ulong_t driver_data /* Data private to the driver */
+ __attribute__((aligned(sizeof(kernel_ulong_t;
+};
This alignment directive seems overkill...
Yes, looks like I can remove this. thanks.
+#define VIRTIO_ID_RPMSG 10 /* virtio remote processor
messaging */
I think you want 6. Plan 9 jumped ahead to grab 9 :)
6 it is :)
Thanks,
Ohad.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 2/8] remoteproc: add omap implementation
On Wed, Jun 22, 2011 at 1:05 PM, Will Newton will.new...@gmail.com wrote: On Tue, Jun 21, 2011 at 8:18 AM, Ohad Ben-Cohen o...@wizery.com wrote: +/* bootaddr isn't needed for the dual M3's */ +static inline int omap_rproc_start(struct rproc *rproc, u64 bootaddr) +static inline int omap_rproc_stop(struct rproc *rproc) These two functions don't need to be inline as far as I can see. They definitely don't need to. Thanks ! ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 0/8] Introducing a generic AMP/IPC framework
Hi Arnd,
On Tue, Jun 21, 2011 at 5:20 PM, Arnd Bergmann a...@arndb.de wrote:
This looks really nice overall, but I don't currently have time for a
more in-depth review. My feeling is that you are definitely on the right
track here, and the plans you list as TODO to continue are all good.
Thanks !
One point I noticed is the use of debugfs, which you should probably
replace at some point with a stable API, e.g. your own debugfs-like
file system, but there is no hurry to do that now.
debugfs is only being used to expose debugging info (namely the power
state of the remote processor and its trace log messages), which is
mostly useful for developers trying to understand what went wrong.
It seems like debugfs fits quite nicely here (e.g. it's perfectly fine
if this is completely compiled out on production systems), but sure,
we can always revisit this later too.
Regarding the firmware, I think it would be nice to have the option
to stick the firmware blob into the flattened device tree as a bininclude,
so you can build systems using the external processors without special
user space support.
That's interesting. we'll definitely explore this - thanks !
I was also hoping that DT will simplify remote resource allocations as
well (just like any other device, remote processors usually needs some
hw resources) and planned to try it out soon to see how it all works
out together. It might completely eliminate the preliminary resource
section thingy we have in the firmware structure currently.
The binary format looks reasonable, but if you need anything more complex,
you should probably go straight for ELF files ;-)
Hopefully we won't need to :)
+struct fw_section {
+ u32 type;
+ u64 da;
+ u32 len;
+ char content[0];
+} __packed;
Unfortunately require __packed. It would be better to sort the members
differently so that each member is naturally aligned in order to
avoid the need for padding or __packed attributes
Definitely. __packed is being used just to be on the safe side; we
didn't intend to introduce unnatural alignment intentionally. will be
fixed.
Thanks!
Ohad.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[RFC 0/8] Introducing a generic AMP/IPC framework
be used to create the rpmsg channels. - to support rpmsg server drivers, which aren't bound to a specific remote rpmsg address. Instead, they just listen on a local address, waiting for incoming messages. To send a message, those server drivers need to use the rpmsg_sendto() API, so they can explicitly indicate the dst address every time. There are already several immediate use cases for rpmsg drivers: OMX offloading (already being used on OMAP4), hardware resource manager (remote processors on OMAP4 need to ask Linux to enable/disable hardware resources on its behalf), remote display driver on Netra (dm8168), where the display is controlled by a remote M3 processor (and a Linux v4l2/fbdev driver will use rpmsg to communicate with that remote display driver). * Remoteproc: a generic driver that maintains the state of the remote processor(s). Simple rproc_get() and rproc_put() API is exposed, which drivers can use when needed (first driver to call get() will load a firmware, configure an iommu if needed, and boot the remote processor, while last driver to call put() will power it down). Hardware differences are abstracted as usual: a platform-specific driver registers its own start/stop handlers in the generic remoteproc driver, and those are invoked when its time to power up/down the processor. As a reference, this patch set include remoteproc support for both OMAP4's cortex-M3 and Davinci's DSP, tested on the pandaboard and hawkboard, respectively. The gory part of remoteproc is the firmware handling. We tried to come up with a simple binary format that will require minimum kernel code to handle, but at the same time be generic enough in the hopes that it would prove useful to others as well. We're not at all hang onto the binary format we picked: if there's any technical reason to change it to support other platforms, please let us know. We do realize that a single binary firmware structure might eventually not work for everyone. it did prove useful for us though; we adopted both the OMAP and Davinci platforms (and their completely different remote processor devices) to this simple binary structure, so we don't have to duplicate the firmware handling code. It's also not entirely clear whether remoteproc should really be an independent layer, or if it should just be squashed with the host-specific component of the rpmsg framework (there isn't really a remoteproc use case that doesn't need rpmsg anyway). Looking ahead, functionality like error recovery and power management might require coupling those two together as well. Important stuff: * Thanks Brian Swetland for great design ideas and fruitful meetings and Arnd Bergmann for pointing us at virtio, which is just awesome. * Thanks Bhavin Shah, Mark Grosen, Suman Anna, Fernando Guzman Lugo, Shreyas Prasad, Gilbert Pitney, Armando Uribe De Leon, Robert Tivy and Alan DeMars for all your help. You know what you did. * Patches are based on 3.0-rc3. Code was tested on OMAP4 using a panda board (and remoteproc was also tested on Davinci da850-evm and hawkboard). * I will be replying to this email with an attachment of the M3 firmware image I work with, so anyone with a panda board can take the code for a ride (put the image in /lib/firmware, and tell me if stuff doesn't work for you: there are a few omap iommu fixes that are circulating but not merged yet). * Licensing: definitions that needs to be shared with remote processors were put in BSD-licensed header files, so anyone can use them to develop compatible peers. * The M3 RTOS source code itself is BSD licensed and will be publicly released too (it's in the works). * This work is not complete; there are still several things to implement, improve and clean up, but the intention is to start the review, and not wait until everything is finished. Guzman Lugo, Fernando (2): remoteproc: add omap implementation omap: add remoteproc devices Mark Grosen (2): remoteproc: add davinci implementation davinci: da850: add remoteproc dsp device Ohad Ben-Cohen (4): drivers: add generic remoteproc framework omap: add carveout memory support for remoteproc drivers: introduce rpmsg, a remote-processor messaging bus rpmsg: add omap host backend Documentation/ABI/testing/sysfs-bus-rpmsg | 75 ++ Documentation/remoteproc.txt| 170 Documentation/rpmsg.txt | 308 +++ arch/arm/mach-davinci/board-da850-evm.c |4 + arch/arm/mach-davinci/board-omapl138-hawk.c |4 + arch/arm/mach-davinci/da850.c | 14 + arch/arm/mach-davinci/devices-da8xx.c | 15 + arch/arm/mach-davinci/include/mach/da8xx.h |1 + arch/arm/mach-davinci/include/mach/remoteproc.h | 28 + arch/arm/mach-omap2/Makefile|2 + arch/arm/mach-omap2/remoteproc.c| 159 arch/arm/plat-omap/Kconfig |8 + arch/arm/plat-omap/devices.c
Re: [RFC 0/8] Introducing a generic AMP/IPC framework
On Wed, Jun 22, 2011 at 4:05 PM, Arnd Bergmann a...@arndb.de wrote: Ok, I see. In that case I agree that using debugfs is fine, but I would recommend trying to use fewer macros and just open-coding the file operations for better readability. Sure thing. It didn't end up saving much code eventually, too, so I'll just remove it. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC 1/8] drivers: add generic remoteproc framework
Hi Randy, On Wed, Jun 22, 2011 at 8:55 PM, Randy Dunlap rdun...@xenotime.net wrote: On Tue, 21 Jun 2011 10:18:27 +0300 Ohad Ben-Cohen wrote: Hi, Just a few minor nits inline... Thanks! Ohad. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: SDIO WIFI for DM355? [Hijacked, was: Is there a USB 802.11n wifi card which suit DM6446?]
On Tue, Aug 24, 2010 at 9:06 AM, Subbrathnam, Swaminathan swami.i...@ti.com wrote: From: Jon Povey jon.po...@racelogic.co.uk A related question; anyone care to recommend an SDIO WIFI card for use with DM355? Solutions from Marvel were used by customers earlier. TI also have their wl1271 wifi sdio device ported to several davinci types and boards. ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
