| Message ID | 1615361290-19238-3-git-send-email-pillair@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add support for sc7280 WPSS PIL loading | expand |
On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote: > Add support for PIL loading of WPSS processor for SC7280 > WPSS boot will be requested by the wifi driver and hence > disable auto-boot for WPSS. Also add a separate shutdown > sequence handler for WPSS. > > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_adsp.c | 77 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 76 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c > index e024502..dc6b91d 100644 > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > @@ -58,6 +58,8 @@ struct adsp_pil_data { > const char *ssr_name; > const char *sysmon_name; > int ssctl_id; > + bool is_wpss; > + bool auto_boot; > > const char **clk_ids; > int num_clks; > @@ -96,8 +98,54 @@ struct qcom_adsp { > struct qcom_rproc_glink glink_subdev; > struct qcom_rproc_ssr ssr_subdev; > struct qcom_sysmon *sysmon; > + > + int (*shutdown)(struct qcom_adsp *adsp); > }; > > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) > +{ > + unsigned long timeout; > + unsigned int val; > + int ret; > + > + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 1); > + > + /* Wait for halt ACK from QDSP6 */ > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > + for (;;) { > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTACK_REG, &val); > + if (ret || val || time_after(jiffies, timeout)) > + break; > + > + usleep_range(1000, 1100); > + } > + > + /* Place the WPSS processor into reset */ > + reset_control_assert(adsp->restart); > + /* wait after asserting subsystem restart from AOSS */ > + usleep_range(100, 105); > + /* Remove the WPSS reset */ > + reset_control_deassert(adsp->restart); > + > + usleep_range(100, 105); > + > + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 0); > + > + /* Wait for halt ACK from QDSP6 */ > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > + for (;;) { > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTACK_REG, &val); > + if (ret || !val || time_after(jiffies, timeout)) > + break; > + > + usleep_range(1000, 1100); > + } > + > + return 0; > +} > + > static int qcom_adsp_shutdown(struct qcom_adsp *adsp) > { > unsigned long timeout; > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) > if (ret == -ETIMEDOUT) > dev_err(adsp->dev, "timed out on wait\n"); > > - ret = qcom_adsp_shutdown(adsp); > + ret = adsp->shutdown(adsp); > if (ret) > dev_err(adsp->dev, "failed to shutdown: %d\n", ret); > > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "unable to allocate remoteproc\n"); > return -ENOMEM; > } > + > + rproc->auto_boot = desc->auto_boot; > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > adsp = (struct qcom_adsp *)rproc->priv; > @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device *pdev) > adsp->info_name = desc->sysmon_name; > platform_set_drvdata(pdev, adsp); > > + if (desc->is_wpss) > + adsp->shutdown = qcom_wpss_shutdown; > + else > + adsp->shutdown = qcom_adsp_shutdown; > + > ret = adsp_alloc_memory_region(adsp); > if (ret) > goto free_rproc; > @@ -515,6 +570,8 @@ static const struct adsp_pil_data adsp_resource_init = { > .ssr_name = "lpass", > .sysmon_name = "adsp", > .ssctl_id = 0x14, > + .is_wpss = false, > + .auto_boot = true; > .clk_ids = (const char*[]) { > "sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr", > "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL > @@ -528,6 +585,8 @@ static const struct adsp_pil_data cdsp_resource_init = { > .ssr_name = "cdsp", > .sysmon_name = "cdsp", > .ssctl_id = 0x17, > + .is_wpss = false, > + .auto_boot = true; > .clk_ids = (const char*[]) { > "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", "q6ss_master", > "q6_axim", NULL > @@ -535,7 +594,23 @@ static const struct adsp_pil_data cdsp_resource_init = { > .num_clks = 7, > }; > > +static const struct adsp_pil_data wpss_resource_init = { > + .crash_reason_smem = 626, > + .firmware_name = "wpss.mdt", > + .ssr_name = "wpss", > + .sysmon_name = "wpss", > + .ssctl_id = 0x19, > + .is_wpss = true, > + .auto_boot = false; Why is auto_boot false for the WPSS? > + .clk_ids = (const char*[]) { > + "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", > + "gcc_wpss_rscp_clk", NULL > + }, > + .num_clks = 3, > +}; > + > static const struct of_device_id adsp_of_match[] = { > + { .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init }, Nit. Please keep things like this sorted alphabetically. Regards, Bjorn > { .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init }, > { .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init }, > { }, > -- > 2.7.4 >
> -----Original Message----- > From: Bjorn Andersson <bjorn.andersson@linaro.org> > Sent: Wednesday, March 10, 2021 10:15 PM > To: Rakesh Pillai <pillair@codeaurora.org> > Cc: agross@kernel.org; ohad@wizery.com; mathieu.poirier@linaro.org; > robh+dt@kernel.org; p.zabel@pengutronix.de; sibis@codeaurora.org; linux- > arm-msm@vger.kernel.org; linux-remoteproc@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for > sc7280 WPSS > > On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote: > > > Add support for PIL loading of WPSS processor for SC7280 > > WPSS boot will be requested by the wifi driver and hence > > disable auto-boot for WPSS. Also add a separate shutdown > > sequence handler for WPSS. > > > > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> > > --- > > drivers/remoteproc/qcom_q6v5_adsp.c | 77 > ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 76 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c > b/drivers/remoteproc/qcom_q6v5_adsp.c > > index e024502..dc6b91d 100644 > > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > > @@ -58,6 +58,8 @@ struct adsp_pil_data { > > const char *ssr_name; > > const char *sysmon_name; > > int ssctl_id; > > + bool is_wpss; > > + bool auto_boot; > > > > const char **clk_ids; > > int num_clks; > > @@ -96,8 +98,54 @@ struct qcom_adsp { > > struct qcom_rproc_glink glink_subdev; > > struct qcom_rproc_ssr ssr_subdev; > > struct qcom_sysmon *sysmon; > > + > > + int (*shutdown)(struct qcom_adsp *adsp); > > }; > > > > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) > > +{ > > + unsigned long timeout; > > + unsigned int val; > > + int ret; > > + > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > LPASS_HALTREQ_REG, 1); > > + > > + /* Wait for halt ACK from QDSP6 */ > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > + for (;;) { > > + ret = regmap_read(adsp->halt_map, > > + adsp->halt_lpass + LPASS_HALTACK_REG, > &val); > > + if (ret || val || time_after(jiffies, timeout)) > > + break; > > + > > + usleep_range(1000, 1100); > > + } > > + > > + /* Place the WPSS processor into reset */ > > + reset_control_assert(adsp->restart); > > + /* wait after asserting subsystem restart from AOSS */ > > + usleep_range(100, 105); > > + /* Remove the WPSS reset */ > > + reset_control_deassert(adsp->restart); > > + > > + usleep_range(100, 105); > > + > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > LPASS_HALTREQ_REG, 0); > > + > > + /* Wait for halt ACK from QDSP6 */ > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > + for (;;) { > > + ret = regmap_read(adsp->halt_map, > > + adsp->halt_lpass + LPASS_HALTACK_REG, > &val); > > + if (ret || !val || time_after(jiffies, timeout)) > > + break; > > + > > + usleep_range(1000, 1100); > > + } > > + > > + return 0; > > +} > > + > > static int qcom_adsp_shutdown(struct qcom_adsp *adsp) > > { > > unsigned long timeout; > > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) > > if (ret == -ETIMEDOUT) > > dev_err(adsp->dev, "timed out on wait\n"); > > > > - ret = qcom_adsp_shutdown(adsp); > > + ret = adsp->shutdown(adsp); > > if (ret) > > dev_err(adsp->dev, "failed to shutdown: %d\n", ret); > > > > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device > *pdev) > > dev_err(&pdev->dev, "unable to allocate remoteproc\n"); > > return -ENOMEM; > > } > > + > > + rproc->auto_boot = desc->auto_boot; > > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > > > adsp = (struct qcom_adsp *)rproc->priv; > > @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device > *pdev) > > adsp->info_name = desc->sysmon_name; > > platform_set_drvdata(pdev, adsp); > > > > + if (desc->is_wpss) > > + adsp->shutdown = qcom_wpss_shutdown; > > + else > > + adsp->shutdown = qcom_adsp_shutdown; > > + > > ret = adsp_alloc_memory_region(adsp); > > if (ret) > > goto free_rproc; > > @@ -515,6 +570,8 @@ static const struct adsp_pil_data adsp_resource_init > = { > > .ssr_name = "lpass", > > .sysmon_name = "adsp", > > .ssctl_id = 0x14, > > + .is_wpss = false, > > + .auto_boot = true; > > .clk_ids = (const char*[]) { > > "sway_cbcr", "lpass_ahbs_aon_cbcr", > "lpass_ahbm_aon_cbcr", > > "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL > > @@ -528,6 +585,8 @@ static const struct adsp_pil_data cdsp_resource_init > = { > > .ssr_name = "cdsp", > > .sysmon_name = "cdsp", > > .ssctl_id = 0x17, > > + .is_wpss = false, > > + .auto_boot = true; > > .clk_ids = (const char*[]) { > > "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", > "q6ss_master", > > "q6_axim", NULL > > @@ -535,7 +594,23 @@ static const struct adsp_pil_data > cdsp_resource_init = { > > .num_clks = 7, > > }; > > > > +static const struct adsp_pil_data wpss_resource_init = { > > + .crash_reason_smem = 626, > > + .firmware_name = "wpss.mdt", > > + .ssr_name = "wpss", > > + .sysmon_name = "wpss", > > + .ssctl_id = 0x19, > > + .is_wpss = true, > > + .auto_boot = false; > > Why is auto_boot false for the WPSS? Wifi driver will start the remote processor when it comes up. We do not want to load it at the start. > > > + .clk_ids = (const char*[]) { > > + "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", > > + "gcc_wpss_rscp_clk", NULL > > + }, > > + .num_clks = 3, > > +}; > > + > > static const struct of_device_id adsp_of_match[] = { > > + { .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init > }, > > Nit. Please keep things like this sorted alphabetically. Will fix this in the next patchset. Thanks, Rakesh > > Regards, > Bjorn > > > { .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init > }, > > { .compatible = "qcom,sdm845-adsp-pil", .data = > &adsp_resource_init }, > > { }, > > -- > > 2.7.4 > >
On 2021-03-10 12:58, Rakesh Pillai wrote: > Add support for PIL loading of WPSS processor for SC7280 > WPSS boot will be requested by the wifi driver and hence > disable auto-boot for WPSS. Also add a separate shutdown > sequence handler for WPSS. > > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_adsp.c | 77 > ++++++++++++++++++++++++++++++++++++- > 1 file changed, 76 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c > b/drivers/remoteproc/qcom_q6v5_adsp.c > index e024502..dc6b91d 100644 > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > @@ -58,6 +58,8 @@ struct adsp_pil_data { > const char *ssr_name; > const char *sysmon_name; > int ssctl_id; > + bool is_wpss; > + bool auto_boot; > > const char **clk_ids; > int num_clks; > @@ -96,8 +98,54 @@ struct qcom_adsp { > struct qcom_rproc_glink glink_subdev; > struct qcom_rproc_ssr ssr_subdev; > struct qcom_sysmon *sysmon; > + > + int (*shutdown)(struct qcom_adsp *adsp); > }; > > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) > +{ > + unsigned long timeout; > + unsigned int val; > + int ret; > + > + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, > 1); > + > + /* Wait for halt ACK from QDSP6 */ > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > + for (;;) { > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTACK_REG, &val); > + if (ret || val || time_after(jiffies, timeout)) > + break; > + > + usleep_range(1000, 1100); > + } please use regmap_read_poll_timeout instead. > + > + /* Place the WPSS processor into reset */ > + reset_control_assert(adsp->restart); > + /* wait after asserting subsystem restart from AOSS */ > + usleep_range(100, 105); > + /* Remove the WPSS reset */ > + reset_control_deassert(adsp->restart); > + > + usleep_range(100, 105); usleep shouldn't be required since aoss-reset drivers put in a usleep of 200-300 on reset assert and de-assert. > + > + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, > 0); > + > + /* Wait for halt ACK from QDSP6 */ > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > + for (;;) { > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTACK_REG, &val); > + if (ret || !val || time_after(jiffies, timeout)) > + break; > + > + usleep_range(1000, 1100); > + } please use regmap_read_poll_timeout instead. > + > + return 0; > +} > + > static int qcom_adsp_shutdown(struct qcom_adsp *adsp) > { > unsigned long timeout; > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) > if (ret == -ETIMEDOUT) > dev_err(adsp->dev, "timed out on wait\n"); > > - ret = qcom_adsp_shutdown(adsp); > + ret = adsp->shutdown(adsp); > if (ret) > dev_err(adsp->dev, "failed to shutdown: %d\n", ret); > > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "unable to allocate remoteproc\n"); > return -ENOMEM; > } > + > + rproc->auto_boot = desc->auto_boot; > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > adsp = (struct qcom_adsp *)rproc->priv; > @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device > *pdev) > adsp->info_name = desc->sysmon_name; > platform_set_drvdata(pdev, adsp); > > + if (desc->is_wpss) > + adsp->shutdown = qcom_wpss_shutdown; > + else > + adsp->shutdown = qcom_adsp_shutdown; > + > ret = adsp_alloc_memory_region(adsp); > if (ret) > goto free_rproc; > @@ -515,6 +570,8 @@ static const struct adsp_pil_data > adsp_resource_init = { > .ssr_name = "lpass", > .sysmon_name = "adsp", > .ssctl_id = 0x14, > + .is_wpss = false, > + .auto_boot = true; > .clk_ids = (const char*[]) { > "sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr", > "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL > @@ -528,6 +585,8 @@ static const struct adsp_pil_data > cdsp_resource_init = { > .ssr_name = "cdsp", > .sysmon_name = "cdsp", > .ssctl_id = 0x17, > + .is_wpss = false, > + .auto_boot = true; > .clk_ids = (const char*[]) { > "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", "q6ss_master", > "q6_axim", NULL > @@ -535,7 +594,23 @@ static const struct adsp_pil_data > cdsp_resource_init = { > .num_clks = 7, > }; > > +static const struct adsp_pil_data wpss_resource_init = { > + .crash_reason_smem = 626, > + .firmware_name = "wpss.mdt", > + .ssr_name = "wpss", > + .sysmon_name = "wpss", > + .ssctl_id = 0x19, > + .is_wpss = true, > + .auto_boot = false; > + .clk_ids = (const char*[]) { > + "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", > + "gcc_wpss_rscp_clk", NULL > + }, > + .num_clks = 3, > +}; > + > static const struct of_device_id adsp_of_match[] = { > + { .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init > }, should be in sort order. > { .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init > }, > { .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init > }, > { },
On Mon 15 Mar 07:08 CDT 2021, Rakesh Pillai wrote: > > > > -----Original Message----- > > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > Sent: Wednesday, March 10, 2021 10:15 PM > > To: Rakesh Pillai <pillair@codeaurora.org> > > Cc: agross@kernel.org; ohad@wizery.com; mathieu.poirier@linaro.org; > > robh+dt@kernel.org; p.zabel@pengutronix.de; sibis@codeaurora.org; linux- > > arm-msm@vger.kernel.org; linux-remoteproc@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for > > sc7280 WPSS > > > > On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote: > > > > > Add support for PIL loading of WPSS processor for SC7280 > > > WPSS boot will be requested by the wifi driver and hence > > > disable auto-boot for WPSS. Also add a separate shutdown > > > sequence handler for WPSS. > > > > > > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> > > > --- > > > drivers/remoteproc/qcom_q6v5_adsp.c | 77 > > ++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 76 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c > > b/drivers/remoteproc/qcom_q6v5_adsp.c > > > index e024502..dc6b91d 100644 > > > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > > > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > > > @@ -58,6 +58,8 @@ struct adsp_pil_data { > > > const char *ssr_name; > > > const char *sysmon_name; > > > int ssctl_id; > > > + bool is_wpss; > > > + bool auto_boot; > > > > > > const char **clk_ids; > > > int num_clks; > > > @@ -96,8 +98,54 @@ struct qcom_adsp { > > > struct qcom_rproc_glink glink_subdev; > > > struct qcom_rproc_ssr ssr_subdev; > > > struct qcom_sysmon *sysmon; > > > + > > > + int (*shutdown)(struct qcom_adsp *adsp); > > > }; > > > > > > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) > > > +{ > > > + unsigned long timeout; > > > + unsigned int val; > > > + int ret; > > > + > > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > > LPASS_HALTREQ_REG, 1); > > > + > > > + /* Wait for halt ACK from QDSP6 */ > > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > > + for (;;) { > > > + ret = regmap_read(adsp->halt_map, > > > + adsp->halt_lpass + LPASS_HALTACK_REG, > > &val); > > > + if (ret || val || time_after(jiffies, timeout)) > > > + break; > > > + > > > + usleep_range(1000, 1100); > > > + } > > > + > > > + /* Place the WPSS processor into reset */ > > > + reset_control_assert(adsp->restart); > > > + /* wait after asserting subsystem restart from AOSS */ > > > + usleep_range(100, 105); > > > + /* Remove the WPSS reset */ > > > + reset_control_deassert(adsp->restart); > > > + > > > + usleep_range(100, 105); > > > + > > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > > LPASS_HALTREQ_REG, 0); > > > + > > > + /* Wait for halt ACK from QDSP6 */ > > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > > + for (;;) { > > > + ret = regmap_read(adsp->halt_map, > > > + adsp->halt_lpass + LPASS_HALTACK_REG, > > &val); > > > + if (ret || !val || time_after(jiffies, timeout)) > > > + break; > > > + > > > + usleep_range(1000, 1100); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static int qcom_adsp_shutdown(struct qcom_adsp *adsp) > > > { > > > unsigned long timeout; > > > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) > > > if (ret == -ETIMEDOUT) > > > dev_err(adsp->dev, "timed out on wait\n"); > > > > > > - ret = qcom_adsp_shutdown(adsp); > > > + ret = adsp->shutdown(adsp); > > > if (ret) > > > dev_err(adsp->dev, "failed to shutdown: %d\n", ret); > > > > > > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device > > *pdev) > > > dev_err(&pdev->dev, "unable to allocate remoteproc\n"); > > > return -ENOMEM; > > > } > > > + > > > + rproc->auto_boot = desc->auto_boot; > > > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > > > > > adsp = (struct qcom_adsp *)rproc->priv; > > > @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device > > *pdev) > > > adsp->info_name = desc->sysmon_name; > > > platform_set_drvdata(pdev, adsp); > > > > > > + if (desc->is_wpss) > > > + adsp->shutdown = qcom_wpss_shutdown; > > > + else > > > + adsp->shutdown = qcom_adsp_shutdown; > > > + > > > ret = adsp_alloc_memory_region(adsp); > > > if (ret) > > > goto free_rproc; > > > @@ -515,6 +570,8 @@ static const struct adsp_pil_data adsp_resource_init > > = { > > > .ssr_name = "lpass", > > > .sysmon_name = "adsp", > > > .ssctl_id = 0x14, > > > + .is_wpss = false, > > > + .auto_boot = true; > > > .clk_ids = (const char*[]) { > > > "sway_cbcr", "lpass_ahbs_aon_cbcr", > > "lpass_ahbm_aon_cbcr", > > > "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL > > > @@ -528,6 +585,8 @@ static const struct adsp_pil_data cdsp_resource_init > > = { > > > .ssr_name = "cdsp", > > > .sysmon_name = "cdsp", > > > .ssctl_id = 0x17, > > > + .is_wpss = false, > > > + .auto_boot = true; > > > .clk_ids = (const char*[]) { > > > "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", > > "q6ss_master", > > > "q6_axim", NULL > > > @@ -535,7 +594,23 @@ static const struct adsp_pil_data > > cdsp_resource_init = { > > > .num_clks = 7, > > > }; > > > > > > +static const struct adsp_pil_data wpss_resource_init = { > > > + .crash_reason_smem = 626, > > > + .firmware_name = "wpss.mdt", > > > + .ssr_name = "wpss", > > > + .sysmon_name = "wpss", > > > + .ssctl_id = 0x19, > > > + .is_wpss = true, > > > + .auto_boot = false; > > > > Why is auto_boot false for the WPSS? > > Wifi driver will start the remote processor when it comes up. We do not want > to load it at the start. > Can you please explain this further? We've had several cases in the past where functional drivers controls a remoteproc instance and makes assumptions about when the remoteproc is up or not. I would like to ensure that we don't design ourselves into such corner (even though I see that the ath11k code for this was merged a long time ago) Regards, Bjorn > > > > > + .clk_ids = (const char*[]) { > > > + "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", > > > + "gcc_wpss_rscp_clk", NULL > > > + }, > > > + .num_clks = 3, > > > +}; > > > + > > > static const struct of_device_id adsp_of_match[] = { > > > + { .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init > > }, > > > > Nit. Please keep things like this sorted alphabetically. > > Will fix this in the next patchset. > > Thanks, > Rakesh > > > > > Regards, > > Bjorn > > > > > { .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init > > }, > > > { .compatible = "qcom,sdm845-adsp-pil", .data = > > &adsp_resource_init }, > > > { }, > > > -- > > > 2.7.4 > > > >
> -----Original Message----- > From: Bjorn Andersson <bjorn.andersson@linaro.org> > Sent: Monday, October 4, 2021 8:59 PM > To: Rakesh Pillai <pillair@codeaurora.org> > Cc: agross@kernel.org; ohad@wizery.com; mathieu.poirier@linaro.org; > robh+dt@kernel.org; p.zabel@pengutronix.de; sibis@codeaurora.org; linux- > arm-msm@vger.kernel.org; linux-remoteproc@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for > sc7280 WPSS > > On Mon 15 Mar 07:08 CDT 2021, Rakesh Pillai wrote: > > > > > > > > -----Original Message----- > > > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > > Sent: Wednesday, March 10, 2021 10:15 PM > > > To: Rakesh Pillai <pillair@codeaurora.org> > > > Cc: agross@kernel.org; ohad@wizery.com; mathieu.poirier@linaro.org; > > > robh+dt@kernel.org; p.zabel@pengutronix.de; sibis@codeaurora.org; > > > robh+linux- > > > arm-msm@vger.kernel.org; linux-remoteproc@vger.kernel.org; > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support > > > for > > > sc7280 WPSS > > > > > > On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote: > > > > > > > Add support for PIL loading of WPSS processor for SC7280 WPSS boot > > > > will be requested by the wifi driver and hence disable auto-boot > > > > for WPSS. Also add a separate shutdown sequence handler for WPSS. > > > > > > > > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> > > > > --- > > > > drivers/remoteproc/qcom_q6v5_adsp.c | 77 > > > ++++++++++++++++++++++++++++++++++++- > > > > 1 file changed, 76 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c > > > b/drivers/remoteproc/qcom_q6v5_adsp.c > > > > index e024502..dc6b91d 100644 > > > > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > > > > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > > > > @@ -58,6 +58,8 @@ struct adsp_pil_data { > > > > const char *ssr_name; > > > > const char *sysmon_name; > > > > int ssctl_id; > > > > + bool is_wpss; > > > > + bool auto_boot; > > > > > > > > const char **clk_ids; > > > > int num_clks; > > > > @@ -96,8 +98,54 @@ struct qcom_adsp { > > > > struct qcom_rproc_glink glink_subdev; > > > > struct qcom_rproc_ssr ssr_subdev; > > > > struct qcom_sysmon *sysmon; > > > > + > > > > + int (*shutdown)(struct qcom_adsp *adsp); > > > > }; > > > > > > > > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) { > > > > + unsigned long timeout; > > > > + unsigned int val; > > > > + int ret; > > > > + > > > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > > > LPASS_HALTREQ_REG, 1); > > > > + > > > > + /* Wait for halt ACK from QDSP6 */ > > > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > > > + for (;;) { > > > > + ret = regmap_read(adsp->halt_map, > > > > + adsp->halt_lpass + LPASS_HALTACK_REG, > > > &val); > > > > + if (ret || val || time_after(jiffies, timeout)) > > > > + break; > > > > + > > > > + usleep_range(1000, 1100); > > > > + } > > > > + > > > > + /* Place the WPSS processor into reset */ > > > > + reset_control_assert(adsp->restart); > > > > + /* wait after asserting subsystem restart from AOSS */ > > > > + usleep_range(100, 105); > > > > + /* Remove the WPSS reset */ > > > > + reset_control_deassert(adsp->restart); > > > > + > > > > + usleep_range(100, 105); > > > > + > > > > + regmap_write(adsp->halt_map, adsp->halt_lpass + > > > LPASS_HALTREQ_REG, 0); > > > > + > > > > + /* Wait for halt ACK from QDSP6 */ > > > > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > > > > + for (;;) { > > > > + ret = regmap_read(adsp->halt_map, > > > > + adsp->halt_lpass + LPASS_HALTACK_REG, > > > &val); > > > > + if (ret || !val || time_after(jiffies, timeout)) > > > > + break; > > > > + > > > > + usleep_range(1000, 1100); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static int qcom_adsp_shutdown(struct qcom_adsp *adsp) { > > > > unsigned long timeout; > > > > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) > > > > if (ret == -ETIMEDOUT) > > > > dev_err(adsp->dev, "timed out on wait\n"); > > > > > > > > - ret = qcom_adsp_shutdown(adsp); > > > > + ret = adsp->shutdown(adsp); > > > > if (ret) > > > > dev_err(adsp->dev, "failed to shutdown: %d\n", ret); > > > > > > > > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device > > > *pdev) > > > > dev_err(&pdev->dev, "unable to allocate remoteproc\n"); > > > > return -ENOMEM; > > > > } > > > > + > > > > + rproc->auto_boot = desc->auto_boot; > > > > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > > > > > > > adsp = (struct qcom_adsp *)rproc->priv; @@ -447,6 +497,11 @@ > > > > static int adsp_probe(struct platform_device > > > *pdev) > > > > adsp->info_name = desc->sysmon_name; > > > > platform_set_drvdata(pdev, adsp); > > > > > > > > + if (desc->is_wpss) > > > > + adsp->shutdown = qcom_wpss_shutdown; > > > > + else > > > > + adsp->shutdown = qcom_adsp_shutdown; > > > > + > > > > ret = adsp_alloc_memory_region(adsp); > > > > if (ret) > > > > goto free_rproc; > > > > @@ -515,6 +570,8 @@ static const struct adsp_pil_data > > > > adsp_resource_init > > > = { > > > > .ssr_name = "lpass", > > > > .sysmon_name = "adsp", > > > > .ssctl_id = 0x14, > > > > + .is_wpss = false, > > > > + .auto_boot = true; > > > > .clk_ids = (const char*[]) { > > > > "sway_cbcr", "lpass_ahbs_aon_cbcr", > > > "lpass_ahbm_aon_cbcr", > > > > "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL @@ - > 528,6 > > > > +585,8 @@ static const struct adsp_pil_data cdsp_resource_init > > > = { > > > > .ssr_name = "cdsp", > > > > .sysmon_name = "cdsp", > > > > .ssctl_id = 0x17, > > > > + .is_wpss = false, > > > > + .auto_boot = true; > > > > .clk_ids = (const char*[]) { > > > > "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", > > > "q6ss_master", > > > > "q6_axim", NULL > > > > @@ -535,7 +594,23 @@ static const struct adsp_pil_data > > > cdsp_resource_init = { > > > > .num_clks = 7, > > > > }; > > > > > > > > +static const struct adsp_pil_data wpss_resource_init = { > > > > + .crash_reason_smem = 626, > > > > + .firmware_name = "wpss.mdt", > > > > + .ssr_name = "wpss", > > > > + .sysmon_name = "wpss", > > > > + .ssctl_id = 0x19, > > > > + .is_wpss = true, > > > > + .auto_boot = false; > > > > > > Why is auto_boot false for the WPSS? > > > > Wifi driver will start the remote processor when it comes up. We do > > not want to load it at the start. > > > > Can you please explain this further? > > We've had several cases in the past where functional drivers controls a > remoteproc instance and makes assumptions about when the remoteproc is > up or not. I would like to ensure that we don't design ourselves into such > corner (even though I see that the ath11k code for this was merged a long > time ago) > > Regards, > Bjorn > Hi Bjorn, Yes, the wpss remoteproc is used by ath11k, where it takes care of starting the rproc during init. Ideally the wpss is not supposed to be started until the wifi driver comes up. If wifi is started/enabled, the wifi driver can take care of starting the wpss. This is the reason behind keeping auto_boot as false for wpss. Thanks, Rakesh Pillai > > > > 2.7.4 > > > > > >
diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c index e024502..dc6b91d 100644 --- a/drivers/remoteproc/qcom_q6v5_adsp.c +++ b/drivers/remoteproc/qcom_q6v5_adsp.c @@ -58,6 +58,8 @@ struct adsp_pil_data { const char *ssr_name; const char *sysmon_name; int ssctl_id; + bool is_wpss; + bool auto_boot; const char **clk_ids; int num_clks; @@ -96,8 +98,54 @@ struct qcom_adsp { struct qcom_rproc_glink glink_subdev; struct qcom_rproc_ssr ssr_subdev; struct qcom_sysmon *sysmon; + + int (*shutdown)(struct qcom_adsp *adsp); }; +static int qcom_wpss_shutdown(struct qcom_adsp *adsp) +{ + unsigned long timeout; + unsigned int val; + int ret; + + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 1); + + /* Wait for halt ACK from QDSP6 */ + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); + for (;;) { + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_HALTACK_REG, &val); + if (ret || val || time_after(jiffies, timeout)) + break; + + usleep_range(1000, 1100); + } + + /* Place the WPSS processor into reset */ + reset_control_assert(adsp->restart); + /* wait after asserting subsystem restart from AOSS */ + usleep_range(100, 105); + /* Remove the WPSS reset */ + reset_control_deassert(adsp->restart); + + usleep_range(100, 105); + + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 0); + + /* Wait for halt ACK from QDSP6 */ + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); + for (;;) { + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_HALTACK_REG, &val); + if (ret || !val || time_after(jiffies, timeout)) + break; + + usleep_range(1000, 1100); + } + + return 0; +} + static int qcom_adsp_shutdown(struct qcom_adsp *adsp) { unsigned long timeout; @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc) if (ret == -ETIMEDOUT) dev_err(adsp->dev, "timed out on wait\n"); - ret = qcom_adsp_shutdown(adsp); + ret = adsp->shutdown(adsp); if (ret) dev_err(adsp->dev, "failed to shutdown: %d\n", ret); @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device *pdev) dev_err(&pdev->dev, "unable to allocate remoteproc\n"); return -ENOMEM; } + + rproc->auto_boot = desc->auto_boot; rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); adsp = (struct qcom_adsp *)rproc->priv; @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device *pdev) adsp->info_name = desc->sysmon_name; platform_set_drvdata(pdev, adsp); + if (desc->is_wpss) + adsp->shutdown = qcom_wpss_shutdown; + else + adsp->shutdown = qcom_adsp_shutdown; + ret = adsp_alloc_memory_region(adsp); if (ret) goto free_rproc; @@ -515,6 +570,8 @@ static const struct adsp_pil_data adsp_resource_init = { .ssr_name = "lpass", .sysmon_name = "adsp", .ssctl_id = 0x14, + .is_wpss = false, + .auto_boot = true; .clk_ids = (const char*[]) { "sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL @@ -528,6 +585,8 @@ static const struct adsp_pil_data cdsp_resource_init = { .ssr_name = "cdsp", .sysmon_name = "cdsp", .ssctl_id = 0x17, + .is_wpss = false, + .auto_boot = true; .clk_ids = (const char*[]) { "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", "q6ss_master", "q6_axim", NULL @@ -535,7 +594,23 @@ static const struct adsp_pil_data cdsp_resource_init = { .num_clks = 7, }; +static const struct adsp_pil_data wpss_resource_init = { + .crash_reason_smem = 626, + .firmware_name = "wpss.mdt", + .ssr_name = "wpss", + .sysmon_name = "wpss", + .ssctl_id = 0x19, + .is_wpss = true, + .auto_boot = false; + .clk_ids = (const char*[]) { + "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", + "gcc_wpss_rscp_clk", NULL + }, + .num_clks = 3, +}; + static const struct of_device_id adsp_of_match[] = { + { .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init }, { .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init }, { .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init }, { },
Add support for PIL loading of WPSS processor for SC7280 WPSS boot will be requested by the wifi driver and hence disable auto-boot for WPSS. Also add a separate shutdown sequence handler for WPSS. Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> --- drivers/remoteproc/qcom_q6v5_adsp.c | 77 ++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)