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

["Artur Świgoń" <a.swigon@samsung.com>]

On Wed, 2019-12-18 at 19:29 +0900, Chanwoo Choi wrote:
> Hi
> 
> 2019년 12월 18일 (수) 오후 7:18, Artur Świgoń <a.swigon@samsung.com>님이 작성:
> > 
> > Hi,
> > 
> > On Mon, 2019-12-16 at 11:59 +0900, Chanwoo Choi wrote:
> > > 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@partner.samsung.com>
> > > > > 
> > > > > 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@partner.samsung.com>
> > > > > ---
> > > > >  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.
> > 
> > OK, for v3 I will add 'exynos,interconnect-parent-node' to bus_dmc,
> > bus_leftbus and bus_display as you suggested below and change the code
> > so that the 'devfreq' (or the upcoming 'exynos,parent-bus') property is
> > not taken into account.
> 
> I'd like you to make the v3 based on my patches[1]
> [1]  https://protect2.fireeye.com/url?k=3fbd62a4-6276e59a-3fbce9eb-0cc47a31309a-5329151b98fc2653&u=https://lkml.org/lkml/2019/12/17/21
> - [PATCH 0/9] PM / devfreq: Remove deprecated 'devfreq' and
> 'devfreq-events' properties
> 
> I uploaded the patches to devfreq-testing branch[2]
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing

OK.

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

-- 
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics