Re: [PATCH V4] remoteproc: qcom: q6v5: Get crash reason from specific SMEM partition
On 3/6/2024 9:25 AM, Bjorn Andersson wrote:
> On Wed, Dec 20, 2023 at 11:25:11AM +0530, Vignesh Viswanathan wrote:
>> q6v5 fatal and watchdog IRQ handlers always retrieves the crash reason
>> information from SMEM global partition (QCOM_SMEM_HOST_ANY).
>>
>> For some targets like IPQ9574 and IPQ5332, crash reason information is
>> present in target specific partition due to which the crash reason is
>> not printed in the current implementation.
>>
>> Add support to pass crash_reason_partition along with crash_reason_item
>> number in qcom_q6v5_init call and use the same to get the crash
>> information from SMEM in fatal and watchdog IRQ handlers.
>>
>> While at it, rename all instances of "crash_reason_smem" with
>> "crash_reason_item" as this reflects the proper meaning.
>>
>> Signed-off-by: Vignesh Viswanathan
>
> No concerns with the patch now, but as this depends on the mpd driver,
> which is being refactored, please resubmit this once the driver is being
> accepted.
>
Thanks Bjorn. Will resubmit this once the refactored MPD driver posted.
>> ---
>> Changes in V4: Rename all instances of crash_reason_smem to crash_reason_item
>> Changes in V3: Updated commit message.
>> Changes in V2: Addressed comments in V1.
>
> Please review go/upstream and start using b4.
Sure, will give b4 a try.
Thanks,
Vignesh
>
> Regards,
> Bjorn
>
>>
>> This patch depends on [1] which adds support for IPQ9574 and IPQ5332
>> remoteproc q5v5_mpd driver.
>>
>> [1]:
>> https://lore.kernel.org/all/20231110091939.3025413-1-quic_mmani...@quicinc.com/
>>
>> drivers/remoteproc/qcom_q6v5.c | 10 +++--
>> drivers/remoteproc/qcom_q6v5.h | 6 ++-
>> drivers/remoteproc/qcom_q6v5_adsp.c | 17 +
>> drivers/remoteproc/qcom_q6v5_mpd.c | 13 ---
>> drivers/remoteproc/qcom_q6v5_mss.c | 5 ++-
>> drivers/remoteproc/qcom_q6v5_pas.c | 59 +++--
>> drivers/remoteproc/qcom_q6v5_wcss.c | 12 +++---
>> 7 files changed, 66 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
>> index 0e32f13c196d..e4a28bf25130 100644
>> --- a/drivers/remoteproc/qcom_q6v5.c
>> +++ b/drivers/remoteproc/qcom_q6v5.c
>> @@ -100,7 +100,7 @@ static irqreturn_t q6v5_wdog_interrupt(int irq, void
>> *data)
>> return IRQ_HANDLED;
>> }
>>
>> -msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, );
>> +msg = qcom_smem_get(q6v5->crash_reason_partition,
>> q6v5->crash_reason_item, );
>> if (!IS_ERR(msg) && len > 0 && msg[0])
>> dev_err(q6v5->dev, "watchdog received: %s\n", msg);
>> else
>> @@ -121,7 +121,7 @@ irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
>> if (!q6v5->running)
>> return IRQ_HANDLED;
>>
>> -msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, );
>> +msg = qcom_smem_get(q6v5->crash_reason_partition,
>> q6v5->crash_reason_item, );
>> if (!IS_ERR(msg) && len > 0 && msg[0])
>> dev_err(q6v5->dev, "fatal error received: %s\n", msg);
>> else
>> @@ -279,14 +279,16 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
>> * Return: 0 on success, negative errno on failure
>> */
>> int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>> - struct rproc *rproc, int crash_reason, const char
>> *load_state,
>> + struct rproc *rproc, int crash_reason_partition,
>> + int crash_reason_item, const char *load_state,
>> void (*handover)(struct qcom_q6v5 *q6v5))
>> {
>> int ret;
>>
>> q6v5->rproc = rproc;
>> q6v5->dev = >dev;
>> -q6v5->crash_reason = crash_reason;
>> +q6v5->crash_reason_partition = crash_reason_partition;
>> +q6v5->crash_reason_item = crash_reason_item;
>> q6v5->handover = handover;
>>
>> init_completion(>start_done);
>> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
>> index 4e1bb1a68284..cd02372e9856 100644
>> --- a/drivers/remoteproc/qcom_q6v5.h
>> +++ b/drivers/remoteproc/qcom_q6v5.h
>> @@ -40,7 +40,8 @@ struct qcom_q6v5 {
>> struct completion stop_done;
>> struct completion spawn_done;
>>
>> -int crash_reason;
>> +int crash_reason_partition;
>> +int crash_reason_item;
>>
>> bool running;
>>
>> @@ -49,7 +50,8 @@ struct qcom_q6v5 {
>> };
>>
>> int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>> - struct rproc *rproc, int crash_reason, const char
>> *load_state,
>> + struct rproc *rproc, int crash_reason_partition,
>> + int crash_reason_item, const char *load_state,
>> void (*handover)(struct qcom_q6v5 *q6v5));
>> void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5);
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 6c67514cc493..055764aa201c 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++
Re: [PATCH V4] remoteproc: qcom: q6v5: Get crash reason from specific SMEM partition
On Wed, Dec 20, 2023 at 11:25:11AM +0530, Vignesh Viswanathan wrote:
> q6v5 fatal and watchdog IRQ handlers always retrieves the crash reason
> information from SMEM global partition (QCOM_SMEM_HOST_ANY).
>
> For some targets like IPQ9574 and IPQ5332, crash reason information is
> present in target specific partition due to which the crash reason is
> not printed in the current implementation.
>
> Add support to pass crash_reason_partition along with crash_reason_item
> number in qcom_q6v5_init call and use the same to get the crash
> information from SMEM in fatal and watchdog IRQ handlers.
>
> While at it, rename all instances of "crash_reason_smem" with
> "crash_reason_item" as this reflects the proper meaning.
>
> Signed-off-by: Vignesh Viswanathan
No concerns with the patch now, but as this depends on the mpd driver,
which is being refactored, please resubmit this once the driver is being
accepted.
> ---
> Changes in V4: Rename all instances of crash_reason_smem to crash_reason_item
> Changes in V3: Updated commit message.
> Changes in V2: Addressed comments in V1.
Please review go/upstream and start using b4.
Regards,
Bjorn
>
> This patch depends on [1] which adds support for IPQ9574 and IPQ5332
> remoteproc q5v5_mpd driver.
>
> [1]:
> https://lore.kernel.org/all/20231110091939.3025413-1-quic_mmani...@quicinc.com/
>
> drivers/remoteproc/qcom_q6v5.c | 10 +++--
> drivers/remoteproc/qcom_q6v5.h | 6 ++-
> drivers/remoteproc/qcom_q6v5_adsp.c | 17 +
> drivers/remoteproc/qcom_q6v5_mpd.c | 13 ---
> drivers/remoteproc/qcom_q6v5_mss.c | 5 ++-
> drivers/remoteproc/qcom_q6v5_pas.c | 59 +++--
> drivers/remoteproc/qcom_q6v5_wcss.c | 12 +++---
> 7 files changed, 66 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 0e32f13c196d..e4a28bf25130 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -100,7 +100,7 @@ static irqreturn_t q6v5_wdog_interrupt(int irq, void
> *data)
> return IRQ_HANDLED;
> }
>
> - msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, );
> + msg = qcom_smem_get(q6v5->crash_reason_partition,
> q6v5->crash_reason_item, );
> if (!IS_ERR(msg) && len > 0 && msg[0])
> dev_err(q6v5->dev, "watchdog received: %s\n", msg);
> else
> @@ -121,7 +121,7 @@ irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
> if (!q6v5->running)
> return IRQ_HANDLED;
>
> - msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, );
> + msg = qcom_smem_get(q6v5->crash_reason_partition,
> q6v5->crash_reason_item, );
> if (!IS_ERR(msg) && len > 0 && msg[0])
> dev_err(q6v5->dev, "fatal error received: %s\n", msg);
> else
> @@ -279,14 +279,16 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
> * Return: 0 on success, negative errno on failure
> */
> int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> -struct rproc *rproc, int crash_reason, const char
> *load_state,
> +struct rproc *rproc, int crash_reason_partition,
> +int crash_reason_item, const char *load_state,
> void (*handover)(struct qcom_q6v5 *q6v5))
> {
> int ret;
>
> q6v5->rproc = rproc;
> q6v5->dev = >dev;
> - q6v5->crash_reason = crash_reason;
> + q6v5->crash_reason_partition = crash_reason_partition;
> + q6v5->crash_reason_item = crash_reason_item;
> q6v5->handover = handover;
>
> init_completion(>start_done);
> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
> index 4e1bb1a68284..cd02372e9856 100644
> --- a/drivers/remoteproc/qcom_q6v5.h
> +++ b/drivers/remoteproc/qcom_q6v5.h
> @@ -40,7 +40,8 @@ struct qcom_q6v5 {
> struct completion stop_done;
> struct completion spawn_done;
>
> - int crash_reason;
> + int crash_reason_partition;
> + int crash_reason_item;
>
> bool running;
>
> @@ -49,7 +50,8 @@ struct qcom_q6v5 {
> };
>
> int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> -struct rproc *rproc, int crash_reason, const char
> *load_state,
> +struct rproc *rproc, int crash_reason_partition,
> +int crash_reason_item, const char *load_state,
> void (*handover)(struct qcom_q6v5 *q6v5));
> void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5);
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
> b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 6c67514cc493..055764aa201c 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -62,7 +62,7 @@
> #define LPASS_EFUSE_Q6SS_EVB_SEL 0x0
>
> struct adsp_pil_data {
> - int crash_reason_smem;
> + int crash_reason_item;
> const char *firmware_name;
>
> const char *ssr_name;
> @@ -98,7
