linux-samsung-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Exynos5422: fix bus related OPPs for Odroid XU3/XU4/HC1
       [not found] <CGME20191219082938eucas1p254fe738574f287a44630d5a7eef7385e@eucas1p2.samsung.com>
@ 2019-12-19  8:29 ` Marek Szyprowski
       [not found]   ` <CGME20191219082939eucas1p248ca8b95ede6f2704b83515f461f6927@eucas1p2.samsung.com>
       [not found]   ` <CGME20191219082939eucas1p2afc32535df1512dc21ca983daa012568@eucas1p2.samsung.com>
  0 siblings, 2 replies; 7+ messages in thread
From: Marek Szyprowski @ 2019-12-19  8:29 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-pm
  Cc: Marek Szyprowski, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Kamil Konieczny, Chanwoo Choi

Dear All,

Currently the only Exynos5422-based boards that support bus frequency
scaling are Hardkernel's Odroid XU3/XU4/HC1. The recent changes in the
devfreq framework revealed that some operating points for the defined
busses cannot be applied, because the rates defined in the OPPs cannot
be derived from the top PLL clocks (due to lack of common integer
dividers). This issue has been first noticed by Lukasz Luba in:
https://lkml.org/lkml/2019/7/15/276

To use the rates currently defined in the OPPs, one would need to change
the rate and the topology of the top PLL clocks. The best place for such
operation is the bootloader, because later when kernel boots, more and
more devices (like UART, MMC, and so on) are enabled and get the clocks
from those top PLLs. Changing the rate of the clock for the already
enabled/operating device is very tricky.

To avoid that issue I've decided to keep the current top PLL clocks
configuration prepared by the bootloader on Odroid XU3/XU4/HC1 boards and
adjust the OPPs for it. This means that the bus related OPPs are board
dependant, so I've moved the to the respective DTS files. For other
boards (for example Peach Pi/Pit Chromebooks), slightly different OPPs
might need to be defined due to different clock topology and top PLLs
rates configured by their bootloader.

The provided approach is probably the simplest fix to let all busses
operate on the highest possible speeds, which match the configuration
applied initially by the bootloader.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (2):
  ARM: dts: exynos: Move bus related OPPs to the boards DTS
  ARM: dts: exynos: Adjust bus related OPPs to the values correct for
    Odroids

 arch/arm/boot/dts/exynos5420.dtsi             | 259 ----------------
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 276 +++++++++++++++++-
 2 files changed, 275 insertions(+), 260 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] ARM: dts: exynos: Move bus related OPPs to the boards DTS
       [not found]   ` <CGME20191219082939eucas1p248ca8b95ede6f2704b83515f461f6927@eucas1p2.samsung.com>
@ 2019-12-19  8:29     ` Marek Szyprowski
  2019-12-19  9:02       ` Chanwoo Choi
  0 siblings, 1 reply; 7+ messages in thread
From: Marek Szyprowski @ 2019-12-19  8:29 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-pm
  Cc: Marek Szyprowski, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Kamil Konieczny, Chanwoo Choi

Currently the only Exynos5422-based boards that support bus frequency
scaling are Hardkernel's Odroid XU3/XU4/HC1. Move the bus related OPPs to
the boards DTS, because those OPPs heavily depend on the clock topology
and top PLL rates, which are being configured by the board's bootloader.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi             | 259 -----------------
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 261 +++++++++++++++++-
 2 files changed, 260 insertions(+), 260 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index f95567bc10e3..f66a2d1b3428 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -1092,7 +1092,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_wcore_opp_table>;
 			status = "disabled";
 		};
 
@@ -1100,7 +1099,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK100_NOC>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_noc_opp_table>;
 			status = "disabled";
 		};
 
@@ -1108,7 +1106,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_fsys_apb_opp_table>;
 			status = "disabled";
 		};
 
@@ -1116,7 +1113,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_fsys_apb_opp_table>;
 			status = "disabled";
 		};
 
@@ -1124,7 +1120,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_fsys2_opp_table>;
 			status = "disabled";
 		};
 
@@ -1132,7 +1127,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK333>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_mfc_opp_table>;
 			status = "disabled";
 		};
 
@@ -1140,7 +1134,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK266>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_gen_opp_table>;
 			status = "disabled";
 		};
 
@@ -1148,7 +1141,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK66>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_peri_opp_table>;
 			status = "disabled";
 		};
 
@@ -1156,7 +1148,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK333_G2D>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_g2d_opp_table>;
 			status = "disabled";
 		};
 
@@ -1164,7 +1155,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK266_G2D>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_g2d_acp_opp_table>;
 			status = "disabled";
 		};
 
@@ -1172,7 +1162,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_jpeg_opp_table>;
 			status = "disabled";
 		};
 
@@ -1180,7 +1169,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK166>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_jpeg_apb_opp_table>;
 			status = "disabled";
 		};
 
@@ -1188,7 +1176,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_disp1_fimd_opp_table>;
 			status = "disabled";
 		};
 
@@ -1196,7 +1183,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_disp1_opp_table>;
 			status = "disabled";
 		};
 
@@ -1204,7 +1190,6 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_gscl_opp_table>;
 			status = "disabled";
 		};
 
@@ -1212,252 +1197,8 @@
 			compatible = "samsung,exynos-bus";
 			clocks = <&clock CLK_DOUT_ACLK400_MSCL>;
 			clock-names = "bus";
-			operating-points-v2 = <&bus_mscl_opp_table>;
 			status = "disabled";
 		};
-
-		bus_wcore_opp_table: opp_table2 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <84000000>;
-				opp-microvolt = <925000 925000 1400000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <111000000>;
-				opp-microvolt = <950000 950000 1400000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <222000000>;
-				opp-microvolt = <950000 950000 1400000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <333000000>;
-				opp-microvolt = <950000 950000 1400000>;
-			};
-			opp04 {
-				opp-hz = /bits/ 64 <400000000>;
-				opp-microvolt = <987500 987500 1400000>;
-			};
-		};
-
-		bus_noc_opp_table: opp_table3 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <67000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <75000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <86000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <100000000>;
-			};
-		};
-
-		bus_fsys_apb_opp_table: opp_table4 {
-			compatible = "operating-points-v2";
-			opp-shared;
-
-			opp00 {
-				opp-hz = /bits/ 64 <100000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <200000000>;
-			};
-		};
-
-		bus_fsys2_opp_table: opp_table5 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <75000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <100000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <150000000>;
-			};
-		};
-
-		bus_mfc_opp_table: opp_table6 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <96000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <111000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <167000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <222000000>;
-			};
-			opp04 {
-				opp-hz = /bits/ 64 <333000000>;
-			};
-		};
-
-		bus_gen_opp_table: opp_table7 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <89000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <133000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <178000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <267000000>;
-			};
-		};
-
-		bus_peri_opp_table: opp_table8 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <67000000>;
-			};
-		};
-
-		bus_g2d_opp_table: opp_table9 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <84000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <167000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <222000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <300000000>;
-			};
-			opp04 {
-				opp-hz = /bits/ 64 <333000000>;
-			};
-		};
-
-		bus_g2d_acp_opp_table: opp_table10 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <67000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <133000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <178000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <267000000>;
-			};
-		};
-
-		bus_jpeg_opp_table: opp_table11 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <75000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <150000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <200000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <300000000>;
-			};
-		};
-
-		bus_jpeg_apb_opp_table: opp_table12 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <84000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <111000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <134000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <167000000>;
-			};
-		};
-
-		bus_disp1_fimd_opp_table: opp_table13 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <120000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <200000000>;
-			};
-		};
-
-		bus_disp1_opp_table: opp_table14 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <120000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <200000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <300000000>;
-			};
-		};
-
-		bus_gscl_opp_table: opp_table15 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <150000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <200000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <300000000>;
-			};
-		};
-
-		bus_mscl_opp_table: opp_table16 {
-			compatible = "operating-points-v2";
-
-			opp00 {
-				opp-hz = /bits/ 64 <84000000>;
-			};
-			opp01 {
-				opp-hz = /bits/ 64 <167000000>;
-			};
-			opp02 {
-				opp-hz = /bits/ 64 <222000000>;
-			};
-			opp03 {
-				opp-hz = /bits/ 64 <333000000>;
-			};
-			opp04 {
-				opp-hz = /bits/ 64 <400000000>;
-			};
-		};
 	};
 
 	thermal-zones {
diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
index 80b0acfec547..663a38d53c9e 100644
--- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
@@ -35,7 +35,250 @@
 		};
 	};
 
-	dmc_opp_table: opp_table2 {
+	bus_wcore_opp_table: opp_table2 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <84000000>;
+			opp-microvolt = <925000 925000 1400000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <111000000>;
+			opp-microvolt = <950000 950000 1400000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <222000000>;
+			opp-microvolt = <950000 950000 1400000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <333000000>;
+			opp-microvolt = <950000 950000 1400000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <987500 987500 1400000>;
+		};
+	};
+
+	bus_noc_opp_table: opp_table3 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <67000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <75000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <86000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <100000000>;
+		};
+	};
+
+	bus_fsys_apb_opp_table: opp_table4 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp00 {
+			opp-hz = /bits/ 64 <100000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <200000000>;
+		};
+	};
+
+	bus_fsys2_opp_table: opp_table5 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <75000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <100000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <150000000>;
+		};
+	};
+
+	bus_mfc_opp_table: opp_table6 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <96000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <111000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <167000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <222000000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <333000000>;
+		};
+	};
+
+	bus_gen_opp_table: opp_table7 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <89000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <133000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <178000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <267000000>;
+		};
+	};
+
+	bus_peri_opp_table: opp_table8 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <67000000>;
+		};
+	};
+
+	bus_g2d_opp_table: opp_table9 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <84000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <167000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <222000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <300000000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <333000000>;
+		};
+	};
+
+	bus_g2d_acp_opp_table: opp_table10 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <67000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <133000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <178000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <267000000>;
+		};
+	};
+
+	bus_jpeg_opp_table: opp_table11 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <75000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <150000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <200000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <300000000>;
+		};
+	};
+
+	bus_jpeg_apb_opp_table: opp_table12 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <84000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <111000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <134000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <167000000>;
+		};
+	};
+
+	bus_disp1_fimd_opp_table: opp_table13 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <120000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <200000000>;
+		};
+	};
+
+	bus_disp1_opp_table: opp_table14 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <120000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <200000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <300000000>;
+		};
+	};
+
+	bus_gscl_opp_table: opp_table15 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <150000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <200000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <300000000>;
+		};
+	};
+
+	bus_mscl_opp_table: opp_table16 {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <84000000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <167000000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <222000000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <333000000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <400000000>;
+		};
+	};
+
+	dmc_opp_table: opp_table18 {
 		compatible = "operating-points-v2";
 
 		opp00 {
@@ -134,6 +377,7 @@
 };
 
 &bus_wcore {
+	operating-points-v2 = <&bus_wcore_opp_table>;
 	devfreq-events = <&nocp_mem0_0>, <&nocp_mem0_1>,
 			<&nocp_mem1_0>, <&nocp_mem1_1>;
 	vdd-supply = <&buck3_reg>;
@@ -142,76 +386,91 @@
 };
 
 &bus_noc {
+	operating-points-v2 = <&bus_noc_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_fsys_apb {
+	operating-points-v2 = <&bus_fsys_apb_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_fsys {
+	operating-points-v2 = <&bus_fsys_apb_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_fsys2 {
+	operating-points-v2 = <&bus_fsys2_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_mfc {
+	operating-points-v2 = <&bus_mfc_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_gen {
+	operating-points-v2 = <&bus_gen_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_peri {
+	operating-points-v2 = <&bus_peri_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_g2d {
+	operating-points-v2 = <&bus_g2d_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_g2d_acp {
+	operating-points-v2 = <&bus_g2d_acp_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_jpeg {
+	operating-points-v2 = <&bus_jpeg_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_jpeg_apb {
+	operating-points-v2 = <&bus_jpeg_apb_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_disp1_fimd {
+	operating-points-v2 = <&bus_disp1_fimd_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_disp1 {
+	operating-points-v2 = <&bus_disp1_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_gscl_scaler {
+	operating-points-v2 = <&bus_gscl_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
 
 &bus_mscl {
+	operating-points-v2 = <&bus_mscl_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids
       [not found]   ` <CGME20191219082939eucas1p2afc32535df1512dc21ca983daa012568@eucas1p2.samsung.com>
@ 2019-12-19  8:29     ` Marek Szyprowski
  2019-12-19  9:07       ` Chanwoo Choi
  0 siblings, 1 reply; 7+ messages in thread
From: Marek Szyprowski @ 2019-12-19  8:29 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-pm
  Cc: Marek Szyprowski, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Kamil Konieczny, Chanwoo Choi

Hardkernel's Odroid XU3/XU4/HC1 boards use bootloader, which configures top
PLLs to the following values: MPLL: 532MHz, CPLL: 666MHz and DPLL: 600MHz.

Adjust all bus related OPPs to the values that are possible to derive from
the top PLL configured by the bootloader. Also add a comment for each bus
describing which PLL is used for it.

The most significant change is the highest rate for wcore bus. It has been
increased to 532MHz as this is the value configured initially by the
bootloader. Also the voltage for this OPP is changed to match the value
set by the bootloader.

This patch finally allows the buses to operate on the rates matching the
values set for each OPP and fixes the following warnings observed on boot:

exynos-bus: new bus device registered: soc:bus_wcore ( 84000 KHz ~ 400000 KHz)
exynos-bus: new bus device registered: soc:bus_noc ( 67000 KHz ~ 100000 KHz)
exynos-bus: new bus device registered: soc:bus_fsys_apb (100000 KHz ~ 200000 KHz)
...
exynos-bus soc:bus_wcore: dev_pm_opp_set_rate: failed to find current OPP for freq 532000000 (-34)
exynos-bus soc:bus_noc: dev_pm_opp_set_rate: failed to find current OPP for freq 111000000 (-34)
exynos-bus soc:bus_fsys_apb: dev_pm_opp_set_rate: failed to find current OPP for freq 222000000 (-34)

The problem with setting incorrect (in some cases much lower) clock rate
for the defined OPP were there from the beginning, but went unnoticed
because the only way to observe it was to manually check the rate of the
respective clocks. The commit 4294a779bd8d ("PM / devfreq: exynos-bus:
Convert to use dev_pm_opp_set_rate()") finally revealed it, because it
enabled use of the generic code from the OPP framework, which issues the
above mentioned warnings.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 75 +++++++++++--------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
index 663a38d53c9e..b6d6022e8735 100644
--- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
@@ -38,42 +38,44 @@
 	bus_wcore_opp_table: opp_table2 {
 		compatible = "operating-points-v2";
 
+		/* derived from 532MHz MPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <84000000>;
+			opp-hz = /bits/ 64 <88700000>;
 			opp-microvolt = <925000 925000 1400000>;
 		};
 		opp01 {
-			opp-hz = /bits/ 64 <111000000>;
+			opp-hz = /bits/ 64 <133000000>;
 			opp-microvolt = <950000 950000 1400000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <222000000>;
+			opp-hz = /bits/ 64 <177400000>;
 			opp-microvolt = <950000 950000 1400000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <333000000>;
+			opp-hz = /bits/ 64 <266000000>;
 			opp-microvolt = <950000 950000 1400000>;
 		};
 		opp04 {
-			opp-hz = /bits/ 64 <400000000>;
-			opp-microvolt = <987500 987500 1400000>;
+			opp-hz = /bits/ 64 <532000000>;
+			opp-microvolt = <1000000 1000000 1400000>;
 		};
 	};
 
 	bus_noc_opp_table: opp_table3 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <67000000>;
+			opp-hz = /bits/ 64 <66600000>;
 		};
 		opp01 {
-			opp-hz = /bits/ 64 <75000000>;
+			opp-hz = /bits/ 64 <74000000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <86000000>;
+			opp-hz = /bits/ 64 <83250000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <100000000>;
+			opp-hz = /bits/ 64 <111000000>;
 		};
 	};
 
@@ -81,39 +83,42 @@
 		compatible = "operating-points-v2";
 		opp-shared;
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <100000000>;
+			opp-hz = /bits/ 64 <111000000>;
 		};
 		opp01 {
-			opp-hz = /bits/ 64 <200000000>;
+			opp-hz = /bits/ 64 <222000000>;
 		};
 	};
 
 	bus_fsys2_opp_table: opp_table5 {
 		compatible = "operating-points-v2";
 
+		/* derived from 600MHz DPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <75000000>;
 		};
 		opp01 {
-			opp-hz = /bits/ 64 <100000000>;
+			opp-hz = /bits/ 64 <120000000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <150000000>;
+			opp-hz = /bits/ 64 <200000000>;
 		};
 	};
 
 	bus_mfc_opp_table: opp_table6 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <96000000>;
+			opp-hz = /bits/ 64 <83250000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <111000000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <167000000>;
+			opp-hz = /bits/ 64 <166500000>;
 		};
 		opp03 {
 			opp-hz = /bits/ 64 <222000000>;
@@ -126,8 +131,9 @@
 	bus_gen_opp_table: opp_table7 {
 		compatible = "operating-points-v2";
 
+		/* derived from 532MHz MPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <89000000>;
+			opp-hz = /bits/ 64 <88700000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <133000000>;
@@ -136,32 +142,34 @@
 			opp-hz = /bits/ 64 <178000000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <267000000>;
+			opp-hz = /bits/ 64 <266000000>;
 		};
 	};
 
 	bus_peri_opp_table: opp_table8 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <67000000>;
+			opp-hz = /bits/ 64 <66600000>;
 		};
 	};
 
 	bus_g2d_opp_table: opp_table9 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <84000000>;
+			opp-hz = /bits/ 64 <83250000>;
 		};
 		opp01 {
-			opp-hz = /bits/ 64 <167000000>;
+			opp-hz = /bits/ 64 <111000000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <222000000>;
+			opp-hz = /bits/ 64 <166500000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <300000000>;
+			opp-hz = /bits/ 64 <222000000>;
 		};
 		opp04 {
 			opp-hz = /bits/ 64 <333000000>;
@@ -171,8 +179,9 @@
 	bus_g2d_acp_opp_table: opp_table10 {
 		compatible = "operating-points-v2";
 
+		/* derived from 532MHz MPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <67000000>;
+			opp-hz = /bits/ 64 <66500000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <133000000>;
@@ -181,13 +190,14 @@
 			opp-hz = /bits/ 64 <178000000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <267000000>;
+			opp-hz = /bits/ 64 <266000000>;
 		};
 	};
 
 	bus_jpeg_opp_table: opp_table11 {
 		compatible = "operating-points-v2";
 
+		/* derived from 600MHz DPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <75000000>;
 		};
@@ -205,23 +215,25 @@
 	bus_jpeg_apb_opp_table: opp_table12 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
-			opp-hz = /bits/ 64 <84000000>;
+			opp-hz = /bits/ 64 <83250000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <111000000>;
 		};
 		opp02 {
-			opp-hz = /bits/ 64 <134000000>;
+			opp-hz = /bits/ 64 <133000000>;
 		};
 		opp03 {
-			opp-hz = /bits/ 64 <167000000>;
+			opp-hz = /bits/ 64 <166500000>;
 		};
 	};
 
 	bus_disp1_fimd_opp_table: opp_table13 {
 		compatible = "operating-points-v2";
 
+		/* derived from 600MHz DPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <120000000>;
 		};
@@ -233,6 +245,7 @@
 	bus_disp1_opp_table: opp_table14 {
 		compatible = "operating-points-v2";
 
+		/* derived from 600MHz DPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <120000000>;
 		};
@@ -247,6 +260,7 @@
 	bus_gscl_opp_table: opp_table15 {
 		compatible = "operating-points-v2";
 
+		/* derived from 600MHz DPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <150000000>;
 		};
@@ -261,6 +275,7 @@
 	bus_mscl_opp_table: opp_table16 {
 		compatible = "operating-points-v2";
 
+		/* derived from 666MHz CPLL */
 		opp00 {
 			opp-hz = /bits/ 64 <84000000>;
 		};
@@ -274,7 +289,7 @@
 			opp-hz = /bits/ 64 <333000000>;
 		};
 		opp04 {
-			opp-hz = /bits/ 64 <400000000>;
+			opp-hz = /bits/ 64 <666000000>;
 		};
 	};
 
@@ -398,7 +413,7 @@
 };
 
 &bus_fsys {
-	operating-points-v2 = <&bus_fsys_apb_opp_table>;
+	operating-points-v2 = <&bus_fsys2_opp_table>;
 	devfreq = <&bus_wcore>;
 	status = "okay";
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] ARM: dts: exynos: Move bus related OPPs to the boards DTS
  2019-12-19  8:29     ` [PATCH 1/2] ARM: dts: exynos: Move bus related OPPs to the boards DTS Marek Szyprowski
@ 2019-12-19  9:02       ` Chanwoo Choi
  0 siblings, 0 replies; 7+ messages in thread
From: Chanwoo Choi @ 2019-12-19  9:02 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-kernel, linux-pm
  Cc: Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Kamil Konieczny

Hi Marek,

On 12/19/19 5:29 PM, Marek Szyprowski wrote:
> Currently the only Exynos5422-based boards that support bus frequency
> scaling are Hardkernel's Odroid XU3/XU4/HC1. Move the bus related OPPs to
> the boards DTS, because those OPPs heavily depend on the clock topology
> and top PLL rates, which are being configured by the board's bootloader.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5420.dtsi             | 259 -----------------
>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 261 +++++++++++++++++-
>  2 files changed, 260 insertions(+), 260 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index f95567bc10e3..f66a2d1b3428 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -1092,7 +1092,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_wcore_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1100,7 +1099,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK100_NOC>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_noc_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1108,7 +1106,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_fsys_apb_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1116,7 +1113,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_fsys_apb_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1124,7 +1120,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_fsys2_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1132,7 +1127,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK333>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_mfc_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1140,7 +1134,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK266>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_gen_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1148,7 +1141,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK66>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_peri_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1156,7 +1148,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK333_G2D>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_g2d_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1164,7 +1155,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK266_G2D>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_g2d_acp_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1172,7 +1162,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_jpeg_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1180,7 +1169,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK166>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_jpeg_apb_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1188,7 +1176,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_disp1_fimd_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1196,7 +1183,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_disp1_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1204,7 +1190,6 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_gscl_opp_table>;
>  			status = "disabled";
>  		};
>  
> @@ -1212,252 +1197,8 @@
>  			compatible = "samsung,exynos-bus";
>  			clocks = <&clock CLK_DOUT_ACLK400_MSCL>;
>  			clock-names = "bus";
> -			operating-points-v2 = <&bus_mscl_opp_table>;
>  			status = "disabled";
>  		};
> -
> -		bus_wcore_opp_table: opp_table2 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <84000000>;
> -				opp-microvolt = <925000 925000 1400000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <111000000>;
> -				opp-microvolt = <950000 950000 1400000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <222000000>;
> -				opp-microvolt = <950000 950000 1400000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <333000000>;
> -				opp-microvolt = <950000 950000 1400000>;
> -			};
> -			opp04 {
> -				opp-hz = /bits/ 64 <400000000>;
> -				opp-microvolt = <987500 987500 1400000>;
> -			};
> -		};
> -
> -		bus_noc_opp_table: opp_table3 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <67000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <75000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <86000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <100000000>;
> -			};
> -		};
> -
> -		bus_fsys_apb_opp_table: opp_table4 {
> -			compatible = "operating-points-v2";
> -			opp-shared;
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <100000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <200000000>;
> -			};
> -		};
> -
> -		bus_fsys2_opp_table: opp_table5 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <75000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <100000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <150000000>;
> -			};
> -		};
> -
> -		bus_mfc_opp_table: opp_table6 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <96000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <111000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <167000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <222000000>;
> -			};
> -			opp04 {
> -				opp-hz = /bits/ 64 <333000000>;
> -			};
> -		};
> -
> -		bus_gen_opp_table: opp_table7 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <89000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <133000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <178000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <267000000>;
> -			};
> -		};
> -
> -		bus_peri_opp_table: opp_table8 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <67000000>;
> -			};
> -		};
> -
> -		bus_g2d_opp_table: opp_table9 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <84000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <167000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <222000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <300000000>;
> -			};
> -			opp04 {
> -				opp-hz = /bits/ 64 <333000000>;
> -			};
> -		};
> -
> -		bus_g2d_acp_opp_table: opp_table10 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <67000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <133000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <178000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <267000000>;
> -			};
> -		};
> -
> -		bus_jpeg_opp_table: opp_table11 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <75000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <150000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <200000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <300000000>;
> -			};
> -		};
> -
> -		bus_jpeg_apb_opp_table: opp_table12 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <84000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <111000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <134000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <167000000>;
> -			};
> -		};
> -
> -		bus_disp1_fimd_opp_table: opp_table13 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <120000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <200000000>;
> -			};
> -		};
> -
> -		bus_disp1_opp_table: opp_table14 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <120000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <200000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <300000000>;
> -			};
> -		};
> -
> -		bus_gscl_opp_table: opp_table15 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <150000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <200000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <300000000>;
> -			};
> -		};
> -
> -		bus_mscl_opp_table: opp_table16 {
> -			compatible = "operating-points-v2";
> -
> -			opp00 {
> -				opp-hz = /bits/ 64 <84000000>;
> -			};
> -			opp01 {
> -				opp-hz = /bits/ 64 <167000000>;
> -			};
> -			opp02 {
> -				opp-hz = /bits/ 64 <222000000>;
> -			};
> -			opp03 {
> -				opp-hz = /bits/ 64 <333000000>;
> -			};
> -			opp04 {
> -				opp-hz = /bits/ 64 <400000000>;
> -			};
> -		};
>  	};
>  
>  	thermal-zones {
> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> index 80b0acfec547..663a38d53c9e 100644
> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> @@ -35,7 +35,250 @@
>  		};
>  	};
>  
> -	dmc_opp_table: opp_table2 {
> +	bus_wcore_opp_table: opp_table2 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <84000000>;
> +			opp-microvolt = <925000 925000 1400000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <111000000>;
> +			opp-microvolt = <950000 950000 1400000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <222000000>;
> +			opp-microvolt = <950000 950000 1400000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <333000000>;
> +			opp-microvolt = <950000 950000 1400000>;
> +		};
> +		opp04 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <987500 987500 1400000>;
> +		};
> +	};
> +
> +	bus_noc_opp_table: opp_table3 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <67000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <75000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <86000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <100000000>;
> +		};
> +	};
> +
> +	bus_fsys_apb_opp_table: opp_table4 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <100000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <200000000>;
> +		};
> +	};
> +
> +	bus_fsys2_opp_table: opp_table5 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <75000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <100000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <150000000>;
> +		};
> +	};
> +
> +	bus_mfc_opp_table: opp_table6 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <96000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <111000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <167000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <222000000>;
> +		};
> +		opp04 {
> +			opp-hz = /bits/ 64 <333000000>;
> +		};
> +	};
> +
> +	bus_gen_opp_table: opp_table7 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <89000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <133000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <178000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <267000000>;
> +		};
> +	};
> +
> +	bus_peri_opp_table: opp_table8 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <67000000>;
> +		};
> +	};
> +
> +	bus_g2d_opp_table: opp_table9 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <84000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <167000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <222000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <300000000>;
> +		};
> +		opp04 {
> +			opp-hz = /bits/ 64 <333000000>;
> +		};
> +	};
> +
> +	bus_g2d_acp_opp_table: opp_table10 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <67000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <133000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <178000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <267000000>;
> +		};
> +	};
> +
> +	bus_jpeg_opp_table: opp_table11 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <75000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <150000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <200000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <300000000>;
> +		};
> +	};
> +
> +	bus_jpeg_apb_opp_table: opp_table12 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <84000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <111000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <134000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <167000000>;
> +		};
> +	};
> +
> +	bus_disp1_fimd_opp_table: opp_table13 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <120000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <200000000>;
> +		};
> +	};
> +
> +	bus_disp1_opp_table: opp_table14 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <120000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <200000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <300000000>;
> +		};
> +	};
> +
> +	bus_gscl_opp_table: opp_table15 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <150000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <200000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <300000000>;
> +		};
> +	};
> +
> +	bus_mscl_opp_table: opp_table16 {
> +		compatible = "operating-points-v2";
> +
> +		opp00 {
> +			opp-hz = /bits/ 64 <84000000>;
> +		};
> +		opp01 {
> +			opp-hz = /bits/ 64 <167000000>;
> +		};
> +		opp02 {
> +			opp-hz = /bits/ 64 <222000000>;
> +		};
> +		opp03 {
> +			opp-hz = /bits/ 64 <333000000>;
> +		};
> +		opp04 {
> +			opp-hz = /bits/ 64 <400000000>;
> +		};
> +	};
> +
> +	dmc_opp_table: opp_table18 {

opp_table18 -> opp_table17?

(snip)

Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids
  2019-12-19  8:29     ` [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids Marek Szyprowski
@ 2019-12-19  9:07       ` Chanwoo Choi
  2019-12-19  9:09         ` Marek Szyprowski
  0 siblings, 1 reply; 7+ messages in thread
From: Chanwoo Choi @ 2019-12-19  9:07 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-kernel, linux-pm
  Cc: Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Kamil Konieczny

Hi Marek,

On 12/19/19 5:29 PM, Marek Szyprowski wrote:
> Hardkernel's Odroid XU3/XU4/HC1 boards use bootloader, which configures top
> PLLs to the following values: MPLL: 532MHz, CPLL: 666MHz and DPLL: 600MHz.
> 
> Adjust all bus related OPPs to the values that are possible to derive from
> the top PLL configured by the bootloader. Also add a comment for each bus
> describing which PLL is used for it.
> 
> The most significant change is the highest rate for wcore bus. It has been
> increased to 532MHz as this is the value configured initially by the
> bootloader. Also the voltage for this OPP is changed to match the value
> set by the bootloader.
> 
> This patch finally allows the buses to operate on the rates matching the
> values set for each OPP and fixes the following warnings observed on boot:
> 
> exynos-bus: new bus device registered: soc:bus_wcore ( 84000 KHz ~ 400000 KHz)
> exynos-bus: new bus device registered: soc:bus_noc ( 67000 KHz ~ 100000 KHz)
> exynos-bus: new bus device registered: soc:bus_fsys_apb (100000 KHz ~ 200000 KHz)
> ...
> exynos-bus soc:bus_wcore: dev_pm_opp_set_rate: failed to find current OPP for freq 532000000 (-34)
> exynos-bus soc:bus_noc: dev_pm_opp_set_rate: failed to find current OPP for freq 111000000 (-34)
> exynos-bus soc:bus_fsys_apb: dev_pm_opp_set_rate: failed to find current OPP for freq 222000000 (-34)
> 
> The problem with setting incorrect (in some cases much lower) clock rate
> for the defined OPP were there from the beginning, but went unnoticed
> because the only way to observe it was to manually check the rate of the
> respective clocks. The commit 4294a779bd8d ("PM / devfreq: exynos-bus:
> Convert to use dev_pm_opp_set_rate()") finally revealed it, because it
> enabled use of the generic code from the OPP framework, which issues the
> above mentioned warnings.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 75 +++++++++++--------
>  1 file changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> index 663a38d53c9e..b6d6022e8735 100644
> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> @@ -38,42 +38,44 @@
>  	bus_wcore_opp_table: opp_table2 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 532MHz MPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <84000000>;
> +			opp-hz = /bits/ 64 <88700000>;
>  			opp-microvolt = <925000 925000 1400000>;
>  		};
>  		opp01 {
> -			opp-hz = /bits/ 64 <111000000>;
> +			opp-hz = /bits/ 64 <133000000>;
>  			opp-microvolt = <950000 950000 1400000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <222000000>;
> +			opp-hz = /bits/ 64 <177400000>;
>  			opp-microvolt = <950000 950000 1400000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <333000000>;
> +			opp-hz = /bits/ 64 <266000000>;
>  			opp-microvolt = <950000 950000 1400000>;
>  		};
>  		opp04 {
> -			opp-hz = /bits/ 64 <400000000>;
> -			opp-microvolt = <987500 987500 1400000>;
> +			opp-hz = /bits/ 64 <532000000>;
> +			opp-microvolt = <1000000 1000000 1400000>;
>  		};
>  	};
>  
>  	bus_noc_opp_table: opp_table3 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <67000000>;
> +			opp-hz = /bits/ 64 <66600000>;
>  		};
>  		opp01 {
> -			opp-hz = /bits/ 64 <75000000>;
> +			opp-hz = /bits/ 64 <74000000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <86000000>;
> +			opp-hz = /bits/ 64 <83250000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <100000000>;
> +			opp-hz = /bits/ 64 <111000000>;
>  		};
>  	};
>  
> @@ -81,39 +83,42 @@
>  		compatible = "operating-points-v2";
>  		opp-shared;
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <100000000>;
> +			opp-hz = /bits/ 64 <111000000>;
>  		};
>  		opp01 {
> -			opp-hz = /bits/ 64 <200000000>;
> +			opp-hz = /bits/ 64 <222000000>;
>  		};
>  	};
>  
>  	bus_fsys2_opp_table: opp_table5 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 600MHz DPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <75000000>;
>  		};
>  		opp01 {
> -			opp-hz = /bits/ 64 <100000000>;
> +			opp-hz = /bits/ 64 <120000000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <150000000>;
> +			opp-hz = /bits/ 64 <200000000>;
>  		};
>  	};
>  
>  	bus_mfc_opp_table: opp_table6 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <96000000>;
> +			opp-hz = /bits/ 64 <83250000>;
>  		};
>  		opp01 {
>  			opp-hz = /bits/ 64 <111000000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <167000000>;
> +			opp-hz = /bits/ 64 <166500000>;
>  		};
>  		opp03 {
>  			opp-hz = /bits/ 64 <222000000>;
> @@ -126,8 +131,9 @@
>  	bus_gen_opp_table: opp_table7 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 532MHz MPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <89000000>;
> +			opp-hz = /bits/ 64 <88700000>;
>  		};
>  		opp01 {
>  			opp-hz = /bits/ 64 <133000000>;
> @@ -136,32 +142,34 @@
>  			opp-hz = /bits/ 64 <178000000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <267000000>;
> +			opp-hz = /bits/ 64 <266000000>;
>  		};
>  	};
>  
>  	bus_peri_opp_table: opp_table8 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <67000000>;
> +			opp-hz = /bits/ 64 <66600000>;
>  		};
>  	};
>  
>  	bus_g2d_opp_table: opp_table9 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <84000000>;
> +			opp-hz = /bits/ 64 <83250000>;
>  		};
>  		opp01 {
> -			opp-hz = /bits/ 64 <167000000>;
> +			opp-hz = /bits/ 64 <111000000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <222000000>;
> +			opp-hz = /bits/ 64 <166500000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <300000000>;
> +			opp-hz = /bits/ 64 <222000000>;
>  		};
>  		opp04 {
>  			opp-hz = /bits/ 64 <333000000>;
> @@ -171,8 +179,9 @@
>  	bus_g2d_acp_opp_table: opp_table10 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 532MHz MPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <67000000>;
> +			opp-hz = /bits/ 64 <66500000>;
>  		};
>  		opp01 {
>  			opp-hz = /bits/ 64 <133000000>;
> @@ -181,13 +190,14 @@
>  			opp-hz = /bits/ 64 <178000000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <267000000>;
> +			opp-hz = /bits/ 64 <266000000>;
>  		};
>  	};
>  
>  	bus_jpeg_opp_table: opp_table11 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 600MHz DPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <75000000>;
>  		};
> @@ -205,23 +215,25 @@
>  	bus_jpeg_apb_opp_table: opp_table12 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
> -			opp-hz = /bits/ 64 <84000000>;
> +			opp-hz = /bits/ 64 <83250000>;
>  		};
>  		opp01 {
>  			opp-hz = /bits/ 64 <111000000>;
>  		};
>  		opp02 {
> -			opp-hz = /bits/ 64 <134000000>;
> +			opp-hz = /bits/ 64 <133000000>;
>  		};
>  		opp03 {
> -			opp-hz = /bits/ 64 <167000000>;
> +			opp-hz = /bits/ 64 <166500000>;
>  		};
>  	};
>  
>  	bus_disp1_fimd_opp_table: opp_table13 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 600MHz DPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <120000000>;
>  		};
> @@ -233,6 +245,7 @@
>  	bus_disp1_opp_table: opp_table14 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 600MHz DPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <120000000>;
>  		};
> @@ -247,6 +260,7 @@
>  	bus_gscl_opp_table: opp_table15 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 600MHz DPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <150000000>;
>  		};
> @@ -261,6 +275,7 @@
>  	bus_mscl_opp_table: opp_table16 {
>  		compatible = "operating-points-v2";
>  
> +		/* derived from 666MHz CPLL */
>  		opp00 {
>  			opp-hz = /bits/ 64 <84000000>;
>  		};
> @@ -274,7 +289,7 @@
>  			opp-hz = /bits/ 64 <333000000>;
>  		};
>  		opp04 {
> -			opp-hz = /bits/ 64 <400000000>;
> +			opp-hz = /bits/ 64 <666000000>;
>  		};
>  	};
>  
> @@ -398,7 +413,7 @@
>  };
>  
>  &bus_fsys {
> -	operating-points-v2 = <&bus_fsys_apb_opp_table>;
> +	operating-points-v2 = <&bus_fsys2_opp_table>;


Need to remove 'opp-shared' property in bus_fsys_apb_opp_table.
And need to add 'opp-shared' property to bus_fsys2_opp_table.


>  	devfreq = <&bus_wcore>;
>  	status = "okay";
>  };
> 

