[PATCH v2 1/2] remoteproc: core: Add rproc OF look-up functions

[PATCH v2 1/2] remoteproc: core: Add rproc OF look-up functions

[Lee Jones]

- of_rproc_by_index(): look-up and obtain a reference to a rproc
                       using the DT phandle "rprocs" and a index.

- of_rproc_by_name():  lookup and obtain a reference to a rproc
                       using the DT phandle "rprocs" and "rproc-names".

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---

v1 => v2:
 - s/ti,rprocs/ti,rproc

drivers/remoteproc/remoteproc_core.c | 78 +++++++++++++++++++++++++++++++++++-
 include/linux/remoteproc.h           | 25 +++++++++++-
 2 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index fe0539e..fe362fb 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -41,6 +41,8 @@
 #include <linux/virtio_ids.h>
 #include <linux/virtio_ring.h>
 #include <asm/byteorder.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 
 #include "remoteproc_internal.h"
 
@@ -1191,6 +1193,81 @@ out:
 }
 EXPORT_SYMBOL(rproc_shutdown);
 
+#ifdef CONFIG_OF
+/**
+ * of_get_rproc_by_index() - lookup and obtain a reference to an rproc
+ * @np: node to search for rproc phandle
+ * @index: index into the phandle list
+ *
+ * This function increments the remote processor's refcount, so always
+ * use rproc_put() to decrement it back once rproc isn't needed anymore.
+ *
+ * Returns a pointer to the rproc struct on success or an appropriate error
+ * code otherwise.
+ */
+struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
+{
+	struct rproc *rproc = NULL, *r;
+	struct device_node *rproc_np;
+
+	if (index < 0) {
+		pr_err("Invalid index: %d\n", index);
+		return ERR_PTR(-EINVAL);
+	}
+
+	rproc_np = of_parse_phandle(np, "rprocs", index);
+	if (!rproc_np) {
+		/* Unfortunately we have to support this,@least for now */
+		rproc_np = of_parse_phandle(np, "ti,rproc", index);
+		if (!rproc_np) {
+			pr_err("Failed to obtain phandle\n");
+			return ERR_PTR(-ENODEV);
+		}
+	}
+
+	mutex_lock(&rproc_list_mutex);
+	list_for_each_entry(r, &rproc_list, node) {
+		if (r->dev.parent && r->dev.parent->of_node == rproc_np) {
+			get_device(&r->dev);
+			rproc = r;
+			break;
+		}
+	}
+	mutex_unlock(&rproc_list_mutex);
+
+	of_node_put(rproc_np);
+
+	if (!rproc)
+		pr_err("Could not find rproc, deferring\n");
+
+	return rproc ?: ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL(of_get_rproc_by_index);
+
+/**
+ * of_get_rproc_by_name() - lookup and obtain a reference to an rproc
+ * @np: node to search for rproc
+ * @name: name of the remoteproc from device's point of view
+ *
+ * This function increments the remote processor's refcount, so always
+ * use rproc_put() to decrement it back once rproc isn't needed anymore.
+ *
+ * Returns a pointer to the rproc struct on success or an appropriate error
+ * code otherwise.
+ */
+struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
+{
+	int index;
+
+	if (unlikely(!name))
+		return ERR_PTR(-EINVAL);
+
+	index = of_property_match_string(np, "rproc-names", name);
+
+	return of_get_rproc_by_index(np, index);
+}
+EXPORT_SYMBOL(of_get_rproc_by_name);
+
 /**
  * rproc_get_by_phandle() - find a remote processor by phandle
  * @phandle: phandle to the rproc
@@ -1203,7 +1280,6 @@ EXPORT_SYMBOL(rproc_shutdown);
  *
  * Returns the rproc handle on success, and NULL on failure.
  */
-#ifdef CONFIG_OF
 struct rproc *rproc_get_by_phandle(phandle phandle)
 {
 	struct rproc *rproc = NULL, *r;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 1c457a8..f130938 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -487,7 +487,6 @@ struct rproc_vdev {
 	u32 rsc_offset;
 };
 
-struct rproc *rproc_get_by_phandle(phandle phandle);
 struct rproc *rproc_alloc(struct device *dev, const char *name,
 				const struct rproc_ops *ops,
 				const char *firmware, int len);
@@ -511,4 +510,28 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
 	return rvdev->rproc;
 }
 
+#ifdef CONFIG_OF
+extern struct rproc *of_get_rproc_by_index(struct device_node *np,
+					   int index);
+extern struct rproc *of_get_rproc_by_name(struct device_node *np,
+					  const char *name);
+extern struct rproc *rproc_get_by_phandle(phandle phandle);
+#else
+static inline
+struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
+{
+	return NULL;
+}
+static inline
+struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
+{
+	return NULL;
+}
+static inline
+struct rproc *rproc_get_by_phandle(phandle phandle)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 #endif /* REMOTEPROC_H */
-- 
2.9.0

[PATCH v2 2/2] remoteproc: core: Rework obtaining a rproc from a DT phandle

[Lee Jones]

In this patch we;
 - Use a subsystem generic phandle to obtain an rproc
   - We have to support TI's bespoke version for the time being
 - Convert wkup_m3_ipc driver to new API
 - Rename the call to be more like other, similar OF calls
 - Move feature-not-enabled inline stub to the headers
 - Strip out duplicate code by calling into of_get_rproc_by_index()

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---

v1 => v2:
 - Strip '_by_phandle' from function name

drivers/remoteproc/remoteproc_core.c | 41 ++++++++----------------------------
 drivers/soc/ti/wkup_m3_ipc.c         | 14 +++---------
 include/linux/remoteproc.h           |  4 ++--
 3 files changed, 14 insertions(+), 45 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index fe362fb..1c5d5d8 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1269,47 +1269,24 @@ struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
 EXPORT_SYMBOL(of_get_rproc_by_name);
 
 /**
- * rproc_get_by_phandle() - find a remote processor by phandle
- * @phandle: phandle to the rproc
+ * of_get_rproc() - lookup and obtain a reference to an rproc
+ * @np: node to search for rproc
  *
- * Finds an rproc handle using the remote processor's phandle, and then
- * return a handle to the rproc.
+ * Finds an rproc handle using the default remote processor's phandle,
+ * and then returns a handle to the rproc.
  *
  * This function increments the remote processor's refcount, so always
  * use rproc_put() to decrement it back once rproc isn't needed anymore.
  *
- * Returns the rproc handle on success, and NULL on failure.
+ * Returns a pointer to the rproc struct on success or an appropriate error
+ * code otherwise.
  */
-struct rproc *rproc_get_by_phandle(phandle phandle)
-{
-	struct rproc *rproc = NULL, *r;
-	struct device_node *np;
-
-	np = of_find_node_by_phandle(phandle);
-	if (!np)
-		return NULL;
-
-	mutex_lock(&rproc_list_mutex);
-	list_for_each_entry(r, &rproc_list, node) {
-		if (r->dev.parent && r->dev.parent->of_node == np) {
-			rproc = r;
-			get_device(&rproc->dev);
-			break;
-		}
-	}
-	mutex_unlock(&rproc_list_mutex);
-
-	of_node_put(np);
-
-	return rproc;
-}
-#else
-struct rproc *rproc_get_by_phandle(phandle phandle)
+struct rproc *of_get_rproc(struct device_node *np)
 {
-	return NULL;
+	return of_get_rproc_by_index(np, 0);
 }
+EXPORT_SYMBOL(of_get_rproc);
 #endif
-EXPORT_SYMBOL(rproc_get_by_phandle);
 
 /**
  * rproc_add() - register a remote processor
diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
index 8823cc8..8518c16 100644
--- a/drivers/soc/ti/wkup_m3_ipc.c
+++ b/drivers/soc/ti/wkup_m3_ipc.c
@@ -385,7 +385,6 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	int irq, ret;
-	phandle rproc_phandle;
 	struct rproc *m3_rproc;
 	struct resource *res;
 	struct task_struct *task;
@@ -430,16 +429,9 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
 		return PTR_ERR(m3_ipc->mbox);
 	}
 
-	if (of_property_read_u32(dev->of_node, "ti,rproc", &rproc_phandle)) {
-		dev_err(&pdev->dev, "could not get rproc phandle\n");
-		ret = -ENODEV;
-		goto err_free_mbox;
-	}
-
-	m3_rproc = rproc_get_by_phandle(rproc_phandle);
-	if (!m3_rproc) {
-		dev_err(&pdev->dev, "could not get rproc handle\n");
-		ret = -EPROBE_DEFER;
+	m3_rproc = of_get_rproc(dev->of_node);
+	if (IS_ERR(m3_rproc)) {
+		ret = PTR_ERR(m3_rproc);
 		goto err_free_mbox;
 	}
 
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index f130938..0e1adc9 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -515,7 +515,7 @@ extern struct rproc *of_get_rproc_by_index(struct device_node *np,
 					   int index);
 extern struct rproc *of_get_rproc_by_name(struct device_node *np,
 					  const char *name);
-extern struct rproc *rproc_get_by_phandle(phandle phandle);
+extern struct rproc *of_get_rproc(struct device_node *np);
 #else
 static inline
 struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
@@ -528,7 +528,7 @@ struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
 	return NULL;
 }
 static inline
-struct rproc *rproc_get_by_phandle(phandle phandle)
+struct rproc *of_get_rproc(struct device_node *np)
 {
 	return NULL;
 }
-- 
2.9.0

[PATCH 3/2+1] remoteproc: core: Move of_get_rproc() helper to header

[Lee Jones]

remoteproc: core: Move of_get_rproc() helper to header

Since of_get_rproc() has been made a simple helper, we can move it
to the remoteproc.h and make it a 'static inline'.

Suggested-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 1c5d5d8..6966fb2 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1267,25 +1267,6 @@ struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
 	return of_get_rproc_by_index(np, index);
 }
 EXPORT_SYMBOL(of_get_rproc_by_name);
-
-/**
- * of_get_rproc() - lookup and obtain a reference to an rproc
- * @np: node to search for rproc
- *
- * Finds an rproc handle using the default remote processor's phandle,
- * and then returns a handle to the rproc.
- *
- * This function increments the remote processor's refcount, so always
- * use rproc_put() to decrement it back once rproc isn't needed anymore.
- *
- * Returns a pointer to the rproc struct on success or an appropriate error
- * code otherwise.
- */
-struct rproc *of_get_rproc(struct device_node *np)
-{
-	return of_get_rproc_by_index(np, 0);
-}
-EXPORT_SYMBOL(of_get_rproc);
 #endif
 
 /**
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 0e1adc9..ec1cffd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -515,7 +515,6 @@ extern struct rproc *of_get_rproc_by_index(struct device_node *np,
 					   int index);
 extern struct rproc *of_get_rproc_by_name(struct device_node *np,
 					  const char *name);
-extern struct rproc *of_get_rproc(struct device_node *np);
 #else
 static inline
 struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
@@ -527,11 +526,24 @@ struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
 {
 	return NULL;
 }
-static inline
-struct rproc *of_get_rproc(struct device_node *np)
+#endif /* CONFIG_OF */
+
+/**
+ * of_get_rproc() - lookup and obtain a reference to an rproc
+ * @np: node to search for rproc
+ *
+ * Finds an rproc handle using the default remote processor's phandle,
+ * and then returns a handle to the rproc.
+ *
+ * This function increments the remote processor's refcount, so always
+ * use rproc_put() to decrement it back once rproc isn't needed anymore.
+ *
+ * Returns a pointer to the rproc struct on success or an appropriate error
+ * code otherwise.
+ */
+static inline struct rproc *of_get_rproc(struct device_node *np)
 {
-	return NULL;
+	return of_get_rproc_by_index(np, 0);
 }
-#endif /* CONFIG_OF */
 
 #endif /* REMOTEPROC_H */

[PATCH v2 1/2] remoteproc: core: Add rproc OF look-up functions

[Bjorn Andersson]

On Tue 16 Aug 07:38 PDT 2016, Lee Jones wrote:

> - of_rproc_by_index(): look-up and obtain a reference to a rproc
>                        using the DT phandle "rprocs" and a index.
> 
> - of_rproc_by_name():  lookup and obtain a reference to a rproc
>                        using the DT phandle "rprocs" and "rproc-names".
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

For the record; I have not picked these patches because I have not yet
seen an acceptable proposal for a client - there are concerns about DT
bindings of such client and questions on how to notify the client about
life cycle changes in the remoteproc (i.e. crash recovery events).

I think the patches looks good, so once these open questions gets
answered (or some new use case appear) I will pick these up again.

Regards,
Bjorn

> ---
> 
> v1 => v2:
>  - s/ti,rprocs/ti,rproc
> 
> drivers/remoteproc/remoteproc_core.c | 78 +++++++++++++++++++++++++++++++++++-
>  include/linux/remoteproc.h           | 25 +++++++++++-
>  2 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index fe0539e..fe362fb 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -41,6 +41,8 @@
>  #include <linux/virtio_ids.h>
>  #include <linux/virtio_ring.h>
>  #include <asm/byteorder.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>  
>  #include "remoteproc_internal.h"
>  
> @@ -1191,6 +1193,81 @@ out:
>  }
>  EXPORT_SYMBOL(rproc_shutdown);
>  
> +#ifdef CONFIG_OF
> +/**
> + * of_get_rproc_by_index() - lookup and obtain a reference to an rproc
> + * @np: node to search for rproc phandle
> + * @index: index into the phandle list
> + *
> + * This function increments the remote processor's refcount, so always
> + * use rproc_put() to decrement it back once rproc isn't needed anymore.
> + *
> + * Returns a pointer to the rproc struct on success or an appropriate error
> + * code otherwise.
> + */
> +struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
> +{
> +	struct rproc *rproc = NULL, *r;
> +	struct device_node *rproc_np;
> +
> +	if (index < 0) {
> +		pr_err("Invalid index: %d\n", index);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	rproc_np = of_parse_phandle(np, "rprocs", index);
> +	if (!rproc_np) {
> +		/* Unfortunately we have to support this, at least for now */
> +		rproc_np = of_parse_phandle(np, "ti,rproc", index);
> +		if (!rproc_np) {
> +			pr_err("Failed to obtain phandle\n");
> +			return ERR_PTR(-ENODEV);
> +		}
> +	}
> +
> +	mutex_lock(&rproc_list_mutex);
> +	list_for_each_entry(r, &rproc_list, node) {
> +		if (r->dev.parent && r->dev.parent->of_node == rproc_np) {
> +			get_device(&r->dev);
> +			rproc = r;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&rproc_list_mutex);
> +
> +	of_node_put(rproc_np);
> +
> +	if (!rproc)
> +		pr_err("Could not find rproc, deferring\n");
> +
> +	return rproc ?: ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL(of_get_rproc_by_index);
> +
> +/**
> + * of_get_rproc_by_name() - lookup and obtain a reference to an rproc
> + * @np: node to search for rproc
> + * @name: name of the remoteproc from device's point of view
> + *
> + * This function increments the remote processor's refcount, so always
> + * use rproc_put() to decrement it back once rproc isn't needed anymore.
> + *
> + * Returns a pointer to the rproc struct on success or an appropriate error
> + * code otherwise.
> + */
> +struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
> +{
> +	int index;
> +
> +	if (unlikely(!name))
> +		return ERR_PTR(-EINVAL);
> +
> +	index = of_property_match_string(np, "rproc-names", name);
> +
> +	return of_get_rproc_by_index(np, index);
> +}
> +EXPORT_SYMBOL(of_get_rproc_by_name);
> +
>  /**
>   * rproc_get_by_phandle() - find a remote processor by phandle
>   * @phandle: phandle to the rproc
> @@ -1203,7 +1280,6 @@ EXPORT_SYMBOL(rproc_shutdown);
>   *
>   * Returns the rproc handle on success, and NULL on failure.
>   */
> -#ifdef CONFIG_OF
>  struct rproc *rproc_get_by_phandle(phandle phandle)
>  {
>  	struct rproc *rproc = NULL, *r;
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 1c457a8..f130938 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -487,7 +487,6 @@ struct rproc_vdev {
>  	u32 rsc_offset;
>  };
>  
> -struct rproc *rproc_get_by_phandle(phandle phandle);
>  struct rproc *rproc_alloc(struct device *dev, const char *name,
>  				const struct rproc_ops *ops,
>  				const char *firmware, int len);
> @@ -511,4 +510,28 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
>  	return rvdev->rproc;
>  }
>  
> +#ifdef CONFIG_OF
> +extern struct rproc *of_get_rproc_by_index(struct device_node *np,
> +					   int index);
> +extern struct rproc *of_get_rproc_by_name(struct device_node *np,
> +					  const char *name);
> +extern struct rproc *rproc_get_by_phandle(phandle phandle);
> +#else
> +static inline
> +struct rproc *of_get_rproc_by_index(struct device_node *np, int index)
> +{
> +	return NULL;
> +}
> +static inline
> +struct rproc *of_get_rproc_by_name(struct device_node *np, const char *name)
> +{
> +	return NULL;
> +}
> +static inline
> +struct rproc *rproc_get_by_phandle(phandle phandle)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_OF */
> +
>  #endif /* REMOTEPROC_H */
> -- 
> 2.9.0
>