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
> 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@xxxxxxxxxxxxxx
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
