Re: [PATCH v1] cpufreq: intel_pstate: Relocate platform preference check

[srinivas pandruvada]

On Thu, 2025-02-20 at 20:30 +0100, Rafael J. Wysocki wrote:
> On Fri, Feb 7, 2025 at 1:48 PM Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
> wrote:
> > 
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > 
> > Move the invocation of intel_pstate_platform_pwr_mgmt_exists()
> > before
> > checking whether or not HWP is enabled because it does not depend
> > on
> > any code running before it except for the vendor check and if CPU
> > performance scaling is going to be carried out by the platform, all
> > of
> > the code that runs before that function (again, except for the
> > vendor
> > check) is redundant.
> > 
> > This is not expected to alter any functionality except for the
> > ordering
> > of messages printed by intel_pstate_init() when it is going to
> > return an
> > error before attempting to register the driver.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> Srinivas, I'm wondering if you have any concerns regarding this one?
None.

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>

Thanks,
Srinivas


> 
> > ---
> >  drivers/cpufreq/intel_pstate.c |   18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -3688,6 +3688,15 @@
> >         if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> >                 return -ENODEV;
> > 
> > +       /*
> > +        * The Intel pstate driver will be ignored if the platform
> > +        * firmware has its own power management modes.
> > +        */
> > +       if (intel_pstate_platform_pwr_mgmt_exists()) {
> > +               pr_info("P-states controlled by the platform\n");
> > +               return -ENODEV;
> > +       }
> > +
> >         id = x86_match_cpu(hwp_support_ids);
> >         if (id) {
> >                 hwp_forced = intel_pstate_hwp_is_enabled();
> > @@ -3743,15 +3752,6 @@
> >                 default_driver = &intel_cpufreq;
> > 
> >  hwp_cpu_matched:
> > -       /*
> > -        * The Intel pstate driver will be ignored if the platform
> > -        * firmware has its own power management modes.
> > -        */
> > -       if (intel_pstate_platform_pwr_mgmt_exists()) {
> > -               pr_info("P-states controlled by the platform\n");
> > -               return -ENODEV;
> > -       }
> > -
> >         if (!hwp_active && hwp_only)
> >                 return -ENOTSUPP;
> > 
> > 
> > 
> > 
> >