On 22-Apr-21 11:02 AM, Richael Zhuang wrote:
-----Original Message-----
From: Burakov, Anatoly <anatoly.bura...@intel.com>
Sent: Thursday, April 22, 2021 6:00 PM
To: Richael Zhuang <richael.zhu...@arm.com>; dev@dpdk.org
Cc: nd <n...@arm.com>; David Hunt <david.h...@intel.com>
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 <anatoly.bura...@intel.com>
Sent: Thursday, April 22, 2021 5:06 PM
To: Richael Zhuang <richael.zhu...@arm.com>; dev@dpdk.org
Cc: nd <n...@arm.com>; David Hunt <david.h...@intel.com>
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 <richael.zhu...@arm.com>
---
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-
anatoly.bura...@intel.com/
--
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-
anatoly.bura...@intel.com/
--
Thanks,
Anatoly
Thanks. I'll rework it to make it look better.
Best Regards,
Richael
Hi,
FYI i've updated my refactor patch [1] so that you could base your work
off it and not have to do most of it yourself. Feel free to take over
the patchset if you like!
[1]
http://patches.dpdk.org/project/dpdk/patch/83b1e89d14834251d4d7e72fcc19d82dfb52686d.1619175790.git.anatoly.bura...@intel.com/
--
Thanks,
Anatoly
