diff mbox series

[v3,5/6] remoteproc: qcom: q6v5-pil: Register segments/dumpfn for coredump

Message ID 20180727152003.11663-6-sibis@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Add coredump support for Q6v5 Modem remoteproc | expand

Commit Message

Sibi Sankar July 27, 2018, 3:20 p.m. UTC
Register the MDT segments and custom dumpfn with the remoteproc core
dump functionality.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_pil.c | 40 ++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Bjorn Andersson Oct. 8, 2018, 6:48 a.m. UTC | #1
On Fri 27 Jul 08:20 PDT 2018, Sibi Sankar wrote:

> Register the MDT segments and custom dumpfn with the remoteproc core
> dump functionality.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/remoteproc/qcom_q6v5_pil.c | 40 ++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> index ac3342f9ea5a..22bb049c3e7f 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -1058,10 +1058,50 @@ static void *q6v5_da_to_va(struct rproc *rproc, u64 da, int len)
>  	return qproc->mpss_region + offset;
>  }
>  
> +static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
> +				const struct firmware *fw_unused)

How about naming it mba_fw instead of unused? Just as unused, but easier
to understand why it isn't used.

> +{
> +	const struct firmware *fw;
> +	const struct elf32_phdr *phdrs;
> +	const struct elf32_phdr *phdr;
> +	const struct elf32_hdr *ehdr;
> +	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;

No need for an explicit typecast from void *.

The rest looks good!

Regards,
Bjorn
Sibi Sankar Oct. 9, 2018, 4:21 p.m. UTC | #2
Hi Bjorn,
Thanks for the review !

On 2018-10-08 12:18, Bjorn Andersson wrote:
> On Fri 27 Jul 08:20 PDT 2018, Sibi Sankar wrote:
> 
>> Register the MDT segments and custom dumpfn with the remoteproc core
>> dump functionality.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  drivers/remoteproc/qcom_q6v5_pil.c | 40 
>> ++++++++++++++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>> 
>> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c 
>> b/drivers/remoteproc/qcom_q6v5_pil.c
>> index ac3342f9ea5a..22bb049c3e7f 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pil.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
>> @@ -1058,10 +1058,50 @@ static void *q6v5_da_to_va(struct rproc 
>> *rproc, u64 da, int len)
>>  	return qproc->mpss_region + offset;
>>  }
>> 
>> +static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
>> +				const struct firmware *fw_unused)
> 
> How about naming it mba_fw instead of unused? Just as unused, but 
> easier
> to understand why it isn't used.
> 

sure

>> +{
>> +	const struct firmware *fw;
>> +	const struct elf32_phdr *phdrs;
>> +	const struct elf32_phdr *phdr;
>> +	const struct elf32_hdr *ehdr;
>> +	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;
> 
> No need for an explicit typecast from void *.
> 

will remove it

> The rest looks good!
> 
> Regards,
> Bjorn
diff mbox series

Patch

diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index ac3342f9ea5a..22bb049c3e7f 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -1058,10 +1058,50 @@  static void *q6v5_da_to_va(struct rproc *rproc, u64 da, int len)
 	return qproc->mpss_region + offset;
 }
 
+static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
+				const struct firmware *fw_unused)
+{
+	const struct firmware *fw;
+	const struct elf32_phdr *phdrs;
+	const struct elf32_phdr *phdr;
+	const struct elf32_hdr *ehdr;
+	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;
+	int ret;
+	int i;
+
+	ret = request_firmware(&fw, "modem.mdt", qproc->dev);
+	if (ret < 0) {
+		dev_err(qproc->dev, "unable to load modem.mdt\n");
+		return ret;
+	}
+
+	qproc->valid_mask = 0;
+	ehdr = (struct elf32_hdr *)fw->data;
+	phdrs = (struct elf32_phdr *)(ehdr + 1);
+
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		phdr = &phdrs[i];
+
+		if (!q6v5_phdr_valid(phdr))
+			continue;
+
+		ret = rproc_coredump_add_custom_segment(rproc, phdr->p_paddr,
+					phdr->p_memsz, qcom_q6v5_dump_segment);
+		if (ret)
+			break;
+
+		qproc->valid_mask++;
+	}
+
+	release_firmware(fw);
+	return ret;
+}
+
 static const struct rproc_ops q6v5_ops = {
 	.start = q6v5_start,
 	.stop = q6v5_stop,
 	.da_to_va = q6v5_da_to_va,
+	.parse_fw = qcom_q6v5_register_dump_segments,
 	.load = q6v5_load,
 };