All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/6] System resource management
@ 2017-11-24 14:36 Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 1/6] remoteproc: add early probed subdevices Arnaud Pouliquen
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

This RFC follows discussions in OpenAMP meeting around a resource manager 
for the remote processor.
Here is a patchset that implements a system resource management.
By system resource we speak about resource that is requested to operate
a peripheral, used by a remote processor. These resources can introduce 
potential contention if they are not controlled by one core.

In this first patch-set the system resource manager configures and enables 
the following system resources.
- Clocks
- GPIOs
- Regulators 

Theses resources are declared in devicetree sub-nodes of the resource manager.
A subnode corresponds to one peripheral used by the remote processor.
For each peripheral a generic driver can be probed to handle the system
resource. Developer can also define his own driver for more complexe use 
cases.

Know limitations:
- Assignement of a peripheral is static. Means that peripheral assignement
  can not switch from one core to another core during runtime.
- Configuration of the system resources are also static. No update of a 
resource configuration during runtime.

=> Both limitations could be treated adding in a dynamic aspect based on a 
RPMsg service.
TI already implements a solution (which could be easily port on the top of this series):  
http://git.omapzoom.org/?p=repo/sysbios-rpmsg.git;a=summary


Fabien Dessenne (6):
  remoteproc: add early probed subdevices
  dt-bindings: remoteproc: add system resource manager (SRM)
  remoteproc: add system resource manager core
  remoteproc: add system resource manager device
  remoteproc: Add posibility to probe a sub device.
  remoteproc: sti: select srm

 .../devicetree/bindings/remoteproc/rproc-srm.txt   |  45 ++
 Documentation/remoteproc.txt                       |  23 +
 drivers/remoteproc/Kconfig                         |  20 +
 drivers/remoteproc/Makefile                        |   2 +
 drivers/remoteproc/remoteproc_core.c               |  70 +++-
 drivers/remoteproc/rproc_srm_core.c                | 202 +++++++++
 drivers/remoteproc/rproc_srm_dev.c                 | 462 +++++++++++++++++++++
 include/linux/remoteproc.h                         |   7 +
 8 files changed, 820 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/rproc-srm.txt
 create mode 100644 drivers/remoteproc/rproc_srm_core.c
 create mode 100644 drivers/remoteproc/rproc_srm_dev.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC 1/6] remoteproc: add early probed subdevices
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  2017-12-14  6:11   ` Bjorn Andersson
  2017-11-24 14:36 ` [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM) Arnaud Pouliquen
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

Add the option to use rproc subdevices that are probed *before* the
remote processor firmware boot.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 62 +++++++++++++++++++++++++++++-------
 include/linux/remoteproc.h           |  7 ++++
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index eab14b4..870b1fb 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -770,12 +770,12 @@ static int rproc_handle_resources(struct rproc *rproc, int len,
 	return ret;
 }
 
-static int rproc_probe_subdevices(struct rproc *rproc)
+static int rproc_probe_subdevices(struct list_head *head)
 {
 	struct rproc_subdev *subdev;
 	int ret;
 
-	list_for_each_entry(subdev, &rproc->subdevs, node) {
+	list_for_each_entry(subdev, head, node) {
 		ret = subdev->probe(subdev);
 		if (ret)
 			goto unroll_registration;
@@ -784,17 +784,17 @@ static int rproc_probe_subdevices(struct rproc *rproc)
 	return 0;
 
 unroll_registration:
-	list_for_each_entry_continue_reverse(subdev, &rproc->subdevs, node)
+	list_for_each_entry_continue_reverse(subdev, head, node)
 		subdev->remove(subdev);
 
 	return ret;
 }
 
-static void rproc_remove_subdevices(struct rproc *rproc)
+static void rproc_remove_subdevices(struct list_head *head)
 {
 	struct rproc_subdev *subdev;
 
-	list_for_each_entry_reverse(subdev, &rproc->subdevs, node)
+	list_for_each_entry_reverse(subdev, head, node)
 		subdev->remove(subdev);
 }
 
@@ -881,20 +881,27 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 		rproc->table_ptr = loaded_table;
 	}
 
+	/* early probe any subdevices for the remote processor */
+	ret = rproc_probe_subdevices(&rproc->early_subdevs);
+	if (ret) {
+		dev_err(dev, "failed to early probe 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 remove_early;
 	}
 
 	/* probe any subdevices for the remote processor */
-	ret = rproc_probe_subdevices(rproc);
+	ret = rproc_probe_subdevices(&rproc->subdevs);
 	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;
@@ -902,6 +909,12 @@ 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);
+remove_early:
+	rproc_remove_subdevices(&rproc->early_subdevs);
+	return ret;
 }
 
 /*
@@ -1019,7 +1032,7 @@ static int rproc_stop(struct rproc *rproc)
 	int ret;
 
 	/* remove any subdevices for the remote processor */
-	rproc_remove_subdevices(rproc);
+	rproc_remove_subdevices(&rproc->subdevs);
 
 	/* power off the remote processor */
 	ret = rproc->ops->stop(rproc);
@@ -1028,6 +1041,9 @@ static int rproc_stop(struct rproc *rproc)
 		return ret;
 	}
 
+	/* remove any early-probed subdevices for the remote processor */
+	rproc_remove_subdevices(&rproc->early_subdevs);
+
 	/* if in crash state, unlock crash handler */
 	if (rproc->state == RPROC_CRASHED)
 		complete_all(&rproc->crash_comp);
@@ -1457,6 +1473,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 	INIT_LIST_HEAD(&rproc->traces);
 	INIT_LIST_HEAD(&rproc->rvdevs);
 	INIT_LIST_HEAD(&rproc->subdevs);
+	INIT_LIST_HEAD(&rproc->early_subdevs);
 
 	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
 	init_completion(&rproc->crash_comp);
@@ -1560,9 +1577,32 @@ void rproc_add_subdev(struct rproc *rproc,
 EXPORT_SYMBOL(rproc_add_subdev);
 
 /**
+ * rproc_add_early_subdev() - add an early subdevice to a remoteproc
+ * @rproc: rproc handle to add the subdevice to
+ * @subdev: subdev handle to register
+ * @probe: function to call before the rproc boots
+ * @remove: function to call after the rproc shuts down
+ *
+ * This function differs from rproc_add_subdev() in the sense that the added
+ * subdevices are probed BEFORE the rproc boots.
+ */
+void rproc_add_early_subdev(struct rproc *rproc,
+			    struct rproc_subdev *subdev,
+			    int (*probe)(struct rproc_subdev *subdev),
+			    void (*remove)(struct rproc_subdev *subdev))
+{
+	subdev->probe = probe;
+	subdev->remove = remove;
+
+	list_add_tail(&subdev->node, &rproc->early_subdevs);
+}
+EXPORT_SYMBOL(rproc_add_early_subdev);
+
+/**
  * rproc_remove_subdev() - remove a subdevice from a remoteproc
  * @rproc: rproc handle to remove the subdevice from
- * @subdev: subdev handle, previously registered with rproc_add_subdev()
+ * @subdev: subdev handle, previously registered with rproc_add_subdev() or
+ *          rproc_add_early_subdev()
  */
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev)
 {
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 44e630e..e6b02d8 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -402,6 +402,7 @@ enum rproc_crash_type {
  * @bootaddr: address of first instruction to boot rproc with (optional)
  * @rvdevs: list of remote virtio devices
  * @subdevs: list of subdevices, to following the running state
+ * @early_subdevs: list of early-probed subdevices
  * @notifyids: idr for dynamically assigning rproc-wide unique notify ids
  * @index: index of this rproc device
  * @crash_handler: workqueue for handling a crash
@@ -433,6 +434,7 @@ struct rproc {
 	u32 bootaddr;
 	struct list_head rvdevs;
 	struct list_head subdevs;
+	struct list_head early_subdevs;
 	struct idr notifyids;
 	int index;
 	struct work_struct crash_handler;
@@ -541,6 +543,11 @@ void rproc_add_subdev(struct rproc *rproc,
 		      int (*probe)(struct rproc_subdev *subdev),
 		      void (*remove)(struct rproc_subdev *subdev));
 
+void rproc_add_early_subdev(struct rproc *rproc,
+			    struct rproc_subdev *subdev,
+			    int (*probe)(struct rproc_subdev *subdev),
+			    void (*remove)(struct rproc_subdev *subdev));
+
 void rproc_remove_subdev(struct rproc *rproc, struct rproc_subdev *subdev);
 
 #endif /* REMOTEPROC_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM)
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 1/6] remoteproc: add early probed subdevices Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  2017-12-14  5:59   ` Bjorn Andersson
  2017-11-24 14:36 ` [RFC 3/6] remoteproc: add system resource manager core Arnaud Pouliquen
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

Add Documentation of devicetree bindings to support the remote processor
system resource manager.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 .../devicetree/bindings/remoteproc/rproc-srm.txt   | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/rproc-srm.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/rproc-srm.txt b/Documentation/devicetree/bindings/remoteproc/rproc-srm.txt
new file mode 100644
index 0000000..986dad1
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/rproc-srm.txt
@@ -0,0 +1,45 @@
+Remoteproc System Resource Manager
+----------------------------------
+
+The remoteproc SRM (System Resource Manager) handles resources allocated
+to remote processors.
+This makes it possible for remote proc to reserve and initialize system
+resources for a peripheral assigned to a coprocessor.
+
+The devices are grouped in a core node
+
+Core
+====
+Required properties:
+- compatible: should be "rproc-srm-core"
+
+Dev
+===
+Required properties:
+- compatible: should be "rproc-srm-dev"
+
+Optional properties:
+- clocks: clocks required by the coprocessor
+- clock-names: see clock-bindings.txt
+- assigned-clocks: see clock-bindings.txt
+- assigned-clock-parents: see clock-bindings.txt
+- assigned-clock-rates: see clock-bindings.txt
+- pinctrl-x: pins configurations required by the coprocessor
+- pinctrl-names: see pinctrl-bindings.txt
+- x-supply: power supplies required by the coprocessor
+
+Example:
+	system_resources {
+		compatible = "rproc-srm-core";
+
+		mmc0: sdhci@09060000 {
+			compatible = "rproc-srm-dev";
+			pinctrl-names = "default", "idle";
+			pinctrl-0 = <&pinctrl_mmc0>;
+			pinctrl-1 = <&pinctrl_mmc1>;
+			clock-names = "mmc", "icn";
+			clocks = <&clk_s_c0_flexgen CLK_MMC_0>,
+				 <&clk_s_c0_flexgen CLK_RX_ICN_HVA>;
+			vdda-supply = <&vdda>;
+		};
+	};
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 3/6] remoteproc: add system resource manager core
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 1/6] remoteproc: add early probed subdevices Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM) Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  2017-12-14  6:15   ` Bjorn Andersson
  2017-11-24 14:36 ` [RFC 4/6] remoteproc: add system resource manager device Arnaud Pouliquen
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

The remoteproc SRM (System Resource Manager) handles resources allocated
to remote processors.
This makes it possible for remote proc to reserve and initialize system
resources for a peripheral assigned to a coprocessor.
This is the core part which is in charge of controlling the device
children.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 Documentation/remoteproc.txt        |  23 ++++
 drivers/remoteproc/Kconfig          |   8 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/rproc_srm_core.c | 202 ++++++++++++++++++++++++++++++++++++
 4 files changed, 234 insertions(+)
 create mode 100644 drivers/remoteproc/rproc_srm_core.c

diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt
index 77fb03a..bec2177 100644
--- a/Documentation/remoteproc.txt
+++ b/Documentation/remoteproc.txt
@@ -353,3 +353,26 @@ Of course, RSC_VDEV resource entries are only good enough for static
 allocation of virtio devices. Dynamic allocations will also be made possible
 using the rpmsg bus (similar to how we already do dynamic allocations of
 rpmsg channels; read more about it in rpmsg.txt).
+
+8. System Resource Manager (SRM)
+
+Since some resources are shared (directly or not) between the processors, a
+processor cannot manage such resources without potentially impacting the other
+processors : as an example, if a processor changes the frequency of a clock, the
+frequency of another clock managed by another processor may be updated too.
+
+The System Resource Manager prevents such resource conflicts between the
+processors : it reserves and initializes the system resources of the peripherals
+assigned to a remote processor.
+
+As of today the following resources are controlled by the SRM:
+- clocks
+- gpios (pinctrl)
+- regulators (power supplies)
+
+The SRM is implemented as an 'rproc_subdev' and registered to remoteproc_core.
+Unlike the virtio device (vdev), the SRM subdev is probed *before* the rproc
+boots, ensuring the availability of the resources before the remoteproc starts.
+
+The resources handled by the SRM are defined in the DeviceTree: please read
+Documentation/devicetree/bindings/remoteproc/rproc-srm.txt for details.
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index bf04479..4b9f187 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -12,6 +12,14 @@ config REMOTEPROC
 
 if REMOTEPROC
 
+config REMOTEPROC_SRM_CORE
+	tristate "Remoteproc System Resource Manager core"
+	help
+	  Say y here to enable the core driver of the remoteproc System Resource
+	  Manager (SRM).
+	  The SRM handles resources allocated to remote processors.
+	  The core part is in charge of controlling the device children.
+
 config IMX_REMOTEPROC
 	tristate "IMX6/7 remoteproc support"
 	depends on SOC_IMX6SX || SOC_IMX7D
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 6e16450..4ec05e3 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -9,6 +9,7 @@ remoteproc-y				+= remoteproc_debugfs.o
 remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
+obj-$(CONFIG_REMOTEPROC_SRM_CORE)	+= rproc_srm_core.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
diff --git a/drivers/remoteproc/rproc_srm_core.c b/drivers/remoteproc/rproc_srm_core.c
new file mode 100644
index 0000000..045f10e
--- /dev/null
+++ b/drivers/remoteproc/rproc_srm_core.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
+ * Authors: Fabien Dessenne <fabien.dessenne@st.com>.
+ *          Arnaud Pouliquen <arnaud.pouliquen@st.com>
+ *          Loic Pallardy <loic.pallardy@st.com>
+
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/component.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/remoteproc.h>
+
+struct rproc_srm_core {
+	struct device *dev;
+	struct completion all_bound;
+	int bind_status;
+	atomic_t do_probed;
+	struct rproc_subdev subdev;
+};
+
+#define to_rproc_srm_core(s) container_of(s, struct rproc_srm_core, subdev)
+
+static int compare_of(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+static void release_of(struct device *dev, void *data)
+{
+	of_node_put(data);
+}
+
+static void rproc_srm_core_unbind(struct device *dev)
+{
+	component_unbind_all(dev, NULL);
+}
+
+static int rproc_srm_core_bind(struct device *dev)
+{
+	struct rproc_srm_core *rproc_srm_core = dev_get_drvdata(dev);
+
+	rproc_srm_core->bind_status = component_bind_all(dev, NULL);
+	complete(&rproc_srm_core->all_bound);
+
+	return rproc_srm_core->bind_status;
+}
+
+static const struct component_master_ops srm_comp_ops = {
+	.bind = rproc_srm_core_bind,
+	.unbind = rproc_srm_core_unbind,
+};
+
+static int rproc_srm_core_do_probe(struct rproc_subdev *subdev)
+{
+	struct rproc_srm_core *rproc_srm_core = to_rproc_srm_core(subdev);
+	struct device *dev = rproc_srm_core->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *child_np;
+	struct component_match *match = NULL;
+	int ret;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	init_completion(&rproc_srm_core->all_bound);
+
+	ret = devm_of_platform_populate(dev);
+	if (ret) {
+		dev_err(dev, "cannot populate node (%d)\n", ret);
+		return ret;
+	}
+
+	child_np = of_get_next_available_child(node, NULL);
+
+	while (child_np) {
+		of_node_get(child_np);
+		component_match_add_release(dev, &match, release_of, compare_of,
+					    child_np);
+		child_np = of_get_next_available_child(node, child_np);
+	}
+
+	if (!match) {
+		dev_dbg(dev, "No available child\n");
+		goto done;
+	}
+
+	ret = component_master_add_with_match(dev, &srm_comp_ops, match);
+	if (ret)
+		goto depopulate;
+
+	/* Wait for every child to be bound */
+	wait_for_completion(&rproc_srm_core->all_bound);
+	ret = rproc_srm_core->bind_status;
+	if (ret) {
+		dev_err(dev, "failed to bind\n");
+		goto master;
+	}
+done:
+	atomic_inc(&rproc_srm_core->do_probed);
+
+	return 0;
+
+master:
+	component_master_del(dev, &srm_comp_ops);
+depopulate:
+	devm_of_platform_depopulate(dev);
+	return ret;
+}
+
+static void rproc_srm_core_do_remove(struct rproc_subdev *subdev)
+{
+	struct rproc_srm_core *rproc_srm_core = to_rproc_srm_core(subdev);
+	struct device *dev = rproc_srm_core->dev;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	atomic_dec(&rproc_srm_core->do_probed);
+
+	component_master_del(dev, &srm_comp_ops);
+	devm_of_platform_depopulate(dev);
+}
+
+static int rproc_srm_core_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rproc *rproc = dev_get_drvdata(dev->parent);
+	struct rproc_srm_core *rproc_srm_core;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	rproc_srm_core = devm_kzalloc(dev, sizeof(struct rproc_srm_core),
+				      GFP_KERNEL);
+	if (!rproc_srm_core)
+		return -ENOMEM;
+
+	rproc_srm_core->dev = dev;
+
+	/* Register as an 'early' rproc subdevice */
+	rproc_add_early_subdev(rproc, &rproc_srm_core->subdev,
+			       rproc_srm_core_do_probe,
+			       rproc_srm_core_do_remove);
+
+	dev_set_drvdata(dev, rproc_srm_core);
+
+	return 0;
+}
+
+static int rproc_srm_core_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rproc_srm_core *rproc_srm_core = dev_get_drvdata(dev);
+	struct rproc *rproc = dev_get_drvdata(dev->parent);
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	if (rproc->state != RPROC_OFFLINE) {
+		dev_warn(dev, "Releasing resources while firmware running!\n");
+		return -EBUSY;
+	}
+
+	if (atomic_read(&rproc_srm_core->do_probed))
+		rproc_srm_core_do_remove(&rproc_srm_core->subdev);
+
+	return 0;
+}
+
+static const struct of_device_id rproc_srm_core_match[] = {
+	{ .compatible = "rproc-srm-core", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, rproc_srm_core_match);
+
+static struct platform_driver rproc_srm_core_driver = {
+	.probe = rproc_srm_core_probe,
+	.remove = rproc_srm_core_remove,
+	.driver = {
+		.name = "rproc-srm-core",
+		.of_match_table = of_match_ptr(rproc_srm_core_match),
+	},
+};
+
+module_platform_driver(rproc_srm_core_driver);
+
+MODULE_AUTHOR("Fabien Dessenne <fabien.dessenne@st.com>");
+MODULE_DESCRIPTION("Remoteproc System Resource Manager driver - core");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 4/6] remoteproc: add system resource manager device
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
                   ` (2 preceding siblings ...)
  2017-11-24 14:36 ` [RFC 3/6] remoteproc: add system resource manager core Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 5/6] remoteproc: Add posibility to probe a sub device Arnaud Pouliquen
  2017-11-24 14:36 ` [RFC 6/6] remoteproc: sti: select srm Arnaud Pouliquen
  5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

The SRM device reserves and enables the resources needed by a remote
processor.
In this initial commit the only managed resources are:
- clocks which are prepared and enabled
- pins which are configured
- regulators which are enabled

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/Kconfig         |  10 +
 drivers/remoteproc/Makefile        |   1 +
 drivers/remoteproc/rproc_srm_dev.c | 462 +++++++++++++++++++++++++++++++++++++
 3 files changed, 473 insertions(+)
 create mode 100644 drivers/remoteproc/rproc_srm_dev.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 4b9f187..a49589e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -20,6 +20,16 @@ config REMOTEPROC_SRM_CORE
 	  The SRM handles resources allocated to remote processors.
 	  The core part is in charge of controlling the device children.
 
+config REMOTEPROC_SRM_DEV
+	tristate "Remoteproc System Resource Manager device"
+	depends on REMOTEPROC_SRM_CORE
+	help
+	  Say y here to enable the device driver of the remoteproc System
+	  Resource Manager (SRM).
+	  The SRM handles resources allocated to remote processors.
+	  The device part is in charge of reserving and initializing resources
+	  for a peripheral assigned to a coprocessor.
+
 config IMX_REMOTEPROC
 	tristate "IMX6/7 remoteproc support"
 	depends on SOC_IMX6SX || SOC_IMX7D
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 4ec05e3..36932da 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -10,6 +10,7 @@ remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_SRM_CORE)	+= rproc_srm_core.o
+obj-$(CONFIG_REMOTEPROC_SRM_DEV)	+= rproc_srm_dev.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
diff --git a/drivers/remoteproc/rproc_srm_dev.c b/drivers/remoteproc/rproc_srm_dev.c
new file mode 100644
index 0000000..4f76da2
--- /dev/null
+++ b/drivers/remoteproc/rproc_srm_dev.c
@@ -0,0 +1,462 @@
+/*
+ * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
+ * Authors: Fabien Dessenne <fabien.dessenne@st.com>.
+ *          Arnaud Pouliquen <arnaud.pouliquen@st.com>
+ *          Loic Pallardy <loic.pallardy@st.com>
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/component.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+#define SUPPLY_SUFFIX   "-supply"
+
+struct rproc_srm_clk_info {
+	struct list_head list;
+	unsigned int index;
+	struct clk *clk;
+	const char *name;
+	bool enabled;
+};
+
+struct rproc_srm_pin_info {
+	struct list_head list;
+	unsigned int index;
+	char *name;
+};
+
+struct rproc_srm_regu_info {
+	struct list_head list;
+	unsigned int index;
+	struct regulator *regu;
+	const char *name;
+	bool enabled;
+};
+
+struct rproc_srm_dev {
+	struct device *dev;
+	struct pinctrl *pctrl;
+
+	struct list_head clk_list_head;
+	struct list_head regu_list_head;
+	struct list_head pin_list_head;
+};
+
+/* Clocks */
+static void rproc_srm_dev_deconfig_clocks(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct rproc_srm_clk_info *c;
+	struct list_head *clk_head = &rproc_srm_dev->clk_list_head;
+
+	list_for_each_entry(c, clk_head, list) {
+		if (!c->enabled)
+			continue;
+
+		clk_disable_unprepare(c->clk);
+		c->enabled = false;
+		dev_dbg(rproc_srm_dev->dev, "clk %d (%s) deconfigured\n",
+			c->index, c->name);
+	}
+}
+
+static int rproc_srm_dev_config_clocks(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct rproc_srm_clk_info *c;
+	struct list_head *clk_head = &rproc_srm_dev->clk_list_head;
+	int ret;
+
+	/* Note: not only configuring, but also enabling */
+
+	list_for_each_entry(c, clk_head, list) {
+		if (c->enabled)
+			continue;
+
+		ret = clk_prepare_enable(c->clk);
+		if (ret) {
+			dev_err(rproc_srm_dev->dev, "clk %d (%s) cfg failed\n",
+				c->index, c->name);
+			rproc_srm_dev_deconfig_clocks(rproc_srm_dev);
+			return ret;
+		}
+		c->enabled = true;
+		dev_dbg(rproc_srm_dev->dev, "clk %d (%s) configured\n",
+			c->index, c->name);
+	}
+
+	return 0;
+}
+
+static void rproc_srm_dev_put_clocks(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct rproc_srm_clk_info *c, *tmp;
+	struct list_head *clk_head = &rproc_srm_dev->clk_list_head;
+
+	list_for_each_entry_safe(c, tmp, clk_head, list) {
+		clk_put(c->clk);
+		dev_dbg(dev, "put clock %d (%s)\n", c->index, c->name);
+		list_del(&c->list);
+	}
+}
+
+static int rproc_srm_dev_get_clocks(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct device_node *np = dev->of_node;
+	struct rproc_srm_clk_info *c;
+	struct list_head *clk_head = &rproc_srm_dev->clk_list_head;
+	const char *name;
+	int nb_c, ret;
+	unsigned int i;
+
+	if (!np)
+		return 0;
+
+	nb_c = of_clk_get_parent_count(np);
+	if (!nb_c)
+		return 0;
+
+	for (i = 0; i < nb_c; i++) {
+		c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+		if (!c) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		c->clk = of_clk_get(np, i);
+		if (IS_ERR(c->clk)) {
+			dev_err(dev, "clock %d KO (%ld)\n", i,
+				PTR_ERR(c->clk));
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		if (!of_property_read_string_index(np, "clock-names", i,
+						   &name))
+			c->name = devm_kstrdup(dev, name, GFP_KERNEL);
+
+		c->index = i;
+
+		list_add_tail(&c->list, clk_head);
+		dev_dbg(dev, "got clock %d (%s)\n", c->index, c->name);
+	}
+
+	return 0;
+
+err:
+	rproc_srm_dev_put_clocks(rproc_srm_dev);
+	return ret;
+}
+
+/* Regulators */
+static void rproc_srm_dev_deconfig_regus(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct rproc_srm_regu_info *r;
+	struct list_head *regu_head = &rproc_srm_dev->regu_list_head;
+
+	list_for_each_entry(r, regu_head, list) {
+		if (!r->enabled)
+			continue;
+
+		regulator_disable(r->regu);
+		r->enabled = false;
+		dev_dbg(rproc_srm_dev->dev, "regu %d (%s) disabled\n",
+			r->index, r->name);
+	}
+}
+
+static int rproc_srm_dev_config_regus(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct rproc_srm_regu_info *r;
+	struct list_head *regu_head = &rproc_srm_dev->regu_list_head;
+	int ret;
+
+	/* Enable all the regulators */
+	list_for_each_entry(r, regu_head, list) {
+		if (r->enabled)
+			continue;
+
+		ret = regulator_enable(r->regu);
+		if (ret) {
+			dev_err(rproc_srm_dev->dev, "regu %d (%s) failed\n",
+				r->index, r->name);
+			rproc_srm_dev_deconfig_regus(rproc_srm_dev);
+			return ret;
+		}
+		r->enabled = true;
+		dev_dbg(rproc_srm_dev->dev, "regu %d (%s) enabled\n",
+			r->index, r->name);
+	}
+
+	return 0;
+}
+
+static void rproc_srm_dev_put_regus(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct rproc_srm_regu_info *r, *tmp;
+	struct list_head *regu_head = &rproc_srm_dev->regu_list_head;
+
+	list_for_each_entry_safe(r, tmp, regu_head, list) {
+		devm_regulator_put(r->regu);
+		dev_dbg(dev, "put regu %d (%s)\n", r->index, r->name);
+		list_del(&r->list);
+	}
+}
+
+static int rproc_srm_dev_get_regus(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct device_node *np = dev->of_node;
+	struct property *p;
+	const char *n;
+	char *name;
+	struct rproc_srm_regu_info *r;
+	struct list_head *regu_head = &rproc_srm_dev->regu_list_head;
+	int ret, nb_s = 0;
+
+	if (!np)
+		return 0;
+
+	for_each_property_of_node(np, p) {
+		n = strstr(p->name, SUPPLY_SUFFIX);
+		if (!n || n == p->name)
+			continue;
+
+		r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL);
+		if (!r) {
+			ret = -ENOMEM;
+			goto err_list;
+		}
+
+		name = devm_kstrdup(dev, p->name, GFP_KERNEL);
+		name[strlen(p->name) - strlen(SUPPLY_SUFFIX)] = '\0';
+		r->name = name;
+
+		r->regu = devm_regulator_get(dev, r->name);
+		if (IS_ERR(r->regu)) {
+			dev_err(dev, "cannot get regu %s\n", r->name);
+			ret = -EINVAL;
+			goto err_list;
+		}
+
+		r->index = nb_s++;
+
+		list_add_tail(&r->list, regu_head);
+		dev_dbg(dev, "got regu %d (%s)\n", r->index, r->name);
+	}
+
+	return 0;
+
+err_list:
+	rproc_srm_dev_put_regus(rproc_srm_dev);
+	return ret;
+}
+
+/* Pins */
+static void rproc_srm_dev_put_pins(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct rproc_srm_pin_info *p, *tmp;
+	struct list_head *pin_head = &rproc_srm_dev->pin_list_head;
+
+	list_for_each_entry_safe(p, tmp, pin_head, list) {
+		devm_kfree(dev, p->name);
+		devm_kfree(dev, p);
+		dev_dbg(dev, "remove pin cfg %d (%s)\n", p->index, p->name);
+		list_del(&p->list);
+	}
+
+	if (!IS_ERR_OR_NULL(rproc_srm_dev->pctrl))
+		devm_pinctrl_put(rproc_srm_dev->pctrl);
+}
+
+static int rproc_srm_dev_get_pins(struct rproc_srm_dev *rproc_srm_dev)
+{
+	struct device *dev = rproc_srm_dev->dev;
+	struct device_node *np = dev->of_node;
+	struct rproc_srm_pin_info *p;
+	struct list_head *pin_head = &rproc_srm_dev->pin_list_head;
+	int ret, nb_p;
+	unsigned int i;
+	const char *name;
+
+	if (!np)
+		return 0;
+
+	rproc_srm_dev->pctrl = devm_pinctrl_get(dev);
+	if (IS_ERR(rproc_srm_dev->pctrl))
+		return 0;
+
+	nb_p = of_property_count_strings(np, "pinctrl-names");
+	if (!nb_p) {
+		dev_err(dev, "pinctrl-names not defined\n");
+		devm_pinctrl_put(rproc_srm_dev->pctrl);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < nb_p; i++) {
+		p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
+		if (!p) {
+			ret = -ENOMEM;
+			goto err_list;
+		}
+
+		if (of_property_read_string_index(np, "pinctrl-names", i,
+						  &name)) {
+			dev_err(dev, "no pinctrl-names (pin %d)\n", i);
+			ret = -EINVAL;
+			goto err_list;
+		}
+		p->name = devm_kstrdup(dev, name, GFP_KERNEL);
+
+		p->index = i;
+
+		list_add_tail(&p->list, pin_head);
+		dev_dbg(dev, "found pin cfg %d (%s)\n", p->index, p->name);
+	}
+	return 0;
+
+err_list:
+	rproc_srm_dev_put_pins(rproc_srm_dev);
+	return ret;
+}
+
+/* Core */
+static void
+rproc_srm_dev_unbind(struct device *dev, struct device *master, void *data)
+{
+	struct rproc_srm_dev *rproc_srm_dev = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	rproc_srm_dev_deconfig_regus(rproc_srm_dev);
+	rproc_srm_dev_deconfig_clocks(rproc_srm_dev);
+
+	/* For pins: nothing to deconfig */
+}
+
+static int
+rproc_srm_dev_bind(struct device *dev, struct device *master, void *data)
+{
+	struct rproc_srm_dev *rproc_srm_dev = dev_get_drvdata(dev);
+	int ret;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	ret = rproc_srm_dev_config_clocks(rproc_srm_dev);
+	if (ret)
+		return ret;
+
+	ret = rproc_srm_dev_config_regus(rproc_srm_dev);
+	if (ret)
+		return ret;
+
+	/* No need for pins config ("default" pinctrl applied before probe) */
+
+	return 0;
+}
+
+static const struct component_ops rproc_srm_dev_ops = {
+	.bind = rproc_srm_dev_bind,
+	.unbind = rproc_srm_dev_unbind,
+};
+
+static int rproc_srm_dev_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rproc_srm_dev *rproc_srm_dev;
+	int ret;
+
+	dev_dbg(dev, "%s for node %s\n", __func__, dev->of_node->name);
+
+	rproc_srm_dev = devm_kzalloc(dev, sizeof(struct rproc_srm_dev),
+				     GFP_KERNEL);
+	if (!rproc_srm_dev)
+		return -ENOMEM;
+
+	rproc_srm_dev->dev = dev;
+	INIT_LIST_HEAD(&rproc_srm_dev->clk_list_head);
+	INIT_LIST_HEAD(&rproc_srm_dev->pin_list_head);
+	INIT_LIST_HEAD(&rproc_srm_dev->regu_list_head);
+
+	/* Get clocks, regu and pinctrl */
+	ret = rproc_srm_dev_get_clocks(rproc_srm_dev);
+	if (ret)
+		return ret;
+
+	ret = rproc_srm_dev_get_regus(rproc_srm_dev);
+	if (ret)
+		goto err;
+
+	ret = rproc_srm_dev_get_pins(rproc_srm_dev);
+	if (ret)
+		goto err;
+
+	dev_set_drvdata(dev, rproc_srm_dev);
+
+	return  component_add(dev, &rproc_srm_dev_ops);
+
+err:
+	rproc_srm_dev_put_pins(rproc_srm_dev);
+	rproc_srm_dev_put_regus(rproc_srm_dev);
+	rproc_srm_dev_put_clocks(rproc_srm_dev);
+	return ret;
+}
+
+static int rproc_srm_dev_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rproc_srm_dev *rproc_srm_dev = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	component_del(dev, &rproc_srm_dev_ops);
+
+	rproc_srm_dev_put_regus(rproc_srm_dev);
+	rproc_srm_dev_put_pins(rproc_srm_dev);
+	rproc_srm_dev_put_clocks(rproc_srm_dev);
+
+	return 0;
+}
+
+static const struct of_device_id rproc_srm_dev_match[] = {
+	{ .compatible = "rproc-srm-dev", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, rproc_srm_dev_match);
+
+static struct platform_driver rproc_srm_dev_driver = {
+	.probe = rproc_srm_dev_probe,
+	.remove = rproc_srm_dev_remove,
+	.driver = {
+		.name = "rproc-srm-dev",
+		.of_match_table = of_match_ptr(rproc_srm_dev_match),
+	},
+};
+
+module_platform_driver(rproc_srm_dev_driver);
+
+MODULE_AUTHOR("Fabien Dessenne <fabien.dessenne@st.com>");
+MODULE_DESCRIPTION("Remoteproc System Resource Manager driver - dev");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 5/6] remoteproc: Add posibility to probe a sub device.
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
                   ` (3 preceding siblings ...)
  2017-11-24 14:36 ` [RFC 4/6] remoteproc: add system resource manager device Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  2017-12-14  6:24   ` Bjorn Andersson
  2017-11-24 14:36 ` [RFC 6/6] remoteproc: sti: select srm Arnaud Pouliquen
  5 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

When a remote processor registers, populate the device tree to probe
a potential sub-device. This capability is added to be able to add
a resource manger for the remote processor.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 870b1fb..3f39def 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -40,6 +40,7 @@
 #include <linux/crc32.h>
 #include <linux/virtio_ids.h>
 #include <linux/virtio_ring.h>
+#include <linux/of_platform.h>
 #include <asm/byteorder.h>
 
 #include "remoteproc_internal.h"
@@ -1343,6 +1344,11 @@ int rproc_add(struct rproc *rproc)
 			return ret;
 	}
 
+	/* add resource manager device */
+	ret = devm_of_platform_populate(dev->parent);
+	if (ret < 0)
+		return ret;
+
 	/* expose to rproc_get_by_phandle users */
 	mutex_lock(&rproc_list_mutex);
 	list_add(&rproc->node, &rproc_list);
@@ -1551,6 +1557,8 @@ int rproc_del(struct rproc *rproc)
 	list_del(&rproc->node);
 	mutex_unlock(&rproc_list_mutex);
 
+	of_platform_depopulate(rproc->dev.parent);
+
 	device_del(&rproc->dev);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 6/6] remoteproc: sti: select srm
  2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
                   ` (4 preceding siblings ...)
  2017-11-24 14:36 ` [RFC 5/6] remoteproc: Add posibility to probe a sub device Arnaud Pouliquen
@ 2017-11-24 14:36 ` Arnaud Pouliquen
  5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-11-24 14:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, arnaud.pouliquen,
	Loic PALLARDY  <loic.pallardy@st.com>,
	Fabien DESSENNE, Suman Anna

