2016-04-08 05:01:07

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

This patchset support the AMBA bus frequency scaling on Exynos5422-based
Odroid-XU3 board. But, this series only support the bus frequency scaling
for INT (Internal) block using VDD_INT power line.

Also, to support the bus frequency scaling for Exynos542x SoC,
Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
to measure the transfered data traffic on NoC (Network on Chip)
instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
provide the utilization for INT block of Exynos542x SoC.

The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
device (drivers/devfreq/event/exynos-nocp.c) without any modification.
Just add the phandle of 'NoC Probe' dt node to bus dt node.

Depend on:
This patchset depends on patch[1] which support the generic exynos-bus
frequency driver.

[1] https://lkml.org/lkml/2016/4/8/14
- [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor

Chanwoo Choi (7):
PM / devfreq: event: Add new Exynos NoC probe driver
PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus
ARM: dts: Add NoC Probe dt node for Exynos542x SoC
dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC
clk: samsung: exynos542x: Add the clock id for ACLK
ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC
ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3

.../bindings/devfreq/event/exynos-nocp.txt | 86 +++++
.../devicetree/bindings/devfreq/exynos-bus.txt | 19 +
arch/arm/boot/dts/exynos5420.dtsi | 407 +++++++++++++++++++++
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 +++++
drivers/clk/samsung/clk-exynos5420.c | 85 +++--
drivers/devfreq/event/Kconfig | 8 +
drivers/devfreq/event/Makefile | 2 +
drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++
drivers/devfreq/event/exynos-nocp.h | 78 ++++
include/dt-bindings/clock/exynos5420.h | 24 +-
10 files changed, 1024 insertions(+), 31 deletions(-)
create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
create mode 100644 drivers/devfreq/event/exynos-nocp.c
create mode 100644 drivers/devfreq/event/exynos-nocp.h

--
1.9.1


2016-04-08 05:00:58

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 6/7] ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC

This patch adds the AMBA bus nodes using VDD_INT for Exynos542x SoC.
Exynos542x has the following AMBA buses to translate data between
DRAM and sub-blocks.

Following list specifies the detailed correlation between sub-block and clock:
- CLK_DOUT_ACLK400_WCORE clock for WCORE's AXI
- CLK_DOUT_ACLK100_NOC for NoC (Network on Chip)'s AXI
- CLK_DOUT_PCLK200_FSYS for FSYS's APB
- CLK_DOUT_ACLK200_FSYS for FSYS's AXI
- CLK_DOUT_ACLK200_FSYS2 for FSYS2's AXI
- CLK_DOUT_ACLK333 for MFC's AXI
- CLK_DOUT_ACLK266 for GEN's AXI
- CLK_DOUT_ACLK66 for PERIC/PERIR's AXI
- CLK_DOUT_ACLK333_G2D for G2D's AXI
- CLK_DOUT_ACLK266_G2D for ACP's AXI
- CLK_DOUT_ACLK300_JPEG for JPEG's AXI
- CLK_DOUT_ACLK166 for JPEG's APB
- CLK_DOUT_ACLK300_DISP1 for FIMD's AXI
- CLK_DOUT_ACLK400_DISP1 for DISP1's AXI
- CLK_DOUT_ACLK300_GSCL for GSCL Scaler's AXI
- CLK_DOUT_ACLK400_MSCL for MSCL's AXI

Signed-off-by: Chanwoo Choi <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 371 ++++++++++++++++++++++++++++++++++++++
1 file changed, 371 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index d80f3b66f017..1340024fa882 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -1224,6 +1224,377 @@
power-domains = <&disp_pd>;
#iommu-cells = <0>;
};
+
+ bus_wcore: bus_wcore {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_wcore_opp_table>;
+ status = "disabled";
+ };
+
+ bus_noc: bus_noc {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK100_NOC>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_noc_opp_table>;
+ status = "disabled";
+ };
+
+ bus_fsys_apb: bus_fsys_apb {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_fsys_apb_opp_table>;
+ status = "disabled";
+ };
+
+ bus_fsys: bus_fsys {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_fsys_apb_opp_table>;
+ status = "disabled";
+ };
+
+ bus_fsys2: bus_fsys2 {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_fsys2_opp_table>;
+ status = "disabled";
+ };
+
+ bus_mfc: bus_mfc {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK333>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_mfc_opp_table>;
+ status = "disabled";
+ };
+
+ bus_gen: bus_gen {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK266>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_gen_opp_table>;
+ status = "disabled";
+ };
+
+ bus_peri: bus_peri {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK66>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_peri_opp_table>;
+ status = "disabled";
+ };
+
+ bus_g2d: bus_g2d {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK333_G2D>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_g2d_opp_table>;
+ status = "disabled";
+ };
+
+ bus_g2d_acp: bus_g2d_acp {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK266_G2D>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_g2d_acp_opp_table>;
+ status = "disabled";
+ };
+
+ bus_jpeg: bus_jpeg {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_jpeg_opp_table>;
+ status = "disabled";
+ };
+
+ bus_jpeg_apb: bus_jpeg_apb {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK166>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_jpeg_apb_opp_table>;
+ status = "disabled";
+ };
+
+ bus_disp1_fimd: bus_disp1_fimd {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_disp1_fimd_opp_table>;
+ status = "disabled";
+ };
+
+ bus_disp1: bus_disp1 {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_disp1_opp_table>;
+ status = "disabled";
+ };
+
+ bus_gscl_scaler: bus_gscl_scaler {
+ compatible = "samsung,exynos-bus";
+ clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
+ clock-names = "bus";
+ operating-points-v2 = <&bus_gscl_opp_table>;
+ status = "disabled";
+ };
+
+ bus_mscl: bus_mscl {
+ 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>;
+ };
+ opp01 {
+ opp-hz = /bits/ 64 <111000000>;
+ opp-microvolt = <950000>;
+ };
+ opp02 {
+ opp-hz = /bits/ 64 <222000000>;
+ opp-microvolt = <950000>;
+ };
+ opp03 {
+ opp-hz = /bits/ 64 <333000000>;
+ opp-microvolt = <950000>;
+ };
+ opp04 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <987500>;
+ };
+ };
+
+ bus_noc_opp_table: opp_table3 {
+ compatible = "operating-points-v2";
+
+ opp00 {
+ opp-hz = /bits/ 64 <66000000>;
+ };
+ 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>;
+ };
+ };
};

&dp {
--
1.9.1

2016-04-08 05:01:05

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 4/7] dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC

This patch adds the clock id for ACLK clock of Exynos542x SoC. ACLK clock mean
the source clock of AMBA AXI bus. This clock id should be used for Bus
frequency scaling.

Cc: Sylwester Nawrocki <[email protected]>
Cc: Tomasz Figa <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
---
include/dt-bindings/clock/exynos5420.h | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/exynos5420.h b/include/dt-bindings/clock/exynos5420.h
index 7699ee9c16c0..17ab8394bec7 100644
--- a/include/dt-bindings/clock/exynos5420.h
+++ b/include/dt-bindings/clock/exynos5420.h
@@ -217,8 +217,30 @@

/* divider clocks */
#define CLK_DOUT_PIXEL 768
+#define CLK_DOUT_ACLK400_WCORE 769
+#define CLK_DOUT_ACLK400_ISP 770
+#define CLK_DOUT_ACLK400_MSCL 771
+#define CLK_DOUT_ACLK200 772
+#define CLK_DOUT_ACLK200_FSYS2 773
+#define CLK_DOUT_ACLK100_NOC 774
+#define CLK_DOUT_PCLK200_FSYS 775
+#define CLK_DOUT_ACLK200_FSYS 776
+#define CLK_DOUT_ACLK333_432_GSCL 777
+#define CLK_DOUT_ACLK333_432_ISP 778
+#define CLK_DOUT_ACLK66 779
+#define CLK_DOUT_ACLK333_432_ISP0 780
+#define CLK_DOUT_ACLK266 781
+#define CLK_DOUT_ACLK166 782
+#define CLK_DOUT_ACLK333 783
+#define CLK_DOUT_ACLK333_G2D 784
+#define CLK_DOUT_ACLK266_G2D 785
+#define CLK_DOUT_ACLK_G3D 786
+#define CLK_DOUT_ACLK300_JPEG 787
+#define CLK_DOUT_ACLK300_DISP1 788
+#define CLK_DOUT_ACLK300_GSCL 789
+#define CLK_DOUT_ACLK400_DISP1 790

/* must be greater than maximal clock id */
-#define CLK_NR_CLKS 769
+#define CLK_NR_CLKS 791

#endif /* _DT_BINDINGS_CLOCK_EXYNOS_5420_H */
--
1.9.1

2016-04-08 05:01:39

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 5/7] clk: samsung: exynos542x: Add the clock id for ACLK

This patch adds the clock id for ACLK clock which is source clock of AMBA AXI
Bus. This clock should be handled in Bus frequency scaling driver.

Signed-off-by: Chanwoo Choi <[email protected]>
---
drivers/clk/samsung/clk-exynos5420.c | 85 +++++++++++++++++++++++-------------
1 file changed, 55 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index be03ed0fcb6b..d7c62dfb1646 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -554,8 +554,9 @@ static struct samsung_mux_clock exynos5800_mux_clks[] __initdata = {
};

