On Tue 10 Jan 07:48 PST 2017, Imran Khan wrote:
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> new file mode 100644
> index 0000000..40c180d
> --- /dev/null
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -0,0 +1,516 @@
> +/*
> + * Copyright (c) 2009-2016, The Linux Foundation. All rights reserved.
Sorry for the slow progress, please bump the year now.
[..]
> +
> +static const char *const pmic_model[] = {
> + [0] = "Unknown PMIC model",
> + [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",
> +};
I thought the conclusion was to drop the "hw_platform",
"qrd_hw_platform_subtype" and hw_platform_subtype" lists, but to keep
the cpu_of_id (although named soc_of_id).
The hw_platform ids are re-used by ODMs and might have different
meaning, but the SoC name is quite useful. So please put the soc_of_id
back in here.
[..]
> +
> +static ssize_t
> +qcom_odm_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return scnprintf(buf, PAGE_SIZE, "%s\n", odm_name);
> +}
> +DEVICE_ATTR_RO(qcom_odm);
In the case that ODMs will start using this implementation for
communicating information to user-space I believe a name will not be
enough - most likely there would be some product name and some
revision/build information.
My suggestion is that you just skip the "odm" attribute in your patch,
this gives a good opportunity for the ODMs to make a simple contribution
of what they actually want here.
[..]
> +
> +void qcom_socinfo_init(struct device *device)
> +{
> + struct soc_device_attribute *attr;
> + const struct fdt_property *prop;
> + struct soc_device *soc_dev;
> + struct device *dev;
> + size_t item_size;
> + size_t size;
> + int i;
> +
> + socinfo = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
> + &item_size);
> + if (IS_ERR(socinfo)) {
> + dev_err(device, "Coudn't find socinfo\n");
> + return;
> + }
> +
> + if ((SOCINFO_VERSION_MAJOR(le32_to_cpu(socinfo->v0_1.fmt)) != 0) ||
> + (SOCINFO_VERSION_MINOR(le32_to_cpu(socinfo->v0_1.fmt)) < 0) ||
> + (le32_to_cpu(socinfo->v0_1.fmt) > MAX_SOCINFO_FORMAT)) {
Indent wrapped lines by the start parenthesis and skip the extra
parenthesis please.
I.e.
if (... ||
... ||
...) {
> + dev_err(device, "Wrong socinfo format\n");
> + return;
> + }
> +
[..]
> +
> + odm_name = of_get_property(device->of_node, "qcom,odm", NULL);
> + if (odm_name)
> + device_create_file(dev, &dev_attr_qcom_odm);
Skip this for now.
> +
> + /* Feed the soc specific unique data into entropy pool */
> + add_device_randomness(socinfo, item_size);
> +}
> +EXPORT_SYMBOL(qcom_socinfo_init);
Please add me as To: on the next spin of your patch so I don't miss it
on the mailing list. I do expect to be able to recommend Andy to merge
that version.
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
