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

From: Chanwoo Choi
Date: Wed Dec 18 2019 - 05:29:59 EST

Hi

2019ë 12ì 18ì (ì) ìí 7:18, Artur ÅwigoÅ <a.swigon@xxxxxxxxxxx>ëì ìì:
>
> 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@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.
>
> 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://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


>
> > 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,
> --
> Artur ÅwigoÅ
> Samsung R&D Institute Poland
> Samsung Electronics
>
>


--
Best Regards,
Chanwoo Choi