remoteproc: maintain a generic child device for each rproc

For each registered rproc, maintain a generic remoteproc device whose
parent is the low level platform-specific device (commonly a pdev, but
it may certainly be any other type of device too).

With this in hand, the resulting device hierarchy might then look like:

omap-rproc.0
 |
 - remoteproc0  <---- new !
    |
    - virtio0
    |
    - virtio1
       |
       - rpmsg0
       |
       - rpmsg1
       |
       - rpmsg2

Where:
- omap-rproc.0 is the low level device that's bound to the
  driver which invokes rproc_register()
- remoteproc0 is the result of this patch, and will be added by the
  remoteproc framework when rproc_register() is invoked
- virtio0 and virtio1 are vdevs that are registered by remoteproc
  when it realizes that they are supported by the firmware
  of the physical remote processor represented by omap-rproc.0
- rpmsg0, rpmsg1 and rpmsg2 are rpmsg devices that represent rpmsg
  channels, and are registerd by the rpmsg bus when it gets notified
  about their existence

Technically, this patch:
- changes 'struct rproc' to contain this generic remoteproc.x device
- creates a new "remoteproc" type, to which this new generic remoteproc.x
  device belong to.
- adds a super simple enumeration method for the indices of the
  remoteproc.x devices
- updates all dev_* messaging to use the generic remoteproc.x device
  instead of the low level platform-specific device
- updates all dma_* allocations to use the parent of remoteproc.x (where
  the platform-specific memory pools, most commonly CMA, are to be found)

Adding this generic device has several merits:
- we can now add remoteproc runtime PM support simply by hooking onto the
  new "remoteproc" type
- all remoteproc log messages will now carry a common name prefix
  instead of having a platform-specific one
- having a device as part of the rproc struct makes it possible to simplify
  refcounting (see subsequent patch)

Thanks to Stephen Boyd <sboyd@codeaurora.org> for suggesting and
discussing these ideas in one of the remoteproc review threads and
to Fernando Guzman Lugo <fernando.lugo@ti.com> for trying them out
with the (upcoming) runtime PM support for remoteproc.

Cc: Fernando Guzman Lugo <fernando.lugo@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>

1 files changed, 92 insertions, 43 deletions

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 288d4175bbf6..25f937843836 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -35,6 +35,7 @@
 #include <linux/debugfs.h>
 #include <linux/remoteproc.h>
 #include <linux/iommu.h>
+#include <linux/idr.h>
 #include <linux/klist.h>
 #include <linux/elf.h>
 #include <linux/virtio_ids.h>
@@ -66,6 +67,9 @@ typedef int (*rproc_handle_resources_t)(struct rproc *rproc,
                                 struct resource_table *table, int len);
 typedef int (*rproc_handle_resource_t)(struct rproc *rproc, void *, int avail);
 
+/* Unique indices for remoteproc devices */
+static DEFINE_IDA(rproc_dev_index);
+
 /*
  * This is the IOMMU fault handler we register with the IOMMU API
  * (when relevant; not all remote processors access memory through
@@ -92,7 +96,7 @@ static int rproc_iommu_fault(struct iommu_domain *domain, struct device *dev,
 static int rproc_enable_iommu(struct rproc *rproc)
 {
         struct iommu_domain *domain;
-        struct device *dev = rproc->dev;
+        struct device *dev = rproc->dev.parent;
         int ret;
 
         /*
@@ -137,7 +141,7 @@ free_domain:
 static void rproc_disable_iommu(struct rproc *rproc)
 {
         struct iommu_domain *domain = rproc->domain;
-        struct device *dev = rproc->dev;
+        struct device *dev = rproc->dev.parent;
 
         if (!domain)
                 return;
@@ -217,7 +221,7 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 static int
 rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct elf32_hdr *ehdr;
         struct elf32_phdr *phdr;
         int i, ret = 0;
@@ -282,7 +286,7 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)
 int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
 {
         struct rproc *rproc = rvdev->rproc;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct rproc_vring *rvring = &rvdev->vring[i];
         dma_addr_t dma;
         void *va;
@@ -301,9 +305,9 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
          * this call will also configure the IOMMU for us
          * TODO: let the rproc know the da of this vring
          */
