[PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420

[PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420

[Padmavathi Venna]

Changes since V3:
	- used existent fixed-clock binding for registering oscillator clock
	  as fixed rate clock as pointed by Tomasz Figa
	- Made some changes in wm8994 regulator nodes as suggested by Tomasz Figa
	- Separated out only adding i2s nodes and enabling sound support on smdk5420
	  into different patch set as they are dependent on on some of already posted
	  but not yet merged i2c, dwmmc, dma and audss clock controller patches on
	  Kukjin for-next branch.

Changes since V2:
        - Separated out driver side changes and dts changes in two
          patch sets
        - Added proper names for wm8994 regulators as commented by Mark
        - Moved common i2s nodes into the exynos5.dtsi
        - Added clock info in wm8994 node as requested by Mark.
        - Registered the 16.9MHz oscillator clock as fixed clock in the
          machine file. Right now no user of this clock but as Mark requested
          to add mclk info in wm8994 node, I added this part.

This patch set is dependent on the following i2c, dma and audio subsystem
clk controller patches.
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20077
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20661
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20668

This patch set is made based on Kukjin Kim for-next branch.


Andrew Bresticker (1):
  ARM: dts: exynos5420: add i2s controllers

Padmavathi Venna (3):
  ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420
  ARM: dts: Add osc clock node on smdk5420.
  ARM: dts: Enable sound support on smdk5420

 arch/arm/boot/dts/exynos5420-smdk5420.dts |   81 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5420.dtsi         |   32 +++++++++++
 2 files changed, 113 insertions(+), 0 deletions(-)

-- 
1.7.4.4

[PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420

[Padmavathi Venna]

Changes since V3:
	- used existent fixed-clock binding for registering oscillator clock
	  as fixed rate clock as pointed by Tomasz Figa
	- Made some changes in wm8994 regulator nodes as suggested by Tomasz Figa
	- Separated out only adding i2s nodes and enabling sound support on smdk5420
	  into different patch set as they are dependent on on some of already posted
	  but not yet merged i2c, dwmmc, dma and audss clock controller patches on
	  Kukjin for-next branch.

Changes since V2:
        - Separated out driver side changes and dts changes in two
          patch sets
        - Added proper names for wm8994 regulators as commented by Mark
        - Moved common i2s nodes into the exynos5.dtsi
        - Added clock info in wm8994 node as requested by Mark.
        - Registered the 16.9MHz oscillator clock as fixed clock in the
          machine file. Right now no user of this clock but as Mark requested
          to add mclk info in wm8994 node, I added this part.

This patch set is dependent on the following i2c, dma and audio subsystem
clk controller patches.
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20077
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20661
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20668

This patch set is made based on Kukjin Kim for-next branch.


Andrew Bresticker (1):
  ARM: dts: exynos5420: add i2s controllers

Padmavathi Venna (3):
  ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420
  ARM: dts: Add osc clock node on smdk5420.
  ARM: dts: Enable sound support on smdk5420

 arch/arm/boot/dts/exynos5420-smdk5420.dts |   81 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5420.dtsi         |   32 +++++++++++
 2 files changed, 113 insertions(+), 0 deletions(-)

-- 
1.7.4.4

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Padmavathi Venna]

From: Andrew Bresticker <abrestic@chromium.org>

This adds device-tree bindings for the i2s controllers on Exynos 5420.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Reviewed-on: https://gerrit.chromium.org/gerrit/57713
---
 arch/arm/boot/dts/exynos5420.dtsi |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index d2fdb87..8d57369 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -242,4 +242,36 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2c3_bus>;
 	};
+
+	i2s_0: i2s@03830000 {
+		compatible = "samsung,exynos5420-i2s";
+		dmas = <&adma 0
+			&adma 2
+			&adma 1>;
+		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s0_bus>;
+		status = "disabled";
+	};
+
+	i2s_1: i2s@12D60000 {
+		clocks = <&clock 275>, <&clock 138>;
+		clock-names = "iis", "i2s_opclk0";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s1_bus>;
+		status = "disabled";
+	};
+
+	i2s_2: i2s@12D70000 {
+		clocks = <&clock 276>, <&clock 139>;
+		clock-names = "iis", "i2s_opclk0";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s2_bus>;
+		status = "disabled";
+	};
+
 };
-- 
1.7.4.4

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Padmavathi Venna]

From: Andrew Bresticker <abrestic@chromium.org>

This adds device-tree bindings for the i2s controllers on Exynos 5420.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Reviewed-on: https://gerrit.chromium.org/gerrit/57713
---
 arch/arm/boot/dts/exynos5420.dtsi |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index d2fdb87..8d57369 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -242,4 +242,36 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2c3_bus>;
 	};
+
+	i2s_0: i2s at 03830000 {
+		compatible = "samsung,exynos5420-i2s";
+		dmas = <&adma 0
+			&adma 2
+			&adma 1>;
+		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s0_bus>;
+		status = "disabled";
+	};
+
+	i2s_1: i2s at 12D60000 {
+		clocks = <&clock 275>, <&clock 138>;
+		clock-names = "iis", "i2s_opclk0";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s1_bus>;
+		status = "disabled";
+	};
+
+	i2s_2: i2s at 12D70000 {
+		clocks = <&clock 276>, <&clock 139>;
+		clock-names = "iis", "i2s_opclk0";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s2_bus>;
+		status = "disabled";
+	};
+
 };
-- 
1.7.4.4

[PATCH V4 2/4] ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420

[Padmavathi Venna]

This patch adds i2c bus 1 and wm8994 codec node on i2c bus1 and the
required regulator supplies and properties on smdk5420 board.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |   62 +++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index d05de7a..1ef7e2e 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -68,4 +68,66 @@
 			bus-width = <4>;
 		};
 	};
+
+	fixed-regulators {
+		compatible = "simple-bus";
+
+		avdd2: fixed-regulator-0 {
+			compatible = "regulator-fixed";
+			regulator-name = "avdd2-supply";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-always-on;
+		};
+
+		cpvdd: fixed-regulator-1 {
+			compatible = "regulator-fixed";
+			regulator-name = "cpvdd-supply";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-always-on;
+		};
+
+		dbvdd: fixed-regulator-2 {
+			compatible = "regulator-fixed";
+			regulator-name = "dbvdd-supply";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		spkvdd: fixed-regulator-3 {
+			compatible = "regulator-fixed";
+			regulator-name = "spkvdd-supply";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			regulator-always-on;
+		};
+	};
+
+	i2c@12C70000 {
+		status = "okay";
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <20000>;
+
+		eeprom@51 {
+			compatible = "samsung,s524ad0xd1";
+			reg = <0x51>;
+		};
+
+		wm8994: wm8994@1a {
+			compatible = "wlf,wm8994";
+			reg = <0x1a>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+
+			AVDD2-supply = <&avdd2>;
+			CPVDD-supply = <&cpvdd>;
+			DBVDD-supply = <&dbvdd>;
+			SPKVDD1-supply = <&spkvdd>;
+			SPKVDD2-supply = <&spkvdd>;
+		};
+	};
+
 };
-- 
1.7.4.4

[PATCH V4 2/4] ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420

[Padmavathi Venna]

This patch adds i2c bus 1 and wm8994 codec node on i2c bus1 and the
required regulator supplies and properties on smdk5420 board.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |   62 +++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index d05de7a..1ef7e2e 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -68,4 +68,66 @@
 			bus-width = <4>;
 		};
 	};
+
+	fixed-regulators {
+		compatible = "simple-bus";
+
+		avdd2: fixed-regulator-0 {
+			compatible = "regulator-fixed";
+			regulator-name = "avdd2-supply";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-always-on;
+		};
+
+		cpvdd: fixed-regulator-1 {
+			compatible = "regulator-fixed";
+			regulator-name = "cpvdd-supply";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-always-on;
+		};
+
+		dbvdd: fixed-regulator-2 {
+			compatible = "regulator-fixed";
+			regulator-name = "dbvdd-supply";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		spkvdd: fixed-regulator-3 {
+			compatible = "regulator-fixed";
+			regulator-name = "spkvdd-supply";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			regulator-always-on;
+		};
+	};
+
+	i2c at 12C70000 {
+		status = "okay";
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <20000>;
+
+		eeprom at 51 {
+			compatible = "samsung,s524ad0xd1";
+			reg = <0x51>;
+		};
+
+		wm8994: wm8994 at 1a {
+			compatible = "wlf,wm8994";
+			reg = <0x1a>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+
+			AVDD2-supply = <&avdd2>;
+			CPVDD-supply = <&cpvdd>;
+			DBVDD-supply = <&dbvdd>;
+			SPKVDD1-supply = <&spkvdd>;
+			SPKVDD2-supply = <&spkvdd>;
+		};
+	};
+
 };
