[RFC PATCH 0/3] Initial Renesas R-Car remoteproc support

[RFC PATCH 0/3] Initial Renesas R-Car remoteproc support

[Julien Massot]

Most of the SoCs in the R-Car gen3 SoC series such as
H3,M3 and E3 have an 'Arm Realtime Core'.
This Realtime core is an Arm Cortex-R7 clocked at 800MHz.
This series adds initial support to load a firmware and start
this remote processor through the remoteproc subsystem.

This series depends on
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20211022122101.66998-1-julien.massot@iot.bzh/
to be able to set the Cortex-R7 boot address.

One of the way to test this driver is to  use the zephyr upstream support
for h3ulcb board 'blinky' demo is my favorite testing firmware.

To generate a firmware with the zephyr project.

follow this starting guide
https://docs.zephyrproject.org/2.7.0/getting_started/index.html

Then compile your zephyr demo
west build -b rcar_h3ulcb_cr7 zephyr/samples/basic/blinky \
    -DCONFIG_KERNEL_ENTRY=\"_vector_table\" \
    --build-dir h3-blinky

Then you can use h3-blinky/zephyr/zephyr.elf as a testing
firmware.

Patch 1/3 adds the dt-bindings

Patch 2/3 adds entries into the dts/dtsi files for r8a77951,
my testing platform. This driver has also been tested on E3 and M3,
but lacks proper zephyr support at the moment.

Modifications to r8a77951-ulcb.dts are given as usage example
and may be dropped in future patchset since it use some memory
range that may be reserved for other usage.

Patch 3/3 is a small driver to cover basic remoteproc
usage: loading firmware from filesystem, starting and stopping the
Cortex-r7 processor.

Julien Massot (3):
  dt-bindings: remoteproc: Add Renesas R-Car
  arm64: dts: renesas: r8a77951: Add CR7 realtime processor
  remoteproc: Add Renesas rcar driver

 .../remoteproc/renesas,rcar-rproc.yaml        |  66 +++++
 arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts |  16 ++
 arch/arm64/boot/dts/renesas/r8a77951.dtsi     |   7 +
 drivers/remoteproc/Kconfig                    |  11 +
 drivers/remoteproc/Makefile                   |   1 +
 drivers/remoteproc/rcar_rproc.c               | 229 ++++++++++++++++++
 6 files changed, 330 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
 create mode 100644 drivers/remoteproc/rcar_rproc.c

-- 
2.31.1

[RFC PATCH 1/3] dt-bindings: remoteproc: Add Renesas R-Car

[Julien Massot]

Renesas R-Car SoCs may contains a Realtime processor.
This patch adds binding for this remote processor.

Signed-off-by: Julien Massot <julien.massot@iot.bzh>
---
 .../remoteproc/renesas,rcar-rproc.yaml        | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
new file mode 100644
index 000000000000..dbf95137ce88
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/renesas,rcar-rproc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Renesas R-Car remote processor controller bindings
+
+maintainers:
+  - Julien Massot <julien.massot@iot.bzh>
+
+description: |
+  This document defines the binding for the remoteproc component that loads and
+  boots firmwares on the Renesas R-Car family chipset.
+  R-Car gen3 family may have a realtime processor, this processor share peripheral
+  and RAM with the host processor with the same address map.
+
+
+properties:
+  compatible:
+    const: renesas,rcar-cr7
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  memory-region:
+    description:
+      List of phandles to the reserved memory regions associated with the
+      remoteproc device. This is variable and describes the memories shared with
+      the remote processor (e.g. remoteproc firmware and carveouts, rpmsg
+      vrings, ...).
+      (see ../reserved-memory/reserved-memory.txt)
+
+required:
+  - compatible
+  - resets
+  - memory-region
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a7795-cpg-mssr.h>
+    #include <dt-bindings/power/r8a7795-sysc.h>
+    reserved-memory {
+        ranges;
+
+        cr7_ram: cr7_ram@40040000 {
+            no-map;
+            reg = <0x40040000 0x1fc0000>;
+        };
+    };
+
+    cr7_rproc: cr7 {
+        compatible = "renesas,rcar-cr7";
+        memory-region = <&cr7_ram>;
+        power-domains = <&sysc R8A7795_PD_CR7>;
+        resets = <&cpg 222>;
+        status = "okay";
+    };
+
+...
-- 
2.31.1

[RFC PATCH 2/3] arm64: dts: renesas: r8a77951: Add CR7 realtime processor

[Julien Massot]

r8a77951 as some other members of rcar gen3 soc series
has a Cortex R7 processor.
This processor shares the same mapped devices and memory mapping.

Choose 0x40040000 area to store the Cortex-R7 firmware.

Signed-off-by: Julien Massot <julien.massot@iot.bzh>
---
 arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts | 16 ++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77951.dtsi     |  7 +++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts b/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
index 06d4e948eb0f..26ab7e3cca18 100644
--- a/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
@@ -34,6 +34,17 @@ memory@700000000 {
 		device_type = "memory";
 		reg = <0x7 0x00000000 0x0 0x40000000>;
 	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		cr7_ram: cr7_ram@40040000 {
+			no-map;
+			reg = <0x0 0x40040000 0x0 0x1fc0000>;
+		};
+	};
 };
 
 &du {
@@ -48,3 +59,8 @@ &du {
 	clock-names = "du.0", "du.1", "du.2", "du.3",
 		      "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
+
+&cr7_rproc {
+	memory-region = <&cr7_ram>;
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/renesas/r8a77951.dtsi b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
index 1768a3e6bb8d..3ee247fc5aec 100644
--- a/arch/arm64/boot/dts/renesas/r8a77951.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
@@ -366,6 +366,13 @@ soc: soc {
 		#size-cells = <2>;
 		ranges;
 
+		cr7_rproc: cr7 {
+			compatible = "renesas,rcar-cr7";
+			power-domains = <&sysc R8A7795_PD_CR7>;
+			resets = <&cpg 222>;
+			status = "disabled";
+		};
+
 		rwdt: watchdog@e6020000 {
 			compatible = "renesas,r8a7795-wdt", "renesas,rcar-gen3-wdt";
 			reg = <0 0xe6020000 0 0x0c>;
-- 
2.31.1

[RFC PATCH 3/3] remoteproc: Add Renesas rcar driver

[Julien Massot]

Renesas Gen3 platform includes a Cortex-r7 processor.

Both: the application cores (A5x) and the realtime core (CR7)
share access to the RAM and devices with the same address map,
so device addresses are equal to the Linux physical addresses.

In order to initialize this remote processor we need to:
- power on the realtime core
- put the firmware in a ram area
- set the boot address for this firmware (reset vector)
- Deassert the reset

This initial driver allows to start and stop the Cortex R7
processor.

Signed-off-by: Julien Massot <julien.massot@iot.bzh>
---
 drivers/remoteproc/Kconfig      |  11 ++
 drivers/remoteproc/Makefile     |   1 +
 drivers/remoteproc/rcar_rproc.c | 229 ++++++++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/remoteproc/rcar_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9a6eedc3994a..3e87eadbaf59 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -261,6 +261,17 @@ config QCOM_WCNSS_PIL
 	  verified and booted with the help of the Peripheral Authentication
 	  System (PAS) in TrustZone.
 
+config RCAR_REMOTEPROC
+	tristate "Renesas RCAR remoteproc support"
+	depends on ARCH_RENESAS
+	depends on REMOTEPROC
+	help
+	  Say y here to support R-Car realtime processor via the
+	  remote processor framework. An elf firmware can be loaded
+	  thanks to sysfs remoteproc entries. The remote processor
+	  can be started and stopped.
+	  This can be either built-in or a loadable module.
+
 config ST_REMOTEPROC
 	tristate "ST remoteproc support"
 	depends on ARCH_STI
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index bb26c9e4ef9c..bb290cc08e99 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_QCOM_SYSMON)		+= qcom_sysmon.o
 obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
 qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
+obj-$(CONFIG_RCAR_REMOTEPROC)		+= rcar_rproc.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
 obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
diff --git a/drivers/remoteproc/rcar_rproc.c b/drivers/remoteproc/rcar_rproc.c
new file mode 100644
index 000000000000..ae0bfc8d4e85
--- /dev/null
+++ b/drivers/remoteproc/rcar_rproc.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) IoT.bzh 2021
+ */
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/pm_runtime.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+#include <linux/soc/renesas/rcar-rst.h>
+
+#include "remoteproc_internal.h"
+
+struct rcar_rproc {
+	struct device			*dev;
+	struct rproc			*rproc;
+	struct reset_control            *rst;
+};
+
+static int rcar_rproc_mem_alloc(struct rproc *rproc,
+				 struct rproc_mem_entry *mem)
+{
+	struct device *dev = rproc->dev.parent;
+	void *va;
+
+	dev_dbg(dev, "map memory: %p+%zx\n", &mem->dma, mem->len);
+	va = ioremap_wc(mem->dma, mem->len);
+	if (IS_ERR_OR_NULL(va)) {
+		dev_err(dev, "Unable to map memory region: %pa+%lx\n",
+			&mem->dma, mem->len);
+		return -ENOMEM;
+	}
+
+	/* Update memory entry va */
+	mem->va = va;
+
+	return 0;
+}
+
+static int rcar_rproc_mem_release(struct rproc *rproc,
+				   struct rproc_mem_entry *mem)
+{
+	dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
+	iounmap(mem->va);
+
+	return 0;
+}
+
+static int rcar_rproc_prepare(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct device_node *np = dev->of_node;
+	struct of_phandle_iterator it;
+	struct rproc_mem_entry *mem;
+	struct reserved_mem *rmem;
+	u64 da;
+
+	/* Register associated reserved memory regions */
+	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+	while (of_phandle_iterator_next(&it) == 0) {
+
+		rmem = of_reserved_mem_lookup(it.node);
+		if (!rmem) {
+			dev_err(dev, "unable to acquire memory-region\n");
+			return -EINVAL;
+		}
+
+		/* No need to translate pa to da, R-Car use same map */
+		da = rmem->base;
+
+		mem = rproc_mem_entry_init(dev, NULL,
+					   (dma_addr_t)rmem->base,
+					   rmem->size, da,
+					   rcar_rproc_mem_alloc,
+					   rcar_rproc_mem_release,
+					   it.node->name);
+
+		if (!mem)
+			return -ENOMEM;
+
+		rproc_add_carveout(rproc, mem);
+	}
+
+	return 0;
+}
+
+static int rcar_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+	int ret;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret)
+		dev_info(&rproc->dev, "No resource table in elf\n");
+
+	return 0;
+}
+
+static int rcar_rproc_start(struct rproc *rproc)
+{
+	struct rcar_rproc *priv = rproc->priv;
+	int err;
+
+	if (!rproc->bootaddr)
+		return -EINVAL;
+
+	err = rcar_rst_set_rproc_boot_addr(rproc->bootaddr);
+	if (err) {
+		dev_err(&rproc->dev, "failed to set rproc boot addr\n");
+		return err;
+	}
+
+	err = reset_control_deassert(priv->rst);
+	if (err)
+		dev_err(&rproc->dev, "failed to deassert reset\n");
+
+	return err;
+}
+
+static int rcar_rproc_stop(struct rproc *rproc)
+{
+	struct rcar_rproc *priv = rproc->priv;
+	int err;
+
+	err = reset_control_assert(priv->rst);
+	if (err)
+		dev_err(&rproc->dev, "failed to assert reset\n");
+
+	return err;
+}
+
+static struct rproc_ops rcar_rproc_ops = {
+	.prepare	= rcar_rproc_prepare,
+	.start		= rcar_rproc_start,
+	.stop		= rcar_rproc_stop,
+	.load		= rproc_elf_load_segments,
+	.parse_fw	= rcar_rproc_parse_fw,
+	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+	.sanity_check	= rproc_elf_sanity_check,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+
+};
+
+static int rcar_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct rcar_rproc *priv;
+	struct rproc *rproc;
+	int ret;
+
+	rproc = rproc_alloc(dev, np->name, &rcar_rproc_ops,
+			    NULL, sizeof(*priv));
+	if (!rproc)
+		return -ENOMEM;
+
+	priv = rproc->priv;
+	priv->rproc = rproc;
+	priv->dev = dev;
+
+	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(priv->rst)) {
+		ret = PTR_ERR(priv->rst);
+		dev_err(dev, "fail to acquire rproc reset\n");
+		goto free_rproc;
+	}
+
+	pm_runtime_enable(priv->dev);
+	ret = pm_runtime_get_sync(priv->dev);
+	if (ret) {
+		dev_err(&rproc->dev, "failed to power up\n");
+		goto free_rproc;
+	}
+
+	dev_set_drvdata(dev, rproc);
+
+	/* Manually start the rproc */
+	rproc->auto_boot = false;
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed\n");
+		goto pm_disable;
+	}
+
+	return 0;
+
+pm_disable:
+	pm_runtime_disable(priv->dev);
+free_rproc:
+	rproc_free(rproc);
+
+	return ret;
+}
+
+static int rcar_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct rcar_rproc *priv = rproc->priv;
+
+	rproc_del(rproc);
+	pm_runtime_disable(priv->dev);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_rproc_of_match[] = {
+	{ .compatible = "renesas,rcar-cr7" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, rcar_rproc_of_match);
+
+static struct platform_driver rcar_rproc_driver = {
+	.probe = rcar_rproc_probe,
+	.remove = rcar_rproc_remove,
+	.driver = {
+		.name = "rcar-rproc",
+		.of_match_table = rcar_rproc_of_match,
+	},
+};
+
+module_platform_driver(rcar_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas Gen3 R-Car remote processor control driver");
+MODULE_AUTHOR("Julien Massot <julien.massot@iot.bzh>");
-- 
2.31.1

Re: [RFC PATCH 1/3] dt-bindings: remoteproc: Add Renesas R-Car

[Rob Herring]

On Wed, 27 Oct 2021 09:30:18 +0200, Julien Massot wrote:
> Renesas R-Car SoCs may contains a Realtime processor.
> This patch adds binding for this remote processor.
> 
> Signed-off-by: Julien Massot <julien.massot@iot.bzh>
> ---
>  .../remoteproc/renesas,rcar-rproc.yaml        | 66 +++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:26.17-46: Warning (reg_format): /example-0/reserved-memory/cr7_ram@40040000:reg: property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:22.13-20: Warning (ranges_format): /example-0/reserved-memory:ranges: empty "ranges" property but its #address-cells (2) differs from /example-0 (1)
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:24.39-27.15: Warning (avoid_default_addr_size): /example-0/reserved-memory/cr7_ram@40040000: Relying on default #address-cells value
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:24.39-27.15: Warning (avoid_default_addr_size): /example-0/reserved-memory/cr7_ram@40040000: Relying on default #size-cells value
Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1546783

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFC PATCH 3/3] remoteproc: Add Renesas rcar driver

[Mathieu Poirier]

Good morning Julien,

On Wed, Oct 27, 2021 at 09:30:20AM +0200, Julien Massot wrote:
> Renesas Gen3 platform includes a Cortex-r7 processor.
> 
> Both: the application cores (A5x) and the realtime core (CR7)
> share access to the RAM and devices with the same address map,
> so device addresses are equal to the Linux physical addresses.
> 
> In order to initialize this remote processor we need to:
> - power on the realtime core
> - put the firmware in a ram area
> - set the boot address for this firmware (reset vector)
> - Deassert the reset
> 
> This initial driver allows to start and stop the Cortex R7
> processor.
> 
> Signed-off-by: Julien Massot <julien.massot@iot.bzh>
> ---
>  drivers/remoteproc/Kconfig      |  11 ++
>  drivers/remoteproc/Makefile     |   1 +
>  drivers/remoteproc/rcar_rproc.c | 229 ++++++++++++++++++++++++++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 drivers/remoteproc/rcar_rproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9a6eedc3994a..3e87eadbaf59 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -261,6 +261,17 @@ config QCOM_WCNSS_PIL
>  	  verified and booted with the help of the Peripheral Authentication
>  	  System (PAS) in TrustZone.
>  
> +config RCAR_REMOTEPROC
> +	tristate "Renesas RCAR remoteproc support"

It is probably a good idea to include the type of SoC being supported, something
like:

        tristate "Renesas Gen3 RCAR remoteproc support"

That will make it easier to support future RCAR processors that may not share
the same architecture.


> +	depends on ARCH_RENESAS
> +	depends on REMOTEPROC
> +	help
> +	  Say y here to support R-Car realtime processor via the
> +	  remote processor framework. An elf firmware can be loaded
> +	  thanks to sysfs remoteproc entries. The remote processor
> +	  can be started and stopped.
> +	  This can be either built-in or a loadable module.  

Please add the name of the module when compiled as such.

> +
>  config ST_REMOTEPROC
>  	tristate "ST remoteproc support"
>  	depends on ARCH_STI
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index bb26c9e4ef9c..bb290cc08e99 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_QCOM_SYSMON)		+= qcom_sysmon.o
>  obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
>  qcom_wcnss_pil-y			+= qcom_wcnss.o
>  qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
> +obj-$(CONFIG_RCAR_REMOTEPROC)		+= rcar_rproc.o
>  obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
>  obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
>  obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
> diff --git a/drivers/remoteproc/rcar_rproc.c b/drivers/remoteproc/rcar_rproc.c
> new file mode 100644
> index 000000000000..ae0bfc8d4e85
> --- /dev/null
> +++ b/drivers/remoteproc/rcar_rproc.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) IoT.bzh 2021
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/remoteproc.h>
> +#include <linux/reset.h>
> +#include <linux/soc/renesas/rcar-rst.h>
> +
> +#include "remoteproc_internal.h"
> +
> +struct rcar_rproc {
> +	struct device			*dev;
> +	struct rproc			*rproc;
> +	struct reset_control            *rst;
> +};
> +
> +static int rcar_rproc_mem_alloc(struct rproc *rproc,
> +				 struct rproc_mem_entry *mem)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	void *va;
> +
> +	dev_dbg(dev, "map memory: %p+%zx\n", &mem->dma, mem->len);

I think this should be "map memory: %pa+%lx\n" to be consistent with dev_err()
below and the original implementation in stm32_rproc.c.

> +	va = ioremap_wc(mem->dma, mem->len);
> +	if (IS_ERR_OR_NULL(va)) {
> +		dev_err(dev, "Unable to map memory region: %pa+%lx\n",
> +			&mem->dma, mem->len);
> +		return -ENOMEM;
> +	}
> +
> +	/* Update memory entry va */
> +	mem->va = va;
> +
> +	return 0;
> +}
> +
> +static int rcar_rproc_mem_release(struct rproc *rproc,
> +				   struct rproc_mem_entry *mem)
> +{
> +	dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
> +	iounmap(mem->va);
> +
> +	return 0;
> +}
> +
> +static int rcar_rproc_prepare(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct device_node *np = dev->of_node;
> +	struct of_phandle_iterator it;
> +	struct rproc_mem_entry *mem;
> +	struct reserved_mem *rmem;
> +	u64 da;
> +
> +	/* Register associated reserved memory regions */
> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
> +	while (of_phandle_iterator_next(&it) == 0) {
> +
> +		rmem = of_reserved_mem_lookup(it.node);
> +		if (!rmem) {
> +			dev_err(dev, "unable to acquire memory-region\n");
> +			return -EINVAL;
> +		}
> +
> +		/* No need to translate pa to da, R-Car use same map */
> +		da = rmem->base;
> +
> +		mem = rproc_mem_entry_init(dev, NULL,
> +					   (dma_addr_t)rmem->base,
> +					   rmem->size, da,
> +					   rcar_rproc_mem_alloc,
> +					   rcar_rproc_mem_release,
> +					   it.node->name);
> +
> +		if (!mem)
> +			return -ENOMEM;
> +
> +		rproc_add_carveout(rproc, mem);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> +	int ret;
> +
> +	ret = rproc_elf_load_rsc_table(rproc, fw);
> +	if (ret)
> +		dev_info(&rproc->dev, "No resource table in elf\n");

In the above functions rproc->dev.parent is used for output.  I don't have a
strong opinion on which of rproc->dev or rproc->dev.parent is used but I would
like to see consistency throughout the driver.

> +
> +	return 0;
> +}
> +
> +static int rcar_rproc_start(struct rproc *rproc)
> +{
> +	struct rcar_rproc *priv = rproc->priv;
> +	int err;
> +
> +	if (!rproc->bootaddr)
> +		return -EINVAL;
> +
> +	err = rcar_rst_set_rproc_boot_addr(rproc->bootaddr);
> +	if (err) {
> +		dev_err(&rproc->dev, "failed to set rproc boot addr\n");

Same comment as above.

> +		return err;
> +	}
> +
> +	err = reset_control_deassert(priv->rst);
> +	if (err)
> +		dev_err(&rproc->dev, "failed to deassert reset\n");
> +
> +	return err;
> +}
> +
> +static int rcar_rproc_stop(struct rproc *rproc)
> +{
> +	struct rcar_rproc *priv = rproc->priv;
> +	int err;
> +
> +	err = reset_control_assert(priv->rst);
> +	if (err)
> +		dev_err(&rproc->dev, "failed to assert reset\n");
> +
> +	return err;
> +}
> +
> +static struct rproc_ops rcar_rproc_ops = {
> +	.prepare	= rcar_rproc_prepare,
> +	.start		= rcar_rproc_start,
> +	.stop		= rcar_rproc_stop,
> +	.load		= rproc_elf_load_segments,
> +	.parse_fw	= rcar_rproc_parse_fw,
> +	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> +	.sanity_check	= rproc_elf_sanity_check,
> +	.get_boot_addr	= rproc_elf_get_boot_addr,
> +
> +};
> +
> +static int rcar_rproc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct rcar_rproc *priv;
> +	struct rproc *rproc;
> +	int ret;
> +
> +	rproc = rproc_alloc(dev, np->name, &rcar_rproc_ops,
> +			    NULL, sizeof(*priv));
> +	if (!rproc)
> +		return -ENOMEM;
> +
> +	priv = rproc->priv;
> +	priv->rproc = rproc;

I don't see rcar_rproc::rproc being used anywhere.

> +	priv->dev = dev;
> +
> +	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rst)) {
> +		ret = PTR_ERR(priv->rst);
> +		dev_err(dev, "fail to acquire rproc reset\n");
> +		goto free_rproc;
> +	}
> +
> +	pm_runtime_enable(priv->dev);
> +	ret = pm_runtime_get_sync(priv->dev);

There is no dev_pm_ops for the platform driver nor clocks to manage for this
device - is there something that requires pm_runtime operations to be called?

> +	if (ret) {
> +		dev_err(&rproc->dev, "failed to power up\n");
> +		goto free_rproc;
> +	}
> +
> +	dev_set_drvdata(dev, rproc);
> +
> +	/* Manually start the rproc */
> +	rproc->auto_boot = false;
> +
> +	ret = rproc_add(rproc);
> +	if (ret) {
> +		dev_err(dev, "rproc_add failed\n");
> +		goto pm_disable;
> +	}
> +
> +	return 0;
> +
> +pm_disable:
> +	pm_runtime_disable(priv->dev);
> +free_rproc:
> +	rproc_free(rproc);
> +
> +	return ret;
> +}
> +
> +static int rcar_rproc_remove(struct platform_device *pdev)
> +{
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct rcar_rproc *priv = rproc->priv;
> +
> +	rproc_del(rproc);
> +	pm_runtime_disable(priv->dev);

As far as I can tell rcar_rproc::dev is not required.  It is only used in
rproc_probe() and rproc_remove() where pdev->dev is available.

> +	rproc_free(rproc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_rproc_of_match[] = {
> +	{ .compatible = "renesas,rcar-cr7" },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, rcar_rproc_of_match);
> +
> +static struct platform_driver rcar_rproc_driver = {
> +	.probe = rcar_rproc_probe,
> +	.remove = rcar_rproc_remove,
> +	.driver = {
> +		.name = "rcar-rproc",
> +		.of_match_table = rcar_rproc_of_match,
> +	},
> +};

Thanks,
Mathieu

> +
> +module_platform_driver(rcar_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Renesas Gen3 R-Car remote processor control driver");
> +MODULE_AUTHOR("Julien Massot <julien.massot@iot.bzh>");
> -- 
> 2.31.1
> 
> 

Re: [RFC PATCH 3/3] remoteproc: Add Renesas rcar driver

[Geert Uytterhoeven]

On Mon, Nov 8, 2021 at 7:42 PM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On Wed, Oct 27, 2021 at 09:30:20AM +0200, Julien Massot wrote:
> > Renesas Gen3 platform includes a Cortex-r7 processor.
> >
> > Both: the application cores (A5x) and the realtime core (CR7)
> > share access to the RAM and devices with the same address map,
> > so device addresses are equal to the Linux physical addresses.
> >
> > In order to initialize this remote processor we need to:
> > - power on the realtime core
> > - put the firmware in a ram area
> > - set the boot address for this firmware (reset vector)
> > - Deassert the reset
> >
> > This initial driver allows to start and stop the Cortex R7
> > processor.
> >
> > Signed-off-by: Julien Massot <julien.massot@iot.bzh>

> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -261,6 +261,17 @@ config QCOM_WCNSS_PIL
> >         verified and booted with the help of the Peripheral Authentication
> >         System (PAS) in TrustZone.
> >
> > +config RCAR_REMOTEPROC
> > +     tristate "Renesas RCAR remoteproc support"
>
> It is probably a good idea to include the type of SoC being supported, something
> like:
>
>         tristate "Renesas Gen3 RCAR remoteproc support"

R-Car Gen3 please

> That will make it easier to support future RCAR processors that may not share
> the same architecture.

> > --- /dev/null
> > +++ b/drivers/remoteproc/rcar_rproc.c

> > +static int rcar_rproc_probe(struct platform_device *pdev)
> > +{
> > +     struct device *dev = &pdev->dev;
> > +     struct device_node *np = dev->of_node;
> > +     struct rcar_rproc *priv;
> > +     struct rproc *rproc;
> > +     int ret;
> > +
> > +     rproc = rproc_alloc(dev, np->name, &rcar_rproc_ops,
> > +                         NULL, sizeof(*priv));
> > +     if (!rproc)
> > +             return -ENOMEM;
> > +
> > +     priv = rproc->priv;
> > +     priv->rproc = rproc;
>
> I don't see rcar_rproc::rproc being used anywhere.
>
> > +     priv->dev = dev;
> > +
> > +     priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > +     if (IS_ERR(priv->rst)) {
> > +             ret = PTR_ERR(priv->rst);
> > +             dev_err(dev, "fail to acquire rproc reset\n");
> > +             goto free_rproc;
> > +     }
> > +
> > +     pm_runtime_enable(priv->dev);
> > +     ret = pm_runtime_get_sync(priv->dev);
>
> There is no dev_pm_ops for the platform driver nor clocks to manage for this
> device - is there something that requires pm_runtime operations to be called?

Given

    cr7_rproc: cr7 {
        compatible = "renesas,rcar-cr7";
        memory-region = <&cr7_ram>;
        power-domains = <&sysc R8A7795_PD_CR7>;
        resets = <&cpg 222>;
        status = "okay";
    };

the pm_runtime_get_sync() is intended to power the CR7 power domain,
right?

However, I have my doubt about the (bindings for) that node, as it
does not represent the hardware.  Shouldn't the Cortex R7 have its
own CPU node instead, with an appropriate enable-method?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 3/3] remoteproc: Add Renesas rcar driver

[Julien Massot]

Hi Mathieu,

Thanks for the review !

>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 9a6eedc3994a..3e87eadbaf59 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -261,6 +261,17 @@ config QCOM_WCNSS_PIL
>>   	  verified and booted with the help of the Peripheral Authentication
>>   	  System (PAS) in TrustZone.
>>   
>> +config RCAR_REMOTEPROC
>> +	tristate "Renesas RCAR remoteproc support"
> 
> It is probably a good idea to include the type of SoC being supported, something
> like:
> 
>          tristate "Renesas Gen3 RCAR remoteproc support"
> 
> That will make it easier to support future RCAR processors that may not share
> the same architecture.

Ok, changed according to Geert's suggestion to:
"Renesas R-CAR Gen3 remoteproc support"

> 
> 
>> +	depends on ARCH_RENESAS
>> +	depends on REMOTEPROC
>> +	help
>> +	  Say y here to support R-Car realtime processor via the
>> +	  remote processor framework. An elf firmware can be loaded
>> +	  thanks to sysfs remoteproc entries. The remote processor
>> +	  can be started and stopped.
>> +	  This can be either built-in or a loadable module.
> 
> Please add the name of the module when compiled as such.
Ok


>> +
>> +#include "remoteproc_internal.h"
>> +
>> +struct rcar_rproc {
>> +	struct device			*dev;
>> +	struct rproc			*rproc;
>> +	struct reset_control            *rst;
>> +};
>> +
>> +static int rcar_rproc_mem_alloc(struct rproc *rproc,
>> +				 struct rproc_mem_entry *mem)
>> +{
>> +	struct device *dev = rproc->dev.parent;
>> +	void *va;
>> +
>> +	dev_dbg(dev, "map memory: %p+%zx\n", &mem->dma, mem->len);
> 
> I think this should be "map memory: %pa+%lx\n" to be consistent with dev_err()
> below and the original implementation in stm32_rproc.c.
Ok

..
>> +
>> +static int rcar_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
>> +{
>> +	int ret;
>> +
>> +	ret = rproc_elf_load_rsc_table(rproc, fw);
>> +	if (ret)
>> +		dev_info(&rproc->dev, "No resource table in elf\n");
> 
> In the above functions rproc->dev.parent is used for output.  I don't have a
> strong opinion on which of rproc->dev or rproc->dev.parent is used but I would
> like to see consistency throughout the driver.
Thanks I choosed to use rproc->dev. Indeed logs are more consistent now.
> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int rcar_rproc_start(struct rproc *rproc)
>> +{
>> +	struct rcar_rproc *priv = rproc->priv;
>> +	int err;
>> +
>> +	if (!rproc->bootaddr)
>> +		return -EINVAL;
>> +
>> +	err = rcar_rst_set_rproc_boot_addr(rproc->bootaddr);
>> +	if (err) {
>> +		dev_err(&rproc->dev, "failed to set rproc boot addr\n");
> 
> Same comment as above.
ok

>> +
>> +static int rcar_rproc_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = dev->of_node;
>> +	struct rcar_rproc *priv;
>> +	struct rproc *rproc;
>> +	int ret;
>> +
>> +	rproc = rproc_alloc(dev, np->name, &rcar_rproc_ops,
>> +			    NULL, sizeof(*priv));
>> +	if (!rproc)
>> +		return -ENOMEM;
>> +
>> +	priv = rproc->priv;
>> +	priv->rproc = rproc;
> 
> I don't see rcar_rproc::rproc being used anywhere.
Indeed, rproc member will be removed in next version.

> 
>> +	priv->dev = dev;
>> +
>> +	priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>> +	if (IS_ERR(priv->rst)) {
>> +		ret = PTR_ERR(priv->rst);
>> +		dev_err(dev, "fail to acquire rproc reset\n");
>> +		goto free_rproc;
>> +	}
>> +
>> +	pm_runtime_enable(priv->dev);
>> +	ret = pm_runtime_get_sync(priv->dev);
> 
> There is no dev_pm_ops for the platform driver nor clocks to manage for this
> device - is there something that requires pm_runtime operations to be called?
Will reply in Geert's reply.
>> +
>> +static int rcar_rproc_remove(struct platform_device *pdev)
>> +{
>> +	struct rproc *rproc = platform_get_drvdata(pdev);
>> +	struct rcar_rproc *priv = rproc->priv;
>> +
>> +	rproc_del(rproc);
>> +	pm_runtime_disable(priv->dev);
> 
> As far as I can tell rcar_rproc::dev is not required.  It is only used in
> rproc_probe() and rproc_remove() where pdev->dev is available.
Thanks rcar_rproc::dev will be removed in next version.


Regards,
-- 
Julien Massot [IoT.bzh]

Re: [RFC PATCH 3/3] remoteproc: Add Renesas rcar driver

[Julien Massot]

Hi Geert, Mathieu,

Thanks for the review

On 11/9/21 09:09, Geert Uytterhoeven wrote:
> On Mon, Nov 8, 2021 at 7:42 PM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
>> On Wed, Oct 27, 2021 at 09:30:20AM +0200, Julien Massot wrote:
>>> Renesas Gen3 platform includes a Cortex-r7 processor.
>>>
>>> Both: the application cores (A5x) and the realtime core (CR7)
>>> share access to the RAM and devices with the same address map,
>>> so device addresses are equal to the Linux physical addresses.
>>>
>>> In order to initialize this remote processor we need to:
>>> - power on the realtime core
>>> - put the firmware in a ram area
>>> - set the boot address for this firmware (reset vector)
>>> - Deassert the reset
>>>
>>> This initial driver allows to start and stop the Cortex R7
>>> processor.
>>>
>>> Signed-off-by: Julien Massot <julien.massot@iot.bzh>
> 
>>> --- a/drivers/remoteproc/Kconfig
>>> +++ b/drivers/remoteproc/Kconfig
>>> @@ -261,6 +261,17 @@ config QCOM_WCNSS_PIL
>>>          verified and booted with the help of the Peripheral Authentication
>>>          System (PAS) in TrustZone.
>>>
>>> +config RCAR_REMOTEPROC
>>> +     tristate "Renesas RCAR remoteproc support"
>>
>> It is probably a good idea to include the type of SoC being supported, something
>> like:
>>
>>          tristate "Renesas Gen3 RCAR remoteproc support"
> 
> R-Car Gen3 please
Thanks changed to "Renesas R-Car Gen3 remoteproc support".

>>
>>> +     priv->dev = dev;
>>> +
>>> +     priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>>> +     if (IS_ERR(priv->rst)) {
>>> +             ret = PTR_ERR(priv->rst);
>>> +             dev_err(dev, "fail to acquire rproc reset\n");
>>> +             goto free_rproc;
>>> +     }
>>> +
>>> +     pm_runtime_enable(priv->dev);
>>> +     ret = pm_runtime_get_sync(priv->dev);
>>
>> There is no dev_pm_ops for the platform driver nor clocks to manage for this
>> device - is there something that requires pm_runtime operations to be called?
> 
> Given
> 
>      cr7_rproc: cr7 {
>          compatible = "renesas,rcar-cr7";
>          memory-region = <&cr7_ram>;
>          power-domains = <&sysc R8A7795_PD_CR7>;
>          resets = <&cpg 222>;
>          status = "okay";
>      };
> 
> the pm_runtime_get_sync() is intended to power the CR7 power domain,
> right?

That's exactly why I'm calling pm_runtime_get_sync. The Cortex R7 power domain
needs to be enabled.
  
> However, I have my doubt about the (bindings for) that node, as it
> does not represent the hardware.  Shouldn't the Cortex R7 have its
> own CPU node instead, with an appropriate enable-method?

Yes, representation is really from Remoteproc point of view, the Cortex R7 is
represented as other devices, and doesn't have a cpu node. As far I know it's
how others remoteproc CPUs are described in other SoCs such as ST, Qcom, TI, or NXP
SoCs. From what I see, CPU nodes are given for binary compatible CPUs where Linux can
execute on.

Regards,

-- 
Julien Massot [IoT.bzh]

Re: [RFC PATCH 1/3] dt-bindings: remoteproc: Add Renesas R-Car

[Julien Massot]

Hi,

On 10/27/21 16:12, Rob Herring wrote:
> On Wed, 27 Oct 2021 09:30:18 +0200, Julien Massot wrote:
>> Renesas R-Car SoCs may contains a Realtime processor.
>> This patch adds binding for this remote processor.
>>
>> Signed-off-by: Julien Massot <julien.massot@iot.bzh>
>> ---
>>   .../remoteproc/renesas,rcar-rproc.yaml        | 66 +++++++++++++++++++
>>   1 file changed, 66 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:26.17-46: Warning (reg_format): /example-0/reserved-memory/cr7_ram@40040000:reg: property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 1)
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:22.13-20: Warning (ranges_format): /example-0/reserved-memory:ranges: empty "ranges" property but its #address-cells (2) differs from /example-0 (1)
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:24.39-27.15: Warning (avoid_default_addr_size): /example-0/reserved-memory/cr7_ram@40040000: Relying on default #address-cells value
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dts:24.39-27.15: Warning (avoid_default_addr_size): /example-0/reserved-memory/cr7_ram@40040000: Relying on default #size-cells value
> Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.example.dt.yaml: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size'
> Indeed address-cells and size-cells was not specified in this binding example,
will be fixed in next version.

Thanks for the test !

Regards,
-- 
Julien Massot [IoT.bzh]