| Message ID | 1479981638-32069-3-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: > Certain regulators and clocks need voting by rproc on behalf of hexagon > only during restart operation but certain clocks and voltage need to be > voted till hexagon is up, these regulators and clocks are identified as > proxy and active resource whose handle is being obtained by supplying > private proxy and active regulator and clock string. > Please split this patch out into the regulator and clock patches respectively, making each a clear functional change. > Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_pil.c | 148 +++++++++++++++++++++++++++---------- > 1 file changed, 107 insertions(+), 41 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c > index 3360312..b0f0fcf 100644 > --- a/drivers/remoteproc/qcom_q6v5_pil.c > +++ b/drivers/remoteproc/qcom_q6v5_pil.c > @@ -37,7 +37,6 @@ > > #include <linux/qcom_scm.h> > > -#define MBA_FIRMWARE_NAME "mba.b00" Please move this to patch 1. > #define MPSS_FIRMWARE_NAME "modem.mdt" > > #define MPSS_CRASH_REASON_SMEM 421 > @@ -132,6 +131,14 @@ struct q6v5 { > 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]; > + struct reg_info proxy_regs[3]; > + int active_reg_count; > + int proxy_reg_count; > + int active_clk_count; > + int proxy_clk_count; Group clocks members together and regulators together. > > struct completion start_done; > struct completion stop_done; > @@ -160,27 +167,43 @@ enum { > Q6V5_SUPPLY_PLL, > }; > > -static int q6v5_regulator_init(struct q6v5 *qproc) > +static int q6v5_regulator_init(struct device *dev, > + struct reg_info *regs, char **reg_str, int volatage_load[][2]) > { > - int ret; > + int reg_count = 0, i; Please, one variable per line, sorted (descending) by line length. > > - qproc->supply[Q6V5_SUPPLY_CX].supply = "cx"; > - qproc->supply[Q6V5_SUPPLY_MX].supply = "mx"; > - qproc->supply[Q6V5_SUPPLY_MSS].supply = "mss"; > - qproc->supply[Q6V5_SUPPLY_PLL].supply = "pll"; > + if (!reg_str) > + return 0; > > - ret = devm_regulator_bulk_get(qproc->dev, > - ARRAY_SIZE(qproc->supply), qproc->supply); > - if (ret < 0) { > - dev_err(qproc->dev, "failed to get supplies\n"); > - return ret; > - } > + while (reg_str[reg_count] != NULL) Drop "!= NULL" from your condition. > + reg_count++; > > - regulator_set_load(qproc->supply[Q6V5_SUPPLY_CX].consumer, 100000); > - regulator_set_load(qproc->supply[Q6V5_SUPPLY_MSS].consumer, 100000); > - regulator_set_load(qproc->supply[Q6V5_SUPPLY_PLL].consumer, 10000); > + if (!reg_count) > + return reg_count; You can omit this check, as the for loop below will iterate 0 times and we will then return 0. > > - return 0; > + if (!regs) > + return -ENOMEM; This will not happen, please drop. > + > + for (i = 0; i < reg_count; i++) { > + const char *reg_name; Please keep local variables at the top of the function. And generally try to use short but clear variable names; in line with my suggestion in patch 1 name it "supply" (or in this case where the original variable is quite short just use it directly). > + > + reg_name = reg_str[i]; > + regs[i].reg = devm_regulator_get(dev, reg_name); > + if (IS_ERR(regs[i].reg)) { > + > + int rc = PTR_ERR(regs[i].reg); > + > + if (rc != -EPROBE_DEFER) > + dev_err(dev, "Failed to get %s\n regulator", > + reg_name); > + return rc; > + } > + > + regs[i].uV = volatage_load[i][0]; > + regs[i].uA = volatage_load[i][1]; > + } > + > + return reg_count; > } > > static int q6v5_regulator_enable(struct q6v5 *qproc) > @@ -725,27 +748,41 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev) > return 0; > } > > -static int q6v5_init_clocks(struct q6v5 *qproc) > +static int q6v5_init_clocks(struct device *dev, struct clk **clks, > + char **clk_str) > { > - qproc->ahb_clk = devm_clk_get(qproc->dev, "iface"); > - if (IS_ERR(qproc->ahb_clk)) { > - dev_err(qproc->dev, "failed to get iface clock\n"); > - return PTR_ERR(qproc->ahb_clk); > - } > + int clk_count = 0, i; One variable per line, please. And "count" is unambiguous enough. > > - qproc->axi_clk = devm_clk_get(qproc->dev, "bus"); > - if (IS_ERR(qproc->axi_clk)) { > - dev_err(qproc->dev, "failed to get bus clock\n"); > - return PTR_ERR(qproc->axi_clk); > - } > + if (!clk_str) > + return 0; > + > + while (clk_str[clk_count] != NULL) Drop "!= NULL" part > + clk_count++; > + > + if (!clk_count) > + return clk_count; This is not needed. > + > + if (!clks) > + return -ENOMEM; This can't happen. > + > + for (i = 0; i < clk_count; i++) { > + const char *clock_name; > + > + clock_name = clk_str[i]; > + clks[i] = devm_clk_get(dev, clock_name); > + if (IS_ERR(clks[i])) { > + > + int rc = PTR_ERR(clks[i]); > + > + if (rc != -EPROBE_DEFER) > + dev_err(dev, "Failed to get %s clock\n", > + clock_name); > + return rc; > + } > > - qproc->rom_clk = devm_clk_get(qproc->dev, "mem"); > - if (IS_ERR(qproc->rom_clk)) { > - dev_err(qproc->dev, "failed to get mem clock\n"); > - return PTR_ERR(qproc->rom_clk); > } > > - return 0; > + return clk_count; > } > > static int q6v5_init_reset(struct q6v5 *qproc) > @@ -830,10 +867,15 @@ static int q6v5_probe(struct platform_device *pdev) > { > struct q6v5 *qproc; > struct rproc *rproc; > - int ret; > + const struct rproc_hexagon_res *desc; > + int ret, count; One variable per line please, and sort (descending) on line length. > + > + desc = of_device_get_match_data(&pdev->dev); > + if (!desc) > + return -EINVAL; > > rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_ops, > - MBA_FIRMWARE_NAME, sizeof(*qproc)); > + desc->hexagon_mba_image, sizeof(*qproc)); Please move this to patch 1. > if (!rproc) { > dev_err(&pdev->dev, "failed to allocate rproc\n"); > return -ENOMEM; > @@ -857,18 +899,42 @@ static int q6v5_probe(struct platform_device *pdev) > if (ret) > goto free_rproc; > > - if (ret) > - goto free_rproc; > + count = q6v5_init_clocks(&pdev->dev, qproc->proxy_clks, > + desc->proxy_clk_string); Your "count" serves the same purpose as "ret", so just use "ret". And align broken lines with the parenthesis. > + if (count < 0) { > + dev_err(&pdev->dev, "Failed to setup proxy clocks.\n"); > + return count; And using "ret" instead of "count" makes it easy to goto free_rproc, which you must do after calling rproc_alloc(). > + } > + qproc->proxy_clk_count = count; > > - ret = q6v5_regulator_init(qproc); > - if (ret) > - goto free_rproc; > + count = q6v5_init_clocks(&pdev->dev, qproc->active_clks, > + desc->active_clk_string); > + if (count < 0) { > + dev_err(&pdev->dev, "Failed to setup active clocks.\n"); > + return count; > + } > + qproc->active_clk_count = count; > > ret = q6v5_init_reset(qproc); > if (ret) > goto free_rproc; > > + count = q6v5_regulator_init(&pdev->dev, qproc->proxy_regs, > + desc->proxy_reg_string, (int (*)[2])desc->proxy_uV_uA); Moving proxy_reg_string and proxy_uV_uA into a struct (as suggested in patch 1) will make this cleaner and we get rid of that typecast. > + if (count < 0) { > + dev_err(&pdev->dev, "Failed to setup active regulators.\n"); > + return count; > + } > + qproc->proxy_reg_count = count; > + > + count = q6v5_regulator_init(&pdev->dev, qproc->active_regs, > + desc->active_reg_string, (int (*)[2])desc->active_uV_uA); > + if (count < 0) { > + dev_err(&pdev->dev, "Failed to setup proxy regulators.\n"); > + return count; > + } > + qproc->active_reg_count = count; > + > ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt); > if (ret < 0) > goto free_rproc; 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:06 AM, Bjorn Andersson wrote: > On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote: > >> Certain regulators and clocks need voting by rproc on behalf of hexagon >> only during restart operation but certain clocks and voltage need to be >> voted till hexagon is up, these regulators and clocks are identified as >> proxy and active resource whose handle is being obtained by supplying >> private proxy and active regulator and clock string. >> > Please split this patch out into the regulator and clock patches > respectively, making each a clear functional change. OK. > >> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> >> --- >> drivers/remoteproc/qcom_q6v5_pil.c | 148 +++++++++++++++++++++++++++---------- >> 1 file changed, 107 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c >> index 3360312..b0f0fcf 100644 >> --- a/drivers/remoteproc/qcom_q6v5_pil.c >> +++ b/drivers/remoteproc/qcom_q6v5_pil.c >> @@ -37,7 +37,6 @@ >> >> #include <linux/qcom_scm.h> >> >> -#define MBA_FIRMWARE_NAME "mba.b00" > Please move this to patch 1. OK. > >> #define MPSS_FIRMWARE_NAME "modem.mdt" >> >> #define MPSS_CRASH_REASON_SMEM 421 >> @@ -132,6 +131,14 @@ struct q6v5 { >> 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]; >> + struct reg_info proxy_regs[3]; >> + int active_reg_count; >> + int proxy_reg_count; >> + int active_clk_count; >> + int proxy_clk_count; > Group clocks members together and regulators together. OK. > >> >> struct completion start_done; >> struct completion stop_done; >> @@ -160,27 +167,43 @@ enum { >> Q6V5_SUPPLY_PLL, >> }; >> >> -static int q6v5_regulator_init(struct q6v5 *qproc) >> +static int q6v5_regulator_init(struct device *dev, >> + struct reg_info *regs, char **reg_str, int volatage_load[][2]) >> { >> - int ret; >> + int reg_count = 0, i; > Please, one variable per line, sorted (descending) by line length. OK. > >> >> - qproc->supply[Q6V5_SUPPLY_CX].supply = "cx"; >> - qproc->supply[Q6V5_SUPPLY_MX].supply = "mx"; >> - qproc->supply[Q6V5_SUPPLY_MSS].supply = "mss"; >> - qproc->supply[Q6V5_SUPPLY_PLL].supply = "pll"; >> + if (!reg_str) >> + return 0; >> >> - ret = devm_regulator_bulk_get(qproc->dev, >> - ARRAY_SIZE(qproc->supply), qproc->supply); >> - if (ret < 0) { >> - dev_err(qproc->dev, "failed to get supplies\n"); >> - return ret; >> - } >> + while (reg_str[reg_count] != NULL) > Drop "!= NULL" from your condition. OK. > >> + reg_count++; >> >> - regulator_set_load(qproc->supply[Q6V5_SUPPLY_CX].consumer, 100000); >> - regulator_set_load(qproc->supply[Q6V5_SUPPLY_MSS].consumer, 100000); >> - regulator_set_load(qproc->supply[Q6V5_SUPPLY_PLL].consumer, 10000); >> + if (!reg_count) >> + return reg_count; > You can omit this check, as the for loop below will iterate 0 times and > we will then return 0. > >> >> - return 0; >> + if (!regs) >> + return -ENOMEM; > This will not happen, please drop. > >> + >> + for (i = 0; i < reg_count; i++) { >> + const char *reg_name; > Please keep local variables at the top of the function. > > And generally try to use short but clear variable names; in line with my > suggestion in patch 1 name it "supply" (or in this case where the > original variable is quite short just use it directly). OK > >> + >> + reg_name = reg_str[i]; >> + regs[i].reg = devm_regulator_get(dev, reg_name); >> + if (IS_ERR(regs[i].reg)) { >> + >> + int rc = PTR_ERR(regs[i].reg); >> + >> + if (rc != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get %s\n regulator", >> + reg_name); >> + return rc; >> + } >> + >> + regs[i].uV = volatage_load[i][0]; >> + regs[i].uA = volatage_load[i][1]; >> + } >> + >> + return reg_count; >> } >> >> static int q6v5_regulator_enable(struct q6v5 *qproc) >> @@ -725,27 +748,41 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev) >> return 0; >> } >> >> -static int q6v5_init_clocks(struct q6v5 *qproc) >> +static int q6v5_init_clocks(struct device *dev, struct clk **clks, >> + char **clk_str) >> { >> - qproc->ahb_clk = devm_clk_get(qproc->dev, "iface"); >> - if (IS_ERR(qproc->ahb_clk)) { >> - dev_err(qproc->dev, "failed to get iface clock\n"); >> - return PTR_ERR(qproc->ahb_clk); >> - } >> + int clk_count = 0, i; > One variable per line, please. And "count" is unambiguous enough. OK. > >> >> - qproc->axi_clk = devm_clk_get(qproc->dev, "bus"); >> - if (IS_ERR(qproc->axi_clk)) { >> - dev_err(qproc->dev, "failed to get bus clock\n"); >> - return PTR_ERR(qproc->axi_clk); >> - } >> + if (!clk_str) >> + return 0; >> + >> + while (clk_str[clk_count] != NULL) > Drop "!= NULL" part OK. > >> + clk_count++; >> + >> + if (!clk_count) >> + return clk_count; > This is not needed. Yes. > >> + >> + if (!clks) >> + return -ENOMEM; > This can't happen. YES. > >> + >> + for (i = 0; i < clk_count; i++) { >> + const char *clock_name; >> + >> + clock_name = clk_str[i]; >> + clks[i] = devm_clk_get(dev, clock_name); >> + if (IS_ERR(clks[i])) { >> + >> + int rc = PTR_ERR(clks[i]); >> + >> + if (rc != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get %s clock\n", >> + clock_name); >> + return rc; >> + } >> >> - qproc->rom_clk = devm_clk_get(qproc->dev, "mem"); >> - if (IS_ERR(qproc->rom_clk)) { >> - dev_err(qproc->dev, "failed to get mem clock\n"); >> - return PTR_ERR(qproc->rom_clk); >> } >> >> - return 0; >> + return clk_count; >> } >> >> static int q6v5_init_reset(struct q6v5 *qproc) >> @@ -830,10 +867,15 @@ static int q6v5_probe(struct platform_device *pdev) >> { >> struct q6v5 *qproc; >> struct rproc *rproc; >> - int ret; >> + const struct rproc_hexagon_res *desc; >> + int ret, count; > One variable per line please, and sort (descending) on line length. ok, > >> + >> + desc = of_device_get_match_data(&pdev->dev); >> + if (!desc) >> + return -EINVAL; >> >> rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_ops, >> - MBA_FIRMWARE_NAME, sizeof(*qproc)); >> + desc->hexagon_mba_image, sizeof(*qproc)); > Please move this to patch 1. OK, > >> if (!rproc) { >> dev_err(&pdev->dev, "failed to allocate rproc\n"); >> return -ENOMEM; >> @@ -857,18 +899,42 @@ static int q6v5_probe(struct platform_device *pdev) >> if (ret) >> goto free_rproc; >> >> - if (ret) >> - goto free_rproc; >> + count = q6v5_init_clocks(&pdev->dev, qproc->proxy_clks, >> + desc->proxy_clk_string); > Your "count" serves the same purpose as "ret", so just use "ret". > And align broken lines with the parenthesis. OK. > >> + if (count < 0) { >> + dev_err(&pdev->dev, "Failed to setup proxy clocks.\n"); >> + return count; > And using "ret" instead of "count" makes it easy to goto free_rproc, > which you must do after calling rproc_alloc(). OK. > >> + } >> + qproc->proxy_clk_count = count; >> >> - ret = q6v5_regulator_init(qproc); >> - if (ret) >> - goto free_rproc; >> + count = q6v5_init_clocks(&pdev->dev, qproc->active_clks, >> + desc->active_clk_string); >> + if (count < 0) { >> + dev_err(&pdev->dev, "Failed to setup active clocks.\n"); >> + return count; >> + } >> + qproc->active_clk_count = count; >> >> ret = q6v5_init_reset(qproc); >> if (ret) >> goto free_rproc; >> >> + count = q6v5_regulator_init(&pdev->dev, qproc->proxy_regs, >> + desc->proxy_reg_string, (int (*)[2])desc->proxy_uV_uA); > Moving proxy_reg_string and proxy_uV_uA into a struct (as suggested in > patch 1) will make this cleaner and we get rid of that typecast. OK. > >> + if (count < 0) { >> + dev_err(&pdev->dev, "Failed to setup active regulators.\n"); >> + return count; >> + } >> + qproc->proxy_reg_count = count; >> + >> + count = q6v5_regulator_init(&pdev->dev, qproc->active_regs, >> + desc->active_reg_string, (int (*)[2])desc->active_uV_uA); >> + if (count < 0) { >> + dev_err(&pdev->dev, "Failed to setup proxy regulators.\n"); >> + return count; >> + } >> + qproc->active_reg_count = count; >> + >> ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt); >> if (ret < 0) >> goto free_rproc; > 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
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c index 3360312..b0f0fcf 100644 --- a/drivers/remoteproc/qcom_q6v5_pil.c +++ b/drivers/remoteproc/qcom_q6v5_pil.c @@ -37,7 +37,6 @@ #include <linux/qcom_scm.h> -#define MBA_FIRMWARE_NAME "mba.b00" #define MPSS_FIRMWARE_NAME "modem.mdt" #define MPSS_CRASH_REASON_SMEM 421 @@ -132,6 +131,14 @@ struct q6v5 { 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]; + struct reg_info proxy_regs[3]; + int active_reg_count; + int proxy_reg_count; + int active_clk_count; + int proxy_clk_count; struct completion start_done; struct completion stop_done; @@ -160,27 +167,43 @@ enum { Q6V5_SUPPLY_PLL, }; -static int q6v5_regulator_init(struct q6v5 *qproc) +static int q6v5_regulator_init(struct device *dev, + struct reg_info *regs, char **reg_str, int volatage_load[][2]) { - int ret; + int reg_count = 0, i; - qproc->supply[Q6V5_SUPPLY_CX].supply = "cx"; - qproc->supply[Q6V5_SUPPLY_MX].supply = "mx"; - qproc->supply[Q6V5_SUPPLY_MSS].supply = "mss"; - qproc->supply[Q6V5_SUPPLY_PLL].supply = "pll"; + if (!reg_str) + return 0; - ret = devm_regulator_bulk_get(qproc->dev, - ARRAY_SIZE(qproc->supply), qproc->supply); - if (ret < 0) { - dev_err(qproc->dev, "failed to get supplies\n"); - return ret; - } + while (reg_str[reg_count] != NULL) + reg_count++; - regulator_set_load(qproc->supply[Q6V5_SUPPLY_CX].consumer, 100000); - regulator_set_load(qproc->supply[Q6V5_SUPPLY_MSS].consumer, 100000); - regulator_set_load(qproc->supply[Q6V5_SUPPLY_PLL].consumer, 10000); + if (!reg_count) + return reg_count; - return 0; + if (!regs) + return -ENOMEM; + + for (i = 0; i < reg_count; i++) { + const char *reg_name; + + reg_name = reg_str[i]; + regs[i].reg = devm_regulator_get(dev, reg_name); + if (IS_ERR(regs[i].reg)) { + + int rc = PTR_ERR(regs[i].reg); + + if (rc != -EPROBE_DEFER) + dev_err(dev, "Failed to get %s\n regulator", + reg_name); + return rc; + } + + regs[i].uV = volatage_load[i][0]; + regs[i].uA = volatage_load[i][1]; + } + + return reg_count; } static int q6v5_regulator_enable(struct q6v5 *qproc) @@ -725,27 +748,41 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev) return 0; } -static int q6v5_init_clocks(struct q6v5 *qproc) +static int q6v5_init_clocks(struct device *dev, struct clk **clks, + char **clk_str) { - qproc->ahb_clk = devm_clk_get(qproc->dev, "iface"); - if (IS_ERR(qproc->ahb_clk)) { - dev_err(qproc->dev, "failed to get iface clock\n"); - return PTR_ERR(qproc->ahb_clk); - } + int clk_count = 0, i; - qproc->axi_clk = devm_clk_get(qproc->dev, "bus"); - if (IS_ERR(qproc->axi_clk)) { - dev_err(qproc->dev, "failed to get bus clock\n"); - return PTR_ERR(qproc->axi_clk); - } + if (!clk_str) + return 0; + + while (clk_str[clk_count] != NULL) + clk_count++; + + if (!clk_count) + return clk_count; + + if (!clks) + return -ENOMEM; + + for (i = 0; i < clk_count; i++) { + const char *clock_name; + + clock_name = clk_str[i]; + clks[i] = devm_clk_get(dev, clock_name); + if (IS_ERR(clks[i])) { + + int rc = PTR_ERR(clks[i]); + + if (rc != -EPROBE_DEFER) + dev_err(dev, "Failed to get %s clock\n", + clock_name); + return rc; + } - qproc->rom_clk = devm_clk_get(qproc->dev, "mem"); - if (IS_ERR(qproc->rom_clk)) { - dev_err(qproc->dev, "failed to get mem clock\n"); - return PTR_ERR(qproc->rom_clk); } - return 0; + return clk_count; } static int q6v5_init_reset(struct q6v5 *qproc) @@ -830,10 +867,15 @@ static int q6v5_probe(struct platform_device *pdev) { struct q6v5 *qproc; struct rproc *rproc; - int ret; + const struct rproc_hexagon_res *desc; + int ret, count; + + desc = of_device_get_match_data(&pdev->dev); + if (!desc) + return -EINVAL; rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_ops, - MBA_FIRMWARE_NAME, sizeof(*qproc)); + desc->hexagon_mba_image, sizeof(*qproc)); if (!rproc) { dev_err(&pdev->dev, "failed to allocate rproc\n"); return -ENOMEM; @@ -857,18 +899,42 @@ static int q6v5_probe(struct platform_device *pdev) if (ret) goto free_rproc; - ret = q6v5_init_clocks(qproc); - if (ret) - goto free_rproc; + count = q6v5_init_clocks(&pdev->dev, qproc->proxy_clks, + desc->proxy_clk_string); + if (count < 0) { + dev_err(&pdev->dev, "Failed to setup proxy clocks.\n"); + return count; + } + qproc->proxy_clk_count = count; - ret = q6v5_regulator_init(qproc); - if (ret) - goto free_rproc; + count = q6v5_init_clocks(&pdev->dev, qproc->active_clks, + desc->active_clk_string); + if (count < 0) { + dev_err(&pdev->dev, "Failed to setup active clocks.\n"); + return count; + } + qproc->active_clk_count = count; ret = q6v5_init_reset(qproc); if (ret) goto free_rproc; + count = q6v5_regulator_init(&pdev->dev, qproc->proxy_regs, + desc->proxy_reg_string, (int (*)[2])desc->proxy_uV_uA); + if (count < 0) { + dev_err(&pdev->dev, "Failed to setup active regulators.\n"); + return count; + } + qproc->proxy_reg_count = count; + + count = q6v5_regulator_init(&pdev->dev, qproc->active_regs, + desc->active_reg_string, (int (*)[2])desc->active_uV_uA); + if (count < 0) { + dev_err(&pdev->dev, "Failed to setup proxy regulators.\n"); + return count; + } + qproc->active_reg_count = count; + ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt); if (ret < 0) goto free_rproc;
Certain regulators and clocks need voting by rproc on behalf of hexagon only during restart operation but certain clocks and voltage need to be voted till hexagon is up, these regulators and clocks are identified as proxy and active resource whose handle is being obtained by supplying private proxy and active regulator and clock string. Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> --- drivers/remoteproc/qcom_q6v5_pil.c | 148 +++++++++++++++++++++++++++---------- 1 file changed, 107 insertions(+), 41 deletions(-)