[PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Sylwester Nawrocki]

This property is required for specifying link between the HDMI IP block
and the SoC's audio subsystem.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 56626d1a4235..71d2ede118e3 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -700,6 +700,7 @@
 					"sclk_hdmiphy", "mout_hdmi";
 			samsung,syscon-phandle = <&pmu_system_controller>;
 			phy = <&hdmiphy>;
+			#sound-dai-cells = <0>;
 			status = "disabled";
 		};
 
-- 
2.14.2

[PATCH 2/4] ARM: dts: Add audio clocks configuration for exynos5250-snow

[Sylwester Nawrocki]

Currently the audio subsystem clocks are not configured properly on Snow
and sound is not working. The MAX98095 CODEC is not getting its master clock
on the MCLK pin connected to the SOC's CLKOUT GPIO. This patch adds CLKOUT
and other clocks configuration so HDMI audio can also be supported.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos5250-snow-common.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index 59cf1b202849..bdf59dd3654b 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -225,6 +225,16 @@
 	};
 };
 
+&clock {
+	assigned-clocks = <&clock CLK_FOUT_EPLL>;
+	assigned-clock-rates = <49152000>;
+};
+
+&clock_audss {
+	assigned-clocks = <&clock_audss EXYNOS_MOUT_AUDSS>;
+	assigned-clock-parents = <&clock CLK_FOUT_EPLL>;
+};
+
 &cpu0 {
 	cpu0-supply = <&buck2_reg>;
 };
@@ -649,6 +659,11 @@
 	};
 };
 
+&pmu_system_controller {
+	assigned-clocks = <&pmu_system_controller 0>;
+	assigned-clock-parents = <&clock CLK_FIN_PLL>;
+};
+
 &rtc {
 	status = "okay";
 	clocks = <&clock CLK_RTC>, <&max77686 MAX77686_CLK_AP>;
-- 
2.14.2

[PATCH 3/4] ARM: dts: exynos5250-snow MAX98095 CODEC node update

[Sylwester Nawrocki]

This patch adds missing clocks, clock-names properties so the CODEC can
properly handle its master clock. Without this change sound on exynos5250-snow
doesn't work.
Missing #sound-dai-cells property is also added so it is possible to specify
the DAI links properly for HDMI audio support.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos5250-snow.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 4827cb506fa3..16e37c337a3f 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -27,6 +27,9 @@
 		reg = <0x11>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&max98095_en>;
+		clocks = <&pmu_system_controller 0>;
+		clock-names = "mclk";
+		#sound-dai-cells = <1>;
 	};
 };

--
2.14.2

[PATCH 4/4] ARM: dts: Enable HDMI audio on Snow Chromebook

[Sylwester Nawrocki]

This patch adds new cpu, codec subnodes according to the updated
"google,snow-audio-max98095" bindings and the I2S clock tree
configuration so sound on the HDMI interface can also be supported.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos5250-snow-common.dtsi | 4 ++++
 arch/arm/boot/dts/exynos5250-snow.dts         | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index bdf59dd3654b..932a00cfd716 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/clock/maxim,max77686.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/sound/samsung-i2s.h>
 #include "exynos5250.dtsi"

 / {
@@ -523,6 +524,9 @@
 };

 &i2s0 {
+	assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
+	assigned-clock-parents = <&clock_audss EXYNOS_I2S_BUS>;
+	#clock-cells = <1>;
 	status = "okay";
 };

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 16e37c337a3f..8c3f1573a994 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -18,6 +18,14 @@

 		samsung,model = "Snow-I2S-MAX98095";
 		samsung,audio-codec = <&max98095>;
+
+		cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		codec {
+			sound-dai = <&hdmi>, <&max98095 0>;
+		};
 	};
 };

--
2.14.2

Re: [PATCH 4/4] ARM: dts: Enable HDMI audio on Snow Chromebook

[Sylwester Nawrocki]

On 03/07/2018 06:27 PM, Sylwester Nawrocki wrote:
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -18,6 +18,14 @@

