Re: [RFC PATCH v2 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412

[Chanwoo Choi]

Hi,

On 12/16/19 9:51 AM, Chanwoo Choi wrote:
> On 9/19/19 11:22 PM, Artur ÅwigoÅ wrote:
>> From: Artur ÅwigoÅ <a.swigon@xxxxxxxxxxxxxxxxxxx>
>>
>> This patch adds two fields to the Exynos4412 DTS:
>>   - parent: to declare connections between nodes that are not in a
>>     parent-child relation in devfreq;
>>   - #interconnect-cells: required by the interconnect framework.
>>
>> Please note that #interconnect-cells is always zero and node IDs are not
>> hardcoded anywhere. The above-mentioned parent-child relation in devfreq
>> means that there is a shared power line ('devfreq' property). The 'parent'
>> property only signifies an interconnect connection.
>>
>> Signed-off-by: Artur ÅwigoÅ <a.swigon@xxxxxxxxxxxxxxxxxxx>
>> ---
>>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
>>  arch/arm/boot/dts/exynos4412.dtsi               | 9 +++++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>> index ea55f377d17c..bdd61ae86103 100644
>> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>> @@ -106,6 +106,7 @@
>>  &bus_leftbus {
>>  	devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
>>  	vdd-supply = <&buck3_reg>;
>> +	parent = <&bus_dmc>;
> 
> As I mentioned on other reply,
> I'm not sure to use the specific 'parent' property to make
> the connection between buses. If possible, you better to
> use the standard way like OF graph. Except for making
> the connection between buses by 'parent' property,
> looks good to me.

I tried to think it continuously. I withdraw the my opinion
using OF graph. If you make the property name like the following
example, it is possible for exynos.
- exynos,interconnect-parent-node = <&bus_dmc>; or other proper name.

Regardless of existing 'devfreq' property, I think you better to
make the connection between buses for only interconnect as following
example: This make it possible user can draw the correct tree by tracking
the 'exynos,interconnect-parent-node' value.

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ea55f377d17c..53f87f46e161 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -90,6 +90,7 @@
 &bus_dmc {
        devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
        vdd-supply = <&buck1_reg>;
+       #interconnect-cells = <0>;
        status = "okay";
 };
 
@@ -106,6 +107,8 @@
 &bus_leftbus {
        devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
        vdd-supply = <&buck3_reg>;
+       exynos,interconnect-parent-node = <&bus_dmc>;
+       #interconnect-cells = <0>;
        status = "okay";
 };
 
@@ -116,6 +119,8 @@
 
 &bus_display {
        devfreq = <&bus_leftbus>;
+       exynos,interconnect-parent-node = <&bus_leftbus>;
+       #interconnect-cells = <0>;
        status = "okay";
 };


> 
> 
>>  	status = "okay";
>>  };
>>  
>> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
>> index d20db2dfe8e2..a70a671acacd 100644
>> --- a/arch/arm/boot/dts/exynos4412.dtsi
>> +++ b/arch/arm/boot/dts/exynos4412.dtsi
>> @@ -390,6 +390,7 @@
>>  			clocks = <&clock CLK_DIV_DMC>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_dmc_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -398,6 +399,7 @@
>>  			clocks = <&clock CLK_DIV_ACP>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_acp_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -406,6 +408,7 @@
>>  			clocks = <&clock CLK_DIV_C2C>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_dmc_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -459,6 +462,7 @@
>>  			clocks = <&clock CLK_DIV_GDL>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_leftbus_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -467,6 +471,7 @@
>>  			clocks = <&clock CLK_DIV_GDR>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_leftbus_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -475,6 +480,7 @@
>>  			clocks = <&clock CLK_ACLK160>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_display_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -483,6 +489,7 @@
>>  			clocks = <&clock CLK_ACLK133>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_fsys_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -491,6 +498,7 @@
>>  			clocks = <&clock CLK_ACLK100>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_peri_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -499,6 +507,7 @@
>>  			clocks = <&clock CLK_SCLK_MFC>;
>>  			clock-names = "bus";
>>  			operating-points-v2 = <&bus_leftbus_opp_table>;
>> +			#interconnect-cells = <0>;
>>  			status = "disabled";
>>  		};
>>  
>>
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics