[PATCH 0/8] interconnect: Add imx support via devfreq

[PATCH 0/8] interconnect: Add imx support via devfreq

[Leonard Crestez]

This series adds interconnect scaling support for imx8m series chips. It uses a
per-SOC interconnect provider layered on top of multiple instances of devfreq
for scalable nodes along the interconnect.

Existing qcom interconnect providers mostly translate bandwidth requests into
firmware calls but equivalent firmware on imx8m is much thinner. Scaling
support for individual nodes is implemented as distinct devfreq drivers
instead.

The imx interconnect provider doesn't communicate with devfreq directly
but rather computes "minimum frequencies" for nodes along the path and
creates dev_pm_qos requests.

Since there is no single devicetree node that can represent the
"interconnect" the main NOC is picked as the "interconnect provider" and
will probe the interconnect platform device if #interconnect-cells is
present. This avoids introducing "virtual" devices but it means that DT
bindings of main NOC includes properties for both devfreq and
interconnect.

Only the ddrc and main noc are scalable right now but more can be added.

Also available on a github branch (with various unrelated changes):
	https://github.com/cdleonard/linux/tree/next
Testing currently requires NXP branch of atf+uboot

Martin: I believe you should be able to use this to control DRAM
frequency from video by just adding interconnect consumer code to
nwl-dsi. Sample code:
	https://github.com/cdleonard/linux/commit/43772762aa5045f1ce5623740f9a4baef988d083
	https://github.com/cdleonard/linux/commit/7b601e981b1f517b5d98b43bde292972ded13086

Changes since RFCv6:
* Replace scalable-nodes stuff with just a fsl,ddrc property. Future
scalable nodes can be added as additional phandles on the NOC
* Allow building interconnect drivers as modules
* Handle icc_provider_del errors in imx_icc_unregister (like EBUSY).
* Rename imx-devfreq to imx-bus, similar to exynos-bus
* Explain why imx bus clock enabling is not required
* All dependencies accepted (some time ago).
Link: https://patchwork.kernel.org/cover/11244421/

Changes since RFCv5:
* Replace scanning for interconnect-node-id with explicit
scalable-nodes/scalable-node-ids property on NoC.
* Now passes make `dtbs_check`
* Remove struct imx_icc_provider
* Switch to of_icc_xlate_onecell
* Use of_find_device_by_node to fetch QoS target, this causes fewer probe
deferrals, removes dependency on devfreq API and even allows reloading ddrc
module at runtime
* Add imx_icc_node_destroy helper
* Remove 0/1 on DEFINE_BUS_SLAVE/MASTER which created spurious links
Link: https://patchwork.kernel.org/cover/11222015/

Changes since RFCv4:
* Drop icc proxy nonsense
* Make devfreq driver for NOC probe the ICC driver if
#interconnect-cells is present
* Move NOC support to interconnect series and rename the node in DT
* Add support for all chips at once, differences are not intereseting
and there is more community interest for 8mq than 8mm.
Link: https://patchwork.kernel.org/cover/11111865/

Changes since RFCv3:
* Remove the virtual "icc" node and add devfreq nodes as proxy providers
* Fix build on 32-bit arm (reported by kbuilt test robot)
* Remove ARCH_MXC_ARM64 (never existed in upstream)
* Remove _numlinks, calculate instead
* Replace __BUSFREQ_H header guard
* Improve commit message and comment spelling
* Fix checkpatch issues
Link to RFCv3: https://patchwork.kernel.org/cover/11078671/

Changes since RFCv2 and initial work by Alexandre Bailon:
* Relying on devfreq and dev_pm_qos instead of CLK
* No more "platform opp" stuff
* No more special suspend handling: use suspend-opp on devfreq instead
* Replace all mentions of "busfreq" with "interconnect"
Link to v2: https://patchwork.kernel.org/cover/11021563/

Leonard Crestez (8):
  dt-bindings: interconnect: Add bindings for imx8m noc
  PM / devfreq: Add generic imx bus scaling driver
  PM / devfreq: imx: Register interconnect device
  interconnect: Add imx core driver
  interconnect: imx: Add platform driver for imx8mm
  interconnect: imx: Add platform driver for imx8mq
  interconnect: imx: Add platform driver for imx8mn
  arm64: dts: imx8m: Add NOC nodes

 .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  24 ++
 arch/arm64/boot/dts/freescale/imx8mn.dtsi     |  24 ++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi     |  24 ++
 drivers/devfreq/Kconfig                       |   9 +
 drivers/devfreq/Makefile                      |   1 +
 drivers/devfreq/imx-bus.c                     | 181 +++++++++++
 drivers/interconnect/Kconfig                  |   1 +
 drivers/interconnect/Makefile                 |   1 +
 drivers/interconnect/imx/Kconfig              |  17 +
 drivers/interconnect/imx/Makefile             |   9 +
 drivers/interconnect/imx/imx.c                | 298 ++++++++++++++++++
 drivers/interconnect/imx/imx.h                |  62 ++++
 drivers/interconnect/imx/imx8mm.c             | 108 +++++++
 drivers/interconnect/imx/imx8mn.c             |  97 ++++++
 drivers/interconnect/imx/imx8mq.c             | 106 +++++++
 include/dt-bindings/interconnect/imx8mm.h     |  49 +++
 include/dt-bindings/interconnect/imx8mn.h     |  41 +++
 include/dt-bindings/interconnect/imx8mq.h     |  48 +++
 19 files changed, 1238 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
 create mode 100644 drivers/devfreq/imx-bus.c
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/imx.c
 create mode 100644 drivers/interconnect/imx/imx.h
 create mode 100644 drivers/interconnect/imx/imx8mm.c
 create mode 100644 drivers/interconnect/imx/imx8mn.c
 create mode 100644 drivers/interconnect/imx/imx8mq.c
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h
 create mode 100644 include/dt-bindings/interconnect/imx8mn.h
 create mode 100644 include/dt-bindings/interconnect/imx8mq.h

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/8] dt-bindings: interconnect: Add bindings for imx8m noc

[Leonard Crestez]

Add initial dt bindings for the interconnects inside i.MX chips.
Multiple external IPs are involved but SOC integration means the
software controllable interfaces are very similar.

Main NOC node acts as interconnect provider if #interconnect-cells is
present. Currently there is a single imx interconnect provider for the
whole SOC.

Other pieces of scalable interconnects can be present, each with their
own OPP table.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml

diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
new file mode 100644
index 000000000000..6c192f1c0edd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/fsl,imx8m-noc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX bus frequency device
+
+maintainers:
+  - Leonard Crestez <leonard.crestez@nxp.com>
+
+description: |
+  The i.MX SoC family has multiple buses for which clock frequency (and
+  sometimes voltage) can be adjusted.
+
+  Some of those buses expose register areas mentioned in the memory maps as GPV
+  ("Global Programmers View") but not all. Access to this area might be denied
+  for normal (non-secure) world.
+
+  The buses are based on externally licensed IPs such as ARM NIC-301 and
+  Arteris FlexNOC but DT bindings are specific to the integration of these bus
+  interconnect IPs into imx SOCs.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - fsl,imx8mn-nic
+          - fsl,imx8mm-nic
+          - fsl,imx8mq-nic
+        - const: fsl,imx8m-nic
+      - items:
+        - enum:
+          - fsl,imx8mn-noc
+          - fsl,imx8mm-noc
+          - fsl,imx8mq-noc
+        - const: fsl,imx8m-noc
+      - const: fsl, imx8m-nic
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  operating-points-v2: true
+  opp-table: true
+
+  fsl,ddrc:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description:
+      Phandle to DDR Controller.
+
+  '#interconnect-cells':
+    description:
+      If specified then also act as an interconnect provider. Should only be
+      set once per soc on the main noc.
+    const: 1
+
+required:
+  - compatible
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+#include <dt-bindings/clock/imx8mm-clock.h>
+#include <dt-bindings/interconnect/imx8mm.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+noc: interconnect@32700000 {
+        compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
+        reg = <0x32700000 0x100000>;
+        clocks = <&clk IMX8MM_CLK_NOC>;
+        #interconnect-cells = <1>;
+
+        /* Used to make requests for DEV_PM_QOS_MIN_FREQUENCY: */
+        fsl,scalable-node-ids = <IMX8MM_ICN_NOC>,
+                                <IMX8MM_ICN_MIPI>,
+                                <IMX8MM_ICS_DRAM>;
+        fsl,scalable-nodes = <&noc>,
+                             <&pl301_mipi>,
+                             <&ddrc>;
+
+        /* For passive governor: */
+        devfreq = <&ddrc>;
+
+        operating-points-v2 = <&noc_opp_table>;
+        noc_opp_table: opp-table {
+                compatible = "operating-points-v2";
+
+                opp-133M {
+                        opp-hz = /bits/ 64 <133333333>;
+                };
+                opp-800M {
+                        opp-hz = /bits/ 64 <800000000>;
+                };
+        };
+};
+
+pl301_mipi: interconnect@32500000 {
+        compatible = "fsl,imx8m-nic";
+        reg = <0x32500000 0x100000>;
+        clocks = <&clk IMX8MM_CLK_DISP_AXI>;
+        operating-points-v2 = <&pl301_mipi_opp_table>;
+
+        pl301_mipi_opp_table: opp-table {
+                compatible = "operating-points-v2";
+
+                opp-200M {
+                        opp-hz = /bits/ 64 <200000000>;
+                };
+                opp-500M {
+                        opp-hz = /bits/ 64 <500000000>;
+                };
+        };
+};
+
+ddrc: memory-controller@3d400000 {
+        compatible = "fsl,imx8mm-ddrc", "fsl,imx8m-ddrc";
+        reg = <0x3d400000 0x400000>;
+        glock-names = "core", "pll", "alt", "apb";
+        clocks = <&clk IMX8MM_CLK_DRAM_CORE>,
+                 <&clk IMX8MM_DRAM_PLL>,
+                 <&clk IMX8MM_CLK_DRAM_ALT>,
+                 <&clk IMX8MM_CLK_DRAM_APB>;
+        /* For ondemand governor: */
+        devfreq-events = <&ddr_pmu>;
+};
+
+ddr_pmu: ddr-pmu@3d800000 {
+        compatible = "fsl,imx8mm-ddr-pmu", "fsl,imx8m-ddr-pmu";
+        reg = <0x3d800000 0x400000>;
+        interrupt-parent = <&gic>;
+        interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+};
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Leonard Crestez]

Add initial support for dynamic frequency switching on pieces of the imx
interconnect fabric.

All this driver does is set a clk rate based on an opp table, it does
not map register areas.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/devfreq/Kconfig   |   9 +++
 drivers/devfreq/Makefile  |   1 +
 drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+)
 create mode 100644 drivers/devfreq/imx-bus.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 0b1df12e0f21..44d26192ddc4 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
 	select DEVFREQ_GOV_USERSPACE
 	help
 	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
 	  adjusting DRAM frequency.
 
+config ARM_IMX_BUS_DEVFREQ
+	tristate "i.MX Generic Bus DEVFREQ Driver"
+	depends on ARCH_MXC || COMPILE_TEST
+	select DEVFREQ_GOV_PASSIVE
+	select DEVFREQ_GOV_USERSPACE
+	help
+	  This adds the generic DEVFREQ driver for i.MX interconnects. It
+	  allows adjusting NIC/NOC frequency.
+
 config ARM_TEGRA_DEVFREQ
 	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
 	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
 		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
 		ARCH_TEGRA_210_SOC || \
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 3eb4d5e6635c..3ca1ad0ecb97 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
 obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
 obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
 
 # DEVFREQ Drivers
 obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
+obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
 obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
 obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
 obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
 obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
 
diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
new file mode 100644
index 000000000000..285e0f1ae6b1
--- /dev/null
+++ b/drivers/devfreq/imx-bus.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/devfreq.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pm_opp.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+struct imx_bus {
+	struct devfreq_dev_profile profile;
+	struct devfreq *devfreq;
+	struct clk *clk;
+	struct devfreq_passive_data passive_data;
+};
+
+static int imx_bus_target(struct device *dev,
+		unsigned long *freq, u32 flags)
+{
+	struct imx_bus *priv = dev_get_drvdata(dev);
+	struct dev_pm_opp *new_opp;
+	unsigned long new_freq;
+	int ret;
+
+	new_opp = devfreq_recommended_opp(dev, freq, flags);
+	if (IS_ERR(new_opp)) {
+		ret = PTR_ERR(new_opp);
+		dev_err(dev, "failed to get recommended opp: %d\n", ret);
+		return ret;
+	}
+	new_freq = dev_pm_opp_get_freq(new_opp);
+	dev_pm_opp_put(new_opp);
+
+	return clk_set_rate(priv->clk, new_freq);
+}
+
+static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+	struct imx_bus *priv = dev_get_drvdata(dev);
+
+	*freq = clk_get_rate(priv->clk);
+
+	return 0;
+}
+
+static int imx_bus_get_dev_status(struct device *dev,
+		struct devfreq_dev_status *stat)
+{
+	struct imx_bus *priv = dev_get_drvdata(dev);
+
+	stat->busy_time = 0;
+	stat->total_time = 0;
+	stat->current_frequency = clk_get_rate(priv->clk);
+
+	return 0;
+}
+
+static void imx_bus_exit(struct device *dev)
+{
+	dev_pm_opp_of_remove_table(dev);
+}
+
+static int imx_bus_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_bus *priv;
+	const char *gov = DEVFREQ_GOV_USERSPACE;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Fetch the clock to adjust but don't explictly enable.
+	 *
+	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
+	 * or off and some peripheral side-buses might be off unless enabled by
+	 * drivers for devices on those specific buses.
+	 *
+	 * Rate adjustment on a disabled bus clock just takes effect later.
+	 */
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		dev_err(dev, "failed to fetch clk: %d\n", ret);
+		return ret;
+	}
+	platform_set_drvdata(pdev, priv);
+
+	ret = dev_pm_opp_of_add_table(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to get OPP table\n");
+		return ret;
+	}
+
+	priv->profile.polling_ms = 1000;
+	priv->profile.target = imx_bus_target;
+	priv->profile.get_dev_status = imx_bus_get_dev_status;
+	priv->profile.exit = imx_bus_exit;
+	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
+	priv->profile.initial_freq = clk_get_rate(priv->clk);
+
+	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
+						gov, NULL);
+	if (IS_ERR(priv->devfreq)) {
+		ret = PTR_ERR(priv->devfreq);
+		dev_err(dev, "failed to add devfreq device: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	dev_pm_opp_of_remove_table(dev);
+	return ret;
+}
+
+static const struct of_device_id imx_bus_of_match[] = {
+	{ .compatible = "fsl,imx8m-noc", },
+	{ .compatible = "fsl,imx8m-nic", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx_bus_of_match);
+
+static struct platform_driver imx_bus_platdrv = {
+	.probe		= imx_bus_probe,
+	.driver = {
+		.name	= "imx-bus-devfreq",
+		.of_match_table = of_match_ptr(imx_bus_of_match),
+	},
+};
+module_platform_driver(imx_bus_platdrv);
+
+MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
+MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/8] PM / devfreq: imx: Register interconnect device

[Leonard Crestez]

There is no single device which can represent the imx interconnect.
Instead of adding a virtual one just make the main &noc act as the
global interconnect provider.

The imx interconnect provider driver will scale the NOC and DDRC based
on bandwidth request. More scalable nodes can be added in the future,
for example for audio/display/vpu/gpu NICs.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/devfreq/imx-bus.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
index 285e0f1ae6b1..e9b13e43bf0a 100644
--- a/drivers/devfreq/imx-bus.c
+++ b/drivers/devfreq/imx-bus.c
@@ -15,10 +15,11 @@
 struct imx_bus {
 	struct devfreq_dev_profile profile;
 	struct devfreq *devfreq;
 	struct clk *clk;
 	struct devfreq_passive_data passive_data;
+	struct platform_device *icc_pdev;
 };
 
 static int imx_bus_target(struct device *dev,
 		unsigned long *freq, u32 flags)
 {
@@ -60,11 +61,42 @@ static int imx_bus_get_dev_status(struct device *dev,
 	return 0;
 }
 
 static void imx_bus_exit(struct device *dev)
 {
+	struct imx_bus *priv = dev_get_drvdata(dev);
+
 	dev_pm_opp_of_remove_table(dev);
+	platform_device_unregister(priv->icc_pdev);
+}
+
+/* imx_bus_init_icc() - register matching icc provider if required */
+static int imx_bus_init_icc(struct device *dev)
+{
+	struct imx_bus *priv = dev_get_drvdata(dev);
+	const char *icc_driver_name;
+
+	if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
+		return 0;
+	if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX)) {
+		dev_warn(dev, "imx interconnect drivers disabled\n");
+		return 0;
+	}
+
+	icc_driver_name = of_device_get_match_data(dev);
+	if (!icc_driver_name)
+		return 0;
+
+	priv->icc_pdev = platform_device_register_data(
+			dev, icc_driver_name, -1, NULL, 0);
+	if (IS_ERR(priv->icc_pdev)) {
+		dev_err(dev, "failed to register icc provider %s: %ld\n",
+				icc_driver_name, PTR_ERR(priv->devfreq));
+		return PTR_ERR(priv->devfreq);
+	}
+
+	return 0;
 }
 
 static int imx_bus_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -112,18 +144,25 @@ static int imx_bus_probe(struct platform_device *pdev)
 		ret = PTR_ERR(priv->devfreq);
 		dev_err(dev, "failed to add devfreq device: %d\n", ret);
 		goto err;
 	}
 
