From: Bjorn Helgaas <helgaas@kernel.org> To: Shuai Xue <xueshuai@linux.alibaba.com> Cc: rafael@kernel.org, bp@alien8.de, tony.luck@intel.com, james.morse@arm.com, lenb@kernel.org, rjw@rjwysocki.net, bhelgaas@google.com, zhangliguang@linux.alibaba.com, zhuo.song@linux.alibaba.com, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org Subject: Re: [PATCH v6] ACPI: explicit init HEST, SDEI and GHES in apci_init Date: Thu, 20 Jan 2022 10:22:41 -0600 [thread overview] Message-ID: <20220120162241.GA1047212@bhelgaas> (raw) In-Reply-To: <20220120050522.23689-1-xueshuai@linux.alibaba.com> On Thu, Jan 20, 2022 at 01:05:22PM +0800, Shuai Xue wrote: > From commit e147133a42cb ("ACPI / APEI: Make hest.c manage the estatus > memory pool") was merged, ghes_init() relies on acpi_hest_init() to manage > the estatus memory pool. On the other hand, ghes_init() relies on > sdei_init() to detect the SDEI version and (un)register events. The > dependencies are as follows: > > ghes_init() => acpi_hest_init() => acpi_bus_init() => acpi_init() > ghes_init() => sdei_init() > > HEST is not PCI-specific and initcall ordering is implicit and not > well-defined within a level. > > Based on above, remove acpi_hest_init() from acpi_pci_root_init() and > convert ghes_init() and sdei_init() from initcalls to explicit calls in the > following order: > > acpi_hest_init() > sdei_init() > ghes_init() > > Suggested-by: Bjorn Helgaas <helgaas@kernel.org> I didn't suggest the approach; I just reviewed the patch and the commit log and proposed moving it out of acpi_pci_root_init(). That doesn't need to be acknowledged. > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com> Minor extra "return" below. > --- > drivers/acpi/apei/ghes.c | 17 +++++++---------- > drivers/acpi/bus.c | 4 ++++ > drivers/acpi/pci_root.c | 3 --- > drivers/firmware/arm_sdei.c | 13 ++----------- > include/acpi/apei.h | 4 +++- > include/linux/arm_sdei.h | 2 ++ > 6 files changed, 18 insertions(+), 25 deletions(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 0c5c9acc6254..bed4f10cfcb8 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -1457,33 +1457,33 @@ static struct platform_driver ghes_platform_driver = { > .remove = ghes_remove, > }; > > -static int __init ghes_init(void) > +void __init ghes_init(void) > { > int rc; > > if (acpi_disabled) > - return -ENODEV; > + return; > > switch (hest_disable) { > case HEST_NOT_FOUND: > - return -ENODEV; > + return; > case HEST_DISABLED: > pr_info(GHES_PFX "HEST is not enabled!\n"); > - return -EINVAL; > + return; > default: > break; > } > > if (ghes_disable) { > pr_info(GHES_PFX "GHES is not enabled!\n"); > - return -EINVAL; > + return; > } > > ghes_nmi_init_cxt(); > > rc = platform_driver_register(&ghes_platform_driver); > if (rc) > - goto err; > + return; > > rc = apei_osc_setup(); > if (rc == 0 && osc_sb_apei_support_acked) > @@ -1495,8 +1495,5 @@ static int __init ghes_init(void) > else > pr_info(GHES_PFX "Failed to enable APEI firmware first mode.\n"); > > - return 0; > -err: > - return rc; > + return; Unnecessary "return". > } > -device_initcall(ghes_init); > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 07f604832fd6..1dcd71df2cd5 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -30,6 +30,7 @@ > #include <linux/acpi_viot.h> > #include <linux/pci.h> > #include <acpi/apei.h> > +#include <linux/arm_sdei.h> This "arm" looks a little out of place in this supposedly arch-generic code. Not really a new thing with this patch, since this #include already appears in drivers/acpi/apei/ghes.c. Maybe it's unavoidable. > #include <linux/suspend.h> > #include <linux/prmt.h> > > @@ -1331,6 +1332,9 @@ static int __init acpi_init(void) > > pci_mmcfg_late_init(); > acpi_iort_init(); > + acpi_hest_init(); > + sdei_init(); > + ghes_init(); > acpi_scan_init(); > acpi_ec_init(); > acpi_debugfs_init(); > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index b76db99cced3..6f9e75d14808 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -22,8 +22,6 @@ > #include <linux/slab.h> > #include <linux/dmi.h> > #include <linux/platform_data/x86/apple.h> > -#include <acpi/apei.h> /* for acpi_hest_init() */ > - > #include "internal.h" > > #define ACPI_PCI_ROOT_CLASS "pci_bridge" > @@ -943,7 +941,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > > void __init acpi_pci_root_init(void) > { > - acpi_hest_init(); > if (acpi_pci_disabled) > return; > > diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c > index a7e762c352f9..1e1a51510e83 100644 > --- a/drivers/firmware/arm_sdei.c > +++ b/drivers/firmware/arm_sdei.c > @@ -1059,14 +1059,14 @@ static bool __init sdei_present_acpi(void) > return true; > } > > -static int __init sdei_init(void) > +void __init sdei_init(void) > { > struct platform_device *pdev; > int ret; > > ret = platform_driver_register(&sdei_driver); > if (ret || !sdei_present_acpi()) > - return ret; > + return; > > pdev = platform_device_register_simple(sdei_driver.driver.name, > 0, NULL, 0); > @@ -1076,17 +1076,8 @@ static int __init sdei_init(void) > pr_info("Failed to register ACPI:SDEI platform device %d\n", > ret); > } > - > - return ret; > } > > -/* > - * On an ACPI system SDEI needs to be ready before HEST:GHES tries to register > - * its events. ACPI is initialised from a subsys_initcall(), GHES is initialised > - * by device_initcall(). We want to be called in the middle. > - */ > -subsys_initcall_sync(sdei_init); > - > int sdei_event_handler(struct pt_regs *regs, > struct sdei_registered_event *arg) > { > diff --git a/include/acpi/apei.h b/include/acpi/apei.h > index ece0a8af2bae..4e60dd73c3bb 100644 > --- a/include/acpi/apei.h > +++ b/include/acpi/apei.h > @@ -27,14 +27,16 @@ extern int hest_disable; > extern int erst_disable; > #ifdef CONFIG_ACPI_APEI_GHES > extern bool ghes_disable; > +void __init ghes_init(void); > #else > #define ghes_disable 1 > +static inline void ghes_init(void) { } > #endif > > #ifdef CONFIG_ACPI_APEI > void __init acpi_hest_init(void); > #else > -static inline void acpi_hest_init(void) { return; } > +static inline void acpi_hest_init(void) { } > #endif > > int erst_write(const struct cper_record_header *record); > diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h > index 0a241c5c911d..14dc461b0e82 100644 > --- a/include/linux/arm_sdei.h > +++ b/include/linux/arm_sdei.h > @@ -46,9 +46,11 @@ int sdei_unregister_ghes(struct ghes *ghes); > /* For use by arch code when CPU hotplug notifiers are not appropriate. */ > int sdei_mask_local_cpu(void); > int sdei_unmask_local_cpu(void); > +void __init sdei_init(void); > #else > static inline int sdei_mask_local_cpu(void) { return 0; } > static inline int sdei_unmask_local_cpu(void) { return 0; } > +static inline void sdei_init(void) { } > #endif /* CONFIG_ARM_SDE_INTERFACE */ > > > -- > 2.20.1.12.g72788fdb > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org> To: Shuai Xue <xueshuai@linux.alibaba.com> Cc: rafael@kernel.org, bp@alien8.de, tony.luck@intel.com, james.morse@arm.com, lenb@kernel.org, rjw@rjwysocki.net, bhelgaas@google.com, zhangliguang@linux.alibaba.com, zhuo.song@linux.alibaba.com, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org Subject: Re: [PATCH v6] ACPI: explicit init HEST, SDEI and GHES in apci_init Date: Thu, 20 Jan 2022 10:22:41 -0600 [thread overview] Message-ID: <20220120162241.GA1047212@bhelgaas> (raw) In-Reply-To: <20220120050522.23689-1-xueshuai@linux.alibaba.com> On Thu, Jan 20, 2022 at 01:05:22PM +0800, Shuai Xue wrote: > From commit e147133a42cb ("ACPI / APEI: Make hest.c manage the estatus > memory pool") was merged, ghes_init() relies on acpi_hest_init() to manage > the estatus memory pool. On the other hand, ghes_init() relies on > sdei_init() to detect the SDEI version and (un)register events. The > dependencies are as follows: > > ghes_init() => acpi_hest_init() => acpi_bus_init() => acpi_init() > ghes_init() => sdei_init() > > HEST is not PCI-specific and initcall ordering is implicit and not > well-defined within a level. > > Based on above, remove acpi_hest_init() from acpi_pci_root_init() and > convert ghes_init() and sdei_init() from initcalls to explicit calls in the > following order: > > acpi_hest_init() > sdei_init() > ghes_init() > > Suggested-by: Bjorn Helgaas <helgaas@kernel.org> I didn't suggest the approach; I just reviewed the patch and the commit log and proposed moving it out of acpi_pci_root_init(). That doesn't need to be acknowledged. > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com> Minor extra "return" below. > --- > drivers/acpi/apei/ghes.c | 17 +++++++---------- > drivers/acpi/bus.c | 4 ++++ > drivers/acpi/pci_root.c | 3 --- > drivers/firmware/arm_sdei.c | 13 ++----------- > include/acpi/apei.h | 4 +++- > include/linux/arm_sdei.h | 2 ++ > 6 files changed, 18 insertions(+), 25 deletions(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 0c5c9acc6254..bed4f10cfcb8 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -1457,33 +1457,33 @@ static struct platform_driver ghes_platform_driver = { > .remove = ghes_remove, > }; > > -static int __init ghes_init(void) > +void __init ghes_init(void) > { > int rc; > > if (acpi_disabled) > - return -ENODEV; > + return; > > switch (hest_disable) { > case HEST_NOT_FOUND: > - return -ENODEV; > + return; > case HEST_DISABLED: > pr_info(GHES_PFX "HEST is not enabled!\n"); > - return -EINVAL; > + return; > default: > break; > } > > if (ghes_disable) { > pr_info(GHES_PFX "GHES is not enabled!\n"); > - return -EINVAL; > + return; > } > > ghes_nmi_init_cxt(); > > rc = platform_driver_register(&ghes_platform_driver); > if (rc) > - goto err; > + return; > > rc = apei_osc_setup(); > if (rc == 0 && osc_sb_apei_support_acked) > @@ -1495,8 +1495,5 @@ static int __init ghes_init(void) > else > pr_info(GHES_PFX "Failed to enable APEI firmware first mode.\n"); > > - return 0; > -err: > - return rc; > + return; Unnecessary "return". > } > -device_initcall(ghes_init); > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 07f604832fd6..1dcd71df2cd5 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -30,6 +30,7 @@ > #include <linux/acpi_viot.h> > #include <linux/pci.h> > #include <acpi/apei.h> > +#include <linux/arm_sdei.h> This "arm" looks a little out of place in this supposedly arch-generic code. Not really a new thing with this patch, since this #include already appears in drivers/acpi/apei/ghes.c. Maybe it's unavoidable. > #include <linux/suspend.h> > #include <linux/prmt.h> > > @@ -1331,6 +1332,9 @@ static int __init acpi_init(void) > > pci_mmcfg_late_init(); > acpi_iort_init(); > + acpi_hest_init(); > + sdei_init(); > + ghes_init(); > acpi_scan_init(); > acpi_ec_init(); > acpi_debugfs_init(); > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index b76db99cced3..6f9e75d14808 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -22,8 +22,6 @@ > #include <linux/slab.h> > #include <linux/dmi.h> > #include <linux/platform_data/x86/apple.h> > -#include <acpi/apei.h> /* for acpi_hest_init() */ > - > #include "internal.h" > > #define ACPI_PCI_ROOT_CLASS "pci_bridge" > @@ -943,7 +941,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > > void __init acpi_pci_root_init(void) > { > - acpi_hest_init(); > if (acpi_pci_disabled) > return; > > diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c > index a7e762c352f9..1e1a51510e83 100644 > --- a/drivers/firmware/arm_sdei.c > +++ b/drivers/firmware/arm_sdei.c > @@ -1059,14 +1059,14 @@ static bool __init sdei_present_acpi(void) > return true; > } > > -static int __init sdei_init(void) > +void __init sdei_init(void) > { > struct platform_device *pdev; > int ret; > > ret = platform_driver_register(&sdei_driver); > if (ret || !sdei_present_acpi()) > - return ret; > + return; > > pdev = platform_device_register_simple(sdei_driver.driver.name, > 0, NULL, 0); > @@ -1076,17 +1076,8 @@ static int __init sdei_init(void) > pr_info("Failed to register ACPI:SDEI platform device %d\n", > ret); > } > - > - return ret; > } > > -/* > - * On an ACPI system SDEI needs to be ready before HEST:GHES tries to register > - * its events. ACPI is initialised from a subsys_initcall(), GHES is initialised > - * by device_initcall(). We want to be called in the middle. > - */ > -subsys_initcall_sync(sdei_init); > - > int sdei_event_handler(struct pt_regs *regs, > struct sdei_registered_event *arg) > { > diff --git a/include/acpi/apei.h b/include/acpi/apei.h > index ece0a8af2bae..4e60dd73c3bb 100644 > --- a/include/acpi/apei.h > +++ b/include/acpi/apei.h > @@ -27,14 +27,16 @@ extern int hest_disable; > extern int erst_disable; > #ifdef CONFIG_ACPI_APEI_GHES > extern bool ghes_disable; > +void __init ghes_init(void); > #else > #define ghes_disable 1 > +static inline void ghes_init(void) { } > #endif > > #ifdef CONFIG_ACPI_APEI > void __init acpi_hest_init(void); > #else > -static inline void acpi_hest_init(void) { return; } > +static inline void acpi_hest_init(void) { } > #endif > > int erst_write(const struct cper_record_header *record); > diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h > index 0a241c5c911d..14dc461b0e82 100644 > --- a/include/linux/arm_sdei.h > +++ b/include/linux/arm_sdei.h > @@ -46,9 +46,11 @@ int sdei_unregister_ghes(struct ghes *ghes); > /* For use by arch code when CPU hotplug notifiers are not appropriate. */ > int sdei_mask_local_cpu(void); > int sdei_unmask_local_cpu(void); > +void __init sdei_init(void); > #else > static inline int sdei_mask_local_cpu(void) { return 0; } > static inline int sdei_unmask_local_cpu(void) { return 0; } > +static inline void sdei_init(void) { } > #endif /* CONFIG_ARM_SDE_INTERFACE */ > > > -- > 2.20.1.12.g72788fdb > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-01-20 16:22 UTC|newest] Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-26 7:04 [RFC PATCH v4] ACPI: Move sdei_init and ghes_init ahead to handle platform errors earlier Shuai Xue 2021-11-26 7:04 ` Shuai Xue 2021-12-02 12:50 ` Shuai Xue 2021-12-02 12:50 ` Shuai Xue 2021-12-16 13:34 ` [RESEND " Shuai Xue 2021-12-16 13:34 ` Shuai Xue 2021-12-17 18:17 ` Rafael J. Wysocki 2021-12-17 18:17 ` Rafael J. Wysocki 2021-12-19 4:04 ` Shuai Xue 2021-12-19 4:04 ` Shuai Xue 2021-12-21 23:17 ` Bjorn Helgaas 2021-12-21 23:17 ` Bjorn Helgaas 2021-12-23 8:11 ` Shuai Xue 2021-12-23 8:11 ` Shuai Xue 2021-12-24 0:17 ` Bjorn Helgaas 2021-12-24 0:17 ` Bjorn Helgaas 2021-12-24 8:55 ` Shuai Xue 2021-12-24 8:55 ` Shuai Xue 2022-01-13 13:38 ` Shuai Xue 2022-01-13 13:38 ` Shuai Xue 2022-01-16 8:43 ` [PATCH v5] " Shuai Xue 2022-01-16 8:43 ` Shuai Xue 2022-01-18 22:49 ` Bjorn Helgaas 2022-01-18 22:49 ` Bjorn Helgaas 2022-01-19 6:40 ` Shuai Xue 2022-01-19 6:40 ` Shuai Xue 2022-01-19 20:42 ` Bjorn Helgaas 2022-01-19 20:42 ` Bjorn Helgaas 2022-01-20 2:40 ` Shuai Xue 2022-01-20 2:40 ` Shuai Xue 2022-01-20 16:24 ` Rafael J. Wysocki 2022-01-20 16:24 ` Rafael J. Wysocki 2022-01-20 5:05 ` [PATCH v6] ACPI: explicit init HEST, SDEI and GHES in apci_init Shuai Xue 2022-01-20 5:05 ` Shuai Xue 2022-01-20 16:22 ` Bjorn Helgaas [this message] 2022-01-20 16:22 ` Bjorn Helgaas 2022-01-21 3:43 ` Shuai Xue 2022-01-21 3:43 ` Shuai Xue 2022-01-21 19:46 ` Bjorn Helgaas 2022-01-21 19:46 ` Bjorn Helgaas 2022-01-22 5:26 ` [PATCH v7 1/2] ACPI: APEI: explicit init HEST " Shuai Xue 2022-01-22 5:26 ` Shuai Xue 2022-02-10 9:39 ` Shuai Xue 2022-02-10 9:39 ` Shuai Xue 2022-02-14 18:51 ` Rafael J. Wysocki 2022-02-14 18:51 ` Rafael J. Wysocki 2022-02-21 18:18 ` Nathan Chancellor 2022-02-21 18:18 ` Nathan Chancellor 2022-02-21 18:25 ` Rafael J. Wysocki 2022-02-21 18:25 ` Rafael J. Wysocki 2022-02-22 6:03 ` Shuai Xue 2022-02-22 6:03 ` Shuai Xue 2022-01-22 5:26 ` [PATCH v7 2/2] ACPI: APEI: rename ghes_init with an "acpi_" prefix Shuai Xue 2022-01-22 5:26 ` Shuai Xue 2022-02-27 12:25 ` [PATCH v8 1/2] ACPI: APEI: explicit init HEST and GHES in apci_init Shuai Xue 2022-02-27 12:25 ` Shuai Xue 2022-03-03 19:27 ` Rafael J. Wysocki 2022-03-03 19:27 ` Rafael J. Wysocki 2022-02-27 12:25 ` [PATCH v8 2/2] ACPI: APEI: rename ghes_init with an "acpi_" prefix Shuai Xue 2022-02-27 12:25 ` Shuai Xue
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220120162241.GA1047212@bhelgaas \ --to=helgaas@kernel.org \ --cc=bhelgaas@google.com \ --cc=bp@alien8.de \ --cc=james.morse@arm.com \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=rafael@kernel.org \ --cc=rjw@rjwysocki.net \ --cc=tony.luck@intel.com \ --cc=xueshuai@linux.alibaba.com \ --cc=zhangliguang@linux.alibaba.com \ --cc=zhuo.song@linux.alibaba.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.