Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Thara Gopinath]

On 6/28/21 10:50 PM, Taniya Das wrote:
> On 6/24/2021 5:28 PM, Thara Gopinath wrote:
> > Add interrupt support to notify the kernel of h/w initiated frequency
> > throttling by LMh. Convey this to scheduler via thermal presssure
> > interface.
> >
> > Signed-off-by: Thara Gopinath <thara.gopinath@xxxxxxxxxx>
> > ---
> >
> > v1->v2:
> >     - Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related 
> > initializations
> >       as per Viresh's review comment.
> >     - Moved the piece of code restarting polling/re-enabling LMh 
> > interrupt to
> >       qcom_lmh_dcvs_notify therby simplifying isr and timer callback 
> > as per Viresh's
> >       suggestion.
> >     - Droped cpus from qcom_cpufreq_data and instead using cpus from 
> > cpufreq_policy in
> >       qcom_lmh_dcvs_notify as per Viresh's review comment.
> >     - Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
> >     - Other minor/cosmetic fixes
> >
> >   drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
> >   1 file changed, 103 insertions(+)
> >
> > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
> > b/drivers/cpufreq/qcom-cpufreq-hw.c
> > index f86859bf76f1..241f6f2b441f 100644
> > --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c

[snip]

> >   static const struct qcom_cpufreq_soc_data qcom_soc_data = {
> >       .reg_enable = 0x0,
> >       .reg_freq_lut = 0x110,
> >       .reg_volt_lut = 0x114,
> > +    .reg_current_vote = 0x704,
> >       .reg_perf_state = 0x920,
> >       .lut_row_size = 32,
> >   };
> > @@ -274,6 +350,23 @@ static const struct of_device_id 
> > qcom_cpufreq_hw_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
> > +static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
> > +{
> > +    struct qcom_cpufreq_data *data = policy->driver_data;
> > +    struct platform_device *pdev = cpufreq_get_driver_data();
> > +    struct device *dev = &pdev->dev;
> > +    int ret;
> > +
> > +    ret = devm_request_irq(dev, data->lmh_dcvs_irq, 
> > qcom_lmh_dcvs_handle_irq,
> > +                   0, "dcvsh-irq", data);
>
>
> It is better if you tag the CPU id while registering the IRQ.
> "dcvsh-irq-x" (0/4/7)

Sure. Will fix it.

> > +    if (ret) {
> > +        dev_err(dev, "Error %d registering irq %x\n", ret, 
> > data->lmh_dcvs_irq);
> > +        return;
> > +    }
> > +    data->policy = policy;
> > +    INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
> > +}
> > +
> >   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
> >   {
> >       struct platform_device *pdev = cpufreq_get_driver_data();
> > @@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct 
> > cpufreq_policy *policy)
> >               dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
> >       }
> > +    /* Look for LMh interrupt. If no interrupt line is specified /
> > +     * if there is an error, allow cpufreq to be enabled as usual.
> > +     */
> > +    data->lmh_dcvs_irq = platform_get_irq(pdev, index);
> > +    if (data->lmh_dcvs_irq > 0) {
> > +        qcom_cpufreq_hw_lmh_init(policy);
> > +    } else if (data->lmh_dcvs_irq != -ENXIO) {
> > +        ret = data->lmh_dcvs_irq;
> > +        goto error;
> > +    }
> >       return 0;
> >   error:
> >       kfree(data);

--
Warm Regards
Thara (She/Her/Hers)