soc: qcom: Add SoC info driver
Subsystems (2)
PATCH v9 1/1 soc: qcom: Add SoC info driver
The SoC info driver provides information such as Chip ID, Chip family, serial number and other such details about Qualcomm SoCs to user space, so that if needed some user space utility(like antutu) can query such information using sysfs interface.
---
v8 --> v9:
- Removed qcom_odm attribute.
- Re-introduce soc_of_id array to map soc-id
into human readable string.
v7 --> v8:
- Fixed compilation warning detected in v7
v6 --> v7:
- Some minor style changes
- Rather than showing string for h/w types, just show the number obtained from SMEM. The reader of this number is supposed to parse this into a human readable string as this information may be different for different ODMs
- Create an attribute 'qcom_odm' depending on the presence of corresponding property in SMEM DT entry. This information can be used for ODM specific interpretation of SMEM socinfo content
- Read machine value from DT, rather than using soc-id as index in an ever increasing array of machine names
v5 --> v6:
- use dev_ext_attribute to represent SMEM image items
- Avoid redundant function calls
- Avoid use of unnecessary global variables
- Remove redundant enums
- Avoid redundant checking of socinfo being NULL or not
- Avoid redundant checking of socinfo format version
- Rename show/store function of attributes as _show/_store rather than _get/_set
- Use an array to represent socinfo attributes and create attribute files in a loop depending on the minimum socinfo format version for which these attributes are supported
- if obtained socinfo format version is greater than the maximum known version return an error rather than falling back to maximum known version
v4 --> v5:
- Removed redundant function socinfo_print
v3 --> v4:
- Corrected makefile so that smem and socinfo are treated as one module
- Moved the code snippet to get socinfo smem item into socinfo.c
- Removed redundant use of socinfo major version as it is always zero
- Removed unused enums
- Removed redundant indirections
- Use image_version object to store information about each entry in the smem image table
- Replaced usage of snprintf with sprintf and scnprintf
- Get the address of image version table at the beginning and setup image version attributes only if image version table is available
- Do the same for platform_subtype
- Make different type of image version objects read only or readable/ writable depending on their types. For example apps image attributes can be modified via sysfs but the same can't be done for modem image
- Show PMIC model in a human readable form rather than a numeric number
- Avoid using table in single sysfs entry
- Removed checkpatch warnings about S_IRUGO
v2 --> v3:
- Add support to toss soc information data into entropy pool
- Since socinfo is rolled into smem driver, compile the relevant portion of socinfo driver with smem driver
v1 --> v2:
- Removed inclusion of system_misc.h
- merged socinfo.h into socinfo.c
- made platform type and subtype arrays static
- replaced uint32_t with u32
- made functions static to avoid exposing vendor specific interface
- Replaced usage of IS_ERR_OR_NULL with IS_ERR.
- Remove raw-id attribute usage as human readable soc-id will suffice here
- Avoid using a separate platform driver for socinfo by rolling it into smem driver itself. The sysfs setup is being done in a separate file (socinfo.c)
- Replaced macro BUILD_ID_LENGTH with SMEM_SOCINFO_BUILD_ID_LENGTH.
- For failure cases where socinfo can not be read use a single dummy socinfo with error values.
- Removed usage of early_machine_is_xxx.
drivers/soc/qcom/Kconfig | 1 + drivers/soc/qcom/Makefile | 3 +- drivers/soc/qcom/smem.c | 5 + drivers/soc/qcom/socinfo.c | 557 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 565 insertions(+), 1 deletion(-) create mode 100644 drivers/soc/qcom/socinfo.c
drivers/soc/qcom/Kconfig +1 -0
@@ -24,6 +24,7 @@ config QCOM_SMEM tristate "Qualcomm Shared Memory Manager (SMEM)" depends on ARCH_QCOM depends on HWSPINLOCK + select SOC_BUS help Say y here to enable support for the Qualcomm Shared Memory Manager. The driver provides an interface to items in a heap shared among all
drivers/soc/qcom/Makefile +2 -1
@@ -2,7 +2,8 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o obj-$(CONFIG_QCOM_PM) += spm.o obj-$(CONFIG_QCOM_SMD) += smd.o obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o -obj-$(CONFIG_QCOM_SMEM) += smem.o +obj-$(CONFIG_QCOM_SMEM) += qcom_smem.o +qcom_smem-y := smem.o socinfo.o obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o obj-$(CONFIG_QCOM_SMP2P) += smp2p.o obj-$(CONFIG_QCOM_SMSM) += smsm.o
drivers/soc/qcom/smem.c +5 -0
@@ -85,6 +85,9 @@/* Max number of processors/hosts in a system */ #define SMEM_HOST_COUNT 9 + +extern void qcom_socinfo_init (struct device *device); +/** * struct smem_proc_comm - proc_comm communication struct (legacy) * @command: current command to be executed
@@ -751,6 +754,8 @@ static int qcom_smem_probe(struct platform_device *pdev) __smem = smem; +qcom_socinfo_init (&pdev->dev); +return 0 ; }
drivers/soc/qcom/socinfo.c new file +557 -0
557 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 6 lines · Imran Khan
The SoC info driver provides information such as Chip ID, Chip family, serial number and other such details about Qualcomm SoCs to user space, so that if needed some user space utility(like antutu) can query such information using sysfs interface.
Regards,
Bjorn
drivers/soc/qcom/smem.c line 90
Quoted 11 lines · Imran Khan
drivers/soc/qcom/smem.c +3
-0@@ -85,6 +85,9 @@/* Max number of processors/hosts in a system */ #define SMEM_HOST_COUNT 9 + +extern void qcom_socinfo_init (struct device *device); +
Sparse still complains... o well.
Quoted 33 lines
drivers/soc/qcom/socinfo.c new file +27
-0@@ -0,0 +1,557 @@ +/* +* Copyright (c) 2009-2017, The Linux Foundation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 and +* only version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +*/ + +#include <linux/export.h> +#include <linux/err.h> +#include <linux/of.h> +#include <linux/of_fdt.h> +#include <linux/libfdt.h> +#include <linux/platform_device.h> +#include <linux/sys_soc.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/types.h> +#include <linux/random.h> +#include <linux/soc/qcom/smem.h> +
drivers/soc/qcom/socinfo.c line 37
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+/* +* SOC version type with major number in the upper 16 bits and minor +* number in the lower 16 bits. For example: +* 1.0 -> 0x00010000 +* 2.3 -> 0x00020003 +*/ +#define SOC_VER_MAJ (ver) (((ver) & 0xffff0000) >> 16) +#define SOC_VER_MIN (ver) ((ver) & 0x0000ffff) +#define SOCINFO_VERSION_MAJOR SOC_VER_MAJ +#define SOCINFO_VERSION_MINOR SOC_VER_MIN
Do we really need two defines for the same thing?
Quoted 44 lines
drivers/soc/qcom/socinfo.c new file +44
-0+ +#define SMEM_SOCINFO_BUILD_ID_LENGTH 32 +#define SMEM_IMAGE_VERSION_BLOCKS_COUNT 32 +#define SMEM_IMAGE_VERSION_SIZE 4096 +#define SMEM_IMAGE_VERSION_NAME_SIZE 75 +#define SMEM_IMAGE_VERSION_VARIANT_SIZE 20 +#define SMEM_IMAGE_VERSION_OEM_SIZE 32 + +/* +* SMEM item ids, used to acquire handles to respective +* SMEM region. +*/ +#define SMEM_IMAGE_VERSION_TABLE 469 +#define SMEM_HW_SW_BUILD_ID 137 + +/* +* SMEM Image table indices +*/ +#define SMEM_IMAGE_TABLE_BOOT_INDEX 0 +#define SMEM_IMAGE_TABLE_TZ_INDEX 1 +#define SMEM_IMAGE_TABLE_RPM_INDEX 3 +#define SMEM_IMAGE_TABLE_APPS_INDEX 10 +#define SMEM_IMAGE_TABLE_MPSS_INDEX 11 +#define SMEM_IMAGE_TABLE_ADSP_INDEX 12 +#define SMEM_IMAGE_TABLE_CNSS_INDEX 13 +#define SMEM_IMAGE_TABLE_VIDEO_INDEX 14 + +struct qcom_socinfo_attr { +struct device_attribute attr; +int min_ver; +}; + +#define QCOM_SOCINFO_ATTR (_name, _show, _min_ver) \ + { __ATTR(_name, 0444, _show, NULL), _min_ver } + + +struct smem_image_attribute { +struct device_attribute version; +struct device_attribute variant; +struct device_attribute crm; +int index; +}; + +#define QCOM_SMEM_IMG_ITEM (_name, _mode, _index) \
drivers/soc/qcom/socinfo.c line 91
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+ static struct smem_image_attribute _name##_image_attrs = { \ + __ATTR(_name##_image_version, _mode, \ + qcom_show_image_version, qcom_store_image_version), \ + __ATTR(_name##_image_variant, _mode, \ + qcom_show_image_variant, qcom_store_image_variant), \ + __ATTR(_name##_image_crm, _mode, \ + qcom_show_image_crm, qcom_store_image_crm), \ + _index \ + }; \ + static struct attribute_group _name##_image_attr_group = { \
can be const.
drivers/soc/qcom/socinfo.c line 101
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+ .attrs = (struct attribute*[]) { \ + &_name##_image_attrs.version.attr, \ + &_name##_image_attrs.variant.attr, \ + &_name##_image_attrs.crm.attr, \ + NULL \ + } \ + } + +static const char *const pmic_model[] = { + [0 ] ="Unknown PMIC model" ,
Missing pm8916, but it doesn't seem to matter for me. My db410c says 0 here.
Quoted 23 lines
drivers/soc/qcom/socinfo.c new file +23
-0+ [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", +}; + +struct smem_image_version { +char name[SMEM_IMAGE_VERSION_NAME_SIZE ]; +char variant[SMEM_IMAGE_VERSION_VARIANT_SIZE ]; +char pad; +char oem[SMEM_IMAGE_VERSION_OEM_SIZE ]; +}; + +struct qcom_soc_info {
drivers/soc/qcom/socinfo.c line 134
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+int id; +const char *name; +}; + +/* Used to parse shared memory. Must match the modem. */ +struct socinfo_v0_1 { +__le32 fmt; +__le32 id; +__le32 ver; +char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH ];
Is this a C style string? Or just a bunch of bytes?
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+}; + +struct socinfo_v0_2 { +struct socinfo_v0_1 v0_1; +__le32 raw_ver; +}; + +struct socinfo_v0_3 { +struct socinfo_v0_2 v0_2; +__le32 hw_plat;
drivers/soc/qcom/socinfo.c line 154
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+}; + +struct socinfo_v0_4 { +struct socinfo_v0_3 v0_3; +__le32 plat_ver; +}; + +struct socinfo_v0_5 { +struct socinfo_v0_4 v0_4; +__le32 accsry_chip;
accesory_chip?
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+}; + +struct socinfo_v0_6 { +struct socinfo_v0_5 v0_5; +__le32 hw_plat_subtype; +}; + +struct socinfo_v0_7 { +struct socinfo_v0_6 v0_6; +__le32 pmic_model;
drivers/soc/qcom/socinfo.c line 174
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+__le32 pmic_die_rev; +}; + +struct socinfo_v0_8 { +struct socinfo_v0_7 v0_7; +__le32 pmic_model_1; +__le32 pmic_die_rev_1; +__le32 pmic_model_2; +__le32 pmic_die_rev_2; +};
So we have multiple ways to print out pmic information. Hopefully we can do the right one at the right time based on version we support?
drivers/soc/qcom/socinfo.c line 178
Quoted 4 lines
drivers/soc/qcom/socinfo.c new file +4
-0+ +struct socinfo_v0_9 { +struct socinfo_v0_8 v0_8; +__le32 fndry_id;
Can we write foundry_id?
drivers/soc/qcom/socinfo.c line 183
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+}; + +struct socinfo_v0_10 { +struct socinfo_v0_9 v0_9; +__le32 srl_num;
And serial_number or serial_num? Not sure why we lost so many vowels.
drivers/soc/qcom/socinfo.c line 195
Quoted 12 lines
drivers/soc/qcom/socinfo.c new file +12
-0+}; + +struct socinfo_v0_11 { +struct socinfo_v0_10 v0_10; +__le32 num_pmics; +__le32 pmic_array_offset; +}; + +struct socinfo_v0_12 { +struct socinfo_v0_11 v0_11; +__le32 chip_family; +__le32 raw_dev_fam;
raw_device_family?
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+__le32 raw_dev_num; +}; + +static union { + struct socinfo_v0_1 v0_1; + struct socinfo_v0_2 v0_2;
drivers/soc/qcom/socinfo.c line 211
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+struct socinfo_v0_3 v0_3; +struct socinfo_v0_4 v0_4; +struct socinfo_v0_5 v0_5; +struct socinfo_v0_6 v0_6; +struct socinfo_v0_7 v0_7; +struct socinfo_v0_8 v0_8; +struct socinfo_v0_9 v0_9; +struct socinfo_v0_10 v0_10; +struct socinfo_v0_11 v0_11; +struct socinfo_v0_12 v0_12;
I find this design odd. Do we really care that all these versions append to each other? Why not just have one structure with comments delimiting where a new version fields start and then dropping the whole v0_1, v0_2 thing?
drivers/soc/qcom/socinfo.c line 214
Quoted 3 lines
drivers/soc/qcom/socinfo.c new file +3
-0+} *socinfo; + +static struct qcom_soc_info soc_of_id[] = {
const?
drivers/soc/qcom/socinfo.c line 215
Quoted 1 line
drivers/soc/qcom/socinfo.c new file +1
-0+ {87, "MSM8960"},
Also please throw some spaces around brackets here.
Quoted 40 lines
drivers/soc/qcom/socinfo.c new file +40
-0+ {109, "APQ8064"}, + {130, "MPQ8064"}, + {122, "MSM8660A"}, + {123, "MSM8260A"}, + {124, "APQ8060A"}, + {126, "MSM8974"}, + {184, "APQ8074"}, + {185, "MSM8274"}, + {186, "MSM8674"}, + {208, "APQ8074-AA"}, + {211, "MSM8274-AA"}, + {214, "MSM8674-AA"}, + {217, "MSM8974-AA"}, + {209, "APQ8074-AB"}, + {212, "MSM8274-AB"}, + {215, "MSM8674-AB"}, + {218, "MSM8974-AB"}, + {194, "MSM8974PRO"}, + {210, "APQ8074PRO"}, + {213, "MSM8274PRO"}, + {216, "MSM8674PRO"}, + {138, "MSM8960AB"}, + {139, "APQ8060AB"}, + {140, "MSM8260AB"}, + {141, "MSM8660AB"}, + {178, "APQ8084"}, + {206, "MSM8916"}, + {247, "APQ8016"}, + {248, "MSM8216"}, + {249, "MSM8116"}, + {250, "MSM8616"}, + {246, "MSM8996"}, + {310, "MSM8996AU"}, + {311, "APQ8096AU"}, + {291, "APQ8096"}, + {305, "MSM8996SG"}, + {312, "APQ8096SG"}, +}; + +static struct smem_image_version *smem_image_version;
drivers/soc/qcom/socinfo.c line 265
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+ +/* max socinfo format version supported */ +#define MAX_SOCINFO_FORMAT 12 + +/* socinfo: sysfs functions */ + +static ssize_t +qcom_show_vendor (struct device *dev,struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"%s" ,"Qualcomm\n" );
We can't just sprintf(buf, "Qualcomm\n") ?
drivers/soc/qcom/socinfo.c line 278
Quoted 13 lines
drivers/soc/qcom/socinfo.c new file +13
-0+} + +static ssize_t +qcom_show_raw_version (struct device *dev,struct device_attribute *attr, +char *buf) +{ +return sprintf (buf,"%u\n" ,le32_to_cpu (socinfo->v0_2.raw_ver)); +} + +static ssize_t +qcom_show_build_id (struct device *dev,struct device_attribute *attr,char *buf) +{ +return scnprintf (buf,PAGE_SIZE ,"%s\n" , socinfo->v0_1.build_id);
Why does this one get PAGE_SIZE but others don't?
Quoted 20 lines
drivers/soc/qcom/socinfo.c new file +20
-0+} + +static ssize_t +qcom_show_hw_platform (struct device *dev,struct device_attribute *attr, +char *buf) +{ +return sprintf (buf,"%u\n" ,le32_to_cpu (socinfo->v0_3.hw_plat)); +} + +static ssize_t +qcom_show_platform_version (struct device *dev,struct device_attribute *attr, +char *buf) +{ +return sprintf (buf,"%u\n" ,le32_to_cpu (socinfo->v0_4.plat_ver)); +} + +static ssize_t +qcom_show_accessory_chip (struct device *dev,struct device_attribute *attr, +char *buf) +{
drivers/soc/qcom/socinfo.c line 308
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+return sprintf (buf,"%u\n" ,le32_to_cpu (socinfo->v0_5.accsry_chip)); +} + +static ssize_t +qcom_show_platform_subtype (struct device *dev,struct device_attribute *attr, +char *buf) +{ +u32 subtype =le32_to_cpu (socinfo->v0_6.hw_plat_subtype); + +if (subtype <0 )
How can an unsigned type be less than zero?
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+return -EINVAL ; + +return sprintf (buf,"%u\n" , subtype); +} +
[...]
Quoted 16 lines
drivers/soc/qcom/socinfo.c new file +16
-0+ +static const char *socinfo_get_id_string (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; + } + +return NULL ; +} + +void qcom_socinfo_init (struct device *device) +{ +struct soc_device_attribute *attr;
drivers/soc/qcom/socinfo.c
Quoted 10 lines
drivers/soc/qcom/socinfo.c new file +10
-0+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" );
Couldn't
drivers/soc/qcom/socinfo.c
Quoted 12 lines
drivers/soc/qcom/socinfo.c new file +12
-0+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 ) { +dev_err (device,"Wrong socinfo format\n" ); +return ; + } + +if (!le32_to_cpu (socinfo->v0_1.id)) +dev_err (device,"socinfo: Unknown SoC ID!\n" );
Drop socinfo: please.
---8<----
drivers/soc/qcom/socinfo.c +7 -6
@@ -88,7 +88,7 @@ struct smem_image_attribute { qcom_show_image_crm, qcom_store_image_crm), \ _index \ }; \ - static struct attribute_group _name##_image_attr_group = { \ + static const struct attribute_group _name##_image_attr_group = { \ .attrs = (struct attribute *[]) { \ &_name##_image_attrs.version.attr, \ &_name##_image_attrs.variant.attr, \
@@ -99,6 +99,7 @@ struct smem_image_attribute {static const char *const pmic_model[] = { [0 ] ="Unknown PMIC model" , + [11 ] ="PM8916" , [13 ] ="PM8058" , [14 ] ="PM8028" , [15 ] ="PM8901" ,
@@ -453,8 +454,8 @@ QCOM_SMEM_IMG_ITEM(adsp, 0444, SMEM_IMAGE_TABLE_ADSP_INDEX);QCOM_SMEM_IMG_ITEM (cnss,0444 ,SMEM_IMAGE_TABLE_CNSS_INDEX );QCOM_SMEM_IMG_ITEM (video,0444 ,SMEM_IMAGE_TABLE_VIDEO_INDEX ); -static const struct attribute_group - *smem_img_tbl[SMEM_IMAGE_VERSION_BLOCKS_COUNT ] = { +static const +struct attribute_group *smem_img_tbl[SMEM_IMAGE_VERSION_BLOCKS_COUNT ] = { [SMEM_IMAGE_TABLE_BOOT_INDEX ] = &boot_image_attr_group, [SMEM_IMAGE_TABLE_TZ_INDEX ] = &tz_image_attr_group, [SMEM_IMAGE_TABLE_RPM_INDEX ] = &rpm_image_attr_group,
@@ -489,7 +490,7 @@ void qcom_socinfo_init(struct device *device) 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" ); +dev_err (device,"Couldn't find socinfo\n" );return ; }
@@ -501,7 +502,7 @@ void qcom_socinfo_init(struct device *device) }if (!le32_to_cpu (socinfo->v0_1.id)) -dev_err (device,"socinfo: Unknown SoC ID!\n" ); +dev_err (device,"Unknown SoC ID!\n" ); smem_image_version = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_IMAGE_VERSION_TABLE,
@@ -534,7 +535,7 @@ void qcom_socinfo_init(struct device *device)/* * Expose SMEM_IMAGE_TABLE to sysfs only when we have IMAGE_TABLE * available in SMEM. As IMAGE_TABLE and SOCINFO are two separate -* items within SMEM, we expose the remaining soc information(i.e +* items within SMEM, we expose the remaining soc information (i.e * only the SOCINFO item available in SMEM) to sysfs even in the * absence of an IMAGE_TABLE. */
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Stephen,
Thanks for your review comments.
Quoted 9 lines · Stephen Boyd
Quoted 8 lines · Imran Khan
drivers/soc/qcom/smem.c +0
-0@@ -85,6 +85,9 @@/* Max number of processors/hosts in a system */ #define SMEM_HOST_COUNT 9
Quoted 6 lines
drivers/soc/qcom/smem.c line 90
Quoted 3 lines
drivers/soc/qcom/smem.c +3
-0+ +extern void qcom_socinfo_init (struct device *device); +Sparse still complains... o well.
It was agreed earlier that we can keep this extern declaration. Please let me know if it needs to be removed.
Quoted 37 lines
Quoted 37 lines
drivers/soc/qcom/socinfo.c new file +31
-0@@ -0,0 +1,557 @@ +/* +* Copyright (c) 2009-2017, The Linux Foundation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 and +* only version 2 as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +*/ + +#include <linux/export.h> +#include <linux/err.h> +#include <linux/of.h> +#include <linux/of_fdt.h> +#include <linux/libfdt.h> +#include <linux/platform_device.h> +#include <linux/sys_soc.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/types.h> +#include <linux/random.h> +#include <linux/soc/qcom/smem.h> + +/* +* SOC version type with major number in the upper 16 bits and minor +* number in the lower 16 bits. For example: +* 1.0 -> 0x00010000
Quoted 9 lines
drivers/soc/qcom/socinfo.c line 37
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+* 2.3 -> 0x00020003 +*/ +#define SOC_VER_MAJ (ver) (((ver) & 0xffff0000) >> 16) +#define SOC_VER_MIN (ver) ((ver) & 0x0000ffff) +#define SOCINFO_VERSION_MAJOR SOC_VER_MAJ +#define SOCINFO_VERSION_MINOR SOC_VER_MINDo we really need two defines for the same thing?
Though both of these defines are doing the same thing, but one is used for version of SoC/PMIC (e.g. 3.1, 3.3 etc.) while other is used for version of socinfo. We have different versions of socinfo and the information conveyed by one version of socinfo differs from that conveyed by other version.
The main purpose of keeping 2 macros was to avoid confusion in readability of code, so that a user of the code can understand where we are dealing version of SoC/PMIC and where we are dealing with version of socinfo. Where we need to deal with version of socinfo we use SOCINFO_VERSION_MAJOR/ MINOR otherwise we use SOC_VER_MAJ/MIN.
Please let me know if this looks okay to you.
Quoted 8 lines
Quoted 8 lines
drivers/soc/qcom/socinfo.c new file +8
-0+ +#define SMEM_SOCINFO_BUILD_ID_LENGTH 32 +#define SMEM_IMAGE_VERSION_BLOCKS_COUNT 32 +#define SMEM_IMAGE_VERSION_SIZE 4096 +#define SMEM_IMAGE_VERSION_NAME_SIZE 75 +#define SMEM_IMAGE_VERSION_VARIANT_SIZE 20 +#define SMEM_IMAGE_VERSION_OEM_SIZE 32 +
<snip>
Quoted 5 lines
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+#define QCOM_SMEM_IMG_ITEM (_name, _mode, _index) \ + static struct smem_image_attribute _name##_image_attrs = { \ + __ATTR(_name##_image_version, _mode, \ + qcom_show_image_version, qcom_store_image_version), \ + __ATTR(_name##_image_variant, _mode, \
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+ qcom_show_image_variant, qcom_store_image_variant), \ + __ATTR(_name##_image_crm, _mode, \ +qcom_show_image_crm ,qcom_store_image_crm ), \ + _index \ + }; \ + static struct attribute_group _name##_image_attr_group = { \can be const.
Done.
Quoted 5 lines
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+ .attrs = (struct attribute*[]) { \ + &_name##_image_attrs.version.attr, \ + &_name##_image_attrs.variant.attr, \ + &_name##_image_attrs.crm.attr, \ +NULL \
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+ } \ + } + +static const char *const pmic_model[] = { + [0 ] ="Unknown PMIC model" ,Missing pm8916, but it doesn't seem to matter for me. My db410c says 0 here.
Okay. PM8916 and PM8994 have been added now. Also I found that I was not interpreting pmic_model field and pmic_die_revision fields properly, I have corrected it in the next patch set. Now we should see correct values for db410c too.
Quoted 15 lines
Quoted 15 lines
drivers/soc/qcom/socinfo.c new file +15
-0+ [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", +}; +
<snip>
Quoted 11 lines
drivers/soc/qcom/socinfo.c
Quoted 8 lines
drivers/soc/qcom/socinfo.c new file +8
-0+}; + +/* Used to parse shared memory. Must match the modem. */ +struct socinfo_v0_1 { +__le32 fmt; +__le32 id; +__le32 ver; +char build_id[SMEM_SOCINFO_BUILD_ID_LENGTH ];Is this a C style string? Or just a bunch of bytes?
build_id is a C style string.
Quoted 14 lines
Quoted 14 lines
drivers/soc/qcom/socinfo.c new file +14
-0+}; + +struct socinfo_v0_2 { +struct socinfo_v0_1 v0_1; +__le32 raw_ver; +}; + +struct socinfo_v0_3 { +struct socinfo_v0_2 v0_2; +__le32 hw_plat; +}; + +struct socinfo_v0_4 { +struct socinfo_v0_3 v0_3;
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+__le32 plat_ver; +}; + +struct socinfo_v0_5 { +struct socinfo_v0_4 v0_4; +__le32 accsry_chip;
accesory_chip?
Done.
Quoted 16 lines
Quoted 16 lines
drivers/soc/qcom/socinfo.c new file +16
-0+}; + +struct socinfo_v0_6 { +struct socinfo_v0_5 v0_5; +__le32 hw_plat_subtype; +}; + +struct socinfo_v0_7 { +struct socinfo_v0_6 v0_6; +__le32 pmic_model; +__le32 pmic_die_rev; +}; + +struct socinfo_v0_8 { +struct socinfo_v0_7 v0_7; +__le32 pmic_model_1;
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 4 lines
drivers/soc/qcom/socinfo.c new file +4
-0+__le32 pmic_die_rev_1; +__le32 pmic_model_2; +__le32 pmic_die_rev_2; +};So we have multiple ways to print out pmic information. Hopefully we can do the right one at the right time based on version we support?
True. But right now we are just providing the information about one PMIC, so we are using the format as specified in socinfo version 7. This allows us to show PMIC information for all socinfo versions from 7 on wards(PMIC information was not available before version 7)
Later on, I intend to enhance this part to show information of multiple PMICs, according to socifo versions 8 and 11.
Please let me know if it sounds okay to you.
Quoted 7 lines
drivers/soc/qcom/socinfo.c
Quoted 4 lines
drivers/soc/qcom/socinfo.c new file +4
-0+ +struct socinfo_v0_9 { +struct socinfo_v0_8 v0_8; +__le32 fndry_id;Can we write
foundry_id?
Done.
Quoted 8 lines
drivers/soc/qcom/socinfo.c
Quoted 5 lines
drivers/soc/qcom/socinfo.c new file +5
-0+}; + +struct socinfo_v0_10 { +struct socinfo_v0_9 v0_9; +__le32 srl_num;And
serial_numberorserial_num? Not sure why we lost so many vowels.
Done. Had trimmed some of the member names to avoid extra lines because of indentation.
Quoted 6 lines
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+}; + +struct socinfo_v0_11 { +struct socinfo_v0_10 v0_10; +__le32 num_pmics; +__le32 pmic_array_offset;
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+}; + +struct socinfo_v0_12 { +struct socinfo_v0_11 v0_11; +__le32 chip_family; +__le32 raw_dev_fam;
raw_device_family?
Done.
Quoted 13 lines
Quoted 13 lines
drivers/soc/qcom/socinfo.c new file +13
-0+__le32 raw_dev_num; +}; + +static union { + struct socinfo_v0_1 v0_1; + struct socinfo_v0_2 v0_2; + struct socinfo_v0_3 v0_3; + struct socinfo_v0_4 v0_4; + struct socinfo_v0_5 v0_5; + struct socinfo_v0_6 v0_6; + struct socinfo_v0_7 v0_7; + struct socinfo_v0_8 v0_8; + struct socinfo_v0_9 v0_9;
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 3 lines
drivers/soc/qcom/socinfo.c new file +3
-0+struct socinfo_v0_10 v0_10; +struct socinfo_v0_11 v0_11; +struct socinfo_v0_12 v0_12;I find this design odd. Do we really care that all these versions append to each other? Why not just have one structure with comments delimiting where a new version fields start and then dropping the whole
v0_1,v0_2thing?
Done. The main reason behind appending subsequent versions was to keep the fields added by newer versions in order, so that they can be accessed properly. However the same can be done using a single structure as you have suggested. I have dropped the version specific objects.
Quoted 6 lines
drivers/soc/qcom/socinfo.c
Quoted 3 lines
drivers/soc/qcom/socinfo.c new file +3
-0+} *socinfo; + +static struct qcom_soc_info soc_of_id[] = {const?
Done.
Quoted 13 lines
drivers/soc/qcom/socinfo.c
Quoted 1 line
drivers/soc/qcom/socinfo.c new file +1
-0+ {87, "MSM8960"},Also please throw some spaces around brackets here.
Quoted 9 lines
drivers/soc/qcom/socinfo.c new file +9
-0+ {109, "APQ8064"}, + {130, "MPQ8064"}, + {122, "MSM8660A"}, + {123, "MSM8260A"}, + {124, "APQ8060A"}, + {126, "MSM8974"}, + {184, "APQ8074"}, + {185, "MSM8274"}, + {186, "MSM8674"},
<snip>
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+/* socinfo: sysfs functions */ + +static ssize_t +qcom_show_vendor (struct device *dev,struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"%s" ,"Qualcomm\n" );We can't just
sprintf(buf, "Qualcomm\n")?
True. But we don't have any information regarding Vendor in the socinfo. So we were using a hard coded value here. But as this information is not coming from socinfo neither it is required in the generic soc_device_attribute, we may altogether drop this attribute as well.
Please let me know your view in this regard.
Quoted 7 lines
Quoted 7 lines
drivers/soc/qcom/socinfo.c new file +7
-0+} + +static ssize_t +qcom_show_raw_version (struct device *dev,struct device_attribute *attr, +char *buf) +{ +return sprintf (buf,"%u\n" ,le32_to_cpu (socinfo->v0_2.raw_ver));
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+} + +static ssize_t +qcom_show_build_id (struct device *dev,struct device_attribute *attr,char *buf) +{ +return scnprintf (buf,PAGE_SIZE ,"%s\n" , socinfo->v0_1.build_id);Why does this one get
PAGE_SIZEbut others don't?
The places, where sprintf is being used, are not getting PAGE_SIZE. Now to keep the implementation consistent I have used scnprintf throughout. As anyways sysfs gives a buffer of PAGE_SIZE, so I have used this size throughout with scnprintf in show functions.
Quoted 2 lines
Quoted 2 lines
drivers/soc/qcom/socinfo.c new file +2
-0+} +
<snip>
Quoted 10 lines
drivers/soc/qcom/socinfo.c
Quoted 7 lines
drivers/soc/qcom/socinfo.c new file +7
-0+static ssize_t +qcom_show_platform_subtype (struct device *dev,struct device_attribute *attr, +char *buf) +{ +u32 subtype =le32_to_cpu (socinfo->v0_6.hw_plat_subtype); + +if (subtype <0 )How can an unsigned type be less than zero?
It can't. Have corrected type for subtype.
Quoted 4 lines
Quoted 4 lines
drivers/soc/qcom/socinfo.c new file +4
-0+return -EINVAL ; + +return sprintf (buf,"%u\n" , subtype); +}
<snip>
Quoted 7 lines
Quoted 7 lines
drivers/soc/qcom/socinfo.c new file +7
-0+void qcom_socinfo_init (struct device *device) +{ +struct soc_device_attribute *attr; +struct soc_device *soc_dev; +struct device *dev; +size_t item_size; +size_t size;
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+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" );Couldn't
Done.
Quoted 6 lines
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+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 ) {
Quoted 9 lines
drivers/soc/qcom/socinfo.c
Quoted 6 lines
drivers/soc/qcom/socinfo.c new file +6
-0+dev_err (device,"Wrong socinfo format\n" ); +return ; + } + +if (!le32_to_cpu (socinfo->v0_1.id)) +dev_err (device,"socinfo: Unknown SoC ID!\n" );Drop socinfo: please.
Done.
Quoted 53 lines
---8<----
drivers/soc/qcom/socinfo.c +6
-5@@ -88,7 +88,7 @@ struct smem_image_attribute { qcom_show_image_crm, qcom_store_image_crm), \ _index \ }; \ - static struct attribute_group _name##_image_attr_group = { \ + static const struct attribute_group _name##_image_attr_group = { \ .attrs = (struct attribute *[]) { \ &_name##_image_attrs.version.attr, \ &_name##_image_attrs.variant.attr, \@@ -99,6 +99,7 @@ struct smem_image_attribute {static const char *const pmic_model[] = { [0 ] ="Unknown PMIC model" , + [11 ] ="PM8916" , [13 ] ="PM8058" , [14 ] ="PM8028" , [15 ] ="PM8901" ,@@ -453,8 +454,8 @@ QCOM_SMEM_IMG_ITEM(adsp, 0444, SMEM_IMAGE_TABLE_ADSP_INDEX);QCOM_SMEM_IMG_ITEM (cnss,0444 ,SMEM_IMAGE_TABLE_CNSS_INDEX );QCOM_SMEM_IMG_ITEM (video,0444 ,SMEM_IMAGE_TABLE_VIDEO_INDEX ); -static const struct attribute_group - *smem_img_tbl[SMEM_IMAGE_VERSION_BLOCKS_COUNT ] = { +static const +struct attribute_group *smem_img_tbl[SMEM_IMAGE_VERSION_BLOCKS_COUNT ] = { [SMEM_IMAGE_TABLE_BOOT_INDEX ] = &boot_image_attr_group, [SMEM_IMAGE_TABLE_TZ_INDEX ] = &tz_image_attr_group, [SMEM_IMAGE_TABLE_RPM_INDEX ] = &rpm_image_attr_group,@@ -489,7 +490,7 @@ void qcom_socinfo_init(struct device *device) 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" ); +dev_err (device,"Couldn't find socinfo\n" );return ; }@@ -501,7 +502,7 @@ void qcom_socinfo_init(struct device *device) }if (!le32_to_cpu (socinfo->v0_1.id)) -dev_err (device,"socinfo: Unknown SoC ID!\n" ); +dev_err (device,"Unknown SoC ID!\n" ); smem_image_version = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_IMAGE_VERSION_TABLE,
drivers/soc/qcom/socinfo.c void qcom_socinfo_init(struct device *device) · line 541
Quoted 8 lines
drivers/soc/qcom/socinfo.c +1
-1@@ -534,7 +535,7 @@ void qcom_socinfo_init(struct device *device)/* * Expose SMEM_IMAGE_TABLE to sysfs only when we have IMAGE_TABLE * available in SMEM. As IMAGE_TABLE and SOCINFO are two separate -* items within SMEM, we expose the remaining soc information(i.e +* items within SMEM, we expose the remaining soc information (i.e * only the SOCINFO item available in SMEM) to sysfs even in the * absence of an IMAGE_TABLE. */
Thanks for the patch.
Thanks and Regards,
Imran
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation