[PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Bjorn Andersson]

The UFS host controller occationally (20%) fails to enable
gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
enabled through the UFS phy driver, but to make sure it's enabled let's
enable it directly from the UFS host controller directly as well.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..03c7904bda14 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -674,6 +674,8 @@
 			vccq-max-microamp = <450000>;
 			vccq2-max-microamp = <450000>;
 
+			power-domains = <&gcc UFS_GDSC>;
+
 			clock-names =
 				"core_clk_src",
 				"core_clk",
-- 
2.17.0

[PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Bjorn Andersson]

The UFS host controller occationally (20%) fails to enable
gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
enabled through the UFS phy driver, but to make sure it's enabled let's
enable it directly from the UFS host controller directly as well.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..03c7904bda14 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -674,6 +674,8 @@
 			vccq-max-microamp = <450000>;
 			vccq2-max-microamp = <450000>;
 
+			power-domains = <&gcc UFS_GDSC>;
+
 			clock-names =
 				"core_clk_src",
 				"core_clk",
-- 
2.17.0

Re: [PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Vivek Gautam]

Hi Bjorn,

On Fri, May 25, 2018 at 4:01 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> The UFS host controller occationally (20%) fails to enable
> gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
> enabled through the UFS phy driver, but to make sure it's enabled let's
> enable it directly from the UFS host controller directly as well.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 380e14591686..03c7904bda14 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -674,6 +674,8 @@
>                         vccq-max-microamp = <450000>;
>                         vccq2-max-microamp = <450000>;
>
> +                       power-domains = <&gcc UFS_GDSC>;
> +

We shouldn't need power-domain with the phy. UFS_GDSC should
be attached to the controller, as the phy is powered up only after
the controller is power-up, and during collapse too, we turn off
the phy first.
Can you try testing keeping UFS_GDSC only with ufs controller and
remove it from the ufs-phy node? We are doing same on the 4.14 release
branch too for db820.
I apologize to have missed this in your patch for ufs-related dt nodes.
Can we please fix this now?

Best regards
Vivek

>                         clock-names =
>                                 "core_clk_src",
>                                 "core_clk",
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Vivek Gautam]

Hi Bjorn,

On Fri, May 25, 2018 at 4:01 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> The UFS host controller occationally (20%) fails to enable
> gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
> enabled through the UFS phy driver, but to make sure it's enabled let's
> enable it directly from the UFS host controller directly as well.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 380e14591686..03c7904bda14 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -674,6 +674,8 @@
>                         vccq-max-microamp = <450000>;
>                         vccq2-max-microamp = <450000>;
>
> +                       power-domains = <&gcc UFS_GDSC>;
> +

We shouldn't need power-domain with the phy. UFS_GDSC should
be attached to the controller, as the phy is powered up only after
the controller is power-up, and during collapse too, we turn off
the phy first.
Can you try testing keeping UFS_GDSC only with ufs controller and
remove it from the ufs-phy node? We are doing same on the 4.14 release
branch too for db820.
I apologize to have missed this in your patch for ufs-related dt nodes.
Can we please fix this now?

Best regards
Vivek

>                         clock-names =
>                                 "core_clk_src",
>                                 "core_clk",
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Bjorn Andersson]

On Fri 25 May 05:51 PDT 2018, Vivek Gautam wrote:

> Hi Bjorn,
> 
> On Fri, May 25, 2018 at 4:01 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > The UFS host controller occationally (20%) fails to enable
> > gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
> > enabled through the UFS phy driver, but to make sure it's enabled let's
> > enable it directly from the UFS host controller directly as well.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > index 380e14591686..03c7904bda14 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > @@ -674,6 +674,8 @@
> >                         vccq-max-microamp = <450000>;
> >                         vccq2-max-microamp = <450000>;
> >
> > +                       power-domains = <&gcc UFS_GDSC>;
> > +
> 
> We shouldn't need power-domain with the phy. UFS_GDSC should
> be attached to the controller, as the phy is powered up only after
> the controller is power-up, and during collapse too, we turn off
> the phy first.

Afaict you're right, it should only be needed by the resources for the
HCD.

> Can you try testing keeping UFS_GDSC only with ufs controller and
> remove it from the ufs-phy node? We are doing same on the 4.14 release
> branch too for db820.

I test booted this 10 times, with 100% success :)

> I apologize to have missed this in your patch for ufs-related dt nodes.
> Can we please fix this now?

I'll send a v2, that moves the power-domain to the HCD, rather than just
adding it there too. Thanks for the quick review!

Regards,
Bjorn

[PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS

[Bjorn Andersson]

On Fri 25 May 05:51 PDT 2018, Vivek Gautam wrote:

> Hi Bjorn,
> 
> On Fri, May 25, 2018 at 4:01 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > The UFS host controller occationally (20%) fails to enable
> > gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
> > enabled through the UFS phy driver, but to make sure it's enabled let's
> > enable it directly from the UFS host controller directly as well.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > index 380e14591686..03c7904bda14 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > @@ -674,6 +674,8 @@
> >                         vccq-max-microamp = <450000>;
> >                         vccq2-max-microamp = <450000>;
> >
> > +                       power-domains = <&gcc UFS_GDSC>;
> > +
> 
> We shouldn't need power-domain with the phy. UFS_GDSC should
> be attached to the controller, as the phy is powered up only after
> the controller is power-up, and during collapse too, we turn off
> the phy first.

Afaict you're right, it should only be needed by the resources for the
HCD.

> Can you try testing keeping UFS_GDSC only with ufs controller and
> remove it from the ufs-phy node? We are doing same on the 4.14 release
> branch too for db820.

I test booted this 10 times, with 100% success :)

> I apologize to have missed this in your patch for ufs-related dt nodes.
> Can we please fix this now?

I'll send a v2, that moves the power-domain to the HCD, rather than just
adding it there too. Thanks for the quick review!

Regards,
Bjorn

[PATCH v2] arm64: dts: qcom: msm8996: Move UFS_GDSC to UFS HCD

[Bjorn Andersson]

The UFS_GDSC backs the resources needed by the UFS HCD, rather than the
PHY. This results in the UFS HCD occationally failing to enable some of
its clocks.

Move the UFS_GDSC reference to the UFS HCD node instead, to correct
this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Dropped UFS_GDSC from phy node

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..8c7f9ca25b53 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -654,8 +654,6 @@
 			clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
 				 <&gcc GCC_UFS_CLKREF_CLK>;
 			status = "disabled";
-
-			power-domains = <&gcc UFS_GDSC>;
 		};
 
 		ufshc@624000 {
@@ -674,6 +672,8 @@
 			vccq-max-microamp = <450000>;
 			vccq2-max-microamp = <450000>;
 
+			power-domains = <&gcc UFS_GDSC>;
+
 			clock-names =
 				"core_clk_src",
 				"core_clk",
-- 
2.17.0

[PATCH v2] arm64: dts: qcom: msm8996: Move UFS_GDSC to UFS HCD

[Bjorn Andersson]

The UFS_GDSC backs the resources needed by the UFS HCD, rather than the
PHY. This results in the UFS HCD occationally failing to enable some of
its clocks.

Move the UFS_GDSC reference to the UFS HCD node instead, to correct
this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Dropped UFS_GDSC from phy node

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..8c7f9ca25b53 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -654,8 +654,6 @@
 			clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
 				 <&gcc GCC_UFS_CLKREF_CLK>;
 			status = "disabled";
-
-			power-domains = <&gcc UFS_GDSC>;
 		};
 
 		ufshc at 624000 {
@@ -674,6 +672,8 @@
 			vccq-max-microamp = <450000>;
 			vccq2-max-microamp = <450000>;
 
+			power-domains = <&gcc UFS_GDSC>;
+
 			clock-names =
 				"core_clk_src",
 				"core_clk",
-- 
2.17.0

Re: [PATCH v2] arm64: dts: qcom: msm8996: Move UFS_GDSC to UFS HCD

[Vivek Gautam]

On Sat, May 26, 2018 at 12:15 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> The UFS_GDSC backs the resources needed by the UFS HCD, rather than the
> PHY. This results in the UFS HCD occationally failing to enable some of
> its clocks.
>
> Move the UFS_GDSC reference to the UFS HCD node instead, to correct
> this.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Dropped UFS_GDSC from phy node
>
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 380e14591686..8c7f9ca25b53 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -654,8 +654,6 @@
>                         clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
>                                  <&gcc GCC_UFS_CLKREF_CLK>;
>                         status = "disabled";
> -
> -                       power-domains = <&gcc UFS_GDSC>;
>                 };
>
>                 ufshc@624000 {
> @@ -674,6 +672,8 @@
>                         vccq-max-microamp = <450000>;
>                         vccq2-max-microamp = <450000>;
>
> +                       power-domains = <&gcc UFS_GDSC>;
> +

Looks good.
Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>

Regards
Vivek
>                         clock-names =
>                                 "core_clk_src",
>                                 "core_clk",
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2] arm64: dts: qcom: msm8996: Move UFS_GDSC to UFS HCD

[Vivek Gautam]

On Sat, May 26, 2018 at 12:15 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> The UFS_GDSC backs the resources needed by the UFS HCD, rather than the
> PHY. This results in the UFS HCD occationally failing to enable some of
> its clocks.
>
> Move the UFS_GDSC reference to the UFS HCD node instead, to correct
> this.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Dropped UFS_GDSC from phy node
>
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 380e14591686..8c7f9ca25b53 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -654,8 +654,6 @@
>                         clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
>                                  <&gcc GCC_UFS_CLKREF_CLK>;
>                         status = "disabled";
> -
> -                       power-domains = <&gcc UFS_GDSC>;
>                 };
>
>                 ufshc at 624000 {
> @@ -674,6 +672,8 @@
>                         vccq-max-microamp = <450000>;
>                         vccq2-max-microamp = <450000>;
>
> +                       power-domains = <&gcc UFS_GDSC>;
> +

Looks good.
Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>

Regards
Vivek
>                         clock-names =
>                                 "core_clk_src",
>                                 "core_clk",
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation