All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/2] soc: qcom: Add SoC info driver
@ 2017-01-10 15:48 Imran Khan
  2017-02-15  0:24 ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Imran Khan @ 2017-01-10 15:48 UTC (permalink / raw)
  To: andy.gross
  Cc: david.brown, linux-kernel, linux-arm-msm, linux-soc, 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.

Signed-off-by: Imran Khan <kimran@codeaurora.org>
---

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 | 516 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 524 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/qcom/socinfo.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387..f89d34d 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -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
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664e..438efec 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -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
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 18ec52f..4f0ccf8 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -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;
 }
 
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.
+ *
+ * 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
+ *   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
+
+#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
+
+static const char *odm_name;
+
+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) \
+	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 = { \
+		.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",
+	[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];
+};
+
+/* 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];
+};
+
+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;
+	__le32 plat_ver;
+};
+
+struct socinfo_v0_5 {
+	struct socinfo_v0_4 v0_4;
+	__le32 accsry_chip;
+};
+
+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;
+	__le32 pmic_die_rev_1;
+	__le32 pmic_model_2;
+	__le32 pmic_die_rev_2;
+};
+
+struct socinfo_v0_9 {
+	struct socinfo_v0_8 v0_8;
+	__le32 fndry_id;
+};
+
+struct socinfo_v0_10 {
+	struct socinfo_v0_9 v0_9;
+	__le32 srl_num;
+};
+
+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;
+	__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;
+	struct socinfo_v0_10 v0_10;
+	struct socinfo_v0_11 v0_11;
+	struct socinfo_v0_12 v0_12;
+} *socinfo;
+
+static struct smem_image_version *smem_image_version;
+
+/* max socinfo format version supported */
+#define MAX_SOCINFO_FORMAT 12
+
+/* socinfo: sysfs functions */
+
+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);
+
+static ssize_t
+qcom_show_vendor(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%s", "Qualcomm\n");
+}
+
+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);
+}
+
+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)
+{
+	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)
+		return -EINVAL;
+
+	return sprintf(buf, "%u\n", subtype);
+}
+
+static ssize_t
+qcom_show_foundry_id(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	return sprintf(buf, "%u\n", le32_to_cpu(socinfo->v0_9.fndry_id));
+}
+
+static ssize_t
+qcom_show_serial_number(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	return sprintf(buf, "%u\n", le32_to_cpu(socinfo->v0_10.srl_num));
+}
+
+static ssize_t
+qcom_show_chip_family(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	return sprintf(buf, "0x%x\n", le32_to_cpu(socinfo->v0_12.chip_family));
+}
+
+static ssize_t
+qcom_show_raw_device_family(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	return sprintf(buf, "0x%x\n", le32_to_cpu(socinfo->v0_12.raw_dev_fam));
+}
+
+static ssize_t
+qcom_show_raw_device_number(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	return sprintf(buf, "0x%x\n", le32_to_cpu(socinfo->v0_12.raw_dev_num));
+}
+
+static ssize_t
+qcom_show_pmic_model(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	return sprintf(buf, "%s\n",
+			pmic_model[le32_to_cpu(socinfo->v0_7.pmic_model)]);
+}
+
+static ssize_t
+qcom_show_pmic_die_revision(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	return sprintf(buf, "%u\n", le32_to_cpu(socinfo->v0_7.pmic_die_rev));
+}
+
+static ssize_t
+qcom_show_image_version(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, version);
+	return scnprintf(buf, PAGE_SIZE, "%s\n",
+			smem_image_version[smem_attr->index].name);
+}
+
+static ssize_t
+qcom_store_image_version(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t size)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, version);
+	return strlcpy(smem_image_version[smem_attr->index].name, buf,
+			SMEM_IMAGE_VERSION_NAME_SIZE);
+}
+
+static ssize_t
+qcom_show_image_variant(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, variant);
+	return scnprintf(buf, PAGE_SIZE, "%s\n",
+			smem_image_version[smem_attr->index].variant);
+}
+
+static ssize_t
+qcom_store_image_variant(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t size)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, variant);
+	return strlcpy(smem_image_version[smem_attr->index].variant, buf,
+			SMEM_IMAGE_VERSION_VARIANT_SIZE);
+}
+
+static ssize_t
+qcom_show_image_crm(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, crm);
+	return scnprintf(buf, PAGE_SIZE, "%s\n",
+			smem_image_version[smem_attr->index].oem);
+}
+
+static ssize_t
+qcom_store_image_crm(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t size)
+{
+	struct smem_image_attribute *smem_attr;
+
+	smem_attr = container_of(attr, struct smem_image_attribute, crm);
+	return strlcpy(smem_image_version[smem_attr->index].oem, buf,
+			SMEM_IMAGE_VERSION_OEM_SIZE);
+}
+
+static const struct qcom_socinfo_attr socinfo_attrs[] = {
+	QCOM_SOCINFO_ATTR(chip_family, qcom_show_chip_family, 12),
+	QCOM_SOCINFO_ATTR(raw_device_family, qcom_show_raw_device_family, 12),
+	QCOM_SOCINFO_ATTR(raw_device_number, qcom_show_raw_device_number, 12),
+	QCOM_SOCINFO_ATTR(serial_number, qcom_show_serial_number, 10),
+	QCOM_SOCINFO_ATTR(foundry_id, qcom_show_foundry_id, 9),
+	QCOM_SOCINFO_ATTR(pmic_model, qcom_show_pmic_model, 7),
+	QCOM_SOCINFO_ATTR(pmic_die_revision, qcom_show_pmic_die_revision, 7),
+	QCOM_SOCINFO_ATTR(platform_subtype, qcom_show_platform_subtype, 6),
+	QCOM_SOCINFO_ATTR(accessory_chip, qcom_show_accessory_chip, 5),
+	QCOM_SOCINFO_ATTR(platform_version, qcom_show_platform_version, 4),
+	QCOM_SOCINFO_ATTR(hw_platform, qcom_show_hw_platform, 3),
+	QCOM_SOCINFO_ATTR(raw_version, qcom_show_raw_version, 2),
+	QCOM_SOCINFO_ATTR(build_id, qcom_show_build_id, 1),
+	QCOM_SOCINFO_ATTR(vendor, qcom_show_vendor, 0),
+};
+
+QCOM_SMEM_IMG_ITEM(boot, 0444, SMEM_IMAGE_TABLE_BOOT_INDEX);
+QCOM_SMEM_IMG_ITEM(tz, 0444, SMEM_IMAGE_TABLE_TZ_INDEX);
+QCOM_SMEM_IMG_ITEM(rpm, 0444, SMEM_IMAGE_TABLE_RPM_INDEX);
+QCOM_SMEM_IMG_ITEM(apps, 0644, SMEM_IMAGE_TABLE_APPS_INDEX);
+QCOM_SMEM_IMG_ITEM(mpss, 0444, SMEM_IMAGE_TABLE_MPSS_INDEX);
+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] = {
+		[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,
+		[SMEM_IMAGE_TABLE_APPS_INDEX] = &apps_image_attr_group,
+		[SMEM_IMAGE_TABLE_MPSS_INDEX] = &mpss_image_attr_group,
+		[SMEM_IMAGE_TABLE_ADSP_INDEX] = &adsp_image_attr_group,
+		[SMEM_IMAGE_TABLE_CNSS_INDEX] = &cnss_image_attr_group,
+		[SMEM_IMAGE_TABLE_VIDEO_INDEX] = &video_image_attr_group,
+};
+
+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)) {
+		dev_err(device, "Wrong socinfo format\n");
+		return;
+	}
+
+	if (!le32_to_cpu(socinfo->v0_1.id))
+		dev_err(device, "socinfo: Unknown SoC ID!\n");
+
+	smem_image_version = qcom_smem_get(QCOM_SMEM_HOST_ANY,
+				SMEM_IMAGE_VERSION_TABLE,
+				&size);
+	if (IS_ERR(smem_image_version) || (size != SMEM_IMAGE_VERSION_SIZE)) {
+		dev_dbg(device, "Image version table absent\n");
+		smem_image_version = NULL;
+	}
+
+	attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+	if (!attr)
+		return;
+
+	attr->soc_id = kasprintf(GFP_KERNEL, "%d",
+				le32_to_cpu(socinfo->v0_1.id));
+	attr->family = "Snapdragon";
+	prop = fdt_get_property(initial_boot_params, 0, "model", NULL);
+	if (prop)
+		attr->machine = kasprintf(GFP_KERNEL, "%s", prop->data);
+
+	attr->revision = kasprintf(GFP_KERNEL, "%u.%u",
+				SOC_VER_MAJ(le32_to_cpu(socinfo->v0_1.ver)),
+				SOC_VER_MIN(le32_to_cpu(socinfo->v0_1.ver)));
+
+	soc_dev = soc_device_register(attr);
+	if (IS_ERR(soc_dev)) {
+		kfree(attr);
+		return;
+	}
+
+	dev = soc_device_to_device(soc_dev);
+
+	/*
+	 * 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
+	 * only the SOCINFO item available in SMEM) to sysfs even in the
+	 * absence of an IMAGE_TABLE.
+	 */
+	if (smem_image_version) {
+		for (i = 0; i < SMEM_IMAGE_VERSION_BLOCKS_COUNT; i++) {
+			if (smem_img_tbl[i])
+				WARN_ON(sysfs_create_group(&dev->kobj,
+					smem_img_tbl[i]));
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(socinfo_attrs); i++) {
+		if (socinfo_attrs[i].min_ver <=	le32_to_cpu(socinfo->v0_1.fmt))
+			device_create_file(dev, &socinfo_attrs[i].attr);
+	}
+
+	odm_name = of_get_property(device->of_node, "qcom,odm", NULL);
+	if (odm_name)
+		device_create_file(dev, &dev_attr_qcom_odm);
+
+	/* Feed the soc specific unique data into entropy pool */
+	add_device_randomness(socinfo, item_size);
+}
+EXPORT_SYMBOL(qcom_socinfo_init);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v8 1/2] soc: qcom: Add SoC info driver
  2017-01-10 15:48 [PATCH v8 1/2] soc: qcom: Add SoC info driver Imran Khan
