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

[PATCH v1 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 |  15 ++
 arch/arm64/boot/dts/renesas/r8a77951.dtsi     |   7 +
 drivers/remoteproc/Kconfig                    |  12 +
 drivers/remoteproc/Makefile                   |   1 +
 drivers/remoteproc/rcar_rproc.c               | 226 ++++++++++++++++++
 6 files changed, 327 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
 create mode 100644 drivers/remoteproc/rcar_rproc.c

-- 
2.31.1

[PATCH v1 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>
---
Change since RFC:
Adds missing address-cells and size-cells and change reg cell
accordingly.
---
 .../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..dcb6b0f75dda
--- /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.yaml)
+
+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 {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        cr7_ram: cr7_ram@40040000 {
+            no-map;
+            reg = <0x0 0x40040000 0x0 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

[PATCH v1 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>
---

No change since RFC, Geert most likely it sounds better
to drop this patch in v2 ?

---
 arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts | 15 +++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77951.dtsi     |  7 +++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts b/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
index 06d4e948eb0f..703d0d33efe6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
@@ -34,6 +34,16 @@ memory@700000000 {
 		device_type = "memory";
 		reg = <0x7 0x00000000 0x0 0x40000000>;
 	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+
+		cr7_ram: cr7_ram@40040000 {
+			no-map;
+			reg = <0x0 0x40040000 0x0 0x1fc0000>;
+		};
+	};
 };
 
 &du {
@@ -48,3 +58,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

[PATCH v1 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>
---

Change since RFC:
- Improve Kconfig description.
- Use consistent device for pr_* messages.
- Drop unused dev and rproc member of struct rcar_rproc.

---
 drivers/remoteproc/Kconfig      |  12 ++
 drivers/remoteproc/Makefile     |   1 +
 drivers/remoteproc/rcar_rproc.c | 226 ++++++++++++++++++++++++++++++++
 3 files changed, 239 insertions(+)
 create mode 100644 drivers/remoteproc/rcar_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9a6eedc3994a..5b0c617d45f6 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -261,6 +261,18 @@ config QCOM_WCNSS_PIL
 	  verified and booted with the help of the Peripheral Authentication
 	  System (PAS) in TrustZone.
 
+config RCAR_REMOTEPROC
+	tristate "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.
+	  If compiled as module (M), the module name is rcar_rproc.
+
 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..795089dc4337
--- /dev/null
+++ b/drivers/remoteproc/rcar_rproc.c
@@ -0,0 +1,226 @@
+// 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 reset_control *rst;
+};
+
+static int rcar_rproc_mem_alloc(struct rproc *rproc,
+				 struct rproc_mem_entry *mem)
+{
+	struct device *dev = &rproc->dev;
+	void *va;
+
+	dev_dbg(dev, "map memory: %pa+%lx\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, "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(&rproc->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->rst = devm_reset_control_get_exclusive(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(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret) {
+		dev_err(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(dev);
+free_rproc:
+	rproc_free(rproc);
+
+	return ret;
+}
+
+static int rcar_rproc_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rproc *rproc = platform_get_drvdata(pdev);
+
+	rproc_del(rproc);
+	pm_runtime_disable(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: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver

[Biju Das]

Hi Julien,

Thanks for the patch.

One question CR7 Can be master boot processor. In that case
How to avoid loading and booting  CR7 processor from Linux?
Reading boot modes??

Regards,
biju


> Subject: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver
> 
> 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>
> ---
> 
> Change since RFC:
> - Improve Kconfig description.
> - Use consistent device for pr_* messages.
> - Drop unused dev and rproc member of struct rcar_rproc.
> 
> ---
>  drivers/remoteproc/Kconfig      |  12 ++
>  drivers/remoteproc/Makefile     |   1 +
>  drivers/remoteproc/rcar_rproc.c | 226 ++++++++++++++++++++++++++++++++
>  3 files changed, 239 insertions(+)
>  create mode 100644 drivers/remoteproc/rcar_rproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index
> 9a6eedc3994a..5b0c617d45f6 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -261,6 +261,18 @@ config QCOM_WCNSS_PIL
>  	  verified and booted with the help of the Peripheral Authentication
>  	  System (PAS) in TrustZone.
> 
> +config RCAR_REMOTEPROC
> +	tristate "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.
> +	  If compiled as module (M), the module name is rcar_rproc.
> +
>  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..795089dc4337
> --- /dev/null
> +++ b/drivers/remoteproc/rcar_rproc.c
> @@ -0,0 +1,226 @@
> +// 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 reset_control *rst;
> +};
> +
> +static int rcar_rproc_mem_alloc(struct rproc *rproc,
> +				 struct rproc_mem_entry *mem)
> +{
> +	struct device *dev = &rproc->dev;
> +	void *va;
> +
> +	dev_dbg(dev, "map memory: %pa+%lx\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, "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(&rproc->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->rst = devm_reset_control_get_exclusive(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(dev);
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret) {
> +		dev_err(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(dev);
> +free_rproc:
> +	rproc_free(rproc);
> +
> +	return ret;
> +}
> +
> +static int rcar_rproc_remove(struct platform_device *pdev) {
> +	struct device *dev = &pdev->dev;
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +
> +	rproc_del(rproc);
> +	pm_runtime_disable(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: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver

[Julien Massot]

Hi Biju,

> 
> One question CR7 Can be master boot processor. In that case
> How to avoid loading and booting  CR7 processor from Linux?
> Reading boot modes??
> 
Thanks for the question.

I did not test this case. There is also other scenarios where the
Cortex-R7 is started by an earlier component such as BL2, u-boot or
OP-Tee.
In these cases Linux should not try to start / stop this remote processor.
One idea could be to read the power status CR7PSTR / PWRSR7, or use
one of the MFIS register as a communication register. STM32 processor
use this last solution to indicate that the remote processor is
already started, in that scenario remoteproc driver starts in 'detached' state
instead of 'offline' state.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/remoteproc.h#n418

In that state, remoteproc driver can initiate communication with
this remote processor but will not try to start or to stop it.

That's something I have in mind, with an existing implementation there
https://github.com/iotbzh/linux/blob/iot/julien/rproc/drivers/remoteproc/rcar_rproc.c#L524
(that is not ready for upstream yet :)).

I guess this issue also exists when one device is dedicated to the secure world, so
the device exists, but not available for Linux. The most obvious (and dirty ?) solution is to keep
the device disabled in dts.

Regards,
Julien

RE: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver

[Biju Das]

Hi Julien,

> Subject: Re: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver
> 
> Hi Biju,
> 
> >
> > One question CR7 Can be master boot processor. In that case How to
> > avoid loading and booting  CR7 processor from Linux?
> > Reading boot modes??
> >
> Thanks for the question.
> 
> I did not test this case. 

OK.

There is also other scenarios where the
> Cortex-R7 is started by an earlier component such as BL2, u-boot or OP-
> Tee.
> In these cases Linux should not try to start / stop this remote processor.


OK. But Linux can check whether CR7 is alive at frequent interval and issue
Soft-reboot, if there is a  hang.

> One idea could be to read the power status CR7PSTR / PWRSR7, or use one of
> the MFIS register as a communication register.

I know MFIS is used for communication between CR7 and CA-57. But don't know
much details.

STM32 processor use this
> last solution to indicate that the remote processor is already started, in
> that scenario remoteproc driver starts in 'detached' state instead of
> 'offline' state.
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2F
> include%2Flinux%2Fremoteproc.h%23n418&data=04%7C01%7Cbiju.das.jz%40bp.
> renesas.com%7C945a25980e5a49cc361a08d9a846020c%7C53d82571da1947e49cb4625a1
> 66a4a2a%7C0%7C0%7C637725840924834250%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=yXH
> 2qBpmUI2YtZRnApFdWt6iONcFKDPSPa45AirThBw%3D&reserved=0
> 
> In that state, remoteproc driver can initiate communication with this
> remote processor but will not try to start or to stop it.
> 
> That's something I have in mind, with an existing implementation there
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
> om%2Fiotbzh%2Flinux%2Fblob%2Fiot%2Fjulien%2Frproc%2Fdrivers%2Fremoteproc%2
> Frcar_rproc.c%23L524&data=04%7C01%7Cbiju.das.jz%40bp.renesas.com%7C945
> a25980e5a49cc361a08d9a846020c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7
> C637725840924834250%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=WYKygEUiXyliOmX2CUxk
> Yfj5pq2QOmZC%2BGtXRAlB%2Bws%3D&reserved=0
> (that is not ready for upstream yet :)).
> 
> I guess this issue also exists when one device is dedicated to the secure
> world, so the device exists, but not available for Linux. 

May be for prototyping activity, if some one wants to run an RTOS on CR7.
This will be a good solution. Linux will load and boot CR7 which
Run an RTOS which communicates with Linux. Once communication established
Linux can check the health of CR7 at frequent intervals and take necessary action.

Regards,
Biju


The most obvious
> (and dirty ?) solution is to keep the device disabled in dts.
> 
> Regards,
> Julien

Re: [PATCH v1 0/3] Initial Renesas R-Car remoteproc support

[Mathieu Poirier]

Good morning,

I have received your patchset but the backlog accumulated over the last little
while as become impressive.  As such I may not be able to provide a review in
a timeframe that usually expected.


Thanks,
Mathieu

On Mon, Nov 15, 2021 at 02:50:29PM +0100, Julien Massot wrote:
> 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 |  15 ++
>  arch/arm64/boot/dts/renesas/r8a77951.dtsi     |   7 +
>  drivers/remoteproc/Kconfig                    |  12 +
>  drivers/remoteproc/Makefile                   |   1 +
>  drivers/remoteproc/rcar_rproc.c               | 226 ++++++++++++++++++
>  6 files changed, 327 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/renesas,rcar-rproc.yaml
>  create mode 100644 drivers/remoteproc/rcar_rproc.c
> 
> -- 
> 2.31.1
> 
> 

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

[Mathieu Poirier]

Hi,

On Mon, Nov 15, 2021 at 02:50:32PM +0100, 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>
> ---
> 
> Change since RFC:
> - Improve Kconfig description.
> - Use consistent device for pr_* messages.
> - Drop unused dev and rproc member of struct rcar_rproc.
> 
> ---
>  drivers/remoteproc/Kconfig      |  12 ++
>  drivers/remoteproc/Makefile     |   1 +
>  drivers/remoteproc/rcar_rproc.c | 226 ++++++++++++++++++++++++++++++++
>  3 files changed, 239 insertions(+)
>  create mode 100644 drivers/remoteproc/rcar_rproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9a6eedc3994a..5b0c617d45f6 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -261,6 +261,18 @@ config QCOM_WCNSS_PIL
>  	  verified and booted with the help of the Peripheral Authentication
>  	  System (PAS) in TrustZone.
>  
> +config RCAR_REMOTEPROC
> +	tristate "Renesas R-CAR Gen3 remoteproc support"
> +	depends on ARCH_RENESAS
> +	depends on REMOTEPROC

You should be able to remove the dependency on REMOTEPROC since this is already in
the "if REMOTEPROC" block.

> +	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.
> +	  If compiled as module (M), the module name is rcar_rproc.
> +
>  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..795089dc4337
> --- /dev/null
> +++ b/drivers/remoteproc/rcar_rproc.c
> @@ -0,0 +1,226 @@
> +// 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 reset_control *rst;
> +};
> +
> +static int rcar_rproc_mem_alloc(struct rproc *rproc,
> +				 struct rproc_mem_entry *mem)
> +{
> +	struct device *dev = &rproc->dev;
> +	void *va;
> +
> +	dev_dbg(dev, "map memory: %pa+%lx\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",

The sparse checker doesn't like %lx so probably be better to go with just %x.
Apologies for suggesting to use %lx.

> +			&mem->dma, mem->len);
> +		return -ENOMEM;
> +	}
> +
> +	/* Update memory entry va */
> +	mem->va = va;

Talking about the sparse checker, you will see complaints about @va not being of
type "void __iomem *".  You can ignore those as this would likely require to
refactor the rproc_mem_entry structure, which is outside the scope of this work.

> +
> +	return 0;
> +}
> +
> +static int rcar_rproc_mem_release(struct rproc *rproc,
> +				   struct rproc_mem_entry *mem)
> +{
> +	dev_dbg(&rproc->dev, "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(&rproc->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->rst = devm_reset_control_get_exclusive(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(dev);
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret) {
> +		dev_err(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(dev);
> +free_rproc:
> +	rproc_free(rproc);
> +
> +	return ret;
> +}
> +
> +static int rcar_rproc_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +
> +	rproc_del(rproc);
> +	pm_runtime_disable(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,
> +	},
> +};

This set is just as clean as the RFC.  If it wasn't for the DTS bindings that
need to be ack'ed by Rob, I probably would have made the above modifications and
applied this patch.

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: [PATCH v1 3/3] remoteproc: Add Renesas rcar driver

[Julien Massot]

Hi Mathieu,
Thanks for the review !

>> +config RCAR_REMOTEPROC
>> +	tristate "Renesas R-CAR Gen3 remoteproc support"
>> +	depends on ARCH_RENESAS
>> +	depends on REMOTEPROC
> 
> You should be able to remove the dependency on REMOTEPROC since this is already in
> the "if REMOTEPROC" block.
Will fix.

...
> 
>> +
>> +	dev_dbg(dev, "map memory: %pa+%lx\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",
> 
> The sparse checker doesn't like %lx so probably be better to go with just %x.
> Apologies for suggesting to use %lx.

With %x gcc complains on arm64 build will go back to %zx.

> 
>> +			&mem->dma, mem->len);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/* Update memory entry va */
>> +	mem->va = va;
> 
> Talking about the sparse checker, you will see complaints about @va not being of
> type "void __iomem *".  You can ignore those as this would likely require to
> refactor the rproc_mem_entry structure, which is outside the scope of this work.

Ok, to be honest, I was not aware of the sparse tool, thanks a lot to point me to
this tool.

> 
> This set is just as clean as the RFC.  If it wasn't for the DTS bindings that
> need to be ack'ed by Rob, I probably would have made the above modifications and
> applied this patch.
> 
> Thanks,
> Mathieu

No problem will send a v2.

Regards,
Julien
-- 
Julien Massot [IoT.bzh]

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

[Mathieu Poirier]

On Wed, 24 Nov 2021 at 04:07, Julien Massot <julien.massot@iot.bzh> wrote:
>
> Hi Mathieu,
> Thanks for the review !
>
> >> +config RCAR_REMOTEPROC
> >> +    tristate "Renesas R-CAR Gen3 remoteproc support"
> >> +    depends on ARCH_RENESAS
> >> +    depends on REMOTEPROC
> >
> > You should be able to remove the dependency on REMOTEPROC since this is already in
> > the "if REMOTEPROC" block.
> Will fix.
>
> ...
> >
> >> +
> >> +    dev_dbg(dev, "map memory: %pa+%lx\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",
> >
> > The sparse checker doesn't like %lx so probably be better to go with just %x.
> > Apologies for suggesting to use %lx.
>
> With %x gcc complains on arm64 build will go back to %zx.

Ok

>
> >
> >> +                    &mem->dma, mem->len);
> >> +            return -ENOMEM;
> >> +    }
> >> +
> >> +    /* Update memory entry va */
> >> +    mem->va = va;
> >
> > Talking about the sparse checker, you will see complaints about @va not being of
> > type "void __iomem *".  You can ignore those as this would likely require to
> > refactor the rproc_mem_entry structure, which is outside the scope of this work.
>
> Ok, to be honest, I was not aware of the sparse tool, thanks a lot to point me to
> this tool.
>
> >
> > This set is just as clean as the RFC.  If it wasn't for the DTS bindings that
> > need to be ack'ed by Rob, I probably would have made the above modifications and
> > applied this patch.
> >
> > Thanks,
> > Mathieu
>
> No problem will send a v2.
>
> Regards,
> Julien
> --
> Julien Massot [IoT.bzh]
>

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

[Rob Herring]

On Mon, Nov 15, 2021 at 02:50:30PM +0100, 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>
> ---
> Change since RFC:
> Adds missing address-cells and size-cells and change reg cell
> accordingly.
> ---
>  .../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..dcb6b0f75dda
> --- /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.yaml)
> +
> +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 {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        cr7_ram: cr7_ram@40040000 {
> +            no-map;
> +            reg = <0x0 0x40040000 0x0 0x1fc0000>;
> +        };
> +    };
> +
> +    cr7_rproc: cr7 {
> +        compatible = "renesas,rcar-cr7";
> +        memory-region = <&cr7_ram>;
> +        power-domains = <&sysc R8A7795_PD_CR7>;
> +        resets = <&cpg 222>;
> +        status = "okay";

Don't show status in examples.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Julien Massot]

Hi Rob,

On 11/29/21 23:01, Rob Herring wrote:
> On Mon, Nov 15, 2021 at 02:50:30PM +0100, 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>
>> ---
>> Change since RFC:
>> Adds missing address-cells and size-cells and change reg cell
>> accordingly.
>> ---
>>   .../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..dcb6b0f75dda
>> --- /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.yaml)
>> +
>> +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 {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        cr7_ram: cr7_ram@40040000 {
>> +            no-map;
>> +            reg = <0x0 0x40040000 0x0 0x1fc0000>;
>> +        };
>> +    };
>> +
>> +    cr7_rproc: cr7 {
>> +        compatible = "renesas,rcar-cr7";
>> +        memory-region = <&cr7_ram>;
>> +        power-domains = <&sysc R8A7795_PD_CR7>;
>> +        resets = <&cpg 222>;
>> +        status = "okay";
> 
> Don't show status in examples.
> 
> With that,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
Thanks,
Will remove the status, and add your Reviewed-by tag in the v2.

Regards,
-- 
Julien Massot [IoT.bzh]

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

[Geert Uytterhoeven]

Hi Julien,

On Mon, Nov 15, 2021 at 2:50 PM Julien Massot <julien.massot@iot.bzh> wrote:
> 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>

Thanks for your patch!

> No change since RFC, Geert most likely it sounds better
> to drop this patch in v2 ?

Indeed, cfr. my comments below.

> --- a/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77951-ulcb.dts
> @@ -34,6 +34,16 @@ memory@700000000 {
>                 device_type = "memory";
>                 reg = <0x7 0x00000000 0x0 0x40000000>;
>         };
> +
> +       reserved-memory {
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +
> +               cr7_ram: cr7_ram@40040000 {
> +                       no-map;
> +                       reg = <0x0 0x40040000 0x0 0x1fc0000>;
> +               };
> +       };

Tgis depends on a specific configuration, and is thus not suitable for
upstream.


>  };
>
>  &du {
> @@ -48,3 +58,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";
> +               };
> +

This part is generic, but I think the cr7 node should be moved outside
the soc node (like the PMUs and the ARMv8 timer), as it does not have
a unit address.

>                 rwdt: watchdog@e6020000 {
>                         compatible = "renesas,r8a7795-wdt", "renesas,rcar-gen3-wdt";
>                         reg = <0 0xe6020000 0 0x0c>;

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