Messages in this thread | | | From | "Yuan, Perry" <> | Subject | RE: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration | Date | Thu, 1 Sep 2022 05:34:49 +0000 |
| |
[AMD Official Use Only - General]
Hi Punit,
> -----Original Message----- > From: Punit Agrawal <punit.agrawal@bytedance.com> > Sent: Monday, August 15, 2022 11:05 PM > To: Yuan, Perry <Perry.Yuan@amd.com> > Cc: rafael.j.wysocki@intel.com; Huang, Ray <Ray.Huang@amd.com>; > viresh.kumar@linaro.org; Sharma, Deepak <Deepak.Sharma@amd.com>; > Limonciello, Mario <Mario.Limonciello@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian > <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux- > pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and > duplicated headers declaration > > [CAUTION: External Email] > > Hi Perry, > > Perry Yuan <Perry.Yuan@amd.com> writes: > > > Cleanup the headers declaration which are not used actually and some > > duplicated declaration which is declarated in some other headers > > already, it will help to simplify the header part. > > We usually don't get rid of indirectly included headers as long as definitions from > header are used in the code. This avoids problems if for some reason the > included header gets dropped - it'll leave the code in an uncompilable state. > > More below. > > > > > Reviewed-by: Huang Rui <ray.huang@amd.com> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 5 ----- > > 1 file changed, 5 deletions(-) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index 9ac75c1cde9c..19a078e232dd 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -31,19 +31,14 @@ > > #include <linux/compiler.h> > > #include <linux/dmi.h> > > #include <linux/slab.h> > > -#include <linux/acpi.h> > > #include <linux/io.h> > > #include <linux/delay.h> > > #include <linux/uaccess.h> > > #include <linux/static_call.h> > > > > -#include <acpi/processor.h> > > #include <acpi/cppc_acpi.h> > > > > #include <asm/msr.h> > > -#include <asm/processor.h> > > On a quick scan, I noticed that "boot_cpu_data" and "boot_cpu_has()" in the > module init function are defined in "asm/processor.h" that is being removed > here. It may compile for now but makes the code more fragile as explained > above. > > Please ensure that only the header files that have no definitions used in this file > (amd-pstate.c) are dropped.
Here is the part of the headers we are working on.
============================================== #include <linux/static_call.h>
#include <acpi/cppc_acpi.h>
#include <asm/msr.h> #include "amd-pstate-trace.h"
============================================
I have tested the latest Linux-pm/bleeding-edge branch to verify the changes working. As I know the "asm/processor.h" is included by the "acpi/cppc_acpi.h" subpath
#include <acpi/cppc_acpi.h> #include <linux/acpi.h> #include <acpi/acpi.h> #include <asm/acpi.h> #include <asm/processor.h> To make it comfortable for us, I will send V6 for this patch (1/7)
Perry.
> > > -#include <asm/cpufeature.h> > > -#include <asm/cpu_device_id.h> > > #include "amd-pstate-trace.h" > > > > #define AMD_PSTATE_TRANSITION_LATENCY 0x20000
| |