Re: [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail

[Ilpo Järvinen]

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.