From: Fabien Dessenne <fabien.dessenne@st.com>

Select the remote proc system resource manager for the sti platform

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index a49589e..1ca0ec5 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -154,6 +154,8 @@ config ST_REMOTEPROC
 	select MAILBOX
 	select STI_MBOX
 	select RPMSG_VIRTIO
+	select REMOTEPROC_SRM_CORE
+	select REMOTEPROC_SRM_DEV
 	help
 	  Say y here to support ST's adjunct processors via the remote
 	  processor framework.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM)
  2017-11-24 14:36 ` [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM) Arnaud Pouliquen
@ 2017-12-14  5:59   ` Bjorn Andersson
  2017-12-21 17:46     ` Arnaud Pouliquen
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2017-12-14  5:59 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: linux-remoteproc, Loic PALLARDY, Fabien DESSENNE, Suman Anna

On Fri 24 Nov 06:36 PST 2017, Arnaud Pouliquen wrote:

> +Example:
> +	system_resources {
> +		compatible = "rproc-srm-core";
> +
> +		mmc0: sdhci@09060000 {
> +			compatible = "rproc-srm-dev";
> +			pinctrl-names = "default", "idle";
> +			pinctrl-0 = <&pinctrl_mmc0>;
> +			pinctrl-1 = <&pinctrl_mmc1>;
> +			clock-names = "mmc", "icn";
> +			clocks = <&clk_s_c0_flexgen CLK_MMC_0>,
> +				 <&clk_s_c0_flexgen CLK_RX_ICN_HVA>;
> +			vdda-supply = <&vdda>;
> +		};
> +	};

>From a DT perspective these properties are all on the remoteproc. This
has the additional benefit of making the dynamic case much saner to
implement. I.e. if you have:

acme_rproc {
	compatible = "acme,rproc";

	clock-names = "ddr", "mmc";
	clocks = <&clocker DDR>, <&clocker MMC>;
};

Then you can declare statically in the acme,rproc that the "ddr" clock
should be enabled between boot and shutdown, or we can do this based on
resource table information, and you can easily acquire a handle to this
clock from a rpmsg device acting as dynamic controller of resources.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 1/6] remoteproc: add early probed subdevices
  2017-11-24 14:36 ` [RFC 1/6] remoteproc: add early probed subdevices Arnaud Pouliquen
@ 2017-12-14  6:11   ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2017-12-14  6:11 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: linux-remoteproc, Loic PALLARDY, Fabien DESSENNE, Suman Anna

On Fri 24 Nov 06:36 PST 2017, Arnaud Pouliquen wrote:

> From: Fabien Dessenne <fabien.dessenne@st.com>
> 
> Add the option to use rproc subdevices that are probed *before* the
> remote processor firmware boot.
> 

Rather than adding a somewhat obscure variant of subdevices I think you
should take Loic's addition of a "release" callback on the memory node
and turn this into a struct rproc_resource that can be embedded in
various resource-types (using container_of to grab the specific struct).

Transitioning carveouts, mappings, vrings etc to using this should allow
us to turn all the different lists of resources that should be
acquired before boot and released after shutdown.


And rather than starting by introducing the SRM I would like to see an
example that hands over the control of "clk" in imx_rproc.c to the
remoteproc core (this is the simplest example I could find).

This can then be built upon to allow referencing additional clocks to
enable statically, by name, from the resource table.

> @@ -881,20 +881,27 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
>  		rproc->table_ptr = loaded_table;
>  	}
>  
> +	/* early probe any subdevices for the remote processor */
> +	ret = rproc_probe_subdevices(&rproc->early_subdevs);
> +	if (ret) {
> +		dev_err(dev, "failed to early probe subdevices for %s: %d\n",
> +			rproc->name, ret);
> +		return ret;

Enabling the "early" resources this late will make it infeasible to use
for things like clocking memories of the remoteproc.

> @@ -1028,6 +1041,9 @@ static int rproc_stop(struct rproc *rproc)
>  		return ret;
>  	}
>  
> +	/* remove any early-probed subdevices for the remote processor */
> +	rproc_remove_subdevices(&rproc->early_subdevs);
> +

