Re: [PATCH 08/11] remoteproc: qcom: Add Hexagon based multipd rproc driver

[Dmitry Baryshkov]

On Sun, 21 May 2023 at 18:48, Manikanta Mylavarapu
<quic_mmanikan@xxxxxxxxxxx> wrote:
>
>
>
> On 3/7/2023 6:47 PM, Sricharan Ramabadhran wrote:
> >
> >
> > On 3/7/2023 10:11 AM, Manikanta Mylavarapu wrote:
> >> APSS brings Q6 out of reset and then Q6 brings
> >> WCSS block (wifi radio's) out of reset.
> >>
> >>                    ---------------
> >>                   -->  |WiFi 2G radio|
> >>                   |       --------------
> >>                   |
> >> --------    -------          |
> >> | APSS | --->   |QDSP6|  -----|
> >> ---------    -------       |
> >>                                |
> >>                          |
> >>                   |   --------------
> >>                   --> |WiFi 5G radio|
> >>                   --------------
> >>
> >> Problem here is if any radio crashes, subsequently other
> >> radio also should crash because Q6 crashed. Let's say
> >> 2G radio crashed, Q6 should pass this info to APSS. Only
> >> Q6 processor interrupts registered with APSS. Obviously
> >> Q6 should crash and raise fatal interrupt to APSS. Due
> >> to this 5G radio also crashed. But no issue in 5G radio,
> >> because of 2G radio crash 5G radio also impacted.
> >>
> >> In multi pd model, this problem is resolved. Here WCSS
> >> functionality (WiFi radio's) moved out from Q6 root pd
> >> to a separate user pd. Due to this, radio's independently
> >> pass their status info to APPS with out crashing Q6. So
> >> other radio's won't be impacted.
> >>
> >>                         ---------
> >>                             |WiFi    |
> >>                         --> |2G radio|
> >>                         |     ---------
> >> ------    Start Q6             -------     |
> >> |    |    ------------------>     |     |     |
> >> |    |  Start WCSS PD1 (2G)       |     |        |
> >> |APSS|    ----------------------->|QDSP6|-----|
> >> |    |    Start WCSS PD1 (5G)    |     |
> >> |    |    ----------------------->|     |-----|
> >> ------                     -------     |
> >>                         |
> >>                         |    -----------
> >>                         |-->|WiFi      |
> >>                         |5G radio |
> >>                         -----------
> >> According to linux terminology, here consider Q6 as root
> >> i.e it provide all services, WCSS (wifi radio's) as user
> >> i.e it uses services provided by root.
> >>
> >> Since Q6 root & WCSS user pd's able to communicate with
> >> APSS individually, multipd remoteproc driver registers
> >> each PD with rproc framework. Here clients (Wifi host drivers)
> >> intrested on WCSS PD rproc, so multipd driver start's root
> >> pd in the context of WCSS user pd rproc start. Similarly
> >> on down path, root pd will be stopped after wcss user pd
> >> stopped.
> >>
> >> Here WCSS(user) PD is dependent on Q6(root) PD, so first
> >> q6 pd should be up before wcss pd. After wcss pd goes down,
> >> q6 pd should be turned off.
> >>
> >> rproc->ops->start(userpd_rproc) {
> >>     /* Boot root pd rproc */
> >>     rproc_boot(upd_dev->parent);
> >>     ---
> >>     /* user pd rproc start sequence */
> >>     ---
> >>     ---
> >> }
> >> With this way we ensure that root pd brought up before userpd.
> >>
> >> rproc->ops->stop(userpd_rproc) {
> >>     ---
> >>     ---
> >>     /* user pd rproc stop sequence */
> >>     ---
> >>     ---
> >>     /* Shutdown root pd rproc */
> >>     rproc_shutdown(upd_dev->parent);
> >> }
> >> After userpd rproc stops, root pd rproc will be stopped.
> >> IPQ5018, IPQ9574 supports multipd remoteproc driver.
> >>
> >> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@xxxxxxxxxxx>
> >> ---
> >>   drivers/firmware/qcom_scm.c            | 114 +++++
> >>   drivers/firmware/qcom_scm.h            |   6 +
> >>   drivers/remoteproc/Kconfig             |  20 +
> >>   drivers/remoteproc/Makefile            |   1 +
> >>   drivers/remoteproc/qcom_common.c       |  23 +
> >>   drivers/remoteproc/qcom_common.h       |   1 +
> >>   drivers/remoteproc/qcom_q6v5.c         |  41 +-
> >>   drivers/remoteproc/qcom_q6v5.h         |  15 +-
> >>   drivers/remoteproc/qcom_q6v5_adsp.c    |   5 +-
> >>   drivers/remoteproc/qcom_q6v5_mpd.c     | 668 +++++++++++++++++++++++++
> >>   drivers/remoteproc/qcom_q6v5_mss.c     |   4 +-
> >>   drivers/remoteproc/qcom_q6v5_pas.c     |   3 +-
> >>   drivers/soc/qcom/mdt_loader.c          | 314 ++++++++++++
> >>   include/linux/firmware/qcom/qcom_scm.h |   3 +
> >>   include/linux/soc/qcom/mdt_loader.h    |  19 +
> >>   15 files changed, 1228 insertions(+), 9 deletions(-)
> >>   create mode 100644 drivers/remoteproc/qcom_q6v5_mpd.c
> >>
> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> >> index d88c5f14bd54..d69560963353 100644
> >> --- a/drivers/firmware/qcom_scm.c
> >> +++ b/drivers/firmware/qcom_scm.c
> >> @@ -654,6 +654,120 @@ int qcom_scm_pas_shutdown(u32 peripheral)
> >>   }
> >>   EXPORT_SYMBOL(qcom_scm_pas_shutdown);
> >> +/**
> >> + * qti_scm_int_radio_powerup - Bring up internal radio userpd
> >> + *
> >> + * @peripheral:    peripheral id
> >> + *
> >> + * Return 0 on success.
> >> + */
> >> +int qti_scm_int_radio_powerup(u32 peripheral)
> >> +{
> >
> > qcom instead and in other places too.
> >
> Internal open source team suggested to use qti.

All existing drivers use qcom_ prefix. Other functions in qcom_scm.c
use qcom_ prefix. Please stick to it.

-- 
With best wishes
Dmitry