From: Peng Fan <[email protected]>
cpufreq_init could be used to do check clk/regulator check,
there is no need to duplicate the work in resources_available.
Signed-off-by: Peng Fan <[email protected]>
---
V1:
Actually we met an issue as below. Per my analysis,
it is regulator_put called in resources_available not remove the devlink
before dev_pm_opp_set_regulators->_regulator_get.
I tried to add a check patch:
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 0971fedeec7d..4f8c7c13bde7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -760,6 +760,7 @@ static void __device_link_del(struct kref *kref)
list_del_rcu(&link->s_node);
list_del_rcu(&link->c_node);
call_srcu(&device_links_srcu, &link->rcu_head, __device_link_free_srcu);
+ synchronize_srcu(&device_links_srcu);
}
It could also fix the warning dump, I not find a good solution about srcu part.
But since we could optimize code to delay the clk/regulator check, I worked out
this patch.
[ 6.799650] sysfs: cannot create duplicate filename '/devices/virtual/devlink/regulator.1--cpu0'
[ 6.808725] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.8.0-next-20200807-dirty #272
[ 6.817868] Hardware name: Freescale i.MX7ULP (Device Tree)
[ 6.823486] [<c01127c0>] (unwind_backtrace) from [<c010c598>] (show_stack+0x10/0x14)
[ 6.831252] [<c010c598>] (show_stack) from [<c05bc410>] (dump_stack+0xd8/0x10c)
[ 6.838585] [<c05bc410>] (dump_stack) from [<c036d5b8>] (sysfs_warn_dup+0x50/0x64)
[ 6.846181] [<c036d5b8>] (sysfs_warn_dup) from [<c036d6e4>] (sysfs_create_dir_ns+0xd4/0xf4)
[ 6.854554] [<c036d6e4>] (sysfs_create_dir_ns) from [<c05c2734>] (kobject_add_internal+0xa0/0x2ec)
[ 6.863531] [<c05c2734>] (kobject_add_internal) from [<c05c29d8>] (kobject_add+0x58/0xc0)
[ 6.871732] [<c05c29d8>] (kobject_add) from [<c07939d4>] (device_add+0xec/0x7c8)
[ 6.879150] [<c07939d4>] (device_add) from [<c0795600>] (device_link_add+0x3cc/0x534)
[ 6.887005] [<c0795600>] (device_link_add) from [<c0694244>] (_regulator_get+0xe8/0x274)
[ 6.895116] [<c0694244>] (_regulator_get) from [<c09f6568>] (dev_pm_opp_set_regulators+0x9c/0x1e8)
[ 6.904092] [<c09f6568>] (dev_pm_opp_set_regulators) from [<c09ffa4c>] (cpufreq_init+0xb4/0x2cc)
[ 6.912897] [<c09ffa4c>] (cpufreq_init) from [<c09fc484>] (cpufreq_online+0x268/0x92c)
[ 6.920837] [<c09fc484>] (cpufreq_online) from [<c09fcbb8>] (cpufreq_add_dev+0x60/0x78)
[ 6.928863] [<c09fcbb8>] (cpufreq_add_dev) from [<c0796f38>] (subsys_interface_register+0xa0/0xf0)
[ 6.937843] [<c0796f38>] (subsys_interface_register) from [<c09fa304>] (cpufreq_register_driver+0x14c/0x228)
[ 6.947684] [<c09fa304>] (cpufreq_register_driver) from [<c09ffd28>] (dt_cpufreq_probe+0xc4/0x140)
[ 6.956663] [<c09ffd28>] (dt_cpufreq_probe) from [<c079adcc>] (platform_drv_probe+0x48/0x98)
[ 6.965120] [<c079adcc>] (platform_drv_probe) from [<c07988cc>] (really_probe+0x214/0x3b4)
[ 6.973405] [<c07988cc>] (really_probe) from [<c0798bd4>] (driver_probe_device+0x58/0xb4)
[ 6.981604] [<c0798bd4>] (driver_probe_device) from [<c0796c08>] (bus_for_each_drv+0x54/0xb8)
[ 6.990150] [<c0796c08>] (bus_for_each_drv) from [<c079863c>] (__device_attach+0xdc/0x150)
[ 6.998435] [<c079863c>] (__device_attach) from [<c07978f8>] (bus_probe_device+0x88/0x90)
[ 7.006635] [<c07978f8>] (bus_probe_device) from [<c0793da0>] (device_add+0x4b8/0x7c8)
[ 7.014572] [<c0793da0>] (device_add) from [<c079aba0>] (platform_device_add+0x100/0x208)
[ 7.022772] [<c079aba0>] (platform_device_add) from [<c079b65c>] (platform_device_register_full+0x104/0x114)
[ 7.032617] [<c079b65c>] (platform_device_register_full) from [<c0a01028>] (imx_cpufreq_dt_probe+0xdc/0x2c0)
[ 7.042461] [<c0a01028>] (imx_cpufreq_dt_probe) from [<c079adcc>] (platform_drv_probe+0x48/0x98)
[ 7.051265] [<c079adcc>] (platform_drv_probe) from [<c07988cc>] (really_probe+0x214/0x3b4)
[ 7.059550] [<c07988cc>] (really_probe) from [<c0798bd4>] (driver_probe_device+0x58/0xb4)
[ 7.067742] [<c0798bd4>] (driver_probe_device) from [<c0796c08>] (bus_for_each_drv+0x54/0xb8)
[ 7.076288] [<c0796c08>] (bus_for_each_drv) from [<c079863c>] (__device_attach+0xdc/0x150)
[ 7.084572] [<c079863c>] (__device_attach) from [<c07978f8>] (bus_probe_device+0x88/0x90)
[ 7.092773] [<c07978f8>] (bus_probe_device) from [<c0793da0>] (device_add+0x4b8/0x7c8)
[ 7.100710] [<c0793da0>] (device_add) from [<c079aba0>] (platform_device_add+0x100/0x208)
[ 7.108909] [<c079aba0>] (platform_device_add) from [<c079b65c>] (platform_device_register_full+0x104/0x114)
[ 7.118760] [<c079b65c>] (platform_device_register_full) from [<c150f7f4>] (imx7ulp_init_late+0x44/0x70)
[ 7.128260] [<c150f7f4>] (imx7ulp_init_late) from [<c15036b4>] (init_machine_late+0x1c/0x8c)
[ 7.136726] [<c15036b4>] (init_machine_late) from [<c010247c>] (do_one_initcall+0x80/0x424)
[ 7.145095] [<c010247c>] (do_one_initcall) from [<c1501018>] (kernel_init_freeable+0x170/0x1f0)
[ 7.153818] [<c1501018>] (kernel_init_freeable) from [<c0e7b938>] (kernel_init+0x8/0x120)
[ 7.162020] [<c0e7b938>] (kernel_init) from [<c0100134>] (ret_from_fork+0x14/0x20)
[ 7.169599] Exception stack(0xec0c5fb0 to 0xec0c5ff8)
[ 7.174665] 5fa0: 00000000 00000000 00000000 00000000
[ 7.182858] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 7.191045] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 7.198202] kobject_add_internal failed for regulator.1--cpu0 with -EEXIST, don't try to register things with the same name in the same directory.
drivers/cpufreq/cpufreq-dt.c | 58 +++++++++++-------------------------
1 file changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 944d7b45afe9..13b291757796 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -93,10 +93,8 @@ static const char *find_supply_name(struct device *dev)
static int resources_available(void)
{
struct device *cpu_dev;
- struct regulator *cpu_reg;
- struct clk *cpu_clk;
- int ret = 0;
const char *name;
+ int ret;
cpu_dev = get_cpu_device(0);
if (!cpu_dev) {
@@ -104,23 +102,6 @@ static int resources_available(void)
return -ENODEV;
}
- cpu_clk = clk_get(cpu_dev, NULL);
- ret = PTR_ERR_OR_ZERO(cpu_clk);
- if (ret) {
- /*
- * If cpu's clk node is present, but clock is not yet
- * registered, we should try defering probe.
- */
- if (ret == -EPROBE_DEFER)
- dev_dbg(cpu_dev, "clock not ready, retry\n");
- else
- dev_err(cpu_dev, "failed to get clock: %d\n", ret);
-
- return ret;
- }
-
- clk_put(cpu_clk);
-
ret = dev_pm_opp_of_find_icc_paths(cpu_dev, NULL);
if (ret)
return ret;
@@ -130,22 +111,6 @@ static int resources_available(void)
if (!name)
return 0;
- cpu_reg = regulator_get_optional(cpu_dev, name);
- ret = PTR_ERR_OR_ZERO(cpu_reg);
- if (ret) {
- /*
- * If cpu's regulator supply node is present, but regulator is
- * not yet registered, we should try defering probe.
- */
- if (ret == -EPROBE_DEFER)
- dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n");
- else
- dev_dbg(cpu_dev, "no regulator for cpu0: %d\n", ret);
-
- return ret;
- }
-
- regulator_put(cpu_reg);
return 0;
}
@@ -168,9 +133,17 @@ static int cpufreq_init(struct cpufreq_policy *policy)
}
cpu_clk = clk_get(cpu_dev, NULL);
- if (IS_ERR(cpu_clk)) {
- ret = PTR_ERR(cpu_clk);
- dev_err(cpu_dev, "%s: failed to get clk: %d\n", __func__, ret);
+ ret = PTR_ERR_OR_ZERO(cpu_clk);
+ if (ret) {
+ /*
+ * If cpu's clk node is present, but clock is not yet
+ * registered, we should try defering probe.
+ */
+ if (ret == -EPROBE_DEFER)
+ dev_dbg(cpu_dev, "clock not ready, retry\n");
+ else
+ dev_err(cpu_dev, "%s: failed to get clk: %d\n", __func__, ret);
+
return ret;
}
@@ -198,8 +171,11 @@ static int cpufreq_init(struct cpufreq_policy *policy)
opp_table = dev_pm_opp_set_regulators(cpu_dev, &name, 1);
if (IS_ERR(opp_table)) {
ret = PTR_ERR(opp_table);
- dev_err(cpu_dev, "Failed to set regulator for cpu%d: %d\n",
- policy->cpu, ret);
+ if (ret == -EPROBE_DEFER)
+ dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n");
+ else
+ dev_err(cpu_dev, "Failed to set regulator for cpu%d: %d\n",
+ policy->cpu, ret);
goto out_put_clk;
}
}
--
2.28.0
On 31-08-20, 16:48, [email protected] wrote:
> From: Peng Fan <[email protected]>
>
> cpufreq_init could be used to do check clk/regulator check,
> there is no need to duplicate the work in resources_available.
>
> Signed-off-by: Peng Fan <[email protected]>
Hi can you please see if this happens on linux-next as well ? The
routine resources_available() isn't there anymore.
--
viresh
> Subject: Re: [PATCH] cpufreq: dt: delay the clk/regulator check
>
> On 31-08-20, 16:48, [email protected] wrote:
> > From: Peng Fan <[email protected]>
> >
> > cpufreq_init could be used to do check clk/regulator check, there is
> > no need to duplicate the work in resources_available.
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> Hi can you please see if this happens on linux-next as well ? The routine
> resources_available() isn't there anymore.
Not happen in Linux-next after I rebase.
Thanks,
Peng.
>
> --
> viresh