RE: [PATCH v2] powercap: Add Power Limit4 support

["Pawnikar, Sumeet R" <sumeet.r.pawnikar@intel.com>]

> -----Original Message-----
> From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Sent: Wednesday, July 15, 2020 5:44 AM
> To: Rafael J. Wysocki <rafael@kernel.org>; Pawnikar, Sumeet R
> <sumeet.r.pawnikar@intel.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Zhang, Rui
> <rui.zhang@intel.com>; Linux PM <linux-pm@vger.kernel.org>; Linux Kernel
> Mailing List <linux-kernel@vger.kernel.org>; Shevchenko, Andriy
> <andriy.shevchenko@intel.com>
> Subject: Re: [PATCH v2] powercap: Add Power Limit4 support
> 
> On Tue, 2020-07-14 at 15:21 +0200, Rafael J. Wysocki wrote:
> > >
> 
> [...]
> 
> > On Tue, Jul 14, 2020 at 10:22 AM Sumeet Pawnikar
> > <sumeet.r.pawnikar@intel.com> wrote:
> >
> > Srinivas, does the patch look good to you?
> 
> Some minor comments then Summet can add my
> 
> Reviewed-and-tested-by: Srinivas Pandruvada <
> srinivas.pandruvada@linux.intel.com>
> 

Thanks Srinivas for testing this. 
I will address your below comment
and submit v3 with above tag. 

> > > ---
> > > Changes in v2:
> > >  - Addressed review comments from Rafael.
> > >  - Made the commit message more clearer.
> > >  - Updated powercap documentation.
> > > ---
> > >
> 
> [...]
> 
> > > 0,1).
> > > +Depending on different power zones, the Intel RAPL technology
> > > allows
> > > +one or multiple constraints like short term, long term and peak
> > > power,
> > > +with different time windows to be applied to each power zone.
> I think better to spell out that time window is not applicable to "peak power".
> Otherwise someone will send a bug report.
> 
> 
> [...]
> 
> > >  static int rapl_msr_probe(struct platform_device *pdev)  {
> > >         int ret;
> > > +       const struct x86_cpu_id *id =
> > > x86_match_cpu(pl4_support_ids);
> 
> To match coding style in this file:
> 	const struct x86_cpu_id *id = x86_match_cpu(pl4_support_ids);
>         int ret;
> 

Thanks for review.  

Regards,
Sumeet.

> > >         rapl_msr_priv.read_raw = rapl_msr_read_raw;
> 
> Thanks,
> Srinivas