linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [V3 0/7] Miscellaneous PAS fixes
@ 2022-07-05 12:08 Sibi Sankar
  2022-07-05 12:08 ` [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem Sibi Sankar
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Sibi Sankar

A collection of misc. fixes for the remoteproc PAS and sysmon driver.

V3:
 * Fixup misc. suggestions and rename to adsp_shutdown_poll_decrypt() [Bjorn]
 * Pick up another lone sysmon fix from the list with R-b.

V2:
 * Add misc. sysmon fix.
 * Dropping patch 1 and 6 from V1.

Sibi Sankar (2):
  remoteproc: qcom: pas: Add decrypt shutdown support for modem
  remoteproc: sysmon: Wait for SSCTL service to come up

Siddharth Gupta (5):
  remoteproc: qcom: pas: Mark va as io memory
  remoteproc: qcom: pas: Mark devices as wakeup capable
  remoteproc: qcom: pas: Check if coredump is enabled
  remoteproc: q6v5: Set q6 state to offline on receiving wdog irq
  remoteproc: sysmon: Send sysmon state only for running rprocs

 drivers/remoteproc/qcom_q6v5.c     |  4 +++
 drivers/remoteproc/qcom_q6v5_pas.c | 53 ++++++++++++++++++++++++++++++++++++--
 drivers/remoteproc/qcom_sysmon.c   | 16 ++++++++++--
 3 files changed, 69 insertions(+), 4 deletions(-)

-- 
2.7.4


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

* [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-06 12:38   ` Konrad Dybcio
  2022-07-05 12:08 ` [V3 2/7] remoteproc: qcom: pas: Mark va as io memory Sibi Sankar
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Sibi Sankar

The initial shutdown request to modem on SM8450 SoCs would start the
decryption process and will keep returning errors until the modem shutdown
is complete. Fix this by retrying shutdowns in fixed intervals.

Err Logs on modem shutdown:
qcom_q6v5_pas 4080000.remoteproc: failed to shutdown: -22
remoteproc remoteproc3: can't stop rproc: -22

Fixes: 5cef9b48458d ("remoteproc: qcom: pas: Add SM8450 remoteproc support")
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---

v3:
 * Fixup misc. suggestions and rename to adsp_shutdown_poll_decrypt() [Bjorn]

 drivers/remoteproc/qcom_q6v5_pas.c | 43 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 6ae39c5653b1..297700f87fe8 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -29,6 +30,8 @@
 #include "qcom_q6v5.h"
 #include "remoteproc_internal.h"
 
+#define ADSP_DECRYPT_SHUTDOWN_DELAY_MS	100
+
 struct adsp_data {
 	int crash_reason_smem;
 	const char *firmware_name;
@@ -36,6 +39,7 @@ struct adsp_data {
 	unsigned int minidump_id;
 	bool has_aggre2_clk;
 	bool auto_boot;
+	bool decrypt_shutdown;
 
 	char **proxy_pd_names;
 
@@ -65,6 +69,7 @@ struct qcom_adsp {
 	unsigned int minidump_id;
 	int crash_reason_smem;
 	bool has_aggre2_clk;
+	bool decrypt_shutdown;
 	const char *info_name;
 
 	struct completion start_done;
@@ -128,6 +133,19 @@ static void adsp_pds_disable(struct qcom_adsp *adsp, struct device **pds,
 	}
 }
 
+static int adsp_shutdown_poll_decrypt(struct qcom_adsp *adsp)
+{
+	unsigned int retry_num = 50;
+	int ret;
+
+	do {
+		msleep(ADSP_DECRYPT_SHUTDOWN_DELAY_MS);
+		ret = qcom_scm_pas_shutdown(adsp->pas_id);
+	} while (ret == -EINVAL && --retry_num);
+
+	return ret;
+}
+
 static int adsp_unprepare(struct rproc *rproc)
 {
 	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
@@ -249,6 +267,9 @@ static int adsp_stop(struct rproc *rproc)
 		dev_err(adsp->dev, "timed out on wait\n");
 
 	ret = qcom_scm_pas_shutdown(adsp->pas_id);
+	if (ret && adsp->decrypt_shutdown)
+		ret = adsp_shutdown_poll_decrypt(adsp);
+
 	if (ret)
 		dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
 
@@ -459,6 +480,7 @@ static int adsp_probe(struct platform_device *pdev)
 	adsp->pas_id = desc->pas_id;
 	adsp->has_aggre2_clk = desc->has_aggre2_clk;
 	adsp->info_name = desc->sysmon_name;
+	adsp->decrypt_shutdown = desc->decrypt_shutdown;
 	platform_set_drvdata(pdev, adsp);
 
 	device_wakeup_enable(adsp->dev);
@@ -877,6 +899,25 @@ static const struct adsp_data sdx55_mpss_resource = {
 	.ssctl_id = 0x22,
 };
 
+static const struct adsp_data sm8450_mpss_resource = {
+	.crash_reason_smem = 421,
+	.firmware_name = "modem.mdt",
+	.pas_id = 4,
+	.minidump_id = 3,
+	.has_aggre2_clk = false,
+	.auto_boot = false,
+	.decrypt_shutdown = true,
+	.proxy_pd_names = (char*[]){
+		"cx",
+		"mss",
+		NULL
+	},
+	.load_state = "modem",
+	.ssr_name = "mpss",
+	.sysmon_name = "modem",
+	.ssctl_id = 0x12,
+};
+
 static const struct of_device_id adsp_of_match[] = {
 	{ .compatible = "qcom,msm8226-adsp-pil", .data = &adsp_resource_init},
 	{ .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init},
@@ -916,7 +957,7 @@ static const struct of_device_id adsp_of_match[] = {
 	{ .compatible = "qcom,sm8450-adsp-pas", .data = &sm8350_adsp_resource},
 	{ .compatible = "qcom,sm8450-cdsp-pas", .data = &sm8350_cdsp_resource},
 	{ .compatible = "qcom,sm8450-slpi-pas", .data = &sm8350_slpi_resource},
-	{ .compatible = "qcom,sm8450-mpss-pas", .data = &mpss_resource_init},
+	{ .compatible = "qcom,sm8450-mpss-pas", .data = &sm8450_mpss_resource},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, adsp_of_match);
-- 
2.7.4


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

* [V3 2/7] remoteproc: qcom: pas: Mark va as io memory
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
  2022-07-05 12:08 ` [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-05 12:08 ` [V3 3/7] remoteproc: qcom: pas: Mark devices as wakeup capable Sibi Sankar
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Siddharth Gupta,
	Sibi Sankar

From: Siddharth Gupta <sidgup@codeaurora.org>

The pas driver remaps the entire carveout region using the dev_ioremap_wc()
call, which is then used in the adsp_da_to_va() calls made by the rproc
framework. This change marks the va returned by this call as an iomem va.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 297700f87fe8..df13cfc3aeb8 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -289,6 +289,9 @@ static void *adsp_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iom
 	if (offset < 0 || offset + len > adsp->mem_size)
 		return NULL;
 
+	if (is_iomem)
+		*is_iomem = true;
+
 	return adsp->mem_region + offset;
 }
 
-- 
2.7.4


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

* [V3 3/7] remoteproc: qcom: pas: Mark devices as wakeup capable
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
  2022-07-05 12:08 ` [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem Sibi Sankar
  2022-07-05 12:08 ` [V3 2/7] remoteproc: qcom: pas: Mark va as io memory Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-05 12:08 ` [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled Sibi Sankar
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Siddharth Gupta,
	Sibi Sankar

From: Siddharth Gupta <sidgup@codeaurora.org>

device_wakeup_enable() on its own is not capable of setting
device as wakeup capable, it needs to be used in conjunction
with device_set_wakeup_capable(). The device_init_wakeup()
calls both these functions on the device passed.

Fixes: a781e5aa5911 ("remoteproc: core: Prevent system suspend during remoteproc recovery")
Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index df13cfc3aeb8..43dde151120f 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -486,7 +486,9 @@ static int adsp_probe(struct platform_device *pdev)
 	adsp->decrypt_shutdown = desc->decrypt_shutdown;
 	platform_set_drvdata(pdev, adsp);
 
-	device_wakeup_enable(adsp->dev);
+	ret = device_init_wakeup(adsp->dev, true);
+	if (ret)
+		goto free_rproc;
 
 	ret = adsp_alloc_memory_region(adsp);
 	if (ret)
-- 
2.7.4


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

* [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
                   ` (2 preceding siblings ...)
  2022-07-05 12:08 ` [V3 3/7] remoteproc: qcom: pas: Mark devices as wakeup capable Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-06 12:41   ` Konrad Dybcio
  2022-07-05 12:08 ` [V3 5/7] remoteproc: q6v5: Set q6 state to offline on receiving wdog irq Sibi Sankar
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Siddharth Gupta,
	Sibi Sankar

From: Siddharth Gupta <sidgup@codeaurora.org>

Client drivers need to check if coredump is enabled for the rproc before
continuing with coredump generation. This change adds a check in the PAS
driver.

Fixes: 8ed8485c4f05 ("remoteproc: qcom: Add capability to collect minidumps")
Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 43dde151120f..d103101a5ea0 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -92,6 +92,9 @@ static void adsp_minidump(struct rproc *rproc)
 {
 	struct qcom_adsp *adsp = rproc->priv;
 
+	if (rproc->dump_conf == RPROC_COREDUMP_DISABLED)
+		return;
+
 	qcom_minidump(rproc, adsp->minidump_id);
 }
 
-- 
2.7.4


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

* [V3 5/7] remoteproc: q6v5: Set q6 state to offline on receiving wdog irq
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
                   ` (3 preceding siblings ...)
  2022-07-05 12:08 ` [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-05 12:08 ` [V3 6/7] remoteproc: sysmon: Wait for SSCTL service to come up Sibi Sankar
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Siddharth Gupta,
	Sibi Sankar

From: Siddharth Gupta <sidgup@codeaurora.org>

Due to firmware bugs on the Q6 the hardware watchdog irq can be triggered
multiple times. As the remoteproc framework schedules work items for the
recovery process, if the other threads do not get a chance to run before
recovery is completed the proceeding threads will see the state of the
remoteproc as running and kill the remoteproc while it is running. This
can result in various SMMU and NOC errors. This change sets the state of
the remoteproc to offline whenever a watchdog irq is received.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 5280ec9b5449..497acfb33f8f 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -112,6 +112,7 @@ static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
 	else
 		dev_err(q6v5->dev, "watchdog without message\n");
 
+	q6v5->running = false;
 	rproc_report_crash(q6v5->rproc, RPROC_WATCHDOG);
 
 	return IRQ_HANDLED;
@@ -123,6 +124,9 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
 	size_t len;
 	char *msg;
 
+	if (!q6v5->running)
+		return IRQ_HANDLED;
+
 	msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
 	if (!IS_ERR(msg) && len > 0 && msg[0])
 		dev_err(q6v5->dev, "fatal error received: %s\n", msg);
-- 
2.7.4


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

* [V3 6/7] remoteproc: sysmon: Wait for SSCTL service to come up
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
                   ` (4 preceding siblings ...)
  2022-07-05 12:08 ` [V3 5/7] remoteproc: q6v5: Set q6 state to offline on receiving wdog irq Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-05 12:08 ` [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs Sibi Sankar
  2022-07-18 22:59 ` [V3 0/7] Miscellaneous PAS fixes Bjorn Andersson
  7 siblings, 0 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Sibi Sankar

The SSCTL service comes up after a finite time when the remote Q6 comes
out of reset. Any graceful shutdowns requested during this period will
be a NOP and abrupt tearing down of the glink channel might lead to pending
transactions on the remote Q6 side and will ultimately lead to a fatal
error. Fix this by waiting for the SSCTL service when a graceful shutdown
is requested.

Fixes: 1fb82ee806d1 ("remoteproc: qcom: Introduce sysmon")
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_sysmon.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index 9fca81492863..a9f04dd83ab6 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -41,6 +41,7 @@ struct qcom_sysmon {
 	struct completion comp;
 	struct completion ind_comp;
 	struct completion shutdown_comp;
+	struct completion ssctl_comp;
 	struct mutex lock;
 
 	bool ssr_ack;
@@ -445,6 +446,8 @@ static int ssctl_new_server(struct qmi_handle *qmi, struct qmi_service *svc)
 
 	svc->priv = sysmon;
 
+	complete(&sysmon->ssctl_comp);
+
 	return 0;
 }
 
@@ -501,6 +504,7 @@ static int sysmon_start(struct rproc_subdev *subdev)
 		.ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP
 	};
 
+	reinit_completion(&sysmon->ssctl_comp);
 	mutex_lock(&sysmon->state_lock);
 	sysmon->state = SSCTL_SSR_EVENT_AFTER_POWERUP;
 	blocking_notifier_call_chain(&sysmon_notifiers, 0, (void *)&event);
@@ -545,6 +549,11 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)
 	if (crashed)
 		return;
 
+	if (sysmon->ssctl_instance) {
+		if (!wait_for_completion_timeout(&sysmon->ssctl_comp, HZ / 2))
+			dev_err(sysmon->dev, "timeout waiting for ssctl service\n");
+	}
+
 	if (sysmon->ssctl_version)
 		sysmon->shutdown_acked = ssctl_request_shutdown(sysmon);
 	else if (sysmon->ept)
@@ -631,6 +640,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 	init_completion(&sysmon->comp);
 	init_completion(&sysmon->ind_comp);
 	init_completion(&sysmon->shutdown_comp);
+	init_completion(&sysmon->ssctl_comp);
 	mutex_init(&sysmon->lock);
 	mutex_init(&sysmon->state_lock);
 
-- 
2.7.4


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

* [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
                   ` (5 preceding siblings ...)
  2022-07-05 12:08 ` [V3 6/7] remoteproc: sysmon: Wait for SSCTL service to come up Sibi Sankar
@ 2022-07-05 12:08 ` Sibi Sankar
  2022-07-06 12:43   ` Konrad Dybcio
  2022-07-18 22:59 ` [V3 0/7] Miscellaneous PAS fixes Bjorn Andersson
  7 siblings, 1 reply; 13+ messages in thread
From: Sibi Sankar @ 2022-07-05 12:08 UTC (permalink / raw)
  To: bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, konrad.dybcio, Siddharth Gupta,
	Sibi Sankar

From: Siddharth Gupta <sidgup@codeaurora.org>

When a new remoteproc boots up, send the sysmon state notification
of only running remoteprocs. Sending state of remoteprocs booting
up in parallel can cause a race between SSR clients of the remoteproc
that is booting up and the sysmon notification for the same remoteproc,
resulting in an inconsistency between which state the remoteproc that
is booting up in parallel.

For example - if remoteproc A and B crash one after the other, after
remoteproc A boots up, if the remoteproc A tries to get the state of
remoteproc B before the sysmon subdevice for B is invoked but after
the ssr subdevice of B has been invoked, clients on remoteproc A
might get confused when the sysmon notification indicates a different
state.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_sysmon.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index a9f04dd83ab6..57dde2a69b9d 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -512,10 +512,12 @@ static int sysmon_start(struct rproc_subdev *subdev)
 
 	mutex_lock(&sysmon_lock);
 	list_for_each_entry(target, &sysmon_list, node) {
-		if (target == sysmon)
+		mutex_lock(&target->state_lock);
+		if (target == sysmon || target->state != SSCTL_SSR_EVENT_AFTER_POWERUP) {
+			mutex_unlock(&target->state_lock);
 			continue;
+		}
 
-		mutex_lock(&target->state_lock);
 		event.subsys_name = target->name;
 		event.ssr_event = target->state;
 
-- 
2.7.4


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

* Re: [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem
  2022-07-05 12:08 ` [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem Sibi Sankar
@ 2022-07-06 12:38   ` Konrad Dybcio
  2022-07-07 10:59     ` Sibi Sankar
  0 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2022-07-06 12:38 UTC (permalink / raw)
  To: Sibi Sankar, bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel



On 5.07.2022 14:08, Sibi Sankar wrote:
> The initial shutdown request to modem on SM8450 SoCs would start the
> decryption process and will keep returning errors until the modem shutdown
> is complete. Fix this by retrying shutdowns in fixed intervals.
> 
I'm sorry, but this message seems a bit cryptic to me.. What
is being decrypted? How is it related to the shutdown sequence?
Why does it need to finish first?

Konrad

[snipped the rest]

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

* Re: [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled
  2022-07-05 12:08 ` [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled Sibi Sankar
@ 2022-07-06 12:41   ` Konrad Dybcio
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2022-07-06 12:41 UTC (permalink / raw)
  To: Sibi Sankar, bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, Siddharth Gupta



On 5.07.2022 14:08, Sibi Sankar wrote:
> From: Siddharth Gupta <sidgup@codeaurora.org>
> 
> Client drivers need to check if coredump is enabled for the rproc before
> continuing with coredump generation. This change adds a check in the PAS
> driver.
> 
> Fixes: 8ed8485c4f05 ("remoteproc: qcom: Add capability to collect minidumps")
> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  drivers/remoteproc/qcom_q6v5_pas.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 43dde151120f..d103101a5ea0 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -92,6 +92,9 @@ static void adsp_minidump(struct rproc *rproc)
>  {
>  	struct qcom_adsp *adsp = rproc->priv;
>  
> +	if (rproc->dump_conf == RPROC_COREDUMP_DISABLED)
> +		return;
> +
>  	qcom_minidump(rproc, adsp->minidump_id);
>  }
>  

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

* Re: [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs
  2022-07-05 12:08 ` [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs Sibi Sankar
@ 2022-07-06 12:43   ` Konrad Dybcio
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2022-07-06 12:43 UTC (permalink / raw)
  To: Sibi Sankar, bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel, Siddharth Gupta



On 5.07.2022 14:08, Sibi Sankar wrote:
> From: Siddharth Gupta <sidgup@codeaurora.org>
> 
> When a new remoteproc boots up, send the sysmon state notification
> of only running remoteprocs. Sending state of remoteprocs booting
> up in parallel can cause a race between SSR clients of the remoteproc
> that is booting up and the sysmon notification for the same remoteproc,
> resulting in an inconsistency between which state the remoteproc that
> is booting up in parallel.
> 
> For example - if remoteproc A and B crash one after the other, after
> remoteproc A boots up, if the remoteproc A tries to get the state of
> remoteproc B before the sysmon subdevice for B is invoked but after
> the ssr subdevice of B has been invoked, clients on remoteproc A
> might get confused when the sysmon notification indicates a different
> state.
> 
> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  drivers/remoteproc/qcom_sysmon.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
> index a9f04dd83ab6..57dde2a69b9d 100644
> --- a/drivers/remoteproc/qcom_sysmon.c
> +++ b/drivers/remoteproc/qcom_sysmon.c
> @@ -512,10 +512,12 @@ static int sysmon_start(struct rproc_subdev *subdev)
>  
>  	mutex_lock(&sysmon_lock);
>  	list_for_each_entry(target, &sysmon_list, node) {
> -		if (target == sysmon)
> +		mutex_lock(&target->state_lock);
> +		if (target == sysmon || target->state != SSCTL_SSR_EVENT_AFTER_POWERUP) {
> +			mutex_unlock(&target->state_lock);
>  			continue;
> +		}
>  
> -		mutex_lock(&target->state_lock);
>  		event.subsys_name = target->name;
>  		event.ssr_event = target->state;
>  

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

* Re: [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem
  2022-07-06 12:38   ` Konrad Dybcio
@ 2022-07-07 10:59     ` Sibi Sankar
  0 siblings, 0 replies; 13+ messages in thread
From: Sibi Sankar @ 2022-07-07 10:59 UTC (permalink / raw)
  To: Konrad Dybcio, bjorn.andersson
  Cc: agross, mathieu.poirier, dmitry.baryshkov, linux-arm-msm,
	linux-remoteproc, linux-kernel

Hey Konrad,
Thanks for taking time to review the series.


On 7/6/22 6:08 PM, Konrad Dybcio wrote:
> 
> 
> On 5.07.2022 14:08, Sibi Sankar wrote:
>> The initial shutdown request to modem on SM8450 SoCs would start the
>> decryption process and will keep returning errors until the modem shutdown
>> is complete. Fix this by retrying shutdowns in fixed intervals.
>>
> I'm sorry, but this message seems a bit cryptic to me.. What
> is being decrypted? How is it related to the shutdown sequence?
> Why does it need to finish first?

I was told some portions of the modem logs in secured modem regions
needs decryption and this needs to be completed before minidump/coredump
are collected.

-Sibi

> 
> Konrad
> 
> [snipped the rest]
> 

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

* Re: [V3 0/7] Miscellaneous PAS fixes
  2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
                   ` (6 preceding siblings ...)
  2022-07-05 12:08 ` [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs Sibi Sankar
@ 2022-07-18 22:59 ` Bjorn Andersson
  7 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2022-07-18 22:59 UTC (permalink / raw)
  To: quic_sibis
  Cc: linux-kernel, konrad.dybcio, agross, dmitry.baryshkov,
	mathieu.poirier, linux-remoteproc, linux-arm-msm

On Tue, 5 Jul 2022 17:38:13 +0530, Sibi Sankar wrote:
> A collection of misc. fixes for the remoteproc PAS and sysmon driver.
> 
> V3:
>  * Fixup misc. suggestions and rename to adsp_shutdown_poll_decrypt() [Bjorn]
>  * Pick up another lone sysmon fix from the list with R-b.
> 
> V2:
>  * Add misc. sysmon fix.
>  * Dropping patch 1 and 6 from V1.
> 
> [...]

Applied, thanks!

[1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem
      commit: 92c7b8ca723d5ac133fb745f9c449fa35acd0139
[2/7] remoteproc: qcom: pas: Mark va as io memory
      commit: 831b85682315631732cb0a67e5ac5d39fa5203ee
[3/7] remoteproc: qcom: pas: Mark devices as wakeup capable
      commit: f90838e44430446f47ecf9a091fea79b5f297972
[4/7] remoteproc: qcom: pas: Check if coredump is enabled
      commit: f0c8a12816333a3444deabf1dbb2f36e216b4370
[5/7] remoteproc: q6v5: Set q6 state to offline on receiving wdog irq
      commit: 905521a06455c13662632df90ee0aeaa666214fa
[6/7] remoteproc: sysmon: Wait for SSCTL service to come up
      commit: de1e8df64360035354741e65d22a07e93747f603
[7/7] remoteproc: sysmon: Send sysmon state only for running rprocs
      commit: 718dd77469350e6702b751dbff03f631dda3f13b

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

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

end of thread, other threads:[~2022-07-18 23:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 12:08 [V3 0/7] Miscellaneous PAS fixes Sibi Sankar
2022-07-05 12:08 ` [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem Sibi Sankar
2022-07-06 12:38   ` Konrad Dybcio
2022-07-07 10:59     ` Sibi Sankar
2022-07-05 12:08 ` [V3 2/7] remoteproc: qcom: pas: Mark va as io memory Sibi Sankar
2022-07-05 12:08 ` [V3 3/7] remoteproc: qcom: pas: Mark devices as wakeup capable Sibi Sankar
2022-07-05 12:08 ` [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled Sibi Sankar
2022-07-06 12:41   ` Konrad Dybcio
2022-07-05 12:08 ` [V3 5/7] remoteproc: q6v5: Set q6 state to offline on receiving wdog irq Sibi Sankar
2022-07-05 12:08 ` [V3 6/7] remoteproc: sysmon: Wait for SSCTL service to come up Sibi Sankar
2022-07-05 12:08 ` [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs Sibi Sankar
2022-07-06 12:43   ` Konrad Dybcio
2022-07-18 22:59 ` [V3 0/7] Miscellaneous PAS fixes Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).