[PATCH 0/5] remoteproc: updates for new events

[PATCH 0/5] remoteproc: updates for new events

[Alex Elder]

This series changes the prototype for rproc_add_subdev().  The
caller is now responsible for populating the function pointers
recorded in the rproc_subdev structure, rather than having them be
passed as arguments.  These two existing function pointers have been
renamed ("probe" is now "start" and "remove" is now "stop"), and
they are now optional.  Callback functions may now also be assigned
for two new events (prior to start and after stop).

					-Alex

Alex Elder (1):
  remoteproc: rename subdev probe and remove functions

Bjorn Andersson (4):
  remoteproc: Rename subdev functions to start/stop
  remoteproc: Make start and stop in subdev optional
  remoteproc: Make client initialize ops in rproc_subdev
  remoteproc: Introduce prepare and unprepare for subdevices

 drivers/remoteproc/qcom_common.c     |  26 ++++---
 drivers/remoteproc/qcom_sysmon.c     |   5 +-
 drivers/remoteproc/remoteproc_core.c | 110 ++++++++++++++++++++-------
 include/linux/remoteproc.h           |  19 ++---
 4 files changed, 109 insertions(+), 51 deletions(-)

-- 
2.17.0

[PATCH 1/5] remoteproc: Rename subdev functions to start/stop

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

"start" and "stop" are more suitable names for how these two operations
are used, and they fit better with the upcoming introduction of two
additional operations in the struct.

[elder@linaro.org: minor comment edits]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 30 ++++++++++++++--------------
 include/linux/remoteproc.h           | 14 ++++++-------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a9609d971f7f..5dd58e6bea88 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -774,13 +774,13 @@ static int rproc_handle_resources(struct rproc *rproc,
 	return ret;
 }
 
-static int rproc_probe_subdevices(struct rproc *rproc)
+static int rproc_start_subdevices(struct rproc *rproc)
 {
 	struct rproc_subdev *subdev;
 	int ret;
 
 	list_for_each_entry(subdev, &rproc->subdevs, node) {
-		ret = subdev->probe(subdev);
+		ret = subdev->start(subdev);
 		if (ret)
 			goto unroll_registration;
 	}
@@ -789,17 +789,17 @@ static int rproc_probe_subdevices(struct rproc *rproc)
 
 unroll_registration:
 	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
-		subdev->remove(subdev, true);
+		subdev->stop(subdev, true);
 
 	return ret;
 }
 
-static void rproc_remove_subdevices(struct rproc *rproc, bool crashed)
+static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 {
 	struct rproc_subdev *subdev;
 
 	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
-		subdev->remove(subdev, crashed);
+		subdev->stop(subdev, crashed);
 }
 
 /**
@@ -901,8 +901,8 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 		return ret;
 	}
 
-	/* probe any subdevices for the remote processor */
-	ret = rproc_probe_subdevices(rproc);
+	/* Start any subdevices for the remote processor */
+	ret = rproc_start_subdevices(rproc);
 	if (ret) {
 		dev_err(dev, "failed to probe subdevices for %s: %d\n",
 			rproc->name, ret);
@@ -1014,8 +1014,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
 	struct device *dev = &rproc->dev;
 	int ret;
 
-	/* remove any subdevices for the remote processor */
-	rproc_remove_subdevices(rproc, crashed);
+	/* Stop any subdevices for the remote processor */
+	rproc_stop_subdevices(rproc, crashed);
 
 	/* the installed resource table is no longer accessible */
 	rproc->table_ptr = rproc->cached_table;
@@ -1657,16 +1657,16 @@ 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
+ * @start: function to call after the rproc is started
+ * @stop: function to call before the rproc is stopped
  */
 void rproc_add_subdev(struct rproc *rproc,
 		      struct rproc_subdev *subdev,
-		      int (*probe)(struct rproc_subdev *subdev),
-		      void (*remove)(struct rproc_subdev *subdev, bool crashed))
+		      int (*start)(struct rproc_subdev *subdev),
+		      void (*stop)(struct rproc_subdev *subdev, bool crashed))
 {
-	subdev->probe = probe;
-	subdev->remove = remove;
+	subdev->start = start;
+	subdev->stop = stop;
 
 	list_add_tail(&subdev->node, &rproc->subdevs);
 }
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index dfdaede9139e..bf55bf2a5ee1 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -477,15 +477,15 @@ struct rproc {
 /**
  * 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 being stopped, the @crashed
- *	    parameter indicates if this originates from the a recovery
+ * @start: start function, called after the rproc has been started
+ * @stop: stop function, called before the rproc is stopped; the @crashed
+ *	    parameter indicates if this originates from a recovery
  */
 struct rproc_subdev {
 	struct list_head node;
 
-	int (*probe)(struct rproc_subdev *subdev);
-	void (*remove)(struct rproc_subdev *subdev, bool crashed);
+	int (*start)(struct rproc_subdev *subdev);
+	void (*stop)(struct rproc_subdev *subdev, bool crashed);
 };
 
 /* we currently support only two vrings per rvdev */
@@ -568,8 +568,8 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
 
 void rproc_add_subdev(struct rproc *rproc,
 		      struct rproc_subdev *subdev,
-		      int (*probe)(struct rproc_subdev *subdev),
-		      void (*remove)(struct rproc_subdev *subdev, bool crashed));
+		      int (*start)(struct rproc_subdev *subdev),
+		      void (*stop)(struct rproc_subdev *subdev, bool crashed));
 
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
-- 
2.17.0

[PATCH 2/5] remoteproc: Make start and stop in subdev optional

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Some subdevices, such as glink ssr only care about the stop operation,
so make the operations optional to reduce client code.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5dd58e6bea88..981ae6dff145 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -780,16 +780,20 @@ static int rproc_start_subdevices(struct rproc *rproc)
 	int ret;
 
 	list_for_each_entry(subdev, &rproc->subdevs, node) {
-		ret = subdev->start(subdev);
-		if (ret)
-			goto unroll_registration;
+		if (subdev->start) {
+			ret = subdev->start(subdev);
+			if (ret)
+				goto unroll_registration;
+		}
 	}
 
 	return 0;
 
 unroll_registration:
-	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
-		subdev->stop(subdev, true);
+	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->stop)
+			subdev->stop(subdev, true);
+	}
 
 	return ret;
 }
@@ -798,8 +802,10 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 {
 	struct rproc_subdev *subdev;
 
-	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
-		subdev->stop(subdev, crashed);
+	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->stop)
+			subdev->stop(subdev, crashed);
+	}
 }
 
 /**
-- 
2.17.0

[PATCH 3/5] remoteproc: Make client initialize ops in rproc_subdev

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

In preparation of adding the additional prepare and unprepare operations
make the client responsible for filling out the function pointers of the
rproc_subdev. This makes the arguments to rproc_add_subdev() more
manageable, in particular when some of the functions are left out.

[elder@linaro.org: added comment about assigning function pointers]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/qcom_common.c     | 18 ++++++++++--------
 drivers/remoteproc/qcom_sysmon.c     |  5 ++++-
 drivers/remoteproc/remoteproc_core.c | 18 +++++++-----------
 include/linux/remoteproc.h           |  5 +----
 4 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index acfc99f82fb8..4ae87c5b8793 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -64,7 +64,10 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
 		return;
 
 	glink->dev = dev;
-	rproc_add_subdev(rproc, &glink->subdev, glink_subdev_probe, glink_subdev_remove);
+	glink->subdev.start = glink_subdev_probe;
+	glink->subdev.stop = glink_subdev_remove;
+
+	rproc_add_subdev(rproc, &glink->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_glink_subdev);
 
@@ -157,7 +160,10 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
 		return;
 
 	smd->dev = dev;
-	rproc_add_subdev(rproc, &smd->subdev, smd_subdev_probe, smd_subdev_remove);
+	smd->subdev.start = smd_subdev_probe;
+	smd->subdev.stop = smd_subdev_remove;
+
+	rproc_add_subdev(rproc, &smd->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_smd_subdev);
 
@@ -202,11 +208,6 @@ void qcom_unregister_ssr_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(qcom_unregister_ssr_notifier);
 
-static int ssr_notify_start(struct rproc_subdev *subdev)
-{
-	return  0;
-}
-
 static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_ssr *ssr = to_ssr_subdev(subdev);
@@ -227,8 +228,9 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 			 const char *ssr_name)
 {
 	ssr->name = ssr_name;
+	ssr->subdev.stop = ssr_notify_stop;
 
-	rproc_add_subdev(rproc, &ssr->subdev, ssr_notify_start, ssr_notify_stop);
+	rproc_add_subdev(rproc, &ssr->subdev);
 }
 EXPORT_SYMBOL_GPL(qcom_add_ssr_subdev);
 
diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index f085545d7da5..e976a602b015 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -469,7 +469,10 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 
 	qmi_add_lookup(&sysmon->qmi, 43, 0, 0);
 
-	rproc_add_subdev(rproc, &sysmon->subdev, sysmon_start, sysmon_stop);
+	sysmon->subdev.start = sysmon_start;
+	sysmon->subdev.stop = sysmon_stop;
+
+	rproc_add_subdev(rproc, &sysmon->subdev);
 
 	sysmon->nb.notifier_call = sysmon_notify;
 	blocking_notifier_chain_register(&sysmon_notifiers, &sysmon->nb);
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 981ae6dff145..ca39fad175f2 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -399,8 +399,10 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 
 	list_add_tail(&rvdev->node, &rproc->rvdevs);
 
-	rproc_add_subdev(rproc, &rvdev->subdev,
-			 rproc_vdev_do_probe, rproc_vdev_do_remove);
+	rvdev->subdev.start = rproc_vdev_do_probe;
+	rvdev->subdev.stop = rproc_vdev_do_remove;
+
+	rproc_add_subdev(rproc, &rvdev->subdev);
 
 	return 0;
 
@@ -1663,17 +1665,11 @@ 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
- * @start: function to call after the rproc is started
- * @stop: function to call before the rproc is stopped
+ *
+ * Caller is responsible for populating optional subdevice function pointers.
  */
-void rproc_add_subdev(struct rproc *rproc,
-		      struct rproc_subdev *subdev,
-		      int (*start)(struct rproc_subdev *subdev),
-		      void (*stop)(struct rproc_subdev *subdev, bool crashed))
+void rproc_add_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
 {
-	subdev->start = start;
-	subdev->stop = stop;
-
 	list_add_tail(&subdev->node, &rproc->subdevs);
 }
 EXPORT_SYMBOL(rproc_add_subdev);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index bf55bf2a5ee1..8f1426330cca 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -566,10 +566,7 @@ 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 (*start)(struct rproc_subdev *subdev),
-		      void (*stop)(struct rproc_subdev *subdev, bool crashed));
+void rproc_add_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
-- 
2.17.0

[PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Alex Elder]

Rename functions used when subdevices are started and stopped to
reflect the new naming scheme.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
 drivers/remoteproc/remoteproc_core.c |  8 ++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4ae87c5b8793..6f77840140bf 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -33,7 +33,7 @@
 
 static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
 
-static int glink_subdev_probe(struct rproc_subdev *subdev)
+static int glink_subdev_start(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
@@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
 	return PTR_ERR_OR_ZERO(glink->edge);
 }
 
-static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
+static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
@@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
 		return;
 
 	glink->dev = dev;
-	glink->subdev.start = glink_subdev_probe;
-	glink->subdev.stop = glink_subdev_remove;
+	glink->subdev.start = glink_subdev_start;
+	glink->subdev.stop = glink_subdev_stop;
 
 	rproc_add_subdev(rproc, &glink->subdev);
 }
@@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
 }
 EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
 
-static int smd_subdev_probe(struct rproc_subdev *subdev)
+static int smd_subdev_start(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
 
@@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
 	return PTR_ERR_OR_ZERO(smd->edge);
 }
 
-static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
+static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
 
@@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
 		return;
 
 	smd->dev = dev;
-	smd->subdev.start = smd_subdev_probe;
-	smd->subdev.stop = smd_subdev_remove;
+	smd->subdev.start = smd_subdev_start;
+	smd->subdev.stop = smd_subdev_stop;
 
 	rproc_add_subdev(rproc, &smd->subdev);
 }
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index ca39fad175f2..2ede7ae6f5bc 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
 	rsc->vring[idx].notifyid = -1;
 }
 
-static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
+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_remove(struct rproc_subdev *subdev, bool crashed)
+static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
 {
 	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
 
@@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 
 	list_add_tail(&rvdev->node, &rproc->rvdevs);
 
-	rvdev->subdev.start = rproc_vdev_do_probe;
-	rvdev->subdev.stop = rproc_vdev_do_remove;
+	rvdev->subdev.start = rproc_vdev_do_start;
+	rvdev->subdev.stop = rproc_vdev_do_stop;
 
 	rproc_add_subdev(rproc, &rvdev->subdev);
 
-- 
2.17.0

[PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Alex Elder]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

On rare occasions a subdevice might need to prepare some hardware
resources before a remote processor is booted, and clean up some
state after it has been shut down.

One such example is the IP Accelerator found in various Qualcomm
platforms, which is accessed directly from both the modem remoteproc
and the application subsystem and requires an intricate lockstep
process when bringing the modem up and down.

[elder@linaro.org: minor description and comment edits]

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Alex Elder <elder@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
 include/linux/remoteproc.h           |  4 ++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 2ede7ae6f5bc..283b258f5e0f 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
 	return ret;
 }
 
+static int rproc_prepare_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+	int ret;
+
+	list_for_each_entry(subdev, &rproc->subdevs, node) {
+		if (subdev->prepare) {
+			ret = subdev->prepare(subdev);
+			if (ret)
+				goto unroll_preparation;
+		}
+	}
+
+	return 0;
+
+unroll_preparation:
+	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->unprepare)
+			subdev->unprepare(subdev);
+	}
+
+	return ret;
+}
+
 static int rproc_start_subdevices(struct rproc *rproc)
 {
 	struct rproc_subdev *subdev;
@@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
 	}
 }
 
+static void rproc_unprepare_subdevices(struct rproc *rproc)
+{
+	struct rproc_subdev *subdev;
+
+	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
+		if (subdev->unprepare)
+			subdev->unprepare(subdev);
+	}
+}
+
 /**
  * rproc_coredump_cleanup() - clean up dump_segments list
  * @rproc: the remote processor handle
@@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 		rproc->table_ptr = loaded_table;
 	}
 
+	ret = rproc_prepare_subdevices(rproc);
+	if (ret) {
+		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
+			rproc->name, ret);
+		return ret;
+	}
+
 	/* power up the remote processor */
 	ret = rproc->ops->start(rproc);
 	if (ret) {
 		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
-		return ret;
+		goto unprepare_subdevices;
 	}
 
 	/* Start any subdevices for the remote processor */
@@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 	if (ret) {
 		dev_err(dev, "failed to probe subdevices for %s: %d\n",
 			rproc->name, ret);
-		rproc->ops->stop(rproc);
-		return ret;
+		goto stop_rproc;
 	}
 
 	rproc->state = RPROC_RUNNING;
@@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 	dev_info(dev, "remote processor %s is now up\n", rproc->name);
 
 	return 0;
+
+stop_rproc:
+	rproc->ops->stop(rproc);
+
+unprepare_subdevices:
+	rproc_unprepare_subdevices(rproc);
+
+	return ret;
 }
 
 /*
@@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
 		return ret;
 	}
 
+	rproc_unprepare_subdevices(rproc);
+
 	rproc->state = RPROC_OFFLINE;
 
 	dev_info(dev, "stopped remote processor %s\n", rproc->name);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8f1426330cca..e3c5d856b6da 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -477,15 +477,19 @@ struct rproc {
 /**
  * struct rproc_subdev - subdevice tied to a remoteproc
  * @node: list node related to the rproc subdevs list
+ * @prepare: prepare function, called before the rproc is started
  * @start: start function, called after the rproc has been started
  * @stop: stop function, called before the rproc is stopped; the @crashed
  *	    parameter indicates if this originates from a recovery
+ * @unprepare: unprepare function, called after the rproc has been stopped
  */
 struct rproc_subdev {
 	struct list_head node;
 
+	int (*prepare)(struct rproc_subdev *subdev);
 	int (*start)(struct rproc_subdev *subdev);
 	void (*stop)(struct rproc_subdev *subdev, bool crashed);
+	void (*unprepare)(struct rproc_subdev *subdev);
 };
 
 /* we currently support only two vrings per rvdev */
