[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+5LGR20WaeHVP8vArbMJV+aqEqZ7dcHxB9KTDPBNt92VPy8Gg@mail.gmail.com>
Date: Mon, 8 Jul 2019 09:27:32 +0300
From: Ilia Lin <ilia.lin@...nel.org>
To: Niklas Cassel <niklas.cassel@...aro.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Viresh Kumar <viresh.kumar@...aro.org>,
Andy Gross <agross@...nel.org>, Ilia Lin <ilia.lin@...nel.org>,
linux-arm-msm@...r.kernel.org, jorge.ramirez-ortiz@...aro.org,
Stephen Boyd <sboyd@...nel.org>, vireshk@...nel.org,
bjorn.andersson@...aro.org, ulf.hansson@...aro.org,
Sricharan R <sricharan@...eaurora.org>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 02/13] cpufreq: qcom: Re-organise kryo cpufreq to use it
for other nvmem based qcom socs
Reviewed-by: Ilia Lin <ilia.lin@...nel.org>
On Fri, Jul 5, 2019 at 12:57 PM Niklas Cassel <niklas.cassel@...aro.org> wrote:
>
> From: Sricharan R <sricharan@...eaurora.org>
>
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
>
> Signed-off-by: Sricharan R <sricharan@...eaurora.org>
> [niklas.cassel@...aro.org: split dt-binding into a separate patch and
> do not rename the compatible string. Update MAINTAINERS file.]
> Signed-off-by: Niklas Cassel <niklas.cassel@...aro.org>
> ---
> Changes since RFC:
> -Made DT bindings a separate patch.
> -Keep the original compatible string, since renaming it breaks DT
> backwards compatibility.
>
> MAINTAINERS | 4 +-
> drivers/cpufreq/Kconfig.arm | 4 +-
> drivers/cpufreq/Makefile | 2 +-
> ...om-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} | 122 +++++++++++-------
> 4 files changed, 78 insertions(+), 54 deletions(-)
> rename drivers/cpufreq/{qcom-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} (70%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04a66ba93c26..38ab8374fa7a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13242,8 +13242,8 @@ QUALCOMM CPUFREQ DRIVER MSM8996/APQ8096
> M: Ilia Lin <ilia.lin@...nel.org>
> L: linux-pm@...r.kernel.org
> S: Maintained
> -F: Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> -F: drivers/cpufreq/qcom-cpufreq-kryo.c
> +F: Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +F: drivers/cpufreq/qcom-cpufreq-nvmem.c
>
> QUALCOMM EMAC GIGABIT ETHERNET DRIVER
> M: Timur Tabi <timur@...nel.org>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 56c31a78c692..b1aa485a28dd 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -120,8 +120,8 @@ config ARM_OMAP2PLUS_CPUFREQ
> depends on ARCH_OMAP2PLUS
> default ARCH_OMAP2PLUS
>
> -config ARM_QCOM_CPUFREQ_KRYO
> - tristate "Qualcomm Kryo based CPUFreq"
> +config ARM_QCOM_CPUFREQ_NVMEM
> + tristate "Qualcomm nvmem based CPUFreq"
> depends on ARM64
> depends on QCOM_QFPROM
> depends on QCOM_SMEM
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 5a6c70d26c98..8572a918aa75 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -64,7 +64,7 @@ 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_HW) += qcom-cpufreq-hw.o
> -obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_NVMEM) += qcom-cpufreq-nvmem.o
> obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o
> obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
> obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
> diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> similarity index 70%
> rename from drivers/cpufreq/qcom-cpufreq-kryo.c
> rename to drivers/cpufreq/qcom-cpufreq-nvmem.c
> index dd64dcf89c74..fad6509eecb5 100644
> --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -9,7 +9,7 @@
> * 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
> + * The qcom-cpufreq 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.
> @@ -22,6 +22,7 @@
> #include <linux/module.h>
> #include <linux/nvmem-consumer.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
> @@ -42,9 +43,9 @@ enum _msm8996_version {
> NUM_OF_MSM8996_VERSIONS,
> };
>
> -static struct platform_device *cpufreq_dt_pdev, *kryo_cpufreq_pdev;
> +static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
>
> -static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void)
> +static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
> {
> size_t len;
> u32 *msm_id;
> @@ -73,28 +74,62 @@ static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void)
> return version;
> }
>
> -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> +static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
> + struct nvmem_cell *speedbin_nvmem,
> + u32 *versions)
> {
> - struct opp_table **opp_tables;
> + size_t len;
> + u8 *speedbin;
> enum _msm8996_version msm8996_version;
> +
> + msm8996_version = qcom_cpufreq_get_msm_id();
> + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> + dev_err(cpu_dev, "Not Snapdragon 820/821!");
> + return -ENODEV;
> + }
> +
> + speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> + if (IS_ERR(speedbin))
> + return PTR_ERR(speedbin);
> +
> + 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;
> + }
> +
> + kfree(speedbin);
> + return 0;
> +}
> +
> +static int qcom_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct opp_table **opp_tables;
> + int (*get_version)(struct device *cpu_dev,
> + struct nvmem_cell *speedbin_nvmem,
> + u32 *versions);
> struct nvmem_cell *speedbin_nvmem;
> struct device_node *np;
> struct device *cpu_dev;
> unsigned cpu;
> - u8 *speedbin;
> u32 versions;
> - size_t len;
> + const struct of_device_id *match;
> int ret;
>
> cpu_dev = get_cpu_device(0);
> if (!cpu_dev)
> return -ENODEV;
>
> - msm8996_version = qcom_cpufreq_kryo_get_msm_id();
> - if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> - dev_err(cpu_dev, "Not Snapdragon 820/821!");
> + match = pdev->dev.platform_data;
> + get_version = match->data;
> + if (!get_version)
> return -ENODEV;
> - }
>
> np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> if (!np)
> @@ -115,23 +150,10 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> return PTR_ERR(speedbin_nvmem);
> }
>
> - speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> + ret = get_version(cpu_dev, speedbin_nvmem, &versions);
> nvmem_cell_put(speedbin_nvmem);
> - if (IS_ERR(speedbin))
> - return PTR_ERR(speedbin);
> -
> - 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;
> - }
> - kfree(speedbin);
> + if (ret)
> + return ret;
>
> opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), GFP_KERNEL);
> if (!opp_tables)
> @@ -174,7 +196,7 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static int qcom_cpufreq_kryo_remove(struct platform_device *pdev)
> +static int qcom_cpufreq_remove(struct platform_device *pdev)
> {
> struct opp_table **opp_tables = platform_get_drvdata(pdev);
> unsigned int cpu;
> @@ -189,18 +211,20 @@ static int qcom_cpufreq_kryo_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static struct platform_driver qcom_cpufreq_kryo_driver = {
> - .probe = qcom_cpufreq_kryo_probe,
> - .remove = qcom_cpufreq_kryo_remove,
> +static struct platform_driver qcom_cpufreq_driver = {
> + .probe = qcom_cpufreq_probe,
> + .remove = qcom_cpufreq_remove,
> .driver = {
> - .name = "qcom-cpufreq-kryo",
> + .name = "qcom-cpufreq",
> },
> };
>
> -static const struct of_device_id qcom_cpufreq_kryo_match_list[] __initconst = {
> - { .compatible = "qcom,apq8096", },
> - { .compatible = "qcom,msm8996", },
> - {}
> +static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> + { .compatible = "qcom,apq8096",
> + .data = qcom_cpufreq_kryo_name_version },
> + { .compatible = "qcom,msm8996",
> + .data = qcom_cpufreq_kryo_name_version },
> + {},
> };
>
> /*
> @@ -209,7 +233,7 @@ static const struct of_device_id qcom_cpufreq_kryo_match_list[] __initconst = {
> * which may be defered as well. The init here is only registering
> * the driver and the platform device.
> */
> -static int __init qcom_cpufreq_kryo_init(void)
> +static int __init qcom_cpufreq_init(void)
> {
> struct device_node *np = of_find_node_by_path("/");
> const struct of_device_id *match;
> @@ -218,32 +242,32 @@ static int __init qcom_cpufreq_kryo_init(void)
> if (!np)
> return -ENODEV;
>
> - match = of_match_node(qcom_cpufreq_kryo_match_list, np);
> + match = of_match_node(qcom_cpufreq_match_list, np);
> of_node_put(np);
> if (!match)
> return -ENODEV;
>
> - ret = platform_driver_register(&qcom_cpufreq_kryo_driver);
> + ret = platform_driver_register(&qcom_cpufreq_driver);
> if (unlikely(ret < 0))
> return ret;
>
> - kryo_cpufreq_pdev = platform_device_register_simple(
> - "qcom-cpufreq-kryo", -1, NULL, 0);
> - ret = PTR_ERR_OR_ZERO(kryo_cpufreq_pdev);
> + cpufreq_pdev = platform_device_register_data(NULL, "qcom-cpufreq",
> + -1, match, sizeof(*match));
> + ret = PTR_ERR_OR_ZERO(cpufreq_pdev);
> if (0 == ret)
> return 0;
>
> - platform_driver_unregister(&qcom_cpufreq_kryo_driver);
> + platform_driver_unregister(&qcom_cpufreq_driver);
> return ret;
> }
> -module_init(qcom_cpufreq_kryo_init);
> +module_init(qcom_cpufreq_init);
>
> -static void __exit qcom_cpufreq_kryo_exit(void)
> +static void __exit qcom_cpufreq_exit(void)
> {
> - platform_device_unregister(kryo_cpufreq_pdev);
> - platform_driver_unregister(&qcom_cpufreq_kryo_driver);
> + platform_device_unregister(cpufreq_pdev);
> + platform_driver_unregister(&qcom_cpufreq_driver);
> }
> -module_exit(qcom_cpufreq_kryo_exit);
> +module_exit(qcom_cpufreq_exit);
>
> -MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. CPUfreq driver");
> MODULE_LICENSE("GPL v2");
> --
> 2.21.0
>
Powered by blists - more mailing lists