2012-06-22 14:32:15

by Sjur BRENDELAND

[permalink] [raw]
Subject: [RFC 0/4] STE-modem remoteproc driver

From: Sjur Brændeland <[email protected]>

Hi Ohad,

This is the initial stab on the remoteproc driver for ST-Ericsson modems.
Please let me know what you think.

remoteproc_ste_modem features:
For kicking to work I need to declare what IDs I'm currently using, so I had to
iterate over the rproc->notifyids to find the IDs.

A sysfs file is introduced for switching on/off the modem, as modem
start-up must be controlled from user space.

As mentioned before I need the firmware to be allocated first in the shared
memory are. So in order to reserve space for firmware at the start of the
memory region, I had to allocate dma memory before calling rproc_register(),
and then releasing it afterward.


I do not yet have an implementation of the stemod_kick module available. It
might take a while before this is upstreamed. The stemod_kick functions are
looked up dynamically so this is not a build issue.

The platform device is not yet upstreamed either. But it should be pretty simple,
it basically just provides the shared memory area to the driver.

I also included one fix in this patch-set. It doesn't seems like carveout for
non-iommu is working correctly. I added a simple patch for this.
Let me know what you think, and how to proceed for this fix.

Best regards,
Sjur Brændeland

Sjur Brændeland (4):
remoteproc: Bugfix assign device address to carveout (noiommu)
remoteproc: Add custom STE-modem firmware loader.
remoteproc: Add API (header file) for kicking STE modems
remoteproc: Add driver for STE Modem

drivers/remoteproc/Kconfig | 10 +
drivers/remoteproc/Makefile | 3 +
drivers/remoteproc/remoteproc_core.c | 4 +
drivers/remoteproc/remoteproc_internal.h | 1 +
drivers/remoteproc/remoteproc_ste_modem_loader.c | 179 ++++++++++++
drivers/remoteproc/ste_modem_rproc.c | 333 ++++++++++++++++++++++
drivers/remoteproc/stemod_kick.h | 113 ++++++++
7 files changed, 643 insertions(+), 0 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_ste_modem_loader.c
create mode 100644 drivers/remoteproc/ste_modem_rproc.c
create mode 100644 drivers/remoteproc/stemod_kick.h

--
1.7.5.4


2012-06-22 14:31:31

by Sjur BRENDELAND

[permalink] [raw]
Subject: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

From: Sjur Brændeland <[email protected]>

Carveout did not set device address when IOMMU is not supported.
Fix this by assigning dma address to device address to carveout
if IOMMU is not supported.

Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index bee4644..8185c11 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -628,6 +628,10 @@ static int rproc_handle_carveout(struct rproc *rproc,
* dual M3 subsystem).
*/
rsc->pa = dma;
+ } else {
+ rsc->da = dma;
+ dev_dbg(dev, "carveout: %s va %p dma %x\n",
+ rsc->name, va, rsc->da);
}

carveout->va = va;
--
1.7.5.4

2012-06-22 14:31:36

by Sjur BRENDELAND

[permalink] [raw]
Subject: [RFC 4/4] remoteproc: Add driver for STE Modem

From: Sjur Brændeland <[email protected]>

Introduce the platform driver for ste-modems.
This driver uses the remoteproc framework for managing the
modem and for creating virtio devices used for communicating
with the modem.

A sysfs file is introduced for switching on/off the modem, as modem
start-up must be controlled from user space.

Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/ste_modem_rproc.c | 333 ++++++++++++++++++++++++++++++++++
2 files changed, 334 insertions(+), 0 deletions(-)
create mode 100644 drivers/remoteproc/ste_modem_rproc.c

diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index b91ecb0b..aec7470 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -9,4 +9,5 @@ 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_remoteproc.o
+ste_modem_remoteproc-y += ste_modem_rproc.o
ste_modem_remoteproc-y += remoteproc_ste_modem_loader.o
diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c
new file mode 100644
index 0000000..a575e2a
--- /dev/null
+++ b/drivers/remoteproc/ste_modem_rproc.c
@@ -0,0 +1,333 @@
+/*
+ * Copyright (C) ST-Ericsson AB 2012
+ * Author: Sjur Brændeland <[email protected]>
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s(): " fmt, __func__
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/remoteproc.h>
+#include <linux/dma-mapping.h>
+#include "remoteproc_internal.h"
+#include "stemod_kick.h"
+
+/* Maxium number of "channels" */
+#define MAX_VQS 14
+
+/* Maxium size of the STE firmwaree image 160Kb */
+#define STEMOD_FW_SIZE (40 * 4096)
+
+struct sproc {
+ struct rproc *rproc;
+ /* Number of kick identifiers needed */
+ u32 nvqs;
+ /* Track the user requested power state */
+ bool powered;
+ int (*kick_modem)(int notifyid);
+};
+
+/* kick a virtqueue */
+static void ste_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct sproc *sproc = rproc->priv;
+ dev_dbg(rproc->dev, "kick vqid:%d\n", vqid);
+ sproc->kick_modem(vqid + MAX_VQS);
+}
+
+/* modem is kicking us */
+static void ste_rproc_callback(int vqid, void *data)
+{
+ struct sproc *sproc = data;
+ if (rproc_vq_interrupt(sproc->rproc, vqid) == IRQ_NONE)
+ dev_dbg(sproc->rproc->dev,
+ "no message was found in vqid %d\n", vqid);
+}
+
+/* Iterator called by idr_for_each(), tracking notification Ids */
+static int ste_rproc_set_vqid(int id, void *p, void *data)
+{
+ u32 *mask = data;
+ *mask |= 1 << id;
+ return 0;
+}
+
+/* Setup the kick API for notification subscriptions */
+static int ste_rproc_subscribe_to_kicks(struct rproc *rproc)
+{
+ int i, err;
+ u32 txmask = 0, rxmask = 0;
+ int (*kick_subscribe)(int notifyid,
+ void (*notify_cb)(int notifyid, void *data), void *data);
+ int (*notifyid_alloc)(u32 setter_mask, u32 getter_mask);
+
+ /*
+ * We need to declare for the HW what notification IDs
+ * we're going to use. So we create a bitmask with the
+ * notification IDs used for RX and TX by iterating over
+ * the notification IDs.
+ */
+ idr_for_each(&rproc->notifyids, ste_rproc_set_vqid, &rxmask);
+
+ /* Verify that notification ID is in the range 0-13 */
+ if (rxmask & 0x3fff) {
+ dev_err(rproc->dev, "Bad Notification IDs used: %x\n", rxmask);
+ return -EINVAL;
+ }
+ /*
+ * Bits 0-13 are used for RX kicks and bits 14-27 for TX.
+ * We simply set TX notification ID to be RX notification ID + 14.
+ */
+ txmask = rxmask << MAX_VQS;
+
+ notifyid_alloc = symbol_get(stemod_kick_notifyid_alloc);
+ if (notifyid_alloc == NULL)
+ return -EINVAL;
+
+ /* Tell kick-hw what bit we use for RX and TX */
+ err = notifyid_alloc(rxmask, txmask);
+ symbol_put(stemod_kick_notifyid_alloc);
+
+ if (err < 0) {
+ dev_err(rproc->dev, "allocation of bits %x/%x failed:%d\n",
+ rxmask, txmask, err);
+ return err;
+ }
+
+ kick_subscribe = symbol_get(stemod_kick_subscribe);
+ if (kick_subscribe == NULL)
+ return -EINVAL;
+
+ /* Subscribe for RX interrupts */
+ for (err = 0, i = 0; i < MAX_VQS && !err; i++)
+ if (rxmask & (1 << i))
+ err = kick_subscribe(i, ste_rproc_callback,
+ rproc->priv);
+ symbol_put(stemod_kick_subscribe);
+ if (err) {
+ dev_err(rproc->dev, "subscription of kicks failed:%d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+int power_switch(bool on)
+{
+ int err = 0;
+ int (*power)(bool on);
+
+ power = symbol_get(stemod_power);
+ if (power == NULL)
+ err = -EINVAL;
+ err = power(on);
+ symbol_put(stemod_power);
+ return err;
+}
+
+/* Start the STE modem */
+static int ste_rproc_start(struct rproc *rproc)
+{
+ int err;
+ dev_info(rproc->dev, "start modem\n");
+
+ err = ste_rproc_subscribe_to_kicks(rproc->priv);
+ if (err)
+ return err;
+
+ /* Power on modem */
+ return power_switch(true);
+}
+
+/* Stop the STE modem */
+static int ste_rproc_stop(struct rproc *rproc)
+{
+ struct device *dev = rproc->dev;
+ int (*reset)(void);
+
+ dev_info(dev, "stop modem\n");
+
+ /* Reset kick HW */
+ reset = symbol_get(stemod_kick_reset);
+ reset();
+ symbol_put(stemod_kick_reset);
+
+ /* Power off modem */
+ return power_switch(true);
+}
+
+static struct rproc_ops ste_rproc_ops = {
+ .start = ste_rproc_start,
+ .stop = ste_rproc_stop,
+ .kick = ste_rproc_kick,
+};
+
+/* Get ste_rproc given platform device */
+static struct sproc *ste_rproc_get_by_dev(const struct device *dev)
+{
+ struct platform_device *pdev;
+ struct rproc *rproc;
+ pdev = container_of(dev, struct platform_device, dev);
+ rproc = platform_get_drvdata(pdev);
+ if (rproc == NULL)
+ return NULL;
+ return rproc->priv;
+}
+
+/* Read sysfs entry 'powered' */
+static ssize_t ste_rproc_powered_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct sproc *sproc = ste_rproc_get_by_dev(dev);
+ ssize_t size;
+
+ if (sproc == NULL)
+ return -EINVAL;
+
+ size = sprintf(buf, "%d\n", sproc->powered);
+ return size;
+}
+
+/* Write sysfs entry 'powered' */
+static ssize_t ste_rproc_powered_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned long val;
+ int ret = -EINVAL;
+ struct sproc *sproc = ste_rproc_get_by_dev(dev);
+
+ if (sproc == NULL)
+ return -EINVAL;
+
+ if (kstrtoul(buf, 10, &val) < 0)
+ goto err;
+
+ if (sproc->powered && val == 0) {
+ rproc_shutdown(sproc->rproc);
+ sproc->powered = false;
+ } else if (!sproc->powered && val == 1) {
+
+ if (rproc_boot(sproc->rproc))
+ goto err;
+ sproc->powered = true;
+ } else {
+ dev_err(dev, "invalid sysfs state entered\n");
+ goto err;
+ }
+ ret = count;
+err:
+ return ret;
+}
+
+/* Declare sysfs entry powered */
+static DEVICE_ATTR(powered, S_IRUGO | S_IWUSR | S_IWGRP ,
+ ste_rproc_powered_show, ste_rproc_powered_store);
+struct device_attribute *remoteproc_attrs[] = { &dev_attr_powered };
+
+/* Platform device for STE modem is registered */
+static int __devinit ste_rproc_probe(struct platform_device *pdev)
+{
+ struct sproc *ste_proc;
+ struct rproc *rproc;
+ int ret;
+ void *reserved;
+ dma_addr_t dma;
+
+ /*
+ * Prerequisite: The platform device must declare the shared
+ * memory region to be used by STE-modem and make memory
+ * available for rproc by using dma_declare_coherent_memory
+ * (or CMA).
+ */
+ rproc = rproc_alloc(&pdev->dev,
+ pdev->name,
+ &ste_rproc_ops,
+ "ste-modem-fw",
+ sizeof(*ste_proc));
+ if (!rproc)
+ return -ENOMEM;
+
+ ste_proc = rproc->priv;
+ ste_proc->rproc = rproc;
+ platform_set_drvdata(pdev, rproc);
+
+ /* Inject the STE-modem specific firmware handler */
+ rproc->fw_ops = &rproc_ste_modem_fw_ops;
+
+ /*
+ * We're dynamically looking up the symbols for the API used
+ * for generating kicks to and from the modem.
+ */
+ ste_proc->kick_modem = symbol_get(stemod_kick_notifyid);
+ if (ste_proc->kick_modem == NULL) {
+ ret = -EINVAL;
+ dev_err(rproc->dev, "cannot load stemod_kick API\n");
+ goto free_rproc;
+ }
+
+ /*
+ * Registration of the ste_modem_rproc will cause firmware to
+ * to be fetched and the virtio resource entries to be allocated
+ * in memory. However STE-modem requires the firmware to be located
+ * at the start of the shared memory region. So we need to
+ * reserve space for firmware at the start of the shared memory
+ * region.
+ */
+ reserved = dma_alloc_coherent(&pdev->dev, STEMOD_FW_SIZE,
+ &dma, GFP_KERNEL);
+
+ ret = rproc_register(rproc);
+ if (ret)
+ goto free_rproc;
+
+ /*
+ * When firmware loading is completed and virtio resource
+ * entries are allocated in memory, we can release the
+ * memory space reserved for modem firmware.
+ * When user switch on the modem, the firmware will be
+ * loaded at the start of the memory region.
+ */
+ wait_for_completion(&rproc->firmware_loading_complete);
+ dma_free_coherent(&pdev->dev, PAGE_SIZE * 10, reserved, dma);
+
+ /* Create powered sysfs entry, to start/stop STE modem */
+ ret = device_create_file(&pdev->dev, &dev_attr_powered);
+ if (ret)
+ goto free_rproc;
+
+ return 0;
+
+free_rproc:
+ platform_set_drvdata(pdev, NULL);
+ rproc_free(rproc);
+ return ret;
+}
+
+/* Platform device for STE modem is unregistered */
+static int __devexit ste_rproc_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ symbol_put(stemod_kick_notifyid);
+ return rproc_unregister(rproc);
+}
+
+static struct platform_driver ste_rproc_driver = {
+ .probe = ste_rproc_probe,
+ .remove = __devexit_p(ste_rproc_remove),
+ .driver = {
+ .name = "ste-modem",
+ .owner = THIS_MODULE,
+ },
+};
+
+module_platform_driver(ste_rproc_driver);
+MODULE_LICENSE("GPL v2");
--
1.7.5.4

2012-06-22 14:31:52

by Sjur BRENDELAND

[permalink] [raw]
Subject: [RFC 3/4] remoteproc: Add API (header file) for kicking STE modems

From: Sjur Brændeland <[email protected]>

Add the API definition for STE Modem interrupt mecanism (kicks).

Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/stemod_kick.h | 113 ++++++++++++++++++++++++++++++++++++++
1 files changed, 113 insertions(+), 0 deletions(-)
create mode 100644 drivers/remoteproc/stemod_kick.h

diff --git a/drivers/remoteproc/stemod_kick.h b/drivers/remoteproc/stemod_kick.h
new file mode 100644
index 0000000..dbfa909
--- /dev/null
+++ b/drivers/remoteproc/stemod_kick.h
@@ -0,0 +1,113 @@
+/*
+ * Copyright (C) ST-Ericsson AB 2011
+ * Author: Sjur Brendeland / [email protected]
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#ifndef __INC_STEMOD_KICK_H
+#define __INC_STEMOD_KICK_H
+#include <linux/types.h>
+
+/**
+ * stemod_kick_subscribe - Subscribe for notification change from the modem.
+ *
+ * @notifyid: The notification ID the callback is associated with.
+ *
+ * @notify_cb: Callback function to be called when the requested notify-id
+ * is set by the external device (modem).
+ *
+ * @data: Client data to be provided in the callback function.
+ *
+ * Install a callback function for a notification ID.
+ * Returns negative upon error.
+ *
+ * This function may block, and cannot be called from IRQ context.
+ * Precondition: @notifyid must be defined as as getter in
+ * stemod_kick_notifyid_alloc().
+ * Returns zero on success, and negative upon error.
+ *
+ * Callback context:
+ * The "notify_cb" callback might be called from the
+ * IRQ context. The callback function is not allowed to block
+ * or spend much CPU time in the callback, It must defer
+ * work to soft-IRQ or work queues.
+ */
+int stemod_kick_subscribe(int notifyid,
+ void (*notify_cb)(int notifyid, void *data),
+ void *data);
+
+/**
+ * stemod_kick_notifyid_alloc - Allocate the usage of notification IDs.
+ *
+ * @rx_mask: Bit-mask defining the notifyids that can be
+ * subscribed by stemod_kick_subscribe().
+ * @tx_mask: Bit-mask defining the notifyids that can be set by
+ * stemod_kick_set_notifyid()
+ *
+ * The @rx_mask defines the notification-id that can be subscribed by
+ * the function stemod_kick_subscribe().
+ * The @tx_mask defines the notification-ids that can be set by the
+ * function stemod_kick_set_notifyid().
+ *
+ * This function may block.
+ *
+ * Returns zero on success, and negative upon error.
+ *
+ */
+int stemod_kick_notifyid_alloc(u32 rx_mask, u32 tx_mask);
+
+/**
+ * stemod_kick_register_errhandler - Register an error handler.
+ *
+ * @errhandler: error handler called from driver upon severe errors
+ * that requires reset of the remote device.
+ */
+void stemod_kick_register_errhandler(void (*errhandler)(int errno));
+
+/**
+ * stemod_kick_reset() - Reset the C2C driver
+ *
+ * Reset the Kick Driver due to remote device (modem) restart.
+ * This shall reset state back to initial state, and should only
+ * be used when remote device (modem) has reset.
+ *
+ * All settings, subscriptions and state information in the driver is
+ * reset.
+ * This function may block.
+ *
+ * Returns zero on success, and negative upon error.
+ */
+int stemod_kick_reset(void);
+
+/**
+ * stemod_kick_notifyid() - Genereate a notification to remote device.
+ *
+ * @notifyid: The notification ID to generate interrupt for.
+ *
+ * This function is used to send notification to the remote
+ * processor (modem).
+ *
+ * This function is non-blocking, and can be called from Soft-IRQ context.
+ *
+ * Returns zero on success, and negative upon error.
+ *
+ * Precondition: @notifyid must be defined as as setter in
+ * stemod_kick_notifyid_alloc().
+ */
+int stemod_kick_notifyid(int notifyid);
+
+
+/**
+ * stemod_power() - On/Off switch for modem
+ *
+ * @on: Switch on or off
+ *
+ * This function is the power switch for the STE-Modem.
+ *
+ * Returns zero on success, and negative upon error.
+ *
+ */
+int stemod_power(bool on);
+
+#endif /*INC_STEMOD_KICK_H*/
--
1.7.5.4

2012-06-22 14:32:13

by Sjur BRENDELAND

[permalink] [raw]
Subject: [RFC 2/4] remoteproc: Add custom STE-modem firmware loader.

From: Sjur Brændeland <[email protected]>

Add custom firmware loader for STE firmware. This plugin adds
functions for extracting the resource table and loading the
firmware image into shared memory.

Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/Kconfig | 10 ++
drivers/remoteproc/Makefile | 2 +
drivers/remoteproc/remoteproc_internal.h | 1 +
drivers/remoteproc/remoteproc_ste_modem_loader.c | 179 ++++++++++++++++++++++
4 files changed, 192 insertions(+), 0 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_ste_modem_loader.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 24d880e..9beb5ed 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -25,4 +25,14 @@ config OMAP_REMOTEPROC
It's safe to say n here if you're not interested in multimedia
offloading or just want a bare minimum kernel.

+config STE_MODEM_RPROC
+ tristate "STE-Modem remoteproc support"
+ select REMOTEPROC
+ depends on EXPERIMENTAL
+ default n
+ help
+ Say y or m here to support STE-Modem shared memory driver.
+ This can be either built-in or a loadable module.
+ If unsure say N.
+
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 934ce6e..b91ecb0b 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -8,3 +8,5 @@ remoteproc-y += remoteproc_debugfs.o
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_remoteproc.o
+ste_modem_remoteproc-y += remoteproc_ste_modem_loader.o
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 7823156..bc36302 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -80,5 +80,6 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc,
}

extern const struct rproc_fw_ops rproc_elf_fw_ops;
+extern const struct rproc_fw_ops rproc_ste_modem_fw_ops;

#endif /* REMOTEPROC_INTERNAL_H */
diff --git a/drivers/remoteproc/remoteproc_ste_modem_loader.c b/drivers/remoteproc/remoteproc_ste_modem_loader.c
new file mode 100644
index 0000000..139222d
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_ste_modem_loader.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) ST-Ericsson AB 2012
+ * Author: Sjur Brendeland / [email protected]
+ * License terms: GNU General Public License (GPL) version 2.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <linux/module.h>
+#include <linux/firmware.h>
+#include <linux/remoteproc.h>
+#include "remoteproc_internal.h"
+
+#define TOC_RSC_TAB_NAME "rsc-table"
+
+/* struct toc_entry - Table of content entry
+ *
+ * @start: Offset to the image data.
+ * @size: Size of the images in bytes.
+ * @flags: Use 0 if no flags are in use.
+ * @entry_point: Modem internal information. Where to jump to start executing.
+ * Only applicable when using SDRAM. Set to 0xffffffff if unused.
+ * @load_addr: Modem internal information. Location in SDRAM to move image.
+ * Set to 0xffffffff if not applicable.
+ * @name: Name of image.
+ */
+struct toc_entry {
+ __le32 start;
+ __le32 size;
+ __le32 flags;
+ __le32 entry_point;
+ __le32 load_addr;
+ char name[12];
+};
+
+/** struct toc - Table of content
+ * @table: Table of toc entries.
+ * The Table Of Content is located at the start of the firmware image and
+ * at offset zero in the shared memory region. The resource table typically
+ * contains the initial boot image (boot strap) and other information elements
+ * such as remoteproc resource table. Each entry is identified by a unique
+ * @name.
+ */
+struct toc {
+ struct toc_entry table[32];
+};
+
+/**
+ * ste_load_segments() - load firmware segments to memory
+ * @rproc: remote processor which will be booted using these fw segments
+ * @fw: the TOC and firmware image to load
+ *
+ * This function loads the firmware segments to memory. STE Modem SHM
+ * does not use an IOMMU, and expects the firmware containing the
+ * "Table Of Content" (TOC) first in the firmware. The TOC specifies the
+ * offset and size of the boot image.
+ */
+static int
+ste_load_segments(struct rproc *rproc, const struct firmware *fw)
+{
+ /*
+ * STE-Modem does not use a IOMMU and the virtual and physical
+ * addresses of the device (modem) is not known. Instead
+ * offsets from the start of the shared memory region is used
+ * as device address (da).
+ *
+ * The STE-Modem must provide a carveout as the first entry with
+ * sufficient space for the firmware image. The device address in
+ * this carveout must be set to zero.
+ *
+ * The firmware must be copied into offset zero, i.e at the start of
+ * the shared memory area.
+ */
+
+ u32 offset = 0;
+ void *ptr = rproc_da_to_va(rproc, offset, fw->size);
+
+ if (!ptr) {
+ dev_err(rproc->dev, "bad offset 0x%x mem 0x%zx\n",
+ offset, fw->size);
+ return -EINVAL;
+ }
+
+ memcpy(ptr, fw->data, fw->size);
+ return 0;
+}
+
+/* Find the entry for resource table in the Table of Content */
+static struct toc_entry *__find_rsc_entry(const struct firmware *fw)
+{
+ int i;
+ struct toc *toc;
+ int entries = ARRAY_SIZE(toc->table);
+
+ if (!fw)
+ return NULL;
+
+ toc = (void *)fw->data;
+
+ /* Search the table for the resource table */
+ for (i = 0; i < entries && toc->table[i].start != 0xffffffff; i++) {
+ if (!strncmp(toc->table[i].name, TOC_RSC_TAB_NAME,
+ sizeof(toc->table[i].name))) {
+ if (toc->table[i].start > fw->size)
+ return NULL;
+ return &toc->table[i];
+ }
+ }
+ return NULL;
+}
+
+/**
+ * ste_find_rsc_table() - find the resource table
+ * @rproc: the rproc handle
+ * @fw: the firmware image
+ * @tablesz: place holder for providing back the table size
+ *
+ * This function finds the resource table inside the remote processor's
+ * firmware. It is used both upon the registration of @rproc (in order
+ * to look for and register the supported virito devices), and when the
+ * @rproc is booted.
+ *
+ * Returns the pointer to the resource table if it is found, and write its
+ * size into @tablesz. If a valid table isn't found, NULL is returned
+ * (and @tablesz isn't set).
+ */
+static struct resource_table *
+ste_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+ int *tablesz)
+{
+ struct resource_table *table;
+ struct device *dev = rproc->dev;
+ struct toc_entry *entry = __find_rsc_entry(fw);
+
+ if (!entry) {
+ dev_err(dev, "resource table not found in fw\n");
+ return NULL;
+ }
+
+ table = (void *) (fw->data + entry->start);
+
+ /* make sure we have the entire table */
+ if (entry->start + entry->size > fw->size) {
+ dev_err(dev, "resource table truncated\n");
+ return NULL;
+ }
+
+ /* make sure table has at least the header */
+ if (sizeof(struct resource_table) > entry->size) {
+ dev_err(dev, "header-less resource table\n");
+ return NULL;
+ }
+
+ /* we don't support any version beyond the first */
+ if (table->ver != 1) {
+ dev_err(dev, "unsupported fw ver: %d\n", table->ver);
+ return NULL;
+ }
+
+ /* make sure reserved bytes are zeroes */
+ if (table->reserved[0] || table->reserved[1]) {
+ dev_err(dev, "non zero reserved bytes\n");
+ return NULL;
+ }
+
+ /* make sure the offsets array isn't truncated */
+ if (table->num * sizeof(table->offset[0]) +
+ sizeof(struct resource_table) > entry->size) {
+ dev_err(dev, "resource table incomplete\n");
+ return NULL;
+ }
+
+ *tablesz = entry->size;
+ return table;
+}
+
+const struct rproc_fw_ops rproc_ste_modem_fw_ops = {
+ .load = ste_load_segments,
+ .find_rsc_table = ste_find_rsc_table
+};
--
1.7.5.4

2012-06-30 14:17:47

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

Hi Sjur,

On Fri, Jun 22, 2012 at 5:31 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Carveout did not set device address when IOMMU is not supported.
> Fix this by assigning dma address to device address to carveout
> if IOMMU is not supported.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
> ---
> ?drivers/remoteproc/remoteproc_core.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index bee4644..8185c11 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -628,6 +628,10 @@ static int rproc_handle_carveout(struct rproc *rproc,
> ? ? ? ? ? ? ? ? ?* dual M3 subsystem).
> ? ? ? ? ? ? ? ? ?*/
> ? ? ? ? ? ? ? ? rsc->pa = dma;
> + ? ? ? } else {
> + ? ? ? ? ? ? ? rsc->da = dma;
> + ? ? ? ? ? ? ? dev_dbg(dev, "carveout: %s va %p dma %x\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? rsc->name, va, rsc->da);

To simplify things, will the below work for you?

diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index c4d4a21..f03d074 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -713,23 +713,27 @@ static int rproc_handle_carveout(struct rproc *rproc,
list_add_tail(&mapping->node, &rproc->mappings);

dev_dbg(dev, "carveout mapped 0x%x to 0x%x\n", rsc->da, dma);
-
- /*
- * Some remote processors might need to know the pa
- * even though they are behind an IOMMU. E.g., OMAP4's
- * remote M3 processor needs this so it can control
- * on-chip hardware accelerators that are not behind
- * the IOMMU, and therefor must know the pa.
- *
- * Generally we don't want to expose physical addresses
- * if we don't have to (remote processors are generally
- * _not_ trusted), so we might want to do this only for
- * remote processor that _must_ have this (e.g. OMAP4's
- * dual M3 subsystem).
- */
- rsc->pa = dma;
}

+ /*
+ * Some remote processors might need to know the pa
+ * even though they are behind an IOMMU. E.g., OMAP4's
+ * remote M3 processor needs this so it can control
+ * on-chip hardware accelerators that are not behind
+ * the IOMMU, and therefor must know the pa.
+ *
+ * Generally we don't want to expose physical addresses
+ * if we don't have to (remote processors are generally
+ * _not_ trusted), so we might want to do this only for
+ * remote processor that _must_ have this (e.g. OMAP4's
+ * dual M3 subsystem).
+ *
+ * Non-IOMMU processors might also want to have this info.
+ * In this case, the device address and the physical address
+ * are the same.
+ */
+ rsc->pa = dma;
+
carveout->va = va;
carveout->len = rsc->len;
carveout->dma = dma;

Thanks,
Ohad.

2012-08-08 17:55:17

by Sjur Brændeland

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

Hi Ohad,

> Carveout did not set device address when IOMMU is not supported.
> Fix this by assigning dma address to device address to carveout
> if IOMMU is not supported.

I realize that we have the same issue with the virtio rings.
Are there any way we can assign the device address of the virtio rings
to the resource table in shared memory? Or do we have to calculate the
virtio ring addresses based on number rings and the number of elements
in the ring?

Regards,
Sjur

2012-08-09 19:55:16

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

Hi Sjur,

On Wed, Aug 8, 2012 at 7:55 PM, Sjur Br?ndeland <[email protected]> wrote:
> I realize that we have the same issue with the virtio rings.
> Are there any way we can assign the device address of the virtio rings
> to the resource table in shared memory?

It's a gap we have today, but it should definitely be fixed.

> Or do we have to calculate the
> virtio ring addresses based on number rings and the number of elements
> in the ring?

No, that's not the long term intention. It can be used as an interim
solution, but I expect we do fix this and start supporting dynamic
assignments of the vrings locations at some point.

Thanks,
Ohad.

2012-08-09 20:35:11

by Sjur Brændeland

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

Hi Ohad,
>> I realize that we have the same issue with the virtio rings.
>> Are there any way we can assign the device address of the virtio rings
>> to the resource table in shared memory?
>
> It's a gap we have today, but it should definitely be fixed.

Ok, good.

Any thoughts on how to go about to fix this?
It does look a more comlex to solve than the carveout,
as the vrings are allocated in the first pass of fw parsing,
but fw is loaded to device memory in the second pass.

Regards,
Sjur

2012-08-10 15:31:01

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

Hi Sjur,

On Thu, Aug 9, 2012 at 11:35 PM, Sjur Br?ndeland <[email protected]> wrote:
> Any thoughts on how to go about to fix this?

The general direction I have in mind is to put the resource table in
its final location while we do the first pass of fw parsing.

This will solve all sort of open issues we have (or going to have soon):

1. dynamically-allocated address of the vrings can be communicated
2. vdev statuses can be communicated
3. virtio config space will finally become bi-directional as it should
4. dynamically probed rproc-to-rproc IPC could then take place

It's the real deal :)

The only problem with this approach is that the resource table isn't
reloaded throughout cycles of power up/down, and that is insecure.
We'll have to manually reload it somewhere after the rproc is powered
down (or before it is powered up again).

This change will break existing firmwares, but it looks required and inevitable.

Thanks,
Ohad.

2012-08-11 12:34:49

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)

On Fri, Aug 10, 2012 at 6:30 PM, Ohad Ben-Cohen <[email protected]> wrote:
> This will solve all sort of open issues we have (or going to have soon):
>
> 1. dynamically-allocated address of the vrings can be communicated
> 2. vdev statuses can be communicated
> 3. virtio config space will finally become bi-directional as it should
> 4. dynamically probed rproc-to-rproc IPC could then take place

and 5. let the remote processor know about the notifyids that we've
dynamically allocated.

2012-08-14 17:06:58

by Sjur BRENDELAND

[permalink] [raw]
Subject: RE: [RFC 4/4] remoteproc: Add driver for STE Modem

Hi Ohad,

>>> It seems to me that a char device will solve these issues: we can use
>>> ioctl to control the power, if the user crashes we'll know about it
>>> via the release handler, and if the remote processor crashes we can
>>> let the user know by sending it a notification for it to read via the fd.

How about skipping definition of ioctls and simply call rproc_boot() upon
open and rproc_shutdown() upon close, this is perhaps quirky, but simpler.
I'd also like to use a char-misc device, then we get the device
node with proper name automatically from existing udev rules.

>> In this case we need a way to make the virtio-drivers release the
>> virtio-queues in order to decrement the refcount, right?
...
>> So it seems that we need to force an unregistration of the virtio devices,
>> in order to make it release its queues.
...
> Yeah, no code is needed; something like this should do the trick:
>
> root@omap4430-panda:/sys/bus/virtio/drivers/virtio_console# echo
> virtio1 > unbind

Looking on this once more after vacation I prefer if the ste-rproc driver
could enforce shutdown if we use the char-device.
In our case user-space will detect modem crashes and power off
the device. So if user-space uses char-device to request power off,
it feels like kernel space should do everything possible to really
trigger the shutdown, including unbinding the virtio devices.
Depending on additional "echo .../virtio > unbind" seems like an
unnecessary extra dependency.


Alternatively, we could let user-space handle the power switch
for the modem directly. Remoteproc could simply notify user-space
(e.g. using gen-netlink) when rproc usage count has dropped to zero.
And this could trigger power-down. This would be more aligned with
our existing solution for power control.

Regards,
Sjur