diff mbox

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

Message ID 20180307172711.15697-1-s.nawrocki@samsung.com (mailing list archive)
State Superseded
Headers show

Commit Message

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(+)

Comments

Inki Dae March 9, 2018, 2:40 a.m. UTC | #1
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";
>  		};
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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.
Krzysztof Kozlowski March 9, 2018, 12:54 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Inki Dae March 12, 2018, 12:48 a.m. UTC | #4
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

> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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");
diff mbox

Patch

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";
 		};