Re: [PATCH 2/2] soc: qcom: socinfo: Add sysfs attributes for fields in v2-v6
From: Naman Jain
Date: Thu Jan 19 2023 - 04:39:45 EST
Thanks Dmitry for reviewing the patches. Sorry, for replying late on
your email, I wanted to collect all the information, before I do it.
On 1/12/2023 4:49 AM, Dmitry Baryshkov wrote:
On 11/01/2023 10:21, Naman Jain wrote:
Add support in sysfs custom attributes for fields in socinfo version
v2-v6. This is to support SoC based operations in userland scripts
and test scripts. Also, add name mappings for hw-platform type to
make the sysfs information more descriptive.
Please include a patch documenting your additions to
Documentation/ABI/testing/sysfs-devices-soc. Please describe usecases
for new attributes and their applicability to non-Qualcomm boards.
The fields added here, are applicable to Qualcomm boards only. I can
include in the same file sysfs-devices-soc, mentioning the same that it
is Qcom specific, or I can create a new file for this,
sysfs-devices-soc-qcom, however you suggest. Mentioning the use cases,
later in the mail.
Note, that testing scripts can access debugfs entries without any issues.
Yes, that is right. Thanks.
Signed-off-by: Naman Jain <quic_namajain@xxxxxxxxxxx>
---
drivers/soc/qcom/socinfo.c | 181 +++++++++++++++++++++++++++++++++++++
1 file changed, 181 insertions(+)
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 251c0fd94962..ff92064c2246 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -41,6 +41,52 @@
*/
#define SMEM_HW_SW_BUILD_ID 137
+enum {
+ HW_PLATFORM_UNKNOWN = 0,
+ HW_PLATFORM_SURF = 1,
+ HW_PLATFORM_FFA = 2,
+ HW_PLATFORM_FLUID = 3,
+ HW_PLATFORM_SVLTE_FFA = 4,
+ HW_PLATFORM_SVLTE_SURF = 5,
+ HW_PLATFORM_MTP_MDM = 7,
+ HW_PLATFORM_MTP = 8,
+ HW_PLATFORM_LIQUID = 9,
+ HW_PLATFORM_DRAGON = 10,
+ HW_PLATFORM_QRD = 11,
+ HW_PLATFORM_HRD = 13,
+ HW_PLATFORM_DTV = 14,
+ HW_PLATFORM_RCM = 21,
+ HW_PLATFORM_STP = 23,
+ HW_PLATFORM_SBC = 24,
+ HW_PLATFORM_HDK = 31,
+ HW_PLATFORM_ATP = 33,
+ HW_PLATFORM_IDP = 34,
+ HW_PLATFORM_INVALID
+};
+
+static const char * const hw_platform[] = {
+ [HW_PLATFORM_UNKNOWN] = "Unknown",
+ [HW_PLATFORM_SURF] = "Surf",
+ [HW_PLATFORM_FFA] = "FFA",
+ [HW_PLATFORM_FLUID] = "Fluid",
+ [HW_PLATFORM_SVLTE_FFA] = "SVLTE_FFA",
+ [HW_PLATFORM_SVLTE_SURF] = "SLVTE_SURF",
+ [HW_PLATFORM_MTP_MDM] = "MDM_MTP_NO_DISPLAY",
+ [HW_PLATFORM_MTP] = "MTP",
+ [HW_PLATFORM_RCM] = "RCM",
+ [HW_PLATFORM_LIQUID] = "Liquid",
+ [HW_PLATFORM_DRAGON] = "Dragon",
+ [HW_PLATFORM_QRD] = "QRD",
+ [HW_PLATFORM_HRD] = "HRD",
+ [HW_PLATFORM_DTV] = "DTV",
+ [HW_PLATFORM_STP] = "STP",
+ [HW_PLATFORM_SBC] = "SBC",
+ [HW_PLATFORM_HDK] = "HDK",
+ [HW_PLATFORM_ATP] = "ATP",
+ [HW_PLATFORM_IDP] = "IDP",
+ [HW_PLATFORM_INVALID] = "Invalid",
+};
This is not a property of the SoC. It is a property of the device. As
such it should not be part of /sys/bus/soc devices.
I understand your point. The Socinfo structure as such on Qualcomm SoC
gives not just SoC related information but also many other info like
serial number, platform subtype etc. Now in order to support the
usecases below, we are proposing sysfs interface extension, as we can't
use debugfs interface in production/end user devices due to debugfs
access restrictions.
Use cases:
1. In post-boot shell scripts, for various chip specific operations,
that are relevant to that particular chip/board only:
a. Setting kernel parameters using sysfs interfaces etc.
b. Enabling particular traces, logs
c. Changing permissions to certain paths
d. Start a userspace service, and pass custom parameters to it on
the fly
e. Set certain device properties using setprop
f. Miscellaneous things like DCC (Data Capture and Compare Engine) etc.
2. In userspace services, that depend on SoC information, for its
configuration. Eg: Audio, Connectivity services use these.
3. adb needs device serial number, sensors need SoC information to
decide its configuration.
You can find board description in /sys/firmware/devicetree/base/model
Thanks for pointing this out. This is giving useful information on the
chip and hw_platform, but the problem is that we need other fields as
well, which we may want to use. Hence the ask.
model = "Qualcomm Technologies, Inc. Kalama MTP";
+
#ifdef CONFIG_DEBUG_FS
#define SMEM_IMAGE_VERSION_BLOCKS_COUNT 32
#define SMEM_IMAGE_VERSION_SIZE 4096
@@ -368,6 +414,140 @@ static const struct soc_id soc_id[] = {
{ qcom_board_id(QRU1062) },
};
+/* sysfs attributes */
+#define ATTR_DEFINE(param) \
+ static DEVICE_ATTR(param, 0644, qcom_get_##param, NULL)
+
+/* Version 2 */
+static ssize_t
+qcom_get_raw_id(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%u\n",
+ le32_to_cpu(soc_info->raw_id));
+}
+ATTR_DEFINE(raw_id);
+
+static ssize_t
+qcom_get_raw_version(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%u\n",
+ le32_to_cpu(soc_info->raw_ver));
+}
+ATTR_DEFINE(raw_version);
Why are they raw? can you unraw them?
Whose version and id are these attributes referring to?
So basically, when we call them raw, it essentially means that it is not
parsed as such (different bits may be giving different information, and
the whole value may mean nothing).
*version* refers to the chip version, which can be like v1, v2, v1.1 etc
in real terms. Its raw value is used to map it to one of these versions.
*id* is used as chip ID for QC SoCs for using JTAG. It is different than
the soc_id that we have.
+
+/* Version 3 */
+static ssize_t
+qcom_get_hw_platform(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ uint32_t hw_plat = le32_to_cpu(soc_info->hw_plat);
+
+ hw_plat = (hw_plat >= HW_PLATFORM_INVALID) ? HW_PLATFORM_INVALID
: hw_plat;
+ return scnprintf(buf, PAGE_SIZE, "%-.32s\n",
+ hw_platform[hw_plat]);
+}
+ATTR_DEFINE(hw_platform);
+
+/* Version 4 */
+static ssize_t
+qcom_get_platform_version(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%u\n",
+ le32_to_cpu(soc_info->plat_ver));
+}
+ATTR_DEFINE(platform_version);
+
+/* Version 5 */
+static ssize_t
+qcom_get_accessory_chip(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%u\n",
+ le32_to_cpu(soc_info->accessory_chip));
+}
+ATTR_DEFINE(accessory_chip);
If this an _accessory_ chip, there should be a separate soc device
describing it, rather than stuffing information into the soc0.
This is used as a boolean currently to tell us whether SoC has an
accessory chip or not.
+
+/* Version 6 */
+static ssize_t
+qcom_get_platform_subtype_id(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%u\n",
+ le32_to_cpu(soc_info->hw_plat_subtype));
+}
+ATTR_DEFINE(platform_subtype_id);
Again, this is the board property, not an SoC one.
Same justification as one of my previous comments.
+
+static struct attribute *qcom_custom_socinfo_attrs[7];
+
+static const struct attribute_group custom_soc_attr_group = {
+ .attrs = qcom_custom_socinfo_attrs,
+};
+
+static void qcom_socinfo_populate_sysfs(struct qcom_socinfo
*qcom_socinfo)
+{
+ int i = 0, socinfo_format = le32_to_cpu(soc_info->fmt);
+
+ /* Note: qcom_custom_socinfo_attrs[] size needs to be in sync
with attributes added here. */
+ switch (socinfo_format) {
+ case SOCINFO_VERSION(0, 16):
+ fallthrough;
+ case SOCINFO_VERSION(0, 15):
+ fallthrough;
+ case SOCINFO_VERSION(0, 14):
+ fallthrough;
+ case SOCINFO_VERSION(0, 13):
+ fallthrough;
+ case SOCINFO_VERSION(0, 12):
+ fallthrough;
+ case SOCINFO_VERSION(0, 11):
+ fallthrough;
+ case SOCINFO_VERSION(0, 10):
+ fallthrough;
+ case SOCINFO_VERSION(0, 9):
+ fallthrough;
+ case SOCINFO_VERSION(0, 8):
+ fallthrough;
+ case SOCINFO_VERSION(0, 7):
+ fallthrough;
+ case SOCINFO_VERSION(0, 6):
+ qcom_custom_socinfo_attrs[i++] =
+ &dev_attr_platform_subtype_id.attr;
+ fallthrough;
+ case SOCINFO_VERSION(0, 5):
+ qcom_custom_socinfo_attrs[i++] = &dev_attr_accessory_chip.attr;
+ fallthrough;
+ case SOCINFO_VERSION(0, 4):
+ qcom_custom_socinfo_attrs[i++] =
&dev_attr_platform_version.attr;
+ fallthrough;
+ case SOCINFO_VERSION(0, 3):
+ qcom_custom_socinfo_attrs[i++] = &dev_attr_hw_platform.attr;
+ fallthrough;
+ case SOCINFO_VERSION(0, 2):
+ qcom_custom_socinfo_attrs[i++] = &dev_attr_raw_id.attr;
+ qcom_custom_socinfo_attrs[i++] = &dev_attr_raw_version.attr;
+ fallthrough;
+ case SOCINFO_VERSION(0, 1):
+ break;
+ default:
+ pr_err("Unknown socinfo format: v%u.%u\n",
+ SOCINFO_MAJOR(socinfo_format),
+ SOCINFO_MINOR(socinfo_format));
+ break;
+ }
+
+ qcom_custom_socinfo_attrs[i] = NULL;
+ qcom_socinfo->attr.custom_attr_group = &custom_soc_attr_group;
+}
+
static const char *socinfo_machine(struct device *dev, unsigned int
id)
{
int idx;
@@ -696,6 +876,7 @@ static int qcom_socinfo_probe(struct
platform_device *pdev)
"%u",
le32_to_cpu(soc_info->serial_num));
+ qcom_socinfo_populate_sysfs(qs);
qs->soc_dev = soc_device_register(&qs->attr);
if (IS_ERR(qs->soc_dev))
return PTR_ERR(qs->soc_dev);
Thanks,
Naman Jain