Re: [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: Use common q6v5 helpers

2018-05-31 Thread Sricharan R
Hi Bjorn,

On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Migrate the MSS remoteproc driver to use the newly extracted helper
> functions.
> 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/remoteproc/Kconfig |   4 +
>  drivers/remoteproc/qcom_q6v5_pil.c | 157 +++--
>  2 files changed, 19 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index d51d155cf8bd..2316908e9788 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -116,6 +116,10 @@ config QCOM_Q6V5_PIL
>   depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   depends on QCOM_SYSMON || QCOM_SYSMON=n
>   select MFD_SYSCON
> + select QCOM_Q6V5_COMMON

 The below three are duplicate.

> + select QCOM_RPROC_COMMON
> + select QCOM_SCM
> + select QCOM_Q6V5_COMMON
>   select QCOM_RPROC_COMMON
>   select QCOM_SCM

<..>

> @@ -946,16 +934,10 @@ static int q6v5_stop(struct rproc *rproc)
>  
>   qproc->running = false;
>  
> - qcom_smem_state_update_bits(qproc->state,
> - BIT(qproc->stop_bit), BIT(qproc->stop_bit));
> -
> - ret = wait_for_completion_timeout(&qproc->stop_done,
> -   msecs_to_jiffies(5000));
> - if (ret == 0)
> + ret = qcom_q6v5_request_stop(&qproc->q6v5);
> + if (ret == -ETIMEDOUT)
>   dev_err(qproc->dev, "timed out on wait\n");
>  
> - qcom_smem_state_update_bits(qproc->state, BIT(qproc->stop_bit), 0);
> -
>   q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6);
>   q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
>   q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
> @@ -976,9 +958,8 @@ static int q6v5_stop(struct rproc *rproc)
>  
>   q6v5_reset_assert(qproc);
>  
> - disable_irq(qproc->handover_irq);
> -
> - if (!qproc->proxy_unvoted) {
> + ret = qcom_q6v5_unprepare(&qproc->q6v5);
> + if (ret) {
>   q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
>qproc->proxy_clk_count);
>   q6v5_regulator_disable(qproc, qproc->proxy_regs,

 Nit, can qcom_msa_handover api be used instead here ?

 Rest all looks good,
reviewed-by: Sricharan R 

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



[RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: Use common q6v5 helpers

2018-05-22 Thread Bjorn Andersson
Migrate the MSS remoteproc driver to use the newly extracted helper
functions.

Signed-off-by: Bjorn Andersson 
---
 drivers/remoteproc/Kconfig |   4 +
 drivers/remoteproc/qcom_q6v5_pil.c | 157 +++--
 2 files changed, 19 insertions(+), 142 deletions(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index d51d155cf8bd..2316908e9788 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -116,6 +116,10 @@ config QCOM_Q6V5_PIL
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
+   select QCOM_Q6V5_COMMON
+   select QCOM_RPROC_COMMON
+   select QCOM_SCM
+   select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
select QCOM_SCM
help
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c 
b/drivers/remoteproc/qcom_q6v5_pil.c
index 2bf8e7c49f2a..e04319573c91 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -30,12 +30,11 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 
 #include "remoteproc_internal.h"
 #include "qcom_common.h"
+#include "qcom_q6v5.h"
 
 #include 
 
@@ -151,12 +150,7 @@ struct q6v5 {
 
struct reset_control *mss_restart;
 
-   struct qcom_smem_state *state;
-   unsigned stop_bit;
-
-   int handover_irq;
-
-   bool proxy_unvoted;
+   struct qcom_q6v5 q6v5;
 
struct clk *active_clks[8];
struct clk *reset_clks[4];
@@ -170,8 +164,6 @@ struct q6v5 {
int active_reg_count;
int proxy_reg_count;
 
-   struct completion start_done;
-   struct completion stop_done;
bool running;
 
phys_addr_t mba_phys;
@@ -798,9 +790,7 @@ static int q6v5_start(struct rproc *rproc)
int xfermemop_ret;
int ret;
 
-   qproc->proxy_unvoted = false;
-
-   enable_irq(qproc->handover_irq);
+   qcom_q6v5_prepare(&qproc->q6v5);
 
ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
qproc->proxy_reg_count);
@@ -875,11 +865,9 @@ static int q6v5_start(struct rproc *rproc)
if (ret)
goto reclaim_mpss;
 
-   ret = wait_for_completion_timeout(&qproc->start_done,
- msecs_to_jiffies(5000));
-   if (ret == 0) {
+   ret = qcom_q6v5_wait_for_start(&qproc->q6v5, msecs_to_jiffies(5000));
+   if (ret == -ETIMEDOUT) {
dev_err(qproc->dev, "start timed out\n");
-   ret = -ETIMEDOUT;
goto reclaim_mpss;
}
 
@@ -933,7 +921,7 @@ static int q6v5_start(struct rproc *rproc)
   qproc->proxy_reg_count);
 
 disable_irqs:
-   disable_irq(qproc->handover_irq);
+   qcom_q6v5_unprepare(&qproc->q6v5);
 
return ret;
 }
@@ -946,16 +934,10 @@ static int q6v5_stop(struct rproc *rproc)
 
qproc->running = false;
 
-   qcom_smem_state_update_bits(qproc->state,
-   BIT(qproc->stop_bit), BIT(qproc->stop_bit));
-
-   ret = wait_for_completion_timeout(&qproc->stop_done,
- msecs_to_jiffies(5000));
-   if (ret == 0)
+   ret = qcom_q6v5_request_stop(&qproc->q6v5);
+   if (ret == -ETIMEDOUT)
dev_err(qproc->dev, "timed out on wait\n");
 
-   qcom_smem_state_update_bits(qproc->state, BIT(qproc->stop_bit), 0);
-
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6);
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
@@ -976,9 +958,8 @@ static int q6v5_stop(struct rproc *rproc)
 
q6v5_reset_assert(qproc);
 
-   disable_irq(qproc->handover_irq);
-
-   if (!qproc->proxy_unvoted) {
+   ret = qcom_q6v5_unprepare(&qproc->q6v5);
+   if (ret) {
q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
 qproc->proxy_clk_count);
q6v5_regulator_disable(qproc, qproc->proxy_regs,
@@ -1014,74 +995,14 @@ static const struct rproc_ops q6v5_ops = {
.load = q6v5_load,
 };
 
-static irqreturn_t q6v5_wdog_interrupt(int irq, void *dev)
-{
-   struct q6v5 *qproc = dev;
-   size_t len;
-   char *msg;
-
-   /* Sometimes the stop triggers a watchdog rather than a stop-ack */
-   if (!qproc->running) {
-   complete(&qproc->stop_done);
-   return IRQ_HANDLED;
-   }
-
-   msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, MPSS_CRASH_REASON_SMEM, &len);
-   if (!IS_ERR(msg) && len > 0 && msg[0])
-   dev_err(qproc->dev, "watchdog received: %s\n", msg);
-   else
-   dev_err(qproc->dev, "watchdog without message\n");
-
-   rproc_report_crash(qproc->rproc, RPROC_WATCHDOG);
-
-   return IRQ_HANDLED;
-}
-
-static irqreturn_t q6v5_fatal_interrupt(in