Hi Geert,
Thanks for the review !
All the point will be addressed in v3.
+static int rcar_rst_set_gen3_rproc_boot_addr(u32 boot_addr);
I think you can avoid this forward declaration, by reordering definitions.
Indeed I will reorder the definition.
-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.
Ok
@@ -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:
Will replace by 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.
Ok will improve this comment.
+ * Boot address is on 32bit, and should be aligned on
+ * 4k boundary.
256 KiB
Ok
+ */
+int rcar_rst_set_gen3_rproc_boot_addr(u32 boot_addr)
Missing static?
Yes will be fixed in v3.
+{
+ 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
ok
+ 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.
Ok
Regards,
--
Julien Massot [IoT.bzh]
