Re: [PATCH 4/4] cpufreq: qcom-nvmem: make qcom_cpufreq_get_msm_id() return the SoC ID

[Konrad Dybcio]

On 3.03.2023 22:38, Robert Marko wrote:
> On Fri, 3 Mar 2023 at 21:46, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>>
>>
>>
>> On 3.03.2023 19:38, Robert Marko wrote:
>>> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>>>>
>>>>
>>>>
>>>> On 18.02.2023 21:36, Dmitry Baryshkov wrote:
>>>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 21.01.2023 12:29, Robert Marko wrote:
>>>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID
>>>>>>> after getting it via SMEM call but instead uses an enum to encode the
>>>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in
>>>>>>> qcom_cpufreq_kryo_name_version() to set the supported version.
>>>>>>>
>>>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing
>>>>>>> more than its name suggests, so lets make it just return the SoC ID
>>>>>>> directly which allows matching directly on the SoC ID and removes the need
>>>>>>> for msm8996_version enum which simplifies the driver.
>>>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s.
>>>>>>>
>>>>>>> Signed-off-by: Robert Marko <robimarko@xxxxxxxxx>
>>>>>>> ---
>>>>>>>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++--------------------
>>>>>>>  1 file changed, 12 insertions(+), 32 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> index da55d2e1925a..9deaf9521d6d 100644
>>>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
>>>>>>> @@ -32,12 +32,6 @@
>>>>>>>
>>>>>>>  #include <dt-bindings/arm/qcom,ids.h>
>>>>>>>
>>>>>>> -enum _msm8996_version {
>>>>>>> -     MSM8996_V3,
>>>>>>> -     MSM8996_SG,
>>>>>>> -     NUM_OF_MSM8996_VERSIONS,
>>>>>>> -};
>>>>>>> -
>>>>>>>  struct qcom_cpufreq_drv;
>>>>>>>
>>>>>>>  struct qcom_cpufreq_match_data {
>>>>>>> @@ -134,30 +128,16 @@ static void get_krait_bin_format_b(struct device *cpu_dev,
>>>>>>>       dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver);
>>>>>>>  }
>>>>>>>
>>>>>>> -static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
>>>>>>> +static int qcom_cpufreq_get_msm_id(void)
>>>>>> This should be u32 as info->id is __le32
>>>
>>> Nice catch.
>>>
>>>
>>>>>>
>>>>>> And please export this function from socinfo, it'll come in
>>>>>> useful for other drivers!
>>>
>>> I intentionally did not do that as socinfo is currently fully optional
>>> and I dont really like
>>> the idea of making it required for anything using SMEM.
>> "anything using SMEM"? As in the drivers, or SoCs?
>> If the former, I don't see how exporting a function from within
>> socid and using it here would make it required for other drivers.
>> If the latter, we're talking non-qcom SoCs. SMEM has been with
>> us forever.
> 
> I feel we have a misunderstanding,
> currently, cpufreq-nvmem does not depend on socinfo being built
> so I don't want to require it as a dependency in order to get the SMEM ID.
Okay yeah we simply weren't on the same page.

> 
> Granted, socinfo is useful on any QCA SoC so if adding new dependecies to
> cpufreq-nvmem is acceptable I am not against exporting it there.
IMO, it would be acceptable. Let's hear if others are on board too.

Konrad
>>
>>
>> I'm planning to reuse this for Adreno speedbin matching. It's one
>> of those blocks that don't have a revision and/or bin reigster
>> within themselves.
> 
> I understand the use case, I am sure it will be required in some other places
> sooner or later as well.
> 
> Regards,
> Robert
>>
>> Konrad
>>>
>>> Regards,
>>> Robert
>>>
>>>>
>>>> Konrad
>>>>>