2022-07-05 13:02:41

by Sibi Sankar

[permalink] [raw]
Subject: [V3 0/7] Miscellaneous PAS fixes

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


2022-07-05 13:18:43

by Sibi Sankar

[permalink] [raw]
Subject: [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem

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 <[email protected]>
---

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

2022-07-05 13:25:50

by Sibi Sankar

[permalink] [raw]
Subject: [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled

From: Siddharth Gupta <[email protected]>

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 <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
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

2022-07-05 13:30:02

by Sibi Sankar

[permalink] [raw]
Subject: [V3 3/7] remoteproc: qcom: pas: Mark devices as wakeup capable

From: Siddharth Gupta <[email protected]>

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 <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
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

2022-07-06 13:09:12

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem



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]

2022-07-06 13:15:53

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [V3 4/7] remoteproc: qcom: pas: Check if coredump is enabled



On 5.07.2022 14:08, Sibi Sankar wrote:
> From: Siddharth Gupta <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

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);
> }
>

2022-07-07 11:02:44

by Sibi Sankar

[permalink] [raw]
Subject: Re: [V3 1/7] remoteproc: qcom: pas: Add decrypt shutdown support for modem

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

2022-07-18 23:01:39

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [V3 0/7] Miscellaneous PAS fixes

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 <[email protected]>