Patchwork [RFC,v1] arm64: dts: qcom: msm8998: Add UFS nodes

login
register
mail settings
Submitter Marc Gonzalez
Date Jan. 11, 2019, 2:50 p.m.
Message ID <7cbafc6f-0a6d-8d43-1005-1d2985d09eb7@free.fr>
Download mbox | patch
Permalink /patch/697777/
State New
Headers show

Comments

Marc Gonzalez - Jan. 11, 2019, 2:50 p.m.
Add UFS host controller and PHY DT nodes.

Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 42 +++++++++++++++
 arch/arm64/boot/dts/qcom/msm8998.dtsi     | 66 +++++++++++++++++++++++
 2 files changed, 108 insertions(+)
Jeffrey Hugo - Jan. 11, 2019, 3:03 p.m.
On 1/11/2019 7:50 AM, Marc Gonzalez wrote:
> Add UFS host controller and PHY DT nodes.
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
>   arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 42 +++++++++++++++
>   arch/arm64/boot/dts/qcom/msm8998.dtsi     | 66 +++++++++++++++++++++++
>   2 files changed, 108 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> index 50e9033aa7f6..0fbc8ba9dc30 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
> @@ -244,6 +244,24 @@
>   
>   &tlmm {
>   	gpio-reserved-ranges = <0 4>, <81 4>;
> +
> +	ufs_dev_reset_assert: ufs_dev_reset_assert {
> +		config {
> +			pins = "ufs_reset";
> +			bias-pull-down;
> +			drive-strength = <8>;
> +			output-low;
> +		};
> +	};
> +
> +	ufs_dev_reset_deassert: ufs_dev_reset_deassert {
> +		config {
> +			pins = "ufs_reset";
> +			bias-pull-down;
> +			drive-strength = <8>;
> +			output-high;
> +		};
> +	};
>   };
>   
>   &sdhc2 {
> @@ -257,3 +275,27 @@
>   	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-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/

Downstream the GDSCs are also exposed as regulators.  It doesn't appear 
that this is the case upstream.  I think this should just be removed.

> +	pinctrl-names = "dev-reset-assert", "dev-reset-deassert";
> +	pinctrl-0 = <&ufs_dev_reset_assert>;
> +	pinctrl-1 = <&ufs_dev_reset_deassert>;
> +	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;

Oh?  It is?

> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index de37415be0a8..ecf8916e70e7 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -711,6 +711,72 @@
>   			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",
> +				"core_clk_ice",

I'm not positive the ICE clk is actually required.

> +				"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>,
> +				<&gcc GCC_UFS_ICE_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>,
> +				<75000000 300000000>,
> +				<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 0x200>;
The proper length is:
			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 0x200>,
> +				      <0x1da7600 0x200>,
> +				      <0x1da7c00 0x200>,
> +				      <0x1da7800 0x200>,
> +				      <0x1da7a00 0x200>;
The proper lengths are:
                                 reg = <0x1da7400 0x128>,
                                       <0x1da7600 0x1fc>,
                                       <0x1da7c00 0x1dc>,
                                       <0x1da7800 0x128>,
                                       <0x1da7a00 0x1fc>;


> +				#phy-cells = <0>;
> +			};
> +		};
>   	};
>   };
>   
>
Marc Gonzalez - Jan. 11, 2019, 3:45 p.m.
On 11/01/2019 16:03, Jeffrey Hugo wrote:

