On Thursday, February 14, 2019 2:58:21 PM CET Xiongfeng Wang wrote:
>
> On 2019/2/14 18:58, Rafael J. Wysocki wrote:
> > On Thu, Feb 14, 2019 at 8:46 AM Xiongfeng Wang
> > <wangxiongfeng2@xxxxxxxxxx> wrote:
> >>
> >> Hisilicon chips do not support delivered performance counter register
> >> and reference performance counter register. But the platform can
> >> calculate the real performance using its own method. This patch provide
> >> a workaround for this problem, and other platforms can also use this
> >> workaround framework. We reuse the desired performance register to
> >> store the real performance calculated by the platform. After the
> >> platform finished the frequency adjust, it gets the real performance and
> >> writes it into desired performance register. OS can use it to calculate
> >> the real frequency.
> >>
> >> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@xxxxxxxxxx>
> >> ---
> >> drivers/cpufreq/cppc_cpufreq.c | 70 ++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 70 insertions(+)
> >>
> >> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> >> index fd25c21c..da96fec 100644
> >> --- a/drivers/cpufreq/cppc_cpufreq.c
> >> +++ b/drivers/cpufreq/cppc_cpufreq.c
> >> @@ -33,6 +33,16 @@
> >> /* Offest in the DMI processor structure for the max frequency */
> >> #define DMI_PROCESSOR_MAX_SPEED 0x14
> >>
> >> +struct cppc_workaround_info {
> >> + char oem_id[ACPI_OEM_ID_SIZE +1];
> >> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> >> + u32 oem_revision;
> >> + unsigned int (*get_rate)(unsigned int cpu);
> >> +};
> >> +
> >> +/* CPPC workaround for get_rate callback */
> >> +unsigned int (*cppc_wa_get_rate)(unsigned int cpu);
> >> +
> >
> > First off, please don't split the workaround material into two parts.
> > IOW, the other new function added below can go here just fine IMO.
> >
> >> /*
> >> * These structs contain information parsed from per CPU
> >> * ACPI _CPC structures.
> >> @@ -334,6 +344,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
> >> struct cppc_cpudata *cpu = all_cpu_data[cpunum];
> >> int ret;
> >>
> >> + if (cppc_wa_get_rate)
> >> + return cppc_wa_get_rate(cpunum);
> >
> > Second, what is the value of using the function pointer above?
> >
> > All we need for now is a flag to indicate whether or not to call
> > hisi_cppc_cpufreq_get_rate() here and return its return value.
>
> How about adding a pointer of 'struct cppc_workaround_info' to indicate whether we have
> found a matches workaround ?
> If I use a flag, I will need another variable to indicate which item of the workaround array 'wa_info'
> to use.
And why do you need to distinguish one of them from the other?
