interconnect: Add imx support via devfreq

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 <[email protected]>

---
 .../bindings/interconnect/fsl,imx8m-noc.yaml  | 138 ++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml

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 <[email protected]>

---
 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

drivers/devfreq/Kconfig +0 -0

index 0b1df12e0f21..44d26192ddc4 100644

drivers/devfreq/Kconfig +4 -0

+	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 || \

drivers/devfreq/Makefile +0 -0

index 3eb4d5e6635c..3ca1ad0ecb97 100644

drivers/devfreq/Makefile +0 -0

 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
 

drivers/devfreq/imx-bus.c new file +9 -0

new file mode 100644
index 000000000000..285e0f1ae6b1
@@ -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>

drivers/devfreq/imx-bus.c new file +7 -0

+};
+
+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;

drivers/devfreq/imx-bus.c new file +31 -0

+}
+
+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;

drivers/devfreq/imx-bus.c new file +62 -0

+	 *
+	 * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");

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. <[email protected]> -> <[email protected]>

Quoted 24 lines

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 <[email protected]>

---
  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

drivers/devfreq/Kconfig +0 -0

index 0b1df12e0f21..44d26192ddc4 100644
@@ -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.
 

drivers/devfreq/Kconfig config ARM_IMX8M_DDRC_DEVFREQ · line 108

Quoted 5 lines

drivers/devfreq/Kconfig +5 -0

+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.

Quoted 17 lines

drivers/devfreq/Kconfig +4 -0

+	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 || \

drivers/devfreq/Makefile +0 -0

index 3eb4d5e6635c..3ca1ad0ecb97 100644
@@ -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
 

drivers/devfreq/Makefile obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o · line 12

Quoted 3 lines

drivers/devfreq/Makefile +1 -0

  # 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.

Quoted 24 lines

drivers/devfreq/Makefile +0 -0

  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
 

drivers/devfreq/imx-bus.c new file +13 -0

new file mode 100644
index 000000000000..285e0f1ae6b1
@@ -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>

drivers/devfreq/imx-bus.c line 19

Quoted 6 lines

drivers/devfreq/imx-bus.c new file +6 -0

+
+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.

Quoted 12 lines

drivers/devfreq/imx-bus.c new file +12 -0

+};
+
+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)) {

drivers/devfreq/imx-bus.c line 36

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+		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().

drivers/devfreq/imx-bus.c line 39

Quoted 3 lines

drivers/devfreq/imx-bus.c new file +3 -0

+	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.

Quoted 37 lines

drivers/devfreq/imx-bus.c new file +36 -0

+}
+
+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);

drivers/devfreq/imx-bus.c line 80

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Fetch the clock to adjust but don't explictly enable.

Need to fix typo.
s/explictly/explicitly

Quoted 63 lines

drivers/devfreq/imx-bus.c new file +62 -0

+	 *
+	 * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");

Quoted 4 lines · Chanwoo Choi

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/

Quoted 5 lines

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. <[email protected]> -> <[email protected]>

Yeah, I received multiple bounces because of this.

Quoted 25 lines · Leonard Crestez

Quoted 24 lines

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 <[email protected]>

---
  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

drivers/devfreq/Kconfig +0 -0

index 0b1df12e0f21..44d26192ddc4 100644
@@ -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.
 

Quoted 8 lines

drivers/devfreq/Kconfig config ARM_IMX8M_DDRC_DEVFREQ · line 108

Quoted 5 lines

drivers/devfreq/Kconfig +5 -0

+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

Quoted 17 lines

Quoted 17 lines

drivers/devfreq/Kconfig +4 -0

+	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 || \

drivers/devfreq/Makefile +0 -0

index 3eb4d5e6635c..3ca1ad0ecb97 100644
@@ -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
 

Quoted 6 lines

drivers/devfreq/Makefile obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o · line 12

Quoted 3 lines

drivers/devfreq/Makefile +1 -0

  # 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.

Quoted 24 lines

Quoted 24 lines

drivers/devfreq/Makefile +0 -0

  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
 

drivers/devfreq/imx-bus.c new file +13 -0

new file mode 100644
index 000000000000..285e0f1ae6b1
@@ -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>

Quoted 8 lines

drivers/devfreq/imx-bus.c line 19

Quoted 6 lines

drivers/devfreq/imx-bus.c new file +6 -0

+
+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

Quoted 12 lines

Quoted 12 lines

drivers/devfreq/imx-bus.c new file +12 -0

