On Sun 24 Feb 22:50 PST 2019, Vaishali Thakkar wrote:
> +#ifdef CONFIG_DEBUG_FS
> +/* pmic model info */
Please drop this comment and make "pmic_model" plural.
> +static const char *const pmic_model[] = {
> + [0] = "Unknown PMIC model",
> + [9] = "PM8994",
> + [11] = "PM8916",
> + [13] = "PM8058",
> + [14] = "PM8028",
> + [15] = "PM8901",
> + [16] = "PM8027",
> + [17] = "ISL9519",
> + [18] = "PM8921",
> + [19] = "PM8018",
> + [20] = "PM8015",
> + [21] = "PM8014",
> + [22] = "PM8821",
> + [23] = "PM8038",
> + [24] = "PM8922",
> + [25] = "PM8917",
> +};
> +#endif /* CONFIG_DEBUG_FS */
[..]
> +static int qcom_show_pmic_model(struct seq_file *seq, void *p)
> +{
> + struct socinfo *socinfo = seq->private;
> + int model = SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_model));
> +
> + if (model < 0)
> + return -EINVAL;
You need to deal with the fact that model might be >=
ARRAY_SIZE(pmic_model) and that pmic_mode[model] might be NULL, in the
event that you missed entries in the list or this driver is used on
newer platforms.
> +
> + seq_printf(seq, "%s\n", pmic_model[model]);
> +
> + return 0;
> +}
> +
> +static int qcom_show_pmic_die_revision(struct seq_file *seq, void *p)
> +{
> + struct socinfo *socinfo = seq->private;
> +
> + seq_printf(seq, "%u.%u\n",
> + SOCINFO_MAJOR(le32_to_cpu(socinfo->pmic_die_rev)),
> + SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_die_rev)));
> +
> + return 0;
> +}
> +
> +UINT_SHOW(raw_version, raw_ver);
> +UINT_SHOW(hardware_platform, hw_plat);
> +UINT_SHOW(platform_version, plat_ver);
> +UINT_SHOW(foundry_id, foundry_id);
> +HEX_SHOW(chip_family, chip_family);
> +HEX_SHOW(raw_device_family, raw_device_family);
> +HEX_SHOW(raw_device_number, raw_device_num);
> +QCOM_OPEN(build_id, qcom_show_build_id);
> +QCOM_OPEN(accessory_chip, qcom_show_accessory_chip);
> +QCOM_OPEN(pmic_model, qcom_show_pmic_model);
> +QCOM_OPEN(platform_subtype, qcom_show_platform_subtype);
> +QCOM_OPEN(pmic_die_revision, qcom_show_pmic_die_revision);
> +
> +static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo)
> +{
> + qcom_socinfo->dbg_root = debugfs_create_dir("qcom_socinfo", NULL);
> +
> + DEBUGFS_UINT_ADD(raw_version);
> + DEBUGFS_UINT_ADD(hardware_platform);
Note that the content of struct socinfo has grown over time, so based on
the comments in the struct the size of the struct would not cover
hw_plat if version < 3.
So you should make the addition of these conditional on socinfo->ver.
As each version adds more entries I suggest that you do this with a:
switch (qcom_socinfo->socinfo->ver) {
case 12:
add v12 entries;
case 11:
add v11 entries;
case 10:
add v10 entries;
...
};
> + DEBUGFS_UINT_ADD(platform_version);
> + DEBUGFS_UINT_ADD(foundry_id);
> + DEBUGFS_HEX_ADD(chip_family);
> + DEBUGFS_HEX_ADD(raw_device_family);
> + DEBUGFS_HEX_ADD(raw_device_number);
> + DEBUGFS_ADD(build_id);
> + DEBUGFS_ADD(accessory_chip);
> + DEBUGFS_ADD(pmic_model);
> + DEBUGFS_ADD(platform_subtype);
> + DEBUGFS_ADD(pmic_die_revision);
> +}
> +
Regards,
Bjorn
