lkml.org 
[lkml]   [2022]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH V2 13/15] cpufreq: mediatek: Link CCI device to CPU
    From
    Date
    On Wed, 2022-04-13 at 14:41 -0700, Kevin Hilman wrote:
    > Rex-BC Chen <rex-bc.chen@mediatek.com> writes:
    >
    > [...]
    >
    > > From the Chanwoo's devfreq passive govonor series, it's impossible
    > > to
    > > let cci devreq probed done before cpufreq because the passive
    > > govonor
    > > will search for cpufreq node and use it.
    > >
    > > Ref: function: cpufreq_passive_register_notifier()
    > >
    > >
    https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=b670978ddc43eb0c60735c3af6e4a370603ab673__;!!CTRNKA9wMg0ARbw!z58Lc1p9REo88oHn-NkxroN_fBd0TsHYmhscNZwnWwT71ecRkTeqZ6vFl5l7HpkTdM6t$
    > >
    >
    > Well this is a problem, because CCI depends on CPUfreq, but CPUfreq
    > depends on CCI, so one of them has to load and then wait for the
    > other.
    >
    > > After I discuss with Angelo and Jia-wei, we think we are keeping
    > > the
    > > function in target_index and if the cci is not ready we will use
    > > the
    > > voltage which is set by bootloader to prevent high freqeuncy low
    > > voltage crash. And then we can keep seting the target frequency.
    > >
    >
    > > We assume the setting of bootloader is correct and we can do this.
    >
    > I'm still not crazy about this because you're lying to the CPUfreq
    > framework. It's requesting one OPP, but you're not setting that,
    > you're
    > just keeping the bootloader frequency.
    >
    > In my earlier reply, I gave two other options for handling this.
    >
    > 1) set a (temporary) constraint on the voltage regulator so that it
    > cannot change.
    >
    > or more clean, IMO:
    >
    > 2) set a CPUfreq policy that restricts available OPPs to ones that
    > will
    > not break CCI.
    >
    > Either of these solutions allow you to load the CPUfreq driver early,
    > and then wait for the CCI driver to be ready before removing the
    > restrictions.

    Hello Kevin,

    I think I do not describe this clearly.
    The proposal is:

    In cpufreq probe:
    we record the voltage value which is set by bootloader.

    In mtk_cpufreq_set_target():
    We do NOT directly return 0.
    Instead, we will find the voltage of target cpufreq and use the value
    max(booting voltage, target cpufreq voltage)

    mtk_cpufreq_set_target() {
    /* NOT return 0 if !is_ccifreq_ready */
    ....
    vproc = get voltage of target cpufreq from opp.

    if (ccifreq_supported && !is_ccifreq_ready)
    vproc = max(vproc, vproc_on_boot)

    //setting voltage and target frequency
    ....
    }

    >
    > > For the SoCs that including ci hardware (8183 and 8186), we think
    > > it's
    > > not ok if we don't probe cci correctly.
    > > If we failed to get cci node, I think we sould return -ENODEV and
    > > the
    > > probe of cpufreq failed.
    > >
    > > What do you think the solution?
    >
    > I think it would be better if CPUfreq probes sucessfully, but
    > restricts
    > the OPPs available until CCI is ready. If CCI fails to probe/load,
    > you
    > still have a working CPUfreq driver, it just has a restricted set of
    > OPPs.
    >
    > Kevin

    If we can use the solution.
    I think it will be ok for this situation.

    Thanks!

    BRs,
    Rex

    \
     
     \ /
      Last update: 2022-04-14 04:33    [W:2.568 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site