-        va = dma_alloc_coherent(dev, size, &dma, GFP_KERNEL);
+        va = dma_alloc_coherent(dev->parent, size, &dma, GFP_KERNEL);
         if (!va) {
-                dev_err(dev, "dma_alloc_coherent failed\n");
+                dev_err(dev->parent, "dma_alloc_coherent failed\n");
                 return -EINVAL;
         }
 
@@ -316,7 +320,7 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
         ret = idr_get_new(&rproc->notifyids, rvring, &notifyid);
         if (ret) {
                 dev_err(dev, "idr_get_new failed: %d\n", ret);
-                dma_free_coherent(dev, size, va, dma);
+                dma_free_coherent(dev->parent, size, va, dma);
                 return ret;
         }
 
@@ -334,7 +338,7 @@ static int
 rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
 {
         struct rproc *rproc = rvdev->rproc;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct fw_rsc_vdev_vring *vring = &rsc->vring[i];
         struct rproc_vring *rvring = &rvdev->vring[i];
 
@@ -366,7 +370,7 @@ void rproc_free_vring(struct rproc_vring *rvring)
         int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
         struct rproc *rproc = rvring->rvdev->rproc;
 
-        dma_free_coherent(rproc->dev, size, rvring->va, rvring->dma);
+        dma_free_coherent(rproc->dev.parent, size, rvring->va, rvring->dma);
         idr_remove(&rproc->notifyids, rvring->notifyid);
 }
 
@@ -400,14 +404,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
 static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
                                                                 int avail)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct rproc_vdev *rvdev;
         int i, ret;
 
         /* make sure resource isn't truncated */
         if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct fw_rsc_vdev_vring)
                         + rsc->config_len > avail) {
-                dev_err(rproc->dev, "vdev rsc is truncated\n");
+                dev_err(dev, "vdev rsc is truncated\n");
                 return -EINVAL;
         }
 
