| Message ID | 20210914094650.15235-2-julien.massot@iot.bzh (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add support for setting Cortex R7 boot address | expand |
Hi Julien, On Tue, Sep 14, 2021 at 11:56 AM Julien Massot <julien.massot@iot.bzh> 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@iot.bzh> Thanks for your patch! > --- 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) > { > @@ -76,7 +78,7 @@ 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 int __init rcar_rst_init(void) > @@ -130,3 +132,27 @@ 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. > + * Boot address is on 32bit, and should be aligned on > + * 4k boundary. > + */ > +int rcar_rst_set_rproc_boot_addr(u32 boot_addr) > +{ > + if (!rcar_rst_base) > + return -EIO; > + > + if (boot_addr % SZ_4K) { > + pr_warn("Invalid boot address for remote processor, should be aligned on 4k\n"); > + boot_addr -= boot_addr % SZ_4K; I think it would be safer to just return -EINVAL. > + } > + > + boot_addr |= CR7BAREN; > + iowrite32(boot_addr, rcar_rst_base + CR7BAR); According to Note 2 for the CR7BAR register, you must do this in two steps, first without CR7BAREN set, then with CR7BAREN set. See also how CA7BAR and CA15BAR are handled in arch/arm/mach-shmobile/pm-rcar-gen2.c. Note that CA15/CA7 on R-Car Gen2 (and CA57/CA53 on Gen3, but that's hidden by ACPI), unlike CR7, also need RESCNT handling. > + > + return 0; > +} > +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__ */ In general, I think this looks like a good abstraction, which we can also use for further abstraction of R-Car Gen2 (cfr. [1]). I'm just wondering if we should pass the BAR offset to rcar_rst_set_rproc_boot_addr() explicitly (and rename the function), or have multiple functions for the different BARs. Comments? [1] "[PATCH/RFC 0/6] ARM: r8a73a4: Add SMP support" https://lore.kernel.org/linux-renesas-soc/20210204135409.1652604-1-geert+renesas@glider.be/ Gr{oetje,eeting}s, Geert
Hi Geert, >> + >> +/* >> + * 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. >> + * Boot address is on 32bit, and should be aligned on >> + * 4k boundary. >> + */ >> +int rcar_rst_set_rproc_boot_addr(u32 boot_addr) >> +{ >> + if (!rcar_rst_base) >> + return -EIO; >> + >> + if (boot_addr % SZ_4K) { >> + pr_warn("Invalid boot address for remote processor, should be aligned on 4k\n"); >> + boot_addr -= boot_addr % SZ_4K; > > I think it would be safer to just return -EINVAL. Indeed, I should better fix my firmware or my remoteproc driver to give correct input to this function. will return -EINVAL in case of bad alignment. > >> + } >> + >> + boot_addr |= CR7BAREN; >> + iowrite32(boot_addr, rcar_rst_base + CR7BAR); > > According to Note 2 for the CR7BAR register, you must do this in two steps, > first without CR7BAREN set, then with CR7BAREN set. You're right will fix. > See also how CA7BAR and CA15BAR are handled in > arch/arm/mach-shmobile/pm-rcar-gen2.c. > > Note that CA15/CA7 on R-Car Gen2 (and CA57/CA53 on Gen3, but > that's hidden by ACPI), unlike CR7, also need RESCNT handling. > >> + >> + return 0; >> +} >> +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__ */ > > In general, I think this looks like a good abstraction, which we can > also use for further abstraction of R-Car Gen2 (cfr. [1]). Yes I was also thinking about future generation like Gen4, but I don't have the documentation at this point. From what I understand in [1]: CA7BAR in Gen2 is managed by the SYSC module and not by the RST module as for CR7BAR in Gen3. So despite the fact that the procedure is similar, we won't be able to set CA7BAR in rcar-rst.c. > > I'm just wondering if we should pass the BAR offset to > rcar_rst_set_rproc_boot_addr() explicitly (and rename the function), > or have multiple functions for the different BARs. Passing BAR offset will imply to spread RST module offsets across different subsystems, and the second question will be to be able to do the correct boundary check for the targeted processor: CR7BAR is aligned on 4k an it looks like CA7BAR is on 256k. It looks like it's manageable thanks to the driver data which already holds the 'mode monitor register' offset per SoC generation. One missing point will be for future R-Car SoCs: trends on others SoC seems to be to go to have several 'realtime', or remote processor. In this case this function will not scale up. Thanks for the review !
Hi Julien, On Wed, Sep 22, 2021 at 11:54 AM Julien Massot <julien.massot@iot.bzh> wrote: > > In general, I think this looks like a good abstraction, which we can > > also use for further abstraction of R-Car Gen2 (cfr. [1]). > Yes I was also thinking about future generation like Gen4, but I don't have the documentation > at this point. > From what I understand in [1]: CA7BAR in Gen2 is managed by the SYSC module and not by the RST module > as for CR7BAR in Gen3. So despite the fact that the procedure is similar, we won't be able to set CA7BAR in > rcar-rst.c. On R-Car Gen2, CA7BAR is managed by the RST module. On R-Mobile APE6, it is managed by the SYSC module. > > I'm just wondering if we should pass the BAR offset to > > rcar_rst_set_rproc_boot_addr() explicitly (and rename the function), > > or have multiple functions for the different BARs. > Passing BAR offset will imply to spread RST module offsets across different subsystems, > and the second question will be to be able to do the correct boundary check for the targeted > processor: CR7BAR is aligned on 4k an it looks like CA7BAR is on 256k. It looks like it's > manageable thanks to the driver data which already holds the 'mode monitor register' offset per SoC generation. > > One missing point will be for future R-Car SoCs: trends on others SoC seems to be to go to have > several 'realtime', or remote processor. In this case this function will not scale up. On R-Car V3U, CR52BAR is managed by the APMU module. There's not much we can do about future processors yet ;-) Gr{oetje,eeting}s, Geert
diff --git a/drivers/soc/renesas/rcar-rst.c b/drivers/soc/renesas/rcar-rst.c index 8a1e402ea799..7f8452b07cdf 100644 --- 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) { @@ -76,7 +78,7 @@ 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 int __init rcar_rst_init(void) @@ -130,3 +132,27 @@ 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. + * Boot address is on 32bit, and should be aligned on + * 4k boundary. + */ +int rcar_rst_set_rproc_boot_addr(u32 boot_addr) +{ + if (!rcar_rst_base) + return -EIO; + + if (boot_addr % SZ_4K) { + pr_warn("Invalid boot address for remote processor, should be aligned on 4k\n"); + boot_addr -= boot_addr % SZ_4K; + } + + boot_addr |= CR7BAREN; + iowrite32(boot_addr, rcar_rst_base + CR7BAR); + + return 0; +} +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__ */
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@iot.bzh> --- drivers/soc/renesas/rcar-rst.c | 28 +++++++++++++++++++++++++++- include/linux/soc/renesas/rcar-rst.h | 2 ++ 2 files changed, 29 insertions(+), 1 deletion(-)