diff mbox

[v2,3/3] arm64: dts: exynos: Add initial configuration for DISP clocks for TM2/TM2e

Message ID 1485434274-6579-4-git-send-email-m.szyprowski@samsung.com (mailing list archive)
State Superseded
Headers show

Commit Message

Marek Szyprowski Jan. 26, 2017, 12:37 p.m. UTC
Add initial clock configuration for display subsystem for Exynos5433
based TM2/TM2e boards in device tree in order to avoid dependency on the
configuration left by the bootloader. This initial configuration is also
needed to ensure that display subsystem is operational if display power
domain gets turned off before clock controller is probed and the inital
clock configuration left by the bootloader saved.

TM2 and TM2e uses different rate for DISP PLL clock, but for better
maintainability all 'assigned-clocks-*' properties for DISP CMU are
defines in each board dts instead of redefining the rates property.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 12 ---------
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 29 ++++++++++++++++++++++
 arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts     | 29 ++++++++++++++++++++++
 3 files changed, 58 insertions(+), 12 deletions(-)

Comments

Chanwoo Choi Jan. 26, 2017, 2:49 p.m. UTC | #1
Hi Marek

2017-01-26 21:37 GMT+09:00 Marek Szyprowski <m.szyprowski@samsung.com>:
> Add initial clock configuration for display subsystem for Exynos5433
> based TM2/TM2e boards in device tree in order to avoid dependency on the
> configuration left by the bootloader. This initial configuration is also
> needed to ensure that display subsystem is operational if display power
> domain gets turned off before clock controller is probed and the inital
> clock configuration left by the bootloader saved.
>
> TM2 and TM2e uses different rate for DISP PLL clock, but for better
> maintainability all 'assigned-clocks-*' properties for DISP CMU are
> defines in each board dts instead of redefining the rates property.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 12 ---------
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 29 ++++++++++++++++++++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts     | 29 ++++++++++++++++++++++
>  3 files changed, 58 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 5c207575ed0a..1c1c03142e6d 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -217,18 +217,6 @@
>         assigned-clock-parents = <&cmu_top CLK_FOUT_AUD_PLL>;
>  };
>
> -&cmu_disp {
> -       assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> -                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> -                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> -                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> -       assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> -                                <0>,
> -                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> -                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> -       assigned-clock-rates = <0>, <400000000>;
> -};
> -
>  &cmu_fsys {
>         assigned-clocks = <&cmu_top CLK_MOUT_SCLK_USBDRD30>,
>                 <&cmu_top CLK_MOUT_SCLK_USBHOST30>,
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index ddba2f889326..b8bb053495af 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -18,6 +18,35 @@
>         compatible = "samsung,tm2", "samsung,exynos5433";
>  };
>
> +&cmu_disp {
> +       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> +                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
> +                         <&cmu_disp CLK_MOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> +       assigned-clock-parents = <0>, <0>,
> +                                <&cmu_mif CLK_ACLK_DISP_333>,
> +                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
> +                                <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> +                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> +       assigned-clock-rates = <250000000>, <400000000>;
> +};
> +
>  &hsi2c_9 {
>         status = "okay";
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> index d8bca75a1afe..c27500b7d8b5 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> @@ -18,6 +18,35 @@
>         compatible = "samsung,tm2e", "samsung,exynos5433";
>  };
>
> +&cmu_disp {
> +       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> +                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
> +                         <&cmu_disp CLK_MOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> +       assigned-clock-parents = <0>, <0>,
> +                                <&cmu_mif CLK_ACLK_DISP_333>,
> +                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
> +                                <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> +                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> +       assigned-clock-rates = <278000000>, <400000000>;

Except for setting the assigned-clock-rate for CLK_FOUT_DISP_PLL,
tm2.dts and tm2e.dts has the same dt node of cmu_disp.
If there is same value between tm2 and tm2e, you should keep the same value
in exynos5433-tm2-common.dtsi.

So, I think that exynos5433-tm2-common.dtsi include the relationships
between clocks and parent clocks as following without assigning the clocks.

And then, each tm2 and tm2e.dts can assign the clock rate for CLK_FOUT_DISP_PLL
and CLK_DIV_SCLK_DECON_TV_ECLK.

For example,
In exynos5433-tm2-common.dtsi

&cmu_disp {
       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
                         <&cmu_disp CLK_MOUT_DISP_PLL>,
                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
       assigned-clock-parents = <0>, <0>,
                                <&cmu_mif CLK_ACLK_DISP_333>,
                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
                                <&cmu_disp CLK_FOUT_DISP_PLL>,
                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
    };


In exynos5433-tm2.dts
&cmu_disp {
     assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
                                   <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
     assigned-clock-rates = <250000000>, <400000000>;
};


In exynos5433-tm2e.dts
&cmu_disp {
     assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
                                  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
     assigned-clock-rates = <278000000>, <400000000>;
};
Andrzej Hajda Jan. 26, 2017, 3:20 p.m. UTC | #2
On 26.01.2017 15:49, Chanwoo Choi wrote:
> Hi Marek
>
> 2017-01-26 21:37 GMT+09:00 Marek Szyprowski <m.szyprowski@samsung.com>:
>> Add initial clock configuration for display subsystem for Exynos5433
>> based TM2/TM2e boards in device tree in order to avoid dependency on the
>> configuration left by the bootloader. This initial configuration is also
>> needed to ensure that display subsystem is operational if display power
>> domain gets turned off before clock controller is probed and the inital
>> clock configuration left by the bootloader saved.
>>
>> TM2 and TM2e uses different rate for DISP PLL clock, but for better
>> maintainability all 'assigned-clocks-*' properties for DISP CMU are
>> defines in each board dts instead of redefining the rates property.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 12 ---------
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      | 29 ++++++++++++++++++++++
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts     | 29 ++++++++++++++++++++++
>>  3 files changed, 58 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index 5c207575ed0a..1c1c03142e6d 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -217,18 +217,6 @@
>>         assigned-clock-parents = <&cmu_top CLK_FOUT_AUD_PLL>;
>>  };
>>
>> -&cmu_disp {
>> -       assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
>> -                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
>> -                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
>> -                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
>> -       assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
>> -                                <0>,
>> -                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
>> -                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
>> -       assigned-clock-rates = <0>, <400000000>;
>> -};
>> -
>>  &cmu_fsys {
>>         assigned-clocks = <&cmu_top CLK_MOUT_SCLK_USBDRD30>,
>>                 <&cmu_top CLK_MOUT_SCLK_USBHOST30>,
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index ddba2f889326..b8bb053495af 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -18,6 +18,35 @@
>>         compatible = "samsung,tm2", "samsung,exynos5433";
>>  };
>>
>> +&cmu_disp {
>> +       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
>> +                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
>> +                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
>> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
>> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
>> +                         <&cmu_disp CLK_MOUT_DISP_PLL>,
>> +                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
>> +       assigned-clock-parents = <0>, <0>,
>> +                                <&cmu_mif CLK_ACLK_DISP_333>,
>> +                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>> +                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
>> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
>> +                                <&cmu_disp CLK_FOUT_DISP_PLL>,
>> +                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
>> +                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
>> +       assigned-clock-rates = <250000000>, <400000000>;
>> +};
>> +
>>  &hsi2c_9 {
>>         status = "okay";
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
>> index d8bca75a1afe..c27500b7d8b5 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
>> @@ -18,6 +18,35 @@
>>         compatible = "samsung,tm2e", "samsung,exynos5433";
>>  };
>>
>> +&cmu_disp {
>> +       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
>> +                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
>> +                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
>> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
>> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
>> +                         <&cmu_disp CLK_MOUT_DISP_PLL>,
>> +                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
>> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
>> +       assigned-clock-parents = <0>, <0>,
>> +                                <&cmu_mif CLK_ACLK_DISP_333>,
>> +                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>> +                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
>> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
>> +                                <&cmu_disp CLK_FOUT_DISP_PLL>,
>> +                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
>> +                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
>> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
>> +       assigned-clock-rates = <278000000>, <400000000>;
> Except for setting the assigned-clock-rate for CLK_FOUT_DISP_PLL,
> tm2.dts and tm2e.dts has the same dt node of cmu_disp.
> If there is same value between tm2 and tm2e, you should keep the same value
> in exynos5433-tm2-common.dtsi.
>
> So, I think that exynos5433-tm2-common.dtsi include the relationships
> between clocks and parent clocks as following without assigning the clocks.
>
> And then, each tm2 and tm2e.dts can assign the clock rate for CLK_FOUT_DISP_PLL
> and CLK_DIV_SCLK_DECON_TV_ECLK.
>
> For example,
> In exynos5433-tm2-common.dtsi
>
> &cmu_disp {
>        assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
>                          <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
>                          <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
>                          <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>                          <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
>                          <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>                          <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
>                          <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
>                          <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
>                          <&cmu_disp CLK_MOUT_DISP_PLL>,
>                          <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
>                          <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
>                          <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
>        assigned-clock-parents = <0>, <0>,
>                                 <&cmu_mif CLK_ACLK_DISP_333>,
>                                 <&cmu_mif CLK_SCLK_DSIM0_DISP>,
>                                 <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
>                                 <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
>                                 <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
>                                 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
>                                 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
>                                 <&cmu_disp CLK_FOUT_DISP_PLL>,
>                                 <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
>                                 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
>                                 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
>     };
>
>
> In exynos5433-tm2.dts
> &cmu_disp {
>      assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
>                                    <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
>      assigned-clock-rates = <250000000>, <400000000>;
> };
>
>
> In exynos5433-tm2e.dts
> &cmu_disp {
>      assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
>                                   <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
>      assigned-clock-rates = <278000000>, <400000000>;
> };
>
I guess in such case common properties will be overwritten.

If one really want to put as much as possible into common part it could
be done this way:

exynos5433-tm2.dts before '#include "exynos5433-tm2-common.dtsi"':

#define CLK_FOUT_DISP_PLL_RATE 250000000

exynos5433-tm2e.dts before '#include "exynos5433-tm2-common.dtsi"':

#define CLK_FOUT_DISP_PLL_RATE 278000000

and in exynos5433-tm2-common.dtsi:

+&cmu_disp {
+       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
+                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
+                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
+                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
+                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
+                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
+                         <&cmu_disp CLK_MOUT_DISP_PLL>,
+                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+       assigned-clock-parents = <0>, <0>,
+                                <&cmu_mif CLK_ACLK_DISP_333>,
+                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
+                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
+                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
+                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
+                                <&cmu_disp CLK_FOUT_DISP_PLL>,
+                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+       assigned-clock-rates = <CLK_FOUT_DISP_PLL_RATE>, <400000000>;
+};
+

The question is if such macro games will be accepted? :)

