Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On 18-12-18, 11:13, Stephen Boyd wrote:
> I don't see any failure returned from cpufreq_dt's cpufreq_init()
> function. Maybe put a static int counter = 0 and then fail
> cpufreq_init() the second time that it's called for the same policy
> pointer? I have a system with two policies, so I made it fail and return
> -EINVAL when the counter == 2 and I see the lockdep splat.
Yuck. You were so clear on this earlier, how can I forget it. :(
Here is the additional diff. And I do get this print message on my screen while
I try to online CPU4.
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 639feca22d27..b836e93fd87d 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -151,6 +151,11 @@ static int cpufreq_init(struct cpufreq_policy *policy)
const char *name;
int ret;
+ if (policy->count++) {
+ pr_info("%s: %d\n", __func__, __LINE__);
+ return -EINVAL;
+ }
+
cpu_dev = get_cpu_device(policy->cpu);
if (!cpu_dev) {
pr_err("failed to get cpu%d device\n", policy->cpu);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 882a9b9e34bc..643141a2013f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -151,6 +151,7 @@ struct cpufreq_policy {
/* For cpufreq driver's internal use */
void*driver_data;
+ int count;
};
> I did the following:
>
> grep . /sys/devices/system/cpu/cpufreq/*/* >/dev/null
> echo 0 > /sys/devices/system/cpu/cpu4/online
> echo 0 > /sys/devices/system/cpu/cpu5/online
> echo 0 > /sys/devices/system/cpu/cpu6/online
> echo 0 > /sys/devices/system/cpu/cpu7/online
> echo 1 > /sys/devices/system/cpu/cpu4/online
> dmesg
>
> And boom, lockdep splat.
502 grep . /sys/devices/system/cpu/cpufreq/*/*
503 echo 0 > /sys/devices/system/cpu/cpu4/online
504 echo 0 > /sys/devices/system/cpu/cpu5/online
505 echo 0 > /sys/devices/system/cpu/cpu6/online
506 echo 0 > /sys/devices/system/cpu/cpu7/online
507 grep . /sys/devices/system/cpu/cpufreq/*/*
508 echo 1 > /sys/devices/system/cpu/cpu4/online
509 dmesg
But still no lockdep :(
Can you try that on mainline as well ?
--
viresh
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Viresh Kumar (2018-12-17 21:45:45)
> Hi Stephen,
>
> On 13-12-18, 02:12, Stephen Boyd wrote:
> > Quoting Viresh Kumar (2018-12-13 02:05:06)
> > > On 13-12-18, 01:58, Stephen Boyd wrote:
> > > > BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error
> > > > upon bringing the policy online the second time. I guess cpufreq_stats
> > > > aren't able to be freed from there because they take locks in different
> > > > order vs. the normal path?
> > >
> > > Please share the lockdep report and the steps to reproduce it. I will
> > > see if I can simulate the failure forcefully..
> > >
> >
> > It's on a v4.19 kernel with this cpufreq hw driver backported to it. I
> > think all it takes is to return an error the second time the policy is
> > initialized when cpufreq_online() calls into the cpufreq driver.
> >
> > ==
> > WARNING: possible circular locking dependency detected
> > 4.19.8 #61 Tainted: GW
> > --
> > cpuhp/5/36 is trying to acquire lock:
> > 3e901e8a (kn->count#326){}, at:
> > kernfs_remove_by_name_ns+0x44/0x80
> >
> > but task is already holding lock:
> > dd7f52c3 (>rwsem){}, at:
> > cpufreq_policy_free+0x17c/0x1cc
> >
> > which lock already depends on the new lock.
> >
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (>rwsem){}:
> > down_read+0x50/0xcc
> > show+0x30/0x78
> > sysfs_kf_seq_show+0x17c/0x25c
> > kernfs_seq_show+0xb4/0xf8
> > seq_read+0x4a8/0x8f0
> > kernfs_fop_read+0xe0/0x360
> > __vfs_read+0x80/0x328
> > vfs_read+0xd0/0x1d4
> > ksys_read+0x88/0x118
> > __arm64_sys_read+0x4c/0x5c
> > el0_svc_common+0x124/0x1c4
> > el0_svc_compat_handler+0x64/0x8c
> > el0_svc_compat+0x8/0x18
>
> I failed to reproduce it over linux/next.
>
> I had the following changes over linux/next:
> https://pastebin.ubuntu.com/p/zkVm77PGdY/
I don't see any failure returned from cpufreq_dt's cpufreq_init()
function. Maybe put a static int counter = 0 and then fail
cpufreq_init() the second time that it's called for the same policy
pointer? I have a system with two policies, so I made it fail and return
-EINVAL when the counter == 2 and I see the lockdep splat.
>
> I also did savedefconfig to show what all I changed in it. I faked multiple
> clusters on my hikey960 board, which is not big little..
>
> And here is the command list from history that I ran after boot.
>
> 501 grep . /sys/devices/system/cpu/cpufreq/*/*
> 502 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 503 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 504 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 505 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 506 grep . /sys/devices/system/cpu/cpufreq/*/*
> 507 grep . /sys/devices/system/cpu/cpufreq/*/*
> 508 echo 0 > /sys/devices/system/cpu/cpu4/online
> 509 echo 0 > /sys/devices/system/cpu/cpu5/online
> 510 echo 0 > /sys/devices/system/cpu/cpu6/online
> 511 echo 0 > /sys/devices/system/cpu/cpu7/online
> 512 grep . /sys/devices/system/cpu/cpufreq/*/*
> 513 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 514 grep . /sys/devices/system/cpu/cpufreq/*/*
> 515 echo 1 > /sys/devices/system/cpu/cpu4/online
> 516 grep . /sys/devices/system/cpu/cpufreq/*/*
> 517 grep . /sys/devices/system/cpu/cpufreq/*/*/*
> 518 dmesg
>
I did the following:
grep . /sys/devices/system/cpu/cpufreq/*/* >/dev/null
echo 0 > /sys/devices/system/cpu/cpu4/online
echo 0 > /sys/devices/system/cpu/cpu5/online
echo 0 > /sys/devices/system/cpu/cpu6/online
echo 0 > /sys/devices/system/cpu/cpu7/online
echo 1 > /sys/devices/system/cpu/cpu4/online
dmesg
And boom, lockdep splat.
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Hi Stephen,
On 13-12-18, 02:12, Stephen Boyd wrote:
> Quoting Viresh Kumar (2018-12-13 02:05:06)
> > On 13-12-18, 01:58, Stephen Boyd wrote:
> > > BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error
> > > upon bringing the policy online the second time. I guess cpufreq_stats
> > > aren't able to be freed from there because they take locks in different
> > > order vs. the normal path?
> >
> > Please share the lockdep report and the steps to reproduce it. I will
> > see if I can simulate the failure forcefully..
> >
>
> It's on a v4.19 kernel with this cpufreq hw driver backported to it. I
> think all it takes is to return an error the second time the policy is
> initialized when cpufreq_online() calls into the cpufreq driver.
>
> ==
> WARNING: possible circular locking dependency detected
> 4.19.8 #61 Tainted: GW
> --
> cpuhp/5/36 is trying to acquire lock:
> 3e901e8a (kn->count#326){}, at:
> kernfs_remove_by_name_ns+0x44/0x80
>
> but task is already holding lock:
> dd7f52c3 (>rwsem){}, at: cpufreq_policy_free+0x17c/0x1cc
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (>rwsem){}:
> down_read+0x50/0xcc
> show+0x30/0x78
> sysfs_kf_seq_show+0x17c/0x25c
> kernfs_seq_show+0xb4/0xf8
> seq_read+0x4a8/0x8f0
> kernfs_fop_read+0xe0/0x360
> __vfs_read+0x80/0x328
> vfs_read+0xd0/0x1d4
> ksys_read+0x88/0x118
> __arm64_sys_read+0x4c/0x5c
> el0_svc_common+0x124/0x1c4
> el0_svc_compat_handler+0x64/0x8c
> el0_svc_compat+0x8/0x18
I failed to reproduce it over linux/next.
I had the following changes over linux/next:
https://pastebin.ubuntu.com/p/zkVm77PGdY/
I also did savedefconfig to show what all I changed in it. I faked multiple
clusters on my hikey960 board, which is not big little..
And here is the command list from history that I ran after boot.
501 grep . /sys/devices/system/cpu/cpufreq/*/*
502 grep . /sys/devices/system/cpu/cpufreq/*/*/*
503 grep . /sys/devices/system/cpu/cpufreq/*/*/*
504 grep . /sys/devices/system/cpu/cpufreq/*/*/*
505 grep . /sys/devices/system/cpu/cpufreq/*/*/*
506 grep . /sys/devices/system/cpu/cpufreq/*/*
507 grep . /sys/devices/system/cpu/cpufreq/*/*
508 echo 0 > /sys/devices/system/cpu/cpu4/online
509 echo 0 > /sys/devices/system/cpu/cpu5/online
510 echo 0 > /sys/devices/system/cpu/cpu6/online
511 echo 0 > /sys/devices/system/cpu/cpu7/online
512 grep . /sys/devices/system/cpu/cpufreq/*/*
513 grep . /sys/devices/system/cpu/cpufreq/*/*/*
514 grep . /sys/devices/system/cpu/cpufreq/*/*
515 echo 1 > /sys/devices/system/cpu/cpu4/online
516 grep . /sys/devices/system/cpu/cpufreq/*/*
517 grep . /sys/devices/system/cpu/cpufreq/*/*/*
518 dmesg
--
viresh
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Hello Stephen, Viresh,
On 12/13/2018 3:28 PM, Stephen Boyd wrote:
Quoting Taniya Das (2018-12-12 23:49:54)
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
for changing the frequency of CPUs. The driver implements the cpufreq
driver interface for this hardware engine.
Signed-off-by: Saravana Kannan
Signed-off-by: Stephen Boyd
Signed-off-by: Taniya Das
---
Reviewed-by: Stephen Boyd
But I noticed that we don't release the I/O region anymore so hotplug
and replug of a whole clk domain fails. I guess devm_ioremap_resource()
was just too much magic so how about we downgrade to devm_ioremap()
instead?
BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error
upon bringing the policy online the second time. I guess cpufreq_stats
aren't able to be freed from there because they take locks in different
order vs. the normal path?
-8<---
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
b/drivers/cpufreq/qcom-cpufreq-hw.c
index fce7a1162e87..0e1105151478 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -182,9 +182,12 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy
*policy)
index = args.args[0];
res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
- base = devm_ioremap_resource(dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ if (!res)
+ return -ENODEV;
+
+ base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!base)
+ return -ENOMEM;
Updated the above in the next series.
/* HW should be in enabled state to proceed */
if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On Thu, Dec 13, 2018 at 01:19:54PM +0530, Taniya Das wrote:
> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
> for changing the frequency of CPUs. The driver implements the cpufreq
> driver interface for this hardware engine.
>
> Signed-off-by: Saravana Kannan
> Signed-off-by: Stephen Boyd
> Signed-off-by: Taniya Das
> ---
> drivers/cpufreq/Kconfig.arm | 11 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-hw.c | 305
> ++
> 3 files changed, 317 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 4e1131e..688f102 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -114,6 +114,17 @@ config ARM_QCOM_CPUFREQ_KRYO
>
> If in doubt, say N.
>
> +config ARM_QCOM_CPUFREQ_HW
> + tristate "QCOM CPUFreq HW driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + help
> + Support for the CPUFreq HW driver.
> + Some QCOM chipsets have a HW engine to offload the steps
> + necessary for changing the frequency of the CPUs. Firmware loaded
> + in this engine exposes a programming interface to the OS.
> + The driver implements the cpufreq interface for this HW engine.
> + Say Y if you want to support CPUFreq HW.
> +
> config ARM_S3C_CPUFREQ
> bool
> help
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index d5ee456..08c071b 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -61,6 +61,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_HW)+= qcom-cpufreq-hw.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
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
> b/drivers/cpufreq/qcom-cpufreq-hw.c
> new file mode 100644
> index 000..a65d0ae
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define LUT_MAX_ENTRIES 40U
> +#define LUT_SRC GENMASK(31, 30)
> +#define LUT_L_VALGENMASK(7, 0)
> +#define LUT_CORE_COUNT GENMASK(18, 16)
> +#define LUT_ROW_SIZE 32
> +#define CLK_HW_DIV 2
> +
> +/* Register offsets */
> +#define REG_ENABLE 0x0
> +#define REG_LUT_TABLE0x110
> +#define REG_PERF_STATE 0x920
> +
> +static unsigned long cpu_hw_rate, xo_rate;
> +static struct platform_device *global_pdev;
> +
> +static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> + unsigned int index)
> +{
> + void __iomem *perf_state_reg = policy->driver_data;
> +
> + writel_relaxed(index, perf_state_reg);
> +
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
> +{
> + void __iomem *perf_state_reg;
> + struct cpufreq_policy *policy;
> + unsigned int index;
> +
> + policy = cpufreq_cpu_get_raw(cpu);
> + if (!policy)
> + return 0;
> +
> + perf_state_reg = policy->driver_data;
> +
> + index = readl_relaxed(perf_state_reg);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy
> *policy,
> + unsigned int target_freq)
> +{
> + void __iomem *perf_state_reg = policy->driver_data;
> + int index;
> +
> + index = policy->cached_resolved_idx;
> + if (index < 0)
> + return 0;
> +
> + writel_relaxed(index, perf_state_reg);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_hw_read_lut(struct device *dev,
> + struct cpufreq_policy *policy,
> + void __iomem *base)
> +{
> + u32 data, src, lval, i, core_count, prev_cc = 0, prev_freq = 0, freq;
> + unsigned int max_cores = cpumask_weight(policy->cpus);
> + struct cpufreq_frequency_table *table;
> +
> + table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
> + if (!table)
> + return -ENOMEM;
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(base +
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On 13-12-18, 02:32, Stephen Boyd wrote: > I have one policy for four CPUs. So take down all four of those CPUs by > writing a 0 to the online file for each CPU, and then bring them back > online. That should make cpufreq_driver->init() be called twice, once > during boot when the CPUs are bound to the cpufreq devices, and second > from the sysfs write when the user brings the first CPU in that policy > online again by writing a 1 to the online file. If that second time it > fails I suspect we hit the lockdep warning. Okay, that is a special sequence. I will try to reproduce that locally. Thanks. -- viresh
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Viresh Kumar (2018-12-13 02:14:17) > On 13-12-18, 02:12, Stephen Boyd wrote: > > It's on a v4.19 kernel with this cpufreq hw driver backported to it. I > > think all it takes is to return an error the second time the policy is > > initialized when cpufreq_online() calls into the cpufreq driver. > > What do you mean by "the second time the policy is initialized" ? > > We call cpufreq_online() only once for each policy. > I have one policy for four CPUs. So take down all four of those CPUs by writing a 0 to the online file for each CPU, and then bring them back online. That should make cpufreq_driver->init() be called twice, once during boot when the CPUs are bound to the cpufreq devices, and second from the sysfs write when the user brings the first CPU in that policy online again by writing a 1 to the online file. If that second time it fails I suspect we hit the lockdep warning. I may have also catted the policy*/stat/* files before offlining any CPUs. If you can't trigger it I can try to reproduce again with a static counter in cpufreq_online() that fails when set to 1 or something.
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On 13-12-18, 02:12, Stephen Boyd wrote: > It's on a v4.19 kernel with this cpufreq hw driver backported to it. I > think all it takes is to return an error the second time the policy is > initialized when cpufreq_online() calls into the cpufreq driver. What do you mean by "the second time the policy is initialized" ? We call cpufreq_online() only once for each policy. -- viresh
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Viresh Kumar (2018-12-13 02:05:06)
> On 13-12-18, 01:58, Stephen Boyd wrote:
> > BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error
> > upon bringing the policy online the second time. I guess cpufreq_stats
> > aren't able to be freed from there because they take locks in different
> > order vs. the normal path?
>
> Please share the lockdep report and the steps to reproduce it. I will
> see if I can simulate the failure forcefully..
>
It's on a v4.19 kernel with this cpufreq hw driver backported to it. I
think all it takes is to return an error the second time the policy is
initialized when cpufreq_online() calls into the cpufreq driver.
==
WARNING: possible circular locking dependency detected
4.19.8 #61 Tainted: GW
--
cpuhp/5/36 is trying to acquire lock:
3e901e8a (kn->count#326){}, at: kernfs_remove_by_name_ns+0x44/0x80
but task is already holding lock:
dd7f52c3 (>rwsem){}, at: cpufreq_policy_free+0x17c/0x1cc
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (>rwsem){}:
down_read+0x50/0xcc
show+0x30/0x78
sysfs_kf_seq_show+0x17c/0x25c
kernfs_seq_show+0xb4/0xf8
seq_read+0x4a8/0x8f0
kernfs_fop_read+0xe0/0x360
__vfs_read+0x80/0x328
vfs_read+0xd0/0x1d4
ksys_read+0x88/0x118
__arm64_sys_read+0x4c/0x5c
el0_svc_common+0x124/0x1c4
el0_svc_compat_handler+0x64/0x8c
el0_svc_compat+0x8/0x18
-> #0 (kn->count#326){}:
lock_acquire+0x244/0x360
__kernfs_remove+0x324/0x4fc
kernfs_remove_by_name_ns+0x44/0x80
remove_files+0x5c/0xd8
sysfs_remove_group+0xb4/0xec
cpufreq_stats_free_table+0x50/0x9c
cpufreq_policy_free+0x184/0x1cc
cpufreq_online+0xa44/0xc4c
cpuhp_cpufreq_online+0x1c/0x2c
cpuhp_invoke_callback+0x608/0x1328
cpuhp_thread_fun+0x1dc/0x440
smpboot_thread_fn+0x46c/0x7c0
kthread+0x248/0x260
ret_from_fork+0x10/0x18
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0CPU1
lock(>rwsem);
lock(kn->count#326);
lock(>rwsem);
lock(kn->count#326);
*** DEADLOCK ***
2 locks held by cpuhp/5/36:
#0: 549a28c3 (cpuhp_state-up){+.+.}, at: cpuhp_lock_acquire+0x8/0x48
#1: dd7f52c3 (>rwsem){}, at:
cpufreq_policy_free+0x17c/0x1cc
stack backtrace:
CPU: 5 PID: 36 Comm: cpuhp/5 Tainted: GW 4.19.8 #61
Call trace:
dump_backtrace+0x0/0x2f8
show_stack+0x20/0x2c
__dump_stack+0x20/0x28
dump_stack+0xcc/0x10c
print_circular_bug+0x2c0/0x328
__lock_acquire+0x22b0/0x2708
lock_acquire+0x244/0x360
__kernfs_remove+0x324/0x4fc
kernfs_remove_by_name_ns+0x44/0x80
remove_files+0x5c/0xd8
sysfs_remove_group+0xb4/0xec
cpufreq_stats_free_table+0x50/0x9c
cpufreq_policy_free+0x184/0x1cc
cpufreq_online+0xa44/0xc4c
cpuhp_cpufreq_online+0x1c/0x2c
cpuhp_invoke_callback+0x608/0x1328
cpuhp_thread_fun+0x1dc/0x440
smpboot_thread_fn+0x46c/0x7c0
kthread+0x248/0x260
ret_from_fork+0x10/0x18
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On 13-12-18, 01:58, Stephen Boyd wrote: > BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error > upon bringing the policy online the second time. I guess cpufreq_stats > aren't able to be freed from there because they take locks in different > order vs. the normal path? Please share the lockdep report and the steps to reproduce it. I will see if I can simulate the failure forcefully.. -- viresh
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Taniya Das (2018-12-12 23:49:54)
> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
> for changing the frequency of CPUs. The driver implements the cpufreq
> driver interface for this hardware engine.
>
> Signed-off-by: Saravana Kannan
> Signed-off-by: Stephen Boyd
> Signed-off-by: Taniya Das
> ---
Reviewed-by: Stephen Boyd
But I noticed that we don't release the I/O region anymore so hotplug
and replug of a whole clk domain fails. I guess devm_ioremap_resource()
was just too much magic so how about we downgrade to devm_ioremap()
instead?
BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error
upon bringing the policy online the second time. I guess cpufreq_stats
aren't able to be freed from there because they take locks in different
order vs. the normal path?
-8<---
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
b/drivers/cpufreq/qcom-cpufreq-hw.c
index fce7a1162e87..0e1105151478 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -182,9 +182,12 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy
*policy)
index = args.args[0];
res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
- base = devm_ioremap_resource(dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ if (!res)
+ return -ENODEV;
+
+ base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!base)
+ return -ENOMEM;
/* HW should be in enabled state to proceed */
if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
On 13-12-18, 13:19, Taniya Das wrote: > The CPUfreq HW present in some QCOM chipsets offloads the steps necessary > for changing the frequency of CPUs. The driver implements the cpufreq > driver interface for this hardware engine. > > Signed-off-by: Saravana Kannan > Signed-off-by: Stephen Boyd > Signed-off-by: Taniya Das > --- > drivers/cpufreq/Kconfig.arm | 11 ++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/qcom-cpufreq-hw.c | 305 > ++ > 3 files changed, 317 insertions(+) > create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c Acked-by: Viresh Kumar -- viresh
[PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
for changing the frequency of CPUs. The driver implements the cpufreq
driver interface for this hardware engine.
Signed-off-by: Saravana Kannan
Signed-off-by: Stephen Boyd
Signed-off-by: Taniya Das
---
drivers/cpufreq/Kconfig.arm | 11 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/qcom-cpufreq-hw.c | 305 ++
3 files changed, 317 insertions(+)
create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 4e1131e..688f102 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -114,6 +114,17 @@ config ARM_QCOM_CPUFREQ_KRYO
If in doubt, say N.
+config ARM_QCOM_CPUFREQ_HW
+ tristate "QCOM CPUFreq HW driver"
+ depends on ARCH_QCOM || COMPILE_TEST
+ help
+ Support for the CPUFreq HW driver.
+ Some QCOM chipsets have a HW engine to offload the steps
+ necessary for changing the frequency of the CPUs. Firmware loaded
+ in this engine exposes a programming interface to the OS.
+ The driver implements the cpufreq interface for this HW engine.
+ Say Y if you want to support CPUFreq HW.
+
config ARM_S3C_CPUFREQ
bool
help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index d5ee456..08c071b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -61,6 +61,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_HW) += qcom-cpufreq-hw.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
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
b/drivers/cpufreq/qcom-cpufreq-hw.c
new file mode 100644
index 000..a65d0ae
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LUT_MAX_ENTRIES40U
+#define LUT_SRCGENMASK(31, 30)
+#define LUT_L_VAL GENMASK(7, 0)
+#define LUT_CORE_COUNT GENMASK(18, 16)
+#define LUT_ROW_SIZE 32
+#define CLK_HW_DIV 2
+
+/* Register offsets */
+#define REG_ENABLE 0x0
+#define REG_LUT_TABLE 0x110
+#define REG_PERF_STATE 0x920
+
+static unsigned long cpu_hw_rate, xo_rate;
+static struct platform_device *global_pdev;
+
+static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
+ unsigned int index)
+{
+ void __iomem *perf_state_reg = policy->driver_data;
+
+ writel_relaxed(index, perf_state_reg);
+
+ return 0;
+}
+
+static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
+{
+ void __iomem *perf_state_reg;
+ struct cpufreq_policy *policy;
+ unsigned int index;
+
+ policy = cpufreq_cpu_get_raw(cpu);
+ if (!policy)
+ return 0;
+
+ perf_state_reg = policy->driver_data;
+
+ index = readl_relaxed(perf_state_reg);
+ index = min(index, LUT_MAX_ENTRIES - 1);
+
+ return policy->freq_table[index].frequency;
+}
+
+static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
+ unsigned int target_freq)
+{
+ void __iomem *perf_state_reg = policy->driver_data;
+ int index;
+
+ index = policy->cached_resolved_idx;
+ if (index < 0)
+ return 0;
+
+ writel_relaxed(index, perf_state_reg);
+
+ return policy->freq_table[index].frequency;
+}
+
+static int qcom_cpufreq_hw_read_lut(struct device *dev,
+ struct cpufreq_policy *policy,
+ void __iomem *base)
+{
+ u32 data, src, lval, i, core_count, prev_cc = 0, prev_freq = 0, freq;
+ unsigned int max_cores = cpumask_weight(policy->cpus);
+ struct cpufreq_frequency_table *table;
+
+ table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
+ if (!table)
+ return -ENOMEM;
+
+ for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+ data = readl_relaxed(base + REG_LUT_TABLE + i * LUT_ROW_SIZE);
+ src = FIELD_GET(LUT_SRC, data);
+ lval = FIELD_GET(LUT_L_VAL, data);
+ core_count = FIELD_GET(LUT_CORE_COUNT, data);
+
+ if (src)
+ freq =
