From: Peng Fan <peng....@nxp.com>

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 <peng....@nxp.com>
---

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

Reply via email to