-- 
2.17.0

Re: [PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Arnaud Pouliquen]

Hello Alex


We have the same needs (prepare unprepare steps) on our platform. We
tested you core patches and they answers to our need.

Just a remark below

On 05/15/2018 10:53 PM, Alex Elder wrote:
> Rename functions used when subdevices are started and stopped to
> reflect the new naming scheme.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
>  drivers/remoteproc/remoteproc_core.c |  8 ++++----
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 4ae87c5b8793..6f77840140bf 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -33,7 +33,7 @@
>  
>  static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
>  
> -static int glink_subdev_probe(struct rproc_subdev *subdev)
> +static int glink_subdev_start(struct rproc_subdev *subdev)
>  {
>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>  
> @@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
>  	return PTR_ERR_OR_ZERO(glink->edge);
>  }
>  
> -static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
> +static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>  
> @@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
>  		return;
>  
>  	glink->dev = dev;
> -	glink->subdev.start = glink_subdev_probe;
> -	glink->subdev.stop = glink_subdev_remove;
> +	glink->subdev.start = glink_subdev_start;
> +	glink->subdev.stop = glink_subdev_stop;
>  
>  	rproc_add_subdev(rproc, &glink->subdev);
>  }
> @@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
>  }
>  EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
>  
> -static int smd_subdev_probe(struct rproc_subdev *subdev)
> +static int smd_subdev_start(struct rproc_subdev *subdev)
>  {
>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>  
> @@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
>  	return PTR_ERR_OR_ZERO(smd->edge);
>  }
>  
> -static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
> +static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>  
> @@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>  		return;
>  
>  	smd->dev = dev;
> -	smd->subdev.start = smd_subdev_probe;
> -	smd->subdev.stop = smd_subdev_remove;
> +	smd->subdev.start = smd_subdev_start;
> +	smd->subdev.stop = smd_subdev_stop;
>  
>  	rproc_add_subdev(rproc, &smd->subdev);
>  }
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index ca39fad175f2..2ede7ae6f5bc 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
>  	rsc->vring[idx].notifyid = -1;
>  }
>  
> -static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
> +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_remove(struct rproc_subdev *subdev, bool crashed)
> +static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
>  
> @@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>  
>  	list_add_tail(&rvdev->node, &rproc->rvdevs);
>  
> -	rvdev->subdev.start = rproc_vdev_do_probe;
> -	rvdev->subdev.stop = rproc_vdev_do_remove;
> +	rvdev->subdev.start = rproc_vdev_do_start;
> +	rvdev->subdev.stop = rproc_vdev_do_stop;
>  
>  	rproc_add_subdev(rproc, &rvdev->subdev);

Could you split in 2 patches one for the core another, the other for the
glink driver?

Regards
Arnaud

Re: [PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Arnaud Pouliquen]

Hello Alex


We have the same needs (prepare unprepare steps) on our platform. We
tested you core patches and they answers to our need.

Just a remark below

On 05/15/2018 10:53 PM, Alex Elder wrote:
> Rename functions used when subdevices are started and stopped to
> reflect the new naming scheme.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
>  drivers/remoteproc/remoteproc_core.c |  8 ++++----
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 4ae87c5b8793..6f77840140bf 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -33,7 +33,7 @@
>  
>  static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
>  
> -static int glink_subdev_probe(struct rproc_subdev *subdev)
> +static int glink_subdev_start(struct rproc_subdev *subdev)
>  {
>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>  
> @@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
>  	return PTR_ERR_OR_ZERO(glink->edge);
>  }
>  
> -static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
> +static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>  
> @@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
>  		return;
>  
>  	glink->dev = dev;
> -	glink->subdev.start = glink_subdev_probe;
> -	glink->subdev.stop = glink_subdev_remove;
> +	glink->subdev.start = glink_subdev_start;
> +	glink->subdev.stop = glink_subdev_stop;
>  
>  	rproc_add_subdev(rproc, &glink->subdev);
>  }
> @@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
>  }
>  EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
>  
> -static int smd_subdev_probe(struct rproc_subdev *subdev)
> +static int smd_subdev_start(struct rproc_subdev *subdev)
>  {
>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>  
> @@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
>  	return PTR_ERR_OR_ZERO(smd->edge);
>  }
>  
> -static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
> +static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>  
> @@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>  		return;
>  
>  	smd->dev = dev;
> -	smd->subdev.start = smd_subdev_probe;
> -	smd->subdev.stop = smd_subdev_remove;
> +	smd->subdev.start = smd_subdev_start;
> +	smd->subdev.stop = smd_subdev_stop;
>  
>  	rproc_add_subdev(rproc, &smd->subdev);
>  }
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index ca39fad175f2..2ede7ae6f5bc 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
>  	rsc->vring[idx].notifyid = -1;
>  }
>  
> -static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
> +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_remove(struct rproc_subdev *subdev, bool crashed)
> +static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
>  {
>  	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
>  
> @@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>  
>  	list_add_tail(&rvdev->node, &rproc->rvdevs);
>  
> -	rvdev->subdev.start = rproc_vdev_do_probe;
> -	rvdev->subdev.stop = rproc_vdev_do_remove;
> +	rvdev->subdev.start = rproc_vdev_do_start;
> +	rvdev->subdev.stop = rproc_vdev_do_stop;
>  
>  	rproc_add_subdev(rproc, &rvdev->subdev);

Could you split in 2 patches one for the core another, the other for the
glink driver?

Regards
Arnaud

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

Hello Alex,


On 05/15/2018 10:53 PM, Alex Elder wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> On rare occasions a subdevice might need to prepare some hardware
> resources before a remote processor is booted, and clean up some
> state after it has been shut down.
> 
> One such example is the IP Accelerator found in various Qualcomm
> platforms, which is accessed directly from both the modem remoteproc
> and the application subsystem and requires an intricate lockstep
> process when bringing the modem up and down.
> 
> [elder@linaro.org: minor description and comment edits]
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>  include/linux/remoteproc.h           |  4 ++
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2ede7ae6f5bc..283b258f5e0f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>  	return ret;
>  }
>  
> +static int rproc_prepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +	int ret;
> +
> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
> +		if (subdev->prepare) {
> +			ret = subdev->prepare(subdev);
> +			if (ret)
> +				goto unroll_preparation;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_preparation:
> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
Here you could call rproc_unprepare_subdevices instead of duplicating
the code.

regards
Arnaud
> +
> +	return ret;
> +}
> +
>  static int rproc_start_subdevices(struct rproc *rproc)
>  {
>  	struct rproc_subdev *subdev;
> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>  	}
>  }
>  
> +static void rproc_unprepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +
> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +}
> +
>  /**
>   * rproc_coredump_cleanup() - clean up dump_segments list
>   * @rproc: the remote processor handle
> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		rproc->table_ptr = loaded_table;
>  	}
>  
> +	ret = rproc_prepare_subdevices(rproc);
> +	if (ret) {
> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
> +			rproc->name, ret);
> +		return ret;
> +	}
> +
>  	/* power up the remote processor */
>  	ret = rproc->ops->start(rproc);
>  	if (ret) {
>  		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
> -		return ret;
> +		goto unprepare_subdevices;
>  	}
>  
>  	/* Start any subdevices for the remote processor */
> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	if (ret) {
>  		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>  			rproc->name, ret);
> -		rproc->ops->stop(rproc);
> -		return ret;
> +		goto stop_rproc;
>  	}
>  
>  	rproc->state = RPROC_RUNNING;
> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>  
>  	return 0;
> +
> +stop_rproc:
> +	rproc->ops->stop(rproc);
> +
> +unprepare_subdevices:
> +	rproc_unprepare_subdevices(rproc);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>  		return ret;
>  	}
>  
> +	rproc_unprepare_subdevices(rproc);
> +
>  	rproc->state = RPROC_OFFLINE;
>  
>  	dev_info(dev, "stopped remote processor %s\n", rproc->name);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8f1426330cca..e3c5d856b6da 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -477,15 +477,19 @@ struct rproc {
>  /**
>   * struct rproc_subdev - subdevice tied to a remoteproc
>   * @node: list node related to the rproc subdevs list
> + * @prepare: prepare function, called before the rproc is started
>   * @start: start function, called after the rproc has been started
>   * @stop: stop function, called before the rproc is stopped; the @crashed
>   *	    parameter indicates if this originates from a recovery
> + * @unprepare: unprepare function, called after the rproc has been stopped
>   */
>  struct rproc_subdev {
>  	struct list_head node;
>  
> +	int (*prepare)(struct rproc_subdev *subdev);
>  	int (*start)(struct rproc_subdev *subdev);
>  	void (*stop)(struct rproc_subdev *subdev, bool crashed);
> +	void (*unprepare)(struct rproc_subdev *subdev);
>  };
>  
>  /* we currently support only two vrings per rvdev */
> 

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

Hello Alex,


On 05/15/2018 10:53 PM, Alex Elder wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> On rare occasions a subdevice might need to prepare some hardware
> resources before a remote processor is booted, and clean up some
> state after it has been shut down.
> 
> One such example is the IP Accelerator found in various Qualcomm
> platforms, which is accessed directly from both the modem remoteproc
> and the application subsystem and requires an intricate lockstep
> process when bringing the modem up and down.
> 
> [elder@linaro.org: minor description and comment edits]
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>  include/linux/remoteproc.h           |  4 ++
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2ede7ae6f5bc..283b258f5e0f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>  	return ret;
>  }
>  
> +static int rproc_prepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +	int ret;
> +
> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
> +		if (subdev->prepare) {
> +			ret = subdev->prepare(subdev);
> +			if (ret)
> +				goto unroll_preparation;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_preparation:
> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
Here you could call rproc_unprepare_subdevices instead of duplicating
the code.

regards
Arnaud
> +
> +	return ret;
> +}
> +
>  static int rproc_start_subdevices(struct rproc *rproc)
>  {
>  	struct rproc_subdev *subdev;
> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>  	}
>  }
>  
> +static void rproc_unprepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +
> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +}
> +
>  /**
>   * rproc_coredump_cleanup() - clean up dump_segments list
>   * @rproc: the remote processor handle
> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		rproc->table_ptr = loaded_table;
>  	}
>  
> +	ret = rproc_prepare_subdevices(rproc);
> +	if (ret) {
> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
> +			rproc->name, ret);
> +		return ret;
> +	}
> +
>  	/* power up the remote processor */
>  	ret = rproc->ops->start(rproc);
>  	if (ret) {
>  		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
> -		return ret;
> +		goto unprepare_subdevices;
>  	}
>  
>  	/* Start any subdevices for the remote processor */
> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	if (ret) {
>  		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>  			rproc->name, ret);
> -		rproc->ops->stop(rproc);
> -		return ret;
> +		goto stop_rproc;
>  	}
>  
>  	rproc->state = RPROC_RUNNING;
> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>  
>  	return 0;
> +
> +stop_rproc:
> +	rproc->ops->stop(rproc);
> +
> +unprepare_subdevices:
> +	rproc_unprepare_subdevices(rproc);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>  		return ret;
>  	}
>  
> +	rproc_unprepare_subdevices(rproc);
> +
>  	rproc->state = RPROC_OFFLINE;
>  
>  	dev_info(dev, "stopped remote processor %s\n", rproc->name);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8f1426330cca..e3c5d856b6da 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -477,15 +477,19 @@ struct rproc {
>  /**
>   * struct rproc_subdev - subdevice tied to a remoteproc
>   * @node: list node related to the rproc subdevs list
> + * @prepare: prepare function, called before the rproc is started
>   * @start: start function, called after the rproc has been started
>   * @stop: stop function, called before the rproc is stopped; the @crashed
>   *	    parameter indicates if this originates from a recovery
> + * @unprepare: unprepare function, called after the rproc has been stopped
>   */
>  struct rproc_subdev {
>  	struct list_head node;
>  
> +	int (*prepare)(struct rproc_subdev *subdev);
>  	int (*start)(struct rproc_subdev *subdev);
>  	void (*stop)(struct rproc_subdev *subdev, bool crashed);
> +	void (*unprepare)(struct rproc_subdev *subdev);
>  };
>  
>  /* we currently support only two vrings per rvdev */
> 

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Alex Elder]

On 05/29/2018 04:16 AM, Arnaud Pouliquen wrote:
. . .

>> +unroll_preparation:
>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>> +		if (subdev->unprepare)
>> +			subdev->unprepare(subdev);
>> +	}
> Here you could call rproc_unprepare_subdevices instead of duplicating
> the code.

I thought the same thing, but it won't work because we only want to
unprepare those devices that were successfully prepared.  Here we are
unwinding the work that was partially done; in rproc_unprepare_subdevices()
*all* subdevices have their unprepare function called.

					-Alex

Re: [PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Alex Elder]

On 05/29/2018 04:12 AM, Arnaud Pouliquen wrote:
> Hello Alex
> 
> 
> We have the same needs (prepare unprepare steps) on our platform. We
> tested you core patches and they answers to our need.

I'm very glad to hear that.  Would you offer your "Tested-by" on these?

On your comment below, yes, I will re-send v2 and will separate the
core from the glink code.

Thanks.

					-Alex

> Just a remark below
> 
> On 05/15/2018 10:53 PM, Alex Elder wrote:
>> Rename functions used when subdevices are started and stopped to
>> reflect the new naming scheme.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
>> ---
>>  drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
>>  drivers/remoteproc/remoteproc_core.c |  8 ++++----
>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
>> index 4ae87c5b8793..6f77840140bf 100644
>> --- a/drivers/remoteproc/qcom_common.c
>> +++ b/drivers/remoteproc/qcom_common.c
>> @@ -33,7 +33,7 @@
>>  
>>  static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
>>  
>> -static int glink_subdev_probe(struct rproc_subdev *subdev)
>> +static int glink_subdev_start(struct rproc_subdev *subdev)
>>  {
>>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>>  
>> @@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
>>  	return PTR_ERR_OR_ZERO(glink->edge);
>>  }
>>  
>> -static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
>> +static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>>  {
>>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>>  
>> @@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
>>  		return;
>>  
>>  	glink->dev = dev;
>> -	glink->subdev.start = glink_subdev_probe;
>> -	glink->subdev.stop = glink_subdev_remove;
>> +	glink->subdev.start = glink_subdev_start;
>> +	glink->subdev.stop = glink_subdev_stop;
>>  
>>  	rproc_add_subdev(rproc, &glink->subdev);
>>  }
>> @@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
>>  }
>>  EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
>>  
>> -static int smd_subdev_probe(struct rproc_subdev *subdev)
>> +static int smd_subdev_start(struct rproc_subdev *subdev)
>>  {
>>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>>  
>> @@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
>>  	return PTR_ERR_OR_ZERO(smd->edge);
>>  }
>>  
>> -static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
>> +static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>>  {
>>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>>  
>> @@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>>  		return;
>>  
>>  	smd->dev = dev;
>> -	smd->subdev.start = smd_subdev_probe;
>> -	smd->subdev.stop = smd_subdev_remove;
>> +	smd->subdev.start = smd_subdev_start;
>> +	smd->subdev.stop = smd_subdev_stop;
>>  
>>  	rproc_add_subdev(rproc, &smd->subdev);
>>  }
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index ca39fad175f2..2ede7ae6f5bc 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
>>  	rsc->vring[idx].notifyid = -1;
>>  }
>>  
>> -static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
>> +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_remove(struct rproc_subdev *subdev, bool crashed)
>> +static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
>>  {
>>  	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
>>  
>> @@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>>  
>>  	list_add_tail(&rvdev->node, &rproc->rvdevs);
>>  
>> -	rvdev->subdev.start = rproc_vdev_do_probe;
>> -	rvdev->subdev.stop = rproc_vdev_do_remove;
>> +	rvdev->subdev.start = rproc_vdev_do_start;
>> +	rvdev->subdev.stop = rproc_vdev_do_stop;
>>  
>>  	rproc_add_subdev(rproc, &rvdev->subdev);
> 
> Could you split in 2 patches one for the core another, the other for the
> glink driver?
> 
> Regards
> Arnaud
> 

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

On 05/29/2018 01:51 PM, Alex Elder wrote:
> On 05/29/2018 04:16 AM, Arnaud Pouliquen wrote:
> . . .
> 
>>> +unroll_preparation:
>>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>>> +		if (subdev->unprepare)
>>> +			subdev->unprepare(subdev);
>>> +	}
>> Here you could call rproc_unprepare_subdevices instead of duplicating
>> the code.
> 
> I thought the same thing, but it won't work because we only want to
> unprepare those devices that were successfully prepared.  Here we are
> unwinding the work that was partially done; in rproc_unprepare_subdevices()
> *all* subdevices have their unprepare function called.

