[PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Andreas Färber]

Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
Spring does not need it, it uses an s5m8767 instead.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v2: New (Doug Anderson)

 arch/arm/boot/dts/exynos5250-cros-common.dtsi | 151 -------------------------
 arch/arm/boot/dts/exynos5250-snow.dts         | 155 ++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 151 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
index 89ac90f..61128d1 100644
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
@@ -27,163 +27,12 @@
 		i2c2_bus: i2c2-bus {
 			samsung,pin-pud = <0>;
 		};
-
-		max77686_irq: max77686-irq {
-			samsung,pins = "gpx3-2";
-			samsung,pin-function = <0>;
-			samsung,pin-pud = <0>;
-			samsung,pin-drv = <0>;
-		};
 	};
 
 	i2c@12C60000 {
 		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <378000>;
-
-		max77686@09 {
-			compatible = "maxim,max77686";
-			interrupt-parent = <&gpx3>;
-			interrupts = <2 0>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&max77686_irq>;
-			wakeup-source;
-			reg = <0x09>;
-			#clock-cells = <1>;
-
-			voltage-regulators {
-				ldo1_reg: LDO1 {
-					regulator-name = "P1.0V_LDO_OUT1";
-					regulator-min-microvolt = <1000000>;
-					regulator-max-microvolt = <1000000>;
-					regulator-always-on;
-				};
-
-				ldo2_reg: LDO2 {
-					regulator-name = "P1.8V_LDO_OUT2";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-				};
-
-				ldo3_reg: LDO3 {
-					regulator-name = "P1.8V_LDO_OUT3";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-				};
-
-				ldo7_reg: LDO7 {
-					regulator-name = "P1.1V_LDO_OUT7";
-					regulator-min-microvolt = <1100000>;
-					regulator-max-microvolt = <1100000>;
-					regulator-always-on;
-				};
-
-				ldo8_reg: LDO8 {
-					regulator-name = "P1.0V_LDO_OUT8";
-					regulator-min-microvolt = <1000000>;
-					regulator-max-microvolt = <1000000>;
-					regulator-always-on;
-				};
-
-				ldo10_reg: LDO10 {
-					regulator-name = "P1.8V_LDO_OUT10";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-				};
-
-				ldo12_reg: LDO12 {
-					regulator-name = "P3.0V_LDO_OUT12";
-					regulator-min-microvolt = <3000000>;
-					regulator-max-microvolt = <3000000>;
-					regulator-always-on;
-				};
-
-				ldo14_reg: LDO14 {
-					regulator-name = "P1.8V_LDO_OUT14";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-				};
-
-				ldo15_reg: LDO15 {
-					regulator-name = "P1.0V_LDO_OUT15";
-					regulator-min-microvolt = <1000000>;
-					regulator-max-microvolt = <1000000>;
-					regulator-always-on;
-				};
-
-				ldo16_reg: LDO16 {
-					regulator-name = "P1.8V_LDO_OUT16";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-				};
-
-				buck1_reg: BUCK1 {
-					regulator-name = "vdd_mif";
-					regulator-min-microvolt = <950000>;
-					regulator-max-microvolt = <1300000>;
-					regulator-always-on;
-					regulator-boot-on;
-				};
-
-				buck2_reg: BUCK2 {
-					regulator-name = "vdd_arm";
-					regulator-min-microvolt = <850000>;
-					regulator-max-microvolt = <1350000>;
-					regulator-always-on;
-					regulator-boot-on;
-				};
-
-				buck3_reg: BUCK3 {
-					regulator-name = "vdd_int";
-					regulator-min-microvolt = <900000>;
-					regulator-max-microvolt = <1200000>;
-					regulator-always-on;
-					regulator-boot-on;
-				};
-
-				buck4_reg: BUCK4 {
-					regulator-name = "vdd_g3d";
-					regulator-min-microvolt = <850000>;
-					regulator-max-microvolt = <1300000>;
-					regulator-always-on;
-					regulator-boot-on;
-				};
-
-				buck5_reg: BUCK5 {
-					regulator-name = "P1.8V_BUCK_OUT5";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
-					regulator-always-on;
-					regulator-boot-on;
-				};
-
-				buck6_reg: BUCK6 {
-					regulator-name = "P1.35V_BUCK_OUT6";
-					regulator-min-microvolt = <1350000>;
-					regulator-max-microvolt = <1350000>;
-					regulator-always-on;
-				};
-
-				buck7_reg: BUCK7 {
-					regulator-name = "P2.0V_BUCK_OUT7";
-					regulator-min-microvolt = <2000000>;
-					regulator-max-microvolt = <2000000>;
-					regulator-always-on;
-				};
-
-				buck8_reg: BUCK8 {
-					regulator-name = "P2.85V_BUCK_OUT8";
-					regulator-min-microvolt = <2850000>;
-					regulator-max-microvolt = <2850000>;
-					regulator-always-on;
-				};
-			};
-		};
 	};
 
 	i2c@12C70000 {
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index c682c88..0f867a3 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -344,4 +344,159 @@
 	};
 };
 
