Re: [PATCH] remoteproc: qcom: q6v5: Add interconnect path proxy vote

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8.03.22 0:42, Bjorn Andersson wrote:
On Mon 07 Mar 10:35 PST 2022, Georgi Djakov wrote:


On 25.02.22 5:32, Bjorn Andersson wrote:
Many remoteproc instances requires that Linux casts a proxy vote for an
interconnect path during boot, until they can do it themselves. Add
support for voting for a single path.

As this is a shared problem between both PAS and MSS drivers, the path
is acquired and votes casted from the common helper code.

Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
---

Sibi posted recently a patch to add interconnect votes from the modem driver,
today I needed the same feature for one of the PAS remoteprocs. After
essentially duplicating Sibi's patch I realized that it doesn't look too bad to
put this in the common Q6V5 code.

The main difference is that this would be messy if we need to support multiple
paths, so we probably would have to push it out to the individual drivers at
that point.

Sibi's patch can be found here.
https://lore.kernel.org/all/1644813252-12897-3-git-send-email-quic_sibis@xxxxxxxxxxx/


This makes the implementation pick up one path, relevant DT bindings would
still need to be updated in order be allowed to this in the DeviceTree files.

   drivers/remoteproc/qcom_q6v5.c | 21 ++++++++++++++++++++-
   drivers/remoteproc/qcom_q6v5.h |  3 +++
   2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 442a388f8102..5280ec9b5449 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -8,6 +8,7 @@
    */
   #include <linux/kernel.h>
   #include <linux/platform_device.h>
+#include <linux/interconnect.h>
   #include <linux/interrupt.h>
   #include <linux/module.h>
   #include <linux/soc/qcom/qcom_aoss.h>
@@ -51,9 +52,17 @@ int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
   {
   	int ret;
+	ret = icc_set_bw(q6v5->path, 0, UINT_MAX);
+	if (ret < 0) {
+		dev_err(q6v5->dev, "failed to set bandwidth request\n");
+		return ret;
+	}
+
   	ret = q6v5_load_state_toggle(q6v5, true);
-	if (ret)
+	if (ret) {
+		icc_set_bw(q6v5->path, 0, 0);
   		return ret;
+	}
   	reinit_completion(&q6v5->start_done);
   	reinit_completion(&q6v5->stop_done);
@@ -78,6 +87,9 @@ int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
   	disable_irq(q6v5->handover_irq);
   	q6v5_load_state_toggle(q6v5, false);
+	/* Disable interconnect vote, in case handover never happened */
+	icc_set_bw(q6v5->path, 0, 0);
+
   	return !q6v5->handover_issued;
   }
   EXPORT_SYMBOL_GPL(qcom_q6v5_unprepare);
@@ -160,6 +172,8 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
   	if (q6v5->handover)
   		q6v5->handover(q6v5);
+	icc_set_bw(q6v5->path, 0, 0);
+
   	q6v5->handover_issued = true;
   	return IRQ_HANDLED;
@@ -332,6 +346,11 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
   		return load_state ? -ENOMEM : -EINVAL;
   	}
+	q6v5->path = devm_of_icc_get(&pdev->dev, NULL);
+	if (IS_ERR(q6v5->path))
+		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
+				     "failed to acquire interconnect path\n");
+
   	return 0;
   }
   EXPORT_SYMBOL_GPL(qcom_q6v5_init);

Probably we should also call icc_put(q6v5->path) in qcom_q6v5_deinit().


The use of devm_of_icc_get() should take care of that for us. Or am I
missing something?

Right, it's fine then. Sorry for the noise.

BR,
Georgi

Reviewed-by: Georgi Djakov <djakov@xxxxxxxxxx>


Thanks,
Bjorn




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux