diff mbox series

[v5,4/6] arm64: dts: qcom: Add UFS node

Message ID g5vlxrttgvfqkktlkhu4uzhtvnp3qtjcbr7l2uztapzqwhrsem@wg574xldh5ar
State New
Headers show
Series Add initial DTS for Samsung Galaxy Book4 Edge | expand

Commit Message

Marcus Glocker Aug. 17, 2024, 8:38 p.m. UTC 
Add the UFS Host Controller node.  This was basically copied from the
arch/arm64/boot/dts/qcom/sc7180.dtsi file.

Signed-off-by: Marcus Glocker <marcus@nazgul.ch>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 72 ++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

Konrad Dybcio Aug. 30, 2024, 12:05 a.m. UTC | #1 
On 17.08.2024 10:38 PM, Marcus Glocker wrote:
> Add the UFS Host Controller node.  This was basically copied from the
> arch/arm64/boot/dts/qcom/sc7180.dtsi file.
> 
> Signed-off-by: Marcus Glocker <marcus@nazgul.ch>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 72 ++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 7bca5fcd7d52..9f01b3ff3737 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -2878,6 +2878,78 @@ mmss_noc: interconnect@1780000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		ufs_mem_hc: ufs@1d84000 {
> +			compatible = "qcom,x1e80100-ufshc", "qcom,ufshc",
> +				     "jedec,ufs-2.0";
> +			reg = <0 0x01d84000 0 0x3000>;
> +			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> +			phys = <&ufs_mem_phy>;
> +			phy-names = "ufsphy";
> +			lanes-per-direction = <1>;
> +			#reset-cells = <1>;
> +			resets = <&gcc GCC_UFS_PHY_BCR>;
> +			reset-names = "rst";
> +
> +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> +
> +			iommus = <&apps_smmu 0xa0 0x0>;

Looks like this should be 0x1a0 maybe
> +
> +			clock-names = "core_clk",
> +				      "bus_aggr_clk",
> +				      "iface_clk",
> +				      "core_clk_unipro",
> +				      "ref_clk",
> +				      "tx_lane0_sync_clk",
> +				      "rx_lane0_sync_clk";
> +			clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> +				 <&gcc GCC_UFS_PHY_AHB_CLK>,
> +				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> +				 <&rpmhcc RPMH_CXO_CLK>,
> +				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> +				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>;

You also want

<&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>

> +			freq-table-hz = <50000000 200000000>,
25000000 300000000

> +					<0 0>,
> +					<0 0>,
> +					<37500000 150000000>,
75000000 300000000

> +					<0 0>,
> +					<0 0>,
> +					<0 0>;
> +
> +			interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					 &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "ufs-ddr", "cpu-ufs";
> +
> +			qcom,ice = <&ice>;
> +
> +			status = "disabled";
> +		};
> +
> +		ufs_mem_phy: phy@1d87000 {
> +			compatible = "qcom,x1e80100-qmp-ufs-phy";
> +			reg = <0 0x01d87000 0 0x1000>;

most definitely should be 0x01d80000 with a size of 0x2000

> +			clocks = <&rpmhcc RPMH_CXO_CLK>,
> +				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> +				 <&tcsr TCSR_UFS_PHY_CLKREF_EN>;
> +			clock-names = "ref",
> +				      "ref_aux",
> +				      "qref";
> +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> +			resets = <&ufs_mem_hc 0>;
> +			reset-names = "ufsphy";
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		ice: crypto@1d90000 {
> +			compatible = "qcom,x1e80100-inline-crypto-engine",
> +				     "qcom,inline-crypto-engine";
> +			reg = <0 0x01d90000 0 0x8000>;

0x1d88000


All this combined means you probably wrote your init sequence into some
free(?) register space and the one left over from the bootloader was
good enough :P

Konrad
Johan Hovold Aug. 30, 2024, 7:02 a.m. UTC | #2 
On Sat, Aug 17, 2024 at 10:38:39PM +0200, Marcus Glocker wrote:
> Add the UFS Host Controller node.  This was basically copied from the
> arch/arm64/boot/dts/qcom/sc7180.dtsi file.
> 
> Signed-off-by: Marcus Glocker <marcus@nazgul.ch>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Also, when respinning, please add the missing "x1e80100: " to the
Subject prefix.

Johan
Marcus Glocker Aug. 30, 2024, 5:25 p.m. UTC | #3 
On Fri, Aug 30, 2024 at 02:05:48AM +0200, Konrad Dybcio wrote:

> On 17.08.2024 10:38 PM, Marcus Glocker wrote:
> > Add the UFS Host Controller node.  This was basically copied from the
> > arch/arm64/boot/dts/qcom/sc7180.dtsi file.
> >
> > Signed-off-by: Marcus Glocker <marcus@nazgul.ch>
> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 72 ++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi 
> > b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index 7bca5fcd7d52..9f01b3ff3737 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -2878,6 +2878,78 @@ mmss_noc: interconnect@1780000 {
> >  			#interconnect-cells = <2>;
> >  		};
> >
> > +		ufs_mem_hc: ufs@1d84000 {
> > +			compatible = "qcom,x1e80100-ufshc", "qcom,ufshc",
> > +				     "jedec,ufs-2.0";
> > +			reg = <0 0x01d84000 0 0x3000>;
> > +			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> > +			phys = <&ufs_mem_phy>;
> > +			phy-names = "ufsphy";
> > +			lanes-per-direction = <1>;
> > +			#reset-cells = <1>;
> > +			resets = <&gcc GCC_UFS_PHY_BCR>;
> > +			reset-names = "rst";
> > +
> > +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> > +
> > +			iommus = <&apps_smmu 0xa0 0x0>;
> 
> Looks like this should be 0x1a0 maybe
> > +
> > +			clock-names = "core_clk",
> > +				      "bus_aggr_clk",
> > +				      "iface_clk",
> > +				      "core_clk_unipro",
> > +				      "ref_clk",
> > +				      "tx_lane0_sync_clk",
> > +				      "rx_lane0_sync_clk";
> > +			clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> > +				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> > +				 <&gcc GCC_UFS_PHY_AHB_CLK>,
> > +				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> > +				 <&rpmhcc RPMH_CXO_CLK>,
> > +				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> > +				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>;
> 
> You also want
> 
> <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>
> 
> > +			freq-table-hz = <50000000 200000000>,
> 25000000 300000000
> 
> > +					<0 0>,
> > +					<0 0>,
> > +					<37500000 150000000>,
> 75000000 300000000
> 
> > +					<0 0>,
> > +					<0 0>,
> > +					<0 0>;
> > +
> > +			interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
> > +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> > +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> > +					 &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;
> > +			interconnect-names = "ufs-ddr", "cpu-ufs";
> > +
> > +			qcom,ice = <&ice>;
> > +
> > +			status = "disabled";
> > +		};
> > +
> > +		ufs_mem_phy: phy@1d87000 {
> > +			compatible = "qcom,x1e80100-qmp-ufs-phy";
> > +			reg = <0 0x01d87000 0 0x1000>;
> 
> most definitely should be 0x01d80000 with a size of 0x2000
> 
> > +			clocks = <&rpmhcc RPMH_CXO_CLK>,
> > +				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> > +				 <&tcsr TCSR_UFS_PHY_CLKREF_EN>;
> > +			clock-names = "ref",
> > +				      "ref_aux",
> > +				      "qref";
> > +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> > +			resets = <&ufs_mem_hc 0>;
> > +			reset-names = "ufsphy";
> > +			#phy-cells = <0>;
> > +			status = "disabled";
> > +		};
> > +
> > +		ice: crypto@1d90000 {
> > +			compatible = "qcom,x1e80100-inline-crypto-engine",
> > +				     "qcom,inline-crypto-engine";
> > +			reg = <0 0x01d90000 0 0x8000>;
> 
> 0x1d88000
> 
> 
> All this combined means you probably wrote your init sequence into some
> free(?) register space and the one left over from the bootloader was
> good enough :P
> 
> Konrad

I have not done anything special in our sub-system to boot this DTB.
Changing the values as suggested by you also doesn't make any difference
to me.

Anyway, I think I'll give up at this point, since this process is
getting too time consuming for me.  We'll go ahead with out downstream
patches, which works for us so far.

Cheers,
Marcus
Daniel Gomez Nov. 9, 2024, 11:31 p.m. UTC | #4 
On Fri Aug 30, 2024 at 7:25 PM CEST, Marcus Glocker wrote:
> On Fri, Aug 30, 2024 at 02:05:48AM +0200, Konrad Dybcio wrote:
>
>> On 17.08.2024 10:38 PM, Marcus Glocker wrote:
>> > Add the UFS Host Controller node.  This was basically copied from the
>> > arch/arm64/boot/dts/qcom/sc7180.dtsi file.
>> >
>> > Signed-off-by: Marcus Glocker <marcus@nazgul.ch>
>> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> > ---
>> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 72 ++++++++++++++++++++++++++
>> >  1 file changed, 72 insertions(+)
>> >
>> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi 
>> > b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> > index 7bca5fcd7d52..9f01b3ff3737 100644
>> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> > @@ -2878,6 +2878,78 @@ mmss_noc: interconnect@1780000 {
>> >  			#interconnect-cells = <2>;
>> >  		};
>> >
>> > +		ufs_mem_hc: ufs@1d84000 {
>> > +			compatible = "qcom,x1e80100-ufshc", "qcom,ufshc",
>> > +				     "jedec,ufs-2.0";
>> > +			reg = <0 0x01d84000 0 0x3000>;
>> > +			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
>> > +			phys = <&ufs_mem_phy>;
>> > +			phy-names = "ufsphy";
>> > +			lanes-per-direction = <1>;
>> > +			#reset-cells = <1>;
>> > +			resets = <&gcc GCC_UFS_PHY_BCR>;
>> > +			reset-names = "rst";
>> > +
>> > +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
>> > +
>> > +			iommus = <&apps_smmu 0xa0 0x0>;
>> 
>> Looks like this should be 0x1a0 maybe
>> > +
>> > +			clock-names = "core_clk",
>> > +				      "bus_aggr_clk",
>> > +				      "iface_clk",
>> > +				      "core_clk_unipro",
>> > +				      "ref_clk",
>> > +				      "tx_lane0_sync_clk",
>> > +				      "rx_lane0_sync_clk";
>> > +			clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
>> > +				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
>> > +				 <&gcc GCC_UFS_PHY_AHB_CLK>,
>> > +				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
>> > +				 <&rpmhcc RPMH_CXO_CLK>,
>> > +				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
>> > +				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>;
>> 
>> You also want
>> 
>> <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>
>> 
>> > +			freq-table-hz = <50000000 200000000>,
>> 25000000 300000000
>> 
>> > +					<0 0>,
>> > +					<0 0>,
>> > +					<37500000 150000000>,
>> 75000000 300000000
>> 
>> > +					<0 0>,
>> > +					<0 0>,
>> > +					<0 0>;
>> > +
>> > +			interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
>> > +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>> > +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> > +					 &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;
>> > +			interconnect-names = "ufs-ddr", "cpu-ufs";
>> > +
>> > +			qcom,ice = <&ice>;
>> > +
>> > +			status = "disabled";
>> > +		};
>> > +
>> > +		ufs_mem_phy: phy@1d87000 {
>> > +			compatible = "qcom,x1e80100-qmp-ufs-phy";
>> > +			reg = <0 0x01d87000 0 0x1000>;
>> 
>> most definitely should be 0x01d80000 with a size of 0x2000
>> 
>> > +			clocks = <&rpmhcc RPMH_CXO_CLK>,
>> > +				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
>> > +				 <&tcsr TCSR_UFS_PHY_CLKREF_EN>;
>> > +			clock-names = "ref",
>> > +				      "ref_aux",
>> > +				      "qref";
>> > +			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
>> > +			resets = <&ufs_mem_hc 0>;
>> > +			reset-names = "ufsphy";
>> > +			#phy-cells = <0>;
>> > +			status = "disabled";
>> > +		};
>> > +
>> > +		ice: crypto@1d90000 {
>> > +			compatible = "qcom,x1e80100-inline-crypto-engine",
>> > +				     "qcom,inline-crypto-engine";
>> > +			reg = <0 0x01d90000 0 0x8000>;
>> 
>> 0x1d88000
>> 
>> 
>> All this combined means you probably wrote your init sequence into some
>> free(?) register space and the one left over from the bootloader was
>> good enough :P
>> 
>> Konrad
>
> I have not done anything special in our sub-system to boot this DTB.
> Changing the values as suggested by you also doesn't make any difference
> to me.
>
> Anyway, I think I'll give up at this point, since this process is
> getting too time consuming for me.  We'll go ahead with out downstream
> patches, which works for us so far.

Hi Marcus,

I came across this and I'd be interested in picking up the patches for testing.
Is there any guide or documentation available that I could follow to boot Linux
on this machine using this patchset? Also, could you share where I might be able
to find those downstream patches?

Thanks,
Daniel

>
> Cheers,
> Marcus
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 7bca5fcd7d52..9f01b3ff3737 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -2878,6 +2878,78 @@  mmss_noc: interconnect@1780000 {
 			#interconnect-cells = <2>;
 		};
 
+		ufs_mem_hc: ufs@1d84000 {
+			compatible = "qcom,x1e80100-ufshc", "qcom,ufshc",
+				     "jedec,ufs-2.0";
+			reg = <0 0x01d84000 0 0x3000>;
+			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&ufs_mem_phy>;
+			phy-names = "ufsphy";
+			lanes-per-direction = <1>;
+			#reset-cells = <1>;
+			resets = <&gcc GCC_UFS_PHY_BCR>;
+			reset-names = "rst";
+
+			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
+
+			iommus = <&apps_smmu 0xa0 0x0>;
+
+			clock-names = "core_clk",
+				      "bus_aggr_clk",
+				      "iface_clk",
+				      "core_clk_unipro",
+				      "ref_clk",
+				      "tx_lane0_sync_clk",
+				      "rx_lane0_sync_clk";
+			clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
+				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+				 <&gcc GCC_UFS_PHY_AHB_CLK>,
+				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>;
+			freq-table-hz = <50000000 200000000>,
+					<0 0>,
+					<0 0>,
+					<37500000 150000000>,
+					<0 0>,
+					<0 0>,
+					<0 0>;
+
+			interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+					 &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "ufs-ddr", "cpu-ufs";
+
+			qcom,ice = <&ice>;
+
+			status = "disabled";
+		};
+
+		ufs_mem_phy: phy@1d87000 {
+			compatible = "qcom,x1e80100-qmp-ufs-phy";
+			reg = <0 0x01d87000 0 0x1000>;
+			clocks = <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
+				 <&tcsr TCSR_UFS_PHY_CLKREF_EN>;
+			clock-names = "ref",
+				      "ref_aux",
+				      "qref";
+			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
+			resets = <&ufs_mem_hc 0>;
+			reset-names = "ufsphy";
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		ice: crypto@1d90000 {
+			compatible = "qcom,x1e80100-inline-crypto-engine",
+				     "qcom,inline-crypto-engine";
+			reg = <0 0x01d90000 0 0x8000>;
+			clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
+		};
+
 		pcie6a: pci@1bf8000 {
 			device_type = "pci";
 			compatible = "qcom,pcie-x1e80100";