+&i2c_0 {
+	max77686@09 {
+		compatible = "maxim,max77686";
+		interrupt-parent = <&gpx3>;
+		interrupts = <2 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&max77686_irq>;
+		wakeup-source;
+		reg = <0x09>;
+		#clock-cells = <1>;
+
+		voltage-regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "P1.0V_LDO_OUT1";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+			};
+
+			ldo2_reg: LDO2 {
+				regulator-name = "P1.8V_LDO_OUT2";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			ldo3_reg: LDO3 {
+				regulator-name = "P1.8V_LDO_OUT3";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			ldo7_reg: LDO7 {
+				regulator-name = "P1.1V_LDO_OUT7";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			ldo8_reg: LDO8 {
+				regulator-name = "P1.0V_LDO_OUT8";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+			};
+
+			ldo10_reg: LDO10 {
+				regulator-name = "P1.8V_LDO_OUT10";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			ldo12_reg: LDO12 {
+				regulator-name = "P3.0V_LDO_OUT12";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-always-on;
+			};
+
+			ldo14_reg: LDO14 {
+				regulator-name = "P1.8V_LDO_OUT14";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			ldo15_reg: LDO15 {
+				regulator-name = "P1.0V_LDO_OUT15";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+			};
+
+			ldo16_reg: LDO16 {
+				regulator-name = "P1.8V_LDO_OUT16";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck2_reg: BUCK2 {
+				regulator-name = "vdd_arm";
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck3_reg: BUCK3 {
+				regulator-name = "vdd_int";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck4_reg: BUCK4 {
+				regulator-name = "vdd_g3d";
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck5_reg: BUCK5 {
+				regulator-name = "P1.8V_BUCK_OUT5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck6_reg: BUCK6 {
+				regulator-name = "P1.35V_BUCK_OUT6";
+				regulator-min-microvolt = <1350000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+			};
+
+			buck7_reg: BUCK7 {
+				regulator-name = "P2.0V_BUCK_OUT7";
+				regulator-min-microvolt = <2000000>;
+				regulator-max-microvolt = <2000000>;
+				regulator-always-on;
+			};
+
+			buck8_reg: BUCK8 {
+				regulator-name = "P2.85V_BUCK_OUT8";
+				regulator-min-microvolt = <2850000>;
+				regulator-max-microvolt = <2850000>;
+				regulator-always-on;
+			};
+		};
+	};
+};
+
+&pinctrl_0 {
+	max77686_irq: max77686-irq {
+		samsung,pins = "gpx3-2";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+};
+
 #include "cros-ec-keyboard.dtsi"
-- 
1.9.3

RE: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Kukjin Kim]

Andreas Färber wrote:
> 
> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
> Spring does not need it, it uses an s5m8767 instead.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---

Hi Andreas,

I think this should be reviewed by chrome guys.


Doug, can you have a look this series for exynos5250-chromebook?
I'm not sure exynos5250-cros-common is still useful for exynos5250-spring...

- Kukjin

>  v2: New (Doug Anderson)
> 
>  arch/arm/boot/dts/exynos5250-cros-common.dtsi | 151 -------------------------
>  arch/arm/boot/dts/exynos5250-snow.dts         | 155 ++++++++++++++++++++++++++
>  2 files changed, 155 insertions(+), 151 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-
> common.dtsi
> index 89ac90f..61128d1 100644
> --- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> @@ -27,163 +27,12 @@
>  		i2c2_bus: i2c2-bus {
>  			samsung,pin-pud = <0>;
>  		};
> -
> -		max77686_irq: max77686-irq {
> -			samsung,pins = "gpx3-2";
> -			samsung,pin-function = <0>;
> -			samsung,pin-pud = <0>;
> -			samsung,pin-drv = <0>;
> -		};
>  	};
> 
>  	i2c@12C60000 {
>  		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <378000>;
> -
> -		max77686@09 {
> -			compatible = "maxim,max77686";
> -			interrupt-parent = <&gpx3>;
> -			interrupts = <2 0>;
> -			pinctrl-names = "default";
> -			pinctrl-0 = <&max77686_irq>;
> -			wakeup-source;
> -			reg = <0x09>;
> -			#clock-cells = <1>;
> -
> -			voltage-regulators {
> -				ldo1_reg: LDO1 {
> -					regulator-name = "P1.0V_LDO_OUT1";
> -					regulator-min-microvolt = <1000000>;
> -					regulator-max-microvolt = <1000000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo2_reg: LDO2 {
> -					regulator-name = "P1.8V_LDO_OUT2";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo3_reg: LDO3 {
> -					regulator-name = "P1.8V_LDO_OUT3";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo7_reg: LDO7 {
> -					regulator-name = "P1.1V_LDO_OUT7";
> -					regulator-min-microvolt = <1100000>;
> -					regulator-max-microvolt = <1100000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo8_reg: LDO8 {
> -					regulator-name = "P1.0V_LDO_OUT8";
> -					regulator-min-microvolt = <1000000>;
> -					regulator-max-microvolt = <1000000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo10_reg: LDO10 {
> -					regulator-name = "P1.8V_LDO_OUT10";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo12_reg: LDO12 {
> -					regulator-name = "P3.0V_LDO_OUT12";
> -					regulator-min-microvolt = <3000000>;
> -					regulator-max-microvolt = <3000000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo14_reg: LDO14 {
> -					regulator-name = "P1.8V_LDO_OUT14";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo15_reg: LDO15 {
> -					regulator-name = "P1.0V_LDO_OUT15";
> -					regulator-min-microvolt = <1000000>;
> -					regulator-max-microvolt = <1000000>;
> -					regulator-always-on;
> -				};
> -
> -				ldo16_reg: LDO16 {
> -					regulator-name = "P1.8V_LDO_OUT16";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -				};
> -
> -				buck1_reg: BUCK1 {
> -					regulator-name = "vdd_mif";
> -					regulator-min-microvolt = <950000>;
> -					regulator-max-microvolt = <1300000>;
> -					regulator-always-on;
> -					regulator-boot-on;
> -				};
> -
> -				buck2_reg: BUCK2 {
> -					regulator-name = "vdd_arm";
> -					regulator-min-microvolt = <850000>;
> -					regulator-max-microvolt = <1350000>;
> -					regulator-always-on;
> -					regulator-boot-on;
> -				};
> -
> -				buck3_reg: BUCK3 {
> -					regulator-name = "vdd_int";
> -					regulator-min-microvolt = <900000>;
> -					regulator-max-microvolt = <1200000>;
> -					regulator-always-on;
> -					regulator-boot-on;
> -				};
> -
> -				buck4_reg: BUCK4 {
> -					regulator-name = "vdd_g3d";
> -					regulator-min-microvolt = <850000>;
> -					regulator-max-microvolt = <1300000>;
> -					regulator-always-on;
> -					regulator-boot-on;
> -				};
> -
> -				buck5_reg: BUCK5 {
> -					regulator-name = "P1.8V_BUCK_OUT5";
> -					regulator-min-microvolt = <1800000>;
> -					regulator-max-microvolt = <1800000>;
> -					regulator-always-on;
> -					regulator-boot-on;
> -				};
> -
> -				buck6_reg: BUCK6 {
> -					regulator-name = "P1.35V_BUCK_OUT6";
> -					regulator-min-microvolt = <1350000>;
> -					regulator-max-microvolt = <1350000>;
> -					regulator-always-on;
> -				};
> -
> -				buck7_reg: BUCK7 {
> -					regulator-name = "P2.0V_BUCK_OUT7";
> -					regulator-min-microvolt = <2000000>;
> -					regulator-max-microvolt = <2000000>;
> -					regulator-always-on;
> -				};
> -
> -				buck8_reg: BUCK8 {
> -					regulator-name = "P2.85V_BUCK_OUT8";
> -					regulator-min-microvolt = <2850000>;
> -					regulator-max-microvolt = <2850000>;
> -					regulator-always-on;
> -				};
> -			};
> -		};
>  	};
> 
>  	i2c@12C70000 {
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
> index c682c88..0f867a3 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -344,4 +344,159 @@
>  	};
>  };
> 
> +&i2c_0 {
> +	max77686@09 {
> +		compatible = "maxim,max77686";
> +		interrupt-parent = <&gpx3>;
> +		interrupts = <2 0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&max77686_irq>;
> +		wakeup-source;
> +		reg = <0x09>;
> +		#clock-cells = <1>;
> +
> +		voltage-regulators {
> +			ldo1_reg: LDO1 {
> +				regulator-name = "P1.0V_LDO_OUT1";
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <1000000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: LDO2 {
> +				regulator-name = "P1.8V_LDO_OUT2";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: LDO3 {
> +				regulator-name = "P1.8V_LDO_OUT3";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo7_reg: LDO7 {
> +				regulator-name = "P1.1V_LDO_OUT7";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo8_reg: LDO8 {
> +				regulator-name = "P1.0V_LDO_OUT8";
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <1000000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo10_reg: LDO10 {
> +				regulator-name = "P1.8V_LDO_OUT10";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo12_reg: LDO12 {
> +				regulator-name = "P3.0V_LDO_OUT12";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo14_reg: LDO14 {
> +				regulator-name = "P1.8V_LDO_OUT14";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo15_reg: LDO15 {
> +				regulator-name = "P1.0V_LDO_OUT15";
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <1000000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo16_reg: LDO16 {
> +				regulator-name = "P1.8V_LDO_OUT16";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			buck1_reg: BUCK1 {
> +				regulator-name = "vdd_mif";
> +				regulator-min-microvolt = <950000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck2_reg: BUCK2 {
> +				regulator-name = "vdd_arm";
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck3_reg: BUCK3 {
> +				regulator-name = "vdd_int";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck4_reg: BUCK4 {
> +				regulator-name = "vdd_g3d";
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck5_reg: BUCK5 {
> +				regulator-name = "P1.8V_BUCK_OUT5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck6_reg: BUCK6 {
> +				regulator-name = "P1.35V_BUCK_OUT6";
> +				regulator-min-microvolt = <1350000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-always-on;
> +			};
> +
> +			buck7_reg: BUCK7 {
> +				regulator-name = "P2.0V_BUCK_OUT7";
> +				regulator-min-microvolt = <2000000>;
> +				regulator-max-microvolt = <2000000>;
> +				regulator-always-on;
> +			};
> +
> +			buck8_reg: BUCK8 {
> +				regulator-name = "P2.85V_BUCK_OUT8";
> +				regulator-min-microvolt = <2850000>;
> +				regulator-max-microvolt = <2850000>;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
> +&pinctrl_0 {
> +	max77686_irq: max77686-irq {
> +		samsung,pins = "gpx3-2";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <0>;
> +		samsung,pin-drv = <0>;
> +	};
> +};
> +
>  #include "cros-ec-keyboard.dtsi"
> --
> 1.9.3

Re: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Doug Anderson]

Hi,

On Thu, Jul 24, 2014 at 10:28 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Andreas Färber wrote:
>>
>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>> Spring does not need it, it uses an s5m8767 instead.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>
> Hi Andreas,
>
> I think this should be reviewed by chrome guys.
>
>
> Doug, can you have a look this series for exynos5250-chromebook?
> I'm not sure exynos5250-cros-common is still useful for exynos5250-spring...

Sorry, I've been on vacation and am now playing catchup.

Right.  The current suggestion is to rework to just duplicate things
and don't use exynos5250-cros-common.  I would structure the series
as:

1. Fold the current exynos5250-cros-common directly into the snow
device tree file.

2. If desired, cleanup the snow device tree file.

3. Create the spring device tree file based on snow.  There will be
duplication, but so be it.  Please try to minimize diffs between the
snow ad spring device tree files.

4. If possible, identify "fragments" like "cros-ec-keyboard.dtsi" and
factor them out of both spring and snow device tree files.


You could certainly reorder some of the above steps if you wish (you
could split out fragments before creating the spring device tree, for
instance).


Please make sure to CC Javier (now on CC) since he's helping with
Chromebook upstreaming and he may have a lower latency in responding
than I do.

-Doug

Re: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Doug Anderson]

Andreas,

On Fri, Jul 25, 2014 at 8:08 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Thu, Jul 24, 2014 at 10:28 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> Andreas Färber wrote:
>>>
>>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>>> Spring does not need it, it uses an s5m8767 instead.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>
>> Hi Andreas,
>>
>> I think this should be reviewed by chrome guys.
>>
>>
>> Doug, can you have a look this series for exynos5250-chromebook?
>> I'm not sure exynos5250-cros-common is still useful for exynos5250-spring...
>
> Sorry, I've been on vacation and am now playing catchup.
>
> Right.  The current suggestion is to rework to just duplicate things
> and don't use exynos5250-cros-common.

Oh, it actually looks like your patch series ends up there, but the
first patch confused me.  Why move just the maxim PMIC first?  Move
everything at once and be done with the common file, then do
cleanup...

Re: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Andreas Färber]

Hi Doug,

Am 25.07.2014 17:13, schrieb Doug Anderson:
> On Fri, Jul 25, 2014 at 8:08 AM, Doug Anderson <dianders@chromium.org> wrote:
>> On Thu, Jul 24, 2014 at 10:28 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>>> Andreas Färber wrote:
>>>>
>>>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>>>> Spring does not need it, it uses an s5m8767 instead.
>>>>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>
>>> Doug, can you have a look this series for exynos5250-chromebook?
>>> I'm not sure exynos5250-cros-common is still useful for exynos5250-spring...
>>
>> Right.  The current suggestion is to rework to just duplicate things
>> and don't use exynos5250-cros-common.
> 
> Oh, it actually looks like your patch series ends up there, but the
> first patch confused me.

NP, guess you just started reading in the middle. ;)

>  Why move just the maxim PMIC first?

Review of the movements surely is easier this way, and it did not yet
seem a done deal that we would drop -cros-common. The merge patch nicely
shows which common parts need to be copied into spring.

>  Move
> everything at once and be done with the common file, then do
> cleanup...

If everyone agrees that we want to proceed that way, we can easily
squash patches 1+2. Separating is always harder.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Doug Anderson]

Andreas,

On Fri, Jul 25, 2014 at 8:30 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Doug,
>
> Am 25.07.2014 17:13, schrieb Doug Anderson:
>> On Fri, Jul 25, 2014 at 8:08 AM, Doug Anderson <dianders@chromium.org> wrote:
>>> On Thu, Jul 24, 2014 at 10:28 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>>>> Andreas Färber wrote:
>>>>>
>>>>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>>>>> Spring does not need it, it uses an s5m8767 instead.
>>>>>
>>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>>> ---
>>>>
>>>> Doug, can you have a look this series for exynos5250-chromebook?
>>>> I'm not sure exynos5250-cros-common is still useful for exynos5250-spring...
>>>
>>> Right.  The current suggestion is to rework to just duplicate things
>>> and don't use exynos5250-cros-common.
>>
>> Oh, it actually looks like your patch series ends up there, but the
>> first patch confused me.
>
> NP, guess you just started reading in the middle. ;)

Yeah, I had this on my list to look at but ended up looking at this
one first due to Kukjin's response.


>>  Why move just the maxim PMIC first?
>
> Review of the movements surely is easier this way, and it did not yet
> seem a done deal that we would drop -cros-common. The merge patch nicely
> shows which common parts need to be copied into spring.

That's fine.  I can review it this way now that I realize what's going on.

Re: [PATCH v2 1/4] ARM: dts: exynos5250: max77686 is Snow only

[Doug Anderson]

Andreas,

On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
> Spring does not need it, it uses an s5m8767 instead.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v2: New (Doug Anderson)
>
>  arch/arm/boot/dts/exynos5250-cros-common.dtsi | 151 -------------------------
>  arch/arm/boot/dts/exynos5250-snow.dts         | 155 ++++++++++++++++++++++++++
>  2 files changed, 155 insertions(+), 151 deletions(-)

Personally I would have squashed patches 1-3, but I don't have a
problem with it being broken up and it does mean that the mondo patch
is smaller, so...

Reviewed-by: Doug Anderson <dianders@chromium.org>

Kukjin could apply this now if he wanted to avoid future conflicts.

-Doug

[PATCH v2 2/4] ARM: dts: exynos5250: cypress,cyapa trackpad is Snow only

[Andreas Färber]

Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
Spring does not need it, it uses an Atmel maXTouch instead.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v2: New (Doug Anderson)

 arch/arm/boot/dts/exynos5250-cros-common.dtsi |  8 --------
 arch/arm/boot/dts/exynos5250-snow.dts         | 10 ++++++++++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
index 61128d1..e603e9c 100644
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
@@ -39,14 +39,6 @@
 		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <378000>;
-
-		trackpad {
-			reg = <0x67>;
-			compatible = "cypress,cyapa";
-			interrupts = <2 0>;
-			interrupt-parent = <&gpx1>;
-			wakeup-source;
-		};
 	};
 
 	i2c@12C80000 {
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 0f867a3..f2b8c41 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -490,6 +490,16 @@
 	};
 };
 
+&i2c_1 {
+	trackpad {
+		reg = <0x67>;
+		compatible = "cypress,cyapa";
+		interrupts = <2 0>;
+		interrupt-parent = <&gpx1>;
+		wakeup-source;
+	};
+};
+
 &pinctrl_0 {
 	max77686_irq: max77686-irq {
 		samsung,pins = "gpx3-2";
-- 
1.9.3

Re: [PATCH v2 2/4] ARM: dts: exynos5250: cypress,cyapa trackpad is Snow only

[Doug Anderson]

Andreas,

On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
> Spring does not need it, it uses an Atmel maXTouch instead.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v2: New (Doug Anderson)
>
>  arch/arm/boot/dts/exynos5250-cros-common.dtsi |  8 --------
>  arch/arm/boot/dts/exynos5250-snow.dts         | 10 ++++++++++
>  2 files changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>

Kukjin could apply this now if he wanted to avoid future conflicts.

Re: [PATCH v2 2/4] ARM: dts: exynos5250: cypress,cyapa trackpad is Snow only

[Kukjin Kim]

On 07/30/14 01:16, Doug Anderson wrote:
> Andreas,
>
> On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber<afaerber@suse.de>  wrote:
>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>> Spring does not need it, it uses an Atmel maXTouch instead.
>>
>> Signed-off-by: Andreas Färber<afaerber@suse.de>
>> ---
>>   v2: New (Doug Anderson)
>>
>>   arch/arm/boot/dts/exynos5250-cros-common.dtsi |  8 --------
>>   arch/arm/boot/dts/exynos5250-snow.dts         | 10 ++++++++++
>>   2 files changed, 10 insertions(+), 8 deletions(-)
>
> Reviewed-by: Doug Anderson<dianders@chromium.org>
>
> Kukjin could apply this now if he wanted to avoid future conflicts.

OK, I read your comments on another this series and I've applied 1/4 and 
2/4 as per your suggestion with your reviewed by tag ;)

And I think, Andreas is going to re-spin remains for exynos5250-spring 
based on updated my -next?

Thanks,
Kukjin

Re: [PATCH v2 2/4] ARM: dts: exynos5250: cypress,cyapa trackpad is Snow only

[Andreas Färber]

Am 30.07.2014 01:06, schrieb Kukjin Kim:
> On 07/30/14 01:16, Doug Anderson wrote:
>> On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber<afaerber@suse.de> 
>> wrote:
>>> Move it from exynos5250-cros-common.dtsi to exynos5250-snow.dts.
>>> Spring does not need it, it uses an Atmel maXTouch instead.
>>>
>>> Signed-off-by: Andreas Färber<afaerber@suse.de>
>>> ---
>>>   v2: New (Doug Anderson)
>>>
>>>   arch/arm/boot/dts/exynos5250-cros-common.dtsi |  8 --------
>>>   arch/arm/boot/dts/exynos5250-snow.dts         | 10 ++++++++++
>>>   2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> Reviewed-by: Doug Anderson<dianders@chromium.org>
>>
>> Kukjin could apply this now if he wanted to avoid future conflicts.
> 
> OK, I read your comments on another this series and I've applied 1/4 and
> 2/4 as per your suggestion with your reviewed by tag ;)

Thanks.

> And I think, Andreas is going to re-spin remains for exynos5250-spring
> based on updated my -next?

Yes. Please don't apply 3/4, I'm fixing a duplicate hdmi node.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Andreas Färber]

The remaining pieces are fairly minor.

Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v2: New (Doug Anderson)

 arch/arm/boot/dts/exynos5250-cros-common.dtsi | 164 -------------------------
 arch/arm/boot/dts/exynos5250-snow.dts         | 169 ++++++++++++++++++++++----
 2 files changed, 148 insertions(+), 185 deletions(-)
 delete mode 100644 arch/arm/boot/dts/exynos5250-cros-common.dtsi

diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
deleted file mode 100644
index e603e9c..0000000
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ /dev/null
@@ -1,164 +0,0 @@
-/*
- * Common device tree include for all Exynos 5250 boards based off of Daisy.
- *
- * Copyright (c) 2012 Google, Inc
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
-*/
-
-/ {
-	aliases {
-	};
-
-	memory {
-		reg = <0x40000000 0x80000000>;
-	};
-
-	chosen {
-	};
-
-	pinctrl@11400000 {
-		/*
-		 * Disabled pullups since external part has its own pullups and
-		 * double-pulling gets us out of spec in some cases.
-		 */
-		i2c2_bus: i2c2-bus {
-			samsung,pin-pud = <0>;
-		};
-	};
-
-	i2c@12C60000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <378000>;
-	};
-
-	i2c@12C70000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <378000>;
-	};
-
-	i2c@12C80000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <66000>;
-
-		hdmiddc@50 {
-			compatible = "samsung,exynos4210-hdmiddc";
-			reg = <0x50>;
-		};
-	};
-
-	i2c@12C90000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <66000>;
-	};
-
-	i2c@12CA0000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <66000>;
-	};
-
-	i2c@12CB0000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <66000>;
-	};
-
-	i2c@12CD0000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <66000>;
-	};
-
-	i2c@12CE0000 {
-		status = "okay";
-		samsung,i2c-sda-delay = <100>;
-		samsung,i2c-max-bus-freq = <378000>;
-
-		hdmiphy: hdmiphy@38 {
-			compatible = "samsung,exynos4212-hdmiphy";
-			reg = <0x38>;
-		};
-	};
-
-	mmc@12200000 {
-		num-slots = <1>;
-		supports-highspeed;
-		broken-cd;
-		card-detect-delay = <200>;
-		samsung,dw-mshc-ciu-div = <3>;
-		samsung,dw-mshc-sdr-timing = <2 3>;
-		samsung,dw-mshc-ddr-timing = <1 2>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
-
-		slot@0 {
-			reg = <0>;
-			bus-width = <8>;
-		};
-	};
-
-	mmc@12220000 {
-		num-slots = <1>;
-		supports-highspeed;
-		card-detect-delay = <200>;
-		samsung,dw-mshc-ciu-div = <3>;
-		samsung,dw-mshc-sdr-timing = <2 3>;
-		samsung,dw-mshc-ddr-timing = <1 2>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-
-		slot@0 {
-			reg = <0>;
-			bus-width = <4>;
-			wp-gpios = <&gpc2 1 0>;
-		};
-	};
-
-	mmc@12230000 {
-		num-slots = <1>;
-		supports-highspeed;
-		broken-cd;
-		card-detect-delay = <200>;
-		samsung,dw-mshc-ciu-div = <3>;
-		samsung,dw-mshc-sdr-timing = <2 3>;
-		samsung,dw-mshc-ddr-timing = <1 2>;
-		/* See board-specific dts files for pin setup */
-
-		slot@0 {
-			reg = <0>;
-			bus-width = <4>;
-		};
-	};
-
-	spi_1: spi@12d30000 {
-		status = "okay";
-		samsung,spi-src-clk = <0>;
-		num-cs = <1>;
-	};
-
-	hdmi {
-		hpd-gpio = <&gpx3 7 0>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&hdmi_hpd_irq>;
-		phy = <&hdmiphy>;
-		ddc = <&i2c_2>;
-	};
-
-	gpio-keys {
-		compatible = "gpio-keys";
-
-		power {
-			label = "Power";
-			gpios = <&gpx1 3 1>;
-			linux,code = <116>; /* KEY_POWER */
-			gpio-key,wakeup;
-		};
-	};
-};
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index f2b8c41..cdf74c5 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -10,7 +10,6 @@
 
 /dts-v1/;
 #include "exynos5250.dtsi"
-#include "exynos5250-cros-common.dtsi"
 
 / {
 	model = "Google Snow";
@@ -20,6 +19,13 @@
 		i2c104 = &i2c_104;
 	};
 
+	memory {
+		reg = <0x40000000 0x80000000>;
+	};
+
+	chosen {
+	};
+
 	rtc@101E0000 {
 		status = "okay";
 	};
@@ -93,6 +99,13 @@
 	gpio-keys {
 		compatible = "gpio-keys";
 
+		power {
+			label = "Power";
+			gpios = <&gpx1 3 1>;
+			linux,code = <116>; /* KEY_POWER */
+			gpio-key,wakeup;
+		};
+
 		lid-switch {
 			label = "Lid";
 			gpios = <&gpx3 5 1>;
@@ -226,26 +239,6 @@
 		};
 	};
 
-	mmc@12200000 {
-		status = "okay";
-	};
-
-	mmc@12220000 {
-		status = "okay";
-	};
-
-	/*
-	 * On Snow we've got SIP WiFi and so can keep drive strengths low to
-	 * reduce EMI.
-	 */
-	mmc@12230000 {
-		status = "okay";
-		slot@0 {
-			pinctrl-names = "default";
-			pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
-		};
-	};
-
 	i2c@12CD0000 {
 		max98095: codec@11 {
 			compatible = "maxim,max98095";
@@ -314,6 +307,14 @@
 		samsung,invert-vclk;
 	};
 
+	hdmi {
+		hpd-gpio = <&gpx3 7 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hdmi_hpd_irq>;
+		phy = <&hdmiphy>;
+		ddc = <&i2c_2>;
+	};
+
 	dp-controller@145B0000 {
 		status = "okay";
 		pinctrl-names = "default";
@@ -345,6 +346,10 @@
 };
 
 &i2c_0 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+
 	max77686@09 {
 		compatible = "maxim,max77686";
 		interrupt-parent = <&gpx3>;
@@ -491,6 +496,10 @@
 };
 
 &i2c_1 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+
 	trackpad {
 		reg = <0x67>;
 		compatible = "cypress,cyapa";
@@ -500,7 +509,119 @@
 	};
 };
 
+&i2c_2 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+
+	hdmiddc@50 {
+		compatible = "samsung,exynos4210-hdmiddc";
+		reg = <0x50>;
+	};
+};
+
+&i2c_3 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_4 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_5 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_7 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_8 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+
+	hdmiphy: hdmiphy@38 {
+		compatible = "samsung,exynos4212-hdmiphy";
+		reg = <0x38>;
+	};
+};
+
+&mmc_0 {
+	status = "okay";
+	num-slots = <1>;
+	supports-highspeed;
+	broken-cd;
+	card-detect-delay = <200>;
+	samsung,dw-mshc-ciu-div = <3>;
+	samsung,dw-mshc-sdr-timing = <2 3>;
+	samsung,dw-mshc-ddr-timing = <1 2>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
+
+	slot@0 {
+		reg = <0>;
+		bus-width = <8>;
+	};
+};
+
+&mmc_2 {
+	status = "okay";
+	num-slots = <1>;
+	supports-highspeed;
+	card-detect-delay = <200>;
+	samsung,dw-mshc-ciu-div = <3>;
+	samsung,dw-mshc-sdr-timing = <2 3>;
+	samsung,dw-mshc-ddr-timing = <1 2>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
+
+	slot@0 {
+		reg = <0>;
+		bus-width = <4>;
+		wp-gpios = <&gpc2 1 0>;
+	};
+};
+
+/*
+ * On Snow we've got SIP WiFi and so can keep drive strengths low to
+ * reduce EMI.
+ */
+&mmc_3 {
+	status = "okay";
+	num-slots = <1>;
+	supports-highspeed;
+	broken-cd;
+	card-detect-delay = <200>;
+	samsung,dw-mshc-ciu-div = <3>;
+	samsung,dw-mshc-sdr-timing = <2 3>;
+	samsung,dw-mshc-ddr-timing = <1 2>;
+
+	slot@0 {
+		reg = <0>;
+		bus-width = <4>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
+	};
+};
+
 &pinctrl_0 {
+	/*
+	 * Disabled pullups since external part has its own pullups and
+	 * double-pulling gets us out of spec in some cases.
+	 */
+	i2c2_bus: i2c2-bus {
+		samsung,pin-pud = <0>;
+	};
+
 	max77686_irq: max77686-irq {
 		samsung,pins = "gpx3-2";
 		samsung,pin-function = <0>;
@@ -509,4 +630,10 @@
 	};
 };
 
+&spi_1 {
+	status = "okay";
+	samsung,spi-src-clk = <0>;
+	num-cs = <1>;
+};
+
 #include "cros-ec-keyboard.dtsi"
-- 
1.9.3

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Doug Anderson]

Andreas,

On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
> The remaining pieces are fairly minor.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v2: New (Doug Anderson)
>
>  arch/arm/boot/dts/exynos5250-cros-common.dtsi | 164 -------------------------
>  arch/arm/boot/dts/exynos5250-snow.dts         | 169 ++++++++++++++++++++++----
>  2 files changed, 148 insertions(+), 185 deletions(-)
>  delete mode 100644 arch/arm/boot/dts/exynos5250-cros-common.dtsi
>
> diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> deleted file mode 100644
> index e603e9c..0000000
> --- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> +++ /dev/null
> @@ -1,164 +0,0 @@
> -/*
> - * Common device tree include for all Exynos 5250 boards based off of Daisy.
> - *
> - * Copyright (c) 2012 Google, Inc
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> -*/
> -
> -/ {
> -       aliases {
> -       };
> -
> -       memory {
> -               reg = <0x40000000 0x80000000>;
> -       };
> -
> -       chosen {
> -       };
> -
> -       pinctrl@11400000 {
> -               /*
> -                * Disabled pullups since external part has its own pullups and
> -                * double-pulling gets us out of spec in some cases.
> -                */
> -               i2c2_bus: i2c2-bus {
> -                       samsung,pin-pud = <0>;
> -               };
> -       };
> -
> -       i2c@12C60000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <378000>;
> -       };
> -
> -       i2c@12C70000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <378000>;
> -       };
> -
> -       i2c@12C80000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <66000>;
> -
> -               hdmiddc@50 {
> -                       compatible = "samsung,exynos4210-hdmiddc";
> -                       reg = <0x50>;
> -               };
> -       };
> -
> -       i2c@12C90000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <66000>;
> -       };
> -
> -       i2c@12CA0000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <66000>;
> -       };
> -
> -       i2c@12CB0000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <66000>;
> -       };
> -
> -       i2c@12CD0000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <66000>;
> -       };
> -
> -       i2c@12CE0000 {
> -               status = "okay";
> -               samsung,i2c-sda-delay = <100>;
> -               samsung,i2c-max-bus-freq = <378000>;
> -
> -               hdmiphy: hdmiphy@38 {
> -                       compatible = "samsung,exynos4212-hdmiphy";
> -                       reg = <0x38>;
> -               };
> -       };
> -
> -       mmc@12200000 {
> -               num-slots = <1>;
> -               supports-highspeed;
> -               broken-cd;
> -               card-detect-delay = <200>;
> -               samsung,dw-mshc-ciu-div = <3>;
> -               samsung,dw-mshc-sdr-timing = <2 3>;
> -               samsung,dw-mshc-ddr-timing = <1 2>;
> -               pinctrl-names = "default";
> -               pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
> -
> -               slot@0 {
> -                       reg = <0>;
> -                       bus-width = <8>;
> -               };
> -       };
> -
> -       mmc@12220000 {
> -               num-slots = <1>;
> -               supports-highspeed;
> -               card-detect-delay = <200>;
> -               samsung,dw-mshc-ciu-div = <3>;
> -               samsung,dw-mshc-sdr-timing = <2 3>;
> -               samsung,dw-mshc-ddr-timing = <1 2>;
> -               pinctrl-names = "default";
> -               pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
> -
> -               slot@0 {
> -                       reg = <0>;
> -                       bus-width = <4>;
> -                       wp-gpios = <&gpc2 1 0>;
> -               };
> -       };
> -
> -       mmc@12230000 {
> -               num-slots = <1>;
> -               supports-highspeed;
> -               broken-cd;
> -               card-detect-delay = <200>;
> -               samsung,dw-mshc-ciu-div = <3>;
> -               samsung,dw-mshc-sdr-timing = <2 3>;
> -               samsung,dw-mshc-ddr-timing = <1 2>;
> -               /* See board-specific dts files for pin setup */
> -
> -               slot@0 {
> -                       reg = <0>;
> -                       bus-width = <4>;
> -               };
> -       };
> -
> -       spi_1: spi@12d30000 {
> -               status = "okay";
> -               samsung,spi-src-clk = <0>;
> -               num-cs = <1>;
> -       };
> -
> -       hdmi {
> -               hpd-gpio = <&gpx3 7 0>;
> -               pinctrl-names = "default";
> -               pinctrl-0 = <&hdmi_hpd_irq>;
> -               phy = <&hdmiphy>;
> -               ddc = <&i2c_2>;
> -       };
> -
> -       gpio-keys {
> -               compatible = "gpio-keys";
> -
> -               power {
> -                       label = "Power";
> -                       gpios = <&gpx1 3 1>;
> -                       linux,code = <116>; /* KEY_POWER */
> -                       gpio-key,wakeup;
> -               };
> -       };
> -};
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
> index f2b8c41..cdf74c5 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -10,7 +10,6 @@
>
>  /dts-v1/;
>  #include "exynos5250.dtsi"
> -#include "exynos5250-cros-common.dtsi"
>
>  / {
>         model = "Google Snow";
> @@ -20,6 +19,13 @@
>                 i2c104 = &i2c_104;
>         };
>
> +       memory {
> +               reg = <0x40000000 0x80000000>;
> +       };
> +
> +       chosen {
> +       };
> +
>         rtc@101E0000 {
>                 status = "okay";
>         };
> @@ -93,6 +99,13 @@
>         gpio-keys {
>                 compatible = "gpio-keys";
>
> +               power {
> +                       label = "Power";
> +                       gpios = <&gpx1 3 1>;
> +                       linux,code = <116>; /* KEY_POWER */
> +                       gpio-key,wakeup;
> +               };
> +
>                 lid-switch {
>                         label = "Lid";
>                         gpios = <&gpx3 5 1>;
> @@ -226,26 +239,6 @@
>                 };
>         };
>
> -       mmc@12200000 {
> -               status = "okay";
> -       };
> -
> -       mmc@12220000 {
> -               status = "okay";
> -       };
> -
> -       /*
> -        * On Snow we've got SIP WiFi and so can keep drive strengths low to
> -        * reduce EMI.
> -        */
> -       mmc@12230000 {
> -               status = "okay";
> -               slot@0 {
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
> -               };
> -       };
> -
>         i2c@12CD0000 {
>                 max98095: codec@11 {
>                         compatible = "maxim,max98095";
> @@ -314,6 +307,14 @@
>                 samsung,invert-vclk;
>         };
>
> +       hdmi {
> +               hpd-gpio = <&gpx3 7 0>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&hdmi_hpd_irq>;
> +               phy = <&hdmiphy>;
> +               ddc = <&i2c_2>;
> +       };
> +
>         dp-controller@145B0000 {
>                 status = "okay";
>                 pinctrl-names = "default";
> @@ -345,6 +346,10 @@
>  };
>
>  &i2c_0 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +
>         max77686@09 {
>                 compatible = "maxim,max77686";
>                 interrupt-parent = <&gpx3>;
> @@ -491,6 +496,10 @@
>  };
>
>  &i2c_1 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +
>         trackpad {
>                 reg = <0x67>;
>                 compatible = "cypress,cyapa";
> @@ -500,7 +509,119 @@
>         };
>  };
>
> +&i2c_2 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +
> +       hdmiddc@50 {
> +               compatible = "samsung,exynos4210-hdmiddc";
> +               reg = <0x50>;
> +       };
> +};
> +
> +&i2c_3 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_4 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_5 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_7 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_8 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +
> +       hdmiphy: hdmiphy@38 {
> +               compatible = "samsung,exynos4212-hdmiphy";
> +               reg = <0x38>;
> +       };
> +};
> +
> +&mmc_0 {
> +       status = "okay";
> +       num-slots = <1>;
> +       supports-highspeed;
> +       broken-cd;
> +       card-detect-delay = <200>;
> +       samsung,dw-mshc-ciu-div = <3>;
> +       samsung,dw-mshc-sdr-timing = <2 3>;
> +       samsung,dw-mshc-ddr-timing = <1 2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
> +
> +       slot@0 {
> +               reg = <0>;
> +               bus-width = <8>;
> +       };
> +};
> +
> +&mmc_2 {
> +       status = "okay";
> +       num-slots = <1>;
> +       supports-highspeed;
> +       card-detect-delay = <200>;
> +       samsung,dw-mshc-ciu-div = <3>;
> +       samsung,dw-mshc-sdr-timing = <2 3>;
> +       samsung,dw-mshc-ddr-timing = <1 2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
> +
> +       slot@0 {
> +               reg = <0>;
> +               bus-width = <4>;
> +               wp-gpios = <&gpc2 1 0>;
> +       };
> +};
> +
> +/*
> + * On Snow we've got SIP WiFi and so can keep drive strengths low to
> + * reduce EMI.
> + */
> +&mmc_3 {
> +       status = "okay";
> +       num-slots = <1>;
> +       supports-highspeed;
> +       broken-cd;
> +       card-detect-delay = <200>;
> +       samsung,dw-mshc-ciu-div = <3>;
> +       samsung,dw-mshc-sdr-timing = <2 3>;
> +       samsung,dw-mshc-ddr-timing = <1 2>;
> +
> +       slot@0 {
> +               reg = <0>;
> +               bus-width = <4>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;

This actually looks like a bug on snow.  I don't think these pinctrl
statements are taking effect and I think they belong at the mmc level,
not the slot level.  That's not a new bug introduced by you, though.
Mind fixing it?  You've already got it right for Spring mmc_1.


> +       };
> +};
> +
>  &pinctrl_0 {
> +       /*
> +        * Disabled pullups since external part has its own pullups and
> +        * double-pulling gets us out of spec in some cases.
> +        */
> +       i2c2_bus: i2c2-bus {
> +               samsung,pin-pud = <0>;
> +       };
> +
>         max77686_irq: max77686-irq {
>                 samsung,pins = "gpx3-2";
>                 samsung,pin-function = <0>;
> @@ -509,4 +630,10 @@
>         };
>  };
>
> +&spi_1 {
> +       status = "okay";
> +       samsung,spi-src-clk = <0>;
> +       num-cs = <1>;
> +};
> +
>  #include "cros-ec-keyboard.dtsi"
> --
> 1.9.3
>

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Andreas Färber]

Doug,

Am 25.07.2014 18:02, schrieb Doug Anderson:
> On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
>> +/*
>> + * On Snow we've got SIP WiFi and so can keep drive strengths low to
>> + * reduce EMI.
>> + */
>> +&mmc_3 {
>> +       status = "okay";
>> +       num-slots = <1>;
>> +       supports-highspeed;
>> +       broken-cd;
>> +       card-detect-delay = <200>;
>> +       samsung,dw-mshc-ciu-div = <3>;
>> +       samsung,dw-mshc-sdr-timing = <2 3>;
>> +       samsung,dw-mshc-ddr-timing = <1 2>;
>> +
>> +       slot@0 {
>> +               reg = <0>;
>> +               bus-width = <4>;
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
> 
> This actually looks like a bug on snow.  I don't think these pinctrl
> statements are taking effect and I think they belong at the mmc level,
> not the slot level.  That's not a new bug introduced by you, though.
> Mind fixing it?  You've already got it right for Spring mmc_1.

I was taught never to mix code movements with functional changes, as it
hides them even if mentioned in the commit message. Would you like me to
fix it pre- or post-move? Post-move would be easiest for me. ;)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Doug Anderson]

Andreas,

On Tue, Jul 29, 2014 at 5:45 AM, Andreas Färber <afaerber@suse.de> wrote:
> Doug,
>
> Am 25.07.2014 18:02, schrieb Doug Anderson:
>> On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
>>> +/*
>>> + * On Snow we've got SIP WiFi and so can keep drive strengths low to
>>> + * reduce EMI.
>>> + */
>>> +&mmc_3 {
>>> +       status = "okay";
>>> +       num-slots = <1>;
>>> +       supports-highspeed;
>>> +       broken-cd;
>>> +       card-detect-delay = <200>;
>>> +       samsung,dw-mshc-ciu-div = <3>;
>>> +       samsung,dw-mshc-sdr-timing = <2 3>;
>>> +       samsung,dw-mshc-ddr-timing = <1 2>;
>>> +
>>> +       slot@0 {
>>> +               reg = <0>;
>>> +               bus-width = <4>;
>>> +               pinctrl-names = "default";
>>> +               pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
>>
>> This actually looks like a bug on snow.  I don't think these pinctrl
>> statements are taking effect and I think they belong at the mmc level,
>> not the slot level.  That's not a new bug introduced by you, though.
>> Mind fixing it?  You've already got it right for Spring mmc_1.
>
> I was taught never to mix code movements with functional changes, as it
> hides them even if mentioned in the commit message. Would you like me to
> fix it pre- or post-move? Post-move would be easiest for me. ;)

Agreed.  Sorry for implying that it belonged as part of this patch.
I'd be happy if it was somewhere in the series, ideally before you
introduce the Spring device tree so diffs are cleaner for it.  I don't
care if it's pre-move or post-move.

-Doug

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Javier Martinez Canillas]

Hello Andreas,

On Fri, Jul 18, 2014 at 7:20 PM, Andreas Färber <afaerber@suse.de> wrote:
>
> +       memory {
> +               reg = <0x40000000 0x80000000>;
> +       };
> +
> +       chosen {
> +       };
> +

Is there a reason for an empty chosen node? Same for the Spring DTS.

Best regards,
Javier

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Doug Anderson]

Javier,

On Fri, Jul 25, 2014 at 9:35 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Andreas,
>
> On Fri, Jul 18, 2014 at 7:20 PM, Andreas Färber <afaerber@suse.de> wrote:
>>
>> +       memory {
>> +               reg = <0x40000000 0x80000000>;
>> +       };
>> +
>> +       chosen {
>> +       };
>> +
>
> Is there a reason for an empty chosen node? Same for the Spring DTS.

I know that the bootloader (U-Boot) fills this in.  I'd have to double
check how the bootloader handles the node not being there to begin
with.

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Javier Martinez Canillas]

Hello Doug,

On Fri, Jul 25, 2014 at 6:43 PM, Doug Anderson <dianders@chromium.org> wrote:
> Javier,
>
> On Fri, Jul 25, 2014 at 9:35 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>> Hello Andreas,
>>
>> On Fri, Jul 18, 2014 at 7:20 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>
>>> +       memory {
>>> +               reg = <0x40000000 0x80000000>;
>>> +       };
>>> +
>>> +       chosen {
>>> +       };
>>> +
>>
>> Is there a reason for an empty chosen node? Same for the Spring DTS.
>
> I know that the bootloader (U-Boot) fills this in.  I'd have to double
> check how the bootloader handles the node not being there to begin
> with.

Yes, U-Boot fills this but if the node does not exist in the FDT, it
creates one. Take a look at fdt_chosen() in common/fdt_support.c [0].

So I think it's better to just remove it since is empty.

Best regards,
Javier

[0]: https://chromium.googlesource.com/chromiumos/third_party/u-boot/+/master/common/fdt_support.c#215

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Andreas Färber]

Hi Javier and Doug,

Am 25.07.2014 19:02, schrieb Javier Martinez Canillas:
> On Fri, Jul 25, 2014 at 6:43 PM, Doug Anderson <dianders@chromium.org> wrote:
>> On Fri, Jul 25, 2014 at 9:35 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>> Hello Andreas,
>>>
>>> On Fri, Jul 18, 2014 at 7:20 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>>
>>>> +       memory {
>>>> +               reg = <0x40000000 0x80000000>;
>>>> +       };
>>>> +
>>>> +       chosen {
>>>> +       };
>>>> +
>>>
>>> Is there a reason for an empty chosen node? Same for the Spring DTS.
>>
>> I know that the bootloader (U-Boot) fills this in.  I'd have to double
>> check how the bootloader handles the node not being there to begin
>> with.
> 
> Yes, U-Boot fills this but if the node does not exist in the FDT, it
> creates one. Take a look at fdt_chosen() in common/fdt_support.c [0].
> 
> So I think it's better to just remove it since is empty.