static struct samsung_div_clock exynos5800_div_clks[] __initdata = {
- DIV(0, "dout_aclk400_wcore", "mout_aclk400_wcore", DIV_TOP0, 16, 3),
-
+ DIV_F(CLK_DOUT_ACLK400_WCORE, "dout_aclk400_wcore",
+ "mout_aclk400_wcore",
+ DIV_TOP0, 16, 3, CLK_SET_RATE_PARENT, 0),
DIV(0, "dout_aclk550_cam", "mout_aclk550_cam",
DIV_TOP8, 16, 3),
DIV(0, "dout_aclkfl1_550_cam", "mout_aclkfl1_550_cam",
@@ -607,8 +608,9 @@ static struct samsung_mux_clock exynos5420_mux_clks[] __initdata = {
};

static struct samsung_div_clock exynos5420_div_clks[] __initdata = {
- DIV(0, "dout_aclk400_wcore", "mout_aclk400_wcore_bpll",
- DIV_TOP0, 16, 3),
+ DIV_F(CLK_DOUT_ACLK400_WCORE, "dout_aclk400_wcore",
+ "mout_aclk400_wcore_bpll",
+ DIV_TOP0, 16, 3, CLK_SET_RATE_PARENT, 0),
};

static struct samsung_mux_clock exynos5x_mux_clks[] __initdata = {
@@ -785,31 +787,52 @@ static struct samsung_div_clock exynos5x_div_clks[] __initdata = {
DIV(0, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3),
DIV(0, "sclk_kpll", "mout_kpll", DIV_KFC0, 24, 3),

- DIV(0, "dout_aclk400_isp", "mout_aclk400_isp", DIV_TOP0, 0, 3),
- DIV(0, "dout_aclk400_mscl", "mout_aclk400_mscl", DIV_TOP0, 4, 3),
- DIV(0, "dout_aclk200", "mout_aclk200", DIV_TOP0, 8, 3),
- DIV(0, "dout_aclk200_fsys2", "mout_aclk200_fsys2", DIV_TOP0, 12, 3),
- DIV(0, "dout_aclk100_noc", "mout_aclk100_noc", DIV_TOP0, 20, 3),
- DIV(0, "dout_pclk200_fsys", "mout_pclk200_fsys", DIV_TOP0, 24, 3),
- DIV(0, "dout_aclk200_fsys", "mout_aclk200_fsys", DIV_TOP0, 28, 3),
-
- DIV(0, "dout_aclk333_432_gscl", "mout_aclk333_432_gscl",
- DIV_TOP1, 0, 3),
- DIV(0, "dout_aclk333_432_isp", "mout_aclk333_432_isp",
- DIV_TOP1, 4, 3),
- DIV(0, "dout_aclk66", "mout_aclk66", DIV_TOP1, 8, 6),
- DIV(0, "dout_aclk333_432_isp0", "mout_aclk333_432_isp0",
- DIV_TOP1, 16, 3),
- DIV(0, "dout_aclk266", "mout_aclk266", DIV_TOP1, 20, 3),
- DIV(0, "dout_aclk166", "mout_aclk166", DIV_TOP1, 24, 3),
- DIV(0, "dout_aclk333", "mout_aclk333", DIV_TOP1, 28, 3),
-
- DIV(0, "dout_aclk333_g2d", "mout_aclk333_g2d", DIV_TOP2, 8, 3),
- DIV(0, "dout_aclk266_g2d", "mout_aclk266_g2d", DIV_TOP2, 12, 3),
- DIV(0, "dout_aclk_g3d", "mout_aclk_g3d", DIV_TOP2, 16, 3),
- DIV(0, "dout_aclk300_jpeg", "mout_aclk300_jpeg", DIV_TOP2, 20, 3),
- DIV(0, "dout_aclk300_disp1", "mout_aclk300_disp1", DIV_TOP2, 24, 3),
- DIV(0, "dout_aclk300_gscl", "mout_aclk300_gscl", DIV_TOP2, 28, 3),
+ DIV_F(CLK_DOUT_ACLK400_ISP, "dout_aclk400_isp", "mout_aclk400_isp",
+ DIV_TOP0, 0, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK400_MSCL, "dout_aclk400_mscl", "mout_aclk400_mscl",
+ DIV_TOP0, 4, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK200, "dout_aclk200", "mout_aclk200",
+ DIV_TOP0, 8, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK200_FSYS2, "dout_aclk200_fsys2",
+ "mout_aclk200_fsys2",
+ DIV_TOP0, 12, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK100_NOC, "dout_aclk100_noc", "mout_aclk100_noc",
+ DIV_TOP0, 20, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PCLK200_FSYS, "dout_pclk200_fsys", "mout_pclk200_fsys",
+ DIV_TOP0, 24, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK200_FSYS, "dout_aclk200_fsys", "mout_aclk200_fsys",
+ DIV_TOP0, 28, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK333_432_GSCL, "dout_aclk333_432_gscl",
+ "mout_aclk333_432_gscl",
+ DIV_TOP1, 0, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK333_432_ISP, "dout_aclk333_432_isp",
+ "mout_aclk333_432_isp",
+ DIV_TOP1, 4, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK66, "dout_aclk66", "mout_aclk66",
+ DIV_TOP1, 8, 6, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK333_432_ISP0, "dout_aclk333_432_isp0",
+ "mout_aclk333_432_isp0",
+ DIV_TOP1, 16, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK266, "dout_aclk266", "mout_aclk266",
+ DIV_TOP1, 20, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK166, "dout_aclk166", "mout_aclk166",
+ DIV_TOP1, 24, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK333, "dout_aclk333", "mout_aclk333",
+ DIV_TOP1, 28, 3, CLK_SET_RATE_PARENT, 0),
+
+ DIV_F(CLK_DOUT_ACLK333_G2D, "dout_aclk333_g2d", "mout_aclk333_g2d",
+ DIV_TOP2, 8, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK266_G2D, "dout_aclk266_g2d", "mout_aclk266_g2d",
+ DIV_TOP2, 12, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK_G3D, "dout_aclk_g3d", "mout_aclk_g3d", DIV_TOP2,
+ 16, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK300_JPEG, "dout_aclk300_jpeg", "mout_aclk300_jpeg",
+ DIV_TOP2, 20, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK300_DISP1, "dout_aclk300_disp1",
+ "mout_aclk300_disp1",
+ DIV_TOP2, 24, 3, CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_ACLK300_GSCL, "dout_aclk300_gscl", "mout_aclk300_gscl",
+ DIV_TOP2, 28, 3, CLK_SET_RATE_PARENT, 0),

/* DISP1 Block */
DIV(0, "dout_fimd1", "mout_fimd1_final", DIV_DISP10, 0, 4),
@@ -817,7 +840,9 @@ static struct samsung_div_clock exynos5x_div_clks[] __initdata = {
DIV(0, "dout_dp1", "mout_dp1", DIV_DISP10, 24, 4),
DIV(CLK_DOUT_PIXEL, "dout_hdmi_pixel", "mout_pixel", DIV_DISP10, 28, 4),
DIV(0, "dout_disp1_blk", "aclk200_disp1", DIV2_RATIO0, 16, 2),
- DIV(0, "dout_aclk400_disp1", "mout_aclk400_disp1", DIV_TOP2, 4, 3),
+ DIV_F(CLK_DOUT_ACLK400_DISP1, "dout_aclk400_disp1",
+ "mout_aclk400_disp1",
+ DIV_TOP2, 4, 3, CLK_SET_RATE_PARENT, 0),

/* Audio Block */
DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
--
1.9.1

2016-04-08 05:01:41

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 2/7] PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus

This patch adds the detailed corrleation between sub-blocks and power line
for Exynos5422.

Signed-off-by: Chanwoo Choi <[email protected]>
---
.../devicetree/bindings/devfreq/exynos-bus.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
index b098fa2ba5d4..5a37f51adacf 100644
--- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
+++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
@@ -104,6 +104,25 @@ Detailed correlation between sub-blocks and power line according to Exynos SoC:
|--- LCD0
|--- ISP

+- In case of Exynos5422, there are two power line as following:
+ VDD_MIF |--- DREX 0 (parent device, DRAM EXpress controller)
+ |--- DREX 1
+
+ VDD_INT |--- NoC_Core (parent device)
+ |--- G2D
+ |--- G3D
+ |--- DISP1
+ |--- NoC_WCORE
+ |--- GSCL
+ |--- MSCL
+ |--- ISP
+ |--- MFC
+ |--- GEN
+ |--- PERIS
+ |--- PERIC
+ |--- FSYS
+ |--- FSYS2
+
Example1:
Show the AXI buses of Exynos3250 SoC. Exynos3250 divides the buses to
power line (regulator). The MIF (Memory Interface) AXI bus is used to
--
1.9.1

2016-04-08 05:02:16

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 1/7] PM / devfreq: event: Add new Exynos NoC probe driver

This patch adds NoC (Network on Chip) Probe driver which provides
the primitive values to get the performance data. The packets that the Network
on Chip (NoC) probes detects are transported over the network infrastructure.
Exynos542x bus has multiple NoC probes to provide bandwidth information about
behavior of the SoC that you can use while analyzing system performance.

Signed-off-by: Chanwoo Choi <[email protected]>
---
.../bindings/devfreq/event/exynos-nocp.txt | 86 +++++++
drivers/devfreq/event/Kconfig | 8 +
drivers/devfreq/event/Makefile | 2 +
drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++++++++++
drivers/devfreq/event/exynos-nocp.h | 78 +++++++
5 files changed, 421 insertions(+)
create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
create mode 100644 drivers/devfreq/event/exynos-nocp.c
create mode 100644 drivers/devfreq/event/exynos-nocp.h

diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
new file mode 100644
index 000000000000..03b74fed034c
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
@@ -0,0 +1,86 @@
+
+* Samsung Exynos NoC (Network on Chip) Probe device
+
+The Samsung Exynos542x SoC has NoC (Network on Chip) Probe for NoC bus.
+NoC provides the primitive values to get the performance data. The packets
+that the Network on Chip (NoC) probes detects are transported over
+the network infrastructure to observer units. You can configure probes to
+capture packets with header or data on the data request response network,
+or as traffic debug or statistic collectors. Exynos542x bus has multiple
+NoC probes to provide bandwidth information about behavior of the SoC
+that you can use while analyzing system performance.
+
+Required properties:
+- compatible: Should be "samsung,exynos5420-nocp"
+- reg: physical base address of each NoC Probe and length of memory mapped region.
+
+Optional properties:
+- clock-names : the name of clock used by the NoC Probe, "nocp"
+- clocks : phandles for clock specified in "clock-names" property
+
+Example1 : NoC Probe nodes in exynos5420.dtsi are listed below.
+
+ nocp_mem0_0: nocp_mem0_0@10CA1000 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1000 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_1: nocp_mem0_1@10CA1400 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1400 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_2: nocp_mem0_2@10CA1800 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1800 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_3: nocp_mem0_0@10CA1C00 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1C00 0x200>;
+ status = "disabled";
+ };
+
+ nocp_g3d_0: nocp_g3d_0@11A51000 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x11A51000 0x200>;
+ status = "disabled";
+ };
+
+ nocp_g3d_1: nocp_g3d_1@11A51400 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x11A51400 0x200>;
+ status = "disabled";
+ };
+
+
+Example2 : Events of each NoC Probe node in exynos5422-odroidxu3-common.dtsi
+ are listed below.
+
+
+ &nocp_mem0_0 {
+ status = "okay";
+ };
+
+ &nocp_mem0_1 {
+ status = "okay";
+ };
+
+ &nocp_mem0_2 {
+ status = "okay";
+ };
+
+ &nocp_mem0_3 {
+ status = "okay";
+ };
+
+ &nocp_g3d_0 {
+ status = "okay";
+ };
+
+ &nocp_g3d_1 {
+ status = "okay";
+ };
diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig
index a11720affc31..1e8b4f469f38 100644
--- a/drivers/devfreq/event/Kconfig
+++ b/drivers/devfreq/event/Kconfig
@@ -13,6 +13,14 @@ menuconfig PM_DEVFREQ_EVENT

if PM_DEVFREQ_EVENT

+config DEVFREQ_EVENT_EXYNOS_NOCP
+ bool "EXYNOS NoC (Network On Chip) Probe DEVFREQ event Driver"
+ depends on ARCH_EXYNOS
+ select PM_OPP
+ help
+ This add the devfreq-event driver for Exynos SoC. It provides NoC
+ (Network on Chip) Probe counters to measure the bandwidth of AXI bus.
+
config DEVFREQ_EVENT_EXYNOS_PPMU
bool "EXYNOS PPMU (Platform Performance Monitoring Unit) DEVFREQ event Driver"
depends on ARCH_EXYNOS
diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
index be146ead79cf..3d6afd352253 100644
--- a/drivers/devfreq/event/Makefile
+++ b/drivers/devfreq/event/Makefile
@@ -1,2 +1,4 @@
# Exynos DEVFREQ Event Drivers
+
+obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o
obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
diff --git a/drivers/devfreq/event/exynos-nocp.c b/drivers/devfreq/event/exynos-nocp.c
new file mode 100644
index 000000000000..b9468a6143f6
--- /dev/null
+++ b/drivers/devfreq/event/exynos-nocp.c
@@ -0,0 +1,247 @@
+/*
+ * exynos-nocp.c - EXYNOS NoC (Network On Chip) Probe support
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Author : Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/devfreq-event.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "exynos-nocp.h"
+
+struct exynos_nocp {
+ struct devfreq_event_dev *edev;
+ struct devfreq_event_desc desc;
+
+ struct device *dev;
+
+ struct regmap *regmap;
+ struct clk *clk;
+};
+
+/*
+ * The devfreq-event ops structure for nocp probe.
+ */
+static int exynos_nocp_set_event(struct devfreq_event_dev *edev)
+{
+ struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
+
+ /* Disable NoC probe */
+ regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
+ NOCP_MAIN_CTL_STATEN_MASK, 0);
+
+ /* Set a statistics dump period to 0 */
+ regmap_write(nocp->regmap, NOCP_STAT_PERIOD, 0x0);
+
+ /* Set the IntEvent fields of *_SRC */
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_SRC,
+ NOCP_CNT_SRC_INTEVENT_MASK,
+ NOCP_CNT_SRC_INTEVENT_BYTE_MASK);
+
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_SRC,
+ NOCP_CNT_SRC_INTEVENT_MASK,
+ NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
+
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_SRC,
+ NOCP_CNT_SRC_INTEVENT_MASK,
+ NOCP_CNT_SRC_INTEVENT_CYCLE_MASK);
+
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_SRC,
+ NOCP_CNT_SRC_INTEVENT_MASK,
+ NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
+
+
+ /* Set an alarm with a max/min value of 0 to generate StatALARM */
+ regmap_write(nocp->regmap, NOCP_STAT_ALARM_MIN, 0x0);
+ regmap_write(nocp->regmap, NOCP_STAT_ALARM_MAX, 0x0);
+
+ /* Set AlarmMode */
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_ALARM_MODE,
+ NOCP_CNT_ALARM_MODE_MASK,
+ NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_ALARM_MODE,
+ NOCP_CNT_ALARM_MODE_MASK,
+ NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_ALARM_MODE,
+ NOCP_CNT_ALARM_MODE_MASK,
+ NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
+ regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_ALARM_MODE,
+ NOCP_CNT_ALARM_MODE_MASK,
+ NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
+
+ /* Enable the measurements by setting AlarmEn and StatEn */
+ regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
+ NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK,
+ NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK);
+
+ /* Set GlobalEN */
+ regmap_update_bits(nocp->regmap, NOCP_CFG_CTL,
+ NOCP_CFG_CTL_GLOBALEN_MASK,
+ NOCP_CFG_CTL_GLOBALEN_MASK);
+
+ /* Enable NoC probe */
+ regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
+ NOCP_MAIN_CTL_STATEN_MASK,
+ NOCP_MAIN_CTL_STATEN_MASK);
+
+ return 0;
+}
+
+static int exynos_nocp_get_event(struct devfreq_event_dev *edev,
+ struct devfreq_event_data *edata)
+{
+ struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
+ unsigned int counter[4];
+
+ /* Read cycle count */
+ regmap_read(nocp->regmap, NOCP_COUNTERS_0_VAL, &counter[0]);
+ regmap_read(nocp->regmap, NOCP_COUNTERS_1_VAL, &counter[1]);
+ regmap_read(nocp->regmap, NOCP_COUNTERS_2_VAL, &counter[2]);
+ regmap_read(nocp->regmap, NOCP_COUNTERS_3_VAL, &counter[3]);
+
+ edata->load_count = ((counter[1] << 16) | counter[0]);
+ edata->total_count = ((counter[3] << 16) | counter[2]);
+
+ dev_dbg(&edev->dev, "%s (event: %ld/%ld)\n", edev->desc->name,
+ edata->load_count, edata->total_count);
+
+ return 0;
+}
+
+static const struct devfreq_event_ops exynos_nocp_ops = {
+ .set_event = exynos_nocp_set_event,
+ .get_event = exynos_nocp_get_event,
+};
+
+static const struct of_device_id exynos_nocp_id_match[] = {
+ { .compatible = "samsung,exynos5420-nocp", },
+ { /* sentinel */ },
+};
+
+static struct regmap_config exynos_nocp_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = NOCP_COUNTERS_3_VAL,
+};
+
+static int exynos_nocp_parse_dt(struct platform_device *pdev,
+ struct exynos_nocp *nocp)
+{
+ struct device *dev = nocp->dev;
+ struct device_node *np = dev->of_node;
+ struct resource *res;
+ void __iomem *base;
+ int ret = 0;
+
+ if (!np) {
+ dev_err(dev, "failed to find devicetree node\n");
+ return -EINVAL;
+ }
+
+ nocp->clk = devm_clk_get(dev, "nocp");
+ if (IS_ERR(nocp->clk))
+ nocp->clk = NULL;
+
+ /* Maps the memory mapped IO to control nocp register */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (IS_ERR(res))
+ return PTR_ERR(res);
+
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+ exynos_nocp_regmap_config.max_register = resource_size(res) - 4;
+
+ nocp->regmap = devm_regmap_init_mmio(dev, base,
+ &exynos_nocp_regmap_config);
+ if (IS_ERR(nocp->regmap)) {
+ dev_err(dev, "failed to initialize regmap\n");
+ ret = PTR_ERR(nocp->regmap);
+ goto err;
+ }
+
+ return 0;
+
+err:
+ devm_iounmap(dev, base);
+
+ return ret;
+}
+
+static int exynos_nocp_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct exynos_nocp *nocp;
+ int ret;
+
+ nocp = devm_kzalloc(&pdev->dev, sizeof(*nocp), GFP_KERNEL);
+ if (!nocp)
+ return -ENOMEM;
+
+ nocp->dev = &pdev->dev;
+
+ /* Parse dt data to get resource */
+ ret = exynos_nocp_parse_dt(pdev, nocp);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "failed to parse devicetree for resource\n");
+ return ret;
+ }
+
+ /* Add devfreq-event device to measure the bandwidth of NoC */
+ nocp->desc.ops = &exynos_nocp_ops;
+ nocp->desc.driver_data = nocp;
+ nocp->desc.name = np->name;
+ nocp->edev = devm_devfreq_event_add_edev(&pdev->dev, &nocp->desc);
+ if (IS_ERR(nocp->edev)) {
+ ret = PTR_ERR(nocp->edev);
+ dev_err(&pdev->dev,
+ "failed to add devfreq-event device\n");
+ goto err;
+ }
+ platform_set_drvdata(pdev, nocp);
+
+ clk_prepare_enable(nocp->clk);
+
+ pr_info("exynos-nocp: new NoC Probe device registered: %s\n",
+ dev_name(dev));
+
+ return 0;
+err:
+ return ret;
+}
+
+static int exynos_nocp_remove(struct platform_device *pdev)
+{
+ struct exynos_nocp *nocp = platform_get_drvdata(pdev);
+
+ clk_disable_unprepare(nocp->clk);
+
+ return 0;
+}
+
+static struct platform_driver exynos_nocp_driver = {
+ .probe = exynos_nocp_probe,
+ .remove = exynos_nocp_remove,
+ .driver = {
+ .name = "exynos-nocp",
+ .of_match_table = exynos_nocp_id_match,
+ },
+};
+module_platform_driver(exynos_nocp_driver);
+
+MODULE_DESCRIPTION("Exynos NoC (Network on Chip) Probe driver");
+MODULE_AUTHOR("Chanwoo Choi <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/devfreq/event/exynos-nocp.h b/drivers/devfreq/event/exynos-nocp.h
new file mode 100644
index 000000000000..28564db0edb8
--- /dev/null
+++ b/drivers/devfreq/event/exynos-nocp.h
@@ -0,0 +1,78 @@
+/*
+ * exynos-nocp.h - EXYNOS NoC (Network on Chip) Probe header file
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Author : Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __EXYNOS_NOCP_H__
+#define __EXYNOS_NOCP_H__
+
+enum nocp_reg {
+ NOCP_ID_REVISION_ID = 0x04,
+ NOCP_MAIN_CTL = 0x08,
+ NOCP_CFG_CTL = 0x0C,
+
+ NOCP_STAT_PERIOD = 0x24,
+ NOCP_STAT_GO = 0x28,
+ NOCP_STAT_ALARM_MIN = 0x2C,
+ NOCP_STAT_ALARM_MAX = 0x30,
+ NOCP_STAT_ALARM_STATUS = 0x34,
+ NOCP_STAT_ALARM_CLR = 0x38,
+
+ NOCP_COUNTERS_0_SRC = 0x138,
+ NOCP_COUNTERS_0_ALARM_MODE = 0x13C,
+ NOCP_COUNTERS_0_VAL = 0x140,
+
+ NOCP_COUNTERS_1_SRC = 0x14C,
+ NOCP_COUNTERS_1_ALARM_MODE = 0x150,
+ NOCP_COUNTERS_1_VAL = 0x154,
+
+ NOCP_COUNTERS_2_SRC = 0x160,
+ NOCP_COUNTERS_2_ALARM_MODE = 0x164,
+ NOCP_COUNTERS_2_VAL = 0x168,
+
+ NOCP_COUNTERS_3_SRC = 0x174,
+ NOCP_COUNTERS_3_ALARM_MODE = 0x178,
+ NOCP_COUNTERS_3_VAL = 0x17C,
+};
+
+/* NOCP_MAIN_CTL register */
+#define NOCP_MAIN_CTL_ERREN_MASK BIT(0)
+#define NOCP_MAIN_CTL_TRACEEN_MASK BIT(1)
+#define NOCP_MAIN_CTL_PAYLOADEN_MASK BIT(2)
+#define NOCP_MAIN_CTL_STATEN_MASK BIT(3)
+#define NOCP_MAIN_CTL_ALARMEN_MASK BIT(4)
+#define NOCP_MAIN_CTL_STATCONDDUMP_MASK BIT(5)
+#define NOCP_MAIN_CTL_INTRUSIVEMODE_MASK BIT(6)
+
+/* NOCP_CFG_CTL register */
+#define NOCP_CFG_CTL_GLOBALEN_MASK BIT(0)
+#define NOCP_CFG_CTL_ACTIVE_MASK BIT(1)
+
+/* NOCP_COUNTERS_x_SRC register */
+#define NOCP_CNT_SRC_INTEVENT_SHIFT 0
+#define NOCP_CNT_SRC_INTEVENT_MASK (0x1F << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_OFF_MASK (0x0 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_CYCLE_MASK (0x1 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_IDLE_MASK (0x2 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_XFER_MASK (0x3 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_BUSY_MASK (0x4 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_WAIT_MASK (0x5 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_PKT_MASK (0x6 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_BYTE_MASK (0x8 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+#define NOCP_CNT_SRC_INTEVENT_CHAIN_MASK (0x10 << NOCP_CNT_SRC_INTEVENT_SHIFT)
+
+/* NOCP_COUNTERS_x_ALARM_MODE register */
+#define NOCP_CNT_ALARM_MODE_SHIFT 0
+#define NOCP_CNT_ALARM_MODE_MASK (0x3 << NOCP_CNT_ALARM_MODE_SHIFT)
+#define NOCP_CNT_ALARM_MODE_OFF_MASK (0x0 << NOCP_CNT_ALARM_MODE_SHIFT)
+#define NOCP_CNT_ALARM_MODE_MIN_MASK (0x1 << NOCP_CNT_ALARM_MODE_SHIFT)
+#define NOCP_CNT_ALARM_MODE_MAX_MASK (0x2 << NOCP_CNT_ALARM_MODE_SHIFT)
+#define NOCP_CNT_ALARM_MODE_MIN_MAX_MASK (0x3 << NOCP_CNT_ALARM_MODE_SHIFT)
+
+#endif /* __EXYNOS_NOCP_H__ */
--
1.9.1

2016-04-08 05:00:56

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 3/7] ARM: dts: Add NoC Probe dt node for Exynos542x SoC

This patch adds the NoCP (Network on Chip Probe) Device Tree node
to measure the bandwidth of memory and g3d in Exynos542x SoC.

Signed-off-by: Chanwoo Choi <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 7b99cb58d82d..d80f3b66f017 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -294,6 +294,42 @@
};
};

+ nocp_mem0_0: nocp_mem0_0@10CA1000 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1000 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_1: nocp_mem0_1@10CA1400 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1400 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_2: nocp_mem0_2@10CA1800 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1800 0x200>;
+ status = "disabled";
+ };
+
+ nocp_mem0_3: nocp_mem0_3@10CA1C00 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x10CA1C00 0x200>;
+ status = "disabled";
+ };
+
+ nocp_g3d_0: nocp_g3d_0@11A51000 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x11A51000 0x200>;
+ status = "disabled";
+ };
+
+ nocp_g3d_1: nocp_g3d_1@11A51400 {
+ compatible = "samsung,exynos5420-nocp";
+ reg = <0x11A51400 0x200>;
+ status = "disabled";
+ };
+
gsc_pd: power-domain@10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
--
1.9.1

2016-04-08 05:02:43

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH 7/7] ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3

This patch adds the bus device tree nodes for INT (Internal) block
to enable the AMBA bus frequency scaling and add the NoC (Network on Chip)
Probe Device Tree node to measure the bandwidht for AMBA AXI bus.

The WCORE bus bus is parent device in INT block using VDD_INT.

Signed-off-by: Chanwoo Choi <[email protected]>
---
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 ++++++++++++++++++++++
1 file changed, 99 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 1bd507bfa750..2a74abe6fc5d 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -361,6 +361,22 @@
cap-sd-highspeed;
};

