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

[Manikanta Mylavarapu <quic_mmanikan@quicinc.com>]

On 5/21/2023 11:40 PM, Dmitry Baryshkov wrote:
> On Sun, 21 May 2023 at 18:48, Manikanta Mylavarapu
> <quic_mmanikan@quicinc.com> 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@quicinc.com>
>>>> ---
>>>>    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.
> 

I will discuss with internal team and update in V3 series.

Thanks & Regards,
Manikanta.