Hm, in different context it was stated that device trees shouldn't rely
on bootloader behavior (for a /memory node on Zynq we ended up
specifying the size cell with the real value despite U-Boot overriding
it to a lesser value).

Can we instead settle on filling this node with defaults? :)
bootargs = "console=tty1"; would be my choice for Spring. Would that be
applicable for Snow as well?
Not sure how to specify that via linux,stdout-path property.

I believe this would be a follow-up patch either way.

Since there's at least two series out there trying to fiddle with
-cros-common.dtsi, including one that drops the slot sub-node of the MMC
and one that adds CPU power supply, it would be nice if we can get the
-cros-common parts sorted out and applied. Can you ack/review 1-2?
Should I squash them in a v3?

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Javier Martinez Canillas]

Hello Andreas,

On Tue, Jul 29, 2014 at 3:00 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Javier and Doug,
>
>
> Hm, in different context it was stated that device trees shouldn't rely
> on bootloader behavior (for a /memory node on Zynq we ended up
> specifying the size cell with the real value despite U-Boot overriding
> it to a lesser value).
>

Well is not strictly the same, the memory node is a hardware
description while the chosen node is one of those exceptional nodes
that don't represent a real device. Also the usage for the memory node
is marked as required in ePAR while the later is marked as optional.

> Can we instead settle on filling this node with defaults? :)
> bootargs = "console=tty1"; would be my choice for Spring. Would that be
> applicable for Snow as well?
> Not sure how to specify that via linux,stdout-path property.
>
> I believe this would be a follow-up patch either way.
>
> Since there's at least two series out there trying to fiddle with
> -cros-common.dtsi, including one that drops the slot sub-node of the MMC
> and one that adds CPU power supply, it would be nice if we can get the
> -cros-common parts sorted out and applied. Can you ack/review 1-2?
> Should I squash them in a v3?
>

