| Message ID | 1485788589-21968-3-git-send-email-akdwived@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon 30 Jan 07:03 PST 2017, Avaneesh Kumar Dwivedi wrote: > This patch add additional clock and regulator resource which are > initialized based on compatible and has no impact on existing driver > working. This resourse addition enable the existing driver to handle. > low pass sensor processor device also. > > Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> Applied, with below modification. > --- > drivers/remoteproc/qcom_adsp_pil.c | 43 +++++++++++++++++++++++++++++++------- > 1 file changed, 36 insertions(+), 7 deletions(-) > > diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c [..] > static int adsp_init_regulator(struct qcom_adsp *adsp) > { > - adsp->cx_supply = devm_regulator_get(adsp->dev, "cx"); > + adsp->cx_supply = devm_regulator_get(adsp->dev, "vdd_cx"); We should not change the name of devicetree properties, so I dropped "vdd_" on both of these. > if (IS_ERR(adsp->cx_supply)) > return PTR_ERR(adsp->cx_supply); > > regulator_set_load(adsp->cx_supply, 100000); > > + adsp->px_supply = devm_regulator_get(adsp->dev, "vdd_px"); > + if (IS_ERR(adsp->px_supply)) > + return PTR_ERR(adsp->px_supply); 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
On 1/31/2017 3:16 AM, Bjorn Andersson wrote: > On Mon 30 Jan 07:03 PST 2017, Avaneesh Kumar Dwivedi wrote: > >> This patch add additional clock and regulator resource which are >> initialized based on compatible and has no impact on existing driver >> working. This resourse addition enable the existing driver to handle. >> low pass sensor processor device also. >> >> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > Applied, with below modification. Thanks Bjorn, but please look below inline comment. >> --- >> drivers/remoteproc/qcom_adsp_pil.c | 43 +++++++++++++++++++++++++++++++------- >> 1 file changed, 36 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c > [..] >> static int adsp_init_regulator(struct qcom_adsp *adsp) >> { >> - adsp->cx_supply = devm_regulator_get(adsp->dev, "cx"); >> + adsp->cx_supply = devm_regulator_get(adsp->dev, "vdd_cx"); > We should not change the name of devicetree properties, so I dropped > "vdd_" on both of these. I observed that giving "cx" or "px" string to devm_regulator_get() was returning with dummy regulator, and if i gave "vdd_cx" and "vdd_px" it did not print dummy regulator warning. in device tree these regulators node were defined as "vdd_cx-supply" and "vdd_px-supply" > >> if (IS_ERR(adsp->cx_supply)) >> return PTR_ERR(adsp->cx_supply); >> >> regulator_set_load(adsp->cx_supply, 100000); >> >> + adsp->px_supply = devm_regulator_get(adsp->dev, "vdd_px"); >> + if (IS_ERR(adsp->px_supply)) >> + return PTR_ERR(adsp->px_supply); > Regards, > Bjorn
On Mon 30 Jan 21:58 PST 2017, Dwivedi, Avaneesh Kumar (avani) wrote: > > > On 1/31/2017 3:16 AM, Bjorn Andersson wrote: > > On Mon 30 Jan 07:03 PST 2017, Avaneesh Kumar Dwivedi wrote: > > > > > This patch add additional clock and regulator resource which are > > > initialized based on compatible and has no impact on existing driver > > > working. This resourse addition enable the existing driver to handle. > > > low pass sensor processor device also. > > > > > > Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > > Applied, with below modification. > Thanks Bjorn, but please look below inline comment. > > > --- > > > drivers/remoteproc/qcom_adsp_pil.c | 43 +++++++++++++++++++++++++++++++------- > > > 1 file changed, 36 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c > > [..] > > > static int adsp_init_regulator(struct qcom_adsp *adsp) > > > { > > > - adsp->cx_supply = devm_regulator_get(adsp->dev, "cx"); > > > + adsp->cx_supply = devm_regulator_get(adsp->dev, "vdd_cx"); > > We should not change the name of devicetree properties, so I dropped > > "vdd_" on both of these. > I observed that giving "cx" or "px" string to devm_regulator_get() was > returning with dummy regulator, and if i gave "vdd_cx" and "vdd_px" it did > not print dummy regulator warning. > in device tree these regulators node were defined as "vdd_cx-supply" and > "vdd_px-supply" They are named "vdd_cx" and "vdd_px" in the downstream dts, I didn't notice this originally and as we have a few other discrepancies to the downstream binding I rather stay compatible with the existing upstream DT binding than the downstream. So please update your dts. Btw, forgot to mention that aggre2 definitely is a "bus" and I think it should be represented separately, but I figured its better to merge the driver as is and then remove aggre2 once we have figured out how to represent/reference it properly. 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
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c index f674301..58cc46d 100644 --- a/drivers/remoteproc/qcom_adsp_pil.c +++ b/drivers/remoteproc/qcom_adsp_pil.c @@ -36,6 +36,7 @@ struct adsp_data { int crash_reason_smem; const char *firmware_name; int pas_id; + bool has_aggre2_clk; }; @@ -53,11 +54,13 @@ struct qcom_adsp { unsigned stop_bit; struct clk *xo; - + struct clk *aggre2_clk; struct regulator *cx_supply; + struct regulator *px_supply; int pas_id; int crash_reason_smem; + bool has_aggre2_clk; struct completion start_done; struct completion stop_done; @@ -115,16 +118,22 @@ static int adsp_start(struct rproc *rproc) ret = clk_prepare_enable(adsp->xo); if (ret) return ret; + ret = clk_prepare_enable(adsp->aggre2_clk); + if (ret) + goto disable_xo_clk; ret = regulator_enable(adsp->cx_supply); if (ret) - goto disable_clocks; + goto disable_aggre2_clk; + ret = regulator_enable(adsp->px_supply); + if (ret) + goto disable_cx_supply; ret = qcom_scm_pas_auth_and_reset(adsp->pas_id); if (ret) { dev_err(adsp->dev, "failed to authenticate image and release reset\n"); - goto disable_regulators; + goto disable_px_supply; } ret = wait_for_completion_timeout(&adsp->start_done, @@ -133,14 +142,18 @@ static int adsp_start(struct rproc *rproc) dev_err(adsp->dev, "start timed out\n"); qcom_scm_pas_shutdown(adsp->pas_id); ret = -ETIMEDOUT; - goto disable_regulators; + goto disable_px_supply; } ret = 0; -disable_regulators: +disable_px_supply: + regulator_disable(adsp->px_supply); +disable_cx_supply: regulator_disable(adsp->cx_supply); -disable_clocks: +disable_aggre2_clk: + clk_disable_unprepare(adsp->aggre2_clk); +disable_xo_clk: clk_disable_unprepare(adsp->xo); return ret; @@ -251,17 +264,31 @@ static int adsp_init_clock(struct qcom_adsp *adsp) return ret; } + if (adsp->has_aggre2_clk) { + adsp->aggre2_clk = devm_clk_get(adsp->dev, "aggre2"); + if (IS_ERR(adsp->aggre2_clk)) { + ret = PTR_ERR(adsp->aggre2_clk); + if (ret != -EPROBE_DEFER) + dev_err(adsp->dev, + "failed to get aggre2 clock"); + return ret; + } + } + return 0; } static int adsp_init_regulator(struct qcom_adsp *adsp) { - adsp->cx_supply = devm_regulator_get(adsp->dev, "cx"); + adsp->cx_supply = devm_regulator_get(adsp->dev, "vdd_cx"); if (IS_ERR(adsp->cx_supply)) return PTR_ERR(adsp->cx_supply); regulator_set_load(adsp->cx_supply, 100000); + adsp->px_supply = devm_regulator_get(adsp->dev, "vdd_px"); + if (IS_ERR(adsp->px_supply)) + return PTR_ERR(adsp->px_supply); return 0; } @@ -358,6 +385,7 @@ static int adsp_probe(struct platform_device *pdev) adsp->pas_id = desc->pas_id; adsp->crash_reason_smem = desc->crash_reason_smem; + adsp->has_aggre2_clk = desc->has_aggre2_clk; ret = adsp_init_clock(adsp); if (ret) goto free_rproc; @@ -425,6 +453,7 @@ static int adsp_remove(struct platform_device *pdev) .crash_reason_smem = 423, .firmware_name = "adsp.mdt", .pas_id = 1, + .has_aggre2_clk = 0, }; static const struct of_device_id adsp_of_match[] = { { .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init},
This patch add additional clock and regulator resource which are initialized based on compatible and has no impact on existing driver working. This resourse addition enable the existing driver to handle. low pass sensor processor device also. Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> --- drivers/remoteproc/qcom_adsp_pil.c | 43 +++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-)