Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <725edcc6-250a-0cd3-4c32-300f48a8ad79@quicinc.com>
Date:   Thu, 19 Jan 2023 15:03:05 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 1/2] soc: qcom: socinfo: Change socinfo variable name and
 scope
Content-Language: en-US
To:     Trilok Soni <quic_tsoni@quicinc.com>,
        Bjorn Andersson <andersson@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>
CC:     <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_pkondeti@quicinc.com>
References: <20230111082141.18109-1-quic_namajain@quicinc.com>
 <20230111082141.18109-2-quic_namajain@quicinc.com>
 <2f1d92e7-08e0-13f3-2087-f0c55d83383c@quicinc.com>
From:   Naman Jain <quic_namajain@quicinc.com>
In-Reply-To: <2f1d92e7-08e0-13f3-2087-f0c55d83383c@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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