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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 !

--
Julien Massot [IoT.bzh]



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux