Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Fri, 13 Oct 2023 14:36:19 +0300 (EEST)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     "David E. Box" <david.e.box@linux.intel.com>
cc:     =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        platform-driver-x86@vger.kernel.org, rajvi.jingar@linux.intel.com
Subject: Re: [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram_init
 to fail
In-Reply-To: <7f2fd7a054912960c6599e4a62e2095d1567aab8.camel@linux.intel.com>
Message-ID: <92594f48-fa9-59d4-e2b7-f5f83ded0ea@linux.intel.com>
References: <20231012023840.3845703-1-david.e.box@linux.intel.com>  <20231012023840.3845703-10-david.e.box@linux.intel.com>  <ac7be397-bc9-a135-9498-72dfa1fe456d@linux.intel.com> <7f2fd7a054912960c6599e4a62e2095d1567aab8.camel@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323329-729097219-1697196785=:2026"
Content-ID: <940e91-6884-23c1-d36b-d17dc5a55f7c@linux.intel.com>
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-729097219-1697196785=:2026
Content-Type: text/plain; CHARSET=ISO-8859-15
Content-Transfer-Encoding: 8BIT
Content-ID: <9a57b987-57a3-2aa9-d42d-c48449944d9@linux.intel.com>

On Thu, 12 Oct 2023, David E. Box wrote:

> On Thu, 2023-10-12 at 18:01 +0300, Ilpo J?rvinen wrote:
> > On Wed, 11 Oct 2023, David E. Box wrote:
> > 
> > > Currently, if the PMC SSRAM initialization fails, no error is returned and
> > > the only indication is that a PMC device has not been created.? Instead,
> > > allow an error to be returned and handled directly by the caller.
> > 
> > You might have a good reason for it but why isn't the call into 
> > pmc_core_pmc_add() changed in this patch to take the error value into 
> > account?
> 
> Good catch. The return value of pmc_core_pmc_add() is first used in the next
> patch but should be used here.
> 
> > 
> > (I vaguely remember this was probably discussed in the context of some 
> > earlier patch touching this area that it was about the other code dealing 
> > with NULLs or something like that).

Okay but please also take into consideration what I tried to imply above:

Since you are doing what looks a major logic change in the next patch, it 
might be okay to _not use_ that return value until then if you e.g., need 
to add lots of rollback that isn't there already to the code that is going 
away anyway in the next patch. (After all, returning an error code from a 
function that was void previously isn't going to magically break the old
calling code).


-- 
 i.
--8323329-729097219-1697196785=:2026--