Re: [PATCH v3] soc: renesas: rcar-rst: Add support to set rproc boot address

[Julien Massot <julien.massot@xxxxxxx>]

Hi Geert,

> > +/*
> > + * Most of the R-Car Gen3 SoCs have an ARM Realtime Core.
> > + * Firmware boot address has to be set in CR7BAR before
> > + * starting the realtime core.
> > + * Boot address must be aligned on a 256k boundary.
> > + */
> > +static int rcar_rst_set_gen3_rproc_boot_addr(u32 boot_addr)
>
> phys_addr_t?
Not sure, in the remoteproc subsystem the boot address is in the
remote processor address space, which can be a different mapping than the host
processor. On Gen3 both the realtime and the application cores share the same address
map. (excepted for 64 bits address)
So a physical address from the CA5x processors is the same on the CR7
processor.
My feeling is that it's better to keep u32.

> > +{
> > +       if (boot_addr % SZ_256K) {
> > +               pr_warn("Invalid boot address for CR7 processor,"
> > +                      "should be aligned on 256KiB got %x\n", boot_addr);
>
> Please don't split printed messages, for easier searching.
Sure will fix.
> > +               return -EINVAL;
> > +       }
> > +
> > +       iowrite32(boot_addr, rcar_rst_base + CR7BAR);
> > +       iowrite32(boot_addr | CR7BAREN, rcar_rst_base + CR7BAR);
> > +
> > +       return 0;
> > +}
> > +
> >   struct rst_config {
> >          unsigned int modemr;            /* Mode Monitoring Register Offset */
> >          int (*configure)(void __iomem *base);   /* Platform specific config */
> > +       int (*set_rproc_boot_addr)(u32 boot_addr);
>
> phys_addr_t
>
> >   };
> >
> >   static const struct rst_config rcar_rst_gen1 __initconst = {
>
> > @@ -130,3 +157,12 @@ int __init rcar_rst_read_mode_pins(u32 *mode)
> >          *mode = saved_mode;
> >          return 0;
> >   }
> > +
> > +int rcar_rst_set_rproc_boot_addr(u32 boot_addr)
>
> phys_addr_t
>
> > +{
> > +       if (!rcar_rst_set_rproc_boot_addr_func)
> > +               return -EIO;
> > +
> > +       return rcar_rst_set_rproc_boot_addr_func(boot_addr);
> > +}
> > +EXPORT_SYMBOL(rcar_rst_set_rproc_boot_addr);
>
> EXPORT_SYMBOL_GPL?
Ok

> Do you have a public user of this code, too?
I have one that I plan to submit to the remoteproc subsystem once
it will be possible to set the CR7 boot address.

Please have a look there
https://github.com/iotbzh/linux/blob/iot/for-upstream/rproc/drivers/remoteproc/rcar_rproc.c#L114

In this function the remoteproc subsystem already fetched a firmware and parsed the elf file.
The firmware entry point is stored in a u64 variable
https://github.com/iotbzh/linux/blob/iot/for-upstream/rproc/include/linux/remoteproc.h#L550

Then we can start the processor by deasserting the reset.


Thanks,
Julien