linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/4] UFS on APQ8098
@ 2019-01-16 10:50 Marc Gonzalez
  2019-01-16 10:53 ` [RFC PATCH v2 1/4] arm64: dts: qcom: msm8998: Add rpmcc node Marc Gonzalez
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-16 10:50 UTC (permalink / raw)
  To: MSM; +Cc: LKML, Jeffrey Hugo, Bjorn Andersson, Andy Gross

Hello,

After weeks (literally!) of poking the system at random, Jeffrey found why UFS
refused to work on APQ8098: we were not setting the load on the vregs.

Difference between v1 and v2:
- New patch to add 'regulator-allow-set-load' prop to all vreg nodes
- Rename rpmcc node to 'clock-controller' + Add Review tags
- Drop UFS pinctrl gymnastics (not required, probably left enabled in bootloader)
- Delete GCC_UFS_ICE_CORE_CLK (ICE not used upstream, I think)
- Fix sizes of ufsphy register areas based on Jeffrey's feedback
- Hack ufshcd_set_vccq_rail_unused into a NOP to work around lock up + reboot


Marc Gonzalez (4):
  arm64: dts: qcom: msm8998: Add rpmcc node
  arm64: dts: qcom: msm8998: Add UFS nodes
  Add regulator-allow-set-load
  ufshcd_set_vccq_rail_unused locks up the board

 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 52 +++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8998.dtsi     | 69 +++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c                 |  1 +
 3 files changed, 122 insertions(+)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH v2 1/4] arm64: dts: qcom: msm8998: Add rpmcc node
  2019-01-16 10:50 [RFC PATCH v2 0/4] UFS on APQ8098 Marc Gonzalez
@ 2019-01-16 10:53 ` Marc Gonzalez
  2019-01-16 10:56 ` [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes Marc Gonzalez
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-16 10:53 UTC (permalink / raw)
  To: MSM; +Cc: LKML, Jeffrey Hugo, Bjorn Andersson, Andy Gross, Rob Herring

Add MSM8998 Resource Power Manager Clock Controller DT node.

Reviewed-by: Jeffrey Hugo <jhugo@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index a6d66cf77403..6f4f4b79853b 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -3,6 +3,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-msm8998.h>
+#include <dt-bindings/clock/qcom,rpmcc.h>
 #include <dt-bindings/gpio/gpio.h>
 
 / {
@@ -266,6 +267,11 @@
 		rpm_requests: rpm-requests {
 			compatible = "qcom,rpm-msm8998";
 			qcom,glink-channels = "rpm_requests";
+
+			rpmcc: clock-controller {
+				compatible = "qcom,rpmcc-msm8998", "qcom,rpmcc";
+				#clock-cells = <1>;
+			};
 		};
 	};
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes
  2019-01-16 10:50 [RFC PATCH v2 0/4] UFS on APQ8098 Marc Gonzalez
  2019-01-16 10:53 ` [RFC PATCH v2 1/4] arm64: dts: qcom: msm8998: Add rpmcc node Marc Gonzalez
@ 2019-01-16 10:56 ` Marc Gonzalez
  2019-01-16 15:36   ` Jeffrey Hugo
  2019-01-16 10:58 ` [RFC PATCH v2 3/4] Add regulator-allow-set-load Marc Gonzalez
  2019-01-16 11:00 ` [RFC PATCH v2 4/4] ufshcd_set_vccq_rail_unused locks up the board Marc Gonzalez
  3 siblings, 1 reply; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-16 10:56 UTC (permalink / raw)
  To: MSM; +Cc: LKML, Jeffrey Hugo, Bjorn Andersson, Andy Gross, Rob Herring

Add host controller and PHY DT nodes.

Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
TODO: check whether the driver uses the 'resets' prop
---
 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 20 +++++++
 arch/arm64/boot/dts/qcom/msm8998.dtsi     | 63 +++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index 50e9033aa7f6..cd1c9e84eab7 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -257,3 +257,23 @@
 	pinctrl-0 = <&sdc2_clk_on  &sdc2_cmd_on  &sdc2_data_on  &sdc2_cd_on>;
 	pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
 };