This will cause "early subdevs" to be disabled during recovery, just to
be enabled immediately again.

So I think that like the memory resources we want to hold on to them
from boot to shutdown. Preferably in a way that allows us to provide the
ordering that allow us to use this mechanism to describe clocks for
memories in the remoteproc.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 3/6] remoteproc: add system resource manager core
  2017-11-24 14:36 ` [RFC 3/6] remoteproc: add system resource manager core Arnaud Pouliquen
@ 2017-12-14  6:15   ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2017-12-14  6:15 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: linux-remoteproc, Loic PALLARDY, Fabien DESSENNE, Suman Anna

On Fri 24 Nov 06:36 PST 2017, Arnaud Pouliquen wrote:

> From: Fabien Dessenne <fabien.dessenne@st.com>
> 
> The remoteproc SRM (System Resource Manager) handles resources allocated
> to remote processors.
> This makes it possible for remote proc to reserve and initialize system
> resources for a peripheral assigned to a coprocessor.
> This is the core part which is in charge of controlling the device
> children.
> 

I'm afraid I find this overly complex for the task.

If we choose to describe these groups of resources like you have done
here I see no point in having the SRM core.

You could just make the SRM devices platform_drivers that acquire
handles to the resources, register (early) subdevices to get the callback
on boot/shutdown and call enable/disable on the resources there.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 5/6] remoteproc: Add posibility to probe a sub device.
  2017-11-24 14:36 ` [RFC 5/6] remoteproc: Add posibility to probe a sub device Arnaud Pouliquen
@ 2017-12-14  6:24   ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2017-12-14  6:24 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: linux-remoteproc, Loic PALLARDY, Fabien DESSENNE, Suman Anna

