Re: [PATCH v2 6/6] remoterproc: qcom: refactor to leverage exported minidump symbol

[Mukesh Ojha]

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@xxxxxxxxxxx>
> > ---
> >   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