| Message ID | 1479981638-32069-5-git-send-email-akdwived@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote: > Clock handling is made generic than earlier way to add new clock > every time when required to support new hexagon version. Also > clock disable interface is separated out into two separate routine > to unvote proxy clock alone when handover interrupt is arrived. > > Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_pil.c | 93 ++++++++++++++++++++++++++------------ > 1 file changed, 65 insertions(+), 28 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c > index 06d5bb2..c55dc9a 100644 > --- a/drivers/remoteproc/qcom_q6v5_pil.c > +++ b/drivers/remoteproc/qcom_q6v5_pil.c > @@ -126,10 +126,6 @@ struct q6v5 { > struct qcom_smem_state *state; > unsigned stop_bit; > > - > - struct clk *ahb_clk; > - struct clk *axi_clk; > - struct clk *rom_clk; > struct clk *active_clks[8]; > struct clk *proxy_clks[4]; > struct reg_info active_regs[1]; > @@ -284,6 +280,51 @@ static void q6v5_regulator_disable(struct q6v5 *qproc) > q6v5_active_regulator_disable(qproc); > } > > +static int q6v5_clk_enable(struct device *dev, struct clk **clks, > + int clk_count) > +{ > + int rc = 0; No need to initialize to 0. > + int i; > + > + for (i = 0; i < clk_count; i++) { > + rc = clk_prepare_enable(clks[i]); > + if (rc) { > + dev_err(dev, "Clock enable failed\n"); > + goto err; > + } > + } > + > + return 0; > +err: > + for (i--; i >= 0; i--) > + clk_disable_unprepare(clks[i]); > + > + return rc; > +} > + > +static void q6v5_proxy_clk_disable(struct q6v5 *qproc) Please make these follow the enable case, by taking the clock list and size as parameters. > +{ > + int i; > + struct clk **clks = qproc->proxy_clks; > + > + for (i = 0; i < qproc->proxy_clk_count; i++) > + clk_disable_unprepare(clks[i]); > +} > + > +static void q6v5_active_clk_disable(struct q6v5 *qproc) > +{ > + int i; > + struct clk **clks = qproc->proxy_clks; > + > + for (i = 0; i < qproc->proxy_clk_count; i++) > + clk_disable_unprepare(clks[i]); > +} > + > +static void q6v5_clk_disable(struct q6v5 *qproc) > +{ > + q6v5_proxy_clk_disable(qproc); > + q6v5_active_clk_disable(qproc); > +} Just inline the two disable calls where you need them, rather than having this wrapper. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/9/2016 8:27 AM, Bjorn Andersson wrote: > On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote: > >> Clock handling is made generic than earlier way to add new clock >> every time when required to support new hexagon version. Also >> clock disable interface is separated out into two separate routine >> to unvote proxy clock alone when handover interrupt is arrived. >> >> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> >> --- >> drivers/remoteproc/qcom_q6v5_pil.c | 93 ++++++++++++++++++++++++++------------ >> 1 file changed, 65 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c >> index 06d5bb2..c55dc9a 100644 >> --- a/drivers/remoteproc/qcom_q6v5_pil.c >> +++ b/drivers/remoteproc/qcom_q6v5_pil.c >> @@ -126,10 +126,6 @@ struct q6v5 { >> struct qcom_smem_state *state; >> unsigned stop_bit; >> >> - >> - struct clk *ahb_clk; >> - struct clk *axi_clk; >> - struct clk *rom_clk; >> struct clk *active_clks[8]; >> struct clk *proxy_clks[4]; >> struct reg_info active_regs[1]; >> @@ -284,6 +280,51 @@ static void q6v5_regulator_disable(struct q6v5 *qproc) >> q6v5_active_regulator_disable(qproc); >> } >> >> +static int q6v5_clk_enable(struct device *dev, struct clk **clks, >> + int clk_count) >> +{ >> + int rc = 0; > No need to initialize to 0. OK. > >> + int i; >> + >> + for (i = 0; i < clk_count; i++) { >> + rc = clk_prepare_enable(clks[i]); >> + if (rc) { >> + dev_err(dev, "Clock enable failed\n"); >> + goto err; >> + } >> + } >> + >> + return 0; >> +err: >> + for (i--; i >= 0; i--) >> + clk_disable_unprepare(clks[i]); >> + >> + return rc; >> +} >> + >> +static void q6v5_proxy_clk_disable(struct q6v5 *qproc) > Please make these follow the enable case, by taking the clock list and > size as parameters. OK. > >> +{ >> + int i; >> + struct clk **clks = qproc->proxy_clks; >> + >> + for (i = 0; i < qproc->proxy_clk_count; i++) >> + clk_disable_unprepare(clks[i]); >> +} >> + >> +static void q6v5_active_clk_disable(struct q6v5 *qproc) >> +{ >> + int i; >> + struct clk **clks = qproc->proxy_clks; >> + >> + for (i = 0; i < qproc->proxy_clk_count; i++) >> + clk_disable_unprepare(clks[i]); >> +} >> + >> +static void q6v5_clk_disable(struct q6v5 *qproc) >> +{ >> + q6v5_proxy_clk_disable(qproc); >> + q6v5_active_clk_disable(qproc); >> +} > Just inline the two disable calls where you need them, rather than > having this wrapper. OK. > > Regards, > Bjorn
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c index 06d5bb2..c55dc9a 100644 --- a/drivers/remoteproc/qcom_q6v5_pil.c +++ b/drivers/remoteproc/qcom_q6v5_pil.c @@ -126,10 +126,6 @@ struct q6v5 { struct qcom_smem_state *state; unsigned stop_bit; - - struct clk *ahb_clk; - struct clk *axi_clk; - struct clk *rom_clk; struct clk *active_clks[8]; struct clk *proxy_clks[4]; struct reg_info active_regs[1]; @@ -284,6 +280,51 @@ static void q6v5_regulator_disable(struct q6v5 *qproc) q6v5_active_regulator_disable(qproc); } +static int q6v5_clk_enable(struct device *dev, struct clk **clks, + int clk_count) +{ + int rc = 0; + int i; + + for (i = 0; i < clk_count; i++) { + rc = clk_prepare_enable(clks[i]); + if (rc) { + dev_err(dev, "Clock enable failed\n"); + goto err; + } + } + + return 0; +err: + for (i--; i >= 0; i--) + clk_disable_unprepare(clks[i]); + + return rc; +} + +static void q6v5_proxy_clk_disable(struct q6v5 *qproc) +{ + int i; + struct clk **clks = qproc->proxy_clks; + + for (i = 0; i < qproc->proxy_clk_count; i++) + clk_disable_unprepare(clks[i]); +} + +static void q6v5_active_clk_disable(struct q6v5 *qproc) +{ + int i; + struct clk **clks = qproc->proxy_clks; + + for (i = 0; i < qproc->proxy_clk_count; i++) + clk_disable_unprepare(clks[i]); +} + +static void q6v5_clk_disable(struct q6v5 *qproc) +{ + q6v5_proxy_clk_disable(qproc); + q6v5_active_clk_disable(qproc); +} static int q6v5_load(struct rproc *rproc, const struct firmware *fw) { @@ -581,11 +622,18 @@ static int q6v5_start(struct rproc *rproc) return ret; } + ret = q6v5_clk_enable(qproc->dev, qproc->proxy_clks, + qproc->proxy_clk_count); + if (ret) { + dev_err(qproc->dev, "failed to enable proxy clocks\n"); + goto disable_proxy_reg; + } + ret = q6v5_regulator_enable(qproc, qproc->active_regs, qproc->active_reg_count); if (ret) { dev_err(qproc->dev, "failed to enable supplies\n"); - goto disable_proxy_reg; + goto disable_proxy_clk; } ret = reset_control_deassert(qproc->mss_restart); if (ret) { @@ -593,17 +641,12 @@ static int q6v5_start(struct rproc *rproc) goto disable_vdd; } - ret = clk_prepare_enable(qproc->ahb_clk); - if (ret) + ret = q6v5_clk_enable(qproc->dev, qproc->active_clks, + qproc->active_clk_count); + if (ret) { + dev_err(qproc->dev, "failed to enable clocks\n"); goto assert_reset; - - ret = clk_prepare_enable(qproc->axi_clk); - if (ret) - goto disable_ahb_clk; - - ret = clk_prepare_enable(qproc->rom_clk); - if (ret) - goto disable_axi_clk; + } writel(qproc->mba_phys, qproc->rmb_base + RMB_MBA_IMAGE_REG); @@ -638,25 +681,21 @@ static int q6v5_start(struct rproc *rproc) qproc->running = true; - /* TODO: All done, release the handover resources */ - + q6v5_proxy_clk_disable(qproc); + q6v5_proxy_regulator_disable(qproc); return 0; halt_axi_ports: 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); - - clk_disable_unprepare(qproc->rom_clk); -disable_axi_clk: - clk_disable_unprepare(qproc->axi_clk); -disable_ahb_clk: - clk_disable_unprepare(qproc->ahb_clk); + q6v5_active_clk_disable(qproc); assert_reset: reset_control_assert(qproc->mss_restart); disable_vdd: - q6v5_regulator_disable(qproc); - + q6v5_active_regulator_disable(qproc); +disable_proxy_clk: + q6v5_proxy_clk_disable(qproc); disable_proxy_reg: q6v5_proxy_regulator_disable(qproc); return ret; @@ -684,9 +723,7 @@ static int q6v5_stop(struct rproc *rproc) q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc); reset_control_assert(qproc->mss_restart); - clk_disable_unprepare(qproc->rom_clk); - clk_disable_unprepare(qproc->axi_clk); - clk_disable_unprepare(qproc->ahb_clk); + q6v5_clk_disable(qproc); q6v5_regulator_disable(qproc); return 0;
Clock handling is made generic than earlier way to add new clock every time when required to support new hexagon version. Also clock disable interface is separated out into two separate routine to unvote proxy clock alone when handover interrupt is arrived. Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> --- drivers/remoteproc/qcom_q6v5_pil.c | 93 ++++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 28 deletions(-)