public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: Armin Wolf <W_Armin@gmx.de>
To: rafael@kernel.org, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/4] ACPI: sbshc: Use ec query notifier call chain
Date: Sat, 25 Feb 2023 12:51:42 +0100	[thread overview]
Message-ID: <20230225115144.31212-3-W_Armin@gmx.de> (raw)
In-Reply-To: <20230225115144.31212-1-W_Armin@gmx.de>

When using acpi_ec_add_query_handler(), a kernel oops
can occur when unloading the sbshc module, since the
handler callback might still be used by a work item
inside the ec workqueue.
Use the new ec query notifier call chain to register
the handler in a safe way. Return NOTIFY_BAD to override
the existing _Qxx handler in case the query was meant
for the EC SMBus controller.

Tested on a Acer Travelmate 4002WLMi.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/sbshc.c | 45 ++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 16f2daaa2c45..e3280f646eb5 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -8,11 +8,14 @@
 #define pr_fmt(fmt) "ACPI: " fmt

 #include <linux/acpi.h>
+#include <linux/notifier.h>
 #include <linux/wait.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+
+#include "internal.h"
 #include "sbshc.h"

 #define ACPI_SMB_HC_CLASS	"smbus_host_ctl"
@@ -20,6 +23,7 @@

 struct acpi_smb_hc {
 	struct acpi_ec *ec;
+	struct notifier_block nb;
 	struct mutex lock;
 	wait_queue_head_t wait;
 	u8 offset;
@@ -194,6 +198,7 @@ int acpi_smbus_unregister_callback(struct acpi_smb_hc *hc)
 	hc->context = NULL;
 	mutex_unlock(&hc->lock);
 	acpi_os_wait_events_complete();
+
 	return 0;
 }

@@ -206,20 +211,28 @@ static inline void acpi_smbus_callback(void *context)
 		hc->callback(hc->context);
 }

-static int smbus_alarm(void *context)
+static int acpi_smbus_hc_notify(struct notifier_block *block, unsigned long action, void *data)
 {
-	struct acpi_smb_hc *hc = context;
+	struct acpi_smb_hc *hc = container_of(block, struct acpi_smb_hc, nb);
 	union acpi_smb_status status;
+	struct acpi_ec *ec = data;
 	u8 address;
+
+	if (ec != hc->ec || action != hc->query_bit)
+		return NOTIFY_DONE;
+
 	if (smb_hc_read(hc, ACPI_SMB_STATUS, &status.raw))
-		return 0;
+		return NOTIFY_OK;
+
 	/* Check if it is only a completion notify */
 	if (status.fields.done && status.fields.status == SMBUS_OK) {
 		hc->done = true;
 		wake_up(&hc->wait);
 	}
+
 	if (!status.fields.alarm)
-		return 0;
+		return NOTIFY_BAD;
+
 	mutex_lock(&hc->lock);
 	smb_hc_read(hc, ACPI_SMB_ALARM_ADDRESS, &address);
 	status.fields.alarm = 0;
@@ -233,20 +246,16 @@ static int smbus_alarm(void *context)
 					acpi_smbus_callback, hc);
 	}
 	mutex_unlock(&hc->lock);
-	return 0;
-}

-typedef int (*acpi_ec_query_func) (void *data);
-
-extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
-			      acpi_handle handle, acpi_ec_query_func func,
-			      void *data);
+	/* We may need to override existing _Qxx handlers */
+	return NOTIFY_BAD;
+}

 static int acpi_smbus_hc_add(struct acpi_device *device)
 {
-	int status;
 	unsigned long long val;
 	struct acpi_smb_hc *hc;
+	int status, ret;

 	if (!device)
 		return -EINVAL;
@@ -271,15 +280,19 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
 	hc->query_bit = val & 0xff;
 	device->driver_data = hc;

-	acpi_ec_add_query_handler(hc->ec, hc->query_bit, NULL, smbus_alarm, hc);
+	hc->nb.notifier_call = acpi_smbus_hc_notify;
+	ret = register_acpi_ec_query_notifier(&hc->nb);
+	if (ret < 0) {
+		kfree(hc);
+		return ret;
+	}
+
 	dev_info(&device->dev, "SBS HC: offset = 0x%0x, query_bit = 0x%0x\n",
 		 hc->offset, hc->query_bit);

 	return 0;
 }

-extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
-
 static void acpi_smbus_hc_remove(struct acpi_device *device)
 {
 	struct acpi_smb_hc *hc;
@@ -288,7 +301,7 @@ static void acpi_smbus_hc_remove(struct acpi_device *device)
 		return;

 	hc = acpi_driver_data(device);
-	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
+	unregister_acpi_ec_query_notifier(&hc->nb);
 	acpi_os_wait_events_complete();
 	kfree(hc);
 	device->driver_data = NULL;
--
2.30.2


  parent reply	other threads:[~2023-02-25 11:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-25 11:51 [PATCH v2 0/4] ACPI: SBS: Fix various issues Armin Wolf
2023-02-25 11:51 ` [PATCH v2 1/4] ACPI: EC: Add query notifier support Armin Wolf
2023-02-25 11:51 ` Armin Wolf [this message]
2023-02-25 11:51 ` [PATCH v2 3/4] ACPI: EC: Make query handlers private Armin Wolf
2023-02-25 11:51 ` [PATCH v2 4/4] ACPI: SBS: Fix handling of Smart Battery Selectors Armin Wolf
2023-03-12 17:15 ` [PATCH v2 0/4] ACPI: SBS: Fix various issues Armin Wolf
2023-03-14 19:49   ` Rafael J. Wysocki
2023-03-14 23:04     ` Armin Wolf

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=20230225115144.31212-3-W_Armin@gmx.de \
    --to=w_armin@gmx.de \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox