Re: [PATCH v4 3/5] soc: qcom: Add socinfo driver
From: Bjorn Andersson
Date: Fri Mar 01 2019 - 14:22:58 EST
On Sun 24 Feb 22:50 PST 2019, Vaishali Thakkar wrote:
[..]
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index f25b54cd6cf8..c817da4f4140 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -14,6 +14,7 @@ qcom_rpmh-y += rpmh-rsc.o
> qcom_rpmh-y += rpmh.o
> obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o
> obj-$(CONFIG_QCOM_SMEM) += smem.o
> +obj-$(CONFIG_QCOM_SOCINFO) += socinfo.o
Try to keep these sorted by moving this entry down a few steps.
> obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> obj-$(CONFIG_QCOM_SMP2P) += smp2p.o
> obj-$(CONFIG_QCOM_SMSM) += smsm.o
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index f80d040601fd..efe0b053ef82 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -276,6 +276,7 @@ struct qcom_smem {
> struct smem_partition_header *partitions[SMEM_HOST_COUNT];
> size_t cacheline[SMEM_HOST_COUNT];
> u32 item_count;
> + struct platform_device *socinfo;
>
> unsigned num_regions;
> struct smem_region regions[];
> @@ -971,11 +972,18 @@ static int qcom_smem_probe(struct platform_device *pdev)
>
> __smem = smem;
>
> + smem->socinfo = platform_device_register_data(&pdev->dev, "qcom-socinfo",
> + PLATFORM_DEVID_NONE, NULL,
> + 0);
> + if (IS_ERR(smem->socinfo))
> + dev_err(&pdev->dev, "failed to register socinfo device\n");
> +
> return 0;
> }
>
> static int qcom_smem_remove(struct platform_device *pdev)
> {
> +
Let's platform_device_unregister(smem->socinfo) here.
Note that it will handle being passed a ERR_PTR, so no need to make the
call conditional on socinfo being valid.
> hwspin_lock_free(__smem->hwlock);
> __smem = NULL;
>
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> new file mode 100644
> index 000000000000..02078049fac7
> --- /dev/null
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2009-2017, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017-2019, Linaro Ltd.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/random.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/string.h>
> +#include <linux/sys_soc.h>
> +#include <linux/types.h>
> +
> +/*
> + * SoC version type with major number in the upper 16 bits and minor
> + * number in the lower 16 bits.
> + */
> +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
> +#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
> +
> +#define SMEM_SOCINFO_BUILD_ID_LENGTH 32
> +
> +/*
> + * SMEM item ids, used to acquire handles to respective
> + * SMEM region.
There's only one socinfo SMEM item, so this sentence should be turned
singular.
> + */
> +#define SMEM_HW_SW_BUILD_ID 137
> +
> +/* Socinfo SMEM item structure */
> +struct socinfo {
> + __le32 fmt;
> + __le32 id;
> + __le32 ver;
> + char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH];
> + /* Version 2 */
> + __le32 raw_id;
> + __le32 raw_ver;
> + /* Version 3 */
> + __le32 hw_plat;
> + /* Version 4 */
> + __le32 plat_ver;
> + /* Version 5 */
> + __le32 accessory_chip;
> + /* Version 6 */
> + __le32 hw_plat_subtype;
> + /* Version 7 */
> + __le32 pmic_model;
> + __le32 pmic_die_rev;
> + /* Version 8 */
> + __le32 pmic_model_1;
> + __le32 pmic_die_rev_1;
> + __le32 pmic_model_2;
> + __le32 pmic_die_rev_2;
> + /* Version 9 */
> + __le32 foundry_id;
> + /* Version 10 */
> + __le32 serial_num;
> + /* Version 11 */
> + __le32 num_pmics;
> + __le32 pmic_array_offset;
> + /* Version 12 */
> + __le32 chip_family;
> + __le32 raw_device_family;
> + __le32 raw_device_num;
> +};
> +
> +struct qcom_socinfo {
> + struct soc_device *soc_dev;
> + struct soc_device_attribute attr;
> +};
> +
> +struct soc_of_id {
> + unsigned int id;
> + const char *name;
> +};
> +
> +static const struct soc_of_id soc_of_id[] = {
> + {87, "MSM8960"},
> + {109, "APQ8064"},
> + {122, "MSM8660A"},
> + {123, "MSM8260A"},
> + {124, "APQ8060A"},
> + {126, "MSM8974"},
> + {130, "MPQ8064"},
> + {138, "MSM8960AB"},
> + {139, "APQ8060AB"},
> + {140, "MSM8260AB"},
> + {141, "MSM8660AB"},
> + {178, "APQ8084"},
> + {184, "APQ8074"},
> + {185, "MSM8274"},
> + {186, "MSM8674"},
> + {194, "MSM8974PRO"},
> + {206, "MSM8916"},
> + {208, "APQ8074-AA"},
> + {209, "APQ8074-AB"},
> + {210, "APQ8074PRO"},
> + {211, "MSM8274-AA"},
> + {212, "MSM8274-AB"},
> + {213, "MSM8274PRO"},
> + {214, "MSM8674-AA"},
> + {215, "MSM8674-AB"},
> + {216, "MSM8674PRO"},
> + {217, "MSM8974-AA"},
> + {218, "MSM8974-AB"},
> + {246, "MSM8996"},
> + {247, "APQ8016"},
> + {248, "MSM8216"},
> + {249, "MSM8116"},
> + {250, "MSM8616"},
> + {291, "APQ8096"},
> + {305, "MSM8996SG"},
> + {310, "MSM8996AU"},
> + {311, "APQ8096AU"},
> + {312, "APQ8096SG"},
> +};
> +
> +static const char *socinfo_machine(struct device *dev, unsigned int id)
> +{
> + int idx;
> +
> + for (idx = 0; idx < ARRAY_SIZE(soc_of_id); idx++) {
> + if (soc_of_id[idx].id == id)
> + return soc_of_id[idx].name;
> + }
> +
> + if (IS_ERR(soc_of_id[idx].name))
idx will be == ARRAY_SIZE(soc_of_id) here, so you're reading outside the
array. You can write your error unconditionally here, because you didn't
find a match - or just skip the error message completely.
If you decide to keep it, that would be to facilitate adding new entries
to the list, so include the value of "id".
> + dev_err(dev, "Unknown soc id\n");
> +
> + return NULL;
> +}
> +
> +static int qcom_socinfo_probe(struct platform_device *pdev)
> +{
> + struct qcom_socinfo *qs;
> + struct socinfo *info;
> + size_t item_size;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
> + &item_size);
> + if (IS_ERR(info)) {
> + dev_err(&pdev->dev, "Couldn't find socinfo\n");
> + return -EINVAL;
> + }
> +
> + qs = devm_kzalloc(&pdev->dev, sizeof(*qs), GFP_KERNEL);
> + if (!qs)
> + return -ENOMEM;
> +
> + qs->attr.family = "Snapdragon";
> + qs->attr.machine = socinfo_machine(&pdev->dev,
> + le32_to_cpu(info->id));
> + qs->attr.revision = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u.%u",
> + SOCINFO_MAJOR(le32_to_cpu(info->ver)),
> + SOCINFO_MINOR(le32_to_cpu(info->ver)));
> + if (le32_to_cpu(info->fmt) >= 10)
> + qs->attr.serial_number = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> + "%u",
> + le32_to_cpu(info->serial_num));
> +
> + qs->soc_dev = soc_device_register(&qs->attr);
> + if (IS_ERR(qs->soc_dev))
> + return PTR_ERR(qs->soc_dev);
> +
> + /* Feed the soc specific unique data into entropy pool */
> + add_device_randomness(info, item_size);
> +
> + platform_set_drvdata(pdev, qs->soc_dev);
> +
> + return 0;
> +}
Regards,
Bjorn