| Message ID | 20240404122559.898930-8-peter.griffin@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HSI2, UFS & UFS phy support for Tensor GS101 | expand |
On 04/04/2024 14:25, Peter Griffin wrote: > Enable the ufs controller, ufs phy and ufs regulator in device tree. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > .../boot/dts/exynos/google/gs101-oriole.dts | 17 +++++++++ > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 35 +++++++++++++++++++ If you wish you can split DTSI and DTS into separate patches. Up to you. > 2 files changed, 52 insertions(+) > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > index 6be15e990b65..986eb5c9898a 100644 > --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > @@ -53,6 +53,14 @@ button-power { > wakeup-source; > }; > }; > + > + ufs_0_fixed_vcc_reg: regulator-0 { > + compatible = "regulator-fixed"; > + regulator-name = "ufs-vcc"; > + gpio = <&gpp0 1 0>; Use defines for GPIO flags, but more important: are you sure this is not coming from a PMIC? What's the voltage? It looks like a stub for missing PMIC, because UFS voltages are usually provided by PMIC. > + regulator-boot-on; > + enable-active-high; > + }; > }; > > &ext_24_5m { > @@ -106,6 +114,15 @@ &serial_0 { > status = "okay"; > }; > > +&ufs_0 { > + status = "okay"; > + vcc-supply = <&ufs_0_fixed_vcc_reg>; > +}; > + > +&ufs_0_phy { > + status = "okay"; > +}; > + > &usi_uart { > samsung,clkreq-on; /* needed for UART mode */ > status = "okay"; > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > index 608369cec47b..9c94829bf14c 100644 > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > @@ -1277,6 +1277,41 @@ pinctrl_hsi2: pinctrl@14440000 { > interrupts = <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH 0>; > }; > > + ufs_0_phy: phy@17e04000 { > + compatible = "google,gs101-ufs-phy"; > + reg = <0x14704000 0x3000>; > + reg-names = "phy-pma"; > + samsung,pmu-syscon = <&pmu_system_controller>; > + #phy-cells = <0>; > + clocks = <&ext_24_5m>; > + clock-names = "ref_clk"; > + status = "disabled"; > + }; > + > + ufs_0: ufs@14700000 { > + compatible = "google,gs101-ufs"; > + Drop blank line. > + reg = <0x14700000 0x200>, > + <0x14701100 0x200>, > + <0x14780000 0xa000>, > + <0x14600000 0x100>; Best regards, Krzysztof
Hi Krzysztof, Thanks for your review feedback. On Fri, 5 Apr 2024 at 08:53, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 04/04/2024 14:25, Peter Griffin wrote: > > Enable the ufs controller, ufs phy and ufs regulator in device tree. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > .../boot/dts/exynos/google/gs101-oriole.dts | 17 +++++++++ > > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 35 +++++++++++++++++++ > > If you wish you can split DTSI and DTS into separate patches. Up to you. Thanks for the heads up > > > 2 files changed, 52 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > index 6be15e990b65..986eb5c9898a 100644 > > --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > @@ -53,6 +53,14 @@ button-power { > > wakeup-source; > > }; > > }; > > + > > + ufs_0_fixed_vcc_reg: regulator-0 { > > + compatible = "regulator-fixed"; > > + regulator-name = "ufs-vcc"; > > + gpio = <&gpp0 1 0>; > > Use defines for GPIO flags, Will fix in v2 > but more important: are you sure this is not > coming from a PMIC? What's the voltage? It looks like a stub for missing > PMIC, because UFS voltages are usually provided by PMIC. UFS vcc is 1.2v. The gpio signal from gs101 SoC is called BOOTLD0, and it is connected to slave pmic (S2MPG11) UFS_EN signal which is a gpio enabled voltage rail for UFS (LDO8S). The downstream driver code declares the UFS supply as regulator-fixed even though it has a fully featured regulator driver for the pmic, with the LDO8S regulator exposed. Checking the DT for the pmic the min and max volt are different, so using regulator-fixed seems wrong for downstream. s_ldo8_reg: LDO8S { regulator-name = "S2MPG11_LDO8"; regulator-min-microvolt = <1127800>; regulator-max-microvolt = <1278600>; regulator-always-on; regulator-initial-mode = <SEC_OPMODE_SUSPEND>; channel-mux-selection = <0x28>; schematic-name = "L8S_UFS_VCCQ"; subsys-name = "UFS"; }; So I think you're correct this is a stub pending full pmic support. I propose in v2 to add a comment similar to what we have in exynos850-e850-96.dts today above the regulator-fixed node like /* TODO: Remove this once PMIC is implemented */? regards, Peter. > > > + regulator-boot-on; > > + enable-active-high; > > + }; > > }; > > > > &ext_24_5m { > > @@ -106,6 +114,15 @@ &serial_0 { > > status = "okay"; > > }; > > > > +&ufs_0 { > > + status = "okay"; > > + vcc-supply = <&ufs_0_fixed_vcc_reg>; > > +}; > > + > > +&ufs_0_phy { > > + status = "okay"; > > +}; > > + > > &usi_uart { > > samsung,clkreq-on; /* needed for UART mode */ > > status = "okay"; > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > index 608369cec47b..9c94829bf14c 100644 > > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > @@ -1277,6 +1277,41 @@ pinctrl_hsi2: pinctrl@14440000 { > > interrupts = <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH 0>; > > }; > > > > + ufs_0_phy: phy@17e04000 { > > + compatible = "google,gs101-ufs-phy"; > > + reg = <0x14704000 0x3000>; > > + reg-names = "phy-pma"; > > + samsung,pmu-syscon = <&pmu_system_controller>; > > + #phy-cells = <0>; > > + clocks = <&ext_24_5m>; > > + clock-names = "ref_clk"; > > + status = "disabled"; > > + }; > > + > > + ufs_0: ufs@14700000 { > > + compatible = "google,gs101-ufs"; > > + > > Drop blank line. > > > + reg = <0x14700000 0x200>, > > + <0x14701100 0x200>, > > + <0x14780000 0xa000>, > > + <0x14600000 0x100>; > > > Best regards, > Krzysztof >
On 18/04/2024 15:20, Peter Griffin wrote: > > s_ldo8_reg: LDO8S { > regulator-name = "S2MPG11_LDO8"; > regulator-min-microvolt = <1127800>; > regulator-max-microvolt = <1278600>; > regulator-always-on; > regulator-initial-mode = <SEC_OPMODE_SUSPEND>; > channel-mux-selection = <0x28>; > schematic-name = "L8S_UFS_VCCQ"; > subsys-name = "UFS"; > }; > > So I think you're correct this is a stub pending full pmic support. I > propose in v2 to add a comment similar to what we have in > exynos850-e850-96.dts today above the regulator-fixed node like /* > TODO: Remove this once PMIC is implemented */? > Sounds good. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts index 6be15e990b65..986eb5c9898a 100644 --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts @@ -53,6 +53,14 @@ button-power { wakeup-source; }; }; + + ufs_0_fixed_vcc_reg: regulator-0 { + compatible = "regulator-fixed"; + regulator-name = "ufs-vcc"; + gpio = <&gpp0 1 0>; + regulator-boot-on; + enable-active-high; + }; }; &ext_24_5m { @@ -106,6 +114,15 @@ &serial_0 { status = "okay"; }; +&ufs_0 { + status = "okay"; + vcc-supply = <&ufs_0_fixed_vcc_reg>; +}; + +&ufs_0_phy { + status = "okay"; +}; + &usi_uart { samsung,clkreq-on; /* needed for UART mode */ status = "okay"; diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi index 608369cec47b..9c94829bf14c 100644 --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi @@ -1277,6 +1277,41 @@ pinctrl_hsi2: pinctrl@14440000 { interrupts = <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH 0>; }; + ufs_0_phy: phy@17e04000 { + compatible = "google,gs101-ufs-phy"; + reg = <0x14704000 0x3000>; + reg-names = "phy-pma"; + samsung,pmu-syscon = <&pmu_system_controller>; + #phy-cells = <0>; + clocks = <&ext_24_5m>; + clock-names = "ref_clk"; + status = "disabled"; + }; + + ufs_0: ufs@14700000 { + compatible = "google,gs101-ufs"; + + reg = <0x14700000 0x200>, + <0x14701100 0x200>, + <0x14780000 0xa000>, + <0x14600000 0x100>; + reg-names = "hci", "vs_hci", "unipro", "ufsp"; + interrupts = <GIC_SPI 532 IRQ_TYPE_LEVEL_HIGH 0>; + clocks = <&cmu_hsi2 CLK_GOUT_HSI2_UFS_EMBD_I_ACLK>, + <&cmu_hsi2 CLK_GOUT_HSI2_UFS_EMBD_I_CLK_UNIPRO>, + <&cmu_hsi2 CLK_GOUT_HSI2_UFS_EMBD_I_FMP_CLK>, + <&cmu_hsi2 CLK_GOUT_HSI2_QE_UFS_EMBD_HSI2_ACLK>, + <&cmu_hsi2 CLK_GOUT_HSI2_QE_UFS_EMBD_HSI2_PCLK>; + clock-names = "core_clk", "sclk_unipro_main", "fmp", "ufs_aclk", "ufs_pclk"; + freq-table-hz = <0 0>, <0 0>, <0 0>, <0 0>, <0 0>; + pinctrl-names = "default"; + pinctrl-0 = <&ufs_rst_n &ufs_refclk_out>; + phys = <&ufs_0_phy>; + phy-names = "ufs-phy"; + samsung,sysreg = <&sysreg_hsi2 0x710>; + status = "disabled"; + }; + cmu_apm: clock-controller@17400000 { compatible = "google,gs101-cmu-apm"; reg = <0x17400000 0x8000>;
Enable the ufs controller, ufs phy and ufs regulator in device tree. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- .../boot/dts/exynos/google/gs101-oriole.dts | 17 +++++++++ arch/arm64/boot/dts/exynos/google/gs101.dtsi | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+)