Hi Julien,
On Wed, Sep 22, 2021 at 4:52 PM Julien Massot <julien.massot@xxxxxxx> wrote:
> R-Car Gen3 SoC series has a realtime processor, the boot
> address of this processor can be set thanks to CR7BAR register
> of the reset module.
>
> Export this function so that it's possible to set the boot
> address from a remoteproc driver.
>
> Also drop the __initdata qualifier on rcar_rst_base,
> since we will use this address later than init time.
>
> Signed-off-by: Julien Massot <julien.massot@xxxxxxx>
> ---
>
> Change since RFC:
> Introduce set_rproc_boot_addr function pointer, so that
> it can be reused for other R-Car SoC generation.
Thanks for the update!
> --- a/drivers/soc/renesas/rcar-rst.c
> +++ b/drivers/soc/renesas/rcar-rst.c
> @@ -12,6 +12,8 @@
>
> #define WDTRSTCR_RESET 0xA55A0002
> #define WDTRSTCR 0x0054
> +#define CR7BAR 0x0070
> +#define CR7BAREN BIT(4)
>
> static int rcar_rst_enable_wdt_reset(void __iomem *base)
> {
> @@ -19,25 +21,29 @@ static int rcar_rst_enable_wdt_reset(void __iomem *base)
> return 0;
> }
>
> +static int rcar_rst_set_gen3_rproc_boot_addr(u32 boot_addr);
I think you can avoid this forward declaration, by reordering definitions.
> +
> 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);
> };
>
> -static const struct rst_config rcar_rst_gen1 __initconst = {
> +static const struct rst_config rcar_rst_gen1 = {
> .modemr = 0x20,
> };
>
> -static const struct rst_config rcar_rst_gen2 __initconst = {
> +static const struct rst_config rcar_rst_gen2 = {
> .modemr = 0x60,
> .configure = rcar_rst_enable_wdt_reset,
> };
>
> -static const struct rst_config rcar_rst_gen3 __initconst = {
> +static const struct rst_config rcar_rst_gen3 = {
> .modemr = 0x60,
> + .set_rproc_boot_addr = rcar_rst_set_gen3_rproc_boot_addr,
> };
>
> -static const struct rst_config rcar_rst_r8a779a0 __initconst = {
> +static const struct rst_config rcar_rst_r8a779a0 = {
> .modemr = 0x00, /* MODEMR0 and it has CPG related bits */
> };
I prefer to keep these in __init, as there are multiple instances.
If you need to access some fields later, just make non-__init copies
during initialization.
>
> @@ -76,13 +82,13 @@ static const struct of_device_id rcar_rst_matches[] __initconst = {
> { /* sentinel */ }
> };
>
> -static void __iomem *rcar_rst_base __initdata;
> +static void __iomem *rcar_rst_base;
> static u32 saved_mode __initdata;
> +static const struct rst_config *cfg;
You don't need a pointer to the whole config struct, just a function pointer:
static int int (*rcar_rst_set_rproc_boot_addr)(u32 boot_addr);
> @@ -130,3 +136,33 @@ int __init rcar_rst_read_mode_pins(u32 *mode)
> *mode = saved_mode;
> return 0;
> }
> +
> +/*
> + * Most of R-Car Gen3 SoCs have an ARM Realtime Core.
> + * Firmware boot address can be set before starting,
> + * the realtime core thanks to CR7BAR register.
That sentence sounds weird to me.
> + * Boot address is on 32bit, and should be aligned on
> + * 4k boundary.
256 KiB
> + */
> +int rcar_rst_set_gen3_rproc_boot_addr(u32 boot_addr)
Missing static?
> +{
> + if (boot_addr % SZ_4K) {
SZ_256K, as noted in your follow-up.
> + pr_warn("Invalid boot address for CR7 processor,"
> + "should be aligned on 4k got %x\n", boot_addr);
256 KiB
> + return -EINVAL;
> + }
> +
> + iowrite32(boot_addr, rcar_rst_base + CR7BAR);
> + iowrite32(boot_addr | CR7BAREN, rcar_rst_base + CR7BAR);
> +
> + return 0;
> +}
> +
> +int rcar_rst_set_rproc_boot_addr(u32 boot_addr)
> +{
> + if (!rcar_rst_base || !cfg->set_rproc_boot_addr)
This can just check the rcar_rst_set_rproc_boot_addr pointer.
> + return -EIO;
> +
> + return cfg->set_rproc_boot_addr(boot_addr);
> +}
> +EXPORT_SYMBOL(rcar_rst_set_rproc_boot_addr);
> diff --git a/include/linux/soc/renesas/rcar-rst.h b/include/linux/soc/renesas/rcar-rst.h
> index 7899a5b8c247..7c97c2c4bba6 100644
> --- a/include/linux/soc/renesas/rcar-rst.h
> +++ b/include/linux/soc/renesas/rcar-rst.h
> @@ -4,8 +4,10 @@
>
> #ifdef CONFIG_RST_RCAR
> int rcar_rst_read_mode_pins(u32 *mode);
> +int rcar_rst_set_rproc_boot_addr(u32 boot_addr);
> #else
> static inline int rcar_rst_read_mode_pins(u32 *mode) { return -ENODEV; }
> +static inline int rcar_rst_set_rproc_boot_addr(u32 boot_addr) { return -ENODEV; }
> #endif
>
> #endif /* __LINUX_SOC_RENESAS_RCAR_RST_H__ */
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
