Skip to contentSkip to conversation

remoteproc: Add support for xo clock

PATCH Superseded Sarangdhar Joshi 2 participants 3 messages Oct 20, 2016 – Oct 21, 2016

Subsystems (2)
Arm Qualcomm mailing list Remote processor (remoteproc)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sarangdhar Joshi

author
#0 lore raw

PATCH 1/1 remoteproc: Add support for xo clock

Add xo clock support required for Qualcomm ADSP
Peripheral Image Loader.

Signed-off-by Sarangdhar Joshi <[email protected]>

---

This patch depends on https://lkml.org/lkml/2016/8/23/50

 drivers/remoteproc/qcom_adsp_pil.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
drivers/remoteproc/qcom_adsp_pil.c +25 -1
index 9141633..a1f2f32 100644
@@ -15,6 +15,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/clk.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>

@@ -48,6 +49,8 @@ struct qcom_adsp {
 	struct qcom_smem_state *state;
 	unsigned stop_bit;
 
+	struct clk *xo;
+
 	struct regulator *cx_supply;
 
 	struct completion start_done;

@@ -102,10 +105,14 @@ static int adsp_start(struct rproc *rproc)
 	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
 	int ret;
 
-	ret = regulator_enable(adsp->cx_supply);
+	ret = clk_prepare_enable(adsp->xo);
 	if (ret)
 		return ret;
 
+	ret = regulator_enable(adsp->cx_supply);
+	if (ret)
+		goto disable_clocks;
+
 	ret = qcom_scm_pas_auth_and_reset(ADSP_PAS_ID);
 	if (ret) {
 		dev_err(adsp->dev,

@@ -126,6 +133,8 @@ static int adsp_start(struct rproc *rproc)
 
 disable_regulators:
 	regulator_disable(adsp->cx_supply);
+disable_clocks:
+	clk_disable_unprepare(adsp->xo);
 
 	return ret;
 }

@@ -223,6 +232,17 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int adsp_init_clock(struct qcom_adsp *adsp)
+{
+	adsp->xo = devm_clk_get(adsp->dev, "xo");
+	if (IS_ERR(adsp->xo)) {
+		dev_err(adsp->dev, "failed to get xo clock");
+		return PTR_ERR(adsp->xo);
+	}
+
+	return 0;
+}
+
 static int adsp_init_regulator(struct qcom_adsp *adsp)
 {
 	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");

@@ -320,6 +340,10 @@ static int adsp_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
+	ret = adsp_init_clock(adsp);
+	if (ret)
+		goto free_rproc;
+
 	ret = adsp_init_regulator(adsp);
 	if (ret)
 		goto free_rproc;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw
Quoted 2 lines · Sarangdhar Joshi

Add xo clock support required for Qualcomm ADSP
Peripheral Image Loader.

Yes but why is xo needed?

drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 239

Quoted 9 lines
drivers/remoteproc/qcom_adsp_pil.c +5 -0
@@ -223,6 +232,17 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int adsp_init_clock(struct qcom_adsp *adsp)
+{
+	adsp->xo = devm_clk_get(adsp->dev, "xo");
+	if (IS_ERR(adsp->xo)) {
+		dev_err(adsp->dev, "failed to get xo clock");

What if it's a probe defer error? Probably best to just be silent/debug level, or we need a specific test for EPROBE_DEFER and then silence in that case.

Quoted 2 lines
(quoted diff) +2 -0
+		return PTR_ERR(adsp->xo);
+	}
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#2 lore raw
Quoted 5 lines · Stephen Boyd
Quoted 2 lines · Sarangdhar Joshi

Add xo clock support required for Qualcomm ADSP
Peripheral Image Loader.

Yes but why is xo needed?

It is required to boot up the ADSP processor. The remoteproc driver keeps the xo clock enabled until the driver receives an "handover" interrupt in order to allow remote processor to vote for xo clock with rpm.

I will update the commit text.

Quoted 5 lines
Quoted 5 lines
drivers/remoteproc/qcom_adsp_pil.c +0 -0
@@ -223,6 +232,17 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
Quoted 9 lines

drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 239

Quoted 5 lines
drivers/remoteproc/qcom_adsp_pil.c +5 -0
+static int adsp_init_clock(struct qcom_adsp *adsp)
+{
+	adsp->xo = devm_clk_get(adsp->dev, "xo");
+	if (IS_ERR(adsp->xo)) {
+		dev_err(adsp->dev, "failed to get xo clock");

What if it's a probe defer error? Probably best to just be silent/debug level, or we need a specific test for EPROBE_DEFER and then silence in that case.

Sure, I will add a check for EPROBE_DEFER. Prefer driver to return meaningful error message in case of failure.

Quoted 4 lines

Review static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 241

Quoted 3 lines
(quoted diff) +2 -0
+		return PTR_ERR(adsp->xo);
+	}

Thanks for reviewing the patch.

Regards,
Sarang

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project