| Message ID | 20220318183004.858707-6-luca.weiss@fairphone.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | UFS support on SM6350 & FP4 | expand |
On 18/03/2022 19:30, Luca Weiss wrote: > Add the necessary nodes for UFS and its PHY. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 79 ++++++++++++++++++++++++++++ > 1 file changed, 79 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > index d7c9edff19f7..c5c93b6bcd2a 100644 > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > @@ -541,6 +541,85 @@ uart2: serial@98c000 { > }; > }; > > + ufs_mem_hc: ufshc@1d84000 { Generic node name, so ufs. > + compatible = "qcom,sm6350-ufshc", "qcom,ufshc", > + "jedec,ufs-2.0"; > + reg = <0 0x01d84000 0 0x3000>, > + <0 0x01d90000 0 0x8000>; > + reg-names = "std", "ice"; > + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&ufs_mem_phy_lanes>; > + phy-names = "ufsphy"; > + lanes-per-direction = <2>; > + #reset-cells = <1>; > + resets = <&gcc GCC_UFS_PHY_BCR>; > + reset-names = "rst"; > + > + power-domains = <&gcc UFS_PHY_GDSC>; > + > + iommus = <&apps_smmu 0x80 0x0>; > + > + clock-names = Drop unneeded blank line, start just after '=' and align next elements with it. > + "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 = The same. Best regards, Krzysztof
Hi Krzysztof, On Sat Mar 19, 2022 at 3:43 PM CET, Krzysztof Kozlowski wrote: > On 18/03/2022 19:30, Luca Weiss wrote: > > Add the necessary nodes for UFS and its PHY. > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > --- > > arch/arm64/boot/dts/qcom/sm6350.dtsi | 79 ++++++++++++++++++++++++++++ > > 1 file changed, 79 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > > index d7c9edff19f7..c5c93b6bcd2a 100644 > > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > > @@ -541,6 +541,85 @@ uart2: serial@98c000 { > > }; > > }; > > > > + ufs_mem_hc: ufshc@1d84000 { > > Generic node name, so ufs. With the node name changes UFS doesn't probe anymore. [ 1.893762] ufshcd-qcom 1d84000.ufs: ufshcd_variant_hba_init: variant qcom init failed err -19 [ 1.902674] ufshcd-qcom 1d84000.ufs: Initialization failed [ 1.908391] ufshcd-qcom 1d84000.ufs: ufshcd_pltfrm_init() failed -19 I didn't debug this in detail but it's likely from the androidboot.bootdevice=1d84000.ufshc parameter in cmdline that ufs-qcom.c uses to fail probe with -ENODEV for all UFS other than the selected one. Not sure why this behavior exists in mainline (didn't look into this either). This cmdline parameter (among many others) is added by the stock bootloader and as far as I know there's no way to turn that off. > > > + compatible = "qcom,sm6350-ufshc", "qcom,ufshc", > > + "jedec,ufs-2.0"; > > + reg = <0 0x01d84000 0 0x3000>, > > + <0 0x01d90000 0 0x8000>; > > + reg-names = "std", "ice"; > > + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; > > + phys = <&ufs_mem_phy_lanes>; > > + phy-names = "ufsphy"; > > + lanes-per-direction = <2>; > > + #reset-cells = <1>; > > + resets = <&gcc GCC_UFS_PHY_BCR>; > > + reset-names = "rst"; > > + > > + power-domains = <&gcc UFS_PHY_GDSC>; > > + > > + iommus = <&apps_smmu 0x80 0x0>; > > + > > + clock-names = > > Drop unneeded blank line, start just after '=' and align next elements > with it. Sure. > > > + "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 = > > The same. Sure. I also adjusted the clock name/order to match the bindings (thanks for spotting the problems there!) Regards Luca > > > Best regards, > Krzysztof
On 19/03/2022 19:29, Luca Weiss wrote: > Hi Krzysztof, > > On Sat Mar 19, 2022 at 3:43 PM CET, Krzysztof Kozlowski wrote: >> On 18/03/2022 19:30, Luca Weiss wrote: >>> Add the necessary nodes for UFS and its PHY. >>> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>> --- >>> arch/arm64/boot/dts/qcom/sm6350.dtsi | 79 ++++++++++++++++++++++++++++ >>> 1 file changed, 79 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi >>> index d7c9edff19f7..c5c93b6bcd2a 100644 >>> --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi >>> @@ -541,6 +541,85 @@ uart2: serial@98c000 { >>> }; >>> }; >>> >>> + ufs_mem_hc: ufshc@1d84000 { >> >> Generic node name, so ufs. > > With the node name changes UFS doesn't probe anymore. > > [ 1.893762] ufshcd-qcom 1d84000.ufs: ufshcd_variant_hba_init: variant qcom init failed err -19 > [ 1.902674] ufshcd-qcom 1d84000.ufs: Initialization failed > [ 1.908391] ufshcd-qcom 1d84000.ufs: ufshcd_pltfrm_init() failed -19 > > I didn't debug this in detail but it's likely from the > androidboot.bootdevice=1d84000.ufshc parameter in cmdline that > ufs-qcom.c uses to fail probe with -ENODEV for all UFS other than the > selected one. Not sure why this behavior exists in mainline (didn't look > into this either). > > This cmdline parameter (among many others) is added by the stock > bootloader and as far as I know there's no way to turn that off. I see now in the driver weird Android code like: static char android_boot_dev[ANDROID_BOOT_DEV_MAX]; .... if (strlen(android_boot_dev) && strcmp(android_boot_dev, dev_name(dev))) This is wrong. How is Android boot arguments needed for UFS? UFS is independent of Android... what if you run it with different bootloader and different system? I understand that it is inconvenient for you to change the name, but looking at driver code, I insist even more. :) Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi index d7c9edff19f7..c5c93b6bcd2a 100644 --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi @@ -541,6 +541,85 @@ uart2: serial@98c000 { }; }; + ufs_mem_hc: ufshc@1d84000 { + compatible = "qcom,sm6350-ufshc", "qcom,ufshc", + "jedec,ufs-2.0"; + reg = <0 0x01d84000 0 0x3000>, + <0 0x01d90000 0 0x8000>; + reg-names = "std", "ice"; + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; + phys = <&ufs_mem_phy_lanes>; + phy-names = "ufsphy"; + lanes-per-direction = <2>; + #reset-cells = <1>; + resets = <&gcc GCC_UFS_PHY_BCR>; + reset-names = "rst"; + + power-domains = <&gcc UFS_PHY_GDSC>; + + iommus = <&apps_smmu 0x80 0x0>; + + 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_PHY_AXI_CLK>, + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>, + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>, + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>, + <&rpmhcc RPMH_QLINK_CLK>, + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>, + <&gcc GCC_UFS_PHY_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>; + + status = "disabled"; + }; + + ufs_mem_phy: phy@1d87000 { + compatible = "qcom,sm6350-qmp-ufs-phy"; + reg = <0 0x01d87000 0 0x18c>; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + clock-names = "ref", + "ref_aux"; + clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; + + resets = <&ufs_mem_hc 0>; + reset-names = "ufsphy"; + + status = "disabled"; + + ufs_mem_phy_lanes: phy@1d87400 { + reg = <0 0x01d87400 0 0x128>, + <0 0x01d87600 0 0x1fc>, + <0 0x01d87c00 0 0x1dc>, + <0 0x01d87800 0 0x128>, + <0 0x01d87a00 0 0x1fc>; + #phy-cells = <0>; + }; + }; + tcsr_mutex: hwlock@1f40000 { compatible = "qcom,tcsr-mutex"; reg = <0x0 0x01f40000 0x0 0x40000>;
Add the necessary nodes for UFS and its PHY. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- arch/arm64/boot/dts/qcom/sm6350.dtsi | 79 ++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+)