Re: [PATCH v1] power: support amd-pstate cpufreq driver

[Thomas Monjalon <thomas@monjalon.net>]

12/04/2023 11:52, Sivaprasad Tummala:
> amd-pstate introduces a new CPU frequency control mechanism for AMD
> processors using the ACPI Collaborative Performance Power Control
> feature for a finer grained frequency management.
> 
> Patch to add support for amd-pstate driver.
> 
> Signed-off-by: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> ---
>  app/test/test_power.c                  |   1 +
>  app/test/test_power_cpufreq.c          |   5 +-
>  doc/guides/rel_notes/release_23_07.rst |   3 +
>  examples/l3fwd-power/main.c            |   1 +
>  lib/power/meson.build                  |   1 +
>  lib/power/power_amd_pstate_cpufreq.c   | 698 +++++++++++++++++++++++++
>  lib/power/power_amd_pstate_cpufreq.h   | 219 ++++++++
>  lib/power/rte_power.c                  |  26 +
>  lib/power/rte_power.h                  |   3 +-
>  lib/power/rte_power_pmd_mgmt.c         |   6 +-
>  10 files changed, 958 insertions(+), 5 deletions(-)

I'm not comfortable to merge this patch without a word from David Hunt.
Given there is 0 review, what do we do?


>       Also, make sure to start the actual text at the margin.
>       =======================================================
>  
> +   * **Added amd-pstate driver support to power management library.**
> +
> +        Added support for amd-pstate driver which works on AMD Zen processors.

Looks like the indent is not correct.

>          'power_pstate_cpufreq.c',
> +        'power_amd_pstate_cpufreq.c',

Can you say briefly why AMD has a different pstate?
Does it mean power_pstate_cpufreq.c should be renamed
power_intel_pstate_cpufreq.c?

> +++ b/lib/power/power_amd_pstate_cpufreq.c
> @@ -0,0 +1,698 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2021 Intel Corporation
> + * Copyright(c) 2021 Arm Limited

Why is there copyright for Intel and Arm?
Does it mean you copied some code and did not try
to keep common code in a common place?

> + * Copyright(c) 2023 Amd Limited