lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 17 Sep 2020 12:39:24 -0500
From:   Alex Elder <elder@...aro.org>
To:     davem@...emloft.net, kuba@...nel.org
Cc:     evgreen@...omium.org, subashab@...eaurora.org,
        cpratapa@...eaurora.org, bjorn.andersson@...aro.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH net-next v3 5/7] net: ipa: repurpose CLOCK_HELD flag

The previous patch causes a system resume to be triggered when a
packet is available for receipt on a suspended RX endpoint.

The CLOCK_HELD flag was previously used to indicate that an extra
clock reference was held, preventing suspend.  But we no longer need
such a flag:
  - We take an initial reference in ipa_config().
  - That reference is held until ipa_suspend() releases it.
  - A subsequent system resume leads to a reference getting
    re-acquired in ipa_resume().
  - This can repeat until ultimately the module is removed, where
    ipa_remove() releases the reference.
We no longer need a special flag to determine whether this extra
reference is held--it is, provided probe has completed successfully
and the driver is not suspended (or removed).

On the other hand, once suspended, it's possible for more than one
endpoint to trip the IPA SUSPEND interrupt, and we only want to
trigger the system resume once.  So repurpose the Boolean CLOCK_HELD
flag to record whether the IPA SUSPEND handler should initiate a
system resume.

The flag will be be cleared each time ipa_suspend() is called,
*before* any endpoints are suspended.  And it will be set inside the
IPA SUSPEND interrupt handler exactly once per suspend.

Rename the flag IPA_FLAG_RESUMED to reflect its new purpose.

Signed-off-by: Alex Elder <elder@...aro.org>
---
v3: New; eliminated need for "clock held" flag, as Bjorn suggested.

 drivers/net/ipa/ipa.h      |  6 +++---
 drivers/net/ipa/ipa_main.c | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ipa/ipa.h b/drivers/net/ipa/ipa.h
index c688155ccf375..6c2371084c55a 100644
--- a/drivers/net/ipa/ipa.h
+++ b/drivers/net/ipa/ipa.h
@@ -29,11 +29,11 @@ struct ipa_interrupt;
 
 /**
  * enum ipa_flag - IPA state flags
- * @IPA_FLAG_CLOCK_HELD:	Whether IPA clock is held to prevent suspend
- * @IPA_FLAG_COUNT:		Number of defined IPA flags
+ * @IPA_FLAG_RESUMED:	Whether resume from suspend has been signaled
+ * @IPA_FLAG_COUNT:	Number of defined IPA flags
  */
 enum ipa_flag {
-	IPA_FLAG_CLOCK_HELD,
+	IPA_FLAG_RESUMED,
 	IPA_FLAG_COUNT,		/* Last; not a flag */
 };
 
diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 4e2508bb1bf80..1044758b501d2 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -86,7 +86,7 @@ static void ipa_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id)
 	 * More than one endpoint could signal this; if so, ignore
 	 * all but the first.
 	 */
-	if (!test_and_set_bit(IPA_FLAG_CLOCK_HELD, ipa->flags))
+	if (!test_and_set_bit(IPA_FLAG_RESUMED, ipa->flags))
 		pm_wakeup_dev_event(&ipa->pdev->dev, 0, true);
 
 	/* Acknowledge/clear the suspend interrupt on all endpoints */
@@ -518,7 +518,6 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data)
 	 * is held after initialization completes, and won't get dropped
 	 * unless/until a system suspend request arrives.
 	 */
-	__set_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 	ipa_clock_get(ipa);
 
 	ipa_hardware_config(ipa);
@@ -554,7 +553,6 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data)
 err_hardware_deconfig:
 	ipa_hardware_deconfig(ipa);
 	ipa_clock_put(ipa);
-	__clear_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 
 	return ret;
 }
@@ -572,7 +570,6 @@ static void ipa_deconfig(struct ipa *ipa)
 	ipa_endpoint_deconfig(ipa);
 	ipa_hardware_deconfig(ipa);
 	ipa_clock_put(ipa);
-	__clear_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 }
 
 static int ipa_firmware_load(struct device *dev)
@@ -778,7 +775,6 @@ static int ipa_probe(struct platform_device *pdev)
 	dev_set_drvdata(dev, ipa);
 	ipa->modem_rproc = rproc;
 	ipa->clock = clock;
-	__clear_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 	ipa->version = data->version;
 
 	ret = ipa_reg_init(ipa);
@@ -908,10 +904,15 @@ static int ipa_suspend(struct device *dev)
 {
 	struct ipa *ipa = dev_get_drvdata(dev);
 
+	/* When a suspended RX endpoint has a packet ready to receive, we
+	 * get an IPA SUSPEND interrupt.  We trigger a system resume in
+	 * that case, but only on the first such interrupt since suspend.
+	 */
+	__clear_bit(IPA_FLAG_RESUMED, ipa->flags);
+
 	ipa_endpoint_suspend(ipa);
 
 	ipa_clock_put(ipa);
-	__clear_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 
 	return 0;
 }
@@ -933,7 +934,6 @@ static int ipa_resume(struct device *dev)
 	/* This clock reference will keep the IPA out of suspend
 	 * until we get a power management suspend request.
 	 */
-	__set_bit(IPA_FLAG_CLOCK_HELD, ipa->flags);
 	ipa_clock_get(ipa);
 
 	ipa_endpoint_resume(ipa);
-- 
2.20.1

Powered by blists - more mailing lists