[PATCH 00/11] rproc subdevice support

[PATCH 00/11] rproc subdevice support

[Bjorn Andersson]

This series consist of three sets of patches:

The first set introduces the concept of subdevices - entities that are probed
and removed following the running state of a remoteproc. This is then used by
the Qualcomm WCNSS PIL to control the life cycle of the associated SMD channels
and devices.

The second set starts by decoupling vring allocation from the virtio device,
both to make the vring allocation follow the style of carveout allocations and
to allow the virtio devices to be implemented as subdevices.

With the second set in place the third set cleans up the vdev resource handling
and lastly cleans up the table_ptr vs cached_table handling.

Bjorn Andersson (11):
  remoteproc: Introduce subdevices
  rpmsg: smd: Expose edge registration functions
  remoteproc: wcnss: Bond SMD edge to remoteproc
  dt-binding: remoteproc: wcnss: Allow describing smd edge
  remoteproc: Assign kref to rproc_vdev
  remoteproc: virtio: Anchor vring life cycle in vdev
  remoteproc: Further extend the vdev life cycle
  remoteproc: Decouple vdev resources and devices
  remoteproc: Update max_notifyid as we allocate vrings
  remoteproc: Remove custom vdev handler list
  remoteproc: Merge table_ptr and cached_table pointers

 .../bindings/remoteproc/qcom,wcnss-pil.txt         |  30 +++-
 drivers/remoteproc/qcom_wcnss.c                    |  27 +++
 drivers/remoteproc/remoteproc_core.c               | 188 +++++++++++++++------
 drivers/remoteproc/remoteproc_internal.h           |   1 +
 drivers/remoteproc/remoteproc_virtio.c             |  17 +-
 drivers/rpmsg/qcom_smd.c                           |   1 +
 include/linux/remoteproc.h                         |  34 +++-
 include/linux/rpmsg/qcom_smd.h                     |  33 ++++
 8 files changed, 267 insertions(+), 64 deletions(-)
 create mode 100644 include/linux/rpmsg/qcom_smd.h

-- 
2.5.0

[PATCH 01/11] remoteproc: Introduce subdevices

[Bjorn Andersson]

A subdevice is an abstract entity that can be used to tie actions to the
booting and shutting down of a remote processor. The subdevice object is
expected to be embedded in concrete implementations, allowing for a
variety of use cases to be implemented.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 72 ++++++++++++++++++++++++++++++++++++
 include/linux/remoteproc.h           | 22 +++++++++++
 2 files changed, 94 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c6bfb3496684..4309996f4335 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -736,6 +736,34 @@ static int rproc_handle_resources(struct rproc *rproc, int len,
 	return ret;
 }
 
+static int rproc_probe_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+	int ret;
+
+	list_for_each_entry(subdev, &rproc->subdevs, node) {
+		ret = subdev->probe(subdev);
+		if (ret)
+			goto unroll_registration;
+	}
+
+	return 0;
+
+unroll_registration:
+	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
+		subdev->remove(subdev);
+
+	return ret;
+}
+
+static void rproc_remove_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+
+	list_for_each_entry(subdev, &rproc->subdevs, node)
+		subdev->remove(subdev);
+}
+
 /**
  * rproc_resource_cleanup() - clean up and free all acquired resources
  * @rproc: rproc handle
@@ -878,12 +906,22 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 		goto clean_up_resources;
 	}
 
+	/* probe any subdevices for the remote processor */
+	ret = rproc_probe_subdevices(rproc);
+	if (ret) {
+		dev_err(dev, "failed to probe subdevices for %s: %d\n",
+			rproc->name, ret);
+		goto stop_rproc;
+	}
+
 	rproc->state = RPROC_RUNNING;
 
 	dev_info(dev, "remote processor %s is now up\n", rproc->name);
 
 	return 0;
 
+stop_rproc:
+	rproc->ops->stop(rproc);
 clean_up_resources:
 	rproc_resource_cleanup(rproc);
 clean_up:
@@ -1121,6 +1159,9 @@ void rproc_shutdown(struct rproc *rproc)
 	if (!atomic_dec_and_test(&rproc->power))
 		goto out;
 
+	/* remove any subdevices for the remote processor */
+	rproc_remove_subdevices(rproc);
+
 	/* power off the remote processor */
 	ret = rproc->ops->stop(rproc);
 	if (ret) {
@@ -1370,6 +1411,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 	INIT_LIST_HEAD(&rproc->mappings);
 	INIT_LIST_HEAD(&rproc->traces);
 	INIT_LIST_HEAD(&rproc->rvdevs);
+	INIT_LIST_HEAD(&rproc->subdevs);
 
 	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
 	init_completion(&rproc->crash_comp);
@@ -1457,6 +1499,36 @@ int rproc_del(struct rproc *rproc)
 EXPORT_SYMBOL(rproc_del);
 
 /**
+ * rproc_add_subdev() - add a subdevice to a remoteproc
+ * @rproc: rproc handle to add the subdevice to
+ * @subdev: subdev handle to register
+ * @probe: function to call when the rproc boots
+ * @remove: function to call when the rproc shuts down
+ */
+void rproc_add_subdev(struct rproc *rproc,
+		      struct rproc_subdev *subdev,
+		      int (*probe)(struct rproc_subdev *subdev),
+		      void (*remove)(struct rproc_subdev *subdev))
+{
+	subdev->probe = probe;
+	subdev->remove = remove;
+
+	list_add_tail(&subdev->node, &rproc->subdevs);
+}
+EXPORT_SYMBOL(rproc_add_subdev);
+
+/**
+ * rproc_remove_subdev() - remove a subdevice from a remoteproc
+ * @rproc: rproc handle to remove the subdevice from
+ * @subdev: subdev handle, previously registered with rproc_add_subdev()
+ */
+void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
+{
+	list_del(&subdev->node);
+}
+EXPORT_SYMBOL(rproc_remove_subdev);
+
+/**
  * rproc_report_crash() - rproc crash reporter function
  * @rproc: remote processor
  * @type: crash type
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 930023b7c825..a05f4569a2a0 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -400,6 +400,7 @@ enum rproc_crash_type {
  * @firmware_loading_complete: marks e/o asynchronous firmware loading
  * @bootaddr: address of first instruction to boot rproc with (optional)
  * @rvdevs: list of remote virtio devices
+ * @subdevs: list of subdevices, to following the running state
  * @notifyids: idr for dynamically assigning rproc-wide unique notify ids
  * @index: index of this rproc device
  * @crash_handler: workqueue for handling a crash
@@ -431,6 +432,7 @@ struct rproc {
 	struct completion firmware_loading_complete;
 	u32 bootaddr;
 	struct list_head rvdevs;
+	struct list_head subdevs;
 	struct idr notifyids;
 	int index;
 	struct work_struct crash_handler;
@@ -444,6 +446,19 @@ struct rproc {
 	bool auto_boot;
 };
 
+/**
+ * struct rproc_subdev - subdevice tied to a remoteproc
+ * @node: list node related to the rproc subdevs list
+ * @probe: probe function, called as the rproc is started
+ * @remove: remove function, called as the rproc is stopped
+ */
+struct rproc_subdev {
+	struct list_head node;
+
+	int (*probe)(struct rproc_subdev *subdev);
+	void (*remove)(struct rproc_subdev *subdev);
+};
+
 /* we currently support only two vrings per rvdev */
 
 #define RVDEV_NUM_VRINGS 2
@@ -511,4 +526,11 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
 	return rvdev->rproc;
 }
 
+void rproc_add_subdev(struct rproc *rproc,
+		      struct rproc_subdev *subdev,
+		      int (*probe)(struct rproc_subdev *subdev),
+		      void (*remove)(struct rproc_subdev *subdev));
+
+void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
+
 #endif /* REMOTEPROC_H */
-- 
2.5.0

[PATCH 02/11] rpmsg: smd: Expose edge registration functions

[Bjorn Andersson]

The edge registration functions is to be used from a remoteproc driver
to register and unregister an edge as the remote processor comes and
goes.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/rpmsg/qcom_smd.c       |  1 +
 include/linux/rpmsg/qcom_smd.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 include/linux/rpmsg/qcom_smd.h

diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index 952338a6296a..aded7352f089 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -25,6 +25,7 @@
 #include <linux/soc/qcom/smem.h>
 #include <linux/wait.h>
 #include <linux/rpmsg.h>
+#include <linux/rpmsg/qcom_smd.h>
 
 #include "rpmsg_internal.h"
 
diff --git a/include/linux/rpmsg/qcom_smd.h b/include/linux/rpmsg/qcom_smd.h
new file mode 100644
index 000000000000..7c12637a500e
--- /dev/null
+++ b/include/linux/rpmsg/qcom_smd.h
@@ -0,0 +1,33 @@
+
+#ifndef _LINUX_RPMSG_QCOM_SMD_H
+#define _LINUX_RPMSG_QCOM_SMD_H
+
+#include <linux/device.h>
+
+struct qcom_smd_edge;
+
+#if IS_ENABLED(CONFIG_RPMSG_QCOM_SMD)
+
+struct qcom_smd_edge *qcom_smd_register_edge(struct device *parent,
+					     struct device_node *node);
+int qcom_smd_unregister_edge(struct qcom_smd_edge *edge);
+
+#else
+
+static inline struct qcom_smd_edge *
+qcom_smd_register_edge(struct device *parent,
+		       struct device_node *node)
+{
+	return ERR_PTR(-ENXIO);
+}
+
+static inline int qcom_smd_unregister_edge(struct qcom_smd_edge *edge)
+{
+	/* This shouldn't be possible */
+	WARN_ON(1);
+	return -ENXIO;
+}
+
+#endif
+
+#endif
-- 
2.5.0