You right, i missed the "continue"... new for me as i never used it,
thank for teaching!

Arnaud

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

On 05/29/2018 01:51 PM, Alex Elder wrote:
> On 05/29/2018 04:16 AM, Arnaud Pouliquen wrote:
> . . .
> 
>>> +unroll_preparation:
>>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>>> +		if (subdev->unprepare)
>>> +			subdev->unprepare(subdev);
>>> +	}
>> Here you could call rproc_unprepare_subdevices instead of duplicating
>> the code.
> 
> I thought the same thing, but it won't work because we only want to
> unprepare those devices that were successfully prepared.  Here we are
> unwinding the work that was partially done; in rproc_unprepare_subdevices()
> *all* subdevices have their unprepare function called.

You right, i missed the "continue"... new for me as i never used it,
thank for teaching!

Arnaud

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

Hi Fabien,
Please , could you add your "Tested-by" as you test it on ST platform?

Thanks
Arnaud

On 05/15/2018 10:53 PM, Alex Elder wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> On rare occasions a subdevice might need to prepare some hardware
> resources before a remote processor is booted, and clean up some
> state after it has been shut down.
> 
> One such example is the IP Accelerator found in various Qualcomm
> platforms, which is accessed directly from both the modem remoteproc
> and the application subsystem and requires an intricate lockstep
> process when bringing the modem up and down.
> 
> [elder@linaro.org: minor description and comment edits]
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>  include/linux/remoteproc.h           |  4 ++
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2ede7ae6f5bc..283b258f5e0f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>  	return ret;
>  }
>  
> +static int rproc_prepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +	int ret;
> +
> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
> +		if (subdev->prepare) {
> +			ret = subdev->prepare(subdev);
> +			if (ret)
> +				goto unroll_preparation;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_preparation:
> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +
> +	return ret;
> +}
> +
>  static int rproc_start_subdevices(struct rproc *rproc)
>  {
>  	struct rproc_subdev *subdev;
> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>  	}
>  }
>  
> +static void rproc_unprepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +
> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +}
> +
>  /**
>   * rproc_coredump_cleanup() - clean up dump_segments list
>   * @rproc: the remote processor handle
> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		rproc->table_ptr = loaded_table;
>  	}
>  
> +	ret = rproc_prepare_subdevices(rproc);
> +	if (ret) {
> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
> +			rproc->name, ret);
> +		return ret;
> +	}
> +
>  	/* power up the remote processor */
>  	ret = rproc->ops->start(rproc);
>  	if (ret) {
>  		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
> -		return ret;
> +		goto unprepare_subdevices;
>  	}
>  
>  	/* Start any subdevices for the remote processor */
> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	if (ret) {
>  		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>  			rproc->name, ret);
> -		rproc->ops->stop(rproc);
> -		return ret;
> +		goto stop_rproc;
>  	}
>  
>  	rproc->state = RPROC_RUNNING;
> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>  
>  	return 0;
> +
> +stop_rproc:
> +	rproc->ops->stop(rproc);
> +
> +unprepare_subdevices:
> +	rproc_unprepare_subdevices(rproc);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>  		return ret;
>  	}
>  
> +	rproc_unprepare_subdevices(rproc);
> +
>  	rproc->state = RPROC_OFFLINE;
>  
>  	dev_info(dev, "stopped remote processor %s\n", rproc->name);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8f1426330cca..e3c5d856b6da 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -477,15 +477,19 @@ struct rproc {
>  /**
>   * struct rproc_subdev - subdevice tied to a remoteproc
>   * @node: list node related to the rproc subdevs list
> + * @prepare: prepare function, called before the rproc is started
>   * @start: start function, called after the rproc has been started
>   * @stop: stop function, called before the rproc is stopped; the @crashed
>   *	    parameter indicates if this originates from a recovery
> + * @unprepare: unprepare function, called after the rproc has been stopped
>   */
>  struct rproc_subdev {
>  	struct list_head node;
>  
> +	int (*prepare)(struct rproc_subdev *subdev);
>  	int (*start)(struct rproc_subdev *subdev);
>  	void (*stop)(struct rproc_subdev *subdev, bool crashed);
> +	void (*unprepare)(struct rproc_subdev *subdev);
>  };
>  
>  /* we currently support only two vrings per rvdev */
> 

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Arnaud Pouliquen]

Hi Fabien,
Please , could you add your "Tested-by" as you test it on ST platform?

Thanks
Arnaud

On 05/15/2018 10:53 PM, Alex Elder wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> On rare occasions a subdevice might need to prepare some hardware
> resources before a remote processor is booted, and clean up some
> state after it has been shut down.
> 
> One such example is the IP Accelerator found in various Qualcomm
> platforms, which is accessed directly from both the modem remoteproc
> and the application subsystem and requires an intricate lockstep
> process when bringing the modem up and down.
> 
> [elder@linaro.org: minor description and comment edits]
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Alex Elder <elder@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>  include/linux/remoteproc.h           |  4 ++
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 2ede7ae6f5bc..283b258f5e0f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>  	return ret;
>  }
>  
> +static int rproc_prepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +	int ret;
> +
> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
> +		if (subdev->prepare) {
> +			ret = subdev->prepare(subdev);
> +			if (ret)
> +				goto unroll_preparation;
> +		}
> +	}
> +
> +	return 0;
> +
> +unroll_preparation:
> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +
> +	return ret;
> +}
> +
>  static int rproc_start_subdevices(struct rproc *rproc)
>  {
>  	struct rproc_subdev *subdev;
> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>  	}
>  }
>  
> +static void rproc_unprepare_subdevices(struct rproc *rproc)
> +{
> +	struct rproc_subdev *subdev;
> +
> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
> +		if (subdev->unprepare)
> +			subdev->unprepare(subdev);
> +	}
> +}
> +
>  /**
>   * rproc_coredump_cleanup() - clean up dump_segments list
>   * @rproc: the remote processor handle
> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		rproc->table_ptr = loaded_table;
>  	}
>  
> +	ret = rproc_prepare_subdevices(rproc);
> +	if (ret) {
> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
> +			rproc->name, ret);
> +		return ret;
> +	}
> +
>  	/* power up the remote processor */
>  	ret = rproc->ops->start(rproc);
>  	if (ret) {
>  		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
> -		return ret;
> +		goto unprepare_subdevices;
>  	}
>  
>  	/* Start any subdevices for the remote processor */
> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	if (ret) {
>  		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>  			rproc->name, ret);
> -		rproc->ops->stop(rproc);
> -		return ret;
> +		goto stop_rproc;
>  	}
>  
>  	rproc->state = RPROC_RUNNING;
> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>  
>  	return 0;
> +
> +stop_rproc:
> +	rproc->ops->stop(rproc);
> +
> +unprepare_subdevices:
> +	rproc_unprepare_subdevices(rproc);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>  		return ret;
>  	}
>  
> +	rproc_unprepare_subdevices(rproc);
> +
>  	rproc->state = RPROC_OFFLINE;
>  
>  	dev_info(dev, "stopped remote processor %s\n", rproc->name);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 8f1426330cca..e3c5d856b6da 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -477,15 +477,19 @@ struct rproc {
>  /**
>   * struct rproc_subdev - subdevice tied to a remoteproc
>   * @node: list node related to the rproc subdevs list
> + * @prepare: prepare function, called before the rproc is started
>   * @start: start function, called after the rproc has been started
>   * @stop: stop function, called before the rproc is stopped; the @crashed
>   *	    parameter indicates if this originates from a recovery
> + * @unprepare: unprepare function, called after the rproc has been stopped
>   */
>  struct rproc_subdev {
>  	struct list_head node;
>  
> +	int (*prepare)(struct rproc_subdev *subdev);
>  	int (*start)(struct rproc_subdev *subdev);
>  	void (*stop)(struct rproc_subdev *subdev, bool crashed);
> +	void (*unprepare)(struct rproc_subdev *subdev);
>  };
>  
>  /* we currently support only two vrings per rvdev */
> 

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Fabien DESSENNE]

