All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add support for sc7280 WPSS PIL loading
@ 2021-03-10  7:28 Rakesh Pillai
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Rakesh Pillai @ 2021-03-10  7:28 UTC (permalink / raw)
  To: agross, bjorn.andersson, ohad, mathieu.poirier, robh+dt, p.zabel
  Cc: sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
	Rakesh Pillai

Add support for PIL loading of WPSS co-processor for SC7280 SOCs.

Rakesh Pillai (2):
  dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS

 .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 +++++-----
 drivers/remoteproc/qcom_q6v5_adsp.c                | 77 +++++++++++++++++++++-
 2 files changed, 96 insertions(+), 16 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  2021-03-10  7:28 [PATCH 0/2] Add support for sc7280 WPSS PIL loading Rakesh Pillai
@ 2021-03-10  7:28 ` Rakesh Pillai
  2021-03-10 16:47   ` Bjorn Andersson
                     ` (3 more replies)
  2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
  2021-07-22 22:12 ` [PATCH 0/2] Add support for sc7280 WPSS PIL loading Stephen Boyd
  2 siblings, 4 replies; 15+ messages in thread
From: Rakesh Pillai @ 2021-03-10  7:28 UTC (permalink / raw)
  To: agross, bjorn.andersson, ohad, mathieu.poirier, robh+dt, p.zabel
  Cc: sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
	Rakesh Pillai

Add WPSS PIL loading support for SC7280 SoCs.

Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 ++++++++++++----------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
index 1337a3d..edad5e8 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
@@ -9,6 +9,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
 	Definition: must be one of:
 		    "qcom,qcs404-cdsp-pil",
 		    "qcom,sdm845-adsp-pil"
+		    "qcom,sc7280-wpss-pil"
 
 - reg:
 	Usage: required
@@ -24,7 +25,13 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
 - interrupt-names:
 	Usage: required
 	Value type: <stringlist>
-	Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
+	Definition: The interrupts needed depends on the compatible string
+	qcom,sdm845-adsp-pil:
+	qcom,qcs404-cdsp-pil:
+		must be "wdog", "fatal", "ready", "handover", "stop-ack"
+	qcom,sc7280-wpss-pil:
+		must be "wdog", "fatal", "ready", "handover", "stop-ack"
+		"shutdown-ack"
 
 - clocks:
 	Usage: required
@@ -35,19 +42,17 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
 - clock-names:
 	Usage: required for SDM845 ADSP
 	Value type: <stringlist>
-	Definition: List of clock input name strings sorted in the same
-		    order as the clocks property. Definition must have
-		    "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
-		    "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
-		    and "qdsp6ss_core".
-
-- clock-names:
-	Usage: required for QCS404 CDSP
-	Value type: <stringlist>
-	Definition: List of clock input name strings sorted in the same
-		    order as the clocks property. Definition must have
-		    "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
-		    "q6ss_master", "q6_axim".
+	Definition: The clocks needed depends on the compatible string
+	qcom,sdm845-adsp-pil:
+		must be "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
+		"lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep",
+		"qdsp6ss_core"
+	qcom,qcs404-cdsp-pil:
+		must be "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
+		"q6ss_master", "q6_axim"
+	qcom,sc7280-wpss-pil:
+		must be "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk",
+		"gcc_wpss_rscp_clk"
 
 - power-domains:
 	Usage: required
@@ -65,7 +70,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
         Definition: must be "pdc_sync" and "cc_lpass"
 
 - reset-names:
-        Usage: required for QCS404 CDSP
+        Usage: required for QCS404 CDSP, SC7280 WPSS
         Value type: <stringlist>
         Definition: must be "restart"
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-10  7:28 [PATCH 0/2] Add support for sc7280 WPSS PIL loading Rakesh Pillai
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
@ 2021-03-10  7:28 ` Rakesh Pillai
  2021-03-10 16:44   ` Bjorn Andersson
                     ` (2 more replies)
  2021-07-22 22:12 ` [PATCH 0/2] Add support for sc7280 WPSS PIL loading Stephen Boyd
  2 siblings, 3 replies; 15+ messages in thread
From: Rakesh Pillai @ 2021-03-10  7:28 UTC (permalink / raw)
  To: agross, bjorn.andersson, ohad, mathieu.poirier, robh+dt, p.zabel
  Cc: sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
	Rakesh Pillai

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;
+	.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 },
 	{ },