@@ -476,12 +480,12 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
                                                                 int avail)
 {
         struct rproc_mem_entry *trace;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         void *ptr;
         char name[15];
 
         if (sizeof(*rsc) > avail) {
-                dev_err(rproc->dev, "trace rsc is truncated\n");
+                dev_err(dev, "trace rsc is truncated\n");
                 return -EINVAL;
         }
 
@@ -558,6 +562,7 @@ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc,
                                                                 int avail)
 {
         struct rproc_mem_entry *mapping;
+        struct device *dev = &rproc->dev;
         int ret;
 
         /* no point in handling this resource without a valid iommu domain */
@@ -565,25 +570,25 @@ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc,
                 return -EINVAL;
 
         if (sizeof(*rsc) > avail) {
-                dev_err(rproc->dev, "devmem rsc is truncated\n");
+                dev_err(dev, "devmem rsc is truncated\n");
                 return -EINVAL;
         }
 
         /* make sure reserved bytes are zeroes */
         if (rsc->reserved) {
-                dev_err(rproc->dev, "devmem rsc has non zero reserved bytes\n");
+                dev_err(dev, "devmem rsc has non zero reserved bytes\n");
                 return -EINVAL;
         }
 
         mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
         if (!mapping) {
-                dev_err(rproc->dev, "kzalloc mapping failed\n");
+                dev_err(dev, "kzalloc mapping failed\n");
                 return -ENOMEM;
         }
 
         ret = iommu_map(rproc->domain, rsc->da, rsc->pa, rsc->len, rsc->flags);
         if (ret) {
-                dev_err(rproc->dev, "failed to map devmem: %d\n", ret);
+                dev_err(dev, "failed to map devmem: %d\n", ret);
                 goto out;
         }
 
@@ -598,7 +603,7 @@ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc,
         mapping->len = rsc->len;
         list_add_tail(&mapping->node, &rproc->mappings);
 
-        dev_dbg(rproc->dev, "mapped devmem pa 0x%x, da 0x%x, len 0x%x\n",
+        dev_dbg(dev, "mapped devmem pa 0x%x, da 0x%x, len 0x%x\n",
                                         rsc->pa, rsc->da, rsc->len);
 
         return 0;
@@ -630,13 +635,13 @@ static int rproc_handle_carveout(struct rproc *rproc,
                                 struct fw_rsc_carveout *rsc, int avail)
 {
         struct rproc_mem_entry *carveout, *mapping;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         dma_addr_t dma;
         void *va;
         int ret;
 
         if (sizeof(*rsc) > avail) {
-                dev_err(rproc->dev, "carveout rsc is truncated\n");
+                dev_err(dev, "carveout rsc is truncated\n");
                 return -EINVAL;
         }
 
@@ -662,9 +667,9 @@ static int rproc_handle_carveout(struct rproc *rproc,
                 goto free_mapping;
         }
 
-        va = dma_alloc_coherent(dev, rsc->len, &dma, GFP_KERNEL);
+        va = dma_alloc_coherent(dev->parent, rsc->len, &dma, GFP_KERNEL);
         if (!va) {
-                dev_err(dev, "failed to dma alloc carveout: %d\n", rsc->len);
+                dev_err(dev->parent, "dma_alloc_coherent err: %d\n", rsc->len);
                 ret = -ENOMEM;
                 goto free_carv;
         }
@@ -735,7 +740,7 @@ static int rproc_handle_carveout(struct rproc *rproc,
         return 0;
 
 dma_free:
-        dma_free_coherent(dev, rsc->len, va, dma);
+        dma_free_coherent(dev->parent, rsc->len, va, dma);
 free_carv:
         kfree(carveout);
 free_mapping:
@@ -758,7 +763,7 @@ static rproc_handle_resource_t rproc_handle_rsc[] = {
 static int
 rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         rproc_handle_resource_t handler;
         int ret = 0, i;
 
@@ -797,7 +802,7 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len
 static int
 rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int len)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         int ret = 0, i;
 
         for (i = 0; i < table->num; i++) {
@@ -850,7 +855,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len,
         struct elf32_hdr *ehdr;
         struct elf32_shdr *shdr;
         const char *name_table;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct resource_table *table = NULL;
         int i;
 
@@ -916,7 +921,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len,
 static void rproc_resource_cleanup(struct rproc *rproc)
 {
         struct rproc_mem_entry *entry, *tmp;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
 
         /* clean up debugfs trace entries */
         list_for_each_entry_safe(entry, tmp, &rproc->traces, node) {
@@ -928,7 +933,7 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 
         /* clean up carveout allocations */
         list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
-                dma_free_coherent(dev, entry->len, entry->va, entry->dma);
+                dma_free_coherent(dev->parent, entry->len, entry->va, entry->dma);
                 list_del(&entry->node);
                 kfree(entry);
         }
@@ -953,7 +958,7 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
 {
         const char *name = rproc->firmware;
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         struct elf32_hdr *ehdr;
         char class;
 
@@ -1014,7 +1019,7 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
  */
 static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         const char *name = rproc->firmware;
         struct elf32_hdr *ehdr;
         struct resource_table *table;
@@ -1138,7 +1143,7 @@ int rproc_boot(struct rproc *rproc)
                 return -EINVAL;
         }
 
-        dev = rproc->dev;
+        dev = &rproc->dev;
 
         ret = mutex_lock_interruptible(&rproc->lock);
         if (ret) {
@@ -1154,7 +1159,7 @@ int rproc_boot(struct rproc *rproc)
         }
 
         /* prevent underlying implementation from being removed */
-        if (!try_module_get(dev->driver->owner)) {
+        if (!try_module_get(dev->parent->driver->owner)) {
                 dev_err(dev, "%s: can't get owner\n", __func__);
                 ret = -EINVAL;
                 goto unlock_mutex;
@@ -1181,7 +1186,7 @@ int rproc_boot(struct rproc *rproc)
 
 downref_rproc:
         if (ret) {
-                module_put(dev->driver->owner);
+                module_put(dev->parent->driver->owner);
                 atomic_dec(&rproc->power);
         }
 unlock_mutex:
@@ -1215,7 +1220,7 @@ EXPORT_SYMBOL(rproc_boot);
  */
 void rproc_shutdown(struct rproc *rproc)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         int ret;
 
         ret = mutex_lock_interruptible(&rproc->lock);
@@ -1248,7 +1253,7 @@ void rproc_shutdown(struct rproc *rproc)
 out:
         mutex_unlock(&rproc->lock);
         if (!ret)
-                module_put(dev->driver->owner);
+                module_put(dev->parent->driver->owner);
 }
 EXPORT_SYMBOL(rproc_shutdown);
 
@@ -1271,7 +1276,7 @@ void rproc_release(struct kref *kref)
 {
         struct rproc *rproc = container_of(kref, struct rproc, refcount);
 
-        dev_info(rproc->dev, "removing %s\n", rproc->name);
+        dev_info(&rproc->dev, "removing %s\n", rproc->name);
 
         rproc_delete_debug_dir(rproc);
 
@@ -1403,13 +1408,17 @@ EXPORT_SYMBOL(rproc_put);
  */
 int rproc_register(struct rproc *rproc)
 {
-        struct device *dev = rproc->dev;
+        struct device *dev = &rproc->dev;
         int ret = 0;
 
+        ret = device_add(dev);
+        if (ret < 0)
+                return ret;
+
         /* expose to rproc_get_by_name users */
         klist_add_tail(&rproc->node, &rprocs);
 
-        dev_info(rproc->dev, "%s is available\n", rproc->name);
+        dev_info(dev, "%s is available\n", rproc->name);
 
         dev_info(dev, "Note: remoteproc is still under development and considered experimental.\n");
         dev_info(dev, "THE BINARY FORMAT IS NOT YET FINALIZED, and backward compatibility isn't yet guaranteed.\n");
@@ -1442,6 +1451,33 @@ int rproc_register(struct rproc *rproc)
 EXPORT_SYMBOL(rproc_register);
 
 /**
+ * rproc_type_release() - release a remote processor instance
+ * @dev: the rproc's device
+ *
+ * This function should _never_ be called directly.
+ *
+ * It will be called by the driver core when no one holds a valid pointer
+ * to @dev anymore.
+ */
+static void rproc_type_release(struct device *dev)
+{
+        struct rproc *rproc = container_of(dev, struct rproc, dev);
+
+        idr_remove_all(&rproc->notifyids);
+        idr_destroy(&rproc->notifyids);
+
+        if (rproc->index >= 0)
+                ida_simple_remove(&rproc_dev_index, rproc->index);
+
+        kfree(rproc);
+}
+
+static struct device_type rproc_type = {
+        .name           = "remoteproc",
+        .release        = rproc_type_release,
+};
+
+/**
  * rproc_alloc() - allocate a remote processor handle
  * @dev: the underlying device
  * @name: name of this remote processor
@@ -1479,12 +1515,25 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
                 return NULL;
         }
 
-        rproc->dev = dev;
         rproc->name = name;
         rproc->ops = ops;
         rproc->firmware = firmware;
         rproc->priv = &rproc[1];
 
+        device_initialize(&rproc->dev);
+        rproc->dev.parent = dev;
+        rproc->dev.type = &rproc_type;
+
+        /* Assign a unique device index and name */
+        rproc->index = ida_simple_get(&rproc_dev_index, 0, 0, GFP_KERNEL);
+        if (rproc->index < 0) {
+                dev_err(dev, "ida_simple_get failed: %d\n", rproc->index);
+                put_device(&rproc->dev);
+                return NULL;
+        }
+
+        dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
+
         atomic_set(&rproc->power, 0);
 
         kref_init(&rproc->refcount);
@@ -1516,10 +1565,7 @@ EXPORT_SYMBOL(rproc_alloc);
  */
 void rproc_free(struct rproc *rproc)
 {
-        idr_remove_all(&rproc->notifyids);
-        idr_destroy(&rproc->notifyids);
-
-        kfree(rproc);
+        put_device(&rproc->dev);
 }
 EXPORT_SYMBOL(rproc_free);
 
@@ -1560,6 +1606,8 @@ int rproc_unregister(struct rproc *rproc)
         /* the rproc is downref'ed as soon as it's removed from the klist */
         klist_del(&rproc->node);
 
+        device_del(&rproc->dev);
+
         /* the rproc will only be released after its refcount drops to zero */
         kref_put(&rproc->refcount, rproc_release);
 
@@ -1570,6 +1618,7 @@ EXPORT_SYMBOL(rproc_unregister);
 static int __init remoteproc_init(void)
 {
         rproc_init_debugfs();
+
         return 0;
 }
 module_init(remoteproc_init);