All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.