-- 
2.7.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
@ 2021-03-10 16:44   ` Bjorn Andersson
  2021-03-15 12:08     ` Rakesh Pillai
  2021-03-10 19:12   ` kernel test robot
  2021-03-15 13:49   ` Sibi Sankar
  2 siblings, 1 reply; 15+ messages in thread
From: Bjorn Andersson @ 2021-03-10 16:44 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: agross, ohad, mathieu.poirier, robh+dt, p.zabel, sibis,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
@ 2021-03-10 16:47   ` Bjorn Andersson
  2021-03-15 13:38   ` Sibi Sankar
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Bjorn Andersson @ 2021-03-10 16:47 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: agross, ohad, mathieu.poirier, robh+dt, p.zabel, sibis,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote:

> Add WPSS PIL loading support for SC7280 SoCs.
> 

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

But can you please follow up with a patch that converts this to yaml?

Regards,
Bjorn

> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>  .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 ++++++++++++----------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> index 1337a3d..edad5e8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> @@ -9,6 +9,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  	Definition: must be one of:
>  		    "qcom,qcs404-cdsp-pil",
>  		    "qcom,sdm845-adsp-pil"
> +		    "qcom,sc7280-wpss-pil"
>  
>  - reg:
>  	Usage: required
> @@ -24,7 +25,13 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - interrupt-names:
>  	Usage: required
>  	Value type: <stringlist>
> -	Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +	Definition: The interrupts needed depends on the compatible string
> +	qcom,sdm845-adsp-pil:
> +	qcom,qcs404-cdsp-pil:
> +		must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +	qcom,sc7280-wpss-pil:
> +		must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +		"shutdown-ack"
>  
>  - clocks:
>  	Usage: required
> @@ -35,19 +42,17 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - clock-names:
>  	Usage: required for SDM845 ADSP
>  	Value type: <stringlist>
> -	Definition: List of clock input name strings sorted in the same
> -		    order as the clocks property. Definition must have
> -		    "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> -		    "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
> -		    and "qdsp6ss_core".
> -
> -- clock-names:
> -	Usage: required for QCS404 CDSP
> -	Value type: <stringlist>
> -	Definition: List of clock input name strings sorted in the same
> -		    order as the clocks property. Definition must have
> -		    "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> -		    "q6ss_master", "q6_axim".
> +	Definition: The clocks needed depends on the compatible string
> +	qcom,sdm845-adsp-pil:
> +		must be "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> +		"lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep",
> +		"qdsp6ss_core"
> +	qcom,qcs404-cdsp-pil:
> +		must be "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> +		"q6ss_master", "q6_axim"
> +	qcom,sc7280-wpss-pil:
> +		must be "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk",
> +		"gcc_wpss_rscp_clk"
>  
>  - power-domains:
>  	Usage: required
> @@ -65,7 +70,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>          Definition: must be "pdc_sync" and "cc_lpass"
>  
>  - reset-names:
> -        Usage: required for QCS404 CDSP
> +        Usage: required for QCS404 CDSP, SC7280 WPSS
>          Value type: <stringlist>
>          Definition: must be "restart"
>  
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
  2021-03-10 16:44   ` Bjorn Andersson
@ 2021-03-10 19:12   ` kernel test robot
  2021-03-15 13:49   ` Sibi Sankar
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-03-10 19:12 UTC (permalink / raw)
  To: Rakesh Pillai, agross, bjorn.andersson, ohad, mathieu.poirier,
	robh+dt, p.zabel
  Cc: kbuild-all, sibis, linux-arm-msm, linux-remoteproc, devicetree

[-- Attachment #1: Type: text/plain, Size: 3235 bytes --]

Hi Rakesh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v5.12-rc2 next-20210310]
[cannot apply to hwspinlock/for-next remoteproc/for-next rpmsg/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Rakesh-Pillai/Add-support-for-sc7280-WPSS-PIL-loading/20210310-153802
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/870fc6e5ff6910ff2f19d07840e300f66f1fd668
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Rakesh-Pillai/Add-support-for-sc7280-WPSS-PIL-loading/20210310-153802
        git checkout 870fc6e5ff6910ff2f19d07840e300f66f1fd668
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/remoteproc/qcom_q6v5_adsp.c:574:19: error: expected '}' before ';' token
     574 |  .auto_boot = true;
         |                   ^
   drivers/remoteproc/qcom_q6v5_adsp.c:567:56: note: to match this '{'
     567 | static const struct adsp_pil_data adsp_resource_init = {
         |                                                        ^
   drivers/remoteproc/qcom_q6v5_adsp.c:589:19: error: expected '}' before ';' token
     589 |  .auto_boot = true;
         |                   ^
   drivers/remoteproc/qcom_q6v5_adsp.c:582:56: note: to match this '{'
     582 | static const struct adsp_pil_data cdsp_resource_init = {
         |                                                        ^
   drivers/remoteproc/qcom_q6v5_adsp.c:604:20: error: expected '}' before ';' token
     604 |  .auto_boot = false;
         |                    ^
   drivers/remoteproc/qcom_q6v5_adsp.c:597:56: note: to match this '{'
     597 | static const struct adsp_pil_data wpss_resource_init = {
         |                                                        ^


vim +574 drivers/remoteproc/qcom_q6v5_adsp.c

   566	
   567	static const struct adsp_pil_data adsp_resource_init = {
   568		.crash_reason_smem = 423,
   569		.firmware_name = "adsp.mdt",
   570		.ssr_name = "lpass",
   571		.sysmon_name = "adsp",
   572		.ssctl_id = 0x14,
   573		.is_wpss = false,
 > 574		.auto_boot = true;
   575		.clk_ids = (const char*[]) {
   576			"sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
   577			"qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL
   578		},
   579		.num_clks = 7,
   580	};
   581	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 76927 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-10 16:44   ` Bjorn Andersson