> +		cpu {
> +			sound-dai = <&i2s0 0>;
> +		};
> +
> +		codec {
> +			sound-dai = <&hdmi>, <&max98095 0>;

Oops, I need to change order of codecs on the list, so it matches
documentation. Will post v2 shortly.

-- 
Thanks,
Sylwester

[PATCH v2 4/4] ARM: dts: Enable HDMI audio on Snow Chromebook

[Sylwester Nawrocki]

This patch adds new cpu, codec subnodes according to the updated
"google,snow-audio-max98095" DT bindings and the I2S clock tree
configuration so sound on the HDMI interface can also be supported.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v2:
 - changed order of codec phandles in codec/cpu-dai property
   to match the DT bindings documentation.
---
 arch/arm/boot/dts/exynos5250-snow-common.dtsi | 4 ++++
 arch/arm/boot/dts/exynos5250-snow.dts         | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index bdf59dd3654b..932a00cfd716 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/clock/maxim,max77686.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/sound/samsung-i2s.h>
 #include "exynos5250.dtsi"

 / {
@@ -523,6 +524,9 @@
 };

 &i2s0 {
+	assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
+	assigned-clock-parents = <&clock_audss EXYNOS_I2S_BUS>;
+	#clock-cells = <1>;
 	status = "okay";
 };

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index 16e37c337a3f..75fdc5e6d423 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -18,6 +18,14 @@

 		samsung,model = "Snow-I2S-MAX98095";
 		samsung,audio-codec = <&max98095>;
+
+		cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		codec {
+			sound-dai = <&max98095 0>, <&hdmi>;
+		};
 	};
 };

--
2.14.2

Re: [PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Inki Dae]

Hi Sylwester,

2018년 03월 08일 02:27에 Sylwester Nawrocki 이(가) 쓴 글:
> This property is required for specifying link between the HDMI IP block
> and the SoC's audio subsystem.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 56626d1a4235..71d2ede118e3 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -700,6 +700,7 @@
>  					"sclk_hdmiphy", "mout_hdmi";
>  			samsung,syscon-phandle = <&pmu_system_controller>;
>  			phy = <&hdmiphy>;
> +			#sound-dai-cells = <0>;

This patch adds this property to hdmi device node which is bound by HDMI driver of Exynos DRM. As we talked about this at other email thread, seems this property is required mandatorily for Odroid XU3/4 board which uses Exynos5422.
There may be something I'm missing so could you let me know how this property is required?

Thanks,
Inki Dae

>  			status = "disabled";
>  		};
>  
> 

Re: [PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Sylwester Nawrocki]

Hi Inki,

On 03/09/2018 03:40 AM, Inki Dae wrote:
> 2018년 03월 08일 02:27에 Sylwester Nawrocki 이(가) 쓴 글:
>> This property is required for specifying link between the HDMI IP block
>> and the SoC's audio subsystem.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>>  arch/arm/boot/dts/exynos5250.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>> index 56626d1a4235..71d2ede118e3 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -700,6 +700,7 @@
>>  					"sclk_hdmiphy", "mout_hdmi";
>>  			samsung,syscon-phandle = <&pmu_system_controller>;
>>  			phy = <&hdmiphy>;
>> +			#sound-dai-cells = <0>;
> 
> This patch adds this property to hdmi device node which is bound by HDMI driver 
> of Exynos DRM. As we talked about this at other email thread, seems this property 
> is required mandatorily for Odroid XU3/4 board which uses Exynos5422.
> There may be something I'm missing so could you let me know how this property 
> is required?

It is required to properly interpret the list of DAI specified in the 'sound-dai'
property. If a device supports only one DAI it will have #sound-dai-cells = <0>
and the sound-dai property values in cpu<->codec link may look like this:

 cpu { ... }
 codec {
	 sound-dai = <&max98090>, <&hdmi>;
 };