Regards

Andrzej


--
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
Krzysztof Kozlowski Jan. 26, 2017, 7:35 p.m. UTC | #3
On Thu, Jan 26, 2017 at 04:20:22PM +0100, Andrzej Hajda wrote:
> On 26.01.2017 15:49, Chanwoo Choi wrote:
> > Hi Marek
> >

(...)

> >> +       assigned-clock-rates = <278000000>, <400000000>;
> > Except for setting the assigned-clock-rate for CLK_FOUT_DISP_PLL,
> > tm2.dts and tm2e.dts has the same dt node of cmu_disp.
> > If there is same value between tm2 and tm2e, you should keep the same value
> > in exynos5433-tm2-common.dtsi.
> >
> > So, I think that exynos5433-tm2-common.dtsi include the relationships
> > between clocks and parent clocks as following without assigning the clocks.
> >
> > And then, each tm2 and tm2e.dts can assign the clock rate for CLK_FOUT_DISP_PLL
> > and CLK_DIV_SCLK_DECON_TV_ECLK.
> >
> > For example,
> > In exynos5433-tm2-common.dtsi
> >
> > &cmu_disp {
> >        assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> >                          <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> >                          <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
> >                          <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
> >                          <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
> >                          <&cmu_disp CLK_MOUT_DISP_PLL>,
> >                          <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> >                          <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> >        assigned-clock-parents = <0>, <0>,
> >                                 <&cmu_mif CLK_ACLK_DISP_333>,
> >                                 <&cmu_mif CLK_SCLK_DSIM0_DISP>,
> >                                 <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> >                                 <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
> >                                 <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> >                                 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
> >                                 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
> >                                 <&cmu_disp CLK_FOUT_DISP_PLL>,
> >                                 <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> >                                 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> >                                 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> >     };
> >
> >
> > In exynos5433-tm2.dts
> > &cmu_disp {
> >      assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> >                                    <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
> >      assigned-clock-rates = <250000000>, <400000000>;
> > };
> >
> >
> > In exynos5433-tm2e.dts
> > &cmu_disp {
> >      assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> >                                   <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>;
> >      assigned-clock-rates = <278000000>, <400000000>;
> > };
> >
> I guess in such case common properties will be overwritten.
> 

Indeed.

> If one really want to put as much as possible into common part it could
> be done this way:
> 
> exynos5433-tm2.dts before '#include "exynos5433-tm2-common.dtsi"':
> 
> #define CLK_FOUT_DISP_PLL_RATE 250000000
> 
> exynos5433-tm2e.dts before '#include "exynos5433-tm2-common.dtsi"':
> 
> #define CLK_FOUT_DISP_PLL_RATE 278000000
> 
> and in exynos5433-tm2-common.dtsi:
> 
> +&cmu_disp {
> +       assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> +                         <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
> +                         <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
> +                         <&cmu_disp CLK_MOUT_DISP_PLL>,
> +                         <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> +                         <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> +       assigned-clock-parents = <0>, <0>,
> +                                <&cmu_mif CLK_ACLK_DISP_333>,
> +                                <&cmu_mif CLK_SCLK_DSIM0_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
> +                                <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
> +                                <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
> +                                <&cmu_disp CLK_FOUT_DISP_PLL>,
> +                                <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> +                                <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> +                                <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> +       assigned-clock-rates = <CLK_FOUT_DISP_PLL_RATE>, <400000000>;
> +};
> +
> 
> The question is if such macro games will be accepted? :)

Interesting idea... The DTSI would look nice but the real problem is
hidden by a dependency on #define. In the same time one would have two
definitions of this value - when looking at DTSI, one would have to
look at DTS as well. This would be rather a new way of overriding
things (reading DTSI and then jumping to DTS) and I prefer
consistency...

assigned-clocks and assigned-clock-parents are the same, right? Only
assigned-clock-rates differ?

Ideas:
1. Make common assigned-clocks+assigned-clock-parents in DTSI and customize
   only the rates in DTS.
   Mention the reason behind the split for assigned-* properties in a
   comment in all files (common + boards).  Anyone looking at final DTS
   will find that assigned-clocks/parents are somewhere else (and
   vice-versa). Also anyone looking at DTSI will see a comment that one
   part is missing - rates are somewhere else.

2. Duplicate the nodes like it is in Marek's patch but please mention in
   a comment that assigned-clocks+assigned-clock-parents are expected to be
   the same between boards.
   The goal behind the comment is to make reader's life easier:
    - no need to figure out why this is duplicated,
    - if there would be a mistake (difference between boards), then
      easily understand that this was a mistake, not a feature.

The benefit of Andrzej's approach is that there will be compile time
dependency - missing define causes build failure.  In my approach, there
will be only comments. Anyway in all cases the reader have to look at
both DTSI and DTS to find the final answer.

What do you think?

Best regards,
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
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 5c207575ed0a..1c1c03142e6d 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -217,18 +217,6 @@ 
 	assigned-clock-parents = <&cmu_top CLK_FOUT_AUD_PLL>;
 };
 
-&cmu_disp {
-	assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
-			  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
-			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
-			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
-	assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
-				 <0>,
-				 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
-				 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
-	assigned-clock-rates = <0>, <400000000>;
-};
-
 &cmu_fsys {
 	assigned-clocks = <&cmu_top CLK_MOUT_SCLK_USBDRD30>,
 		<&cmu_top CLK_MOUT_SCLK_USBHOST30>,
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index ddba2f889326..b8bb053495af 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -18,6 +18,35 @@ 
 	compatible = "samsung,tm2", "samsung,exynos5433";
 };
 
+&cmu_disp {
+	assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
+			  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+			  <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
+			  <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
+			  <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
+			  <&cmu_disp CLK_MOUT_DISP_PLL>,
+			  <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+	assigned-clock-parents = <0>, <0>,
+				 <&cmu_mif CLK_ACLK_DISP_333>,
+				 <&cmu_mif CLK_SCLK_DSIM0_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+				 <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+				 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
+				 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
+				 <&cmu_disp CLK_FOUT_DISP_PLL>,
+				 <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+	assigned-clock-rates = <250000000>, <400000000>;
+};
+
 &hsi2c_9 {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
index d8bca75a1afe..c27500b7d8b5 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
@@ -18,6 +18,35 @@ 
 	compatible = "samsung,tm2e", "samsung,exynos5433";
 };
 
+&cmu_disp {
+	assigned-clocks = <&cmu_disp CLK_FOUT_DISP_PLL>,
+			  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+			  <&cmu_disp CLK_MOUT_ACLK_DISP_333_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DSIM0>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK>,
+			  <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_RXCLKESC0_USER>,
+			  <&cmu_disp CLK_MOUT_PHYCLK_MIPIDPHY0_BITCLKDIV8_USER>,
+			  <&cmu_disp CLK_MOUT_DISP_PLL>,
+			  <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+	assigned-clock-parents = <0>, <0>,
+				 <&cmu_mif CLK_ACLK_DISP_333>,
+				 <&cmu_mif CLK_SCLK_DSIM0_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DSIM0_USER>,
+				 <&cmu_mif CLK_SCLK_DECON_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_ECLK_USER>,
+				 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0_PHY>,
+				 <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8_PHY>,
+				 <&cmu_disp CLK_FOUT_DISP_PLL>,
+				 <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+	assigned-clock-rates = <278000000>, <400000000>;
+};
+
 &ldo31_reg {
 	regulator-name = "TSP_VDD_1.8V_AP";
 	regulator-min-microvolt = <1800000>;