| Message ID | 1526375616-16904-13-git-send-email-ilialin@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 15-05-18, 12:13, Ilia Lin wrote: > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, > the CPU ferequencies 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. > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > --- > drivers/cpufreq/Kconfig.arm | 11 +++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > drivers/cpufreq/qcom-cpufreq-kryo.c | 150 +++++++++++++++++++++++++++++++++++ > 4 files changed, 165 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Tuesday, May 15, 2018 11:19:12 AM CEST Viresh Kumar wrote: > On 15-05-18, 12:13, Ilia Lin wrote: > > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, > > the CPU ferequencies 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. > > > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > > --- > > drivers/cpufreq/Kconfig.arm | 11 +++ > > drivers/cpufreq/Makefile | 1 + > > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > > drivers/cpufreq/qcom-cpufreq-kryo.c | 150 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 165 insertions(+) > > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> OK, but I'm assuming that this will go in via arm-soc. Thanks, Rafael
Hi Ilia,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.17-rc5]
[cannot apply to clk/clk-next next-20180515]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ilia-Lin/CPU-scaling-support-for-msm8996/20180516-064721
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
aarch64-linux-gnu-ld: arch/arm64/kernel/head.o: relocation R_AARCH64_ABS32 against `_kernel_offset_le_lo32' can not be used when making a shared object
arch/arm64/kernel/head.o: In function `kimage_vaddr':
(.idmap.text+0x0): dangerous relocation: unsupported relocation
arch/arm64/kernel/head.o: In function `__primary_switch':
(.idmap.text+0x350): dangerous relocation: unsupported relocation
(.idmap.text+0x358): dangerous relocation: unsupported relocation
drivers/cpufreq/qcom-cpufreq-kryo.o: In function `qcom_cpufreq_kryo_driver_init':
>> qcom-cpufreq-kryo.c:(.init.text+0xd8): undefined reference to `qcom_smem_get'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, May 15, 2018 at 12:13 PM, Ilia Lin <ilialin@codeaurora.org> wrote: > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, > the CPU ferequencies subset and voltage value of each OPP varies s/ferequencies/frequency > 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. > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > --- > drivers/cpufreq/Kconfig.arm | 11 +++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > drivers/cpufreq/qcom-cpufreq-kryo.c | 150 +++++++++++++++++++++++++++++++++++ > 4 files changed, 165 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > index de55c7d..5c16f05 100644 > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -124,6 +124,17 @@ config ARM_OMAP2PLUS_CPUFREQ > depends on ARCH_OMAP2PLUS > default ARCH_OMAP2PLUS > > +config ARM_QCOM_CPUFREQ_KRYO > + bool "Qualcomm Technologies, Inc. Kryo based CPUFreq" "Qualcomm Kryo CPUFreq support" should be enough. Kconfig isn't the place for Trademark compliance :-) > + depends on QCOM_QFPROM > + depends on QCOM_SMEM > + select PM_OPP > + help > + This adds the CPUFreq driver for > + Qualcomm Technologies, Inc. Kryo SoC based boards. > + > + If in doubt, say N. > + > config ARM_S3C_CPUFREQ > bool > help > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > index 8d24ade..fb4a2ec 100644 > --- a/drivers/cpufreq/Makefile > +++ b/drivers/cpufreq/Makefile > @@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o > obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o > obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > +obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o > obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o > obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o > obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index 3b585e4..77d6ab8 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -118,6 +118,9 @@ > > { .compatible = "nvidia,tegra124", }, > > + { .compatible = "qcom,apq8096", }, > + { .compatible = "qcom,msm8996", }, > + > { .compatible = "st,stih407", }, > { .compatible = "st,stih410", }, > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c > new file mode 100644 > index 0000000..10d7236 > --- /dev/null > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > @@ -0,0 +1,150 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Stray space here. > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#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) > +{ > + size_t len; > + int ret; > + u32 versions; > + enum _msm8996_version msm8996_version; > + u8 *speedbin; > + struct device *cpu_dev; > + struct device_node *np; > + struct nvmem_cell *speedbin_nvmem; > + struct opp_table *opp_temp = NULL; > + > + cpu_dev = get_cpu_device(SILVER_LEAD); > + if (IS_ERR_OR_NULL(cpu_dev)) > + return PTR_ERR(cpu_dev); > + > + msm8996_version = qcom_cpufreq_kryo_get_msm_id(); > + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) { > + dev_err(cpu_dev, "Not Snapdragon 820/821!"); > + return -ENODEV; > + } Use tab instead of spaces. > + > + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev); > + 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, "Could not get nvmem cell: %d\n", ret); > + goto free_np; > + } > + > + speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + > + 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; > + } > + > + ret = PTR_ERR_OR_ZERO(opp_temp = > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > + if (0 > ret) Any particular reason to prefer this over (ret < 0) that is generally used? I've seen it used to avoid the == vs. = typos, but not for other comparisons. Suggest sticking to what is commonly used i.e. ret < 0. > + goto free_opp; > + > + cpu_dev = get_cpu_device(GOLD_LEAD); Error check cpu_dev here? > + ret = PTR_ERR_OR_ZERO(opp_temp = > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > + if (0 > ret) > + goto free_opp; > + > + > + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", > + -1, NULL, 0)); > + > + if (0 == ret) > + return 0; > + > +free_opp: > + dev_pm_opp_put_supported_hw(opp_temp); This is not needed because dev_pm_opp_set_supported_hw will free memory in case of failure. This call in only needed in case of a successful get. > + > +free_np: > + of_node_put(np); > + return ret; Suggest something like this instead: . . opp_temp = dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); if (IS_ERR(opp_temp)) { dev_err(cpu_dev, "Failed to set supported hardware\n"); ret = PTR_ERR(opp_temp); goto free_np; } cpu_dev = get_cpu_device(GOLD_LEAD); opp_temp = dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); if (IS_ERR(opp_temp)) { dev_err(cpu_dev, "Failed to set supported hardware\n"); ret = PTR_ERR(opp_temp); goto free_np; } ret = platform_device_register_simple("cpufreq-dt", -1, NULL, 0)); if (!IS_ERR_OR_NULL(ret)) goto out; free_np: of_node_put(np); out: return ret; > +} > +late_initcall(qcom_cpufreq_kryo_driver_init); > + > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver"); > +MODULE_LICENSE("GPL v2"); > -- > 1.9.1 >
On 16-05-18, 16:12, Amit Kucheria wrote: > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > + if (0 > ret) > > Any particular reason to prefer this over (ret < 0) that is generally > used? I've seen it used to avoid the == vs. = typos, but not for other > comparisons. > > Suggest sticking to what is commonly used i.e. ret < 0. > > > + goto free_opp; > > + > > + cpu_dev = get_cpu_device(GOLD_LEAD); > > Error check cpu_dev here? > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > + if (0 > ret) > > + goto free_opp; The goto here is wrong > > + > > + > > + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", > > + -1, NULL, 0)); > > + > > + if (0 == ret) > > + return 0; > > + > > +free_opp: > > + dev_pm_opp_put_supported_hw(opp_temp); > > This is not needed because dev_pm_opp_set_supported_hw will free > memory in case of failure. This call in only needed in case of a > successful get. But this is still required for the case where platform device registration fails.
On Wed, May 16, 2018 at 5:11 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 16-05-18, 16:12, Amit Kucheria wrote: >> > + ret = PTR_ERR_OR_ZERO(opp_temp = >> > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); >> > + if (0 > ret) >> >> Any particular reason to prefer this over (ret < 0) that is generally >> used? I've seen it used to avoid the == vs. = typos, but not for other >> comparisons. >> >> Suggest sticking to what is commonly used i.e. ret < 0. >> >> > + goto free_opp; >> > + >> > + cpu_dev = get_cpu_device(GOLD_LEAD); >> >> Error check cpu_dev here? >> >> > + ret = PTR_ERR_OR_ZERO(opp_temp = >> > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); >> > + if (0 > ret) >> > + goto free_opp; > > The goto here is wrong > >> > + >> > + >> > + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", >> > + -1, NULL, 0)); >> > + >> > + if (0 == ret) >> > + return 0; >> > + >> > +free_opp: >> > + dev_pm_opp_put_supported_hw(opp_temp); >> >> This is not needed because dev_pm_opp_set_supported_hw will free >> memory in case of failure. This call in only needed in case of a >> successful get. > > But this is still required for the case where platform device > registration fails. Agreed. The overall error path needs to be re-written with proper check of return values.
> -----Original Message----- > From: amit.kucheria@verdurent.com <amit.kucheria@verdurent.com> On > Behalf Of Amit Kucheria > Sent: Wednesday, May 16, 2018 16:13 > To: Ilia Lin <ilialin@codeaurora.org> > Cc: Michael Turquette <mturquette@baylibre.com>; sboyd@kernel.org; Rob > Herring <robh@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Viresh > Kumar <viresh.kumar@linaro.org>; nm@ti.com; lgirdwood@gmail.com; > broonie@kernel.org; Andy Gross <andy.gross@linaro.org>; David Brown > <david.brown@linaro.org>; catalin.marinas@arm.com; > will.deacon@arm.com; Rafael J. Wysocki <rjw@rjwysocki.net>; linux- > clk@vger.kernel.org; devicetree@vger.kernel.org; LKML <linux- > kernel@vger.kernel.org>; Linux PM list <linux-pm@vger.kernel.org>; linux- > arm-msm@vger.kernel.org; linux-soc@vger.kernel.org; lakml <linux-arm- > kernel@lists.infradead.org>; Rajendra Nayak <rnayak@codeaurora.org>; > nicolas.dechesne@linaro.org; celster@codeaurora.org; > tfinkel@codeaurora.org > Subject: Re: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver > > On Tue, May 15, 2018 at 12:13 PM, Ilia Lin <ilialin@codeaurora.org> wrote: > > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO > > processors, the CPU ferequencies subset and voltage value of each OPP > > varies > > s/ferequencies/frequency > > > 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. > > > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > > --- > > drivers/cpufreq/Kconfig.arm | 11 +++ > > drivers/cpufreq/Makefile | 1 + > > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > > drivers/cpufreq/qcom-cpufreq-kryo.c | 150 > > +++++++++++++++++++++++++++++++++++ > > 4 files changed, 165 insertions(+) > > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c > > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > index de55c7d..5c16f05 100644 > > --- a/drivers/cpufreq/Kconfig.arm > > +++ b/drivers/cpufreq/Kconfig.arm > > @@ -124,6 +124,17 @@ config ARM_OMAP2PLUS_CPUFREQ > > depends on ARCH_OMAP2PLUS > > default ARCH_OMAP2PLUS > > > > +config ARM_QCOM_CPUFREQ_KRYO > > + bool "Qualcomm Technologies, Inc. Kryo based CPUFreq" > > "Qualcomm Kryo CPUFreq support" should be enough. Kconfig isn't the place > for Trademark compliance :-) This is mandatory requirement of the QTIs legal. > > > + depends on QCOM_QFPROM > > + depends on QCOM_SMEM > > + select PM_OPP > > + help > > + This adds the CPUFreq driver for > > + Qualcomm Technologies, Inc. Kryo SoC based boards. > > + > > + If in doubt, say N. > > + > > config ARM_S3C_CPUFREQ > > bool > > help > > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index > > 8d24ade..fb4a2ec 100644 > > --- a/drivers/cpufreq/Makefile > > +++ b/drivers/cpufreq/Makefile > > @@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu- > cpufreq.o > > obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o > > obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o > > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > > +obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o > > obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o > > obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o > > obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c > > b/drivers/cpufreq/cpufreq-dt-platdev.c > > index 3b585e4..77d6ab8 100644 > > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > > @@ -118,6 +118,9 @@ > > > > { .compatible = "nvidia,tegra124", }, > > > > + { .compatible = "qcom,apq8096", }, > > + { .compatible = "qcom,msm8996", }, > > + > > { .compatible = "st,stih407", }, > > { .compatible = "st,stih410", }, > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c > > b/drivers/cpufreq/qcom-cpufreq-kryo.c > > new file mode 100644 > > index 0000000..10d7236 > > --- /dev/null > > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > > @@ -0,0 +1,150 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > Stray space here. > > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or > > + modify > > + * it under the terms of the GNU General Public License version 2 and > > + * only version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#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) > > +{ > > + size_t len; > > + int ret; > > + u32 versions; > > + enum _msm8996_version msm8996_version; > > + u8 *speedbin; > > + struct device *cpu_dev; > > + struct device_node *np; > > + struct nvmem_cell *speedbin_nvmem; > > + struct opp_table *opp_temp = NULL; > > + > > + cpu_dev = get_cpu_device(SILVER_LEAD); > > + if (IS_ERR_OR_NULL(cpu_dev)) > > + return PTR_ERR(cpu_dev); > > + > > + msm8996_version = qcom_cpufreq_kryo_get_msm_id(); > > + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) { > > + dev_err(cpu_dev, "Not Snapdragon 820/821!"); > > + return -ENODEV; > > + } > > Use tab instead of spaces. > > > + > > + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev); > > + 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, "Could not get nvmem cell: %d\n", ret); > > + goto free_np; > > + } > > + > > + speedbin = nvmem_cell_read(speedbin_nvmem, &len); > > + > > + 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; > > + } > > + > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > + if (0 > ret) > > Any particular reason to prefer this over (ret < 0) that is generally used? I've > seen it used to avoid the == vs. = typos, but not for other comparisons. > > Suggest sticking to what is commonly used i.e. ret < 0. > > > + goto free_opp; > > + > > + cpu_dev = get_cpu_device(GOLD_LEAD); > > Error check cpu_dev here? > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > + if (0 > ret) > > + goto free_opp; > > + > > + > > + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq- > dt", > > + -1, > > + NULL, 0)); > > + > > + if (0 == ret) > > + return 0; > > + > > +free_opp: > > + dev_pm_opp_put_supported_hw(opp_temp); > > This is not needed because dev_pm_opp_set_supported_hw will free > memory in case of failure. This call in only needed in case of a successful get. > > > + > > +free_np: > > + of_node_put(np); > > + return ret; > > > Suggest something like this instead: > > . > . > > opp_temp = dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > if (IS_ERR(opp_temp)) { > dev_err(cpu_dev, "Failed to set supported hardware\n"); > ret = PTR_ERR(opp_temp); > goto free_np; > } > > cpu_dev = get_cpu_device(GOLD_LEAD); > > opp_temp = dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > if (IS_ERR(opp_temp)) { > dev_err(cpu_dev, "Failed to set supported hardware\n"); > ret = PTR_ERR(opp_temp); > goto free_np; > } > > ret = platform_device_register_simple("cpufreq-dt", -1, NULL, 0)); > > if (!IS_ERR_OR_NULL(ret)) > goto out; > > free_np: > of_node_put(np); > out: > return ret; > > > +} > > +late_initcall(qcom_cpufreq_kryo_driver_init); > > + > > +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq > > +driver"); MODULE_LICENSE("GPL v2"); > > -- > > 1.9.1 > >
> -----Original Message----- > From: Viresh Kumar <viresh.kumar@linaro.org> > Sent: Wednesday, May 16, 2018 17:12 > To: Amit Kucheria <amit.kucheria@linaro.org> > Cc: Ilia Lin <ilialin@codeaurora.org>; Michael Turquette > <mturquette@baylibre.com>; sboyd@kernel.org; Rob Herring > <robh@kernel.org>; Mark Rutland <mark.rutland@arm.com>; nm@ti.com; > lgirdwood@gmail.com; broonie@kernel.org; Andy Gross > <andy.gross@linaro.org>; David Brown <david.brown@linaro.org>; > catalin.marinas@arm.com; will.deacon@arm.com; Rafael J. Wysocki > <rjw@rjwysocki.net>; linux-clk@vger.kernel.org; > devicetree@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Linux > PM list <linux-pm@vger.kernel.org>; linux-arm-msm@vger.kernel.org; linux- > soc@vger.kernel.org; lakml <linux-arm-kernel@lists.infradead.org>; > Rajendra Nayak <rnayak@codeaurora.org>; nicolas.dechesne@linaro.org; > celster@codeaurora.org; tfinkel@codeaurora.org > Subject: Re: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver > > On 16-05-18, 16:12, Amit Kucheria wrote: > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > > + > dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > > + if (0 > ret) > > > > Any particular reason to prefer this over (ret < 0) that is generally > > used? I've seen it used to avoid the == vs. = typos, but not for other > > comparisons. > > > > Suggest sticking to what is commonly used i.e. ret < 0. > > > > > + goto free_opp; > > > + > > > + cpu_dev = get_cpu_device(GOLD_LEAD); > > > > Error check cpu_dev here? > > > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > > + > dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > > + if (0 > ret) > > > + goto free_opp; > > The goto here is wrong If we are here, then the first dev_pm_opp_set_supported_hw() succeeded. And should be deallocated before exit with error. > > > > + > > > + > > > + ret = > PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", > > > + -1, > > > + NULL, 0)); > > > + > > > + if (0 == ret) > > > + return 0; > > > + > > > +free_opp: > > > + dev_pm_opp_put_supported_hw(opp_temp); > > > > This is not needed because dev_pm_opp_set_supported_hw will free > > memory in case of failure. This call in only needed in case of a > > successful get. > > But this is still required for the case where platform device registration fails. > > -- > viresh
> -----Original Message----- > From: ilialin@codeaurora.org <ilialin@codeaurora.org> > Sent: Thursday, May 17, 2018 10:51 > To: 'Viresh Kumar' <viresh.kumar@linaro.org>; 'Amit Kucheria' > <amit.kucheria@linaro.org> > Cc: 'Michael Turquette' <mturquette@baylibre.com>; 'sboyd@kernel.org' > <sboyd@kernel.org>; 'Rob Herring' <robh@kernel.org>; 'Mark Rutland' > <mark.rutland@arm.com>; 'nm@ti.com' <nm@ti.com>; > 'lgirdwood@gmail.com' <lgirdwood@gmail.com>; 'broonie@kernel.org' > <broonie@kernel.org>; 'Andy Gross' <andy.gross@linaro.org>; 'David Brown' > <david.brown@linaro.org>; 'catalin.marinas@arm.com' > <catalin.marinas@arm.com>; 'will.deacon@arm.com' > <will.deacon@arm.com>; 'Rafael J. Wysocki' <rjw@rjwysocki.net>; 'linux- > clk@vger.kernel.org' <linux-clk@vger.kernel.org>; > 'devicetree@vger.kernel.org' <devicetree@vger.kernel.org>; 'LKML' <linux- > kernel@vger.kernel.org>; 'Linux PM list' <linux-pm@vger.kernel.org>; 'linux- > arm-msm@vger.kernel.org' <linux-arm-msm@vger.kernel.org>; 'linux- > soc@vger.kernel.org' <linux-soc@vger.kernel.org>; 'lakml' <linux-arm- > kernel@lists.infradead.org>; 'Rajendra Nayak' <rnayak@codeaurora.org>; > 'nicolas.dechesne@linaro.org' <nicolas.dechesne@linaro.org>; > 'celster@codeaurora.org' <celster@codeaurora.org>; > 'tfinkel@codeaurora.org' <tfinkel@codeaurora.org> > Subject: RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver > > > > > -----Original Message----- > > From: Viresh Kumar <viresh.kumar@linaro.org> > > Sent: Wednesday, May 16, 2018 17:12 > > To: Amit Kucheria <amit.kucheria@linaro.org> > > Cc: Ilia Lin <ilialin@codeaurora.org>; Michael Turquette > > <mturquette@baylibre.com>; sboyd@kernel.org; Rob Herring > > <robh@kernel.org>; Mark Rutland <mark.rutland@arm.com>; nm@ti.com; > > lgirdwood@gmail.com; broonie@kernel.org; Andy Gross > > <andy.gross@linaro.org>; David Brown <david.brown@linaro.org>; > > catalin.marinas@arm.com; will.deacon@arm.com; Rafael J. Wysocki > > <rjw@rjwysocki.net>; linux-clk@vger.kernel.org; > > devicetree@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Linux > > PM list <linux-pm@vger.kernel.org>; linux-arm-msm@vger.kernel.org; > > linux- soc@vger.kernel.org; lakml > > <linux-arm-kernel@lists.infradead.org>; > > Rajendra Nayak <rnayak@codeaurora.org>; nicolas.dechesne@linaro.org; > > celster@codeaurora.org; tfinkel@codeaurora.org > > Subject: Re: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver > > > > On 16-05-18, 16:12, Amit Kucheria wrote: > > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > > > + > > dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > > > + if (0 > ret) > > > > > > Any particular reason to prefer this over (ret < 0) that is > > > generally used? I've seen it used to avoid the == vs. = typos, but > > > not for other comparisons. > > > > > > Suggest sticking to what is commonly used i.e. ret < 0. > > > > > > > + goto free_opp; > > > > + > > > > + cpu_dev = get_cpu_device(GOLD_LEAD); > > > > > > Error check cpu_dev here? > > > > > > > + ret = PTR_ERR_OR_ZERO(opp_temp = > > > > + > > dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); > > > > + if (0 > ret) > > > > + goto free_opp; > > > > The goto here is wrong > > If we are here, then the first dev_pm_opp_set_supported_hw() succeeded. > And should be deallocated before exit with error. My bad. Got you. > > > > > > > + > > > > + > > > > + ret = > > PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", > > > > + -1, > > > > + NULL, 0)); > > > > + > > > > + if (0 == ret) > > > > + return 0; > > > > + > > > > +free_opp: > > > > + dev_pm_opp_put_supported_hw(opp_temp); > > > > > > This is not needed because dev_pm_opp_set_supported_hw will free > > > memory in case of failure. This call in only needed in case of a > > > successful get. > > > > But this is still required for the case where platform device registration fails. > > > > -- > > viresh
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index de55c7d..5c16f05 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -124,6 +124,17 @@ config ARM_OMAP2PLUS_CPUFREQ depends on ARCH_OMAP2PLUS default ARCH_OMAP2PLUS +config ARM_QCOM_CPUFREQ_KRYO + bool "Qualcomm Technologies, Inc. Kryo based CPUFreq" + depends on QCOM_QFPROM + depends on QCOM_SMEM + select PM_OPP + help + This adds the CPUFreq driver for + Qualcomm Technologies, Inc. Kryo SoC based boards. + + If in doubt, say N. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 8d24ade..fb4a2ec 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o +obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index 3b585e4..77d6ab8 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -118,6 +118,9 @@ { .compatible = "nvidia,tegra124", }, + { .compatible = "qcom,apq8096", }, + { .compatible = "qcom,msm8996", }, + { .compatible = "st,stih407", }, { .compatible = "st,stih410", }, diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c new file mode 100644 index 0000000..10d7236 --- /dev/null +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#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) +{ + size_t len; + int ret; + u32 versions; + enum _msm8996_version msm8996_version; + u8 *speedbin; + struct device *cpu_dev; + struct device_node *np; + struct nvmem_cell *speedbin_nvmem; + struct opp_table *opp_temp = NULL; + + cpu_dev = get_cpu_device(SILVER_LEAD); + if (IS_ERR_OR_NULL(cpu_dev)) + return PTR_ERR(cpu_dev); + + msm8996_version = qcom_cpufreq_kryo_get_msm_id(); + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) { + dev_err(cpu_dev, "Not Snapdragon 820/821!"); + return -ENODEV; + } + + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev); + 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, "Could not get nvmem cell: %d\n", ret); + goto free_np; + } + + speedbin = nvmem_cell_read(speedbin_nvmem, &len); + + 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; + } + + ret = PTR_ERR_OR_ZERO(opp_temp = + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); + if (0 > ret) + goto free_opp; + + cpu_dev = get_cpu_device(GOLD_LEAD); + ret = PTR_ERR_OR_ZERO(opp_temp = + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); + if (0 > ret) + goto free_opp; + + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", + -1, NULL, 0)); + + if (0 == ret) + return 0; + +free_opp: + dev_pm_opp_put_supported_hw(opp_temp); + +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");
In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU ferequencies 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. Signed-off-by: Ilia Lin <ilialin@codeaurora.org> --- drivers/cpufreq/Kconfig.arm | 11 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/cpufreq-dt-platdev.c | 3 + drivers/cpufreq/qcom-cpufreq-kryo.c | 150 +++++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c