+&nocp_mem0_0 {
+ status = "okay";
+};
+
+&nocp_mem0_1 {
+ status = "okay";
+};
+
+&nocp_mem0_2 {
+ status = "okay";
+};
+
+&nocp_mem0_3 {
+ status = "okay";
+};
+
&pinctrl_0 {
hdmi_hpd_irq: hdmi-hpd-irq {
samsung,pins = "gpx3-7";
@@ -432,3 +448,86 @@
vdd33-supply = <&ldo9_reg>;
vdd10-supply = <&ldo11_reg>;
};
+
+&bus_wcore {
+ devfreq-events = <&nocp_mem0_0>, <&nocp_mem0_1>,
+ <&nocp_mem0_2>, <&nocp_mem0_3>;
+ vdd-supply = <&buck3_reg>;
+ exynos,saturation-ratio = <100>;
+ status = "okay";
+};
+
+&bus_noc {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_fsys_apb {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_fsys {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_fsys2 {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_mfc {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_gen {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_peri {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_g2d {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_g2d_acp {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_jpeg {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_jpeg_apb {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_disp1_fimd {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_disp1 {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_gscl_scaler {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
+
+&bus_mscl {
+ devfreq = <&bus_wcore>;
+ status = "okay";
+};
--
1.9.1

2016-04-08 12:55:58

by Markus Reichl

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Chanwoo,

I tested both sets on odroid XU4 and U3.
Tested-by: Markus Reichl <[email protected]>

Am 08.04.2016 um 07:00 schrieb Chanwoo Choi:
> This patchset support the AMBA bus frequency scaling on Exynos5422-based
> Odroid-XU3 board. But, this series only support the bus frequency scaling
> for INT (Internal) block using VDD_INT power line.
>
> Also, to support the bus frequency scaling for Exynos542x SoC,
> Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
> to measure the transfered data traffic on NoC (Network on Chip)
> instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
> provide the utilization for INT block of Exynos542x SoC.
>
> The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
> device (drivers/devfreq/event/exynos-nocp.c) without any modification.
> Just add the phandle of 'NoC Probe' dt node to bus dt node.
>
> Depend on:
> This patchset depends on patch[1] which support the generic exynos-bus
> frequency driver.
>
> [1] https://lkml.org/lkml/2016/4/8/14
> - [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor
>
> Chanwoo Choi (7):
> PM / devfreq: event: Add new Exynos NoC probe driver
> PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus
> ARM: dts: Add NoC Probe dt node for Exynos542x SoC
> dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC
> clk: samsung: exynos542x: Add the clock id for ACLK
> ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC
> ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3
>
> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++
> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +
> arch/arm/boot/dts/exynos5420.dtsi | 407 +++++++++++++++++++++
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 +++++
> drivers/clk/samsung/clk-exynos5420.c | 85 +++--
> drivers/devfreq/event/Kconfig | 8 +
> drivers/devfreq/event/Makefile | 2 +
> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++
> drivers/devfreq/event/exynos-nocp.h | 78 ++++
> include/dt-bindings/clock/exynos5420.h | 24 +-
> 10 files changed, 1024 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
> create mode 100644 drivers/devfreq/event/exynos-nocp.c
> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>

Thanks,
--
Markus Reichl

2016-04-08 18:11:43

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Chanwoo,

On 8 April 2016 at 10:30, Chanwoo Choi <[email protected]> wrote:
> This patchset support the AMBA bus frequency scaling on Exynos5422-based
> Odroid-XU3 board. But, this series only support the bus frequency scaling
> for INT (Internal) block using VDD_INT power line.
>
> Also, to support the bus frequency scaling for Exynos542x SoC,
> Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
> to measure the transfered data traffic on NoC (Network on Chip)
> instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
> provide the utilization for INT block of Exynos542x SoC.
>
> The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
> device (drivers/devfreq/event/exynos-nocp.c) without any modification.
> Just add the phandle of 'NoC Probe' dt node to bus dt node.
>
> Depend on:
> This patchset depends on patch[1] which support the generic exynos-bus
> frequency driver.
>
> [1] https://lkml.org/lkml/2016/4/8/14
> - [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor
>
> Chanwoo Choi (7):
> PM / devfreq: event: Add new Exynos NoC probe driver
> PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus
> ARM: dts: Add NoC Probe dt node for Exynos542x SoC
> dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC
> clk: samsung: exynos542x: Add the clock id for ACLK
> ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC
> ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3
>
> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++
> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +
> arch/arm/boot/dts/exynos5420.dtsi | 407 +++++++++++++++++++++
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 +++++
> drivers/clk/samsung/clk-exynos5420.c | 85 +++--
> drivers/devfreq/event/Kconfig | 8 +
> drivers/devfreq/event/Makefile | 2 +
> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++
> drivers/devfreq/event/exynos-nocp.h | 78 ++++
> include/dt-bindings/clock/exynos5420.h | 24 +-
> 10 files changed, 1024 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
> create mode 100644 drivers/devfreq/event/exynos-nocp.c
> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>
> --
> 1.9.1
>

I am observing following deadlock. Both on Odroid U3 and Odroid XU4.

[ 7.718372] [ INFO: possible recursive locking detected ]
[ 7.723747] 4.6.0-rc2-xu4ml #4 Not tainted
[ 7.727817] ---------------------------------------------
[ 7.733190] kworker/u16:1/136 is trying to acquire lock:
[ 7.738476] (&devfreq->lock){+.+.+.}, at: [<c05be41c>]
update_devfreq_passive+0x2c/0x88
[ 7.746534]
[ 7.746534] but task is already holding lock:
[ 7.752339] (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
devfreq_monitor+0x1c/0x78
[ 7.759791]
[ 7.759791] other info that might help us debug this:
[ 7.766291] Possible unsafe locking scenario:
[ 7.766291]
[ 7.766307] usb 5-1: new high-speed USB device number 2 using xhci-hcd
[ 7.778682] CPU0
[ 7.781107] ----
[ 7.783533] lock(&devfreq->lock);
[ 7.786999] lock(&devfreq->lock);
[ 7.790467]
[ 7.790467] *** DEADLOCK ***
[ 7.790467]
[ 7.796359] May be due to missing lock nesting notation
[ 7.796359]
[ 7.803120] 4 locks held by kworker/u16:1/136:
[ 7.807537] #0: ("%s"("devfreq_wq")){.+.+..}, at: [<c013b41c>]
process_one_work+0x13c/0x454
[ 7.816029] #1: ((&(&devfreq->work)->work)){+.+...}, at:
[<c013b41c>] process_one_work+0x13c/0x454
[ 7.825128] #2: (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
devfreq_monitor+0x1c/0x78
[ 7.833013] #3: (&nh->srcu){......}, at: [<c014309c>]
__srcu_notifier_call_chain+0x54/0xe0
[ 7.841419]
[ 7.841419] stack backtrace:
[ 7.845758] CPU: 1 PID: 136 Comm: kworker/u16:1 Not tainted
4.6.0-rc2-xu4ml #4
[ 7.852946] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 7.859017] Workqueue: devfreq_wq devfreq_monitor
[ 7.863717] [<c010eb48>] (unwind_backtrace) from [<c010b884>]
(show_stack+0x10/0x14)
[ 7.871417] [<c010b884>] (show_stack) from [<c037715c>]
(dump_stack+0xa4/0xd0)
[ 7.878613] [<c037715c>] (dump_stack) from [<c017112c>]
(__lock_acquire+0x1ce8/0x207c)
[ 7.886492] [<c017112c>] (__lock_acquire) from [<c0172010>]
(lock_acquire+0x90/0xd4)
[ 7.892726] usb 5-1: New USB device found, idVendor=0bda, idProduct=8153
[ 7.892737] usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6
[ 7.892745] usb 5-1: Product: USB 10/100/1000 LAN
[ 7.892753] usb 5-1: Manufacturer: Realtek
[ 7.892761] usb 5-1: SerialNumber: 000001000000
[ 7.921248] [<c0172010>] (lock_acquire) from [<c075d298>]
(mutex_lock_nested+0x78/0x4ec)
[ 7.929301] [<c075d298>] (mutex_lock_nested) from [<c05be41c>]
(update_devfreq_passive+0x2c/0x88)
[ 7.938138] [<c05be41c>] (update_devfreq_passive) from [<c05be4c0>]
(devfreq_passive_notifier_call+0x48/0x50)
[ 7.948016] [<c05be4c0>] (devfreq_passive_notifier_call) from
[<c0142d84>] (notifier_call_chain+0x54/0xa4)
[ 7.957633] [<c0142d84>] (notifier_call_chain) from [<c01430e0>]
(__srcu_notifier_call_chain+0x98/0xe0)
[ 7.966991] [<c01430e0>] (__srcu_notifier_call_chain) from
[<c0143140>] (srcu_notifier_call_chain+0x18/0x20)
[ 7.976783] [<c0143140>] (srcu_notifier_call_chain) from
[<c05bd774>] (update_devfreq+0xe4/0x18c)
[ 7.985622] [<c05bd774>] (update_devfreq) from [<c05bd840>]
(devfreq_monitor+0x24/0x78)
[ 7.993595] [<c05bd840>] (devfreq_monitor) from [<c013b48c>]
(process_one_work+0x1ac/0x454)
[ 8.001913] [<c013b48c>] (process_one_work) from [<c013b784>]
(worker_thread+0x50/0x508)
[ 8.009976] [<c013b784>] (worker_thread) from [<c0141cf4>]
(kthread+0xf8/0x110)
[ 8.017254] [<c0141cf4>] (kthread) from [<c0107750>]
(ret_from_fork+0x14/0x24)

Best Regards
-Anand Moon

2016-04-08 18:24:45

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Anand,

On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <[email protected]> wrote:
> Hi Chanwoo,
>
> On 8 April 2016 at 10:30, Chanwoo Choi <[email protected]> wrote:
>> This patchset support the AMBA bus frequency scaling on Exynos5422-based
>> Odroid-XU3 board. But, this series only support the bus frequency scaling
>> for INT (Internal) block using VDD_INT power line.
>>
>> Also, to support the bus frequency scaling for Exynos542x SoC,
>> Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
>> to measure the transfered data traffic on NoC (Network on Chip)
>> instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
>> provide the utilization for INT block of Exynos542x SoC.
>>
>> The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
>> device (drivers/devfreq/event/exynos-nocp.c) without any modification.
>> Just add the phandle of 'NoC Probe' dt node to bus dt node.
>>
>> Depend on:
>> This patchset depends on patch[1] which support the generic exynos-bus
>> frequency driver.
>>
>> [1] https://lkml.org/lkml/2016/4/8/14
>> - [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor
>>
>> Chanwoo Choi (7):
>> PM / devfreq: event: Add new Exynos NoC probe driver
>> PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus
>> ARM: dts: Add NoC Probe dt node for Exynos542x SoC
>> dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC
>> clk: samsung: exynos542x: Add the clock id for ACLK
>> ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC
>> ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3
>>
>> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++
>> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +
>> arch/arm/boot/dts/exynos5420.dtsi | 407 +++++++++++++++++++++
>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 +++++
>> drivers/clk/samsung/clk-exynos5420.c | 85 +++--
>> drivers/devfreq/event/Kconfig | 8 +
>> drivers/devfreq/event/Makefile | 2 +
>> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++
>> drivers/devfreq/event/exynos-nocp.h | 78 ++++
>> include/dt-bindings/clock/exynos5420.h | 24 +-
>> 10 files changed, 1024 insertions(+), 31 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
>> create mode 100644 drivers/devfreq/event/exynos-nocp.c
>> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>>
>> --
>> 1.9.1
>>
>
> I am observing following deadlock. Both on Odroid U3 and Odroid XU4.

Thanks for your test. I'll test it again and fix it.

Best Regards,
Chanwoo Choi

>
> [ 7.718372] [ INFO: possible recursive locking detected ]
> [ 7.723747] 4.6.0-rc2-xu4ml #4 Not tainted
> [ 7.727817] ---------------------------------------------
> [ 7.733190] kworker/u16:1/136 is trying to acquire lock:
> [ 7.738476] (&devfreq->lock){+.+.+.}, at: [<c05be41c>]
> update_devfreq_passive+0x2c/0x88
> [ 7.746534]
> [ 7.746534] but task is already holding lock:
> [ 7.752339] (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
> devfreq_monitor+0x1c/0x78
> [ 7.759791]
> [ 7.759791] other info that might help us debug this:
> [ 7.766291] Possible unsafe locking scenario:
> [ 7.766291]
> [ 7.766307] usb 5-1: new high-speed USB device number 2 using xhci-hcd
> [ 7.778682] CPU0
> [ 7.781107] ----
> [ 7.783533] lock(&devfreq->lock);
> [ 7.786999] lock(&devfreq->lock);
> [ 7.790467]
> [ 7.790467] *** DEADLOCK ***
> [ 7.790467]
> [ 7.796359] May be due to missing lock nesting notation
> [ 7.796359]
> [ 7.803120] 4 locks held by kworker/u16:1/136:
> [ 7.807537] #0: ("%s"("devfreq_wq")){.+.+..}, at: [<c013b41c>]
> process_one_work+0x13c/0x454
> [ 7.816029] #1: ((&(&devfreq->work)->work)){+.+...}, at:
> [<c013b41c>] process_one_work+0x13c/0x454
> [ 7.825128] #2: (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
> devfreq_monitor+0x1c/0x78
> [ 7.833013] #3: (&nh->srcu){......}, at: [<c014309c>]
> __srcu_notifier_call_chain+0x54/0xe0
> [ 7.841419]
> [ 7.841419] stack backtrace:
> [ 7.845758] CPU: 1 PID: 136 Comm: kworker/u16:1 Not tainted
> 4.6.0-rc2-xu4ml #4
> [ 7.852946] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 7.859017] Workqueue: devfreq_wq devfreq_monitor
> [ 7.863717] [<c010eb48>] (unwind_backtrace) from [<c010b884>]
> (show_stack+0x10/0x14)
> [ 7.871417] [<c010b884>] (show_stack) from [<c037715c>]
> (dump_stack+0xa4/0xd0)
> [ 7.878613] [<c037715c>] (dump_stack) from [<c017112c>]
> (__lock_acquire+0x1ce8/0x207c)
> [ 7.886492] [<c017112c>] (__lock_acquire) from [<c0172010>]
> (lock_acquire+0x90/0xd4)
> [ 7.892726] usb 5-1: New USB device found, idVendor=0bda, idProduct=8153
> [ 7.892737] usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6
> [ 7.892745] usb 5-1: Product: USB 10/100/1000 LAN
> [ 7.892753] usb 5-1: Manufacturer: Realtek
> [ 7.892761] usb 5-1: SerialNumber: 000001000000
> [ 7.921248] [<c0172010>] (lock_acquire) from [<c075d298>]
> (mutex_lock_nested+0x78/0x4ec)
> [ 7.929301] [<c075d298>] (mutex_lock_nested) from [<c05be41c>]
> (update_devfreq_passive+0x2c/0x88)
> [ 7.938138] [<c05be41c>] (update_devfreq_passive) from [<c05be4c0>]
> (devfreq_passive_notifier_call+0x48/0x50)
> [ 7.948016] [<c05be4c0>] (devfreq_passive_notifier_call) from
> [<c0142d84>] (notifier_call_chain+0x54/0xa4)
> [ 7.957633] [<c0142d84>] (notifier_call_chain) from [<c01430e0>]
> (__srcu_notifier_call_chain+0x98/0xe0)
> [ 7.966991] [<c01430e0>] (__srcu_notifier_call_chain) from
> [<c0143140>] (srcu_notifier_call_chain+0x18/0x20)
> [ 7.976783] [<c0143140>] (srcu_notifier_call_chain) from
> [<c05bd774>] (update_devfreq+0xe4/0x18c)
> [ 7.985622] [<c05bd774>] (update_devfreq) from [<c05bd840>]
> (devfreq_monitor+0x24/0x78)
> [ 7.993595] [<c05bd840>] (devfreq_monitor) from [<c013b48c>]
> (process_one_work+0x1ac/0x454)
> [ 8.001913] [<c013b48c>] (process_one_work) from [<c013b784>]
> (worker_thread+0x50/0x508)
> [ 8.009976] [<c013b784>] (worker_thread) from [<c0141cf4>]
> (kthread+0xf8/0x110)
> [ 8.017254] [<c0141cf4>] (kthread) from [<c0107750>]
> (ret_from_fork+0x14/0x24)
>
> Best Regards
> -Anand Moon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2016-04-08 18:25:26

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Markus,

On Fri, Apr 8, 2016 at 9:55 PM, Markus Reichl <[email protected]> wrote:
> Hi Chanwoo,
>
> I tested both sets on odroid XU4 and U3.
> Tested-by: Markus Reichl <[email protected]>

Thanks for your test always.

Best Regards,
Chanwoo Choi

>
> Am 08.04.2016 um 07:00 schrieb Chanwoo Choi:
>> This patchset support the AMBA bus frequency scaling on Exynos5422-based
>> Odroid-XU3 board. But, this series only support the bus frequency scaling
>> for INT (Internal) block using VDD_INT power line.
>>
>> Also, to support the bus frequency scaling for Exynos542x SoC,
>> Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
>> to measure the transfered data traffic on NoC (Network on Chip)
>> instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
>> provide the utilization for INT block of Exynos542x SoC.
>>
>> The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
>> device (drivers/devfreq/event/exynos-nocp.c) without any modification.
>> Just add the phandle of 'NoC Probe' dt node to bus dt node.
>>
>> Depend on:
>> This patchset depends on patch[1] which support the generic exynos-bus
>> frequency driver.
>>
>> [1] https://lkml.org/lkml/2016/4/8/14
>> - [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor
>>
>> Chanwoo Choi (7):
>> PM / devfreq: event: Add new Exynos NoC probe driver
>> PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus
>> ARM: dts: Add NoC Probe dt node for Exynos542x SoC
>> dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC
>> clk: samsung: exynos542x: Add the clock id for ACLK
>> ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC
>> ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3
>>
>> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++
>> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +
>> arch/arm/boot/dts/exynos5420.dtsi | 407 +++++++++++++++++++++
>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 +++++
>> drivers/clk/samsung/clk-exynos5420.c | 85 +++--
>> drivers/devfreq/event/Kconfig | 8 +
>> drivers/devfreq/event/Makefile | 2 +
>> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++
>> drivers/devfreq/event/exynos-nocp.h | 78 ++++
>> include/dt-bindings/clock/exynos5420.h | 24 +-
>> 10 files changed, 1024 insertions(+), 31 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
>> create mode 100644 drivers/devfreq/event/exynos-nocp.c
>> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>>
>
> Thanks,
> --
> Markus Reichl
>

2016-04-11 02:16:47

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Anand,

On 2016년 04월 09일 03:24, Chanwoo Choi wrote:
> Hi Anand,
>
> On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <[email protected]> wrote:
>> Hi Chanwoo,
>>

[snip]

>>>
>>
>> I am observing following deadlock. Both on Odroid U3 and Odroid XU4.
>
> Thanks for your test. I'll test it again and fix it.

This possible recursive locking is fixed with following diff:

Thanks for your report. I'll fix it on next patchset[1] (v9).
[1] https://lkml.org/lkml/2016/4/8/14


diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 28a9ae32d330..a4b0b02ee797 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -102,7 +102,7 @@ static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
if (!devfreq->governor)
return -EINVAL;

- mutex_lock(&devfreq->lock);
+ mutex_lock_nested(&devfreq->lock, SINGLE_DEPTH_NESTING);

ret = devfreq->governor->get_target_freq(devfreq, &freq);
if (ret < 0)

Best Regards,
Chanwoo Choi

>
>>
>> [ 7.718372] [ INFO: possible recursive locking detected ]
>> [ 7.723747] 4.6.0-rc2-xu4ml #4 Not tainted
>> [ 7.727817] ---------------------------------------------
>> [ 7.733190] kworker/u16:1/136 is trying to acquire lock:
>> [ 7.738476] (&devfreq->lock){+.+.+.}, at: [<c05be41c>]
>> update_devfreq_passive+0x2c/0x88
>> [ 7.746534]
>> [ 7.746534] but task is already holding lock:
>> [ 7.752339] (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
>> devfreq_monitor+0x1c/0x78
>> [ 7.759791]
>> [ 7.759791] other info that might help us debug this:
>> [ 7.766291] Possible unsafe locking scenario:
>> [ 7.766291]
>> [ 7.766307] usb 5-1: new high-speed USB device number 2 using xhci-hcd
>> [ 7.778682] CPU0
>> [ 7.781107] ----
>> [ 7.783533] lock(&devfreq->lock);
>> [ 7.786999] lock(&devfreq->lock);
>> [ 7.790467]
>> [ 7.790467] *** DEADLOCK ***
>> [ 7.790467]
>> [ 7.796359] May be due to missing lock nesting notation
>> [ 7.796359]
>> [ 7.803120] 4 locks held by kworker/u16:1/136:
>> [ 7.807537] #0: ("%s"("devfreq_wq")){.+.+..}, at: [<c013b41c>]
>> process_one_work+0x13c/0x454
>> [ 7.816029] #1: ((&(&devfreq->work)->work)){+.+...}, at:
>> [<c013b41c>] process_one_work+0x13c/0x454
>> [ 7.825128] #2: (&devfreq->lock){+.+.+.}, at: [<c05bd838>]
>> devfreq_monitor+0x1c/0x78
>> [ 7.833013] #3: (&nh->srcu){......}, at: [<c014309c>]
>> __srcu_notifier_call_chain+0x54/0xe0
>> [ 7.841419]
>> [ 7.841419] stack backtrace:
>> [ 7.845758] CPU: 1 PID: 136 Comm: kworker/u16:1 Not tainted
>> 4.6.0-rc2-xu4ml #4
>> [ 7.852946] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [ 7.859017] Workqueue: devfreq_wq devfreq_monitor
>> [ 7.863717] [<c010eb48>] (unwind_backtrace) from [<c010b884>]
>> (show_stack+0x10/0x14)
>> [ 7.871417] [<c010b884>] (show_stack) from [<c037715c>]
>> (dump_stack+0xa4/0xd0)
>> [ 7.878613] [<c037715c>] (dump_stack) from [<c017112c>]
>> (__lock_acquire+0x1ce8/0x207c)
>> [ 7.886492] [<c017112c>] (__lock_acquire) from [<c0172010>]
>> (lock_acquire+0x90/0xd4)
>> [ 7.892726] usb 5-1: New USB device found, idVendor=0bda, idProduct=8153
>> [ 7.892737] usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6
>> [ 7.892745] usb 5-1: Product: USB 10/100/1000 LAN
>> [ 7.892753] usb 5-1: Manufacturer: Realtek
>> [ 7.892761] usb 5-1: SerialNumber: 000001000000
>> [ 7.921248] [<c0172010>] (lock_acquire) from [<c075d298>]
>> (mutex_lock_nested+0x78/0x4ec)
>> [ 7.929301] [<c075d298>] (mutex_lock_nested) from [<c05be41c>]
>> (update_devfreq_passive+0x2c/0x88)
>> [ 7.938138] [<c05be41c>] (update_devfreq_passive) from [<c05be4c0>]
>> (devfreq_passive_notifier_call+0x48/0x50)
>> [ 7.948016] [<c05be4c0>] (devfreq_passive_notifier_call) from
>> [<c0142d84>] (notifier_call_chain+0x54/0xa4)
>> [ 7.957633] [<c0142d84>] (notifier_call_chain) from [<c01430e0>]
>> (__srcu_notifier_call_chain+0x98/0xe0)
>> [ 7.966991] [<c01430e0>] (__srcu_notifier_call_chain) from
>> [<c0143140>] (srcu_notifier_call_chain+0x18/0x20)
>> [ 7.976783] [<c0143140>] (srcu_notifier_call_chain) from
>> [<c05bd774>] (update_devfreq+0xe4/0x18c)
>> [ 7.985622] [<c05bd774>] (update_devfreq) from [<c05bd840>]
>> (devfreq_monitor+0x24/0x78)
>> [ 7.993595] [<c05bd840>] (devfreq_monitor) from [<c013b48c>]
>> (process_one_work+0x1ac/0x454)
>> [ 8.001913] [<c013b48c>] (process_one_work) from [<c013b784>]
>> (worker_thread+0x50/0x508)
>> [ 8.009976] [<c013b784>] (worker_thread) from [<c0141cf4>]
>> (kthread+0xf8/0x110)
>> [ 8.017254] [<c0141cf4>] (kthread) from [<c0107750>]
>> (ret_from_fork+0x14/0x24)
>>
>> Best Regards
>> -Anand Moon
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>

2016-04-11 04:02:04

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Chanwoo,

On 11 April 2016 at 07:46, Chanwoo Choi <[email protected]> wrote:
> Hi Anand,
>
> On 2016년 04월 09일 03:24, Chanwoo Choi wrote:
>> Hi Anand,
>>
>> On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <[email protected]> wrote:
>>> Hi Chanwoo,
>>>
>
> [snip]
>
>>>>
>>>
>>> I am observing following deadlock. Both on Odroid U3 and Odroid XU4.
>>
>> Thanks for your test. I'll test it again and fix it.
>
> This possible recursive locking is fixed with following diff:
>
> Thanks for your report. I'll fix it on next patchset[1] (v9).
> [1] https://lkml.org/lkml/2016/4/8/14
>
>
> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> index 28a9ae32d330..a4b0b02ee797 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -102,7 +102,7 @@ static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
> if (!devfreq->governor)
> return -EINVAL;
>
> - mutex_lock(&devfreq->lock);
> + mutex_lock_nested(&devfreq->lock, SINGLE_DEPTH_NESTING);
>
> ret = devfreq->governor->get_target_freq(devfreq, &freq);
> if (ret < 0)
>
> Best Regards,
> Chanwoo Choi
>

Thanks you for these patches on devfreq.
These changes fix the warning.

Tested-by: Anand Moon <[email protected]>

Tested on Odroid XU4 and Odroid U3.

Best Regards
-Anand Moon

2016-04-11 04:04:24

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Hi Anand,

On 2016년 04월 11일 13:01, Anand Moon wrote:
> Hi Chanwoo,
>
> On 11 April 2016 at 07:46, Chanwoo Choi <[email protected]> wrote:
>> Hi Anand,
>>
>> On 2016년 04월 09일 03:24, Chanwoo Choi wrote:
>>> Hi Anand,
>>>
>>> On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <[email protected]> wrote:
>>>> Hi Chanwoo,
>>>>
>>
>> [snip]
>>
>>>>>
>>>>
>>>> I am observing following deadlock. Both on Odroid U3 and Odroid XU4.
>>>
>>> Thanks for your test. I'll test it again and fix it.
>>
>> This possible recursive locking is fixed with following diff:
>>
>> Thanks for your report. I'll fix it on next patchset[1] (v9).
>> [1] https://lkml.org/lkml/2016/4/8/14
>>
>>
>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
>> index 28a9ae32d330..a4b0b02ee797 100644
>> --- a/drivers/devfreq/governor_passive.c
>> +++ b/drivers/devfreq/governor_passive.c
>> @@ -102,7 +102,7 @@ static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
>> if (!devfreq->governor)
>> return -EINVAL;
>>
>> - mutex_lock(&devfreq->lock);
>> + mutex_lock_nested(&devfreq->lock, SINGLE_DEPTH_NESTING);
>>
>> ret = devfreq->governor->get_target_freq(devfreq, &freq);
>> if (ret < 0)
>>
>> Best Regards,
>> Chanwoo Choi
>>
>
> Thanks you for these patches on devfreq.
> These changes fix the warning.
>
> Tested-by: Anand Moon <[email protected]>
>
> Tested on Odroid XU4 and Odroid U3.

Thanks for your test and report.

Best Regards,
Chanwoo Choi

2016-04-11 04:15:58

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/7] PM / devfreq: Add NoCP devfreq-event and support busfreq on exyno5422-odroidxu3

Dear all,

On 2016년 04월 08일 14:00, Chanwoo Choi wrote:
> This patchset support the AMBA bus frequency scaling on Exynos5422-based
> Odroid-XU3 board. But, this series only support the bus frequency scaling
> for INT (Internal) block using VDD_INT power line.
>
> Also, to support the bus frequency scaling for Exynos542x SoC,
> Exynos542x SoC has the specific 'NoC (Network on Chip) Probe' device
> to measure the transfered data traffic on NoC (Network on Chip)
> instead of PPMU (Platform Performance Monitoring Unit). NoC Probe device
> provide the utilization for INT block of Exynos542x SoC.
>
> The generic exynos-bus frequency driver uses the 'NoC Probe' devfreq-event
> device (drivers/devfreq/event/exynos-nocp.c) without any modification.
> Just add the phandle of 'NoC Probe' dt node to bus dt node.
>
> Depend on:
> This patchset depends on patch[1] which support the generic exynos-bus
> frequency driver.
>
> [1] https://lkml.org/lkml/2016/4/8/14
> - [PATCH v8 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor

The dependency of these patch-set is changed from patches[1] to patches[2].
Because patches[1] has the possible recursive locking issue. Patches[2] fix this issue.

[2] http://www.spinics.net/lists/arm-kernel/msg495976.html
- [PATCH v9 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor

[snip]

Best Regards,
Chanwoo Choi

2016-04-11 15:45:22

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/7] PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus

On Fri, Apr 08, 2016 at 02:00:41PM +0900, Chanwoo Choi wrote:
> This patch adds the detailed corrleation between sub-blocks and power line
> for Exynos5422.
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)

This belongs with your previous series. Probably, all of this should be
1 DT patch rather than piecemeal. Any division should be by h/w block,
not as a driver is using features.

Rob

2016-04-12 08:09:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/7] PM / devfreq: event: Add new Exynos NoC probe driver

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds NoC (Network on Chip) Probe driver which provides
> the primitive values to get the performance data. The packets that the Network
> on Chip (NoC) probes detects are transported over the network infrastructure.
> Exynos542x bus has multiple NoC probes to provide bandwidth information about
> behavior of the SoC that you can use while analyzing system performance.
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++++
> drivers/devfreq/event/Kconfig | 8 +
> drivers/devfreq/event/Makefile | 2 +
> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++++++++++
> drivers/devfreq/event/exynos-nocp.h | 78 +++++++
> 5 files changed, 421 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
> create mode 100644 drivers/devfreq/event/exynos-nocp.c
> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>
> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
> new file mode 100644
> index 000000000000..03b74fed034c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
> @@ -0,0 +1,86 @@
> +
> +* Samsung Exynos NoC (Network on Chip) Probe device
> +
> +The Samsung Exynos542x SoC has NoC (Network on Chip) Probe for NoC bus.
> +NoC provides the primitive values to get the performance data. The packets
> +that the Network on Chip (NoC) probes detects are transported over
> +the network infrastructure to observer units. You can configure probes to
> +capture packets with header or data on the data request response network,
> +or as traffic debug or statistic collectors. Exynos542x bus has multiple
> +NoC probes to provide bandwidth information about behavior of the SoC
> +that you can use while analyzing system performance.
> +
> +Required properties:
> +- compatible: Should be "samsung,exynos5420-nocp"
> +- reg: physical base address of each NoC Probe and length of memory mapped region.
> +
> +Optional properties:
> +- clock-names : the name of clock used by the NoC Probe, "nocp"
> +- clocks : phandles for clock specified in "clock-names" property
> +
> +Example1 : NoC Probe nodes in exynos5420.dtsi are listed below.
> +
> + nocp_mem0_0: nocp_mem0_0@10CA1000 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1000 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_1: nocp_mem0_1@10CA1400 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1400 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_2: nocp_mem0_2@10CA1800 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1800 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_3: nocp_mem0_0@10CA1C00 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1C00 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_g3d_0: nocp_g3d_0@11A51000 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x11A51000 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_g3d_1: nocp_g3d_1@11A51400 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x11A51400 0x200>;
> + status = "disabled";
> + };
> +
> +
> +Example2 : Events of each NoC Probe node in exynos5422-odroidxu3-common.dtsi
> + are listed below.
> +
> +
> + &nocp_mem0_0 {
> + status = "okay";
> + };
> +
> + &nocp_mem0_1 {
> + status = "okay";
> + };
> +
> + &nocp_mem0_2 {
> + status = "okay";
> + };
> +
> + &nocp_mem0_3 {
> + status = "okay";
> + };
> +
> + &nocp_g3d_0 {
> + status = "okay";
> + };
> +
> + &nocp_g3d_1 {
> + status = "okay";
> + };

I think this split in documentation between DTSI and DTS is not needed
for example. Just add one example without the status and it should be
sufficient to get the idea.

> diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig
> index a11720affc31..1e8b4f469f38 100644
> --- a/drivers/devfreq/event/Kconfig
> +++ b/drivers/devfreq/event/Kconfig
> @@ -13,6 +13,14 @@ menuconfig PM_DEVFREQ_EVENT
>
> if PM_DEVFREQ_EVENT
>
> +config DEVFREQ_EVENT_EXYNOS_NOCP
> + bool "EXYNOS NoC (Network On Chip) Probe DEVFREQ event Driver"
> + depends on ARCH_EXYNOS
> + select PM_OPP
> + help
> + This add the devfreq-event driver for Exynos SoC. It provides NoC
> + (Network on Chip) Probe counters to measure the bandwidth of AXI bus.
> +
> config DEVFREQ_EVENT_EXYNOS_PPMU
> bool "EXYNOS PPMU (Platform Performance Monitoring Unit) DEVFREQ event Driver"
> depends on ARCH_EXYNOS
> diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
> index be146ead79cf..3d6afd352253 100644
> --- a/drivers/devfreq/event/Makefile
> +++ b/drivers/devfreq/event/Makefile
> @@ -1,2 +1,4 @@
> # Exynos DEVFREQ Event Drivers
> +
> +obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o
> obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
> diff --git a/drivers/devfreq/event/exynos-nocp.c b/drivers/devfreq/event/exynos-nocp.c
> new file mode 100644
> index 000000000000..b9468a6143f6
> --- /dev/null
> +++ b/drivers/devfreq/event/exynos-nocp.c
> @@ -0,0 +1,247 @@
> +/*
> + * exynos-nocp.c - EXYNOS NoC (Network On Chip) Probe support
> + *
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Author : Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/devfreq-event.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "exynos-nocp.h"
> +
> +struct exynos_nocp {
> + struct devfreq_event_dev *edev;
> + struct devfreq_event_desc desc;
> +
> + struct device *dev;
> +
> + struct regmap *regmap;
> + struct clk *clk;
> +};
> +
> +/*
> + * The devfreq-event ops structure for nocp probe.
> + */
> +static int exynos_nocp_set_event(struct devfreq_event_dev *edev)
> +{
> + struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
> +
> + /* Disable NoC probe */
> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
> + NOCP_MAIN_CTL_STATEN_MASK, 0);
> +
> + /* Set a statistics dump period to 0 */
> + regmap_write(nocp->regmap, NOCP_STAT_PERIOD, 0x0);
> +
> + /* Set the IntEvent fields of *_SRC */
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_SRC,
> + NOCP_CNT_SRC_INTEVENT_MASK,
> + NOCP_CNT_SRC_INTEVENT_BYTE_MASK);
> +
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_SRC,
> + NOCP_CNT_SRC_INTEVENT_MASK,
> + NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
> +
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_SRC,
> + NOCP_CNT_SRC_INTEVENT_MASK,
> + NOCP_CNT_SRC_INTEVENT_CYCLE_MASK);
> +
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_SRC,
> + NOCP_CNT_SRC_INTEVENT_MASK,
> + NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
> +
> +
> + /* Set an alarm with a max/min value of 0 to generate StatALARM */
> + regmap_write(nocp->regmap, NOCP_STAT_ALARM_MIN, 0x0);
> + regmap_write(nocp->regmap, NOCP_STAT_ALARM_MAX, 0x0);
> +
> + /* Set AlarmMode */
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_ALARM_MODE,
> + NOCP_CNT_ALARM_MODE_MASK,
> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_ALARM_MODE,
> + NOCP_CNT_ALARM_MODE_MASK,
> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_ALARM_MODE,
> + NOCP_CNT_ALARM_MODE_MASK,
> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_ALARM_MODE,
> + NOCP_CNT_ALARM_MODE_MASK,
> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
> +
> + /* Enable the measurements by setting AlarmEn and StatEn */
> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
> + NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK,
> + NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK);
> +
> + /* Set GlobalEN */
> + regmap_update_bits(nocp->regmap, NOCP_CFG_CTL,
> + NOCP_CFG_CTL_GLOBALEN_MASK,
> + NOCP_CFG_CTL_GLOBALEN_MASK);
> +
> + /* Enable NoC probe */
> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
> + NOCP_MAIN_CTL_STATEN_MASK,
> + NOCP_MAIN_CTL_STATEN_MASK);

In all these regmap_update_bits() calls you are ignoring the return
value. This does not look right.

> +
> + return 0;
> +}
> +
> +static int exynos_nocp_get_event(struct devfreq_event_dev *edev,
> + struct devfreq_event_data *edata)
> +{
> + struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
> + unsigned int counter[4];
> +
> + /* Read cycle count */
> + regmap_read(nocp->regmap, NOCP_COUNTERS_0_VAL, &counter[0]);
> + regmap_read(nocp->regmap, NOCP_COUNTERS_1_VAL, &counter[1]);
> + regmap_read(nocp->regmap, NOCP_COUNTERS_2_VAL, &counter[2]);
> + regmap_read(nocp->regmap, NOCP_COUNTERS_3_VAL, &counter[3]);

Ditto. If read fails, the data in counter should not be trusted (not
initialized) so the devfreq_event_get_event() should not use it. The
simplest would be to return error on each failure.

> +
> + edata->load_count = ((counter[1] << 16) | counter[0]);
> + edata->total_count = ((counter[3] << 16) | counter[2]);
> +
> + dev_dbg(&edev->dev, "%s (event: %ld/%ld)\n", edev->desc->name,
> + edata->load_count, edata->total_count);
> +
> + return 0;
> +}
> +
> +static const struct devfreq_event_ops exynos_nocp_ops = {
> + .set_event = exynos_nocp_set_event,
> + .get_event = exynos_nocp_get_event,
> +};
> +
> +static const struct of_device_id exynos_nocp_id_match[] = {
> + { .compatible = "samsung,exynos5420-nocp", },
> + { /* sentinel */ },
> +};
> +
> +static struct regmap_config exynos_nocp_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = NOCP_COUNTERS_3_VAL,
> +};
> +
> +static int exynos_nocp_parse_dt(struct platform_device *pdev,
> + struct exynos_nocp *nocp)
> +{
> + struct device *dev = nocp->dev;
> + struct device_node *np = dev->of_node;
> + struct resource *res;
> + void __iomem *base;
> + int ret = 0;
> +
> + if (!np) {
> + dev_err(dev, "failed to find devicetree node\n");
> + return -EINVAL;
> + }
> +
> + nocp->clk = devm_clk_get(dev, "nocp");
> + if (IS_ERR(nocp->clk))
> + nocp->clk = NULL;
> +
> + /* Maps the memory mapped IO to control nocp register */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (IS_ERR(res))
> + return PTR_ERR(res);
> +
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> + exynos_nocp_regmap_config.max_register = resource_size(res) - 4;
> +
> + nocp->regmap = devm_regmap_init_mmio(dev, base,
> + &exynos_nocp_regmap_config);
> + if (IS_ERR(nocp->regmap)) {
> + dev_err(dev, "failed to initialize regmap\n");
> + ret = PTR_ERR(nocp->regmap);
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + devm_iounmap(dev, base);

Why you need this? This is obtained by devm-like() interface so it
should be released by the core.

> +
> + return ret;
> +}
> +
> +static int exynos_nocp_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct exynos_nocp *nocp;
> + int ret;
> +
> + nocp = devm_kzalloc(&pdev->dev, sizeof(*nocp), GFP_KERNEL);
> + if (!nocp)
> + return -ENOMEM;
> +
> + nocp->dev = &pdev->dev;
> +
> + /* Parse dt data to get resource */
> + ret = exynos_nocp_parse_dt(pdev, nocp);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "failed to parse devicetree for resource\n");
> + return ret;
> + }
> +
> + /* Add devfreq-event device to measure the bandwidth of NoC */
> + nocp->desc.ops = &exynos_nocp_ops;
> + nocp->desc.driver_data = nocp;
> + nocp->desc.name = np->name;
> + nocp->edev = devm_devfreq_event_add_edev(&pdev->dev, &nocp->desc);
> + if (IS_ERR(nocp->edev)) {
> + ret = PTR_ERR(nocp->edev);
> + dev_err(&pdev->dev,
> + "failed to add devfreq-event device\n");
> + goto err;

This looks not needed and does not improve readability to me. Just
'return ret' always. At this point (before this if() statement) 'ret'
equals to 0. Then you could remove the 'err' label and always 'return ret'.

Best regards,
Krzysztof

2016-04-12 08:14:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/7] ARM: dts: Add NoC Probe dt node for Exynos542x SoC

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds the NoCP (Network on Chip Probe) Device Tree node
> to measure the bandwidth of memory and g3d in Exynos542x SoC.
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 7b99cb58d82d..d80f3b66f017 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -294,6 +294,42 @@
> };
> };
>
> + nocp_mem0_0: nocp_mem0_0@10CA1000 {

The node name should describe general class of device, so:

nocp_mem0_0: nocp@10CA1000 {
...
}
?

> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1000 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_1: nocp_mem0_1@10CA1400 {
> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1400 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_2: nocp_mem0_2@10CA1800 {

Shouldn't it be nocp_mem1_0?

> + compatible = "samsung,exynos5420-nocp";
> + reg = <0x10CA1800 0x200>;
> + status = "disabled";
> + };
> +
> + nocp_mem0_3: nocp_mem0_3@10CA1C00 {

Shouldn't it be nocp_mem1_1?


Best regards,
Krzysztof

2016-04-12 10:53:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 6/7] ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds the AMBA bus nodes using VDD_INT for Exynos542x SoC.
> Exynos542x has the following AMBA buses to translate data between
> DRAM and sub-blocks.
>
> Following list specifies the detailed correlation between sub-block and clock:
> - CLK_DOUT_ACLK400_WCORE clock for WCORE's AXI
> - CLK_DOUT_ACLK100_NOC for NoC (Network on Chip)'s AXI
> - CLK_DOUT_PCLK200_FSYS for FSYS's APB
> - CLK_DOUT_ACLK200_FSYS for FSYS's AXI
> - CLK_DOUT_ACLK200_FSYS2 for FSYS2's AXI
> - CLK_DOUT_ACLK333 for MFC's AXI
> - CLK_DOUT_ACLK266 for GEN's AXI
> - CLK_DOUT_ACLK66 for PERIC/PERIR's AXI
> - CLK_DOUT_ACLK333_G2D for G2D's AXI
> - CLK_DOUT_ACLK266_G2D for ACP's AXI
> - CLK_DOUT_ACLK300_JPEG for JPEG's AXI
> - CLK_DOUT_ACLK166 for JPEG's APB
> - CLK_DOUT_ACLK300_DISP1 for FIMD's AXI
> - CLK_DOUT_ACLK400_DISP1 for DISP1's AXI
> - CLK_DOUT_ACLK300_GSCL for GSCL Scaler's AXI
> - CLK_DOUT_ACLK400_MSCL for MSCL's AXI
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 371 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 371 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index d80f3b66f017..1340024fa882 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -1224,6 +1224,377 @@
> power-domains = <&disp_pd>;
> #iommu-cells = <0>;
> };
> +
> + bus_wcore: bus_wcore {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_wcore_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_noc: bus_noc {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK100_NOC>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_noc_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_fsys_apb: bus_fsys_apb {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_fsys_apb_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_fsys: bus_fsys {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_fsys_apb_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_fsys2: bus_fsys2 {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_fsys2_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_mfc: bus_mfc {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK333>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_mfc_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_gen: bus_gen {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK266>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_gen_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_peri: bus_peri {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK66>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_peri_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_g2d: bus_g2d {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK333_G2D>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_g2d_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_g2d_acp: bus_g2d_acp {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK266_G2D>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_g2d_acp_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_jpeg: bus_jpeg {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_jpeg_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_jpeg_apb: bus_jpeg_apb {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK166>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_jpeg_apb_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_disp1_fimd: bus_disp1_fimd {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_disp1_fimd_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_disp1: bus_disp1 {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_disp1_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_gscl_scaler: bus_gscl_scaler {
> + compatible = "samsung,exynos-bus";
> + clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
> + clock-names = "bus";
> + operating-points-v2 = <&bus_gscl_opp_table>;
> + status = "disabled";
> + };
> +
> + bus_mscl: bus_mscl {
> + 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>;
> + };
> + opp01 {
> + opp-hz = /bits/ 64 <111000000>;
> + opp-microvolt = <950000>;
> + };
> + opp02 {
> + opp-hz = /bits/ 64 <222000000>;
> + opp-microvolt = <950000>;
> + };
> + opp03 {
> + opp-hz = /bits/ 64 <333000000>;
> + opp-microvolt = <950000>;
> + };
> + opp04 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <987500>;
> + };
> + };
> +
> + bus_noc_opp_table: opp_table3 {
> + compatible = "operating-points-v2";
> +
> + opp00 {
> + opp-hz = /bits/ 64 <66000000>;

67000000 so it won't be round down?

Beside that looks good. I am assuming the same apply strategy - I can
take the DTS changes the bindings got accepted.

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2016-04-12 10:59:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 7/7] ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds the bus device tree nodes for INT (Internal) block
> to enable the AMBA bus frequency scaling and add the NoC (Network on Chip)
> Probe Device Tree node to measure the bandwidht for AMBA AXI bus.
>
> The WCORE bus bus is parent device in INT block using VDD_INT.
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 ++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 1bd507bfa750..2a74abe6fc5d 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -361,6 +361,22 @@
> cap-sd-highspeed;
> };
>
> +&nocp_mem0_0 {
> + status = "okay";
> +};
> +
> +&nocp_mem0_1 {
> + status = "okay";
> +};
> +
> +&nocp_mem0_2 {
> + status = "okay";
> +};
> +
> +&nocp_mem0_3 {
> + status = "okay";
> +};
> +
> &pinctrl_0 {
> hdmi_hpd_irq: hdmi-hpd-irq {
> samsung,pins = "gpx3-7";
> @@ -432,3 +448,86 @@
> vdd33-supply = <&ldo9_reg>;
> vdd10-supply = <&ldo11_reg>;
> };
> +
> +&bus_wcore {
> + devfreq-events = <&nocp_mem0_0>, <&nocp_mem0_1>,
> + <&nocp_mem0_2>, <&nocp_mem0_3>;
> + vdd-supply = <&buck3_reg>;
> + exynos,saturation-ratio = <100>;
> + status = "okay";
> +};
> +
> +&bus_noc {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_fsys_apb {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_fsys {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_fsys2 {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_mfc {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_gen {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_peri {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_g2d {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_g2d_acp {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_jpeg {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_jpeg_apb {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_disp1_fimd {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_disp1 {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_gscl_scaler {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};
> +
> +&bus_mscl {
> + devfreq = <&bus_wcore>;
> + status = "okay";
> +};

Could you put the bus nodes in alphabetical order, both between them and
in relation to others (so before &clock_audss)? Let's keep the file ordered.

Best regards,
Krzysztof

2016-04-12 11:25:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 5/7] clk: samsung: exynos542x: Add the clock id for ACLK

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds the clock id for ACLK clock which is source clock of AMBA AXI
> Bus. This clock should be handled in Bus frequency scaling driver.
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos5420.c | 85 +++++++++++++++++++++++-------------
> 1 file changed, 55 insertions(+), 30 deletions(-)

The IDs itself look good but you are not adding only clock ID. You are
changing all of them from NULL-flags to CLK_SET_RATE_PARENT. This should
be explained in the commit message why you need it. Probably it should
be also in separate commit.

Best regards,
Krzysztof

2016-04-12 11:29:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: clock: Add the clock id for ACLK clock of Exynos542x SoC

On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
> This patch adds the clock id for ACLK clock of Exynos542x SoC. ACLK clock mean
> the source clock of AMBA AXI bus. This clock id should be used for Bus
> frequency scaling.
>
> Cc: Sylwester Nawrocki <[email protected]>
> Cc: Tomasz Figa <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> include/dt-bindings/clock/exynos5420.h | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2016-04-13 06:20:45

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 5/7] clk: samsung: exynos542x: Add the clock id for ACLK

2016-04-12 14:25 GMT+03:00 Krzysztof Kozlowski <[email protected]>:
> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>> This patch adds the clock id for ACLK clock which is source clock of AMBA AXI
>> Bus. This clock should be handled in Bus frequency scaling driver.
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> drivers/clk/samsung/clk-exynos5420.c | 85 +++++++++++++++++++++++-------------
>> 1 file changed, 55 insertions(+), 30 deletions(-)
>
> The IDs itself look good but you are not adding only clock ID. You are
> changing all of them from NULL-flags to CLK_SET_RATE_PARENT. This should
> be explained in the commit message why you need it. Probably it should
> be also in separate commit.

+1. And CLK_SET_RATE_PARENT for a clock which has a mux as its parent
is a bit suspicious, so I'd like to know the rationale for each single
clock with CLK_SET_RATE_PARENT being added.

Best regards,
Tomasz

2016-04-13 23:15:06

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 7/7] ARM: dts: Add support of Bus frequency using VDD_INT for exynos5422-odroidxu3

On 2016년 04월 12일 19:59, Krzysztof Kozlowski wrote:
> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>> This patch adds the bus device tree nodes for INT (Internal) block
>> to enable the AMBA bus frequency scaling and add the NoC (Network on Chip)
>> Probe Device Tree node to measure the bandwidht for AMBA AXI bus.
>>
>> The WCORE bus bus is parent device in INT block using VDD_INT.
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 99 ++++++++++++++++++++++
>> 1 file changed, 99 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> index 1bd507bfa750..2a74abe6fc5d 100644
>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>> @@ -361,6 +361,22 @@
>> cap-sd-highspeed;
>> };
>>
>> +&nocp_mem0_0 {
>> + status = "okay";
>> +};
>> +
>> +&nocp_mem0_1 {
>> + status = "okay";
>> +};
>> +
>> +&nocp_mem0_2 {
>> + status = "okay";
>> +};
>> +
>> +&nocp_mem0_3 {
>> + status = "okay";
>> +};
>> +
>> &pinctrl_0 {
>> hdmi_hpd_irq: hdmi-hpd-irq {
>> samsung,pins = "gpx3-7";
>> @@ -432,3 +448,86 @@
>> vdd33-supply = <&ldo9_reg>;
>> vdd10-supply = <&ldo11_reg>;
>> };
>> +
>> +&bus_wcore {
>> + devfreq-events = <&nocp_mem0_0>, <&nocp_mem0_1>,
>> + <&nocp_mem0_2>, <&nocp_mem0_3>;
>> + vdd-supply = <&buck3_reg>;
>> + exynos,saturation-ratio = <100>;
>> + status = "okay";
>> +};
>> +
>> +&bus_noc {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_fsys_apb {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_fsys {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_fsys2 {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_mfc {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_gen {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_peri {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_g2d {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_g2d_acp {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_jpeg {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_jpeg_apb {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_disp1_fimd {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_disp1 {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_gscl_scaler {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>> +
>> +&bus_mscl {
>> + devfreq = <&bus_wcore>;
>> + status = "okay";
>> +};
>
> Could you put the bus nodes in alphabetical order, both between them and
> in relation to others (so before &clock_audss)? Let's keep the file ordered.

OK. I'll reorder them.

Best Regards,
Chanwoo Choi

2016-04-13 23:16:53

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 6/7] ARM: dts: Add bus nodes using VDD_INT for Exynos542x SoC

On 2016년 04월 12일 19:53, Krzysztof Kozlowski wrote:
> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>> This patch adds the AMBA bus nodes using VDD_INT for Exynos542x SoC.
>> Exynos542x has the following AMBA buses to translate data between
>> DRAM and sub-blocks.
>>
>> Following list specifies the detailed correlation between sub-block and clock:
>> - CLK_DOUT_ACLK400_WCORE clock for WCORE's AXI
>> - CLK_DOUT_ACLK100_NOC for NoC (Network on Chip)'s AXI
>> - CLK_DOUT_PCLK200_FSYS for FSYS's APB
>> - CLK_DOUT_ACLK200_FSYS for FSYS's AXI
>> - CLK_DOUT_ACLK200_FSYS2 for FSYS2's AXI
>> - CLK_DOUT_ACLK333 for MFC's AXI
>> - CLK_DOUT_ACLK266 for GEN's AXI
>> - CLK_DOUT_ACLK66 for PERIC/PERIR's AXI
>> - CLK_DOUT_ACLK333_G2D for G2D's AXI
>> - CLK_DOUT_ACLK266_G2D for ACP's AXI
>> - CLK_DOUT_ACLK300_JPEG for JPEG's AXI
>> - CLK_DOUT_ACLK166 for JPEG's APB
>> - CLK_DOUT_ACLK300_DISP1 for FIMD's AXI
>> - CLK_DOUT_ACLK400_DISP1 for DISP1's AXI
>> - CLK_DOUT_ACLK300_GSCL for GSCL Scaler's AXI
>> - CLK_DOUT_ACLK400_MSCL for MSCL's AXI
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> arch/arm/boot/dts/exynos5420.dtsi | 371 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 371 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
>> index d80f3b66f017..1340024fa882 100644
>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> @@ -1224,6 +1224,377 @@
>> power-domains = <&disp_pd>;
>> #iommu-cells = <0>;
>> };
>> +
>> + bus_wcore: bus_wcore {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_wcore_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_noc: bus_noc {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK100_NOC>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_noc_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_fsys_apb: bus_fsys_apb {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_fsys_apb_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_fsys: bus_fsys {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_fsys_apb_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_fsys2: bus_fsys2 {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_fsys2_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_mfc: bus_mfc {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK333>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_mfc_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_gen: bus_gen {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK266>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_gen_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_peri: bus_peri {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK66>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_peri_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_g2d: bus_g2d {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK333_G2D>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_g2d_acp: bus_g2d_acp {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK266_G2D>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_acp_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_jpeg: bus_jpeg {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_jpeg_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_jpeg_apb: bus_jpeg_apb {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK166>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_jpeg_apb_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_disp1_fimd: bus_disp1_fimd {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_disp1_fimd_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_disp1: bus_disp1 {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_disp1_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_gscl_scaler: bus_gscl_scaler {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_gscl_opp_table>;
>> + status = "disabled";
>> + };
>> +
>> + bus_mscl: bus_mscl {
>> + 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>;
>> + };
>> + opp01 {
>> + opp-hz = /bits/ 64 <111000000>;
>> + opp-microvolt = <950000>;
>> + };
>> + opp02 {
>> + opp-hz = /bits/ 64 <222000000>;
>> + opp-microvolt = <950000>;
>> + };
>> + opp03 {
>> + opp-hz = /bits/ 64 <333000000>;
>> + opp-microvolt = <950000>;
>> + };
>> + opp04 {
>> + opp-hz = /bits/ 64 <400000000>;
>> + opp-microvolt = <987500>;
>> + };
>> + };
>> +
>> + bus_noc_opp_table: opp_table3 {
>> + compatible = "operating-points-v2";
>> +
>> + opp00 {
>> + opp-hz = /bits/ 64 <66000000>;
>
> 67000000 so it won't be round down?

I'll change it. 66000000 -> 67000000.

>
> Beside that looks good. I am assuming the same apply strategy - I can
> take the DTS changes the bindings got accepted.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>

Thanks for the review.

Best Regards,
Chanwoo Choi

2016-04-14 05:48:33

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 3/7] ARM: dts: Add NoC Probe dt node for Exynos542x SoC

On 2016년 04월 12일 17:14, Krzysztof Kozlowski wrote:
> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>> This patch adds the NoCP (Network on Chip Probe) Device Tree node
>> to measure the bandwidth of memory and g3d in Exynos542x SoC.
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> arch/arm/boot/dts/exynos5420.dtsi | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
>> index 7b99cb58d82d..d80f3b66f017 100644
>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> @@ -294,6 +294,42 @@
>> };
>> };
>>
>> + nocp_mem0_0: nocp_mem0_0@10CA1000 {
>
> The node name should describe general class of device, so:
>
> nocp_mem0_0: nocp@10CA1000 {
> ...
> }
> ?

I'll modify it.

>
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1000 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_1: nocp_mem0_1@10CA1400 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1400 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_2: nocp_mem0_2@10CA1800 {
>
> Shouldn't it be nocp_mem1_0?

I was mistaken. I'll fix it.

>
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1800 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_3: nocp_mem0_3@10CA1C00 {
>
> Shouldn't it be nocp_mem1_1?

ditto.

Best Regards,
Chanwoo Choi

2016-04-14 06:28:25

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 5/7] clk: samsung: exynos542x: Add the clock id for ACLK

Hi Tomasz and Krzysztof,

On 2016년 04월 13일 15:20, Tomasz Figa wrote:
> 2016-04-12 14:25 GMT+03:00 Krzysztof Kozlowski <[email protected]>:
>> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>>> This patch adds the clock id for ACLK clock which is source clock of AMBA AXI
>>> Bus. This clock should be handled in Bus frequency scaling driver.
>>>
>>> Signed-off-by: Chanwoo Choi <[email protected]>
>>> ---
>>> drivers/clk/samsung/clk-exynos5420.c | 85 +++++++++++++++++++++++-------------
>>> 1 file changed, 55 insertions(+), 30 deletions(-)
>>
>> The IDs itself look good but you are not adding only clock ID. You are
>> changing all of them from NULL-flags to CLK_SET_RATE_PARENT. This should
>> be explained in the commit message why you need it. Probably it should
>> be also in separate commit.
>
> +1. And CLK_SET_RATE_PARENT for a clock which has a mux as its parent
> is a bit suspicious, so I'd like to know the rationale for each single
> clock with CLK_SET_RATE_PARENT being added.

This patch don't need the CLK_SET_RATE_PARENT flag. As Tomasz's comment,
the mux of divider's parent don't need this flag. Sorry for confusion.

I'll drop the CLK_SET_RATE_PARENT flag.

Best Regards,
Chanwoo Choi

2016-04-15 05:14:02

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 1/7] PM / devfreq: event: Add new Exynos NoC probe driver

On 2016년 04월 12일 17:07, Krzysztof Kozlowski wrote:
> On 04/08/2016 07:00 AM, Chanwoo Choi wrote:
>> This patch adds NoC (Network on Chip) Probe driver which provides
>> the primitive values to get the performance data. The packets that the Network
>> on Chip (NoC) probes detects are transported over the network infrastructure.
>> Exynos542x bus has multiple NoC probes to provide bandwidth information about
>> behavior of the SoC that you can use while analyzing system performance.
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> .../bindings/devfreq/event/exynos-nocp.txt | 86 +++++++
>> drivers/devfreq/event/Kconfig | 8 +
>> drivers/devfreq/event/Makefile | 2 +
>> drivers/devfreq/event/exynos-nocp.c | 247 +++++++++++++++++++++
>> drivers/devfreq/event/exynos-nocp.h | 78 +++++++
>> 5 files changed, 421 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
>> create mode 100644 drivers/devfreq/event/exynos-nocp.c
>> create mode 100644 drivers/devfreq/event/exynos-nocp.h
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
>> new file mode 100644
>> index 000000000000..03b74fed034c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/event/exynos-nocp.txt
>> @@ -0,0 +1,86 @@
>> +
>> +* Samsung Exynos NoC (Network on Chip) Probe device
>> +
>> +The Samsung Exynos542x SoC has NoC (Network on Chip) Probe for NoC bus.
>> +NoC provides the primitive values to get the performance data. The packets
>> +that the Network on Chip (NoC) probes detects are transported over
>> +the network infrastructure to observer units. You can configure probes to
>> +capture packets with header or data on the data request response network,
>> +or as traffic debug or statistic collectors. Exynos542x bus has multiple
>> +NoC probes to provide bandwidth information about behavior of the SoC
>> +that you can use while analyzing system performance.
>> +
>> +Required properties:
>> +- compatible: Should be "samsung,exynos5420-nocp"
>> +- reg: physical base address of each NoC Probe and length of memory mapped region.
>> +
>> +Optional properties:
>> +- clock-names : the name of clock used by the NoC Probe, "nocp"
>> +- clocks : phandles for clock specified in "clock-names" property
>> +
>> +Example1 : NoC Probe nodes in exynos5420.dtsi are listed below.
>> +
>> + nocp_mem0_0: nocp_mem0_0@10CA1000 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1000 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_1: nocp_mem0_1@10CA1400 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1400 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_2: nocp_mem0_2@10CA1800 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1800 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_mem0_3: nocp_mem0_0@10CA1C00 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x10CA1C00 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_g3d_0: nocp_g3d_0@11A51000 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x11A51000 0x200>;
>> + status = "disabled";
>> + };
>> +
>> + nocp_g3d_1: nocp_g3d_1@11A51400 {
>> + compatible = "samsung,exynos5420-nocp";
>> + reg = <0x11A51400 0x200>;
>> + status = "disabled";
>> + };
>> +
>> +
>> +Example2 : Events of each NoC Probe node in exynos5422-odroidxu3-common.dtsi
>> + are listed below.
>> +
>> +
>> + &nocp_mem0_0 {
>> + status = "okay";
>> + };
>> +
>> + &nocp_mem0_1 {
>> + status = "okay";
>> + };
>> +
>> + &nocp_mem0_2 {
>> + status = "okay";
>> + };
>> +
>> + &nocp_mem0_3 {
>> + status = "okay";
>> + };
>> +
>> + &nocp_g3d_0 {
>> + status = "okay";
>> + };
>> +
>> + &nocp_g3d_1 {
>> + status = "okay";
>> + };
>
> I think this split in documentation between DTSI and DTS is not needed
> for example. Just add one example without the status and it should be
> sufficient to get the idea.

I'll modify it as following:

Example1 : NoC Probe nodes in Device Tree are listed below.

nocp_mem0_0: nocp@10CA1000 {
compatible = "samsung,exynos5420-nocp";
reg = <0x10CA1000 0x200>;
};

>
>> diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig
>> index a11720affc31..1e8b4f469f38 100644
>> --- a/drivers/devfreq/event/Kconfig
>> +++ b/drivers/devfreq/event/Kconfig
>> @@ -13,6 +13,14 @@ menuconfig PM_DEVFREQ_EVENT
>>
>> if PM_DEVFREQ_EVENT
>>
>> +config DEVFREQ_EVENT_EXYNOS_NOCP
>> + bool "EXYNOS NoC (Network On Chip) Probe DEVFREQ event Driver"
>> + depends on ARCH_EXYNOS
>> + select PM_OPP
>> + help
>> + This add the devfreq-event driver for Exynos SoC. It provides NoC
>> + (Network on Chip) Probe counters to measure the bandwidth of AXI bus.
>> +
>> config DEVFREQ_EVENT_EXYNOS_PPMU
>> bool "EXYNOS PPMU (Platform Performance Monitoring Unit) DEVFREQ event Driver"
>> depends on ARCH_EXYNOS
>> diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
>> index be146ead79cf..3d6afd352253 100644
>> --- a/drivers/devfreq/event/Makefile
>> +++ b/drivers/devfreq/event/Makefile
>> @@ -1,2 +1,4 @@
>> # Exynos DEVFREQ Event Drivers
>> +
>> +obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o
>> obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
>> diff --git a/drivers/devfreq/event/exynos-nocp.c b/drivers/devfreq/event/exynos-nocp.c
>> new file mode 100644
>> index 000000000000..b9468a6143f6
>> --- /dev/null
>> +++ b/drivers/devfreq/event/exynos-nocp.c
>> @@ -0,0 +1,247 @@
>> +/*
>> + * exynos-nocp.c - EXYNOS NoC (Network On Chip) Probe support
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * Author : Chanwoo Choi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/devfreq-event.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "exynos-nocp.h"
>> +
>> +struct exynos_nocp {
>> + struct devfreq_event_dev *edev;
>> + struct devfreq_event_desc desc;
>> +
>> + struct device *dev;
>> +
>> + struct regmap *regmap;
>> + struct clk *clk;
>> +};
>> +
>> +/*
>> + * The devfreq-event ops structure for nocp probe.
>> + */
>> +static int exynos_nocp_set_event(struct devfreq_event_dev *edev)
>> +{
>> + struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
>> +
>> + /* Disable NoC probe */
>> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
>> + NOCP_MAIN_CTL_STATEN_MASK, 0);
>> +
>> + /* Set a statistics dump period to 0 */
>> + regmap_write(nocp->regmap, NOCP_STAT_PERIOD, 0x0);
>> +
>> + /* Set the IntEvent fields of *_SRC */
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_SRC,
>> + NOCP_CNT_SRC_INTEVENT_MASK,
>> + NOCP_CNT_SRC_INTEVENT_BYTE_MASK);
>> +
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_SRC,
>> + NOCP_CNT_SRC_INTEVENT_MASK,
>> + NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
>> +
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_SRC,
>> + NOCP_CNT_SRC_INTEVENT_MASK,
>> + NOCP_CNT_SRC_INTEVENT_CYCLE_MASK);
>> +
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_SRC,
>> + NOCP_CNT_SRC_INTEVENT_MASK,
>> + NOCP_CNT_SRC_INTEVENT_CHAIN_MASK);
>> +
>> +
>> + /* Set an alarm with a max/min value of 0 to generate StatALARM */
>> + regmap_write(nocp->regmap, NOCP_STAT_ALARM_MIN, 0x0);
>> + regmap_write(nocp->regmap, NOCP_STAT_ALARM_MAX, 0x0);
>> +
>> + /* Set AlarmMode */
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_0_ALARM_MODE,
>> + NOCP_CNT_ALARM_MODE_MASK,
>> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_1_ALARM_MODE,
>> + NOCP_CNT_ALARM_MODE_MASK,
>> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_2_ALARM_MODE,
>> + NOCP_CNT_ALARM_MODE_MASK,
>> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
>> + regmap_update_bits(nocp->regmap, NOCP_COUNTERS_3_ALARM_MODE,
>> + NOCP_CNT_ALARM_MODE_MASK,
>> + NOCP_CNT_ALARM_MODE_MIN_MAX_MASK);
>> +
>> + /* Enable the measurements by setting AlarmEn and StatEn */
>> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
>> + NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK,
>> + NOCP_MAIN_CTL_STATEN_MASK | NOCP_MAIN_CTL_ALARMEN_MASK);
>> +
>> + /* Set GlobalEN */
>> + regmap_update_bits(nocp->regmap, NOCP_CFG_CTL,
>> + NOCP_CFG_CTL_GLOBALEN_MASK,
>> + NOCP_CFG_CTL_GLOBALEN_MASK);
>> +
>> + /* Enable NoC probe */
>> + regmap_update_bits(nocp->regmap, NOCP_MAIN_CTL,
>> + NOCP_MAIN_CTL_STATEN_MASK,
>> + NOCP_MAIN_CTL_STATEN_MASK);
>
> In all these regmap_update_bits() calls you are ignoring the return
> value. This does not look right.

OK. I'll check the return value of regmap function.

>
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos_nocp_get_event(struct devfreq_event_dev *edev,
>> + struct devfreq_event_data *edata)
>> +{
>> + struct exynos_nocp *nocp = devfreq_event_get_drvdata(edev);
>> + unsigned int counter[4];
>> +
>> + /* Read cycle count */
>> + regmap_read(nocp->regmap, NOCP_COUNTERS_0_VAL, &counter[0]);
>> + regmap_read(nocp->regmap, NOCP_COUNTERS_1_VAL, &counter[1]);
>> + regmap_read(nocp->regmap, NOCP_COUNTERS_2_VAL, &counter[2]);
>> + regmap_read(nocp->regmap, NOCP_COUNTERS_3_VAL, &counter[3]);
>
> Ditto. If read fails, the data in counter should not be trusted (not
> initialized) so the devfreq_event_get_event() should not use it. The
> simplest would be to return error on each failure.

ditto.

>
>> +
>> + edata->load_count = ((counter[1] << 16) | counter[0]);
>> + edata->total_count = ((counter[3] << 16) | counter[2]);
>> +
>> + dev_dbg(&edev->dev, "%s (event: %ld/%ld)\n", edev->desc->name,
>> + edata->load_count, edata->total_count);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct devfreq_event_ops exynos_nocp_ops = {
>> + .set_event = exynos_nocp_set_event,
>> + .get_event = exynos_nocp_get_event,
>> +};
>> +
>> +static const struct of_device_id exynos_nocp_id_match[] = {
>> + { .compatible = "samsung,exynos5420-nocp", },
>> + { /* sentinel */ },
>> +};
>> +
>> +static struct regmap_config exynos_nocp_regmap_config = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .max_register = NOCP_COUNTERS_3_VAL,
>> +};
>> +
>> +static int exynos_nocp_parse_dt(struct platform_device *pdev,
>> + struct exynos_nocp *nocp)
>> +{
>> + struct device *dev = nocp->dev;
>> + struct device_node *np = dev->of_node;
>> + struct resource *res;
>> + void __iomem *base;
>> + int ret = 0;
>> +
>> + if (!np) {
>> + dev_err(dev, "failed to find devicetree node\n");
>> + return -EINVAL;
>> + }
>> +
>> + nocp->clk = devm_clk_get(dev, "nocp");
>> + if (IS_ERR(nocp->clk))
>> + nocp->clk = NULL;
>> +
>> + /* Maps the memory mapped IO to control nocp register */
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (IS_ERR(res))
>> + return PTR_ERR(res);
>> +
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> + exynos_nocp_regmap_config.max_register = resource_size(res) - 4;
>> +
>> + nocp->regmap = devm_regmap_init_mmio(dev, base,
>> + &exynos_nocp_regmap_config);
>> + if (IS_ERR(nocp->regmap)) {
>> + dev_err(dev, "failed to initialize regmap\n");
>> + ret = PTR_ERR(nocp->regmap);
>> + goto err;
>> + }
>> +
>> + return 0;
>> +
>> +err:
>> + devm_iounmap(dev, base);
>
> Why you need this? This is obtained by devm-like() interface so it
> should be released by the core.

I'll remove it.

>
>> +
>> + return ret;
>> +}
>> +
>> +static int exynos_nocp_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct exynos_nocp *nocp;
>> + int ret;
>> +
>> + nocp = devm_kzalloc(&pdev->dev, sizeof(*nocp), GFP_KERNEL);
>> + if (!nocp)
>> + return -ENOMEM;
>> +
>> + nocp->dev = &pdev->dev;
>> +
>> + /* Parse dt data to get resource */
>> + ret = exynos_nocp_parse_dt(pdev, nocp);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev,
>> + "failed to parse devicetree for resource\n");
>> + return ret;
>> + }
>> +
>> + /* Add devfreq-event device to measure the bandwidth of NoC */
>> + nocp->desc.ops = &exynos_nocp_ops;
>> + nocp->desc.driver_data = nocp;
>> + nocp->desc.name = np->name;
>> + nocp->edev = devm_devfreq_event_add_edev(&pdev->dev, &nocp->desc);
>> + if (IS_ERR(nocp->edev)) {
>> + ret = PTR_ERR(nocp->edev);
>> + dev_err(&pdev->dev,
>> + "failed to add devfreq-event device\n");
>> + goto err;
>
> This looks not needed and does not improve readability to me. Just
> 'return ret' always. At this point (before this if() statement) 'ret'
> equals to 0. Then you could remove the 'err' label and always 'return ret'.

OK.

Best Regards,
Chanwoo Choi

2016-04-15 06:08:31

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 2/7] PM / devfreq: exynos: Add the detailed correlation for Exynos5422 bus

Hi Rob,

On 2016년 04월 12일 00:45, Rob Herring wrote:
> On Fri, Apr 08, 2016 at 02:00:41PM +0900, Chanwoo Choi wrote:
>> This patch adds the detailed corrleation between sub-blocks and power line
>> for Exynos5422.
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> .../devicetree/bindings/devfreq/exynos-bus.txt | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>
> This belongs with your previous series. Probably, all of this should be
> 1 DT patch rather than piecemeal. Any division should be by h/w block,
> not as a driver is using features.

On previous patch-set[1] didn't support the Exynos5420 and include any information
about Exynos542x SoC because Exynos5420 must need the specific device driver
which is NoC (Network On Chip) Probe device. Only this patch-set support
the NoC Probe device. So, I make this separate patch on this patch-set.

[1] http://www.spinics.net/lists/arm-kernel/msg495976.html
- [PATCH v9 00/20] PM / devferq: Add generic exynos bus frequency driver and new passive governor

Best Regards,
Chanwoo Choi