-- 
1.7.4.4

[PATCH V4 3/4] ARM: dts: Add osc clock node on smdk5420.

[Padmavathi Venna]

This patch adds 16MHz oscillator clock node required for audio
on smdk5420 and adds the phandle of the same in wm8994 clock info.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 1ef7e2e..a898b3f 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -31,6 +31,12 @@
 		};
 	};
 
+	osc3_clk16mhz: oscillator-0 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <16934400>;
+	};
+
 	dwmmc0@12200000 {
 		status = "okay";
 		num-slots = <1>;
@@ -127,6 +133,9 @@
 			DBVDD-supply = <&dbvdd>;
 			SPKVDD1-supply = <&spkvdd>;
 			SPKVDD2-supply = <&spkvdd>;
+
+			clocks = <&osc3_clk16mhz>;
+			clock-names = "mclk1";
 		};
 	};
 
-- 
1.7.4.4

[PATCH V4 3/4] ARM: dts: Add osc clock node on smdk5420.

[Padmavathi Venna]

This patch adds 16MHz oscillator clock node required for audio
on smdk5420 and adds the phandle of the same in wm8994 clock info.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index 1ef7e2e..a898b3f 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -31,6 +31,12 @@
 		};
 	};
 
+	osc3_clk16mhz: oscillator-0 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <16934400>;
+	};
+
 	dwmmc0 at 12200000 {
 		status = "okay";
 		num-slots = <1>;
@@ -127,6 +133,9 @@
 			DBVDD-supply = <&dbvdd>;
 			SPKVDD1-supply = <&spkvdd>;
 			SPKVDD2-supply = <&spkvdd>;
+
+			clocks = <&osc3_clk16mhz>;
+			clock-names = "mclk1";
 		};
 	};
 
-- 
1.7.4.4

[PATCH V4 4/4] ARM: dts: Enable sound support on smdk5420

[Padmavathi Venna]

This patch enables i2s0 and sound support on smdk5420.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index a898b3f..b1b745c 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -139,4 +139,14 @@
 		};
 	};
 
+	i2s0: i2s@03830000 {
+		status = "okay";
+	};
+
+	sound {
+		compatible = "samsung,smdk-wm8994";
+
+		samsung,i2s-controller = <&i2s0>;
+		samsung,audio-codec = <&wm8994>;
+	};
 };
-- 
1.7.4.4

[PATCH V4 4/4] ARM: dts: Enable sound support on smdk5420

[Padmavathi Venna]

This patch enables i2s0 and sound support on smdk5420.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5420-smdk5420.dts |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index a898b3f..b1b745c 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -139,4 +139,14 @@
 		};
 	};
 
+	i2s0: i2s at 03830000 {
+		status = "okay";
+	};
+
+	sound {
+		compatible = "samsung,smdk-wm8994";
+
+		samsung,i2s-controller = <&i2s0>;
+		samsung,audio-codec = <&wm8994>;
+	};
 };
-- 
1.7.4.4

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

Hi Padmavathi, Andrew,

On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
> 
> This adds device-tree bindings for the i2s controllers on Exynos 5420.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> Reviewed-on: https://gerrit.chromium.org/gerrit/57713
> ---
>  arch/arm/boot/dts/exynos5420.dtsi |   32
> ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0
> deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi
> b/arch/arm/boot/dts/exynos5420.dtsi index d2fdb87..8d57369 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -242,4 +242,36 @@
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&i2c3_bus>;
>  	};
> +
> +	i2s_0: i2s@03830000 {
> +		compatible = "samsung,exynos5420-i2s";
> +		dmas = <&adma 0
> +			&adma 2
> +			&adma 1>;
> +		dma-names = "tx", "rx", "tx-sec";
> +		clocks = <&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_SCLK_I2S>;
> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s0_bus>;
> +		status = "disabled";

If a node does not require any board-specific properties for the device to 
operate properly, there is no point in disabling it, just to add a single 
status property at board level.

> +	};
> +
> +	i2s_1: i2s@12D60000 {
> +		clocks = <&clock 275>, <&clock 138>;
> +		clock-names = "iis", "i2s_opclk0";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s1_bus>;
> +		status = "disabled";

Ditto.

> +	};
> +
> +	i2s_2: i2s@12D70000 {
> +		clocks = <&clock 276>, <&clock 139>;
> +		clock-names = "iis", "i2s_opclk0";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s2_bus>;
> +		status = "disabled";

Ditto.

Best regards,
Tomasz

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

Hi Padmavathi, Andrew,

On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
> 
> This adds device-tree bindings for the i2s controllers on Exynos 5420.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> Reviewed-on: https://gerrit.chromium.org/gerrit/57713
> ---
>  arch/arm/boot/dts/exynos5420.dtsi |   32
> ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0
> deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi
> b/arch/arm/boot/dts/exynos5420.dtsi index d2fdb87..8d57369 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -242,4 +242,36 @@
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&i2c3_bus>;
>  	};
> +
> +	i2s_0: i2s at 03830000 {
> +		compatible = "samsung,exynos5420-i2s";
> +		dmas = <&adma 0
> +			&adma 2
> +			&adma 1>;
> +		dma-names = "tx", "rx", "tx-sec";
> +		clocks = <&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_SCLK_I2S>;
> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s0_bus>;
> +		status = "disabled";

If a node does not require any board-specific properties for the device to 
operate properly, there is no point in disabling it, just to add a single 
status property at board level.

> +	};
> +
> +	i2s_1: i2s at 12D60000 {
> +		clocks = <&clock 275>, <&clock 138>;
> +		clock-names = "iis", "i2s_opclk0";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s1_bus>;
> +		status = "disabled";

Ditto.

> +	};
> +
> +	i2s_2: i2s at 12D70000 {
> +		clocks = <&clock 276>, <&clock 139>;
> +		clock-names = "iis", "i2s_opclk0";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2s2_bus>;
> +		status = "disabled";

Ditto.

Best regards,
Tomasz

Re: [PATCH V4 4/4] ARM: dts: Enable sound support on smdk5420

[Tomasz Figa]

Hi Padmavathi,

On Monday 12 of August 2013 15:37:50 Padmavathi Venna wrote:
> This patch enables i2s0 and sound support on smdk5420.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> b/arch/arm/boot/dts/exynos5420-smdk5420.dts index a898b3f..b1b745c
> 100644
> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> @@ -139,4 +139,14 @@
>  		};
>  	};
> 
> +	i2s0: i2s@03830000 {
> +		status = "okay";
> +	};

After addressing my comment on node status for patch 1/4, you could get rid 
of the node above.

Best regards,
Tomasz

[PATCH V4 4/4] ARM: dts: Enable sound support on smdk5420

[Tomasz Figa]

Hi Padmavathi,

On Monday 12 of August 2013 15:37:50 Padmavathi Venna wrote:
> This patch enables i2s0 and sound support on smdk5420.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> b/arch/arm/boot/dts/exynos5420-smdk5420.dts index a898b3f..b1b745c
> 100644
> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> @@ -139,4 +139,14 @@
>  		};
>  	};
> 
> +	i2s0: i2s at 03830000 {
> +		status = "okay";
> +	};

After addressing my comment on node status for patch 1/4, you could get rid 
of the node above.

Best regards,
Tomasz

