* [PATCH net 0/2] net: ipa: enable register retention
@ 2022-02-01 14:04 Alex Elder
2022-02-01 14:04 ` [PATCH net 1/2] dt-bindings: net: qcom,ipa: add optional qcom,qmp property Alex Elder
2022-02-01 14:04 ` [PATCH net 2/2] net: ipa: request IPA register values be retained Alex Elder
0 siblings, 2 replies; 6+ messages in thread
From: Alex Elder @ 2022-02-01 14:04 UTC (permalink / raw)
To: robh+dt, davem, kuba
Cc: bjorn.andersson, mka, evgreen, cpratapa, avuyyuru, jponduru,
subashab, elder, netdev, linux-arm-msm, linux-kernel
With runtime power management in place, we sometimes need to issue
a command to enable retention of IPA register values before power
collapse. This requires a new Device Tree property, whose presence
will also be used to signal that the command is required.
-Alex
Alex Elder (2):
dt-bindings: net: qcom,ipa: add optional qcom,qmp property
net: ipa: request IPA register values be retained
.../devicetree/bindings/net/qcom,ipa.yaml | 6 +++
drivers/net/ipa/ipa_power.c | 52 +++++++++++++++++++
drivers/net/ipa/ipa_power.h | 7 +++
drivers/net/ipa/ipa_uc.c | 5 ++
4 files changed, 70 insertions(+)
--
2.32.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] dt-bindings: net: qcom,ipa: add optional qcom,qmp property
2022-02-01 14:04 [PATCH net 0/2] net: ipa: enable register retention Alex Elder
@ 2022-02-01 14:04 ` Alex Elder
2022-02-01 14:04 ` [PATCH net 2/2] net: ipa: request IPA register values be retained Alex Elder
1 sibling, 0 replies; 6+ messages in thread
From: Alex Elder @ 2022-02-01 14:04 UTC (permalink / raw)
To: robh+dt, davem, kuba
Cc: bjorn.andersson, mka, evgreen, cpratapa, avuyyuru, jponduru,
subashab, elder, netdev, linux-arm-msm, linux-kernel
For some systems, the IPA driver must make a request to ensure that
its registers are retained across power collapse of the IPA hardware.
On such systems, we'll use the existence of the "qcom,qmp" property
as a signal that this request is required.
Signed-off-by: Alex Elder <elder@linaro.org>
---
Documentation/devicetree/bindings/net/qcom,ipa.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
index b86edf67ce626..58ecc62adfaae 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
@@ -107,6 +107,10 @@ properties:
- const: imem
- const: config
+ qcom,qmp:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: phandle to the AOSS side-channel message RAM
+
qcom,smem-states:
$ref: /schemas/types.yaml#/definitions/phandle-array
description: State bits used in by the AP to signal the modem.
@@ -222,6 +226,8 @@ examples:
"imem",
"config";
+ qcom,qmp = <&aoss_qmp>;
+
qcom,smem-states = <&ipa_smp2p_out 0>,
<&ipa_smp2p_out 1>;
qcom,smem-state-names = "ipa-clock-enabled-valid",
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net: ipa: request IPA register values be retained
2022-02-01 14:04 [PATCH net 0/2] net: ipa: enable register retention Alex Elder
2022-02-01 14:04 ` [PATCH net 1/2] dt-bindings: net: qcom,ipa: add optional qcom,qmp property Alex Elder
@ 2022-02-01 14:04 ` Alex Elder
2022-02-03 5:02 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Alex Elder @ 2022-02-01 14:04 UTC (permalink / raw)
To: davem, kuba
Cc: robh+dt, bjorn.andersson, agross, mka, evgreen, cpratapa,
avuyyuru, jponduru, subashab, elder, netdev, linux-arm-msm,
linux-kernel
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: 2775cbc5afeb6 ("net: ipa: rename "ipa_clock.c"")
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ipa/ipa_power.c | 52 +++++++++++++++++++++++++++++++++++++
drivers/net/ipa/ipa_power.h | 7 +++++
drivers/net/ipa/ipa_uc.c | 5 ++++
3 files changed, 64 insertions(+)
diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c
index b1c6c0fcb654f..f2989aac47a62 100644
--- a/drivers/net/ipa/ipa_power.c
+++ b/drivers/net/ipa/ipa_power.c
@@ -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);
diff --git a/drivers/net/ipa/ipa_power.h b/drivers/net/ipa/ipa_power.h
index 2151805d7fbb0..6f84f057a2095 100644
--- a/drivers/net/ipa/ipa_power.h
+++ b/drivers/net/ipa/ipa_power.h
@@ -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
diff --git a/drivers/net/ipa/ipa_uc.c b/drivers/net/ipa/ipa_uc.c
index 856e55a080a7f..fe11910518d95 100644
--- a/drivers/net/ipa/ipa_uc.c
+++ b/drivers/net/ipa/ipa_uc.c
@@ -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;
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: ipa: request IPA register values be retained
2022-02-01 14:04 ` [PATCH net 2/2] net: ipa: request IPA register values be retained Alex Elder
@ 2022-02-03 5:02 ` Jakub Kicinski
2022-02-03 11:22 ` Alex Elder
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2022-02-03 5:02 UTC (permalink / raw)
To: Alex Elder
Cc: davem, robh+dt, bjorn.andersson, agross, mka, evgreen, cpratapa,
avuyyuru, jponduru, subashab, elder, netdev, linux-arm-msm,
linux-kernel
On Tue, 1 Feb 2022 08:04:12 -0600 Alex Elder wrote:
> Fixes: 2775cbc5afeb6 ("net: ipa: rename "ipa_clock.c"")
The Fixes tag should point at the place the code was introduced,
even if it moved or otherwise the patch won't apply as far back.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: ipa: request IPA register values be retained
2022-02-03 5:02 ` Jakub Kicinski
@ 2022-02-03 11:22 ` Alex Elder
2022-02-03 16:03 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Alex Elder @ 2022-02-03 11:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, robh+dt, bjorn.andersson, agross, mka, evgreen, cpratapa,
avuyyuru, jponduru, subashab, elder, netdev, linux-arm-msm,
linux-kernel
On 2/2/22 11:02 PM, Jakub Kicinski wrote:
> On Tue, 1 Feb 2022 08:04:12 -0600 Alex Elder wrote:
>> Fixes: 2775cbc5afeb6 ("net: ipa: rename "ipa_clock.c"")
>
> The Fixes tag should point at the place the code was introduced,
> even if it moved or otherwise the patch won't apply as far back.
The problem was not "activated" until this commit:
1aac309d32075 net: ipa: use autosuspend
And that commit was merged together in a series that
included the one I mentioned above:
2775cbc5afeb6 net: ipa: rename "ipa_clock.c"
The rename commit is two commits after "use autosuspend".
The merge commit was:
863434886497d Merge branch 'ipa-autosuspend'
Until autosuspend is enabled, this new code is
completely unnecessary, so back-porting it beyond
that is pointless. I supplied the commit in the
"Fixes" tag because I thought it would be close
to equivalent and would avoid some trouble back-porting.
Perhaps the "use autosuspend" commit is the one that
should be in the "Fixes" tag, but I don't believe it
should be back-ported any further than that.
Re-spinning the series to fix the tag is trivial, but
before I do that, can you tell me which commit you
recommend I use in the "Fixes" tag?
The original commit that introduced the microcontroller
code (and also included the clock/power code) is:
a646d6ec90983 soc: qcom: ipa: modem and microcontroller
Thanks.
-Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: ipa: request IPA register values be retained
2022-02-03 11:22 ` Alex Elder
@ 2022-02-03 16:03 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-02-03 16:03 UTC (permalink / raw)
To: Alex Elder
Cc: davem, robh+dt, bjorn.andersson, agross, mka, evgreen, cpratapa,
avuyyuru, jponduru, subashab, elder, netdev, linux-arm-msm,
linux-kernel
On Thu, 3 Feb 2022 05:22:23 -0600 Alex Elder wrote:
> On 2/2/22 11:02 PM, Jakub Kicinski wrote:
> > On Tue, 1 Feb 2022 08:04:12 -0600 Alex Elder wrote:
> >> Fixes: 2775cbc5afeb6 ("net: ipa: rename "ipa_clock.c"")
> >
> > The Fixes tag should point at the place the code was introduced,
> > even if it moved or otherwise the patch won't apply as far back.
>
> The problem was not "activated" until this commit:
> 1aac309d32075 net: ipa: use autosuspend
>
>
> And that commit was merged together in a series that
> included the one I mentioned above:
> 2775cbc5afeb6 net: ipa: rename "ipa_clock.c"
>
> The rename commit is two commits after "use autosuspend".
>
> The merge commit was:
> 863434886497d Merge branch 'ipa-autosuspend'
>
>
> Until autosuspend is enabled, this new code is
> completely unnecessary, so back-porting it beyond
> that is pointless. I supplied the commit in the
> "Fixes" tag because I thought it would be close
> to equivalent and would avoid some trouble back-porting.
>
> Perhaps the "use autosuspend" commit is the one that
> should be in the "Fixes" tag, but I don't believe it
> should be back-ported any further than that.
>
> Re-spinning the series to fix the tag is trivial, but
> before I do that, can you tell me which commit you
> recommend I use in the "Fixes" tag?
>
> The original commit that introduced the microcontroller
> code (and also included the clock/power code) is:
> a646d6ec90983 soc: qcom: ipa: modem and microcontroller
Thanks for the explanation 1aac309d32075 sounds like the right choice.
Let me just swap it for you and apply the v2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-03 16:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 14:04 [PATCH net 0/2] net: ipa: enable register retention Alex Elder
2022-02-01 14:04 ` [PATCH net 1/2] dt-bindings: net: qcom,ipa: add optional qcom,qmp property Alex Elder
2022-02-01 14:04 ` [PATCH net 2/2] net: ipa: request IPA register values be retained Alex Elder
2022-02-03 5:02 ` Jakub Kicinski
2022-02-03 11:22 ` Alex Elder
2022-02-03 16:03 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).