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

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

[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

-- 
1.9.1

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
---
 .../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 <cw00.choi@samsung.com>
+ *
+ * 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 <cw00.choi@samsung.com>");
+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 <cw00.choi@samsung.com>
+ *
+ * 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

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

[Krzysztof Kozlowski]

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 <cw00.choi@samsung.com>
> ---
>  .../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 <cw00.choi@samsung.com>
> + *
> + * 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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
>> ---
>>  .../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 <cw00.choi@samsung.com>
>> + *
>> + * 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

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

[Chanwoo Choi]

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

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../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

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

[Rob Herring]

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 <cw00.choi@samsung.com>
> ---
>  .../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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
>> ---
>>  .../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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
---
 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

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

[Krzysztof Kozlowski]

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 <cw00.choi@samsung.com>
> ---
>  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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
>> ---
>>  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

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

[Chanwoo Choi]

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 <s.nawrocki@samsung.com>
Cc: Tomasz Figa <tomasz.figa@gmail.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 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

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

[Krzysztof Kozlowski]

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 <s.nawrocki@samsung.com>
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  include/dt-bindings/clock/exynos5420.h | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
---
 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

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

[Krzysztof Kozlowski]

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 <cw00.choi@samsung.com>
> ---
>  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

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

[Tomasz Figa]

2016-04-12 14:25 GMT+03:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> 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 <cw00.choi@samsung.com>
>> ---
>>  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

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

[Chanwoo Choi]

Hi Tomasz and Krzysztof,

On 2016년 04월 13일 15:20, Tomasz Figa wrote:
> 2016-04-12 14:25 GMT+03:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
>> 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 <cw00.choi@samsung.com>
>>> ---
>>>  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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
---
 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

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

[Krzysztof Kozlowski]

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 <cw00.choi@samsung.com>
> ---
>  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 <k.kozlowski@samsung.com>

Best regards,
Krzysztof

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
>> ---
>>  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 <k.kozlowski@samsung.com>

Thanks for the review.

Best Regards,
Chanwoo Choi

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
---
 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

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

[Krzysztof Kozlowski]

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 <cw00.choi@samsung.com>
> ---
>  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

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

[Chanwoo Choi]

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 <cw00.choi@samsung.com>
>> ---
>>  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

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

[Markus Reichl]

Hi Chanwoo,

I tested both sets on odroid XU4 and U3.
Tested-by: Markus Reichl <m.reichl@fivetechno.de>

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

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

[Chanwoo Choi]

Hi Markus,

On Fri, Apr 8, 2016 at 9:55 PM, Markus Reichl <m.reichl@fivetechno.de> wrote:
> Hi Chanwoo,
>
> I tested both sets on odroid XU4 and U3.
> Tested-by: Markus Reichl <m.reichl@fivetechno.de>

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
>

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

[Anand Moon]

Hi Chanwoo,

On 8 April 2016 at 10:30, Chanwoo Choi <cw00.choi@samsung.com> 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

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

[Chanwoo Choi]

Hi Anand,

On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <linux.amoon@gmail.com> wrote:
> Hi Chanwoo,
>
> On 8 April 2016 at 10:30, Chanwoo Choi <cw00.choi@samsung.com> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chanwoo Choi]

Hi Anand,

On 2016년 04월 09일 03:24, Chanwoo Choi wrote:
> Hi Anand,
> 
> On Sat, Apr 9, 2016 at 3:11 AM, Anand Moon <linux.amoon@gmail.com> 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 majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

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

[Anand Moon]

Hi Chanwoo,

On 11 April 2016 at 07:46, Chanwoo Choi <cw00.choi@samsung.com> 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 <linux.amoon@gmail.com> 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 <linux.amoon@gmail.com>

Tested on Odroid XU4 and Odroid U3.

Best Regards
-Anand Moon

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

[Chanwoo Choi]

Hi Anand,

On 2016년 04월 11일 13:01, Anand Moon wrote:
> Hi Chanwoo,
> 
> On 11 April 2016 at 07:46, Chanwoo Choi <cw00.choi@samsung.com> 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 <linux.amoon@gmail.com> 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 <linux.amoon@gmail.com>
> 
> Tested on Odroid XU4 and Odroid U3.

Thanks for your test and report.

Best Regards,
Chanwoo Choi

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

[Chanwoo Choi]

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