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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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@xxxxxxx>

--- 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]




[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux