[PATCH 1/3] ARM: dts: Disable Exynos5250 I2S controllers by default

[PATCH 1/3] ARM: dts: Disable Exynos5250 I2S controllers by default

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Rather than requiring each board to explicitly disable the I2S controllers
it is not using instead require boards to enable those that they are using.

This is required for audio operation on Arndale, one of the unused I2S
controllers is pinmuxed with the LDO enable GPIOs for the WM1811A.

Signed-off-by: Mark Brown <broonie@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm/boot/dts/exynos5250-arndale.dts  | 4 ++++
 arch/arm/boot/dts/exynos5250-smdk5250.dts | 8 --------
 arch/arm/boot/dts/exynos5250.dtsi         | 3 +++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index cee55fa..4687fa0 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -412,6 +412,10 @@
 		status = "disabled";
 	};
 
+	i2s0: i2s@03830000 {
+		status = "okay";
+	};
+
 	spi_0: spi@12d20000 {
 		status = "disabled";
 	};
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 2538b32..f86d567 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -231,14 +231,6 @@
 		status = "okay";
 	};
 
-	i2s1: i2s@12D60000 {
-		status = "disabled";
-	};
-
-	i2s2: i2s@12D70000 {
-		status = "disabled";
-	};
-
 	sound {
 		compatible = "samsung,smdk-wm8994";
 
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 7d7cc77..c863113 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -417,6 +417,7 @@
 
 	i2s0: i2s@03830000 {
 		compatible = "samsung,s5pv210-i2s";
+		status = "disabled";
 		reg = <0x03830000 0x100>;
 		dmas = <&pdma0 10
 			&pdma0 9
@@ -433,6 +434,7 @@
 
 	i2s1: i2s@12D60000 {
 		compatible = "samsung,s3c6410-i2s";
+		status = "disabled";
 		reg = <0x12D60000 0x100>;
 		dmas = <&pdma1 12
 			&pdma1 11>;
@@ -445,6 +447,7 @@
 
 	i2s2: i2s@12D70000 {
 		compatible = "samsung,s3c6410-i2s";
+		status = "disabled";
 		reg = <0x12D70000 0x100>;
 		dmas = <&pdma0 12
 			&pdma0 11>;
-- 
1.8.4.rc3

[PATCH 2/3] ARM: dts: Leave Exynos5250 SPI controller disabled by default

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Rather than requiring each board to explicitly disable the SPI controllers
it is not using instead require boards to enable those that they are using.
This is less work overall since normally at most one of the controllers is
in use and avoids issues caused by inappropriate pinmuxing.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm/boot/dts/exynos5250-smdk5250.dts | 10 ++--------
 arch/arm/boot/dts/exynos5250.dtsi         |  3 +++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index f86d567..67d3c43 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -184,11 +184,9 @@
 		status = "disabled";
 	};
 
-	spi_0: spi@12d20000 {
-		status = "disabled";
-	};
-
 	spi_1: spi@12d30000 {
+		status = "okay";
+
 		w25q80bw@0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
@@ -214,10 +212,6 @@
 		};
 	};
 
-	spi_2: spi@12d40000 {
-		status = "disabled";
-	};
-
 	hdmi {
 		hpd-gpio = <&gpx3 7 0>;
 	};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index c863113..a886ee4 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -344,6 +344,7 @@
 
 	spi_0: spi@12d20000 {
 		compatible = "samsung,exynos4210-spi";
+		status = "disabled";
 		reg = <0x12d20000 0x100>;
 		interrupts = <0 66 0>;
 		dmas = <&pdma0 5
@@ -359,6 +360,7 @@
 
 	spi_1: spi@12d30000 {
 		compatible = "samsung,exynos4210-spi";
+		status = "disabled";
 		reg = <0x12d30000 0x100>;
 		interrupts = <0 67 0>;
 		dmas = <&pdma1 5
@@ -374,6 +376,7 @@
 
 	spi_2: spi@12d40000 {
 		compatible = "samsung,exynos4210-spi";
+		status = "disabled";
 		reg = <0x12d40000 0x100>;
 		interrupts = <0 68 0>;
 		dmas = <&pdma0 7
-- 
1.8.4.rc3

[PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Mark Brown]

From: Mark Brown <broonie@linaro.org>

Ensure that unused I2C controllers are not activated, causing problems due
to inappropriate pinmuxing or similar, by marking the controllers as
disabled by default and requiring boards to explicitly enable those that
are in use.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 arch/arm/boot/dts/exynos5250-arndale.dts  | 31 ++-----------------------------
 arch/arm/boot/dts/exynos5250-smdk5250.dts | 25 +++++--------------------
 arch/arm/boot/dts/exynos5250-snow.dts     |  4 ++++
 arch/arm/boot/dts/exynos5250.dtsi         | 10 ++++++++++
 4 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 4687fa0..4c10901 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -31,6 +31,7 @@
 	};
 
 	i2c@12C60000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <20000>;
 		samsung,i2c-slave-addr = <0x66>;
@@ -319,16 +320,8 @@
 		};
 	};
 
-	i2c@12C70000 {
-		status = "disabled";
-	};
-
-	i2c@12C80000 {
-		status = "disabled";
-	};
-
 	i2c@12C90000 {
-		wm1811a@1a {
+		status = "okay";
 			compatible = "wlf,wm1811";
 			reg = <0x1a>;
 
@@ -346,26 +339,6 @@
 		};
 	};
 
-	i2c@12CA0000 {
-		status = "disabled";
-	};
-
-	i2c@12CB0000 {
-		status = "disabled";
-	};
-
-	i2c@12CC0000 {
-		status = "disabled";
-	};
-
-	i2c@12CD0000 {
-		status = "disabled";
-	};
-
-	i2c@121D0000 {
-		status = "disabled";
-	};
-
 	dwmmc_0: dwmmc0@12200000 {
 		num-slots = <1>;
 		supports-highspeed;
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 67d3c43..90127e3 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -28,6 +28,7 @@
 	};
 
 	i2c@12C60000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <20000>;
 
@@ -62,6 +63,7 @@
 	};
 
 	i2c@12C70000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <20000>;
 
@@ -86,6 +88,7 @@
 	};
 
 	i2c@121D0000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <40000>;
 		samsung,i2c-slave-addr = <0x38>;