Hi,

Adding my 'Tested-by'.

BR,

Fabien


On 29/05/18 17:26, Arnaud Pouliquen wrote:
> Hi Fabien,
> Please , could you add your "Tested-by" as you test it on ST platform?
>
> Thanks
> Arnaud
>
> On 05/15/2018 10:53 PM, Alex Elder wrote:
>> From: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>> On rare occasions a subdevice might need to prepare some hardware
>> resources before a remote processor is booted, and clean up some
>> state after it has been shut down.
>>
>> One such example is the IP Accelerator found in various Qualcomm
>> platforms, which is accessed directly from both the modem remoteproc
>> and the application subsystem and requires an intricate lockstep
>> process when bringing the modem up and down.
>>
>> [elder@linaro.org: minor description and comment edits]
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> Acked-by: Alex Elder <elder@linaro.org>

Tested-by: Fabien Dessenne <fabien.dessenne@st.com>

>> ---
>>   drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>>   include/linux/remoteproc.h           |  4 ++
>>   2 files changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index 2ede7ae6f5bc..283b258f5e0f 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>>   	return ret;
>>   }
>>   
>> +static int rproc_prepare_subdevices(struct rproc *rproc)
>> +{
>> +	struct rproc_subdev *subdev;
>> +	int ret;
>> +
>> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
>> +		if (subdev->prepare) {
>> +			ret = subdev->prepare(subdev);
>> +			if (ret)
>> +				goto unroll_preparation;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +unroll_preparation:
>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>> +		if (subdev->unprepare)
>> +			subdev->unprepare(subdev);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   static int rproc_start_subdevices(struct rproc *rproc)
>>   {
>>   	struct rproc_subdev *subdev;
>> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>>   	}
>>   }
>>   
>> +static void rproc_unprepare_subdevices(struct rproc *rproc)
>> +{
>> +	struct rproc_subdev *subdev;
>> +
>> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
>> +		if (subdev->unprepare)
>> +			subdev->unprepare(subdev);
>> +	}
>> +}
>> +
>>   /**
>>    * rproc_coredump_cleanup() - clean up dump_segments list
>>    * @rproc: the remote processor handle
>> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>   		rproc->table_ptr = loaded_table;
>>   	}
>>   
>> +	ret = rproc_prepare_subdevices(rproc);
>> +	if (ret) {
>> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
>> +			rproc->name, ret);
>> +		return ret;
>> +	}
>> +
>>   	/* power up the remote processor */
>>   	ret = rproc->ops->start(rproc);
>>   	if (ret) {
>>   		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
>> -		return ret;
>> +		goto unprepare_subdevices;
>>   	}
>>   
>>   	/* Start any subdevices for the remote processor */
>> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>   	if (ret) {
>>   		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>>   			rproc->name, ret);
>> -		rproc->ops->stop(rproc);
>> -		return ret;
>> +		goto stop_rproc;
>>   	}
>>   
>>   	rproc->state = RPROC_RUNNING;
>> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>   	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>>   
>>   	return 0;
>> +
>> +stop_rproc:
>> +	rproc->ops->stop(rproc);
>> +
>> +unprepare_subdevices:
>> +	rproc_unprepare_subdevices(rproc);
>> +
>> +	return ret;
>>   }
>>   
>>   /*
>> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>>   		return ret;
>>   	}
>>   
>> +	rproc_unprepare_subdevices(rproc);
>> +
>>   	rproc->state = RPROC_OFFLINE;
>>   
>>   	dev_info(dev, "stopped remote processor %s\n", rproc->name);
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 8f1426330cca..e3c5d856b6da 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -477,15 +477,19 @@ struct rproc {
>>   /**
>>    * struct rproc_subdev - subdevice tied to a remoteproc
>>    * @node: list node related to the rproc subdevs list
>> + * @prepare: prepare function, called before the rproc is started
>>    * @start: start function, called after the rproc has been started
>>    * @stop: stop function, called before the rproc is stopped; the @crashed
>>    *	    parameter indicates if this originates from a recovery
>> + * @unprepare: unprepare function, called after the rproc has been stopped
>>    */
>>   struct rproc_subdev {
>>   	struct list_head node;
>>   
>> +	int (*prepare)(struct rproc_subdev *subdev);
>>   	int (*start)(struct rproc_subdev *subdev);
>>   	void (*stop)(struct rproc_subdev *subdev, bool crashed);
>> +	void (*unprepare)(struct rproc_subdev *subdev);
>>   };
>>   
>>   /* we currently support only two vrings per rvdev */
>>

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Alex Elder]

On 05/29/2018 10:30 AM, Fabien DESSENNE wrote:
> Hi,
> 
> Adding my 'Tested-by'.

Normally you would say:

    Tested-by: Fabien DESSENNE <fabien.dessenne@st.com>

The reason it might be important is you might wish to indicate the
name and/or e-mail as something different from what you're now
sending from.

Is what I wrote above the way you would like it to appear?

I will add that line to every one of my patches when I update them.

Thanks.

					-Alex
> 
> BR,
> 
> Fabien
> 
> 
> On 29/05/18 17:26, Arnaud Pouliquen wrote:
>> Hi Fabien,
>> Please , could you add your "Tested-by" as you test it on ST platform?
>>
>> Thanks
>> Arnaud
>>
>> On 05/15/2018 10:53 PM, Alex Elder wrote:
>>> From: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>
>>> On rare occasions a subdevice might need to prepare some hardware
>>> resources before a remote processor is booted, and clean up some
>>> state after it has been shut down.
>>>
>>> One such example is the IP Accelerator found in various Qualcomm
>>> platforms, which is accessed directly from both the modem remoteproc
>>> and the application subsystem and requires an intricate lockstep
>>> process when bringing the modem up and down.
>>>
>>> [elder@linaro.org: minor description and comment edits]
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> Acked-by: Alex Elder <elder@linaro.org>
> 
> Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
> 
>>> ---
>>>   drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>>>   include/linux/remoteproc.h           |  4 ++
>>>   2 files changed, 57 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>>> index 2ede7ae6f5bc..283b258f5e0f 100644
>>> --- a/drivers/remoteproc/remoteproc_core.c
>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>>>   	return ret;
>>>   }
>>>   
>>> +static int rproc_prepare_subdevices(struct rproc *rproc)
>>> +{
>>> +	struct rproc_subdev *subdev;
>>> +	int ret;
>>> +
>>> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
>>> +		if (subdev->prepare) {
>>> +			ret = subdev->prepare(subdev);
>>> +			if (ret)
>>> +				goto unroll_preparation;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +unroll_preparation:
>>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>>> +		if (subdev->unprepare)
>>> +			subdev->unprepare(subdev);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>>   static int rproc_start_subdevices(struct rproc *rproc)
>>>   {
>>>   	struct rproc_subdev *subdev;
>>> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>>>   	}
>>>   }
>>>   
>>> +static void rproc_unprepare_subdevices(struct rproc *rproc)
>>> +{
>>> +	struct rproc_subdev *subdev;
>>> +
>>> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
>>> +		if (subdev->unprepare)
>>> +			subdev->unprepare(subdev);
>>> +	}
>>> +}
>>> +
>>>   /**
>>>    * rproc_coredump_cleanup() - clean up dump_segments list
>>>    * @rproc: the remote processor handle
>>> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>   		rproc->table_ptr = loaded_table;
>>>   	}
>>>   
>>> +	ret = rproc_prepare_subdevices(rproc);
>>> +	if (ret) {
>>> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
>>> +			rproc->name, ret);
>>> +		return ret;
>>> +	}
>>> +
>>>   	/* power up the remote processor */
>>>   	ret = rproc->ops->start(rproc);
>>>   	if (ret) {
>>>   		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
>>> -		return ret;
>>> +		goto unprepare_subdevices;
>>>   	}
>>>   
>>>   	/* Start any subdevices for the remote processor */
>>> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>   	if (ret) {
>>>   		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>>>   			rproc->name, ret);
>>> -		rproc->ops->stop(rproc);
>>> -		return ret;
>>> +		goto stop_rproc;
>>>   	}
>>>   
>>>   	rproc->state = RPROC_RUNNING;
>>> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>   	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>>>   
>>>   	return 0;
>>> +
>>> +stop_rproc:
>>> +	rproc->ops->stop(rproc);
>>> +
>>> +unprepare_subdevices:
>>> +	rproc_unprepare_subdevices(rproc);
>>> +
>>> +	return ret;
>>>   }
>>>   
>>>   /*
>>> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>>>   		return ret;
>>>   	}
>>>   
>>> +	rproc_unprepare_subdevices(rproc);
>>> +
>>>   	rproc->state = RPROC_OFFLINE;
>>>   
>>>   	dev_info(dev, "stopped remote processor %s\n", rproc->name);
>>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>>> index 8f1426330cca..e3c5d856b6da 100644
>>> --- a/include/linux/remoteproc.h
>>> +++ b/include/linux/remoteproc.h
>>> @@ -477,15 +477,19 @@ struct rproc {
>>>   /**
>>>    * struct rproc_subdev - subdevice tied to a remoteproc
>>>    * @node: list node related to the rproc subdevs list
>>> + * @prepare: prepare function, called before the rproc is started
>>>    * @start: start function, called after the rproc has been started
>>>    * @stop: stop function, called before the rproc is stopped; the @crashed
>>>    *	    parameter indicates if this originates from a recovery
>>> + * @unprepare: unprepare function, called after the rproc has been stopped
>>>    */
>>>   struct rproc_subdev {
>>>   	struct list_head node;
>>>   
>>> +	int (*prepare)(struct rproc_subdev *subdev);
>>>   	int (*start)(struct rproc_subdev *subdev);
>>>   	void (*stop)(struct rproc_subdev *subdev, bool crashed);
>>> +	void (*unprepare)(struct rproc_subdev *subdev);
>>>   };
>>>   
>>>   /* we currently support only two vrings per rvdev */
>>>

Re: [PATCH 5/5] remoteproc: Introduce prepare and unprepare for subdevices

[Fabien DESSENNE]

Hi Alex,


That's the correct syntax (I wrote it with the official form a few lines 
later (after the Signed-off-by / Acked-by)).

You can add it to the full patch set (although I tested only the core 
part + the future ST driver part)

BR


Fabien



On 29/05/18 17:31, Alex Elder wrote:
> On 05/29/2018 10:30 AM, Fabien DESSENNE wrote:
>> Hi,
>>
>> Adding my 'Tested-by'.
> Normally you would say:
>
>      Tested-by: Fabien DESSENNE <fabien.dessenne@st.com>
>
> The reason it might be important is you might wish to indicate the
> name and/or e-mail as something different from what you're now
> sending from.
>
> Is what I wrote above the way you would like it to appear?
>
> I will add that line to every one of my patches when I update them.
>
> Thanks.
>
> 					-Alex
>> BR,
>>
>> Fabien
>>
>>
>> On 29/05/18 17:26, Arnaud Pouliquen wrote:
>>> Hi Fabien,
>>> Please , could you add your "Tested-by" as you test it on ST platform?
>>>
>>> Thanks
>>> Arnaud
>>>
>>> On 05/15/2018 10:53 PM, Alex Elder wrote:
>>>> From: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>>
>>>> On rare occasions a subdevice might need to prepare some hardware
>>>> resources before a remote processor is booted, and clean up some
>>>> state after it has been shut down.
>>>>
>>>> One such example is the IP Accelerator found in various Qualcomm
>>>> platforms, which is accessed directly from both the modem remoteproc
>>>> and the application subsystem and requires an intricate lockstep
>>>> process when bringing the modem up and down.
>>>>
>>>> [elder@linaro.org: minor description and comment edits]
>>>>
>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>> Acked-by: Alex Elder <elder@linaro.org>
>> Tested-by: Fabien Dessenne <fabien.dessenne@st.com>
>>
>>>> ---
>>>>    drivers/remoteproc/remoteproc_core.c | 56 ++++++++++++++++++++++++++--
>>>>    include/linux/remoteproc.h           |  4 ++
>>>>    2 files changed, 57 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>>>> index 2ede7ae6f5bc..283b258f5e0f 100644
>>>> --- a/drivers/remoteproc/remoteproc_core.c
>>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>>> @@ -776,6 +776,30 @@ static int rproc_handle_resources(struct rproc *rproc,
>>>>    	return ret;
>>>>    }
>>>>    
>>>> +static int rproc_prepare_subdevices(struct rproc *rproc)
>>>> +{
>>>> +	struct rproc_subdev *subdev;
>>>> +	int ret;
>>>> +
>>>> +	list_for_each_entry(subdev, &rproc->subdevs, node) {
>>>> +		if (subdev->prepare) {
>>>> +			ret = subdev->prepare(subdev);
>>>> +			if (ret)
>>>> +				goto unroll_preparation;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +unroll_preparation:
>>>> +	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node) {
>>>> +		if (subdev->unprepare)
>>>> +			subdev->unprepare(subdev);
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>>    static int rproc_start_subdevices(struct rproc *rproc)
>>>>    {
>>>>    	struct rproc_subdev *subdev;
>>>> @@ -810,6 +834,16 @@ static void rproc_stop_subdevices(struct rproc *rproc, bool crashed)
>>>>    	}
>>>>    }
>>>>    
>>>> +static void rproc_unprepare_subdevices(struct rproc *rproc)
>>>> +{
>>>> +	struct rproc_subdev *subdev;
>>>> +
>>>> +	list_for_each_entry_reverse(subdev, &rproc->subdevs, node) {
>>>> +		if (subdev->unprepare)
>>>> +			subdev->unprepare(subdev);
>>>> +	}
>>>> +}
>>>> +
>>>>    /**
>>>>     * rproc_coredump_cleanup() - clean up dump_segments list
>>>>     * @rproc: the remote processor handle
>>>> @@ -902,11 +936,18 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>>    		rproc->table_ptr = loaded_table;
>>>>    	}
>>>>    
>>>> +	ret = rproc_prepare_subdevices(rproc);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "failed to prepare subdevices for %s: %d\n",
>>>> +			rproc->name, ret);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>>    	/* power up the remote processor */
>>>>    	ret = rproc->ops->start(rproc);
>>>>    	if (ret) {
>>>>    		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
>>>> -		return ret;
>>>> +		goto unprepare_subdevices;
>>>>    	}
>>>>    
>>>>    	/* Start any subdevices for the remote processor */
>>>> @@ -914,8 +955,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>>    	if (ret) {
>>>>    		dev_err(dev, "failed to probe subdevices for %s: %d\n",
>>>>    			rproc->name, ret);
>>>> -		rproc->ops->stop(rproc);
>>>> -		return ret;
>>>> +		goto stop_rproc;
>>>>    	}
>>>>    
>>>>    	rproc->state = RPROC_RUNNING;
>>>> @@ -923,6 +963,14 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>>>>    	dev_info(dev, "remote processor %s is now up\n", rproc->name);
>>>>    
>>>>    	return 0;
>>>> +
>>>> +stop_rproc:
>>>> +	rproc->ops->stop(rproc);
>>>> +
>>>> +unprepare_subdevices:
>>>> +	rproc_unprepare_subdevices(rproc);
>>>> +
>>>> +	return ret;
>>>>    }
>>>>    
>>>>    /*
>>>> @@ -1035,6 +1083,8 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>>>>    		return ret;
>>>>    	}
>>>>    
>>>> +	rproc_unprepare_subdevices(rproc);
>>>> +
>>>>    	rproc->state = RPROC_OFFLINE;
>>>>    
>>>>    	dev_info(dev, "stopped remote processor %s\n", rproc->name);
>>>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>>>> index 8f1426330cca..e3c5d856b6da 100644
>>>> --- a/include/linux/remoteproc.h
>>>> +++ b/include/linux/remoteproc.h
>>>> @@ -477,15 +477,19 @@ struct rproc {
>>>>    /**
>>>>     * struct rproc_subdev - subdevice tied to a remoteproc
>>>>     * @node: list node related to the rproc subdevs list
>>>> + * @prepare: prepare function, called before the rproc is started
>>>>     * @start: start function, called after the rproc has been started
>>>>     * @stop: stop function, called before the rproc is stopped; the @crashed
>>>>     *	    parameter indicates if this originates from a recovery
>>>> + * @unprepare: unprepare function, called after the rproc has been stopped
>>>>     */
>>>>    struct rproc_subdev {
>>>>    	struct list_head node;
>>>>    
>>>> +	int (*prepare)(struct rproc_subdev *subdev);
>>>>    	int (*start)(struct rproc_subdev *subdev);
>>>>    	void (*stop)(struct rproc_subdev *subdev, bool crashed);
>>>> +	void (*unprepare)(struct rproc_subdev *subdev);
>>>>    };
>>>>    
>>>>    /* we currently support only two vrings per rvdev */
>>>>

Re: [PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Alex Elder]

On 05/29/2018 06:53 AM, Alex Elder wrote:
> On 05/29/2018 04:12 AM, Arnaud Pouliquen wrote:
>> Hello Alex
>>
>>
>> We have the same needs (prepare unprepare steps) on our platform. We
>> tested you core patches and they answers to our need.
> 
> I'm very glad to hear that.  Would you offer your "Tested-by" on these?
> 
> On your comment below, yes, I will re-send v2 and will separate the
> core from the glink code.

Arnaud, despite what I said above, I'm about to resend the code but
will *not* be separating the core code from glink code.  It turns
out that the glink code (and smd and ssr) are really part of the
core code at the moment.  So after talking with Bjorn I agreed to
just send the code without splitting them.

Sorry.

					-Alex

> Thanks.
> 
> 					-Alex
> 
>> Just a remark below
>>
>> On 05/15/2018 10:53 PM, Alex Elder wrote:
>>> Rename functions used when subdevices are started and stopped to
>>> reflect the new naming scheme.
>>>
>>> Signed-off-by: Alex Elder <elder@linaro.org>
>>> ---
>>>  drivers/remoteproc/qcom_common.c     | 16 ++++++++--------
>>>  drivers/remoteproc/remoteproc_core.c |  8 ++++----
>>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
>>> index 4ae87c5b8793..6f77840140bf 100644
>>> --- a/drivers/remoteproc/qcom_common.c
>>> +++ b/drivers/remoteproc/qcom_common.c
>>> @@ -33,7 +33,7 @@
>>>  
>>>  static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
>>>  
>>> -static int glink_subdev_probe(struct rproc_subdev *subdev)
>>> +static int glink_subdev_start(struct rproc_subdev *subdev)
>>>  {
>>>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>>>  
>>> @@ -42,7 +42,7 @@ static int glink_subdev_probe(struct rproc_subdev *subdev)
>>>  	return PTR_ERR_OR_ZERO(glink->edge);
>>>  }
>>>  
>>> -static void glink_subdev_remove(struct rproc_subdev *subdev, bool crashed)
>>> +static void glink_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>>>  {
>>>  	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
>>>  
>>> @@ -64,8 +64,8 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink)
>>>  		return;
>>>  
>>>  	glink->dev = dev;
>>> -	glink->subdev.start = glink_subdev_probe;
>>> -	glink->subdev.stop = glink_subdev_remove;
>>> +	glink->subdev.start = glink_subdev_start;
>>> +	glink->subdev.stop = glink_subdev_stop;
>>>  
>>>  	rproc_add_subdev(rproc, &glink->subdev);
>>>  }
>>> @@ -129,7 +129,7 @@ int qcom_register_dump_segments(struct rproc *rproc,
>>>  }
>>>  EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
>>>  
>>> -static int smd_subdev_probe(struct rproc_subdev *subdev)
>>> +static int smd_subdev_start(struct rproc_subdev *subdev)
>>>  {
>>>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>>>  
>>> @@ -138,7 +138,7 @@ static int smd_subdev_probe(struct rproc_subdev *subdev)
>>>  	return PTR_ERR_OR_ZERO(smd->edge);
>>>  }
>>>  
>>> -static void smd_subdev_remove(struct rproc_subdev *subdev, bool crashed)
>>> +static void smd_subdev_stop(struct rproc_subdev *subdev, bool crashed)
>>>  {
>>>  	struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
>>>  
>>> @@ -160,8 +160,8 @@ void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd)
>>>  		return;
>>>  
>>>  	smd->dev = dev;
>>> -	smd->subdev.start = smd_subdev_probe;
>>> -	smd->subdev.stop = smd_subdev_remove;
>>> +	smd->subdev.start = smd_subdev_start;
>>> +	smd->subdev.stop = smd_subdev_stop;
>>>  
>>>  	rproc_add_subdev(rproc, &smd->subdev);
>>>  }
>>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>>> index ca39fad175f2..2ede7ae6f5bc 100644
>>> --- a/drivers/remoteproc/remoteproc_core.c
>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>> @@ -301,14 +301,14 @@ void rproc_free_vring(struct rproc_vring *rvring)
>>>  	rsc->vring[idx].notifyid = -1;
>>>  }
>>>  
>>> -static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
>>> +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_remove(struct rproc_subdev *subdev, bool crashed)
>>> +static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
>>>  {
>>>  	struct rproc_vdev *rvdev = container_of(subdev, struct rproc_vdev, subdev);
>>>  
>>> @@ -399,8 +399,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
>>>  
>>>  	list_add_tail(&rvdev->node, &rproc->rvdevs);
>>>  
>>> -	rvdev->subdev.start = rproc_vdev_do_probe;
>>> -	rvdev->subdev.stop = rproc_vdev_do_remove;
>>> +	rvdev->subdev.start = rproc_vdev_do_start;
>>> +	rvdev->subdev.stop = rproc_vdev_do_stop;
>>>  
>>>  	rproc_add_subdev(rproc, &rvdev->subdev);
>>
>> Could you split in 2 patches one for the core another, the other for the
>> glink driver?
>>
>> Regards
>> Arnaud
>>
> 

Re: [PATCH 4/5] remoteproc: rename subdev probe and remove functions

[Bjorn Andersson]

On Mon 25 Jun 18:32 PDT 2018, Alex Elder wrote:

> On 05/29/2018 06:53 AM, Alex Elder wrote:
> > On 05/29/2018 04:12 AM, Arnaud Pouliquen wrote:
> >> Hello Alex
> >>
> >>
> >> We have the same needs (prepare unprepare steps) on our platform. We
> >> tested you core patches and they answers to our need.
> > 
> > I'm very glad to hear that.  Would you offer your "Tested-by" on these?
> > 
> > On your comment below, yes, I will re-send v2 and will separate the
> > core from the glink code.
> 
> Arnaud, despite what I said above, I'm about to resend the code but
> will *not* be separating the core code from glink code.  It turns
> out that the glink code (and smd and ssr) are really part of the
> core code at the moment.  So after talking with Bjorn I agreed to
> just send the code without splitting them.
> 

I wasn't trying to say that it's part of the core, but after reading the
patches again I see that my memory failed me on how these where split.

I'm okay merging this patch even though it touches the two separate
files.

Regards,
Bjorn