@ 2021-03-15 12:08     ` Rakesh Pillai
  2021-10-04 15:29       ` Bjorn Andersson
  0 siblings, 1 reply; 15+ messages in thread
From: Rakesh Pillai @ 2021-03-15 12:08 UTC (permalink / raw)
  To: 'Bjorn Andersson'
  Cc: agross, ohad, mathieu.poirier, robh+dt, p.zabel, sibis,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel



> -----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
> >


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
  2021-03-10 16:47   ` Bjorn Andersson
@ 2021-03-15 13:38   ` Sibi Sankar
  2021-03-16 22:33   ` Rob Herring
  2021-07-22 22:16   ` Stephen Boyd
  3 siblings, 0 replies; 15+ messages in thread
From: Sibi Sankar @ 2021-03-15 13:38 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: agross, bjorn.andersson, ohad, mathieu.poirier, robh+dt, p.zabel,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

On 2021-03-10 12:58, Rakesh Pillai wrote:
> Add WPSS PIL loading support for SC7280 SoCs.
> 
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>  .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 
> ++++++++++++----------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> index 1337a3d..edad5e8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> @@ -9,6 +9,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  	Definition: must be one of:
>  		    "qcom,qcs404-cdsp-pil",
>  		    "qcom,sdm845-adsp-pil"
> +		    "qcom,sc7280-wpss-pil"
> 
>  - reg:
>  	Usage: required
> @@ -24,7 +25,13 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - interrupt-names:
>  	Usage: required
>  	Value type: <stringlist>
> -	Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +	Definition: The interrupts needed depends on the compatible string
> +	qcom,sdm845-adsp-pil:
> +	qcom,qcs404-cdsp-pil:
> +		must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +	qcom,sc7280-wpss-pil:
> +		must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +		"shutdown-ack"
> 
>  - clocks:
>  	Usage: required
> @@ -35,19 +42,17 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - clock-names:
>  	Usage: required for SDM845 ADSP
>  	Value type: <stringlist>
> -	Definition: List of clock input name strings sorted in the same
> -		    order as the clocks property. Definition must have
> -		    "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> -		    "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
> -		    and "qdsp6ss_core".
> -
> -- clock-names:
> -	Usage: required for QCS404 CDSP
> -	Value type: <stringlist>
> -	Definition: List of clock input name strings sorted in the same
> -		    order as the clocks property. Definition must have
> -		    "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> -		    "q6ss_master", "q6_axim".
> +	Definition: The clocks needed depends on the compatible string
> +	qcom,sdm845-adsp-pil:
> +		must be "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> +		"lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep",
> +		"qdsp6ss_core"
> +	qcom,qcs404-cdsp-pil:
> +		must be "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> +		"q6ss_master", "q6_axim"
> +	qcom,sc7280-wpss-pil:
> +		must be "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk",
> +		"gcc_wpss_rscp_clk"
> 
>  - power-domains:

IIRC wpss requires both cx and mx. So you'll
need to add driver code to support multiple
power-domains.

>  	Usage: required
> @@ -65,7 +70,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>          Definition: must be "pdc_sync" and "cc_lpass"
> 
>  - reset-names:
> -        Usage: required for QCS404 CDSP
> +        Usage: required for QCS404 CDSP, SC7280 WPSS
>          Value type: <stringlist>
>          Definition: must be "restart"

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
  2021-03-10 16:44   ` Bjorn Andersson
  2021-03-10 19:12   ` kernel test robot
@ 2021-03-15 13:49   ` Sibi Sankar
  2 siblings, 0 replies; 15+ messages in thread
From: Sibi Sankar @ 2021-03-15 13:49 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: agross, bjorn.andersson, ohad, mathieu.poirier, robh+dt, p.zabel,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
	pillair=codeaurora.org

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 
> },
>  	{ },

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
  2021-03-10 16:47   ` Bjorn Andersson
  2021-03-15 13:38   ` Sibi Sankar
@ 2021-03-16 22:33   ` Rob Herring
  2021-07-22 22:16   ` Stephen Boyd
  3 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2021-03-16 22:33 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: p.zabel, linux-remoteproc, bjorn.andersson, sibis,
	mathieu.poirier, ohad, linux-arm-msm, agross, robh+dt,
	devicetree, linux-kernel

On Wed, 10 Mar 2021 12:58:09 +0530, Rakesh Pillai wrote:
> Add WPSS PIL loading support for SC7280 SoCs.
> 
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>  .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 ++++++++++++----------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add support for sc7280 WPSS PIL loading
  2021-03-10  7:28 [PATCH 0/2] Add support for sc7280 WPSS PIL loading Rakesh Pillai
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
  2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
@ 2021-07-22 22:12 ` Stephen Boyd
  2021-08-10 18:04   ` Rakesh Pillai
  2 siblings, 1 reply; 15+ messages in thread
From: Stephen Boyd @ 2021-07-22 22:12 UTC (permalink / raw)
  To: Rakesh Pillai, agross, bjorn.andersson, mathieu.poirier, ohad,
	p.zabel, robh+dt
  Cc: sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
	Rakesh Pillai

Quoting Rakesh Pillai (2021-03-09 23:28:08)
> Add support for PIL loading of WPSS co-processor for SC7280 SOCs.
> 
> Rakesh Pillai (2):
>   dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
>   remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
> 

Is this patch series going to be resent?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
  2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
                     ` (2 preceding siblings ...)
  2021-03-16 22:33   ` Rob Herring
@ 2021-07-22 22:16   ` Stephen Boyd
  3 siblings, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2021-07-22 22:16 UTC (permalink / raw)
  To: Rakesh Pillai, agross, bjorn.andersson, mathieu.poirier, ohad,
	p.zabel, robh+dt
  Cc: sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

Quoting Rakesh Pillai (2021-03-09 23:28:09)
> Add WPSS PIL loading support for SC7280 SoCs.
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>  .../bindings/remoteproc/qcom,hexagon-v56.txt       | 35 ++++++++++++----------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> index 1337a3d..edad5e8 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt
> @@ -9,6 +9,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>         Definition: must be one of:
>                     "qcom,qcs404-cdsp-pil",
>                     "qcom,sdm845-adsp-pil"
> +                   "qcom,sc7280-wpss-pil"

alphabet sort please.

>
>  - reg:
>         Usage: required
> @@ -24,7 +25,13 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - interrupt-names:
>         Usage: required
>         Value type: <stringlist>
> -       Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +       Definition: The interrupts needed depends on the compatible string
> +       qcom,sdm845-adsp-pil:
> +       qcom,qcs404-cdsp-pil:
> +               must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +       qcom,sc7280-wpss-pil:

Alphabet sort too?

