Jonathan Cameron wrote: > On Mon, 18 Dec 2023 15:59:12 -0800 > Dan Williams <dan.j.williams@xxxxxxxxx> 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@xxxxxxxxxx> > > > > 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.
