| Message ID | 20190225065044.11023-4-vaishali.thakkar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: qcom: Add SoC info driver | expand |
Quoting Vaishali Thakkar (2019-02-24 22:50:42) > 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 > @@ -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"); But we don't fail the probe? Maybe a comment should be added indicating why it's ok to not fail here, and dev_err should be changed to dev_dbg() then. > + > return 0; > } > > static int qcom_smem_remove(struct platform_device *pdev) > { > + Nitpick: Drop this change? Or add the code to remove the socinfo device that is created in probe? > 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 @@ [...] > + > +struct soc_of_id { > + unsigned int id; > + const char *name; > +}; > + > +static const struct soc_of_id soc_of_id[] = { > + {87, "MSM8960"}, Nitpick: Please space out the numbers and strings: { 87, "MSM8960" }, > +}; > + > +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; Why is it called soc_of_id? Is that supposed to be "SoC of ID" or is it DT based and is "SoC OF ID"? If it's the latter, I'd prefer some non-DT type of name to provide more clarity that this has nothing to do with DeviceTree. > + } > + > + if (IS_ERR(soc_of_id[idx].name)) > + 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; Why not return PTR_ERR(info)? > + } > + > + 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) Maybe this would make more sense if it was written with item_size and offset of info->fmt? Something like if (offsetof(struct qcom_socinfo, serial_num) <= item_size) > + 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); Weird, this is setting it to qs->soc_dev.... > + > + return 0; > +} > + > +static int qcom_socinfo_remove(struct platform_device *pdev) > +{ > + struct qcom_socinfo *qs = platform_get_drvdata(pdev); And then extracting this as qs only... > + > + soc_device_unregister(qs->soc_dev); And so it looks wrong? Probably already have qs->soc_dev from the platform_get_drvdata() call? > + > + return 0; > +} > + > +static struct platform_driver qcom_socinfo_driver = { > + .probe = qcom_socinfo_probe, > + .remove = qcom_socinfo_remove, > + .driver = { > + .name = "qcom-socinfo", > + }, > +}; > + > +module_platform_driver(qcom_socinfo_driver); > + > +MODULE_DESCRIPTION("Qualcomm socinfo driver"); Maybe write socinfo as SoCinfo here? > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:qcom-socinfo");
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
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index fcbf8a2e4080..1e31eda07934 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -144,6 +144,14 @@ config QCOM_SMSM Say yes here to support the Qualcomm Shared Memory State Machine. The state machine is represented by bits in shared memory. +config QCOM_SOCINFO + tristate "Qualcomm socinfo driver" + depends on QCOM_SMEM + select SOC_BUS + help + Say yes here to support the Qualcomm socinfo driver, providing + information about the SoC to user space. + config QCOM_WCNSS_CTRL tristate "Qualcomm WCNSS control driver" depends on ARCH_QCOM || COMPILE_TEST 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 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) { + 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. + */ +#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)) + 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; +} + +static int qcom_socinfo_remove(struct platform_device *pdev) +{ + struct qcom_socinfo *qs = platform_get_drvdata(pdev); + + soc_device_unregister(qs->soc_dev); + + return 0; +} + +static struct platform_driver qcom_socinfo_driver = { + .probe = qcom_socinfo_probe, + .remove = qcom_socinfo_remove, + .driver = { + .name = "qcom-socinfo", + }, +}; + +module_platform_driver(qcom_socinfo_driver); + +MODULE_DESCRIPTION("Qualcomm socinfo driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:qcom-socinfo");