> -----Original Message----- > From: Burakov, Anatoly <[email protected]> > Sent: Thursday, April 22, 2021 6:00 PM > To: Richael Zhuang <[email protected]>; [email protected] > Cc: nd <[email protected]>; David Hunt <[email protected]> > Subject: Re: [dpdk-dev] [PATCH v1 1/1] power: add support for cppc cpufreq > > On 22-Apr-21 10:29 AM, Richael Zhuang wrote: > > > > > >> -----Original Message----- > >> From: Burakov, Anatoly <[email protected]> > >> Sent: Thursday, April 22, 2021 5:06 PM > >> To: Richael Zhuang <[email protected]>; [email protected] > >> Cc: nd <[email protected]>; David Hunt <[email protected]> > >> Subject: Re: [dpdk-dev] [PATCH v1 1/1] power: add support for cppc > >> cpufreq > >> > >> On 22-Apr-21 7:15 AM, Richael Zhuang wrote: > >>> Currently in DPDK only acpi_cpufreq and pstate_cpufreq drivers are > >>> supported, which are both not available on arm64 platforms. Add > >>> support for cppc_cpufreq driver which works on most arm64 platforms. > >>> > >>> Signed-off-by: Richael Zhuang <[email protected]> > >>> --- > >> > >> Just a general note: this looks like a copy-paste of pstate code. > >> Which is perfectly fine, except that we can do better than copying > >> some faults of the pstate code to other drivers. I've submitted a > >> patch [1] attempting to fix some of the pressing issues and code > >> duplication in pstate driver, but i'm sure with a fresh driver, you > >> can do even better :) > >> > >> [1] > >> http://patches.dpdk.org/project/dpdk/patch/20210402092701.258316-1- > >> [email protected]/ > >> > >> -- > >> Thanks, > >> Anatoly > > > > For CPPC is defined in acpi v5.0+ spec, I reused most code in acpi_cpufreq > to get a quick workable version on our platform with only cppc driver. I have > verified its basic functions. If you find some problems please help to point > out thus I can rework it. Thanks . > > > > Best Regards, > > Richael > > > > Well, pstate code was copied from ACPI so it does share the same flaws: > > - Lots of code duplication (e.g. snprintf for filename, fopen sequences, > etc.) > - Confusing and bug-prone error handling (e.g. return macros in the middle > of a function) > - Mixing power management logic and gory details of string handling > > Good examples of the above are in your `power_check_turbo()` function - > lots of string handling code interspersed with file opens, and actual logic of > power management. > > Please see the patch i linked earlier [1] to understand what kind of changes > i'm suggesting. Perhaps you could do even better :) > > [1] > http://patches.dpdk.org/project/dpdk/patch/20210402092701.258316-1- > [email protected]/ > > -- > Thanks, > Anatoly Thanks. I'll rework it to make it look better.
Best Regards, Richael
