diff mbox series

[1/2] soc: qcom: socinfo: Change socinfo variable name and scope

Message ID 20230111082141.18109-2-quic_namajain@quicinc.com (mailing list archive)
State Changes Requested
Headers show
Series soc: qcom: socinfo: Add fields in sysfs custom attributes | expand

Commit Message

Naman Jain Jan. 11, 2023, 8:21 a.m. UTC 
Change socinfo structure variable scope from function to file
to make it easy to support custom attributes for sysfs. Also,
change variable name to make it more descriptive.

Signed-off-by: Naman Jain <quic_namajain@quicinc.com>
---
 drivers/soc/qcom/socinfo.c | 80 ++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 38 deletions(-)

Comments

Trilok Soni Jan. 11, 2023, 9:37 p.m. UTC | #1 
On 1/11/2023 12:21 AM, Naman Jain wrote:
> Change socinfo structure variable scope from function to file
> to make it easy to support custom attributes for sysfs. Also,
> change variable name to make it more descriptive.

Did you mean debugfs?

Can you one example of custom attribute in the commit text so that we
understand the motivation better?

> 
> Signed-off-by: Naman Jain <quic_namajain@quicinc.com>
> ---
>   drivers/soc/qcom/socinfo.c | 80 ++++++++++++++++++++------------------
>   1 file changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 10efdbcfdf05..251c0fd94962 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -175,6 +175,7 @@ struct socinfo {
>   	__le32  npartnamemap_offset;
>   	__le32  nnum_partname_mapping;
>   };
> +static struct socinfo *soc_info;

Is there any better way to do it? Should not asume the just one object
and dynamically allocate it? Let's wait for Bjorn to check as well.

---Trilok Soni
Naman Jain Jan. 19, 2023, 9:33 a.m. UTC | #2 
Hi Trilok,

Thanks for reviewing the patches.

On 1/12/2023 3:07 AM, Trilok Soni wrote:
> On 1/11/2023 12:21 AM, Naman Jain wrote:
>> Change socinfo structure variable scope from function to file
>> to make it easy to support custom attributes for sysfs. Also,
>> change variable name to make it more descriptive.
>
> Did you mean debugfs?


No, I meant sysfs only. debugfs support is generally added with every 
version update, in kernel. Since debugfs can't be used for these 
purposes in production devices, we are proposing to extend current sysfs 
interface.


>
> Can you one example of custom attribute in the commit text so that we
> understand the motivation better?
>

I'll add the examples in next patch. Thanks.


>>
>> Signed-off-by: Naman Jain <quic_namajain@quicinc.com>
>> ---
>>   drivers/soc/qcom/socinfo.c | 80 ++++++++++++++++++++------------------
>>   1 file changed, 42 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
>> index 10efdbcfdf05..251c0fd94962 100644
>> --- a/drivers/soc/qcom/socinfo.c
>> +++ b/drivers/soc/qcom/socinfo.c
>> @@ -175,6 +175,7 @@ struct socinfo {
>>       __le32  npartnamemap_offset;
>>       __le32  nnum_partname_mapping;
>>   };
>> +static struct socinfo *soc_info;
>
> Is there any better way to do it? Should not asume the just one object
> and dynamically allocate it? Let's wait for Bjorn to check as well.


So current sysfs attributes are added in probe function, where this 
"info" variable is defined and used. For additions to current sysfs 
interface, using a separate function, the need for having this variable 
with file scope came. Now, I can keep the variable name, same, as "info" 
and not change it to "soc_info" if the forum suggests that, just that we 
thought it feels more descriptive to change it to "soc_info", when we 
make it's scope to file. Also, in future, with this variable global, if 
we decide to support kernel clients by exporting these fields through 
APIs, we can easily make use of this and implement.


>
> ---Trilok Soni


Thanks,

Naman Jain
diff mbox series

Patch

diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 10efdbcfdf05..251c0fd94962 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -175,6 +175,7 @@  struct socinfo {
 	__le32  npartnamemap_offset;
 	__le32  nnum_partname_mapping;
 };