A sensible default makes sense to me and yes it should be a follow-up
patch anyways.

> Thanks,
> Andreas
>
> --

Best regards,
Javier

Re: [PATCH v2 3/4] ARM: dts: exynos5250: Fold common ChromeOS parts into Snow

[Doug Anderson]

Andreas,

On Tue, Jul 29, 2014 at 6:00 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hi Javier and Doug,
>
> Am 25.07.2014 19:02, schrieb Javier Martinez Canillas:
>> On Fri, Jul 25, 2014 at 6:43 PM, Doug Anderson <dianders@chromium.org> wrote:
>>> On Fri, Jul 25, 2014 at 9:35 AM, Javier Martinez Canillas
>>> <javier@dowhile0.org> wrote:
>>>> Hello Andreas,
>>>>
>>>> On Fri, Jul 18, 2014 at 7:20 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>>>
>>>>> +       memory {
>>>>> +               reg = <0x40000000 0x80000000>;
>>>>> +       };
>>>>> +
>>>>> +       chosen {
>>>>> +       };
>>>>> +
>>>>
>>>> Is there a reason for an empty chosen node? Same for the Spring DTS.
>>>
>>> I know that the bootloader (U-Boot) fills this in.  I'd have to double
>>> check how the bootloader handles the node not being there to begin
>>> with.
>>
>> Yes, U-Boot fills this but if the node does not exist in the FDT, it
>> creates one. Take a look at fdt_chosen() in common/fdt_support.c [0].
>>
>> So I think it's better to just remove it since is empty.
>
> Hm, in different context it was stated that device trees shouldn't rely
> on bootloader behavior (for a /memory node on Zynq we ended up
> specifying the size cell with the real value despite U-Boot overriding
> it to a lesser value).
>
> Can we instead settle on filling this node with defaults? :)
> bootargs = "console=tty1"; would be my choice for Spring. Would that be
> applicable for Snow as well?

Personally I think that there's not a lot of use for including
bootargs.  Every single person will need different bootargs since
everyone will have a different boot source (eMMC partitioin 0, eMMC
partition3, SD card, USB, etc).  You have to rely on the bootloader
for _something_.


> Not sure how to specify that via linux,stdout-path property.
>
> I believe this would be a follow-up patch either way.

Right


> Since there's at least two series out there trying to fiddle with
> -cros-common.dtsi, including one that drops the slot sub-node of the MMC
> and one that adds CPU power supply, it would be nice if we can get the
> -cros-common parts sorted out and applied. Can you ack/review 1-2?
> Should I squash them in a v3?

Yeah, it will be a fight to see who gets landed first...  :-P

I was thinking you were going to spin the patches and I'd do another
batch of review, but I think I can mark the first two as Reveiwed-by
if it's helpful to you.  Let me do one more double-check when I get to
my desk.

-Doug

[PATCH v2 4/4] ARM: dts: exynos5250: Add Spring device tree

[Andreas Färber]

Adds initial support for the HP Chromebook 11.

Cc: Vincent Palatin <vpalatin@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Stephan van Schaik <stephan@synkhronix.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 v1 -> v2:
 * Use label-based overriding/extension of nodes. (Doug Anderson)
 * Dropped tps65090 for now, until we know where to place it.
 * Dropped non-Spring nodes from -cros-common.dtsi rather than disabling them.
 * Enabled a missing MMC node for access to internal storage.
 * Dropped display-timings from dp-controller node. (Ajay Kumar)

 arch/arm/boot/dts/Makefile              |   1 +
 arch/arm/boot/dts/exynos5250-spring.dts | 485 ++++++++++++++++++++++++++++++++
 2 files changed, 486 insertions(+)
 create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 80a781f..dec4c29 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -76,6 +76,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
 	exynos5250-arndale.dtb \
 	exynos5250-smdk5250.dtb \
 	exynos5250-snow.dtb \
