Re: [PATCH v8 2/5] ACPI, APEI, EINJ: Add wrapper __init function

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 18 Dec 2023 15:59:12 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Ben Cheatham wrote:
> > The CXL core module should be able to load regardless of whether the
> > EINJ module initializes correctly. Instead of porting the EINJ module to
> > a library module, add a wrapper __init function around einj_init() to  
> 
> Small quibble with this wording... the larger EINJ module refactoring
> would be separating module_init() from EINJ probe(). As is this simple
> introduction of an einit_init() wrapper *is* refactoring this module to
> be used as a module dependency.
> 
> > pin the EINJ module even if it does not initialize correctly. This
> > should be fine since the EINJ module is only ever unloaded manually.
> > 
> > One note: since the CXL core will be calling into the EINJ module
> > directly, even though it may not have initialized, all CXL helper
> > functions *have* to check if the EINJ module is initialized before
> > doing any work.  
> 
> Another small quibble here, perhaps s/may not have initialized/may not
> have successfully initialized/? Because initialization will have
> definitely completed one way or the other, but callers need to abort if
> it completed in error.
> 
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Did Jonathan really get in and review this new patch in the series
> before me? If yes, apologies I missed it, if no I think it is best
> practice to not carry forward series Reviewed-by's if new patches appear
> in the series between revisions.

I'm not keen on the solution as it's esoteric and to me seems fragile.
I've looked at discussion on v7 and can see why you ended up with this
but I'd have preferred to see the 'violent' approach :)

I don't mind if there is consensus on this, but not reviewed by from
me for this one.

> 
> 
> > Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>  
> 
> With above fixups, feel free to add:
> 
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>