| Message ID | 20250411095025.4067964-3-mukesh.ojha@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] soc: qcom: socinfo: Add support for new fields in revision 20 | expand |
On 4/11/25 11:50 AM, Mukesh Ojha wrote: > Add the ncluster_cores_array_offset field with socinfo structure > revision 22 which specifies no of cores present in each cluster. > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > --- So with all three of your patches, you neither introduce a user for them, nor even expose them in debugfs. Please definitely add the latter, and let's talk about the former. What's 'subpart feture'? How should we interpret the value added in patch 1? Does it expose the higher temperature threshold in degrees, or do we need to add some hardcoded variants for each platform separately? Konrad
On Fri, Apr 11, 2025 at 12:01:48PM +0200, Konrad Dybcio wrote: > On 4/11/25 11:50 AM, Mukesh Ojha wrote: > > Add the ncluster_cores_array_offset field with socinfo structure > > revision 22 which specifies no of cores present in each cluster. > > > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > > --- > > So with all three of your patches, you neither introduce a user for them, > nor even expose them in debugfs. > > Please definitely add the latter, and let's talk about the former. These all revision is added as part of latest boot firmware's socinfo struct version and that also necessitates updating Linux socinfo struct version. I don't have a problem in adding debugfs entry for all of them however, I don't feel the need unless there is already some user or kernel space code using it. If you still feel like we should add it, let me know, will do it. > > What's 'subpart feture'? Ah, my bad I did not explain that field in the patch. Subpart_feat_offset, it is subpart like camera, display, etc., internal feature available on a bin. > How should we interpret the value added in patch 1? Does it expose the > higher temperature threshold in degrees, or do we need to add some hardcoded > variants for each platform separately? As the name feature suggest some of thermal policy could change based on this value and currently, this will contain only 0 or 1 and 1 means its heat dissipation is better and more relaxed thermal scheme can be put in place. -Mukesh
On 4/11/25 6:57 PM, Mukesh Ojha wrote: > On Fri, Apr 11, 2025 at 12:01:48PM +0200, Konrad Dybcio wrote: >> On 4/11/25 11:50 AM, Mukesh Ojha wrote: >>> Add the ncluster_cores_array_offset field with socinfo structure >>> revision 22 which specifies no of cores present in each cluster. >>> >>> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> >>> --- >> >> So with all three of your patches, you neither introduce a user for them, >> nor even expose them in debugfs. >> >> Please definitely add the latter, and let's talk about the former. > > These all revision is added as part of latest boot firmware's socinfo > struct version and that also necessitates updating Linux socinfo struct > version. > > I don't have a problem in adding debugfs entry for all of them however, I > don't feel the need unless there is already some user or kernel space code > using it. > > If you still feel like we should add it, let me know, will do it. Yeah please do, debugfs is precisely for the cases where *someone* may want to get a read out, but it's not especially useful in general, plus most (all?) other values that this driver retrieves are already exposed there. >> What's 'subpart feture'? > > Ah, my bad I did not explain that field in the patch. > > Subpart_feat_offset, it is subpart like camera, display, etc., internal > feature available on a bin. > > >> How should we interpret the value added in patch 1? Does it expose the >> higher temperature threshold in degrees, or do we need to add some hardcoded >> variants for each platform separately? > > As the name feature suggest some of thermal policy could change based on > this value and currently, this will contain only 0 or 1 and 1 means > its heat dissipation is better and more relaxed thermal scheme can be > put in place. Please add some comments in both cases Konrad
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c index 0a6eb8060f5b..6319a73a660b 100644 --- a/drivers/soc/qcom/socinfo.c +++ b/drivers/soc/qcom/socinfo.c @@ -607,6 +607,7 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo, &qcom_socinfo->info.fmt); switch (qcom_socinfo->info.fmt) { + case SOCINFO_VERSION(0, 22): case SOCINFO_VERSION(0, 21): case SOCINFO_VERSION(0, 20): case SOCINFO_VERSION(0, 19): diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h index 3666870b7988..0c12090311aa 100644 --- a/include/linux/soc/qcom/socinfo.h +++ b/include/linux/soc/qcom/socinfo.h @@ -86,6 +86,8 @@ struct socinfo { __le32 raw_package_type; /* Version 21 */ __le32 nsubpart_feat_array_offset; + /* Version 22 */ + __le32 ncluster_cores_array_offset; }; /* Internal feature codes */
Add the ncluster_cores_array_offset field with socinfo structure revision 22 which specifies no of cores present in each cluster. Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> --- drivers/soc/qcom/socinfo.c | 1 + include/linux/soc/qcom/socinfo.h | 2 ++ 2 files changed, 3 insertions(+)