RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

From: ilialin
Date: Sat May 19 2018 - 06:49:45 EST

Next message: tip-bot for Sandipan Das: "[tip:perf/core] perf script: Show symbol offsets by default"
Previous message: tip-bot for Sandipan Das: "[tip:perf/core] perf script: Show virtual addresses instead of offsets"
In reply to: ilialin: "RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver"
Next in thread: Viresh Kumar: "Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Viresh,

If I send patches in reply, it will produce new patches, instead of answers
in the thread. Please find below the file dump.

->cat drivers/cpufreq/qcom-cpufreq-kryo.c
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2018, The Linux Foundation. All rights reserved.
*/

/*
* In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
* the CPU frequency subset and voltage value of each OPP varies
* based on the silicon variant in use. Qualcomm Process Voltage Scaling
Tables
* defines the voltage and frequency value based on the msm-id in SMEM
* and speedbin blown in the efuse combination.
* The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the
SoC
* to provide the OPP framework with required information.
* This is used to determine the voltage and frequency value for each OPP of
* operating-points-v2 table when it is parsed by the OPP framework.
*/

#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/soc/qcom/smem.h>

#define MSM_ID_SMEM 137
#define SILVER_LEAD 0
#define GOLD_LEAD 2

enum _msm_id {
MSM8996V3 = 0xF6ul,
APQ8096V3 = 0x123ul,
MSM8996SG = 0x131ul,
APQ8096SG = 0x138ul,
};

enum _msm8996_version {
MSM8996_V3,
MSM8996_SG,
NUM_OF_MSM8996_VERSIONS,
};

static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
{
size_t len;
u32 *msm_id;
enum _msm8996_version version;

msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
/* The first 4 bytes are format, next to them is the actual msm-id
*/
msm_id++;

switch ((enum _msm_id)*msm_id) {
case MSM8996V3:
case APQ8096V3:
version = MSM8996_V3;
break;
case MSM8996SG:
case APQ8096SG:
version = MSM8996_SG;
break;
default:
version = NUM_OF_MSM8996_VERSIONS;
}

return version;
}

static int __init qcom_cpufreq_kryo_driver_init(void)
{
struct device *cpu_dev_silver, *cpu_dev_gold;
struct opp_table *opp_silver, *opp_gold;
enum _msm8996_version msm8996_version;
struct nvmem_cell *speedbin_nvmem;
struct platform_device *pdev;
struct device_node *np;
u8 *speedbin;
u32 versions;
size_t len;
int ret;

cpu_dev_silver = get_cpu_device(SILVER_LEAD);
if (IS_ERR_OR_NULL(cpu_dev_silver))
return PTR_ERR(cpu_dev_silver);

cpu_dev_gold = get_cpu_device(SILVER_LEAD);
if (IS_ERR_OR_NULL(cpu_dev_gold))
return PTR_ERR(cpu_dev_gold);

msm8996_version = qcom_cpufreq_kryo_get_msm_id();
if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
return -ENODEV;
}

np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
if (IS_ERR_OR_NULL(np))
return PTR_ERR(np);

if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
ret = -ENOENT;
goto free_np;
}

speedbin_nvmem = of_nvmem_cell_get(np, NULL);
if (IS_ERR(speedbin_nvmem)) {
ret = PTR_ERR(speedbin_nvmem);
dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
goto free_np;
}

speedbin = nvmem_cell_read(speedbin_nvmem, &len);
nvmem_cell_put(speedbin_nvmem);

switch (msm8996_version) {
case MSM8996_V3:
versions = 1 << (unsigned int)(*speedbin);
break;
case MSM8996_SG:
versions = 1 << ((unsigned int)(*speedbin) + 4);
break;
default:
BUG();
break;
}

opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
if (IS_ERR(opp_silver)) {
dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
ret = PTR_ERR(opp_silver);
goto free_np;
}

opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
if (IS_ERR(opp_gold)) {
dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
ret = PTR_ERR(opp_gold);
goto free_opp_silver;
}

pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
if (!IS_ERR_OR_NULL(pdev))
return 0;

ret = PTR_ERR(pdev);
dev_err(cpu_dev_silver, "Failed to register platform device\n");
dev_pm_opp_put_supported_hw(opp_gold);

free_opp_silver:
dev_pm_opp_put_supported_hw(opp_silver);

free_np:
of_node_put(np);

return ret;
}
late_initcall(qcom_cpufreq_kryo_driver_init);

MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
MODULE_LICENSE("GPL v2");

