Re: [PATCH 3/3] soc: qcom: socinfo: Add support for new fields in revision 22

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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