Re: [PATCH 4/8] remoteproc: qcom_q6v5_mss: Add MSM8917

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 12/30/25 4:28 PM, barnabas.czeman@mainlining.org wrote:
> On 2025-12-30 14:11, Konrad Dybcio wrote:
>> On 12/30/25 4:25 AM, barnabas.czeman@mainlining.org wrote:
>>> On 2025-12-29 14:07, Konrad Dybcio wrote:
>>>> On 12/29/25 1:59 PM, barnabas.czeman@mainlining.org wrote:
>>>>> On 2025-12-29 13:51, barnabas.czeman@mainlining.org wrote:
>>>>>> On 2025-12-29 13:40, Konrad Dybcio wrote:
>>>>>>> On 12/29/25 1:33 PM, barnabas.czeman@mainlining.org wrote:
>>>>>>>> On 2025-12-29 12:08, Stephan Gerhold wrote:
>>>>>>>>> On Sun, Dec 28, 2025 at 03:21:54PM +0100, Barnabás Czémán wrote:
>>>>>>>>>> Add support for MSM8917 MSS it is similar for MDM9607 MSS
>>>>>>>>>> only difference is the mss power domain.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>>>>>>>>>> ---
>>>>>>>>>>  drivers/remoteproc/qcom_q6v5_mss.c | 46 ++++++++++++++++++++++++++++++++++++--
>>>>>>>>>>  1 file changed, 44 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>>>>>>>>>> index ffafbe501a05..2579558fb567 100644
>>>>>>>>>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>>>>>>>>>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>>>>>>>>>> @@ -259,6 +259,7 @@ enum {
>>>>>>>>>>      MSS_MSM8226,
>>>>>>>>>>      MSS_MSM8909,
>>>>>>>>>>      MSS_MSM8916,
>>>>>>>>>> +    MSS_MSM8917,
>>>>>>>>>>      MSS_MSM8926,
>>>>>>>>>>      MSS_MSM8953,
>>>>>>>>>>      MSS_MSM8974,
>>>>>>>>>> @@ -749,13 +750,15 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>>>>>>>>>          goto pbl_wait;
>>>>>>>>>>      } else if (qproc->version == MSS_MDM9607 ||
>>>>>>>>>>             qproc->version == MSS_MSM8909 ||
>>>>>>>>>> +           qproc->version == MSS_MSM8917 ||
>>>>>>>>>>             qproc->version == MSS_MSM8953 ||
>>>>>>>>>>             qproc->version == MSS_MSM8996 ||
>>>>>>>>>>             qproc->version == MSS_MSM8998 ||
>>>>>>>>>>             qproc->version == MSS_SDM660) {
>>>>>>>>>>
>>>>>>>>>>          /* Override the ACC value if required */
>>>>>>>>>> -        if (qproc->version == MSS_MDM9607)
>>>>>>>>>> +        if (qproc->version == MSS_MDM9607 ||
>>>>>>>>>> +            qproc->version == MSS_MSM8917)
>>>>>>>>>>              writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
>>>>>>>>>>                     qproc->reg_base + QDSP6SS_STRAP_ACC);
>>>>>>>>>>          else if (qproc->version != MSS_MSM8909 &&
>>>>>>>>>> @@ -817,6 +820,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>>>>>>>>>
>>>>>>>>>>              /* Turn on L1, L2, ETB and JU memories 1 at a time */
>>>>>>>>>>              if (qproc->version == MSS_MDM9607 ||
>>>>>>>>>> +                qproc->version == MSS_MSM8917 ||
>>>>>>>>>>                  qproc->version == MSS_MSM8953 ||
>>>>>>>>>>                  qproc->version == MSS_MSM8996) {
>>>>>>>>>>                  mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
>>>>>>>>>> @@ -826,7 +830,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>>>>>>>>>                   * Set first 5 bits in reverse to avoid
>>>>>>>>>>                   * "inrush current" issues.
>>>>>>>>>>                   */
>>>>>>>>>> -                if (qproc->version == MSS_MDM9607)
>>>>>>>>>> +                if (qproc->version == MSS_MDM9607 ||
>>>>>>>>>> +                    qproc->version == MSS_MSM8917)
>>>>>>>>>>                      reverse = 6;
>>>>>>>>>>              } else {
>>>>>>>>>>                  /* MSS_MSM8998, MSS_SDM660 */
>>>>>>>>>> @@ -2538,6 +2543,42 @@ static const struct rproc_hexagon_res msm8916_mss = {
>>>>>>>>>>      .version = MSS_MSM8916,
>>>>>>>>>>  };
>>>>>>>>>>
>>>>>>>>>> +static const struct rproc_hexagon_res msm8917_mss = {
>>>>>>>>>> +    .hexagon_mba_image = "mba.mbn",
>>>>>>>>>> +    .proxy_supply = (struct qcom_mss_reg_res[]) {
>>>>>>>>>> +        {
>>>>>>>>>> +            .supply = "pll",
>>>>>>>>>> +            .uA = 100000,
>>>>>>>>>> +        },
>>>>>>>>>> +        {}
>>>>>>>>>> +    },
>>>>>>>>>> +    .proxy_clk_names = (char*[]){
>>>>>>>>>> +        "xo",
>>>>>>>>>> +        NULL
>>>>>>>>>> +    },
>>>>>>>>>> +    .active_clk_names = (char*[]){
>>>>>>>>>> +        "iface",
>>>>>>>>>> +        "bus",
>>>>>>>>>> +        "mem",
>>>>>>>>>> +        NULL
>>>>>>>>>> +    },
>>>>>>>>>> +    .proxy_pd_names = (char*[]) {
>>>>>>>>>> +        "cx",
>>>>>>>>>> +        "mx",
>>>>>>>>>> +        "mss",
>>>>>>>>>
>>>>>>>>> Are you sure mss/pm8937_s1 also works as a power domain? It seems to be
>>>>>>>>> a plain regulator downstream (similar to msm8226/msm8974).
>>>>>>>>>
>>>>>>>>> Same thing applies to MSM8953 as well though and there we seem to have
>>>>>>>>> decided to model it as a power domain ...
>>>>>>>> They have this at downstream, i guess this is why handled as a power domain.
>>>>>>>> vdd_mss-uV = <RPM_SMD_REGULATOR_LEVEL_TURBO>;
>>>>>>>
>>>>>>> It seems to be just a normal regulator on both 8917 and 8953
>>>>>>>
>>>>>>> https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/arch/arm/boot/dts/qcom/msm8953-regulator.dtsi
>>>>>>> https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/arch/arm/boot/dts/qcom/msm8953.dtsi
>>>>>>>
>>>>>>> https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/arch/arm/boot/dts/qcom/msm8917-regulator.dtsi
>>>>>>> https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/arch/arm/boot/dts/qcom/msm8917.dtsi
>>>>>>>
>>>>>>> Konrad
>>>>>> It is different between 3.18 and 4.9, 3.18 seems simpler and easier to use it as regulator i will change it
>>>>>> https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/arch/arm64/boot/dts/qcom/msm8917.dtsi#L1375
>>>>>> https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.6.6.c32-05500-89xx.0/arch/arm/boot/dts/qcom/msm8917.dtsi#L1655
>>>>> Only question is left what about SDM439 and SDM632? They were introduced at 4.9 maybe that is why it was modified both are inheriting
>>>>> MSS bindings from 8953 and 8937.
>>>>
>>>> Seems both are the same too
>>> https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/arch/arm64/boot/dts/qcom/sdm439-regulator.dtsi#L18
>>> https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/arch/arm64/boot/dts/qcom/msm8953-regulator.dtsi#L17
>>> SDM439 and SDM632 do not use simple regulator, s1 seems power domain to me. I have checked it in use at Fairphone 3, Redmi 7 and 7A
>>> all have S1 as a power domain.
>>
>> Hm, it seems you're right. It does indeed use level-based voting which
>> would qualify it as a power domain in our upstream understanding
> What would be the better solution?
> 1. Keep mss as power domain?
> 2. Set mss to a regulator for MSM8917 and keep as power domain for MSM8937 and SDM439.
> 3. Split SDM632 and SDM439 MSS from MSM8937 and MSM8953 MSS and use mss-supply
> as regulator for MSM8917, MSM8937, MSM8940 and MSM8953 and use mss as power domain for SDM439 and SDM632.

8917 and 8937 -> regulator
439 and 632 -> power domain

To match reality as we've established above

Konrad