+	exynos5250-spring.dtb \
 	exynos5260-xyref5260.dtb \
 	exynos5410-smdk5410.dtb \
 	exynos5420-arndale-octa.dtb \
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
new file mode 100644
index 0000000..6695973
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -0,0 +1,485 @@
+/*
+ * Google Spring board device tree source
+ *
+ * Copyright (c) 2013 Google, Inc
+ * Copyright (c) 2014 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/dts-v1/;
+#include "exynos5250.dtsi"
+
+/ {
+	model = "Google Spring";
+	compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5";
+
+	memory {
+		reg = <0x40000000 0x80000000>;
+	};
+
+	chosen {
+	};
+
+	usb@12000000 {
+		status = "okay";
+	};
+
+	usb3_vbus_reg: regulator-usb3 {
+		compatible = "regulator-fixed";
+		regulator-name = "P5.0V_USB3CON";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&gpe1 0 1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hsic_reset>;
+		enable-active-high;
+	};
+
+	usb@12110000 {
+		samsung,vbus-gpio = <&gpx1 1 0>;
+		status = "okay";
+	};
+
+	usb@12120000 {
+		status = "okay";
+	};
+
+	fimd@14400000 {
+		status = "okay";
+		samsung,invert-vclk;
+	};
+
+	hdmi {
+		hpd-gpio = <&gpx3 7 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hdmi_hpd_irq>;
+		phy = <&hdmiphy>;
+		ddc = <&i2c_2>;
+		hdmi-en-supply = <&s5m_ldo8_reg>;
+		vdd-supply = <&s5m_ldo8_reg>;
+		vdd_osc-supply = <&s5m_ldo10_reg>;
+		vdd_pll-supply = <&s5m_ldo8_reg>;
+	};
+
+	dp-controller@145B0000 {
+		status = "okay";
+		pinctrl-names = "default";
+		pinctrl-0 = <&dp_hpd>;
+		samsung,color-space = <0>;
+		samsung,dynamic-range = <0>;
+		samsung,ycbcr-coeff = <0>;
+		samsung,color-depth = <1>;
+		samsung,link-rate = <0x0a>;
+		samsung,lane-count = <1>;
+		samsung,hpd-gpio = <&gpc3 0 0>;
+	};
+
+	fixed-rate-clocks {
+		xxti {
+			compatible = "samsung,clock-xxti";
+			clock-frequency = <24000000>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		power {
+			label = "Power";
+			gpios = <&gpx1 3 1>;
+			linux,code = <116>; /* KEY_POWER */
+			gpio-key,wakeup;
+		};
+	};
+};
+
+&i2c_0 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+
+	s5m8767_pmic@66 {
+		compatible = "samsung,s5m8767-pmic";
+		reg = <0x66>;
+		interrupt-parent = <&gpx3>;
+		interrupts = <2 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>;
+		wakeup-source;
+
+		s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */
+		                              <&gpd1 1 1>, /* DVS2 */
+		                              <&gpd1 2 1>; /* DVS3 */
+
+		s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */
+		                             <&gpx2 4 1>, /* SET2 */
+		                             <&gpx2 5 1>; /* SET3 */
+
+		/*
+		 * The following arrays of DVS voltages are not used, since we are
+		 * not using GPIOs to control PMIC bucks, but they must be defined
+		 * to please the driver.
+		 */
+		s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
+		                                 <1250000>, <1200000>,
+		                                 <1150000>, <1100000>,
+		                                 <1000000>, <950000>;
+
+		s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
+		                                 <1100000>, <1100000>,
+		                                 <1000000>, <1000000>,
+		                                 <1000000>, <1000000>;
+
+		s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
+		                                 <1200000>, <1200000>,
+		                                 <1200000>, <1200000>,
+		                                 <1200000>, <1200000>;
+
+		clocks {
+			compatible = "samsung,s5m8767-clk";
+			#clock-cells = <1>;
+			clock-output-names = "en32khz_ap",
+			                     "en32khz_cp",
+			                     "en32khz_bt";
+		};
+
+		regulators {
+			s5m_ldo4_reg: LDO4 {
+				regulator-name = "P1.0V_LDO_OUT4";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <0>;
+			};
+
+			s5m_ldo5_reg: LDO5 {
+				regulator-name = "P1.0V_LDO_OUT5";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <0>;
+			};
+
+			s5m_ldo6_reg: LDO6 {
+				regulator-name = "vdd_mydp";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo7_reg: LDO7 {
+				regulator-name = "P1.1V_LDO_OUT7";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo8_reg: LDO8 {
+				regulator-name = "P1.0V_LDO_OUT8";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo10_reg: LDO10 {
+				regulator-name = "P1.8V_LDO_OUT10";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo11_reg: LDO11 {
+				regulator-name = "P1.8V_LDO_OUT11";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				op_mode = <0>;
+			};
+
+			s5m_ldo12_reg: LDO12 {
+				regulator-name = "P3.0V_LDO_OUT12";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo13_reg: LDO13 {
+				regulator-name = "P1.8V_LDO_OUT13";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				op_mode = <0>;
+			};
+
+			s5m_ldo14_reg: LDO14 {
+				regulator-name = "P1.8V_LDO_OUT14";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo15_reg: LDO15 {
+				regulator-name = "P1.0V_LDO_OUT15";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo16_reg: LDO16 {
+				regulator-name = "P1.8V_LDO_OUT16";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				op_mode = <3>;
+			};
+
+			s5m_ldo17_reg: LDO17 {
+				regulator-name = "P2.8V_LDO_OUT17";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-always-on;
+				op_mode = <0>;
+			};
+
+			s5m_ldo25_reg: LDO25 {
+				regulator-name = "vdd_bridge";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				op_mode = <1>;
+			};
+
+			BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <3>;
+			};
+
+			BUCK2 {
+				regulator-name = "vdd_arm";
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <3>;
+			};
+
+			BUCK3 {
+				regulator-name = "vdd_int";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <3>;
+			};
+
+			BUCK4 {
+				regulator-name = "vdd_g3d";
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-boot-on;
+				op_mode = <3>;
+			};
+
+			BUCK5 {
+				regulator-name = "P1.8V_BUCK_OUT5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <1>;
+			};
+
+			BUCK6 {
+				regulator-name = "P1.2V_BUCK_OUT6";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <0>;
+			};
+
+			BUCK9 {
+				regulator-name = "vdd_ummc";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-always-on;
+				regulator-boot-on;
+				op_mode = <3>;
+			};
+		};
+	};
+};
+
+&i2c_1 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+};
+
+&i2c_2 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+
+	hdmiddc@50 {
+		compatible = "samsung,exynos4210-hdmiddc";
+		reg = <0x50>;
+	};
+};
+
+&i2c_3 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_4 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+
+	cros_ec: embedded-controller {
+		compatible = "google,cros-ec-i2c";
+		reg = <0x1e>;
+		interrupts = <6 0>;
+		interrupt-parent = <&gpx1>;
+		wakeup-source;
+	};
+};
+
+&i2c_5 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_7 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <66000>;
+};
+
+&i2c_8 {
+	status = "okay";
+	samsung,i2c-sda-delay = <100>;
+	samsung,i2c-max-bus-freq = <378000>;
+
+	hdmiphy: hdmiphy@38 {
+		compatible = "samsung,exynos4212-hdmiphy";
+		reg = <0x38>;
+	};
+};
+
+&mmc_0 {
+	status = "okay";
+	num-slots = <1>;
+	supports-highspeed;
+	broken-cd;
+	card-detect-delay = <200>;
+	samsung,dw-mshc-ciu-div = <3>;
+	samsung,dw-mshc-sdr-timing = <2 3>;
+	samsung,dw-mshc-ddr-timing = <1 2>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
+
+	slot@0 {
+		reg = <0>;
+		bus-width = <8>;
+	};
+};
+
+&mmc_1 {
+	status = "okay";
+	num-slots = <1>;
+	supports-highspeed;
+	broken-cd;
+	card-detect-delay = <200>;
+	samsung,dw-mshc-ciu-div = <3>;
+	samsung,dw-mshc-sdr-timing = <2 3>;
+	samsung,dw-mshc-ddr-timing = <1 2>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_cd &sd1_bus4>;
+
+	slot@0 {
+		reg = <0>;
+		bus-width = <4>;
+	};
+};
+
+&mmc_2 {
+	/* MMC2 pins are used as GPIO for eDP bridge control. */
+	status = "disabled";
+};
+
+&pinctrl_0 {
+	/*
+	 * Disabled pullups since external part has its own pullups and
+	 * double-pulling gets us out of spec in some cases.
+	 */
+	i2c2_bus: i2c2-bus {
+		samsung,pin-pud = <0>;
+	};
+
+	s5m8767_dvs: s5m8767-dvs {
+		samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <1>;
+		samsung,pin-drv = <0>;
+	};
+
+	s5m8767_ds: s5m8767-ds {
+		samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <1>;
+		samsung,pin-drv = <0>;
+	};
+
+	s5m8767_irq: s5m8767-irq {
+		samsung,pins = "gpx3-2";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+
+	hdmi_hpd_irq: hdmi-hpd-irq {
+		samsung,pins = "gpx3-7";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <1>;
+		samsung,pin-drv = <0>;
+	};
+};
+
+&pinctrl_1 {
+	hsic_reset: hsic-reset {
+		samsung,pins = "gpe1-0";
+		samsung,pin-function = <1>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+};
+
+&spi_1 {
+	status = "okay";
+	samsung,spi-src-clk = <0>;
+	num-cs = <1>;
+};
+
+&usbdrd_phy {
+	vbus-supply = <&usb3_vbus_reg>;
+};
+
+#include "cros-ec-keyboard.dtsi"
-- 
1.9.3

Re: [PATCH v2 4/4] ARM: dts: exynos5250: Add Spring device tree

[Doug Anderson]

Andreas,

On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
> Adds initial support for the HP Chromebook 11.
>
> Cc: Vincent Palatin <vpalatin@chromium.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Stephan van Schaik <stephan@synkhronix.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  v1 -> v2:
>  * Use label-based overriding/extension of nodes. (Doug Anderson)
>  * Dropped tps65090 for now, until we know where to place it.
>  * Dropped non-Spring nodes from -cros-common.dtsi rather than disabling them.
>  * Enabled a missing MMC node for access to internal storage.
>  * Dropped display-timings from dp-controller node. (Ajay Kumar)
>
>  arch/arm/boot/dts/Makefile              |   1 +
>  arch/arm/boot/dts/exynos5250-spring.dts | 485 ++++++++++++++++++++++++++++++++
>  2 files changed, 486 insertions(+)
>  create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 80a781f..dec4c29 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -76,6 +76,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
>         exynos5250-arndale.dtb \
>         exynos5250-smdk5250.dtb \
>         exynos5250-snow.dtb \
> +       exynos5250-spring.dtb \
>         exynos5260-xyref5260.dtb \
>         exynos5410-smdk5410.dtb \
>         exynos5420-arndale-octa.dtb \
> diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
> new file mode 100644
> index 0000000..6695973
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos5250-spring.dts
> @@ -0,0 +1,485 @@
> +/*
> + * Google Spring board device tree source
> + *
> + * Copyright (c) 2013 Google, Inc
> + * Copyright (c) 2014 SUSE LINUX Products GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/dts-v1/;
> +#include "exynos5250.dtsi"
> +
> +/ {
> +       model = "Google Spring";
> +       compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5";
> +
> +       memory {
> +               reg = <0x40000000 0x80000000>;
> +       };
> +
> +       chosen {
> +       };
> +
> +       usb@12000000 {
> +               status = "okay";
> +       };
> +
> +       usb3_vbus_reg: regulator-usb3 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "P5.0V_USB3CON";
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +               gpio = <&gpe1 0 1>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&hsic_reset>;
> +               enable-active-high;
> +       };
> +
> +       usb@12110000 {
> +               samsung,vbus-gpio = <&gpx1 1 0>;
> +               status = "okay";
> +       };
> +
> +       usb@12120000 {
> +               status = "okay";
> +       };
> +
> +       fimd@14400000 {
> +               status = "okay";
> +               samsung,invert-vclk;
> +       };
> +
> +       hdmi {
> +               hpd-gpio = <&gpx3 7 0>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&hdmi_hpd_irq>;
> +               phy = <&hdmiphy>;
> +               ddc = <&i2c_2>;
> +               hdmi-en-supply = <&s5m_ldo8_reg>;
> +               vdd-supply = <&s5m_ldo8_reg>;
> +               vdd_osc-supply = <&s5m_ldo10_reg>;
> +               vdd_pll-supply = <&s5m_ldo8_reg>;
> +       };
> +
> +       dp-controller@145B0000 {
> +               status = "okay";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&dp_hpd>;
> +               samsung,color-space = <0>;
> +               samsung,dynamic-range = <0>;
> +               samsung,ycbcr-coeff = <0>;
> +               samsung,color-depth = <1>;
> +               samsung,link-rate = <0x0a>;
> +               samsung,lane-count = <1>;
> +               samsung,hpd-gpio = <&gpc3 0 0>;
> +       };
> +
> +       fixed-rate-clocks {
> +               xxti {
> +                       compatible = "samsung,clock-xxti";
> +                       clock-frequency = <24000000>;
> +               };
> +       };
> +
> +       gpio-keys {
> +               compatible = "gpio-keys";
> +
> +               power {
> +                       label = "Power";
> +                       gpios = <&gpx1 3 1>;
> +                       linux,code = <116>; /* KEY_POWER */
> +                       gpio-key,wakeup;
> +               };

Is the lid switch not working?

Where is your pinctrl?

Can you match this in sort order with snow to make diffs easier?


> +       };
> +};
> +
> +&i2c_0 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +
> +       s5m8767_pmic@66 {
> +               compatible = "samsung,s5m8767-pmic";
> +               reg = <0x66>;
> +               interrupt-parent = <&gpx3>;
> +               interrupts = <2 0>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>;
> +               wakeup-source;
> +
> +               s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */
> +                                             <&gpd1 1 1>, /* DVS2 */
> +                                             <&gpd1 2 1>; /* DVS3 */
> +
> +               s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */
> +                                            <&gpx2 4 1>, /* SET2 */
> +                                            <&gpx2 5 1>; /* SET3 */
> +
> +               /*
> +                * The following arrays of DVS voltages are not used, since we are
> +                * not using GPIOs to control PMIC bucks, but they must be defined
> +                * to please the driver.
> +                */
> +               s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
> +                                                <1250000>, <1200000>,
> +                                                <1150000>, <1100000>,
> +                                                <1000000>, <950000>;
> +
> +               s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
> +                                                <1100000>, <1100000>,
> +                                                <1000000>, <1000000>,
> +                                                <1000000>, <1000000>;
> +
> +               s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
> +                                                <1200000>, <1200000>,
> +                                                <1200000>, <1200000>,
> +                                                <1200000>, <1200000>;
> +
> +               clocks {
> +                       compatible = "samsung,s5m8767-clk";
> +                       #clock-cells = <1>;
> +                       clock-output-names = "en32khz_ap",
> +                                            "en32khz_cp",
> +                                            "en32khz_bt";
> +               };
> +
> +               regulators {
> +                       s5m_ldo4_reg: LDO4 {
> +                               regulator-name = "P1.0V_LDO_OUT4";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       s5m_ldo5_reg: LDO5 {
> +                               regulator-name = "P1.0V_LDO_OUT5";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       s5m_ldo6_reg: LDO6 {
> +                               regulator-name = "vdd_mydp";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo7_reg: LDO7 {
> +                               regulator-name = "P1.1V_LDO_OUT7";
> +                               regulator-min-microvolt = <1100000>;
> +                               regulator-max-microvolt = <1100000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo8_reg: LDO8 {
> +                               regulator-name = "P1.0V_LDO_OUT8";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo10_reg: LDO10 {
> +                               regulator-name = "P1.8V_LDO_OUT10";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo11_reg: LDO11 {
> +                               regulator-name = "P1.8V_LDO_OUT11";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       s5m_ldo12_reg: LDO12 {
> +                               regulator-name = "P3.0V_LDO_OUT12";
> +                               regulator-min-microvolt = <3000000>;
> +                               regulator-max-microvolt = <3000000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo13_reg: LDO13 {
> +                               regulator-name = "P1.8V_LDO_OUT13";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       s5m_ldo14_reg: LDO14 {
> +                               regulator-name = "P1.8V_LDO_OUT14";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo15_reg: LDO15 {
> +                               regulator-name = "P1.0V_LDO_OUT15";
> +                               regulator-min-microvolt = <1000000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo16_reg: LDO16 {
> +                               regulator-name = "P1.8V_LDO_OUT16";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       s5m_ldo17_reg: LDO17 {
> +                               regulator-name = "P2.8V_LDO_OUT17";
> +                               regulator-min-microvolt = <2800000>;
> +                               regulator-max-microvolt = <2800000>;
> +                               regulator-always-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       s5m_ldo25_reg: LDO25 {
> +                               regulator-name = "vdd_bridge";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               op_mode = <1>;
> +                       };
> +
> +                       BUCK1 {
> +                               regulator-name = "vdd_mif";
> +                               regulator-min-microvolt = <950000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       BUCK2 {
> +                               regulator-name = "vdd_arm";
> +                               regulator-min-microvolt = <850000>;
> +                               regulator-max-microvolt = <1350000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       BUCK3 {
> +                               regulator-name = "vdd_int";
> +                               regulator-min-microvolt = <900000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       BUCK4 {
> +                               regulator-name = "vdd_g3d";
> +                               regulator-min-microvolt = <850000>;
> +                               regulator-max-microvolt = <1300000>;
> +                               regulator-boot-on;
> +                               op_mode = <3>;
> +                       };
> +
> +                       BUCK5 {
> +                               regulator-name = "P1.8V_BUCK_OUT5";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <1800000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <1>;
> +                       };
> +
> +                       BUCK6 {
> +                               regulator-name = "P1.2V_BUCK_OUT6";
> +                               regulator-min-microvolt = <1200000>;
> +                               regulator-max-microvolt = <1200000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <0>;
> +                       };
> +
> +                       BUCK9 {
> +                               regulator-name = "vdd_ummc";
> +                               regulator-min-microvolt = <950000>;
> +                               regulator-max-microvolt = <3000000>;
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               op_mode = <3>;
> +                       };
> +               };
> +       };
> +};
> +
> +&i2c_1 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +};
> +
> +&i2c_2 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +
> +       hdmiddc@50 {
> +               compatible = "samsung,exynos4210-hdmiddc";
> +               reg = <0x50>;
> +       };
> +};
> +
> +&i2c_3 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_4 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +
> +       cros_ec: embedded-controller {
> +               compatible = "google,cros-ec-i2c";
> +               reg = <0x1e>;
> +               interrupts = <6 0>;
> +               interrupt-parent = <&gpx1>;

I think you want a pinctrl for gpx1-6, like Snow's "ec_irq".


> +               wakeup-source;
> +       };
> +};
> +
> +&i2c_5 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_7 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <66000>;
> +};
> +
> +&i2c_8 {
> +       status = "okay";
> +       samsung,i2c-sda-delay = <100>;
> +       samsung,i2c-max-bus-freq = <378000>;
> +
> +       hdmiphy: hdmiphy@38 {
> +               compatible = "samsung,exynos4212-hdmiphy";
> +               reg = <0x38>;
> +       };
> +};
> +
> +&mmc_0 {
> +       status = "okay";
> +       num-slots = <1>;
> +       supports-highspeed;
> +       broken-cd;
> +       card-detect-delay = <200>;
> +       samsung,dw-mshc-ciu-div = <3>;
> +       samsung,dw-mshc-sdr-timing = <2 3>;
> +       samsung,dw-mshc-ddr-timing = <1 2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
> +
> +       slot@0 {
> +               reg = <0>;
> +               bus-width = <8>;
> +       };
> +};
> +
> +&mmc_1 {
> +       status = "okay";
> +       num-slots = <1>;
> +       supports-highspeed;
> +       broken-cd;
> +       card-detect-delay = <200>;
> +       samsung,dw-mshc-ciu-div = <3>;
> +       samsung,dw-mshc-sdr-timing = <2 3>;
> +       samsung,dw-mshc-ddr-timing = <1 2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_cd &sd1_bus4>;
> +
> +       slot@0 {
> +               reg = <0>;
> +               bus-width = <4>;
> +       };
> +};

mmc_1 on Spring matches with mmc_3 on Snow.  That means you need to
make sure you've got the reduced drive strengths, too.  I don't think
your device tree has these.  Specifically you need local overrides of
sd1_clk, sd1_cmd, etc to lower drive strengths.


> +
> +&mmc_2 {
> +       /* MMC2 pins are used as GPIO for eDP bridge control. */
> +       status = "disabled";
> +};

Remove the mmc_2 reference.  It's disabled in exynos5250.dtsi already.


> +
> +&pinctrl_0 {
> +       /*
> +        * Disabled pullups since external part has its own pullups and
> +        * double-pulling gets us out of spec in some cases.
> +        */
> +       i2c2_bus: i2c2-bus {
> +               samsung,pin-pud = <0>;
> +       };
> +
> +       s5m8767_dvs: s5m8767-dvs {
> +               samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2";
> +               samsung,pin-function = <0>;
> +               samsung,pin-pud = <1>;
> +               samsung,pin-drv = <0>;
> +       };
> +
> +       s5m8767_ds: s5m8767-ds {
> +               samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5";
> +               samsung,pin-function = <0>;
> +               samsung,pin-pud = <1>;
> +               samsung,pin-drv = <0>;
> +       };
> +
> +       s5m8767_irq: s5m8767-irq {
> +               samsung,pins = "gpx3-2";
> +               samsung,pin-function = <0>;
> +               samsung,pin-pud = <0>;
> +               samsung,pin-drv = <0>;
> +       };
> +
> +       hdmi_hpd_irq: hdmi-hpd-irq {
> +               samsung,pins = "gpx3-7";
> +               samsung,pin-function = <0>;
> +               samsung,pin-pud = <1>;
> +               samsung,pin-drv = <0>;
> +       };

Ideally you could do one more cleanup patch to snow first to make
diffs easier between the two dts files.  Moving things in the snow
file from "pinctrl@11400000" to the pinctrl_0 section would help.
...and other cleanups you could do to snow to make diffs similar would
also be really great.


> +};
> +
> +&pinctrl_1 {
> +       hsic_reset: hsic-reset {
> +               samsung,pins = "gpe1-0";
> +               samsung,pin-function = <1>;
> +               samsung,pin-pud = <0>;
> +               samsung,pin-drv = <0>;
> +       };
> +};
> +
> +&spi_1 {
> +       status = "okay";
> +       samsung,spi-src-clk = <0>;
> +       num-cs = <1>;
> +};
> +
> +&usbdrd_phy {
> +       vbus-supply = <&usb3_vbus_reg>;
> +};
> +
> +#include "cros-ec-keyboard.dtsi"
> --
> 1.9.3
>

Re: [PATCH v2 4/4] ARM: dts: exynos5250: Add Spring device tree

[Andreas Färber]

Hi Doug,

Am 25.07.2014 18:02, schrieb Doug Anderson:
> Andreas,
> 
> On Fri, Jul 18, 2014 at 10:20 AM, Andreas Färber <afaerber@suse.de> wrote:
>> +       gpio-keys {
>> +               compatible = "gpio-keys";
>> +
>> +               power {
>> +                       label = "Power";
>> +                       gpios = <&gpx1 3 1>;
>> +                       linux,code = <116>; /* KEY_POWER */
>> +                       gpio-key,wakeup;
>> +               };
> 
> Is the lid switch not working?

No, I don't see any reaction at least. I notice that 3.8 has pinctrls on
gpio-keys directly though...

> Where is your pinctrl?

Maybe that's what you mean here?

> Can you match this in sort order with snow to make diffs easier?

Done.

>> +       };
>> +};
>> +
>> +&i2c_0 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <378000>;
>> +
>> +       s5m8767_pmic@66 {
>> +               compatible = "samsung,s5m8767-pmic";
>> +               reg = <0x66>;
>> +               interrupt-parent = <&gpx3>;
>> +               interrupts = <2 0>;
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&s5m8767_irq &s5m8767_dvs &s5m8767_ds>;
>> +               wakeup-source;
>> +
>> +               s5m8767,pmic-buck-dvs-gpios = <&gpd1 0 1>, /* DVS1 */
>> +                                             <&gpd1 1 1>, /* DVS2 */
>> +                                             <&gpd1 2 1>; /* DVS3 */
>> +
>> +               s5m8767,pmic-buck-ds-gpios = <&gpx2 3 1>, /* SET1 */
>> +                                            <&gpx2 4 1>, /* SET2 */
>> +                                            <&gpx2 5 1>; /* SET3 */
>> +
>> +               /*
>> +                * The following arrays of DVS voltages are not used, since we are
>> +                * not using GPIOs to control PMIC bucks, but they must be defined
>> +                * to please the driver.
>> +                */
>> +               s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
>> +                                                <1250000>, <1200000>,
>> +                                                <1150000>, <1100000>,
>> +                                                <1000000>, <950000>;
>> +
>> +               s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
>> +                                                <1100000>, <1100000>,
>> +                                                <1000000>, <1000000>,
>> +                                                <1000000>, <1000000>;
>> +
>> +               s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
>> +                                                <1200000>, <1200000>,
>> +                                                <1200000>, <1200000>,
>> +                                                <1200000>, <1200000>;
>> +
>> +               clocks {
>> +                       compatible = "samsung,s5m8767-clk";
>> +                       #clock-cells = <1>;
>> +                       clock-output-names = "en32khz_ap",
>> +                                            "en32khz_cp",
>> +                                            "en32khz_bt";
>> +               };
>> +
>> +               regulators {
>> +                       s5m_ldo4_reg: LDO4 {
>> +                               regulator-name = "P1.0V_LDO_OUT4";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       s5m_ldo5_reg: LDO5 {
>> +                               regulator-name = "P1.0V_LDO_OUT5";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       s5m_ldo6_reg: LDO6 {
>> +                               regulator-name = "vdd_mydp";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo7_reg: LDO7 {
>> +                               regulator-name = "P1.1V_LDO_OUT7";
>> +                               regulator-min-microvolt = <1100000>;
>> +                               regulator-max-microvolt = <1100000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo8_reg: LDO8 {
>> +                               regulator-name = "P1.0V_LDO_OUT8";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo10_reg: LDO10 {
>> +                               regulator-name = "P1.8V_LDO_OUT10";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo11_reg: LDO11 {
>> +                               regulator-name = "P1.8V_LDO_OUT11";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       s5m_ldo12_reg: LDO12 {
>> +                               regulator-name = "P3.0V_LDO_OUT12";
>> +                               regulator-min-microvolt = <3000000>;
>> +                               regulator-max-microvolt = <3000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo13_reg: LDO13 {
>> +                               regulator-name = "P1.8V_LDO_OUT13";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       s5m_ldo14_reg: LDO14 {
>> +                               regulator-name = "P1.8V_LDO_OUT14";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo15_reg: LDO15 {
>> +                               regulator-name = "P1.0V_LDO_OUT15";
>> +                               regulator-min-microvolt = <1000000>;
>> +                               regulator-max-microvolt = <1000000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo16_reg: LDO16 {
>> +                               regulator-name = "P1.8V_LDO_OUT16";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       s5m_ldo17_reg: LDO17 {
>> +                               regulator-name = "P2.8V_LDO_OUT17";
>> +                               regulator-min-microvolt = <2800000>;
>> +                               regulator-max-microvolt = <2800000>;
>> +                               regulator-always-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       s5m_ldo25_reg: LDO25 {
>> +                               regulator-name = "vdd_bridge";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               op_mode = <1>;
>> +                       };
>> +
>> +                       BUCK1 {
>> +                               regulator-name = "vdd_mif";
>> +                               regulator-min-microvolt = <950000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       BUCK2 {
>> +                               regulator-name = "vdd_arm";
>> +                               regulator-min-microvolt = <850000>;
>> +                               regulator-max-microvolt = <1350000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       BUCK3 {
>> +                               regulator-name = "vdd_int";
>> +                               regulator-min-microvolt = <900000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       BUCK4 {
>> +                               regulator-name = "vdd_g3d";
>> +                               regulator-min-microvolt = <850000>;
>> +                               regulator-max-microvolt = <1300000>;
>> +                               regulator-boot-on;
>> +                               op_mode = <3>;
>> +                       };
>> +
>> +                       BUCK5 {
>> +                               regulator-name = "P1.8V_BUCK_OUT5";
>> +                               regulator-min-microvolt = <1800000>;
>> +                               regulator-max-microvolt = <1800000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <1>;
>> +                       };
>> +
>> +                       BUCK6 {
>> +                               regulator-name = "P1.2V_BUCK_OUT6";
>> +                               regulator-min-microvolt = <1200000>;
>> +                               regulator-max-microvolt = <1200000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <0>;
>> +                       };
>> +
>> +                       BUCK9 {
>> +                               regulator-name = "vdd_ummc";
>> +                               regulator-min-microvolt = <950000>;
>> +                               regulator-max-microvolt = <3000000>;
>> +                               regulator-always-on;
>> +                               regulator-boot-on;
>> +                               op_mode = <3>;
>> +                       };
>> +               };
>> +       };
>> +};
>> +
>> +&i2c_1 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <378000>;
>> +};
>> +
>> +&i2c_2 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <66000>;
>> +
>> +       hdmiddc@50 {
>> +               compatible = "samsung,exynos4210-hdmiddc";
>> +               reg = <0x50>;
>> +       };
>> +};
>> +
>> +&i2c_3 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <66000>;
>> +};
>> +
>> +&i2c_4 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <66000>;
>> +
>> +       cros_ec: embedded-controller {
>> +               compatible = "google,cros-ec-i2c";
>> +               reg = <0x1e>;
>> +               interrupts = <6 0>;
>> +               interrupt-parent = <&gpx1>;
> 
> I think you want a pinctrl for gpx1-6, like Snow's "ec_irq".

Done. On v1 you had asked about a pinctrl for the keyboard, which has
none, but cros_ec itself does in fact.

>> +               wakeup-source;
>> +       };
>> +};
>> +
>> +&i2c_5 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <66000>;
>> +};
>> +
>> +&i2c_7 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <66000>;
>> +};
>> +
>> +&i2c_8 {
>> +       status = "okay";
>> +       samsung,i2c-sda-delay = <100>;
>> +       samsung,i2c-max-bus-freq = <378000>;
>> +
>> +       hdmiphy: hdmiphy@38 {
>> +               compatible = "samsung,exynos4212-hdmiphy";
>> +               reg = <0x38>;
>> +       };
>> +};
>> +
>> +&mmc_0 {
>> +       status = "okay";
>> +       num-slots = <1>;
>> +       supports-highspeed;
>> +       broken-cd;
>> +       card-detect-delay = <200>;
>> +       samsung,dw-mshc-ciu-div = <3>;
>> +       samsung,dw-mshc-sdr-timing = <2 3>;
>> +       samsung,dw-mshc-ddr-timing = <1 2>;
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_cd &sd0_bus4 &sd0_bus8>;
>> +
>> +       slot@0 {
>> +               reg = <0>;
>> +               bus-width = <8>;
>> +       };
>> +};
>> +
>> +&mmc_1 {
>> +       status = "okay";
>> +       num-slots = <1>;
>> +       supports-highspeed;
>> +       broken-cd;
>> +       card-detect-delay = <200>;
>> +       samsung,dw-mshc-ciu-div = <3>;
>> +       samsung,dw-mshc-sdr-timing = <2 3>;
>> +       samsung,dw-mshc-ddr-timing = <1 2>;
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_cd &sd1_bus4>;
>> +
>> +       slot@0 {
>> +               reg = <0>;
>> +               bus-width = <4>;
>> +       };
>> +};
> 
> mmc_1 on Spring matches with mmc_3 on Snow.  That means you need to
> make sure you've got the reduced drive strengths, too.  I don't think
> your device tree has these.  Specifically you need local overrides of
> sd1_clk, sd1_cmd, etc to lower drive strengths.

Done. Wasn't sure about _cd, so I included it.

>> +
>> +&mmc_2 {
>> +       /* MMC2 pins are used as GPIO for eDP bridge control. */
>> +       status = "disabled";
>> +};
> 
> Remove the mmc_2 reference.  It's disabled in exynos5250.dtsi already.

Done. That drops the documentation of why, too, though.

>> +
>> +&pinctrl_0 {
>> +       /*
>> +        * Disabled pullups since external part has its own pullups and
>> +        * double-pulling gets us out of spec in some cases.
>> +        */
>> +       i2c2_bus: i2c2-bus {
>> +               samsung,pin-pud = <0>;
>> +       };
>> +
>> +       s5m8767_dvs: s5m8767-dvs {
>> +               samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2";
>> +               samsung,pin-function = <0>;
>> +               samsung,pin-pud = <1>;
>> +               samsung,pin-drv = <0>;
>> +       };
>> +
>> +       s5m8767_ds: s5m8767-ds {
>> +               samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5";
>> +               samsung,pin-function = <0>;
>> +               samsung,pin-pud = <1>;
>> +               samsung,pin-drv = <0>;
>> +       };
>> +
>> +       s5m8767_irq: s5m8767-irq {
>> +               samsung,pins = "gpx3-2";
>> +               samsung,pin-function = <0>;
>> +               samsung,pin-pud = <0>;
>> +               samsung,pin-drv = <0>;
>> +       };
>> +
>> +       hdmi_hpd_irq: hdmi-hpd-irq {
>> +               samsung,pins = "gpx3-7";
>> +               samsung,pin-function = <0>;
>> +               samsung,pin-pud = <1>;
>> +               samsung,pin-drv = <0>;
>> +       };
> 
> Ideally you could do one more cleanup patch to snow first to make
> diffs easier between the two dts files.  Moving things in the snow
> file from "pinctrl@11400000" to the pinctrl_0 section would help.
> ...and other cleanups you could do to snow to make diffs similar would
> also be really great.

Done.

>> +};
>> +
>> +&pinctrl_1 {
>> +       hsic_reset: hsic-reset {
>> +               samsung,pins = "gpe1-0";
>> +               samsung,pin-function = <1>;
>> +               samsung,pin-pud = <0>;
>> +               samsung,pin-drv = <0>;
>> +       };
>> +};
>> +
>> +&spi_1 {
>> +       status = "okay";
>> +       samsung,spi-src-clk = <0>;
>> +       num-cs = <1>;
>> +};
>> +
>> +&usbdrd_phy {
>> +       vbus-supply = <&usb3_vbus_reg>;
>> +};
>> +
>> +#include "cros-ec-keyboard.dtsi"
>> --
>> 1.9.3

Also found some unresolved review comments from v1 that I incorporated.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
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