> -----Original Message-----
> From: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> Sent: Friday, May 18, 2018 04:46
> To: Ilia Lin <ilialin@xxxxxxxxxxxxxx>
> Cc: mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; robh@xxxxxxxxxx;
> mark.rutland@xxxxxxx; nm@xxxxxx; lgirdwood@xxxxxxxxx;
> broonie@xxxxxxxxxx; andy.gross@xxxxxxxxxx; david.brown@xxxxxxxxxx;
> catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; rjw@xxxxxxxxxxxxx; linux-
> clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; linux-arm-
> msm@xxxxxxxxxxxxxxx; linux-soc@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; rnayak@xxxxxxxxxxxxxx;
> amit.kucheria@xxxxxxxxxx; nicolas.dechesne@xxxxxxxxxx;
> celster@xxxxxxxxxxxxxx; tfinkel@xxxxxxxxxxxxxx
> Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver
>
> On 17-05-18, 14:19, Ilia Lin wrote:
> > +static int __init qcom_cpufreq_kryo_driver_init(void)
> > +{
> > + size_t len;
> > + int ret = 0;
> > + u32 versions;
> > + enum _msm8996_version msm8996_version;
> > + u8 *speedbin;
> > + struct device *cpu_dev_silver, *cpu_dev_gold;
> > + struct device_node *np;
> > + struct nvmem_cell *speedbin_nvmem;
> > + struct platform_device *pdev;
> > + struct opp_table *opp_silver = NULL;
> > + struct opp_table *opp_gold = NULL;
>
> No need to initialize them and you may want to arrange all above in
> decreasing order of their length.
>
> > +
> > + cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > + if (IS_ERR_OR_NULL(cpu_dev_silver))
> > + return PTR_ERR(cpu_dev_silver);
> > +
> > + cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > + if (IS_ERR_OR_NULL(cpu_dev_gold))
> > + return PTR_ERR(cpu_dev_gold);
> > +
> > + msm8996_version = qcom_cpufreq_kryo_get_msm_id();
> > + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> > + dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
> > + return -ENODEV;
> > + }
> > +
> > + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
> > + if (IS_ERR_OR_NULL(np))
> > + return PTR_ERR(np);
> > +
> > + if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
> > + ret = -ENOENT;
> > + goto free_np;
> > + }
> > +
> > + speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> > + if (IS_ERR(speedbin_nvmem)) {
> > + ret = PTR_ERR(speedbin_nvmem);
> > + dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
> ret);
> > + goto free_np;
> > + }
> > +
> > + speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> > + nvmem_cell_put(speedbin_nvmem);
> > +
> > + switch (msm8996_version) {
> > + case MSM8996_V3:
> > + versions = 1 << (unsigned int)(*speedbin);
> > + break;
> > + case MSM8996_SG:
> > + versions = 1 << ((unsigned int)(*speedbin) + 4);
> > + break;
> > + default:
> > + BUG();
> > + break;
> > + }
> > +
> > + opp_silver =
> dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
> > + if (IS_ERR_OR_NULL(opp_silver)) {
>
> This API doesn't return NULL and so IS_ERR() would be sufficient.
>
> > + dev_err(cpu_dev_silver, "Failed to set supported
> hardware\n");
> > + ret = PTR_ERR(opp_silver);
> > + goto free_np;
> > + }
> > +
> > + opp_gold =
> dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
> > + if (IS_ERR_OR_NULL(opp_gold)) {
>
> same here.
>
> > + dev_err(cpu_dev_gold, "Failed to set supported
> hardware\n");
> > + ret = PTR_ERR(opp_gold);
> > + goto free_opp_silver;
> > + }
> > +
> > + pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > + if (!IS_ERR_OR_NULL(pdev))
> > + goto out;
>
> Simply return from here and remove the useless label out.
>
> > +
> > + ret = PTR_ERR(pdev);
> > + dev_err(cpu_dev_silver, "Failed to register platform device\n");
> > + dev_pm_opp_put_supported_hw(opp_gold);
> > +
> > +free_opp_silver:
> > + dev_pm_opp_put_supported_hw(opp_silver);
> > +
> > +free_np:
> > + of_node_put(np);
> > +
> > +out:
> > + return ret;
> > +}
> > +late_initcall(qcom_cpufreq_kryo_driver_init);
>
> Please resend only this patch now or just paste the new code in a mail
here
> so that I can review it quickly and then you can resend the final version.
Most
> of the patches aren't changing anyway.
>
> --
> viresh

Next message: tip-bot for Sandipan Das: "[tip:perf/core] perf script: Show symbol offsets by default"
Previous message: tip-bot for Sandipan Das: "[tip:perf/core] perf script: Show virtual addresses instead of offsets"
In reply to: ilialin: "RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver"
Next in thread: Viresh Kumar: "Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]