Unregistering a ghes shouldn't fail (because how can firmware refuse to
disable an event on unregister). And the callers are not really in a
position to handle errors. (Note: The return value of platform remove
callbacks is ignored.) So make sdei_unregister_ghes() return void and
add warnings at the few locations that can theoretically fail.
!IS_ENABLED(CONFIG_ACPI_APEI_GHES) and
!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE) cannot be hit here, because if
these aren't given, ghex_probe() already fails and so ghes_remove()
isn't called.
This change improves the behaviour in the error case. Without it the
platform code emits a very generic (and so very unhelpful) error
message. Now the warning is at least helpful.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
Hello,
Changes since (implicit) v1: Add the if (...) BUG() part to fix a linker
error with CONFIG_ARM_SDE_INTERFACE disabled.
Best regards
Uwe
drivers/acpi/apei/ghes.c | 19 ++++++++++++-------
drivers/firmware/arm_sdei.c | 14 +++++++-------
include/linux/arm_sdei.h | 2 +-
3 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 066dc1f5c235..7d705930e21b 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1275,12 +1275,20 @@ static int apei_sdei_register_ghes(struct ghes *ghes)
ghes_sdei_critical_callback);
}
-static int apei_sdei_unregister_ghes(struct ghes *ghes)
+static void apei_sdei_unregister_ghes(struct ghes *ghes)
{
+ /*
+ * If CONFIG_ARM_SDE_INTERFACE isn't enabled apei_sdei_register_ghes()
+ * cannot have been called successfully. So ghes_remove() won't be
+ * called because either ghes_probe() failed or the notify type isn't
+ * ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED.
+ * Note the if statement below is necessary to prevent a linker error as
+ * the compiler has no chance to understand the above correlation.
+ */
if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
- return -EOPNOTSUPP;
+ BUG();
- return sdei_unregister_ghes(ghes);
+ sdei_unregister_ghes(ghes);
}
static int ghes_probe(struct platform_device *ghes_dev)
@@ -1421,7 +1429,6 @@ static int ghes_probe(struct platform_device *ghes_dev)
static int ghes_remove(struct platform_device *ghes_dev)
{
- int rc;
struct ghes *ghes;
struct acpi_hest_generic *generic;
@@ -1455,9 +1462,7 @@ static int ghes_remove(struct platform_device *ghes_dev)
ghes_nmi_remove(ghes);
break;
case ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED:
- rc = apei_sdei_unregister_ghes(ghes);
- if (rc)
- return rc;
+ apei_sdei_unregister_ghes(ghes);
break;
default:
BUG();
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index 1e1a51510e83..7af619464183 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -889,7 +889,7 @@ int sdei_register_ghes(struct ghes *ghes, sdei_event_callback *normal_cb,
return err;
}
-int sdei_unregister_ghes(struct ghes *ghes)
+void sdei_unregister_ghes(struct ghes *ghes)
{
int i;
int err;
@@ -897,16 +897,15 @@ int sdei_unregister_ghes(struct ghes *ghes)
might_sleep();
- if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
- return -EOPNOTSUPP;
-
/*
* The event may be running on another CPU. Disable it
* to stop new events, then try to unregister a few times.
*/
err = sdei_event_disable(event_num);
- if (err)
- return err;
+ if (err) {
+ dev_warn(ghes->dev, "Failed to disable event %u: %pe\n", event_num, ERR_PTR(err));
+ return;
+ }
for (i = 0; i < 3; i++) {
err = sdei_event_unregister(event_num);
@@ -916,7 +915,8 @@ int sdei_unregister_ghes(struct ghes *ghes)
schedule();
}
- return err;
+ if (err)
+ dev_warn(ghes->dev, "Failed to disable event %u: %pe\n", event_num, ERR_PTR(err));
}
static int sdei_get_conduit(struct platform_device *pdev)
diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h
index 14dc461b0e82..0812af4530a4 100644
--- a/include/linux/arm_sdei.h
+++ b/include/linux/arm_sdei.h
@@ -40,7 +40,7 @@ int sdei_event_disable(u32 event_num);
/* GHES register/unregister helpers */
int sdei_register_ghes(struct ghes *ghes, sdei_event_callback *normal_cb,
sdei_event_callback *critical_cb);
-int sdei_unregister_ghes(struct ghes *ghes);
+void sdei_unregister_ghes(struct ghes *ghes);
#ifdef CONFIG_ARM_SDE_INTERFACE
/* For use by arch code when CPU hotplug notifiers are not appropriate. */
--
2.38.1