@@ -101,6 +104,7 @@
 	};
 
 	i2c@12C80000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <66000>;
 
@@ -110,27 +114,8 @@
 		};
 	};
 
-	i2c@12C90000 {
-		status = "disabled";
-	};
-
-	i2c@12CA0000 {
-		status = "disabled";
-	};
-
-	i2c@12CB0000 {
-		status = "disabled";
-	};
-
-	i2c@12CC0000 {
-		status = "disabled";
-	};
-
-	i2c@12CD0000 {
-		status = "disabled";
-	};
-
 	i2c@12CE0000 {
+		status = "okay";
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <66000>;
 
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index fd711e2..b7674d5 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -48,6 +48,10 @@
 		};
 	};
 
+	i2c@12CA0000 {
+		status = "okay";
+	};
+
 	i2c-arbitrator {
 		compatible = "i2c-arb-gpio-challenge";
 		#address-cells = <1>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index a886ee4..4d6312e 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -229,6 +229,7 @@
 
 	i2c_0: i2c@12C60000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12C60000 0x100>;
 		interrupts = <0 56 0>;
 		#address-cells = <1>;
@@ -241,6 +242,7 @@
 
 	i2c_1: i2c@12C70000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12C70000 0x100>;
 		interrupts = <0 57 0>;
 		#address-cells = <1>;
@@ -253,6 +255,7 @@
 
 	i2c_2: i2c@12C80000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12C80000 0x100>;
 		interrupts = <0 58 0>;
 		#address-cells = <1>;
@@ -265,6 +268,7 @@
 
 	i2c_3: i2c@12C90000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12C90000 0x100>;
 		interrupts = <0 59 0>;
 		#address-cells = <1>;
@@ -277,6 +281,7 @@
 
 	i2c_4: i2c@12CA0000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12CA0000 0x100>;
 		interrupts = <0 60 0>;
 		#address-cells = <1>;
@@ -289,6 +294,7 @@
 
 	i2c_5: i2c@12CB0000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12CB0000 0x100>;
 		interrupts = <0 61 0>;
 		#address-cells = <1>;
@@ -301,6 +307,7 @@
 
 	i2c_6: i2c@12CC0000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12CC0000 0x100>;
 		interrupts = <0 62 0>;
 		#address-cells = <1>;
@@ -313,6 +320,7 @@
 
 	i2c_7: i2c@12CD0000 {
 		compatible = "samsung,s3c2440-i2c";
+		status = "disabled";
 		reg = <0x12CD0000 0x100>;
 		interrupts = <0 63 0>;
 		#address-cells = <1>;
@@ -325,6 +333,7 @@
 
 	i2c_8: i2c@12CE0000 {
 		compatible = "samsung,s3c2440-hdmiphy-i2c";
+		status = "disabled";
 		reg = <0x12CE0000 0x1000>;
 		interrupts = <0 64 0>;
 		#address-cells = <1>;
@@ -335,6 +344,7 @@
 
 	i2c@121D0000 {
                 compatible = "samsung,exynos5-sata-phy-i2c";
+		status = "disabled";
                 reg = <0x121D0000 0x100>;
                 #address-cells = <1>;
                 #size-cells = <0>;
-- 
1.8.4.rc3

Re: [PATCH 1/3] ARM: dts: Disable Exynos5250 I2S controllers by default

[Tomasz Figa]

Hi Mark,

On Thursday 12 of September 2013 11:40:25 Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> Rather than requiring each board to explicitly disable the I2S
> controllers it is not using instead require boards to enable those that
> they are using.
> 
> This is required for audio operation on Arndale, one of the unused I2S
> controllers is pinmuxed with the LDO enable GPIOs for the WM1811A.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm/boot/dts/exynos5250-arndale.dts  | 4 ++++
>  arch/arm/boot/dts/exynos5250-smdk5250.dts | 8 --------
>  arch/arm/boot/dts/exynos5250.dtsi         | 3 +++
>  3 files changed, 7 insertions(+), 8 deletions(-)

Looks reasonably.

Acked-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz

Re: [PATCH 2/3] ARM: dts: Leave Exynos5250 SPI controller disabled by default

[Tomasz Figa]

Hi Mark,

On Thursday 12 of September 2013 11:40:26 Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> Rather than requiring each board to explicitly disable the SPI
> controllers it is not using instead require boards to enable those that
> they are using. This is less work overall since normally at most one of
> the controllers is in use and avoids issues caused by inappropriate
> pinmuxing.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-smdk5250.dts | 10 ++--------
>  arch/arm/boot/dts/exynos5250.dtsi         |  3 +++
>  2 files changed, 5 insertions(+), 8 deletions(-)

I wonder if exynos5250-arndale shouldn't be also considered in this patch.

Best regards,
Tomasz

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Tomasz Figa]

Hi Mark,

On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
> 
> Ensure that unused I2C controllers are not activated, causing problems
> due to inappropriate pinmuxing or similar, by marking the controllers as
> disabled by default and requiring boards to explicitly enable those that
> are in use.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
>  arch/arm/boot/dts/exynos5250-arndale.dts  | 31
> ++-----------------------------
> arch/arm/boot/dts/exynos5250-smdk5250.dts | 25 +++++--------------------
> arch/arm/boot/dts/exynos5250-snow.dts     |  4 ++++
>  arch/arm/boot/dts/exynos5250.dtsi         | 10 ++++++++++
>  4 files changed, 21 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts
> b/arch/arm/boot/dts/exynos5250-arndale.dts index 4687fa0..4c10901 100644
> --- a/arch/arm/boot/dts/exynos5250-arndale.dts
> +++ b/arch/arm/boot/dts/exynos5250-arndale.dts
> @@ -31,6 +31,7 @@
>  	};
> 
>  	i2c@12C60000 {
> +		status = "okay";

I would keep the status properties at the end of all properties, indicating 
that at this point the description is complete and also to be consistent 
with other dts files around.

>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <20000>;
>  		samsung,i2c-slave-addr = <0x66>;
> @@ -319,16 +320,8 @@
>  		};
>  	};
> 
> -	i2c@12C70000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12C80000 {
> -		status = "disabled";
> -	};
> -
>  	i2c@12C90000 {
> -		wm1811a@1a {
> +		status = "okay";

Hmm, I sense something wrong going on here.

>  			compatible = "wlf,wm1811";
>  			reg = <0x1a>;
> 
> @@ -346,26 +339,6 @@
>  		};
>  	};
> 
> -	i2c@12CA0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CB0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CC0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CD0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@121D0000 {
> -		status = "disabled";
> -	};
> -
>  	dwmmc_0: dwmmc0@12200000 {
>  		num-slots = <1>;
>  		supports-highspeed;
> diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> b/arch/arm/boot/dts/exynos5250-smdk5250.dts index 67d3c43..90127e3
> 100644
> --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
> +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
> @@ -28,6 +28,7 @@
>  	};
> 
>  	i2c@12C60000 {
> +		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <20000>;
> 
> @@ -62,6 +63,7 @@
>  	};
> 
>  	i2c@12C70000 {
> +		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <20000>;
> 
> @@ -86,6 +88,7 @@
>  	};
> 
>  	i2c@121D0000 {
> +		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <40000>;
>  		samsung,i2c-slave-addr = <0x38>;
> @@ -101,6 +104,7 @@
>  	};
> 
>  	i2c@12C80000 {
> +		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <66000>;
> 
> @@ -110,27 +114,8 @@
>  		};
>  	};
> 
> -	i2c@12C90000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CA0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CB0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CC0000 {
> -		status = "disabled";
> -	};
> -
> -	i2c@12CD0000 {
> -		status = "disabled";
> -	};
> -
>  	i2c@12CE0000 {
> +		status = "okay";
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-max-bus-freq = <66000>;
> 
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
> b/arch/arm/boot/dts/exynos5250-snow.dts index fd711e2..b7674d5 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -48,6 +48,10 @@
>  		};
>  	};
> 
> +	i2c@12CA0000 {
> +		status = "okay";

Hmm, no other properties needed here?

> +	};
> +
>  	i2c-arbitrator {
>  		compatible = "i2c-arb-gpio-challenge";
>  		#address-cells = <1>;
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi index a886ee4..4d6312e 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -229,6 +229,7 @@
> 
>  	i2c_0: i2c@12C60000 {
>  		compatible = "samsung,s3c2440-i2c";
> +		status = "disabled";

For the sake of consistency, I would place the status property as last here 
as well.

Best regards,
Tomasz

Re: [PATCH 2/3] ARM: dts: Leave Exynos5250 SPI controller disabled by default

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Thu, Sep 12, 2013 at 01:56:44PM +0200, Tomasz Figa wrote:

> I wonder if exynos5250-arndale shouldn't be also considered in this patch.

There are no SPI devices on that board (at least not ones that are
currently instantiated by the DT).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Thu, Sep 12, 2013 at 02:58:59PM +0200, Tomasz Figa wrote:
> On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:

Please delete unneeded context from your mails, it makes it much easier
to find the new content.

> > +	i2c@12CA0000 {
> > +		status = "okay";

> Hmm, no other properties needed here?

Apparently not, it looks like the binding for the aribtrator doesn't sit
as a child of the bus it's arbitrating - this is the controller
referenced from the arbitrator node.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Stephen Warren]

On 09/12/2013 06:58 AM, Tomasz Figa wrote:
> Hi Mark,
> 
> On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:
>> From: Mark Brown <broonie@linaro.org>
>>
>> Ensure that unused I2C controllers are not activated, causing problems
>> due to inappropriate pinmuxing or similar, by marking the controllers as
>> disabled by default and requiring boards to explicitly enable those that
>> are in use.

>> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts

>>  	i2c@12C60000 {
>> +		status = "okay";
> 
> I would keep the status properties at the end of all properties, indicating 
> that at this point the description is complete and also to be consistent 
> with other dts files around.

DT doesn't define any kind of ordering for the properties AFAIK, so the
order shouldn't matter in practice; it conveys no semantic
representation as far as the parsing code is concerned even if a human
may be influenced otherwise:-)

Just as an FYI, the rule I've been trying to follow in Tegra DT files is
that properties that are overridden from any included .dtsi file come
first, followed by any new properties. Still, there's no particular
reason anyone else has to follow that layout.

Re: [PATCH 2/3] ARM: dts: Leave Exynos5250 SPI controller disabled by default

[Tomasz Figa]

On Thursday 12 of September 2013 14:50:28 Mark Brown wrote:
> On Thu, Sep 12, 2013 at 01:56:44PM +0200, Tomasz Figa wrote:
> > I wonder if exynos5250-arndale shouldn't be also considered in this
> > patch.
> There are no SPI devices on that board (at least not ones that are
> currently instantiated by the DT).

I can see a bunch of nodes simply adding status = "disabled" in dts of 
arndale.

Best regards,
Tomasz

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Tomasz Figa]

On Thursday 12 of September 2013 14:52:33 Mark Brown wrote:
> On Thu, Sep 12, 2013 at 02:58:59PM +0200, Tomasz Figa wrote:
> > On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:
> Please delete unneeded context from your mails, it makes it much easier
> to find the new content.

I always snip unnecessary parts of quoted message if its really long, but 
if its length is in acceptable range then I try to keep the whole message 
just for reference.

> > > +	i2c@12CA0000 {
> > > +		status = "okay";
> > 
> > Hmm, no other properties needed here?
> 
> Apparently not, it looks like the binding for the aribtrator doesn't sit
> as a child of the bus it's arbitrating - this is the controller
> referenced from the arbitrator node.

i2c@12CA0000 does look like a normal hardware I2C controller, which needs 
some extra properties like samsung,i2c-sda-delay or samsung,i2c-max-bus-
freq. However they are specified in cros5250-common.dtsi, so I'd say that 
this might be better place for the status override as well.

Best regards,
Tomasz

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Tomasz Figa]

On Thursday 12 of September 2013 09:03:23 Stephen Warren wrote:
> On 09/12/2013 06:58 AM, Tomasz Figa wrote:
> > Hi Mark,
> > 
> > On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:
> >> From: Mark Brown <broonie@linaro.org>
> >> 
> >> Ensure that unused I2C controllers are not activated, causing problems
> >> due to inappropriate pinmuxing or similar, by marking the controllers
> >> as
> >> disabled by default and requiring boards to explicitly enable those
> >> that
> >> are in use.
> >> 
> >> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts
> >> 
> >>  	i2c@12C60000 {
> >> 
> >> +		status = "okay";
> > 
> > I would keep the status properties at the end of all properties,
> > indicating that at this point the description is complete and also to
> > be consistent with other dts files around.
> 
> DT doesn't define any kind of ordering for the properties AFAIK, so the
> order shouldn't matter in practice; it conveys no semantic
> representation as far as the parsing code is concerned even if a human
> may be influenced otherwise:-)

Well, this is somehow similar to code, maybe not exactly in case of 
ordering, but coding style and conventions. Unreadable code can do exactly 
the same thing as readable code, while we somehow prefer the readable one.

> Just as an FYI, the rule I've been trying to follow in Tegra DT files is
> that properties that are overridden from any included .dtsi file come
> first, followed by any new properties. Still, there's no particular
> reason anyone else has to follow that layout.

On Exynos I've been trying to keep status as the last property, compatible 
as first, followed by reg and without any specific order of any other 
properties. Again, this is simply to keep some consistence across the dtses 
making them easier to read.

Best regards,
Tomasz

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

On Thu, Sep 12, 2013 at 05:55:59PM +0200, Tomasz Figa wrote:
> On Thursday 12 of September 2013 14:52:33 Mark Brown wrote:

> > Please delete unneeded context from your mails, it makes it much easier
> > to find the new content.

> I always snip unnecessary parts of quoted message if its really long, but 
> if its length is in acceptable range then I try to keep the whole message 
> just for reference.

I tend to say that when I find myself paging through entire screens of
quote and can't see the relevance to any content I eventually find.

> i2c@12CA0000 does look like a normal hardware I2C controller, which needs 
> some extra properties like samsung,i2c-sda-delay or samsung,i2c-max-bus-

Shouldn't these have sane defaults?

> freq. However they are specified in cros5250-common.dtsi, so I'd say that 
> this might be better place for the status override as well.

Can we rename that file to something like exynos5250-cros-common.dtsi
so that it's obvious it's relevant to exynos?  I had completely failed
to notice that it was anything to do with Exynos when I was searching
for affected DTS files.

Especially given that it's included in exactly one DTS at the minute...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] ARM: dts: Leave Exynos5250 SPI controller disabled by default

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Thu, Sep 12, 2013 at 05:51:58PM +0200, Tomasz Figa wrote:
> On Thursday 12 of September 2013 14:50:28 Mark Brown wrote:

> > There are no SPI devices on that board (at least not ones that are
> > currently instantiated by the DT).

> I can see a bunch of nodes simply adding status = "disabled" in dts of 
> arndale.

Meh, gone now.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Tomasz Figa]