[PATCH 03/11] remoteproc: wcnss: Bond SMD edge to remoteproc

[Bjorn Andersson]

Allow the wcnss smd edge to be described as a child of the wcnss
remoteproc node and make the edge life cycle follow the running state of
the remoteproc.

This bond is necessary to clean up the smd state when the remote
processor is suddenly removed, and in some cases even when it shut down
in a controlled fasion.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/qcom_wcnss.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index f5cedeaafba1..0c47fa286794 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -30,6 +30,7 @@
 #include <linux/remoteproc.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
+#include <linux/rpmsg/qcom_smd.h>
 
 #include "qcom_mdt_loader.h"
 #include "remoteproc_internal.h"
@@ -94,6 +95,10 @@ struct qcom_wcnss {
 	phys_addr_t mem_reloc;
 	void *mem_region;
 	size_t mem_size;
+
+	struct device_node *smd_node;
+	struct qcom_smd_edge *smd_edge;
+	struct rproc_subdev smd_subdev;
 };
 
 static const struct wcnss_data riva_data = {
@@ -396,6 +401,23 @@ static irqreturn_t wcnss_stop_ack_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int wcnss_smd_probe(struct rproc_subdev *subdev)
+{
+	struct qcom_wcnss *wcnss = container_of(subdev, struct qcom_wcnss, smd_subdev);
+
+	wcnss->smd_edge = qcom_smd_register_edge(wcnss->dev, wcnss->smd_node);
+
+	return IS_ERR(wcnss->smd_edge) ? PTR_ERR(wcnss->smd_edge) : 0;
+}
+
+static void wcnss_smd_remove(struct rproc_subdev *subdev)
+{
+	struct qcom_wcnss *wcnss = container_of(subdev, struct qcom_wcnss, smd_subdev);
+
+	qcom_smd_unregister_edge(wcnss->smd_edge);
+	wcnss->smd_edge = NULL;
+}
+
 static int wcnss_init_regulators(struct qcom_wcnss *wcnss,
 				 const struct wcnss_vreg_info *info,
 				 int num_vregs)
@@ -578,6 +600,10 @@ static int wcnss_probe(struct platform_device *pdev)
 		}
 	}
 
+	wcnss->smd_node = of_get_child_by_name(pdev->dev.of_node, "smd-edge");
+	if (wcnss->smd_node)
+		rproc_add_subdev(rproc, &wcnss->smd_subdev, wcnss_smd_probe, wcnss_smd_remove);
+
 	ret = rproc_add(rproc);
 	if (ret)
 		goto free_rproc;
@@ -596,6 +622,7 @@ static int wcnss_remove(struct platform_device *pdev)
 
 	of_platform_depopulate(&pdev->dev);
 
+	of_node_put(wcnss->smd_node);
 	qcom_smem_state_put(wcnss->state);
 	rproc_del(wcnss->rproc);
 	rproc_free(wcnss->rproc);
-- 
2.5.0

[PATCH 04/11] dt-binding: remoteproc: wcnss: Allow describing smd edge

[Bjorn Andersson]

Allow the associated smd edge to be described within the wcnss
remoteproc node. This creates a bond between the remoteproc and the
associated smd channels and devices, showing the interaction between the
two parts and provides both a natural reference to the other.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../bindings/remoteproc/qcom,wcnss-pil.txt         | 30 ++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt
index 0d2361ebe3d7..d420f84ddfb0 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt
@@ -60,8 +60,8 @@ on the Qualcomm WCNSS core.
 		    see ../reserved-memory/reserved-memory.txt
 
 = SUBNODES
-A single subnode of the WCNSS PIL describes the attached rf module and its
-resource dependencies.
+A required subnode of the WCNSS PIL is used to describe the attached rf module
+and its resource dependencies. It is described by the following properties:
 
 - compatible:
 	Usage: required
@@ -90,6 +90,11 @@ resource dependencies.
 	Definition: reference to the regulators to be held on behalf of the
 		    booting of the WCNSS core
 
+
+The wcnss node can also have an subnode named "smd-edge" that describes the SMD
+edge, channels and devices related to the WCNSS.
+See ../soc/qcom/qcom,smd.txt for details on how to describe the SMD edge.
+
 = EXAMPLE
 The following example describes the resources needed to boot control the WCNSS,
 with attached WCN3680, as it is commonly found on MSM8974 boards.
@@ -129,4 +134,25 @@ pronto@fb204000 {
 		vddpa-supply = <&pm8941_l19>;
 		vdddig-supply = <&pm8941_s3>;
 	};
