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

[Thara Gopinath <thara.gopinath@linaro.org>]

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@linaro.org>
>> ---
>>
>> 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)