RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

OK, got you.
Is "operating-points-v2-kryo-cpu" good enough?

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 17:48
> To: ilia...@codeaurora.org
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> vire...@kernel.org; n...@ti.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 15:10, ilia...@codeaurora.org wrote:
> > Thank you for the explanation. However, could you suggest, which
> > condition should I check then? Device tree?
> >
> 
> Yes some compatible which is applicable for all the SoCs or platforms on
> which this driver can work ?
> 
> --
> Regards,
> Sudeep

RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

OK, got you.
Is "operating-points-v2-kryo-cpu" good enough?

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 17:48
> To: ilia...@codeaurora.org
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> vire...@kernel.org; n...@ti.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 15:10, ilia...@codeaurora.org wrote:
> > Thank you for the explanation. However, could you suggest, which
> > condition should I check then? Device tree?
> >
> 
> Yes some compatible which is applicable for all the SoCs or platforms on
> which this driver can work ?
> 
> --
> Regards,
> Sudeep

RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

Thank you for the explanation. However, could you suggest, which condition 
should I check then? Device tree?

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 17:01
> To: ilia...@codeaurora.org; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 14:03, ilia...@codeaurora.org wrote:
> >
> >
> 
> [...]
> 
> >>> +
> >>> + ret = PTR_ERR_OR_ZERO(platform_device_register_simple(
> >>> + "qcom-cpufreq-kryo", -1, NULL, 0));
> >>
> >>
> >> You simply can't do this unconditionally here. This will blow up on
> >> platforms where this driver is not supposed to work. The probe will
> >> be called on non- QCOM or non-Kryo QCOM platforms and I reckon it
> >> will crash trying to execute something in qcom_smem_get.
> >
> > What do you mean by 'unconditionally'?
> 
> Why should you even add/register a device "qcom-cpufreq-kryo" on other
> platforms. Drivers can get registered, but only devices that are present or
> required by the platform need to be registered.
> 
> > The driver depends on the smem and nvmem drivers, which depend on
> ARCH_QCOM:
> >  +  depends on QCOM_QFPROM
> >  +  depends on QCOM_SMEM
> >
> 
> Sure, but we have something called single image for all ARM64 platforms.
> May be QCOM still used to tweeking config to build binary for your particular
> mobile platform but the distro kernel need single binary to work on all
> platforms. We have moved far away from platform specific builds long back
> now IIRC.
> 
> > And if SMEM read in the probe returns something other than Kryo, it will
> exit.
> >
> 
> Check what this driver does ?
> 
>   msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY,
> MSM_ID_SMEM, );
>   msm_id++;
>   switch ((enum _msm_id)*msm_id)
> 
> I think it *will and should* crash here ? You need to check the return value
> for sure. But since qcom_smem_get return -EPROBE_DEFER, we keep
> retrying even on non QCOM platforms which is something I would like to
> avoid.
> 
> Therefore that's not the main concern. Why do I have to see "qcom-cpufreq-
> kryo" device registered on my non QCOM platform ?
> 
> --
> Regards,
> Sudeep

RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

Thank you for the explanation. However, could you suggest, which condition 
should I check then? Device tree?

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 17:01
> To: ilia...@codeaurora.org; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 14:03, ilia...@codeaurora.org wrote:
> >
> >
> 
> [...]
> 
> >>> +
> >>> + ret = PTR_ERR_OR_ZERO(platform_device_register_simple(
> >>> + "qcom-cpufreq-kryo", -1, NULL, 0));
> >>
> >>
> >> You simply can't do this unconditionally here. This will blow up on
> >> platforms where this driver is not supposed to work. The probe will
> >> be called on non- QCOM or non-Kryo QCOM platforms and I reckon it
> >> will crash trying to execute something in qcom_smem_get.
> >
> > What do you mean by 'unconditionally'?
> 
> Why should you even add/register a device "qcom-cpufreq-kryo" on other
> platforms. Drivers can get registered, but only devices that are present or
> required by the platform need to be registered.
> 
> > The driver depends on the smem and nvmem drivers, which depend on
> ARCH_QCOM:
> >  +  depends on QCOM_QFPROM
> >  +  depends on QCOM_SMEM
> >
> 
> Sure, but we have something called single image for all ARM64 platforms.
> May be QCOM still used to tweeking config to build binary for your particular
> mobile platform but the distro kernel need single binary to work on all
> platforms. We have moved far away from platform specific builds long back
> now IIRC.
> 
> > And if SMEM read in the probe returns something other than Kryo, it will
> exit.
> >
> 
> Check what this driver does ?
> 
>   msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY,
> MSM_ID_SMEM, );
>   msm_id++;
>   switch ((enum _msm_id)*msm_id)
> 
> I think it *will and should* crash here ? You need to check the return value
> for sure. But since qcom_smem_get return -EPROBE_DEFER, we keep
> retrying even on non QCOM platforms which is something I would like to
> avoid.
> 
> Therefore that's not the main concern. Why do I have to see "qcom-cpufreq-
> kryo" device registered on my non QCOM platform ?
> 
> --
> Regards,
> Sudeep

RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 15:52
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> Hi Ilia,
> 
> 
> On 24/05/18 09:57, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  10 ++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 194
> > +++
> >  4 files changed, 208 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > 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 000..9fe379c
> > --- /dev/null
> > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > @@ -0,0 +1,194 @@
> > +// 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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define MSM_ID_SMEM137
> > +
> > +enum _msm_id {
> > +   MSM8996V3 = 0xF6ul,
> > +   APQ8096V3 = 0x123ul,
> > +   MSM8996SG = 0x131ul,
> > +   APQ8096SG = 0x138ul,
> > +};
> > +
> > +enum _msm8996_version {
> > +   MSM8996_V3,
> > +   MSM8996_SG,
> > +   NUM_OF_MSM8996_VERSIONS,
> > +};
> > +
> > +static enum 

RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Thursday, May 24, 2018 15:52
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net
> Cc: Sudeep Holla ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> Hi Ilia,
> 
> 
> On 24/05/18 09:57, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  10 ++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 194
> > +++
> >  4 files changed, 208 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > 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 000..9fe379c
> > --- /dev/null
> > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > @@ -0,0 +1,194 @@
> > +// 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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define MSM_ID_SMEM137
> > +
> > +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 

RE: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Wednesday, May 23, 2018 16:25
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net; linux...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Sudeep Holla 
> Subject: Re: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 23/05/18 13:38, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  10 ++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 181
> > +++
> >  4 files changed, 195 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > 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.
> > +
> 
> Sorry but just noticed now, any reason why this can't be module. I can't
> imagine any.

I was asked previously to change this from tristate to bool.