+
+&ufshc {
+	vdd-hba-fixed-regulator;
+	vcc-supply = <&vreg_l20a_2p95>;
+	vccq-supply = <&vreg_l26a_1p2>;
+	vccq2-supply = <&vreg_s4a_1p8>;
+	vcc-max-microamp = <750000>;
+	vccq-max-microamp = <560000>;
+	vccq2-max-microamp = <750000>;
+};
+
+&ufsphy {
+	vdda-phy-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l2a_1p2>;
+	vddp-ref-clk-supply = <&vreg_l26a_1p2>;
+	vdda-phy-max-microamp = <51400>;
+	vdda-pll-max-microamp = <14600>;
+	vddp-ref-clk-max-microamp = <100>;
+	vddp-ref-clk-always-on;
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 6f4f4b79853b..36fd2e614464 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -711,6 +711,69 @@
 			redistributor-stride = <0x0 0x20000>;
 			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		ufshc: ufshc@1da4000 {
+			compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
+				     "jedec,ufs-2.0";
+			reg = <0x1da4000 0x2500>;
+			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&ufsphy_lanes>;
+			phy-names = "ufsphy";
+			lanes-per-direction = <2>;
+			power-domains = <&gcc UFS_GDSC>;
+
+			clock-names =
+				"core_clk",
+				"bus_aggr_clk",
+				"iface_clk",
+				"core_clk_unipro",
+				"ref_clk",
+				"tx_lane0_sync_clk",
+				"rx_lane0_sync_clk",
+				"rx_lane1_sync_clk";
+			clocks =
+				<&gcc GCC_UFS_AXI_CLK>,
+				<&gcc GCC_AGGRE1_UFS_AXI_CLK>,
+				<&gcc GCC_UFS_AHB_CLK>,
+				<&gcc GCC_UFS_UNIPRO_CORE_CLK>,
+				<&rpmcc RPM_SMD_LN_BB_CLK1>,
+				<&gcc GCC_UFS_TX_SYMBOL_0_CLK>,
+				<&gcc GCC_UFS_RX_SYMBOL_0_CLK>,
+				<&gcc GCC_UFS_RX_SYMBOL_1_CLK>;
+			freq-table-hz =
+				<50000000 200000000>,
+				<0 0>,
+				<0 0>,
+				<37500000 150000000>,
+				<0 0>,
+				<0 0>,
+				<0 0>,
+				<0 0>;
+
+			resets = <&gcc GCC_UFS_BCR>;
+			reset-names = "rst";
+		};
+
+		ufsphy: phy@1da7000 {
+			compatible = "qcom,sdm845-qmp-ufs-phy";
+			reg = <0x1da7000 0x18c>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			clock-names = "ref", "ref_aux";
+			clocks =
+				<&gcc GCC_UFS_CLKREF_CLK>,
+				<&gcc GCC_UFS_PHY_AUX_CLK>;
+
+			ufsphy_lanes: lanes@1da7400 {
+				reg = <0x1da7400 0x128>,
+				      <0x1da7600 0x1fc>,
+				      <0x1da7c00 0x1dc>,
+				      <0x1da7800 0x128>,
+				      <0x1da7a00 0x1fc>;
+				#phy-cells = <0>;
+			};
+		};
 	};
 };
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH v2 3/4] Add regulator-allow-set-load
  2019-01-16 10:50 [RFC PATCH v2 0/4] UFS on APQ8098 Marc Gonzalez
  2019-01-16 10:53 ` [RFC PATCH v2 1/4] arm64: dts: qcom: msm8998: Add rpmcc node Marc Gonzalez
  2019-01-16 10:56 ` [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes Marc Gonzalez
@ 2019-01-16 10:58 ` Marc Gonzalez
  2019-01-16 11:00 ` [RFC PATCH v2 4/4] ufshcd_set_vccq_rail_unused locks up the board Marc Gonzalez
  3 siblings, 0 replies; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-16 10:58 UTC (permalink / raw)
  To: MSM; +Cc: LKML, Jeffrey Hugo, Bjorn Andersson, Andy Gross, Rob Herring

BLURB
---
Set the prop for all vregs or just the UFS vregs?
---
 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 32 +++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index cd1c9e84eab7..3d18a97d1563 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -100,132 +100,164 @@
 		vreg_s3a_1p35: s3 {
 			regulator-min-microvolt = <1352000>;
 			regulator-max-microvolt = <1352000>;
+			regulator-allow-set-load;
 		};
 		vreg_s4a_1p8: s4 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 		vreg_s5a_2p04: s5 {
 			regulator-min-microvolt = <1904000>;
 			regulator-max-microvolt = <2040000>;
+			regulator-allow-set-load;
 		};
 		vreg_s7a_1p025: s7 {
 			regulator-min-microvolt = <900000>;
 			regulator-max-microvolt = <1028000>;
+			regulator-allow-set-load;
 		};
 		vreg_l1a_0p875: l1 {
 			regulator-min-microvolt = <880000>;
 			regulator-max-microvolt = <880000>;
+			regulator-allow-set-load;
 		};
 		vreg_l2a_1p2: l2 {
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1200000>;
+			regulator-allow-set-load;
 		};
 		vreg_l3a_1p0: l3 {
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
+			regulator-allow-set-load;
 		};
 		vreg_l5a_0p8: l5 {
 			regulator-min-microvolt = <800000>;
 			regulator-max-microvolt = <800000>;
+			regulator-allow-set-load;
 		};
 		vreg_l6a_1p8: l6 {
 			regulator-min-microvolt = <1808000>;
 			regulator-max-microvolt = <1808000>;
+			regulator-allow-set-load;
 		};
 		vreg_l7a_1p8: l7 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 		vreg_l8a_1p2: l8 {
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1200000>;
+			regulator-allow-set-load;
 		};
 		vreg_l9a_1p8: l9 {
 			regulator-min-microvolt = <1808000>;
 			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
 		};
 		vreg_l10a_1p8: l10 {
 			regulator-min-microvolt = <1808000>;
 			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
 		};
 		vreg_l11a_1p0: l11 {
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
+			regulator-allow-set-load;
 		};
 		vreg_l12a_1p8: l12 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 		vreg_l13a_2p95: l13 {
 			regulator-min-microvolt = <1808000>;
 			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
 		};
 		vreg_l14a_1p88: l14 {
 			regulator-min-microvolt = <1880000>;
 			regulator-max-microvolt = <1880000>;
+			regulator-allow-set-load;
 		};
 		vreg_15a_1p8: l15 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 		vreg_l16a_2p7: l16 {
 			regulator-min-microvolt = <2704000>;
 			regulator-max-microvolt = <2704000>;
+			regulator-allow-set-load;
 		};
 		vreg_l17a_1p3: l17 {
 			regulator-min-microvolt = <1304000>;
 			regulator-max-microvolt = <1304000>;
+			regulator-allow-set-load;
 		};
 		vreg_l18a_2p7: l18 {
 			regulator-min-microvolt = <2704000>;
 			regulator-max-microvolt = <2704000>;
+			regulator-allow-set-load;
 		};
 		vreg_l19a_3p0: l19 {
 			regulator-min-microvolt = <3008000>;
 			regulator-max-microvolt = <3008000>;
+			regulator-allow-set-load;
 		};
 		vreg_l20a_2p95: l20 {
 			regulator-min-microvolt = <2960000>;
 			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
 		};
 		vreg_l21a_2p95: l21 {
 			regulator-min-microvolt = <2960000>;
 			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
 		};
 		vreg_l22a_2p85: l22 {
 			regulator-min-microvolt = <2864000>;
 			regulator-max-microvolt = <2864000>;
+			regulator-allow-set-load;
 		};
 		vreg_l23a_3p3: l23 {
 			regulator-min-microvolt = <3312000>;
 			regulator-max-microvolt = <3312000>;
+			regulator-allow-set-load;
 		};
 		vreg_l24a_3p075: l24 {
 			regulator-min-microvolt = <3088000>;
 			regulator-max-microvolt = <3088000>;
+			regulator-allow-set-load;
 		};
 		vreg_l25a_3p3: l25 {
 			regulator-min-microvolt = <3104000>;
 			regulator-max-microvolt = <3312000>;
+			regulator-allow-set-load;
 		};
 		vreg_l26a_1p2: l26 {
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1200000>;
+			regulator-allow-set-load;
 		};
 		vreg_l28_3p0: l28 {
 			regulator-min-microvolt = <3008000>;
 			regulator-max-microvolt = <3008000>;
+			regulator-allow-set-load;
 		};
 
 		vreg_lvs1a_1p8: lvs1 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 
 		vreg_lvs2a_1p8: lvs2 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
 		};
 
 	};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH v2 4/4] ufshcd_set_vccq_rail_unused locks up the board
  2019-01-16 10:50 [RFC PATCH v2 0/4] UFS on APQ8098 Marc Gonzalez
                   ` (2 preceding siblings ...)
  2019-01-16 10:58 ` [RFC PATCH v2 3/4] Add regulator-allow-set-load Marc Gonzalez