When the CODEC support more DAIs it will have #sound-dai-cells = <1> and we can
additionally specify which DAI we exactly refer to, in this case DAI 0:

 cpu { ... }
 codec {
	sound-dai = <&max98095 0>, <&hdmi>;
 };

More details can be found in documentation of of_parse_phandle_with_args()
function.

The above refers to cpu/codec mapping like I2S0 -> [max9809x, hdmi], on Exynos5433
we have I2S0 -> wm5110 and I2S1 -> hdmi.


-- 
Regards,
Sylwester

Re: [PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Krzysztof Kozlowski]

On Wed, Mar 7, 2018 at 6:27 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> This property is required for specifying link between the HDMI IP block
> and the SoC's audio subsystem.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Subject prefix:
ARM: dts: exynos: Add..... to Exynos5250

Please fix it here and in other places. For arm64 prefix is slightly
different (arm64: dts: exynos:). I did not notice that in your last
patches and applied them but please take care about this in future.

In case you forget how the prefix should look like - run "git log
--oneline -- arch/arm/boot/dts/exynos*" or "git log --oneline --
arch/arm64/boot/dts/exynos/".

BR,
Krzysztof

Re: [PATCH v2 4/4] ARM: dts: Enable HDMI audio on Snow Chromebook

[Krzysztof Kozlowski]

On Thu, Mar 8, 2018 at 5:25 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> This patch adds new cpu, codec subnodes according to the updated
> "google,snow-audio-max98095" DT bindings and the I2S clock tree
> configuration so sound on the HDMI interface can also be supported.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v2:
>  - changed order of codec phandles in codec/cpu-dai property
>    to match the DT bindings documentation.
> ---
>  arch/arm/boot/dts/exynos5250-snow-common.dtsi | 4 ++++
>  arch/arm/boot/dts/exynos5250-snow.dts         | 8 ++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> index bdf59dd3654b..932a00cfd716 100644
> --- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/clock/maxim,max77686.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/input/input.h>
> +#include <dt-bindings/sound/samsung-i2s.h>
>  #include "exynos5250.dtsi"
>
>  / {
> @@ -523,6 +524,9 @@
>  };
>
>  &i2s0 {
> +       assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
> +       assigned-clock-parents = <&clock_audss EXYNOS_I2S_BUS>;
> +       #clock-cells = <1>;

Shouldn't this be property of exynos5250.dtsi? Along with clock-output-names?

BR,
Krzysztof

>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
> index 16e37c337a3f..75fdc5e6d423 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -18,6 +18,14 @@
>
>                 samsung,model = "Snow-I2S-MAX98095";
>                 samsung,audio-codec = <&max98095>;
> +
> +               cpu {
> +                       sound-dai = <&i2s0 0>;
> +               };
> +
> +               codec {
> +                       sound-dai = <&max98095 0>, <&hdmi>;
> +               };
>         };
>  };
>
> --
> 2.14.2
>

Re: [PATCH v2 4/4] ARM: dts: Enable HDMI audio on Snow Chromebook

[Sylwester Nawrocki]

