All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 10:04 ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 10:04 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: k.kozlowski, devicetree, linux-kernel, linux-arm-kernel, kgene

This patch add pmic (s2mps15) node of espresso board,
which includes addition of regulators and pmic-clk sub-nodes.

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
This patch should go in after driver side changes [1] lands.
[1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html

 arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
 1 file changed, 349 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
index 838a3626dac1..8ce04a0ec928 100644
--- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
+++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
@@ -53,6 +53,355 @@
 	status = "okay";
 };
 
+&hsi2c_4 {
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <200000>;
+	status = "okay";
+
+	s2mps15_pmic@66 {
+		compatible = "samsung,s2mps15-pmic";
+		reg = <0x66>;
+		interrupts = <2 0>;
+		interrupt-parent = <&gpa0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_irq>;
+		wakeup-source;
+
+		s2mps15_osc: clocks {
+			compatible = "samsung,s2mps13-clk";
+			#clock-cells = <1>;
+			clock-output-names = "s2mps13_ap", "s2mps13_cp",
+				"s2mps13_bt";
+		};
+
+		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "vdd_ldo1";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <900000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo2_reg: LDO2 {
+				regulator-name = "vdd_ldo2";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <125>;
+				regulator-always-on;
+			};
+
+			ldo3_reg: LDO3 {
+				regulator-name = "vdd_ldo3";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <1980000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo4_reg: LDO4 {
+				regulator-name = "vdd_ldo4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1110000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo5_reg: LDO5 {
+				regulator-name = "vdd_ldo5";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <1980000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo6_reg: LDO6 {
+				regulator-name = "vdd_ldo6";
+				regulator-min-microvolt = <2250000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo7_reg: LDO7 {
+				regulator-name = "vdd_ldo7";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1150000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo8_reg: LDO8 {
+				regulator-name = "vdd_ldo8";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo9_reg: LDO9 {
+				regulator-name = "vdd_ldo9";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo10_reg: LDO10 {
+				regulator-name = "vdd_ldo10";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo11_reg: LDO11 {
+				regulator-name = "vdd_ldo11";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo12_reg: LDO12 {
+				regulator-name = "vdd_ldo12";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo13_reg: LDO13 {
+				regulator-name = "vdd_ldo13";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo14_reg: LDO14 {
+				regulator-name = "vdd_ldo14";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo15_reg: LDO15 {
+				regulator-name = "vdd_ldo15";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo16_reg: LDO16 {
+				regulator-name = "vdd_ldo16";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo17_reg: LDO17 {
+				regulator-name = "vdd_ldo17";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo18_reg: LDO18 {
+				regulator-name = "vdd_ldo18";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo19_reg: LDO19 {
+				regulator-name = "vdd_ldo19";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo20_reg: LDO20 {
+				regulator-name = "vdd_ldo20";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo21_reg: LDO21 {
+				regulator-name = "vdd_ldo21";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo22_reg: LDO22 {
+				regulator-name = "vdd_ldo22";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo23_reg: LDO23 {
+				regulator-name = "vdd_ldo23";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo24_reg: LDO24 {
+				regulator-name = "vdd_ldo24";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo25_reg: LDO25 {
+				regulator-name = "vdd_ldo25";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo26_reg: LDO26 {
+				regulator-name = "vdd_ldo26";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1470000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo27_reg: LDO27 {
+				regulator-name = "vdd_ldo27";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck2_reg: BUCK2 {
+				regulator-name = "vdd_atlas";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck3_reg: BUCK3 {
+				regulator-name = "vdd_apollo";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck4_reg: BUCK4 {
+				regulator-name = "vdd_int";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck5_reg: BUCK5 {
+				regulator-name = "vdd_buck5";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck6_reg: BUCK6 {
+				regulator-name = "vdd_g3d";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1400000>;
+				regulator-always-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck7_reg: BUCK7 {
+				regulator-name = "vdd_buck7";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1500000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck8_reg: BUCK8 {
+				regulator-name = "vdd_buck8";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1500000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck9_reg: BUCK9 {
+				regulator-name = "vdd_buck9";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <2100000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck10_reg: BUCK10 {
+				regulator-name = "vdd_buck10";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+		};
+	};
+};
+
+&pinctrl_alive {
+	pmic_irq: pmic-irq {
+		samsung,pins = "gpa0-2";
+		samsung,pin-pud = <3>;
+		samsung,pin-drv = <3>;
+	};
+};
+
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 10:04 ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 10:04 UTC (permalink / raw)
  To: linux-arm-kernel

This patch add pmic (s2mps15) node of espresso board,
which includes addition of regulators and pmic-clk sub-nodes.

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
This patch should go in after driver side changes [1] lands.
[1]-> https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg47736.html

 arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
 1 file changed, 349 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
index 838a3626dac1..8ce04a0ec928 100644
--- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
+++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
@@ -53,6 +53,355 @@
 	status = "okay";
 };
 
+&hsi2c_4 {
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <200000>;
+	status = "okay";
+
+	s2mps15_pmic at 66 {
+		compatible = "samsung,s2mps15-pmic";
+		reg = <0x66>;
+		interrupts = <2 0>;
+		interrupt-parent = <&gpa0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_irq>;
+		wakeup-source;
+
+		s2mps15_osc: clocks {
+			compatible = "samsung,s2mps13-clk";
+			#clock-cells = <1>;
+			clock-output-names = "s2mps13_ap", "s2mps13_cp",
+				"s2mps13_bt";
+		};
+
+		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "vdd_ldo1";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <900000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo2_reg: LDO2 {
+				regulator-name = "vdd_ldo2";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <125>;
+				regulator-always-on;
+			};
+
+			ldo3_reg: LDO3 {
+				regulator-name = "vdd_ldo3";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <1980000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo4_reg: LDO4 {
+				regulator-name = "vdd_ldo4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1110000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo5_reg: LDO5 {
+				regulator-name = "vdd_ldo5";
+				regulator-min-microvolt = <1620000>;
+				regulator-max-microvolt = <1980000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo6_reg: LDO6 {
+				regulator-name = "vdd_ldo6";
+				regulator-min-microvolt = <2250000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo7_reg: LDO7 {
+				regulator-name = "vdd_ldo7";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1150000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo8_reg: LDO8 {
+				regulator-name = "vdd_ldo8";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo9_reg: LDO9 {
+				regulator-name = "vdd_ldo9";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo10_reg: LDO10 {
+				regulator-name = "vdd_ldo10";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo11_reg: LDO11 {
+				regulator-name = "vdd_ldo11";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo12_reg: LDO12 {
+				regulator-name = "vdd_ldo12";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo13_reg: LDO13 {
+				regulator-name = "vdd_ldo13";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo14_reg: LDO14 {
+				regulator-name = "vdd_ldo14";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo15_reg: LDO15 {
+				regulator-name = "vdd_ldo15";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo16_reg: LDO16 {
+				regulator-name = "vdd_ldo16";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo17_reg: LDO17 {
+				regulator-name = "vdd_ldo17";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo18_reg: LDO18 {
+				regulator-name = "vdd_ldo18";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo19_reg: LDO19 {
+				regulator-name = "vdd_ldo19";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo20_reg: LDO20 {
+				regulator-name = "vdd_ldo20";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo21_reg: LDO21 {
+				regulator-name = "vdd_ldo21";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo22_reg: LDO22 {
+				regulator-name = "vdd_ldo22";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo23_reg: LDO23 {
+				regulator-name = "vdd_ldo23";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo24_reg: LDO24 {
+				regulator-name = "vdd_ldo24";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo25_reg: LDO25 {
+				regulator-name = "vdd_ldo25";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3375000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo26_reg: LDO26 {
+				regulator-name = "vdd_ldo26";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1470000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			ldo27_reg: LDO27 {
+				regulator-name = "vdd_ldo27";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2275000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <125>;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck2_reg: BUCK2 {
+				regulator-name = "vdd_atlas";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck3_reg: BUCK3 {
+				regulator-name = "vdd_apollo";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck4_reg: BUCK4 {
+				regulator-name = "vdd_int";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck5_reg: BUCK5 {
+				regulator-name = "vdd_buck5";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck6_reg: BUCK6 {
+				regulator-name = "vdd_g3d";
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1400000>;
+				regulator-always-on;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck7_reg: BUCK7 {
+				regulator-name = "vdd_buck7";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1500000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck8_reg: BUCK8 {
+				regulator-name = "vdd_buck8";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1500000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck9_reg: BUCK9 {
+				regulator-name = "vdd_buck9";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <2100000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+
+			buck10_reg: BUCK10 {
+				regulator-name = "vdd_buck10";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <25000>;
+				regulator-enable-ramp-delay = <250>;
+			};
+		};
+	};
+};
+
+&pinctrl_alive {
+	pmic_irq: pmic-irq {
+		samsung,pins = "gpa0-2";
+		samsung,pin-pud = <3>;
+		samsung,pin-drv = <3>;
+	};
+};
+
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
  2015-11-02 10:04 ` Alim Akhtar
@ 2015-11-02 10:04   ` Alim Akhtar
  -1 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 10:04 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: k.kozlowski, devicetree, linux-kernel, linux-arm-kernel, kgene

This patch enables HS200 mode operation on exynos7 based
espresso board.
This also remove _broken-cd_ property as per mmc binding documentation
which say one of the properties between broken-cd and non-removable
should be used. And we already use _non-removable_ as emmc mounted
on board which is a non-removable device.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
index 8ce04a0ec928..7f19b99fc85b 100644
--- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
+++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
@@ -405,8 +405,8 @@
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
-	broken-cd;
 	cap-mmc-highspeed;
+	mmc-hs200-1_8v;
 	non-removable;
 	card-detect-delay = <200>;
 	clock-frequency = <800000000>;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
@ 2015-11-02 10:04   ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 10:04 UTC (permalink / raw)
  To: linux-arm-kernel

This patch enables HS200 mode operation on exynos7 based
espresso board.
This also remove _broken-cd_ property as per mmc binding documentation
which say one of the properties between broken-cd and non-removable
should be used. And we already use _non-removable_ as emmc mounted
on board which is a non-removable device.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
index 8ce04a0ec928..7f19b99fc85b 100644
--- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
+++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
@@ -405,8 +405,8 @@
 &mmc_0 {
 	status = "okay";
 	num-slots = <1>;
-	broken-cd;
 	cap-mmc-highspeed;
+	mmc-hs200-1_8v;
 	non-removable;
 	card-detect-delay = <200>;
 	clock-frequency = <800000000>;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 11:30   ` Javier Martinez Canillas
  0 siblings, 0 replies; 28+ messages in thread
From: Javier Martinez Canillas @ 2015-11-02 11:30 UTC (permalink / raw)
  To: Alim Akhtar, linux-samsung-soc
  Cc: k.kozlowski, devicetree, linux-kernel, linux-arm-kernel, kgene

Hello Alim,

This patch looks mostly good to me, I just have two comments below:

On 11/02/2015 11:04 AM, Alim Akhtar wrote:
> This patch add pmic (s2mps15) node of espresso board,
> which includes addition of regulators and pmic-clk sub-nodes.
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> This patch should go in after driver side changes [1] lands.
> [1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html
> 
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>  1 file changed, 349 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>  	status = "okay";
>  };
>  
> +&hsi2c_4 {
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <200000>;
> +	status = "okay";
> +
> +	s2mps15_pmic@66 {
> +		compatible = "samsung,s2mps15-pmic";
> +		reg = <0x66>;
> +		interrupts = <2 0>;

Maybe using IRQ_TYPE_NONE instead of 0?

> +		interrupt-parent = <&gpa0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_irq>;
> +		wakeup-source;
> +
> +		s2mps15_osc: clocks {
> +			compatible = "samsung,s2mps13-clk";
> +			#clock-cells = <1>;
> +			clock-output-names = "s2mps13_ap", "s2mps13_cp",
> +				"s2mps13_bt";
> +		};
> +
> +		regulators {
> +			ldo1_reg: LDO1 {
> +				regulator-name = "vdd_ldo1";
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-always-on;

I see that all regulators are marked as regulator-always-on but that will
prevent the regulator subsystem to disable unused regulators. Can you please
double check which regulators should really be always on and which ones can
be disabled if are not used?

After those two changes:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 11:30   ` Javier Martinez Canillas
  0 siblings, 0 replies; 28+ messages in thread
From: Javier Martinez Canillas @ 2015-11-02 11:30 UTC (permalink / raw)
  To: Alim Akhtar, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
  Cc: k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kgene-DgEjT+Ai2ygdnm+yROfE0A

Hello Alim,

This patch looks mostly good to me, I just have two comments below:

On 11/02/2015 11:04 AM, Alim Akhtar wrote:
> This patch add pmic (s2mps15) node of espresso board,
> which includes addition of regulators and pmic-clk sub-nodes.
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Alim Akhtar <alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> This patch should go in after driver side changes [1] lands.
> [1]-> https://www.mail-archive.com/linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg47736.html
> 
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>  1 file changed, 349 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>  	status = "okay";
>  };
>  
> +&hsi2c_4 {
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <200000>;
> +	status = "okay";
> +
> +	s2mps15_pmic@66 {
> +		compatible = "samsung,s2mps15-pmic";
> +		reg = <0x66>;
> +		interrupts = <2 0>;

Maybe using IRQ_TYPE_NONE instead of 0?

> +		interrupt-parent = <&gpa0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_irq>;
> +		wakeup-source;
> +
> +		s2mps15_osc: clocks {
> +			compatible = "samsung,s2mps13-clk";
> +			#clock-cells = <1>;
> +			clock-output-names = "s2mps13_ap", "s2mps13_cp",
> +				"s2mps13_bt";
> +		};
> +
> +		regulators {
> +			ldo1_reg: LDO1 {
> +				regulator-name = "vdd_ldo1";
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-always-on;

I see that all regulators are marked as regulator-always-on but that will
prevent the regulator subsystem to disable unused regulators. Can you please
double check which regulators should really be always on and which ones can
be disabled if are not used?

After those two changes:

Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 11:30   ` Javier Martinez Canillas
  0 siblings, 0 replies; 28+ messages in thread
From: Javier Martinez Canillas @ 2015-11-02 11:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Alim,

This patch looks mostly good to me, I just have two comments below:

On 11/02/2015 11:04 AM, Alim Akhtar wrote:
> This patch add pmic (s2mps15) node of espresso board,
> which includes addition of regulators and pmic-clk sub-nodes.
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> This patch should go in after driver side changes [1] lands.
> [1]-> https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg47736.html
> 
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>  1 file changed, 349 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>  	status = "okay";
>  };
>  
> +&hsi2c_4 {
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-max-bus-freq = <200000>;
> +	status = "okay";
> +
> +	s2mps15_pmic at 66 {
> +		compatible = "samsung,s2mps15-pmic";
> +		reg = <0x66>;
> +		interrupts = <2 0>;

Maybe using IRQ_TYPE_NONE instead of 0?

> +		interrupt-parent = <&gpa0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_irq>;
> +		wakeup-source;
> +
> +		s2mps15_osc: clocks {
> +			compatible = "samsung,s2mps13-clk";
> +			#clock-cells = <1>;
> +			clock-output-names = "s2mps13_ap", "s2mps13_cp",
> +				"s2mps13_bt";
> +		};
> +
> +		regulators {
> +			ldo1_reg: LDO1 {
> +				regulator-name = "vdd_ldo1";
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-always-on;

I see that all regulators are marked as regulator-always-on but that will
prevent the regulator subsystem to disable unused regulators. Can you please
double check which regulators should really be always on and which ones can
be disabled if are not used?

After those two changes:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
  2015-11-02 10:04   ` Alim Akhtar
@ 2015-11-02 11:32     ` Javier Martinez Canillas
  -1 siblings, 0 replies; 28+ messages in thread
From: Javier Martinez Canillas @ 2015-11-02 11:32 UTC (permalink / raw)
  To: Alim Akhtar, linux-samsung-soc
  Cc: k.kozlowski, devicetree, linux-kernel, linux-arm-kernel, kgene

Hello Alim,

On 11/02/2015 11:04 AM, Alim Akhtar wrote:
> This patch enables HS200 mode operation on exynos7 based
> espresso board.
> This also remove _broken-cd_ property as per mmc binding documentation
> which say one of the properties between broken-cd and non-removable
> should be used. And we already use _non-removable_ as emmc mounted
> on board which is a non-removable device.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 8ce04a0ec928..7f19b99fc85b 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -405,8 +405,8 @@
>  &mmc_0 {
>  	status = "okay";
>  	num-slots = <1>;
> -	broken-cd;
>  	cap-mmc-highspeed;
> +	mmc-hs200-1_8v;
>  	non-removable;
>  	card-detect-delay = <200>;
>  	clock-frequency = <800000000>;
> 

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
@ 2015-11-02 11:32     ` Javier Martinez Canillas
  0 siblings, 0 replies; 28+ messages in thread
From: Javier Martinez Canillas @ 2015-11-02 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Alim,

On 11/02/2015 11:04 AM, Alim Akhtar wrote:
> This patch enables HS200 mode operation on exynos7 based
> espresso board.
> This also remove _broken-cd_ property as per mmc binding documentation
> which say one of the properties between broken-cd and non-removable
> should be used. And we already use _non-removable_ as emmc mounted
> on board which is a non-removable device.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 8ce04a0ec928..7f19b99fc85b 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -405,8 +405,8 @@
>  &mmc_0 {
>  	status = "okay";
>  	num-slots = <1>;
> -	broken-cd;
>  	cap-mmc-highspeed;
> +	mmc-hs200-1_8v;
>  	non-removable;
>  	card-detect-delay = <200>;
>  	clock-frequency = <800000000>;
> 

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
  2015-11-02 11:30   ` Javier Martinez Canillas
@ 2015-11-02 11:58     ` Alim Akhtar
  -1 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 11:58 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-samsung-soc
  Cc: k.kozlowski, devicetree, linux-kernel, linux-arm-kernel, kgene



On 11/02/2015 05:00 PM, Javier Martinez Canillas wrote:
> Hello Alim,
>
> This patch looks mostly good to me, I just have two comments below:
>
> On 11/02/2015 11:04 AM, Alim Akhtar wrote:
>> This patch add pmic (s2mps15) node of espresso board,
>> which includes addition of regulators and pmic-clk sub-nodes.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> This patch should go in after driver side changes [1] lands.
>> [1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html
>>
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>>   1 file changed, 349 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 838a3626dac1..8ce04a0ec928 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -53,6 +53,355 @@
>>   	status = "okay";
>>   };
>>
>> +&hsi2c_4 {
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <200000>;
>> +	status = "okay";
>> +
>> +	s2mps15_pmic@66 {
>> +		compatible = "samsung,s2mps15-pmic";
>> +		reg = <0x66>;
>> +		interrupts = <2 0>;
>
> Maybe using IRQ_TYPE_NONE instead of 0?
>
Ok will check
>> +		interrupt-parent = <&gpa0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pmic_irq>;
>> +		wakeup-source;
>> +
>> +		s2mps15_osc: clocks {
>> +			compatible = "samsung,s2mps13-clk";
>> +			#clock-cells = <1>;
>> +			clock-output-names = "s2mps13_ap", "s2mps13_cp",
>> +				"s2mps13_bt";
>> +		};
>> +
>> +		regulators {
>> +			ldo1_reg: LDO1 {
>> +				regulator-name = "vdd_ldo1";
>> +				regulator-min-microvolt = <500000>;
>> +				regulator-max-microvolt = <900000>;
>> +				regulator-always-on;
>
> I see that all regulators are marked as regulator-always-on but that will
> prevent the regulator subsystem to disable unused regulators. Can you please
> double check which regulators should really be always on and which ones can
> be disabled if are not used?
>
I kept it on always, as I am still in a process for validating all IPs 
on board. Will see if I can remove some of them, which I feel can be done.
> After those two changes:
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
Thanks!
> Best regards,
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 11:58     ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 11:58 UTC (permalink / raw)
  To: linux-arm-kernel



On 11/02/2015 05:00 PM, Javier Martinez Canillas wrote:
> Hello Alim,
>
> This patch looks mostly good to me, I just have two comments below:
>
> On 11/02/2015 11:04 AM, Alim Akhtar wrote:
>> This patch add pmic (s2mps15) node of espresso board,
>> which includes addition of regulators and pmic-clk sub-nodes.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> This patch should go in after driver side changes [1] lands.
>> [1]-> https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg47736.html
>>
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>>   1 file changed, 349 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 838a3626dac1..8ce04a0ec928 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -53,6 +53,355 @@
>>   	status = "okay";
>>   };
>>
>> +&hsi2c_4 {
>> +	samsung,i2c-sda-delay = <100>;
>> +	samsung,i2c-max-bus-freq = <200000>;
>> +	status = "okay";
>> +
>> +	s2mps15_pmic at 66 {
>> +		compatible = "samsung,s2mps15-pmic";
>> +		reg = <0x66>;
>> +		interrupts = <2 0>;
>
> Maybe using IRQ_TYPE_NONE instead of 0?
>
Ok will check
>> +		interrupt-parent = <&gpa0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pmic_irq>;
>> +		wakeup-source;
>> +
>> +		s2mps15_osc: clocks {
>> +			compatible = "samsung,s2mps13-clk";
>> +			#clock-cells = <1>;
>> +			clock-output-names = "s2mps13_ap", "s2mps13_cp",
>> +				"s2mps13_bt";
>> +		};
>> +
>> +		regulators {
>> +			ldo1_reg: LDO1 {
>> +				regulator-name = "vdd_ldo1";
>> +				regulator-min-microvolt = <500000>;
>> +				regulator-max-microvolt = <900000>;
>> +				regulator-always-on;
>
> I see that all regulators are marked as regulator-always-on but that will
> prevent the regulator subsystem to disable unused regulators. Can you please
> double check which regulators should really be always on and which ones can
> be disabled if are not used?
>
I kept it on always, as I am still in a process for validating all IPs 
on board. Will see if I can remove some of them, which I feel can be done.
> After those two changes:
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
Thanks!
> Best regards,
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
  2015-11-02 10:04   ` Alim Akhtar
@ 2015-11-02 11:58     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 11:58 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: linux-samsung-soc, Krzysztof Kozlowski, devicetree, linux-kernel,
	linux-arm-kernel, kgene

Subject - name of the board at the end (not the name of the SoC):
arm64: dts: Enable HS200 mode operation on exynos7-espresso

2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> This patch enables HS200 mode operation on exynos7 based
> espresso board.
> This also remove _broken-cd_ property as per mmc binding documentation

s/remove/removes/

> which say one of the properties between broken-cd and non-removable

s/say/says/

> should be used. And we already use _non-removable_ as emmc mounted
> on board which is a non-removable device.
>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 8ce04a0ec928..7f19b99fc85b 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -405,8 +405,8 @@
>  &mmc_0 {
>         status = "okay";
>         num-slots = <1>;
> -       broken-cd;
>         cap-mmc-highspeed;
> +       mmc-hs200-1_8v;

The "mmc-ddr-1_8v" should also be supported but I guess you did not
add it on purpose?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
@ 2015-11-02 11:58     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 11:58 UTC (permalink / raw)
  To: linux-arm-kernel

Subject - name of the board at the end (not the name of the SoC):
arm64: dts: Enable HS200 mode operation on exynos7-espresso

2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> This patch enables HS200 mode operation on exynos7 based
> espresso board.
> This also remove _broken-cd_ property as per mmc binding documentation

s/remove/removes/

> which say one of the properties between broken-cd and non-removable

s/say/says/

> should be used. And we already use _non-removable_ as emmc mounted
> on board which is a non-removable device.
>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 8ce04a0ec928..7f19b99fc85b 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -405,8 +405,8 @@
>  &mmc_0 {
>         status = "okay";
>         num-slots = <1>;
> -       broken-cd;
>         cap-mmc-highspeed;
> +       mmc-hs200-1_8v;

The "mmc-ddr-1_8v" should also be supported but I guess you did not
add it on purpose?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
  2015-11-02 11:58     ` Krzysztof Kozlowski
@ 2015-11-02 12:08       ` Alim Akhtar
  -1 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 12:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, devicetree, linux-kernel, linux-arm-kernel, kgene

Hi Krzysztof,

On 11/02/2015 05:28 PM, Krzysztof Kozlowski wrote:
> Subject - name of the board at the end (not the name of the SoC):
> arm64: dts: Enable HS200 mode operation on exynos7-espresso
>
> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>> This patch enables HS200 mode operation on exynos7 based
>> espresso board.
>> This also remove _broken-cd_ property as per mmc binding documentation
>
> s/remove/removes/
>
>> which say one of the properties between broken-cd and non-removable
>
> s/say/says/
>
>> should be used. And we already use _non-removable_ as emmc mounted
>> on board which is a non-removable device.
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 8ce04a0ec928..7f19b99fc85b 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -405,8 +405,8 @@
>>   &mmc_0 {
>>          status = "okay";
>>          num-slots = <1>;
>> -       broken-cd;
>>          cap-mmc-highspeed;
>> +       mmc-hs200-1_8v;
>
> The "mmc-ddr-1_8v" should also be supported but I guess you did not
> add it on purpose?
>
Well, I have not tested it, that why didn't added. Will check if this 
works then will add.
Thanks for your comments.

> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
@ 2015-11-02 12:08       ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Krzysztof,

On 11/02/2015 05:28 PM, Krzysztof Kozlowski wrote:
> Subject - name of the board at the end (not the name of the SoC):
> arm64: dts: Enable HS200 mode operation on exynos7-espresso
>
> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>> This patch enables HS200 mode operation on exynos7 based
>> espresso board.
>> This also remove _broken-cd_ property as per mmc binding documentation
>
> s/remove/removes/
>
>> which say one of the properties between broken-cd and non-removable
>
> s/say/says/
>
>> should be used. And we already use _non-removable_ as emmc mounted
>> on board which is a non-removable device.
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 8ce04a0ec928..7f19b99fc85b 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -405,8 +405,8 @@
>>   &mmc_0 {
>>          status = "okay";
>>          num-slots = <1>;
>> -       broken-cd;
>>          cap-mmc-highspeed;
>> +       mmc-hs200-1_8v;
>
> The "mmc-ddr-1_8v" should also be supported but I guess you did not
> add it on purpose?
>
Well, I have not tested it, that why didn't added. Will check if this 
works then will add.
Thanks for your comments.

> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
  2015-11-02 10:04 ` Alim Akhtar
@ 2015-11-02 12:25   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 12:25 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: linux-samsung-soc, Krzysztof Kozlowski, devicetree, linux-kernel,
	linux-arm-kernel, kgene

2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> This patch add pmic (s2mps15) node of espresso board,

This patch adds PMIC (S2MPS15)...

> which includes addition of regulators and pmic-clk sub-nodes.
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> This patch should go in after driver side changes [1] lands.
> [1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html

Eh? Why? Usually there is not such requirement.

>
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>  1 file changed, 349 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>         status = "okay";
>  };
>
> +&hsi2c_4 {
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <200000>;
> +       status = "okay";
> +
> +       s2mps15_pmic@66 {
> +               compatible = "samsung,s2mps15-pmic";
> +               reg = <0x66>;
> +               interrupts = <2 0>;
> +               interrupt-parent = <&gpa0>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pmic_irq>;
> +               wakeup-source;
> +
> +               s2mps15_osc: clocks {
> +                       compatible = "samsung,s2mps13-clk";
> +                       #clock-cells = <1>;
> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
> +                               "s2mps13_bt";
> +               };

Don't you want to use one of these clocks for s3c-rtc (&rtc node)?

> +
> +               regulators {
> +                       ldo1_reg: LDO1 {
> +                               regulator-name = "vdd_ldo1";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <900000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo2_reg: LDO2 {
> +                               regulator-name = "vdd_ldo2";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <3300000>;
> +                               regulator-enable-ramp-delay = <125>;
> +                               regulator-always-on;
> +                       };
> +
> +                       ldo3_reg: LDO3 {
> +                               regulator-name = "vdd_ldo3";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <1980000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo4_reg: LDO4 {
> +                               regulator-name = "vdd_ldo4";
> +                               regulator-min-microvolt = <800000>;
> +                               regulator-max-microvolt = <1110000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo5_reg: LDO5 {
> +                               regulator-name = "vdd_ldo5";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <1980000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo6_reg: LDO6 {
> +                               regulator-name = "vdd_ldo6";
> +                               regulator-min-microvolt = <2250000>;
> +                               regulator-max-microvolt = <3300000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo7_reg: LDO7 {
> +                               regulator-name = "vdd_ldo7";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1150000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo8_reg: LDO8 {
> +                               regulator-name = "vdd_ldo8";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo9_reg: LDO9 {
> +                               regulator-name = "vdd_ldo9";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo10_reg: LDO10 {
> +                               regulator-name = "vdd_ldo10";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo11_reg: LDO11 {
> +                               regulator-name = "vdd_ldo11";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo12_reg: LDO12 {
> +                               regulator-name = "vdd_ldo12";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo13_reg: LDO13 {
> +                               regulator-name = "vdd_ldo13";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo14_reg: LDO14 {
> +                               regulator-name = "vdd_ldo14";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo15_reg: LDO15 {
> +                               regulator-name = "vdd_ldo15";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo16_reg: LDO16 {
> +                               regulator-name = "vdd_ldo16";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo17_reg: LDO17 {
> +                               regulator-name = "vdd_ldo17";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo18_reg: LDO18 {
> +                               regulator-name = "vdd_ldo18";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo19_reg: LDO19 {
> +                               regulator-name = "vdd_ldo19";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo20_reg: LDO20 {
> +                               regulator-name = "vdd_ldo20";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo21_reg: LDO21 {
> +                               regulator-name = "vdd_ldo21";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo22_reg: LDO22 {
> +                               regulator-name = "vdd_ldo22";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo23_reg: LDO23 {
> +                               regulator-name = "vdd_ldo23";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo24_reg: LDO24 {
> +                               regulator-name = "vdd_ldo24";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo25_reg: LDO25 {
> +                               regulator-name = "vdd_ldo25";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo26_reg: LDO26 {
> +                               regulator-name = "vdd_ldo26";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1470000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo27_reg: LDO27 {
> +                               regulator-name = "vdd_ldo27";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       buck1_reg: BUCK1 {
> +                               regulator-name = "vdd_mif";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck2_reg: BUCK2 {
> +                               regulator-name = "vdd_atlas";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck3_reg: BUCK3 {
> +                               regulator-name = "vdd_apollo";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck4_reg: BUCK4 {
> +                               regulator-name = "vdd_int";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck5_reg: BUCK5 {
> +                               regulator-name = "vdd_buck5";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck6_reg: BUCK6 {
> +                               regulator-name = "vdd_g3d";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1400000>;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck7_reg: BUCK7 {
> +                               regulator-name = "vdd_buck7";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1500000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck8_reg: BUCK8 {
> +                               regulator-name = "vdd_buck8";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1500000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck9_reg: BUCK9 {
> +                               regulator-name = "vdd_buck9";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <2100000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck10_reg: BUCK10 {
> +                               regulator-name = "vdd_buck10";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <3000000>;
> +                               regulator-boot-on;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };

All of these ldo3 and bucks in vendor tree for Espresso board have
ramp delay of 12000. Also they don't have enable-ramp-delay set and
voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
know what is the true value... I'll leave it up to you but it looks
suspicious.

> +               };
> +       };
> +};

What will be the benefit of defining all of these regulators if they
are always on and without consumers? No one will disable them, no one
will change the voltage. Please provide some consumers.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 12:25   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 12:25 UTC (permalink / raw)
  To: linux-arm-kernel

2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> This patch add pmic (s2mps15) node of espresso board,

This patch adds PMIC (S2MPS15)...

> which includes addition of regulators and pmic-clk sub-nodes.
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> This patch should go in after driver side changes [1] lands.
> [1]-> https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg47736.html

Eh? Why? Usually there is not such requirement.

>
>  arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>  1 file changed, 349 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>         status = "okay";
>  };
>
> +&hsi2c_4 {
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <200000>;
> +       status = "okay";
> +
> +       s2mps15_pmic at 66 {
> +               compatible = "samsung,s2mps15-pmic";
> +               reg = <0x66>;
> +               interrupts = <2 0>;
> +               interrupt-parent = <&gpa0>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pmic_irq>;
> +               wakeup-source;
> +
> +               s2mps15_osc: clocks {
> +                       compatible = "samsung,s2mps13-clk";
> +                       #clock-cells = <1>;
> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
> +                               "s2mps13_bt";
> +               };

Don't you want to use one of these clocks for s3c-rtc (&rtc node)?

> +
> +               regulators {
> +                       ldo1_reg: LDO1 {
> +                               regulator-name = "vdd_ldo1";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <900000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo2_reg: LDO2 {
> +                               regulator-name = "vdd_ldo2";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <3300000>;
> +                               regulator-enable-ramp-delay = <125>;
> +                               regulator-always-on;
> +                       };
> +
> +                       ldo3_reg: LDO3 {
> +                               regulator-name = "vdd_ldo3";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <1980000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo4_reg: LDO4 {
> +                               regulator-name = "vdd_ldo4";
> +                               regulator-min-microvolt = <800000>;
> +                               regulator-max-microvolt = <1110000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo5_reg: LDO5 {
> +                               regulator-name = "vdd_ldo5";
> +                               regulator-min-microvolt = <1620000>;
> +                               regulator-max-microvolt = <1980000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo6_reg: LDO6 {
> +                               regulator-name = "vdd_ldo6";
> +                               regulator-min-microvolt = <2250000>;
> +                               regulator-max-microvolt = <3300000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo7_reg: LDO7 {
> +                               regulator-name = "vdd_ldo7";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1150000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo8_reg: LDO8 {
> +                               regulator-name = "vdd_ldo8";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo9_reg: LDO9 {
> +                               regulator-name = "vdd_ldo9";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo10_reg: LDO10 {
> +                               regulator-name = "vdd_ldo10";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo11_reg: LDO11 {
> +                               regulator-name = "vdd_ldo11";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo12_reg: LDO12 {
> +                               regulator-name = "vdd_ldo12";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo13_reg: LDO13 {
> +                               regulator-name = "vdd_ldo13";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo14_reg: LDO14 {
> +                               regulator-name = "vdd_ldo14";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo15_reg: LDO15 {
> +                               regulator-name = "vdd_ldo15";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo16_reg: LDO16 {
> +                               regulator-name = "vdd_ldo16";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo17_reg: LDO17 {
> +                               regulator-name = "vdd_ldo17";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo18_reg: LDO18 {
> +                               regulator-name = "vdd_ldo18";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo19_reg: LDO19 {
> +                               regulator-name = "vdd_ldo19";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo20_reg: LDO20 {
> +                               regulator-name = "vdd_ldo20";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo21_reg: LDO21 {
> +                               regulator-name = "vdd_ldo21";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo22_reg: LDO22 {
> +                               regulator-name = "vdd_ldo22";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo23_reg: LDO23 {
> +                               regulator-name = "vdd_ldo23";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo24_reg: LDO24 {
> +                               regulator-name = "vdd_ldo24";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo25_reg: LDO25 {
> +                               regulator-name = "vdd_ldo25";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3375000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo26_reg: LDO26 {
> +                               regulator-name = "vdd_ldo26";
> +                               regulator-min-microvolt = <700000>;
> +                               regulator-max-microvolt = <1470000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       ldo27_reg: LDO27 {
> +                               regulator-name = "vdd_ldo27";
> +                               regulator-min-microvolt = <1500000>;
> +                               regulator-max-microvolt = <2275000>;
> +                               regulator-always-on;
> +                               regulator-enable-ramp-delay = <125>;
> +                       };
> +
> +                       buck1_reg: BUCK1 {
> +                               regulator-name = "vdd_mif";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck2_reg: BUCK2 {
> +                               regulator-name = "vdd_atlas";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck3_reg: BUCK3 {
> +                               regulator-name = "vdd_apollo";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck4_reg: BUCK4 {
> +                               regulator-name = "vdd_int";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck5_reg: BUCK5 {
> +                               regulator-name = "vdd_buck5";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck6_reg: BUCK6 {
> +                               regulator-name = "vdd_g3d";
> +                               regulator-min-microvolt = <500000>;
> +                               regulator-max-microvolt = <1400000>;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <12500>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck7_reg: BUCK7 {
> +                               regulator-name = "vdd_buck7";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1500000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck8_reg: BUCK8 {
> +                               regulator-name = "vdd_buck8";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1500000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck9_reg: BUCK9 {
> +                               regulator-name = "vdd_buck9";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <2100000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };
> +
> +                       buck10_reg: BUCK10 {
> +                               regulator-name = "vdd_buck10";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <3000000>;
> +                               regulator-boot-on;
> +                               regulator-always-on;
> +                               regulator-ramp-delay = <25000>;
> +                               regulator-enable-ramp-delay = <250>;
> +                       };

All of these ldo3 and bucks in vendor tree for Espresso board have
ramp delay of 12000. Also they don't have enable-ramp-delay set and
voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
know what is the true value... I'll leave it up to you but it looks
suspicious.

> +               };
> +       };
> +};

What will be the benefit of defining all of these regulators if they
are always on and without consumers? No one will disable them, no one
will change the voltage. Please provide some consumers.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
  2015-11-02 12:08       ` Alim Akhtar
@ 2015-11-02 12:27         ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 12:27 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-samsung-soc, devicetree, linux-kernel,
	linux-arm-kernel, kgene

2015-11-02 21:08 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> Hi Krzysztof,
>
>
> On 11/02/2015 05:28 PM, Krzysztof Kozlowski wrote:
>>
>> Subject - name of the board at the end (not the name of the SoC):
>> arm64: dts: Enable HS200 mode operation on exynos7-espresso
>>
>> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>
>>> This patch enables HS200 mode operation on exynos7 based
>>> espresso board.
>>> This also remove _broken-cd_ property as per mmc binding documentation
>>
>>
>> s/remove/removes/
>>
>>> which say one of the properties between broken-cd and non-removable
>>
>>
>> s/say/says/
>>
>>> should be used. And we already use _non-removable_ as emmc mounted
>>> on board which is a non-removable device.
>>>
>>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> ---
>>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> index 8ce04a0ec928..7f19b99fc85b 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> @@ -405,8 +405,8 @@
>>>   &mmc_0 {
>>>          status = "okay";
>>>          num-slots = <1>;
>>> -       broken-cd;
>>>          cap-mmc-highspeed;
>>> +       mmc-hs200-1_8v;
>>
>>
>> The "mmc-ddr-1_8v" should also be supported but I guess you did not
>> add it on purpose?
>>
> Well, I have not tested it, that why didn't added. Will check if this works
> then will add.
> Thanks for your comments.

Right, it would require testing... Just came to my mind. Anyway this
is up to you.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation
@ 2015-11-02 12:27         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 12:27 UTC (permalink / raw)
  To: linux-arm-kernel

2015-11-02 21:08 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
> Hi Krzysztof,
>
>
> On 11/02/2015 05:28 PM, Krzysztof Kozlowski wrote:
>>
>> Subject - name of the board at the end (not the name of the SoC):
>> arm64: dts: Enable HS200 mode operation on exynos7-espresso
>>
>> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>
>>> This patch enables HS200 mode operation on exynos7 based
>>> espresso board.
>>> This also remove _broken-cd_ property as per mmc binding documentation
>>
>>
>> s/remove/removes/
>>
>>> which say one of the properties between broken-cd and non-removable
>>
>>
>> s/say/says/
>>
>>> should be used. And we already use _non-removable_ as emmc mounted
>>> on board which is a non-removable device.
>>>
>>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> ---
>>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> index 8ce04a0ec928..7f19b99fc85b 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> @@ -405,8 +405,8 @@
>>>   &mmc_0 {
>>>          status = "okay";
>>>          num-slots = <1>;
>>> -       broken-cd;
>>>          cap-mmc-highspeed;
>>> +       mmc-hs200-1_8v;
>>
>>
>> The "mmc-ddr-1_8v" should also be supported but I guess you did not
>> add it on purpose?
>>
> Well, I have not tested it, that why didn't added. Will check if this works
> then will add.
> Thanks for your comments.

Right, it would require testing... Just came to my mind. Anyway this
is up to you.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 13:01     ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 13:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, devicetree, linux-kernel, linux-arm-kernel, kgene



On 11/02/2015 05:55 PM, Krzysztof Kozlowski wrote:
> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>> This patch add pmic (s2mps15) node of espresso board,
>
> This patch adds PMIC (S2MPS15)...
>
>> which includes addition of regulators and pmic-clk sub-nodes.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> This patch should go in after driver side changes [1] lands.
>> [1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html
>
> Eh? Why? Usually there is not such requirement.
>
Got it, I just thought without driver changes these dts modification 
does not make sense. Thanks for clarification.
>>
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>>   1 file changed, 349 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 838a3626dac1..8ce04a0ec928 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -53,6 +53,355 @@
>>          status = "okay";
>>   };
>>
>> +&hsi2c_4 {
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <200000>;
>> +       status = "okay";
>> +
>> +       s2mps15_pmic@66 {
>> +               compatible = "samsung,s2mps15-pmic";
>> +               reg = <0x66>;
>> +               interrupts = <2 0>;
>> +               interrupt-parent = <&gpa0>;
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&pmic_irq>;
>> +               wakeup-source;
>> +
>> +               s2mps15_osc: clocks {
>> +                       compatible = "samsung,s2mps13-clk";
>> +                       #clock-cells = <1>;
>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>> +                               "s2mps13_bt";
>> +               };
>
> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>
yes, you are right, rtc on this board is currently broken, mainly 
because of the introduction of rtc_src clock in the s3c-rtc driver.
That is on my do list next. will take a look.

Are you suggesting to remove this -clk node now and add along with rtc 
changes? I feel this should go in along with this patch.

>> +
>> +               regulators {
>> +                       ldo1_reg: LDO1 {
>> +                               regulator-name = "vdd_ldo1";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <900000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo2_reg: LDO2 {
>> +                               regulator-name = "vdd_ldo2";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                               regulator-always-on;
>> +                       };
>> +
>> +                       ldo3_reg: LDO3 {
>> +                               regulator-name = "vdd_ldo3";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo4_reg: LDO4 {
>> +                               regulator-name = "vdd_ldo4";
>> +                               regulator-min-microvolt = <800000>;
>> +                               regulator-max-microvolt = <1110000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo5_reg: LDO5 {
>> +                               regulator-name = "vdd_ldo5";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo6_reg: LDO6 {
>> +                               regulator-name = "vdd_ldo6";
>> +                               regulator-min-microvolt = <2250000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo7_reg: LDO7 {
>> +                               regulator-name = "vdd_ldo7";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1150000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo8_reg: LDO8 {
>> +                               regulator-name = "vdd_ldo8";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo9_reg: LDO9 {
>> +                               regulator-name = "vdd_ldo9";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo10_reg: LDO10 {
>> +                               regulator-name = "vdd_ldo10";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo11_reg: LDO11 {
>> +                               regulator-name = "vdd_ldo11";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo12_reg: LDO12 {
>> +                               regulator-name = "vdd_ldo12";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo13_reg: LDO13 {
>> +                               regulator-name = "vdd_ldo13";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo14_reg: LDO14 {
>> +                               regulator-name = "vdd_ldo14";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo15_reg: LDO15 {
>> +                               regulator-name = "vdd_ldo15";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo16_reg: LDO16 {
>> +                               regulator-name = "vdd_ldo16";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo17_reg: LDO17 {
>> +                               regulator-name = "vdd_ldo17";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo18_reg: LDO18 {
>> +                               regulator-name = "vdd_ldo18";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo19_reg: LDO19 {
>> +                               regulator-name = "vdd_ldo19";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo20_reg: LDO20 {
>> +                               regulator-name = "vdd_ldo20";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo21_reg: LDO21 {
>> +                               regulator-name = "vdd_ldo21";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo22_reg: LDO22 {
>> +                               regulator-name = "vdd_ldo22";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo23_reg: LDO23 {
>> +                               regulator-name = "vdd_ldo23";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo24_reg: LDO24 {
>> +                               regulator-name = "vdd_ldo24";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo25_reg: LDO25 {
>> +                               regulator-name = "vdd_ldo25";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo26_reg: LDO26 {
>> +                               regulator-name = "vdd_ldo26";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1470000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo27_reg: LDO27 {
>> +                               regulator-name = "vdd_ldo27";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       buck1_reg: BUCK1 {
>> +                               regulator-name = "vdd_mif";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck2_reg: BUCK2 {
>> +                               regulator-name = "vdd_atlas";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck3_reg: BUCK3 {
>> +                               regulator-name = "vdd_apollo";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck4_reg: BUCK4 {
>> +                               regulator-name = "vdd_int";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck5_reg: BUCK5 {
>> +                               regulator-name = "vdd_buck5";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck6_reg: BUCK6 {
>> +                               regulator-name = "vdd_g3d";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1400000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck7_reg: BUCK7 {
>> +                               regulator-name = "vdd_buck7";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck8_reg: BUCK8 {
>> +                               regulator-name = "vdd_buck8";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck9_reg: BUCK9 {
>> +                               regulator-name = "vdd_buck9";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <2100000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck10_reg: BUCK10 {
>> +                               regulator-name = "vdd_buck10";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <3000000>;
>> +                               regulator-boot-on;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>
> All of these ldo3 and bucks in vendor tree for Espresso board have
> ramp delay of 12000. Also they don't have enable-ramp-delay set and
> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
> know what is the true value... I'll leave it up to you but it looks
> suspicious.
>
These values generally comes from our board design team, so I cann't 
really comment on that, it may vary from board revision etc.
I will check if we have any updated version of recommended value and 
update accordingly.

>> +               };
>> +       };
>> +};
>
> What will be the benefit of defining all of these regulators if they
> are always on and without consumers? No one will disable them, no one
> will change the voltage. Please provide some consumers.
>
As many drivers are not yet enabled in arm64 defconfig, that is one of 
the reason why we are not seeing many consumer for these nodes.
This is the ground work being done for enabling those. If you insist 
will try to reduce what is being used now. Moreover this was used to 
verify functionality of pmic driver as well.


> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 13:01     ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 13:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kgene-DgEjT+Ai2ygdnm+yROfE0A



On 11/02/2015 05:55 PM, Krzysztof Kozlowski wrote:
> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>:
>> This patch add pmic (s2mps15) node of espresso board,
>
> This patch adds PMIC (S2MPS15)...
>
>> which includes addition of regulators and pmic-clk sub-nodes.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Alim Akhtar <alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>> This patch should go in after driver side changes [1] lands.
>> [1]-> https://www.mail-archive.com/linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg47736.html
>
> Eh? Why? Usually there is not such requirement.
>
Got it, I just thought without driver changes these dts modification 
does not make sense. Thanks for clarification.
>>
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>>   1 file changed, 349 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 838a3626dac1..8ce04a0ec928 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -53,6 +53,355 @@
>>          status = "okay";
>>   };
>>
>> +&hsi2c_4 {
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <200000>;
>> +       status = "okay";
>> +
>> +       s2mps15_pmic@66 {
>> +               compatible = "samsung,s2mps15-pmic";
>> +               reg = <0x66>;
>> +               interrupts = <2 0>;
>> +               interrupt-parent = <&gpa0>;
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&pmic_irq>;
>> +               wakeup-source;
>> +
>> +               s2mps15_osc: clocks {
>> +                       compatible = "samsung,s2mps13-clk";
>> +                       #clock-cells = <1>;
>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>> +                               "s2mps13_bt";
>> +               };
>
> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>
yes, you are right, rtc on this board is currently broken, mainly 
because of the introduction of rtc_src clock in the s3c-rtc driver.
That is on my do list next. will take a look.

Are you suggesting to remove this -clk node now and add along with rtc 
changes? I feel this should go in along with this patch.

>> +
>> +               regulators {
>> +                       ldo1_reg: LDO1 {
>> +                               regulator-name = "vdd_ldo1";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <900000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo2_reg: LDO2 {
>> +                               regulator-name = "vdd_ldo2";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                               regulator-always-on;
>> +                       };
>> +
>> +                       ldo3_reg: LDO3 {
>> +                               regulator-name = "vdd_ldo3";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo4_reg: LDO4 {
>> +                               regulator-name = "vdd_ldo4";
>> +                               regulator-min-microvolt = <800000>;
>> +                               regulator-max-microvolt = <1110000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo5_reg: LDO5 {
>> +                               regulator-name = "vdd_ldo5";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo6_reg: LDO6 {
>> +                               regulator-name = "vdd_ldo6";
>> +                               regulator-min-microvolt = <2250000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo7_reg: LDO7 {
>> +                               regulator-name = "vdd_ldo7";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1150000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo8_reg: LDO8 {
>> +                               regulator-name = "vdd_ldo8";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo9_reg: LDO9 {
>> +                               regulator-name = "vdd_ldo9";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo10_reg: LDO10 {
>> +                               regulator-name = "vdd_ldo10";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo11_reg: LDO11 {
>> +                               regulator-name = "vdd_ldo11";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo12_reg: LDO12 {
>> +                               regulator-name = "vdd_ldo12";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo13_reg: LDO13 {
>> +                               regulator-name = "vdd_ldo13";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo14_reg: LDO14 {
>> +                               regulator-name = "vdd_ldo14";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo15_reg: LDO15 {
>> +                               regulator-name = "vdd_ldo15";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo16_reg: LDO16 {
>> +                               regulator-name = "vdd_ldo16";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo17_reg: LDO17 {
>> +                               regulator-name = "vdd_ldo17";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo18_reg: LDO18 {
>> +                               regulator-name = "vdd_ldo18";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo19_reg: LDO19 {
>> +                               regulator-name = "vdd_ldo19";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo20_reg: LDO20 {
>> +                               regulator-name = "vdd_ldo20";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo21_reg: LDO21 {
>> +                               regulator-name = "vdd_ldo21";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo22_reg: LDO22 {
>> +                               regulator-name = "vdd_ldo22";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo23_reg: LDO23 {
>> +                               regulator-name = "vdd_ldo23";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo24_reg: LDO24 {
>> +                               regulator-name = "vdd_ldo24";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo25_reg: LDO25 {
>> +                               regulator-name = "vdd_ldo25";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo26_reg: LDO26 {
>> +                               regulator-name = "vdd_ldo26";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1470000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo27_reg: LDO27 {
>> +                               regulator-name = "vdd_ldo27";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       buck1_reg: BUCK1 {
>> +                               regulator-name = "vdd_mif";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck2_reg: BUCK2 {
>> +                               regulator-name = "vdd_atlas";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck3_reg: BUCK3 {
>> +                               regulator-name = "vdd_apollo";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck4_reg: BUCK4 {
>> +                               regulator-name = "vdd_int";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck5_reg: BUCK5 {
>> +                               regulator-name = "vdd_buck5";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck6_reg: BUCK6 {
>> +                               regulator-name = "vdd_g3d";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1400000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck7_reg: BUCK7 {
>> +                               regulator-name = "vdd_buck7";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck8_reg: BUCK8 {
>> +                               regulator-name = "vdd_buck8";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck9_reg: BUCK9 {
>> +                               regulator-name = "vdd_buck9";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <2100000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck10_reg: BUCK10 {
>> +                               regulator-name = "vdd_buck10";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <3000000>;
>> +                               regulator-boot-on;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>
> All of these ldo3 and bucks in vendor tree for Espresso board have
> ramp delay of 12000. Also they don't have enable-ramp-delay set and
> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
> know what is the true value... I'll leave it up to you but it looks
> suspicious.
>
These values generally comes from our board design team, so I cann't 
really comment on that, it may vary from board revision etc.
I will check if we have any updated version of recommended value and 
update accordingly.

>> +               };
>> +       };
>> +};
>
> What will be the benefit of defining all of these regulators if they
> are always on and without consumers? No one will disable them, no one
> will change the voltage. Please provide some consumers.
>
As many drivers are not yet enabled in arm64 defconfig, that is one of 
the reason why we are not seeing many consumer for these nodes.
This is the ground work being done for enabling those. If you insist 
will try to reduce what is being used now. Moreover this was used to 
verify functionality of pmic driver as well.


> Best regards,
> Krzysztof
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 13:01     ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-02 13:01 UTC (permalink / raw)
  To: linux-arm-kernel



On 11/02/2015 05:55 PM, Krzysztof Kozlowski wrote:
> 2015-11-02 19:04 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>> This patch add pmic (s2mps15) node of espresso board,
>
> This patch adds PMIC (S2MPS15)...
>
>> which includes addition of regulators and pmic-clk sub-nodes.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>> This patch should go in after driver side changes [1] lands.
>> [1]-> https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg47736.html
>
> Eh? Why? Usually there is not such requirement.
>
Got it, I just thought without driver changes these dts modification 
does not make sense. Thanks for clarification.
>>
>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349 +++++++++++++++++++++++
>>   1 file changed, 349 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> index 838a3626dac1..8ce04a0ec928 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>> @@ -53,6 +53,355 @@
>>          status = "okay";
>>   };
>>
>> +&hsi2c_4 {
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <200000>;
>> +       status = "okay";
>> +
>> +       s2mps15_pmic at 66 {
>> +               compatible = "samsung,s2mps15-pmic";
>> +               reg = <0x66>;
>> +               interrupts = <2 0>;
>> +               interrupt-parent = <&gpa0>;
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&pmic_irq>;
>> +               wakeup-source;
>> +
>> +               s2mps15_osc: clocks {
>> +                       compatible = "samsung,s2mps13-clk";
>> +                       #clock-cells = <1>;
>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>> +                               "s2mps13_bt";
>> +               };
>
> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>
yes, you are right, rtc on this board is currently broken, mainly 
because of the introduction of rtc_src clock in the s3c-rtc driver.
That is on my do list next. will take a look.

Are you suggesting to remove this -clk node now and add along with rtc 
changes? I feel this should go in along with this patch.

>> +
>> +               regulators {
>> +                       ldo1_reg: LDO1 {
>> +                               regulator-name = "vdd_ldo1";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <900000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo2_reg: LDO2 {
>> +                               regulator-name = "vdd_ldo2";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                               regulator-always-on;
>> +                       };
>> +
>> +                       ldo3_reg: LDO3 {
>> +                               regulator-name = "vdd_ldo3";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo4_reg: LDO4 {
>> +                               regulator-name = "vdd_ldo4";
>> +                               regulator-min-microvolt = <800000>;
>> +                               regulator-max-microvolt = <1110000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo5_reg: LDO5 {
>> +                               regulator-name = "vdd_ldo5";
>> +                               regulator-min-microvolt = <1620000>;
>> +                               regulator-max-microvolt = <1980000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo6_reg: LDO6 {
>> +                               regulator-name = "vdd_ldo6";
>> +                               regulator-min-microvolt = <2250000>;
>> +                               regulator-max-microvolt = <3300000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo7_reg: LDO7 {
>> +                               regulator-name = "vdd_ldo7";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1150000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo8_reg: LDO8 {
>> +                               regulator-name = "vdd_ldo8";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo9_reg: LDO9 {
>> +                               regulator-name = "vdd_ldo9";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo10_reg: LDO10 {
>> +                               regulator-name = "vdd_ldo10";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo11_reg: LDO11 {
>> +                               regulator-name = "vdd_ldo11";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo12_reg: LDO12 {
>> +                               regulator-name = "vdd_ldo12";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo13_reg: LDO13 {
>> +                               regulator-name = "vdd_ldo13";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo14_reg: LDO14 {
>> +                               regulator-name = "vdd_ldo14";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo15_reg: LDO15 {
>> +                               regulator-name = "vdd_ldo15";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo16_reg: LDO16 {
>> +                               regulator-name = "vdd_ldo16";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo17_reg: LDO17 {
>> +                               regulator-name = "vdd_ldo17";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo18_reg: LDO18 {
>> +                               regulator-name = "vdd_ldo18";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo19_reg: LDO19 {
>> +                               regulator-name = "vdd_ldo19";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo20_reg: LDO20 {
>> +                               regulator-name = "vdd_ldo20";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo21_reg: LDO21 {
>> +                               regulator-name = "vdd_ldo21";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo22_reg: LDO22 {
>> +                               regulator-name = "vdd_ldo22";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo23_reg: LDO23 {
>> +                               regulator-name = "vdd_ldo23";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo24_reg: LDO24 {
>> +                               regulator-name = "vdd_ldo24";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo25_reg: LDO25 {
>> +                               regulator-name = "vdd_ldo25";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <3375000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo26_reg: LDO26 {
>> +                               regulator-name = "vdd_ldo26";
>> +                               regulator-min-microvolt = <700000>;
>> +                               regulator-max-microvolt = <1470000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       ldo27_reg: LDO27 {
>> +                               regulator-name = "vdd_ldo27";
>> +                               regulator-min-microvolt = <1500000>;
>> +                               regulator-max-microvolt = <2275000>;
>> +                               regulator-always-on;
>> +                               regulator-enable-ramp-delay = <125>;
>> +                       };
>> +
>> +                       buck1_reg: BUCK1 {
>> +                               regulator-name = "vdd_mif";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck2_reg: BUCK2 {
>> +                               regulator-name = "vdd_atlas";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck3_reg: BUCK3 {
>> +                               regulator-name = "vdd_apollo";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck4_reg: BUCK4 {
>> +                               regulator-name = "vdd_int";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck5_reg: BUCK5 {
>> +                               regulator-name = "vdd_buck5";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck6_reg: BUCK6 {
>> +                               regulator-name = "vdd_g3d";
>> +                               regulator-min-microvolt = <500000>;
>> +                               regulator-max-microvolt = <1400000>;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <12500>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck7_reg: BUCK7 {
>> +                               regulator-name = "vdd_buck7";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck8_reg: BUCK8 {
>> +                               regulator-name = "vdd_buck8";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1500000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck9_reg: BUCK9 {
>> +                               regulator-name = "vdd_buck9";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <2100000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>> +
>> +                       buck10_reg: BUCK10 {
>> +                               regulator-name = "vdd_buck10";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <3000000>;
>> +                               regulator-boot-on;
>> +                               regulator-always-on;
>> +                               regulator-ramp-delay = <25000>;
>> +                               regulator-enable-ramp-delay = <250>;
>> +                       };
>
> All of these ldo3 and bucks in vendor tree for Espresso board have
> ramp delay of 12000. Also they don't have enable-ramp-delay set and
> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
> know what is the true value... I'll leave it up to you but it looks
> suspicious.
>
These values generally comes from our board design team, so I cann't 
really comment on that, it may vary from board revision etc.
I will check if we have any updated version of recommended value and 
update accordingly.

>> +               };
>> +       };
>> +};
>
> What will be the benefit of defining all of these regulators if they
> are always on and without consumers? No one will disable them, no one
> will change the voltage. Please provide some consumers.
>
As many drivers are not yet enabled in arm64 defconfig, that is one of 
the reason why we are not seeing many consumer for these nodes.
This is the ground work being done for enabling those. If you insist 
will try to reduce what is being used now. Moreover this was used to 
verify functionality of pmic driver as well.


> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
  2015-11-02 13:01     ` Alim Akhtar
@ 2015-11-02 13:52       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 13:52 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-samsung-soc, devicetree, linux-kernel,
	linux-arm-kernel, kgene

2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>
>>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>> +++++++++++++++++++++++
>>>   1 file changed, 349 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> index 838a3626dac1..8ce04a0ec928 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> @@ -53,6 +53,355 @@
>>>          status = "okay";
>>>   };
>>>
>>> +&hsi2c_4 {
>>> +       samsung,i2c-sda-delay = <100>;
>>> +       samsung,i2c-max-bus-freq = <200000>;
>>> +       status = "okay";
>>> +
>>> +       s2mps15_pmic@66 {
>>> +               compatible = "samsung,s2mps15-pmic";
>>> +               reg = <0x66>;
>>> +               interrupts = <2 0>;
>>> +               interrupt-parent = <&gpa0>;
>>> +               pinctrl-names = "default";
>>> +               pinctrl-0 = <&pmic_irq>;
>>> +               wakeup-source;
>>> +
>>> +               s2mps15_osc: clocks {
>>> +                       compatible = "samsung,s2mps13-clk";
>>> +                       #clock-cells = <1>;
>>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>>> +                               "s2mps13_bt";
>>> +               };
>>
>>
>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>
> yes, you are right, rtc on this board is currently broken, mainly because of
> the introduction of rtc_src clock in the s3c-rtc driver.
> That is on my do list next. will take a look.
>
> Are you suggesting to remove this -clk node now and add along with rtc
> changes? I feel this should go in along with this patch.

Just add it in consecutive patch in this series. You added here some
providers (clock and regulators) without consumers. This of course
looks good as a way of providing full description of the board but:
1. For regulators always on: may be meaningless for kernel. Kernel
does not use it. Existence of regulator subnode will fulfill driver's
needs for probe.
2. For clocks: actually will disable these clocks because of lack of
consumers... which is fine but probably not what you wanted.

The standard approach is to add such providers when they are needed -
there are some consumers using them.

>>> +
>>> +               regulators {
>>> +                       ldo1_reg: LDO1 {
>>> +                               regulator-name = "vdd_ldo1";
>>> +                               regulator-min-microvolt = <500000>;
>>> +                               regulator-max-microvolt = <900000>;
>>> +                               regulator-always-on;
>>> +                               regulator-enable-ramp-delay = <125>;
>>> +                       };

(...)

>>> +
>>> +                       buck10_reg: BUCK10 {
>>> +                               regulator-name = "vdd_buck10";
>>> +                               regulator-min-microvolt = <1000000>;
>>> +                               regulator-max-microvolt = <3000000>;
>>> +                               regulator-boot-on;
>>> +                               regulator-always-on;
>>> +                               regulator-ramp-delay = <25000>;
>>> +                               regulator-enable-ramp-delay = <250>;
>>> +                       };
>>
>>
>> All of these ldo3 and bucks in vendor tree for Espresso board have
>> ramp delay of 12000. Also they don't have enable-ramp-delay set and
>> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
>> know what is the true value... I'll leave it up to you but it looks
>> suspicious.
>>
> These values generally comes from our board design team, so I cann't really
> comment on that, it may vary from board revision etc.
> I will check if we have any updated version of recommended value and update
> accordingly.

Okay, just pointing the difference. I cannot verify them.

>
>>> +               };
>>> +       };
>>> +};
>>
>>
>> What will be the benefit of defining all of these regulators if they
>> are always on and without consumers? No one will disable them, no one
>> will change the voltage. Please provide some consumers.
>>
> As many drivers are not yet enabled in arm64 defconfig, that is one of the
> reason why we are not seeing many consumer for these nodes.

That is not a problem. Please send a patch changing the defconfig.
Usually defconfig (for armv7 this would be exynos and multi_v7) should
provide bootable and working environment for all of our supported
boards.

> This is the ground work being done for enabling those. If you insist will
> try to reduce what is being used now. Moreover this was used to verify
> functionality of pmic driver as well.

Actually as a verification I think even adding simple node
"regulators" is sufficient - driver will add all regulators anyway.
However seeing all regulators described for particular board is
good... but lack of consumers is disturbing because this may mean that
it was not really fully modeled.

>From my point of view - all of regulators in DT are welcomed but at
least some of them should have a consumer. This means that someone
took care and looked at the relationships between them.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-02 13:52       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-02 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>
>>>   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>> +++++++++++++++++++++++
>>>   1 file changed, 349 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> index 838a3626dac1..8ce04a0ec928 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>> @@ -53,6 +53,355 @@
>>>          status = "okay";
>>>   };
>>>
>>> +&hsi2c_4 {
>>> +       samsung,i2c-sda-delay = <100>;
>>> +       samsung,i2c-max-bus-freq = <200000>;
>>> +       status = "okay";
>>> +
>>> +       s2mps15_pmic at 66 {
>>> +               compatible = "samsung,s2mps15-pmic";
>>> +               reg = <0x66>;
>>> +               interrupts = <2 0>;
>>> +               interrupt-parent = <&gpa0>;
>>> +               pinctrl-names = "default";
>>> +               pinctrl-0 = <&pmic_irq>;
>>> +               wakeup-source;
>>> +
>>> +               s2mps15_osc: clocks {
>>> +                       compatible = "samsung,s2mps13-clk";
>>> +                       #clock-cells = <1>;
>>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>>> +                               "s2mps13_bt";
>>> +               };
>>
>>
>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>
> yes, you are right, rtc on this board is currently broken, mainly because of
> the introduction of rtc_src clock in the s3c-rtc driver.
> That is on my do list next. will take a look.
>
> Are you suggesting to remove this -clk node now and add along with rtc
> changes? I feel this should go in along with this patch.

Just add it in consecutive patch in this series. You added here some
providers (clock and regulators) without consumers. This of course
looks good as a way of providing full description of the board but:
1. For regulators always on: may be meaningless for kernel. Kernel
does not use it. Existence of regulator subnode will fulfill driver's
needs for probe.
2. For clocks: actually will disable these clocks because of lack of
consumers... which is fine but probably not what you wanted.

The standard approach is to add such providers when they are needed -
there are some consumers using them.

>>> +
>>> +               regulators {
>>> +                       ldo1_reg: LDO1 {
>>> +                               regulator-name = "vdd_ldo1";
>>> +                               regulator-min-microvolt = <500000>;
>>> +                               regulator-max-microvolt = <900000>;
>>> +                               regulator-always-on;
>>> +                               regulator-enable-ramp-delay = <125>;
>>> +                       };

(...)

>>> +
>>> +                       buck10_reg: BUCK10 {
>>> +                               regulator-name = "vdd_buck10";
>>> +                               regulator-min-microvolt = <1000000>;
>>> +                               regulator-max-microvolt = <3000000>;
>>> +                               regulator-boot-on;
>>> +                               regulator-always-on;
>>> +                               regulator-ramp-delay = <25000>;
>>> +                               regulator-enable-ramp-delay = <250>;
>>> +                       };
>>
>>
>> All of these ldo3 and bucks in vendor tree for Espresso board have
>> ramp delay of 12000. Also they don't have enable-ramp-delay set and
>> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
>> know what is the true value... I'll leave it up to you but it looks
>> suspicious.
>>
> These values generally comes from our board design team, so I cann't really
> comment on that, it may vary from board revision etc.
> I will check if we have any updated version of recommended value and update
> accordingly.

Okay, just pointing the difference. I cannot verify them.

>
>>> +               };
>>> +       };
>>> +};
>>
>>
>> What will be the benefit of defining all of these regulators if they
>> are always on and without consumers? No one will disable them, no one
>> will change the voltage. Please provide some consumers.
>>
> As many drivers are not yet enabled in arm64 defconfig, that is one of the
> reason why we are not seeing many consumer for these nodes.

That is not a problem. Please send a patch changing the defconfig.
Usually defconfig (for armv7 this would be exynos and multi_v7) should
provide bootable and working environment for all of our supported
boards.

> This is the ground work being done for enabling those. If you insist will
> try to reduce what is being used now. Moreover this was used to verify
> functionality of pmic driver as well.

Actually as a verification I think even adding simple node
"regulators" is sufficient - driver will add all regulators anyway.
However seeing all regulators described for particular board is
good... but lack of consumers is disturbing because this may mean that
it was not really fully modeled.

>From my point of view - all of regulators in DT are welcomed but at
least some of them should have a consumer. This means that someone
took care and looked at the relationships between them.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
  2015-11-02 13:52       ` Krzysztof Kozlowski
@ 2015-11-05  5:37         ` Alim Akhtar
  -1 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-05  5:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, devicetree, linux-kernel, linux-arm-kernel, kgene

Hi Krzysztof

On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:
> 2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>>
>>>>    arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>>> +++++++++++++++++++++++
>>>>    1 file changed, 349 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> index 838a3626dac1..8ce04a0ec928 100644
>>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> @@ -53,6 +53,355 @@
>>>>           status = "okay";
>>>>    };
>>>>
>>>> +&hsi2c_4 {
>>>> +       samsung,i2c-sda-delay = <100>;
>>>> +       samsung,i2c-max-bus-freq = <200000>;
>>>> +       status = "okay";
>>>> +
>>>> +       s2mps15_pmic@66 {
>>>> +               compatible = "samsung,s2mps15-pmic";
>>>> +               reg = <0x66>;
>>>> +               interrupts = <2 0>;
>>>> +               interrupt-parent = <&gpa0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&pmic_irq>;
>>>> +               wakeup-source;
>>>> +
>>>> +               s2mps15_osc: clocks {
>>>> +                       compatible = "samsung,s2mps13-clk";
>>>> +                       #clock-cells = <1>;
>>>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>>>> +                               "s2mps13_bt";
>>>> +               };
>>>
>>>
>>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>>
>> yes, you are right, rtc on this board is currently broken, mainly because of
>> the introduction of rtc_src clock in the s3c-rtc driver.
>> That is on my do list next. will take a look.
>>
>> Are you suggesting to remove this -clk node now and add along with rtc
>> changes? I feel this should go in along with this patch.
>
> Just add it in consecutive patch in this series. You added here some
> providers (clock and regulators) without consumers. This of course
> looks good as a way of providing full description of the board but:
> 1. For regulators always on: may be meaningless for kernel. Kernel
> does not use it. Existence of regulator subnode will fulfill driver's
> needs for probe.
> 2. For clocks: actually will disable these clocks because of lack of
> consumers... which is fine but probably not what you wanted.
>
> The standard approach is to add such providers when they are needed -
> there are some consumers using them.
>
OK. for now will keep the pmic clock added as clock will be in disabled 
state, so it wont harm.
- will keep system related regulator like supply to arm,mif,int etc ..
will remove supplies to other peripherals IPs. Hope thats fine.

>>>> +
>>>> +               regulators {
>>>> +                       ldo1_reg: LDO1 {
>>>> +                               regulator-name = "vdd_ldo1";
>>>> +                               regulator-min-microvolt = <500000>;
>>>> +                               regulator-max-microvolt = <900000>;
>>>> +                               regulator-always-on;
>>>> +                               regulator-enable-ramp-delay = <125>;
>>>> +                       };
>
> (...)
>
>>>> +
>>>> +                       buck10_reg: BUCK10 {
>>>> +                               regulator-name = "vdd_buck10";
>>>> +                               regulator-min-microvolt = <1000000>;
>>>> +                               regulator-max-microvolt = <3000000>;
>>>> +                               regulator-boot-on;
>>>> +                               regulator-always-on;
>>>> +                               regulator-ramp-delay = <25000>;
>>>> +                               regulator-enable-ramp-delay = <250>;
>>>> +                       };
>>>
>>>
>>> All of these ldo3 and bucks in vendor tree for Espresso board have
>>> ramp delay of 12000. Also they don't have enable-ramp-delay set and
>>> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
>>> know what is the true value... I'll leave it up to you but it looks
>>> suspicious.
>>>
>> These values generally comes from our board design team, so I cann't really
>> comment on that, it may vary from board revision etc.
>> I will check if we have any updated version of recommended value and update
>> accordingly.
>
> Okay, just pointing the difference. I cannot verify them.
>
>>
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>> What will be the benefit of defining all of these regulators if they
>>> are always on and without consumers? No one will disable them, no one
>>> will change the voltage. Please provide some consumers.
>>>
>> As many drivers are not yet enabled in arm64 defconfig, that is one of the
>> reason why we are not seeing many consumer for these nodes.
>
> That is not a problem. Please send a patch changing the defconfig.
> Usually defconfig (for armv7 this would be exynos and multi_v7) should
> provide bootable and working environment for all of our supported
> boards.
>
>> This is the ground work being done for enabling those. If you insist will
>> try to reduce what is being used now. Moreover this was used to verify
>> functionality of pmic driver as well.
>
> Actually as a verification I think even adding simple node
> "regulators" is sufficient - driver will add all regulators anyway.
> However seeing all regulators described for particular board is
> good... but lack of consumers is disturbing because this may mean that
> it was not really fully modeled.
>
>>From my point of view - all of regulators in DT are welcomed but at
> least some of them should have a consumer. This means that someone
> took care and looked at the relationships between them.
>
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-05  5:37         ` Alim Akhtar
  0 siblings, 0 replies; 28+ messages in thread
From: Alim Akhtar @ 2015-11-05  5:37 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Krzysztof

On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:
> 2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>>
>>>>    arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>>> +++++++++++++++++++++++
>>>>    1 file changed, 349 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> index 838a3626dac1..8ce04a0ec928 100644
>>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>> @@ -53,6 +53,355 @@
>>>>           status = "okay";
>>>>    };
>>>>
>>>> +&hsi2c_4 {
>>>> +       samsung,i2c-sda-delay = <100>;
>>>> +       samsung,i2c-max-bus-freq = <200000>;
>>>> +       status = "okay";
>>>> +
>>>> +       s2mps15_pmic at 66 {
>>>> +               compatible = "samsung,s2mps15-pmic";
>>>> +               reg = <0x66>;
>>>> +               interrupts = <2 0>;
>>>> +               interrupt-parent = <&gpa0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&pmic_irq>;
>>>> +               wakeup-source;
>>>> +
>>>> +               s2mps15_osc: clocks {
>>>> +                       compatible = "samsung,s2mps13-clk";
>>>> +                       #clock-cells = <1>;
>>>> +                       clock-output-names = "s2mps13_ap", "s2mps13_cp",
>>>> +                               "s2mps13_bt";
>>>> +               };
>>>
>>>
>>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>>
>> yes, you are right, rtc on this board is currently broken, mainly because of
>> the introduction of rtc_src clock in the s3c-rtc driver.
>> That is on my do list next. will take a look.
>>
>> Are you suggesting to remove this -clk node now and add along with rtc
>> changes? I feel this should go in along with this patch.
>
> Just add it in consecutive patch in this series. You added here some
> providers (clock and regulators) without consumers. This of course
> looks good as a way of providing full description of the board but:
> 1. For regulators always on: may be meaningless for kernel. Kernel
> does not use it. Existence of regulator subnode will fulfill driver's
> needs for probe.
> 2. For clocks: actually will disable these clocks because of lack of
> consumers... which is fine but probably not what you wanted.
>
> The standard approach is to add such providers when they are needed -
> there are some consumers using them.
>
OK. for now will keep the pmic clock added as clock will be in disabled 
state, so it wont harm.
- will keep system related regulator like supply to arm,mif,int etc ..
will remove supplies to other peripherals IPs. Hope thats fine.

>>>> +
>>>> +               regulators {
>>>> +                       ldo1_reg: LDO1 {
>>>> +                               regulator-name = "vdd_ldo1";
>>>> +                               regulator-min-microvolt = <500000>;
>>>> +                               regulator-max-microvolt = <900000>;
>>>> +                               regulator-always-on;
>>>> +                               regulator-enable-ramp-delay = <125>;
>>>> +                       };
>
> (...)
>
>>>> +
>>>> +                       buck10_reg: BUCK10 {
>>>> +                               regulator-name = "vdd_buck10";
>>>> +                               regulator-min-microvolt = <1000000>;
>>>> +                               regulator-max-microvolt = <3000000>;
>>>> +                               regulator-boot-on;
>>>> +                               regulator-always-on;
>>>> +                               regulator-ramp-delay = <25000>;
>>>> +                               regulator-enable-ramp-delay = <250>;
>>>> +                       };
>>>
>>>
>>> All of these ldo3 and bucks in vendor tree for Espresso board have
>>> ramp delay of 12000. Also they don't have enable-ramp-delay set and
>>> voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
>>> know what is the true value... I'll leave it up to you but it looks
>>> suspicious.
>>>
>> These values generally comes from our board design team, so I cann't really
>> comment on that, it may vary from board revision etc.
>> I will check if we have any updated version of recommended value and update
>> accordingly.
>
> Okay, just pointing the difference. I cannot verify them.
>
>>
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>> What will be the benefit of defining all of these regulators if they
>>> are always on and without consumers? No one will disable them, no one
>>> will change the voltage. Please provide some consumers.
>>>
>> As many drivers are not yet enabled in arm64 defconfig, that is one of the
>> reason why we are not seeing many consumer for these nodes.
>
> That is not a problem. Please send a patch changing the defconfig.
> Usually defconfig (for armv7 this would be exynos and multi_v7) should
> provide bootable and working environment for all of our supported
> boards.
>
>> This is the ground work being done for enabling those. If you insist will
>> try to reduce what is being used now. Moreover this was used to verify
>> functionality of pmic driver as well.
>
> Actually as a verification I think even adding simple node
> "regulators" is sufficient - driver will add all regulators anyway.
> However seeing all regulators described for particular board is
> good... but lack of consumers is disturbing because this may mean that
> it was not really fully modeled.
>
>>From my point of view - all of regulators in DT are welcomed but at
> least some of them should have a consumer. This means that someone
> took care and looked at the relationships between them.
>
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
  2015-11-05  5:37         ` Alim Akhtar
@ 2015-11-05  5:40           ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-05  5:40 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: linux-samsung-soc, devicetree, linux-kernel, linux-arm-kernel, kgene

On 05.11.2015 14:37, Alim Akhtar wrote:
> Hi Krzysztof
> 
> On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:
>> 2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>>>
>>>>>    arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>>>> +++++++++++++++++++++++
>>>>>    1 file changed, 349 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> index 838a3626dac1..8ce04a0ec928 100644
>>>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> @@ -53,6 +53,355 @@
>>>>>           status = "okay";
>>>>>    };
>>>>>
>>>>> +&hsi2c_4 {
>>>>> +       samsung,i2c-sda-delay = <100>;
>>>>> +       samsung,i2c-max-bus-freq = <200000>;
>>>>> +       status = "okay";
>>>>> +
>>>>> +       s2mps15_pmic@66 {
>>>>> +               compatible = "samsung,s2mps15-pmic";
>>>>> +               reg = <0x66>;
>>>>> +               interrupts = <2 0>;
>>>>> +               interrupt-parent = <&gpa0>;
>>>>> +               pinctrl-names = "default";
>>>>> +               pinctrl-0 = <&pmic_irq>;
>>>>> +               wakeup-source;
>>>>> +
>>>>> +               s2mps15_osc: clocks {
>>>>> +                       compatible = "samsung,s2mps13-clk";
>>>>> +                       #clock-cells = <1>;
>>>>> +                       clock-output-names = "s2mps13_ap",
>>>>> "s2mps13_cp",
>>>>> +                               "s2mps13_bt";
>>>>> +               };
>>>>
>>>>
>>>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>>>
>>> yes, you are right, rtc on this board is currently broken, mainly
>>> because of
>>> the introduction of rtc_src clock in the s3c-rtc driver.
>>> That is on my do list next. will take a look.
>>>
>>> Are you suggesting to remove this -clk node now and add along with rtc
>>> changes? I feel this should go in along with this patch.
>>
>> Just add it in consecutive patch in this series. You added here some
>> providers (clock and regulators) without consumers. This of course
>> looks good as a way of providing full description of the board but:
>> 1. For regulators always on: may be meaningless for kernel. Kernel
>> does not use it. Existence of regulator subnode will fulfill driver's
>> needs for probe.
>> 2. For clocks: actually will disable these clocks because of lack of
>> consumers... which is fine but probably not what you wanted.
>>
>> The standard approach is to add such providers when they are needed -
>> there are some consumers using them.
>>
> OK. for now will keep the pmic clock added as clock will be in disabled
> state, so it wont harm.
> - will keep system related regulator like supply to arm,mif,int etc ..
> will remove supplies to other peripherals IPs. Hope thats fine.

You don't have to remove other regulators (these without consumers).
Describe in DT all of regulators but:
1. Add to some of them consumers;
2. Disable these which are not needed (by not marking always-enabled).

Just like for all other boards.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node
@ 2015-11-05  5:40           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-05  5:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 05.11.2015 14:37, Alim Akhtar wrote:
> Hi Krzysztof
> 
> On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:
>> 2015-11-02 22:01 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>:
>>>>>
>>>>>    arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
>>>>> +++++++++++++++++++++++
>>>>>    1 file changed, 349 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> index 838a3626dac1..8ce04a0ec928 100644
>>>>> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
>>>>> @@ -53,6 +53,355 @@
>>>>>           status = "okay";
>>>>>    };
>>>>>
>>>>> +&hsi2c_4 {
>>>>> +       samsung,i2c-sda-delay = <100>;
>>>>> +       samsung,i2c-max-bus-freq = <200000>;
>>>>> +       status = "okay";
>>>>> +
>>>>> +       s2mps15_pmic at 66 {
>>>>> +               compatible = "samsung,s2mps15-pmic";
>>>>> +               reg = <0x66>;
>>>>> +               interrupts = <2 0>;
>>>>> +               interrupt-parent = <&gpa0>;
>>>>> +               pinctrl-names = "default";
>>>>> +               pinctrl-0 = <&pmic_irq>;
>>>>> +               wakeup-source;
>>>>> +
>>>>> +               s2mps15_osc: clocks {
>>>>> +                       compatible = "samsung,s2mps13-clk";
>>>>> +                       #clock-cells = <1>;
>>>>> +                       clock-output-names = "s2mps13_ap",
>>>>> "s2mps13_cp",
>>>>> +                               "s2mps13_bt";
>>>>> +               };
>>>>
>>>>
>>>> Don't you want to use one of these clocks for s3c-rtc (&rtc node)?
>>>>
>>> yes, you are right, rtc on this board is currently broken, mainly
>>> because of
>>> the introduction of rtc_src clock in the s3c-rtc driver.
>>> That is on my do list next. will take a look.
>>>
>>> Are you suggesting to remove this -clk node now and add along with rtc
>>> changes? I feel this should go in along with this patch.
>>
>> Just add it in consecutive patch in this series. You added here some
>> providers (clock and regulators) without consumers. This of course
>> looks good as a way of providing full description of the board but:
>> 1. For regulators always on: may be meaningless for kernel. Kernel
>> does not use it. Existence of regulator subnode will fulfill driver's
>> needs for probe.
>> 2. For clocks: actually will disable these clocks because of lack of
>> consumers... which is fine but probably not what you wanted.
>>
>> The standard approach is to add such providers when they are needed -
>> there are some consumers using them.
>>
> OK. for now will keep the pmic clock added as clock will be in disabled
> state, so it wont harm.
> - will keep system related regulator like supply to arm,mif,int etc ..
> will remove supplies to other peripherals IPs. Hope thats fine.

You don't have to remove other regulators (these without consumers).
Describe in DT all of regulators but:
1. Add to some of them consumers;
2. Disable these which are not needed (by not marking always-enabled).

Just like for all other boards.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2015-11-05  5:40 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-02 10:04 [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node Alim Akhtar
2015-11-02 10:04 ` Alim Akhtar
2015-11-02 10:04 ` [PATCH 2/2] arm64: dts: exynos7: Enable HS200 mode operation Alim Akhtar
2015-11-02 10:04   ` Alim Akhtar
2015-11-02 11:32   ` Javier Martinez Canillas
2015-11-02 11:32     ` Javier Martinez Canillas
2015-11-02 11:58   ` Krzysztof Kozlowski
2015-11-02 11:58     ` Krzysztof Kozlowski
2015-11-02 12:08     ` Alim Akhtar
2015-11-02 12:08       ` Alim Akhtar
2015-11-02 12:27       ` Krzysztof Kozlowski
2015-11-02 12:27         ` Krzysztof Kozlowski
2015-11-02 11:30 ` [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node Javier Martinez Canillas
2015-11-02 11:30   ` Javier Martinez Canillas
2015-11-02 11:30   ` Javier Martinez Canillas
2015-11-02 11:58   ` Alim Akhtar
2015-11-02 11:58     ` Alim Akhtar
2015-11-02 12:25 ` Krzysztof Kozlowski
2015-11-02 12:25   ` Krzysztof Kozlowski
2015-11-02 13:01   ` Alim Akhtar
2015-11-02 13:01     ` Alim Akhtar
2015-11-02 13:01     ` Alim Akhtar
2015-11-02 13:52     ` Krzysztof Kozlowski
2015-11-02 13:52       ` Krzysztof Kozlowski
2015-11-05  5:37       ` Alim Akhtar
2015-11-05  5:37         ` Alim Akhtar
2015-11-05  5:40         ` Krzysztof Kozlowski
2015-11-05  5:40           ` Krzysztof Kozlowski

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.