> On 1/11/2019 7:50 AM, Marc Gonzalez wrote:
>
>> Add UFS host controller and PHY DT nodes.
>>
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> ---
>>   arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 42 +++++++++++++++
>>   arch/arm64/boot/dts/qcom/msm8998.dtsi     | 66 +++++++++++++++++++++++
>>   2 files changed, 108 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> index 50e9033aa7f6..0fbc8ba9dc30 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>> @@ -244,6 +244,24 @@
>>   
>>   &tlmm {
>>   	gpio-reserved-ranges = <0 4>, <81 4>;
>> +
>> +	ufs_dev_reset_assert: ufs_dev_reset_assert {
>> +		config {
>> +			pins = "ufs_reset";
>> +			bias-pull-down;
>> +			drive-strength = <8>;
>> +			output-low;
>> +		};
>> +	};
>> +
>> +	ufs_dev_reset_deassert: ufs_dev_reset_deassert {
>> +		config {
>> +			pins = "ufs_reset";
>> +			bias-pull-down;
>> +			drive-strength = <8>;
>> +			output-high;
>> +		};
>> +	};
>>   };
>>   
>>   &sdhc2 {
>> @@ -257,3 +275,27 @@
>>   	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-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/
> 
> Downstream the GDSCs are also exposed as regulators.  It doesn't appear 
> that this is the case upstream.  I think this should just be removed.

Makes sense. I only left that definition to elicit comments.


>> +&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;
> 
> Oh?  It is?

I just copied what I found downstream:

https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-cdp.dtsi?h=LE.UM.1.3.r3.25#n45


>> +			clock-names =
>> +				"core_clk",
>> +				"bus_aggr_clk",
>> +				"iface_clk",
>> +				"core_clk_unipro",
>> +				"core_clk_ice",
> 
> I'm not positive the ICE clk is actually required.

I think I copied that part from the sdm845 DT node.
I'll try removing it.


>> +				"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>,
>> +				<&gcc GCC_UFS_ICE_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>,
>> +				<75000000 300000000>,
>> +				<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 0x200>;
>
> The proper length is:
> 			reg = <0x1da7000 0x18c>;

It seemed to me that the HW designer had decided to carve out 0x200-byte
sized regions for the different PHY blocks.

Do you mean there are no registers documented from 0x1da718c to 0x1da7200?
Is there nothing from 0x1da7200 to 0x1da7400?


>> +			#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 0x200>,
>> +				      <0x1da7600 0x200>,
>> +				      <0x1da7c00 0x200>,
>> +				      <0x1da7800 0x200>,
>> +				      <0x1da7a00 0x200>;
>
> The proper lengths are:
>                                  reg = <0x1da7400 0x128>,
>                                        <0x1da7600 0x1fc>,
>                                        <0x1da7c00 0x1dc>,
>                                        <0x1da7800 0x128>,
>                                        <0x1da7a00 0x1fc>;

It's weird to specify a length of 0x1fc IMO.
Is it because there's a register at +0x1f8 and none at +0x1fc?

(All this talk is academic, since the granularity of a virtual mapping
is minimum 4096 bytes page-aligned.)

Some maintainers suggest rounding the number up to the next power of 2.
I don't know what the convention is for qcom DTBs.

Regards.
Jeffrey Hugo - Jan. 11, 2019, 4:12 p.m.
On 1/11/2019 8:45 AM, Marc Gonzalez wrote:
> On 11/01/2019 16:03, Jeffrey Hugo wrote:
> 
>> On 1/11/2019 7:50 AM, Marc Gonzalez wrote:
>>
>>> Add UFS host controller and PHY DT nodes.
>>>
>>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>>> ---
>>>    arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 42 +++++++++++++++
>>>    arch/arm64/boot/dts/qcom/msm8998.dtsi     | 66 +++++++++++++++++++++++
>>>    2 files changed, 108 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>>> index 50e9033aa7f6..0fbc8ba9dc30 100644
>>> --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
>>> @@ -244,6 +244,24 @@
>>>    
>>>    &tlmm {
>>>    	gpio-reserved-ranges = <0 4>, <81 4>;
>>> +
>>> +	ufs_dev_reset_assert: ufs_dev_reset_assert {
>>> +		config {
>>> +			pins = "ufs_reset";
>>> +			bias-pull-down;
>>> +			drive-strength = <8>;
>>> +			output-low;
>>> +		};
>>> +	};
>>> +
>>> +	ufs_dev_reset_deassert: ufs_dev_reset_deassert {
>>> +		config {
>>> +			pins = "ufs_reset";
>>> +			bias-pull-down;
>>> +			drive-strength = <8>;
>>> +			output-high;
>>> +		};
>>> +	};
>>>    };
>>>    
>>>    &sdhc2 {
>>> @@ -257,3 +275,27 @@
>>>    	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-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/
>>
>> Downstream the GDSCs are also exposed as regulators.  It doesn't appear
>> that this is the case upstream.  I think this should just be removed.
> 
> Makes sense. I only left that definition to elicit comments.
> 
> 
>>> +&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;
>>
>> Oh?  It is?
> 
> I just copied what I found downstream:
> 
> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-cdp.dtsi?h=LE.UM.1.3.r3.25#n45
> 
> 
>>> +			clock-names =
>>> +				"core_clk",
>>> +				"bus_aggr_clk",
>>> +				"iface_clk",
>>> +				"core_clk_unipro",
>>> +				"core_clk_ice",
>>
>> I'm not positive the ICE clk is actually required.
> 
> I think I copied that part from the sdm845 DT node.
> I'll try removing it.
> 
> 
>>> +				"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>,
>>> +				<&gcc GCC_UFS_ICE_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>,
>>> +				<75000000 300000000>,
>>> +				<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 0x200>;
>>
>> The proper length is:
>> 			reg = <0x1da7000 0x18c>;
> 
> It seemed to me that the HW designer had decided to carve out 0x200-byte
> sized regions for the different PHY blocks.
> 
> Do you mean there are no registers documented from 0x1da718c to 0x1da7200?

Correct.

> Is there nothing from 0x1da7200 to 0x1da7400?

Not that I see.

> 
> 
>>> +			#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 0x200>,
>>> +				      <0x1da7600 0x200>,
>>> +				      <0x1da7c00 0x200>,
>>> +				      <0x1da7800 0x200>,
>>> +				      <0x1da7a00 0x200>;
>>
>> The proper lengths are:
>>                                   reg = <0x1da7400 0x128>,
>>                                         <0x1da7600 0x1fc>,
>>                                         <0x1da7c00 0x1dc>,
>>                                         <0x1da7800 0x128>,
>>                                         <0x1da7a00 0x1fc>;
> 
> It's weird to specify a length of 0x1fc IMO.

Its the documented size of the hardware block.

> Is it because there's a register at +0x1f8 and none at +0x1fc?

Generally that is what defines the block size.  The last register I 
happen to see in this case is +0x1dc

> 
> (All this talk is academic, since the granularity of a virtual mapping
> is minimum 4096 bytes page-aligned.)

While I agree, The DT should reflect the hardware and not necessarily 
depend on a specific OS implementation.  Some other OS may provide more 
granular mapping and do range checking.
> 
> Some maintainers suggest rounding the number up to the next power of 2.
> I don't know what the convention is for qcom DTBs.
> 
> Regards.
>
Marc Gonzalez - Jan. 14, 2019, 10:29 a.m.
On 11/01/2019 16:03, Jeffrey Hugo wrote:

> On 1/11/2019 7:50 AM, Marc Gonzalez wrote:
>
>> +&ufshc {
>> +/***	vdd-hba-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/
> 
> Downstream the GDSCs are also exposed as regulators.  It doesn't appear 
> that this is the case upstream.  I think this should just be removed.

However, the driver might need to be updated?

ufshcd-qcom 1da4000.ufshc: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled

drivers/scsi/ufs/ufshcd-pltfrm.c:               dev_info(dev, "%s: Unable to find %s regulator, assuming enabled\n",

	snprintf(prop_name, MAX_PROP_SIZE, "%s-supply", name);
	if (!of_parse_phandle(np, prop_name, 0)) {
		dev_info(dev, "%s: Unable to find %s regulator, assuming enabled\n",
				__func__, prop_name);
		goto out;
	}


/**
 * ufshcd_parse_regulator_info - get regulator info from device tree
 * @hba: per adapter instance
 *
 * Get regulator info from device tree for vcc, vccq, vccq2 power supplies.
 * If any of the supplies are not defined it is assumed that they are always-on
 * and hence return zero. If the property is defined but parsing is failed
 * then return corresponding error.
 */
static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
{
	int err;
	struct device *dev = hba->dev;
	struct ufs_vreg_info *info = &hba->vreg_info;

	err = ufshcd_populate_vreg(dev, "vdd-hba", &info->vdd_hba);
	if (err)
		goto out;

	err = ufshcd_populate_vreg(dev, "vcc", &info->vcc);
	if (err)
		goto out;

	err = ufshcd_populate_vreg(dev, "vccq", &info->vccq);
	if (err)
		goto out;

	err = ufshcd_populate_vreg(dev, "vccq2", &info->vccq2);
out:
	return err;
}
Jeffrey Hugo - Jan. 14, 2019, 2:56 p.m.
On 1/14/2019 3:29 AM, Marc Gonzalez wrote:
> On 11/01/2019 16:03, Jeffrey Hugo wrote:
> 
>> On 1/11/2019 7:50 AM, Marc Gonzalez wrote:
>>
>>> +&ufshc {
>>> +/***	vdd-hba-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/
>>
>> Downstream the GDSCs are also exposed as regulators.  It doesn't appear
>> that this is the case upstream.  I think this should just be removed.
> 
> However, the driver might need to be updated?

I would expect 845 to have the same behavior based on the DT I see. 
Seems ok for now.

> 
> ufshcd-qcom 1da4000.ufshc: ufshcd_populate_vreg: Unable to find vdd-hba-supply regulator, assuming enabled
> 
> drivers/scsi/ufs/ufshcd-pltfrm.c:               dev_info(dev, "%s: Unable to find %s regulator, assuming enabled\n",
> 
> 	snprintf(prop_name, MAX_PROP_SIZE, "%s-supply", name);
> 	if (!of_parse_phandle(np, prop_name, 0)) {
> 		dev_info(dev, "%s: Unable to find %s regulator, assuming enabled\n",
> 				__func__, prop_name);
> 		goto out;
> 	}
> 
> 
> /**
>   * ufshcd_parse_regulator_info - get regulator info from device tree
>   * @hba: per adapter instance
>   *
>   * Get regulator info from device tree for vcc, vccq, vccq2 power supplies.
>   * If any of the supplies are not defined it is assumed that they are always-on
>   * and hence return zero. If the property is defined but parsing is failed
>   * then return corresponding error.
>   */
> static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
> {
> 	int err;
> 	struct device *dev = hba->dev;
> 	struct ufs_vreg_info *info = &hba->vreg_info;
> 
> 	err = ufshcd_populate_vreg(dev, "vdd-hba", &info->vdd_hba);
> 	if (err)
> 		goto out;
> 
> 	err = ufshcd_populate_vreg(dev, "vcc", &info->vcc);
> 	if (err)
> 		goto out;
> 
> 	err = ufshcd_populate_vreg(dev, "vccq", &info->vccq);
> 	if (err)
> 		goto out;
> 
> 	err = ufshcd_populate_vreg(dev, "vccq2", &info->vccq2);
> out:
> 	return err;
> }
>

Patch

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index 50e9033aa7f6..0fbc8ba9dc30 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -244,6 +244,24 @@ 
 
 &tlmm {
 	gpio-reserved-ranges = <0 4>, <81 4>;
+
+	ufs_dev_reset_assert: ufs_dev_reset_assert {
+		config {
+			pins = "ufs_reset";
+			bias-pull-down;
+			drive-strength = <8>;
+			output-low;
+		};
+	};
+
+	ufs_dev_reset_deassert: ufs_dev_reset_deassert {
+		config {
+			pins = "ufs_reset";
+			bias-pull-down;
+			drive-strength = <8>;
+			output-high;
+		};
+	};
 };
 
 &sdhc2 {
@@ -257,3 +275,27 @@ 
 	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-supply = <&gcc UFS_GDSC>;	-EPROBE_DEFER ***/
+	pinctrl-names = "dev-reset-assert", "dev-reset-deassert";
+	pinctrl-0 = <&ufs_dev_reset_assert>;
+	pinctrl-1 = <&ufs_dev_reset_deassert>;
+	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 de37415be0a8..ecf8916e70e7 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -711,6 +711,72 @@ 
 			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",
+				"core_clk_ice",
+				"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>,
+				<&gcc GCC_UFS_ICE_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>,
+				<75000000 300000000>,
+				<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 0x200>;
+			#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 0x200>,
+				      <0x1da7600 0x200>,
+				      <0x1da7c00 0x200>,
+				      <0x1da7800 0x200>,
+				      <0x1da7a00 0x200>;
+				#phy-cells = <0>;
+			};
+		};
 	};
 };