RE: [RESEND PATCH v3 5/9] EDAC: Don't load chipset-specific edac drivers when ghes_edac is preferred

Justin He
2022-08-25T09:46:09Z

> -----Original Message-----
> From: Kani, Toshi <toshi.kani@hpe.com>
> Sent: Thursday, August 25, 2022 7:04 AM
> To: Justin He <Justin.He@arm.com>; Len Brown <lenb@kernel.org>; James
> Morse <James.Morse@arm.com>; Tony Luck <tony.luck@intel.com>; Borislav
> Petkov <bp@alien8.de>; Mauro Carvalho Chehab <mchehab@kernel.org>;
> Robert Richter <rric@kernel.org>; Robert Moore <robert.moore@intel.com>;
> Qiuxu Zhuo <qiuxu.zhuo@intel.com>; Yazen Ghannam
> <yazen.ghannam@amd.com>; Jonathan Corbet <corbet@lwn.net>; Jan
> Luebbe <jlu@pengutronix.de>; Khuong Dinh
> <khuong@os.amperecomputing.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-edac@vger.kernel.org; devel@acpica.org;
> Rafael J . Wysocki <rafael@kernel.org>; Shuai Xue
> <xueshuai@linux.alibaba.com>; Jarkko Sakkinen <jarkko@kernel.org>;
> linux-efi@vger.kernel.org; nd <nd@arm.com>; Paul E. McKenney
> <paulmck@kernel.org>; Andrew Morton <akpm@linux-foundation.org>;
> Neeraj Upadhyay <quic_neeraju@quicinc.com>; Randy Dunlap
> <rdunlap@infradead.org>; Damien Le Moal
> <damien.lemoal@opensource.wdc.com>; Muchun Song
> <songmuchun@bytedance.com>; linux-doc@vger.kernel.org
> Subject: RE: [RESEND PATCH v3 5/9] EDAC: Don't load chipset-specific edac
> drivers when ghes_edac is preferred
>
> On Monday, August 22, 2022 9:41 AM, Jia He wrote:
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index
> > e17e0ee8f842..327386f3cf33 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -1537,16 +1537,25 @@ static struct acpi_platform_list plat_list[] = {
> > { } /* End */
> > };
> >
> > -struct list_head *ghes_get_devices(void)
> > +bool ghes_edac_preferred(void)
> > {
> > int idx = -1;
> >
> > if (IS_ENABLED(CONFIG_X86)) {
> > idx = acpi_match_platform_list(plat_list);
> > if (idx < 0 && !ghes_edac_force)
> > - return NULL;
> > + return false;
> > }
> >
> > + return true;
> > +}
> > +EXPORT_SYMBOL_GPL(ghes_edac_preferred);
> > +
> > +struct list_head *ghes_get_devices(void) {
> > + if (!ghes_edac_preferred())
> > + return NULL;
> > +
> > return &ghes_devs;
> > }
>
> ghes_get_devices() changing multiple times in the series is
> confusing to me. Can you simply introduce ghes_get_devices()
> and ghes_preferred() in the right state in a patch? Perhaps, patch #2, #5, #6
> can collapse to introduce the two funcs?
>

My purpose was to make it easy for review. I am ok to merge these patches into one.

> The rest of patch #5 adding the call to ghes_edac_preferred() into other edac
> drivers can remain as a separate patch.

Okay, I assume you mean to merge all of the ghes_edac_preferred() checking for intel and
Arm edac drivers into 1 separate patch, am I understanding well?

--
Cheers,
Justin (Jia He)