+	ret = imx_bus_init_icc(dev);
+	if (ret)
+		goto err;
+
 	return 0;
 
 err:
 	dev_pm_opp_of_remove_table(dev);
 	return ret;
 }
 
 static const struct of_device_id imx_bus_of_match[] = {
+	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
+	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
+	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
 	{ .compatible = "fsl,imx8m-noc", },
 	{ .compatible = "fsl,imx8m-nic", },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, imx_bus_of_match);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/8] interconnect: Add imx core driver

[Leonard Crestez]

This adds support for i.MX SoC family to interconnect framework.

Platform drivers can describe the interconnect graph and several
adjustment knobs where icc node bandwidth is converted to a
DEV_PM_QOS_MIN_FREQUENCY request.

The interconnect provider is probed through the main NOC device and
other adjustable nodes on the same graph are found from a
fsl,scalable-nodes phandle array property.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/interconnect/Kconfig      |   1 +
 drivers/interconnect/Makefile     |   1 +
 drivers/interconnect/imx/Kconfig  |   5 +
 drivers/interconnect/imx/Makefile |   3 +
 drivers/interconnect/imx/imx.c    | 298 ++++++++++++++++++++++++++++++
 drivers/interconnect/imx/imx.h    |  62 +++++++
 6 files changed, 370 insertions(+)
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/imx.c
 create mode 100644 drivers/interconnect/imx/imx.h

diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..e61802230f90 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -10,7 +10,8 @@ menuconfig INTERCONNECT
 	  If unsure, say no.
 
 if INTERCONNECT
 
 source "drivers/interconnect/qcom/Kconfig"
+source "drivers/interconnect/imx/Kconfig"
 
 endif
diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 725029ae7a2c..6998288a7d98 100644
--- a/drivers/interconnect/Makefile
+++ b/drivers/interconnect/Makefile
@@ -3,5 +3,6 @@
 CFLAGS_core.o				:= -I$(src)
 icc-core-objs				:= core.o
 
 obj-$(CONFIG_INTERCONNECT)		+= icc-core.o
 obj-$(CONFIG_INTERCONNECT_QCOM)		+= qcom/
+obj-$(CONFIG_INTERCONNECT_IMX)		+= imx/
diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
new file mode 100644
index 000000000000..f39336f8d603
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,5 @@
+config INTERCONNECT_IMX
+	tristate "i.MX interconnect drivers"
+	depends on ARCH_MXC || COMPILE_TEST
+	help
+	  Generic interconnect drivers for i.MX SOCs
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..86ae0bd28d8c
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1,3 @@
+imx-interconnect-objs			:= imx.o
+
+obj-$(CONFIG_INTERCONNECT_IMX)		+= imx-interconnect.o
diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
new file mode 100644
index 000000000000..527b1de1c41a
--- /dev/null
+++ b/drivers/interconnect/imx/imx.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+
+#include "imx.h"
+
+/* private icc_node data */
+struct imx_icc_node {
+	const struct imx_icc_node_desc *desc;
+	struct device *qos_dev;
+	struct dev_pm_qos_request qos_req;
+};
+
+static int imx_icc_aggregate(struct icc_node *node, u32 tag,
+			     u32 avg_bw, u32 peak_bw,
+			     u32 *agg_avg, u32 *agg_peak)
+{
+	*agg_avg += avg_bw;
+	*agg_peak = max(*agg_peak, peak_bw);
+
+	return 0;
+}
+
+static int imx_icc_node_set(struct icc_node *node)
+{
+	struct device *dev = node->provider->dev;
+	struct imx_icc_node *node_data = node->data;
+	u64 freq;
+
+	if (!node_data->qos_dev)
+		return 0;
+
+	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
+	do_div(freq, node_data->desc->adj->bw_div);
+	dev_dbg(dev, "node %s device %s avg_bw %ukBps peak_bw %ukBps min_freq %llukHz\n",
+		node->name, dev_name(node_data->qos_dev),
+		node->avg_bw, node->peak_bw, freq);
+
+	if (freq > S32_MAX) {
+		dev_err(dev, "%s can't request more than S32_MAX freq\n",
+				node->name);
+		return -ERANGE;
+	}
+
+	dev_pm_qos_update_request(&node_data->qos_req, freq);
+
+	return 0;
+}
+
+static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	return imx_icc_node_set(dst);
+}
+
+/* imx_icc_node_destroy() - Destroy an imx icc_node, including private data */
+static void imx_icc_node_destroy(struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	int ret;
+
+	if (dev_pm_qos_request_active(&node_data->qos_req)) {
+		ret = dev_pm_qos_remove_request(&node_data->qos_req);
+		if (ret)
+			dev_warn(node->provider->dev, "failed to remove qos request for %s\n",
+				 dev_name(node_data->qos_dev));
+	}
+
+	put_device(node_data->qos_dev);
+	icc_node_del(node);
+	icc_node_destroy(node->id);
+}
+
+static int imx_icc_node_init_qos(struct icc_provider *provider,
+				 struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	const struct imx_icc_node_adj_desc *adj = node_data->desc->adj;
+	struct device *dev = provider->dev;
+	struct device_node *dn = NULL;
+	struct platform_device *pdev;
+
+	if (adj->main_noc) {
+		node_data->qos_dev = dev;
+		dev_info(dev, "icc node %s[%d] is main noc itself\n",
+			 node->name, node->id);
+	} else {
+		dn = of_parse_phandle(dev->of_node, adj->phandle_name, 0);
+		if (IS_ERR(dn)) {
+			dev_warn(dev, "Failed to parse %s: %ld\n",
+					adj->phandle_name, PTR_ERR(dn));
+			return PTR_ERR(dn);
+		}
+		/* Allow scaling to be disabled on a per-node basis */
+		if (!dn || !of_device_is_available(dn)) {
+			dev_warn(dev, "Missing property %s, skip scaling %s\n",
+					adj->phandle_name, node->name);
+			return 0;
+		}
+
+		pdev = of_find_device_by_node(dn);
+		of_node_put(dn);
+		if (!pdev) {
+			dev_warn(dev, "node %s[%d] missing device for %pOF\n",
+					node->name, node->id, dn);
+			return -EPROBE_DEFER;
+		}
+		node_data->qos_dev = &pdev->dev;
+		dev_info(dev, "node %s[%d] has device node %pOF\n",
+			 node->name, node->id, dn);
+	}
+
+	return dev_pm_qos_add_request(node_data->qos_dev,
+				      &node_data->qos_req,
+				      DEV_PM_QOS_MIN_FREQUENCY, 0);
+}
+
+static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+					 const struct imx_icc_node_desc *node_desc)
+{
+	struct device *dev = provider->dev;
+	struct imx_icc_node *node_data;
+	struct icc_node *node;
+	int ret;
+
+	node = icc_node_create(node_desc->id);
+	if (IS_ERR(node)) {
+		dev_err(dev, "failed to create node %d\n", node_desc->id);
+		return node;
+	}
+
+	if (node->data) {
+		dev_err(dev, "already created node %s id=%d\n",
+				node_desc->name, node_desc->id);
+		return ERR_PTR(-EEXIST);
+	}
+
+	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
+	if (!node_data) {
+		icc_node_destroy(node->id);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	node->name = node_desc->name;
+	node->data = node_data;
+	node_data->desc = node_desc;
+	icc_node_add(node, provider);
+
+	if (node_desc->adj) {
+		ret = imx_icc_node_init_qos(provider, node);
+		if (ret < 0) {
+			imx_icc_node_destroy(node);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return node;
+}
+
+static void imx_icc_unregister_nodes(struct icc_provider *provider)
+{
+	struct icc_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list)
+		imx_icc_node_destroy(node);
+}
+
+static int imx_icc_register_nodes(struct icc_provider *provider,
+				  const struct imx_icc_node_desc *descs,
+				  int count)
+{
+	struct icc_onecell_data *provider_data = provider->data;
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct icc_node *node;
+		const struct imx_icc_node_desc *node_desc = &descs[i];
+		size_t j;
+
+		node = imx_icc_node_add(provider, node_desc);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			if (ret != -EPROBE_DEFER)
+				dev_err(provider->dev, "failed to add %s: %d\n",
+					node_desc->name, ret);
+			goto err;
+		}
+		provider_data->nodes[node->id] = node;
+
+		for (j = 0; j < node_desc->num_links; j++) {
+			ret = icc_link_create(node, node_desc->links[j]);
+			if (ret) {
+				dev_err(provider->dev, "failed to link node %d to %d: %d\n",
+					node->id, node_desc->links[j], ret);
+				goto err;
+			}
+		}
+	}
+
+	return 0;
+
+err:
+	imx_icc_unregister_nodes(provider);
+
+	return ret;
+}
+
+static int get_max_node_id(struct imx_icc_node_desc *nodes, int nodes_count)
+{
+	int i, ret = 0;
+
+	for (i = 0; i < nodes_count; ++i)
+		if (nodes[i].id > ret)
+			ret = nodes[i].id;
+
+	return ret;
+}
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes, int nodes_count)
+{
+	struct device *dev = &pdev->dev;
+	struct icc_onecell_data *data;
+	struct icc_provider *provider;
+	int max_node_id;
+	int ret;
+
+	/* icc_onecell_data is indexed by node_id, unlike nodes param */
+	max_node_id = get_max_node_id(nodes, nodes_count);
+	data = devm_kzalloc(dev, struct_size(data, nodes, max_node_id),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	data->num_nodes = max_node_id;
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+	provider->set = imx_icc_set;
+	provider->aggregate = imx_icc_aggregate;
+	provider->xlate = of_icc_xlate_onecell;
+	provider->data = data;
+	provider->dev = dev->parent;
+	platform_set_drvdata(pdev, provider);
+
+	ret = icc_provider_add(provider);
+	if (ret) {
+		dev_err(dev, "error adding interconnect provider: %d\n", ret);
+		return ret;
+	}
+
+	ret = imx_icc_register_nodes(provider, nodes, nodes_count);
+	if (ret)
+		goto provider_del;
+
+	return 0;
+
+provider_del:
+	icc_provider_del(provider);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_icc_register);
+
+int imx_icc_unregister(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+	int ret;
+
+	if (provider->users) {
+		dev_warn(&pdev->dev, "interconnect provider still has %d users\n",
+			provider->users);
+		return -EBUSY;
+	}
+	imx_icc_unregister_nodes(provider);
+
+	ret = icc_provider_del(provider);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(imx_icc_unregister);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
new file mode 100644
index 000000000000..aa811e4ebb7e
--- /dev/null
+++ b/drivers/interconnect/imx/imx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+#ifndef __DRIVERS_INTERCONNECT_IMX_H
+#define __DRIVERS_INTERCONNECT_IMX_H
+
+#include <linux/kernel.h>
+
+#define IMX_ICC_MAX_LINKS	4
+
+/*
+ * struct imx_icc_node_adj - Describe a dynamic adjustable node
+ */
+struct imx_icc_node_adj_desc {
+	unsigned int bw_mul, bw_div;
+	const char *phandle_name;
+	bool main_noc;
+};
+
+/*
+ * struct imx_icc_node - Describe an interconnect node
+ * @name: name of the node
+ * @id: an unique id to identify the node
+ * @links: an array of slaves' node id
+ * @num_links: number of id defined in links
+ */
+struct imx_icc_node_desc {
+	const char *name;
+	u16 id;
+	u16 links[IMX_ICC_MAX_LINKS];
+	u16 num_links;
+
+	const struct imx_icc_node_adj_desc *adj;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, ...)			\
+	{								\
+		.id = _id,						\
+		.name = _name,						\
+		.adj = _adj,						\
+		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\
+		.links = { __VA_ARGS__ },				\
+	}
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, _adj)
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes,
+		     int nodes_count);
+int imx_icc_unregister(struct platform_device *pdev);
+
+#endif /* __DRIVERS_INTERCONNECT_IMX_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/8] interconnect: imx: Add platform driver for imx8mm

[Leonard Crestez]

Add a platform driver for the i.MX8MM SoC describing bus topology.

Bandwidth adjustments is currently only supported on the DDRC and main
NOC. Scaling for the vpu/gpu/display NICs could be added in the future.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/interconnect/imx/Kconfig          |   4 +
 drivers/interconnect/imx/Makefile         |   2 +
 drivers/interconnect/imx/imx8mm.c         | 108 ++++++++++++++++++++++
 include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
 4 files changed, 163 insertions(+)
 create mode 100644 drivers/interconnect/imx/imx8mm.c
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index f39336f8d603..2cd4fad4976a 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -1,5 +1,9 @@
 config INTERCONNECT_IMX
 	tristate "i.MX interconnect drivers"
 	depends on ARCH_MXC || COMPILE_TEST
 	help
 	  Generic interconnect drivers for i.MX SOCs
+
+config INTERCONNECT_IMX8MM
+	tristate "i.MX8MM interconnect driver"
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index 86ae0bd28d8c..c234e5d3dfd1 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1,3 +1,5 @@
 imx-interconnect-objs			:= imx.o
+imx8mm-interconnect-objs       		:= imx8mm.o
 
 obj-$(CONFIG_INTERCONNECT_IMX)		+= imx-interconnect.o