Re: [PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420

[Tomasz Figa]

Hi Padmavathi,

On Monday 12 of August 2013 15:37:46 Padmavathi Venna wrote:
> Changes since V3:
> 	- used existent fixed-clock binding for registering oscillator clock
> 	  as fixed rate clock as pointed by Tomasz Figa
> 	- Made some changes in wm8994 regulator nodes as suggested by Tomasz
> Figa - Separated out only adding i2s nodes and enabling sound support on
> smdk5420 into different patch set as they are dependent on on some of
> already posted but not yet merged i2c, dwmmc, dma and audss clock
> controller patches on Kukjin for-next branch.
> 
> Changes since V2:
>         - Separated out driver side changes and dts changes in two
>           patch sets
>         - Added proper names for wm8994 regulators as commented by Mark
>         - Moved common i2s nodes into the exynos5.dtsi
>         - Added clock info in wm8994 node as requested by Mark.
>         - Registered the 16.9MHz oscillator clock as fixed clock in the
>           machine file. Right now no user of this clock but as Mark
> requested to add mclk info in wm8994 node, I added this part.
> 
> This patch set is dependent on the following i2c, dma and audio subsystem
> clk controller patches.
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20077
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20661
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20668
> 
> This patch set is made based on Kukjin Kim for-next branch.
> 
> 
> Andrew Bresticker (1):
>   ARM: dts: exynos5420: add i2s controllers
> 
> Padmavathi Venna (3):
>   ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420
>   ARM: dts: Add osc clock node on smdk5420.
>   ARM: dts: Enable sound support on smdk5420
> 
>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   81
> +++++++++++++++++++++++++++++ arch/arm/boot/dts/exynos5420.dtsi        
> |   32 +++++++++++
>  2 files changed, 113 insertions(+), 0 deletions(-)

Except the node status misuse that I commented on in patches 1/4 and 4/4, 
looks good to me:

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

Best regards,
Tomasz

[PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420

[Tomasz Figa]

Hi Padmavathi,

On Monday 12 of August 2013 15:37:46 Padmavathi Venna wrote:
> Changes since V3:
> 	- used existent fixed-clock binding for registering oscillator clock
> 	  as fixed rate clock as pointed by Tomasz Figa
> 	- Made some changes in wm8994 regulator nodes as suggested by Tomasz
> Figa - Separated out only adding i2s nodes and enabling sound support on
> smdk5420 into different patch set as they are dependent on on some of
> already posted but not yet merged i2c, dwmmc, dma and audss clock
> controller patches on Kukjin for-next branch.
> 
> Changes since V2:
>         - Separated out driver side changes and dts changes in two
>           patch sets
>         - Added proper names for wm8994 regulators as commented by Mark
>         - Moved common i2s nodes into the exynos5.dtsi
>         - Added clock info in wm8994 node as requested by Mark.
>         - Registered the 16.9MHz oscillator clock as fixed clock in the
>           machine file. Right now no user of this clock but as Mark
> requested to add mclk info in wm8994 node, I added this part.
> 
> This patch set is dependent on the following i2c, dma and audio subsystem
> clk controller patches.
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20077
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20661
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/20668
> 
> This patch set is made based on Kukjin Kim for-next branch.
> 
> 
> Andrew Bresticker (1):
>   ARM: dts: exynos5420: add i2s controllers
> 
> Padmavathi Venna (3):
>   ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420
>   ARM: dts: Add osc clock node on smdk5420.
>   ARM: dts: Enable sound support on smdk5420
> 
>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   81
> +++++++++++++++++++++++++++++ arch/arm/boot/dts/exynos5420.dtsi        
> |   32 +++++++++++
>  2 files changed, 113 insertions(+), 0 deletions(-)

Except the node status misuse that I commented on in patches 1/4 and 4/4, 
looks good to me:

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

Best regards,
Tomasz

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Mark Brown]

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

On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
> On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:

> > +	i2s_0: i2s@03830000 {
> > +		status = "disabled";

> If a node does not require any board-specific properties for the device to 
> operate properly, there is no point in disabling it, just to add a single 
> status property at board level.

I'd expect that to interact badly with the pinmuxing - unless the device
is disabled it'll try to grab its pins on probe which is not going to be
a good idea unless it is actually wired up for use in the system.  Or is
there some other mechanism for handling that?

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

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Mark Brown]

On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
> On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:

> > +	i2s_0: i2s at 03830000 {
> > +		status = "disabled";

> If a node does not require any board-specific properties for the device to 
> operate properly, there is no point in disabling it, just to add a single 
> status property at board level.

I'd expect that to interact badly with the pinmuxing - unless the device
is disabled it'll try to grab its pins on probe which is not going to be
a good idea unless it is actually wired up for use in the system.  Or is
there some other mechanism for handling that?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130812/b14ff50a/attachment.sig>

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
> > > +	i2s_0: i2s@03830000 {
> > > +		status = "disabled";
> > 
> > If a node does not require any board-specific properties for the device
> > to operate properly, there is no point in disabling it, just to add a
> > single status property at board level.
> 
> I'd expect that to interact badly with the pinmuxing - unless the device
> is disabled it'll try to grab its pins on probe which is not going to be
> a good idea unless it is actually wired up for use in the system.  Or is
> there some other mechanism for handling that?

Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered 
board-specific and specified in board-level dts instead?

Best regards,
Tomasz

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
> > > +	i2s_0: i2s at 03830000 {
> > > +		status = "disabled";
> > 
> > If a node does not require any board-specific properties for the device
> > to operate properly, there is no point in disabling it, just to add a
> > single status property at board level.
> 
> I'd expect that to interact badly with the pinmuxing - unless the device
> is disabled it'll try to grab its pins on probe which is not going to be
> a good idea unless it is actually wired up for use in the system.  Or is
> there some other mechanism for handling that?

Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered 
board-specific and specified in board-level dts instead?

Best regards,
Tomasz

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Mark Brown]

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

On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> On Monday 12 of August 2013 12:34:48 Mark Brown wrote:

> > I'd expect that to interact badly with the pinmuxing - unless the device
> > is disabled it'll try to grab its pins on probe which is not going to be
> > a good idea unless it is actually wired up for use in the system.  Or is
> > there some other mechanism for handling that?

> Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered 
> board-specific and specified in board-level dts instead?

It seems a bit cleaner to use the current mechanism in that it stops the
device appearing at all and hence repeated efforts to probe, plus a
simple enable is less error prone, the way these SoCs are designed you
don't have to pick which pinmux is in use for most of the IPs.  Where
there are multiple options it does seem like a good approach though.

Tastes may differ though.

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

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Mark Brown]

On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> On Monday 12 of August 2013 12:34:48 Mark Brown wrote:

> > I'd expect that to interact badly with the pinmuxing - unless the device
> > is disabled it'll try to grab its pins on probe which is not going to be
> > a good idea unless it is actually wired up for use in the system.  Or is
> > there some other mechanism for handling that?

> Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered 
> board-specific and specified in board-level dts instead?

It seems a bit cleaner to use the current mechanism in that it stops the
device appearing at all and hence repeated efforts to probe, plus a
simple enable is less error prone, the way these SoCs are designed you
don't have to pick which pinmux is in use for most of the IPs.  Where
there are multiple options it does seem like a good approach though.

Tastes may differ though.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130812/80389ac4/attachment-0001.sig>

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> > > I'd expect that to interact badly with the pinmuxing - unless the
> > > device is disabled it'll try to grab its pins on probe which is not
> > > going to be a good idea unless it is actually wired up for use in
> > > the system.  Or is there some other mechanism for handling that?
> > 
> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> > considered board-specific and specified in board-level dts instead?
> 
> It seems a bit cleaner to use the current mechanism in that it stops the
> device appearing at all and hence repeated efforts to probe, plus a
> simple enable is less error prone, the way these SoCs are designed you
> don't have to pick which pinmux is in use for most of the IPs.  Where
> there are multiple options it does seem like a good approach though.
> 
> Tastes may differ though.

Right, if this SoC has only one pinmux setting for this IP, then it's 
fine.

Padmavathi, this was the only issue I spotted, so have my:

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

Best regards,
Tomasz

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> > > I'd expect that to interact badly with the pinmuxing - unless the
> > > device is disabled it'll try to grab its pins on probe which is not
> > > going to be a good idea unless it is actually wired up for use in
> > > the system.  Or is there some other mechanism for handling that?
> > 
> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> > considered board-specific and specified in board-level dts instead?
> 
> It seems a bit cleaner to use the current mechanism in that it stops the
> device appearing at all and hence repeated efforts to probe, plus a
> simple enable is less error prone, the way these SoCs are designed you
> don't have to pick which pinmux is in use for most of the IPs.  Where
> there are multiple options it does seem like a good approach though.
> 
> Tastes may differ though.

Right, if this SoC has only one pinmux setting for this IP, then it's 
fine.

Padmavathi, this was the only issue I spotted, so have my:

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

Best regards,
Tomasz

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Padma Venkat]

Hi Tomasz,

On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
>> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
>> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
>> > > I'd expect that to interact badly with the pinmuxing - unless the
>> > > device is disabled it'll try to grab its pins on probe which is not
>> > > going to be a good idea unless it is actually wired up for use in
>> > > the system.  Or is there some other mechanism for handling that?
>> >
>> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
>> > considered board-specific and specified in board-level dts instead?
>>
>> It seems a bit cleaner to use the current mechanism in that it stops the
>> device appearing at all and hence repeated efforts to probe, plus a
>> simple enable is less error prone, the way these SoCs are designed you
>> don't have to pick which pinmux is in use for most of the IPs.  Where
>> there are multiple options it does seem like a good approach though.
>>
>> Tastes may differ though.
>
> Right, if this SoC has only one pinmux setting for this IP, then it's
> fine.

Yes. This IP has only default pin configuration.

>
> Padmavathi, this was the only issue I spotted, so have my:
>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Thanks for your review.

>
> Best regards,
> Tomasz
>

Best Regards,
Padma

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Padma Venkat]