+};
+
+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)) {

Quoted 8 lines

drivers/devfreq/imx-bus.c line 36

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+		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

Quoted 5 lines

drivers/devfreq/imx-bus.c line 39

Quoted 3 lines

drivers/devfreq/imx-bus.c new file +3 -0

+	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.

Quoted 37 lines

Quoted 37 lines

drivers/devfreq/imx-bus.c new file +36 -0

+}
+
+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);

Quoted 8 lines

drivers/devfreq/imx-bus.c line 80

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Fetch the clock to adjust but don't explictly enable.

Need to fix typo.
s/explictly/explicitly

fixed

Quoted 65 lines

Quoted 63 lines

drivers/devfreq/imx-bus.c new file +62 -0

+	 *
+	 * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");

Quoted 6 lines · Leonard Crestez

Quoted 4 lines · Chanwoo Choi

Hi,

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

Quoted 3 lines

DT bindings were included:

https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11458981%2F&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C7381d117a4d1468cd2c608d7d68ecfac%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637213781167514099&amp;sdata=Qu8x14cXnuxlwOT2SlUOf%2FLgCVWbnJRKA4TBjMIWQeA%3D&amp;reserved=0

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.

Quoted 256 lines

Quoted 4 lines

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. <[email protected]> -> <[email protected]>

Yeah, I received multiple bounces because of this.

Quoted 25 lines · Leonard Crestez

Quoted 24 lines

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 <[email protected]>

---
   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

drivers/devfreq/Kconfig +0 -0

index 0b1df12e0f21..44d26192ddc4 100644
@@ -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.
 

Quoted 8 lines

drivers/devfreq/Kconfig config ARM_IMX8M_DDRC_DEVFREQ · line 108

Quoted 5 lines

drivers/devfreq/Kconfig +5 -0

+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

Quoted 17 lines

Quoted 17 lines

drivers/devfreq/Kconfig +4 -0

+	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 || \

drivers/devfreq/Makefile +0 -0

index 3eb4d5e6635c..3ca1ad0ecb97 100644
@@ -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
 

Quoted 6 lines

drivers/devfreq/Makefile obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o · line 12

Quoted 3 lines

drivers/devfreq/Makefile +1 -0

   # 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.

Quoted 24 lines

Quoted 24 lines

drivers/devfreq/Makefile +0 -0

   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
 

drivers/devfreq/imx-bus.c new file +13 -0

new file mode 100644
index 000000000000..285e0f1ae6b1
@@ -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>

Quoted 8 lines

drivers/devfreq/imx-bus.c line 19

Quoted 6 lines

drivers/devfreq/imx-bus.c new file +6 -0

+
+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

Quoted 12 lines

Quoted 12 lines

drivers/devfreq/imx-bus.c new file +12 -0

+};
+
+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)) {

Quoted 8 lines

drivers/devfreq/imx-bus.c line 36

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+		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

Quoted 5 lines

drivers/devfreq/imx-bus.c line 39

Quoted 3 lines

drivers/devfreq/imx-bus.c new file +3 -0

+	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.

Quoted 37 lines

Quoted 37 lines

drivers/devfreq/imx-bus.c new file +36 -0

+}
+
+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);

Quoted 8 lines

drivers/devfreq/imx-bus.c line 80

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Fetch the clock to adjust but don't explictly enable.

Need to fix typo.
s/explictly/explicitly

fixed

Quoted 65 lines

Quoted 63 lines

drivers/devfreq/imx-bus.c new file +62 -0

+	 *
+	 * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");

Quoted 7 lines · Chanwoo Choi

Quoted 6 lines · Leonard Crestez

Quoted 4 lines · Chanwoo Choi

Hi,

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

Quoted 6 lines

Quoted 3 lines

DT bindings were included:

https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11458981%2F&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C7381d117a4d1468cd2c608d7d68ecfac%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637213781167514099&amp;sdata=Qu8x14cXnuxlwOT2SlUOf%2FLgCVWbnJRKA4TBjMIWQeA%3D&amp;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.

Quoted 5 lines

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.

Quoted 258 lines

Quoted 256 lines

Quoted 4 lines

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. <[email protected]> -> <[email protected]>

Yeah, I received multiple bounces because of this.

Quoted 25 lines · Leonard Crestez

Quoted 24 lines

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 <[email protected]>

---
   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

drivers/devfreq/Kconfig +0 -0

index 0b1df12e0f21..44d26192ddc4 100644
@@ -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.
 

Quoted 8 lines

drivers/devfreq/Kconfig config ARM_IMX8M_DDRC_DEVFREQ · line 108

Quoted 5 lines

drivers/devfreq/Kconfig +5 -0

+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

Quoted 17 lines

Quoted 17 lines

drivers/devfreq/Kconfig +4 -0

+	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 || \

drivers/devfreq/Makefile +0 -0

index 3eb4d5e6635c..3ca1ad0ecb97 100644
@@ -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
 

Quoted 6 lines

drivers/devfreq/Makefile obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o · line 12

Quoted 3 lines

drivers/devfreq/Makefile +1 -0

   # 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.

Quoted 24 lines

Quoted 24 lines

drivers/devfreq/Makefile +0 -0

   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
 

drivers/devfreq/imx-bus.c new file +13 -0

new file mode 100644
index 000000000000..285e0f1ae6b1
@@ -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>

Quoted 8 lines

drivers/devfreq/imx-bus.c line 19

Quoted 6 lines

drivers/devfreq/imx-bus.c new file +6 -0

+
+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

Quoted 12 lines

Quoted 12 lines

drivers/devfreq/imx-bus.c new file +12 -0

+};
+
+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)) {

Quoted 8 lines

drivers/devfreq/imx-bus.c line 36

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+		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

Quoted 5 lines

drivers/devfreq/imx-bus.c line 39

Quoted 3 lines

drivers/devfreq/imx-bus.c new file +3 -0

+	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.

Quoted 37 lines

Quoted 37 lines

drivers/devfreq/imx-bus.c new file +36 -0

+}
+
+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);

Quoted 8 lines

drivers/devfreq/imx-bus.c line 80

Quoted 5 lines

drivers/devfreq/imx-bus.c new file +5 -0

+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Fetch the clock to adjust but don't explictly enable.

Need to fix typo.
s/explictly/explicitly

fixed

Quoted 65 lines

Quoted 63 lines

drivers/devfreq/imx-bus.c new file +62 -0

+	 *
+	 * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");

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 <[email protected]>

---
 drivers/devfreq/imx-bus.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

drivers/devfreq/imx-bus.c +1 -0

index 285e0f1ae6b1..e9b13e43bf0a 100644
@@ -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;
 }

drivers/devfreq/imx-bus.c +5 -0

+static int imx_bus_init_icc(struct device *dev)
+{
+	struct imx_bus *priv = dev_get_drvdata(dev);
+	const char *icc_driver_name;
+

drivers/devfreq/imx-bus.c +10 -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;

drivers/devfreq/imx-bus.c +5 -0

+		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);

Hi Leonard,

Quoted 33 lines · 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 <[email protected]>

---
  drivers/devfreq/imx-bus.c | 39 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 39 insertions(+)

drivers/devfreq/imx-bus.c +1 -0

index 285e0f1ae6b1..e9b13e43bf0a 100644
@@ -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;
  }
 

drivers/devfreq/imx-bus.c static int imx_bus_get_dev_status(struct device *dev, · line 72

Quoted 9 lines

drivers/devfreq/imx-bus.c +6 -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 */

Quoted 9 lines

drivers/devfreq/imx-bus.c +9 -0

+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");

drivers/devfreq/imx-bus.c static int imx_bus_get_dev_status(struct device *dev, · line 87

Quoted 6 lines

drivers/devfreq/imx-bus.c +6 -0

+		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.

Quoted 21 lines

drivers/devfreq/imx-bus.c +10 -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;
  	}
 

drivers/devfreq/imx-bus.c static int imx_bus_probe(struct platform_device *pdev) · line 150

Quoted 2 lines

drivers/devfreq/imx-bus.c +2 -0

+	ret = imx_bus_init_icc(dev);
+	if (ret)

Recommend to add the error message.

Quoted 9 lines

drivers/devfreq/imx-bus.c +2 -0

+		goto err;
+
  	return 0;
 
  err:
  	dev_pm_opp_of_remove_table(dev);
  	return ret;
  }
 

drivers/devfreq/imx-bus.c static int imx_bus_probe(struct platform_device *pdev) · line 168

Quoted 10 lines

drivers/devfreq/imx-bus.c +3 -0

  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);

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

Quoted 26 lines · 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

Quoted 6 lines

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

Quoted 83 lines

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

Quoted 9 lines

 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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Martin,

Quoted 37 lines · Martin Kepplinger

Quoted 26 lines · 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

Quoted 6 lines

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:

Quoted 8 lines

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

Quoted 36 lines · Guido Günther

Hi Martin,

Quoted 34 lines · Martin Kepplinger

Quoted 32 lines · 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://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

Quoted 5 lines

Quoted 5 lines

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.

Quoted 11 lines

Quoted 5 lines

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

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?

Tested-by Martin Kepplinger <[email protected]>