If you fix the things related to 'opp-shared', Looks good to me.
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids
  2019-12-19  9:07       ` Chanwoo Choi
@ 2019-12-19  9:09         ` Marek Szyprowski
  2019-12-19  9:23           ` Chanwoo Choi
  0 siblings, 1 reply; 7+ messages in thread
From: Marek Szyprowski @ 2019-12-19  9:09 UTC (permalink / raw)
  To: Chanwoo Choi, linux-samsung-soc, linux-kernel, linux-pm
  Cc: Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Kamil Konieczny

Hi Chanwoo,

On 19.12.2019 10:07, Chanwoo Choi wrote:
> On 12/19/19 5:29 PM, Marek Szyprowski wrote:
>> Hardkernel's Odroid XU3/XU4/HC1 boards use bootloader, which configures top
>> PLLs to the following values: MPLL: 532MHz, CPLL: 666MHz and DPLL: 600MHz.
>>
>> Adjust all bus related OPPs to the values that are possible to derive from
>> the top PLL configured by the bootloader. Also add a comment for each bus
>> describing which PLL is used for it.
>>
>> The most significant change is the highest rate for wcore bus. It has been
>> increased to 532MHz as this is the value configured initially by the
>> bootloader. Also the voltage for this OPP is changed to match the value
>> set by the bootloader.
>>
>> This patch finally allows the buses to operate on the rates matching the
>> values set for each OPP and fixes the following warnings observed on boot:
>>
>> exynos-bus: new bus device registered: soc:bus_wcore ( 84000 KHz ~ 400000 KHz)
>> exynos-bus: new bus device registered: soc:bus_noc ( 67000 KHz ~ 100000 KHz)
>> exynos-bus: new bus device registered: soc:bus_fsys_apb (100000 KHz ~ 200000 KHz)
>> ...
>> exynos-bus soc:bus_wcore: dev_pm_opp_set_rate: failed to find current OPP for freq 532000000 (-34)
>> exynos-bus soc:bus_noc: dev_pm_opp_set_rate: failed to find current OPP for freq 111000000 (-34)
>> exynos-bus soc:bus_fsys_apb: dev_pm_opp_set_rate: failed to find current OPP for freq 222000000 (-34)
>>
>> The problem with setting incorrect (in some cases much lower) clock rate
>> for the defined OPP were there from the beginning, but went unnoticed
>> because the only way to observe it was to manually check the rate of the
>> respective clocks. The commit 4294a779bd8d ("PM / devfreq: exynos-bus:
>> Convert to use dev_pm_opp_set_rate()") finally revealed it, because it
>> enabled use of the generic code from the OPP framework, which issues the
>> above mentioned warnings.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 75 +++++++++++--------
>>   1 file changed, 45 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>> index 663a38d53c9e..b6d6022e8735 100644
>> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>> @@ -38,42 +38,44 @@
>>   	bus_wcore_opp_table: opp_table2 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 532MHz MPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <84000000>;
>> +			opp-hz = /bits/ 64 <88700000>;
>>   			opp-microvolt = <925000 925000 1400000>;
>>   		};
>>   		opp01 {
>> -			opp-hz = /bits/ 64 <111000000>;
>> +			opp-hz = /bits/ 64 <133000000>;
>>   			opp-microvolt = <950000 950000 1400000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <222000000>;
>> +			opp-hz = /bits/ 64 <177400000>;
>>   			opp-microvolt = <950000 950000 1400000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <333000000>;
>> +			opp-hz = /bits/ 64 <266000000>;
>>   			opp-microvolt = <950000 950000 1400000>;
>>   		};
>>   		opp04 {
>> -			opp-hz = /bits/ 64 <400000000>;
>> -			opp-microvolt = <987500 987500 1400000>;
>> +			opp-hz = /bits/ 64 <532000000>;
>> +			opp-microvolt = <1000000 1000000 1400000>;
>>   		};
>>   	};
>>   
>>   	bus_noc_opp_table: opp_table3 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <67000000>;
>> +			opp-hz = /bits/ 64 <66600000>;
>>   		};
>>   		opp01 {
>> -			opp-hz = /bits/ 64 <75000000>;
>> +			opp-hz = /bits/ 64 <74000000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <86000000>;
>> +			opp-hz = /bits/ 64 <83250000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <100000000>;
>> +			opp-hz = /bits/ 64 <111000000>;
>>   		};
>>   	};
>>   
>> @@ -81,39 +83,42 @@
>>   		compatible = "operating-points-v2";
>>   		opp-shared;
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <100000000>;
>> +			opp-hz = /bits/ 64 <111000000>;
>>   		};
>>   		opp01 {
>> -			opp-hz = /bits/ 64 <200000000>;
>> +			opp-hz = /bits/ 64 <222000000>;
>>   		};
>>   	};
>>   
>>   	bus_fsys2_opp_table: opp_table5 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 600MHz DPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <75000000>;
>>   		};
>>   		opp01 {
>> -			opp-hz = /bits/ 64 <100000000>;
>> +			opp-hz = /bits/ 64 <120000000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <150000000>;
>> +			opp-hz = /bits/ 64 <200000000>;
>>   		};
>>   	};
>>   
>>   	bus_mfc_opp_table: opp_table6 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <96000000>;
>> +			opp-hz = /bits/ 64 <83250000>;
>>   		};
>>   		opp01 {
>>   			opp-hz = /bits/ 64 <111000000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <167000000>;
>> +			opp-hz = /bits/ 64 <166500000>;
>>   		};
>>   		opp03 {
>>   			opp-hz = /bits/ 64 <222000000>;
>> @@ -126,8 +131,9 @@
>>   	bus_gen_opp_table: opp_table7 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 532MHz MPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <89000000>;
>> +			opp-hz = /bits/ 64 <88700000>;
>>   		};
>>   		opp01 {
>>   			opp-hz = /bits/ 64 <133000000>;
>> @@ -136,32 +142,34 @@
>>   			opp-hz = /bits/ 64 <178000000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <267000000>;
>> +			opp-hz = /bits/ 64 <266000000>;
>>   		};
>>   	};
>>   
>>   	bus_peri_opp_table: opp_table8 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <67000000>;
>> +			opp-hz = /bits/ 64 <66600000>;
>>   		};
>>   	};
>>   
>>   	bus_g2d_opp_table: opp_table9 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <84000000>;
>> +			opp-hz = /bits/ 64 <83250000>;
>>   		};
>>   		opp01 {
>> -			opp-hz = /bits/ 64 <167000000>;
>> +			opp-hz = /bits/ 64 <111000000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <222000000>;
>> +			opp-hz = /bits/ 64 <166500000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <300000000>;
>> +			opp-hz = /bits/ 64 <222000000>;
>>   		};
>>   		opp04 {
>>   			opp-hz = /bits/ 64 <333000000>;
>> @@ -171,8 +179,9 @@
>>   	bus_g2d_acp_opp_table: opp_table10 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 532MHz MPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <67000000>;
>> +			opp-hz = /bits/ 64 <66500000>;
>>   		};
>>   		opp01 {
>>   			opp-hz = /bits/ 64 <133000000>;
>> @@ -181,13 +190,14 @@
>>   			opp-hz = /bits/ 64 <178000000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <267000000>;
>> +			opp-hz = /bits/ 64 <266000000>;
>>   		};
>>   	};
>>   
>>   	bus_jpeg_opp_table: opp_table11 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 600MHz DPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <75000000>;
>>   		};
>> @@ -205,23 +215,25 @@
>>   	bus_jpeg_apb_opp_table: opp_table12 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>> -			opp-hz = /bits/ 64 <84000000>;
>> +			opp-hz = /bits/ 64 <83250000>;
>>   		};
>>   		opp01 {
>>   			opp-hz = /bits/ 64 <111000000>;
>>   		};
>>   		opp02 {
>> -			opp-hz = /bits/ 64 <134000000>;
>> +			opp-hz = /bits/ 64 <133000000>;
>>   		};
>>   		opp03 {
>> -			opp-hz = /bits/ 64 <167000000>;
>> +			opp-hz = /bits/ 64 <166500000>;
>>   		};
>>   	};
>>   
>>   	bus_disp1_fimd_opp_table: opp_table13 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 600MHz DPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <120000000>;
>>   		};
>> @@ -233,6 +245,7 @@
>>   	bus_disp1_opp_table: opp_table14 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 600MHz DPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <120000000>;
>>   		};
>> @@ -247,6 +260,7 @@
>>   	bus_gscl_opp_table: opp_table15 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 600MHz DPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <150000000>;
>>   		};
>> @@ -261,6 +275,7 @@
>>   	bus_mscl_opp_table: opp_table16 {
>>   		compatible = "operating-points-v2";
>>   
>> +		/* derived from 666MHz CPLL */
>>   		opp00 {
>>   			opp-hz = /bits/ 64 <84000000>;
>>   		};
>> @@ -274,7 +289,7 @@
>>   			opp-hz = /bits/ 64 <333000000>;
>>   		};
>>   		opp04 {
>> -			opp-hz = /bits/ 64 <400000000>;
>> +			opp-hz = /bits/ 64 <666000000>;
>>   		};
>>   	};
>>   
>> @@ -398,7 +413,7 @@
>>   };
>>   
>>   &bus_fsys {
>> -	operating-points-v2 = <&bus_fsys_apb_opp_table>;
>> +	operating-points-v2 = <&bus_fsys2_opp_table>;
>
> Need to remove 'opp-shared' property in bus_fsys_apb_opp_table.
> And need to add 'opp-shared' property to bus_fsys2_opp_table.

I've checked the dt bindings and I think that opp-shared property has to 
be removed at all. Clocks between fsys and fsys2 buses are not related 
and regulator is currently already handled by in a different way by the 
passive governor.

>>   	devfreq = <&bus_wcore>;
>>   	status = "okay";
>>   };
>>
> If you fix the things related to 'opp-shared', Looks good to me.
> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids
  2019-12-19  9:09         ` Marek Szyprowski
@ 2019-12-19  9:23           ` Chanwoo Choi
  0 siblings, 0 replies; 7+ messages in thread
From: Chanwoo Choi @ 2019-12-19  9:23 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-kernel, linux-pm
  Cc: Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Kamil Konieczny

On 12/19/19 6:09 PM, Marek Szyprowski wrote:
> Hi Chanwoo,
> 
> On 19.12.2019 10:07, Chanwoo Choi wrote:
>> On 12/19/19 5:29 PM, Marek Szyprowski wrote:
>>> Hardkernel's Odroid XU3/XU4/HC1 boards use bootloader, which configures top
>>> PLLs to the following values: MPLL: 532MHz, CPLL: 666MHz and DPLL: 600MHz.
>>>
>>> Adjust all bus related OPPs to the values that are possible to derive from
>>> the top PLL configured by the bootloader. Also add a comment for each bus
>>> describing which PLL is used for it.
>>>
>>> The most significant change is the highest rate for wcore bus. It has been
>>> increased to 532MHz as this is the value configured initially by the
>>> bootloader. Also the voltage for this OPP is changed to match the value
>>> set by the bootloader.
>>>
>>> This patch finally allows the buses to operate on the rates matching the
>>> values set for each OPP and fixes the following warnings observed on boot:
>>>
>>> exynos-bus: new bus device registered: soc:bus_wcore ( 84000 KHz ~ 400000 KHz)
>>> exynos-bus: new bus device registered: soc:bus_noc ( 67000 KHz ~ 100000 KHz)
>>> exynos-bus: new bus device registered: soc:bus_fsys_apb (100000 KHz ~ 200000 KHz)
>>> ...
>>> exynos-bus soc:bus_wcore: dev_pm_opp_set_rate: failed to find current OPP for freq 532000000 (-34)
>>> exynos-bus soc:bus_noc: dev_pm_opp_set_rate: failed to find current OPP for freq 111000000 (-34)
>>> exynos-bus soc:bus_fsys_apb: dev_pm_opp_set_rate: failed to find current OPP for freq 222000000 (-34)
>>>
>>> The problem with setting incorrect (in some cases much lower) clock rate
>>> for the defined OPP were there from the beginning, but went unnoticed
>>> because the only way to observe it was to manually check the rate of the
>>> respective clocks. The commit 4294a779bd8d ("PM / devfreq: exynos-bus:
>>> Convert to use dev_pm_opp_set_rate()") finally revealed it, because it
>>> enabled use of the generic code from the OPP framework, which issues the
>>> above mentioned warnings.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 75 +++++++++++--------
>>>   1 file changed, 45 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>>> index 663a38d53c9e..b6d6022e8735 100644
>>> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
>>> @@ -38,42 +38,44 @@
>>>   	bus_wcore_opp_table: opp_table2 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 532MHz MPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <84000000>;
>>> +			opp-hz = /bits/ 64 <88700000>;
>>>   			opp-microvolt = <925000 925000 1400000>;
>>>   		};
>>>   		opp01 {
>>> -			opp-hz = /bits/ 64 <111000000>;
>>> +			opp-hz = /bits/ 64 <133000000>;
>>>   			opp-microvolt = <950000 950000 1400000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <222000000>;
>>> +			opp-hz = /bits/ 64 <177400000>;
>>>   			opp-microvolt = <950000 950000 1400000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <333000000>;
>>> +			opp-hz = /bits/ 64 <266000000>;
>>>   			opp-microvolt = <950000 950000 1400000>;
>>>   		};
>>>   		opp04 {
>>> -			opp-hz = /bits/ 64 <400000000>;
>>> -			opp-microvolt = <987500 987500 1400000>;
>>> +			opp-hz = /bits/ 64 <532000000>;
>>> +			opp-microvolt = <1000000 1000000 1400000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_noc_opp_table: opp_table3 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <67000000>;
>>> +			opp-hz = /bits/ 64 <66600000>;
>>>   		};
>>>   		opp01 {
>>> -			opp-hz = /bits/ 64 <75000000>;
>>> +			opp-hz = /bits/ 64 <74000000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <86000000>;
>>> +			opp-hz = /bits/ 64 <83250000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <100000000>;
>>> +			opp-hz = /bits/ 64 <111000000>;
>>>   		};
>>>   	};
>>>   
>>> @@ -81,39 +83,42 @@
>>>   		compatible = "operating-points-v2";
>>>   		opp-shared;
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <100000000>;
>>> +			opp-hz = /bits/ 64 <111000000>;
>>>   		};
>>>   		opp01 {
>>> -			opp-hz = /bits/ 64 <200000000>;
>>> +			opp-hz = /bits/ 64 <222000000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_fsys2_opp_table: opp_table5 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 600MHz DPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <75000000>;
>>>   		};
>>>   		opp01 {
>>> -			opp-hz = /bits/ 64 <100000000>;
>>> +			opp-hz = /bits/ 64 <120000000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <150000000>;
>>> +			opp-hz = /bits/ 64 <200000000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_mfc_opp_table: opp_table6 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <96000000>;
>>> +			opp-hz = /bits/ 64 <83250000>;
>>>   		};
>>>   		opp01 {
>>>   			opp-hz = /bits/ 64 <111000000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <167000000>;
>>> +			opp-hz = /bits/ 64 <166500000>;
>>>   		};
>>>   		opp03 {
>>>   			opp-hz = /bits/ 64 <222000000>;
>>> @@ -126,8 +131,9 @@
>>>   	bus_gen_opp_table: opp_table7 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 532MHz MPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <89000000>;
>>> +			opp-hz = /bits/ 64 <88700000>;
>>>   		};
>>>   		opp01 {
>>>   			opp-hz = /bits/ 64 <133000000>;
>>> @@ -136,32 +142,34 @@
>>>   			opp-hz = /bits/ 64 <178000000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <267000000>;
>>> +			opp-hz = /bits/ 64 <266000000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_peri_opp_table: opp_table8 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <67000000>;
>>> +			opp-hz = /bits/ 64 <66600000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_g2d_opp_table: opp_table9 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <84000000>;
>>> +			opp-hz = /bits/ 64 <83250000>;
>>>   		};
>>>   		opp01 {
>>> -			opp-hz = /bits/ 64 <167000000>;
>>> +			opp-hz = /bits/ 64 <111000000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <222000000>;
>>> +			opp-hz = /bits/ 64 <166500000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <300000000>;
>>> +			opp-hz = /bits/ 64 <222000000>;
>>>   		};
>>>   		opp04 {
>>>   			opp-hz = /bits/ 64 <333000000>;
>>> @@ -171,8 +179,9 @@
>>>   	bus_g2d_acp_opp_table: opp_table10 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 532MHz MPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <67000000>;
>>> +			opp-hz = /bits/ 64 <66500000>;
>>>   		};
>>>   		opp01 {
>>>   			opp-hz = /bits/ 64 <133000000>;
>>> @@ -181,13 +190,14 @@
>>>   			opp-hz = /bits/ 64 <178000000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <267000000>;
>>> +			opp-hz = /bits/ 64 <266000000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_jpeg_opp_table: opp_table11 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 600MHz DPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <75000000>;
>>>   		};
>>> @@ -205,23 +215,25 @@
>>>   	bus_jpeg_apb_opp_table: opp_table12 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>> -			opp-hz = /bits/ 64 <84000000>;
>>> +			opp-hz = /bits/ 64 <83250000>;
>>>   		};
>>>   		opp01 {
>>>   			opp-hz = /bits/ 64 <111000000>;
>>>   		};
>>>   		opp02 {
>>> -			opp-hz = /bits/ 64 <134000000>;
>>> +			opp-hz = /bits/ 64 <133000000>;
>>>   		};
>>>   		opp03 {
>>> -			opp-hz = /bits/ 64 <167000000>;
>>> +			opp-hz = /bits/ 64 <166500000>;
>>>   		};
>>>   	};
>>>   
>>>   	bus_disp1_fimd_opp_table: opp_table13 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 600MHz DPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <120000000>;
>>>   		};
>>> @@ -233,6 +245,7 @@
>>>   	bus_disp1_opp_table: opp_table14 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 600MHz DPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <120000000>;
>>>   		};
>>> @@ -247,6 +260,7 @@
>>>   	bus_gscl_opp_table: opp_table15 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 600MHz DPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <150000000>;
>>>   		};
>>> @@ -261,6 +275,7 @@
>>>   	bus_mscl_opp_table: opp_table16 {
>>>   		compatible = "operating-points-v2";
>>>   
>>> +		/* derived from 666MHz CPLL */
>>>   		opp00 {
>>>   			opp-hz = /bits/ 64 <84000000>;
>>>   		};
>>> @@ -274,7 +289,7 @@
>>>   			opp-hz = /bits/ 64 <333000000>;
>>>   		};
>>>   		opp04 {
>>> -			opp-hz = /bits/ 64 <400000000>;
>>> +			opp-hz = /bits/ 64 <666000000>;
>>>   		};
>>>   	};
>>>   
>>> @@ -398,7 +413,7 @@
>>>   };
>>>   
>>>   &bus_fsys {
>>> -	operating-points-v2 = <&bus_fsys_apb_opp_table>;
>>> +	operating-points-v2 = <&bus_fsys2_opp_table>;
>>
>> Need to remove 'opp-shared' property in bus_fsys_apb_opp_table.
>> And need to add 'opp-shared' property to bus_fsys2_opp_table.
> 
> I've checked the dt bindings and I think that opp-shared property has to 
> be removed at all. Clocks between fsys and fsys2 buses are not related 
> and regulator is currently already handled by in a different way by the 
> passive governor.

You're right. I realized 'opp-shared' is not necessary
by reading Documentation/devicetree/bindings/opp/opp.txt. Thanks.

> 
>>>   	devfreq = <&bus_wcore>;
>>>   	status = "okay";
>>>   };
>>>
>> If you fix the things related to 'opp-shared', Looks good to me.
>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>
> Best regards
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-12-19  9:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20191219082938eucas1p254fe738574f287a44630d5a7eef7385e@eucas1p2.samsung.com>
2019-12-19  8:29 ` [PATCH 0/2] Exynos5422: fix bus related OPPs for Odroid XU3/XU4/HC1 Marek Szyprowski
     [not found]   ` <CGME20191219082939eucas1p248ca8b95ede6f2704b83515f461f6927@eucas1p2.samsung.com>
2019-12-19  8:29     ` [PATCH 1/2] ARM: dts: exynos: Move bus related OPPs to the boards DTS Marek Szyprowski
2019-12-19  9:02       ` Chanwoo Choi
     [not found]   ` <CGME20191219082939eucas1p2afc32535df1512dc21ca983daa012568@eucas1p2.samsung.com>
2019-12-19  8:29     ` [PATCH 2/2] ARM: dts: exynos: Adjust bus related OPPs to the values correct for Odroids Marek Szyprowski
2019-12-19  9:07       ` Chanwoo Choi
2019-12-19  9:09         ` Marek Szyprowski
2019-12-19  9:23           ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).