@ 2017-02-15  0:24 ` Bjorn Andersson
  2017-02-15  5:31   ` Imran Khan
  0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2017-02-15  0:24 UTC (permalink / raw)
  To: Imran Khan
  Cc: andy.gross, david.brown, linux-kernel, linux-arm-msm, linux-soc

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v8 1/2] soc: qcom: Add SoC info driver
  2017-02-15  0:24 ` Bjorn Andersson
@ 2017-02-15  5:31   ` Imran Khan
  2017-02-16  4:32     ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Imran Khan @ 2017-02-15  5:31 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: andy.gross, david.brown, linux-kernel, linux-arm-msm, linux-soc

On 2/15/2017 5:54 AM, Bjorn Andersson wrote:
> 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.
>

Sure, Will update the year.
 
> [..]
>> +
>> +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.
> 

cpu_of_id was mapping soc-id read from SMEM into SoC name, which in turn
was being used as machine name in soc_device_attribute. But we can also
read machine name from DT. Reading the machine name from DT would make the 
solution more flexible as we don't have to make an entry in soc_of_id every
time we get a new SoC.
Also as soc_of_id uses soc-id as index, there is theoretically no limit on how
many elements it may end up having.
Because of the above reasons, I wanted to get rid off soc_of_id (or cpu_of_id)
array and use DT to get machine name as shown in the following snippet:

+	attr->family = "Snapdragon";
+	prop = fdt_get_property(initial_boot_params, 0, "model", NULL);
+	if (prop)
+		attr->machine = kasprintf(GFP_KERNEL, "%s", prop->data);

Could you please let me know if this approach looks fine to you?

> [..]
>> +
>> +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.
> 
> [..]

Okay, will skip the odm attribute.
>> +
>> +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.
> 

Okay.
>> +
>> +	/* 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

Sure. Will send the next patch set once I have confirmation of the above
query. 

Thanks and Regards,
Imran

> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v8 1/2] soc: qcom: Add SoC info driver
  2017-02-15  5:31   ` Imran Khan
@ 2017-02-16  4:32     ` Bjorn Andersson
  0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2017-02-16  4:32 UTC (permalink / raw)
  To: Imran Khan
  Cc: andy.gross, david.brown, linux-kernel, linux-arm-msm, linux-soc

On Tue 14 Feb 21:31 PST 2017, Imran Khan wrote:

> On 2/15/2017 5:54 AM, Bjorn Andersson wrote:
[..]
> > 
> > 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.
> > 
> 
> cpu_of_id was mapping soc-id read from SMEM into SoC name, which in turn
> was being used as machine name in soc_device_attribute. But we can also
> read machine name from DT. Reading the machine name from DT would make the 
> solution more flexible as we don't have to make an entry in soc_of_id every
> time we get a new SoC.

I don't think we need a more flexible solution, as the purpose of this
driver is to expose the socinfo information to user space; i.e. there's
nothing flexible about that.

If we just expose this information as a proprietary magic number, then
it will be up to each consumer to keep some mapping table. So you will
not get rid of the work of keeping things updated, you will just push
the problem to somewhere else.

> Also as soc_of_id uses soc-id as index, there is theoretically no limit on how
> many elements it may end up having.

There's a practical - and reasonable - limit on how much space this data
will take, one could however argue that it's unwise to keep the sparse
data in a simple array. A better solution would be to maintain an array
of id,soc pairs and loop through that on lookup.

> Because of the above reasons, I wanted to get rid off soc_of_id (or cpu_of_id)
> array and use DT to get machine name as shown in the following snippet:
> 
> +	attr->family = "Snapdragon";
> +	prop = fdt_get_property(initial_boot_params, 0, "model", NULL);
> +	if (prop)
> +		attr->machine = kasprintf(GFP_KERNEL, "%s", prop->data);

The data is already in socinfo, we should not manually duplicate it in
DT.

> 
> Could you please let me know if this approach looks fine to you?
> 

I would prefer that the socinfo doesn't expose magic numbers without a
publicly available lookup table. And I don't see a need for the added
flexibility.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-02-16  4:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10 15:48 [PATCH v8 1/2] soc: qcom: Add SoC info driver Imran Khan
2017-02-15  0:24 ` Bjorn Andersson
2017-02-15  5:31   ` Imran Khan
2017-02-16  4:32     ` Bjorn Andersson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.