| Message ID | 1526729701-8589-1-git-send-email-ilialin@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
More comments after Russell's reply. On 19-05-18, 14:35, Ilia Lin wrote: > +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)) get_cpu_device() returns only NULL on error. > + 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)) same here. > + 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); and this only returns ERR_PTR() on error. > + 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"); > -- > 1.9.1
On 19/05/18 12:35, Ilia Lin wrote: > 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. > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/Kconfig.arm | 10 +++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > drivers/cpufreq/qcom-cpufreq-kryo.c | 164 +++++++++++++++++++++++++++++++++++ > 4 files changed, 178 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c > [..] > + > +/* > + * 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 > + So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus. [...] > + > +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); s/SILVER/GOLD/ ?
> -----Original Message----- > From: Sudeep Holla <sudeep.holla@arm.com> > Sent: Monday, May 21, 2018 15:50 > To: Ilia Lin <ilialin@codeaurora.org>; mturquette@baylibre.com; > sboyd@kernel.org; robh@kernel.org; mark.rutland@arm.com; > viresh.kumar@linaro.org; nm@ti.com; lgirdwood@gmail.com; > broonie@kernel.org; andy.gross@linaro.org; david.brown@linaro.org; > catalin.marinas@arm.com; will.deacon@arm.com; rjw@rjwysocki.net; linux- > clk@vger.kernel.org > Cc: Sudeep Holla <sudeep.holla@arm.com>; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; linux-arm- > msm@vger.kernel.org; linux-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; rnayak@codeaurora.org; > amit.kucheria@linaro.org; nicolas.dechesne@linaro.org; > celster@codeaurora.org; tfinkel@codeaurora.org > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver > > > > On 19/05/18 12:35, Ilia Lin wrote: > > 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. > > > > Signed-off-by: Ilia Lin <ilialin@codeaurora.org> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > drivers/cpufreq/Kconfig.arm | 10 +++ > > drivers/cpufreq/Makefile | 1 + > > drivers/cpufreq/cpufreq-dt-platdev.c | 3 + > > drivers/cpufreq/qcom-cpufreq-kryo.c | 164 > > +++++++++++++++++++++++++++++++++++ > > 4 files changed, 178 insertions(+) > > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c > > > > [..] > > > + > > +/* > > + * 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 > > + > > So I gather form other emails, that these are physical cpu number(not even > unique identifier like MPIDR). Will this work on parts or platforms that need > to boot in GOLD LEAD cpus. The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) always boots on the CPU0. > > [...] > > > + > > +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); > > s/SILVER/GOLD/ ? Yes, you are right. This is already fixed in the respin. > > -- > Regards, > Sudeep
On 21/05/18 13:57, ilialin@codeaurora.org wrote: > [...] >>> +#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 >>> + >> >> So I gather form other emails, that these are physical cpu number(not even >> unique identifier like MPIDR). Will this work on parts or platforms that need >> to boot in GOLD LEAD cpus. > > The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) > always boots on the CPU0. That may be true and I am not that bothered about it. But assuming physical ordering from the logical cpu number is *incorrect* and will break if kernel decides to change the allocation algorithm. Kernel provides no guarantee on that, so you need to depend on some physical ID or may be DT to achieve what your want. But the current code as it stands is wrong.
> -----Original Message----- > From: Sudeep Holla <sudeep.holla@arm.com> > Sent: Monday, May 21, 2018 16:05 > To: ilialin@codeaurora.org; mturquette@baylibre.com; sboyd@kernel.org; > robh@kernel.org; mark.rutland@arm.com; viresh.kumar@linaro.org; > nm@ti.com; lgirdwood@gmail.com; broonie@kernel.org; > andy.gross@linaro.org; david.brown@linaro.org; catalin.marinas@arm.com; > will.deacon@arm.com; rjw@rjwysocki.net; linux-clk@vger.kernel.org > Cc: Sudeep Holla <sudeep.holla@arm.com>; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; linux-arm- > msm@vger.kernel.org; linux-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; rnayak@codeaurora.org; > amit.kucheria@linaro.org; nicolas.dechesne@linaro.org; > celster@codeaurora.org; tfinkel@codeaurora.org > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver > > > > On 21/05/18 13:57, ilialin@codeaurora.org wrote: > > > [...] > > >>> +#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 > >>> + > >> > >> So I gather form other emails, that these are physical cpu number(not > >> even unique identifier like MPIDR). Will this work on parts or > >> platforms that need to boot in GOLD LEAD cpus. > > > > The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) > > always boots on the CPU0. > > > That may be true and I am not that bothered about it. But assuming physical > ordering from the logical cpu number is *incorrect* and will break if kernel > decides to change the allocation algorithm. Kernel provides no guarantee on > that, so you need to depend on some physical ID or may be DT to achieve > what your want. But the current code as it stands is wrong. Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in the DT: cpus { #address-cells = <2>; #size-cells = <0>; CPU0: cpu@0 { ... reg = <0x0 0x0>; ... }; CPU1: cpu@1 { ... reg = <0x0 0x1>; ... }; CPU2: cpu@100 { ... reg = <0x0 0x100>; ... }; CPU3: cpu@101 { ... reg = <0x0 0x101>; ... }; cpu-map { cluster0 { core0 { cpu = <&CPU0>; }; core1 { cpu = <&CPU1>; }; }; cluster1 { core0 { cpu = <&CPU2>; }; core1 { cpu = <&CPU3>; }; }; }; }; As far, as I understand, they are probed in the same order. However, to be certain that the physical CPU is the one I intend to configure, I have to fetch the device structure pointer for the cpu-map -> clusterX -> core0 -> cpu path. Could you suggest a kernel API to do that? > > -- > Regards, > Sudeep
OK, I think I found out the way. Would this be correct?
-----------------------------------------------------------------------------------------------
extern struct cpu_topology cpu_topology[NR_CPUS];
static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster)
{
unsigned cpu;
for_each_possible_cpu(cpu) {
if ((cluster == cpu_topology[cpu].cluster_id) &&
(0 == cpu_topology[cpu].core_id))
return get_cpu_device(cpu);
}
return NULL;
}
-----------------------------------------------------------------------------------------------
> -----Original Message-----
> From: ilialin@codeaurora.org <ilialin@codeaurora.org>
> Sent: Tuesday, May 22, 2018 09:56
> To: 'Sudeep Holla' <sudeep.holla@arm.com>; 'mturquette@baylibre.com'
> <mturquette@baylibre.com>; 'sboyd@kernel.org' <sboyd@kernel.org>;
> 'robh@kernel.org' <robh@kernel.org>; 'mark.rutland@arm.com'
> <mark.rutland@arm.com>; 'viresh.kumar@linaro.org'
> <viresh.kumar@linaro.org>; 'nm@ti.com' <nm@ti.com>;
> 'lgirdwood@gmail.com' <lgirdwood@gmail.com>; 'broonie@kernel.org'
> <broonie@kernel.org>; 'andy.gross@linaro.org' <andy.gross@linaro.org>;
> 'david.brown@linaro.org' <david.brown@linaro.org>;
> 'catalin.marinas@arm.com' <catalin.marinas@arm.com>;
> 'will.deacon@arm.com' <will.deacon@arm.com>; 'rjw@rjwysocki.net'
> <rjw@rjwysocki.net>; 'linux-clk@vger.kernel.org' <linux-
> clk@vger.kernel.org>
> Cc: 'devicetree@vger.kernel.org' <devicetree@vger.kernel.org>; 'linux-
> kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; 'linux-
> pm@vger.kernel.org' <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>; 'linux-arm-
> kernel@lists.infradead.org' <linux-arm-kernel@lists.infradead.org>;
> 'rnayak@codeaurora.org' <rnayak@codeaurora.org>;
> 'amit.kucheria@linaro.org' <amit.kucheria@linaro.org>;
> 'nicolas.dechesne@linaro.org' <nicolas.dechesne@linaro.org>;
> 'celster@codeaurora.org' <celster@codeaurora.org>;
> 'tfinkel@codeaurora.org' <tfinkel@codeaurora.org>
> Subject: RE: [PATCH] cpufreq: Add Kryo CPU scaling driver
>
>
>
> > -----Original Message-----
> > From: Sudeep Holla <sudeep.holla@arm.com>
> > Sent: Monday, May 21, 2018 16:05
> > To: ilialin@codeaurora.org; mturquette@baylibre.com; sboyd@kernel.org;
> > robh@kernel.org; mark.rutland@arm.com; viresh.kumar@linaro.org;
> > nm@ti.com; lgirdwood@gmail.com; broonie@kernel.org;
> > andy.gross@linaro.org; david.brown@linaro.org;
> > catalin.marinas@arm.com; will.deacon@arm.com; rjw@rjwysocki.net;
> > linux-clk@vger.kernel.org
> > Cc: Sudeep Holla <sudeep.holla@arm.com>; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-pm@vger.kernel.org; linux-arm-
> > msm@vger.kernel.org; linux-soc@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; rnayak@codeaurora.org;
> > amit.kucheria@linaro.org; nicolas.dechesne@linaro.org;
> > celster@codeaurora.org; tfinkel@codeaurora.org
> > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> >
> >
> >
> > On 21/05/18 13:57, ilialin@codeaurora.org wrote:
> > >
> > [...]
> >
> > >>> +#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
> > >>> +
> > >>
> > >> So I gather form other emails, that these are physical cpu
> > >> number(not even unique identifier like MPIDR). Will this work on
> > >> parts or platforms that need to boot in GOLD LEAD cpus.
> > >
> > > The driver is for Kryo CPU, which (and AFAIK all multicore MSMs)
> > > always boots on the CPU0.
> >
> >
> > That may be true and I am not that bothered about it. But assuming
> > physical ordering from the logical cpu number is *incorrect* and will
> > break if kernel decides to change the allocation algorithm. Kernel
> > provides no guarantee on that, so you need to depend on some physical
> > ID or may be DT to achieve what your want. But the current code as it
> stands is wrong.
>
> Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in
> the DT:
>
> cpus {
> #address-cells = <2>;
> #size-cells = <0>;
>
> CPU0: cpu@0 {
> ...
> reg = <0x0 0x0>;
> ...
> };
>
> CPU1: cpu@1 {
> ...
> reg = <0x0 0x1>;
> ...
> };
>
> CPU2: cpu@100 {
> ...
> reg = <0x0 0x100>;
> ...
> };
>
> CPU3: cpu@101 {
> ...
> reg = <0x0 0x101>;
> ...
> };
>
> cpu-map {
> cluster0 {
> core0 {
> cpu = <&CPU0>;
> };
>
> core1 {
> cpu = <&CPU1>;
> };
> };
>
> cluster1 {
> core0 {
> cpu = <&CPU2>;
> };
>
> core1 {
> cpu = <&CPU3>;
> };
> };
> };
> };
>
> As far, as I understand, they are probed in the same order. However, to be
> certain that the physical CPU is the one I intend to configure, I have to fetch
> the device structure pointer for the cpu-map -> clusterX -> core0 -> cpu path.
> Could you suggest a kernel API to do that?
>
>
>
> >
> > --
> > Regards,
> > Sudeep
On 22/05/18 07:56, ilialin@codeaurora.org wrote: > > >> -----Original Message----- >> From: Sudeep Holla <sudeep.holla@arm.com> >> Sent: Monday, May 21, 2018 16:05 [...] >> >> >> That may be true and I am not that bothered about it. But assuming physical >> ordering from the logical cpu number is *incorrect* and will break if kernel >> decides to change the allocation algorithm. Kernel provides no guarantee on >> that, so you need to depend on some physical ID or may be DT to achieve >> what your want. But the current code as it stands is wrong. > > Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in the DT: > > cpus { > #address-cells = <2>; > #size-cells = <0>; > > CPU0: cpu@0 { > ... > reg = <0x0 0x0>; > ... > }; > > CPU1: cpu@1 { > ... > reg = <0x0 0x1>; > ... > }; > > CPU2: cpu@100 { > ... > reg = <0x0 0x100>; > ... > }; > > CPU3: cpu@101 { > ... > reg = <0x0 0x101>; > ... > }; > > cpu-map { > cluster0 { > core0 { > cpu = <&CPU0>; > }; > > core1 { > cpu = <&CPU1>; > }; > }; > > cluster1 { > core0 { > cpu = <&CPU2>; > }; > > core1 { > cpu = <&CPU3>; > }; > }; > }; > }; > > As far, as I understand, they are probed in the same order. Yes that's correct today, will that have to remain same for ever ? No it's not user ABI and kernel can decide to change the allocation order. What if for some reason one/more CPUs fails to boot or even configured not to boot ? > However, to be certain that the physical CPU is the one I intend to > configure, I have to fetch the device structure pointer for the cpu-map -> > clusterX -> core0 -> cpu path. Could you suggest a kernel API to do > that? > Let's rewind a bit. Do you supply OPPs only on CPU0 and CPU2 ? If yes, that's again wrong. Simple solution is to parse all logical CPUs and skip if the share OPPs with some other CPUs. I think that logic already exists in OPP library IIRC.
On 22/05/18 08:59, ilialin@codeaurora.org wrote: > OK, I think I found out the way. Would this be correct? No. > ----------------------------------------------------------------------------------------------- > extern struct cpu_topology cpu_topology[NR_CPUS]; > > static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster) > { > unsigned cpu; > > for_each_possible_cpu(cpu) { > if ((cluster == cpu_topology[cpu].cluster_id) && cluster_id is going away soon, so avoid relying on that. IIUC there's a way already something like opp_of_get_shared_cpus.
On 22-05-18, 10:59, ilialin@codeaurora.org wrote: > OK, I think I found out the way. Would this be correct? > ----------------------------------------------------------------------------------------------- > extern struct cpu_topology cpu_topology[NR_CPUS]; > > static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster) > { > unsigned cpu; > > for_each_possible_cpu(cpu) { > if ((cluster == cpu_topology[cpu].cluster_id) && > (0 == cpu_topology[cpu].core_id)) > return get_cpu_device(cpu); > } > > return NULL; > } Okay, this is what you should do IMHO. for_each_possible_cpu(cpu) { cpu_dev = xxx.. ret = dev_pm_opp_set_supported_hw(cpu_dev, xxx, xxx); if (ret && ret != -EBUSY) error-out. } This would require a trivial patch for the OPP core to not throw an error message with -EBUSY. I can do that separately.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index de55c7d..0bfd40e 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ depends on ARCH_OMAP2PLUS default ARCH_OMAP2PLUS +config ARM_QCOM_CPUFREQ_KRYO + bool "Qualcomm Kryo based CPUFreq" + depends on QCOM_QFPROM + depends on QCOM_SMEM + select PM_OPP + help + This adds the CPUFreq driver for Qualcomm 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..ae2d1b9 --- /dev/null +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c @@ -0,0 +1,164 @@ +// 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");