| Message ID | 20241122064428.278752-3-quic_liuxin@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable UFS on QCS615 | expand |
On Fri, Nov 22, 2024 at 02:44:27PM +0800, 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> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > arch/arm64/boot/dts/qcom/qcs615.dtsi | 114 +++++++++++++++++++++++++++ > 1 file changed, 114 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi > index 590beb37f441..5e501511e6db 100644 > --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi > +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi > @@ -458,6 +458,120 @@ mmss_noc: interconnect@1740000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > + ufs_mem_hc: ufshc@1d84000 { > + compatible = "qcom,qcs615-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; > + reg = <0x0 0x01d84000 0x0 0x3000>, > + <0x0 0x01d90000 0x0 0x8000>; > + reg-names = "std", > + "ice"; > + > + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, > + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, > + <&gcc GCC_UFS_PHY_AHB_CLK>, > + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>, > + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, > + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>; > + clock-names = "core_clk", > + "bus_aggr_clk", > + "iface_clk", > + "core_clk_unipro", > + "core_clk_ice", > + "ref_clk", > + "tx_lane0_sync_clk", > + "rx_lane0_sync_clk"; > + > + resets = <&gcc GCC_UFS_PHY_BCR>; > + reset-names = "rst"; > + > + 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>; > + interconnect-names = "ufs-ddr", > + "cpu-ufs"; > + > + power-domains = <&gcc UFS_PHY_GDSC>; > + required-opps = <&rpmhpd_opp_nom>; > + > + iommus = <&apps_smmu 0x300 0x0>; > + dma-coherent; > + > + lanes-per-direction = <1>; > + > + phys = <&ufs_mem_phy>; > + phy-names = "ufsphy"; > + > + #reset-cells = <1>; > + > + status = "disabled"; > + > + ufs_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-50000000 { > + opp-hz = /bits/ 64 <50000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <37500000>, > + /bits/ 64 <75000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <0>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-100000000 { > + opp-hz = /bits/ 64 <100000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <75000000>, > + /bits/ 64 <150000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <0>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-200000000 { > + opp-hz = /bits/ 64 <200000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <150000000>, > + /bits/ 64 <300000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <0>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > + }; > + > + ufs_mem_phy: phy@1d87000 { > + compatible = "qcom,qcs615-qmp-ufs-phy", "qcom,sm6115-qmp-ufs-phy"; > + reg = <0x0 0x01d87000 0x0 0xe00>; > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>, > + <&gcc GCC_UFS_MEM_CLKREF_CLK>; > + clock-names = "ref", > + "ref_aux", > + "qref"; > + > + power-domains = <&gcc UFS_PHY_GDSC>; > + > + resets = <&ufs_mem_hc 0>; > + reset-names = "ufsphy"; > + > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > tcsr_mutex: hwlock@1f40000 { > compatible = "qcom,tcsr-mutex"; > reg = <0x0 0x01f40000 0x0 0x20000>; > -- > 2.34.1 >
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 > + 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: > + > + iommus = <&apps_smmu 0x300 0x0>; > + dma-coherent; > + > + lanes-per-direction = <1>; > + > + phys = <&ufs_mem_phy>; > + phy-names = "ufsphy"; > + > + #reset-cells = <1>; > + > + status = "disabled"; > + > + ufs_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-50000000 { > + opp-hz = /bits/ 64 <50000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <37500000>, > + /bits/ 64 <75000000>, > + /bits/ 64 <0>, > + /bits/ 64 <0>, > + /bits/ 64 <0>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + Konrad
在 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? > >> + 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. > >> + >> + iommus = <&apps_smmu 0x300 0x0>; >> + dma-coherent; >> + >> + lanes-per-direction = <1>; >> + >> + phys = <&ufs_mem_phy>; >> + phy-names = "ufsphy"; >> + >> + #reset-cells = <1>; >> + >> + status = "disabled"; >> + >> + ufs_opp_table: opp-table { >> + compatible = "operating-points-v2"; >> + >> + opp-50000000 { >> + opp-hz = /bits/ 64 <50000000>, >> + /bits/ 64 <0>, >> + /bits/ 64 <0>, >> + /bits/ 64 <37500000>, >> + /bits/ 64 <75000000>, >> + /bits/ 64 <0>, >> + /bits/ 64 <0>, >> + /bits/ 64 <0>; >> + required-opps = <&rpmhpd_opp_low_svs>; >> + }; >> + > > Konrad
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
diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi index 590beb37f441..5e501511e6db 100644 --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi @@ -458,6 +458,120 @@ mmss_noc: interconnect@1740000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + ufs_mem_hc: ufshc@1d84000 { + compatible = "qcom,qcs615-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; + reg = <0x0 0x01d84000 0x0 0x3000>, + <0x0 0x01d90000 0x0 0x8000>; + reg-names = "std", + "ice"; + + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>, + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>, + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>, + <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>; + clock-names = "core_clk", + "bus_aggr_clk", + "iface_clk", + "core_clk_unipro", + "core_clk_ice", + "ref_clk", + "tx_lane0_sync_clk", + "rx_lane0_sync_clk"; + + resets = <&gcc GCC_UFS_PHY_BCR>; + reset-names = "rst"; + + 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>; + interconnect-names = "ufs-ddr", + "cpu-ufs"; + + power-domains = <&gcc UFS_PHY_GDSC>; + required-opps = <&rpmhpd_opp_nom>; + + iommus = <&apps_smmu 0x300 0x0>; + dma-coherent; + + lanes-per-direction = <1>; + + phys = <&ufs_mem_phy>; + phy-names = "ufsphy"; + + #reset-cells = <1>; + + status = "disabled"; + + ufs_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-50000000 { + opp-hz = /bits/ 64 <50000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <37500000>, + /bits/ 64 <75000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <0>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-100000000 { + opp-hz = /bits/ 64 <100000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <75000000>, + /bits/ 64 <150000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <0>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-200000000 { + opp-hz = /bits/ 64 <200000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <150000000>, + /bits/ 64 <300000000>, + /bits/ 64 <0>, + /bits/ 64 <0>, + /bits/ 64 <0>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + }; + + ufs_mem_phy: phy@1d87000 { + compatible = "qcom,qcs615-qmp-ufs-phy", "qcom,sm6115-qmp-ufs-phy"; + reg = <0x0 0x01d87000 0x0 0xe00>; + clocks = <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>, + <&gcc GCC_UFS_MEM_CLKREF_CLK>; + clock-names = "ref", + "ref_aux", + "qref"; + + power-domains = <&gcc UFS_PHY_GDSC>; + + resets = <&ufs_mem_hc 0>; + reset-names = "ufsphy"; + + #clock-cells = <1>; + #phy-cells = <0>; + + status = "disabled"; + }; + tcsr_mutex: hwlock@1f40000 { compatible = "qcom,tcsr-mutex"; reg = <0x0 0x01f40000 0x0 0x20000>;