> +               must be "wdog", "fatal", "ready", "handover", "stop-ack"
> +               "shutdown-ack"
>
>  - clocks:
>         Usage: required
> @@ -35,19 +42,17 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>  - clock-names:
>         Usage: required for SDM845 ADSP
>         Value type: <stringlist>
> -       Definition: List of clock input name strings sorted in the same
> -                   order as the clocks property. Definition must have
> -                   "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> -                   "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
> -                   and "qdsp6ss_core".
> -
> -- clock-names:
> -       Usage: required for QCS404 CDSP
> -       Value type: <stringlist>
> -       Definition: List of clock input name strings sorted in the same
> -                   order as the clocks property. Definition must have
> -                   "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> -                   "q6ss_master", "q6_axim".
> +       Definition: The clocks needed depends on the compatible string
> +       qcom,sdm845-adsp-pil:
> +               must be "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
> +               "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep",
> +               "qdsp6ss_core"
> +       qcom,qcs404-cdsp-pil:
> +               must be "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave",
> +               "q6ss_master", "q6_axim"
> +       qcom,sc7280-wpss-pil:

Alphabet sort too?

> +               must be "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk",
> +               "gcc_wpss_rscp_clk"
>
>  - power-domains:
>         Usage: required
> @@ -65,7 +70,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core.
>          Definition: must be "pdc_sync" and "cc_lpass"
>
>  - reset-names:
> -        Usage: required for QCS404 CDSP
> +        Usage: required for QCS404 CDSP, SC7280 WPSS
>          Value type: <stringlist>
>          Definition: must be "restart"
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add support for sc7280 WPSS PIL loading
  2021-07-22 22:12 ` [PATCH 0/2] Add support for sc7280 WPSS PIL loading Stephen Boyd
@ 2021-08-10 18:04   ` Rakesh Pillai
  0 siblings, 0 replies; 15+ messages in thread
From: Rakesh Pillai @ 2021-08-10 18:04 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: agross, bjorn.andersson, mathieu.poirier, ohad, p.zabel, robh+dt,
	sibis, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

On 2021-07-23 03:42, Stephen Boyd wrote:
> Quoting Rakesh Pillai (2021-03-09 23:28:08)
>> Add support for PIL loading of WPSS co-processor for SC7280 SOCs.
>> 
>> Rakesh Pillai (2):
>>   dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
>>   remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
>> 
> 
> Is this patch series going to be resent?

Hi Stephen,
I posted a v2 for this patch series, with the dt-bindings converted to 
yaml.

Thanks,
Rakesh Pillai.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-03-15 12:08     ` Rakesh Pillai
@ 2021-10-04 15:29       ` Bjorn Andersson
  2021-10-28  7:43         ` pillair
  0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Andersson @ 2021-10-04 15:29 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: agross, ohad, mathieu.poirier, robh+dt, p.zabel, sibis,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel

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
> > >
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
  2021-10-04 15:29       ` Bjorn Andersson
@ 2021-10-28  7:43         ` pillair
  0 siblings, 0 replies; 15+ messages in thread
From: pillair @ 2021-10-28  7:43 UTC (permalink / raw)
  To: 'Bjorn Andersson'
  Cc: agross, ohad, mathieu.poirier, robh+dt, p.zabel, sibis,
	linux-arm-msm, linux-remoteproc, devicetree, linux-kernel



> -----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
> > > >
> >


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-10-28  7:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10  7:28 [PATCH 0/2] Add support for sc7280 WPSS PIL loading Rakesh Pillai
2021-03-10  7:28 ` [PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support Rakesh Pillai
2021-03-10 16:47   ` Bjorn Andersson
2021-03-15 13:38   ` Sibi Sankar
2021-03-16 22:33   ` Rob Herring
2021-07-22 22:16   ` Stephen Boyd
2021-03-10  7:28 ` [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS Rakesh Pillai
2021-03-10 16:44   ` Bjorn Andersson
2021-03-15 12:08     ` Rakesh Pillai
2021-10-04 15:29       ` Bjorn Andersson
2021-10-28  7:43         ` pillair
2021-03-10 19:12   ` kernel test robot
2021-03-15 13:49   ` Sibi Sankar
2021-07-22 22:12 ` [PATCH 0/2] Add support for sc7280 WPSS PIL loading Stephen Boyd
2021-08-10 18:04   ` Rakesh Pillai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.