+obj-$(CONFIG_INTERCONNECT_IMX8MM)	+= imx8mm-interconnect.o
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
new file mode 100644
index 000000000000..ee3783a98c01
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mm.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
+	.bw_mul = 1,
+	.bw_div = 16,
+	.phandle_name = "fsl,ddrc",
+};
+
+static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
+	.bw_mul = 1,
+	.bw_div = 16,
+	.main_noc = true,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+			IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
+
+	/* VPUMIX */
+	DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, IMX8MM_ICN_NOC),
+
+	/* GPUMIX */
+	DEFINE_BUS_MASTER("GPU 2D", IMX8MM_ICM_GPU2D, IMX8MM_ICN_GPU),
+	DEFINE_BUS_MASTER("GPU 3D", IMX8MM_ICM_GPU3D, IMX8MM_ICN_GPU),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MM_ICN_GPU, NULL, IMX8MM_ICN_NOC),
+
+	/* DISPLAYMIX */
+	DEFINE_BUS_MASTER("CSI", IMX8MM_ICM_CSI, IMX8MM_ICN_MIPI),
+	DEFINE_BUS_MASTER("LCDIF", IMX8MM_ICM_LCDIF, IMX8MM_ICN_MIPI),
+	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MM_ICN_MIPI, NULL, IMX8MM_ICN_NOC),
+
+	/* HSIO */
+	DEFINE_BUS_MASTER("USB1", IMX8MM_ICM_USB1, IMX8MM_ICN_HSIO),
+	DEFINE_BUS_MASTER("USB2", IMX8MM_ICM_USB2, IMX8MM_ICN_HSIO),
+	DEFINE_BUS_MASTER("PCIE", IMX8MM_ICM_PCIE, IMX8MM_ICN_HSIO),
+	DEFINE_BUS_INTERCONNECT("PL301_HSIO", IMX8MM_ICN_HSIO, NULL, IMX8MM_ICN_NOC),
+
+	/* Audio */
+	DEFINE_BUS_MASTER("SDMA2", IMX8MM_ICM_SDMA2, IMX8MM_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA3", IMX8MM_ICM_SDMA3, IMX8MM_ICN_AUDIO),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MM_ICN_AUDIO, NULL, IMX8MM_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, IMX8MM_ICN_MAIN),
+
+	/* Other */
+	DEFINE_BUS_MASTER("SDMA1", IMX8MM_ICM_SDMA1, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_MASTER("NAND", IMX8MM_ICM_NAND, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC1", IMX8MM_ICM_USDHC1, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC2", IMX8MM_ICM_USDHC2, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC3", IMX8MM_ICM_USDHC3, IMX8MM_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MM_ICN_MAIN, NULL,
+			IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
+};
+
+static int imx8mm_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mm_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static struct platform_driver imx8mm_icc_driver = {
+	.probe = imx8mm_icc_probe,
+	.remove = imx8mm_icc_remove,
+	.driver = {
+		.name = "imx8mm-interconnect",
+	},
+};
+
+module_platform_driver(imx8mm_icc_driver);
+MODULE_AUTHOR("Alexandre Bailon <abailon@baylibre.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx8mm-interconnect");
diff --git a/include/dt-bindings/interconnect/imx8mm.h b/include/dt-bindings/interconnect/imx8mm.h
new file mode 100644
index 000000000000..5404f2af15c3
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#ifndef __IMX8MM_ICM_INTERCONNECT_IDS_H
+#define __IMX8MM_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MM_ICN_NOC		1
+#define IMX8MM_ICS_DRAM		2
+#define IMX8MM_ICS_OCRAM	3
+#define IMX8MM_ICM_A53		4
+
+#define IMX8MM_ICM_VPU_H1	5
+#define IMX8MM_ICM_VPU_G1	6
+#define IMX8MM_ICM_VPU_G2	7
+#define IMX8MM_ICN_VIDEO	8
+
+#define IMX8MM_ICM_GPU2D	9
+#define IMX8MM_ICM_GPU3D	10
+#define IMX8MM_ICN_GPU		11
+
+#define IMX8MM_ICM_CSI		12
+#define IMX8MM_ICM_LCDIF	13
+#define IMX8MM_ICN_MIPI		14
+
+#define IMX8MM_ICM_USB1		15
+#define IMX8MM_ICM_USB2		16
+#define IMX8MM_ICM_PCIE		17
+#define IMX8MM_ICN_HSIO		18
+
+#define IMX8MM_ICM_SDMA2	19
+#define IMX8MM_ICM_SDMA3	20
+#define IMX8MM_ICN_AUDIO	21
+
+#define IMX8MM_ICN_ENET		22
+#define IMX8MM_ICM_ENET		23
+
+#define IMX8MM_ICN_MAIN		24
+#define IMX8MM_ICM_NAND		25
+#define IMX8MM_ICM_SDMA1	26
+#define IMX8MM_ICM_USDHC1	27
+#define IMX8MM_ICM_USDHC2	28
+#define IMX8MM_ICM_USDHC3	29
+
+#endif /* __IMX8MM_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/8] interconnect: imx: Add platform driver for imx8mq

[Leonard Crestez]

Add a platform driver for the i.MX8MQ SoC describing bus topology,
based on internal documentation.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/interconnect/imx/Kconfig          |   4 +
 drivers/interconnect/imx/Makefile         |   2 +
 drivers/interconnect/imx/imx8mq.c         | 106 ++++++++++++++++++++++
 include/dt-bindings/interconnect/imx8mq.h |  48 ++++++++++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/interconnect/imx/imx8mq.c
 create mode 100644 include/dt-bindings/interconnect/imx8mq.h

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index 2cd4fad4976a..a2e6127f25b7 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -5,5 +5,9 @@ config INTERCONNECT_IMX
 	  Generic interconnect drivers for i.MX SOCs
 
 config INTERCONNECT_IMX8MM
 	tristate "i.MX8MM interconnect driver"
 	depends on INTERCONNECT_IMX
+
+config INTERCONNECT_IMX8MQ
+	tristate "i.MX8MQ interconnect driver"
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index c234e5d3dfd1..e7d7e029d6c7 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1,5 +1,7 @@
 imx-interconnect-objs			:= imx.o
 imx8mm-interconnect-objs       		:= imx8mm.o
+imx8mq-interconnect-objs       		:= imx8mq.o
 
 obj-$(CONFIG_INTERCONNECT_IMX)		+= imx-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MM)	+= imx8mm-interconnect.o
+obj-$(CONFIG_INTERCONNECT_IMX8MQ)	+= imx8mq-interconnect.o
diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
new file mode 100644
index 000000000000..8fb953a1b9ff
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mq.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, NXP
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mq.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mq_dram_adj = {
+	.bw_mul = 1,
+	.bw_div = 4,
+	.phandle_name = "fsl,ddrc",
+};
+
+static const struct imx_icc_node_adj_desc imx8mq_noc_adj = {
+	.bw_mul = 1,
+	.bw_div = 4,
+	.main_noc = true,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MQ_ICN_NOC, &imx8mq_noc_adj,
+			IMX8MQ_ICS_DRAM, IMX8MQ_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MQ_ICS_DRAM, &imx8mq_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MQ_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MQ_ICM_A53, IMX8MQ_ICN_NOC),
+
+	/* VPUMIX */
+	DEFINE_BUS_MASTER("VPU", IMX8MQ_ICM_VPU, IMX8MQ_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MQ_ICN_VIDEO, NULL, IMX8MQ_ICN_NOC),
+
+	/* GPUMIX */
+	DEFINE_BUS_MASTER("GPU", IMX8MQ_ICM_GPU, IMX8MQ_ICN_GPU),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MQ_ICN_GPU, NULL, IMX8MQ_ICN_NOC),
+
+	/* DISPMIX (only for DCSS) */
+	DEFINE_BUS_MASTER("DC", IMX8MQ_ICM_DCSS, IMX8MQ_ICN_DCSS),
+	DEFINE_BUS_INTERCONNECT("PL301_DC", IMX8MQ_ICN_DCSS, NULL, IMX8MQ_ICN_NOC),
+
+	/* USBMIX */
+	DEFINE_BUS_MASTER("USB1", IMX8MQ_ICM_USB1, IMX8MQ_ICN_USB),
+	DEFINE_BUS_MASTER("USB2", IMX8MQ_ICM_USB2, IMX8MQ_ICN_USB),
+	DEFINE_BUS_INTERCONNECT("PL301_USB", IMX8MQ_ICN_USB, NULL, IMX8MQ_ICN_NOC),
+
+	/* PL301_DISPLAY (IPs other than DCSS, inside SUPERMIX) */
+	DEFINE_BUS_MASTER("CSI1", IMX8MQ_ICM_CSI1, IMX8MQ_ICN_DISPLAY),
+	DEFINE_BUS_MASTER("CSI2", IMX8MQ_ICM_CSI2, IMX8MQ_ICN_DISPLAY),
+	DEFINE_BUS_MASTER("LCDIF", IMX8MQ_ICM_LCDIF, IMX8MQ_ICN_DISPLAY),
+	DEFINE_BUS_INTERCONNECT("PL301_DISPLAY", IMX8MQ_ICN_DISPLAY, NULL, IMX8MQ_ICN_MAIN),
+
+	/* AUDIO */
+	DEFINE_BUS_MASTER("SDMA2", IMX8MQ_ICM_SDMA2, IMX8MQ_ICN_AUDIO),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MQ_ICN_AUDIO, NULL, IMX8MQ_ICN_DISPLAY),
+
+	/* ENET */
+	DEFINE_BUS_MASTER("ENET", IMX8MQ_ICM_ENET, IMX8MQ_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MQ_ICN_ENET, NULL, IMX8MQ_ICN_MAIN),
+
+	/* OTHER */
+	DEFINE_BUS_MASTER("SDMA1", IMX8MQ_ICM_SDMA1, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_MASTER("NAND", IMX8MQ_ICM_NAND, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC1", IMX8MQ_ICM_USDHC1, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC2", IMX8MQ_ICM_USDHC2, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_MASTER("PCIE1", IMX8MQ_ICM_PCIE1, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_MASTER("PCIE2", IMX8MQ_ICM_PCIE2, IMX8MQ_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MQ_ICN_MAIN, NULL,
+			IMX8MQ_ICN_NOC, IMX8MQ_ICS_OCRAM),
+};
+
+static int imx8mq_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mq_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static struct platform_driver imx8mq_icc_driver = {
+	.probe = imx8mq_icc_probe,
+	.remove = imx8mq_icc_remove,
+	.driver = {
+		.name = "imx8mq-interconnect",
+	},
+};
+
+module_platform_driver(imx8mq_icc_driver);
+MODULE_ALIAS("platform:imx8mq-interconnect");
+MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/interconnect/imx8mq.h b/include/dt-bindings/interconnect/imx8mq.h
new file mode 100644
index 000000000000..94701b19f35a
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mq.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, NXP
+ */
+
+#ifndef __IMX8MQ_ICM_INTERCONNECT_IDS_H
+#define __IMX8MQ_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MQ_ICN_NOC		1
+#define IMX8MQ_ICS_DRAM		2
+#define IMX8MQ_ICS_OCRAM	3
+#define IMX8MQ_ICM_A53		4
+
+#define IMX8MQ_ICM_VPU		5
+#define IMX8MQ_ICN_VIDEO	6
+
+#define IMX8MQ_ICM_GPU		7
+#define IMX8MQ_ICN_GPU		8
+
+#define IMX8MQ_ICM_DCSS		9
+#define IMX8MQ_ICN_DCSS		10
+
+#define IMX8MQ_ICM_USB1		11
+#define IMX8MQ_ICM_USB2		12
+#define IMX8MQ_ICN_USB		13
+
+#define IMX8MQ_ICM_CSI1		14
+#define IMX8MQ_ICM_CSI2		15
+#define IMX8MQ_ICM_LCDIF	16
+#define IMX8MQ_ICN_DISPLAY	17
+
+#define IMX8MQ_ICM_SDMA2	18
+#define IMX8MQ_ICN_AUDIO	19
+
+#define IMX8MQ_ICN_ENET		20
+#define IMX8MQ_ICM_ENET		21
+
+#define IMX8MQ_ICM_SDMA1	22
+#define IMX8MQ_ICM_NAND		23
+#define IMX8MQ_ICM_USDHC1	24
+#define IMX8MQ_ICM_USDHC2	25
+#define IMX8MQ_ICM_PCIE1	26
+#define IMX8MQ_ICM_PCIE2	27
+#define IMX8MQ_ICN_MAIN		28
+
+#endif /* __IMX8MQ_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/8] interconnect: imx: Add platform driver for imx8mn

[Leonard Crestez]

Add a platform driver for the i.MX8MN SoC describing bus topology, based
on internal documentation.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/interconnect/imx/Kconfig          |  4 +
 drivers/interconnect/imx/Makefile         |  2 +
 drivers/interconnect/imx/imx8mn.c         | 97 +++++++++++++++++++++++
 include/dt-bindings/interconnect/imx8mn.h | 41 ++++++++++
 4 files changed, 144 insertions(+)
 create mode 100644 drivers/interconnect/imx/imx8mn.c
 create mode 100644 include/dt-bindings/interconnect/imx8mn.h

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index a2e6127f25b7..be2928362bb7 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -6,8 +6,12 @@ config INTERCONNECT_IMX
 
 config INTERCONNECT_IMX8MM
 	tristate "i.MX8MM interconnect driver"
 	depends on INTERCONNECT_IMX
 
+config INTERCONNECT_IMX8MN
+	tristate "i.MX8MN interconnect driver"
+	depends on INTERCONNECT_IMX
+
 config INTERCONNECT_IMX8MQ
 	tristate "i.MX8MQ interconnect driver"
 	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index e7d7e029d6c7..21fd5233754f 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1,7 +1,9 @@
 imx-interconnect-objs			:= imx.o
 imx8mm-interconnect-objs       		:= imx8mm.o
 imx8mq-interconnect-objs       		:= imx8mq.o
+imx8mn-interconnect-objs       		:= imx8mn.o
 
 obj-$(CONFIG_INTERCONNECT_IMX)		+= imx-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MM)	+= imx8mm-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MQ)	+= imx8mq-interconnect.o
+obj-$(CONFIG_INTERCONNECT_IMX8MN)	+= imx8mn-interconnect.o
diff --git a/drivers/interconnect/imx/imx8mn.c b/drivers/interconnect/imx/imx8mn.c
new file mode 100644
index 000000000000..e57d77cd36fd
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mn.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, NXP
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mn.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mn_dram_adj = {
+	.bw_mul = 1,
+	.bw_div = 4,
+	.phandle_name = "fsl,ddrc",
+};
+
+static const struct imx_icc_node_adj_desc imx8mn_noc_adj = {
+	.bw_mul = 1,
+	.bw_div = 4,
+	.main_noc = true,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MN_ICN_NOC, &imx8mn_noc_adj,
+			IMX8MN_ICS_DRAM, IMX8MN_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MN_ICS_DRAM, &imx8mn_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MN_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MN_ICM_A53, IMX8MN_ICN_NOC),
+
+	/* GPUMIX */
+	DEFINE_BUS_MASTER("GPU", IMX8MN_ICM_GPU, IMX8MN_ICN_GPU),
+	DEFINE_BUS_INTERCONNECT("PL301_GPU", IMX8MN_ICN_GPU, NULL, IMX8MN_ICN_NOC),
+
+	/* DISPLAYMIX */
+	DEFINE_BUS_MASTER("CSI1", IMX8MN_ICM_CSI1, IMX8MN_ICN_MIPI),
+	DEFINE_BUS_MASTER("CSI2", IMX8MN_ICM_CSI2, IMX8MN_ICN_MIPI),
+	DEFINE_BUS_MASTER("ISI", IMX8MN_ICM_ISI, IMX8MN_ICN_MIPI),
+	DEFINE_BUS_MASTER("LCDIF", IMX8MN_ICM_LCDIF, IMX8MN_ICN_MIPI),
+	DEFINE_BUS_INTERCONNECT("PL301_MIPI", IMX8MN_ICN_MIPI, NULL, IMX8MN_ICN_NOC),
+
+	/* USB goes straight to NOC */
+	DEFINE_BUS_MASTER("USB", IMX8MN_ICM_USB, IMX8MN_ICN_NOC),
+
+	/* Audio */
+	DEFINE_BUS_MASTER("SDMA2", IMX8MN_ICM_SDMA2, IMX8MN_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA3", IMX8MN_ICM_SDMA3, IMX8MN_ICN_AUDIO),
+	DEFINE_BUS_INTERCONNECT("PL301_AUDIO", IMX8MN_ICN_AUDIO, NULL, IMX8MN_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MN_ICM_ENET, IMX8MN_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MN_ICN_ENET, NULL, IMX8MN_ICN_MAIN),
+
+	/* Other */
+	DEFINE_BUS_MASTER("SDMA1", IMX8MN_ICM_SDMA1, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_MASTER("NAND", IMX8MN_ICM_NAND, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC1", IMX8MN_ICM_USDHC1, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC2", IMX8MN_ICM_USDHC2, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_MASTER("USDHC3", IMX8MN_ICM_USDHC3, IMX8MN_ICN_MAIN),
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MN_ICN_MAIN, NULL,
+			IMX8MN_ICN_NOC, IMX8MN_ICS_OCRAM),
+};
+
+static int imx8mn_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mn_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static struct platform_driver imx8mn_icc_driver = {
+	.probe = imx8mn_icc_probe,
+	.remove = imx8mn_icc_remove,
+	.driver = {
+		.name = "imx8mn-interconnect",
+	},
+};
+
+module_platform_driver(imx8mn_icc_driver);
+MODULE_ALIAS("platform:imx8mn-interconnect");
+MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/interconnect/imx8mn.h b/include/dt-bindings/interconnect/imx8mn.h
new file mode 100644
index 000000000000..03d099dd71f8
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mn.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, NXP
+ */
+
+#ifndef __IMX8MN_ICM_INTERCONNECT_IDS_H
+#define __IMX8MN_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MN_ICN_NOC		1
+#define IMX8MN_ICS_DRAM		2
+#define IMX8MN_ICS_OCRAM	3
+#define IMX8MN_ICM_A53		4
+
+#define IMX8MN_ICM_GPU		5
+#define IMX8MN_ICN_GPU		6
+
+#define IMX8MN_ICM_CSI1		7
+#define IMX8MN_ICM_CSI2		8
+#define IMX8MN_ICM_ISI		9
+#define IMX8MN_ICM_LCDIF	10
+#define IMX8MN_ICN_MIPI		11
+
+#define IMX8MN_ICM_USB		12
+
+#define IMX8MN_ICM_SDMA2	13
+#define IMX8MN_ICM_SDMA3	14
+#define IMX8MN_ICN_AUDIO	15
+
+#define IMX8MN_ICN_ENET		16
+#define IMX8MN_ICM_ENET		17
+
+#define IMX8MN_ICM_NAND		18
+#define IMX8MN_ICM_SDMA1	19
+#define IMX8MN_ICM_USDHC1	20
+#define IMX8MN_ICM_USDHC2	21
+#define IMX8MN_ICM_USDHC3	22
+#define IMX8MN_ICN_MAIN		23
+
+#endif /* __IMX8MN_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/8] arm64: dts: imx8m: Add NOC nodes

[Leonard Crestez]

Add nodes for the main interconnect of the imx8m series chips.

These nodes are bound to by devfreq and interconnect drivers.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 24 +++++++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 24 +++++++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 24 +++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 175c28ae10cf..41047b6709b6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -6,10 +6,11 @@
 #include <dt-bindings/clock/imx8mm-clock.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/interconnect/imx8mm.h>
 
 #include "imx8mm-pinfunc.h"
 
 / {
 	interrupt-parent = <&gic>;
@@ -860,10 +861,33 @@
 				status = "disabled";
 			};
 
 		};
 
+		noc: interconnect@32700000 {
+			compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MM_CLK_NOC>;
+			fsl,ddrc = <&ddrc>;
+			#interconnect-cells = <1>;
+			operating-points-v2 = <&noc_opp_table>;
+
+			noc_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-150M {
+					opp-hz = /bits/ 64 <150000000>;
+				};
+				opp-375M {
+					opp-hz = /bits/ 64 <375000000>;
+				};
+				opp-750M {
+					opp-hz = /bits/ 64 <750000000>;
+				};
+			};
+		};
+
 		aips4: bus@32c00000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			reg = <0x32df0000 0x10000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 88e7d74e077f..e8a55956813f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -6,10 +6,11 @@
 #include <dt-bindings/clock/imx8mn-clock.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/interconnect/imx8mn.h>
 
 #include "imx8mn-pinfunc.h"
 
 / {
 	interrupt-parent = <&gic>;
@@ -751,10 +752,33 @@
 				status = "disabled";
 			};
 
 		};
 
+		noc: interconnect@32700000 {
+			compatible = "fsl,imx8mn-noc", "fsl,imx8m-noc";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MN_CLK_NOC>;
+			fsl,ddrc = <&ddrc>;
+			#interconnect-cells = <1>;
+			operating-points-v2 = <&noc_opp_table>;
+
+			noc_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-100M {
+					opp-hz = /bits/ 64 <100000000>;
+				};
+				opp-600M {
+					opp-hz = /bits/ 64 <600000000>;
+				};
+				opp-800M {
+					opp-hz = /bits/ 64 <800000000>;
+				};
+			};
+		};
+
 		aips4: bus@32c00000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			reg = <0x32df0000 0x10000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index ea93bc4b7d7e..3a208feec74c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -9,10 +9,11 @@
 #include <dt-bindings/reset/imx8mq-reset.h>
 #include <dt-bindings/gpio/gpio.h>
 #include "dt-bindings/input/input.h"
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/interconnect/imx8mq.h>
 #include "imx8mq-pinfunc.h"
 
 / {
 	interrupt-parent = <&gpc>;
 
@@ -1026,10 +1027,33 @@
 				fsl,num-rx-queues = <3>;
 				status = "disabled";
 			};
 		};
 
+		noc: interconnect@32700000 {
+			compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MQ_CLK_NOC>;
+			fsl,ddrc = <&ddrc>;
+			#interconnect-cells = <1>;
+			operating-points-v2 = <&noc_opp_table>;
+
+			noc_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-133M {
+					opp-hz = /bits/ 64 <133333333>;
+				};
+				opp-400M {
+					opp-hz = /bits/ 64 <400000000>;
+				};
+				opp-800M {
+					opp-hz = /bits/ 64 <800000000>;
+				};
+			};
+		};
+
 		bus@32c00000 { /* AIPS4 */
 			compatible = "fsl,aips-bus", "simple-bus";
 			reg = <0x32df0000 0x10000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] interconnect: Add imx support via devfreq

[Martin Kepplinger]

On 26.03.20 03:16, Leonard Crestez wrote:
> This series adds interconnect scaling support for imx8m series chips. It uses a
> per-SOC interconnect provider layered on top of multiple instances of devfreq
> for scalable nodes along the interconnect.
> 
> Existing qcom interconnect providers mostly translate bandwidth requests into
> firmware calls but equivalent firmware on imx8m is much thinner. Scaling
> support for individual nodes is implemented as distinct devfreq drivers
> instead.
> 
> The imx interconnect provider doesn't communicate with devfreq directly
> but rather computes "minimum frequencies" for nodes along the path and
> creates dev_pm_qos requests.
> 
> Since there is no single devicetree node that can represent the
> "interconnect" the main NOC is picked as the "interconnect provider" and
> will probe the interconnect platform device if #interconnect-cells is
> present. This avoids introducing "virtual" devices but it means that DT
> bindings of main NOC includes properties for both devfreq and
> interconnect.
> 
> Only the ddrc and main noc are scalable right now but more can be added.
> 
> Also available on a github branch (with various unrelated changes):
> 	https://github.com/cdleonard/linux/tree/next
> Testing currently requires NXP branch of atf+uboot
> 
> Martin: I believe you should be able to use this to control DRAM
> frequency from video by just adding interconnect consumer code to
> nwl-dsi. Sample code:
> 	https://github.com/cdleonard/linux/commit/43772762aa5045f1ce5623740f9a4baef988d083
> 	https://github.com/cdleonard/linux/commit/7b601e981b1f517b5d98b43bde292972ded13086
> 

Thanks for updating this series Leonard! A few questions for my
understanding before trying to test:

Isn't the ddrc_opp_table missing in these additions to the DT? That's
what I want to scale after all.

If I want to keep calling the "request", now icc_set_bw(), in nwl-dsi:
I'd add an "interconnects" property to the node, but what would be my
interconnect master? i.e.: interconnects = <&noc master? &noc
IMX8MQ_ICS_DRAM>; At least it's not obvious to me from
interconnect/imx/imx8mq.c

the interconnect framework seems to be powerful indeed, but I still need
to fully wrap my head around it.

thanks for the help so far,

                               martin


> Changes since RFCv6:
> * Replace scalable-nodes stuff with just a fsl,ddrc property. Future
> scalable nodes can be added as additional phandles on the NOC
> * Allow building interconnect drivers as modules
> * Handle icc_provider_del errors in imx_icc_unregister (like EBUSY).
> * Rename imx-devfreq to imx-bus, similar to exynos-bus
> * Explain why imx bus clock enabling is not required
> * All dependencies accepted (some time ago).
> Link: https://patchwork.kernel.org/cover/11244421/
> 
> Changes since RFCv5:
> * Replace scanning for interconnect-node-id with explicit
> scalable-nodes/scalable-node-ids property on NoC.
> * Now passes make `dtbs_check`
> * Remove struct imx_icc_provider
> * Switch to of_icc_xlate_onecell
> * Use of_find_device_by_node to fetch QoS target, this causes fewer probe
> deferrals, removes dependency on devfreq API and even allows reloading ddrc
> module at runtime
> * Add imx_icc_node_destroy helper
> * Remove 0/1 on DEFINE_BUS_SLAVE/MASTER which created spurious links
> Link: https://patchwork.kernel.org/cover/11222015/
> 
> Changes since RFCv4:
> * Drop icc proxy nonsense
> * Make devfreq driver for NOC probe the ICC driver if
> #interconnect-cells is present
> * Move NOC support to interconnect series and rename the node in DT
> * Add support for all chips at once, differences are not intereseting
> and there is more community interest for 8mq than 8mm.
> Link: https://patchwork.kernel.org/cover/11111865/
> 
> Changes since RFCv3:
> * Remove the virtual "icc" node and add devfreq nodes as proxy providers
> * Fix build on 32-bit arm (reported by kbuilt test robot)
> * Remove ARCH_MXC_ARM64 (never existed in upstream)
> * Remove _numlinks, calculate instead
> * Replace __BUSFREQ_H header guard
> * Improve commit message and comment spelling
> * Fix checkpatch issues
> Link to RFCv3: https://patchwork.kernel.org/cover/11078671/
> 
> Changes since RFCv2 and initial work by Alexandre Bailon:
> * Relying on devfreq and dev_pm_qos instead of CLK
> * No more "platform opp" stuff
> * No more special suspend handling: use suspend-opp on devfreq instead
> * Replace all mentions of "busfreq" with "interconnect"
> Link to v2: https://patchwork.kernel.org/cover/11021563/
> 
> Leonard Crestez (8):
>   dt-bindings: interconnect: Add bindings for imx8m noc
>   PM / devfreq: Add generic imx bus scaling driver
>   PM / devfreq: imx: Register interconnect device
>   interconnect: Add imx core driver
>   interconnect: imx: Add platform driver for imx8mm
>   interconnect: imx: Add platform driver for imx8mq
>   interconnect: imx: Add platform driver for imx8mn
>   arm64: dts: imx8m: Add NOC nodes
> 
>  .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  24 ++
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi     |  24 ++
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi     |  24 ++
>  drivers/devfreq/Kconfig                       |   9 +
>  drivers/devfreq/Makefile                      |   1 +
>  drivers/devfreq/imx-bus.c                     | 181 +++++++++++
>  drivers/interconnect/Kconfig                  |   1 +
>  drivers/interconnect/Makefile                 |   1 +
>  drivers/interconnect/imx/Kconfig              |  17 +
>  drivers/interconnect/imx/Makefile             |   9 +
>  drivers/interconnect/imx/imx.c                | 298 ++++++++++++++++++
>  drivers/interconnect/imx/imx.h                |  62 ++++
>  drivers/interconnect/imx/imx8mm.c             | 108 +++++++
>  drivers/interconnect/imx/imx8mn.c             |  97 ++++++
>  drivers/interconnect/imx/imx8mq.c             | 106 +++++++
>  include/dt-bindings/interconnect/imx8mm.h     |  49 +++
>  include/dt-bindings/interconnect/imx8mn.h     |  41 +++
>  include/dt-bindings/interconnect/imx8mq.h     |  48 +++
>  19 files changed, 1238 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>  create mode 100644 drivers/devfreq/imx-bus.c
>  create mode 100644 drivers/interconnect/imx/Kconfig
>  create mode 100644 drivers/interconnect/imx/Makefile
>  create mode 100644 drivers/interconnect/imx/imx.c
>  create mode 100644 drivers/interconnect/imx/imx.h
>  create mode 100644 drivers/interconnect/imx/imx8mm.c
>  create mode 100644 drivers/interconnect/imx/imx8mn.c
>  create mode 100644 drivers/interconnect/imx/imx8mq.c
>  create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>  create mode 100644 include/dt-bindings/interconnect/imx8mn.h
>  create mode 100644 include/dt-bindings/interconnect/imx8mq.h
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] interconnect: Add imx support via devfreq

[Guido Günther]

Hi Martin,
On Thu, Mar 26, 2020 at 02:55:27PM +0100, Martin Kepplinger wrote:
> On 26.03.20 03:16, Leonard Crestez wrote:
> > This series adds interconnect scaling support for imx8m series chips. It uses a
> > per-SOC interconnect provider layered on top of multiple instances of devfreq
> > for scalable nodes along the interconnect.
> > 
> > Existing qcom interconnect providers mostly translate bandwidth requests into
> > firmware calls but equivalent firmware on imx8m is much thinner. Scaling
> > support for individual nodes is implemented as distinct devfreq drivers
> > instead.
> > 
> > The imx interconnect provider doesn't communicate with devfreq directly
> > but rather computes "minimum frequencies" for nodes along the path and
> > creates dev_pm_qos requests.
> > 
> > Since there is no single devicetree node that can represent the
> > "interconnect" the main NOC is picked as the "interconnect provider" and
> > will probe the interconnect platform device if #interconnect-cells is
> > present. This avoids introducing "virtual" devices but it means that DT
> > bindings of main NOC includes properties for both devfreq and
> > interconnect.
> > 
> > Only the ddrc and main noc are scalable right now but more can be added.
> > 
> > Also available on a github branch (with various unrelated changes):
> > 	https://github.com/cdleonard/linux/tree/next
> > Testing currently requires NXP branch of atf+uboot
> > 
> > Martin: I believe you should be able to use this to control DRAM
> > frequency from video by just adding interconnect consumer code to
> > nwl-dsi. Sample code:
> > 	https://github.com/cdleonard/linux/commit/43772762aa5045f1ce5623740f9a4baef988d083
> > 	https://github.com/cdleonard/linux/commit/7b601e981b1f517b5d98b43bde292972ded13086
> > 
> 
> Thanks for updating this series Leonard! A few questions for my
> understanding before trying to test:
> 
> Isn't the ddrc_opp_table missing in these additions to the DT? That's
> what I want to scale after all.
> 
> If I want to keep calling the "request", now icc_set_bw(), in nwl-dsi:
> I'd add an "interconnects" property to the node, but what would be my
> interconnect master? i.e.: interconnects = <&noc master? &noc
> IMX8MQ_ICS_DRAM>; At least it's not obvious to me from
> interconnect/imx/imx8mq.c

The NWL DSI host controller is fed by DCSS or mxsfb so any bandwidth
requirements should (as far as I understand things) go into the display
controller driver since that's what fetches from RAM.
Cheers,
 -- Guido

> 
> the interconnect framework seems to be powerful indeed, but I still need
> to fully wrap my head around it.
> 
> thanks for the help so far,
> 
>                                martin
> 
> 
> > Changes since RFCv6:
> > * Replace scalable-nodes stuff with just a fsl,ddrc property. Future
> > scalable nodes can be added as additional phandles on the NOC
> > * Allow building interconnect drivers as modules
> > * Handle icc_provider_del errors in imx_icc_unregister (like EBUSY).
> > * Rename imx-devfreq to imx-bus, similar to exynos-bus
> > * Explain why imx bus clock enabling is not required
> > * All dependencies accepted (some time ago).
> > Link: https://patchwork.kernel.org/cover/11244421/
> > 
> > Changes since RFCv5:
> > * Replace scanning for interconnect-node-id with explicit
> > scalable-nodes/scalable-node-ids property on NoC.
> > * Now passes make `dtbs_check`
> > * Remove struct imx_icc_provider
> > * Switch to of_icc_xlate_onecell
> > * Use of_find_device_by_node to fetch QoS target, this causes fewer probe
> > deferrals, removes dependency on devfreq API and even allows reloading ddrc
> > module at runtime
> > * Add imx_icc_node_destroy helper
> > * Remove 0/1 on DEFINE_BUS_SLAVE/MASTER which created spurious links
> > Link: https://patchwork.kernel.org/cover/11222015/
> > 
> > Changes since RFCv4:
> > * Drop icc proxy nonsense
> > * Make devfreq driver for NOC probe the ICC driver if
> > #interconnect-cells is present
> > * Move NOC support to interconnect series and rename the node in DT
> > * Add support for all chips at once, differences are not intereseting
> > and there is more community interest for 8mq than 8mm.
> > Link: https://patchwork.kernel.org/cover/11111865/
> > 
> > Changes since RFCv3:
> > * Remove the virtual "icc" node and add devfreq nodes as proxy providers
> > * Fix build on 32-bit arm (reported by kbuilt test robot)
> > * Remove ARCH_MXC_ARM64 (never existed in upstream)
> > * Remove _numlinks, calculate instead
> > * Replace __BUSFREQ_H header guard
> > * Improve commit message and comment spelling
> > * Fix checkpatch issues
> > Link to RFCv3: https://patchwork.kernel.org/cover/11078671/
> > 
> > Changes since RFCv2 and initial work by Alexandre Bailon:
> > * Relying on devfreq and dev_pm_qos instead of CLK
> > * No more "platform opp" stuff
> > * No more special suspend handling: use suspend-opp on devfreq instead
> > * Replace all mentions of "busfreq" with "interconnect"
> > Link to v2: https://patchwork.kernel.org/cover/11021563/
> > 
> > Leonard Crestez (8):
> >   dt-bindings: interconnect: Add bindings for imx8m noc
> >   PM / devfreq: Add generic imx bus scaling driver
> >   PM / devfreq: imx: Register interconnect device
> >   interconnect: Add imx core driver
> >   interconnect: imx: Add platform driver for imx8mm
> >   interconnect: imx: Add platform driver for imx8mq
> >   interconnect: imx: Add platform driver for imx8mn
> >   arm64: dts: imx8m: Add NOC nodes
> > 
> >  .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++
> >  arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  24 ++
> >  arch/arm64/boot/dts/freescale/imx8mn.dtsi     |  24 ++
> >  arch/arm64/boot/dts/freescale/imx8mq.dtsi     |  24 ++
> >  drivers/devfreq/Kconfig                       |   9 +
> >  drivers/devfreq/Makefile                      |   1 +
> >  drivers/devfreq/imx-bus.c                     | 181 +++++++++++
> >  drivers/interconnect/Kconfig                  |   1 +
> >  drivers/interconnect/Makefile                 |   1 +
> >  drivers/interconnect/imx/Kconfig              |  17 +
> >  drivers/interconnect/imx/Makefile             |   9 +
> >  drivers/interconnect/imx/imx.c                | 298 ++++++++++++++++++
> >  drivers/interconnect/imx/imx.h                |  62 ++++
> >  drivers/interconnect/imx/imx8mm.c             | 108 +++++++
> >  drivers/interconnect/imx/imx8mn.c             |  97 ++++++
> >  drivers/interconnect/imx/imx8mq.c             | 106 +++++++
> >  include/dt-bindings/interconnect/imx8mm.h     |  49 +++
> >  include/dt-bindings/interconnect/imx8mn.h     |  41 +++
> >  include/dt-bindings/interconnect/imx8mq.h     |  48 +++
> >  19 files changed, 1238 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> >  create mode 100644 drivers/devfreq/imx-bus.c
> >  create mode 100644 drivers/interconnect/imx/Kconfig
> >  create mode 100644 drivers/interconnect/imx/Makefile
> >  create mode 100644 drivers/interconnect/imx/imx.c
> >  create mode 100644 drivers/interconnect/imx/imx.h
> >  create mode 100644 drivers/interconnect/imx/imx8mm.c
> >  create mode 100644 drivers/interconnect/imx/imx8mn.c
> >  create mode 100644 drivers/interconnect/imx/imx8mq.c
> >  create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> >  create mode 100644 include/dt-bindings/interconnect/imx8mn.h
> >  create mode 100644 include/dt-bindings/interconnect/imx8mq.h
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] interconnect: Add imx support via devfreq

[Martin Kepplinger]

On 27.03.20 12:36, Guido Günther wrote:
> Hi Martin,
> On Thu, Mar 26, 2020 at 02:55:27PM +0100, Martin Kepplinger wrote:
>> On 26.03.20 03:16, Leonard Crestez wrote:
>>> This series adds interconnect scaling support for imx8m series chips. It uses a
>>> per-SOC interconnect provider layered on top of multiple instances of devfreq
>>> for scalable nodes along the interconnect.
>>>
>>> Existing qcom interconnect providers mostly translate bandwidth requests into
>>> firmware calls but equivalent firmware on imx8m is much thinner. Scaling
>>> support for individual nodes is implemented as distinct devfreq drivers
>>> instead.
>>>
>>> The imx interconnect provider doesn't communicate with devfreq directly
>>> but rather computes "minimum frequencies" for nodes along the path and
>>> creates dev_pm_qos requests.
>>>
>>> Since there is no single devicetree node that can represent the
>>> "interconnect" the main NOC is picked as the "interconnect provider" and
>>> will probe the interconnect platform device if #interconnect-cells is
>>> present. This avoids introducing "virtual" devices but it means that DT
>>> bindings of main NOC includes properties for both devfreq and
>>> interconnect.
>>>
>>> Only the ddrc and main noc are scalable right now but more can be added.
>>>
>>> Also available on a github branch (with various unrelated changes):
>>> 	https://github.com/cdleonard/linux/tree/next
>>> Testing currently requires NXP branch of atf+uboot
>>>
>>> Martin: I believe you should be able to use this to control DRAM
>>> frequency from video by just adding interconnect consumer code to
>>> nwl-dsi. Sample code:
>>> 	https://github.com/cdleonard/linux/commit/43772762aa5045f1ce5623740f9a4baef988d083
>>> 	https://github.com/cdleonard/linux/commit/7b601e981b1f517b5d98b43bde292972ded13086
>>>
>>
>> Thanks for updating this series Leonard! A few questions for my
>> understanding before trying to test:
>>
>> Isn't the ddrc_opp_table missing in these additions to the DT? That's
>> what I want to scale after all.
>>
>> If I want to keep calling the "request", now icc_set_bw(), in nwl-dsi:
>> I'd add an "interconnects" property to the node, but what would be my
>> interconnect master? i.e.: interconnects = <&noc master? &noc
>> IMX8MQ_ICS_DRAM>; At least it's not obvious to me from
>> interconnect/imx/imx8mq.c
> 
> The NWL DSI host controller is fed by DCSS or mxsfb so any bandwidth
> requirements should (as far as I understand things) go into the display
> controller driver since that's what fetches from RAM.
> Cheers,
>  -- Guido
> 

Hi,

Thanks a lot Leonard and Guido! Here's the tree I'm running, which is
your patches based on Linus' tree, with icc request in mxsfb:

https://source.puri.sm/martin.kepplinger/linux-next/commits/5.6-rc7/librem5__integration_devfreq1

The path from icc via pm_qos to devfreq does work (which is great) -
however only after setting the minimum frequencies via a governor - I
set the "powersave" governor.

After that, frequencies are both set to high / low correctly.

My impression was that I should be able to use the "passive" governor (a
passive devfreq device?). What am I missing with using devfreq
correctly? Or do I already?

other than the above uncertainty:

Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm>

thanks so far!

                          martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] dt-bindings: interconnect: Add bindings for imx8m noc

[Rob Herring]

On Thu, 26 Mar 2020 04:16:13 +0200, Leonard Crestez wrote:
> Add initial dt bindings for the interconnects inside i.MX chips.
> Multiple external IPs are involved but SOC integration means the
> software controllable interfaces are very similar.
> 
> Main NOC node acts as interconnect provider if #interconnect-cells is
> present. Currently there is a single imx interconnect provider for the
> whole SOC.
> 
> Other pieces of scalable interconnects can be present, each with their
> own OPP table.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++++++++++++
>  1 file changed, 138 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml:  while parsing a block mapping
  in "<unicode string>", line 4, column 1
did not find expected key
  in "<unicode string>", line 79, column 9
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1261720

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] interconnect: Add imx support via devfreq

[Leonard Crestez]

On 2020-03-30 11:52 AM, Martin Kepplinger wrote:
> On 27.03.20 12:36, Guido Günther wrote:
>> Hi Martin,
>> On Thu, Mar 26, 2020 at 02:55:27PM +0100, Martin Kepplinger wrote:
>>> On 26.03.20 03:16, Leonard Crestez wrote:
>>>> This series adds interconnect scaling support for imx8m series chips. It uses a
>>>> per-SOC interconnect provider layered on top of multiple instances of devfreq
>>>> for scalable nodes along the interconnect.
>>>>
>>>> Existing qcom interconnect providers mostly translate bandwidth requests into
>>>> firmware calls but equivalent firmware on imx8m is much thinner. Scaling
>>>> support for individual nodes is implemented as distinct devfreq drivers
>>>> instead.
>>>>
>>>> The imx interconnect provider doesn't communicate with devfreq directly
>>>> but rather computes "minimum frequencies" for nodes along the path and
>>>> creates dev_pm_qos requests.
>>>>
>>>> Since there is no single devicetree node that can represent the
>>>> "interconnect" the main NOC is picked as the "interconnect provider" and
>>>> will probe the interconnect platform device if #interconnect-cells is
>>>> present. This avoids introducing "virtual" devices but it means that DT
>>>> bindings of main NOC includes properties for both devfreq and
>>>> interconnect.
>>>>
>>>> Only the ddrc and main noc are scalable right now but more can be added.
>>>>
>>>> Also available on a github branch (with various unrelated changes):
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Ftree%2Fnext&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=A0l5FBF%2BT3k7H5HMtRMfX8WfVSqQm9jijgr8aexCoUA%3D&reserved=0
>>>> Testing currently requires NXP branch of atf+uboot
>>>>
>>>> Martin: I believe you should be able to use this to control DRAM
>>>> frequency from video by just adding interconnect consumer code to
>>>> nwl-dsi. Sample code:
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Fcommit%2F43772762aa5045f1ce5623740f9a4baef988d083&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=%2B%2BGWQTaQLLk98yFRHJ0o3Sks9DHGuKv7twBvn89f1Tg%3D&reserved=0
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Fcommit%2F7b601e981b1f517b5d98b43bde292972ded13086&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=Jy2%2FI3CE1H3ilmLvZAVhjlPHO3KRNK6%2F9dX%2BS124ROA%3D&reserved=0
>>>>
>>>
>>> Thanks for updating this series Leonard! A few questions for my
>>> understanding before trying to test:
>>>
>>> Isn't the ddrc_opp_table missing in these additions to the DT? That's
>>> what I want to scale after all.
DDRC OPP table belongs in board file because it can vary with RAM type 
on boards.

>>> If I want to keep calling the "request", now icc_set_bw(), in nwl-dsi:
>>> I'd add an "interconnects" property to the node, but what would be my
>>> interconnect master? i.e.: interconnects = <&noc master? &noc
>>> IMX8MQ_ICS_DRAM>; At least it's not obvious to me from
>>> interconnect/imx/imx8mq.c
>>
>> The NWL DSI host controller is fed by DCSS or mxsfb so any bandwidth
>> requirements should (as far as I understand things) go into the display
>> controller driver since that's what fetches from RAM.
>> Cheers,
>>   -- Guido

This is correct.

> Thanks a lot Leonard and Guido! Here's the tree I'm running, which is
> your patches based on Linus' tree, with icc request in mxsfb:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2Fcommits%2F5.6-rc7%2Flibrem5__integration_devfreq1&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&amp;sdata=FM8JoOuNa2gg09XVJ%2FLTLqK9rlL4hAwig2iM9cMYFhg%3D&amp;reserved=0
> 
> The path from icc via pm_qos to devfreq does work (which is great) -
> however only after setting the minimum frequencies via a governor - I
> set the "powersave" governor.
> 
> After that, frequencies are both set to high / low correctly.
> 
> My impression was that I should be able to use the "passive" governor (a
> passive devfreq device?). What am I missing with using devfreq
> correctly? Or do I already?

The devfreq governor is something else: it's used to make one devfreq 
device match frequencies with another devfreq device.

Setting the default governor to "powersave" is correct and roughly 
matches behavior in NXP kernel.

> Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm>

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Chanwoo Choi]

Hi,

Looks good to me. I added the comments.
But, it need to add the dt binding documentation for this device.

The old email of Artur Świgoń is not used. On next time,
use following the new email address Because when I reply the mail,
always show the fail message from thunderbird due to the Artur's old email.
<a.swigon@partnet.samsung.com> -> <a.swigon@samsung.com>

On 3/26/20 11:16 AM, Leonard Crestez wrote:
> Add initial support for dynamic frequency switching on pieces of the imx
> interconnect fabric.
> 
> All this driver does is set a clk rate based on an opp table, it does
> not map register areas.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/Kconfig   |   9 +++
>  drivers/devfreq/Makefile  |   1 +
>  drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 152 insertions(+)
>  create mode 100644 drivers/devfreq/imx-bus.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 0b1df12e0f21..44d26192ddc4 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
>  	select DEVFREQ_GOV_USERSPACE
>  	help
>  	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>  	  adjusting DRAM frequency.
>  
> +config ARM_IMX_BUS_DEVFREQ
> +	tristate "i.MX Generic Bus DEVFREQ Driver"
> +	depends on ARCH_MXC || COMPILE_TEST
> +	select DEVFREQ_GOV_PASSIVE
> +	select DEVFREQ_GOV_USERSPACE

Maybe, you would update it by using passive governor?
But, in this version, it doesn't handle the any passive governor.

> +	help
> +	  This adds the generic DEVFREQ driver for i.MX interconnects. It
> +	  allows adjusting NIC/NOC frequency.
> +
>  config ARM_TEGRA_DEVFREQ
>  	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>  	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>  		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>  		ARCH_TEGRA_210_SOC || \
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 3eb4d5e6635c..3ca1ad0ecb97 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>  obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>  
>  # DEVFREQ Drivers
>  obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
> +obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o

The ARM_IMX_BUS_DEVFREQ config is under ARM_IMX8M_DDRC_DEVFREQ
and imx-bus.o is over imx8m-ddrc.o. Need to edit the sequence.

>  obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>  obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>  obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>  obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
>  
> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
> new file mode 100644
> index 000000000000..285e0f1ae6b1
> --- /dev/null
> +++ b/drivers/devfreq/imx-bus.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +struct imx_bus {
> +	struct devfreq_dev_profile profile;
> +	struct devfreq *devfreq;
> +	struct clk *clk;
> +	struct devfreq_passive_data passive_data;

This patch doesn't touch the passive_data.

> +};
> +
> +static int imx_bus_target(struct device *dev,
> +		unsigned long *freq, u32 flags)
> +{
> +	struct imx_bus *priv = dev_get_drvdata(dev);
> +	struct dev_pm_opp *new_opp;
> +	unsigned long new_freq;
> +	int ret;
> +
> +	new_opp = devfreq_recommended_opp(dev, freq, flags);
> +	if (IS_ERR(new_opp)) {
> +		ret = PTR_ERR(new_opp);
> +		dev_err(dev, "failed to get recommended opp: %d\n", ret);
> +		return ret;
> +	}
> +	new_freq = dev_pm_opp_get_freq(new_opp);

It doesn't need. Because the new frequency is stored to 'freq'
by calling devfreq_recommended_opp().

> +	dev_pm_opp_put(new_opp);
> +
> +	return clk_set_rate(priv->clk, new_freq);

nitpick. you can use dev_pm_opp_set_rate(). But, I'm not forcing to use it.

> +}
> +
> +static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> +	struct imx_bus *priv = dev_get_drvdata(dev);
> +
> +	*freq = clk_get_rate(priv->clk);
> +
> +	return 0;
> +}
> +
> +static int imx_bus_get_dev_status(struct device *dev,
> +		struct devfreq_dev_status *stat)
> +{
> +	struct imx_bus *priv = dev_get_drvdata(dev);
> +
> +	stat->busy_time = 0;
> +	stat->total_time = 0;
> +	stat->current_frequency = clk_get_rate(priv->clk);
> +
> +	return 0;
> +}
> +
> +static void imx_bus_exit(struct device *dev)
> +{
> +	dev_pm_opp_of_remove_table(dev);
> +}
> +
> +static int imx_bus_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx_bus *priv;
> +	const char *gov = DEVFREQ_GOV_USERSPACE;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Fetch the clock to adjust but don't explictly enable.

Need to fix typo.
s/explictly/explicitly

> +	 *
> +	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
> +	 * or off and some peripheral side-buses might be off unless enabled by
> +	 * drivers for devices on those specific buses.
> +	 *
> +	 * Rate adjustment on a disabled bus clock just takes effect later.
> +	 */
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		ret = PTR_ERR(priv->clk);
> +		dev_err(dev, "failed to fetch clk: %d\n", ret);
> +		return ret;
> +	}
> +	platform_set_drvdata(pdev, priv);
> +
> +	ret = dev_pm_opp_of_add_table(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get OPP table\n");
> +		return ret;
> +	}
> +
> +	priv->profile.polling_ms = 1000;
> +	priv->profile.target = imx_bus_target;
> +	priv->profile.get_dev_status = imx_bus_get_dev_status;
> +	priv->profile.exit = imx_bus_exit;
> +	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
> +
> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
> +						gov, NULL);
> +	if (IS_ERR(priv->devfreq)) {
> +		ret = PTR_ERR(priv->devfreq);
> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	dev_pm_opp_of_remove_table(dev);
> +	return ret;
> +}
> +
> +static const struct of_device_id imx_bus_of_match[] = {
> +	{ .compatible = "fsl,imx8m-noc", },
> +	{ .compatible = "fsl,imx8m-nic", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, imx_bus_of_match);
> +
> +static struct platform_driver imx_bus_platdrv = {
> +	.probe		= imx_bus_probe,
> +	.driver = {
> +		.name	= "imx-bus-devfreq",
> +		.of_match_table = of_match_ptr(imx_bus_of_match),
> +	},
> +};
> +module_platform_driver(imx_bus_platdrv);
> +
> +MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
> +MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
> +MODULE_LICENSE("GPL v2");
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/8] PM / devfreq: imx: Register interconnect device

[Chanwoo Choi]

Hi Leonard,

On 3/26/20 11:16 AM, Leonard Crestez wrote:
> There is no single device which can represent the imx interconnect.
> Instead of adding a virtual one just make the main &noc act as the
> global interconnect provider.
> 
> The imx interconnect provider driver will scale the NOC and DDRC based
> on bandwidth request. More scalable nodes can be added in the future,
> for example for audio/display/vpu/gpu NICs.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/imx-bus.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
> index 285e0f1ae6b1..e9b13e43bf0a 100644
> --- a/drivers/devfreq/imx-bus.c
> +++ b/drivers/devfreq/imx-bus.c
> @@ -15,10 +15,11 @@
>  struct imx_bus {
>  	struct devfreq_dev_profile profile;
>  	struct devfreq *devfreq;
>  	struct clk *clk;
>  	struct devfreq_passive_data passive_data;
> +	struct platform_device *icc_pdev;
>  };
>  
>  static int imx_bus_target(struct device *dev,
>  		unsigned long *freq, u32 flags)
>  {
> @@ -60,11 +61,42 @@ static int imx_bus_get_dev_status(struct device *dev,
>  	return 0;
>  }
>  
>  static void imx_bus_exit(struct device *dev)
>  {
> +	struct imx_bus *priv = dev_get_drvdata(dev);
> +
>  	dev_pm_opp_of_remove_table(dev);
> +	platform_device_unregister(priv->icc_pdev);
> +}
> +
> +/* imx_bus_init_icc() - register matching icc provider if required */

Better to add following comments without 'imx_bus_init_icc() -' comment.
/* Register matching icc provider if required */

> +static int imx_bus_init_icc(struct device *dev)
> +{
> +	struct imx_bus *priv = dev_get_drvdata(dev);
> +	const char *icc_driver_name;
> +
> +	if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
> +		return 0;
> +	if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX)) {
> +		dev_warn(dev, "imx interconnect drivers disabled\n");
> +		return 0;
> +	}
> +
> +	icc_driver_name = of_device_get_match_data(dev);
> +	if (!icc_driver_name)
> +		return 0;

Recommend to add the error or warning message.

> +
> +	priv->icc_pdev = platform_device_register_data(
> +			dev, icc_driver_name, -1, NULL, 0);
> +	if (IS_ERR(priv->icc_pdev)) {
> +		dev_err(dev, "failed to register icc provider %s: %ld\n",
> +				icc_driver_name, PTR_ERR(priv->devfreq));
> +		return PTR_ERR(priv->devfreq);
> +	}
> +
> +	return 0;
>  }
>  
>  static int imx_bus_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -112,18 +144,25 @@ static int imx_bus_probe(struct platform_device *pdev)
>  		ret = PTR_ERR(priv->devfreq);
>  		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>  		goto err;
>  	}
>  
> +	ret = imx_bus_init_icc(dev);
> +	if (ret)

Recommend to add the error message.

> +		goto err;
> +
>  	return 0;
>  
>  err:
>  	dev_pm_opp_of_remove_table(dev);
>  	return ret;
>  }
>  
>  static const struct of_device_id imx_bus_of_match[] = {
> +	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
> +	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
> +	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
>  	{ .compatible = "fsl,imx8m-noc", },
>  	{ .compatible = "fsl,imx8m-nic", },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, imx_bus_of_match);
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/8] PM / devfreq: imx: Register interconnect device

[Leonard Crestez]

On 2020-04-01 2:00 AM, Chanwoo Choi wrote:
> Hi Leonard,
> 
> On 3/26/20 11:16 AM, Leonard Crestez wrote:
>> There is no single device which can represent the imx interconnect.
>> Instead of adding a virtual one just make the main &noc act as the
>> global interconnect provider.
>>
>> The imx interconnect provider driver will scale the NOC and DDRC based
>> on bandwidth request. More scalable nodes can be added in the future,
>> for example for audio/display/vpu/gpu NICs.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> ---
>>   drivers/devfreq/imx-bus.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
>> index 285e0f1ae6b1..e9b13e43bf0a 100644
>> --- a/drivers/devfreq/imx-bus.c
>> +++ b/drivers/devfreq/imx-bus.c
>> @@ -15,10 +15,11 @@
>>   struct imx_bus {
>>   	struct devfreq_dev_profile profile;
>>   	struct devfreq *devfreq;
>>   	struct clk *clk;
>>   	struct devfreq_passive_data passive_data;
>> +	struct platform_device *icc_pdev;
>>   };
>>   
>>   static int imx_bus_target(struct device *dev,
>>   		unsigned long *freq, u32 flags)
>>   {
>> @@ -60,11 +61,42 @@ static int imx_bus_get_dev_status(struct device *dev,
>>   	return 0;
>>   }
>>   
>>   static void imx_bus_exit(struct device *dev)
>>   {
>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>> +
>>   	dev_pm_opp_of_remove_table(dev);
>> +	platform_device_unregister(priv->icc_pdev);
>> +}
>> +
>> +/* imx_bus_init_icc() - register matching icc provider if required */
> 
> Better to add following comments without 'imx_bus_init_icc() -' comment.
> /* Register matching icc provider if required */

This form looks like standard kernel-doc


>> +static int imx_bus_init_icc(struct device *dev)
>> +{
>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>> +	const char *icc_driver_name;
>> +
>> +	if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
>> +		return 0;
>> +	if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX)) {
>> +		dev_warn(dev, "imx interconnect drivers disabled\n");
>> +		return 0;
>> +	}
>> +
>> +	icc_driver_name = of_device_get_match_data(dev);
>> +	if (!icc_driver_name)
>> +		return 0;
> 
> Recommend to add the error or warning message.

Makes sense, imx8m-nic shouldn't have #interconnect-cells so if this 
condition is reach an error should be printed.

>> +
>> +	priv->icc_pdev = platform_device_register_data(
>> +			dev, icc_driver_name, -1, NULL, 0);
>> +	if (IS_ERR(priv->icc_pdev)) {
>> +		dev_err(dev, "failed to register icc provider %s: %ld\n",
>> +				icc_driver_name, PTR_ERR(priv->devfreq));
>> +		return PTR_ERR(priv->devfreq);
>> +	}
>> +
>> +	return 0;
>>   }
>>   
>>   static int imx_bus_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>> @@ -112,18 +144,25 @@ static int imx_bus_probe(struct platform_device *pdev)
>>   		ret = PTR_ERR(priv->devfreq);
>>   		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>>   		goto err;
>>   	}
>>   
>> +	ret = imx_bus_init_icc(dev);
>> +	if (ret)
> 
> Recommend to add the error message.

imx_bus_init_icc already prints errors. Printing again here would always 
result in a second message.

>> +		goto err;
>> +
>>   	return 0;
>>   
>>   err:
>>   	dev_pm_opp_of_remove_table(dev);
>>   	return ret;
>>   }
>>   
>>   static const struct of_device_id imx_bus_of_match[] = {
>> +	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
>> +	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
>> +	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
>>   	{ .compatible = "fsl,imx8m-noc", },
>>   	{ .compatible = "fsl,imx8m-nic", },
>>   	{ /* sentinel */ },
>>   };
>>   MODULE_DEVICE_TABLE(of, imx_bus_of_match);
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Leonard Crestez]

On 2020-04-01 1:55 AM, Chanwoo Choi wrote:
> Hi,
> 
> Looks good to me. I added the comments.
> But, it need to add the dt binding documentation for this device.

DT bindings were included:

https://patchwork.kernel.org/patch/11458981/

> The old email of Artur Świgoń is not used. On next time,
> use following the new email address Because when I reply the mail,
> always show the fail message from thunderbird due to the Artur's old email.
> <a.swigon@partnet.samsung.com> -> <a.swigon@samsung.com>

Yeah, I received multiple bounces because of this.

> On 3/26/20 11:16 AM, Leonard Crestez wrote:
>> Add initial support for dynamic frequency switching on pieces of the imx
>> interconnect fabric.
>>
>> All this driver does is set a clk rate based on an opp table, it does
>> not map register areas.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> ---
>>   drivers/devfreq/Kconfig   |   9 +++
>>   drivers/devfreq/Makefile  |   1 +
>>   drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 152 insertions(+)
>>   create mode 100644 drivers/devfreq/imx-bus.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 0b1df12e0f21..44d26192ddc4 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
>>   	select DEVFREQ_GOV_USERSPACE
>>   	help
>>   	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>>   	  adjusting DRAM frequency.
>>   
>> +config ARM_IMX_BUS_DEVFREQ
>> +	tristate "i.MX Generic Bus DEVFREQ Driver"
>> +	depends on ARCH_MXC || COMPILE_TEST
>> +	select DEVFREQ_GOV_PASSIVE
>> +	select DEVFREQ_GOV_USERSPACE
> 
> Maybe, you would update it by using passive governor?
> But, in this version, it doesn't handle the any passive governor.

dropped

>> +	help
>> +	  This adds the generic DEVFREQ driver for i.MX interconnects. It
>> +	  allows adjusting NIC/NOC frequency.
>> +
>>   config ARM_TEGRA_DEVFREQ
>>   	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>>   	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>>   		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>>   		ARCH_TEGRA_210_SOC || \
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 3eb4d5e6635c..3ca1ad0ecb97 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>   obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>   obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>>   
>>   # DEVFREQ Drivers
>>   obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>> +obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
> 
> The ARM_IMX_BUS_DEVFREQ config is under ARM_IMX8M_DDRC_DEVFREQ
> and imx-bus.o is over imx8m-ddrc.o. Need to edit the sequence.

Reordered kconfig to match. 8M_DDRC sorts before _BUS alphabetically but 
it's pettier this way, and matches tegra.

