From 6db20ea8d85064175c7ef594c433c6c2e6bbab83 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Thu, 17 May 2012 14:23:59 +0300 Subject: remoteproc: allocate vrings on demand, free when not needed Dynamically allocate the vrings' DMA when the remote processor is about to be powered on (i.e. when ->find_vqs() is invoked), and release them as soon as it is powered off (i.e. when ->del_vqs() is invoked). The obvious and immediate benefit is better memory utilization, since memory for the vrings is now only allocated when the relevant remote processor is used. Additionally, this approach also makes recovery of a (crashing) remote processor easier: one just needs to remove the relevant vdevs, and the entire vrings cleanup takes place automagically. Tested-by: Fernando Guzman Lugo Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 109 +++++++++++++++---------------- drivers/remoteproc/remoteproc_internal.h | 2 + drivers/remoteproc/remoteproc_virtio.c | 13 +++- 3 files changed, 67 insertions(+), 57 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 8ea7bcc..288d417 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -279,34 +279,17 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len) return ret; } -static int -__rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i) +int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) { struct rproc *rproc = rvdev->rproc; struct device *dev = rproc->dev; - struct fw_rsc_vdev_vring *vring = &rsc->vring[i]; + struct rproc_vring *rvring = &rvdev->vring[i]; dma_addr_t dma; void *va; int ret, size, notifyid; - dev_dbg(dev, "vdev rsc: vring%d: da %x, qsz %d, align %d\n", - i, vring->da, vring->num, vring->align); - - /* make sure reserved bytes are zeroes */ - if (vring->reserved) { - dev_err(dev, "vring rsc has non zero reserved bytes\n"); - return -EINVAL; - } - - /* verify queue size and vring alignment are sane */ - if (!vring->num || !vring->align) { - dev_err(dev, "invalid qsz (%d) or alignment (%d)\n", - vring->num, vring->align); - return -EINVAL; - } - /* actual size of vring (in bytes) */ - size = PAGE_ALIGN(vring_size(vring->num, vring->align)); + size = PAGE_ALIGN(vring_size(rvring->len, rvring->align)); if (!idr_pre_get(&rproc->notifyids, GFP_KERNEL)) { dev_err(dev, "idr_pre_get failed\n"); @@ -316,6 +299,7 @@ __rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i) /* * Allocate non-cacheable memory for the vring. In the future * 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); if (!va) { @@ -323,44 +307,67 @@ __rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i) return -EINVAL; } - /* assign an rproc-wide unique index for this vring */ - /* TODO: assign a notifyid for rvdev updates as well */ - ret = idr_get_new(&rproc->notifyids, &rvdev->vring[i], ¬ifyid); + /* + * Assign an rproc-wide unique index for this vring + * TODO: assign a notifyid for rvdev updates as well + * TODO: let the rproc know the notifyid of this vring + * TODO: support predefined notifyids (via resource table) + */ + ret = idr_get_new(&rproc->notifyids, rvring, ¬ifyid); if (ret) { dev_err(dev, "idr_get_new failed: %d\n", ret); dma_free_coherent(dev, size, va, dma); return ret; } - /* let the rproc know the da and notifyid of this vring */ - /* TODO: expose this to remote processor */ - vring->da = dma; - vring->notifyid = notifyid; - dev_dbg(dev, "vring%d: va %p dma %x size %x idr %d\n", i, va, dma, size, notifyid); - rvdev->vring[i].len = vring->num; - rvdev->vring[i].align = vring->align; - rvdev->vring[i].va = va; - rvdev->vring[i].dma = dma; - rvdev->vring[i].notifyid = notifyid; - rvdev->vring[i].rvdev = rvdev; + rvring->va = va; + rvring->dma = dma; + rvring->notifyid = notifyid; return 0; } -static void __rproc_free_vrings(struct rproc_vdev *rvdev, int i) +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 fw_rsc_vdev_vring *vring = &rsc->vring[i]; + struct rproc_vring *rvring = &rvdev->vring[i]; - for (i--; i >= 0; i--) { - struct rproc_vring *rvring = &rvdev->vring[i]; - int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align)); + dev_dbg(dev, "vdev rsc: vring%d: da %x, qsz %d, align %d\n", + i, vring->da, vring->num, vring->align); + + /* make sure reserved bytes are zeroes */ + if (vring->reserved) { + dev_err(dev, "vring rsc has non zero reserved bytes\n"); + return -EINVAL; + } - dma_free_coherent(rproc->dev, size, rvring->va, rvring->dma); - idr_remove(&rproc->notifyids, rvring->notifyid); + /* verify queue size and vring alignment are sane */ + if (!vring->num || !vring->align) { + dev_err(dev, "invalid qsz (%d) or alignment (%d)\n", + vring->num, vring->align); + return -EINVAL; } + + rvring->len = vring->num; + rvring->align = vring->align; + rvring->rvdev = rvdev; + + return 0; +} + +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); + idr_remove(&rproc->notifyids, rvring->notifyid); } /** @@ -425,11 +432,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, rvdev->rproc = rproc; - /* allocate the vrings */ + /* parse the vrings */ for (i = 0; i < rsc->num_of_vrings; i++) { - ret = __rproc_handle_vring(rvdev, rsc, i); + ret = rproc_parse_vring(rvdev, rsc, i); if (ret) - goto free_vrings; + goto free_rvdev; } /* remember the device features */ @@ -440,12 +447,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, /* it is now safe to add the virtio device */ ret = rproc_add_virtio_dev(rvdev, rsc->id); if (ret) - goto free_vrings; + goto free_rvdev; return 0; -free_vrings: - __rproc_free_vrings(rvdev, i); +free_rvdev: kfree(rvdev); return ret; } @@ -1264,18 +1270,11 @@ EXPORT_SYMBOL(rproc_shutdown); void rproc_release(struct kref *kref) { struct rproc *rproc = container_of(kref, struct rproc, refcount); - struct rproc_vdev *rvdev, *rvtmp; dev_info(rproc->dev, "removing %s\n", rproc->name); rproc_delete_debug_dir(rproc); - /* clean up remote vdev entries */ - list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node) { - __rproc_free_vrings(rvdev, RVDEV_NUM_VRINGS); - list_del(&rvdev->node); - } - /* * At this point no one holds a reference to rproc anymore, * so we can directly unroll rproc_alloc() @@ -1546,7 +1545,7 @@ EXPORT_SYMBOL(rproc_free); */ int rproc_unregister(struct rproc *rproc) { - struct rproc_vdev *rvdev; + struct rproc_vdev *rvdev, *tmp; if (!rproc) return -EINVAL; @@ -1555,7 +1554,7 @@ int rproc_unregister(struct rproc *rproc) wait_for_completion(&rproc->firmware_loading_complete); /* clean up remote vdev entries */ - list_for_each_entry(rvdev, &rproc->rvdevs, node) + list_for_each_entry_safe(rvdev, tmp, &rproc->rvdevs, node) rproc_remove_virtio_dev(rvdev); /* the rproc is downref'ed as soon as it's removed from the klist */ diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index 9f336d6..f4957cf 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -41,4 +41,6 @@ void rproc_create_debug_dir(struct rproc *rproc); void rproc_init_debugfs(void); void rproc_exit_debugfs(void); +void rproc_free_vring(struct rproc_vring *rvring); +int rproc_alloc_vring(struct rproc_vdev *rvdev, int i); #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index ecf6121..26a7144 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -77,14 +77,17 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, struct rproc_vring *rvring; struct virtqueue *vq; void *addr; - int len, size; + int len, size, ret; /* we're temporarily limited to two virtqueues per rvdev */ if (id >= ARRAY_SIZE(rvdev->vring)) return ERR_PTR(-EINVAL); - rvring = &rvdev->vring[id]; + ret = rproc_alloc_vring(rvdev, id); + if (ret) + return ERR_PTR(ret); + rvring = &rvdev->vring[id]; addr = rvring->va; len = rvring->len; @@ -103,6 +106,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, rproc_virtio_notify, callback, name); if (!vq) { dev_err(rproc->dev, "vring_new_virtqueue %s failed\n", name); + rproc_free_vring(rvring); return ERR_PTR(-ENOMEM); } @@ -125,6 +129,7 @@ static void rproc_virtio_del_vqs(struct virtio_device *vdev) rvring = vq->priv; rvring->vq = NULL; vring_del_virtqueue(vq); + rproc_free_vring(rvring); } } @@ -228,8 +233,12 @@ static struct virtio_config_ops rproc_virtio_config_ops = { static void rproc_vdev_release(struct device *dev) { struct virtio_device *vdev = dev_to_virtio(dev); + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); struct rproc *rproc = vdev_to_rproc(vdev); + list_del(&rvdev->node); + kfree(rvdev); + kref_put(&rproc->refcount, rproc_release); } -- cgit v1.1 From b5ab5e24e960b9f780a4cc96815cfd4b0d412720 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Wed, 30 May 2012 22:01:25 +0300 Subject: 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 for suggesting and discussing these ideas in one of the remoteproc review threads and to Fernando Guzman Lugo for trying them out with the (upcoming) runtime PM support for remoteproc. Cc: Fernando Guzman Lugo Reviewed-by: Stephen Boyd Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/omap_remoteproc.c | 17 ++-- drivers/remoteproc/remoteproc_core.c | 135 ++++++++++++++++++++++---------- drivers/remoteproc/remoteproc_debugfs.c | 4 +- drivers/remoteproc/remoteproc_virtio.c | 13 +-- drivers/rpmsg/virtio_rpmsg_bus.c | 3 +- include/linux/remoteproc.h | 6 +- 6 files changed, 117 insertions(+), 61 deletions(-) diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c index 69425c4..b5e6d29 100644 --- a/drivers/remoteproc/omap_remoteproc.c +++ b/drivers/remoteproc/omap_remoteproc.c @@ -66,7 +66,7 @@ static int omap_rproc_mbox_callback(struct notifier_block *this, { mbox_msg_t msg = (mbox_msg_t) data; struct omap_rproc *oproc = container_of(this, struct omap_rproc, nb); - struct device *dev = oproc->rproc->dev; + struct device *dev = oproc->rproc->dev.parent; const char *name = oproc->rproc->name; dev_dbg(dev, "mbox msg: 0x%x\n", msg); @@ -92,12 +92,13 @@ static int omap_rproc_mbox_callback(struct notifier_block *this, static void omap_rproc_kick(struct rproc *rproc, int vqid) { struct omap_rproc *oproc = rproc->priv; + struct device *dev = rproc->dev.parent; int ret; /* send the index of the triggered virtqueue in the mailbox payload */ ret = omap_mbox_msg_send(oproc->mbox, vqid); if (ret) - dev_err(rproc->dev, "omap_mbox_msg_send failed: %d\n", ret); + dev_err(dev, "omap_mbox_msg_send failed: %d\n", ret); } /* @@ -110,7 +111,8 @@ static void omap_rproc_kick(struct rproc *rproc, int vqid) static int omap_rproc_start(struct rproc *rproc) { struct omap_rproc *oproc = rproc->priv; - struct platform_device *pdev = to_platform_device(rproc->dev); + struct device *dev = rproc->dev.parent; + struct platform_device *pdev = to_platform_device(dev); struct omap_rproc_pdata *pdata = pdev->dev.platform_data; int ret; @@ -120,7 +122,7 @@ static int omap_rproc_start(struct rproc *rproc) oproc->mbox = omap_mbox_get(pdata->mbox_name, &oproc->nb); if (IS_ERR(oproc->mbox)) { ret = PTR_ERR(oproc->mbox); - dev_err(rproc->dev, "omap_mbox_get failed: %d\n", ret); + dev_err(dev, "omap_mbox_get failed: %d\n", ret); return ret; } @@ -133,13 +135,13 @@ static int omap_rproc_start(struct rproc *rproc) */ ret = omap_mbox_msg_send(oproc->mbox, RP_MBOX_ECHO_REQUEST); if (ret) { - dev_err(rproc->dev, "omap_mbox_get failed: %d\n", ret); + dev_err(dev, "omap_mbox_get failed: %d\n", ret); goto put_mbox; } ret = pdata->device_enable(pdev); if (ret) { - dev_err(rproc->dev, "omap_device_enable failed: %d\n", ret); + dev_err(dev, "omap_device_enable failed: %d\n", ret); goto put_mbox; } @@ -153,7 +155,8 @@ put_mbox: /* power off the remote processor */ static int omap_rproc_stop(struct rproc *rproc) { - struct platform_device *pdev = to_platform_device(rproc->dev); + struct device *dev = rproc->dev.parent; + struct platform_device *pdev = to_platform_device(dev); struct omap_rproc_pdata *pdata = pdev->dev.platform_data; struct omap_rproc *oproc = rproc->priv; int ret; diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 288d417..25f9378 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -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, ¬ifyid); 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); diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 85d31a6..0383385 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -124,7 +124,7 @@ struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc, tfile = debugfs_create_file(name, 0400, rproc->dbg_dir, trace, &trace_rproc_ops); if (!tfile) { - dev_err(rproc->dev, "failed to create debugfs trace entry\n"); + dev_err(&rproc->dev, "failed to create debugfs trace entry\n"); return NULL; } @@ -141,7 +141,7 @@ void rproc_delete_debug_dir(struct rproc *rproc) void rproc_create_debug_dir(struct rproc *rproc) { - struct device *dev = rproc->dev; + struct device *dev = &rproc->dev; if (!rproc_dbg) return; diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index 26a7144..b662183 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -36,7 +36,7 @@ static void rproc_virtio_notify(struct virtqueue *vq) struct rproc *rproc = rvring->rvdev->rproc; int notifyid = rvring->notifyid; - dev_dbg(rproc->dev, "kicking vq index: %d\n", notifyid); + dev_dbg(&rproc->dev, "kicking vq index: %d\n", notifyid); rproc->ops->kick(rproc, notifyid); } @@ -57,7 +57,7 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid) { struct rproc_vring *rvring; - dev_dbg(rproc->dev, "vq index %d is interrupted\n", notifyid); + dev_dbg(&rproc->dev, "vq index %d is interrupted\n", notifyid); rvring = idr_find(&rproc->notifyids, notifyid); if (!rvring || !rvring->vq) @@ -74,6 +74,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); struct rproc *rproc = vdev_to_rproc(vdev); + struct device *dev = &rproc->dev; struct rproc_vring *rvring; struct virtqueue *vq; void *addr; @@ -95,7 +96,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, size = vring_size(len, rvring->align); memset(addr, 0, size); - dev_dbg(rproc->dev, "vring%d: va %p qsz %d notifyid %d\n", + dev_dbg(dev, "vring%d: va %p qsz %d notifyid %d\n", id, addr, len, rvring->notifyid); /* @@ -105,7 +106,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, vq = vring_new_virtqueue(len, rvring->align, vdev, false, addr, rproc_virtio_notify, callback, name); if (!vq) { - dev_err(rproc->dev, "vring_new_virtqueue %s failed\n", name); + dev_err(dev, "vring_new_virtqueue %s failed\n", name); rproc_free_vring(rvring); return ERR_PTR(-ENOMEM); } @@ -152,7 +153,7 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, /* now that the vqs are all set, boot the remote processor */ ret = rproc_boot(rproc); if (ret) { - dev_err(rproc->dev, "rproc_boot() failed %d\n", ret); + dev_err(&rproc->dev, "rproc_boot() failed %d\n", ret); goto error; } @@ -254,7 +255,7 @@ static void rproc_vdev_release(struct device *dev) int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id) { struct rproc *rproc = rvdev->rproc; - struct device *dev = rproc->dev; + struct device *dev = &rproc->dev; struct virtio_device *vdev = &rvdev->vdev; int ret; diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index 75506ec..691e52e 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -911,7 +911,8 @@ static int rpmsg_probe(struct virtio_device *vdev) vrp->svq = vqs[1]; /* allocate coherent memory for the buffers */ - bufs_va = dma_alloc_coherent(vdev->dev.parent, RPMSG_TOTAL_BUF_SPACE, + bufs_va = dma_alloc_coherent(vdev->dev.parent->parent, + RPMSG_TOTAL_BUF_SPACE, &vrp->bufs_dma, GFP_KERNEL); if (!bufs_va) goto vqs_del; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index f1ffabb..7f806dc 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -369,7 +369,7 @@ enum rproc_state { * @firmware: name of firmware file to be loaded * @priv: private data which belongs to the platform-specific rproc module * @ops: platform-specific start/stop rproc handlers - * @dev: underlying device + * @dev: virtual device for refcounting and common remoteproc behavior * @refcount: refcount of users that have a valid pointer to this rproc * @power: refcount of users who need this rproc powered up * @state: state of the device @@ -383,6 +383,7 @@ enum rproc_state { * @bootaddr: address of first instruction to boot rproc with (optional) * @rvdevs: list of remote virtio devices * @notifyids: idr for dynamically assigning rproc-wide unique notify ids + * @index: index of this rproc device */ struct rproc { struct klist_node node; @@ -391,7 +392,7 @@ struct rproc { const char *firmware; void *priv; const struct rproc_ops *ops; - struct device *dev; + struct device dev; struct kref refcount; atomic_t power; unsigned int state; @@ -405,6 +406,7 @@ struct rproc { u32 bootaddr; struct list_head rvdevs; struct idr notifyids; + int index; }; /* we currently support only two vrings per rvdev */ -- cgit v1.1 From 7183a2a799b81490354973117ecd810c23cdc668 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Wed, 30 May 2012 22:02:24 +0300 Subject: remoteproc: remove the now-redundant kref Now that every rproc instance contains a device, we don't need a kref anymore to maintain the refcount of the rproc instances: that's what device are good with! This patch removes the now-redundant kref, and switches to {get, put}_device instead of kref_{get, put}. We also don't need the kref's release function anymore, and instead, we just utilize the class's release handler (which is now responsible for all memory de-allocations). Cc: Stephen Boyd Cc: Fernando Guzman Lugo Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 50 ++++++++-------------------------- drivers/remoteproc/remoteproc_virtio.c | 8 +++--- include/linux/remoteproc.h | 3 -- 3 files changed, 15 insertions(+), 46 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 25f9378..aa713aa 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1257,42 +1257,12 @@ out: } EXPORT_SYMBOL(rproc_shutdown); -/** - * rproc_release() - completely deletes the existence of a remote processor - * @kref: the rproc's kref - * - * This function should _never_ be called directly. - * - * The only reasonable location to use it is as an argument when kref_put'ing - * @rproc's refcount. - * - * This way it will be called when no one holds a valid pointer to this @rproc - * anymore (and obviously after it is removed from the rprocs klist). - * - * Note: this function is not static because rproc_vdev_release() needs it when - * it decrements @rproc's refcount. - */ -void rproc_release(struct kref *kref) -{ - struct rproc *rproc = container_of(kref, struct rproc, refcount); - - dev_info(&rproc->dev, "removing %s\n", rproc->name); - - rproc_delete_debug_dir(rproc); - - /* - * At this point no one holds a reference to rproc anymore, - * so we can directly unroll rproc_alloc() - */ - rproc_free(rproc); -} - /* will be called when an rproc is added to the rprocs klist */ static void klist_rproc_get(struct klist_node *n) { struct rproc *rproc = container_of(n, struct rproc, node); - kref_get(&rproc->refcount); + get_device(&rproc->dev); } /* will be called when an rproc is removed from the rprocs klist */ @@ -1300,7 +1270,7 @@ static void klist_rproc_put(struct klist_node *n) { struct rproc *rproc = container_of(n, struct rproc, node); - kref_put(&rproc->refcount, rproc_release); + put_device(&rproc->dev); } static struct rproc *next_rproc(struct klist_iter *i) @@ -1342,7 +1312,7 @@ struct rproc *rproc_get_by_name(const char *name) klist_iter_init(&rprocs, &i); while ((rproc = next_rproc(&i)) != NULL) if (!strcmp(rproc->name, name)) { - kref_get(&rproc->refcount); + get_device(&rproc->dev); break; } klist_iter_exit(&i); @@ -1355,7 +1325,7 @@ struct rproc *rproc_get_by_name(const char *name) ret = rproc_boot(rproc); if (ret < 0) { - kref_put(&rproc->refcount, rproc_release); + put_device(&rproc->dev); return NULL; } @@ -1382,7 +1352,7 @@ void rproc_put(struct rproc *rproc) rproc_shutdown(rproc); /* downref rproc's refcount */ - kref_put(&rproc->refcount, rproc_release); + put_device(&rproc->dev); } EXPORT_SYMBOL(rproc_put); @@ -1463,6 +1433,10 @@ static void rproc_type_release(struct device *dev) { struct rproc *rproc = container_of(dev, struct rproc, dev); + dev_info(&rproc->dev, "releasing %s\n", rproc->name); + + rproc_delete_debug_dir(rproc); + idr_remove_all(&rproc->notifyids); idr_destroy(&rproc->notifyids); @@ -1536,8 +1510,6 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, atomic_set(&rproc->power, 0); - kref_init(&rproc->refcount); - mutex_init(&rproc->lock); idr_init(&rproc->notifyids); @@ -1608,8 +1580,8 @@ int rproc_unregister(struct rproc *rproc) device_del(&rproc->dev); - /* the rproc will only be released after its refcount drops to zero */ - kref_put(&rproc->refcount, rproc_release); + /* unroll rproc_alloc. TODO: we may want to let the users do that */ + put_device(&rproc->dev); return 0; } diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index b662183..3541b44 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -225,7 +225,7 @@ static struct virtio_config_ops rproc_virtio_config_ops = { /* * This function is called whenever vdev is released, and is responsible - * to decrement the remote processor's refcount taken when vdev was + * to decrement the remote processor's refcount which was taken when vdev was * added. * * Never call this function directly; it will be called by the driver @@ -240,7 +240,7 @@ static void rproc_vdev_release(struct device *dev) list_del(&rvdev->node); kfree(rvdev); - kref_put(&rproc->refcount, rproc_release); + put_device(&rproc->dev); } /** @@ -272,11 +272,11 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id) * Therefore we must increment the rproc refcount here, and decrement * it _only_ when the vdev is released. */ - kref_get(&rproc->refcount); + get_device(&rproc->dev); ret = register_virtio_device(vdev); if (ret) { - kref_put(&rproc->refcount, rproc_release); + put_device(&rproc->dev); dev_err(dev, "failed to register vdev: %d\n", ret); goto out; } diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 7f806dc..cbe8a51 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -36,7 +36,6 @@ #define REMOTEPROC_H #include -#include #include #include #include @@ -370,7 +369,6 @@ enum rproc_state { * @priv: private data which belongs to the platform-specific rproc module * @ops: platform-specific start/stop rproc handlers * @dev: virtual device for refcounting and common remoteproc behavior - * @refcount: refcount of users that have a valid pointer to this rproc * @power: refcount of users who need this rproc powered up * @state: state of the device * @lock: lock which protects concurrent manipulations of the rproc @@ -393,7 +391,6 @@ struct rproc { void *priv; const struct rproc_ops *ops; struct device dev; - struct kref refcount; atomic_t power; unsigned int state; struct mutex lock; -- cgit v1.1 From c6b5a27628faf6657b741d828a1462d832d0dbc5 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Mon, 2 Jul 2012 11:41:16 +0300 Subject: remoteproc: simplify unregister/free interfaces Simplify the unregister/free interfaces, and make them easier to understand and use, by moving to a symmetric and consistent alloc() -> register() -> unregister() -> free() flow. To create and register an rproc instance, one needed to invoke rproc_alloc() followed by rproc_register(). To unregister and free an rproc instance, one now needs to invoke rproc_unregister() followed by rproc_free(). Cc: Stephen Boyd Signed-off-by: Ohad Ben-Cohen --- Documentation/remoteproc.txt | 21 ++++++++------------- drivers/remoteproc/omap_remoteproc.c | 5 ++++- drivers/remoteproc/remoteproc_core.c | 25 ++++++++----------------- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt index 70a048c..ad6ded4 100644 --- a/Documentation/remoteproc.txt +++ b/Documentation/remoteproc.txt @@ -120,14 +120,14 @@ int dummy_rproc_example(struct rproc *my_rproc) On success, the new rproc is returned, and on failure, NULL. Note: _never_ directly deallocate @rproc, even if it was not registered - yet. Instead, if you just need to unroll rproc_alloc(), use rproc_free(). + yet. Instead, when you need to unroll rproc_alloc(), use rproc_free(). void rproc_free(struct rproc *rproc) - Free an rproc handle that was allocated by rproc_alloc. - This function should _only_ be used if @rproc was only allocated, - but not registered yet. - If @rproc was already successfully registered (by calling - rproc_register()), then use rproc_unregister() instead. + This function essentially unrolls rproc_alloc(), by decrementing the + rproc's refcount. It doesn't directly free rproc; that would happen + only if there are no other references to rproc and its refcount now + dropped to zero. int rproc_register(struct rproc *rproc) - Register @rproc with the remoteproc framework, after it has been @@ -143,19 +143,14 @@ int dummy_rproc_example(struct rproc *my_rproc) probed. int rproc_unregister(struct rproc *rproc) - - Unregister a remote processor, and decrement its refcount. - If its refcount drops to zero, then @rproc will be freed. If not, - it will be freed later once the last reference is dropped. - + - Unroll rproc_register(). This function should be called when the platform specific rproc implementation decides to remove the rproc device. it should _only_ be called if a previous invocation of rproc_register() has completed successfully. - After rproc_unregister() returns, @rproc is _not_ valid anymore and - it shouldn't be used. More specifically, don't call rproc_free() - or try to directly free @rproc after rproc_unregister() returns; - none of these are needed, and calling them is a bug. + After rproc_unregister() returns, @rproc is still valid, and its + last refcount should be decremented by calling rproc_free(). Returns 0 on success and -EINVAL if @rproc isn't valid. diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c index b5e6d29..4f2fe8f 100644 --- a/drivers/remoteproc/omap_remoteproc.c +++ b/drivers/remoteproc/omap_remoteproc.c @@ -214,7 +214,10 @@ static int __devexit omap_rproc_remove(struct platform_device *pdev) { struct rproc *rproc = platform_get_drvdata(pdev); - return rproc_unregister(rproc); + rproc_unregister(rproc); + rproc_free(rproc); + + return 0; } static struct platform_driver omap_rproc_driver = { diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index aa713aa..4a77dc1 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1472,7 +1472,7 @@ static struct device_type rproc_type = { * On success the new rproc is returned, and on failure, NULL. * * Note: _never_ directly deallocate @rproc, even if it was not registered - * yet. Instead, if you just need to unroll rproc_alloc(), use rproc_free(). + * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free(). */ struct rproc *rproc_alloc(struct device *dev, const char *name, const struct rproc_ops *ops, @@ -1526,14 +1526,13 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, EXPORT_SYMBOL(rproc_alloc); /** - * rproc_free() - free an rproc handle that was allocated by rproc_alloc + * rproc_free() - unroll rproc_alloc() * @rproc: the remote processor handle * - * This function should _only_ be used if @rproc was only allocated, - * but not registered yet. + * This function decrements the rproc dev refcount. * - * If @rproc was already successfully registered (by calling rproc_register()), - * then use rproc_unregister() instead. + * If no one holds any reference to rproc anymore, then its refcount would + * now drop to zero, and it would be freed. */ void rproc_free(struct rproc *rproc) { @@ -1545,19 +1544,14 @@ EXPORT_SYMBOL(rproc_free); * rproc_unregister() - unregister a remote processor * @rproc: rproc handle to unregister * - * Unregisters a remote processor, and decrements its refcount. - * If its refcount drops to zero, then @rproc will be freed. If not, - * it will be freed later once the last reference is dropped. - * * This function should be called when the platform specific rproc * implementation decides to remove the rproc device. it should * _only_ be called if a previous invocation of rproc_register() * has completed successfully. * - * After rproc_unregister() returns, @rproc is _not_ valid anymore and - * it shouldn't be used. More specifically, don't call rproc_free() - * or try to directly free @rproc after rproc_unregister() returns; - * none of these are needed, and calling them is a bug. + * After rproc_unregister() returns, @rproc isn't freed yet, because + * of the outstanding reference created by rproc_alloc. To decrement that + * one last refcount, one still needs to call rproc_free(). * * Returns 0 on success and -EINVAL if @rproc isn't valid. */ @@ -1580,9 +1574,6 @@ int rproc_unregister(struct rproc *rproc) device_del(&rproc->dev); - /* unroll rproc_alloc. TODO: we may want to let the users do that */ - put_device(&rproc->dev); - return 0; } EXPORT_SYMBOL(rproc_unregister); -- cgit v1.1 From 0e49b72c8c91f9ea65ae62ca3061f885aa06a6f6 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Sun, 1 Jul 2012 11:30:57 +0300 Subject: remoteproc: support non-iommu carveout assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Publish carveout addresses on non-iommu setups too. Reported-and-acked-by: Sjur Brændeland Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 4a77dc1..c85db12 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; -- cgit v1.1 From 40e575b1d0b34b38519d361c10bdf8e0c688957b Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Mon, 2 Jul 2012 20:20:53 +0300 Subject: remoteproc: remove the get_by_name/put API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove rproc_get_by_name() and rproc_put(), and the associated remoteproc infrastructure that supports it (i.e. klist and friends), because: 1. No one uses them 2. Using them is highly discouraged, and any potential user will be deeply scrutinized and encouraged to move. If a user, that absolutely can't live with the direct boot/shutdown model, does show up one day, then bringing this functionality back is going to be trivial. At this point though, keeping this functionality around is way too much of a maintenance burden. Cc: Sjur Brændeland Cc: Loic Pallardy Cc: Ludovic BARRE Cc: Michal Simek Cc: Fernando Guzman Lugo Cc: Suman Anna Cc: Mark Grosen Acked-by: Stephen Boyd Signed-off-by: Ohad Ben-Cohen --- Documentation/remoteproc.txt | 27 +------- drivers/remoteproc/remoteproc_core.c | 130 ----------------------------------- include/linux/remoteproc.h | 3 - 3 files changed, 1 insertion(+), 159 deletions(-) diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt index ad6ded4..f33c3bb 100644 --- a/Documentation/remoteproc.txt +++ b/Documentation/remoteproc.txt @@ -36,8 +36,7 @@ cost. Note: to use this function you should already have a valid rproc handle. There are several ways to achieve that cleanly (devres, pdata, the way remoteproc_rpmsg.c does this, or, if this becomes prevalent, we - might also consider using dev_archdata for this). See also - rproc_get_by_name() below. + might also consider using dev_archdata for this). void rproc_shutdown(struct rproc *rproc) - Power off a remote processor (previously booted with rproc_boot()). @@ -51,30 +50,6 @@ cost. which means that the @rproc handle stays valid even after rproc_shutdown() returns, and users can still use it with a subsequent rproc_boot(), if needed. - - don't call rproc_shutdown() to unroll rproc_get_by_name(), exactly - because rproc_shutdown() _does not_ decrement the refcount of @rproc. - To decrement the refcount of @rproc, use rproc_put() (but _only_ if - you acquired @rproc using rproc_get_by_name()). - - struct rproc *rproc_get_by_name(const char *name) - - Find an rproc handle using the remote processor's name, and then - boot it. If it's already powered on, then just immediately return - (successfully). Returns the rproc handle on success, and NULL on failure. - This function increments the remote processor's refcount, so always - use rproc_put() to decrement it back once rproc isn't needed anymore. - Note: currently rproc_get_by_name() and rproc_put() are not used anymore - by the rpmsg bus and its drivers. We need to scrutinize the use cases - that still need them, and see if we can migrate them to use the non - name-based boot/shutdown interface. - - void rproc_put(struct rproc *rproc) - - Decrement @rproc's power refcount and shut it down if it reaches zero - (essentially by just calling rproc_shutdown), and then decrement @rproc's - validity refcount too. - After this function returns, @rproc may _not_ be used anymore, and its - handle should be considered invalid. - This function should be called _iff_ the @rproc handle was grabbed by - calling rproc_get_by_name(). 3. Typical usage diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index c85db12..0c77c4f 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -44,25 +43,6 @@ #include "remoteproc_internal.h" -static void klist_rproc_get(struct klist_node *n); -static void klist_rproc_put(struct klist_node *n); - -/* - * klist of the available remote processors. - * - * We need this in order to support name-based lookups (needed by the - * rproc_get_by_name()). - * - * That said, we don't use rproc_get_by_name() at this point. - * The use cases that do require its existence should be - * scrutinized, and hopefully migrated to rproc_boot() using device-based - * binding. - * - * If/when this materializes, we could drop the klist (and the by_name - * API). - */ -static DEFINE_KLIST(rprocs, klist_rproc_get, klist_rproc_put); - 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); @@ -1217,10 +1197,6 @@ EXPORT_SYMBOL(rproc_boot); * which means that the @rproc handle stays valid even after rproc_shutdown() * returns, and users can still use it with a subsequent rproc_boot(), if * needed. - * - don't call rproc_shutdown() to unroll rproc_get_by_name(), exactly - * because rproc_shutdown() _does not_ decrement the refcount of @rproc. - * To decrement the refcount of @rproc, use rproc_put() (but _only_ if - * you acquired @rproc using rproc_get_by_name()). */ void rproc_shutdown(struct rproc *rproc) { @@ -1261,105 +1237,6 @@ out: } EXPORT_SYMBOL(rproc_shutdown); -/* will be called when an rproc is added to the rprocs klist */ -static void klist_rproc_get(struct klist_node *n) -{ - struct rproc *rproc = container_of(n, struct rproc, node); - - get_device(&rproc->dev); -} - -/* will be called when an rproc is removed from the rprocs klist */ -static void klist_rproc_put(struct klist_node *n) -{ - struct rproc *rproc = container_of(n, struct rproc, node); - - put_device(&rproc->dev); -} - -static struct rproc *next_rproc(struct klist_iter *i) -{ - struct klist_node *n; - - n = klist_next(i); - if (!n) - return NULL; - - return container_of(n, struct rproc, node); -} - -/** - * rproc_get_by_name() - find a remote processor by name and boot it - * @name: name of the remote processor - * - * Finds an rproc handle using the remote processor's name, and then - * boot it. If it's already powered on, then just immediately return - * (successfully). - * - * Returns the rproc handle on success, and NULL on failure. - * - * This function increments the remote processor's refcount, so always - * use rproc_put() to decrement it back once rproc isn't needed anymore. - * - * Note: currently this function (and its counterpart rproc_put()) are not - * being used. We need to scrutinize the use cases - * that still need them, and see if we can migrate them to use the non - * name-based boot/shutdown interface. - */ -struct rproc *rproc_get_by_name(const char *name) -{ - struct rproc *rproc; - struct klist_iter i; - int ret; - - /* find the remote processor, and upref its refcount */ - klist_iter_init(&rprocs, &i); - while ((rproc = next_rproc(&i)) != NULL) - if (!strcmp(rproc->name, name)) { - get_device(&rproc->dev); - break; - } - klist_iter_exit(&i); - - /* can't find this rproc ? */ - if (!rproc) { - pr_err("can't find remote processor %s\n", name); - return NULL; - } - - ret = rproc_boot(rproc); - if (ret < 0) { - put_device(&rproc->dev); - return NULL; - } - - return rproc; -} -EXPORT_SYMBOL(rproc_get_by_name); - -/** - * rproc_put() - decrement the refcount of a remote processor, and shut it down - * @rproc: the remote processor - * - * This function tries to shutdown @rproc, and it then decrements its - * refcount. - * - * After this function returns, @rproc may _not_ be used anymore, and its - * handle should be considered invalid. - * - * This function should be called _iff_ the @rproc handle was grabbed by - * calling rproc_get_by_name(). - */ -void rproc_put(struct rproc *rproc) -{ - /* try to power off the remote processor */ - rproc_shutdown(rproc); - - /* downref rproc's refcount */ - put_device(&rproc->dev); -} -EXPORT_SYMBOL(rproc_put); - /** * rproc_register() - register a remote processor * @rproc: the remote processor handle to register @@ -1389,9 +1266,6 @@ int rproc_register(struct rproc *rproc) if (ret < 0) return ret; - /* expose to rproc_get_by_name users */ - klist_add_tail(&rproc->node, &rprocs); - dev_info(dev, "%s is available\n", rproc->name); dev_info(dev, "Note: remoteproc is still under development and considered experimental.\n"); @@ -1417,7 +1291,6 @@ int rproc_register(struct rproc *rproc) if (ret < 0) { dev_err(dev, "request_firmware_nowait failed: %d\n", ret); complete_all(&rproc->firmware_loading_complete); - klist_remove(&rproc->node); } return ret; @@ -1573,9 +1446,6 @@ int rproc_unregister(struct rproc *rproc) list_for_each_entry_safe(rvdev, tmp, &rproc->rvdevs, node) rproc_remove_virtio_dev(rvdev); - /* the rproc is downref'ed as soon as it's removed from the klist */ - klist_del(&rproc->node); - device_del(&rproc->dev); return 0; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index cbe8a51..b88d6af 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -449,9 +449,6 @@ struct rproc_vdev { unsigned long gfeatures; }; -struct rproc *rproc_get_by_name(const char *name); -void rproc_put(struct rproc *rproc); - struct rproc *rproc_alloc(struct device *dev, const char *name, const struct rproc_ops *ops, const char *firmware, int len); -- cgit v1.1 From 160e7c840fe85836040c43e0058d5afced470c85 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Wed, 4 Jul 2012 16:25:06 +0300 Subject: remoteproc: adopt the driver core's alloc/add/del/put naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make remoteproc's API more intuitive for developers, we adopt the driver core's naming, i.e. alloc -> add -> del -> put. We'll also add register/unregister when their first user shows up. Otherwise - there's no functional change here. Suggested by Russell King . Cc: Russell King Cc: Fernando Guzman Lugo Cc: Sjur Brændeland Reviewed-by: Linus Walleij Acked-by: Stephen Boyd Signed-off-by: Ohad Ben-Cohen --- Documentation/remoteproc.txt | 18 +++++++++--------- drivers/remoteproc/omap_remoteproc.c | 8 ++++---- drivers/remoteproc/remoteproc_core.c | 32 ++++++++++++++++---------------- include/linux/remoteproc.h | 6 +++--- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt index f33c3bb..23a09b8 100644 --- a/Documentation/remoteproc.txt +++ b/Documentation/remoteproc.txt @@ -90,21 +90,21 @@ int dummy_rproc_example(struct rproc *my_rproc) This function should be used by rproc implementations during initialization of the remote processor. After creating an rproc handle using this function, and when ready, - implementations should then call rproc_register() to complete + implementations should then call rproc_add() to complete the registration of the remote processor. On success, the new rproc is returned, and on failure, NULL. Note: _never_ directly deallocate @rproc, even if it was not registered - yet. Instead, when you need to unroll rproc_alloc(), use rproc_free(). + yet. Instead, when you need to unroll rproc_alloc(), use rproc_put(). - void rproc_free(struct rproc *rproc) + void rproc_put(struct rproc *rproc) - Free an rproc handle that was allocated by rproc_alloc. This function essentially unrolls rproc_alloc(), by decrementing the rproc's refcount. It doesn't directly free rproc; that would happen only if there are no other references to rproc and its refcount now dropped to zero. - int rproc_register(struct rproc *rproc) + int rproc_add(struct rproc *rproc) - Register @rproc with the remoteproc framework, after it has been allocated with rproc_alloc(). This is called by the platform-specific rproc implementation, whenever @@ -117,15 +117,15 @@ int dummy_rproc_example(struct rproc *my_rproc) of registering this remote processor, additional virtio drivers might get probed. - int rproc_unregister(struct rproc *rproc) - - Unroll rproc_register(). + int rproc_del(struct rproc *rproc) + - Unroll rproc_add(). This function should be called when the platform specific rproc implementation decides to remove the rproc device. it should - _only_ be called if a previous invocation of rproc_register() + _only_ be called if a previous invocation of rproc_add() has completed successfully. - After rproc_unregister() returns, @rproc is still valid, and its - last refcount should be decremented by calling rproc_free(). + After rproc_del() returns, @rproc is still valid, and its + last refcount should be decremented by calling rproc_put(). Returns 0 on success and -EINVAL if @rproc isn't valid. diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c index 4f2fe8f..02bae3a 100644 --- a/drivers/remoteproc/omap_remoteproc.c +++ b/drivers/remoteproc/omap_remoteproc.c @@ -199,14 +199,14 @@ static int __devinit omap_rproc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, rproc); - ret = rproc_register(rproc); + ret = rproc_add(rproc); if (ret) goto free_rproc; return 0; free_rproc: - rproc_free(rproc); + rproc_put(rproc); return ret; } @@ -214,8 +214,8 @@ static int __devexit omap_rproc_remove(struct platform_device *pdev) { struct rproc *rproc = platform_get_drvdata(pdev); - rproc_unregister(rproc); - rproc_free(rproc); + rproc_del(rproc); + rproc_put(rproc); return 0; } diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 0c77c4f..25fd973 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1101,7 +1101,7 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context) out: release_firmware(fw); - /* allow rproc_unregister() contexts, if any, to proceed */ + /* allow rproc_del() contexts, if any, to proceed */ complete_all(&rproc->firmware_loading_complete); } @@ -1238,7 +1238,7 @@ out: EXPORT_SYMBOL(rproc_shutdown); /** - * rproc_register() - register a remote processor + * rproc_add() - register a remote processor * @rproc: the remote processor handle to register * * Registers @rproc with the remoteproc framework, after it has been @@ -1257,7 +1257,7 @@ EXPORT_SYMBOL(rproc_shutdown); * of registering this remote processor, additional virtio drivers might be * probed. */ -int rproc_register(struct rproc *rproc) +int rproc_add(struct rproc *rproc) { struct device *dev = &rproc->dev; int ret = 0; @@ -1274,7 +1274,7 @@ int rproc_register(struct rproc *rproc) /* create debugfs entries */ rproc_create_debug_dir(rproc); - /* rproc_unregister() calls must wait until async loader completes */ + /* rproc_del() calls must wait until async loader completes */ init_completion(&rproc->firmware_loading_complete); /* @@ -1295,7 +1295,7 @@ int rproc_register(struct rproc *rproc) return ret; } -EXPORT_SYMBOL(rproc_register); +EXPORT_SYMBOL(rproc_add); /** * rproc_type_release() - release a remote processor instance @@ -1343,13 +1343,13 @@ static struct device_type rproc_type = { * of the remote processor. * * After creating an rproc handle using this function, and when ready, - * implementations should then call rproc_register() to complete + * implementations should then call rproc_add() to complete * the registration of the remote processor. * * On success the new rproc is returned, and on failure, NULL. * * Note: _never_ directly deallocate @rproc, even if it was not registered - * yet. Instead, when you need to unroll rproc_alloc(), use rproc_free(). + * yet. Instead, when you need to unroll rproc_alloc(), use rproc_put(). */ struct rproc *rproc_alloc(struct device *dev, const char *name, const struct rproc_ops *ops, @@ -1403,7 +1403,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, EXPORT_SYMBOL(rproc_alloc); /** - * rproc_free() - unroll rproc_alloc() + * rproc_put() - unroll rproc_alloc() * @rproc: the remote processor handle * * This function decrements the rproc dev refcount. @@ -1411,28 +1411,28 @@ EXPORT_SYMBOL(rproc_alloc); * If no one holds any reference to rproc anymore, then its refcount would * now drop to zero, and it would be freed. */ -void rproc_free(struct rproc *rproc) +void rproc_put(struct rproc *rproc) { put_device(&rproc->dev); } -EXPORT_SYMBOL(rproc_free); +EXPORT_SYMBOL(rproc_put); /** - * rproc_unregister() - unregister a remote processor + * rproc_del() - unregister a remote processor * @rproc: rproc handle to unregister * * This function should be called when the platform specific rproc * implementation decides to remove the rproc device. it should - * _only_ be called if a previous invocation of rproc_register() + * _only_ be called if a previous invocation of rproc_add() * has completed successfully. * - * After rproc_unregister() returns, @rproc isn't freed yet, because + * After rproc_del() returns, @rproc isn't freed yet, because * of the outstanding reference created by rproc_alloc. To decrement that - * one last refcount, one still needs to call rproc_free(). + * one last refcount, one still needs to call rproc_put(). * * Returns 0 on success and -EINVAL if @rproc isn't valid. */ -int rproc_unregister(struct rproc *rproc) +int rproc_del(struct rproc *rproc) { struct rproc_vdev *rvdev, *tmp; @@ -1450,7 +1450,7 @@ int rproc_unregister(struct rproc *rproc) return 0; } -EXPORT_SYMBOL(rproc_unregister); +EXPORT_SYMBOL(rproc_del); static int __init remoteproc_init(void) { diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index b88d6af..eea3ac8 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -452,9 +452,9 @@ struct rproc_vdev { struct rproc *rproc_alloc(struct device *dev, const char *name, const struct rproc_ops *ops, const char *firmware, int len); -void rproc_free(struct rproc *rproc); -int rproc_register(struct rproc *rproc); -int rproc_unregister(struct rproc *rproc); +void rproc_put(struct rproc *rproc); +int rproc_add(struct rproc *rproc); +int rproc_del(struct rproc *rproc); int rproc_boot(struct rproc *rproc); void rproc_shutdown(struct rproc *rproc); -- cgit v1.1 From bd48498487735a01abc68edb76e3d86393e096e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= Date: Tue, 19 Jun 2012 09:55:34 +0300 Subject: remoteproc: Pass struct fw to load_segments and find_rsc_table. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare for introduction of custom firmware loaders by changing the functions rproc_find_rcs_table() and rproc_load_segments() to use struct firmware as parameter. When the custom loader framework is introduced all calls into the firmware specific function must use struct firmware as parameter. Signed-off-by: Sjur Brændeland Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 25fd973..e75424b 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -176,8 +176,7 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) /** * rproc_load_segments() - load firmware segments to memory * @rproc: remote processor which will be booted using these fw segments - * @elf_data: the content of the ELF firmware image - * @len: firmware size (in bytes) + * @fw: the ELF firmware image * * This function loads the firmware segments to memory, where the remote * processor expects them. @@ -199,12 +198,13 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) * supported, though. */ static int -rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len) +rproc_load_segments(struct rproc *rproc, const struct firmware *fw) { struct device *dev = &rproc->dev; struct elf32_hdr *ehdr; struct elf32_phdr *phdr; int i, ret = 0; + const u8 *elf_data = fw->data; ehdr = (struct elf32_hdr *)elf_data; phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff); @@ -230,9 +230,9 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len) break; } - if (offset + filesz > len) { + if (offset + filesz > fw->size) { dev_err(dev, "truncated fw: need 0x%x avail 0x%x\n", - offset + filesz, len); + offset + filesz, fw->size); ret = -EINVAL; break; } @@ -819,8 +819,7 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l /** * rproc_find_rsc_table() - find the resource table * @rproc: the rproc handle - * @elf_data: the content of the ELF firmware image - * @len: firmware size (in bytes) + * @fw: the ELF firmware image * @tablesz: place holder for providing back the table size * * This function finds the resource table inside the remote processor's @@ -833,7 +832,7 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l * (and @tablesz isn't set). */ static struct resource_table * -rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len, +rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, int *tablesz) { struct elf32_hdr *ehdr; @@ -842,6 +841,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len, struct device *dev = &rproc->dev; struct resource_table *table = NULL; int i; + const u8 *elf_data = fw->data; ehdr = (struct elf32_hdr *)elf_data; shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff); @@ -858,7 +858,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len, table = (struct resource_table *)(elf_data + offset); /* make sure we have the entire table */ - if (offset + size > len) { + if (offset + size > fw->size) { dev_err(dev, "resource table truncated\n"); return NULL; } @@ -1035,7 +1035,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) rproc->bootaddr = ehdr->e_entry; /* look for the resource table */ - table = rproc_find_rsc_table(rproc, fw->data, fw->size, &tablesz); + table = rproc_find_rsc_table(rproc, fw, &tablesz); if (!table) goto clean_up; @@ -1047,7 +1047,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) } /* load the ELF segments to memory */ - ret = rproc_load_segments(rproc, fw->data, fw->size); + ret = rproc_load_segments(rproc, fw); if (ret) { dev_err(dev, "Failed to load program segments: %d\n", ret); goto clean_up; @@ -1090,7 +1090,7 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context) goto out; /* look for the resource table */ - table = rproc_find_rsc_table(rproc, fw->data, fw->size, &tablesz); + table = rproc_find_rsc_table(rproc, fw, &tablesz); if (!table) goto out; -- cgit v1.1 From 3e5f9eb5d91e430ca908a61615f9a89c189a0d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= Date: Tue, 19 Jun 2012 09:56:44 +0300 Subject: remoteproc: Add function rproc_get_boot_addr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare for introduction of custom firmware loaders by moving the function operating on ELF data-structures into separate functions. Move lookup of the boot_addr in the ELF binary to the function rproc_get_boot_addr(). Signed-off-by: Sjur Brændeland [rproc_get_boot_addr's kerneldoc: add missing @rproc line] [rproc_get_boot_addr's kerneldoc: minor style changes] Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index e75424b..3173e21 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -998,6 +998,24 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) return 0; } +/** + * rproc_get_boot_addr() - Get rproc's boot address. + * @rproc: the remote processor handle + * @fw: the ELF firmware image + * + * This function returns the entry point address of the ELF + * image. + * + * Note that the boot address is not a configurable property of all remote + * processors. Some will always boot at a specific hard-coded address. + */ +u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) +{ + struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data; + + return ehdr->e_entry; +} + /* * take a firmware and boot a remote processor with it. */ @@ -1005,7 +1023,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) { struct device *dev = &rproc->dev; const char *name = rproc->firmware; - struct elf32_hdr *ehdr; struct resource_table *table; int ret, tablesz; @@ -1013,8 +1030,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) if (ret) return ret; - ehdr = (struct elf32_hdr *)fw->data; - dev_info(dev, "Booting fw image %s, size %d\n", name, fw->size); /* @@ -1027,12 +1042,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) return ret; } - /* - * The ELF entry point is the rproc's boot addr (though this is not - * a configurable property of all remote processors: some will always - * boot at a specific hardcoded address). - */ - rproc->bootaddr = ehdr->e_entry; + rproc->bootaddr = rproc_get_boot_addr(rproc, fw); /* look for the resource table */ table = rproc_find_rsc_table(rproc, fw, &tablesz); -- cgit v1.1 From 72854fb042b15b6139031a59c4725b3d86708352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= Date: Sun, 15 Jul 2012 11:25:27 +0300 Subject: remoteproc: Move Elf related functions to separate file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare for introduction of custom firmware loaders by moving all ELF related handling into a separate file. The functions: rproc_find_rsc_table(), rproc_fw_sanity_check(), rproc_find_rsc_table() and rproc_get_boot_addr() are moved to the new file remoteproc_elf_loader.c. The function rproc_da_to_va() is made non-static and is declared in remoteproc_internal.h No functional changes are introduced in this patch. Signed-off-by: Sjur Brændeland [ohad: rebase, fix kerneldoc, put prototypes in remoteproc_internal.h] Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/Makefile | 1 + drivers/remoteproc/remoteproc_core.c | 249 +------------------------ drivers/remoteproc/remoteproc_elf_loader.c | 287 +++++++++++++++++++++++++++++ drivers/remoteproc/remoteproc_internal.h | 11 ++ 4 files changed, 300 insertions(+), 248 deletions(-) create mode 100644 drivers/remoteproc/remoteproc_elf_loader.c diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 5445d9b..934ce6e 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -6,4 +6,5 @@ obj-$(CONFIG_REMOTEPROC) += remoteproc.o remoteproc-y := remoteproc_core.o remoteproc-y += remoteproc_debugfs.o remoteproc-y += remoteproc_virtio.o +remoteproc-y += remoteproc_elf_loader.o obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 3173e21..c68b3bb5 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -149,7 +149,7 @@ static void rproc_disable_iommu(struct rproc *rproc) * but only on kernel direct mapped RAM memory. Instead, we're just using * here the output of the DMA API, which should be more correct. */ -static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) +void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) { struct rproc_mem_entry *carveout; void *ptr = NULL; @@ -173,96 +173,6 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) return ptr; } -/** - * rproc_load_segments() - load firmware segments to memory - * @rproc: remote processor which will be booted using these fw segments - * @fw: the ELF firmware image - * - * This function loads the firmware segments to memory, where the remote - * processor expects them. - * - * Some remote processors will expect their code and data to be placed - * in specific device addresses, and can't have them dynamically assigned. - * - * We currently support only those kind of remote processors, and expect - * the program header's paddr member to contain those addresses. We then go - * through the physically contiguous "carveout" memory regions which we - * allocated (and mapped) earlier on behalf of the remote processor, - * and "translate" device address to kernel addresses, so we can copy the - * segments where they are expected. - * - * Currently we only support remote processors that required carveout - * allocations and got them mapped onto their iommus. Some processors - * might be different: they might not have iommus, and would prefer to - * directly allocate memory for every segment/resource. This is not yet - * supported, though. - */ -static int -rproc_load_segments(struct rproc *rproc, const struct firmware *fw) -{ - struct device *dev = &rproc->dev; - struct elf32_hdr *ehdr; - struct elf32_phdr *phdr; - int i, ret = 0; - const u8 *elf_data = fw->data; - - ehdr = (struct elf32_hdr *)elf_data; - phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff); - - /* go through the available ELF segments */ - for (i = 0; i < ehdr->e_phnum; i++, phdr++) { - u32 da = phdr->p_paddr; - u32 memsz = phdr->p_memsz; - u32 filesz = phdr->p_filesz; - u32 offset = phdr->p_offset; - void *ptr; - - if (phdr->p_type != PT_LOAD) - continue; - - dev_dbg(dev, "phdr: type %d da 0x%x memsz 0x%x filesz 0x%x\n", - phdr->p_type, da, memsz, filesz); - - if (filesz > memsz) { - dev_err(dev, "bad phdr filesz 0x%x memsz 0x%x\n", - filesz, memsz); - ret = -EINVAL; - break; - } - - if (offset + filesz > fw->size) { - dev_err(dev, "truncated fw: need 0x%x avail 0x%x\n", - offset + filesz, fw->size); - ret = -EINVAL; - break; - } - - /* grab the kernel address for this device address */ - ptr = rproc_da_to_va(rproc, da, memsz); - if (!ptr) { - dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz); - ret = -EINVAL; - break; - } - - /* put the segment where the remote processor expects it */ - if (phdr->p_filesz) - memcpy(ptr, elf_data + phdr->p_offset, filesz); - - /* - * Zero out remaining memory for this segment. - * - * This isn't strictly required since dma_alloc_coherent already - * did this for us. albeit harmless, we may consider removing - * this. - */ - if (memsz > filesz) - memset(ptr + filesz, 0, memsz - filesz); - } - - return ret; -} - int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) { struct rproc *rproc = rvdev->rproc; @@ -817,85 +727,6 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l } /** - * rproc_find_rsc_table() - find the resource table - * @rproc: the rproc handle - * @fw: the ELF 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 * -rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, - int *tablesz) -{ - struct elf32_hdr *ehdr; - struct elf32_shdr *shdr; - const char *name_table; - struct device *dev = &rproc->dev; - struct resource_table *table = NULL; - int i; - const u8 *elf_data = fw->data; - - ehdr = (struct elf32_hdr *)elf_data; - shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff); - name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset; - - /* look for the resource table and handle it */ - for (i = 0; i < ehdr->e_shnum; i++, shdr++) { - int size = shdr->sh_size; - int offset = shdr->sh_offset; - - if (strcmp(name_table + shdr->sh_name, ".resource_table")) - continue; - - table = (struct resource_table *)(elf_data + offset); - - /* make sure we have the entire table */ - if (offset + 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) > 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) > size) { - dev_err(dev, "resource table incomplete\n"); - return NULL; - } - - *tablesz = shdr->sh_size; - break; - } - - return table; -} - -/** * rproc_resource_cleanup() - clean up and free all acquired resources * @rproc: rproc handle * @@ -938,84 +769,6 @@ static void rproc_resource_cleanup(struct rproc *rproc) } } -/* make sure this fw image is sane */ -static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) -{ - const char *name = rproc->firmware; - struct device *dev = &rproc->dev; - struct elf32_hdr *ehdr; - char class; - - if (!fw) { - dev_err(dev, "failed to load %s\n", name); - return -EINVAL; - } - - if (fw->size < sizeof(struct elf32_hdr)) { - dev_err(dev, "Image is too small\n"); - return -EINVAL; - } - - ehdr = (struct elf32_hdr *)fw->data; - - /* We only support ELF32 at this point */ - class = ehdr->e_ident[EI_CLASS]; - if (class != ELFCLASS32) { - dev_err(dev, "Unsupported class: %d\n", class); - return -EINVAL; - } - - /* We assume the firmware has the same endianess as the host */ -# ifdef __LITTLE_ENDIAN - if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) { -# else /* BIG ENDIAN */ - if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { -# endif - dev_err(dev, "Unsupported firmware endianess\n"); - return -EINVAL; - } - - if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) { - dev_err(dev, "Image is too small\n"); - return -EINVAL; - } - - if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) { - dev_err(dev, "Image is corrupted (bad magic)\n"); - return -EINVAL; - } - - if (ehdr->e_phnum == 0) { - dev_err(dev, "No loadable segments\n"); - return -EINVAL; - } - - if (ehdr->e_phoff > fw->size) { - dev_err(dev, "Firmware size is too small\n"); - return -EINVAL; - } - - return 0; -} - -/** - * rproc_get_boot_addr() - Get rproc's boot address. - * @rproc: the remote processor handle - * @fw: the ELF firmware image - * - * This function returns the entry point address of the ELF - * image. - * - * Note that the boot address is not a configurable property of all remote - * processors. Some will always boot at a specific hard-coded address. - */ -u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) -{ - struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data; - - return ehdr->e_entry; -} - /* * take a firmware and boot a remote processor with it. */ diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c new file mode 100644 index 0000000..2c6fe6a --- /dev/null +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -0,0 +1,287 @@ +/* + * Remote Processor Framework Elf loader + * + * Copyright (C) 2011 Texas Instruments, Inc. + * Copyright (C) 2011 Google, Inc. + * + * Ohad Ben-Cohen + * Brian Swetland + * Mark Grosen + * Fernando Guzman Lugo + * Suman Anna + * Robert Tivy + * Armando Uribe De Leon + * Sjur Brændeland + * + * 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 +#include +#include +#include + +#include "remoteproc_internal.h" + +/** + * rproc_fw_sanity_check() - Sanity Check ELF firmware image + * @rproc: the remote processor handle + * @fw: the ELF firmware image + * + * Make sure this fw image is sane. + */ +int +rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) +{ + const char *name = rproc->firmware; + struct device *dev = &rproc->dev; + struct elf32_hdr *ehdr; + char class; + + if (!fw) { + dev_err(dev, "failed to load %s\n", name); + return -EINVAL; + } + + if (fw->size < sizeof(struct elf32_hdr)) { + dev_err(dev, "Image is too small\n"); + return -EINVAL; + } + + ehdr = (struct elf32_hdr *)fw->data; + + /* We only support ELF32 at this point */ + class = ehdr->e_ident[EI_CLASS]; + if (class != ELFCLASS32) { + dev_err(dev, "Unsupported class: %d\n", class); + return -EINVAL; + } + + /* We assume the firmware has the same endianess as the host */ +# ifdef __LITTLE_ENDIAN + if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) { +# else /* BIG ENDIAN */ + if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { +# endif + dev_err(dev, "Unsupported firmware endianess\n"); + return -EINVAL; + } + + if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) { + dev_err(dev, "Image is too small\n"); + return -EINVAL; + } + + if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) { + dev_err(dev, "Image is corrupted (bad magic)\n"); + return -EINVAL; + } + + if (ehdr->e_phnum == 0) { + dev_err(dev, "No loadable segments\n"); + return -EINVAL; + } + + if (ehdr->e_phoff > fw->size) { + dev_err(dev, "Firmware size is too small\n"); + return -EINVAL; + } + + return 0; +} + +/** + * rproc_get_boot_addr() - Get rproc's boot address. + * @rproc: the remote processor handle + * @fw: the ELF firmware image + * + * This function returns the entry point address of the ELF + * image. + * + * Note that the boot address is not a configurable property of all remote + * processors. Some will always boot at a specific hard-coded address. + */ +u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) +{ + struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data; + + return ehdr->e_entry; +} + +/** + * rproc_load_segments() - load firmware segments to memory + * @rproc: remote processor which will be booted using these fw segments + * @fw: the ELF firmware image + * + * This function loads the firmware segments to memory, where the remote + * processor expects them. + * + * Some remote processors will expect their code and data to be placed + * in specific device addresses, and can't have them dynamically assigned. + * + * We currently support only those kind of remote processors, and expect + * the program header's paddr member to contain those addresses. We then go + * through the physically contiguous "carveout" memory regions which we + * allocated (and mapped) earlier on behalf of the remote processor, + * and "translate" device address to kernel addresses, so we can copy the + * segments where they are expected. + * + * Currently we only support remote processors that required carveout + * allocations and got them mapped onto their iommus. Some processors + * might be different: they might not have iommus, and would prefer to + * directly allocate memory for every segment/resource. This is not yet + * supported, though. + */ +int +rproc_load_segments(struct rproc *rproc, const struct firmware *fw) +{ + struct device *dev = &rproc->dev; + struct elf32_hdr *ehdr; + struct elf32_phdr *phdr; + int i, ret = 0; + const u8 *elf_data = fw->data; + + ehdr = (struct elf32_hdr *)elf_data; + phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff); + + /* go through the available ELF segments */ + for (i = 0; i < ehdr->e_phnum; i++, phdr++) { + u32 da = phdr->p_paddr; + u32 memsz = phdr->p_memsz; + u32 filesz = phdr->p_filesz; + u32 offset = phdr->p_offset; + void *ptr; + + if (phdr->p_type != PT_LOAD) + continue; + + dev_dbg(dev, "phdr: type %d da 0x%x memsz 0x%x filesz 0x%x\n", + phdr->p_type, da, memsz, filesz); + + if (filesz > memsz) { + dev_err(dev, "bad phdr filesz 0x%x memsz 0x%x\n", + filesz, memsz); + ret = -EINVAL; + break; + } + + if (offset + filesz > fw->size) { + dev_err(dev, "truncated fw: need 0x%x avail 0x%x\n", + offset + filesz, fw->size); + ret = -EINVAL; + break; + } + + /* grab the kernel address for this device address */ + ptr = rproc_da_to_va(rproc, da, memsz); + if (!ptr) { + dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz); + ret = -EINVAL; + break; + } + + /* put the segment where the remote processor expects it */ + if (phdr->p_filesz) + memcpy(ptr, elf_data + phdr->p_offset, filesz); + + /* + * Zero out remaining memory for this segment. + * + * This isn't strictly required since dma_alloc_coherent already + * did this for us. albeit harmless, we may consider removing + * this. + */ + if (memsz > filesz) + memset(ptr + filesz, 0, memsz - filesz); + } + + return ret; +} + +/** + * rproc_find_rsc_table() - find the resource table + * @rproc: the rproc handle + * @fw: the ELF 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). + */ +struct resource_table * +rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, + int *tablesz) +{ + struct elf32_hdr *ehdr; + struct elf32_shdr *shdr; + const char *name_table; + struct device *dev = &rproc->dev; + struct resource_table *table = NULL; + int i; + const u8 *elf_data = fw->data; + + ehdr = (struct elf32_hdr *)elf_data; + shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff); + name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset; + + /* look for the resource table and handle it */ + for (i = 0; i < ehdr->e_shnum; i++, shdr++) { + int size = shdr->sh_size; + int offset = shdr->sh_offset; + + if (strcmp(name_table + shdr->sh_name, ".resource_table")) + continue; + + table = (struct resource_table *)(elf_data + offset); + + /* make sure we have the entire table */ + if (offset + 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) > 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) > size) { + dev_err(dev, "resource table incomplete\n"); + return NULL; + } + + *tablesz = shdr->sh_size; + break; + } + + return table; +} diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index f4957cf..a44e192 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -21,6 +21,7 @@ #define REMOTEPROC_INTERNAL_H #include +#include struct rproc; @@ -43,4 +44,14 @@ void rproc_exit_debugfs(void); void rproc_free_vring(struct rproc_vring *rvring); int rproc_alloc_vring(struct rproc_vdev *rvdev, int i); + +void *rproc_da_to_va(struct rproc *rproc, u64 da, int len); + +struct resource_table *rproc_find_rsc_table(struct rproc *rproc, + const struct firmware *fw, + int *tablesz); +int rproc_load_segments(struct rproc *rproc, const struct firmware *fw); +int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw); +u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw); + #endif /* REMOTEPROC_INTERNAL_H */ -- cgit v1.1 From 4afc89d66c60a372ec15e99eee93621f650b5d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= Date: Tue, 19 Jun 2012 10:08:18 +0300 Subject: remoteproc: Support custom firmware handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firmware handling is made customizable. This is done by creating a separate ops structure for the firmware functions that depends on a particular firmware format (such as ELF). The ELF functions are default used unless the HW driver explicitly injects another firmware handler by updating rproc->fw_ops. The function rproc_da_to_va() is exported, as custom firmware handlers may need to use this function. Signed-off-by: Sjur Brændeland [ohad: namespace fixes, whitespace fixes, style fixes] Signed-off-by: Ohad Ben-Cohen --- drivers/remoteproc/remoteproc_core.c | 4 ++ drivers/remoteproc/remoteproc_elf_loader.c | 30 +++++++++------ drivers/remoteproc/remoteproc_internal.h | 59 +++++++++++++++++++++++++++--- include/linux/remoteproc.h | 2 + 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index c68b3bb5..f4d6f7b 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -172,6 +172,7 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len) return ptr; } +EXPORT_SYMBOL(rproc_da_to_va); int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) { @@ -1150,6 +1151,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, atomic_set(&rproc->power, 0); + /* Set ELF as the default fw_ops handler */ + rproc->fw_ops = &rproc_elf_fw_ops; + mutex_init(&rproc->lock); idr_init(&rproc->notifyids); diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index 2c6fe6a..6eebd01 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -33,14 +33,14 @@ #include "remoteproc_internal.h" /** - * rproc_fw_sanity_check() - Sanity Check ELF firmware image + * rproc_elf_sanity_check() - Sanity Check ELF firmware image * @rproc: the remote processor handle * @fw: the ELF firmware image * * Make sure this fw image is sane. */ -int -rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) +static int +rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw) { const char *name = rproc->firmware; struct device *dev = &rproc->dev; @@ -100,7 +100,7 @@ rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) } /** - * rproc_get_boot_addr() - Get rproc's boot address. + * rproc_elf_get_boot_addr() - Get rproc's boot address. * @rproc: the remote processor handle * @fw: the ELF firmware image * @@ -110,7 +110,8 @@ rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) * Note that the boot address is not a configurable property of all remote * processors. Some will always boot at a specific hard-coded address. */ -u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) +static +u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw) { struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data; @@ -118,7 +119,7 @@ u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) } /** - * rproc_load_segments() - load firmware segments to memory + * rproc_elf_load_segments() - load firmware segments to memory * @rproc: remote processor which will be booted using these fw segments * @fw: the ELF firmware image * @@ -141,8 +142,8 @@ u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) * directly allocate memory for every segment/resource. This is not yet * supported, though. */ -int -rproc_load_segments(struct rproc *rproc, const struct firmware *fw) +static int +rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw) { struct device *dev = &rproc->dev; struct elf32_hdr *ehdr; @@ -208,7 +209,7 @@ rproc_load_segments(struct rproc *rproc, const struct firmware *fw) } /** - * rproc_find_rsc_table() - find the resource table + * rproc_elf_find_rsc_table() - find the resource table * @rproc: the rproc handle * @fw: the ELF firmware image * @tablesz: place holder for providing back the table size @@ -222,8 +223,8 @@ rproc_load_segments(struct rproc *rproc, const struct firmware *fw) * size into @tablesz. If a valid table isn't found, NULL is returned * (and @tablesz isn't set). */ -struct resource_table * -rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, +static struct resource_table * +rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, int *tablesz) { struct elf32_hdr *ehdr; @@ -285,3 +286,10 @@ rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table; } + +const struct rproc_fw_ops rproc_elf_fw_ops = { + .load = rproc_elf_load_segments, + .find_rsc_table = rproc_elf_find_rsc_table, + .sanity_check = rproc_elf_sanity_check, + .get_boot_addr = rproc_elf_get_boot_addr +}; diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index a44e192..a690ebe 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -25,6 +25,23 @@ struct rproc; +/** + * struct rproc_fw_ops - firmware format specific operations. + * @find_rsc_table: finds the resource table inside the firmware image + * @load: load firmeware to memory, where the remote processor + * expects to find it + * @sanity_check: sanity check the fw image + * @get_boot_addr: get boot address to entry point specified in firmware + */ +struct rproc_fw_ops { + struct resource_table *(*find_rsc_table) (struct rproc *rproc, + const struct firmware *fw, + int *tablesz); + int (*load)(struct rproc *rproc, const struct firmware *fw); + int (*sanity_check)(struct rproc *rproc, const struct firmware *fw); + u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw); +}; + /* from remoteproc_core.c */ void rproc_release(struct kref *kref); irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id); @@ -47,11 +64,43 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i); void *rproc_da_to_va(struct rproc *rproc, u64 da, int len); +static inline +int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw) +{ + if (rproc->fw_ops->sanity_check) + return rproc->fw_ops->sanity_check(rproc, fw); + + return 0; +} + +static inline +u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw) +{ + if (rproc->fw_ops->get_boot_addr) + return rproc->fw_ops->get_boot_addr(rproc, fw); + + return 0; +} + +static inline +int rproc_load_segments(struct rproc *rproc, const struct firmware *fw) +{ + if (rproc->fw_ops->load) + return rproc->fw_ops->load(rproc, fw); + + return -EINVAL; +} + +static inline struct resource_table *rproc_find_rsc_table(struct rproc *rproc, - const struct firmware *fw, - int *tablesz); -int rproc_load_segments(struct rproc *rproc, const struct firmware *fw); -int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw); -u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw); + const struct firmware *fw, int *tablesz) +{ + if (rproc->fw_ops->find_rsc_table) + return rproc->fw_ops->find_rsc_table(rproc, fw, tablesz); + + return NULL; +} + +extern const struct rproc_fw_ops rproc_elf_fw_ops; #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index eea3ac8..131b539 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -369,6 +369,7 @@ enum rproc_state { * @priv: private data which belongs to the platform-specific rproc module * @ops: platform-specific start/stop rproc handlers * @dev: virtual device for refcounting and common remoteproc behavior + * @fw_ops: firmware-specific handlers * @power: refcount of users who need this rproc powered up * @state: state of the device * @lock: lock which protects concurrent manipulations of the rproc @@ -391,6 +392,7 @@ struct rproc { void *priv; const struct rproc_ops *ops; struct device dev; + const struct rproc_fw_ops *fw_ops; atomic_t power; unsigned int state; struct mutex lock; -- cgit v1.1 From 6bb697b6b06041d2d0affc862b17b4b443a107e0 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen Date: Tue, 19 Jun 2012 10:22:35 +0300 Subject: MAINTAINERS: add remoteproc's git Add remoteproc's git tree to the MAINTAINERS entry Signed-off-by: Ohad Ben-Cohen --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3e30a3a..5275ca9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5708,6 +5708,7 @@ F: include/linux/regmap.h REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM M: Ohad Ben-Cohen +T: git git://git.kernel.org/pub/scm/linux/kernel/git/ohad/remoteproc.git S: Maintained F: drivers/remoteproc/ F: Documentation/remoteproc.txt -- cgit v1.1