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

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

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 
>>> ---
>>>  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 = <_system_controller>;
>>> phy = <>;
>>> +   #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 = <>, <>;
>  };
> 
> 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 = < 0>, <>;
>  };
> 
> 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

[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 
>>> ---
>>>  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 = <_system_controller>;
>>> phy = <>;
>>> +   #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 = <>, <>;
>  };
> 
> 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 = < 0>, <>;
>  };
> 
> 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

[Krzysztof Kozlowski]

On Wed, Mar 7, 2018 at 6:27 PM, Sylwester Nawrocki
 wrote:
> This property is required for specifying link between the HDMI IP block
> and the SoC's audio subsystem.
>
> Signed-off-by: Sylwester Nawrocki 
> ---
>  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 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
 wrote:
> This property is required for specifying link between the HDMI IP block
> and the SoC's audio subsystem.
>
> Signed-off-by: Sylwester Nawrocki 
> ---
>  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 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 
>> ---
>>  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 = <_system_controller>;
>>  phy = <>;
>> +#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 = <>, <>;
 };

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 = < 0>, <>;
 };

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

[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 
>> ---
>>  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 = <_system_controller>;
>>  phy = <>;
>> +#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 = <>, <>;
 };

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 = < 0>, <>;
 };

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

[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 
> ---
>  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 = <_system_controller>;
>   phy = <>;
> + #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

[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 
> ---
>  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 = <_system_controller>;
>   phy = <>;
> + #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";
>   };
>  
> 

[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 
---
 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 = <_system_controller>;
phy = <>;
+   #sound-dai-cells = <0>;
status = "disabled";
};
 
-- 
2.14.2

[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 
---
 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 = <_system_controller>;
phy = <>;
+   #sound-dai-cells = <0>;
status = "disabled";
};
 
-- 
2.14.2