| Message ID | 1679491817-2498-7-git-send-email-quic_mojha@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/6] remoteproc: qcom: Expand MD_* as MINIDUMP_* | expand |
On 22/03/2023 13:30, Mukesh Ojha wrote: > qcom_minidump driver provides qcom_minidump_subsystem_desc() > exported API which other driver can use it query subsystem > descriptor. Refactor qcom_minidump() to use this symbol. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > drivers/remoteproc/qcom_common.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c > index 88fc984..240e9f7 100644 > --- a/drivers/remoteproc/qcom_common.c > +++ b/drivers/remoteproc/qcom_common.c > @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, > { > int ret; > struct minidump_subsystem *subsystem; > - struct minidump_global_toc *toc; > > - /* Get Global minidump ToC*/ > - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL); > - > - /* check if global table pointer exists and init is set */ > - if (IS_ERR(toc) || !toc->status) { > - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); > + subsystem = qcom_minidump_subsystem_desc(minidump_id); > + if (IS_ERR(subsystem)) > return; Sorry If I am missing something but I got lost looking at the below code snippet in drivers/remoteproc/qcom_common.c -------------------->cut<----------------------------- subsystem = qcom_minidump_subsystem_desc(minidump_id); if (IS_ERR(subsystem)) return; /** * Collect minidump if SS ToC is valid and segment table * is initialized in memory and encryption status is set. */ if (subsystem->regions_baseptr == 0 || le32_to_cpu(subsystem->status) != 1 || le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED || le32_to_cpu(subsystem->encryption_status) != MINIDUMP_SS_ENCR_DONE) { dev_err(&rproc->dev, "Minidump not ready, skipping\n"); return; } -------------------->cut<----------------------------- where does "subsystem->regions_baseptr" for this ADSP minidump descriptor get set? --srini > - } > - > - /* Get subsystem table of contents using the minidump id */ > - subsystem = &toc->subsystems[minidump_id]; > > /** > * Collect minidump if SS ToC is valid and segment table
On 4/14/2023 4:14 PM, Srinivas Kandagatla wrote: > > > On 22/03/2023 13:30, Mukesh Ojha wrote: >> qcom_minidump driver provides qcom_minidump_subsystem_desc() >> exported API which other driver can use it query subsystem >> descriptor. Refactor qcom_minidump() to use this symbol. >> >> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >> --- >> drivers/remoteproc/qcom_common.c | 13 ++----------- >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_common.c >> b/drivers/remoteproc/qcom_common.c >> index 88fc984..240e9f7 100644 >> --- a/drivers/remoteproc/qcom_common.c >> +++ b/drivers/remoteproc/qcom_common.c >> @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned >> int minidump_id, >> { >> int ret; >> struct minidump_subsystem *subsystem; >> - struct minidump_global_toc *toc; >> - /* Get Global minidump ToC*/ >> - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL); >> - >> - /* check if global table pointer exists and init is set */ >> - if (IS_ERR(toc) || !toc->status) { >> - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); >> + subsystem = qcom_minidump_subsystem_desc(minidump_id); >> + if (IS_ERR(subsystem)) >> return; > > Sorry If I am missing something but I got lost looking at the below code > snippet in drivers/remoteproc/qcom_common.c > > > -------------------->cut<----------------------------- > subsystem = qcom_minidump_subsystem_desc(minidump_id); > if (IS_ERR(subsystem)) > return; > > /** > * Collect minidump if SS ToC is valid and segment table > * is initialized in memory and encryption status is set. > */ > if (subsystem->regions_baseptr == 0 || > le32_to_cpu(subsystem->status) != 1 || > le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED || > le32_to_cpu(subsystem->encryption_status) != > MINIDUMP_SS_ENCR_DONE) { > dev_err(&rproc->dev, "Minidump not ready, skipping\n"); > return; > } > -------------------->cut<----------------------------- > > where does "subsystem->regions_baseptr" for this ADSP minidump > descriptor get set? Other co-processor such as adsp/cdsp/Mpss has their own way of registering their region/segment (mostly they are static known regions) with minidump global infra and which could be happening from firmware side . -Mukesh > > > --srini > >> - } >> - >> - /* Get subsystem table of contents using the minidump id */ >> - subsystem = &toc->subsystems[minidump_id]; >> /** >> * Collect minidump if SS ToC is valid and segment table
On 14/04/2023 12:14, Mukesh Ojha wrote: > > > On 4/14/2023 4:14 PM, Srinivas Kandagatla wrote: >> >> >> On 22/03/2023 13:30, Mukesh Ojha wrote: >>> qcom_minidump driver provides qcom_minidump_subsystem_desc() >>> exported API which other driver can use it query subsystem >>> descriptor. Refactor qcom_minidump() to use this symbol. >>> >>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >>> --- >>> drivers/remoteproc/qcom_common.c | 13 ++----------- >>> 1 file changed, 2 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/remoteproc/qcom_common.c >>> b/drivers/remoteproc/qcom_common.c >>> index 88fc984..240e9f7 100644 >>> --- a/drivers/remoteproc/qcom_common.c >>> +++ b/drivers/remoteproc/qcom_common.c >>> @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned >>> int minidump_id, >>> { >>> int ret; >>> struct minidump_subsystem *subsystem; >>> - struct minidump_global_toc *toc; >>> - /* Get Global minidump ToC*/ >>> - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, >>> NULL); >>> - >>> - /* check if global table pointer exists and init is set */ >>> - if (IS_ERR(toc) || !toc->status) { >>> - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); >>> + subsystem = qcom_minidump_subsystem_desc(minidump_id); >>> + if (IS_ERR(subsystem)) >>> return; >> >> Sorry If I am missing something but I got lost looking at the below >> code snippet in drivers/remoteproc/qcom_common.c >> >> >> -------------------->cut<----------------------------- >> subsystem = qcom_minidump_subsystem_desc(minidump_id); >> if (IS_ERR(subsystem)) >> return; >> >> /** >> * Collect minidump if SS ToC is valid and segment table >> * is initialized in memory and encryption status is set. >> */ >> if (subsystem->regions_baseptr == 0 || >> le32_to_cpu(subsystem->status) != 1 || >> le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED || >> le32_to_cpu(subsystem->encryption_status) != >> MINIDUMP_SS_ENCR_DONE) { >> dev_err(&rproc->dev, "Minidump not ready, skipping\n"); >> return; >> } >> -------------------->cut<----------------------------- >> >> where does "subsystem->regions_baseptr" for this ADSP minidump >> descriptor get set? > > Other co-processor such as adsp/cdsp/Mpss has their own way of > registering their region/segment (mostly they are static known > regions) with minidump global infra and which could be happening > from firmware side . If its happening from firmware side, then that ram phys address range should be reserved from kernel usage I guess. Do you have more details on where exactly is this reserved from within linux kernel? --srini > > > -Mukesh > >> >> >> --srini >> >>> - } >>> - >>> - /* Get subsystem table of contents using the minidump id */ >>> - subsystem = &toc->subsystems[minidump_id]; >>> /** >>> * Collect minidump if SS ToC is valid and segment table
On 4/14/2023 5:10 PM, Srinivas Kandagatla wrote: > > > On 14/04/2023 12:14, Mukesh Ojha wrote: >> >> >> On 4/14/2023 4:14 PM, Srinivas Kandagatla wrote: >>> >>> >>> On 22/03/2023 13:30, Mukesh Ojha wrote: >>>> qcom_minidump driver provides qcom_minidump_subsystem_desc() >>>> exported API which other driver can use it query subsystem >>>> descriptor. Refactor qcom_minidump() to use this symbol. >>>> >>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >>>> --- >>>> drivers/remoteproc/qcom_common.c | 13 ++----------- >>>> 1 file changed, 2 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/remoteproc/qcom_common.c >>>> b/drivers/remoteproc/qcom_common.c >>>> index 88fc984..240e9f7 100644 >>>> --- a/drivers/remoteproc/qcom_common.c >>>> +++ b/drivers/remoteproc/qcom_common.c >>>> @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned >>>> int minidump_id, >>>> { >>>> int ret; >>>> struct minidump_subsystem *subsystem; >>>> - struct minidump_global_toc *toc; >>>> - /* Get Global minidump ToC*/ >>>> - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, >>>> NULL); >>>> - >>>> - /* check if global table pointer exists and init is set */ >>>> - if (IS_ERR(toc) || !toc->status) { >>>> - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); >>>> + subsystem = qcom_minidump_subsystem_desc(minidump_id); >>>> + if (IS_ERR(subsystem)) >>>> return; >>> >>> Sorry If I am missing something but I got lost looking at the below >>> code snippet in drivers/remoteproc/qcom_common.c >>> >>> >>> -------------------->cut<----------------------------- >>> subsystem = qcom_minidump_subsystem_desc(minidump_id); >>> if (IS_ERR(subsystem)) >>> return; >>> >>> /** >>> * Collect minidump if SS ToC is valid and segment table >>> * is initialized in memory and encryption status is set. >>> */ >>> if (subsystem->regions_baseptr == 0 || >>> le32_to_cpu(subsystem->status) != 1 || >>> le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED || >>> le32_to_cpu(subsystem->encryption_status) != >>> MINIDUMP_SS_ENCR_DONE) { >>> dev_err(&rproc->dev, "Minidump not ready, skipping\n"); >>> return; >>> } >>> -------------------->cut<----------------------------- >>> >>> where does "subsystem->regions_baseptr" for this ADSP minidump >>> descriptor get set? >> >> Other co-processor such as adsp/cdsp/Mpss has their own way of >> registering their region/segment (mostly they are static known >> regions) with minidump global infra and which could be happening >> from firmware side . > If its happening from firmware side, then that ram phys address range > should be reserved from kernel usage I guess. > > Do you have more details on where exactly is this reserved from within > linux kernel? These regions are inside remoteproc memory carve-out. like. adsp_mem: memory@85e00000 { reg = <0x0 0x85e00000 0x0 0x2100000>; no-map; }; remoteproc_adsp: remoteproc@30000000 { compatible = "qcom,sm8450-adsp-pas"; reg = <0 0x30000000 0 0x100>; ... ... memory-region = <&adsp_mem>; <== -Mukesh > > > --srini > >> >> >> -Mukesh >> >>> >>> >>> --srini >>> >>>> - } >>>> - >>>> - /* Get subsystem table of contents using the minidump id */ >>>> - subsystem = &toc->subsystems[minidump_id]; >>>> /** >>>> * Collect minidump if SS ToC is valid and segment table
On 14/04/2023 12:49, Mukesh Ojha wrote: > > > On 4/14/2023 5:10 PM, Srinivas Kandagatla wrote: >> >> >> On 14/04/2023 12:14, Mukesh Ojha wrote: >>> >>> >>> On 4/14/2023 4:14 PM, Srinivas Kandagatla wrote: >>>> >>>> >>>> On 22/03/2023 13:30, Mukesh Ojha wrote: >>>>> qcom_minidump driver provides qcom_minidump_subsystem_desc() >>>>> exported API which other driver can use it query subsystem >>>>> descriptor. Refactor qcom_minidump() to use this symbol. >>>>> >>>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >>>>> --- >>>>> drivers/remoteproc/qcom_common.c | 13 ++----------- >>>>> 1 file changed, 2 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/drivers/remoteproc/qcom_common.c >>>>> b/drivers/remoteproc/qcom_common.c >>>>> index 88fc984..240e9f7 100644 >>>>> --- a/drivers/remoteproc/qcom_common.c >>>>> +++ b/drivers/remoteproc/qcom_common.c >>>>> @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, >>>>> unsigned int minidump_id, >>>>> { >>>>> int ret; >>>>> struct minidump_subsystem *subsystem; >>>>> - struct minidump_global_toc *toc; >>>>> - /* Get Global minidump ToC*/ >>>>> - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, >>>>> NULL); >>>>> - >>>>> - /* check if global table pointer exists and init is set */ >>>>> - if (IS_ERR(toc) || !toc->status) { >>>>> - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); >>>>> + subsystem = qcom_minidump_subsystem_desc(minidump_id); >>>>> + if (IS_ERR(subsystem)) >>>>> return; >>>> >>>> Sorry If I am missing something but I got lost looking at the below >>>> code snippet in drivers/remoteproc/qcom_common.c >>>> >>>> >>>> -------------------->cut<----------------------------- >>>> subsystem = qcom_minidump_subsystem_desc(minidump_id); >>>> if (IS_ERR(subsystem)) >>>> return; >>>> >>>> /** >>>> * Collect minidump if SS ToC is valid and segment table >>>> * is initialized in memory and encryption status is set. >>>> */ >>>> if (subsystem->regions_baseptr == 0 || >>>> le32_to_cpu(subsystem->status) != 1 || >>>> le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED || >>>> le32_to_cpu(subsystem->encryption_status) != >>>> MINIDUMP_SS_ENCR_DONE) { >>>> dev_err(&rproc->dev, "Minidump not ready, skipping\n"); >>>> return; >>>> } >>>> -------------------->cut<----------------------------- >>>> >>>> where does "subsystem->regions_baseptr" for this ADSP minidump >>>> descriptor get set? >>> >>> Other co-processor such as adsp/cdsp/Mpss has their own way of >>> registering their region/segment (mostly they are static known >>> regions) with minidump global infra and which could be happening >>> from firmware side . >> If its happening from firmware side, then that ram phys address range >> should be reserved from kernel usage I guess. >> >> Do you have more details on where exactly is this reserved from within >> linux kernel? > > These regions are inside remoteproc memory carve-out. > like. > > adsp_mem: memory@85e00000 { > reg = <0x0 0x85e00000 0x0 0x2100000>; > no-map; > }; thanks for explaining. --srini > > > > remoteproc_adsp: remoteproc@30000000 { > compatible = "qcom,sm8450-adsp-pas"; > reg = <0 0x30000000 0 0x100>; > ... > ... > memory-region = <&adsp_mem>; <== > > -Mukesh > >> >> >> --srini >> >>> >>> >>> -Mukesh >>> >>>> >>>> >>>> --srini >>>> >>>>> - } >>>>> - >>>>> - /* Get subsystem table of contents using the minidump id */ >>>>> - subsystem = &toc->subsystems[minidump_id]; >>>>> /** >>>>> * Collect minidump if SS ToC is valid and segment table
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c index 88fc984..240e9f7 100644 --- a/drivers/remoteproc/qcom_common.c +++ b/drivers/remoteproc/qcom_common.c @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, { int ret; struct minidump_subsystem *subsystem; - struct minidump_global_toc *toc; - /* Get Global minidump ToC*/ - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL); - - /* check if global table pointer exists and init is set */ - if (IS_ERR(toc) || !toc->status) { - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n"); + subsystem = qcom_minidump_subsystem_desc(minidump_id); + if (IS_ERR(subsystem)) return; - } - - /* Get subsystem table of contents using the minidump id */ - subsystem = &toc->subsystems[minidump_id]; /** * Collect minidump if SS ToC is valid and segment table
qcom_minidump driver provides qcom_minidump_subsystem_desc() exported API which other driver can use it query subsystem descriptor. Refactor qcom_minidump() to use this symbol. Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> --- drivers/remoteproc/qcom_common.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)