On Fri 24 Nov 06:36 PST 2017, Arnaud Pouliquen wrote:

> @@ -1343,6 +1344,11 @@ int rproc_add(struct rproc *rproc)
>  			return ret;
>  	}
>  
> +	/* add resource manager device */
> +	ret = devm_of_platform_populate(dev->parent);
> +	if (ret < 0)
> +		return ret;

With the Qualcomm remoteprocs we describe the communication links as
sub-nodes of the remoteproc node to allow binding drivers to channels by
compatible, provide properties to the devices and expose remote services
in DT (e.g. for the purpose of implementing regulators, clocks and other
resources for the system).

I'm surprised this has not been brought up yet for rpmsg, but I assume
it will come at some point, e.g. that we would have something like:

remoteproc {
	compatible = "acme,rproc";

	vdev0 {
		reg = <0>;

		acme_clocks: clock-controller {
			compatible = "acme,clock-controller";
			#clock-cells = <1>;
		};
	};
};


So blindly creating platform_devices for any subnodes of the remoteproc
node doesn't work for the Qualcomm drivers and won't work if we ever
want to describe something else than a platform driver as a child of any
other remoteproc.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM)
  2017-12-14  5:59   ` Bjorn Andersson
@ 2017-12-21 17:46     ` Arnaud Pouliquen
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2017-12-21 17:46 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, Loic PALLARDY, Fabien DESSENNE, Suman Anna

Hello Bjorn,

On 12/14/2017 06:59 AM, Bjorn Andersson wrote:
> On Fri 24 Nov 06:36 PST 2017, Arnaud Pouliquen wrote:
> 
>> +Example:
>> +	system_resources {
>> +		compatible = "rproc-srm-core";
>> +
>> +		mmc0: sdhci@09060000 {
>> +			compatible = "rproc-srm-dev";
>> +			pinctrl-names = "default", "idle";
>> +			pinctrl-0 = <&pinctrl_mmc0>;
>> +			pinctrl-1 = <&pinctrl_mmc1>;
>> +			clock-names = "mmc", "icn";
>> +			clocks = <&clk_s_c0_flexgen CLK_MMC_0>,
>> +				 <&clk_s_c0_flexgen CLK_RX_ICN_HVA>;
>> +			vdda-supply = <&vdda>;
>> +		};
>> +	};
> 
> From a DT perspective these properties are all on the remoteproc. This
> has the additional benefit of making the dynamic case much saner to
> implement. I.e. if you have:
> 
> acme_rproc {
> 	compatible = "acme,rproc";
> 
> 	clock-names = "ddr", "mmc";
> 	clocks = <&clocker DDR>, <&clocker MMC>;
> };
> 
> Then you can declare statically in the acme,rproc that the "ddr" clock
> should be enabled between boot and shutdown, or we can do this based on
> resource table information, and you can easily acquire a handle to this
> clock from a rpmsg device acting as dynamic controller of resources.

Let's try to clarify the concept we propose.

We distinguish 2 kinds of resources.

1) The resource "inherent" to the remote processor itself:
	- Core clocks
	- associated memories RAM, DDR,...
	- reset...
These resources are already handled by remoteproc core and platform.

2) The peripheral resources like a serial bus, an ADC, a DAC, GPIOs...
This peripheral can be allocated to the remote processor.
In this case the remote processor is in charge of managing the
peripheral itself (peripheral registers).
This peripheral needs clocks, regulators, pins...that are managed by
Linux frameworks.
Aim of the SRM is to handles these particular resources, and so to
provide a way to configure clock, pins, regulators... requested by these
peripherals to operate.

For instance, if a remote processor needs to drive an I2C master.
Linux needs to:
- configure the clock tree to provide a clocks for the I2C peripheral
and bus,
- configure pintrl for the I2C bus.

2 additional assumptions:
- remote processor doesn't have to know the clock and pins used, Linux
manages them.
- remote processor should be able to get and release the I2C peripheral.

This is the purpose of the SRM:
- Provide a simple way to declare system resource associated to a
peripheral in DT. The declaration is similar to a peripheral used by
linux processor.
- Provide a dynamic way to get/release and reconfigure system resources
associated to a peripheral. This part will need RPMsg add-on to be fully
dynamic.

Associate a Linux device per peripheral allows to bind peripheral to its
resources and to choice either a generic driver (proc-srm-dev) or a
specific platform driver to handle it.
for instance, a specific platform driver can be need if platform has
some specific "system" resources that need to be handle only on Linux
side to avoid concurrent access.

Extra usecases that could be managed by srm ( to be confirmed by deeper
analysis):
- Remote peripheral suspend/resume.
- DMA channels.
- Pins configurations before and after peripheral initialization to
avoid glitches
- Dynamic reallocation of a peripheral, from one processor to the other
( low power use cases).
...

Notice that, perhaps "srm" name is confusing and should be changed...

Regards
Arnaud
> 
> Regards,
> Bjorn
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-12-21 17:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-24 14:36 [RFC 0/6] System resource management Arnaud Pouliquen
2017-11-24 14:36 ` [RFC 1/6] remoteproc: add early probed subdevices Arnaud Pouliquen
2017-12-14  6:11   ` Bjorn Andersson
2017-11-24 14:36 ` [RFC 2/6] dt-bindings: remoteproc: add system resource manager (SRM) Arnaud Pouliquen
2017-12-14  5:59   ` Bjorn Andersson
2017-12-21 17:46     ` Arnaud Pouliquen
2017-11-24 14:36 ` [RFC 3/6] remoteproc: add system resource manager core Arnaud Pouliquen
2017-12-14  6:15   ` Bjorn Andersson
2017-11-24 14:36 ` [RFC 4/6] remoteproc: add system resource manager device Arnaud Pouliquen
2017-11-24 14:36 ` [RFC 5/6] remoteproc: Add posibility to probe a sub device Arnaud Pouliquen
2017-12-14  6:24   ` Bjorn Andersson
2017-11-24 14:36 ` [RFC 6/6] remoteproc: sti: select srm Arnaud Pouliquen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.