+
+	smd-edge {
+		interrupts = <0 142 1>;
+
+		qcom,ipc = <&apcs 8 17>;
+		qcom,smd-edge = <6>;
+		qcom,remote-pid = <4>;
+
+		label = "pronto";
+
+		wcnss {
+			compatible = "qcom,wcnss";
+			qcom,smd-channels = "WCNSS_CTRL";
+
+			qcom,mmio = <&pronto>;
+
+			bt {
+				compatible = "qcom,wcnss-bt";
+			};
+		};
+	};
 };
-- 
2.5.0

Re: [PATCH 04/11] dt-binding: remoteproc: wcnss: Allow describing smd edge

[Rob Herring]

On Wed, Oct 19, 2016 at 07:40:05PM -0700, Bjorn Andersson wrote:
> Allow the associated smd edge to be described within the wcnss
> remoteproc node. This creates a bond between the remoteproc and the
> associated smd channels and devices, showing the interaction between the
> two parts and provides both a natural reference to the other.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../bindings/remoteproc/qcom,wcnss-pil.txt         | 30 ++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/11] remoteproc: Merge table_ptr and cached_table pointers

[Bjorn Andersson]

As all vdev resources are allocated before we boot the remote processor
we no longer need to support modifying the resource table while the
remote is running.

This saves us from the table_ptr dance, but more importantly allow the
remote processor to enable security lock down of the loaded table memory
region.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 26 ++++++++++----------------
 include/linux/remoteproc.h           |  4 +---
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index f31783d27196..46078c959a9c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -889,15 +889,13 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	/*
 	 * Create a copy of the resource table. When a virtio device starts
 	 * and calls vring_new_virtqueue() the address of the allocated vring
-	 * will be stored in the cached_table. Before the device is started,
-	 * cached_table will be copied into device memory.
+	 * will be stored in the table_ptr. Before the device is started,
+	 * table_ptr will be copied into device memory.
 	 */
-	rproc->cached_table = kmemdup(table, tablesz, GFP_KERNEL);
-	if (!rproc->cached_table)
+	rproc->table_ptr = kmemdup(table, tablesz, GFP_KERNEL);
+	if (!rproc->table_ptr)
 		goto clean_up;
 
-	rproc->table_ptr = rproc->cached_table;
-
 	/* reset max_notifyid */
 	rproc->max_notifyid = -1;
 
@@ -916,18 +914,16 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	}
 
 	/*
-	 * The starting device has been given the rproc->cached_table as the
+	 * The starting device has been given the rproc->table_ptr as the
 	 * resource table. The address of the vring along with the other
-	 * allocated resources (carveouts etc) is stored in cached_table.
+	 * allocated resources (carveouts etc) is stored in table_ptr.
 	 * In order to pass this information to the remote device we must copy
 	 * this information to device memory. We also update the table_ptr so
 	 * that any subsequent changes will be applied to the loaded version.
 	 */
 	loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
-	if (loaded_table) {
-		memcpy(loaded_table, rproc->cached_table, tablesz);
-		rproc->table_ptr = loaded_table;
-	}
+	if (loaded_table)
+		memcpy(loaded_table, rproc->table_ptr, tablesz);
 
 	/* power up the remote processor */
 	ret = rproc->ops->start(rproc);
@@ -955,8 +951,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 clean_up_resources:
 	rproc_resource_cleanup(rproc);
 clean_up:
-	kfree(rproc->cached_table);
-	rproc->cached_table = NULL;
+	kfree(rproc->table_ptr);
 	rproc->table_ptr = NULL;
 
 	rproc_disable_iommu(rproc);
@@ -1206,8 +1201,7 @@ void rproc_shutdown(struct rproc *rproc)
 	rproc_disable_iommu(rproc);
 
 	/* Free the copy of the resource table */
-	kfree(rproc->cached_table);
-	rproc->cached_table = NULL;
+	kfree(rproc->table_ptr);
 	rproc->table_ptr = NULL;
 
 	/* if in crash state, unlock crash handler */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index ff05b88730ad..51662ee7d032 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -408,8 +408,7 @@ enum rproc_crash_type {
  * @crash_comp: completion used to sync crash handler and the rproc reload
  * @recovery_disabled: flag that state if recovery was disabled
  * @max_notifyid: largest allocated notify id.
- * @table_ptr: pointer to the resource table in effect
- * @cached_table: copy of the resource table
+ * @table_ptr: our copy of the resource table
  * @has_iommu: flag to indicate if remote processor is behind an MMU
  */
 struct rproc {
@@ -441,7 +440,6 @@ struct rproc {
 	bool recovery_disabled;
 	int max_notifyid;
 	struct resource_table *table_ptr;
-	struct resource_table *cached_table;
 	bool has_iommu;
 	bool auto_boot;
 };
-- 
2.5.0