All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux ACPI <linux-acpi@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Michal Wilczynski <michal.wilczynski@intel.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Daniel Drake <drake@endlessm.com>
Subject: [PATCH v1 3/3] ACPI: EC: Use a spin lock without disabing interrupts
Date: Fri, 15 Dec 2023 12:27:30 +0100	[thread overview]
Message-ID: <3287818.aeNJFYEL58@kreacher> (raw)
In-Reply-To: <2330034.ElGaqSPkdT@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since all of the ACPI EC driver code runs in thread context after recent
changes, it does not need to disable interrupts on the local CPU when
acquiring a spin lock.

Make it use the spin lock without disabling interrupts.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/ec.c |  112 ++++++++++++++++++++++--------------------------------
 1 file changed, 46 insertions(+), 66 deletions(-)

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -525,12 +525,10 @@ static void acpi_ec_clear(struct acpi_ec
 
 static void acpi_ec_enable_event(struct acpi_ec *ec)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	if (acpi_ec_started(ec))
 		__acpi_ec_enable_event(ec);
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 
 	/* Drain additional events if hardware requires that */
 	if (EC_FLAGS_CLEAR_ON_RESUME)
@@ -546,11 +544,9 @@ static void __acpi_ec_flush_work(void)
 
 static void acpi_ec_disable_event(struct acpi_ec *ec)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	__acpi_ec_disable_event(ec);
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 
 	/*
 	 * When ec_freeze_events is true, we need to flush events in
@@ -571,10 +567,9 @@ void acpi_ec_flush_work(void)
 
 static bool acpi_ec_guard_event(struct acpi_ec *ec)
 {
-	unsigned long flags;
 	bool guarded;
 
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	/*
 	 * If firmware SCI_EVT clearing timing is "event", we actually
 	 * don't know when the SCI_EVT will be cleared by firmware after
@@ -590,31 +585,29 @@ static bool acpi_ec_guard_event(struct a
 	guarded = ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT &&
 		ec->event_state != EC_EVENT_READY &&
 		(!ec->curr || ec->curr->command != ACPI_EC_COMMAND_QUERY);
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 	return guarded;
 }
 
 static int ec_transaction_polled(struct acpi_ec *ec)
 {
-	unsigned long flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	if (ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_POLL))
 		ret = 1;
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 	return ret;
 }
 
 static int ec_transaction_completed(struct acpi_ec *ec)
 {
-	unsigned long flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	if (ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_COMPLETE))
 		ret = 1;
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 	return ret;
 }
 
@@ -756,7 +749,6 @@ static int ec_guard(struct acpi_ec *ec)
 
 static int ec_poll(struct acpi_ec *ec)
 {
-	unsigned long flags;
 	int repeat = 5; /* number of command restarts */
 
 	while (repeat--) {
@@ -765,14 +757,14 @@ static int ec_poll(struct acpi_ec *ec)
 		do {
 			if (!ec_guard(ec))
 				return 0;
-			spin_lock_irqsave(&ec->lock, flags);
+			spin_lock(&ec->lock);
 			advance_transaction(ec, false);
-			spin_unlock_irqrestore(&ec->lock, flags);
+			spin_unlock(&ec->lock);
 		} while (time_before(jiffies, delay));
 		pr_debug("controller reset, restart transaction\n");
-		spin_lock_irqsave(&ec->lock, flags);
+		spin_lock(&ec->lock);
 		start_transaction(ec);
-		spin_unlock_irqrestore(&ec->lock, flags);
+		spin_unlock(&ec->lock);
 	}
 	return -ETIME;
 }
@@ -780,11 +772,10 @@ static int ec_poll(struct acpi_ec *ec)
 static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 					struct transaction *t)
 {
-	unsigned long tmp;
 	int ret = 0;
 
 	/* start transaction */
-	spin_lock_irqsave(&ec->lock, tmp);
+	spin_lock(&ec->lock);
 	/* Enable GPE for command processing (IBF=0/OBF=1) */
 	if (!acpi_ec_submit_flushable_request(ec)) {
 		ret = -EINVAL;
@@ -795,11 +786,11 @@ static int acpi_ec_transaction_unlocked(
 	ec->curr = t;
 	ec_dbg_req("Command(%s) started", acpi_ec_cmd_string(t->command));
 	start_transaction(ec);
-	spin_unlock_irqrestore(&ec->lock, tmp);
+	spin_unlock(&ec->lock);
 
 	ret = ec_poll(ec);
 
-	spin_lock_irqsave(&ec->lock, tmp);
+	spin_lock(&ec->lock);
 	if (t->irq_count == ec_storm_threshold)
 		acpi_ec_unmask_events(ec);
 	ec_dbg_req("Command(%s) stopped", acpi_ec_cmd_string(t->command));
@@ -808,7 +799,7 @@ static int acpi_ec_transaction_unlocked(
 	acpi_ec_complete_request(ec);
 	ec_dbg_ref(ec, "Decrease command");
 unlock:
-	spin_unlock_irqrestore(&ec->lock, tmp);
+	spin_unlock(&ec->lock);
 	return ret;
 }
 
@@ -936,9 +927,7 @@ EXPORT_SYMBOL(ec_get_handle);
 
 static void acpi_ec_start(struct acpi_ec *ec, bool resuming)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	if (!test_and_set_bit(EC_FLAGS_STARTED, &ec->flags)) {
 		ec_dbg_drv("Starting EC");
 		/* Enable GPE for event processing (SCI_EVT=1) */
@@ -948,31 +937,28 @@ static void acpi_ec_start(struct acpi_ec
 		}
 		ec_log_drv("EC started");
 	}
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 }
 
 static bool acpi_ec_stopped(struct acpi_ec *ec)
 {
-	unsigned long flags;
 	bool flushed;
 
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	flushed = acpi_ec_flushed(ec);
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 	return flushed;
 }
 
 static void acpi_ec_stop(struct acpi_ec *ec, bool suspending)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	if (acpi_ec_started(ec)) {
 		ec_dbg_drv("Stopping EC");
 		set_bit(EC_FLAGS_STOPPED, &ec->flags);
-		spin_unlock_irqrestore(&ec->lock, flags);
+		spin_unlock(&ec->lock);
 		wait_event(ec->wait, acpi_ec_stopped(ec));
-		spin_lock_irqsave(&ec->lock, flags);
+		spin_lock(&ec->lock);
 		/* Disable GPE for event processing (SCI_EVT=1) */
 		if (!suspending) {
 			acpi_ec_complete_request(ec);
@@ -983,29 +969,25 @@ static void acpi_ec_stop(struct acpi_ec
 		clear_bit(EC_FLAGS_STOPPED, &ec->flags);
 		ec_log_drv("EC stopped");
 	}
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 }
 
 static void acpi_ec_enter_noirq(struct acpi_ec *ec)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	ec->busy_polling = true;
 	ec->polling_guard = 0;
 	ec_log_drv("interrupt blocked");
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 }
 
 static void acpi_ec_leave_noirq(struct acpi_ec *ec)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 	ec->busy_polling = ec_busy_polling;
 	ec->polling_guard = ec_polling_guard;
 	ec_log_drv("interrupt unblocked");
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 }
 
 void acpi_ec_block_transactions(void)
@@ -1137,9 +1119,9 @@ static void acpi_ec_event_processor(stru
 
 	ec_dbg_evt("Query(0x%02x) stopped", handler->query_bit);
 
-	spin_lock_irq(&ec->lock);
+	spin_lock(&ec->lock);
 	ec->queries_in_progress--;
-	spin_unlock_irq(&ec->lock);
+	spin_unlock(&ec->lock);
 
 	acpi_ec_put_query_handler(handler);
 	kfree(q);
@@ -1202,12 +1184,12 @@ static int acpi_ec_submit_query(struct a
 	 */
 	ec_dbg_evt("Query(0x%02x) scheduled", value);
 
-	spin_lock_irq(&ec->lock);
+	spin_lock(&ec->lock);
 
 	ec->queries_in_progress++;
 	queue_work(ec_query_wq, &q->work);
 
-	spin_unlock_irq(&ec->lock);
+	spin_unlock(&ec->lock);
 
 	return 0;
 
@@ -1223,14 +1205,14 @@ static void acpi_ec_event_handler(struct
 
 	ec_dbg_evt("Event started");
 
-	spin_lock_irq(&ec->lock);
+	spin_lock(&ec->lock);
 
 	while (ec->events_to_process) {
-		spin_unlock_irq(&ec->lock);
+		spin_unlock(&ec->lock);
 
 		acpi_ec_submit_query(ec);
 
-		spin_lock_irq(&ec->lock);
+		spin_lock(&ec->lock);
 
 		ec->events_to_process--;
 	}
@@ -1247,11 +1229,11 @@ static void acpi_ec_event_handler(struct
 
 		ec_dbg_evt("Event stopped");
 
-		spin_unlock_irq(&ec->lock);
+		spin_unlock(&ec->lock);
 
 		guard_timeout = !!ec_guard(ec);
 
-		spin_lock_irq(&ec->lock);
+		spin_lock(&ec->lock);
 
 		/* Take care of SCI_EVT unless someone else is doing that. */
 		if (guard_timeout && !ec->curr)
@@ -1264,7 +1246,7 @@ static void acpi_ec_event_handler(struct
 
 	ec->events_in_progress--;
 
-	spin_unlock_irq(&ec->lock);
+	spin_unlock(&ec->lock);
 }
 
 static void clear_gpe_and_advance_transaction(struct acpi_ec *ec, bool interrupt)
@@ -1289,13 +1271,11 @@ static void clear_gpe_and_advance_transa
 
 static void acpi_ec_handle_interrupt(struct acpi_ec *ec)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ec->lock, flags);
+	spin_lock(&ec->lock);
 
 	clear_gpe_and_advance_transaction(ec, true);
 
-	spin_unlock_irqrestore(&ec->lock, flags);
+	spin_unlock(&ec->lock);
 }
 
 static u32 acpi_ec_gpe_handler(acpi_handle gpe_device,
@@ -2105,7 +2085,7 @@ bool acpi_ec_dispatch_gpe(void)
 	 * Dispatch the EC GPE in-band, but do not report wakeup in any case
 	 * to allow the caller to process events properly after that.
 	 */
-	spin_lock_irq(&first_ec->lock);
+	spin_lock(&first_ec->lock);
 
 	if (acpi_ec_gpe_status_set(first_ec)) {
 		pm_pr_dbg("ACPI EC GPE status set\n");
@@ -2114,7 +2094,7 @@ bool acpi_ec_dispatch_gpe(void)
 		work_in_progress = acpi_ec_work_in_progress(first_ec);
 	}
 
-	spin_unlock_irq(&first_ec->lock);
+	spin_unlock(&first_ec->lock);
 
 	if (!work_in_progress)
 		return false;
@@ -2127,11 +2107,11 @@ bool acpi_ec_dispatch_gpe(void)
 
 		pm_pr_dbg("ACPI EC work flushed\n");
 
-		spin_lock_irq(&first_ec->lock);
+		spin_lock(&first_ec->lock);
 
 		work_in_progress = acpi_ec_work_in_progress(first_ec);
 
-		spin_unlock_irq(&first_ec->lock);
+		spin_unlock(&first_ec->lock);
 	} while (work_in_progress && !pm_wakeup_pending());
 
 	return false;




  parent reply	other threads:[~2023-12-15 11:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-15 11:23 [PATCH v1 0/3] ACPI: OSL/EC: Use spin locks without disabling interrupts Rafael J. Wysocki
2023-12-15 11:25 ` [PATCH v1 1/3] ACPI: OSL: " Rafael J. Wysocki
2023-12-15 11:26 ` [PATCH v1 2/3] ACPI: EC: Use a threaded handler for dedicated IRQ Rafael J. Wysocki
2023-12-15 11:27 ` Rafael J. Wysocki [this message]
2023-12-28 13:18 ` [PATCH v1 0/3] ACPI: OSL/EC: Use spin locks without disabling interrupts Rafael J. Wysocki

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=3287818.aeNJFYEL58@kreacher \
    --to=rjw@rjwysocki.net \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=drake@endlessm.com \
    --cc=hdegoede@redhat.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=michal.wilczynski@intel.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rui.zhang@intel.com \
    --cc=srinivas.pandruvada@linux.intel.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: link
Be 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.