@ 2019-01-16 11:00 ` Marc Gonzalez
  3 siblings, 0 replies; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-16 11:00 UTC (permalink / raw)
  To: MSM; +Cc: LKML, Jeffrey Hugo, Bjorn Andersson, Andy Gross

Better solution required.
Suggestions? A quirk?
Specify that the vccq rail cannot be disabled in the DT?
---
 drivers/scsi/ufs/ufshcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9ba7671b84f8..d0d340210ccf 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7196,6 +7196,7 @@ static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
 {
 	int ret = 0;
 	struct ufs_vreg_info *info = &hba->vreg_info;
+	return 0; /*** NOP ***/
 
 	if (!info)
 		goto out;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes
  2019-01-16 10:56 ` [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes Marc Gonzalez
@ 2019-01-16 15:36   ` Jeffrey Hugo
  2019-01-21 17:15     ` Marc Gonzalez
  0 siblings, 1 reply; 7+ messages in thread
From: Jeffrey Hugo @ 2019-01-16 15:36 UTC (permalink / raw)
  To: Marc Gonzalez, MSM; +Cc: LKML, Bjorn Andersson, Andy Gross, Rob Herring

On 1/16/2019 3:56 AM, Marc Gonzalez wrote:
> Add host controller and PHY DT nodes.
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> TODO: check whether the driver uses the 'resets' prop
> ---
>   arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 20 +++++++
>   arch/arm64/boot/dts/qcom/msm8998.dtsi     | 63 +++++++++++++++++++++++
>   2 files changed, 83 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 50e9033aa7f6..cd1c9e84eab7 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -257,3 +257,23 @@
>   	pinctrl-0 = <&sdc2_clk_on  &sdc2_cmd_on  &sdc2_data_on  &sdc2_cd_on>;
>   	pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
>   };
> +
> +&ufshc {
> +	vdd-hba-fixed-regulator;

Since we are not specifying the vdd anymore, I suspect this should be 
dropped.  Do you know of any reason why we'd still need it?

> +	vcc-supply = <&vreg_l20a_2p95>;
> +	vccq-supply = <&vreg_l26a_1p2>;
> +	vccq2-supply = <&vreg_s4a_1p8>;
> +	vcc-max-microamp = <750000>;
> +	vccq-max-microamp = <560000>;
> +	vccq2-max-microamp = <750000>;
> +};
> +
> +&ufsphy {
> +	vdda-phy-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l2a_1p2>;
> +	vddp-ref-clk-supply = <&vreg_l26a_1p2>;
> +	vdda-phy-max-microamp = <51400>;
> +	vdda-pll-max-microamp = <14600>;
> +	vddp-ref-clk-max-microamp = <100>;
> +	vddp-ref-clk-always-on;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 6f4f4b79853b..36fd2e614464 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -711,6 +711,69 @@
>   			redistributor-stride = <0x0 0x20000>;
>   			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>   		};
> +
> +		ufshc: ufshc@1da4000 {
> +			compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
> +				     "jedec,ufs-2.0";
> +			reg = <0x1da4000 0x2500>;

Bjorn would like it if reg addresses are full width, ie 0x01da4000

> +			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> +			phys = <&ufsphy_lanes>;
> +			phy-names = "ufsphy";
> +			lanes-per-direction = <2>;
> +			power-domains = <&gcc UFS_GDSC>;
> +
> +			clock-names =
> +				"core_clk",
> +				"bus_aggr_clk",
> +				"iface_clk",
> +				"core_clk_unipro",
> +				"ref_clk",
> +				"tx_lane0_sync_clk",
> +				"rx_lane0_sync_clk",
> +				"rx_lane1_sync_clk";
> +			clocks =
> +				<&gcc GCC_UFS_AXI_CLK>,
> +				<&gcc GCC_AGGRE1_UFS_AXI_CLK>,
> +				<&gcc GCC_UFS_AHB_CLK>,
> +				<&gcc GCC_UFS_UNIPRO_CORE_CLK>,
> +				<&rpmcc RPM_SMD_LN_BB_CLK1>,
> +				<&gcc GCC_UFS_TX_SYMBOL_0_CLK>,
> +				<&gcc GCC_UFS_RX_SYMBOL_0_CLK>,
> +				<&gcc GCC_UFS_RX_SYMBOL_1_CLK>;
> +			freq-table-hz =
> +				<50000000 200000000>,
> +				<0 0>,
> +				<0 0>,
> +				<37500000 150000000>,
> +				<0 0>,
> +				<0 0>,
> +				<0 0>,
> +				<0 0>;
> +
> +			resets = <&gcc GCC_UFS_BCR>;
> +			reset-names = "rst";
> +		};
> +
> +		ufsphy: phy@1da7000 {
> +			compatible = "qcom,sdm845-qmp-ufs-phy";

We should make an 8998 compatible.  Also, don't you have phy changes 
since the init sequence differs between 845 and 8998?

> +			reg = <0x1da7000 0x18c>;

0x01da7000, see above comment

> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			clock-names = "ref", "ref_aux";
> +			clocks =
> +				<&gcc GCC_UFS_CLKREF_CLK>,
> +				<&gcc GCC_UFS_PHY_AUX_CLK>;
> +
> +			ufsphy_lanes: lanes@1da7400 {
> +				reg = <0x1da7400 0x128>,
> +				      <0x1da7600 0x1fc>,
> +				      <0x1da7c00 0x1dc>,
> +				      <0x1da7800 0x128>,
> +				      <0x1da7a00 0x1fc>;
> +				#phy-cells = <0>;
> +			};
> +		};
>   	};
>   };
>   
> 


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes
  2019-01-16 15:36   ` Jeffrey Hugo
@ 2019-01-21 17:15     ` Marc Gonzalez
  0 siblings, 0 replies; 7+ messages in thread
