| Message ID | 20190919142236.4071-9-a.swigon@samsung.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | Simple QoS for exynos-bus driver using interconnect | expand |
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. > 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"; > }; > >
Hi, 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 patch, I'm not sure to use 'parent' property for this driver. If possible, we better to use the standard way like OF graph in order to make the tree between buses. Except for making the connection between the buses with 'parent', looks good to me. > 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"; > }; > >
Hi, Please ignore second reply. It is my mistake to send the duplicate message because of my company firewall issue. Regards, Chanwoo Choi On 12/16/19 9:55 AM, Chanwoo Choi wrote: > Hi, > > 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 patch, > I'm not sure to use 'parent' property for this driver. > If possible, we better to use the standard way like OF graph > in order to make the tree between buses. Except for making > the connection between the buses with 'parent', looks good to me. > >> 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"; >> }; >> >> > >
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. 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"; >> }; >> >> > >
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. > 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,
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://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
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"; > > > > > }; > > > > > > > > > >
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>; 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"; };