+static struct socinfo *soc_info;
 
 #ifdef CONFIG_DEBUG_FS
 struct socinfo_params {
@@ -502,7 +503,7 @@  DEFINE_IMAGE_OPS(variant);
 DEFINE_IMAGE_OPS(oem);
 
 static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
-				 struct socinfo *info, size_t info_size)
+				 size_t info_size)
 {
 	struct smem_image_version *versions;
 	struct dentry *dentry;
@@ -513,15 +514,15 @@  static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 
 	qcom_socinfo->dbg_root = debugfs_create_dir("qcom_socinfo", NULL);
 
-	qcom_socinfo->info.fmt = __le32_to_cpu(info->fmt);
+	qcom_socinfo->info.fmt = __le32_to_cpu(soc_info->fmt);
 
 	debugfs_create_x32("info_fmt", 0444, qcom_socinfo->dbg_root,
 			   &qcom_socinfo->info.fmt);
 
 	switch (qcom_socinfo->info.fmt) {
 	case SOCINFO_VERSION(0, 16):
-		qcom_socinfo->info.feature_code = __le32_to_cpu(info->feature_code);
-		qcom_socinfo->info.pcode = __le32_to_cpu(info->pcode);
+		qcom_socinfo->info.feature_code = __le32_to_cpu(soc_info->feature_code);
+		qcom_socinfo->info.pcode = __le32_to_cpu(soc_info->pcode);
 
 		debugfs_create_u32("feature_code", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.feature_code);
@@ -529,16 +530,20 @@  static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 				   &qcom_socinfo->info.pcode);
 		fallthrough;
 	case SOCINFO_VERSION(0, 15):
-		qcom_socinfo->info.nmodem_supported = __le32_to_cpu(info->nmodem_supported);
+		qcom_socinfo->info.nmodem_supported = __le32_to_cpu(soc_info->nmodem_supported);
 
 		debugfs_create_u32("nmodem_supported", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.nmodem_supported);
 		fallthrough;
 	case SOCINFO_VERSION(0, 14):
-		qcom_socinfo->info.num_clusters = __le32_to_cpu(info->num_clusters);
-		qcom_socinfo->info.ncluster_array_offset = __le32_to_cpu(info->ncluster_array_offset);
-		qcom_socinfo->info.num_defective_parts = __le32_to_cpu(info->num_defective_parts);
-		qcom_socinfo->info.ndefective_parts_array_offset = __le32_to_cpu(info->ndefective_parts_array_offset);
+		qcom_socinfo->info.num_clusters =
+			__le32_to_cpu(soc_info->num_clusters);
+		qcom_socinfo->info.ncluster_array_offset =
+			__le32_to_cpu(soc_info->ncluster_array_offset);
+		qcom_socinfo->info.num_defective_parts =
+			__le32_to_cpu(soc_info->num_defective_parts);
+		qcom_socinfo->info.ndefective_parts_array_offset =
+			__le32_to_cpu(soc_info->ndefective_parts_array_offset);
 
 		debugfs_create_u32("num_clusters", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.num_clusters);
@@ -550,19 +555,19 @@  static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 				   &qcom_socinfo->info.ndefective_parts_array_offset);
 		fallthrough;
 	case SOCINFO_VERSION(0, 13):
-		qcom_socinfo->info.nproduct_id = __le32_to_cpu(info->nproduct_id);
+		qcom_socinfo->info.nproduct_id = __le32_to_cpu(soc_info->nproduct_id);
 
 		debugfs_create_u32("nproduct_id", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.nproduct_id);
-		DEBUGFS_ADD(info, chip_id);
+		DEBUGFS_ADD(soc_info, chip_id);
 		fallthrough;
 	case SOCINFO_VERSION(0, 12):
 		qcom_socinfo->info.chip_family =
-			__le32_to_cpu(info->chip_family);
+			__le32_to_cpu(soc_info->chip_family);
 		qcom_socinfo->info.raw_device_family =
-			__le32_to_cpu(info->raw_device_family);
+			__le32_to_cpu(soc_info->raw_device_family);
 		qcom_socinfo->info.raw_device_num =
-			__le32_to_cpu(info->raw_device_num);
+			__le32_to_cpu(soc_info->raw_device_num);
 
 		debugfs_create_x32("chip_family", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.chip_family);
@@ -574,26 +579,26 @@  static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 				   &qcom_socinfo->info.raw_device_num);
 		fallthrough;
 	case SOCINFO_VERSION(0, 11):
-		num_pmics = le32_to_cpu(info->num_pmics);
-		pmic_array_offset = le32_to_cpu(info->pmic_array_offset);
+		num_pmics = le32_to_cpu(soc_info->num_pmics);
+		pmic_array_offset = le32_to_cpu(soc_info->pmic_array_offset);
 		if (pmic_array_offset + 2 * num_pmics * sizeof(u32) <= info_size)
-			DEBUGFS_ADD(info, pmic_model_array);
+			DEBUGFS_ADD(soc_info, pmic_model_array);
 		fallthrough;
 	case SOCINFO_VERSION(0, 10):
 	case SOCINFO_VERSION(0, 9):
-		qcom_socinfo->info.foundry_id = __le32_to_cpu(info->foundry_id);
+		qcom_socinfo->info.foundry_id = __le32_to_cpu(soc_info->foundry_id);
 
 		debugfs_create_u32("foundry_id", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.foundry_id);
 		fallthrough;
 	case SOCINFO_VERSION(0, 8):
 	case SOCINFO_VERSION(0, 7):
-		DEBUGFS_ADD(info, pmic_model);
-		DEBUGFS_ADD(info, pmic_die_rev);
+		DEBUGFS_ADD(soc_info, pmic_model);
+		DEBUGFS_ADD(soc_info, pmic_die_rev);
 		fallthrough;
 	case SOCINFO_VERSION(0, 6):
 		qcom_socinfo->info.hw_plat_subtype =
-			__le32_to_cpu(info->hw_plat_subtype);
+			__le32_to_cpu(soc_info->hw_plat_subtype);
 
 		debugfs_create_u32("hardware_platform_subtype", 0444,
 				   qcom_socinfo->dbg_root,
@@ -601,34 +606,34 @@  static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
 		fallthrough;
 	case SOCINFO_VERSION(0, 5):
 		qcom_socinfo->info.accessory_chip =
-			__le32_to_cpu(info->accessory_chip);
+			__le32_to_cpu(soc_info->accessory_chip);
 
 		debugfs_create_u32("accessory_chip", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.accessory_chip);
 		fallthrough;
 	case SOCINFO_VERSION(0, 4):
-		qcom_socinfo->info.plat_ver = __le32_to_cpu(info->plat_ver);
+		qcom_socinfo->info.plat_ver = __le32_to_cpu(soc_info->plat_ver);
 
 		debugfs_create_u32("platform_version", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.plat_ver);
 		fallthrough;
 	case SOCINFO_VERSION(0, 3):
-		qcom_socinfo->info.hw_plat = __le32_to_cpu(info->hw_plat);
+		qcom_socinfo->info.hw_plat = __le32_to_cpu(soc_info->hw_plat);
 
 		debugfs_create_u32("hardware_platform", 0444,
 				   qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.hw_plat);
 		fallthrough;
 	case SOCINFO_VERSION(0, 2):
-		qcom_socinfo->info.raw_ver  = __le32_to_cpu(info->raw_ver);
+		qcom_socinfo->info.raw_ver  = __le32_to_cpu(soc_info->raw_ver);
 
 		debugfs_create_u32("raw_version", 0444, qcom_socinfo->dbg_root,
 				   &qcom_socinfo->info.raw_ver);
 		fallthrough;
 	case SOCINFO_VERSION(0, 1):
-		DEBUGFS_ADD(info, build_id);
+		DEBUGFS_ADD(soc_info, build_id);
 		break;
 	}
 
@@ -656,7 +661,7 @@  static void socinfo_debugfs_exit(struct qcom_socinfo *qcom_socinfo)
 }
 #else
 static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
-				 struct socinfo *info, size_t info_size)
+				 size_t info_size)
 {
 }
 static void socinfo_debugfs_exit(struct qcom_socinfo *qcom_socinfo) {  }
@@ -665,14 +670,13 @@  static void socinfo_debugfs_exit(struct qcom_socinfo *qcom_socinfo) {  }
 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,
+	soc_info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
 			      &item_size);
-	if (IS_ERR(info)) {
+	if (IS_ERR(soc_info)) {
 		dev_err(&pdev->dev, "Couldn't find socinfo\n");
-		return PTR_ERR(info);
+		return PTR_ERR(soc_info);
 	}
 
 	qs = devm_kzalloc(&pdev->dev, sizeof(*qs), GFP_KERNEL);
@@ -681,25 +685,25 @@  static int qcom_socinfo_probe(struct platform_device *pdev)
 
 	qs->attr.family = "Snapdragon";
 	qs->attr.machine = socinfo_machine(&pdev->dev,
-					   le32_to_cpu(info->id));
+					   le32_to_cpu(soc_info->id));
 	qs->attr.soc_id = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u",
-					 le32_to_cpu(info->id));
+					 le32_to_cpu(soc_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)));
+					   SOCINFO_MAJOR(le32_to_cpu(soc_info->ver)),
+					   SOCINFO_MINOR(le32_to_cpu(soc_info->ver)));
 	if (offsetof(struct socinfo, serial_num) <= item_size)
 		qs->attr.serial_number = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 							"%u",
-							le32_to_cpu(info->serial_num));
+							le32_to_cpu(soc_info->serial_num));
 
 	qs->soc_dev = soc_device_register(&qs->attr);
 	if (IS_ERR(qs->soc_dev))
 		return PTR_ERR(qs->soc_dev);
 
-	socinfo_debugfs_init(qs, info, item_size);
+	socinfo_debugfs_init(qs, item_size);
 
 	/* Feed the soc specific unique data into entropy pool */
-	add_device_randomness(info, item_size);
+	add_device_randomness(soc_info, item_size);
 
 	platform_set_drvdata(pdev, qs);