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? > Reviewed-by: Georgi Djakov <djakov@xxxxxxxxxx> > Thanks, Bjorn
