Re: [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers

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

 



Hi Bjorn,

On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Migrate the Hexagon V5 PAS (ADSP) driver to using the newly extracted
> helper functions. The use of the handover callback does introduce latent
> disabling of proxy resources. But apart from this there should be no
> change in functionality.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> ---
>  drivers/remoteproc/Kconfig         |   1 +
>  drivers/remoteproc/qcom_adsp_pil.c | 156 +++++------------------------
>  2 files changed, 28 insertions(+), 129 deletions(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 63b79ea91a21..d51d155cf8bd 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -93,6 +93,7 @@ config QCOM_ADSP_PIL
>  	depends on QCOM_SYSMON || QCOM_SYSMON=n
>  	select MFD_SYSCON
>  	select QCOM_MDT_LOADER
> +	select QCOM_Q6V5_COMMON
>  	select QCOM_RPROC_COMMON
>  	select QCOM_SCM
>  	help
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
> index 89a86ce07f99..d4339a6da616 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -31,6 +31,7 @@
>  #include <linux/soc/qcom/smem_state.h>
>  
>  #include "qcom_common.h"
> +#include "qcom_q6v5.h"
>  #include "remoteproc_internal.h"
>  
>  struct adsp_data {
> @@ -48,14 +49,7 @@ struct qcom_adsp {
>  	struct device *dev;
>  	struct rproc *rproc;
>  
> -	int wdog_irq;
> -	int fatal_irq;
> -	int ready_irq;
> -	int handover_irq;
> -	int stop_ack_irq;
> -
> -	struct qcom_smem_state *state;
> -	unsigned stop_bit;
> +	struct qcom_q6v5 q6v5;
>  
>  	struct clk *xo;
>  	struct clk *aggre2_clk;
> @@ -96,6 +90,8 @@ static int adsp_start(struct rproc *rproc)
>  	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>  	int ret;
>  
> +	qcom_q6v5_prepare(&adsp->q6v5);
> +
>  	ret = clk_prepare_enable(adsp->xo);
>  	if (ret)
>  		return ret;
> @@ -119,16 +115,14 @@ static int adsp_start(struct rproc *rproc)
>  		goto disable_px_supply;
>  	}
>  
> -	ret = wait_for_completion_timeout(&adsp->start_done,
> -					  msecs_to_jiffies(5000));
> -	if (!ret) {
> +	ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
> +	if (ret == -ETIMEDOUT) {
>  		dev_err(adsp->dev, "start timed out\n");
>  		qcom_scm_pas_shutdown(adsp->pas_id);
> -		ret = -ETIMEDOUT;
>  		goto disable_px_supply;
>  	}
>  
> -	ret = 0;
> +	return 0;
>  
>  disable_px_supply:
>  	regulator_disable(adsp->px_supply);
> @@ -142,28 +136,34 @@ static int adsp_start(struct rproc *rproc)
>  	return ret;
>  }
>  
> +static void qcom_pas_handover(struct qcom_q6v5 *q6v5)
> +{
> +	struct qcom_adsp *adsp = container_of(q6v5, struct qcom_adsp, q6v5);
> +
> +	regulator_disable(adsp->px_supply);
> +	regulator_disable(adsp->cx_supply);
> +	clk_disable_unprepare(adsp->aggre2_clk);
> +	clk_disable_unprepare(adsp->xo);
> +}
> +
>  static int adsp_stop(struct rproc *rproc)
>  {
>  	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> +	int handover;
>  	int ret;
>  
> -	qcom_smem_state_update_bits(adsp->state,
> -				    BIT(adsp->stop_bit),
> -				    BIT(adsp->stop_bit));
> -
> -	ret = wait_for_completion_timeout(&adsp->stop_done,
> -					  msecs_to_jiffies(5000));
> -	if (ret == 0)
> +	ret = qcom_q6v5_request_stop(&adsp->q6v5);
> +	if (ret == -ETIMEDOUT)
>  		dev_err(adsp->dev, "timed out on wait\n");
>  
> -	qcom_smem_state_update_bits(adsp->state,
> -				    BIT(adsp->stop_bit),
> -				    0);
> -
>  	ret = qcom_scm_pas_shutdown(adsp->pas_id);
>  	if (ret)
>  		dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>  
> +	handover = qcom_q6v5_unprepare(&adsp->q6v5);
> +	if (handover)
> +		qcom_pas_handover(&adsp->q6v5);
> +
>  	return ret;
>  }
>  
> @@ -187,53 +187,6 @@ static const struct rproc_ops adsp_ops = {
>  	.load = adsp_load,
>  };
>  
> -static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
> -{
> -	struct qcom_adsp *adsp = dev;
> -
> -	rproc_report_crash(adsp->rproc, RPROC_WATCHDOG);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_fatal_interrupt(int irq, void *dev)
> -{
> -	struct qcom_adsp *adsp = dev;
> -	size_t len;
> -	char *msg;
> -
> -	msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, adsp->crash_reason_smem, &len);
> -	if (!IS_ERR(msg) && len > 0 && msg[0])
> -		dev_err(adsp->dev, "fatal error received: %s\n", msg);
> -
> -	rproc_report_crash(adsp->rproc, RPROC_FATAL_ERROR);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_ready_interrupt(int irq, void *dev)
> -{
> -	return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_handover_interrupt(int irq, void *dev)
> -{
> -	struct qcom_adsp *adsp = dev;
> -
> -	complete(&adsp->start_done);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
> -{
> -	struct qcom_adsp *adsp = dev;
> -
> -	complete(&adsp->stop_done);
> -
> -	return IRQ_HANDLED;
> -}
> -
>  static int adsp_init_clock(struct qcom_adsp *adsp)
>  {
>  	int ret;
> @@ -272,29 +225,6 @@ static int adsp_init_regulator(struct qcom_adsp *adsp)
>  	return PTR_ERR_OR_ZERO(adsp->px_supply);
>  }
>  
> -static int adsp_request_irq(struct qcom_adsp *adsp,
> -			     struct platform_device *pdev,
> -			     const char *name,
> -			     irq_handler_t thread_fn)
> -{
> -	int ret;
> -
> -	ret = platform_get_irq_byname(pdev, name);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "no %s IRQ defined\n", name);
> -		return ret;
> -	}
> -
> -	ret = devm_request_threaded_irq(&pdev->dev, ret,
> -					NULL, thread_fn,
> -					IRQF_ONESHOT,
> -					"adsp", adsp);
> -	if (ret)
> -		dev_err(&pdev->dev, "request %s IRQ failed\n", name);
> -
> -	return ret;
> -}
> -
>  static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
>  {
>  	struct device_node *node;
> @@ -348,13 +278,9 @@ static int adsp_probe(struct platform_device *pdev)
>  	adsp->dev = &pdev->dev;
>  	adsp->rproc = rproc;
>  	adsp->pas_id = desc->pas_id;
> -	adsp->crash_reason_smem = desc->crash_reason_smem;
>  	adsp->has_aggre2_clk = desc->has_aggre2_clk;
>  	platform_set_drvdata(pdev, adsp);
>  
> -	init_completion(&adsp->start_done);
> -	init_completion(&adsp->stop_done);
> -
>  	ret = adsp_alloc_memory_region(adsp);
>  	if (ret)
>  		goto free_rproc;
> @@ -367,37 +293,10 @@ static int adsp_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto free_rproc;
>  
> -	ret = adsp_request_irq(adsp, pdev, "wdog", adsp_wdog_interrupt);
> -	if (ret < 0)
> -		goto free_rproc;
> -	adsp->wdog_irq = ret;
> -
> -	ret = adsp_request_irq(adsp, pdev, "fatal", adsp_fatal_interrupt);
> -	if (ret < 0)
> -		goto free_rproc;
> -	adsp->fatal_irq = ret;
> -
> -	ret = adsp_request_irq(adsp, pdev, "ready", adsp_ready_interrupt);
> -	if (ret < 0)
> -		goto free_rproc;
> -	adsp->ready_irq = ret;
> -
> -	ret = adsp_request_irq(adsp, pdev, "handover", adsp_handover_interrupt);
> -	if (ret < 0)
> -		goto free_rproc;
> -	adsp->handover_irq = ret;
> -
> -	ret = adsp_request_irq(adsp, pdev, "stop-ack", adsp_stop_ack_interrupt);
> -	if (ret < 0)
> -		goto free_rproc;
> -	adsp->stop_ack_irq = ret;
> -
> -	adsp->state = qcom_smem_state_get(&pdev->dev, "stop",
> -					  &adsp->stop_bit);
> -	if (IS_ERR(adsp->state)) {
> -		ret = PTR_ERR(adsp->state);
> +	ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem,
> +			     qcom_pas_handover);
> +	if (ret)
>  		goto free_rproc;
> -	}
>  
>  	qcom_add_glink_subdev(rproc, &adsp->glink_subdev);
>  	qcom_add_smd_subdev(rproc, &adsp->smd_subdev);
> @@ -422,7 +321,6 @@ static int adsp_remove(struct platform_device *pdev)
>  {
>  	struct qcom_adsp *adsp = platform_get_drvdata(pdev);
>  
> -	qcom_smem_state_put(adsp->state);

 Is this change required ?

  Otherwise,
       reviewed-by: Sricharan R <sricharan@xxxxxxxxxxxxxx>

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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