On 3/21/2023 11:07 PM, Krzysztof Kozlowski wrote:
On 21/03/2023 14:51, Souradeep Chowdhury wrote:
All of Qualcomm's proprietary Android boot-loaders capture boot time
stats, like the time when the bootloader started execution and at what
point the bootloader handed over control to the kernel etc. in the IMEM
region. This information is captured in a specific format by this driver
by mapping a structure to the IMEM memory region and then accessing the
members of the structure to print the information. This information is
useful in verifying if the existing boot KPIs have regre
+/**
+ * struct boot_stats - timestamp information related to boot stats
+ * @bootloader_start: Time for the starting point of the abl bootloader
+ * @bootloader_end: Time when the kernel starts loading from abl bootloader
+ */
+struct boot_stats {
+ u32 bootloader_start;
+ u32 bootloader_end;
+} __packed;
+
+static struct boot_stats __iomem *boot_stats;
+static void __iomem *mpm_counter_base;
+static u32 mpm_counter_freq;
No file-scope variables. Does not scale, not easy for review and
maintenance. Avoid such code.
Ack
+
+static int mpm_parse_dt(void)
+{
+ struct device_node *np_imem, *np_mpm2;
+
+ np_imem = of_find_compatible_node(NULL, NULL,
+ "qcom,imem-boot_stats");
+ if (!np_imem) {
+ pr_err("can't find qcom,imem node\n");
So you are printing errors everywhere, on every soc and with compile
test on every platform there is in the world... sorry, it does not work
like that.
Ack
+ return -ENODEV;
+ }
+ boot_stats = of_iomap(np_imem, 0);
+ if (!boot_stats) {
+ pr_err("boot_stats: Can't map imem\n");
+ goto err1;
+ }
+
+static void __exit boot_stats_exit(void)
+{
+}
+module_exit(boot_stats_exit)
I don't think this is some special code which deserves init calls. Make
it module_platform_driver().
Since this just reads some values from the Imem region and prints it to
the user and doesn't have a specific device associated with it, a
generic module code is written for it and not a module_platform_driver().
Best regards,
Krzysztof
