Re: [PATCH v3 2/3] arm64: dts: qcom: qcs615: add UFS node

Konrad Dybcio
2024-12-12T17:10:23Z

On 9.12.2024 8:56 AM, Xin Liu wrote:
>
>
> 在 2024/12/6 5:21, Konrad Dybcio 写道:
>> On 22.11.2024 7:44 AM, Xin Liu wrote:
>>> From: Sayali Lokhande <quic_sayalil@quicinc.com>
>>>
>>> Add the UFS Host Controller node and its PHY for QCS615 SoC.
>>>
>>> Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
>>> Co-developed-by: Xin Liu <quic_liuxin@quicinc.com>
>>> Signed-off-by: Xin Liu <quic_liuxin@quicinc.com>
>>> ---
>>
>> [...]
>>
>>> +
>>> +            operating-points-v2 = <&ufs_opp_table>;
>>> +            interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
>>> +                     &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>> +                    <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>> +                     &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;
>>
>> QCOM_ICC_TAG_ACTIVE_ONLY for the cpu path
> I need to ask you for advice. I have reviewed the ufs_mem_hc of many devices and found that all of them use QCOM_ICC_TAG_ALWAYS for their interconnects cpu path. Why do I need to use QCOM_ICC_TAG_ACTIVE_ONLY here?

QCOM_ICC_TAG_ACTIVE_ONLY instructs RPMh to shut off the interconnect
path when the CPUs go offline (without OS intervention) to save power
and bus bandwidth.

It's the natural choice for paths that directly connect hardware to
the CPU, as nothing else should be accessing these ports.

Currently, many platforms do not set that, because nobody cared enough
to point it out :(

One day when we lay some more groundwork on the suspend side, I'll
send a treewide fixup.

>>
>>> +            interconnect-names = "ufs-ddr",
>>> +                         "cpu-ufs";
>>> +
>>> +            power-domains = <&gcc UFS_PHY_GDSC>;
>>> +            required-opps = <&rpmhpd_opp_nom>;
>>
>> this contradicts the levels in the OPP table:
> The required-opps here corresponds to opp-200000000 in the opp_table below. Similarly, I referred to sm8550.dtsi, whose required-opps also corresponds to the opp table.

What I'm saying is, specifying required-opps of NOM here will make
VDD_CX always stay at >= NOM, because the vote is max()-ed

Konrad