> 
> [..]
> 
> > +static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) {
> > +   struct opp_table *opp_tables[NR_CPUS] = {0};
> > +   struct platform_device *cpufreq_dt_pdev;
> > +   enum _msm8996_version msm8996_version;
> > +   struct nvmem_cell *speedbin_nvmem;
> > +   struct device_node *np;
> > +   struct device *cpu_dev;
> 
> [..]
> 
> > +
> > +   cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -
> 1, NULL, 0);
> > +   if (!IS_ERR(cpufreq_dt_pdev))
> > +   return 0;
> > +
> > +   ret = PTR_ERR(cpufreq_dt_pdev);
> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> > +
> > +free_opp:
> > +   for_each_possible_cpu(cpu) {
> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> > +   break;
> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> > +static int __init qcom_cpufreq_kryo_init(void) {
> > +   /*
> > +* Since the driver depends on smem and nvmem drivers, which may
> > +* return EPROBE_DEFER, all the real activity is done in the probe,
> > +* which may be defered as well. The init here is only registering
> > +* a platform device.
> > +*/
> > +   platform_device_register_simple("qcom-cpufreq-kryo", -1, NULL, 0);
> > +   return 0;
> > +}
> > +module_init(qcom_cpufreq_kryo_init);
> 
> Do you need this at all ? See below on how to eliminate the need for this.
> 
> > +
> > +static struct platform_driver qcom_cpufreq_kryo_driver = {
> > +   .probe = qcom_cpufreq_kryo_probe,
> > +   .driver = {
> > +   .name = "qcom-cpufreq-kryo",
> > +   },
> > +};
> > +builtin_platform_driver(qcom_cpufreq_kryo_driver);
> 
> Use builtin_platform_driver_probe and remove qcom_cpufreq_kryo_init or
> use module_platform_driver_probe if it can be module.
> 
> --
> Regards,
> Sudeep

RE: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Wednesday, May 23, 2018 16:25
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net; linux...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Sudeep Holla 
> Subject: Re: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 23/05/18 13:38, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  10 ++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 181
> > +++
> >  4 files changed, 195 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > 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.
> > +
> 
> Sorry but just noticed now, any reason why this can't be module. I can't
> imagine any.

I was asked previously to change this from tristate to bool.

> 
> [..]
> 
> > +static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) {
> > +   struct opp_table *opp_tables[NR_CPUS] = {0};
> > +   struct platform_device *cpufreq_dt_pdev;
> > +   enum _msm8996_version msm8996_version;
> > +   struct nvmem_cell *speedbin_nvmem;
> > +   struct device_node *np;
> > +   struct device *cpu_dev;
> 
> [..]
> 
> > +
> > +   cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -
> 1, NULL, 0);
> > +   if (!IS_ERR(cpufreq_dt_pdev))
> > +   return 0;
> > +
> > +   ret = PTR_ERR(cpufreq_dt_pdev);
> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> > +
> > +free_opp:
> > +   for_each_possible_cpu(cpu) {
> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> > +   break;
> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> > +static int __init qcom_cpufreq_kryo_init(void) {
> > +   /*
> > +* Since the driver depends on smem and nvmem drivers, which may
> > +* return EPROBE_DEFER, all the real activity is done in the probe,
> > +* which may be defered as well. The init here is only registering
> > +* a platform device.
> > +*/
> > +   platform_device_register_simple("qcom-cpufreq-kryo", -1, NULL, 0);
> > +   return 0;
> > +}
> > +module_init(qcom_cpufreq_kryo_init);
> 
> Do you need this at all ? See below on how to eliminate the need for this.
> 
> > +
> > +static struct platform_driver qcom_cpufreq_kryo_driver = {
> > +   .probe = qcom_cpufreq_kryo_probe,
> > +   .driver = {
> > +   .name = "qcom-cpufreq-kryo",
> > +   },
> > +};
> > +builtin_platform_driver(qcom_cpufreq_kryo_driver);
> 
> Use builtin_platform_driver_probe and remove qcom_cpufreq_kryo_init or
> use module_platform_driver_probe if it can be module.
> 
> --
> Regards,
> Sudeep

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

The nvmem will return EPROBE_DEFER, and so will my driver's init. But then no 
one will call the init again.

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 23, 2018 14:46
> To: Ilia Lin 
> Cc: Sudeep Holla ; Viresh Kumar
> ; Nishanth Menon ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; Linux Kernel Mailing List
> 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> On 23 May 2018 at 17:04,   wrote:
> > You are right. I already checked that in the code...
> > However, with module_init() the driver fails on reading the nvmem.
> 
> And why is it failing ?

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

The nvmem will return EPROBE_DEFER, and so will my driver's init. But then no 
one will call the init again.

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 23, 2018 14:46
> To: Ilia Lin 
> Cc: Sudeep Holla ; Viresh Kumar
> ; Nishanth Menon ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; Linux Kernel Mailing List
> 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> On 23 May 2018 at 17:04,   wrote:
> > You are right. I already checked that in the code...
> > However, with module_init() the driver fails on reading the nvmem.
> 
> And why is it failing ?

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 23, 2018 14:31
> To: Ilia Lin 
> Cc: Sudeep Holla ; Viresh Kumar
> ; Nishanth Menon ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; Linux Kernel Mailing List
> 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> On 23 May 2018 at 16:36,   wrote:
> >
> >
> >> -Original Message-
> >> From: Sudeep Holla 
> >> Sent: Wednesday, May 23, 2018 13:40
> >> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> >> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> >> r...@rjwysocki.net; linux...@vger.kernel.org;
> >> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Cc: Sudeep Holla 
> >> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> >>
> >>
> >>
> >> On 23/05/18 10:40, 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 
> >>
> >> [...]
> >>
> >> > +   pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> >> > +   if (!IS_ERR(pdev))
> >> > +   return 0;
> >> > +
> >> > +   ret = PTR_ERR(pdev);
> >> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> >> > +
> >> > +free_opp:
> >> > +   for_each_possible_cpu(cpu) {
> >> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> >> > +   break;
> >> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> >> > +   }
> >> > +free_np:
> >> > +   of_node_put(np);
> >> > +
> >> > +   return ret;
> >> > +}
> >> > +late_initcall(qcom_cpufreq_kryo_driver_init);
> >>
> >> Any particular reason why this *has* to be late initcall ?
> >> Please change it to module_initcall otherwise.
> >
> > The purpose is to give the cpufreq-dt the time to register the driver and
> only then my driver will add the platform device.
> 
> That isn't required, the device and its driver can be registered in any order.

You are right. I already checked that in the code...
However, with module_init() the driver fails on reading the nvmem.

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 23, 2018 14:31
> To: Ilia Lin 
> Cc: Sudeep Holla ; Viresh Kumar
> ; Nishanth Menon ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; Linux Kernel Mailing List
> 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> On 23 May 2018 at 16:36,   wrote:
> >
> >
> >> -Original Message-
> >> From: Sudeep Holla 
> >> Sent: Wednesday, May 23, 2018 13:40
> >> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> >> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> >> r...@rjwysocki.net; linux...@vger.kernel.org;
> >> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Cc: Sudeep Holla 
> >> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> >>
> >>
> >>
> >> On 23/05/18 10:40, 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 
> >>
> >> [...]
> >>
> >> > +   pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> >> > +   if (!IS_ERR(pdev))
> >> > +   return 0;
> >> > +
> >> > +   ret = PTR_ERR(pdev);
> >> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> >> > +
> >> > +free_opp:
> >> > +   for_each_possible_cpu(cpu) {
> >> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> >> > +   break;
> >> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> >> > +   }
> >> > +free_np:
> >> > +   of_node_put(np);
> >> > +
> >> > +   return ret;
> >> > +}
> >> > +late_initcall(qcom_cpufreq_kryo_driver_init);
> >>
> >> Any particular reason why this *has* to be late initcall ?
> >> Please change it to module_initcall otherwise.
> >
> > The purpose is to give the cpufreq-dt the time to register the driver and
> only then my driver will add the platform device.
> 
> That isn't required, the device and its driver can be registered in any order.

You are right. I already checked that in the code...
However, with module_init() the driver fails on reading the nvmem.

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Wednesday, May 23, 2018 13:40
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net; linux...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Sudeep Holla 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 23/05/18 10:40, 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 
> 
> [...]
> 
> > +   pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > +   if (!IS_ERR(pdev))
> > +   return 0;
> > +
> > +   ret = PTR_ERR(pdev);
> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> > +
> > +free_opp:
> > +   for_each_possible_cpu(cpu) {
> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> > +   break;
> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> > +   }
> > +free_np:
> > +   of_node_put(np);
> > +
> > +   return ret;
> > +}
> > +late_initcall(qcom_cpufreq_kryo_driver_init);
> 
> Any particular reason why this *has* to be late initcall ?
> Please change it to module_initcall otherwise.

The purpose is to give the cpufreq-dt the time to register the driver and only 
then my driver will add the platform device.

> Also address the of_node comments from Viresh.
> 
> Otherwise, it looks good.
> --
> Regards,
> Sudeep

RE: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Wednesday, May 23, 2018 13:40
> To: Ilia Lin ; vire...@kernel.org; n...@ti.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> r...@rjwysocki.net; linux...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Sudeep Holla 
> Subject: Re: [PATCH v10 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 23/05/18 10:40, 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 
> 
> [...]
> 
> > +   pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > +   if (!IS_ERR(pdev))
> > +   return 0;
> > +
> > +   ret = PTR_ERR(pdev);
> > +   dev_err(cpu_dev, "Failed to register platform device\n");
> > +
> > +free_opp:
> > +   for_each_possible_cpu(cpu) {
> > +   if (IS_ERR_OR_NULL(opp_tables[cpu]))
> > +   break;
> > +   dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> > +   }
> > +free_np:
> > +   of_node_put(np);
> > +
> > +   return ret;
> > +}
> > +late_initcall(qcom_cpufreq_kryo_driver_init);
> 
> Any particular reason why this *has* to be late initcall ?
> Please change it to module_initcall otherwise.

The purpose is to give the cpufreq-dt the time to register the driver and only 
then my driver will add the platform device.

