Messages in this thread | | | Date | Thu, 23 Nov 2023 16:08:23 +0200 (EET) | From | Ilpo Järvinen <> | Subject | Re: [PATCH V5 16/20] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs |
| |
On Wed, 22 Nov 2023, David E. Box wrote:
> From: Rajvi Jingar <rajvi.jingar@linux.intel.com> > > Update the substate_requirements attribute to display the requirements for > all the PMCs on a package. > > Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com> > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > ---
This change looked fine.
However, I spent considerable time in figuring out the addition of lpm_req_regs NULL check so it would be nice to mention it in the commit message that since the caller's primary_pmc->lpm_req_regs check no longer covers all pmcs, it has to checked within the loop. It's was far from obvious whether it was a bugfix accidently mixed together with this patch or required by this change itself but I managed to finally convince myself it's the latter.
With that noted down for the benefit of the next unfortunate victim who has to read this change from the git history, :-)
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
-- i.
| |