>>   obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>>   obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>>   obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>>   obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
>>   
>> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
>> new file mode 100644
>> index 000000000000..285e0f1ae6b1
>> --- /dev/null
>> +++ b/drivers/devfreq/imx-bus.c
>> @@ -0,0 +1,142 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2019 NXP
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/pm_opp.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +struct imx_bus {
>> +	struct devfreq_dev_profile profile;
>> +	struct devfreq *devfreq;
>> +	struct clk *clk;
>> +	struct devfreq_passive_data passive_data;
> 
> This patch doesn't touch the passive_data.

dropped

>> +};
>> +
>> +static int imx_bus_target(struct device *dev,
>> +		unsigned long *freq, u32 flags)
>> +{
>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>> +	struct dev_pm_opp *new_opp;
>> +	unsigned long new_freq;
>> +	int ret;
>> +
>> +	new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +	if (IS_ERR(new_opp)) {
>> +		ret = PTR_ERR(new_opp);
>> +		dev_err(dev, "failed to get recommended opp: %d\n", ret);
>> +		return ret;
>> +	}
>> +	new_freq = dev_pm_opp_get_freq(new_opp);
> 
> It doesn't need. Because the new frequency is stored to 'freq'
> by calling devfreq_recommended_opp().

fixed

>> +	dev_pm_opp_put(new_opp);
>> +
>> +	return clk_set_rate(priv->clk, new_freq);
> 
> nitpick. you can use dev_pm_opp_set_rate(). But, I'm not forcing to use it.

Switched to dev_pm_opp_set_rate.

It might be interesting to add regulators control later, on some chips 
the main NOC can run at different voltages.

> 
>> +}
>> +
>> +static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
>> +{
>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>> +
>> +	*freq = clk_get_rate(priv->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_bus_get_dev_status(struct device *dev,
>> +		struct devfreq_dev_status *stat)
>> +{
>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>> +
>> +	stat->busy_time = 0;
>> +	stat->total_time = 0;
>> +	stat->current_frequency = clk_get_rate(priv->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static void imx_bus_exit(struct device *dev)
>> +{
>> +	dev_pm_opp_of_remove_table(dev);
>> +}
>> +
>> +static int imx_bus_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct imx_bus *priv;
>> +	const char *gov = DEVFREQ_GOV_USERSPACE;
>> +	int ret;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * Fetch the clock to adjust but don't explictly enable.
> 
> Need to fix typo.
> s/explictly/explicitly

fixed

>> +	 *
>> +	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
>> +	 * or off and some peripheral side-buses might be off unless enabled by
>> +	 * drivers for devices on those specific buses.
>> +	 *
>> +	 * Rate adjustment on a disabled bus clock just takes effect later.
>> +	 */
>> +	priv->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(priv->clk)) {
>> +		ret = PTR_ERR(priv->clk);
>> +		dev_err(dev, "failed to fetch clk: %d\n", ret);
>> +		return ret;
>> +	}
>> +	platform_set_drvdata(pdev, priv);
>> +
>> +	ret = dev_pm_opp_of_add_table(dev);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to get OPP table\n");
>> +		return ret;
>> +	}
>> +
>> +	priv->profile.polling_ms = 1000;
>> +	priv->profile.target = imx_bus_target;
>> +	priv->profile.get_dev_status = imx_bus_get_dev_status;
>> +	priv->profile.exit = imx_bus_exit;
>> +	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
>> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
>> +
>> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
>> +						gov, NULL);
>> +	if (IS_ERR(priv->devfreq)) {
>> +		ret = PTR_ERR(priv->devfreq);
>> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	return 0;
>> +
>> +err:
>> +	dev_pm_opp_of_remove_table(dev);
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id imx_bus_of_match[] = {
>> +	{ .compatible = "fsl,imx8m-noc", },
>> +	{ .compatible = "fsl,imx8m-nic", },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, imx_bus_of_match);
>> +
>> +static struct platform_driver imx_bus_platdrv = {
>> +	.probe		= imx_bus_probe,
>> +	.driver = {
>> +		.name	= "imx-bus-devfreq",
>> +		.of_match_table = of_match_ptr(imx_bus_of_match),
>> +	},
>> +};
>> +module_platform_driver(imx_bus_platdrv);
>> +
>> +MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
>> +MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
>> +MODULE_LICENSE("GPL v2");
>>
> 
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Chanwoo Choi]

On 4/1/20 11:20 PM, Leonard Crestez wrote:
> On 2020-04-01 1:55 AM, Chanwoo Choi wrote:
>> Hi,
>>
>> Looks good to me. I added the comments.
>> But, it need to add the dt binding documentation for this device.
> 
> DT bindings were included:
> 
> https://patchwork.kernel.org/patch/11458981/

The dt-binding document for this driver is required under
Documentation/devicetree/binding/devfreq.

It is difficult to catch where is the dt-binding document
for this driver for who don't know the detailed history
of this driver. I don't said that add the duplicate documentation
But, at least the some document have to point out the reference.

> 
>> The old email of Artur Świgoń is not used. On next time,
>> use following the new email address Because when I reply the mail,
>> always show the fail message from thunderbird due to the Artur's old email.
>> <a.swigon@partnet.samsung.com> -> <a.swigon@samsung.com>
> 
> Yeah, I received multiple bounces because of this.
> 
>> On 3/26/20 11:16 AM, Leonard Crestez wrote:
>>> Add initial support for dynamic frequency switching on pieces of the imx
>>> interconnect fabric.
>>>
>>> All this driver does is set a clk rate based on an opp table, it does
>>> not map register areas.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> ---
>>>   drivers/devfreq/Kconfig   |   9 +++
>>>   drivers/devfreq/Makefile  |   1 +
>>>   drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 152 insertions(+)
>>>   create mode 100644 drivers/devfreq/imx-bus.c
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index 0b1df12e0f21..44d26192ddc4 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
>>>   	select DEVFREQ_GOV_USERSPACE
>>>   	help
>>>   	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>>>   	  adjusting DRAM frequency.
>>>   
>>> +config ARM_IMX_BUS_DEVFREQ
>>> +	tristate "i.MX Generic Bus DEVFREQ Driver"
>>> +	depends on ARCH_MXC || COMPILE_TEST
>>> +	select DEVFREQ_GOV_PASSIVE
>>> +	select DEVFREQ_GOV_USERSPACE
>>
>> Maybe, you would update it by using passive governor?
>> But, in this version, it doesn't handle the any passive governor.
> 
> dropped
> 
>>> +	help
>>> +	  This adds the generic DEVFREQ driver for i.MX interconnects. It
>>> +	  allows adjusting NIC/NOC frequency.
>>> +
>>>   config ARM_TEGRA_DEVFREQ
>>>   	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>>>   	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>>>   		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>>>   		ARCH_TEGRA_210_SOC || \
>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> index 3eb4d5e6635c..3ca1ad0ecb97 100644
>>> --- a/drivers/devfreq/Makefile
>>> +++ b/drivers/devfreq/Makefile
>>> @@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>>   obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>>   obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>>>   
>>>   # DEVFREQ Drivers
>>>   obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>>> +obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
>>
>> The ARM_IMX_BUS_DEVFREQ config is under ARM_IMX8M_DDRC_DEVFREQ
>> and imx-bus.o is over imx8m-ddrc.o. Need to edit the sequence.
> 
> Reordered kconfig to match. 8M_DDRC sorts before _BUS alphabetically but 
> it's pettier this way, and matches tegra.
> 
>>>   obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>>>   obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>>>   obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>>>   obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
>>>   
>>> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
>>> new file mode 100644
>>> index 000000000000..285e0f1ae6b1
>>> --- /dev/null
>>> +++ b/drivers/devfreq/imx-bus.c
>>> @@ -0,0 +1,142 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright 2019 NXP
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/devfreq.h>
>>> +#include <linux/device.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/pm_opp.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +
>>> +struct imx_bus {
>>> +	struct devfreq_dev_profile profile;
>>> +	struct devfreq *devfreq;
>>> +	struct clk *clk;
>>> +	struct devfreq_passive_data passive_data;
>>
>> This patch doesn't touch the passive_data.
> 
> dropped
> 
>>> +};
>>> +
>>> +static int imx_bus_target(struct device *dev,
>>> +		unsigned long *freq, u32 flags)
>>> +{
>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>> +	struct dev_pm_opp *new_opp;
>>> +	unsigned long new_freq;
>>> +	int ret;
>>> +
>>> +	new_opp = devfreq_recommended_opp(dev, freq, flags);
>>> +	if (IS_ERR(new_opp)) {
>>> +		ret = PTR_ERR(new_opp);
>>> +		dev_err(dev, "failed to get recommended opp: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +	new_freq = dev_pm_opp_get_freq(new_opp);
>>
>> It doesn't need. Because the new frequency is stored to 'freq'
>> by calling devfreq_recommended_opp().
> 
> fixed
> 
>>> +	dev_pm_opp_put(new_opp);
>>> +
>>> +	return clk_set_rate(priv->clk, new_freq);
>>
>> nitpick. you can use dev_pm_opp_set_rate(). But, I'm not forcing to use it.
> 
> Switched to dev_pm_opp_set_rate.
> 
> It might be interesting to add regulators control later, on some chips 
> the main NOC can run at different voltages.
> 
>>
>>> +}
>>> +
>>> +static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
>>> +{
>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>> +
>>> +	*freq = clk_get_rate(priv->clk);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int imx_bus_get_dev_status(struct device *dev,
>>> +		struct devfreq_dev_status *stat)
>>> +{
>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>> +
>>> +	stat->busy_time = 0;
>>> +	stat->total_time = 0;
>>> +	stat->current_frequency = clk_get_rate(priv->clk);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void imx_bus_exit(struct device *dev)
>>> +{
>>> +	dev_pm_opp_of_remove_table(dev);
>>> +}
>>> +
>>> +static int imx_bus_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct imx_bus *priv;
>>> +	const char *gov = DEVFREQ_GOV_USERSPACE;
>>> +	int ret;
>>> +
>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> +	if (!priv)
>>> +		return -ENOMEM;
>>> +
>>> +	/*
>>> +	 * Fetch the clock to adjust but don't explictly enable.
>>
>> Need to fix typo.
>> s/explictly/explicitly
> 
> fixed
> 
>>> +	 *
>>> +	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
>>> +	 * or off and some peripheral side-buses might be off unless enabled by
>>> +	 * drivers for devices on those specific buses.
>>> +	 *
>>> +	 * Rate adjustment on a disabled bus clock just takes effect later.
>>> +	 */
>>> +	priv->clk = devm_clk_get(dev, NULL);
>>> +	if (IS_ERR(priv->clk)) {
>>> +		ret = PTR_ERR(priv->clk);
>>> +		dev_err(dev, "failed to fetch clk: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +	platform_set_drvdata(pdev, priv);
>>> +
>>> +	ret = dev_pm_opp_of_add_table(dev);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "failed to get OPP table\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	priv->profile.polling_ms = 1000;
>>> +	priv->profile.target = imx_bus_target;
>>> +	priv->profile.get_dev_status = imx_bus_get_dev_status;
>>> +	priv->profile.exit = imx_bus_exit;
>>> +	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
>>> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
>>> +
>>> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
>>> +						gov, NULL);
>>> +	if (IS_ERR(priv->devfreq)) {
>>> +		ret = PTR_ERR(priv->devfreq);
>>> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>>> +		goto err;
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +err:
>>> +	dev_pm_opp_of_remove_table(dev);
>>> +	return ret;
>>> +}
>>> +
>>> +static const struct of_device_id imx_bus_of_match[] = {
>>> +	{ .compatible = "fsl,imx8m-noc", },
>>> +	{ .compatible = "fsl,imx8m-nic", },
>>> +	{ /* sentinel */ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, imx_bus_of_match);
>>> +
>>> +static struct platform_driver imx_bus_platdrv = {
>>> +	.probe		= imx_bus_probe,
>>> +	.driver = {
>>> +		.name	= "imx-bus-devfreq",
>>> +		.of_match_table = of_match_ptr(imx_bus_of_match),
>>> +	},
>>> +};
>>> +module_platform_driver(imx_bus_platdrv);
>>> +
>>> +MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
>>> +MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>>
> 
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Leonard Crestez]

On 2020-04-02 1:48 AM, Chanwoo Choi wrote:
> On 4/1/20 11:20 PM, Leonard Crestez wrote:
>> On 2020-04-01 1:55 AM, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> Looks good to me. I added the comments.
>>> But, it need to add the dt binding documentation for this device.
>>
>> DT bindings were included:
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11458981%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C7381d117a4d1468cd2c608d7d68ecfac%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637213781167514099&sdata=Qu8x14cXnuxlwOT2SlUOf%2FLgCVWbnJRKA4TBjMIWQeA%3D&reserved=0
> 
> The dt-binding document for this driver is required under
> Documentation/devicetree/binding/devfreq.

Bindings for imx8m-ddrc were at one point posted for 
devicetree/bindings/devfreq but Rob Herring suggested to move them under 
"memory-controller" instead and I expect same logic makes sense here. 
Link to previous discussion:

https://patchwork.kernel.org/patch/11221919/

DT bindings should try to describe "hardware" rather than "drivers" and 
an "interconnect" is a class of hardware while "devfreq" isn't.

Not only that but the main noc has properties parsed by interconnect driver.

> It is difficult to catch where is the dt-binding document
> for this driver for who don't know the detailed history
> of this driver. I don't said that add the duplicate documentation
> But, at least the some document have to point out the reference.

What I usually do to find information about a device is grep for the 
compat string in the entire tree.

>>> The old email of Artur Świgoń is not used. On next time,
>>> use following the new email address Because when I reply the mail,
>>> always show the fail message from thunderbird due to the Artur's old email.
>>> <a.swigon@partnet.samsung.com> -> <a.swigon@samsung.com>
>>
>> Yeah, I received multiple bounces because of this.
>>
>>> On 3/26/20 11:16 AM, Leonard Crestez wrote:
>>>> Add initial support for dynamic frequency switching on pieces of the imx
>>>> interconnect fabric.
>>>>
>>>> All this driver does is set a clk rate based on an opp table, it does
>>>> not map register areas.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> ---
>>>>    drivers/devfreq/Kconfig   |   9 +++
>>>>    drivers/devfreq/Makefile  |   1 +
>>>>    drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 152 insertions(+)
>>>>    create mode 100644 drivers/devfreq/imx-bus.c
>>>>
>>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>>> index 0b1df12e0f21..44d26192ddc4 100644
>>>> --- a/drivers/devfreq/Kconfig
>>>> +++ b/drivers/devfreq/Kconfig
>>>> @@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
>>>>    	select DEVFREQ_GOV_USERSPACE
>>>>    	help
>>>>    	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>>>>    	  adjusting DRAM frequency.
>>>>    
>>>> +config ARM_IMX_BUS_DEVFREQ
>>>> +	tristate "i.MX Generic Bus DEVFREQ Driver"
>>>> +	depends on ARCH_MXC || COMPILE_TEST
>>>> +	select DEVFREQ_GOV_PASSIVE
>>>> +	select DEVFREQ_GOV_USERSPACE
>>>
>>> Maybe, you would update it by using passive governor?
>>> But, in this version, it doesn't handle the any passive governor.
>>
>> dropped
>>
>>>> +	help
>>>> +	  This adds the generic DEVFREQ driver for i.MX interconnects. It
>>>> +	  allows adjusting NIC/NOC frequency.
>>>> +
>>>>    config ARM_TEGRA_DEVFREQ
>>>>    	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>>>>    	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>>>>    		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>>>>    		ARCH_TEGRA_210_SOC || \
>>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>>> index 3eb4d5e6635c..3ca1ad0ecb97 100644
>>>> --- a/drivers/devfreq/Makefile
>>>> +++ b/drivers/devfreq/Makefile
>>>> @@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>>>    obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>>>    obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>>>>    
>>>>    # DEVFREQ Drivers
>>>>    obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>>>> +obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
>>>
>>> The ARM_IMX_BUS_DEVFREQ config is under ARM_IMX8M_DDRC_DEVFREQ
>>> and imx-bus.o is over imx8m-ddrc.o. Need to edit the sequence.
>>
>> Reordered kconfig to match. 8M_DDRC sorts before _BUS alphabetically but
>> it's pettier this way, and matches tegra.
>>
>>>>    obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>>>>    obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>>>>    obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>>>>    obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
>>>>    
>>>> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
>>>> new file mode 100644
>>>> index 000000000000..285e0f1ae6b1
>>>> --- /dev/null
>>>> +++ b/drivers/devfreq/imx-bus.c
>>>> @@ -0,0 +1,142 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright 2019 NXP
>>>> + */
>>>> +
>>>> +#include <linux/clk.h>
>>>> +#include <linux/devfreq.h>
>>>> +#include <linux/device.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/pm_opp.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +struct imx_bus {
>>>> +	struct devfreq_dev_profile profile;
>>>> +	struct devfreq *devfreq;
>>>> +	struct clk *clk;
>>>> +	struct devfreq_passive_data passive_data;
>>>
>>> This patch doesn't touch the passive_data.
>>
>> dropped
>>
>>>> +};
>>>> +
>>>> +static int imx_bus_target(struct device *dev,
>>>> +		unsigned long *freq, u32 flags)
>>>> +{
>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>> +	struct dev_pm_opp *new_opp;
>>>> +	unsigned long new_freq;
>>>> +	int ret;
>>>> +
>>>> +	new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>> +	if (IS_ERR(new_opp)) {
>>>> +		ret = PTR_ERR(new_opp);
>>>> +		dev_err(dev, "failed to get recommended opp: %d\n", ret);
>>>> +		return ret;
>>>> +	}
>>>> +	new_freq = dev_pm_opp_get_freq(new_opp);
>>>
>>> It doesn't need. Because the new frequency is stored to 'freq'
>>> by calling devfreq_recommended_opp().
>>
>> fixed
>>
>>>> +	dev_pm_opp_put(new_opp);
>>>> +
>>>> +	return clk_set_rate(priv->clk, new_freq);
>>>
>>> nitpick. you can use dev_pm_opp_set_rate(). But, I'm not forcing to use it.
>>
>> Switched to dev_pm_opp_set_rate.
>>
>> It might be interesting to add regulators control later, on some chips
>> the main NOC can run at different voltages.
>>
>>>
>>>> +}
>>>> +
>>>> +static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
>>>> +{
>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>> +
>>>> +	*freq = clk_get_rate(priv->clk);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int imx_bus_get_dev_status(struct device *dev,
>>>> +		struct devfreq_dev_status *stat)
>>>> +{
>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>> +
>>>> +	stat->busy_time = 0;
>>>> +	stat->total_time = 0;
>>>> +	stat->current_frequency = clk_get_rate(priv->clk);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void imx_bus_exit(struct device *dev)
>>>> +{
>>>> +	dev_pm_opp_of_remove_table(dev);
>>>> +}
>>>> +
>>>> +static int imx_bus_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct imx_bus *priv;
>>>> +	const char *gov = DEVFREQ_GOV_USERSPACE;
>>>> +	int ret;
>>>> +
>>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>> +	if (!priv)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	/*
>>>> +	 * Fetch the clock to adjust but don't explictly enable.
>>>
>>> Need to fix typo.
>>> s/explictly/explicitly
>>
>> fixed
>>
>>>> +	 *
>>>> +	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
>>>> +	 * or off and some peripheral side-buses might be off unless enabled by
>>>> +	 * drivers for devices on those specific buses.
>>>> +	 *
>>>> +	 * Rate adjustment on a disabled bus clock just takes effect later.
>>>> +	 */
>>>> +	priv->clk = devm_clk_get(dev, NULL);
>>>> +	if (IS_ERR(priv->clk)) {
>>>> +		ret = PTR_ERR(priv->clk);
>>>> +		dev_err(dev, "failed to fetch clk: %d\n", ret);
>>>> +		return ret;
>>>> +	}
>>>> +	platform_set_drvdata(pdev, priv);
>>>> +
>>>> +	ret = dev_pm_opp_of_add_table(dev);
>>>> +	if (ret < 0) {
>>>> +		dev_err(dev, "failed to get OPP table\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	priv->profile.polling_ms = 1000;
>>>> +	priv->profile.target = imx_bus_target;
>>>> +	priv->profile.get_dev_status = imx_bus_get_dev_status;
>>>> +	priv->profile.exit = imx_bus_exit;
>>>> +	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
>>>> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
>>>> +
>>>> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
>>>> +						gov, NULL);
>>>> +	if (IS_ERR(priv->devfreq)) {
>>>> +		ret = PTR_ERR(priv->devfreq);
>>>> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>>>> +		goto err;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err:
>>>> +	dev_pm_opp_of_remove_table(dev);
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static const struct of_device_id imx_bus_of_match[] = {
>>>> +	{ .compatible = "fsl,imx8m-noc", },
>>>> +	{ .compatible = "fsl,imx8m-nic", },
>>>> +	{ /* sentinel */ },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, imx_bus_of_match);
>>>> +
>>>> +static struct platform_driver imx_bus_platdrv = {
>>>> +	.probe		= imx_bus_probe,
>>>> +	.driver = {
>>>> +		.name	= "imx-bus-devfreq",
>>>> +		.of_match_table = of_match_ptr(imx_bus_of_match),
>>>> +	},
>>>> +};
>>>> +module_platform_driver(imx_bus_platdrv);
>>>> +
>>>> +MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
>>>> +MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
>>>> +MODULE_LICENSE("GPL v2");
>>>>
>>>
>>>
>>
>>
>>
>>
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] PM / devfreq: Add generic imx bus scaling driver

[Chanwoo Choi]

On 4/2/20 6:53 PM, Leonard Crestez wrote:
> On 2020-04-02 1:48 AM, Chanwoo Choi wrote:
>> On 4/1/20 11:20 PM, Leonard Crestez wrote:
>>> On 2020-04-01 1:55 AM, Chanwoo Choi wrote:
>>>> Hi,
>>>>
>>>> Looks good to me. I added the comments.
>>>> But, it need to add the dt binding documentation for this device.
>>>
>>> DT bindings were included:
>>>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11458981%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C7381d117a4d1468cd2c608d7d68ecfac%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637213781167514099&sdata=Qu8x14cXnuxlwOT2SlUOf%2FLgCVWbnJRKA4TBjMIWQeA%3D&reserved=0
>>
>> The dt-binding document for this driver is required under
>> Documentation/devicetree/binding/devfreq.
> 
> Bindings for imx8m-ddrc were at one point posted for 
> devicetree/bindings/devfreq but Rob Herring suggested to move them under 
> "memory-controller" instead and I expect same logic makes sense here. 
> Link to previous discussion:
> 
> https://patchwork.kernel.org/patch/11221919/
> 
> DT bindings should try to describe "hardware" rather than "drivers" and 
> an "interconnect" is a class of hardware while "devfreq" isn't.
> 
> Not only that but the main noc has properties parsed by interconnect driver.

OK. Thanks for reply.

> 
>> It is difficult to catch where is the dt-binding document
>> for this driver for who don't know the detailed history
>> of this driver. I don't said that add the duplicate documentation
>> But, at least the some document have to point out the reference.
> 
> What I usually do to find information about a device is grep for the 
> compat string in the entire tree.
> 
>>>> The old email of Artur Świgoń is not used. On next time,
>>>> use following the new email address Because when I reply the mail,
>>>> always show the fail message from thunderbird due to the Artur's old email.
>>>> <a.swigon@partnet.samsung.com> -> <a.swigon@samsung.com>
>>>
>>> Yeah, I received multiple bounces because of this.
>>>
>>>> On 3/26/20 11:16 AM, Leonard Crestez wrote:
>>>>> Add initial support for dynamic frequency switching on pieces of the imx
>>>>> interconnect fabric.
>>>>>
>>>>> All this driver does is set a clk rate based on an opp table, it does
>>>>> not map register areas.
>>>>>
>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>> ---
>>>>>    drivers/devfreq/Kconfig   |   9 +++
>>>>>    drivers/devfreq/Makefile  |   1 +
>>>>>    drivers/devfreq/imx-bus.c | 142 ++++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 152 insertions(+)
>>>>>    create mode 100644 drivers/devfreq/imx-bus.c
>>>>>
>>>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>>>> index 0b1df12e0f21..44d26192ddc4 100644
>>>>> --- a/drivers/devfreq/Kconfig
>>>>> +++ b/drivers/devfreq/Kconfig
>>>>> @@ -99,10 +99,19 @@ config ARM_IMX8M_DDRC_DEVFREQ
>>>>>    	select DEVFREQ_GOV_USERSPACE
>>>>>    	help
>>>>>    	  This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>>>>>    	  adjusting DRAM frequency.
>>>>>    
>>>>> +config ARM_IMX_BUS_DEVFREQ
>>>>> +	tristate "i.MX Generic Bus DEVFREQ Driver"
>>>>> +	depends on ARCH_MXC || COMPILE_TEST
>>>>> +	select DEVFREQ_GOV_PASSIVE
>>>>> +	select DEVFREQ_GOV_USERSPACE
>>>>
>>>> Maybe, you would update it by using passive governor?
>>>> But, in this version, it doesn't handle the any passive governor.
>>>
>>> dropped
>>>
>>>>> +	help
>>>>> +	  This adds the generic DEVFREQ driver for i.MX interconnects. It
>>>>> +	  allows adjusting NIC/NOC frequency.
>>>>> +
>>>>>    config ARM_TEGRA_DEVFREQ
>>>>>    	tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>>>>>    	depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>>>>>    		ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>>>>>    		ARCH_TEGRA_210_SOC || \
>>>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>>>> index 3eb4d5e6635c..3ca1ad0ecb97 100644
>>>>> --- a/drivers/devfreq/Makefile
>>>>> +++ b/drivers/devfreq/Makefile
>>>>> @@ -7,10 +7,11 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>>>>    obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>>>>    obj-$(CONFIG_DEVFREQ_GOV_PASSIVE)	+= governor_passive.o
>>>>>    
>>>>>    # DEVFREQ Drivers
>>>>>    obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)	+= exynos-bus.o
>>>>> +obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
>>>>
>>>> The ARM_IMX_BUS_DEVFREQ config is under ARM_IMX8M_DDRC_DEVFREQ
>>>> and imx-bus.o is over imx8m-ddrc.o. Need to edit the sequence.
>>>
>>> Reordered kconfig to match. 8M_DDRC sorts before _BUS alphabetically but
>>> it's pettier this way, and matches tegra.
>>>
>>>>>    obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
>>>>>    obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
>>>>>    obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
>>>>>    obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
>>>>>    
>>>>> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
>>>>> new file mode 100644
>>>>> index 000000000000..285e0f1ae6b1
>>>>> --- /dev/null
>>>>> +++ b/drivers/devfreq/imx-bus.c
>>>>> @@ -0,0 +1,142 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * Copyright 2019 NXP
>>>>> + */
>>>>> +
>>>>> +#include <linux/clk.h>
>>>>> +#include <linux/devfreq.h>
>>>>> +#include <linux/device.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/of_device.h>
>>>>> +#include <linux/pm_opp.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/slab.h>
>>>>> +
>>>>> +struct imx_bus {
>>>>> +	struct devfreq_dev_profile profile;
>>>>> +	struct devfreq *devfreq;
>>>>> +	struct clk *clk;
>>>>> +	struct devfreq_passive_data passive_data;
>>>>
>>>> This patch doesn't touch the passive_data.
>>>
>>> dropped
>>>
>>>>> +};
>>>>> +
>>>>> +static int imx_bus_target(struct device *dev,
>>>>> +		unsigned long *freq, u32 flags)
>>>>> +{
>>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>>> +	struct dev_pm_opp *new_opp;
>>>>> +	unsigned long new_freq;
>>>>> +	int ret;
>>>>> +
>>>>> +	new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>> +	if (IS_ERR(new_opp)) {
>>>>> +		ret = PTR_ERR(new_opp);
>>>>> +		dev_err(dev, "failed to get recommended opp: %d\n", ret);
>>>>> +		return ret;
>>>>> +	}
>>>>> +	new_freq = dev_pm_opp_get_freq(new_opp);
>>>>
>>>> It doesn't need. Because the new frequency is stored to 'freq'
>>>> by calling devfreq_recommended_opp().
>>>
>>> fixed
>>>
>>>>> +	dev_pm_opp_put(new_opp);
>>>>> +
>>>>> +	return clk_set_rate(priv->clk, new_freq);
>>>>
>>>> nitpick. you can use dev_pm_opp_set_rate(). But, I'm not forcing to use it.
>>>
>>> Switched to dev_pm_opp_set_rate.
>>>
>>> It might be interesting to add regulators control later, on some chips
>>> the main NOC can run at different voltages.
>>>
>>>>
>>>>> +}
>>>>> +
>>>>> +static int imx_bus_get_cur_freq(struct device *dev, unsigned long *freq)
>>>>> +{
>>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>>> +
>>>>> +	*freq = clk_get_rate(priv->clk);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int imx_bus_get_dev_status(struct device *dev,
>>>>> +		struct devfreq_dev_status *stat)
>>>>> +{
>>>>> +	struct imx_bus *priv = dev_get_drvdata(dev);
>>>>> +
>>>>> +	stat->busy_time = 0;
>>>>> +	stat->total_time = 0;
>>>>> +	stat->current_frequency = clk_get_rate(priv->clk);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static void imx_bus_exit(struct device *dev)
>>>>> +{
>>>>> +	dev_pm_opp_of_remove_table(dev);
>>>>> +}
>>>>> +
>>>>> +static int imx_bus_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct device *dev = &pdev->dev;
>>>>> +	struct imx_bus *priv;
>>>>> +	const char *gov = DEVFREQ_GOV_USERSPACE;
>>>>> +	int ret;
>>>>> +
>>>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>>> +	if (!priv)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	/*
>>>>> +	 * Fetch the clock to adjust but don't explictly enable.
>>>>
>>>> Need to fix typo.
>>>> s/explictly/explicitly
>>>
>>> fixed
>>>
>>>>> +	 *
>>>>> +	 * For imx bus clock clk_set_rate is safe no matter if the clock is on
>>>>> +	 * or off and some peripheral side-buses might be off unless enabled by
>>>>> +	 * drivers for devices on those specific buses.
>>>>> +	 *
>>>>> +	 * Rate adjustment on a disabled bus clock just takes effect later.
>>>>> +	 */
>>>>> +	priv->clk = devm_clk_get(dev, NULL);
>>>>> +	if (IS_ERR(priv->clk)) {
>>>>> +		ret = PTR_ERR(priv->clk);
>>>>> +		dev_err(dev, "failed to fetch clk: %d\n", ret);
>>>>> +		return ret;
>>>>> +	}
>>>>> +	platform_set_drvdata(pdev, priv);
>>>>> +
>>>>> +	ret = dev_pm_opp_of_add_table(dev);
>>>>> +	if (ret < 0) {
>>>>> +		dev_err(dev, "failed to get OPP table\n");
>>>>> +		return ret;
>>>>> +	}
>>>>> +
>>>>> +	priv->profile.polling_ms = 1000;
>>>>> +	priv->profile.target = imx_bus_target;
>>>>> +	priv->profile.get_dev_status = imx_bus_get_dev_status;
>>>>> +	priv->profile.exit = imx_bus_exit;
>>>>> +	priv->profile.get_cur_freq = imx_bus_get_cur_freq;
>>>>> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
>>>>> +
>>>>> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
>>>>> +						gov, NULL);
>>>>> +	if (IS_ERR(priv->devfreq)) {
>>>>> +		ret = PTR_ERR(priv->devfreq);
>>>>> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
>>>>> +		goto err;
>>>>> +	}
>>>>> +
>>>>> +	return 0;
>>>>> +
>>>>> +err:
>>>>> +	dev_pm_opp_of_remove_table(dev);
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id imx_bus_of_match[] = {
>>>>> +	{ .compatible = "fsl,imx8m-noc", },
>>>>> +	{ .compatible = "fsl,imx8m-nic", },
>>>>> +	{ /* sentinel */ },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, imx_bus_of_match);
>>>>> +
>>>>> +static struct platform_driver imx_bus_platdrv = {
>>>>> +	.probe		= imx_bus_probe,
>>>>> +	.driver = {
>>>>> +		.name	= "imx-bus-devfreq",
>>>>> +		.of_match_table = of_match_ptr(imx_bus_of_match),
>>>>> +	},
>>>>> +};
>>>>> +module_platform_driver(imx_bus_platdrv);
>>>>> +
>>>>> +MODULE_DESCRIPTION("Generic i.MX bus frequency scaling driver");
>>>>> +MODULE_AUTHOR("Leonard Crestez <leonard.crestez@nxp.com>");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel