RE: [PATCH v2 5/7] EDAC/ghes: Prevent chipset-specific edac from loading after ghes_edac is registered

From: Kani, Toshi
Date: Thu Aug 18 2022 - 21:30:52 EST


On Wednesday, August 17, 2022 8:35 AM, Jia He wrote:
> Previous, there is only one edac can be registering in the EDAC core. After
> ghes_edac is modularized, the registering of ghes devices may be deferred
> when ghes_edac is loaded. Prevent the chipset-specific edac drivers from
> loading after ghes_edac is registered, which allow the edac drivers to
> get selected in e.g. HPE platform.
...
> @@ -1382,6 +1395,7 @@ static int ghes_probe(struct platform_device
> *ghes_dev)
> platform_set_drvdata(ghes_dev, ghes);
>
> ghes->dev = &ghes_dev->dev;
> + set_ghes_devs_registered(false);

This does not look right to me.

The condition of using ghes_edac is (ghes-present) && (ghes-preferred),
where:
- ghes-present is latched on ghes_probe()
- ghes-preferred is true if the platform-check passes.

ghes_get_device() introduced in the previous patch works as the
ghes-preferred check.

We cannot use ghes_edac registration as the ghes-present check in
this scheme since it is deferred to module_init().

I'd suggest the following changes:
- Update ghes_get_device() to check a flag latched on ghes_probe().
- Remove 'force' argument from ghes_get_device(). ghes_edac_init()
is too late to set this flag. Also, other edac drivers should not be able
to control this flag. I think this force_load kernel option needs to
belong to ghes with this scheme.
- ghes_get_device() exposes 'ghes_devs' to chipset-specific edac drivers,
which should be internal to ghes. ghes_get_device() may be renamed
to something like ghes_edac_preferred() which returns true / false.

Toshi