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

[Dan Williams <dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> 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 :)

The issue though is similar to the argument for the creation of the
ACPI0017 device for CXL, there is not a great place to hang the einj
device-driver.

However, since einj has no legacy "auto-load" behavior, I think it is
not a lot of code to have einj's module_init() do something like this:

	einj_dev = platform_device_register_full(&einj_dev_info);
	platform_driver_register(&einj_driver);

Ben, you want to give that a shot? Jonathan is right that my proposed
hack is *a* solution but probably not *the* solution where this should
end up.