linux-kernel - [PATCH net-next v3 5/7] net: ipa: repurpose CLOCK

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 linux-cve-announce 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]

Message-Id: <20200917173926.24266-6-elder@linaro.org>
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