All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lv Zheng <lv.zheng@intel.com>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <len.brown@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>, Lv Zheng <zetalog@gmail.com>,
	linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: [PATCH v2 1/5] ACPI / EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag.
Date: Fri,  6 Feb 2015 08:57:52 +0800	[thread overview]
Message-ID: <aefbdd96f6594b4e7eb0c448aea4944f4806c42c.1423183648.git.lv.zheng@intel.com> (raw)
In-Reply-To: <cover.1423183647.git.lv.zheng@intel.com>

By using the 2 flags, we can indicate an inter-mediate state where the
current transactions should be completed while the new transactions should
be dropped.

The comparison of the old flag and the new flags:
  Old			New
  about to set BLOCKED	STOPPED set / STARTED set
  BLOCKED set		STOPPED clear / STARTED clear
  BLOCKED clear		STOPPED clear / STARTED set
A new period can be indicated by the 2 flags. The new period is between the
point where we are about to set BLOCKED and the point when the BLOCKED is
set. The new flags facilitate us with acpi_ec_started() check to allow the
EC transaction to be submitted during the new period. This period thus can
be used as a grace period for the EC transaction flushing.

The only functional change after applying this patch is:
1. The GPE enabling/disabling is protected by the EC specific lock. We can
   do this because of recent ACPICA GPE API enhancement. This is reasonable
   as the GPE disabling/enabling state should only be determined by the EC
   driver's state machine which is protected by the EC spinlock.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Ortwin Glück <odi@odi.ch>
---
 drivers/acpi/ec.c |   65 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 5563253..a6179b7 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -80,7 +80,8 @@ enum {
 	EC_FLAGS_GPE_STORM,		/* GPE storm detected */
 	EC_FLAGS_HANDLERS_INSTALLED,	/* Handlers for GPE and
 					 * OpReg are installed */
-	EC_FLAGS_BLOCKED,		/* Transactions are blocked */
+	EC_FLAGS_STARTED,		/* Driver is started */
+	EC_FLAGS_STOPPED,		/* Driver is stopped */
 };
 
 #define ACPI_EC_COMMAND_POLL		0x01 /* Available for command byte */
@@ -135,6 +136,16 @@ static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */
 static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */
 
 /* --------------------------------------------------------------------------
+ *                           Device Flags
+ * -------------------------------------------------------------------------- */
+
+static bool acpi_ec_started(struct acpi_ec *ec)
+{
+	return test_bit(EC_FLAGS_STARTED, &ec->flags) &&
+	       !test_bit(EC_FLAGS_STOPPED, &ec->flags);
+}
+
+/* --------------------------------------------------------------------------
  *                           EC Registers
  * -------------------------------------------------------------------------- */
 
@@ -415,6 +426,10 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 		udelay(ACPI_EC_MSI_UDELAY);
 	/* start transaction */
 	spin_lock_irqsave(&ec->lock, tmp);
+	if (!acpi_ec_started(ec)) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 	/* following two actions should be kept atomic */
 	ec->curr = t;
 	pr_debug("***** Command(%s) started *****\n",
@@ -426,6 +441,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 	pr_debug("***** Command(%s) stopped *****\n",
 		 acpi_ec_cmd_string(t->command));
 	ec->curr = NULL;
+unlock:
 	spin_unlock_irqrestore(&ec->lock, tmp);
 	return ret;
 }
@@ -440,10 +456,6 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
 	if (t->rdata)
 		memset(t->rdata, 0, t->rlen);
 	mutex_lock(&ec->mutex);
-	if (test_bit(EC_FLAGS_BLOCKED, &ec->flags)) {
-		status = -EINVAL;
-		goto unlock;
-	}
 	if (ec->global_lock) {
 		status = acpi_acquire_global_lock(ACPI_EC_UDELAY_GLK, &glk);
 		if (ACPI_FAILURE(status)) {
@@ -595,6 +607,37 @@ static void acpi_ec_clear(struct acpi_ec *ec)
 		pr_info("%d stale EC events cleared\n", i);
 }
 
+static void acpi_ec_start(struct acpi_ec *ec, bool resuming)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ec->lock, flags);
+	if (!test_and_set_bit(EC_FLAGS_STARTED, &ec->flags)) {
+		pr_debug("+++++ Starting EC +++++\n");
+		if (!resuming)
+			acpi_ec_enable_gpe(ec, true);
+		pr_info("+++++ EC started +++++\n");
+	}
+	spin_unlock_irqrestore(&ec->lock, flags);
+}
+
+static void acpi_ec_stop(struct acpi_ec *ec, bool suspending)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ec->lock, flags);
+	if (acpi_ec_started(ec)) {
+		pr_debug("+++++ Stopping EC +++++\n");
+		set_bit(EC_FLAGS_STOPPED, &ec->flags);
+		if (!suspending)
+			acpi_ec_disable_gpe(ec, true);
+		clear_bit(EC_FLAGS_STARTED, &ec->flags);
+		clear_bit(EC_FLAGS_STOPPED, &ec->flags);
+		pr_info("+++++ EC stopped +++++\n");
+	}
+	spin_unlock_irqrestore(&ec->lock, flags);
+}
+
 void acpi_ec_block_transactions(void)
 {
 	struct acpi_ec *ec = first_ec;
@@ -604,7 +647,7 @@ void acpi_ec_block_transactions(void)
 
 	mutex_lock(&ec->mutex);
 	/* Prevent transactions from being carried out */
-	set_bit(EC_FLAGS_BLOCKED, &ec->flags);
+	acpi_ec_stop(ec, true);
 	mutex_unlock(&ec->mutex);
 }
 
@@ -616,7 +659,7 @@ void acpi_ec_unblock_transactions(void)
 		return;
 
 	/* Allow transactions to be carried out again */
-	clear_bit(EC_FLAGS_BLOCKED, &ec->flags);
+	acpi_ec_start(ec, true);
 
 	if (EC_FLAGS_CLEAR_ON_RESUME)
 		acpi_ec_clear(ec);
@@ -629,7 +672,7 @@ void acpi_ec_unblock_transactions_early(void)
 	 * atomic context during wakeup, so we don't need to acquire the mutex).
 	 */
 	if (first_ec)
-		clear_bit(EC_FLAGS_BLOCKED, &first_ec->flags);
+		acpi_ec_start(first_ec, true);
 }
 
 /* --------------------------------------------------------------------------
@@ -894,7 +937,7 @@ static int ec_install_handlers(struct acpi_ec *ec)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	acpi_ec_enable_gpe(ec, true);
+	acpi_ec_start(ec, false);
 	status = acpi_install_address_space_handler(ec->handle,
 						    ACPI_ADR_SPACE_EC,
 						    &acpi_ec_space_handler,
@@ -909,7 +952,7 @@ static int ec_install_handlers(struct acpi_ec *ec)
 			pr_err("Fail in evaluating the _REG object"
 				" of EC device. Broken bios is suspected.\n");
 		} else {
-			acpi_ec_disable_gpe(ec, true);
+			acpi_ec_stop(ec, false);
 			acpi_remove_gpe_handler(NULL, ec->gpe,
 				&acpi_ec_gpe_handler);
 			return -ENODEV;
@@ -924,7 +967,7 @@ static void ec_remove_handlers(struct acpi_ec *ec)
 {
 	if (!test_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags))
 		return;
-	acpi_ec_disable_gpe(ec, true);
+	acpi_ec_stop(ec, false);
 	if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
 				ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
 		pr_err("failed to remove space handler\n");
-- 
1.7.10

WARNING: multiple messages have this Message-ID (diff)
From: Lv Zheng <lv.zheng@intel.com>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <len.brown@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>, Lv Zheng <zetalog@gmail.com>,
	<linux-kernel@vger.kernel.org>,
	linux-acpi@vger.kernel.org
Subject: [PATCH v2 1/5] ACPI / EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag.
Date: Fri,  6 Feb 2015 08:57:52 +0800	[thread overview]
Message-ID: <aefbdd96f6594b4e7eb0c448aea4944f4806c42c.1423183648.git.lv.zheng@intel.com> (raw)
In-Reply-To: <cover.1423183647.git.lv.zheng@intel.com>

By using the 2 flags, we can indicate an inter-mediate state where the
current transactions should be completed while the new transactions should
be dropped.

The comparison of the old flag and the new flags:
  Old			New
  about to set BLOCKED	STOPPED set / STARTED set
  BLOCKED set		STOPPED clear / STARTED clear
  BLOCKED clear		STOPPED clear / STARTED set
A new period can be indicated by the 2 flags. The new period is between the
point where we are about to set BLOCKED and the point when the BLOCKED is
set. The new flags facilitate us with acpi_ec_started() check to allow the
EC transaction to be submitted during the new period. This period thus can
be used as a grace period for the EC transaction flushing.

The only functional change after applying this patch is:
1. The GPE enabling/disabling is protected by the EC specific lock. We can
   do this because of recent ACPICA GPE API enhancement. This is reasonable
   as the GPE disabling/enabling state should only be determined by the EC
   driver's state machine which is protected by the EC spinlock.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Ortwin Glück <odi@odi.ch>
---
 drivers/acpi/ec.c |   65 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 5563253..a6179b7 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -80,7 +80,8 @@ enum {
 	EC_FLAGS_GPE_STORM,		/* GPE storm detected */
 	EC_FLAGS_HANDLERS_INSTALLED,	/* Handlers for GPE and
 					 * OpReg are installed */
-	EC_FLAGS_BLOCKED,		/* Transactions are blocked */
+	EC_FLAGS_STARTED,		/* Driver is started */
+	EC_FLAGS_STOPPED,		/* Driver is stopped */
 };
 
 #define ACPI_EC_COMMAND_POLL		0x01 /* Available for command byte */
@@ -135,6 +136,16 @@ static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */
 static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */
 
 /* --------------------------------------------------------------------------
+ *                           Device Flags
+ * -------------------------------------------------------------------------- */
+
+static bool acpi_ec_started(struct acpi_ec *ec)
+{
+	return test_bit(EC_FLAGS_STARTED, &ec->flags) &&
+	       !test_bit(EC_FLAGS_STOPPED, &ec->flags);
+}
+
+/* --------------------------------------------------------------------------
  *                           EC Registers
  * -------------------------------------------------------------------------- */
 
@@ -415,6 +426,10 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 		udelay(ACPI_EC_MSI_UDELAY);
 	/* start transaction */
 	spin_lock_irqsave(&ec->lock, tmp);
+	if (!acpi_ec_started(ec)) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 	/* following two actions should be kept atomic */
 	ec->curr = t;
 	pr_debug("***** Command(%s) started *****\n",
@@ -426,6 +441,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 	pr_debug("***** Command(%s) stopped *****\n",
 		 acpi_ec_cmd_string(t->command));
 	ec->curr = NULL;
+unlock:
 	spin_unlock_irqrestore(&ec->lock, tmp);
 	return ret;
 }
@@ -440,10 +456,6 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
 	if (t->rdata)
 		memset(t->rdata, 0, t->rlen);
 	mutex_lock(&ec->mutex);
-	if (test_bit(EC_FLAGS_BLOCKED, &ec->flags)) {
-		status = -EINVAL;
-		goto unlock;
-	}
 	if (ec->global_lock) {
 		status = acpi_acquire_global_lock(ACPI_EC_UDELAY_GLK, &glk);
 		if (ACPI_FAILURE(status)) {
@@ -595,6 +607,37 @@ static void acpi_ec_clear(struct acpi_ec *ec)
 		pr_info("%d stale EC events cleared\n", i);
 }
 
+static void acpi_ec_start(struct acpi_ec *ec, bool resuming)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ec->lock, flags);
+	if (!test_and_set_bit(EC_FLAGS_STARTED, &ec->flags)) {
+		pr_debug("+++++ Starting EC +++++\n");
+		if (!resuming)
+			acpi_ec_enable_gpe(ec, true);
+		pr_info("+++++ EC started +++++\n");
+	}
+	spin_unlock_irqrestore(&ec->lock, flags);
+}
+
+static void acpi_ec_stop(struct acpi_ec *ec, bool suspending)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ec->lock, flags);
+	if (acpi_ec_started(ec)) {
+		pr_debug("+++++ Stopping EC +++++\n");
+		set_bit(EC_FLAGS_STOPPED, &ec->flags);
+		if (!suspending)
+			acpi_ec_disable_gpe(ec, true);
+		clear_bit(EC_FLAGS_STARTED, &ec->flags);
+		clear_bit(EC_FLAGS_STOPPED, &ec->flags);
+		pr_info("+++++ EC stopped +++++\n");
+	}
+	spin_unlock_irqrestore(&ec->lock, flags);
+}
+
 void acpi_ec_block_transactions(void)
 {
 	struct acpi_ec *ec = first_ec;
@@ -604,7 +647,7 @@ void acpi_ec_block_transactions(void)
 
 	mutex_lock(&ec->mutex);
 	/* Prevent transactions from being carried out */
-	set_bit(EC_FLAGS_BLOCKED, &ec->flags);
+	acpi_ec_stop(ec, true);
 	mutex_unlock(&ec->mutex);
 }
 
@@ -616,7 +659,7 @@ void acpi_ec_unblock_transactions(void)
 		return;
 
 	/* Allow transactions to be carried out again */
-	clear_bit(EC_FLAGS_BLOCKED, &ec->flags);
+	acpi_ec_start(ec, true);
 
 	if (EC_FLAGS_CLEAR_ON_RESUME)
 		acpi_ec_clear(ec);
@@ -629,7 +672,7 @@ void acpi_ec_unblock_transactions_early(void)
 	 * atomic context during wakeup, so we don't need to acquire the mutex).
 	 */
 	if (first_ec)
-		clear_bit(EC_FLAGS_BLOCKED, &first_ec->flags);
+		acpi_ec_start(first_ec, true);
 }
 
 /* --------------------------------------------------------------------------
@@ -894,7 +937,7 @@ static int ec_install_handlers(struct acpi_ec *ec)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	acpi_ec_enable_gpe(ec, true);
+	acpi_ec_start(ec, false);
 	status = acpi_install_address_space_handler(ec->handle,
 						    ACPI_ADR_SPACE_EC,
 						    &acpi_ec_space_handler,
@@ -909,7 +952,7 @@ static int ec_install_handlers(struct acpi_ec *ec)
 			pr_err("Fail in evaluating the _REG object"
 				" of EC device. Broken bios is suspected.\n");
 		} else {
-			acpi_ec_disable_gpe(ec, true);
+			acpi_ec_stop(ec, false);
 			acpi_remove_gpe_handler(NULL, ec->gpe,
 				&acpi_ec_gpe_handler);
 			return -ENODEV;
@@ -924,7 +967,7 @@ static void ec_remove_handlers(struct acpi_ec *ec)
 {
 	if (!test_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags))
 		return;
-	acpi_ec_disable_gpe(ec, true);
+	acpi_ec_stop(ec, false);
 	if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
 				ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
 		pr_err("failed to remove space handler\n");
-- 
1.7.10


  reply	other threads:[~2015-02-06  0:57 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-03  5:16 [PATCH 0/6] ACPI/EC: Cleanups of command flushing and event polling Lv Zheng
2014-11-03  5:16 ` Lv Zheng
2014-11-03  5:16 ` [PATCH 1/6] ACPI/EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Lv Zheng
2014-11-03  5:16   ` Lv Zheng
2014-11-05  2:52   ` Zheng, Lv
2014-11-05  2:52     ` Zheng, Lv
2014-11-18 13:23     ` Kirill A. Shutemov
2014-11-18 21:20       ` Rafael J. Wysocki
2014-11-19  8:55         ` Zheng, Lv
2014-11-19  8:55           ` Zheng, Lv
2014-11-19 12:16         ` Kirill A. Shutemov
2014-11-20  2:19           ` [RFC PATCH v3 1/2] ACPICA: Events: Remove duplicated sanity check in acpi_ev_enable_gpe() Lv Zheng
2014-11-20  2:19             ` Lv Zheng
2014-11-20  2:19           ` [RFC PATCH v3 2/2] ACPICA: Events: Introduce ACPI_GPE_HANDLER_RAW to fix 2 issues for the current GPE APIs Lv Zheng
2014-11-20  2:19             ` Lv Zheng
2014-11-20  2:20           ` [PATCH 1/6] ACPI/EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Zheng, Lv
2014-11-20 21:33             ` Kirill A. Shutemov
2014-11-21  0:42               ` Zheng, Lv
2014-11-21  0:55               ` Zheng, Lv
2014-11-20  2:34           ` Zheng, Lv
2014-11-20 21:34             ` Kirill A. Shutemov
2014-11-20  6:44           ` [RFC PATCH v4] ACPICA/Events: Add support to ensure GPE is disabled by default for handlers Lv Zheng
2014-11-20  6:44             ` Lv Zheng
2014-11-20  6:47             ` Zheng, Lv
2014-11-20 22:15               ` Kirill A. Shutemov
2014-11-21  1:36                 ` Zheng, Lv
2015-02-06  0:57           ` [PATCH v2 0/5] ACPI / EC: Add reference counting for requests and cleans up the grace periods support Lv Zheng
2015-02-06  0:57             ` Lv Zheng
2015-02-06  0:57             ` Lv Zheng [this message]
2015-02-06  0:57               ` [PATCH v2 1/5] ACPI / EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Lv Zheng
2015-02-06  0:57             ` [PATCH v2 2/5] ACPI / EC: Add command flushing support Lv Zheng
2015-02-06  0:57               ` Lv Zheng
2015-02-06  0:58             ` [PATCH v2 3/5] ACPI / EC: Refine command storm prevention support Lv Zheng
2015-02-06  0:58               ` Lv Zheng
2015-02-06  0:58             ` [PATCH v2 4/5] ACPI / EC: Add query flushing support Lv Zheng
2015-02-06  0:58               ` Lv Zheng
2015-02-06  0:58             ` [PATCH v2 5/5] ACPI / EC: Add GPE reference counting debugging messages Lv Zheng
2015-02-06  0:58               ` Lv Zheng
2015-02-06  1:26             ` [PATCH v2 0/5] ACPI / EC: Add reference counting for requests and cleans up the grace periods support Rafael J. Wysocki
2015-02-09  2:23               ` Zheng, Lv
2015-02-09  2:23                 ` Zheng, Lv
2015-02-12  1:17                 ` Rafael J. Wysocki
2015-02-16  6:41                   ` Zheng, Lv
2015-02-16  6:41                     ` Zheng, Lv
2014-11-03  5:16 ` [PATCH 2/6] ACPI/EC: Enhance the checks to apply to QR_EC transactions Lv Zheng
2014-11-03  5:16   ` Lv Zheng
2014-11-03  5:16 ` [PATCH 3/6] ACPI/EC: Add reference counting for query handlers Lv Zheng
2014-11-03  5:16   ` Lv Zheng
2014-11-03  5:16 ` [PATCH 4/6] ACPI/EC: Add command flushing support Lv Zheng
2014-11-03  5:16   ` Lv Zheng
2014-11-03  5:16 ` [PATCH 5/6] ACPI/EC: Cleanup QR_SC command processing by adding a kernel thread to poll EC events Lv Zheng
2014-11-03  5:16   ` Lv Zheng
2014-11-12  1:16   ` Rafael J. Wysocki
2014-11-12  1:16     ` Rafael J. Wysocki
2014-11-13  2:31     ` Zheng, Lv
2014-11-13  2:31       ` Zheng, Lv
2014-11-13  2:58       ` Rafael J. Wysocki
2014-11-13  2:52         ` Zheng, Lv
2014-11-13  2:52           ` Zheng, Lv
2014-11-13 22:37           ` Rafael J. Wysocki
2014-11-14  1:21             ` Zheng, Lv
2014-11-14  1:21               ` Zheng, Lv
2014-11-14 23:38               ` Rafael J. Wysocki
2014-11-03  5:16 ` [PATCH 6/6] ACPI/EC: Add GPE reference counting debugging messages Lv Zheng
2014-11-03  5:16   ` Lv Zheng

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=aefbdd96f6594b4e7eb0c448aea4944f4806c42c.1423183648.git.lv.zheng@intel.com \
    --to=lv.zheng@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=zetalog@gmail.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.