[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <397fc3d6419d1047b5f99dc4b2d02593@mainlining.org>
Date: Tue, 30 Dec 2025 16:28:42 +0100
From: barnabas.czeman@...nlining.org
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Stephan Gerhold <stephan.gerhold@...aro.org>, Bjorn Andersson
<andersson@...nel.org>, Mathieu Poirier <mathieu.poirier@...aro.org>, Rob
Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
Dooley <conor+dt@...nel.org>, Stephan Gerhold <stephan@...hold.net>,
linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/8] remoteproc: qcom_q6v5_mss: Add MSM8917
On 2025-12-30 14:11, Konrad Dybcio wrote:
> On 12/30/25 4:25 AM, barnabas.czeman@...nlining.org wrote:
>> On 2025-12-29 14:07, Konrad Dybcio wrote:
>>> On 12/29/25 1:59 PM, barnabas.czeman@...nlining.org wrote:
>>>> On 2025-12-29 13:51, barnabas.czeman@...nlining.org wrote:
>>>>> On 2025-12-29 13:40, Konrad Dybcio wrote:
>>>>>> On 12/29/25 1:33 PM, barnabas.czeman@...nlining.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@...nlining.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.
>
> Konrad
Powered by blists - more mailing lists