Hi Tomasz,

On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
>> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
>> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
>> > > I'd expect that to interact badly with the pinmuxing - unless the
>> > > device is disabled it'll try to grab its pins on probe which is not
>> > > going to be a good idea unless it is actually wired up for use in
>> > > the system.  Or is there some other mechanism for handling that?
>> >
>> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
>> > considered board-specific and specified in board-level dts instead?
>>
>> It seems a bit cleaner to use the current mechanism in that it stops the
>> device appearing at all and hence repeated efforts to probe, plus a
>> simple enable is less error prone, the way these SoCs are designed you
>> don't have to pick which pinmux is in use for most of the IPs.  Where
>> there are multiple options it does seem like a good approach though.
>>
>> Tastes may differ though.
>
> Right, if this SoC has only one pinmux setting for this IP, then it's
> fine.

Yes. This IP has only default pin configuration.

>
> Padmavathi, this was the only issue I spotted, so have my:
>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Thanks for your review.

>
> Best regards,
> Tomasz
>

Best Regards,
Padma

Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Wednesday 14 of August 2013 11:21:40 Padma Venkat wrote:
> Hi Tomasz,
> 
> On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa <tomasz.figa@gmail.com> 
wrote:
> > On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> >> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> >> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> >> > > I'd expect that to interact badly with the pinmuxing - unless the
> >> > > device is disabled it'll try to grab its pins on probe which is
> >> > > not
> >> > > going to be a good idea unless it is actually wired up for use in
> >> > > the system.  Or is there some other mechanism for handling that?
> >> > 
> >> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> >> > considered board-specific and specified in board-level dts instead?
> >> 
> >> It seems a bit cleaner to use the current mechanism in that it stops
> >> the device appearing at all and hence repeated efforts to probe,
> >> plus a simple enable is less error prone, the way these SoCs are
> >> designed you don't have to pick which pinmux is in use for most of
> >> the IPs.  Where there are multiple options it does seem like a good
> >> approach though.
> >> 
> >> Tastes may differ though.
> > 
> > Right, if this SoC has only one pinmux setting for this IP, then it's
> > fine.
> 
> Yes. This IP has only default pin configuration.
> 
> > Padmavathi, this was the only issue I spotted, so have my:
> > 
> > Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> 
> Thanks for your review.

You're welcome.

Thanks for keeping up with this series and addressing all the comments. :)

Best regards,
Tomasz

[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers

[Tomasz Figa]

On Wednesday 14 of August 2013 11:21:40 Padma Venkat wrote:
> Hi Tomasz,
> 
> On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa <tomasz.figa@gmail.com> 
wrote:
> > On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> >> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> >> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> >> > > I'd expect that to interact badly with the pinmuxing - unless the
> >> > > device is disabled it'll try to grab its pins on probe which is
> >> > > not
> >> > > going to be a good idea unless it is actually wired up for use in
> >> > > the system.  Or is there some other mechanism for handling that?
> >> > 
> >> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> >> > considered board-specific and specified in board-level dts instead?
> >> 
> >> It seems a bit cleaner to use the current mechanism in that it stops
> >> the device appearing at all and hence repeated efforts to probe,
> >> plus a simple enable is less error prone, the way these SoCs are
> >> designed you don't have to pick which pinmux is in use for most of
> >> the IPs.  Where there are multiple options it does seem like a good
> >> approach though.
> >> 
> >> Tastes may differ though.
> > 
> > Right, if this SoC has only one pinmux setting for this IP, then it's
> > fine.
> 
> Yes. This IP has only default pin configuration.
> 
> > Padmavathi, this was the only issue I spotted, so have my:
> > 
> > Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> 
> Thanks for your review.

You're welcome.

Thanks for keeping up with this series and addressing all the comments. :)

Best regards,
Tomasz