[PATCH 2/2] cpufreq: ti: Add cpufreq driver to determine available OPPs at runtime
Dave Gerlach
d-gerlach at ti.com
Thu May 19 11:33:40 PDT 2016
On 05/18/2016 11:39 PM, Viresh Kumar wrote:
> On 18-05-16, 18:30, Dave Gerlach wrote:
>> Some TI SoCs, like those in the AM335x, AM437x, DRA7x, and AM57x families,
>> have different OPPs available for the MPU depending on which specific
>> variant of the SoC is in use. This can be determined through use of the
>> revision and an eFuse register present in the silicon. Introduce a
>> ti-cpufreq driver that can read the aformentioned values and provide
>> them as version matching data to the opp framework. Through this the
>> opp-supported-hw dt binding that is part of the operating-points-v2
>> table can be used to indicate availability of OPPs for each device.
>>
>> This driver also creates the "cpufreq-dt" platform_device after passing
>> the version matching data to the OPP framework so that the cpufreq-dt
>> handles the actual cpufreq implementation. Even without the necessary
>> data to pass the version matching data the driver will still create this
>> device to maintain backwards compatibility with operating-points v1
>> tables.
>>
>> Signed-off-by: Dave Gerlach <d-gerlach at ti.com>
>> ---
>> drivers/cpufreq/Kconfig.arm | 11 ++
>> drivers/cpufreq/Makefile | 1 +
>> drivers/cpufreq/ti-cpufreq.c | 254 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 266 insertions(+)
>> create mode 100644 drivers/cpufreq/ti-cpufreq.c
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index d89b8afe23b6..0dea6849ac3e 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -234,6 +234,17 @@ config ARM_TEGRA124_CPUFREQ
>> help
>> This adds the CPUFreq driver support for Tegra124 SOCs.
>>
>> +config ARM_TI_CPUFREQ
>> + tristate "Texas Instruments CPUFreq support"
>
> You sure you want to get it compiled as a module? And don't provide
> module_exit() at all?
I thought about this for a while, and I was going to make it bool but
decided to model it after what has already been accepted (sti-cpufreq).
Wasn't sure what preferred path is, but I will go ahead and switch this
to bool for this driver as I think that makes the most sense.
>
>> + depends on ARCH_OMAP2PLUS
>> + help
>> + This driver enables valid OPPs on the running platform based on
>> + values contained within the SoC in use. Enable this in order to
>> + use the cpufreq-dt driver on all Texas Instruments platforms that
>> + provide dt based operating-points-v2 tables with opp-supported-hw
>> + data provided. Required for cpufreq support on AM335x, AM437x,
>> + DRA7x, and AM57x platforms.
>> +
>> config ARM_PXA2xx_CPUFREQ
>> tristate "Intel PXA2xx CPUfreq driver"
>> depends on PXA27x || PXA25x
>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>> index 0a9b6a093646..5b1b6ec0a9f0 100644
>> --- a/drivers/cpufreq/Makefile
>> +++ b/drivers/cpufreq/Makefile
>> @@ -77,6 +77,7 @@ obj-$(CONFIG_ARM_SPEAR_CPUFREQ) += spear-cpufreq.o
>> obj-$(CONFIG_ARM_STI_CPUFREQ) += sti-cpufreq.o
>> obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
>> obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
>> +obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
>> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
>> obj-$(CONFIG_ACPI_CPPC_CPUFREQ) += cppc_cpufreq.o
>> obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o
>> diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
>> new file mode 100644
>> index 000000000000..e47b452aadd0
>> --- /dev/null
>> +++ b/drivers/cpufreq/ti-cpufreq.c
>> @@ -0,0 +1,254 @@
>> +/*
>> + * TI CPUFreq/OPP hw-supported driver
>> + *
>> + * Copyright (C) 2016 Texas Instruments, Inc.
>> + * Dave Gerlach <d-gerlach at ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/cpu.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pm_opp.h>
>> +#include <linux/regmap.h>
>> +
>> +#define REVISION_MASK (0xF << 28)
>
> Use below shift-mask here ?
Yes, but this will change due to later change where this will actually
be used.
>
>> +#define REVISION_SHIFT 28
>> +
>> +#define DRA7_EFUSE_HAS_OD_MPU_OPP 11
>> +#define DRA7_EFUSE_HAS_HIGH_MPU_OPP 15
>> +#define DRA7_EFUSE_HAS_ALL_MPU_OPP 23
>> +
>> +#define DRA7_EFUSE_NOM_MPU_OPP BIT(0)
>> +#define DRA7_EFUSE_OD_MPU_OPP BIT(1)
>> +#define DRA7_EFUSE_HIGH_MPU_OPP BIT(2)
>> +
>> +#define VERSION_COUNT 2
>> +
>> +static struct ti_cpufreq_data {
>> + struct device *cpu;
>> + struct regmap *opp_efuse;
>> + struct regmap *revision;
>> +} opp_data;
>> +
>> +static struct ti_cpufreq_soc_data {
>> + unsigned long (*efuse_xlate)(unsigned long efuse);
>> +} *soc_data;
>> +
>> +static unsigned long amx3_efuse_xlate(unsigned long efuse)
>> +{
>> + /* AM335x and AM437x use "OPP disable" bits, so invert */
>> + return ~efuse;
>> +}
>> +
>> +static unsigned long dra7_efuse_xlate(unsigned long efuse)
>> +{
>> + unsigned long calculated_efuse = DRA7_EFUSE_NOM_MPU_OPP;
>> +
>> + /*
>> + * The efuse on dra7 and am57 parts contains a specific
>> + * value indicating the highest available OPP.
>> + */
>> +
>> + switch (efuse) {
>> + case DRA7_EFUSE_HAS_ALL_MPU_OPP:
>> + case DRA7_EFUSE_HAS_HIGH_MPU_OPP:
>> + calculated_efuse |= DRA7_EFUSE_HIGH_MPU_OPP;
>> + case DRA7_EFUSE_HAS_OD_MPU_OPP:
>> + calculated_efuse |= DRA7_EFUSE_OD_MPU_OPP;
>> + }
>> +
>> + return calculated_efuse;
>> +}
>> +
>> +static struct ti_cpufreq_soc_data amx3_soc_data = {
>> + .efuse_xlate = amx3_efuse_xlate,
>> +};
>> +
>> +static struct ti_cpufreq_soc_data dra7_soc_data = {
>> + .efuse_xlate = dra7_efuse_xlate,
>> +};
>> +
>> +/**
>> + * ti_cpufreq_get_efuse() - Parse and return efuse value present on SoC
>> + * @efuse_value: Set to the value parsed from efuse
>> + *
>> + * Returns error code if efuse not read properly.
>> + */
>> +static int ti_cpufreq_get_efuse(u32 *efuse_value)
>> +{
>> + struct device *dev = opp_data.cpu;
>> + struct device_node *np = dev->of_node;
>> + unsigned int efuse_offset;
>> + u32 efuse, efuse_mask, efuse_shift;
>> + int ret;
>> +
>> + ret = of_property_read_u32_index(np, "ti,syscon-efuse",
>> + 1, &efuse_offset);
>> + if (ret) {
>> + dev_err(dev,
>
> Line break here isn't required.
Ok thanks for catching this.
>
>> + "No efuse offset provided %s: %d\n",
>> + np->full_name, ret);
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32_index(np, "ti,syscon-efuse", 2,
>> + &efuse_mask);
>> + if (ret)
>> + efuse_mask = 0xffffffff;
>> +
>> + ret = of_property_read_u32_index(np, "ti,syscon-efuse", 3,
>> + &efuse_shift);
>> + if (ret)
>> + efuse_shift = 0;
>
> Why don't you read an array of 3 integers in one go?
To be honest, didn't know I could, will update to use
of_property_read_u32_array.
>
>> +
>> + ret = regmap_read(opp_data.opp_efuse, efuse_offset, &efuse);
>> + if (ret) {
>> + dev_err(dev,
>> + "Failed to read the efuse value from syscon: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + efuse = (efuse & efuse_mask) >> efuse_shift;
>> +
>> + *efuse_value = soc_data->efuse_xlate(efuse);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * ti_cpufreq_get_rev() - Parse and return rev value present on SoC
>> + * @revision_value: Set to the value parsed from revision register
>> + *
>> + * Returns error code if revision not read properly.
>> + */
>> +static int ti_cpufreq_get_rev(u32 *revision_value)
>> +{
>> + struct device *dev = opp_data.cpu;
>> + struct device_node *np = dev->of_node;
>> + unsigned int revision_offset;
>> + u32 revision;
>> + int ret;
>> +
>> + ret = of_property_read_u32_index(np, "ti,syscon-rev",
>> + 1, &revision_offset);
>> + if (ret) {
>> + dev_err(dev,
>> + "No revision offset provided %s [%d]\n",
>> + np->full_name, ret);
>> + return ret;
>> + }
>> +
>> + ret = regmap_read(opp_data.revision, revision_offset, &revision);
>> + if (ret) {
>> + dev_err(dev,
>> + "Failed to read the revision number from syscon: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + *revision_value = BIT((revision & REVISION_MASK) >> REVISION_SHIFT);
>
> That's an crazy operation.
>
> So you first shifted 0xF << 27, and then right shifted everything by 27 bits :)
>
> You should rather do:
>
> #define REVISION_MASK 0xF
> (revision >> REVISION_SHIFT) & REVISION_MASK
Yeah that's much cleaner I can update.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int ti_cpufreq_setup_syscon_registers(void)
>> +{
>> + struct device *dev = opp_data.cpu;
>> + struct device_node *np = dev->of_node;
>> +
>> + opp_data.opp_efuse = syscon_regmap_lookup_by_phandle(np,
>> + "ti,syscon-efuse");
>> + if (IS_ERR(opp_data.opp_efuse)) {
>> + dev_dbg(dev, "\"ti,syscon-efuse\" is missing, cannot use OPPv2 table.\n");
>> + return PTR_ERR(opp_data.opp_efuse);
>> + }
>> +
>> + opp_data.revision = syscon_regmap_lookup_by_phandle(np,
>> + "ti,syscon-rev");
>> + if (IS_ERR(opp_data.revision)) {
>> + dev_dbg(dev, "\"ti,syscon-rev\" is missing, cannot use OPPv2 table.\n");
>
> These messages are wrong as your code is going to use opp-v2 anyway.
Yes you are right for the case when these properties aren't provided
isn't provided but opp-v2 table is, I was thinking more of when no
properties are provided but opp-v1 table has been provided, like on
legacy DT files. I will need to consider both for this message.
>
>> + return PTR_ERR(opp_data.revision);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct ti_cpufreq_soc_data *ti_cpufreq_get_soc_data(void)
>> +{
>> + if (of_machine_is_compatible("ti,am33xx") ||
>> + of_machine_is_compatible("ti,am4372"))
>> + return &amx3_soc_data;
>> + else if (of_machine_is_compatible("ti,dra7"))
>> + return &dra7_soc_data;
>> + else
>> + return NULL;
>> +}
>> +
>> +static int ti_cpufreq_init(void)
>
> __init ?
Whoops, left that out.
>
>> +{
>> + int ret;
>> + u32 version[VERSION_COUNT];
>> +
>> + soc_data = ti_cpufreq_get_soc_data();
>> + if (!soc_data)
>> + return -ENODEV;
>
> Why not use of_match_node() and an array of type struct of_device_id instead of
> above function?
Interesting idea, I like it and will do that.
Thanks for your comments.
Regards,
Dave
>
>> +
>> + opp_data.cpu = get_cpu_device(0);
>> + if (!opp_data.cpu) {
>> + pr_err("%s: Failed to get device for CPU0\n", __func__);
>> + return -ENODEV;
>> + }
>> +
>> + if (!of_get_property(opp_data.cpu->of_node, "operating-points-v2",
>> + NULL)) {
>> + dev_info(opp_data.cpu, "OPP-v2 not supported, cpufreq-dt will attempt to use legacy tables.\n");
>> + goto register_cpufreq_dt;
>> + }
>> +
>> + ret = ti_cpufreq_setup_syscon_registers();
>> + if (ret)
>> + goto register_cpufreq_dt;
>> +
>> + /*
>> + * OPPs determine whether or not they are supported based on
>> + * two metrics:
>> + * 0 - SoC Revision
>> + * 1 - eFuse value
>> + */
>> + ret = ti_cpufreq_get_rev(&version[0]);
>> + if (ret)
>> + return ret;
>> +
>> + ret = ti_cpufreq_get_efuse(&version[1]);
>> + if (ret)
>> + return ret;
>> +
>> + ret = dev_pm_opp_set_supported_hw(opp_data.cpu, version, VERSION_COUNT);
>> + if (ret) {
>> + dev_err(opp_data.cpu, "Failed to set supported hardware\n");
>> + return ret;
>> + }
>> +
>> +register_cpufreq_dt:
>> + platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
>> +
>> + return 0;
>> +}
>> +module_init(ti_cpufreq_init);
>> +
>> +MODULE_DESCRIPTION("TI CPUFreq/OPP hw-supported driver");
>> +MODULE_AUTHOR("Dave Gerlach <d-gerlach at ti.com>");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.7.3
>
More information about the linux-arm-kernel
mailing list