> Also address the of_node comments from Viresh.
> 
> Otherwise, it looks good.
> --
> Regards,
> Sudeep

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

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: ilia...@codeaurora.org 
> Sent: Tuesday, May 22, 2018 09:56
> To: 'Sudeep Holla' ; 'mturque...@baylibre.com'
> ; 'sb...@kernel.org' ;
> 'r...@kernel.org' ; 'mark.rutl...@arm.com'
> ; 'viresh.ku...@linaro.org'
> ; 'n...@ti.com' ;
> 'lgirdw...@gmail.com' ; 'broo...@kernel.org'
> ; 'andy.gr...@linaro.org' ;
> 'david.br...@linaro.org' ;
> 'catalin.mari...@arm.com' ;
> 'will.dea...@arm.com' ; 'r...@rjwysocki.net'
> ; 'linux-...@vger.kernel.org'  c...@vger.kernel.org>
> Cc: 'devicet...@vger.kernel.org' ; 'linux-
> ker...@vger.kernel.org' ; 'linux-
> p...@vger.kernel.org' ; 'linux-arm-
> m...@vger.kernel.org' ; 'linux-
> s...@vger.kernel.org' ; 'linux-arm-
> ker...@lists.infradead.org' ;
> 'rna...@codeaurora.org' ;
> 'amit.kuche...@linaro.org' ;
> 'nicolas.deche...@linaro.org' ;
> 'cels...@codeaurora.org' ;
> 'tfin...@codeaurora.org' 
> Subject: RE: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> > -Original Message-
> > From: Sudeep Holla 
> > Sent: Monday, May 21, 2018 16:05
> > To: ilia...@codeaurora.org; mturque...@baylibre.com; sb...@kernel.org;
> > r...@kernel.org; mark.rutl...@arm.com; viresh.ku...@linaro.org;
> > n...@ti.com; lgirdw...@gmail.com; broo...@kernel.org;
> > andy.gr...@linaro.org; david.br...@linaro.org;
> > catalin.mari...@arm.com; will.dea...@arm.com; r...@rjwysocki.net;
> > linux-...@vger.kernel.org
> > Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> > m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; rna...@codeaurora.org;
> > amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> > cels...@codeaurora.org; tfin...@codeaurora.org
> > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> >
> >
> >
> > On 21/05/18 13:57, ilia...@codeaurora.org wrote:
> > >
> > [...]
> >
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include  #include  #include
> > >>> + #include  #include
> > >>> + #include 
> > >>> +
> > >>> +#define MSM_ID_SMEM137
> > >>> +#define SILVER_LEAD0
> > >>> +#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 = <>;
>   };
> 
>   

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

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: ilia...@codeaurora.org 
> Sent: Tuesday, May 22, 2018 09:56
> To: 'Sudeep Holla' ; 'mturque...@baylibre.com'
> ; 'sb...@kernel.org' ;
> 'r...@kernel.org' ; 'mark.rutl...@arm.com'
> ; 'viresh.ku...@linaro.org'
> ; 'n...@ti.com' ;
> 'lgirdw...@gmail.com' ; 'broo...@kernel.org'
> ; 'andy.gr...@linaro.org' ;
> 'david.br...@linaro.org' ;
> 'catalin.mari...@arm.com' ;
> 'will.dea...@arm.com' ; 'r...@rjwysocki.net'
> ; 'linux-...@vger.kernel.org'  c...@vger.kernel.org>
> Cc: 'devicet...@vger.kernel.org' ; 'linux-
> ker...@vger.kernel.org' ; 'linux-
> p...@vger.kernel.org' ; 'linux-arm-
> m...@vger.kernel.org' ; 'linux-
> s...@vger.kernel.org' ; 'linux-arm-
> ker...@lists.infradead.org' ;
> 'rna...@codeaurora.org' ;
> 'amit.kuche...@linaro.org' ;
> 'nicolas.deche...@linaro.org' ;
> 'cels...@codeaurora.org' ;
> 'tfin...@codeaurora.org' 
> Subject: RE: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> > -Original Message-
> > From: Sudeep Holla 
> > Sent: Monday, May 21, 2018 16:05
> > To: ilia...@codeaurora.org; mturque...@baylibre.com; sb...@kernel.org;
> > r...@kernel.org; mark.rutl...@arm.com; viresh.ku...@linaro.org;
> > n...@ti.com; lgirdw...@gmail.com; broo...@kernel.org;
> > andy.gr...@linaro.org; david.br...@linaro.org;
> > catalin.mari...@arm.com; will.dea...@arm.com; r...@rjwysocki.net;
> > linux-...@vger.kernel.org
> > Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> > m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; rna...@codeaurora.org;
> > amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> > cels...@codeaurora.org; tfin...@codeaurora.org
> > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> >
> >
> >
> > On 21/05/18 13:57, ilia...@codeaurora.org wrote:
> > >
> > [...]
> >
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include 
> > >>> +#include  #include  #include
> > >>> + #include  #include
> > >>> + #include 
> > >>> +
> > >>> +#define MSM_ID_SMEM137
> > >>> +#define SILVER_LEAD0
> > >>> +#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 = <>;
>   };
> 
>   core1 {
>   cpu = <>;
>   };
>   };
> 
>   cluster1 {
>   core0 {
>   cpu = <>;
>   };
> 
>   core1 {
>   cpu = <>;
>   };
>   };
>   };
> };
> 
> 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?
> 
> 
> 
> >
> > 

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Monday, May 21, 2018 16:05
> To: ilia...@codeaurora.org; mturque...@baylibre.com; sb...@kernel.org;
> r...@kernel.org; mark.rutl...@arm.com; viresh.ku...@linaro.org;
> n...@ti.com; lgirdw...@gmail.com; broo...@kernel.org;
> andy.gr...@linaro.org; david.br...@linaro.org; catalin.mari...@arm.com;
> will.dea...@arm.com; r...@rjwysocki.net; linux-...@vger.kernel.org
> Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; rna...@codeaurora.org;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@codeaurora.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 21/05/18 13:57, ilia...@codeaurora.org wrote:
> >
> [...]
> 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +
> >>> +#define MSM_ID_SMEM  137
> >>> +#define SILVER_LEAD  0
> >>> +#define GOLD_LEAD2
> >>> +
> >>
> >> 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 = <>;
};

core1 {
cpu = <>;
};
};

cluster1 {
core0 {
cpu = <>;
};

core1 {
cpu = <>;
};
};
};
};

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

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Monday, May 21, 2018 16:05
> To: ilia...@codeaurora.org; mturque...@baylibre.com; sb...@kernel.org;
> r...@kernel.org; mark.rutl...@arm.com; viresh.ku...@linaro.org;
> n...@ti.com; lgirdw...@gmail.com; broo...@kernel.org;
> andy.gr...@linaro.org; david.br...@linaro.org; catalin.mari...@arm.com;
> will.dea...@arm.com; r...@rjwysocki.net; linux-...@vger.kernel.org
> Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; rna...@codeaurora.org;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@codeaurora.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 21/05/18 13:57, ilia...@codeaurora.org wrote:
> >
> [...]
> 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +
> >>> +#define MSM_ID_SMEM  137
> >>> +#define SILVER_LEAD  0
> >>> +#define GOLD_LEAD2
> >>> +
> >>
> >> 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 = <>;
};

core1 {
cpu = <>;
};
};

cluster1 {
core0 {
cpu = <>;
};

core1 {
cpu = <>;
};
};
};
};

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

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Monday, May 21, 2018 15:50
> To: Ilia Lin ; mturque...@baylibre.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> viresh.ku...@linaro.org; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; r...@rjwysocki.net; linux-
> c...@vger.kernel.org
> Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; rna...@codeaurora.org;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@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 
> > Acked-by: Viresh Kumar 
> > ---
> >  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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define MSM_ID_SMEM137
> > +#define SILVER_LEAD0
> > +#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

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Sudeep Holla 
> Sent: Monday, May 21, 2018 15:50
> To: Ilia Lin ; mturque...@baylibre.com;
> sb...@kernel.org; r...@kernel.org; mark.rutl...@arm.com;
> viresh.ku...@linaro.org; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; r...@rjwysocki.net; linux-
> c...@vger.kernel.org
> Cc: Sudeep Holla ; devicet...@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; rna...@codeaurora.org;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@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 
> > Acked-by: Viresh Kumar 
> > ---
> >  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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define MSM_ID_SMEM137
> > +#define SILVER_LEAD0
> > +#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

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

There are 2 CPU clusters in the Kryo, CPU 0 and 1 are called Silver Cluster
and CPU 2 and 3 - Gold Cluster. Each cluster has single clock. The clusters
differ in terms of speed capabilities, computing power and power
consumption. Therefore, I define separate OPP table for each cluster, and my
driver will choose the appropriate OPP subset for each cluster.
Lead refers to first CPU in the cluster.

> -Original Message-
> From: Russell King - ARM Linux 
> Sent: Monday, May 21, 2018 15:12
> To: ilia...@codeaurora.org
> Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> On Mon, May 21, 2018 at 02:05:41PM +0300, ilia...@codeaurora.org wrote:
> > You are right.
> > cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well.
> > Thank you.
> >
> > > -Original Message-
> > > From: Russell King - ARM Linux 
> > > Sent: Monday, May 21, 2018 13:54
> > > To: Ilia Lin 
> > > Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> > > p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> > > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> > > c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> > > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> > >
> > > On Mon, May 21, 2018 at 01:31:30PM +0300, Ilia Lin wrote:
> > > > +#define SILVER_LEAD0
> > > > +#define GOLD_LEAD  2
> > >
> > > Okay, two different values here, but "GOLD_LEAD" appears unused.
> > >
> > > > +   cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > > > +   if (NULL == cpu_dev_silver)
> > > > +   return -ENODEV;
> > > > +
> > > > +   cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > > > +   if (NULL == cpu_dev_gold)
> > > > +   return -ENODEV;
> > >
> > > get_cpu_device() takes the logical CPU number.  So the above gets
> > > CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here.  So
> > > what's the point of the second get_cpu_device() ?  If it's supposed to
be:
> > >
> > >   cpu_dev_gold = get_cpu_device(GOLD_LEAD);
> > >
> > > That would get CPU 2, but in terms of these defines, it doesn't make
> > > that much sense.  What exactly does "silver lead" and "gold lead"
> > > refer to in
> > these
> > > definitions?
> 
> I think you still need to explain this.
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps
> up According to speedtest.net: 8.21Mbps down 510kbps up

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

There are 2 CPU clusters in the Kryo, CPU 0 and 1 are called Silver Cluster
and CPU 2 and 3 - Gold Cluster. Each cluster has single clock. The clusters
differ in terms of speed capabilities, computing power and power
consumption. Therefore, I define separate OPP table for each cluster, and my
driver will choose the appropriate OPP subset for each cluster.
Lead refers to first CPU in the cluster.

> -Original Message-
> From: Russell King - ARM Linux 
> Sent: Monday, May 21, 2018 15:12
> To: ilia...@codeaurora.org
> Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> On Mon, May 21, 2018 at 02:05:41PM +0300, ilia...@codeaurora.org wrote:
> > You are right.
> > cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well.
> > Thank you.
> >
> > > -Original Message-
> > > From: Russell King - ARM Linux 
> > > Sent: Monday, May 21, 2018 13:54
> > > To: Ilia Lin 
> > > Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> > > p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> > > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> > > c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> > > Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> > >
> > > On Mon, May 21, 2018 at 01:31:30PM +0300, Ilia Lin wrote:
> > > > +#define SILVER_LEAD0
> > > > +#define GOLD_LEAD  2
> > >
> > > Okay, two different values here, but "GOLD_LEAD" appears unused.
> > >
> > > > +   cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > > > +   if (NULL == cpu_dev_silver)
> > > > +   return -ENODEV;
> > > > +
> > > > +   cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > > > +   if (NULL == cpu_dev_gold)
> > > > +   return -ENODEV;
> > >
> > > get_cpu_device() takes the logical CPU number.  So the above gets
> > > CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here.  So
> > > what's the point of the second get_cpu_device() ?  If it's supposed to
be:
> > >
> > >   cpu_dev_gold = get_cpu_device(GOLD_LEAD);
> > >
> > > That would get CPU 2, but in terms of these defines, it doesn't make
> > > that much sense.  What exactly does "silver lead" and "gold lead"
> > > refer to in
> > these
> > > definitions?
> 
> I think you still need to explain this.
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps
> up According to speedtest.net: 8.21Mbps down 510kbps up

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well.
Thank you.

> -Original Message-
> From: Russell King - ARM Linux 
> Sent: Monday, May 21, 2018 13:54
> To: Ilia Lin 
> Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> On Mon, May 21, 2018 at 01:31:30PM +0300, Ilia Lin wrote:
> > +#define SILVER_LEAD0
> > +#define GOLD_LEAD  2
> 
> Okay, two different values here, but "GOLD_LEAD" appears unused.
> 
> > +   cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > +   if (NULL == cpu_dev_silver)
> > +   return -ENODEV;
> > +
> > +   cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > +   if (NULL == cpu_dev_gold)
> > +   return -ENODEV;
> 
> get_cpu_device() takes the logical CPU number.  So the above gets CPU 0
> each time, and so cpu_dev_silver == cpu_dev_gold here.  So what's the
> point of the second get_cpu_device() ?  If it's supposed to be:
> 
>   cpu_dev_gold = get_cpu_device(GOLD_LEAD);
> 
> That would get CPU 2, but in terms of these defines, it doesn't make that
> much sense.  What exactly does "silver lead" and "gold lead" refer to in
these
> definitions?
> 
> > +   opp_silver =
> dev_pm_opp_set_supported_hw(cpu_dev_silver,,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,,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;
> > +   }
> 
> Given that cpu_dev_silver == cpu_dev_gold, doesn't the second call to
> dev_pm_opp_set_supported_hw() always fail, as opp_table-
> >supported_hw will be set by the first call?
> 
> To me, this driver looks completely useless as it will always fail to
initialise,
> and I question whether this code has even been runtime tested.
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps
> up According to speedtest.net: 8.21Mbps down 510kbps up

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

[ilialin]

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well.
Thank you.

> -Original Message-
> From: Russell King - ARM Linux 
> Sent: Monday, May 21, 2018 13:54
> To: Ilia Lin 
> Cc: viresh.ku...@linaro.org; devicet...@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> c...@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
> 
> On Mon, May 21, 2018 at 01:31:30PM +0300, Ilia Lin wrote:
> > +#define SILVER_LEAD0
> > +#define GOLD_LEAD  2
> 
> Okay, two different values here, but "GOLD_LEAD" appears unused.
> 
> > +   cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > +   if (NULL == cpu_dev_silver)
> > +   return -ENODEV;
> > +
> > +   cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > +   if (NULL == cpu_dev_gold)
> > +   return -ENODEV;
> 
> get_cpu_device() takes the logical CPU number.  So the above gets CPU 0
> each time, and so cpu_dev_silver == cpu_dev_gold here.  So what's the
> point of the second get_cpu_device() ?  If it's supposed to be:
> 
>   cpu_dev_gold = get_cpu_device(GOLD_LEAD);
> 
> That would get CPU 2, but in terms of these defines, it doesn't make that
> much sense.  What exactly does "silver lead" and "gold lead" refer to in
these
> definitions?
> 
> > +   opp_silver =
> dev_pm_opp_set_supported_hw(cpu_dev_silver,,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,,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;
> > +   }
> 
> Given that cpu_dev_silver == cpu_dev_gold, doesn't the second call to
> dev_pm_opp_set_supported_hw() always fail, as opp_table-
> >supported_hw will be set by the first call?
> 
> To me, this driver looks completely useless as it will always fail to
initialise,
> and I question whether this code has even been runtime tested.
> 
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps
> up According to speedtest.net: 8.21Mbps down 510kbps up

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

[ilialin]

Final version (addressing Russel's comment as well):




// 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 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#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, );
/* 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 (NULL == cpu_dev_silver)
return -ENODEV;

cpu_dev_gold = get_cpu_device(SILVER_LEAD);
if (NULL == cpu_dev_gold)
return -ENODEV;

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(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, );
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,,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,,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(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 
> Sent: Monday, May 21, 2018 07:50
> To: ilia...@codeaurora.org
> Cc: mturque...@baylibre.com; 

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

[ilialin]

Final version (addressing Russel's comment as well):




// 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 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#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, );
/* 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 (NULL == cpu_dev_silver)
return -ENODEV;

cpu_dev_gold = get_cpu_device(SILVER_LEAD);
if (NULL == cpu_dev_gold)
return -ENODEV;

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(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, );
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,,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,,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(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 
> Sent: Monday, May 21, 2018 07:50
> To: ilia...@codeaurora.org
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> 

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

[ilialin]

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 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#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, );
/* 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, );
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,,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,,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");

> 

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

[ilialin]

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 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#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, );
/* 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, );
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,,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,,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");

> 

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

[ilialin]

>From c5804e1d17578a63ca87cc8fd839bf756cfe3567 Mon Sep 17 00:00:00 2001
In-Reply-To: <1526555955-29960-11-git-send-email-ilia...@codeaurora.org>
References: <1526555955-29960-11-git-send-email-ilia...@codeaurora.org>
From: Ilia Lin 
Date: Thu, 17 May 2018 13:55:12 +0300
Subject: [PATCH] cpufreq: Add Kryo CPU scaling driver

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 
Acked-by: Viresh Kumar 
---
 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

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 000..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MSM_ID_SMEM137
+#define SILVER_LEAD0
+#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, );
+   /* 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 

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

[ilialin]

>From c5804e1d17578a63ca87cc8fd839bf756cfe3567 Mon Sep 17 00:00:00 2001
In-Reply-To: <1526555955-29960-11-git-send-email-ilia...@codeaurora.org>
References: <1526555955-29960-11-git-send-email-ilia...@codeaurora.org>
From: Ilia Lin 
Date: Thu, 17 May 2018 13:55:12 +0300
Subject: [PATCH] cpufreq: Add Kryo CPU scaling driver

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 
Acked-by: Viresh Kumar 
---
 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

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 000..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MSM_ID_SMEM137
+#define SILVER_LEAD0
+#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, );
+   /* 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;
+   

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

[ilialin]

commit 4abe2cd7176a43c77e9a462e4f6ec51aa7552e73
Author: Ilia Lin 
Date:   Thu May 17 13:55:12 2018 +0300

cpufreq: Add Kryo CPU scaling driver

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 
Acked-by: Viresh Kumar 

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 000..b024b23
--- /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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MSM_ID_SMEM137
+#define SILVER_LEAD0
+#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, );
+   /* 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 

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

[ilialin]

commit 4abe2cd7176a43c77e9a462e4f6ec51aa7552e73
Author: Ilia Lin 
Date:   Thu May 17 13:55:12 2018 +0300

cpufreq: Add Kryo CPU scaling driver

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 
Acked-by: Viresh Kumar 

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 000..b024b23
--- /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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MSM_ID_SMEM137
+#define SILVER_LEAD0
+#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, );
+   /* 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;
+ 

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: ilia...@codeaurora.org 
> Sent: Thursday, May 17, 2018 10:51
> To: 'Viresh Kumar' ; 'Amit Kucheria'
> 
> Cc: 'Michael Turquette' ; 'sb...@kernel.org'
> ; 'Rob Herring' ; 'Mark Rutland'
> ; 'n...@ti.com' ;
> 'lgirdw...@gmail.com' ; 'broo...@kernel.org'
> ; 'Andy Gross' ; 'David Brown'
> ; 'catalin.mari...@arm.com'
> ; 'will.dea...@arm.com'
> ; 'Rafael J. Wysocki' ; 'linux-
> c...@vger.kernel.org' ;
> 'devicet...@vger.kernel.org' ; 'LKML'  ker...@vger.kernel.org>; 'Linux PM list' ;
'linux-
> arm-...@vger.kernel.org' ; 'linux-
> s...@vger.kernel.org' ; 'lakml'  ker...@lists.infradead.org>; 'Rajendra Nayak' ;
> 'nicolas.deche...@linaro.org' ;
> 'cels...@codeaurora.org' ;
> 'tfin...@codeaurora.org' 
> Subject: RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> > -Original Message-
> > From: Viresh Kumar 
> > Sent: Wednesday, May 16, 2018 17:12
> > To: Amit Kucheria 
> > Cc: Ilia Lin ; Michael Turquette
> > ; sb...@kernel.org; Rob Herring
> > ; Mark Rutland ; n...@ti.com;
> > lgirdw...@gmail.com; broo...@kernel.org; Andy Gross
> > ; David Brown ;
> > catalin.mari...@arm.com; will.dea...@arm.com; Rafael J. Wysocki
> > ; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; LKML ; Linux
> > PM list ; linux-arm-...@vger.kernel.org;
> > linux- s...@vger.kernel.org; lakml
> > ;
> > Rajendra Nayak ; nicolas.deche...@linaro.org;
> > cels...@codeaurora.org; tfin...@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,,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,,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

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: ilia...@codeaurora.org 
> Sent: Thursday, May 17, 2018 10:51
> To: 'Viresh Kumar' ; 'Amit Kucheria'
> 
> Cc: 'Michael Turquette' ; 'sb...@kernel.org'
> ; 'Rob Herring' ; 'Mark Rutland'
> ; 'n...@ti.com' ;
> 'lgirdw...@gmail.com' ; 'broo...@kernel.org'
> ; 'Andy Gross' ; 'David Brown'
> ; 'catalin.mari...@arm.com'
> ; 'will.dea...@arm.com'
> ; 'Rafael J. Wysocki' ; 'linux-
> c...@vger.kernel.org' ;
> 'devicet...@vger.kernel.org' ; 'LKML'  ker...@vger.kernel.org>; 'Linux PM list' ;
'linux-
> arm-...@vger.kernel.org' ; 'linux-
> s...@vger.kernel.org' ; 'lakml'  ker...@lists.infradead.org>; 'Rajendra Nayak' ;
> 'nicolas.deche...@linaro.org' ;
> 'cels...@codeaurora.org' ;
> 'tfin...@codeaurora.org' 
> Subject: RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> > -Original Message-
> > From: Viresh Kumar 
> > Sent: Wednesday, May 16, 2018 17:12
> > To: Amit Kucheria 
> > Cc: Ilia Lin ; Michael Turquette
> > ; sb...@kernel.org; Rob Herring
> > ; Mark Rutland ; n...@ti.com;
> > lgirdw...@gmail.com; broo...@kernel.org; Andy Gross
> > ; David Brown ;
> > catalin.mari...@arm.com; will.dea...@arm.com; Rafael J. Wysocki
> > ; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; LKML ; Linux
> > PM list ; linux-arm-...@vger.kernel.org;
> > linux- s...@vger.kernel.org; lakml
> > ;
> > Rajendra Nayak ; nicolas.deche...@linaro.org;
> > cels...@codeaurora.org; tfin...@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,,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,,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

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 16, 2018 17:12
> To: Amit Kucheria 
> Cc: Ilia Lin ; Michael Turquette
> ; sb...@kernel.org; Rob Herring
> ; Mark Rutland ; n...@ti.com;
> lgirdw...@gmail.com; broo...@kernel.org; Andy Gross
> ; David Brown ;
> catalin.mari...@arm.com; will.dea...@arm.com; Rafael J. Wysocki
> ; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; LKML ; Linux
> PM list ; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; lakml ;
> Rajendra Nayak ; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@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,,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,,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

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Wednesday, May 16, 2018 17:12
> To: Amit Kucheria 
> Cc: Ilia Lin ; Michael Turquette
> ; sb...@kernel.org; Rob Herring
> ; Mark Rutland ; n...@ti.com;
> lgirdw...@gmail.com; broo...@kernel.org; Andy Gross
> ; David Brown ;
> catalin.mari...@arm.com; will.dea...@arm.com; Rafael J. Wysocki
> ; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; LKML ; Linux
> PM list ; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; lakml ;
> Rajendra Nayak ; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@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,,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,,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

RE: [PATCH v7 10/14] dt-bindings: qcom_spmi: Add support for SAW documentation

[ilialin]

> -Original Message-
> From: amit.kuche...@verdurent.com  On
> Behalf Of Amit Kucheria
> Sent: Wednesday, May 16, 2018 16:13
> To: Ilia Lin 
> Cc: Michael Turquette ; sb...@kernel.org; Rob
> Herring ; Mark Rutland ; Viresh
> Kumar ; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; Andy Gross ; David Brown
> ; catalin.mari...@arm.com;
> will.dea...@arm.com; Rafael J. Wysocki ; linux-
> c...@vger.kernel.org; devicet...@vger.kernel.org; LKML  ker...@vger.kernel.org>; Linux PM list ; linux-
> arm-...@vger.kernel.org; linux-...@vger.kernel.org; lakml  ker...@lists.infradead.org>; Rajendra Nayak ;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v7 10/14] dt-bindings: qcom_spmi: Add support for SAW
> documentation
> 
> On Tue, May 15, 2018 at 12:13 PM, Ilia Lin  wrote:
> > Add support for SAW controlled regulators.
> > The regulators defined as SAW controlled in the device tree will be
> > controlled through special CPU registers instead of direct SPMI
> > accesses.
> > This is required especially for CPU supply regulators to synchronize
> > with clock scaling and for Automatic Voltage Switching.
> > Document it.
> 
> Replace this boiler plate with what this patch actual does. Besides changing
> the subject, it could be, for example,
> 
> "Document the DT bindings for the SAW regulators.
> 
> The saw-slave property allows ganging (grouping) of several regulators so
> that their outputs can be combined... blah blah.
> 
> The saw-leader is the only one that then is configurable in DT"

Actually, I invested some fantasy to write this explanation. But I'll try to 
revise it.

> 
> 
> > Signed-off-by: Ilia Lin 
> > Reviewed-by: Rob Herring 
> > ---
> >  .../bindings/regulator/qcom,spmi-regulator.txt | 45
> ++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > index 57d2c65..406f2e5 100644
> > ---
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-
> regulator.
> > +++ txt
> > @@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
> > Definition: Reference to regulator supplying the input pin, as
> > described in the data sheet.
> >
> > +- qcom,saw-reg:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: Reference to syscon node defining the SAW registers.
> > +
> >
> >  The regulator node houses sub-nodes for each regulator within the
> > device. Each  sub-node is identified using the node's name, with valid
> > values listed for each @@ -201,6 +206,17 @@ see regulator.txt - with
> additional custom properties described below:
> > 2 = 0.55 uA
> > 3 = 0.75 uA
> >
> > +- qcom,saw-slave:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang slave. Will not be configured.
> > +
> > +- qcom,saw-leader:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang leader. Will be configured as
> > +SAW regulator.
> > +
> >  Example:
> >
> > regulators {
> > @@ -221,3 +237,32 @@ Example:
> >
> > 
> > };
> > +
> > +Example 2:
> > +
> > +   saw3: syscon@9A1 {
> > +   compatible = "syscon";
> > +   reg = <0x9A1 0x1000>;
> > +   };
> > +
> > +   ...
> > +
> > +   spm-regulators {
> > +   compatible = "qcom,pm8994-regulators";
> > +   qcom,saw-reg = <>;
> > +   s8 {
> > +   qcom,saw-slave;
> > +   };
> > +   s9 {
> > +   qcom,saw-slave;
> > +   };
> > +   s10 {
> > +   qcom,saw-slave;
> > +   };
> > +   pm8994_s11_saw: s11 {
> > +   qcom,saw-leader;
> > +   regulator-always-on;
> > +   regulator-min-microvolt = <90>;
> > +   regulator-max-microvolt = <114>;
> > +   };
> > +   };
> > --
> > 1.9.1
> >

RE: [PATCH v7 10/14] dt-bindings: qcom_spmi: Add support for SAW documentation

[ilialin]

> -Original Message-
> From: amit.kuche...@verdurent.com  On
> Behalf Of Amit Kucheria
> Sent: Wednesday, May 16, 2018 16:13
> To: Ilia Lin 
> Cc: Michael Turquette ; sb...@kernel.org; Rob
> Herring ; Mark Rutland ; Viresh
> Kumar ; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; Andy Gross ; David Brown
> ; catalin.mari...@arm.com;
> will.dea...@arm.com; Rafael J. Wysocki ; linux-
> c...@vger.kernel.org; devicet...@vger.kernel.org; LKML  ker...@vger.kernel.org>; Linux PM list ; linux-
> arm-...@vger.kernel.org; linux-...@vger.kernel.org; lakml  ker...@lists.infradead.org>; Rajendra Nayak ;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v7 10/14] dt-bindings: qcom_spmi: Add support for SAW
> documentation
> 
> On Tue, May 15, 2018 at 12:13 PM, Ilia Lin  wrote:
> > Add support for SAW controlled regulators.
> > The regulators defined as SAW controlled in the device tree will be
> > controlled through special CPU registers instead of direct SPMI
> > accesses.
> > This is required especially for CPU supply regulators to synchronize
> > with clock scaling and for Automatic Voltage Switching.
> > Document it.
> 
> Replace this boiler plate with what this patch actual does. Besides changing
> the subject, it could be, for example,
> 
> "Document the DT bindings for the SAW regulators.
> 
> The saw-slave property allows ganging (grouping) of several regulators so
> that their outputs can be combined... blah blah.
> 
> The saw-leader is the only one that then is configurable in DT"

Actually, I invested some fantasy to write this explanation. But I'll try to 
revise it.

> 
> 
> > Signed-off-by: Ilia Lin 
> > Reviewed-by: Rob Herring 
> > ---
> >  .../bindings/regulator/qcom,spmi-regulator.txt | 45
> ++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > index 57d2c65..406f2e5 100644
> > ---
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-
> regulator.
> > +++ txt
> > @@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
> > Definition: Reference to regulator supplying the input pin, as
> > described in the data sheet.
> >
> > +- qcom,saw-reg:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: Reference to syscon node defining the SAW registers.
> > +
> >
> >  The regulator node houses sub-nodes for each regulator within the
> > device. Each  sub-node is identified using the node's name, with valid
> > values listed for each @@ -201,6 +206,17 @@ see regulator.txt - with
> additional custom properties described below:
> > 2 = 0.55 uA
> > 3 = 0.75 uA
> >
> > +- qcom,saw-slave:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang slave. Will not be configured.
> > +
> > +- qcom,saw-leader:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang leader. Will be configured as
> > +SAW regulator.
> > +
> >  Example:
> >
> > regulators {
> > @@ -221,3 +237,32 @@ Example:
> >
> > 
> > };
> > +
> > +Example 2:
> > +
> > +   saw3: syscon@9A1 {
> > +   compatible = "syscon";
> > +   reg = <0x9A1 0x1000>;
> > +   };
> > +
> > +   ...
> > +
> > +   spm-regulators {
> > +   compatible = "qcom,pm8994-regulators";
> > +   qcom,saw-reg = <>;
> > +   s8 {
> > +   qcom,saw-slave;
> > +   };
> > +   s9 {
> > +   qcom,saw-slave;
> > +   };
> > +   s10 {
> > +   qcom,saw-slave;
> > +   };
> > +   pm8994_s11_saw: s11 {
> > +   qcom,saw-leader;
> > +   regulator-always-on;
> > +   regulator-min-microvolt = <90>;
> > +   regulator-max-microvolt = <114>;
> > +   };
> > +   };
> > --
> > 1.9.1
> >

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: amit.kuche...@verdurent.com  On
> Behalf Of Amit Kucheria
> Sent: Wednesday, May 16, 2018 16:13
> To: Ilia Lin 
> Cc: Michael Turquette ; sb...@kernel.org; Rob
> Herring ; Mark Rutland ; Viresh
> Kumar ; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; Andy Gross ; David Brown
> ; catalin.mari...@arm.com;
> will.dea...@arm.com; Rafael J. Wysocki ; linux-
> c...@vger.kernel.org; devicet...@vger.kernel.org; LKML  ker...@vger.kernel.org>; Linux PM list ; linux-
> arm-...@vger.kernel.org; linux-...@vger.kernel.org; lakml  ker...@lists.infradead.org>; Rajendra Nayak ;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@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  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 
> > ---
> >  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 000..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 

RE: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: amit.kuche...@verdurent.com  On
> Behalf Of Amit Kucheria
> Sent: Wednesday, May 16, 2018 16:13
> To: Ilia Lin 
> Cc: Michael Turquette ; sb...@kernel.org; Rob
> Herring ; Mark Rutland ; Viresh
> Kumar ; n...@ti.com; lgirdw...@gmail.com;
> broo...@kernel.org; Andy Gross ; David Brown
> ; catalin.mari...@arm.com;
> will.dea...@arm.com; Rafael J. Wysocki ; linux-
> c...@vger.kernel.org; devicet...@vger.kernel.org; LKML  ker...@vger.kernel.org>; Linux PM list ; linux-
> arm-...@vger.kernel.org; linux-...@vger.kernel.org; lakml  ker...@lists.infradead.org>; Rajendra Nayak ;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@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  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 
> > ---
> >  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 000..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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > 

RE: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:57
> To: ilia...@codeaurora.org
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> On 04-05-18, 09:44, ilia...@codeaurora.org wrote:
> >
> >
> > > -Original Message-
> > > From: Viresh Kumar 
> > > Sent: Friday, May 4, 2018 09:08
> > > To: Ilia Lin 
> > > Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> > > mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> > > broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> > > catalin.mari...@arm.com; will.dea...@arm.com;
> > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux- p...@vger.kernel.org;
> > > linux-arm-...@vger.kernel.org; linux- s...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org;
> > > rna...@codeaurora.org; amit.kuche...@linaro.org;
> > > nicolas.deche...@linaro.org; cels...@codeaurora.org;
> > > tfin...@codeaurora.org
> > > Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> > >
> > > On 03-05-18, 14:52, 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 
> > > > ---
> > > >  drivers/cpufreq/Kconfig.arm  |  11 +++
> > > >  drivers/cpufreq/Makefile |   1 +
> > > >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> > > >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 153
> > > > +++
> > > >  4 files changed, 168 insertions(+)  create mode 100644
> > > > drivers/cpufreq/qcom-cpufreq-kryo.c
> > > >
> > > > diff --git a/drivers/cpufreq/Kconfig.arm
> > > > b/drivers/cpufreq/Kconfig.arm index de55c7d..f9da18c 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
> > > > +   tristate "Qualcomm Technologies, Inc. Kryo based CPUFreq"
> > >
> > > I don't see any reply to Sricharan's query on this being tristate.
> >
> > Why shouldn't we leave possibility to compile the cpufreq-dt built-in,
> > and the qcom-cpufreq-kryo module?
> 
> I was not saying this is incorrect, all I am saying is that you never
replied to a
> comment from one of the reviewers.
> 
> And I don't see a reason why this should be a tristate really.
> cpufreq-dt is already capable of being a module, all your driver does is
that it
> creates the cpufreq-dt platform device after setting the OPP hw
properties..
> 
> Over that, have you ever tried inserting, then removing and inserting the
> driver module again? I feel it will fail.
> 
> The reason is that you never provided an exit routine which can get rid of
the
> platform device created in the first place.

Convinced. I'll change it to bool.

> 
> > > > +   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
> > > >  

RE: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:57
> To: ilia...@codeaurora.org
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> On 04-05-18, 09:44, ilia...@codeaurora.org wrote:
> >
> >
> > > -Original Message-
> > > From: Viresh Kumar 
> > > Sent: Friday, May 4, 2018 09:08
> > > To: Ilia Lin 
> > > Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> > > mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> > > broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> > > catalin.mari...@arm.com; will.dea...@arm.com;
> > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux- p...@vger.kernel.org;
> > > linux-arm-...@vger.kernel.org; linux- s...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org;
> > > rna...@codeaurora.org; amit.kuche...@linaro.org;
> > > nicolas.deche...@linaro.org; cels...@codeaurora.org;
> > > tfin...@codeaurora.org
> > > Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> > >
> > > On 03-05-18, 14:52, 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 
> > > > ---
> > > >  drivers/cpufreq/Kconfig.arm  |  11 +++
> > > >  drivers/cpufreq/Makefile |   1 +
> > > >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> > > >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 153
> > > > +++
> > > >  4 files changed, 168 insertions(+)  create mode 100644
> > > > drivers/cpufreq/qcom-cpufreq-kryo.c
> > > >
> > > > diff --git a/drivers/cpufreq/Kconfig.arm
> > > > b/drivers/cpufreq/Kconfig.arm index de55c7d..f9da18c 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
> > > > +   tristate "Qualcomm Technologies, Inc. Kryo based CPUFreq"
> > >
> > > I don't see any reply to Sricharan's query on this being tristate.
> >
> > Why shouldn't we leave possibility to compile the cpufreq-dt built-in,
> > and the qcom-cpufreq-kryo module?
> 
> I was not saying this is incorrect, all I am saying is that you never
replied to a
> comment from one of the reviewers.
> 
> And I don't see a reason why this should be a tristate really.
> cpufreq-dt is already capable of being a module, all your driver does is
that it
> creates the cpufreq-dt platform device after setting the OPP hw
properties..
> 
> Over that, have you ever tried inserting, then removing and inserting the
> driver module again? I feel it will fail.
> 
> The reason is that you never provided an exit routine which can get rid of
the
> platform device created in the first place.

Convinced. I'll change it to bool.

> 
> > > > +   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

RE: [PATCH v4 08/14] dt: qcom: Add opp and thermal to the msm8996

[ilialin]

Kryo has single clock per cluster. I define here a shared OPP table per cluster.

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:04
> To: Ilia Lin 
> Cc: Michael Turquette ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; Liam
> Girdwood ; Mark Brown ;
> Andy Gross ; David Brown
> ; Catalin Marinas ; Will
> Deacon ; Linux Clock List  c...@vger.kernel.org>; devicet...@vger.kernel.org; Linux Kernel Mailing List
> ; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; open list:ARM/QUALCOMM SUPPORT  s...@vger.kernel.org>; linux-arm-ker...@lists.infradead.org; Nayak,
> Rajendra ; Amit Kucheria
> ; Nicolas Dechesne
> ; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v4 08/14] dt: qcom: Add opp and thermal to the
> msm8996
> 
> On 2 April 2018 at 14:46, Viresh Kumar  wrote:
> 
> >> + cluster0_opp: opp_table0 {
> >> + compatible = "operating-points-v2";
> >> + opp-shared;
> >
> > Is Kryo like krait where CPUs do DVFS independently ? If yes, then
> > opp-shared thing should be dropped.
> 
> Have you ever replied to this question ? Sorry, but I am not able to find it 
> in
> my inbox :(

RE: [PATCH v4 08/14] dt: qcom: Add opp and thermal to the msm8996

[ilialin]

Kryo has single clock per cluster. I define here a shared OPP table per cluster.

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:04
> To: Ilia Lin 
> Cc: Michael Turquette ; Stephen Boyd
> ; Rob Herring ; Mark Rutland
> ; Rafael J. Wysocki ; Liam
> Girdwood ; Mark Brown ;
> Andy Gross ; David Brown
> ; Catalin Marinas ; Will
> Deacon ; Linux Clock List  c...@vger.kernel.org>; devicet...@vger.kernel.org; Linux Kernel Mailing List
> ; linux...@vger.kernel.org; linux-arm-
> m...@vger.kernel.org; open list:ARM/QUALCOMM SUPPORT  s...@vger.kernel.org>; linux-arm-ker...@lists.infradead.org; Nayak,
> Rajendra ; Amit Kucheria
> ; Nicolas Dechesne
> ; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v4 08/14] dt: qcom: Add opp and thermal to the
> msm8996
> 
> On 2 April 2018 at 14:46, Viresh Kumar  wrote:
> 
> >> + cluster0_opp: opp_table0 {
> >> + compatible = "operating-points-v2";
> >> + opp-shared;
> >
> > Is Kryo like krait where CPUs do DVFS independently ? If yes, then
> > opp-shared thing should be dropped.
> 
> Have you ever replied to this question ? Sorry, but I am not able to find it 
> in
> my inbox :(

RE: [PATCH v5 13/14] dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:11
> To: Ilia Lin 
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 13/14] dt-bindings: cpufreq: Document operating-
> points-v2-kryo-cpu
> 
> On 03-05-18, 14:52, Ilia Lin wrote:
> > In Certain Qualcomm Technologies, Inc. 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 Technologies, Inc. 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.
> >
> > This change adds documentation.
> >
> > Signed-off-by: Ilia Lin 
> > ---
> >  .../devicetree/bindings/opp/kryo-cpufreq.txt   | 693
> +
> >  1 file changed, 693 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> >
> > diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > new file mode 100644
> > index 000..20cef9d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > @@ -0,0 +1,693 @@
> > +Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
> > +===
> > +
> > +In Certain Qualcomm Technologies, Inc. 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 Technologies, Inc. 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
> (existing HW bitmap).
> > +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.
> > +
> > +Required properties:
> > +
> > +In 'cpus' nodes:
> > +- operating-points-v2: Phandle to the operating-points-v2 table to use.
> > +
> > +In 'operating-points-v2' table:
> > +- compatible: Should be
> > +   - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
> > +- nvmem-cells: A phandle pointing to a nvmem-cells node representing
> the
> > +   efuse registers that has information about the
> > +   speedbin that is used to select the right frequency/voltage
> > +   value pair.
> > +   Please refer the for nvmem-cells
> > +   bindings
> Documentation/devicetree/bindings/nvmem/nvmem.txt
> > +   and also examples below.
> > +
> > +In every OPP node:
> > +- opp-supported-hw: A single 32 bit bitmap value, representing
> compatible HW.
> > +   Bitmap:
> > +   0:  MSM8996 V3, speedbin 0
> > +   1:  MSM8996 V3, speedbin 1
> > +   2:  MSM8996 V3, speedbin 2
> > +   3:  unused
> > +   4:  MSM8996 SG, speedbin 0
> > +   5:  MSM8996 SG, speedbin 1
> > +   6:  MSM8996 SG, speedbin 2
> > +   7-31:   unused
> > +
> > +Example 1:
> > +-
> > +
> > +   cpus {
> > +   #address-cells = <2>;
> > +   #size-cells = <0>;
> > +
> > +   CPU0: cpu@0 {
> > +   device_type = "cpu";
> > +   compatible = "qcom,kryo";
> > +   reg = <0x0 0x0>;
> > +   enable-method = "psci";
> > +   clocks = < 0>;
> > +   cpu-supply = <_s11_saw>;
> > +   operating-points-v2 = <_opp>;
> > +   /* cooling options */
> > +   cooling-min-level = <0>;
> > +   cooling-max-level = <15>;
> 
> cooling min/max aren't required anymore, as I told you in the previous
> version :)

RE: [PATCH v5 13/14] dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:11
> To: Ilia Lin 
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 13/14] dt-bindings: cpufreq: Document operating-
> points-v2-kryo-cpu
> 
> On 03-05-18, 14:52, Ilia Lin wrote:
> > In Certain Qualcomm Technologies, Inc. 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 Technologies, Inc. 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.
> >
> > This change adds documentation.
> >
> > Signed-off-by: Ilia Lin 
> > ---
> >  .../devicetree/bindings/opp/kryo-cpufreq.txt   | 693
> +
> >  1 file changed, 693 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> >
> > diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > new file mode 100644
> > index 000..20cef9d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> > @@ -0,0 +1,693 @@
> > +Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
> > +===
> > +
> > +In Certain Qualcomm Technologies, Inc. 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 Technologies, Inc. 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
> (existing HW bitmap).
> > +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.
> > +
> > +Required properties:
> > +
> > +In 'cpus' nodes:
> > +- operating-points-v2: Phandle to the operating-points-v2 table to use.
> > +
> > +In 'operating-points-v2' table:
> > +- compatible: Should be
> > +   - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
> > +- nvmem-cells: A phandle pointing to a nvmem-cells node representing
> the
> > +   efuse registers that has information about the
> > +   speedbin that is used to select the right frequency/voltage
> > +   value pair.
> > +   Please refer the for nvmem-cells
> > +   bindings
> Documentation/devicetree/bindings/nvmem/nvmem.txt
> > +   and also examples below.
> > +
> > +In every OPP node:
> > +- opp-supported-hw: A single 32 bit bitmap value, representing
> compatible HW.
> > +   Bitmap:
> > +   0:  MSM8996 V3, speedbin 0
> > +   1:  MSM8996 V3, speedbin 1
> > +   2:  MSM8996 V3, speedbin 2
> > +   3:  unused
> > +   4:  MSM8996 SG, speedbin 0
> > +   5:  MSM8996 SG, speedbin 1
> > +   6:  MSM8996 SG, speedbin 2
> > +   7-31:   unused
> > +
> > +Example 1:
> > +-
> > +
> > +   cpus {
> > +   #address-cells = <2>;
> > +   #size-cells = <0>;
> > +
> > +   CPU0: cpu@0 {
> > +   device_type = "cpu";
> > +   compatible = "qcom,kryo";
> > +   reg = <0x0 0x0>;
> > +   enable-method = "psci";
> > +   clocks = < 0>;
> > +   cpu-supply = <_s11_saw>;
> > +   operating-points-v2 = <_opp>;
> > +   /* cooling options */
> > +   cooling-min-level = <0>;
> > +   cooling-max-level = <15>;
> 
> cooling min/max aren't required anymore, as I told you in the previous
> version :)

Sure, I removed them in the DT, but forgot in the documentation. Will 

RE: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:08
> To: Ilia Lin 
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> On 03-05-18, 14:52, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  11 +++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 153
> > +++
> >  4 files changed, 168 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index de55c7d..f9da18c 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
> > +   tristate "Qualcomm Technologies, Inc. Kryo based CPUFreq"
> 
> I don't see any reply to Sricharan's query on this being tristate.

Why shouldn't we leave possibility to compile the cpufreq-dt built-in, and
the qcom-cpufreq-kryo module?

> 
> > +   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 000..32371cc
> > --- /dev/null
> > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> 
> Incorrect multi line comment.

This was done as per Bjorn's instruction.

> 
> > + *
> > + * 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 
> > +#include 
> > +#include 
> > +//#include 
> 

RE: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver

[ilialin]

> -Original Message-
> From: Viresh Kumar 
> Sent: Friday, May 4, 2018 09:08
> To: Ilia Lin 
> Cc: mturque...@baylibre.com; sb...@kernel.org; r...@kernel.org;
> mark.rutl...@arm.com; r...@rjwysocki.net; lgirdw...@gmail.com;
> broo...@kernel.org; andy.gr...@linaro.org; david.br...@linaro.org;
> catalin.mari...@arm.com; will.dea...@arm.com; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> p...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> rna...@codeaurora.org; amit.kuche...@linaro.org;
> nicolas.deche...@linaro.org; cels...@codeaurora.org;
> tfin...@codeaurora.org
> Subject: Re: [PATCH v5 12/14] cpufreq: Add Kryo CPU scaling driver
> 
> On 03-05-18, 14:52, 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 
> > ---
> >  drivers/cpufreq/Kconfig.arm  |  11 +++
> >  drivers/cpufreq/Makefile |   1 +
> >  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
> >  drivers/cpufreq/qcom-cpufreq-kryo.c  | 153
> > +++
> >  4 files changed, 168 insertions(+)
> >  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index de55c7d..f9da18c 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
> > +   tristate "Qualcomm Technologies, Inc. Kryo based CPUFreq"
> 
> I don't see any reply to Sricharan's query on this being tristate.

Why shouldn't we leave possibility to compile the cpufreq-dt built-in, and
the qcom-cpufreq-kryo module?

> 
> > +   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 000..32371cc
> > --- /dev/null
> > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> 
> Incorrect multi line comment.

This was done as per Bjorn's instruction.

> 
> > + *
> > + * 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 
> > +#include 
> > +#include 
> > +//#include 
> 
> ??

Not good. Will fix this.

> 
> > +#include 
> > +#include 

RE: [PATCH 2/2] dt-bindings: Add support for SAW documentation

[ilialin]

Thanks for the review, Rob.

I used existing drivers as reference and defined in the same way. Could you
point me to a reference that is correct from your point of view?

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Wednesday, March 7, 2018 22:41
> To: Ilia Lin 
> Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org;
> lgirdw...@gmail.com; broo...@kernel.org; mark.rutl...@arm.com;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@codeaurora.org
> Subject: Re: [PATCH 2/2] dt-bindings: Add support for SAW documentation
> 
> On Sun, Mar 04, 2018 at 02:31:49PM +0200, Ilia Lin wrote:
> > Add support for SAW controlled regulators in 8x96.
> > Document it.
> >
> > Signed-off-by: Ilia Lin 
> > ---
> >  .../bindings/regulator/qcom,spmi-regulator.txt | 45
> ++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > index 57d2c65..406f2e5 100644
> > ---
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-
> regulator.
> > +++ txt
> > @@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
> > Definition: Reference to regulator supplying the input pin, as
> > described in the data sheet.
> >
> > +- qcom,saw-reg:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: Reference to syscon node defining the SAW registers.
> > +
> >
> >  The regulator node houses sub-nodes for each regulator within the
> > device. Each  sub-node is identified using the node's name, with valid
> > values listed for each @@ -201,6 +206,17 @@ see regulator.txt - with
> additional custom properties described below:
> > 2 = 0.55 uA
> > 3 = 0.75 uA
> >
> > +- qcom,saw-slave:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang slave. Will not be configured.
> > +
> > +- qcom,saw-leader:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang leader. Will be configured as
> > +SAW regulator.
> > +
> >  Example:
> >
> > regulators {
> > @@ -221,3 +237,32 @@ Example:
> >
> > 
> > };
> > +
> > +Example 2:
> > +
> > +   saw3: syscon@9A1 {
> > +   compatible = "syscon";
> 
> syscon should not be used alone. It should have a specific compatible too.
> 
> > +   reg = <0x9A1 0x1000>;
> > +   };
> > +
> > +   ...
> > +
> > +   spm-regulators {
> > +   compatible = "qcom,pm8994-regulators";
> > +   qcom,saw-reg = <>;
> 
> Why not just a child of the syscon?
> 
> > +   s8 {
> > +   qcom,saw-slave;
> > +   };
> > +   s9 {
> > +   qcom,saw-slave;
> > +   };
> > +   s10 {
> > +   qcom,saw-slave;
> > +   };
> > +   pm8994_s11_saw: s11 {
> > +   qcom,saw-leader;
> > +   regulator-always-on;
> > +   regulator-min-microvolt = <90>;
> > +   regulator-max-microvolt = <114>;
> > +   };
> > +   };
> > --
> > Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >

RE: [PATCH 2/2] dt-bindings: Add support for SAW documentation

[ilialin]

Thanks for the review, Rob.

I used existing drivers as reference and defined in the same way. Could you
point me to a reference that is correct from your point of view?

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Wednesday, March 7, 2018 22:41
> To: Ilia Lin 
> Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org;
> lgirdw...@gmail.com; broo...@kernel.org; mark.rutl...@arm.com;
> amit.kuche...@linaro.org; nicolas.deche...@linaro.org;
> cels...@codeaurora.org; tfin...@codeaurora.org
> Subject: Re: [PATCH 2/2] dt-bindings: Add support for SAW documentation
> 
> On Sun, Mar 04, 2018 at 02:31:49PM +0200, Ilia Lin wrote:
> > Add support for SAW controlled regulators in 8x96.
> > Document it.
> >
> > Signed-off-by: Ilia Lin 
> > ---
> >  .../bindings/regulator/qcom,spmi-regulator.txt | 45
> ++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > index 57d2c65..406f2e5 100644
> > ---
> > a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-
> regulator.
> > +++ txt
> > @@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
> > Definition: Reference to regulator supplying the input pin, as
> > described in the data sheet.
> >
> > +- qcom,saw-reg:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: Reference to syscon node defining the SAW registers.
> > +
> >
> >  The regulator node houses sub-nodes for each regulator within the
> > device. Each  sub-node is identified using the node's name, with valid
> > values listed for each @@ -201,6 +206,17 @@ see regulator.txt - with
> additional custom properties described below:
> > 2 = 0.55 uA
> > 3 = 0.75 uA
> >
> > +- qcom,saw-slave:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang slave. Will not be configured.
> > +
> > +- qcom,saw-leader:
> > +   Usage: optional
> > +   Value type: 
> > +   Description: SAW controlled gang leader. Will be configured as
> > +SAW regulator.
> > +
> >  Example:
> >
> > regulators {
> > @@ -221,3 +237,32 @@ Example:
> >
> > 
> > };
> > +
> > +Example 2:
> > +
> > +   saw3: syscon@9A1 {
> > +   compatible = "syscon";
> 
> syscon should not be used alone. It should have a specific compatible too.
> 
> > +   reg = <0x9A1 0x1000>;
> > +   };
> > +
> > +   ...
> > +
> > +   spm-regulators {
> > +   compatible = "qcom,pm8994-regulators";
> > +   qcom,saw-reg = <>;
> 
> Why not just a child of the syscon?
> 
> > +   s8 {
> > +   qcom,saw-slave;
> > +   };
> > +   s9 {
> > +   qcom,saw-slave;
> > +   };
> > +   s10 {
> > +   qcom,saw-slave;
> > +   };
> > +   pm8994_s11_saw: s11 {
> > +   qcom,saw-leader;
> > +   regulator-always-on;
> > +   regulator-min-microvolt = <90>;
> > +   regulator-max-microvolt = <114>;
> > +   };
> > +   };
> > --
> > Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >