Skip to contentSkip to conversation

FAILED: patch "[PATCH] net: ipa: request IPA register values be retained" failed to apply to 5.15-stable tree

PATCH [email protected] 2 participants 6 messages Feb 4, 2022 – Feb 7, 2022

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 34a081761e4e3c35381cbfad609ebae2962fe2f8 Mon Sep 17 00:00:00 2001
From: Alex Elder <[email protected]>
Date: Tue, 1 Feb 2022 09:02:05 -0600
Subject: [PATCH] net: ipa: request IPA register values be retained

In some cases, the IPA hardware needs to request the always-on subsystem (AOSS) to coordinate with the IPA microcontroller to retain IPA register values at power collapse. This is done by issuing a QMP request to the AOSS microcontroller. A similar request ondoes that request.

We must get and hold the "QMP" handle early, because we might get back EPROBE_DEFER for that. But the actual request should be sent while we know the IPA clock is active, and when we know the microcontroller is operational.

Fixes 1aac309d3207 ("net: ipa: use autosuspend")
drivers/net/ipa/ipa_power.c +52 -0
index b1c6c0fcb654..f2989aac47a6 100644
@@ -11,6 +11,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/bitops.h>
 
+#include "linux/soc/qcom/qcom_aoss.h"
+
 #include "ipa.h"
 #include "ipa_power.h"
 #include "ipa_endpoint.h"
