diff mbox

[v2,1/1] remoteproc: Add support for xo clock

Message ID 1477429046-26855-1-git-send-email-spjoshi@codeaurora.org (mailing list archive)
State Accepted
Headers show

Commit Message

Sarangdhar Joshi Oct. 25, 2016, 8:57 p.m. UTC
Add xo clock support required to boot up Qualcomm ADSP processor.
The ADSP remoteproc driver keeps xo clock enabled until the
driver receives "handover" irq, in order to allow ADSP processor
to vote for xo clock with rpm.

Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
---
 drivers/remoteproc/qcom_adsp_pil.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

Comments

Sarangdhar Joshi Oct. 25, 2016, 9:03 p.m. UTC | #1
On 10/25/2016 01:57 PM, Sarangdhar Joshi wrote:
> Add xo clock support required to boot up Qualcomm ADSP processor.
> The ADSP remoteproc driver keeps xo clock enabled until the
> driver receives "handover" irq, in order to allow ADSP processor
> to vote for xo clock with rpm.
>
> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
> ---

Changes since v1:
  - Avoid logging error for probe deferral case (Stephen)
  - Update commit text (Stephen)

>  drivers/remoteproc/qcom_adsp_pil.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
> index 9141633..5bb25d1 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -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,21 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>
> +static int adsp_init_clock(struct qcom_adsp *adsp)
> +{
> +	int ret;
> +
> +	adsp->xo = devm_clk_get(adsp->dev, "xo");
> +	if (IS_ERR(adsp->xo)) {
> +		ret = PTR_ERR(adsp->xo);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(adsp->dev, "failed to get xo clock");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int adsp_init_regulator(struct qcom_adsp *adsp)
>  {
>  	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");
> @@ -320,6 +344,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;
>
Bjorn Andersson Nov. 1, 2016, 12:05 a.m. UTC | #2
On Tue 25 Oct 13:57 PDT 2016, Sarangdhar Joshi wrote:

> Add xo clock support required to boot up Qualcomm ADSP processor.
> The ADSP remoteproc driver keeps xo clock enabled until the
> driver receives "handover" irq, in order to allow ADSP processor
> to vote for xo clock with rpm.

Looks good, thanks.

We have to add the clock to the DT binding and run that by Rob again,
before merging the driver and this patch.

Regards,
Bjorn

> 
> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
> ---
>  drivers/remoteproc/qcom_adsp_pil.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
> index 9141633..5bb25d1 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -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,21 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> +static int adsp_init_clock(struct qcom_adsp *adsp)
> +{
> +	int ret;
> +
> +	adsp->xo = devm_clk_get(adsp->dev, "xo");
> +	if (IS_ERR(adsp->xo)) {
> +		ret = PTR_ERR(adsp->xo);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(adsp->dev, "failed to get xo clock");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int adsp_init_regulator(struct qcom_adsp *adsp)
>  {
>  	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");
> @@ -320,6 +344,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;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Nov. 1, 2016, 5:41 p.m. UTC | #3
On 10/31/2016 05:05 PM, Bjorn Andersson wrote:
> On Tue 25 Oct 13:57 PDT 2016, Sarangdhar Joshi wrote:
>
>> Add xo clock support required to boot up Qualcomm ADSP processor.
>> The ADSP remoteproc driver keeps xo clock enabled until the
>> driver receives "handover" irq, in order to allow ADSP processor
>> to vote for xo clock with rpm.
> Looks good, thanks.
>
> We have to add the clock to the DT binding and run that by Rob again,
> before merging the driver and this patch.
>

Maybe we should make the xo clock required. We always have a clock for
it somewhere, either RPM controlled or as a fixed rate clock so it
should work just as well.
Bjorn Andersson Nov. 1, 2016, 5:46 p.m. UTC | #4
On Tue, Nov 1, 2016 at 10:41 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/31/2016 05:05 PM, Bjorn Andersson wrote:
>> On Tue 25 Oct 13:57 PDT 2016, Sarangdhar Joshi wrote:
>>
>>> Add xo clock support required to boot up Qualcomm ADSP processor.
>>> The ADSP remoteproc driver keeps xo clock enabled until the
>>> driver receives "handover" irq, in order to allow ADSP processor
>>> to vote for xo clock with rpm.
>> Looks good, thanks.
>>
>> We have to add the clock to the DT binding and run that by Rob again,
>> before merging the driver and this patch.
>>
>
> Maybe we should make the xo clock required. We always have a clock for
> it somewhere, either RPM controlled or as a fixed rate clock so it
> should work just as well.
>

As far as I can see it is required, after Sarangdhar's patch - so this
is what I meant we need to add to the DT binding before merging that.
And as you say, we can always make it reference "xo_board" for now.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Nov. 1, 2016, 5:52 p.m. UTC | #5
On 11/01/2016 10:46 AM, Bjorn Andersson wrote:
> On Tue, Nov 1, 2016 at 10:41 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/31/2016 05:05 PM, Bjorn Andersson wrote:
>>> On Tue 25 Oct 13:57 PDT 2016, Sarangdhar Joshi wrote:
>>>
>>>> Add xo clock support required to boot up Qualcomm ADSP processor.
>>>> The ADSP remoteproc driver keeps xo clock enabled until the
>>>> driver receives "handover" irq, in order to allow ADSP processor
>>>> to vote for xo clock with rpm.
>>> Looks good, thanks.
>>>
>>> We have to add the clock to the DT binding and run that by Rob again,
>>> before merging the driver and this patch.
>>>
>> Maybe we should make the xo clock required. We always have a clock for
>> it somewhere, either RPM controlled or as a fixed rate clock so it
>> should work just as well.
>>
> As far as I can see it is required, after Sarangdhar's patch - so this
> is what I meant we need to add to the DT binding before merging that.
> And as you say, we can always make it reference "xo_board" for now.
>

Ah good then. Brain must have misread the patch.
Sarangdhar Joshi Nov. 2, 2016, 6:58 p.m. UTC | #6
On 10/31/2016 05:05 PM, Bjorn Andersson wrote:
> On Tue 25 Oct 13:57 PDT 2016, Sarangdhar Joshi wrote:
>
>> Add xo clock support required to boot up Qualcomm ADSP processor.
>> The ADSP remoteproc driver keeps xo clock enabled until the
>> driver receives "handover" irq, in order to allow ADSP processor
>> to vote for xo clock with rpm.
>
> Looks good, thanks.
>
> We have to add the clock to the DT binding and run that by Rob again,
> before merging the driver and this patch.
>
> Regards,
> Bjorn

Sure, will add it to the DT bindings. Thanks for reviewing.

Regards,
Sarang

>
>>
>> Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
>> ---
>>  drivers/remoteproc/qcom_adsp_pil.c | 30 +++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
>> index 9141633..5bb25d1 100644
>> --- a/drivers/remoteproc/qcom_adsp_pil.c
>> +++ b/drivers/remoteproc/qcom_adsp_pil.c
>> @@ -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,21 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
>>  	return IRQ_HANDLED;
>>  }
>>
>> +static int adsp_init_clock(struct qcom_adsp *adsp)
>> +{
>> +	int ret;
>> +
>> +	adsp->xo = devm_clk_get(adsp->dev, "xo");
>> +	if (IS_ERR(adsp->xo)) {
>> +		ret = PTR_ERR(adsp->xo);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(adsp->dev, "failed to get xo clock");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int adsp_init_regulator(struct qcom_adsp *adsp)
>>  {
>>  	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");
>> @@ -320,6 +344,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;
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
diff mbox

Patch

diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 9141633..5bb25d1 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -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,21 @@  static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int adsp_init_clock(struct qcom_adsp *adsp)
+{
+	int ret;
+
+	adsp->xo = devm_clk_get(adsp->dev, "xo");
+	if (IS_ERR(adsp->xo)) {
+		ret = PTR_ERR(adsp->xo);
+		if (ret != -EPROBE_DEFER)
+			dev_err(adsp->dev, "failed to get xo clock");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int adsp_init_regulator(struct qcom_adsp *adsp)
 {
 	adsp->cx_supply = devm_regulator_get(adsp->dev, "cx");
@@ -320,6 +344,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;