From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F14BCC433DF for ; Tue, 23 Jun 2020 07:04:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D746720724 for ; Tue, 23 Jun 2020 07:04:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730776AbgFWHEH (ORCPT ); Tue, 23 Jun 2020 03:04:07 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:6386 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730765AbgFWHEH (ORCPT ); Tue, 23 Jun 2020 03:04:07 -0400 Received: from DGGEMS409-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 14EE8C2AE916BDDE6F2F; Tue, 23 Jun 2020 15:04:05 +0800 (CST) Received: from [127.0.0.1] (10.166.215.101) by DGGEMS409-HUB.china.huawei.com (10.3.19.209) with Microsoft SMTP Server id 14.3.487.0; Tue, 23 Jun 2020 15:03:59 +0800 Subject: Re: [PATCH] cpufreq: cppc: Reorder code and remove apply_hisi_workaround variable To: Viresh Kumar , Rafael Wysocki CC: , Vincent Guittot , References: From: Xiongfeng Wang Message-ID: Date: Tue, 23 Jun 2020 15:03:59 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.166.215.101] X-CFilter-Loop: Reflected Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org Hi Viresh, On 2020/6/23 13:15, Viresh Kumar wrote: > With the current approach we have an extra check in the > cppc_cpufreq_get_rate() callback, which checks if hisilicon's get rate > implementation should be used instead. While it works fine, the approach > isn't very straight forward, over that we have an extra check in the > routine. > > Rearrange code and update the cpufreq driver's get() callback pointer > directly for the hisilicon case. This gets the extra variable is removed > and the extra check isn't required anymore as well. > > Signed-off-by: Viresh Kumar > --- > Xiongfeng Wang, will it be possible for you to give this a try as I > can't really test it locally. I have tested it on D05. It works well. Tested-by: Xiongfeng Wang Thanks, Xiongfeng > > drivers/cpufreq/cppc_cpufreq.c | 91 ++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 49 deletions(-) > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c > index 257d726a4456..03a21daddbec 100644 > --- a/drivers/cpufreq/cppc_cpufreq.c > +++ b/drivers/cpufreq/cppc_cpufreq.c > @@ -45,8 +45,6 @@ struct cppc_workaround_oem_info { > u32 oem_revision; > }; > > -static bool apply_hisi_workaround; > - > static struct cppc_workaround_oem_info wa_info[] = { > { > .oem_id = "HISI ", > @@ -59,50 +57,6 @@ static struct cppc_workaround_oem_info wa_info[] = { > } > }; > > -static unsigned int cppc_cpufreq_perf_to_khz(struct cppc_cpudata *cpu, > - unsigned int perf); > - > -/* > - * HISI platform does not support delivered performance counter and > - * reference performance counter. It can calculate the performance using the > - * platform specific mechanism. We reuse the desired performance register to > - * store the real performance calculated by the platform. > - */ > -static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpunum) > -{ > - struct cppc_cpudata *cpudata = all_cpu_data[cpunum]; > - u64 desired_perf; > - int ret; > - > - ret = cppc_get_desired_perf(cpunum, &desired_perf); > - if (ret < 0) > - return -EIO; > - > - return cppc_cpufreq_perf_to_khz(cpudata, desired_perf); > -} > - > -static void cppc_check_hisi_workaround(void) > -{ > - struct acpi_table_header *tbl; > - acpi_status status = AE_OK; > - int i; > - > - status = acpi_get_table(ACPI_SIG_PCCT, 0, &tbl); > - if (ACPI_FAILURE(status) || !tbl) > - return; > - > - for (i = 0; i < ARRAY_SIZE(wa_info); i++) { > - if (!memcmp(wa_info[i].oem_id, tbl->oem_id, ACPI_OEM_ID_SIZE) && > - !memcmp(wa_info[i].oem_table_id, tbl->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) && > - wa_info[i].oem_revision == tbl->oem_revision) { > - apply_hisi_workaround = true; > - break; > - } > - } > - > - acpi_put_table(tbl); > -} > - > /* Callback function used to retrieve the max frequency from DMI */ > static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private) > { > @@ -402,9 +356,6 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) > struct cppc_cpudata *cpu = all_cpu_data[cpunum]; > int ret; > > - if (apply_hisi_workaround) > - return hisi_cppc_cpufreq_get_rate(cpunum); > - > ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t0); > if (ret) > return ret; > @@ -455,6 +406,48 @@ static struct cpufreq_driver cppc_cpufreq_driver = { > .name = "cppc_cpufreq", > }; > > +/* > + * HISI platform does not support delivered performance counter and > + * reference performance counter. It can calculate the performance using the > + * platform specific mechanism. We reuse the desired performance register to > + * store the real performance calculated by the platform. > + */ > +static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpunum) > +{ > + struct cppc_cpudata *cpudata = all_cpu_data[cpunum]; > + u64 desired_perf; > + int ret; > + > + ret = cppc_get_desired_perf(cpunum, &desired_perf); > + if (ret < 0) > + return -EIO; > + > + return cppc_cpufreq_perf_to_khz(cpudata, desired_perf); > +} > + > +static void cppc_check_hisi_workaround(void) > +{ > + struct acpi_table_header *tbl; > + acpi_status status = AE_OK; > + int i; > + > + status = acpi_get_table(ACPI_SIG_PCCT, 0, &tbl); > + if (ACPI_FAILURE(status) || !tbl) > + return; > + > + for (i = 0; i < ARRAY_SIZE(wa_info); i++) { > + if (!memcmp(wa_info[i].oem_id, tbl->oem_id, ACPI_OEM_ID_SIZE) && > + !memcmp(wa_info[i].oem_table_id, tbl->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) && > + wa_info[i].oem_revision == tbl->oem_revision) { > + /* Overwrite the get() callback */ > + cppc_cpufreq_driver.get = hisi_cppc_cpufreq_get_rate; > + break; > + } > + } > + > + acpi_put_table(tbl); > +} > + > static int __init cppc_cpufreq_init(void) > { > int i, ret = 0; >