@@ -64,6 +66,7 @@ enum ipa_power_flag {
  * struct ipa_power - IPA power management information
  * @dev:		IPA device pointer
  * @core:		IPA core clock
+ * @qmp:		QMP handle for AOSS communication
  * @spinlock:		Protects modem TX queue enable/disable
  * @flags:		Boolean state flags
  * @interconnect_count:	Number of elements in interconnect[]
@@ -72,6 +75,7 @@ enum ipa_power_flag {
 struct ipa_power {
 	struct device *dev;
 	struct clk *core;
+	struct qmp *qmp;
 	spinlock_t spinlock;	/* used with STOPPED/STARTED power flags */
 	DECLARE_BITMAP(flags, IPA_POWER_FLAG_COUNT);
 	u32 interconnect_count;
@@ -382,6 +386,47 @@ void ipa_power_modem_queue_active(struct ipa *ipa)
 	clear_bit(IPA_POWER_FLAG_STARTED, ipa->power->flags);
 }
 
+static int ipa_power_retention_init(struct ipa_power *power)
+{
+	struct qmp *qmp = qmp_get(power->dev);
+
+	if (IS_ERR(qmp)) {
+		if (PTR_ERR(qmp) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		/* We assume any other error means it's not defined/needed */
+		qmp = NULL;
+	}
+	power->qmp = qmp;
+
+	return 0;
+}
+
+static void ipa_power_retention_exit(struct ipa_power *power)
+{
+	qmp_put(power->qmp);
+	power->qmp = NULL;
+}
+
+/* Control register retention on power collapse */
+void ipa_power_retention(struct ipa *ipa, bool enable)
+{
+	static const char fmt[] = "{ class: bcm, res: ipa_pc, val: %c }";
+	struct ipa_power *power = ipa->power;
+	char buf[36];	/* Exactly enough for fmt[]; size a multiple of 4 */
+	int ret;
+
+	if (!power->qmp)
+		return;		/* Not needed on this platform */
+
+	(void)snprintf(buf, sizeof(buf), fmt, enable ? '1' : '0');
+
+	ret = qmp_send(power->qmp, buf, sizeof(buf));
+	if (ret)
+		dev_err(power->dev, "error %d sending QMP %sable request\n",
+			ret, enable ? "en" : "dis");
+}
+
 int ipa_power_setup(struct ipa *ipa)
 {
 	int ret;
@@ -438,12 +483,18 @@ ipa_power_init(struct device *dev, const struct ipa_power_data *data)
 	if (ret)
 		goto err_kfree;
 
+	ret = ipa_power_retention_init(power);
+	if (ret)
+		goto err_interconnect_exit;
+
 	pm_runtime_set_autosuspend_delay(dev, IPA_AUTOSUSPEND_DELAY);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_enable(dev);
 
 	return power;
 
+err_interconnect_exit:
+	ipa_interconnect_exit(power);
 err_kfree:
 	kfree(power);
 err_clk_put:
@@ -460,6 +511,7 @@ void ipa_power_exit(struct ipa_power *power)
 
 	pm_runtime_disable(dev);
 	pm_runtime_dont_use_autosuspend(dev);
+	ipa_power_retention_exit(power);
 	ipa_interconnect_exit(power);
 	kfree(power);
 	clk_put(clk);
drivers/net/ipa/ipa_power.h +7 -0
index 2151805d7fbb..6f84f057a209 100644
@@ -40,6 +40,13 @@ void ipa_power_modem_queue_wake(struct ipa *ipa);
  */
 void ipa_power_modem_queue_active(struct ipa *ipa);
 
+/**
+ * ipa_power_retention() - Control register retention on power collapse
+ * @ipa:	IPA pointer
+ * @enable:	Whether retention should be enabled or disabled
+ */
+void ipa_power_retention(struct ipa *ipa, bool enable);
+
 /**
  * ipa_power_setup() - Set up IPA power management
  * @ipa:	IPA pointer
drivers/net/ipa/ipa_uc.c +5 -0
index 856e55a080a7..fe11910518d9 100644
@@ -11,6 +11,7 @@
 
 #include "ipa.h"
 #include "ipa_uc.h"
+#include "ipa_power.h"
 
 /**
  * DOC:  The IPA embedded microcontroller
@@ -154,6 +155,7 @@ static void ipa_uc_response_hdlr(struct ipa *ipa, enum ipa_irq_id irq_id)
 	case IPA_UC_RESPONSE_INIT_COMPLETED:
 		if (ipa->uc_powered) {
 			ipa->uc_loaded = true;
+			ipa_power_retention(ipa, true);
 			pm_runtime_mark_last_busy(dev);
 			(void)pm_runtime_put_autosuspend(dev);
 			ipa->uc_powered = false;
@@ -184,6 +186,9 @@ void ipa_uc_deconfig(struct ipa *ipa)
 
 	ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_1);
 	ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_0);
+	if (ipa->uc_loaded)
+		ipa_power_retention(ipa, false);
+
 	if (!ipa->uc_powered)
 		return;
 
#1 lore raw
Quoted 5 lines · On 2/4/22 2:50 AM, [email protected] wrote:

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

I just tried to apply this patch on v5.15.19 and it applied cleanly for me.

----------------

{2314} elder@presto-> git checkout -b test
Switched to a new branch 'test'
{2315} elder@presto-> git reset --hard v5.15.19
HEAD is now at 47cccb1eb2fec Linux 5.15.19
{2316} elder@presto-> git cherry-pick -x 34a081761e4e3c35381cbfad609ebae2962fe2f8 [test 71a06f5acbb05] net: ipa: request IPA register values be retained
Date: Tue Feb 1 09:02:05 2022 -0600

 3 files changed, 64 insertions(+)

{2317} elder@presto-> git log --oneline -2
71a06f5acbb05 (HEAD -> test) net: ipa: request IPA register values be retained 47cccb1eb2fec (tag: v5.15.19, stable/linux-5.15.y) Linux 5.15.19 {2318} elder@presto->

----------------

I don't understand. If you know that I'm doing something wrong, can you let me know what it might be?

While I have your attention on this, there is another commit that should be back-ported with this. It did not have the "Fixes" tag, unfortunately.

Bjorn has committed it in the "arm64-for-5.18" branch in the Qualcomm repository:
git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
73419e4d2fd1b arm64: dts: qcom: add IPA qcom,qmp property

He says he has no plans to rebase this branch before it gets pulled.

I can send a followup message when that commit lands in v5.18-rc1, but it might be simplest to take care of it now.

What do you think?

FYI, here's the message:
https://lore.kernel.org/lkml/[email protected]/

Thanks.

-Alex

Quoted 179 lines

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 34a081761e4e3c35381cbfad609ebae2962fe2f8 Mon Sep 17 00:00:00 2001
From: Alex Elder <[email protected]>
Date: Tue, 1 Feb 2022 09:02:05 -0600
Subject: [PATCH] net: ipa: request IPA register values be retained

In some cases, the IPA hardware needs to request the always-on subsystem (AOSS) to coordinate with the IPA microcontroller to retain IPA register values at power collapse. This is done by issuing a QMP request to the AOSS microcontroller. A similar request ondoes that request.

We must get and hold the "QMP" handle early, because we might get back EPROBE_DEFER for that. But the actual request should be sent while we know the IPA clock is active, and when we know the microcontroller is operational.

Fixes 1aac309d3207 ("net: ipa: use autosuspend")
drivers/net/ipa/ipa_power.c +52 -0
index b1c6c0fcb654..f2989aac47a6 100644
@@ -11,6 +11,8 @@
  #include <linux/pm_runtime.h>
  #include <linux/bitops.h>
 
+#include "linux/soc/qcom/qcom_aoss.h"
+
  #include "ipa.h"
  #include "ipa_power.h"
  #include "ipa_endpoint.h"
@@ -64,6 +66,7 @@ enum ipa_power_flag {
   * struct ipa_power - IPA power management information
   * @dev:		IPA device pointer
   * @core:		IPA core clock
+ * @qmp:		QMP handle for AOSS communication
   * @spinlock:		Protects modem TX queue enable/disable
   * @flags:		Boolean state flags
   * @interconnect_count:	Number of elements in interconnect[]
@@ -72,6 +75,7 @@ enum ipa_power_flag {
  struct ipa_power {
  	struct device *dev;
  	struct clk *core;
+	struct qmp *qmp;
  	spinlock_t spinlock;	/* used with STOPPED/STARTED power flags */
  	DECLARE_BITMAP(flags, IPA_POWER_FLAG_COUNT);
  	u32 interconnect_count;
@@ -382,6 +386,47 @@ void ipa_power_modem_queue_active(struct ipa *ipa)
  	clear_bit(IPA_POWER_FLAG_STARTED, ipa->power->flags);
  }
 
+static int ipa_power_retention_init(struct ipa_power *power)
+{
+	struct qmp *qmp = qmp_get(power->dev);
+
+	if (IS_ERR(qmp)) {
+		if (PTR_ERR(qmp) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		/* We assume any other error means it's not defined/needed */
+		qmp = NULL;
+	}
+	power->qmp = qmp;
+
+	return 0;
+}
+
+static void ipa_power_retention_exit(struct ipa_power *power)
+{
+	qmp_put(power->qmp);
+	power->qmp = NULL;
+}
+
+/* Control register retention on power collapse */
+void ipa_power_retention(struct ipa *ipa, bool enable)
+{
+	static const char fmt[] = "{ class: bcm, res: ipa_pc, val: %c }";
+	struct ipa_power *power = ipa->power;
+	char buf[36];	/* Exactly enough for fmt[]; size a multiple of 4 */
+	int ret;
+
+	if (!power->qmp)
+		return;		/* Not needed on this platform */
+
+	(void)snprintf(buf, sizeof(buf), fmt, enable ? '1' : '0');
+
+	ret = qmp_send(power->qmp, buf, sizeof(buf));
+	if (ret)
+		dev_err(power->dev, "error %d sending QMP %sable request\n",
+			ret, enable ? "en" : "dis");
+}
+
  int ipa_power_setup(struct ipa *ipa)
  {
  	int ret;
@@ -438,12 +483,18 @@ ipa_power_init(struct device *dev, const struct ipa_power_data *data)
  	if (ret)
  		goto err_kfree;
 
+	ret = ipa_power_retention_init(power);
+	if (ret)
+		goto err_interconnect_exit;
+
  	pm_runtime_set_autosuspend_delay(dev, IPA_AUTOSUSPEND_DELAY);
  	pm_runtime_use_autosuspend(dev);
  	pm_runtime_enable(dev);
 
  	return power;
 
+err_interconnect_exit:
+	ipa_interconnect_exit(power);
  err_kfree:
  	kfree(power);
  err_clk_put:
@@ -460,6 +511,7 @@ void ipa_power_exit(struct ipa_power *power)
 
  	pm_runtime_disable(dev);
  	pm_runtime_dont_use_autosuspend(dev);
+	ipa_power_retention_exit(power);
  	ipa_interconnect_exit(power);
  	kfree(power);
  	clk_put(clk);
drivers/net/ipa/ipa_power.h +7 -0
index 2151805d7fbb..6f84f057a209 100644
@@ -40,6 +40,13 @@ void ipa_power_modem_queue_wake(struct ipa *ipa);
   */
  void ipa_power_modem_queue_active(struct ipa *ipa);
 
+/**
+ * ipa_power_retention() - Control register retention on power collapse
+ * @ipa:	IPA pointer
+ * @enable:	Whether retention should be enabled or disabled
+ */
+void ipa_power_retention(struct ipa *ipa, bool enable);
+
  /**
   * ipa_power_setup() - Set up IPA power management
   * @ipa:	IPA pointer
drivers/net/ipa/ipa_uc.c +5 -0
index 856e55a080a7..fe11910518d9 100644
@@ -11,6 +11,7 @@
 
  #include "ipa.h"
  #include "ipa_uc.h"
+#include "ipa_power.h"
 
  /**
   * DOC:  The IPA embedded microcontroller
@@ -154,6 +155,7 @@ static void ipa_uc_response_hdlr(struct ipa *ipa, enum ipa_irq_id irq_id)
  	case IPA_UC_RESPONSE_INIT_COMPLETED:
  		if (ipa->uc_powered) {
  			ipa->uc_loaded = true;
+			ipa_power_retention(ipa, true);
  			pm_runtime_mark_last_busy(dev);
  			(void)pm_runtime_put_autosuspend(dev);
  			ipa->uc_powered = false;
@@ -184,6 +186,9 @@ void ipa_uc_deconfig(struct ipa *ipa)
 
  	ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_1);
  	ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_0);
+	if (ipa->uc_loaded)
+		ipa_power_retention(ipa, false);
+
  	if (!ipa->uc_powered)
  		return;
 
author maintainer
#2 lore raw
Quoted 27 lines · Alex Elder
Quoted 5 lines · On 2/4/22 2:50 AM, [email protected] wrote:

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

I just tried to apply this patch on v5.15.19 and it applied cleanly for me.

----------------

{2314} elder@presto-> git checkout -b test
Switched to a new branch 'test'
{2315} elder@presto-> git reset --hard v5.15.19
HEAD is now at 47cccb1eb2fec Linux 5.15.19
{2316} elder@presto-> git cherry-pick -x 34a081761e4e3c35381cbfad609ebae2962fe2f8
[test 71a06f5acbb05] net: ipa: request IPA register values be retained
Date: Tue Feb 1 09:02:05 2022 -0600

 3 files changed, 64 insertions(+)

{2317} elder@presto-> git log --oneline -2
71a06f5acbb05 (HEAD -> test) net: ipa: request IPA register values be retained
47cccb1eb2fec (tag: v5.15.19, stable/linux-5.15.y) Linux 5.15.19 {2318} elder@presto->

Quoted 4 lines

----------------

I don't understand. If you know that I'm doing something wrong, can you let me know what it might be?

It fails to build :)

Quoted 9 lines

While I have your attention on this, there is another commit that should be back-ported with this. It did not have the "Fixes" tag, unfortunately.

Bjorn has committed it in the "arm64-for-5.18" branch in the Qualcomm repository:
git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
73419e4d2fd1b arm64: dts: qcom: add IPA qcom,qmp property

Quoted 5 lines

He says he has no plans to rebase this branch before it gets pulled.

I can send a followup message when that commit lands in v5.18-rc1, but it might be simplest to take care of it now.

According to the stable rules, I have to wait until it hits Linus's tree before I can add it to a stable release.

So if this is a bugfix that everyone needs, it should go to Linus before 5.17-final is out.

thanks,

greg k-h

#3 lore raw
Quoted 28 lines · Greg KH
Quoted 27 lines · Alex Elder
Quoted 5 lines · On 2/4/22 2:50 AM, [email protected] wrote:

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

I just tried to apply this patch on v5.15.19 and it applied cleanly for me.

----------------

{2314} elder@presto-> git checkout -b test
Switched to a new branch 'test'
{2315} elder@presto-> git reset --hard v5.15.19
HEAD is now at 47cccb1eb2fec Linux 5.15.19
{2316} elder@presto-> git cherry-pick -x 34a081761e4e3c35381cbfad609ebae2962fe2f8
[test 71a06f5acbb05] net: ipa: request IPA register values be retained
Date: Tue Feb 1 09:02:05 2022 -0600

  3 files changed, 64 insertions(+)

{2317} elder@presto-> git log --oneline -2
71a06f5acbb05 (HEAD -> test) net: ipa: request IPA register values be retained
47cccb1eb2fec (tag: v5.15.19, stable/linux-5.15.y) Linux 5.15.19 {2318} elder@presto->

Quoted 6 lines
Quoted 4 lines

----------------

I don't understand. If you know that I'm doing something wrong, can you let me know what it might be?

It fails to build :)

Oh! Well that's different! Sorry about that, I'll spend a little more time on this I guess...

Quoted 9 lines
Quoted 9 lines

While I have your attention on this, there is another commit that should be back-ported with this. It did not have the "Fixes" tag, unfortunately.

Bjorn has committed it in the "arm64-for-5.18" branch in the Qualcomm repository:
git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
73419e4d2fd1b arm64: dts: qcom: add IPA qcom,qmp property

Quoted 8 lines
Quoted 5 lines

He says he has no plans to rebase this branch before it gets pulled.

I can send a followup message when that commit lands in v5.18-rc1, but it might be simplest to take care of it now.

According to the stable rules, I have to wait until it hits Linus's tree before I can add it to a stable release.

OK understood. I'll work out with Bjorn what to do.

Thanks a lot.

-Alex

Quoted 7 lines

So if this is a bugfix that everyone needs, it should go to Linus before 5.17-final is out.

thanks,

greg k-h

author maintainer
#4 lore raw
Quoted 31 lines · Alex Elder
Quoted 30 lines · Greg KH
Quoted 29 lines · Alex Elder
Quoted 5 lines · On 2/4/22 2:50 AM, [email protected] wrote:

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

I just tried to apply this patch on v5.15.19 and it applied cleanly for me.

----------------

{2314} elder@presto-> git checkout -b test
Switched to a new branch 'test'
{2315} elder@presto-> git reset --hard v5.15.19
HEAD is now at 47cccb1eb2fec Linux 5.15.19
{2316} elder@presto-> git cherry-pick -x 34a081761e4e3c35381cbfad609ebae2962fe2f8
[test 71a06f5acbb05] net: ipa: request IPA register values be retained
Date: Tue Feb 1 09:02:05 2022 -0600

  3 files changed, 64 insertions(+)

{2317} elder@presto-> git log --oneline -2
71a06f5acbb05 (HEAD -> test) net: ipa: request IPA register values be retained
47cccb1eb2fec (tag: v5.15.19, stable/linux-5.15.y) Linux 5.15.19 {2318} elder@presto->

----------------

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

I don't understand. If you know that I'm doing something wrong, can you let me know what it might be?

It fails to build :)

Oh! Well that's different! Sorry about that, I'll spend a little more time on this I guess...

It was a "include file not found" type of error if I remember correctly.

#5 lore raw
Quoted 35 lines · Greg KH
Quoted 34 lines · Alex Elder
Quoted 33 lines · Greg KH
Quoted 31 lines · Alex Elder
Quoted 5 lines · On 2/4/22 2:50 AM, [email protected] wrote:

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

I just tried to apply this patch on v5.15.19 and it applied cleanly for me.

----------------

{2314} elder@presto-> git checkout -b test
Switched to a new branch 'test'
{2315} elder@presto-> git reset --hard v5.15.19
HEAD is now at 47cccb1eb2fec Linux 5.15.19
{2316} elder@presto-> git cherry-pick -x 34a081761e4e3c35381cbfad609ebae2962fe2f8
[test 71a06f5acbb05] net: ipa: request IPA register values be retained
Date: Tue Feb 1 09:02:05 2022 -0600

   3 files changed, 64 insertions(+)

{2317} elder@presto-> git log --oneline -2
71a06f5acbb05 (HEAD -> test) net: ipa: request IPA register values be retained
47cccb1eb2fec (tag: v5.15.19, stable/linux-5.15.y) Linux 5.15.19 {2318} elder@presto->

----------------

I don't understand. If you know that I'm doing something wrong, can you let me know what it might be?

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

It fails to build :)

Oh! Well that's different! Sorry about that, I'll spend a little more time on this I guess...

It was a "include file not found" type of error if I remember correctly.

OK, yes, I did discover the build problem. And it can be easily fixed with one more prerequisite commit.

HOWEVER, there is a "matching" DTS patch that won't land in Linus' master branch until v5.18-rc1, so I'll have to get back to you on that later anyway.

Which means you can simply drop my attempted 5.15 back-port from your queue (if you haven't already...).

Thanks, and sorry for the noise.

-Alex