From: Marc Gonzalez @ 2019-01-21 17:15 UTC (permalink / raw)
  To: Jeffrey Hugo, MSM; +Cc: LKML, Bjorn Andersson, Andy Gross, Rob Herring

On 16/01/2019 16:36, Jeffrey Hugo wrote:

> On 1/16/2019 3:56 AM, Marc Gonzalez wrote:
>
>> Add host controller and PHY DT nodes.
>>
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> ---
>> TODO: check whether the driver uses the 'resets' prop
>> ---
>>   arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 20 +++++++
>>   arch/arm64/boot/dts/qcom/msm8998.dtsi     | 63 +++++++++++++++++++++++
>>   2 files changed, 83 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> index 50e9033aa7f6..cd1c9e84eab7 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> @@ -257,3 +257,23 @@
>>   	pinctrl-0 = <&sdc2_clk_on  &sdc2_cmd_on  &sdc2_data_on  &sdc2_cd_on>;
>>   	pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
>>   };
>> +
>> +&ufshc {
>> +	vdd-hba-fixed-regulator;
> 
> Since we are not specifying the vdd anymore, I suspect this should be 
> dropped.  Do you know of any reason why we'd still need it?

Will drop in v3.

>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> index 6f4f4b79853b..36fd2e614464 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> @@ -711,6 +711,69 @@
>>   			redistributor-stride = <0x0 0x20000>;
>>   			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>>   		};
>> +
>> +		ufshc: ufshc@1da4000 {
>> +			compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
>> +				     "jedec,ufs-2.0";
>> +			reg = <0x1da4000 0x2500>;
> 
> Bjorn would like it if reg addresses are full width, ie 0x01da4000

Will tweak in v3.

>> +		ufsphy: phy@1da7000 {
>> +			compatible = "qcom,sdm845-qmp-ufs-phy";
> 
> We should make an 8998 compatible.  Also, don't you have phy changes 
> since the init sequence differs between 845 and 8998?

Will create a specific binding.
I don't have any PHY changes, I just used the 845 init sequence.
I tested this by using the 845 init sequence downstream.

However, no point in sending v3 until someone comments on patches 3 and 4 :-)

Patch 4 needs to become a real patch.

Regards.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-21 17:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16 10:50 [RFC PATCH v2 0/4] UFS on APQ8098 Marc Gonzalez
2019-01-16 10:53 ` [RFC PATCH v2 1/4] arm64: dts: qcom: msm8998: Add rpmcc node Marc Gonzalez
2019-01-16 10:56 ` [RFC PATCH v2 2/4] arm64: dts: qcom: msm8998: Add UFS nodes Marc Gonzalez
2019-01-16 15:36   ` Jeffrey Hugo
2019-01-21 17:15     ` Marc Gonzalez
2019-01-16 10:58 ` [RFC PATCH v2 3/4] Add regulator-allow-set-load Marc Gonzalez
2019-01-16 11:00 ` [RFC PATCH v2 4/4] ufshcd_set_vccq_rail_unused locks up the board Marc Gonzalez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).