This series proposes to introduce the notion of platform device for the
rproc virtio management. One obective is to allow virtio to declare is
own memory resources without the usage of dma_declare_coherent_memory
that seems deprecated since the introduction of the device tree.
Proposal:
- the rproc virtio is processed in a separate platform and could be handled
as a generic platform device.
- Several vdev devices can be declared in DT:
- which allows to declare their own memory regions and answer to [1].
- as next steps it would be also possible to:
- declare their own mailboxes, rpmsg drivers, ...
- use dma-range to handle the pa<->da translation at virtio level
Several notions are introduced here:
- Virtio platform registration which allows to decorelate virtio from the
remote proc core device.
- Synchronization of the child devices relying on component bind/unbind.
This mechanism ensures the synchronization of the child devices before
the boot of the remote processor and before the release of the resources
on the remote processor shutdown.
- Ability to populate child devices declared in rproc device tree node.
- Possibility to declare the memory regions reserved to a virtio devices in
the devicetree.
Known limitations:
- the patchset has been tested on a st32mP1 plaform only
- it is based on the v5.6 (need to evoluate depending on V5.7 and on going
patchsets).
- The virtio memory allocation does not take into account memory
controllers such as IOMMU and MPU.
Arnaud Pouliquen (18):
remoteproc: Store resource table address in rvdev
remoteproc: Introduce virtio device add/remove functions in core.
remoteproc: Move rvdev management in rproc_virtio
remoteproc: Add rproc_get_by_node helper
remoteproc: Create platform device for vdev
remoteproc: Add component in core for child devices synchronization
remoteproc: Add component bind/unbind for virtio platform
remoteproc: Externalize carveout functions
remoteproc: Move vring management from core to virtio
remoteproc: Add capability to populate rproc subnode devices
remoteproc: Add child node component in rproc match list
remoteproc: Support of pre-registered virtio device
remoteproc: Add memory default allocator helper
remoteproc: Add pa to da translation API
remoteproc: associate memory entry to a device
remoteproc: Parse virtio node for memory region
remoteproc: stm32: add the pa to da ops.
ARM: dts: stm32: Declare a virtio device
arch/arm/boot/dts/stm32mp15xx-dkx.dtsi | 10 +
drivers/remoteproc/remoteproc_core.c | 469 ++++++++++++-----------
drivers/remoteproc/remoteproc_internal.h | 23 +-
drivers/remoteproc/remoteproc_virtio.c | 415 ++++++++++++++++++--
drivers/remoteproc/stm32_rproc.c | 1 +
include/linux/remoteproc.h | 27 +-
6 files changed, 673 insertions(+), 272 deletions(-)
--
2.17.1
Management of the virtio device declared in the devicetree as
a sub device of the remoteproc.
Instead of creating a new platform device, we parse first the
rvdevs list to look for a pre-registered rvdev with an index, that
matches with the vdev instance of the resource table.
If no pre-registered vdev is found a new platform device is created.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 51 +++++++++++++++++-----------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index ecb36f64b1a0..9238aa292644 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -383,8 +383,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
{
struct device *dev = &rproc->dev;
struct rproc_vdev_data vdev_data;
+ struct rproc_vdev *rvdev = NULL, *tmp_rvdev;
struct platform_device *pdev;
- int ret;
/* make sure resource isn't truncated */
if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
@@ -399,27 +399,40 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
return -EINVAL;
}
- vdev_data.rsc_offset = offset;
- vdev_data.rsc = rsc;
- vdev_data.id = rsc->id;
- vdev_data.index = rproc->nb_vdev;
+ /* Try to find a pre-registered rproc virtio device */
+ list_for_each_entry(tmp_rvdev, &rproc->rvdevs, node) {
+ if (tmp_rvdev->index == rproc->nb_vdev) {
+ rvdev = tmp_rvdev;
+ break;
+ }
+ }
- dev_dbg(dev, "%s: rsc_offset = %d rsc = %p id = %d\n",
- __func__, vdev_data.rsc_offset, vdev_data.rsc, vdev_data.id);
+ if (rvdev) {
+ /* assign the resource offset */
+ rvdev->rsc_offset = offset;
+ rvdev->rsc = rsc;
+ rvdev->id = rsc->id;
- pdev = platform_device_register_data(dev, "rproc-virtio",
- rproc->nb_vdev, &vdev_data,
- sizeof(vdev_data));
- ret = PTR_ERR_OR_ZERO(pdev);
- if (ret) {
- dev_err(rproc->dev.parent,
- "failed to create rproc-virtio device\n");
- return ret;
+ } else {
+ /* no rproc vdev found, register one */
+ vdev_data.rsc_offset = offset;
+ vdev_data.rsc = rsc;
+ vdev_data.id = rsc->id;
+ vdev_data.index = rproc->nb_vdev;
+
+ pdev = platform_device_register_data(dev, "rproc-virtio",
+ rproc->nb_vdev, &vdev_data,
+ sizeof(vdev_data));
+ if (PTR_ERR_OR_ZERO(pdev)) {
+ dev_err(rproc->dev.parent,
+ "failed to create rproc-virtio device\n");
+ return PTR_ERR_OR_ZERO(pdev);
+ }
+ /* register a component associated to the virtio platform */
+ component_match_add_release(&pdev->dev, &rproc->match,
+ rproc_release_of, rproc_compare_of,
+ &pdev->dev);
}
- /* register a component associated to the virtio platform */
- component_match_add_release(&pdev->dev, &rproc->match,
- rproc_release_of, rproc_compare_of,
- &pdev->dev);
rproc->nb_vdev++;
return 0;
--
2.17.1
Add a default basic allocator based on ioremap, to allocate
carveout memories. These functions can be used by
platforms that do not need specific management of the memory
region (no MPU, no IOMMU, ...)
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 50 ++++++++++++++++++++++++
drivers/remoteproc/remoteproc_internal.h | 2 +
2 files changed, 52 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9238aa292644..f9d04e59081c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -220,6 +220,56 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
}
EXPORT_SYMBOL(rproc_da_to_va);
+/**
+ * rproc_default_mem_alloc() - Simple mmap of the memory
+ * @rproc: handle of a remote processor
+ * @mem: memory entry descriptor
+ *
+ * Memory allocator for basic remote processors that do not need to manage
+ * specific memory allocation ( no MPU; no IOMMU, ....). In this case the
+ * memory is just mapped in in kernel space.
+ *
+ * Return: 0 on success else error
+ */
+int rproc_default_mem_alloc(struct rproc *rproc, struct rproc_mem_entry *mem)
+{
+ struct device *dev = rproc->dev.parent;
+ void *va;
+
+ dev_dbg(dev, "map memory: %pa+%x\n", &mem->dma, mem->len);
+ va = ioremap_wc(mem->dma, mem->len);
+ if (IS_ERR_OR_NULL(va)) {
+ dev_err(dev, "Unable to map memory region: %pa+%x\n",
+ &mem->dma, mem->len);
+ return -ENOMEM;
+ }
+
+ /* Update memory entry va */
+ mem->va = va;
+
+ return 0;
+}
+EXPORT_SYMBOL(rproc_default_mem_alloc);
+
+/**
+ * rproc_default_mem_release() - release of the mmaped memory
+ * @rproc: handle of a remote processor
+ * @mem: memory entry descriptor
+ *
+ * Memory release for basic remote processors allocated by
+ * @rproc_default_mem_alloc
+ *
+ * Return: 0 on success else error
+ */
+int rproc_default_mem_release(struct rproc *rproc, struct rproc_mem_entry *mem)
+{
+ dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
+ iounmap(mem->va);
+
+ return 0;
+}
+EXPORT_SYMBOL(rproc_default_mem_release);
+
/**
* rproc_find_carveout_by_name() - lookup the carveout region by a name
* @rproc: handle of a remote processor
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 5139cca646ca..608aeea564b4 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -65,6 +65,8 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw);
int rproc_elf_load_rsc_table(struct rproc *rproc, const struct firmware *fw);
struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
const struct firmware *fw);
+int rproc_default_mem_alloc(struct rproc *rproc, struct rproc_mem_entry *mem);
+int rproc_default_mem_release(struct rproc *rproc, struct rproc_mem_entry *mem);
struct rproc_mem_entry *
rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
--
2.17.1
Add remote proc API to allow IPC client to translate address from
local to device paradigm.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 24 ++++++++++++++++++++++++
include/linux/remoteproc.h | 4 ++++
2 files changed, 28 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index f9d04e59081c..72fb97f28048 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1939,6 +1939,30 @@ int rproc_add(struct rproc *rproc)
}
EXPORT_SYMBOL(rproc_add);
+/**
+ * rproc_pa_to_da() - memory translation from local physical address to
+ * remote device address
+ * @rproc: the remote processor handle to register
+ * @pa: local physical address
+ * @da: remote device address
+ *
+ * Return the device address associated to the physical address
+ * The translation is delegated to the platform driver if the ops is
+ * implemented. By default this function returns the physical address.
+ *
+ * Returns 0 on success and an appropriate error code otherwise.
+ */
+int rproc_pa_to_da(struct rproc *rproc, phys_addr_t pa, u64 *da)
+{
+ if (!rproc->ops->pa_to_da) {
+ *da = pa;
+ return 0;
+ }
+
+ return rproc->ops->pa_to_da(rproc, pa, da);
+}
+EXPORT_SYMBOL(rproc_pa_to_da);
+
/**
* rproc_type_release() - release a remote processor instance
* @dev: the rproc's device
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index d7235f7356e2..7b8a6c3ef519 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -359,6 +359,7 @@ enum rsc_handling_status {
* @stop: power off the device
* @kick: kick a virtqueue (virtqueue id given as a parameter)
* @da_to_va: optional platform hook to perform address translations
+ * @pa_to_da: optional platform hook to perform address translations
* @parse_fw: parse firmware to extract information (e.g. resource table)
* @handle_rsc: optional platform hook to handle vendor resources. Should return
* RSC_HANDLED if resource was handled, RSC_IGNORED if not handled and a
@@ -375,6 +376,7 @@ struct rproc_ops {
int (*stop)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
+ int (*pa_to_da)(struct rproc *rproc, phys_addr_t pa, u64 *da);
int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
int (*handle_rsc)(struct rproc *rproc, u32 rsc_type, void *rsc,
int offset, int avail);
@@ -618,6 +620,8 @@ struct rproc_mem_entry *
rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, int len,
u32 da, const char *name, ...);
+int rproc_pa_to_da(struct rproc *rproc, phys_addr_t pa, u64 *da);
+
int rproc_boot(struct rproc *rproc);
void rproc_shutdown(struct rproc *rproc);
void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
--
2.17.1
Add the capability to add rproc sub nodes in device tree.
Devices declared as rproc sub node will be probed
before the remote firmware boot sequence.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c9e07c53ed0c..75f84fd3bd60 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1605,6 +1605,26 @@ static void rproc_crash_handler_work(struct work_struct *work)
rproc_trigger_recovery(rproc);
}
+static int rproc_platform_populate(struct rproc *rproc)
+{
+ struct device *dev = rproc->dev.parent;
+ int ret;
+
+ ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to populate child devices (%d)\n", ret);
+
+ goto depopulate;
+ }
+
+ return 0;
+
+depopulate:
+ of_platform_depopulate(dev);
+
+ return ret;
+}
+
/**
* rproc_boot() - boot a remote processor
* @rproc: handle of a remote processor
@@ -1629,6 +1649,10 @@ int rproc_boot(struct rproc *rproc)
dev = &rproc->dev;
+ ret = rproc_platform_populate(rproc);
+ if (ret)
+ return ret;
+
ret = mutex_lock_interruptible(&rproc->lock);
if (ret) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
@@ -1735,6 +1759,7 @@ void rproc_shutdown(struct rproc *rproc)
rproc->table_ptr = NULL;
out:
mutex_unlock(&rproc->lock);
+ of_platform_depopulate(rproc->dev.parent);
}
EXPORT_SYMBOL(rproc_shutdown);
--
2.17.1
Add the components, associated to the child devices populated, in
the match list for synchronization relying on bind/unbind mechanism.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 75f84fd3bd60..ecb36f64b1a0 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1608,6 +1608,8 @@ static void rproc_crash_handler_work(struct work_struct *work)
static int rproc_platform_populate(struct rproc *rproc)
{
struct device *dev = rproc->dev.parent;
+ struct device_node *node = dev->of_node;
+ struct device_node *child_np;
int ret;
ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
@@ -1617,6 +1619,19 @@ static int rproc_platform_populate(struct rproc *rproc)
goto depopulate;
}
+ child_np = of_get_next_available_child(node, NULL);
+
+ while (child_np) {
+ of_node_get(node);
+ component_match_add_release(dev, &rproc->match,
+ rproc_release_of, rproc_compare_of,
+ child_np);
+ child_np = of_get_next_available_child(node, child_np);
+ }
+
+ if (!rproc->match)
+ dev_dbg(dev, "No available child\n");
+
return 0;
depopulate:
--
2.17.1
mem entry could be registered at different levels, platform driver
or virtio device itself.
Add device information in rproc_mem_entry struct to retrieve a memory
region registered by another device.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 2 ++
drivers/remoteproc/remoteproc_virtio.c | 2 +-
include/linux/remoteproc.h | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 72fb97f28048..e72d681033d3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -892,6 +892,7 @@ rproc_mem_entry_init(struct device *dev,
if (!mem)
return mem;
+ mem->dev = dev;
mem->va = va;
mem->dma = dma;
mem->da = da;
@@ -932,6 +933,7 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, int len,
if (!mem)
return mem;
+ mem->dev = dev;
mem->da = da;
mem->len = len;
mem->rsc_offset = FW_RSC_ADDR_ANY;
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 4634cd63d547..0fd938afd146 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -439,7 +439,7 @@ static int rproc_vitio_start(struct rproc_subdev *subdev)
phys_addr_t pa;
if (mem->of_resm_idx != -1) {
- struct device_node *np = rproc->dev.parent->of_node;
+ struct device_node *np = mem->dev->of_node;
/* Associate reserved memory to vdev device */
ret = of_reserved_mem_device_init_by_idx(dev, np,
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 7b8a6c3ef519..36abc9bc4def 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -331,6 +331,7 @@ struct rproc_mem_entry {
dma_addr_t dma;
int len;
u32 da;
+ struct device *dev;
void *priv;
char name[32];
struct list_head node;
--
2.17.1
Allow to retrieve the rproc structure from a rproc platform
child declared in the device tree.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 14 ++++++--------
include/linux/remoteproc.h | 7 ++++++-
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 4fcd685cbfd8..32056449bb72 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1739,10 +1739,10 @@ void rproc_shutdown(struct rproc *rproc)
EXPORT_SYMBOL(rproc_shutdown);
/**
- * rproc_get_by_phandle() - find a remote processor by phandle
- * @phandle: phandle to the rproc
+ * rproc_get_by_node() - find a remote processor by device node
+ * @np: device tree node
*
- * Finds an rproc handle using the remote processor's phandle, and then
+ * Finds an rproc handle using the remote processor's node, and then
* return a handle to the rproc.
*
* This function increments the remote processor's refcount, so always
@@ -1751,12 +1751,10 @@ EXPORT_SYMBOL(rproc_shutdown);
* Returns the rproc handle on success, and NULL on failure.
*/
#ifdef CONFIG_OF
-struct rproc *rproc_get_by_phandle(phandle phandle)
+struct rproc *rproc_get_by_node(struct device_node *np)
{
struct rproc *rproc = NULL, *r;
- struct device_node *np;
- np = of_find_node_by_phandle(phandle);
if (!np)
return NULL;
@@ -1781,12 +1779,12 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
return rproc;
}
#else
-struct rproc *rproc_get_by_phandle(phandle phandle)
+struct rproc *rproc_get_by_node(struct device_node *np)
{
return NULL;
}
#endif
-EXPORT_SYMBOL(rproc_get_by_phandle);
+EXPORT_SYMBOL(rproc_get_by_node);
/**
* rproc_add() - register a remote processor
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index a78e28bda962..ab9762563ae0 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -586,9 +586,14 @@ struct rproc_vdev {
u32 index;
};
-struct rproc *rproc_get_by_phandle(phandle phandle);
+struct rproc *rproc_get_by_node(struct device_node *np);
struct rproc *rproc_get_by_child(struct device *dev);
+static inline struct rproc *rproc_get_by_phandle(phandle phandle)
+{
+ return rproc_get_by_node(of_find_node_by_phandle(phandle));
+}
+
struct rproc *rproc_alloc(struct device *dev, const char *name,
const struct rproc_ops *ops,
const char *firmware, int len);
--
2.17.1
The carveout functions are also used for the vring memories.
Externalize related functions to prepare migration of
the vring management in rproc virtio.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 18 +++++++-----------
drivers/remoteproc/remoteproc_internal.h | 4 ++++
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index cb40aae12b98..ac57cd8016be 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -51,11 +51,6 @@ static LIST_HEAD(rproc_list);
typedef int (*rproc_handle_resource_t)(struct rproc *rproc,
void *, int offset, int avail);
-static int rproc_alloc_carveout(struct rproc *rproc,
- struct rproc_mem_entry *mem);
-static int rproc_release_carveout(struct rproc *rproc,
- struct rproc_mem_entry *mem);
-
/* Unique indices for remoteproc devices */
static DEFINE_IDA(rproc_dev_index);
@@ -281,8 +276,8 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...)
*
* Return: 0 if carveout matches request else error
*/
-static int rproc_check_carveout_da(struct rproc *rproc,
- struct rproc_mem_entry *mem, u32 da, u32 len)
+int rproc_check_carveout_da(struct rproc *rproc,
+ struct rproc_mem_entry *mem, u32 da, u32 len)
{
struct device *dev = &rproc->dev;
int delta;
@@ -315,6 +310,7 @@ static int rproc_check_carveout_da(struct rproc *rproc,
return 0;
}
+EXPORT_SYMBOL(rproc_check_carveout_da);
int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
{
@@ -695,8 +691,7 @@ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc,
* This function allocate specified memory entry @mem using
* dma_alloc_coherent() as default allocator
*/
-static int rproc_alloc_carveout(struct rproc *rproc,
- struct rproc_mem_entry *mem)
+int rproc_alloc_carveout(struct rproc *rproc, struct rproc_mem_entry *mem)
{
struct rproc_mem_entry *mapping = NULL;
struct device *dev = &rproc->dev;
@@ -791,6 +786,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
dma_free_coherent(dev->parent, mem->len, va, dma);
return ret;
}
+EXPORT_SYMBOL(rproc_alloc_carveout);
/**
* rproc_release_carveout() - release acquired carveout
@@ -800,8 +796,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
* This function releases specified memory entry @mem allocated via
* rproc_alloc_carveout() function by @rproc.
*/
-static int rproc_release_carveout(struct rproc *rproc,
- struct rproc_mem_entry *mem)
+int rproc_release_carveout(struct rproc *rproc, struct rproc_mem_entry *mem)
{
struct device *dev = &rproc->dev;
@@ -809,6 +804,7 @@ static int rproc_release_carveout(struct rproc *rproc,
dma_free_coherent(dev->parent, mem->len, mem->va, mem->dma);
return 0;
}
+EXPORT_SYMBOL(rproc_release_carveout);
/**
* rproc_handle_carveout() - handle phys contig memory allocation requests
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index f5eaffac2fcd..f230296908ac 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -71,6 +71,10 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
const struct firmware *fw);
struct rproc_mem_entry *
rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
+int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
+ u32 da, u32 len);
+int rproc_alloc_carveout(struct rproc *rproc, struct rproc_mem_entry *mem);
+int rproc_release_carveout(struct rproc *rproc, struct rproc_mem_entry *mem);
static inline
int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
--
2.17.1
Store the resource table address in rvdev struct to be able to
retrieve it from vdev device.
This patch prepares the migration of rdev management in rproc_virtio.
Indeed remoteproc virtio will have to complete the vdev and vrings
resource table structures.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 1 +
include/linux/remoteproc.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 097f33e4f1f3..2a0425ab82a7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -504,6 +504,7 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
kref_init(&rvdev->refcount);
+ rvdev->rsc = rsc;
rvdev->id = rsc->id;
rvdev->rproc = rproc;
rvdev->index = rproc->nb_vdev++;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 16ad66683ad0..a78e28bda962 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -567,6 +567,7 @@ struct rproc_vring {
* @rproc: the rproc handle
* @vdev: the virio device
* @vring: the vrings for this vdev
+ * @rsc: address of the resource table
* @rsc_offset: offset of the vdev's resource entry
* @index: vdev position versus other vdev declared in resource table
*/
@@ -580,6 +581,7 @@ struct rproc_vdev {
struct list_head node;
struct rproc *rproc;
struct rproc_vring vring[RVDEV_NUM_VRINGS];
+ struct fw_rsc_vdev *rsc;
u32 rsc_offset;
u32 index;
};
--
2.17.1
The vrings are correlated to the virtio implementation.
Move following functions to rproc virtio:
- rproc_alloc_vring
- rproc_free_vring
- rproc_parse_vring
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 97 -----------------------
drivers/remoteproc/remoteproc_internal.h | 4 -
drivers/remoteproc/remoteproc_virtio.c | 98 ++++++++++++++++++++++++
3 files changed, 98 insertions(+), 101 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index ac57cd8016be..c9e07c53ed0c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -312,103 +312,6 @@ int rproc_check_carveout_da(struct rproc *rproc,
}
EXPORT_SYMBOL(rproc_check_carveout_da);
-int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
-{
- struct rproc *rproc = rvdev->rproc;
- struct device *dev = &rproc->dev;
- struct rproc_vring *rvring = &rvdev->vring[i];
- struct fw_rsc_vdev *rsc;
- int ret, size, notifyid;
- struct rproc_mem_entry *mem;
-
- /* actual size of vring (in bytes) */
- size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
-
- rsc = (void *)rproc->table_ptr + rvdev->rsc_offset;
-
- /* Search for pre-registered carveout */
- mem = rproc_find_carveout_by_name(rproc, "vdev%dvring%d", rvdev->index,
- i);
- if (mem) {
- if (rproc_check_carveout_da(rproc, mem, rsc->vring[i].da, size))
- return -ENOMEM;
- } else {
- /* Register carveout in in list */
- mem = rproc_mem_entry_init(dev, NULL, 0,
- size, rsc->vring[i].da,
- rproc_alloc_carveout,
- rproc_release_carveout,
- "vdev%dvring%d",
- rvdev->index, i);
- if (!mem) {
- dev_err(dev, "Can't allocate memory entry structure\n");
- return -ENOMEM;
- }
-
- rproc_add_carveout(rproc, mem);
- }
-
- /*
- * Assign an rproc-wide unique index for this vring
- * TODO: assign a notifyid for rvdev updates as well
- * TODO: support predefined notifyids (via resource table)
- */
- ret = idr_alloc(&rproc->notifyids, rvring, 0, 0, GFP_KERNEL);
- if (ret < 0) {
- dev_err(dev, "idr_alloc failed: %d\n", ret);
- return ret;
- }
- notifyid = ret;
-
- /* Potentially bump max_notifyid */
- if (notifyid > rproc->max_notifyid)
- rproc->max_notifyid = notifyid;
-
- rvring->notifyid = notifyid;
-
- /* Let the rproc know the notifyid of this vring.*/
- rsc->vring[i].notifyid = notifyid;
- return 0;
-}
-
-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];
-
- dev_dbg(dev, "vdev rsc: vring%d: da 0x%x, qsz %d, align %d\n",
- i, vring->da, vring->num, vring->align);
-
- /* 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)
-{
- struct rproc *rproc = rvring->rvdev->rproc;
- int idx = rvring - rvring->rvdev->vring;
- struct fw_rsc_vdev *rsc;
-
- idr_remove(&rproc->notifyids, rvring->notifyid);
-
- /* reset resource entry info */
- rsc = (void *)rproc->table_ptr + rvring->rvdev->rsc_offset;
- rsc->vring[idx].da = 0;
- rsc->vring[idx].notifyid = -1;
-}
-
static int rproc_compare_of(struct device *dev, void *data)
{
if (dev->of_node)
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index f230296908ac..5139cca646ca 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -55,10 +55,6 @@ extern struct class rproc_class;
int rproc_init_sysfs(void);
void rproc_exit_sysfs(void);
-int rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i);
-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);
phys_addr_t rproc_va_to_pa(void *cpu_addr);
int rproc_trigger_recovery(struct rproc *rproc);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index e1d7371d2d64..4634cd63d547 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -26,6 +26,104 @@
#include "remoteproc_internal.h"
+static int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
+{
+ struct rproc *rproc = rvdev->rproc;
+ struct device *dev = &rproc->dev;
+ struct rproc_vring *rvring = &rvdev->vring[i];
+ struct fw_rsc_vdev *rsc;
+ int ret, size, notifyid;
+ struct rproc_mem_entry *mem;
+
+ /* actual size of vring (in bytes) */
+ size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
+
+ rsc = (void *)rproc->table_ptr + rvdev->rsc_offset;
+
+ /* Search for pre-registered carveout */
+ mem = rproc_find_carveout_by_name(rproc, "vdev%dvring%d", rvdev->index,
+ i);
+ if (mem) {
+ if (rproc_check_carveout_da(rproc, mem, rsc->vring[i].da, size))
+ return -ENOMEM;
+ } else {
+ /* Register carveout in in list */
+ mem = rproc_mem_entry_init(dev, NULL, 0,
+ size, rsc->vring[i].da,
+ rproc_alloc_carveout,
+ rproc_release_carveout,
+ "vdev%dvring%d",
+ rvdev->index, i);
+ if (!mem) {
+ dev_err(dev, "Can't allocate memory entry structure\n");
+ return -ENOMEM;
+ }
+
+ rproc_add_carveout(rproc, mem);
+ }
+
+ /*
+ * Assign an rproc-wide unique index for this vring
+ * TODO: assign a notifyid for rvdev updates as well
+ * TODO: support predefined notifyids (via resource table)
+ */
+ ret = idr_alloc(&rproc->notifyids, rvring, 0, 0, GFP_KERNEL);
+ if (ret < 0) {
+ dev_err(dev, "idr_alloc failed: %d\n", ret);
+ return ret;
+ }
+ notifyid = ret;
+
+ /* Potentially bump max_notifyid */
+ if (notifyid > rproc->max_notifyid)
+ rproc->max_notifyid = notifyid;
+
+ rvring->notifyid = notifyid;
+
+ /* Let the rproc know the notifyid of this vring.*/
+ rsc->vring[i].notifyid = notifyid;
+ return 0;
+}
+
+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];
+
+ dev_dbg(dev, "vdev rsc: vring%d: da 0x%x, qsz %d, align %d\n",
+ i, vring->da, vring->num, vring->align);
+
+ /* 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;
+}
+
+static void rproc_free_vring(struct rproc_vring *rvring)
+{
+ struct rproc *rproc = rvring->rvdev->rproc;
+ int idx = rvring - rvring->rvdev->vring;
+ struct fw_rsc_vdev *rsc;
+
+ idr_remove(&rproc->notifyids, rvring->notifyid);
+
+ /* reset resource entry info */
+ rsc = (void *)rproc->table_ptr + rvring->rvdev->rsc_offset;
+ rsc->vring[idx].da = 0;
+ rsc->vring[idx].notifyid = -1;
+}
+
/* kick the remote processor, and let it know which virtqueue to poke at */
static bool rproc_virtio_notify(struct virtqueue *vq)
{
--
2.17.1
Migrate the management of the rvdev device and subdev from core
to virtio, to prepare the rpmsg virtio platform device creation.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 118 +-------------------
drivers/remoteproc/remoteproc_internal.h | 5 +-
drivers/remoteproc/remoteproc_virtio.c | 136 +++++++++++++++++++++--
3 files changed, 131 insertions(+), 128 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5c90d569c0f7..4fcd685cbfd8 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -371,8 +371,7 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
return 0;
}
-static int
-rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
+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;
@@ -410,117 +409,6 @@ void rproc_free_vring(struct rproc_vring *rvring)
rsc->vring[idx].notifyid = -1;
}
-static int rproc_vdev_do_start(struct rproc_subdev *subdev)
-{
- struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
-
- return rproc_add_virtio_dev(rvdev, rvdev->id);
-}
-
-static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
-{
- struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
- int ret;
-
- ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
- if (ret)
- dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
-}
-
-/**
- * rproc_rvdev_release() - release the existence of a rvdev
- *
- * @dev: the subdevice's dev
- */
-static void rproc_rvdev_release(struct device *dev)
-{
- struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
-
- of_reserved_mem_device_release(dev);
-
- kfree(rvdev);
-}
-
-static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
-{
- struct rproc *rproc = rvdev->rproc;
- struct fw_rsc_vdev *rsc = rvdev->rsc;
- char name[16];
- int ret, i;
-
- /* Initialise vdev subdevice */
- snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
- rvdev->dev.parent = &rproc->dev;
- rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
- rvdev->dev.release = rproc_rvdev_release;
- dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
- dev_set_drvdata(&rvdev->dev, rvdev);
-
- ret = device_register(&rvdev->dev);
- if (ret) {
- put_device(&rvdev->dev);
- return ret;
- }
- /* Make device dma capable by inheriting from parent's capabilities */
- set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
-
- ret = dma_coerce_mask_and_coherent(&rvdev->dev,
- dma_get_mask(rproc->dev.parent));
- if (ret) {
- dev_warn(&rvdev->dev,
- "Failed to set DMA mask %llx. Trying to continue... %x\n",
- dma_get_mask(rproc->dev.parent), ret);
- }
-
- /* parse the vrings */
- for (i = 0; i < rsc->num_of_vrings; i++) {
- ret = rproc_parse_vring(rvdev, rsc, i);
- if (ret)
- goto free_rvdev;
- }
-
- /* allocate the vring resources */
- for (i = 0; i < rsc->num_of_vrings; i++) {
- ret = rproc_alloc_vring(rvdev, i);
- if (ret)
- goto free_vg;
- }
-
- rvdev->subdev.start = rproc_vdev_do_start;
- rvdev->subdev.stop = rproc_vdev_do_stop;
-
- rproc_add_subdev(rproc, &rvdev->subdev);
-
- return 0;
-
-free_vg:
- for (i--; i >= 0; i--) {
- struct rproc_vring *rvring = &rvdev->vring[i];
-
- rproc_free_vring(rvring);
- }
-
-free_rvdev:
- device_unregister(&rvdev->dev);
-
- return ret;
-}
-
-static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
-{
- struct rproc *rproc = rvdev->rproc;
- struct rproc_vring *rvring;
- int id;
-
- for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
- rvring = &rvdev->vring[id];
- rproc_free_vring(rvring);
- }
-
- rproc_remove_subdev(rproc, &rvdev->subdev);
- device_unregister(&rvdev->dev);
-}
-
/**
* rproc_handle_vdev() - handle a vdev fw resource
* @rproc: the remote processor
@@ -590,14 +478,14 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
list_add_tail(&rvdev->node, &rproc->rvdevs);
- return rproc_rvdev_add_device(rvdev);
+ return rproc_virtio_device_add(rvdev);
}
void rproc_vdev_release(struct kref *ref)
{
struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
- rproc_rvdev_remove_device(rvdev);
+ rproc_virtio_device_remove(rvdev);
list_del(&rvdev->node);
}
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 493ef9262411..fad95f1a50c1 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -30,8 +30,8 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
void rproc_vdev_release(struct kref *ref);
/* from remoteproc_virtio.c */
-int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id);
-int rproc_remove_virtio_dev(struct device *dev, void *data);
+int rproc_virtio_device_add(struct rproc_vdev *rvdev);
+void rproc_virtio_device_remove(struct rproc_vdev *rvdev);
/* from remoteproc_debugfs.c */
void rproc_remove_trace_file(struct dentry *tfile);
@@ -47,6 +47,7 @@ extern struct class rproc_class;
int rproc_init_sysfs(void);
void rproc_exit_sysfs(void);
+int rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i);
void rproc_free_vring(struct rproc_vring *rvring);
int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 8c07cb2ca8ba..0f7efac7d4f3 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -296,6 +296,20 @@ static const struct virtio_config_ops rproc_virtio_config_ops = {
.set = rproc_virtio_set,
};
+/**
+ * rproc_rvdev_release() - release the existence of a rvdev
+ *
+ * @dev: the subdevice's dev
+ */
+static void rproc_virtio_rvdev_release(struct device *dev)
+{
+ struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
+
+ of_reserved_mem_device_release(dev);
+
+ kfree(rvdev);
+}
+
/*
* This function is called whenever vdev is released, and is responsible
* to decrement the remote processor's refcount which was taken when vdev was
@@ -318,16 +332,18 @@ static void rproc_virtio_dev_release(struct device *dev)
}
/**
- * rproc_add_virtio_dev() - register an rproc-induced virtio device
- * @rvdev: the remote vdev
+ * rproc_vdev_start() - register an rproc-induced virtio device
+ * @subdev: the rproc virtio subdevice
*
* This function registers a virtio device. This vdev's partent is
* the rproc device.
*
* Returns 0 on success or an appropriate error value otherwise.
*/
-int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
+static int rproc_vitio_start(struct rproc_subdev *subdev)
{
+ struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
+ subdev);
struct rproc *rproc = rvdev->rproc;
struct device *dev = &rvdev->dev;
struct virtio_device *vdev;
@@ -376,7 +392,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
ret = -ENOMEM;
goto out;
}
- vdev->id.device = id,
+ vdev->id.device = rvdev->id,
vdev->config = &rproc_virtio_config_ops,
vdev->dev.parent = dev;
vdev->dev.release = rproc_virtio_dev_release;
@@ -401,23 +417,121 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
goto out;
}
- dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
+ dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev),
+ rvdev->id);
out:
return ret;
}
+static int rproc_remove_virtio_dev(struct device *dev, void *data)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+ struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
+ struct rproc_vring *rvring;
+ int id;
+
+ unregister_virtio_device(vdev);
+
+ for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
+ rvring = &rvdev->vring[id];
+ rproc_free_vring(rvring);
+ }
+
+ return 0;
+}
/**
* rproc_remove_virtio_dev() - remove an rproc-induced virtio device
- * @dev: the virtio device
- * @data: must be null
+ * @subdev: the rproc virtio subdevice
+ * @crashed: indicate if the stop is the result of a crash
*
- * This function unregisters an existing virtio device.
+ * This function unregisters existing virtio devices.
*/
-int rproc_remove_virtio_dev(struct device *dev, void *data)
+static void rproc_vitio_stop(struct rproc_subdev *subdev, bool crashed)
{
- struct virtio_device *vdev = dev_to_virtio(dev);
+ struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
+ subdev);
+ int ret;
+
+ ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
+ if (ret)
+ dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n",
+ ret);
+}
+
+static const struct rproc_subdev rproc_virtio_subdev = {
+ .start = rproc_vitio_start,
+ .stop = rproc_vitio_stop
+};
+
+int rproc_virtio_device_add(struct rproc_vdev *rvdev)
+{
+ struct rproc *rproc = rvdev->rproc;
+ struct fw_rsc_vdev *rsc = rvdev->rsc;
+ char name[16];
+ int ret, i;
+
+ /* Initialise vdev subdevice */
+ snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
+ rvdev->dev.parent = &rproc->dev;
+ rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
+ rvdev->dev.release = rproc_virtio_rvdev_release;
+ dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
+ dev_set_drvdata(&rvdev->dev, rvdev);
+
+ ret = device_register(&rvdev->dev);
+ if (ret) {
+ put_device(&rvdev->dev);
+ return ret;
+ }
+ /* Make device dma capable by inheriting from parent's capabilities */
+ set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+
+ ret = dma_coerce_mask_and_coherent(&rvdev->dev,
+ dma_get_mask(rproc->dev.parent));
+ if (ret) {
+ dev_warn(&rvdev->dev,
+ "Failed to set DMA mask %llx. Trying to continue... %x\n",
+ dma_get_mask(rproc->dev.parent), ret);
+ }
+
+ /* parse the vrings */
+ for (i = 0; i < rsc->num_of_vrings; i++) {
+ ret = rproc_parse_vring(rvdev, rsc, i);
+ if (ret)
+ goto free_rvdev;
+ }
+
+ /* allocate the vring resources */
+ for (i = 0; i < rsc->num_of_vrings; i++) {
+ ret = rproc_alloc_vring(rvdev, i);
+ if (ret)
+ goto free_vg;
+ }
+
+ rvdev->subdev = rproc_virtio_subdev;
+
+ rproc_add_subdev(rproc, &rvdev->subdev);
- unregister_virtio_device(vdev);
return 0;
+
+free_vg:
+ for (i--; i >= 0; i--) {
+ struct rproc_vring *rvring = &rvdev->vring[i];
+
+ rproc_free_vring(rvring);
+ }
+
+free_rvdev:
+ device_unregister(&rvdev->dev);
+
+ return ret;
+}
+
+void rproc_virtio_device_remove(struct rproc_vdev *rvdev)
+{
+ struct rproc *rproc = rvdev->rproc;
+
+ rproc_remove_subdev(rproc, &rvdev->subdev);
+ device_unregister(&rvdev->dev);
}
--
2.17.1
As virtio is now a platform device. We need to ensure that the virtio
device is initialized before subdev ops are called.
Add a rproc master component to synchronize child devices bind/unbind
before the resource allocation and the firmware start.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 96 +++++++++++++++++++++++++++-
include/linux/remoteproc.h | 4 ++
2 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5dcef62d8d1d..cb40aae12b98 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -16,6 +16,7 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <linux/component.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/device.h>
@@ -42,6 +43,8 @@
#define HIGH_BITS_MASK 0xFFFFFFFF00000000ULL
+#define BIND_TIMEOUT_MS 1000
+
static DEFINE_MUTEX(rproc_list_mutex);
static LIST_HEAD(rproc_list);
@@ -410,6 +413,45 @@ void rproc_free_vring(struct rproc_vring *rvring)
rsc->vring[idx].notifyid = -1;
}
+static int rproc_compare_of(struct device *dev, void *data)
+{
+ if (dev->of_node)
+ return dev->of_node == data;
+ else
+ return dev == data;
+}
+
+static void rproc_release_of(struct device *dev, void *data)
+{
+ if (dev->of_node)
+ of_node_put(data);
+}
+
+static void rproc_unbind(struct device *dev)
+{
+ struct rproc *rproc = container_of(dev, struct rproc, dev);
+
+ /* undbind all child components */
+ component_unbind_all(dev, NULL);
+ complete(&rproc->completed);
+}
+
+static int rproc_bind(struct device *dev)
+{
+ struct rproc *rproc = container_of(dev, struct rproc, dev);
+
+ /* bind all child components */
+ rproc->bind_status = component_bind_all(dev, NULL);
+ complete(&rproc->completed);
+
+ return rproc->bind_status;
+}
+
+static const struct component_master_ops rproc_cmp_ops = {
+ .bind = rproc_bind,
+ .unbind = rproc_unbind,
+};
+
/**
* rproc_handle_vdev() - handle a vdev fw resource
* @rproc: the remote processor
@@ -475,6 +517,10 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
"failed to create rproc-virtio device\n");
return ret;
}
+ /* register a component associated to the virtio platform */
+ component_match_add_release(&pdev->dev, &rproc->match,
+ rproc_release_of, rproc_compare_of,
+ &pdev->dev);
rproc->nb_vdev++;
return 0;
@@ -1318,20 +1364,51 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
goto clean_up_resources;
}
+ /*
+ * bind all the children associated to the resources before starting
+ * the remote processor. This synchro point ensures that everything is
+ * ready to run.
+ */
+ init_completion(&rproc->completed);
+ if (rproc->match) {
+ ret = component_master_add_with_match(dev, &rproc_cmp_ops,
+ rproc->match);
+ if (ret) {
+ dev_err(dev, "failed to bind rproc\n");
+ goto clean_up_resources;
+ }
+ }
+ /* Wait for all children to be bound */
+ if (!wait_for_completion_timeout(&rproc->completed,
+ msecs_to_jiffies(BIND_TIMEOUT_MS))) {
+ dev_err(dev, "failed to bind child device(s)\n");
+ ret = -ETIMEDOUT;
+ goto clean_up_resources;
+ }
+
+ ret = rproc->bind_status;
+ if (ret) {
+ dev_err(dev, "failed to bind\n");
+ goto clean_up_resources;
+ }
+
/* Allocate carveout resources associated to rproc */
ret = rproc_alloc_registered_carveouts(rproc);
if (ret) {
dev_err(dev, "Failed to allocate associated carveouts: %d\n",
ret);
- goto clean_up_resources;
+ goto unbind_comp;
}
ret = rproc_start(rproc, fw);
if (ret)
- goto clean_up_resources;
+ goto unbind_comp;
return 0;
+unbind_comp:
+ component_master_del(dev, &rproc_cmp_ops);
+ rproc->match = NULL;
clean_up_resources:
rproc_resource_cleanup(rproc);
kfree(rproc->cached_table);
@@ -1733,6 +1810,21 @@ void rproc_shutdown(struct rproc *rproc)
goto out;
}
+ /*
+ * Unbind all the children before cleaning resources. This synchro
+ * point ensures that everything has been released before resources are
+ * freed.
+ */
+ init_completion(&rproc->completed);
+
+ component_master_del(dev, &rproc_cmp_ops);
+ rproc->match = NULL;
+
+ if (!wait_for_completion_timeout(&rproc->completed,
+ msecs_to_jiffies(BIND_TIMEOUT_MS))) {
+ dev_err(dev, "failed to unbind child device(s)\n");
+ }
+
/* clean up all acquired resources */
rproc_resource_cleanup(rproc);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index eb5bd568f11e..d7235f7356e2 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -514,6 +514,10 @@ struct rproc {
bool auto_boot;
struct list_head dump_segments;
int nb_vdev;
+ struct component_match *match;
+ struct completion completed;
+ int bind_status;
+
};
/**
--
2.17.1
Add component to declare bind and unbind functions. Theses functions
are used to ensure that the remoteproc virtio device is probed
and registered as a subdev of the rproc device before rproc
request the the prepare and start of the subdevice.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_virtio.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 2a0f33ccd929..e1d7371d2d64 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -10,6 +10,7 @@
* Brian Swetland <[email protected]>
*/
+#include <linux/component.h>
#include <linux/dma-mapping.h>
#include <linux/export.h>
#include <linux/module.h>
@@ -426,7 +427,8 @@ static const struct rproc_subdev rproc_virtio_subdev = {
.stop = rproc_vitio_stop
};
-static int rproc_virtio_bind(struct device *dev)
+static int rproc_virtio_bind(struct device *dev, struct device *master,
+ void *data)
{
struct rproc_vdev *rvdev = dev_get_drvdata(dev);
struct rproc *rproc = rvdev->rproc;
@@ -483,7 +485,8 @@ static int rproc_virtio_bind(struct device *dev)
return ret;
}
-static void rproc_virtio_unbind(struct device *dev)
+static void rproc_virtio_unbind(struct device *dev, struct device *master,
+ void *data)
{
struct rproc_vdev *rvdev = dev_get_drvdata(dev);
struct rproc *rproc = rvdev->rproc;
@@ -504,6 +507,11 @@ static void rproc_virtio_unbind(struct device *dev)
dev_dbg(dev, "virtio dev %d unbound\n", rvdev->index);
}
+static const struct component_ops rproc_virtio_ops = {
+ .bind = rproc_virtio_bind,
+ .unbind = rproc_virtio_unbind,
+};
+
static int rproc_virtio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -548,14 +556,21 @@ static int rproc_virtio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rvdev);
+ ret = component_add(&pdev->dev, &rproc_virtio_ops);
+ if (ret)
+ return ret;
+
rproc_register_rvdev(rproc, rvdev);
- return rproc_virtio_bind(dev);
+ return 0;
}
static int rproc_virtio_remove(struct platform_device *pdev)
{
- rproc_virtio_unbind(&pdev->dev);
+ struct rproc_vdev *rvdev = dev_get_drvdata(&pdev->dev);
+
+ component_del(&pdev->dev, &rproc_virtio_ops);
+ rproc_unregister_rvdev(rvdev);
return 0;
}
--
2.17.1
In preparation of the migration of the management of rvdev in
rproc_virtio, this patch spins off new functions to manage the
virtio device.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 149 +++++++++++++++------------
1 file changed, 83 insertions(+), 66 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 2a0425ab82a7..5c90d569c0f7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -441,6 +441,86 @@ static void rproc_rvdev_release(struct device *dev)
kfree(rvdev);
}
+static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
+{
+ struct rproc *rproc = rvdev->rproc;
+ struct fw_rsc_vdev *rsc = rvdev->rsc;
+ char name[16];
+ int ret, i;
+
+ /* Initialise vdev subdevice */
+ snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
+ rvdev->dev.parent = &rproc->dev;
+ rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
+ rvdev->dev.release = rproc_rvdev_release;
+ dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
+ dev_set_drvdata(&rvdev->dev, rvdev);
+
+ ret = device_register(&rvdev->dev);
+ if (ret) {
+ put_device(&rvdev->dev);
+ return ret;
+ }
+ /* Make device dma capable by inheriting from parent's capabilities */
+ set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+
+ ret = dma_coerce_mask_and_coherent(&rvdev->dev,
+ dma_get_mask(rproc->dev.parent));
+ if (ret) {
+ dev_warn(&rvdev->dev,
+ "Failed to set DMA mask %llx. Trying to continue... %x\n",
+ dma_get_mask(rproc->dev.parent), ret);
+ }
+
+ /* parse the vrings */
+ for (i = 0; i < rsc->num_of_vrings; i++) {
+ ret = rproc_parse_vring(rvdev, rsc, i);
+ if (ret)
+ goto free_rvdev;
+ }
+
+ /* allocate the vring resources */
+ for (i = 0; i < rsc->num_of_vrings; i++) {
+ ret = rproc_alloc_vring(rvdev, i);
+ if (ret)
+ goto free_vg;
+ }
+
+ rvdev->subdev.start = rproc_vdev_do_start;
+ rvdev->subdev.stop = rproc_vdev_do_stop;
+
+ rproc_add_subdev(rproc, &rvdev->subdev);
+
+ return 0;
+
+free_vg:
+ for (i--; i >= 0; i--) {
+ struct rproc_vring *rvring = &rvdev->vring[i];
+
+ rproc_free_vring(rvring);
+ }
+
+free_rvdev:
+ device_unregister(&rvdev->dev);
+
+ return ret;
+}
+
+static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
+{
+ struct rproc *rproc = rvdev->rproc;
+ struct rproc_vring *rvring;
+ int id;
+
+ for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
+ rvring = &rvdev->vring[id];
+ rproc_free_vring(rvring);
+ }
+
+ rproc_remove_subdev(rproc, &rvdev->subdev);
+ device_unregister(&rvdev->dev);
+}
+
/**
* rproc_handle_vdev() - handle a vdev fw resource
* @rproc: the remote processor
@@ -473,8 +553,6 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
{
struct device *dev = &rproc->dev;
struct rproc_vdev *rvdev;
- int i, ret;
- char name[16];
/* make sure resource isn't truncated */
if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
@@ -505,83 +583,22 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
kref_init(&rvdev->refcount);
rvdev->rsc = rsc;
+ rvdev->rsc_offset = offset;
rvdev->id = rsc->id;
rvdev->rproc = rproc;
rvdev->index = rproc->nb_vdev++;
- /* Initialise vdev subdevice */
- snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
- rvdev->dev.parent = rproc->dev.parent;
- rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
- rvdev->dev.release = rproc_rvdev_release;
- dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
- dev_set_drvdata(&rvdev->dev, rvdev);
-
- ret = device_register(&rvdev->dev);
- if (ret) {
- put_device(&rvdev->dev);
- return ret;
- }
- /* Make device dma capable by inheriting from parent's capabilities */
- set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
-
- ret = dma_coerce_mask_and_coherent(&rvdev->dev,
- dma_get_mask(rproc->dev.parent));
- if (ret) {
- dev_warn(dev,
- "Failed to set DMA mask %llx. Trying to continue... %x\n",
- dma_get_mask(rproc->dev.parent), ret);
- }
-
- /* parse the vrings */
- for (i = 0; i < rsc->num_of_vrings; i++) {
- ret = rproc_parse_vring(rvdev, rsc, i);
- if (ret)
- goto free_rvdev;
- }
-
- /* remember the resource offset*/
- rvdev->rsc_offset = offset;
-
- /* allocate the vring resources */
- for (i = 0; i < rsc->num_of_vrings; i++) {
- ret = rproc_alloc_vring(rvdev, i);
- if (ret)
- goto unwind_vring_allocations;
- }
-
list_add_tail(&rvdev->node, &rproc->rvdevs);
- rvdev->subdev.start = rproc_vdev_do_start;
- rvdev->subdev.stop = rproc_vdev_do_stop;
-
- rproc_add_subdev(rproc, &rvdev->subdev);
-
- return 0;
-
-unwind_vring_allocations:
- for (i--; i >= 0; i--)
- rproc_free_vring(&rvdev->vring[i]);
-free_rvdev:
- device_unregister(&rvdev->dev);
- return ret;
+ return rproc_rvdev_add_device(rvdev);
}
void rproc_vdev_release(struct kref *ref)
{
struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
- struct rproc_vring *rvring;
- struct rproc *rproc = rvdev->rproc;
- int id;
-
- for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
- rvring = &rvdev->vring[id];
- rproc_free_vring(rvring);
- }
- rproc_remove_subdev(rproc, &rvdev->subdev);
+ rproc_rvdev_remove_device(rvdev);
list_del(&rvdev->node);
- device_unregister(&rvdev->dev);
}
/**
--
2.17.1
Declare a virtio device as sub device of the stm32 remote proc
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
arch/arm/boot/dts/stm32mp15xx-dkx.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
index f6672e87aef3..2bb16c860c82 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
@@ -414,6 +414,16 @@
interrupt-parent = <&exti>;
interrupts = <68 1>;
status = "okay";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vdev@0 {
+ memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
+ compatible = "rproc-virtio";
+ reg = <0>;
+ status = "okay";
+ };
};
&pwr_regulators {
--
2.17.1
Add pa_to_da ops to translate the physical address into
device address.
Signed-off-by: Arnaud Pouliquen <[email protected]>
Change-Id: Ie1aa26769635d6d4c4581486700c4061f0f99ff1
---
drivers/remoteproc/stm32_rproc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index a18f88044111..1dd4c0f9c423 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -507,6 +507,7 @@ static struct rproc_ops st_rproc_ops = {
.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
.sanity_check = rproc_elf_sanity_check,
.get_boot_addr = rproc_elf_get_boot_addr,
+ .pa_to_da = stm32_rproc_pa_to_da,
};
static const struct of_device_id stm32_rproc_match[] = {
--
2.17.1
Add the possibility to declare memories associated to the virtio paltform
in the device tree.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_virtio.c | 77 +++++++++++++++++++++++---
1 file changed, 68 insertions(+), 9 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0fd938afd146..3302ee7d6c14 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -610,12 +610,71 @@ static const struct component_ops rproc_virtio_ops = {
.unbind = rproc_virtio_unbind,
};
+static int rproc_virtio_of_parse(struct device *dev, struct rproc_vdev *rvdev)
+{
+ struct device_node *np = dev->of_node;
+ struct rproc *rproc = rvdev->rproc;
+ struct of_phandle_iterator it;
+ struct rproc_mem_entry *mem;
+ struct reserved_mem *rmem;
+ char name[32];
+ u64 da;
+ int index = 0;
+
+ /* the reg is used to specify the vdev index */
+ if (of_property_read_u32(np, "reg", &rvdev->index))
+ return -EINVAL;
+ /* Register associated reserved memory regions */
+ of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+
+ while (of_phandle_iterator_next(&it) == 0) {
+ rmem = of_reserved_mem_lookup(it.node);
+ if (!rmem) {
+ dev_err(dev, "unable to acquire memory-region\n");
+ return -EINVAL;
+ }
+
+ if (rproc_pa_to_da(rproc, rmem->base, &da) < 0) {
+ dev_err(dev, "memory region not valid %pa\n",
+ &rmem->base);
+ return -EINVAL;
+ }
+
+ snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
+ if (strcmp(it.node->name, name)) {
+ /* Register memory region */
+ mem = rproc_mem_entry_init(dev, NULL,
+ (dma_addr_t)rmem->base,
+ rmem->size, da,
+ rproc_default_mem_alloc,
+ rproc_default_mem_release,
+ it.node->name);
+
+ if (mem)
+ rproc_coredump_add_segment(rproc, da,
+ rmem->size);
+ } else {
+ /* Register reserved memory for vdev buffer alloc */
+ mem = rproc_of_resm_mem_entry_init(dev, index,
+ rmem->size,
+ rmem->base,
+ it.node->name);
+ }
+
+ if (!mem)
+ return -ENOMEM;
+
+ rproc_add_carveout(rproc, mem);
+ index++;
+ }
+
+ return 0;
+}
+
static int rproc_virtio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node;
struct rproc_vdev *rvdev;
- struct rproc *rproc;
int ret;
rvdev = devm_kzalloc(&pdev->dev, sizeof(*rvdev), GFP_KERNEL);
@@ -627,18 +686,16 @@ static int rproc_virtio_probe(struct platform_device *pdev)
* The platform device is declared in the device tree
* retrieve rproc struct through the remoteproc platform
*/
- rproc = rproc_get_by_node(dev->parent->of_node);
+ rvdev->rproc = rproc_get_by_node(dev->parent->of_node);
- /* the reg is used to specify the vdev index */
- if (of_property_read_u32(np, "reg", &rvdev->index))
- return -EINVAL;
+ ret = rproc_virtio_of_parse(dev, rvdev);
} else {
struct rproc_vdev_data *vdev_data = pdev->dev.platform_data;
if (!vdev_data)
return -EINVAL;
- rproc = container_of(dev->parent, struct rproc, dev);
+ rvdev->rproc = container_of(dev->parent, struct rproc, dev);
rvdev->rsc_offset = vdev_data->rsc_offset;
rvdev->rsc = vdev_data->rsc;
@@ -649,8 +706,10 @@ static int rproc_virtio_probe(struct platform_device *pdev)
if (ret)
return ret;
}
+ if (ret)
+ return ret;
+
rvdev->pdev = pdev;
- rvdev->rproc = rproc;
platform_set_drvdata(pdev, rvdev);
@@ -658,7 +717,7 @@ static int rproc_virtio_probe(struct platform_device *pdev)
if (ret)
return ret;
- rproc_register_rvdev(rproc, rvdev);
+ rproc_register_rvdev(rvdev->rproc, rvdev);
return 0;
}
--
2.17.1
Manage the vdev as a platform device. The platform device is created
when a vdev resource is detected in the resource table.
The rvdev is now managed in rproc_virtio platform.
The rdev->refcount is now useless.
Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 57 +++---
drivers/remoteproc/remoteproc_internal.h | 10 +-
drivers/remoteproc/remoteproc_virtio.c | 213 ++++++++++++++---------
include/linux/remoteproc.h | 9 +-
4 files changed, 175 insertions(+), 114 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 32056449bb72..5dcef62d8d1d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -31,6 +31,7 @@
#include <linux/idr.h>
#include <linux/elf.h>
#include <linux/crc32.h>
+#include <linux/of_platform.h>
#include <linux/of_reserved_mem.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
@@ -440,7 +441,9 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
int offset, int avail)
{
struct device *dev = &rproc->dev;
- struct rproc_vdev *rvdev;
+ struct rproc_vdev_data vdev_data;
+ struct platform_device *pdev;
+ int ret;
/* make sure resource isn't truncated */
if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
@@ -455,37 +458,43 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
return -EINVAL;
}
- dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%x, cfg len %d, %d vrings\n",
- rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
+ vdev_data.rsc_offset = offset;
+ vdev_data.rsc = rsc;
+ vdev_data.id = rsc->id;
+ vdev_data.index = rproc->nb_vdev;
- /* we currently support only two vrings per rvdev */
- if (rsc->num_of_vrings > ARRAY_SIZE(rvdev->vring)) {
- dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
- return -EINVAL;
+ dev_dbg(dev, "%s: rsc_offset = %d rsc = %p id = %d\n",
+ __func__, vdev_data.rsc_offset, vdev_data.rsc, vdev_data.id);
+
+ pdev = platform_device_register_data(dev, "rproc-virtio",
+ rproc->nb_vdev, &vdev_data,
+ sizeof(vdev_data));
+ ret = PTR_ERR_OR_ZERO(pdev);
+ if (ret) {
+ dev_err(rproc->dev.parent,
+ "failed to create rproc-virtio device\n");
+ return ret;
}
+ rproc->nb_vdev++;
- rvdev = kzalloc(sizeof(*rvdev), GFP_KERNEL);
- if (!rvdev)
- return -ENOMEM;
+ return 0;
+}
- kref_init(&rvdev->refcount);
+static void rproc_vdev_release(struct rproc_vdev *rvdev)
+{
+ struct device *dev = &rvdev->pdev->dev;
- rvdev->rsc = rsc;
- rvdev->rsc_offset = offset;
- rvdev->id = rsc->id;
- rvdev->rproc = rproc;
- rvdev->index = rproc->nb_vdev++;
+ if (!dev->of_node)
+ platform_device_unregister(rvdev->pdev);
+}
+void rproc_register_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev)
+{
list_add_tail(&rvdev->node, &rproc->rvdevs);
-
- return rproc_virtio_device_add(rvdev);
}
-void rproc_vdev_release(struct kref *ref)
+void rproc_unregister_rvdev(struct rproc_vdev *rvdev)
{
- struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
-
- rproc_virtio_device_remove(rvdev);
list_del(&rvdev->node);
}
@@ -1192,9 +1201,9 @@ static void rproc_resource_cleanup(struct rproc *rproc)
kfree(entry);
}
- /* clean up remote vdev entries */
+ /* clean up virtio platform devices */
list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node)
- kref_put(&rvdev->refcount, rproc_vdev_release);
+ rproc_vdev_release(rvdev);
rproc_coredump_cleanup(rproc);
}
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index fad95f1a50c1..f5eaffac2fcd 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -24,10 +24,18 @@ struct rproc_debug_trace {
struct rproc_mem_entry trace_mem;
};
+struct rproc_vdev_data {
+ u32 rsc_offset;
+ struct fw_rsc_vdev *rsc;
+ unsigned int id;
+ unsigned int index;
+};
+
/* from remoteproc_core.c */
void rproc_release(struct kref *kref);
irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
-void rproc_vdev_release(struct kref *ref);
+void rproc_register_rvdev(struct rproc *rproc, struct rproc_vdev *rvdev);
+void rproc_unregister_rvdev(struct rproc_vdev *rvdev);
/* from remoteproc_virtio.c */
int rproc_virtio_device_add(struct rproc_vdev *rvdev);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0f7efac7d4f3..2a0f33ccd929 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -2,6 +2,7 @@
/*
* Remote processor messaging transport (OMAP platform-specific bits)
*
+ * Copyright (C) 2020 ST STMicroelectronics.
* Copyright (C) 2011 Texas Instruments, Inc.
* Copyright (C) 2011 Google, Inc.
*
@@ -11,14 +12,15 @@
#include <linux/dma-mapping.h>
#include <linux/export.h>
+#include <linux/module.h>
#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
#include <linux/remoteproc.h>
#include <linux/virtio.h>
#include <linux/virtio_config.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
#include <linux/err.h>
-#include <linux/kref.h>
#include <linux/slab.h>
#include "remoteproc_internal.h"
@@ -296,41 +298,23 @@ static const struct virtio_config_ops rproc_virtio_config_ops = {
.set = rproc_virtio_set,
};
-/**
- * rproc_rvdev_release() - release the existence of a rvdev
- *
- * @dev: the subdevice's dev
- */
-static void rproc_virtio_rvdev_release(struct device *dev)
-{
- struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
-
- of_reserved_mem_device_release(dev);
-
- kfree(rvdev);
-}
-
-/*
- * This function is called whenever vdev is released, and is responsible
- * 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
- * core when needed.
- */
static void rproc_virtio_dev_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);
- kfree(vdev);
-
- kref_put(&rvdev->refcount, rproc_vdev_release);
-
put_device(&rproc->dev);
}
+static void rproc_virtio_dev_init(struct rproc_vdev *rvdev)
+{
+ memset(&rvdev->vdev, 0, sizeof(rvdev->vdev));
+ rvdev->vdev.dev.parent = &rvdev->pdev->dev;
+ rvdev->vdev.dev.release = rproc_virtio_dev_release;
+ rvdev->vdev.config = &rproc_virtio_config_ops;
+ rvdev->vdev.id.device = rvdev->id;
+}
+
/**
* rproc_vdev_start() - register an rproc-induced virtio device
* @subdev: the rproc virtio subdevice
@@ -345,8 +329,8 @@ static int rproc_vitio_start(struct rproc_subdev *subdev)
struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
subdev);
struct rproc *rproc = rvdev->rproc;
- struct device *dev = &rvdev->dev;
- struct virtio_device *vdev;
+ struct device *dev = &rvdev->pdev->dev;
+ struct virtio_device *vdev = &rvdev->vdev;
struct rproc_mem_entry *mem;
int ret;
@@ -387,29 +371,15 @@ static int rproc_vitio_start(struct rproc_subdev *subdev)
}
/* Allocate virtio device */
- vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
- if (!vdev) {
- ret = -ENOMEM;
- goto out;
- }
- vdev->id.device = rvdev->id,
- vdev->config = &rproc_virtio_config_ops,
- vdev->dev.parent = dev;
- vdev->dev.release = rproc_virtio_dev_release;
+ rproc_virtio_dev_init(rvdev);
/*
* We're indirectly making a non-temporary copy of the rproc pointer
* here, because drivers probed with this vdev will indirectly
* access the wrapping rproc.
- *
- * Therefore we must increment the rproc refcount here, and decrement
- * it _only_ when the vdev is released.
*/
get_device(&rproc->dev);
- /* Reference the vdev and vring allocations */
- kref_get(&rvdev->refcount);
-
ret = register_virtio_device(vdev);
if (ret) {
put_device(&vdev->dev);
@@ -427,17 +397,9 @@ static int rproc_vitio_start(struct rproc_subdev *subdev)
static int rproc_remove_virtio_dev(struct device *dev, void *data)
{
struct virtio_device *vdev = dev_to_virtio(dev);
- struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
- struct rproc_vring *rvring;
- int id;
unregister_virtio_device(vdev);
- for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
- rvring = &rvdev->vring[id];
- rproc_free_vring(rvring);
- }
-
return 0;
}
/**
@@ -451,12 +413,12 @@ static void rproc_vitio_stop(struct rproc_subdev *subdev, bool crashed)
{
struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
subdev);
+ struct device *dev = &rvdev->pdev->dev;
int ret;
- ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
+ ret = device_for_each_child(dev, NULL, rproc_remove_virtio_dev);
if (ret)
- dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n",
- ret);
+ dev_warn(dev, "can't remove vdev child device: %d\n", ret);
}
static const struct rproc_subdev rproc_virtio_subdev = {
@@ -464,42 +426,35 @@ static const struct rproc_subdev rproc_virtio_subdev = {
.stop = rproc_vitio_stop
};
-int rproc_virtio_device_add(struct rproc_vdev *rvdev)
+static int rproc_virtio_bind(struct device *dev)
{
+ struct rproc_vdev *rvdev = dev_get_drvdata(dev);
struct rproc *rproc = rvdev->rproc;
struct fw_rsc_vdev *rsc = rvdev->rsc;
- char name[16];
int ret, i;
- /* Initialise vdev subdevice */
- snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
- rvdev->dev.parent = &rproc->dev;
- rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
- rvdev->dev.release = rproc_virtio_rvdev_release;
- dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
- dev_set_drvdata(&rvdev->dev, rvdev);
+ /* test if a resource has been associated to the vdev */
+ if (!rvdev->rsc)
+ return 0;
- ret = device_register(&rvdev->dev);
- if (ret) {
- put_device(&rvdev->dev);
- return ret;
- }
- /* Make device dma capable by inheriting from parent's capabilities */
- set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+ dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%x, cfg len %d, %d vrings\n",
+ rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
- ret = dma_coerce_mask_and_coherent(&rvdev->dev,
- dma_get_mask(rproc->dev.parent));
- if (ret) {
- dev_warn(&rvdev->dev,
- "Failed to set DMA mask %llx. Trying to continue... %x\n",
- dma_get_mask(rproc->dev.parent), ret);
+ /* we currently support only two vrings per rvdev */
+ if (rsc->num_of_vrings > ARRAY_SIZE(rvdev->vring)) {
+ dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
+ return -EINVAL;
}
+ /* If no resource table or no vdev nothing to do */
+ if (!rsc || !rsc->num_of_vrings)
+ return 0;
+
/* parse the vrings */
for (i = 0; i < rsc->num_of_vrings; i++) {
ret = rproc_parse_vring(rvdev, rsc, i);
if (ret)
- goto free_rvdev;
+ return ret;
}
/* allocate the vring resources */
@@ -510,9 +465,12 @@ int rproc_virtio_device_add(struct rproc_vdev *rvdev)
}
rvdev->subdev = rproc_virtio_subdev;
+ rvdev->id = rsc->id;
rproc_add_subdev(rproc, &rvdev->subdev);
+ dev_dbg(dev, "virtio dev %d bound\n", rvdev->index);
+
return 0;
free_vg:
@@ -522,16 +480,103 @@ int rproc_virtio_device_add(struct rproc_vdev *rvdev)
rproc_free_vring(rvring);
}
-free_rvdev:
- device_unregister(&rvdev->dev);
-
return ret;
}
-void rproc_virtio_device_remove(struct rproc_vdev *rvdev)
+static void rproc_virtio_unbind(struct device *dev)
{
+ struct rproc_vdev *rvdev = dev_get_drvdata(dev);
struct rproc *rproc = rvdev->rproc;
+ struct rproc_vring *rvring;
+ int id;
+
+ /*test if a resource has been associated to the vdev */
+ if (!rvdev->rsc)
+ return;
+
+ for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
+ rvring = &rvdev->vring[id];
+ rproc_free_vring(rvring);
+ }
rproc_remove_subdev(rproc, &rvdev->subdev);
- device_unregister(&rvdev->dev);
+
+ dev_dbg(dev, "virtio dev %d unbound\n", rvdev->index);
}
+
+static int rproc_virtio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct rproc_vdev *rvdev;
+ struct rproc *rproc;
+ int ret;
+
+ rvdev = devm_kzalloc(&pdev->dev, sizeof(*rvdev), GFP_KERNEL);
+ if (!rvdev)
+ return -ENOMEM;
+
+ if (dev->of_node) {
+ /*
+ * The platform device is declared in the device tree
+ * retrieve rproc struct through the remoteproc platform
+ */
+ rproc = rproc_get_by_node(dev->parent->of_node);
+
+ /* the reg is used to specify the vdev index */
+ if (of_property_read_u32(np, "reg", &rvdev->index))
+ return -EINVAL;
+ } else {
+ struct rproc_vdev_data *vdev_data = pdev->dev.platform_data;
+
+ if (!vdev_data)
+ return -EINVAL;
+
+ rproc = container_of(dev->parent, struct rproc, dev);
+
+ rvdev->rsc_offset = vdev_data->rsc_offset;
+ rvdev->rsc = vdev_data->rsc;
+ rvdev->id = vdev_data->id;
+ rvdev->index = vdev_data->index;
+
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+ }
+ rvdev->pdev = pdev;
+ rvdev->rproc = rproc;
+
+ platform_set_drvdata(pdev, rvdev);
+
+ rproc_register_rvdev(rproc, rvdev);
+
+ return rproc_virtio_bind(dev);
+}
+
+static int rproc_virtio_remove(struct platform_device *pdev)
+{
+ rproc_virtio_unbind(&pdev->dev);
+
+ return 0;
+}
+
+/* Platform driver */
+static const struct of_device_id rproc_virtio_match[] = {
+ { .compatible = "rproc-virtio", },
+ {},
+};
+
+static struct platform_driver rproc_virtio_driver = {
+ .probe = rproc_virtio_probe,
+ .remove = rproc_virtio_remove,
+ .driver = {
+ .name = "rproc-virtio",
+ .of_match_table = rproc_virtio_match,
+ },
+};
+
+module_platform_driver(rproc_virtio_driver);
+
+MODULE_AUTHOR("Arnaud Pouliquen <[email protected]>");
+MODULE_DESCRIPTION("Platform bus driver for rproc based virtio devices");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index ab9762563ae0..eb5bd568f11e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -560,8 +560,8 @@ struct rproc_vring {
/**
* struct rproc_vdev - remoteproc state for a supported virtio device
- * @refcount: reference counter for the vdev and vring allocations
* @subdev: handle for registering the vdev as a rproc subdevice
+ * @pdev: remoteproc virtio platform device
* @id: virtio device id (as in virtio_ids.h)
* @node: list node
* @rproc: the rproc handle
@@ -572,10 +572,9 @@ struct rproc_vring {
* @index: vdev position versus other vdev declared in resource table
*/
struct rproc_vdev {
- struct kref refcount;
-
struct rproc_subdev subdev;
- struct device dev;
+ struct platform_device *pdev;
+ struct virtio_device vdev;
unsigned int id;
struct list_head node;
@@ -628,7 +627,7 @@ int rproc_coredump_add_custom_segment(struct rproc *rproc,
static inline struct rproc_vdev *vdev_to_rvdev(struct virtio_device *vdev)
{
- return container_of(vdev->dev.parent, struct rproc_vdev, dev);
+ return container_of(vdev, struct rproc_vdev, vdev);
}
static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
--
2.17.1
Hey Arnaud,
I have started to review this set. Comments will come in over the next few days
and I will be sure to let you know when I'm done.
On Thu, Apr 16, 2020 at 06:13:15PM +0200, Arnaud Pouliquen wrote:
> In preparation of the migration of the management of rvdev in
> rproc_virtio, this patch spins off new functions to manage the
> virtio device.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 149 +++++++++++++++------------
> 1 file changed, 83 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2a0425ab82a7..5c90d569c0f7 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -441,6 +441,86 @@ static void rproc_rvdev_release(struct device *dev)
> kfree(rvdev);
> }
>
> +static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct fw_rsc_vdev *rsc = rvdev->rsc;
> + char name[16];
> + int ret, i;
> +
> + /* Initialise vdev subdevice */
> + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> + rvdev->dev.parent = &rproc->dev;
> + rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> + rvdev->dev.release = rproc_rvdev_release;
> + dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> + dev_set_drvdata(&rvdev->dev, rvdev);
> +
> + ret = device_register(&rvdev->dev);
> + if (ret) {
> + put_device(&rvdev->dev);
> + return ret;
> + }
> + /* Make device dma capable by inheriting from parent's capabilities */
> + set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> +
> + ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> + dma_get_mask(rproc->dev.parent));
> + if (ret) {
> + dev_warn(&rvdev->dev,
> + "Failed to set DMA mask %llx. Trying to continue... %x\n",
> + dma_get_mask(rproc->dev.parent), ret);
> + }
> +
> + /* parse the vrings */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_parse_vring(rvdev, rsc, i);
> + if (ret)
> + goto free_rvdev;
> + }
> +
> + /* allocate the vring resources */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_alloc_vring(rvdev, i);
> + if (ret)
> + goto free_vg;
I don't get the "free_vg" part... At the moment this patch is only about
refactoring and as such I would encourage you to keep things the same as
much as possible. It is certainly ok to make modifications but they should be
done in an incremental patch. Otherwise reviewers needlessly have to scrutinize
the changes thinking there is something more to figure out.
> + }
> +
> + rvdev->subdev.start = rproc_vdev_do_start;
> + rvdev->subdev.stop = rproc_vdev_do_stop;
> +
> + rproc_add_subdev(rproc, &rvdev->subdev);
> +
> + return 0;
> +
> +free_vg:
> + for (i--; i >= 0; i--) {
> + struct rproc_vring *rvring = &rvdev->vring[i];
> +
> + rproc_free_vring(rvring);
> + }
> +
> +free_rvdev:
> + device_unregister(&rvdev->dev);
> +
> + return ret;
> +}
> +
> +static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct rproc_vring *rvring;
> + int id;
> +
> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> + rvring = &rvdev->vring[id];
> + rproc_free_vring(rvring);
> + }
> +
> + rproc_remove_subdev(rproc, &rvdev->subdev);
> + device_unregister(&rvdev->dev);
> +}
> +
> /**
> * rproc_handle_vdev() - handle a vdev fw resource
> * @rproc: the remote processor
> @@ -473,8 +553,6 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
> {
> struct device *dev = &rproc->dev;
> struct rproc_vdev *rvdev;
> - int i, ret;
> - char name[16];
>
> /* make sure resource isn't truncated */
> if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
> @@ -505,83 +583,22 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
> kref_init(&rvdev->refcount);
>
> rvdev->rsc = rsc;
> + rvdev->rsc_offset = offset;
> rvdev->id = rsc->id;
> rvdev->rproc = rproc;
> rvdev->index = rproc->nb_vdev++;
>
> - /* Initialise vdev subdevice */
> - snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> - rvdev->dev.parent = rproc->dev.parent;
> - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> - rvdev->dev.release = rproc_rvdev_release;
> - dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> - dev_set_drvdata(&rvdev->dev, rvdev);
> -
> - ret = device_register(&rvdev->dev);
> - if (ret) {
> - put_device(&rvdev->dev);
> - return ret;
> - }
> - /* Make device dma capable by inheriting from parent's capabilities */
> - set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> -
> - ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> - dma_get_mask(rproc->dev.parent));
> - if (ret) {
> - dev_warn(dev,
> - "Failed to set DMA mask %llx. Trying to continue... %x\n",
> - dma_get_mask(rproc->dev.parent), ret);
> - }
> -
> - /* parse the vrings */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_parse_vring(rvdev, rsc, i);
> - if (ret)
> - goto free_rvdev;
> - }
> -
> - /* remember the resource offset*/
> - rvdev->rsc_offset = offset;
> -
> - /* allocate the vring resources */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_alloc_vring(rvdev, i);
> - if (ret)
> - goto unwind_vring_allocations;
> - }
> -
> list_add_tail(&rvdev->node, &rproc->rvdevs);
>
> - rvdev->subdev.start = rproc_vdev_do_start;
> - rvdev->subdev.stop = rproc_vdev_do_stop;
> -
> - rproc_add_subdev(rproc, &rvdev->subdev);
> -
> - return 0;
> -
> -unwind_vring_allocations:
> - for (i--; i >= 0; i--)
> - rproc_free_vring(&rvdev->vring[i]);
> -free_rvdev:
> - device_unregister(&rvdev->dev);
> - return ret;
> + return rproc_rvdev_add_device(rvdev);
> }
>
> void rproc_vdev_release(struct kref *ref)
> {
> struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
> - struct rproc_vring *rvring;
> - struct rproc *rproc = rvdev->rproc;
> - int id;
> -
> - for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> - rvring = &rvdev->vring[id];
> - rproc_free_vring(rvring);
> - }
>
> - rproc_remove_subdev(rproc, &rvdev->subdev);
> + rproc_rvdev_remove_device(rvdev);
At this time I don't see how introducing rproc_rvdev_remore_device() is
advantageous. Maybe I'll find an answer as I review upcoming patches...
Thanks,
Mathieu
> list_del(&rvdev->node);
> - device_unregister(&rvdev->dev);
> }
>
> /**
> --
> 2.17.1
>
On Thu, Apr 16, 2020 at 06:13:16PM +0200, Arnaud Pouliquen wrote:
> Migrate the management of the rvdev device and subdev from core
> to virtio, to prepare the rpmsg virtio platform device creation.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 118 +-------------------
> drivers/remoteproc/remoteproc_internal.h | 5 +-
> drivers/remoteproc/remoteproc_virtio.c | 136 +++++++++++++++++++++--
> 3 files changed, 131 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 5c90d569c0f7..4fcd685cbfd8 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -371,8 +371,7 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
> return 0;
> }
>
> -static int
> -rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
> +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;
> @@ -410,117 +409,6 @@ void rproc_free_vring(struct rproc_vring *rvring)
> rsc->vring[idx].notifyid = -1;
> }
>
> -static int rproc_vdev_do_start(struct rproc_subdev *subdev)
> -{
> - struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
> -
> - return rproc_add_virtio_dev(rvdev, rvdev->id);
> -}
> -
> -static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
> -{
> - struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
> - int ret;
> -
> - ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
> - if (ret)
> - dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
> -}
> -
> -/**
> - * rproc_rvdev_release() - release the existence of a rvdev
> - *
> - * @dev: the subdevice's dev
> - */
> -static void rproc_rvdev_release(struct device *dev)
> -{
> - struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
> -
> - of_reserved_mem_device_release(dev);
> -
> - kfree(rvdev);
> -}
> -
> -static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
> -{
> - struct rproc *rproc = rvdev->rproc;
> - struct fw_rsc_vdev *rsc = rvdev->rsc;
> - char name[16];
> - int ret, i;
> -
> - /* Initialise vdev subdevice */
> - snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> - rvdev->dev.parent = &rproc->dev;
> - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> - rvdev->dev.release = rproc_rvdev_release;
> - dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> - dev_set_drvdata(&rvdev->dev, rvdev);
> -
> - ret = device_register(&rvdev->dev);
> - if (ret) {
> - put_device(&rvdev->dev);
> - return ret;
> - }
> - /* Make device dma capable by inheriting from parent's capabilities */
> - set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> -
> - ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> - dma_get_mask(rproc->dev.parent));
> - if (ret) {
> - dev_warn(&rvdev->dev,
> - "Failed to set DMA mask %llx. Trying to continue... %x\n",
> - dma_get_mask(rproc->dev.parent), ret);
> - }
> -
> - /* parse the vrings */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_parse_vring(rvdev, rsc, i);
> - if (ret)
> - goto free_rvdev;
> - }
> -
> - /* allocate the vring resources */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_alloc_vring(rvdev, i);
> - if (ret)
> - goto free_vg;
> - }
> -
> - rvdev->subdev.start = rproc_vdev_do_start;
> - rvdev->subdev.stop = rproc_vdev_do_stop;
> -
> - rproc_add_subdev(rproc, &rvdev->subdev);
> -
> - return 0;
> -
> -free_vg:
> - for (i--; i >= 0; i--) {
> - struct rproc_vring *rvring = &rvdev->vring[i];
> -
> - rproc_free_vring(rvring);
> - }
> -
> -free_rvdev:
> - device_unregister(&rvdev->dev);
> -
> - return ret;
> -}
> -
> -static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
> -{
> - struct rproc *rproc = rvdev->rproc;
> - struct rproc_vring *rvring;
> - int id;
> -
> - for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> - rvring = &rvdev->vring[id];
> - rproc_free_vring(rvring);
> - }
> -
> - rproc_remove_subdev(rproc, &rvdev->subdev);
> - device_unregister(&rvdev->dev);
> -}
> -
> /**
> * rproc_handle_vdev() - handle a vdev fw resource
> * @rproc: the remote processor
> @@ -590,14 +478,14 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>
> list_add_tail(&rvdev->node, &rproc->rvdevs);
>
> - return rproc_rvdev_add_device(rvdev);
> + return rproc_virtio_device_add(rvdev);
> }
>
> void rproc_vdev_release(struct kref *ref)
> {
> struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
>
> - rproc_rvdev_remove_device(rvdev);
> + rproc_virtio_device_remove(rvdev);
> list_del(&rvdev->node);
> }
>
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index 493ef9262411..fad95f1a50c1 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -30,8 +30,8 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
> void rproc_vdev_release(struct kref *ref);
>
> /* from remoteproc_virtio.c */
> -int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id);
> -int rproc_remove_virtio_dev(struct device *dev, void *data);
> +int rproc_virtio_device_add(struct rproc_vdev *rvdev);
> +void rproc_virtio_device_remove(struct rproc_vdev *rvdev);
>
> /* from remoteproc_debugfs.c */
> void rproc_remove_trace_file(struct dentry *tfile);
> @@ -47,6 +47,7 @@ extern struct class rproc_class;
> int rproc_init_sysfs(void);
> void rproc_exit_sysfs(void);
>
> +int rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i);
> void rproc_free_vring(struct rproc_vring *rvring);
> int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 8c07cb2ca8ba..0f7efac7d4f3 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -296,6 +296,20 @@ static const struct virtio_config_ops rproc_virtio_config_ops = {
> .set = rproc_virtio_set,
> };
>
> +/**
> + * rproc_rvdev_release() - release the existence of a rvdev
> + *
> + * @dev: the subdevice's dev
> + */
> +static void rproc_virtio_rvdev_release(struct device *dev)
> +{
> + struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
> +
> + of_reserved_mem_device_release(dev);
> +
> + kfree(rvdev);
> +}
> +
> /*
> * This function is called whenever vdev is released, and is responsible
> * to decrement the remote processor's refcount which was taken when vdev was
> @@ -318,16 +332,18 @@ static void rproc_virtio_dev_release(struct device *dev)
> }
>
> /**
> - * rproc_add_virtio_dev() - register an rproc-induced virtio device
> - * @rvdev: the remote vdev
> + * rproc_vdev_start() - register an rproc-induced virtio device
s/rproc_vdev_start/rproc_virtio_start
> + * @subdev: the rproc virtio subdevice
> *
> * This function registers a virtio device. This vdev's partent is
> * the rproc device.
> *
> * Returns 0 on success or an appropriate error value otherwise.
> */
> -int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> +static int rproc_vitio_start(struct rproc_subdev *subdev)
s/rproc_vitio_start/rproc_virtio_start
> {
> + struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
> + subdev);
> struct rproc *rproc = rvdev->rproc;
> struct device *dev = &rvdev->dev;
> struct virtio_device *vdev;
> @@ -376,7 +392,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> ret = -ENOMEM;
> goto out;
> }
> - vdev->id.device = id,
> + vdev->id.device = rvdev->id,
> vdev->config = &rproc_virtio_config_ops,
> vdev->dev.parent = dev;
> vdev->dev.release = rproc_virtio_dev_release;
> @@ -401,23 +417,121 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> goto out;
> }
>
> - dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
> + dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev),
> + rvdev->id);
>
> out:
> return ret;
> }
>
> +static int rproc_remove_virtio_dev(struct device *dev, void *data)
> +{
> + struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> + struct rproc_vring *rvring;
> + int id;
> +
> + unregister_virtio_device(vdev);
> +
> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> + rvring = &rvdev->vring[id];
> + rproc_free_vring(rvring);
> + }
Why freeing the rings here and not in rproc_virtio_device_remove() as it was
done in rproc_rvdev_remove_device()? Renaming function and splitting them in
the same patch is extremely difficult to follow.
> +
> + return 0;
> +}
> /**
> * rproc_remove_virtio_dev() - remove an rproc-induced virtio device
> - * @dev: the virtio device
> - * @data: must be null
> + * @subdev: the rproc virtio subdevice
> + * @crashed: indicate if the stop is the result of a crash
> *
> - * This function unregisters an existing virtio device.
> + * This function unregisters existing virtio devices.
> */
> -int rproc_remove_virtio_dev(struct device *dev, void *data)
> +static void rproc_vitio_stop(struct rproc_subdev *subdev, bool crashed)
Same as above
> {
> - struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
> + subdev);
> + int ret;
> +
> + ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
> + if (ret)
> + dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n",
> + ret);
> +}
> +
> +static const struct rproc_subdev rproc_virtio_subdev = {
> + .start = rproc_vitio_start,
> + .stop = rproc_vitio_stop
> +};
> +
> +int rproc_virtio_device_add(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct fw_rsc_vdev *rsc = rvdev->rsc;
> + char name[16];
> + int ret, i;
> +
> + /* Initialise vdev subdevice */
> + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> + rvdev->dev.parent = &rproc->dev;
> + rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> + rvdev->dev.release = rproc_virtio_rvdev_release;
> + dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> + dev_set_drvdata(&rvdev->dev, rvdev);
> +
> + ret = device_register(&rvdev->dev);
> + if (ret) {
> + put_device(&rvdev->dev);
> + return ret;
> + }
> + /* Make device dma capable by inheriting from parent's capabilities */
> + set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> +
> + ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> + dma_get_mask(rproc->dev.parent));
> + if (ret) {
> + dev_warn(&rvdev->dev,
> + "Failed to set DMA mask %llx. Trying to continue... %x\n",
> + dma_get_mask(rproc->dev.parent), ret);
> + }
> +
> + /* parse the vrings */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_parse_vring(rvdev, rsc, i);
> + if (ret)
> + goto free_rvdev;
> + }
> +
> + /* allocate the vring resources */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_alloc_vring(rvdev, i);
> + if (ret)
> + goto free_vg;
> + }
> +
> + rvdev->subdev = rproc_virtio_subdev;
> +
> + rproc_add_subdev(rproc, &rvdev->subdev);
>
> - unregister_virtio_device(vdev);
> return 0;
> +
> +free_vg:
> + for (i--; i >= 0; i--) {
> + struct rproc_vring *rvring = &rvdev->vring[i];
> +
> + rproc_free_vring(rvring);
> + }
> +
> +free_rvdev:
> + device_unregister(&rvdev->dev);
> +
> + return ret;
> +}
> +
> +void rproc_virtio_device_remove(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> +
> + rproc_remove_subdev(rproc, &rvdev->subdev);
> + device_unregister(&rvdev->dev);
> }
> --
> 2.17.1
>
Hi Mathieu
On 4/21/20 10:41 PM, Mathieu Poirier wrote:
> Hey Arnaud,
>
> I have started to review this set. Comments will come in over the next few days
> and I will be sure to let you know when I'm done.
Take as much time you need, there is already a lot in your pipe.
This RFC could be probably split into a few series, but i preferred to keep all
together to have a whole picture. Aim of this RFC is to open the discussion on
the restructuring of the rproc_virtio and the use of components to synchronize child devices.
Thanks!
Arnaud
>
> On Thu, Apr 16, 2020 at 06:13:15PM +0200, Arnaud Pouliquen wrote:
>> In preparation of the migration of the management of rvdev in
>> rproc_virtio, this patch spins off new functions to manage the
>> virtio device.
>>
>> Signed-off-by: Arnaud Pouliquen <[email protected]>
>> ---
>> drivers/remoteproc/remoteproc_core.c | 149 +++++++++++++++------------
>> 1 file changed, 83 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index 2a0425ab82a7..5c90d569c0f7 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -441,6 +441,86 @@ static void rproc_rvdev_release(struct device *dev)
>> kfree(rvdev);
>> }
>>
>> +static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
>> +{
>> + struct rproc *rproc = rvdev->rproc;
>> + struct fw_rsc_vdev *rsc = rvdev->rsc;
>> + char name[16];
>> + int ret, i;
>> +
>> + /* Initialise vdev subdevice */
>> + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
>> + rvdev->dev.parent = &rproc->dev;
>> + rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
>> + rvdev->dev.release = rproc_rvdev_release;
>> + dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
>> + dev_set_drvdata(&rvdev->dev, rvdev);
>> +
>> + ret = device_register(&rvdev->dev);
>> + if (ret) {
>> + put_device(&rvdev->dev);
>> + return ret;
>> + }
>> + /* Make device dma capable by inheriting from parent's capabilities */
>> + set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
>> +
>> + ret = dma_coerce_mask_and_coherent(&rvdev->dev,
>> + dma_get_mask(rproc->dev.parent));
>> + if (ret) {
>> + dev_warn(&rvdev->dev,
>> + "Failed to set DMA mask %llx. Trying to continue... %x\n",
>> + dma_get_mask(rproc->dev.parent), ret);
>> + }
>> +
>> + /* parse the vrings */
>> + for (i = 0; i < rsc->num_of_vrings; i++) {
>> + ret = rproc_parse_vring(rvdev, rsc, i);
>> + if (ret)
>> + goto free_rvdev;
>> + }
>> +
>> + /* allocate the vring resources */
>> + for (i = 0; i < rsc->num_of_vrings; i++) {
>> + ret = rproc_alloc_vring(rvdev, i);
>> + if (ret)
>> + goto free_vg;
>
> I don't get the "free_vg" part... At the moment this patch is only about
> refactoring and as such I would encourage you to keep things the same as
> much as possible. It is certainly ok to make modifications but they should be
> done in an incremental patch. Otherwise reviewers needlessly have to scrutinize
> the changes thinking there is something more to figure out.
>
>> + }
>> +
>> + rvdev->subdev.start = rproc_vdev_do_start;
>> + rvdev->subdev.stop = rproc_vdev_do_stop;
>> +
>> + rproc_add_subdev(rproc, &rvdev->subdev);
>> +
>> + return 0;
>> +
>> +free_vg:
>> + for (i--; i >= 0; i--) {
>> + struct rproc_vring *rvring = &rvdev->vring[i];
>> +
>> + rproc_free_vring(rvring);
>> + }
>> +
>> +free_rvdev:
>> + device_unregister(&rvdev->dev);
>> +
>> + return ret;
>> +}
>> +
>> +static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
>> +{
>> + struct rproc *rproc = rvdev->rproc;
>> + struct rproc_vring *rvring;
>> + int id;
>> +
>> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
>> + rvring = &rvdev->vring[id];
>> + rproc_free_vring(rvring);
>> + }
>> +
>> + rproc_remove_subdev(rproc, &rvdev->subdev);
>> + device_unregister(&rvdev->dev);
>> +}
>> +
>> /**
>> * rproc_handle_vdev() - handle a vdev fw resource
>> * @rproc: the remote processor
>> @@ -473,8 +553,6 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>> {
>> struct device *dev = &rproc->dev;
>> struct rproc_vdev *rvdev;
>> - int i, ret;
>> - char name[16];
>>
>> /* make sure resource isn't truncated */
>> if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
>> @@ -505,83 +583,22 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>> kref_init(&rvdev->refcount);
>>
>> rvdev->rsc = rsc;
>> + rvdev->rsc_offset = offset;
>> rvdev->id = rsc->id;
>> rvdev->rproc = rproc;
>> rvdev->index = rproc->nb_vdev++;
>>
>> - /* Initialise vdev subdevice */
>> - snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
>> - rvdev->dev.parent = rproc->dev.parent;
>> - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
>> - rvdev->dev.release = rproc_rvdev_release;
>> - dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
>> - dev_set_drvdata(&rvdev->dev, rvdev);
>> -
>> - ret = device_register(&rvdev->dev);
>> - if (ret) {
>> - put_device(&rvdev->dev);
>> - return ret;
>> - }
>> - /* Make device dma capable by inheriting from parent's capabilities */
>> - set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
>> -
>> - ret = dma_coerce_mask_and_coherent(&rvdev->dev,
>> - dma_get_mask(rproc->dev.parent));
>> - if (ret) {
>> - dev_warn(dev,
>> - "Failed to set DMA mask %llx. Trying to continue... %x\n",
>> - dma_get_mask(rproc->dev.parent), ret);
>> - }
>> -
>> - /* parse the vrings */
>> - for (i = 0; i < rsc->num_of_vrings; i++) {
>> - ret = rproc_parse_vring(rvdev, rsc, i);
>> - if (ret)
>> - goto free_rvdev;
>> - }
>> -
>> - /* remember the resource offset*/
>> - rvdev->rsc_offset = offset;
>> -
>> - /* allocate the vring resources */
>> - for (i = 0; i < rsc->num_of_vrings; i++) {
>> - ret = rproc_alloc_vring(rvdev, i);
>> - if (ret)
>> - goto unwind_vring_allocations;
>> - }
>> -
>> list_add_tail(&rvdev->node, &rproc->rvdevs);
>>
>> - rvdev->subdev.start = rproc_vdev_do_start;
>> - rvdev->subdev.stop = rproc_vdev_do_stop;
>> -
>> - rproc_add_subdev(rproc, &rvdev->subdev);
>> -
>> - return 0;
>> -
>> -unwind_vring_allocations:
>> - for (i--; i >= 0; i--)
>> - rproc_free_vring(&rvdev->vring[i]);
>> -free_rvdev:
>> - device_unregister(&rvdev->dev);
>> - return ret;
>> + return rproc_rvdev_add_device(rvdev);
>> }
>>
>> void rproc_vdev_release(struct kref *ref)
>> {
>> struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
>> - struct rproc_vring *rvring;
>> - struct rproc *rproc = rvdev->rproc;
>> - int id;
>> -
>> - for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
>> - rvring = &rvdev->vring[id];
>> - rproc_free_vring(rvring);
>> - }
>>
>> - rproc_remove_subdev(rproc, &rvdev->subdev);
>> + rproc_rvdev_remove_device(rvdev);
>
> At this time I don't see how introducing rproc_rvdev_remore_device() is
> advantageous. Maybe I'll find an answer as I review upcoming patches...
>
> Thanks,
> Mathieu
>
>> list_del(&rvdev->node);
>> - device_unregister(&rvdev->dev);
>> }
>>
>> /**
>> --
>> 2.17.1
>>
This morning I'm attempting to take a fresh look at this set...
On Thu, Apr 16, 2020 at 06:13:15PM +0200, Arnaud Pouliquen wrote:
> In preparation of the migration of the management of rvdev in
> rproc_virtio, this patch spins off new functions to manage the
> virtio device.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 149 +++++++++++++++------------
> 1 file changed, 83 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2a0425ab82a7..5c90d569c0f7 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -441,6 +441,86 @@ static void rproc_rvdev_release(struct device *dev)
> kfree(rvdev);
> }
>
> +static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct fw_rsc_vdev *rsc = rvdev->rsc;
> + char name[16];
> + int ret, i;
> +
> + /* Initialise vdev subdevice */
> + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> + rvdev->dev.parent = &rproc->dev;
> + rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> + rvdev->dev.release = rproc_rvdev_release;
> + dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> + dev_set_drvdata(&rvdev->dev, rvdev);
> +
> + ret = device_register(&rvdev->dev);
> + if (ret) {
> + put_device(&rvdev->dev);
> + return ret;
> + }
> + /* Make device dma capable by inheriting from parent's capabilities */
> + set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> +
> + ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> + dma_get_mask(rproc->dev.parent));
> + if (ret) {
> + dev_warn(&rvdev->dev,
> + "Failed to set DMA mask %llx. Trying to continue... %x\n",
> + dma_get_mask(rproc->dev.parent), ret);
> + }
> +
> + /* parse the vrings */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_parse_vring(rvdev, rsc, i);
> + if (ret)
> + goto free_rvdev;
> + }
> +
> + /* allocate the vring resources */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_alloc_vring(rvdev, i);
> + if (ret)
> + goto free_vg;
> + }
> +
> + rvdev->subdev.start = rproc_vdev_do_start;
> + rvdev->subdev.stop = rproc_vdev_do_stop;
> +
> + rproc_add_subdev(rproc, &rvdev->subdev);
> +
> + return 0;
> +
> +free_vg:
> + for (i--; i >= 0; i--) {
> + struct rproc_vring *rvring = &rvdev->vring[i];
> +
> + rproc_free_vring(rvring);
> + }
> +
> +free_rvdev:
> + device_unregister(&rvdev->dev);
> +
> + return ret;
> +}
> +
> +static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct rproc_vring *rvring;
> + int id;
> +
> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> + rvring = &rvdev->vring[id];
> + rproc_free_vring(rvring);
> + }
> +
> + rproc_remove_subdev(rproc, &rvdev->subdev);
> + device_unregister(&rvdev->dev);
> +}
> +
> /**
> * rproc_handle_vdev() - handle a vdev fw resource
> * @rproc: the remote processor
> @@ -473,8 +553,6 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
> {
> struct device *dev = &rproc->dev;
> struct rproc_vdev *rvdev;
> - int i, ret;
> - char name[16];
>
> /* make sure resource isn't truncated */
> if (struct_size(rsc, vring, rsc->num_of_vrings) + rsc->config_len >
> @@ -505,83 +583,22 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
> kref_init(&rvdev->refcount);
>
> rvdev->rsc = rsc;
> + rvdev->rsc_offset = offset;
> rvdev->id = rsc->id;
> rvdev->rproc = rproc;
> rvdev->index = rproc->nb_vdev++;
>
> - /* Initialise vdev subdevice */
> - snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> - rvdev->dev.parent = rproc->dev.parent;
> - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> - rvdev->dev.release = rproc_rvdev_release;
> - dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> - dev_set_drvdata(&rvdev->dev, rvdev);
> -
> - ret = device_register(&rvdev->dev);
> - if (ret) {
> - put_device(&rvdev->dev);
> - return ret;
> - }
> - /* Make device dma capable by inheriting from parent's capabilities */
> - set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> -
> - ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> - dma_get_mask(rproc->dev.parent));
> - if (ret) {
> - dev_warn(dev,
> - "Failed to set DMA mask %llx. Trying to continue... %x\n",
> - dma_get_mask(rproc->dev.parent), ret);
> - }
> -
> - /* parse the vrings */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_parse_vring(rvdev, rsc, i);
> - if (ret)
> - goto free_rvdev;
> - }
> -
> - /* remember the resource offset*/
> - rvdev->rsc_offset = offset;
> -
> - /* allocate the vring resources */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_alloc_vring(rvdev, i);
> - if (ret)
> - goto unwind_vring_allocations;
> - }
> -
> list_add_tail(&rvdev->node, &rproc->rvdevs);
This should go in rproc_rvdev_add_device()
>
> - rvdev->subdev.start = rproc_vdev_do_start;
> - rvdev->subdev.stop = rproc_vdev_do_stop;
> -
> - rproc_add_subdev(rproc, &rvdev->subdev);
> -
> - return 0;
> -
> -unwind_vring_allocations:
> - for (i--; i >= 0; i--)
> - rproc_free_vring(&rvdev->vring[i]);
> -free_rvdev:
> - device_unregister(&rvdev->dev);
> - return ret;
> + return rproc_rvdev_add_device(rvdev);
> }
>
> void rproc_vdev_release(struct kref *ref)
> {
> struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
> - struct rproc_vring *rvring;
> - struct rproc *rproc = rvdev->rproc;
> - int id;
> -
> - for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> - rvring = &rvdev->vring[id];
> - rproc_free_vring(rvring);
> - }
>
> - rproc_remove_subdev(rproc, &rvdev->subdev);
> + rproc_rvdev_remove_device(rvdev);
> list_del(&rvdev->node);
> - device_unregister(&rvdev->dev);
Keep this function intact, rename it rproc_rvdev_remove_device() to balance out
rproc_rvdev_add_device() and modify rproc_resource_cleanup() to reflect the
change. I suppose we have nothing to loose since rproc_handle_vdev() and
rproc_vdev_release(), from a syntactic point of view, didn't balance each other
out.
> }
>
> /**
> --
> 2.17.1
>
On Thu, Apr 16, 2020 at 06:13:16PM +0200, Arnaud Pouliquen wrote:
> Migrate the management of the rvdev device and subdev from core
> to virtio, to prepare the rpmsg virtio platform device creation.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 118 +-------------------
> drivers/remoteproc/remoteproc_internal.h | 5 +-
> drivers/remoteproc/remoteproc_virtio.c | 136 +++++++++++++++++++++--
> 3 files changed, 131 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 5c90d569c0f7..4fcd685cbfd8 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -371,8 +371,7 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)
> return 0;
> }
>
> -static int
> -rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
> +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;
> @@ -410,117 +409,6 @@ void rproc_free_vring(struct rproc_vring *rvring)
> rsc->vring[idx].notifyid = -1;
> }
>
> -static int rproc_vdev_do_start(struct rproc_subdev *subdev)
> -{
> - struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
> -
> - return rproc_add_virtio_dev(rvdev, rvdev->id);
> -}
> -
> -static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
> -{
> - struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
> - int ret;
> -
> - ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
> - if (ret)
> - dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n", ret);
> -}
> -
> -/**
> - * rproc_rvdev_release() - release the existence of a rvdev
> - *
> - * @dev: the subdevice's dev
> - */
> -static void rproc_rvdev_release(struct device *dev)
> -{
> - struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
> -
> - of_reserved_mem_device_release(dev);
> -
> - kfree(rvdev);
> -}
> -
> -static int rproc_rvdev_add_device(struct rproc_vdev *rvdev)
> -{
> - struct rproc *rproc = rvdev->rproc;
> - struct fw_rsc_vdev *rsc = rvdev->rsc;
> - char name[16];
> - int ret, i;
> -
> - /* Initialise vdev subdevice */
> - snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> - rvdev->dev.parent = &rproc->dev;
> - rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> - rvdev->dev.release = rproc_rvdev_release;
> - dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> - dev_set_drvdata(&rvdev->dev, rvdev);
> -
> - ret = device_register(&rvdev->dev);
> - if (ret) {
> - put_device(&rvdev->dev);
> - return ret;
> - }
> - /* Make device dma capable by inheriting from parent's capabilities */
> - set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> -
> - ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> - dma_get_mask(rproc->dev.parent));
> - if (ret) {
> - dev_warn(&rvdev->dev,
> - "Failed to set DMA mask %llx. Trying to continue... %x\n",
> - dma_get_mask(rproc->dev.parent), ret);
> - }
> -
> - /* parse the vrings */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_parse_vring(rvdev, rsc, i);
> - if (ret)
> - goto free_rvdev;
> - }
> -
> - /* allocate the vring resources */
> - for (i = 0; i < rsc->num_of_vrings; i++) {
> - ret = rproc_alloc_vring(rvdev, i);
> - if (ret)
> - goto free_vg;
> - }
> -
> - rvdev->subdev.start = rproc_vdev_do_start;
> - rvdev->subdev.stop = rproc_vdev_do_stop;
> -
> - rproc_add_subdev(rproc, &rvdev->subdev);
> -
> - return 0;
> -
> -free_vg:
> - for (i--; i >= 0; i--) {
> - struct rproc_vring *rvring = &rvdev->vring[i];
> -
> - rproc_free_vring(rvring);
> - }
> -
> -free_rvdev:
> - device_unregister(&rvdev->dev);
> -
> - return ret;
> -}
> -
> -static void rproc_rvdev_remove_device(struct rproc_vdev *rvdev)
> -{
> - struct rproc *rproc = rvdev->rproc;
> - struct rproc_vring *rvring;
> - int id;
> -
> - for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> - rvring = &rvdev->vring[id];
> - rproc_free_vring(rvring);
> - }
> -
> - rproc_remove_subdev(rproc, &rvdev->subdev);
> - device_unregister(&rvdev->dev);
> -}
> -
> /**
> * rproc_handle_vdev() - handle a vdev fw resource
> * @rproc: the remote processor
> @@ -590,14 +478,14 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>
> list_add_tail(&rvdev->node, &rproc->rvdevs);
>
> - return rproc_rvdev_add_device(rvdev);
> + return rproc_virtio_device_add(rvdev);
> }
>
> void rproc_vdev_release(struct kref *ref)
> {
> struct rproc_vdev *rvdev = container_of(ref, struct rproc_vdev, refcount);
>
> - rproc_rvdev_remove_device(rvdev);
> + rproc_virtio_device_remove(rvdev);
> list_del(&rvdev->node);
> }
>
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index 493ef9262411..fad95f1a50c1 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -30,8 +30,8 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
> void rproc_vdev_release(struct kref *ref);
>
> /* from remoteproc_virtio.c */
> -int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id);
> -int rproc_remove_virtio_dev(struct device *dev, void *data);
> +int rproc_virtio_device_add(struct rproc_vdev *rvdev);
> +void rproc_virtio_device_remove(struct rproc_vdev *rvdev);
>
> /* from remoteproc_debugfs.c */
> void rproc_remove_trace_file(struct dentry *tfile);
> @@ -47,6 +47,7 @@ extern struct class rproc_class;
> int rproc_init_sysfs(void);
> void rproc_exit_sysfs(void);
>
> +int rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i);
> void rproc_free_vring(struct rproc_vring *rvring);
> int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 8c07cb2ca8ba..0f7efac7d4f3 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -296,6 +296,20 @@ static const struct virtio_config_ops rproc_virtio_config_ops = {
> .set = rproc_virtio_set,
> };
>
> +/**
> + * rproc_rvdev_release() - release the existence of a rvdev
> + *
> + * @dev: the subdevice's dev
> + */
> +static void rproc_virtio_rvdev_release(struct device *dev)
> +{
> + struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
> +
> + of_reserved_mem_device_release(dev);
> +
> + kfree(rvdev);
> +}
> +
> /*
> * This function is called whenever vdev is released, and is responsible
> * to decrement the remote processor's refcount which was taken when vdev was
> @@ -318,16 +332,18 @@ static void rproc_virtio_dev_release(struct device *dev)
> }
>
> /**
> - * rproc_add_virtio_dev() - register an rproc-induced virtio device
> - * @rvdev: the remote vdev
> + * rproc_vdev_start() - register an rproc-induced virtio device
> + * @subdev: the rproc virtio subdevice
> *
> * This function registers a virtio device. This vdev's partent is
> * the rproc device.
> *
> * Returns 0 on success or an appropriate error value otherwise.
> */
> -int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> +static int rproc_vitio_start(struct rproc_subdev *subdev)
> {
> + struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
> + subdev);
> struct rproc *rproc = rvdev->rproc;
> struct device *dev = &rvdev->dev;
> struct virtio_device *vdev;
> @@ -376,7 +392,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> ret = -ENOMEM;
> goto out;
> }
> - vdev->id.device = id,
> + vdev->id.device = rvdev->id,
> vdev->config = &rproc_virtio_config_ops,
> vdev->dev.parent = dev;
> vdev->dev.release = rproc_virtio_dev_release;
> @@ -401,23 +417,121 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
> goto out;
> }
>
> - dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
> + dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev),
> + rvdev->id);
>
> out:
> return ret;
> }
>
> +static int rproc_remove_virtio_dev(struct device *dev, void *data)
> +{
> + struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> + struct rproc_vring *rvring;
> + int id;
> +
> + unregister_virtio_device(vdev);
> +
> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> + rvring = &rvdev->vring[id];
> + rproc_free_vring(rvring);
> + }
> +
> + return 0;
> +}
> /**
> * rproc_remove_virtio_dev() - remove an rproc-induced virtio device
> - * @dev: the virtio device
> - * @data: must be null
> + * @subdev: the rproc virtio subdevice
> + * @crashed: indicate if the stop is the result of a crash
> *
> - * This function unregisters an existing virtio device.
> + * This function unregisters existing virtio devices.
> */
> -int rproc_remove_virtio_dev(struct device *dev, void *data)
> +static void rproc_vitio_stop(struct rproc_subdev *subdev, bool crashed)
> {
> - struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev,
> + subdev);
> + int ret;
> +
> + ret = device_for_each_child(&rvdev->dev, NULL, rproc_remove_virtio_dev);
> + if (ret)
> + dev_warn(&rvdev->dev, "can't remove vdev child device: %d\n",
> + ret);
> +}
> +
> +static const struct rproc_subdev rproc_virtio_subdev = {
> + .start = rproc_vitio_start,
> + .stop = rproc_vitio_stop
> +};
> +
> +int rproc_virtio_device_add(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> + struct fw_rsc_vdev *rsc = rvdev->rsc;
> + char name[16];
> + int ret, i;
> +
> + /* Initialise vdev subdevice */
> + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
> + rvdev->dev.parent = &rproc->dev;
> + rvdev->dev.dma_pfn_offset = rproc->dev.parent->dma_pfn_offset;
> + rvdev->dev.release = rproc_virtio_rvdev_release;
> + dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
> + dev_set_drvdata(&rvdev->dev, rvdev);
> +
> + ret = device_register(&rvdev->dev);
> + if (ret) {
> + put_device(&rvdev->dev);
> + return ret;
> + }
> + /* Make device dma capable by inheriting from parent's capabilities */
> + set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
> +
> + ret = dma_coerce_mask_and_coherent(&rvdev->dev,
> + dma_get_mask(rproc->dev.parent));
> + if (ret) {
> + dev_warn(&rvdev->dev,
> + "Failed to set DMA mask %llx. Trying to continue... %x\n",
> + dma_get_mask(rproc->dev.parent), ret);
> + }
> +
> + /* parse the vrings */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_parse_vring(rvdev, rsc, i);
> + if (ret)
> + goto free_rvdev;
> + }
> +
> + /* allocate the vring resources */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = rproc_alloc_vring(rvdev, i);
> + if (ret)
> + goto free_vg;
> + }
> +
> + rvdev->subdev = rproc_virtio_subdev;
> +
> + rproc_add_subdev(rproc, &rvdev->subdev);
>
> - unregister_virtio_device(vdev);
> return 0;
> +
> +free_vg:
> + for (i--; i >= 0; i--) {
> + struct rproc_vring *rvring = &rvdev->vring[i];
> +
> + rproc_free_vring(rvring);
> + }
> +
> +free_rvdev:
> + device_unregister(&rvdev->dev);
> +
> + return ret;
> +}
> +
> +void rproc_virtio_device_remove(struct rproc_vdev *rvdev)
> +{
> + struct rproc *rproc = rvdev->rproc;
> +
> + rproc_remove_subdev(rproc, &rvdev->subdev);
> + device_unregister(&rvdev->dev);
This used to be rproc_rvdev_remove_device(), and now it is
rproc_virtio_device_remove()... I would have expected
rproc_virtio_remove_device() to be consistent. At the very least, it will sure
make it easier for me to connect the dots.
Not only did the name flipped but freeing the vrings has been moved to
rproc_remove_virtio_dev() without explanation.
> }
> --
> 2.17.1
>
On Thu, Apr 16, 2020 at 06:13:13PM +0200, Arnaud Pouliquen wrote:
> This series proposes to introduce the notion of platform device for the
> rproc virtio management. One obective is to allow virtio to declare is
> own memory resources without the usage of dma_declare_coherent_memory
> that seems deprecated since the introduction of the device tree.
Just to follow up with the rest of the community...
During the openAMP remoteproc sub-group conference call [1] Arnaud and I have agreed
the best way forward for this patchset is to split it up and make a few
adjustment that will make it easier for people to review the work.
Thanks,
Mathieu
[1]. These conference call are open to anyone who wishes to participate.
>
> Proposal:
> - the rproc virtio is processed in a separate platform and could be handled
> as a generic platform device.
> - Several vdev devices can be declared in DT:
> - which allows to declare their own memory regions and answer to [1].
> - as next steps it would be also possible to:
> - declare their own mailboxes, rpmsg drivers, ...
> - use dma-range to handle the pa<->da translation at virtio level
>
> Several notions are introduced here:
> - Virtio platform registration which allows to decorelate virtio from the
> remote proc core device.
> - Synchronization of the child devices relying on component bind/unbind.
> This mechanism ensures the synchronization of the child devices before
> the boot of the remote processor and before the release of the resources
> on the remote processor shutdown.
> - Ability to populate child devices declared in rproc device tree node.
> - Possibility to declare the memory regions reserved to a virtio devices in
> the devicetree.
>
> Known limitations:
> - the patchset has been tested on a st32mP1 plaform only
> - it is based on the v5.6 (need to evoluate depending on V5.7 and on going
> patchsets).
> - The virtio memory allocation does not take into account memory
> controllers such as IOMMU and MPU.
>
> Arnaud Pouliquen (18):
> remoteproc: Store resource table address in rvdev
> remoteproc: Introduce virtio device add/remove functions in core.
> remoteproc: Move rvdev management in rproc_virtio
> remoteproc: Add rproc_get_by_node helper
> remoteproc: Create platform device for vdev
> remoteproc: Add component in core for child devices synchronization
> remoteproc: Add component bind/unbind for virtio platform
> remoteproc: Externalize carveout functions
> remoteproc: Move vring management from core to virtio
> remoteproc: Add capability to populate rproc subnode devices
> remoteproc: Add child node component in rproc match list
> remoteproc: Support of pre-registered virtio device
> remoteproc: Add memory default allocator helper
> remoteproc: Add pa to da translation API
> remoteproc: associate memory entry to a device
> remoteproc: Parse virtio node for memory region
> remoteproc: stm32: add the pa to da ops.
> ARM: dts: stm32: Declare a virtio device
>
> arch/arm/boot/dts/stm32mp15xx-dkx.dtsi | 10 +
> drivers/remoteproc/remoteproc_core.c | 469 ++++++++++++-----------
> drivers/remoteproc/remoteproc_internal.h | 23 +-
> drivers/remoteproc/remoteproc_virtio.c | 415 ++++++++++++++++++--
> drivers/remoteproc/stm32_rproc.c | 1 +
> include/linux/remoteproc.h | 27 +-
> 6 files changed, 673 insertions(+), 272 deletions(-)
>
> --
> 2.17.1
>