On Thursday 12 of September 2013 21:00:13 Mark Brown wrote:
> On Thu, Sep 12, 2013 at 05:55:59PM +0200, Tomasz Figa wrote:
> > On Thursday 12 of September 2013 14:52:33 Mark Brown wrote:
> > > Please delete unneeded context from your mails, it makes it much
> > > easier
> > > to find the new content.
> > 
> > I always snip unnecessary parts of quoted message if its really long,
> > but if its length is in acceptable range then I try to keep the whole
> > message just for reference.
> 
> I tend to say that when I find myself paging through entire screens of
> quote and can't see the relevance to any content I eventually find.

Hmm, looking at it here from my desktop, I could have dropped some of the 
context between second and third reply indeed.

> > i2c@12CA0000 does look like a normal hardware I2C controller, which
> > needs some extra properties like samsung,i2c-sda-delay or
> > samsung,i2c-max-bus-
> Shouldn't these have sane defaults?

AFAIK those parameters are highly board specific so it would be hard to 
set any reasonable default for them.

> > freq. However they are specified in cros5250-common.dtsi, so I'd say
> > that this might be better place for the status override as well.
> 
> Can we rename that file to something like exynos5250-cros-common.dtsi
> so that it's obvious it's relevant to exynos?  I had completely failed
> to notice that it was anything to do with Exynos when I was searching
> for affected DTS files.
> 
> Especially given that it's included in exactly one DTS at the minute...

Yes, it would surely make sense to do so, unless there is another cros 
board that could use this dtsi and is based on another SoC, which I doubt, 
because there are multiple SoC specific values specified inside it.

Best regards,
Tomasz

--
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

Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

On Fri, Sep 13, 2013 at 01:02:11AM +0200, Tomasz Figa wrote:
> On Thursday 12 of September 2013 21:00:13 Mark Brown wrote:

> > > i2c@12CA0000 does look like a normal hardware I2C controller, which
> > > needs some extra properties like samsung,i2c-sda-delay or
> > > samsung,i2c-max-bus-

> > Shouldn't these have sane defaults?

> AFAIK those parameters are highly board specific so it would be hard to 
> set any reasonable default for them.

At least the maximum bus frequency should be able to be set to 100Hz by
default, that's the minimum from the I2C spec though pretty much
everything does at least 400Hz.  sda-delay should really be able to
default to either zero or something per-SoC, I don't remember having to
tune it when using board files and I2C is generally pretty robust so
it'd be surprising if it were that sensitive to system design.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]