lkml.org 
[lkml]   [2021]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] cpufreq: dt: check the error returned by dev_pm_opp_of_cpumask_add_table
    From
    Date
    Hi Viresh,

    On 3/25/21 12:45 PM, Viresh Kumar wrote:
    > On 25-03-21, 12:31, quanyang.wang@windriver.com wrote:
    >> From: Quanyang Wang <quanyang.wang@windriver.com>
    >>
    >> The function dev_pm_opp_of_cpumask_add_table may return zero or an
    >> error. When it returns an error, this means that no OPP table is
    >> added for the cpumask because _dev_pm_opp_cpumask_remove_table is
    >> called to free all OPPs associated with the cpu devices in the error
    >> label "remove_table". So continuing to run the next function
    >> dev_pm_opp_get_opp_count is meaningless since it always return the
    >> count value as 0.
    >>
    >> There is another reason why we should check the error returned by
    >> dev_pm_opp_of_cpumask_add_table is that it may return -EPROBE_DEFER
    >> which comes from clk_get(dev, NULL) in _update_opp_table_clk. When
    >> the clk for cpu device isn't ready, dt_cpufreq_probe should be deferred
    >> and wait to be called again. But if we ignore the return error of
    >> dev_pm_opp_of_cpumask_add_table, dt_cpufreq_probe will return -ENODEV
    >> because dev_pm_opp_get_opp_count returns the count value as 0,
    >> the cpufreq-dt driver will fail with the error log as below:
    >>
    >> [ 0.724069] cpu cpu0: OPP table can't be empty
    >>
    >> Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
    >> ---
    >> drivers/cpufreq/cpufreq-dt.c | 12 +++++++++---
    >> 1 file changed, 9 insertions(+), 3 deletions(-)
    >>
    >> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
    >> index b1e1bdc63b01..f24359f47b1a 100644
    >> --- a/drivers/cpufreq/cpufreq-dt.c
    >> +++ b/drivers/cpufreq/cpufreq-dt.c
    >> @@ -255,10 +255,16 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
    >> * before updating priv->cpus. Otherwise, we will end up creating
    >> * duplicate OPPs for the CPUs.
    >> *
    >> - * OPPs might be populated at runtime, don't check for error here.
    > As the comment (which you removed) clearly says, the OPPs maybe added
    > at runtime, don't check for error here.
    >
    > When we say runtime, we mean someone may have called dev_pm_opp_add()
    > for the devices.

    Thank you for pointing it out.  Do you mean that even if
    dev_pm_opp_of_cpumask_add_table returns

    an error, dev_pm_opp_get_opp_count may still return count > 0 because
    someone may call dev_pm_opp_add

    to add OPP to cpu succcessfully at somewhere else?

    Thanks,

    Quanyang

    >
    >> + * We need check the return value here, if it is non-zero, there is
    >> + * need to go on.
    >> */
    >> - if (!dev_pm_opp_of_cpumask_add_table(priv->cpus))
    >> - priv->have_static_opps = true;
    >> + ret = dev_pm_opp_of_cpumask_add_table(priv->cpus);
    >> + if (ret) {
    >> + dev_err(cpu_dev, "Failed to add OPP table for CPUs\n");
    >> + goto out;
    >> + }
    >> +
    >> + priv->have_static_opps = true;
    >>
    >> /*
    >> * The OPP table must be initialized, statically or dynamically, by this

    \
     
     \ /
      Last update: 2021-03-25 06:17    [W:3.979 / U:0.420 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site