Re: [PATCH v3] soc: qcom: Add SoC info driver

[Imran Khan <kimran@codeaurora.org>]

On 11/15/2016 10:57 AM, Bjorn Andersson wrote:
> On Mon 14 Nov 06:30 PST 2016, Imran Khan wrote:
> 
>> On 11/8/2016 1:05 AM, Bjorn Andersson wrote:
>>> On Mon 07 Nov 06:35 PST 2016, Imran Khan wrote:
>>>
>>>
>>>
>>> [..]
>>>
>>>>>> +static void socinfo_populate(struct soc_device_attribute *soc_dev_attr)
>>>>>> +{
>>>>>> +	u32 soc_version = socinfo_get_version();
>>>>>> +
>>>>>> +	soc_dev_attr->soc_id   = kasprintf(GFP_KERNEL, "%d", socinfo_get_id());
>>>>>
>>>>> I believe soc_id is supposed to be a human readable name; e.g. "MSM8996"
>>>>> not "246".
>>>>>
>>>>
>>>> I am not sure about this. I see other vendors also exposing soc_id as numeric value
>>>> and machine is perhaps used for a human readable name. Please let me if I 
>>>> am getting something wrong here.
>>>>
>>>
>>> I'm slightly confused to what these various properties are supposed to
>>> contain, according to Documentation/ABI/testing/sysfs-devices-soc soc_id
>>> should contain the SoC serial number, while most implementations does
>>> like you and put something telling which SoC it is.
>>>
>>> 246 is however not a useful number, as everyone reading it - be it human
>>> or computer - will have to carry the translation table to figure out
>>> what it actually says.
>>>
>>
>> Yeah. I agree on this point. I was just following the lead of other SoCs here.
>> Just worried if having a string here breaks the convention. At least having
>> a numeric number is more in line with the documentation which expects a 
>> serial number. May be here by serial number the documentation means numeric
>> id itself. Can someone please provide some feedback? 
>>  
> 
> Yeah, the more i look at this the more puzzled I become about what
> should go where.
> 
>>>>>> +	soc_dev_attr->family  =  "Snapdragon";
>>>
>>> I think family should be e.g. "MSM8996" and then machine should be e.g.
>>> "MSM8996AU".
>>>
>>
>> I think here family should be Snapdragon.The following site also mentions
>> the SoCs as Snapdragon family of processors.
>>
>> https://www.qualcomm.com/products/snapdragon/processors/comparison
>>
>> Could you please confirm if it's okay?
>>
> 
> In our previous technical discussions regarding Qualcomm platforms the
> possible values for "family" would be U, A and B (maybe something new
> these days?).
> 
> But I don't think we gain anything from having the kernel tell us this.
> 
> So I'm fine with you reporting "Snapdragon" as family and I guess
> machine would then get e.g. "APQ8096". I don't know what to put in
> soc_id.
> 
> I think this would be sufficient for user space's needs.
> 

I have removed the socinfo_print function in Patch-v5. Family is 
being shown as Snapdragon and machine as you guessed is something
like APQ8096. I am showing numeric-id as soc_id as of now since 
I could not get any specific feedback in this regard. Will wait
for feedback regarding soc_id.

> Regards,
> Bjorn
> 
Thanks again Bjorn for your time and feedback

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