Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Thu, 23 Nov 2023 16:08:23 +0200 (EET)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     "David E. Box" <david.e.box@linux.intel.com>
cc:     LKML <linux-kernel@vger.kernel.org>,
        platform-driver-x86@vger.kernel.org, rajvi.jingar@linux.intel.com
Subject: Re: [PATCH V5 16/20] platform/x86/intel/pmc: Display LPM requirements
 for multiple PMCs
In-Reply-To: <20231123040355.82139-17-david.e.box@linux.intel.com>
Message-ID: <fe419f5-3ff3-3677-419-735dd7e66473@linux.intel.com>
References: <20231123040355.82139-1-david.e.box@linux.intel.com> <20231123040355.82139-17-david.e.box@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-2093636943-1700748506=:1676"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-2093636943-1700748506=:1676
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 8BIT

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.

--8323329-2093636943-1700748506=:1676--