On 03/09/2018 02:01 PM, Krzysztof Kozlowski wrote:
>> diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
>> index bdf59dd3654b..932a00cfd716 100644
>> --- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
>> @@ -9,6 +9,7 @@
>>  #include <dt-bindings/clock/maxim,max77686.h>
>>  #include <dt-bindings/interrupt-controller/irq.h>
>>  #include <dt-bindings/input/input.h>
>> +#include <dt-bindings/sound/samsung-i2s.h>
>>  #include "exynos5250.dtsi"
>>
>>  / {
>> @@ -523,6 +524,9 @@
>>  };
>>
>>  &i2s0 {
>> +       assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
>> +       assigned-clock-parents = <&clock_audss EXYNOS_I2S_BUS>;
>> +       #clock-cells = <1>;
>
> Shouldn't this be property of exynos5250.dtsi? Along with clock-output-names?

Hmm, it should be safe to move it there. By specifying this property or not
we can control whether the I2S clocks are exposed or not. If we move it to 
exynos5250.dtsi the clocks we always be exposed. But should be the right thing
to do anyway. I will post v3.

Of course we recently made the clock-output-names property deprecated, it will
not bee needed any more. It was there just to ensure the clk names are unique.

-- 
Regards,
Sylwester

Re: [PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Inki Dae]

Hi Sylwester,

2018년 03월 09일 20:52에 Sylwester Nawrocki 이(가) 쓴 글:
> Hi Inki,
> 
> On 03/09/2018 03:40 AM, Inki Dae wrote:
>> 2018년 03월 08일 02:27에 Sylwester Nawrocki 이(가) 쓴 글:
>>> This property is required for specifying link between the HDMI IP block
>>> and the SoC's audio subsystem.
>>>
>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos5250.dtsi | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>>> index 56626d1a4235..71d2ede118e3 100644
>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>> @@ -700,6 +700,7 @@
>>>  					"sclk_hdmiphy", "mout_hdmi";
>>>  			samsung,syscon-phandle = <&pmu_system_controller>;
>>>  			phy = <&hdmiphy>;
>>> +			#sound-dai-cells = <0>;
>>
>> This patch adds this property to hdmi device node which is bound by HDMI driver 
>> of Exynos DRM. As we talked about this at other email thread, seems this property 
>> is required mandatorily for Odroid XU3/4 board which uses Exynos5422.
>> There may be something I'm missing so could you let me know how this property 
>> is required?
> 
> It is required to properly interpret the list of DAI specified in the 'sound-dai'
> property. If a device supports only one DAI it will have #sound-dai-cells = <0>
> and the sound-dai property values in cpu<->codec link may look like this:
> 
>  cpu { ... }
>  codec {
> 	 sound-dai = <&max98090>, <&hdmi>;
>  };
> 
> When the CODEC support more DAIs it will have #sound-dai-cells = <1> and we can
> additionally specify which DAI we exactly refer to, in this case DAI 0:
> 
>  cpu { ... }
>  codec {
> 	sound-dai = <&max98095 0>, <&hdmi>;
>  };
> 
> More details can be found in documentation of of_parse_phandle_with_args()
> function.
> 
> The above refers to cpu/codec mapping like I2S0 -> [max9809x, hdmi], on Exynos5433
> we have I2S0 -> wm5110 and I2S1 -> hdmi.

Thanks for explanation. By the way, who binds '#sound-dai-cells' property?.
This patch adds this property to hdmi device node which will be bound by HDMI driver fo Exynos DRM but I don't see any place to bind '#sound-dai-cells' property in this driver.
So I think some driver like Odroid XU3/4 audio driver(sound/soc/samsung/odroid.c) or ASoC simple audio card driver(sound/soc/generic/simple-card.c) should exist and this property should be placed to such device node.

Thanks,
Inki Dae

> 
> 

Re: [PATCH 1/4] ARM: dts: exynos5250: Add #sound-dai-cells property to hdmi device node

[Sylwester Nawrocki]

Hi Inki,

On 03/12/2018 01:48 AM, Inki Dae wrote:
> Thanks for explanation. By the way, who binds '#sound-dai-cells' property?.
> This patch adds this property to hdmi device node which will be bound by 
> HDMI driver fo Exynos DRM but I don't see any place to bind '#sound-dai-cells' 
> property in this driver.
> So I think some driver like Odroid XU3/4 audio driver(sound/soc/samsung/odroid.c) 
> or ASoC simple audio card driver(sound/soc/generic/simple-card.c) should exist 
> and this property should be placed to such device node.

The *-cells property is supposed to be in a node that a phandle points to,
and in this case the phandle entry in 'sound-dai' points to the hdmi node.
>From the DT bindings perspective it not that important how Linux parses
the bindings, it's more a matter of describing the HW configuration.

There is even a related check in dtc:

$ grep -n sound scripts/dtc/checks.c
1208:WARNING_PROPERTY_PHANDLE_CELLS(sound_dai, "sound-dai", "#sound-dai-cells");

-- 
Regards,
Sylwester