| Message ID | 20220303082140.240745-3-bhupesh.sharma@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add ufs power-domain entries for SM8150 | expand |
On Thu, 3 Mar 2022 at 11:22, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: > > Add power-domain entries for UFS controller & phy nodes > in sm8150 dts. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index 6012322a5984..7aa879eb24d7 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -1637,6 +1637,8 @@ ufs_mem_hc: ufshc@1d84000 { > phy-names = "ufsphy"; > lanes-per-direction = <2>; > #reset-cells = <1>; > + > + power-domains = <&gcc UFS_PHY_GDSC>; > resets = <&gcc GCC_UFS_PHY_BCR>; > reset-names = "rst"; > > @@ -1687,6 +1689,9 @@ ufs_mem_phy: phy@1d87000 { > clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, > <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > > + power-domains = <&gcc UFS_CARD_GDSC>, > + <&gcc UFS_PHY_GDSC>; > + power-domain-names = "ufs_card_gdsc", "ufs_phy_gdsc"; This will not work, if I'm not mistaken. Platform drivers won't bind two power-domains by default. And the qmp driver lacks handling for power domains. Also a generic question. I see that other platforms use UFS_PHY_GDSC for the host controller and completely ingore the UFS_CARD_GDSC. What makes sm8150 so different from the rest of the platforms? > resets = <&ufs_mem_hc 0>; > reset-names = "ufsphy"; > status = "disabled"; > -- > 2.35.1 >
On Thu 03 Mar 02:21 CST 2022, Bhupesh Sharma wrote: > Add power-domain entries for UFS controller & phy nodes > in sm8150 dts. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index 6012322a5984..7aa879eb24d7 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -1637,6 +1637,8 @@ ufs_mem_hc: ufshc@1d84000 { > phy-names = "ufsphy"; > lanes-per-direction = <2>; > #reset-cells = <1>; > + > + power-domains = <&gcc UFS_PHY_GDSC>; It seems odd that the controller would be in the PHY power-domain? > resets = <&gcc GCC_UFS_PHY_BCR>; > reset-names = "rst"; > > @@ -1687,6 +1689,9 @@ ufs_mem_phy: phy@1d87000 { > clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, > <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > > + power-domains = <&gcc UFS_CARD_GDSC>, > + <&gcc UFS_PHY_GDSC>; And "card" is typically related to the second UFS interface, so I suspect you only would need the last one of these? Regards, Bjorn > + power-domain-names = "ufs_card_gdsc", "ufs_phy_gdsc"; > resets = <&ufs_mem_hc 0>; > reset-names = "ufsphy"; > status = "disabled"; > -- > 2.35.1 >
Hi Dmitry, On Thu, 3 Mar 2022 at 16:02, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 3 Mar 2022 at 11:22, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: > > > > Add power-domain entries for UFS controller & phy nodes > > in sm8150 dts. > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Rob Herring <robh@kernel.org> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > index 6012322a5984..7aa879eb24d7 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > @@ -1637,6 +1637,8 @@ ufs_mem_hc: ufshc@1d84000 { > > phy-names = "ufsphy"; > > lanes-per-direction = <2>; > > #reset-cells = <1>; > > + > > + power-domains = <&gcc UFS_PHY_GDSC>; > > resets = <&gcc GCC_UFS_PHY_BCR>; > > reset-names = "rst"; > > > > @@ -1687,6 +1689,9 @@ ufs_mem_phy: phy@1d87000 { > > clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, > > <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > > > > + power-domains = <&gcc UFS_CARD_GDSC>, > > + <&gcc UFS_PHY_GDSC>; > > + power-domain-names = "ufs_card_gdsc", "ufs_phy_gdsc"; > > This will not work, if I'm not mistaken. Platform drivers won't bind > two power-domains by default. And the qmp driver lacks handling for > power domains. > > Also a generic question. I see that other platforms use UFS_PHY_GDSC > for the host controller and completely ingore the UFS_CARD_GDSC. What > makes sm8150 so different from the rest of the platforms? You are right. I used the UFS power-domain constructs presently used downstream, but I think using UFS_PHY_GDSC only is sufficient for our use case upstream. So, I will send a fixed v4 version. Thanks, Bhupesh > > resets = <&ufs_mem_hc 0>; > > reset-names = "ufsphy"; > > status = "disabled"; > > -- > > 2.35.1 > > > > > -- > With best wishes > Dmitry
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index 6012322a5984..7aa879eb24d7 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -1637,6 +1637,8 @@ ufs_mem_hc: ufshc@1d84000 { phy-names = "ufsphy"; lanes-per-direction = <2>; #reset-cells = <1>; + + power-domains = <&gcc UFS_PHY_GDSC>; resets = <&gcc GCC_UFS_PHY_BCR>; reset-names = "rst"; @@ -1687,6 +1689,9 @@ ufs_mem_phy: phy@1d87000 { clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; + power-domains = <&gcc UFS_CARD_GDSC>, + <&gcc UFS_PHY_GDSC>; + power-domain-names = "ufs_card_gdsc", "ufs_phy_gdsc"; resets = <&ufs_mem_hc 0>; reset-names = "ufsphy"; status = "disabled";
Add power-domain entries for UFS controller & phy nodes in sm8150 dts. Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Rob Herring <robh@kernel.org> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 5 +++++ 1 file changed, 5 insertions(+)