[RFCv2 0/8] Add imx8mm bus frequency switching

[RFCv2 0/8] Add imx8mm bus frequency switching

[Leonard Crestez]

This series attempts to add upstream DVFS support for imx8mm, covering dynamic
scaling of internal buses and dram. It uses the interconnect framework for
proactive scaling (in response to explicit bandwidth requests from devices) and
devfreq in order expose the buses and eventually implement reactive scaling (in
response to measuredtraffic).

Actual scaling is performed through the clk framework: The NOC and main NICs
are driven by composite clks and a new 'imx8m-dram' clk is included for
scaling dram using firmware calls.

The interconnect and devfreq parts do not communicate explicitly: they both
just call clk_set_min_rate and the clk core picks the minimum value that can
satisfy both. They are thus completely independent.

This is easily extensible to more members of the imx8m family, some of which
expose more detailed controls over interconnect fabric frequencies.

TODO:
* Clarify DT bindings
* Clarify interconnect OPP picking logic
* Implement devfreq_event for imx8m ddrc
* Expose more dram frequencies

The clk_set_min_rate approach does not mesh very well with the OPP framework.
Some of interconnect nodes on imx8m can run at different voltages: OPP can
handle this well but not in response to a clk_set_min_rate from an unrelated
subsystem. Maybe set voltage on a clk notifier?

Vendor tree does not support voltage switching, independent freqs for
different parts of the fabric or any reactive scaling. I think it's important
to pick an upstreaming approach which can support as much as possible.

Feedback welcome.

Some objections were apparently raised to doing DRAM switch inside CLK:
perhaps ICC should make min_freq requests to devfreq instead?

Link to v1 (multiple chunks):
 * https://patchwork.kernel.org/patch/10976897/
 * https://patchwork.kernel.org/patch/10968303/
 * https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=91251

Also as a github branch (with few other changes):
    https://github.com/cdleonard/linux/tree/next_imx8mm_busfreq

Alexandre Bailon (2):
  interconnect: Add generic driver for imx
  interconnect: imx: Add platform driver for imx8mm

Leonard Crestez (6):
  clk: imx8mm: Add dram freq switch support
  clk: imx8m-composite: Switch to determine_rate
  arm64: dts: imx8mm: Add dram dvfs irqs to ccm node
  devfreq: Add imx-devfreq driver
  arm64: dts: imx8mm: Add interconnect node
  arm64: dts: imx8mm: Add devfreq-imx nodes

 arch/arm64/boot/dts/freescale/imx8mm.dtsi |  73 +++
 drivers/clk/imx/Makefile                  |   1 +
 drivers/clk/imx/clk-composite-8m.c        |  34 +-
 drivers/clk/imx/clk-imx8m-dram.c          | 357 ++++++++++++
 drivers/clk/imx/clk-imx8mm.c              |  12 +
 drivers/clk/imx/clk.h                     |  13 +
 drivers/devfreq/Kconfig                   |  10 +
 drivers/devfreq/Makefile                  |   1 +
 drivers/devfreq/imx-devfreq.c             | 142 +++++
 drivers/interconnect/Kconfig              |   1 +
 drivers/interconnect/Makefile             |   1 +
 drivers/interconnect/imx/Kconfig          |  17 +
 drivers/interconnect/imx/Makefile         |   2 +
 drivers/interconnect/imx/busfreq-imx8mm.c | 151 ++++++
 drivers/interconnect/imx/busfreq.c        | 628 ++++++++++++++++++++++
 drivers/interconnect/imx/busfreq.h        | 123 +++++
 include/dt-bindings/clock/imx8mm-clock.h  |   4 +-
 include/dt-bindings/interconnect/imx8mm.h |  49 ++
 18 files changed, 1606 insertions(+), 13 deletions(-)
 create mode 100644 drivers/clk/imx/clk-imx8m-dram.c
 create mode 100644 drivers/devfreq/imx-devfreq.c
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/busfreq-imx8mm.c
 create mode 100644 drivers/interconnect/imx/busfreq.c
 create mode 100644 drivers/interconnect/imx/busfreq.h
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h

-- 
2.17.1


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

[RFCv2 1/8] clk: imx8mm: Add dram freq switch support

[Leonard Crestez]

Add a compound clock encapsulating dram frequency switch support for
imx8m chips. This allows higher-level DVFS code to manipulate dram
frequency using standard clock framework APIs.

Linux-side implementation is similar in principle to imx_clk_cpu or a
composite clock. Only some preparation is done inside the kernel, the
actual freq switch is performed from TF-A code which runs from an SRAM
area.

This is an early proof-of-concept which only support low/high mode on
imx8mm.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---
Changes since v1:
* Implement determin_rate so that consumers can set_min_rate
* Initial freq table
Link to v1:

Among the possible cleanups:
 * Handle errors in low/high busfreq code and back off
 * Move irq to secure world
 * Try to use fewer clk parameters
 * Use a table of frequencies
 * More chips and frequencies
---
 drivers/clk/imx/Makefile                 |   1 +
 drivers/clk/imx/clk-imx8m-dram.c         | 357 +++++++++++++++++++++++
 drivers/clk/imx/clk-imx8mm.c             |  12 +
 drivers/clk/imx/clk.h                    |  13 +
 include/dt-bindings/clock/imx8mm-clock.h |   4 +-
 5 files changed, 386 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-imx8m-dram.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 05641c64b317..0fc7195d6d3a 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -10,10 +10,11 @@ obj-$(CONFIG_MXC_CLK) += \
 	clk-fixup-div.o \
 	clk-fixup-mux.o \
 	clk-frac-pll.o \
 	clk-gate-exclusive.o \
 	clk-gate2.o \
+	clk-imx8m-dram.o \
 	clk-pfd.o \
 	clk-pfdv2.o \
 	clk-pllv1.o \
 	clk-pllv2.o \
 	clk-pllv3.o \
diff --git a/drivers/clk/imx/clk-imx8m-dram.c b/drivers/clk/imx/clk-imx8m-dram.c
new file mode 100644
index 000000000000..1d75be4a2f3a
--- /dev/null
+++ b/drivers/clk/imx/clk-imx8m-dram.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#define DEBUG
+#include <linux/arm-smccc.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/of_irq.h>
+#include "clk.h"
+
+#define FSL_SIP_DDR_DVFS                0xc2000004
+
+/* Freq of setpoints is soc-dependent */
+#define FSL_SIP_DDR_FREQ_SET_HIGH	0x0
+#define FSL_SIP_DDR_FREQ_WAIT_DONE	0xf
+
+struct imx8m_dram_rate {
+	unsigned long	rate;
+	unsigned int	smcarg;
+};
+
+/*
+ * This clk roughly wraps the following clk structure:
+ *
+ * +----------+       |\            +------+
+ * | dram_pll |-------|M| dram_core |      |
+ * +----------+       |U|---------->| D    |
+ *                 /--|X|           |  D   |
+ *   dram_alt_root |  |/            |   R  |
+ *                 |                |    C |
+ *            +---------+           |      |
+ *            |FIX DIV/4|           |      |
+ *            +---------+           |      |
+ *  composite:     |                |      |
+ * +----------+    |                |      |
+ * | dram_alt |----/                |      |
+ * +----------+                     |      |
+ * | dram_apb |-------------------->|      |
+ * +----------+                     +------+
+ *
+ * The DDR data rate is 4x dram_core
+ *
+ * The APB interface is only used for control registers and can otherwise
+ * be shut off.
+ *
+ * The dram_pll is used for higher rates and dram_alt is used for lower rates.
+ *
+ * The actual switch is done inside ATF, what this wrapper does is:
+ *  - Enable the new parents
+ *  - Call into ATF
+ *  - Set the new rates
+ *  - Set the new parents
+ *  - Drop the reference count added to new parents at step 1
+ *
+ * In practice only 2 rates are supported: low and high.
+ */
+
+struct dram_clk {
+	struct clk_hw	hw;
+	struct clk	*dram_core;
+	struct clk	*dram_apb;
+	struct clk	*dram_pll;
+	struct clk	*dram_alt;
+	struct clk	*dram_alt_root;
+	struct clk	*sys1_pll_40m;
+	struct clk	*sys1_pll_100m;
+	struct clk	*sys1_pll_800m;
+	int		irqs[CONFIG_NR_CPUS];
+
+	unsigned int rate_count;
+	const struct imx8m_dram_rate *rate_table;
+};
+
+static inline struct dram_clk *to_dram_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct dram_clk, hw);
+}
+
+static irqreturn_t wait_in_wfe_irq(int irq, void *dev_id)
+{
+	struct arm_smccc_res res;
+
+	/* call smc trap to ATF */
+	arm_smccc_smc(FSL_SIP_DDR_DVFS, FSL_SIP_DDR_FREQ_WAIT_DONE, 0,
+		0, 0, 0, 0, 0, &res);
+
+	return IRQ_HANDLED;
+}
+
+static void update_bus_freq(int target_freq)
+{
+	struct arm_smccc_res res;
+	u32 online_cpus = 0;
+	int cpu = 0;
+
+	local_irq_disable();
+
+	for_each_online_cpu(cpu)
+		online_cpus |= (1 << (cpu * 8));
+
+	/* change the ddr freqency */
+	arm_smccc_smc(FSL_SIP_DDR_DVFS, target_freq, online_cpus,
+		0, 0, 0, 0, 0, &res);
+
+	local_irq_enable();
+}
+
+static int dram_clk_ensure_irq_affinity(struct dram_clk* priv)
+{
+	int err, cpu;
+
+	for_each_online_cpu(cpu) {
+		err = irq_set_affinity(priv->irqs[cpu], cpumask_of(cpu));
+		if (err) {
+			pr_err("imx8m_dram_clk set irqs[%d] affinity failed: %d\n",
+				cpu, err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+/* Round UP */
+static const struct imx8m_dram_rate *dram_clk_find_rate(
+		struct dram_clk *priv,
+		unsigned long rate)
+{
+	int i;
+
+	for (i = priv->rate_count - 1; i >= 0; --i)
+		if (priv->rate_table[i].rate >= rate)
+			return &priv->rate_table[i];
+
+	return &priv->rate_table[0];
+}
+
+/* Round UP taking min and max into account */
+static int dram_clk_determine_rate(
+		struct clk_hw *hw,
+		struct clk_rate_request *req)
+{
+	struct dram_clk *priv = to_dram_clk(hw);
+	unsigned long tab_rate;
+	int i;
+
+	for (i = priv->rate_count - 1; i >= 0; --i) {
+		tab_rate = priv->rate_table[i].rate;
+		if (tab_rate >= req->rate &&
+		    tab_rate >= req->min_rate &&
+		    tab_rate <= req->max_rate)
+		{
+			req->rate = tab_rate;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int dram_clk_set_low(struct dram_clk *priv)
+{
+	clk_prepare_enable(priv->sys1_pll_40m);
+	clk_prepare_enable(priv->dram_alt_root);
+	clk_prepare_enable(priv->sys1_pll_100m);
+
+	/* switch the DDR frequency */
+	update_bus_freq(0x2);
+
+	/* correct the clock tree info */
+	clk_set_parent(priv->dram_alt, priv->sys1_pll_100m);
+	clk_set_parent(priv->dram_core, priv->dram_alt_root);
+	clk_set_parent(priv->dram_apb, priv->sys1_pll_40m);
+	clk_set_rate(priv->dram_apb, 20000000);
+	clk_disable_unprepare(priv->sys1_pll_100m);
+	clk_disable_unprepare(priv->sys1_pll_40m);
+	clk_disable_unprepare(priv->dram_alt_root);
+	return 0;
+}
+
+static int dram_clk_set_high(struct dram_clk *priv)
+{
+	clk_prepare_enable(priv->sys1_pll_800m);
+	clk_prepare_enable(priv->dram_pll);
+
+	/* switch the DDR frequency */
+	update_bus_freq(0x0);
+
+	/* correct the clock tree info */
+	clk_set_parent(priv->dram_apb, priv->sys1_pll_800m);
+	clk_set_rate(priv->dram_apb, 160000000);
+	clk_set_parent(priv->dram_core, priv->dram_pll);
+	clk_disable_unprepare(priv->sys1_pll_800m);
+	clk_disable_unprepare(priv->dram_pll);
+
+	return 0;
+}
+
+static int dram_clk_set_rate(
+		struct clk_hw *hw,
+		unsigned long rate,
+		unsigned long parent_rate)
+{
+	struct dram_clk *priv = to_dram_clk(hw);
+	const struct imx8m_dram_rate *opp = dram_clk_find_rate(priv, rate);
+	int ret;
+
+	ret = dram_clk_ensure_irq_affinity(priv);
+	if (ret)
+		return ret;
+
+	if (opp->smcarg == 2) {
+		ret = dram_clk_set_low(priv);
+	} else if (opp->smcarg == 0) {
+		ret = dram_clk_set_high(priv);
+	} else {
+		ret = -EINVAL;
+	}
+
+	if (ret == 0)
+		pr_debug("%s freq set to %lu\n", clk_hw_get_name(hw), opp->rate);
+	else
+		pr_debug("%s freq set fail: %d\n", clk_hw_get_name(hw), ret);
+
+	return ret;
+}
+
+static unsigned long dram_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct dram_clk *priv = to_dram_clk(hw);
+
+	return clk_get_rate(priv->dram_core);
+}
+
+static const struct clk_ops dram_clk_ops = {
+	.determine_rate	= dram_clk_determine_rate,
+	.recalc_rate	= dram_clk_recalc_rate,
+	.set_rate	= dram_clk_set_rate,
+};
+
+static int dram_clk_init_irqs(struct dram_clk* priv, struct device_node *np)
+{
+	int err, irq, cpu;
+
+	for_each_possible_cpu(cpu) {
+		irq = of_irq_get(np, cpu);
+		if (irq < 0) {
+			pr_err("imx8m_dram_clk fail of_irq_get %d\n", irq);
+			return irq;
+		}
+
+		err = request_irq(irq, wait_in_wfe_irq,
+				IRQF_PERCPU, "ddrc", NULL);
+		if (err) {
+			pr_err("imx8m_dram_clk request irq %d failed: %d\n",
+				irq, err);
+			return err;
+		}
+		priv->irqs[cpu] = irq;
+	}
+
+	return 0;
+}
+
+static void dram_clk_free_irqs(struct dram_clk* priv)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		free_irq(priv->irqs[cpu], NULL);
+		priv->irqs[cpu] = 0;
+	}
+}
+
+static const struct imx8m_dram_rate imx8mq_dram_rate_table[] = {
+	{
+		.rate	= 800000000,
+		.smcarg	= 0x0,
+	},
+	{
+		.rate	= 25000000,
+		.smcarg	= 0x2,
+	},
+};
+
+static const struct imx8m_dram_rate imx8mm_dram_rate_table[] = {
+	{
+		.rate	= 750000000,
+		.smcarg	= 0x0,
+	},
+	{
+		.rate	= 25000000,
+		.smcarg	= 0x2,
+	},
+};
+
+struct clk* imx8m_dram_clk(
+		struct device_node* np,
+		const char *name, const char* parent_name,
+		struct clk* dram_core,
+		struct clk* dram_apb,
+		struct clk* dram_pll,
+		struct clk* dram_alt,
+		struct clk* dram_alt_root,
+		struct clk* sys1_pll_40m,
+		struct clk* sys1_pll_100m,
+		struct clk* sys1_pll_800m)
+{
+	struct dram_clk *priv;
+	struct clk *clk;
+	struct clk_init_data init;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+
+	priv->dram_apb = dram_apb;
+	priv->dram_core = dram_core;
+	priv->dram_pll = dram_pll;
+	priv->dram_alt = dram_alt;
+	priv->dram_alt_root = dram_alt_root;
+	priv->sys1_pll_40m = sys1_pll_40m;
+	priv->sys1_pll_100m = sys1_pll_100m;
+	priv->sys1_pll_800m = sys1_pll_800m;
+	priv->rate_count = ARRAY_SIZE(imx8mm_dram_rate_table);
+	priv->rate_table = imx8mm_dram_rate_table;
+
+	init.name = name;
+	init.ops = &dram_clk_ops;
+	init.flags = CLK_IS_CRITICAL;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	err = dram_clk_init_irqs(priv, np);
+	if (err)
+		goto err_free_priv;
+
+	priv->hw.init = &init;
+	clk = clk_register(NULL, &priv->hw);
+	if (IS_ERR(clk)) {
+		err = PTR_ERR(clk);
+		goto err_free_irqs;
+	}
+	return clk;
+
+err_free_irqs:
+	dram_clk_free_irqs(priv);
+err_free_priv:
+	kfree(priv);
+	return ERR_PTR(err);
+}
diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 6b8e75df994d..8e19a6fdc415 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -660,10 +660,22 @@ static int __init imx8mm_clocks_init(struct device_node *ccm_node)
 	clks[IMX8MM_CLK_GPT_3M] = imx_clk_fixed_factor("gpt_3m", "osc_24m", 1, 8);
 
 	clks[IMX8MM_CLK_DRAM_ALT_ROOT] = imx_clk_fixed_factor("dram_alt_root", "dram_alt", 1, 4);
 	clks[IMX8MM_CLK_DRAM_CORE] = imx_clk_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, imx8mm_dram_core_sels, ARRAY_SIZE(imx8mm_dram_core_sels), CLK_IS_CRITICAL);
 
+	clks[IMX8MM_CLK_DRAM] = imx8m_dram_clk(
+			ccm_node,
+			"dram", "dram_core_clk",
+			clks[IMX8MM_CLK_DRAM_CORE],
+			clks[IMX8MM_CLK_DRAM_APB],
+			clks[IMX8MM_DRAM_PLL_OUT],
+			clks[IMX8MM_CLK_DRAM_ALT],
+			clks[IMX8MM_CLK_DRAM_ALT_ROOT],
+			clks[IMX8MM_SYS_PLL1_40M],
+			clks[IMX8MM_SYS_PLL1_100M],
+			clks[IMX8MM_SYS_PLL1_800M]);
+
 	clks[IMX8MM_CLK_ARM] = imx_clk_cpu("arm", "arm_a53_div",
 					   clks[IMX8MM_CLK_A53_DIV],
 					   clks[IMX8MM_CLK_A53_SRC],
 					   clks[IMX8MM_ARM_PLL_OUT],
 					   clks[IMX8MM_CLK_24M]);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index d94d9cb079d3..858547fbad58 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -468,6 +468,19 @@ struct clk *imx8m_clk_composite_flags(const char *name,
 
 struct clk_hw *imx_clk_divider_gate(const char *name, const char *parent_name,
 		unsigned long flags, void __iomem *reg, u8 shift, u8 width,
 		u8 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock);
+
+struct clk* imx8m_dram_clk(
+		struct device_node *np,
+		const char *name, const char* parent_name,
+		struct clk* dram_core,
+		struct clk* dram_apb,
+		struct clk* dram_pll,
+		struct clk* dram_alt,
+		struct clk* dram_alt_root,
+		struct clk* sys1_pll_40m,
+		struct clk* sys1_pll_100m,
+		struct clk* sys1_pll_800m);
+
 #endif
diff --git a/include/dt-bindings/clock/imx8mm-clock.h b/include/dt-bindings/clock/imx8mm-clock.h
index 07e6c686f3ef..dde146b923a8 100644
--- a/include/dt-bindings/clock/imx8mm-clock.h
+++ b/include/dt-bindings/clock/imx8mm-clock.h
@@ -246,8 +246,10 @@
 #define IMX8MM_CLK_GPIO5_ROOT			227
 
 #define IMX8MM_CLK_SNVS_ROOT			228
 #define IMX8MM_CLK_GIC				229
 
-#define IMX8MM_CLK_END				230
+#define IMX8MM_CLK_DRAM				230
+
+#define IMX8MM_CLK_END				231
 
 #endif
-- 
2.17.1


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

[RFCv2 2/8] clk: imx8m-composite: Switch to determine_rate

[Leonard Crestez]

This allows consumers to use min_rate max_rate.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/clk/imx/clk-composite-8m.c | 34 +++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index 388bdb94f841..1be82ec08ecd 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -45,10 +45,12 @@ static unsigned long imx8m_clk_composite_divider_recalc_rate(struct clk_hw *hw,
 				   divider->flags, PCG_DIV_WIDTH);
 }
 
 static int imx8m_clk_composite_compute_dividers(unsigned long rate,
 						unsigned long parent_rate,
+						unsigned long min_rate,
+						unsigned long max_rate,
 						int *prediv, int *postdiv)
 {
 	int div1, div2;
 	int error = INT_MAX;
 	int ret = -EINVAL;
@@ -56,11 +58,17 @@ static int imx8m_clk_composite_compute_dividers(unsigned long rate,
 	*prediv = 1;
 	*postdiv = 1;
 
 	for (div1 = 1; div1 <= PCG_PREDIV_MAX; div1++) {
 		for (div2 = 1; div2 <= PCG_DIV_MAX; div2++) {
-			int new_error = ((parent_rate / div1) / div2) - rate;
+			unsigned long new_rate;
+			int new_error;
+
+			new_rate = ((parent_rate / div1) / div2);
+			if (new_rate < min_rate || new_rate > max_rate)
+				continue;
+			new_error = new_rate - rate;
 
 			if (abs(new_error) < abs(error)) {
 				*prediv = div1;
 				*postdiv = div2;
 				error = new_error;
@@ -69,38 +77,40 @@ static int imx8m_clk_composite_compute_dividers(unsigned long rate,
 		}
 	}
 	return ret;
 }
 
-static long imx8m_clk_composite_divider_round_rate(struct clk_hw *hw,
-						unsigned long rate,
-						unsigned long *prate)
+static int imx8m_clk_composite_divider_determine_rate(struct clk_hw *hw,
+						       struct clk_rate_request *req)
 {
 	int prediv_value;
 	int div_value;
 
-	imx8m_clk_composite_compute_dividers(rate, *prate,
-						&prediv_value, &div_value);
-	rate = DIV_ROUND_UP(*prate, prediv_value);
+	imx8m_clk_composite_compute_dividers(req->rate, req->best_parent_rate,
+					     req->min_rate, req->max_rate,
+					     &prediv_value, &div_value);
 
-	return DIV_ROUND_UP(rate, div_value);
+	req->rate = DIV_ROUND_UP(req->best_parent_rate, prediv_value);
+	req->rate = DIV_ROUND_UP(req->rate, div_value);
 
+	return 0;
 }
 
 static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
-					unsigned long rate,
-					unsigned long parent_rate)
+						unsigned long rate,
+						unsigned long parent_rate)
 {
 	struct clk_divider *divider = to_clk_divider(hw);
 	unsigned long flags = 0;
 	int prediv_value;
 	int div_value;
 	int ret;
 	u32 val;
 
 	ret = imx8m_clk_composite_compute_dividers(rate, parent_rate,
-						&prediv_value, &div_value);
+						   0, ULONG_MAX,
+						   &prediv_value, &div_value);
 	if (ret)
 		return -EINVAL;
 
 	spin_lock_irqsave(divider->lock, flags);
 
@@ -117,11 +127,11 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
 	return ret;
 }
 
 static const struct clk_ops imx8m_clk_composite_divider_ops = {
 	.recalc_rate = imx8m_clk_composite_divider_recalc_rate,
-	.round_rate = imx8m_clk_composite_divider_round_rate,
+	.determine_rate = imx8m_clk_composite_divider_determine_rate,
 	.set_rate = imx8m_clk_composite_divider_set_rate,
 };
 
 struct clk *imx8m_clk_composite_flags(const char *name,
 					const char * const *parent_names,
-- 
2.17.1


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

Re: [RFCv2 2/8] clk: imx8m-composite: Switch to determine_rate

[Abel Vesa]

On 19-06-28 10:39:50, Leonard Crestez wrote:
> This allows consumers to use min_rate max_rate.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/clk/imx/clk-composite-8m.c | 34 +++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
> index 388bdb94f841..1be82ec08ecd 100644
> --- a/drivers/clk/imx/clk-composite-8m.c
> +++ b/drivers/clk/imx/clk-composite-8m.c
> @@ -45,10 +45,12 @@ static unsigned long imx8m_clk_composite_divider_recalc_rate(struct clk_hw *hw,
>  				   divider->flags, PCG_DIV_WIDTH);
>  }
>  
>  static int imx8m_clk_composite_compute_dividers(unsigned long rate,
>  						unsigned long parent_rate,
> +						unsigned long min_rate,
> +						unsigned long max_rate,

You should pass on the req instead of min_rate and max_rate here.

>  						int *prediv, int *postdiv)
>  {
>  	int div1, div2;
>  	int error = INT_MAX;
>  	int ret = -EINVAL;
> @@ -56,11 +58,17 @@ static int imx8m_clk_composite_compute_dividers(unsigned long rate,
>  	*prediv = 1;
>  	*postdiv = 1;
>  
>  	for (div1 = 1; div1 <= PCG_PREDIV_MAX; div1++) {
>  		for (div2 = 1; div2 <= PCG_DIV_MAX; div2++) {
> -			int new_error = ((parent_rate / div1) / div2) - rate;
> +			unsigned long new_rate;
> +			int new_error;
> +
> +			new_rate = ((parent_rate / div1) / div2);
> +			if (new_rate < min_rate || new_rate > max_rate)
> +				continue;
> +			new_error = new_rate - rate;
>  
>  			if (abs(new_error) < abs(error)) {
>  				*prediv = div1;
>  				*postdiv = div2;
>  				error = new_error;
> @@ -69,38 +77,40 @@ static int imx8m_clk_composite_compute_dividers(unsigned long rate,
>  		}
>  	}
>  	return ret;
>  }
>  
> -static long imx8m_clk_composite_divider_round_rate(struct clk_hw *hw,
> -						unsigned long rate,
> -						unsigned long *prate)
> +static int imx8m_clk_composite_divider_determine_rate(struct clk_hw *hw,
> +						       struct clk_rate_request *req)
>  {
>  	int prediv_value;
>  	int div_value;
>  
> -	imx8m_clk_composite_compute_dividers(rate, *prate,
> -						&prediv_value, &div_value);
> -	rate = DIV_ROUND_UP(*prate, prediv_value);
> +	imx8m_clk_composite_compute_dividers(req->rate, req->best_parent_rate,
> +					     req->min_rate, req->max_rate,
> +					     &prediv_value, &div_value);
>  
> -	return DIV_ROUND_UP(rate, div_value);
> +	req->rate = DIV_ROUND_UP(req->best_parent_rate, prediv_value);
> +	req->rate = DIV_ROUND_UP(req->rate, div_value);
>  
> +	return 0;
>  }
>  
>  static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
> -					unsigned long rate,
> -					unsigned long parent_rate)
> +						unsigned long rate,
> +						unsigned long parent_rate)
>  {
>  	struct clk_divider *divider = to_clk_divider(hw);
>  	unsigned long flags = 0;
>  	int prediv_value;
>  	int div_value;
>  	int ret;
>  	u32 val;
>  
>  	ret = imx8m_clk_composite_compute_dividers(rate, parent_rate,
> -						&prediv_value, &div_value);
> +						   0, ULONG_MAX,
> +						   &prediv_value, &div_value);
>  	if (ret)
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(divider->lock, flags);
>  
> @@ -117,11 +127,11 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
>  	return ret;
>  }
>  
>  static const struct clk_ops imx8m_clk_composite_divider_ops = {
>  	.recalc_rate = imx8m_clk_composite_divider_recalc_rate,
> -	.round_rate = imx8m_clk_composite_divider_round_rate,
> +	.determine_rate = imx8m_clk_composite_divider_determine_rate,
>  	.set_rate = imx8m_clk_composite_divider_set_rate,
>  };
>  
>  struct clk *imx8m_clk_composite_flags(const char *name,
>  					const char * const *parent_names,
> -- 
> 2.17.1
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFCv2 2/8] clk: imx8m-composite: Switch to determine_rate

[Leonard Crestez]

On 28.06.2019 11:45, Abel Vesa wrote:
> On 19-06-28 10:39:50, Leonard Crestez wrote:

>> This allows consumers to use min_rate max_rate.
>>
>> @@ -45,10 +45,12 @@ static unsigned long imx8m_clk_composite_divider_recalc_rate(struct clk_hw *hw,
>>   				   divider->flags, PCG_DIV_WIDTH);
>>   }
>>   
>>   static int imx8m_clk_composite_compute_dividers(unsigned long rate,
>>   						unsigned long parent_rate,
>> +						unsigned long min_rate,
>> +						unsigned long max_rate,
> 
> You should pass on the req instead of min_rate and max_rate here.

Then I'd have to switch imx8m_clk_composite_divider_set_rate to allocate 
a dummy struct clk_rate_request on the stack. It's clearer if I just 
pass the minimum parameters required.

--
Regards,
Leonard

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

Re: [RFCv2 2/8] clk: imx8m-composite: Switch to determine_rate

[Abel Vesa]

On 19-06-28 08:56:35, Leonard Crestez wrote:
> On 28.06.2019 11:45, Abel Vesa wrote:
> > On 19-06-28 10:39:50, Leonard Crestez wrote:
> 
> >> This allows consumers to use min_rate max_rate.
> >>
> >> @@ -45,10 +45,12 @@ static unsigned long imx8m_clk_composite_divider_recalc_rate(struct clk_hw *hw,
> >>   				   divider->flags, PCG_DIV_WIDTH);
> >>   }
> >>   
> >>   static int imx8m_clk_composite_compute_dividers(unsigned long rate,
> >>   						unsigned long parent_rate,
> >> +						unsigned long min_rate,
> >> +						unsigned long max_rate,
> > 
> > You should pass on the req instead of min_rate and max_rate here.
> 
> Then I'd have to switch imx8m_clk_composite_divider_set_rate to allocate 
> a dummy struct clk_rate_request on the stack. It's clearer if I just 
> pass the minimum parameters required.
> 

That is correct. Nevermind my earlier comment then.

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

[RFCv2 3/8] arm64: dts: imx8mm: Add dram dvfs irqs to ccm node

[Leonard Crestez]

This could probably be avoided by handling these in secure world.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 232a7412755a..5da905c257ad 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -449,10 +449,18 @@
 				#clock-cells = <1>;
 				clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>,
 					 <&clk_ext3>, <&clk_ext4>;
 				clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2",
 					      "clk_ext3", "clk_ext4";
+				interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-name = "irq_busfreq_0",
+						 "irq_busfreq_1",
+						 "irq_busfreq_2",
+						 "irq_busfreq_3";
 			};
 
 			src: reset-controller@30390000 {
 				compatible = "fsl,imx8mm-src", "syscon";
 				reg = <0x30390000 0x10000>;
-- 
2.17.1


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

[RFCv2 4/8] interconnect: Add generic driver for imx

[Leonard Crestez]

From: Alexandre Bailon <abailon@baylibre.com>

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

This is based on busfreq from NXP vendor tree tree. The implementation
is sufficiently generic to be applies to any imx SOC and perhaps others.

Using the interconnect framework driver requests for bandwidth are used
to determine a global "platform opp" which is then and then scale the
frequency. These "platform opps" are similar to the "low/audio/high"
busfreq levels from NXP tree.

Busfreq platform drivers just have to register interconnect nodes and
OPPs.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Changes by Leonard:
* Make it probe: add xlate and provider->dev
* Clarify commit message
* Use unsigned long for rate

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/interconnect/Kconfig       |   1 +
 drivers/interconnect/Makefile      |   1 +
 drivers/interconnect/imx/Kconfig   |  13 +
 drivers/interconnect/imx/Makefile  |   1 +
 drivers/interconnect/imx/busfreq.c | 628 +++++++++++++++++++++++++++++
 drivers/interconnect/imx/busfreq.h | 123 ++++++
 6 files changed, 767 insertions(+)
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/busfreq.c
 create mode 100644 drivers/interconnect/imx/busfreq.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 28f2ab0824d5..20a13b7eb37f 100644
--- a/drivers/interconnect/Makefile
+++ b/drivers/interconnect/Makefile
@@ -2,5 +2,6 @@
 
 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..afd7b71bbd82
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,13 @@
+config INTERCONNECT_IMX
+	bool "i.MX interconnect drivers"
+	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
+	help
+	  Support for i.MX interconnect hardware.
+
+config BUSFREQ
+	bool "busfreq interconnect driver"
+	depends on INTERCONNECT_IMX
+	help
+	  A generic interconnect driver that could be used for any i.MX.
+	  This provides a way to register master and slave and some opp
+	  to use when one or more master are in use.
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..fea647183815
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BUSFREQ) += busfreq.o
diff --git a/drivers/interconnect/imx/busfreq.c b/drivers/interconnect/imx/busfreq.c
new file mode 100644
index 000000000000..19578728e1b3
--- /dev/null
+++ b/drivers/interconnect/imx/busfreq.c
@@ -0,0 +1,628 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/suspend.h>
+#include <linux/of.h>
+
+#include "busfreq.h"
+
+/*
+ * struct busfreq_opp_node - Describe the minimum bandwidth required by a node
+ * to enable the opp
+ * @icc_node: icc_node to test
+ * @min_avg_bw: minimum average bandwidth in kbps required to enable opp
+ * @min_peak_bw: minimum peak bandwidth in kbps required to enable opp
+ */
+struct busfreq_opp_node {
+	struct icc_node *icc_node;
+	u32 min_avg_bw;
+	u32 min_peak_bw;
+};
+
+/*
+ * struct busfreq_opp - Describe an opp
+ * This is used to configure multiple clock at once with the respect of
+ * hardware and performance requirements.
+ * @clks_count: number of clocks to configure
+ * @clks: array of clock
+ * @rates: array of rate, to apply for each clock when the opp is enabled
+ * @perf_level: Used to select the opp that would allow the lowest power
+ *              consumption when two or more opp satisfies the performances
+ *              requirements
+ * @node: entry in list of opp
+ * @nodes_count: number of opp node
+ * @nodes: array of opp node, to check node by node if opp satisfies the
+ *         the required performances
+ */
+struct busfreq_opp {
+	int clks_count;
+	struct clk **clks;
+	unsigned long *rates;
+	u32 perf_level;
+
+	struct list_head node;
+
+	int nodes_count;
+	struct busfreq_opp_node *nodes;
+};
+
+/*
+ * struct busfreq_icc_desc - Hold data required to control the interconnects
+ * @dev: device pointer for the overall interconnect
+ * @opps: list of opp
+ * @default_opp: the opp opp to use when the system is in special states
+ *               (boot, suspend, resume, shutdown)
+ * @current_opp: the opp currently in use
+ * @opp_locked: prevent opp to change while this is set
+ * @pm_notifier: used to set the default opp before suspend and
+ *               and select the best one after resume
+ * @pm_notifier: used to set the default opp before to reboot
+ */
+struct busfreq_icc_desc {
+	struct device *dev;
+
+	struct list_head opps;
+	struct busfreq_opp *default_opp;
+	struct busfreq_opp *current_opp;
+	bool opp_locked;
+
+	struct notifier_block pm_notifier;
+	struct notifier_block reboot_notifier;
+
+	struct mutex mutex;
+};
+
+static int busfreq_icc_aggregate(struct icc_node *node, 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 struct icc_node* busfreq_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+	struct busfreq_icc_desc *desc = data;
+	struct icc_provider *provider = dev_get_drvdata(desc->dev);
+	unsigned int id = spec->args[0];
+	struct icc_node *node;
+
+	list_for_each_entry (node, &provider->nodes, node_list)
+		if (node->id == id)
+			return node;
+
+	return ERR_PTR(-EINVAL);
+}
+
+static int busfreq_doset_opp(struct busfreq_opp *opp)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < opp->clks_count; i++) {
+		ret = clk_set_min_rate(opp->clks[i], opp->rates[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+err:
+	for (; i >= 0; i--)
+		clk_set_min_rate(opp->clks[i], 0);
+
+	return ret;
+}
+
+static void busfreq_unset_opp(struct busfreq_opp *opp)
+{
+	int i;
+
+	for (i = 0; i < opp->clks_count; i++)
+		clk_set_min_rate(opp->clks[i], 0);
+}
+
+static int busfreq_set_opp_no_lock(struct busfreq_icc_desc *desc,
+				   struct busfreq_opp *requested_opp)
+{
+	int ret;
+	int i;
+
+	if (!requested_opp && !desc->default_opp)
+		return -EINVAL;
+
+	if (!requested_opp)
+		requested_opp = desc->default_opp;
+
+	if (desc->current_opp == requested_opp)
+		return 0;
+
+	if (desc->opp_locked)
+		return -EBUSY;
+
+	/*
+	 * Each OPP fetches clks separately and min_rate is aggregated by ccf
+	 * so we need to "unset" the min_rate explicitly afterwards.
+	 */
+	ret = busfreq_doset_opp(requested_opp);
+	if (ret)
+		return ret;
+
+	if (desc->current_opp)
+		busfreq_unset_opp(desc->current_opp);
+	desc->current_opp = requested_opp;
+
+	for (i = 0; i < requested_opp->clks_count; i++) {
+		/* clk must implement determine_rate to deal with min_rate */
+		ret = clk_set_rate(requested_opp->clks[i], 0);
+		if (ret)
+			dev_err(desc->dev, "failed to request min rate\n");
+	}
+
+	/* Read back for debugging: */
+	for (i = 0; i < requested_opp->clks_count; i++) {
+		unsigned long new_rate, round_zero_rate;
+		new_rate = clk_get_rate(requested_opp->clks[i]);
+		round_zero_rate = clk_round_rate(requested_opp->clks[i], 0);
+		dev_dbg(desc->dev, "clk[%d] rate %lu round0 %lu set_min %lu\n",
+				i, new_rate, round_zero_rate,
+				requested_opp->rates[i]);
+	}
+
+	return 0;
+}
+
+static int busfreq_set_opp(struct busfreq_icc_desc *desc,
+			   struct busfreq_opp *requested_opp)
+{
+	int ret;
+
+	mutex_lock(&desc->mutex);
+	ret = busfreq_set_opp_no_lock(desc, requested_opp);
+	mutex_unlock(&desc->mutex);
+
+	return ret;
+}
+
+static int busfreq_opp_bw_gt(struct busfreq_opp_node *opp_node,
+			      u32 avg_bw, u32 peak_bw)
+{
+	if (!opp_node)
+		return 0;
+	if (opp_node->min_avg_bw == BUSFREQ_UNDEFINED_BW) {
+		if (avg_bw)
+			return 2;
+		else
+			return 1;
+	}
+	if (opp_node->min_peak_bw == BUSFREQ_UNDEFINED_BW) {
+		if (peak_bw)
+			return 2;
+		else
+			return 1;
+	}
+	if (avg_bw >= opp_node->min_avg_bw)
+		return 1;
+	if (peak_bw >= opp_node->min_peak_bw)
+		return 1;
+	return 0;
+}
+
+static struct busfreq_opp *busfreq_cmp_bw_opp(struct busfreq_icc_desc *desc,
+					      struct busfreq_opp *opp1,
+					      struct busfreq_opp *opp2)
+{
+	int i;
+	int opp1_valid;
+	int opp2_valid;
+	int opp1_count = 0;
+	int opp2_count = 0;
+
+	if (!opp1 && !opp2)
+		return desc->current_opp;
+
+	if (!opp1)
+		return opp2;
+
+	if (!opp2)
+		return opp1;
+
+	if (opp1 == opp2)
+		return opp1;
+
+	for (i = 0; i < opp1->nodes_count; i++) {
+		struct busfreq_opp_node *opp_node1, *opp_node2;
+		struct icc_node *icc_node;
+		u32 avg_bw;
+		u32 peak_bw;
+
+		opp_node1 = &opp1->nodes[i];
+		opp_node2 = &opp2->nodes[i];
+		icc_node = opp_node1->icc_node;
+		avg_bw = icc_node->avg_bw;
+		peak_bw = icc_node->peak_bw;
+
+		opp1_valid = busfreq_opp_bw_gt(opp_node1, avg_bw, peak_bw);
+		opp2_valid = busfreq_opp_bw_gt(opp_node2, avg_bw, peak_bw);
+
+		if (opp1_valid == opp2_valid && opp1_valid == 1) {
+			if (opp_node1->min_avg_bw > opp_node2->min_avg_bw &&
+				opp_node1->min_avg_bw != BUSFREQ_UNDEFINED_BW)
+				opp1_valid++;
+			if (opp_node1->min_avg_bw < opp_node2->min_avg_bw &&
+				opp_node2->min_avg_bw != BUSFREQ_UNDEFINED_BW)
+				opp2_valid++;
+		}
+
+		opp1_count += opp1_valid;
+		opp2_count += opp2_valid;
+	}
+
+	if (opp1_count > opp2_count)
+		return opp1;
+	if (opp1_count < opp2_count)
+		return opp2;
+	return opp1->perf_level >= opp2->perf_level ? opp2 : opp1;
+}
+
+static int busfreq_set_best_opp(struct busfreq_icc_desc *desc)
+{
+	struct busfreq_opp *opp, *best_opp = desc->current_opp;
+
+	list_for_each_entry(opp, &desc->opps, node)
+		best_opp = busfreq_cmp_bw_opp(desc, opp, best_opp);
+	return busfreq_set_opp(desc, best_opp);
+}
+
+static int busfreq_set_locked_opp(struct busfreq_icc_desc *desc,
+				  struct busfreq_opp *requested_opp)
+{
+	int ret;
+
+	mutex_lock(&desc->mutex);
+	ret = busfreq_set_opp_no_lock(desc, requested_opp);
+	if (ret)
+		goto err;
+	desc->opp_locked = true;
+err:
+	mutex_unlock(&desc->mutex);
+
+	return ret;
+}
+
+static int busfreq_unlock_opp(struct busfreq_icc_desc *desc)
+{
+	mutex_lock(&desc->mutex);
+	desc->opp_locked = false;
+	mutex_unlock(&desc->mutex);
+
+	return busfreq_set_best_opp(desc);
+}
+
+static int busfreq_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	struct busfreq_icc_desc *desc = src->provider->data;
+
+	if (!dst->num_links)
+		return busfreq_set_best_opp(desc);
+
+	return 0;
+}
+
+static int busfreq_pm_notify(struct notifier_block *nb, unsigned long event,
+			     void *ptr)
+{
+	struct busfreq_icc_desc *desc;
+
+	desc = container_of(nb, struct busfreq_icc_desc, pm_notifier);
+	if (event == PM_SUSPEND_PREPARE)
+		busfreq_set_locked_opp(desc, desc->default_opp);
+	else if (event == PM_POST_SUSPEND)
+		busfreq_unlock_opp(desc);
+
+	return NOTIFY_OK;
+}
+
+static int busfreq_reboot_notify(struct notifier_block *nb, unsigned long event,
+				 void *ptr)
+{
+	struct busfreq_icc_desc *desc;
+
+	desc = container_of(nb, struct busfreq_icc_desc, reboot_notifier);
+	busfreq_set_locked_opp(desc, desc->default_opp);
+
+	return NOTIFY_OK;
+}
+
+static struct icc_node *busfreq_icc_node_add(struct icc_provider *provider,
+					     int id, const char *name)
+{
+	struct busfreq_icc_desc *desc = provider->data;
+	struct device *dev = desc->dev;
+	struct icc_node *icc_node;
+
+	icc_node = icc_node_create(id);
+	if (IS_ERR(icc_node)) {
+		dev_err(dev, "Failed to create node %d\n", id);
+		return icc_node;
+	}
+
+	if (icc_node->data)
+		return icc_node;
+
+	icc_node->name = name;
+	icc_node->data = &icc_node;
+	icc_node_add(icc_node, provider);
+
+	return icc_node;
+}
+
+static struct icc_node *busfreq_icc_node_get(struct icc_provider *provider,
+					     int id)
+{
+	return busfreq_icc_node_add(provider, id, NULL);
+}
+
+static void busfreq_unregister_nodes(struct icc_provider *provider)
+{
+	struct icc_node *icc_node, *tmp;
+
+	list_for_each_entry_safe(icc_node, tmp, &provider->nodes, node_list)
+		icc_node_destroy(icc_node->id);
+}
+
+static int busfreq_register_nodes(struct icc_provider *provider,
+				  struct busfreq_icc_node *busfreq_nodes,
+				  int count)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct icc_node *icc_node;
+		size_t j;
+
+		icc_node = busfreq_icc_node_add(provider,
+						busfreq_nodes[i].id,
+						busfreq_nodes[i].name);
+		if (IS_ERR(icc_node)) {
+			ret = PTR_ERR(icc_node);
+			goto err;
+		}
+
+		for (j = 0; j < busfreq_nodes[i].num_links; j++)
+			icc_link_create(icc_node, busfreq_nodes[i].links[j]);
+	}
+
+	return 0;
+
+err:
+	busfreq_unregister_nodes(provider);
+
+	return ret;
+}
+
+static struct busfreq_opp *busfreq_opp_alloc(struct icc_provider *provider,
+					     int count)
+{
+	struct busfreq_icc_desc *desc = provider->data;
+	struct device *dev = desc->dev;
+	struct busfreq_opp *opp;
+	struct icc_node *icc_node;
+
+	opp = devm_kzalloc(dev, sizeof(*opp), GFP_KERNEL);
+	if (!opp)
+		return ERR_PTR(-ENOMEM);
+
+	opp->clks_count = count;
+	opp->clks = devm_kzalloc(dev, sizeof(struct clk *) * count, GFP_KERNEL);
+	if (!opp->clks)
+		return ERR_PTR(-ENOMEM);
+
+	opp->rates = devm_kzalloc(dev, sizeof(unsigned long) * count, GFP_KERNEL);
+	if (!opp->rates)
+		return ERR_PTR(-ENOMEM);
+
+	count = 0;
+	list_for_each_entry(icc_node, &provider->nodes, node_list)
+		count++;
+
+	opp->nodes = devm_kzalloc(dev, sizeof(*opp->nodes) * count, GFP_KERNEL);
+	if (!opp->nodes)
+		return ERR_PTR(-ENOMEM);
+	opp->nodes_count = count;
+
+	return opp;
+}
+
+static int busfreq_init_opp(struct icc_provider *provider,
+			    struct busfreq_opp *opp,
+			    struct busfreq_plat_opp *plat_opp)
+{
+	struct busfreq_icc_desc *desc = provider->data;
+	struct device *dev = desc->dev;
+	struct busfreq_opp_node *node;
+	struct icc_node *icc_node;
+	int i, j;
+
+	opp->perf_level = 0;
+	for (i = 0; i < opp->clks_count; i++) {
+		opp->clks[i] = devm_clk_get(dev, plat_opp->clks[i].name);
+		if (IS_ERR(opp->clks[i])) {
+			dev_err(dev, "Failed to get clock %s\n",
+				plat_opp->clks[i].name);
+			return PTR_ERR(opp->clks[i]);
+		}
+		opp->rates[i] = plat_opp->clks[i].rate;
+		opp->perf_level += opp->rates[i] >> 10;
+	}
+
+	i = 0;
+	list_for_each_entry(icc_node, &provider->nodes, node_list) {
+		node = &opp->nodes[i++];
+		node->icc_node = icc_node;
+	}
+
+	for (i = 0; i < plat_opp->nodes_count; i++) {
+		icc_node = busfreq_icc_node_get(provider,
+						plat_opp->nodes[i].id);
+		if (IS_ERR_OR_NULL(icc_node))
+			return -EINVAL;
+
+		for (j = 0, node = &opp->nodes[j]; j < opp->nodes_count;
+			j++, node = &opp->nodes[j]) {
+			if (node->icc_node == icc_node) {
+				node->min_avg_bw = BUSFREQ_UNDEFINED_BW;
+				node->min_peak_bw = BUSFREQ_UNDEFINED_BW;
+			}
+		}
+	}
+
+	INIT_LIST_HEAD(&opp->node);
+
+	return 0;
+}
+
+static int busfreq_register_opps(struct device *dev,
+				 struct icc_provider *provider,
+				 struct busfreq_plat_opp *busfreq_opps,
+				 int count)
+{
+	struct busfreq_icc_desc *desc = provider->data;
+	struct busfreq_opp *opp;
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		opp = busfreq_opp_alloc(provider, busfreq_opps[i].clks_count);
+		if (IS_ERR(opp))
+			return PTR_ERR(opp);
+
+		ret = busfreq_init_opp(provider, opp, &busfreq_opps[i]);
+		if (ret)
+			return ret;
+
+		if (busfreq_opps[i].default_opp)
+			desc->default_opp = opp;
+
+		list_add(&opp->node, &desc->opps);
+	}
+
+	return 0;
+}
+
+static void busfreq_unregister_opps(struct icc_provider *provider)
+{
+	struct busfreq_icc_desc *desc = provider->data;
+	struct device *dev = desc->dev;
+	struct busfreq_opp *opp, *tmp_opp;
+
+	list_for_each_entry_safe(opp, tmp_opp, &desc->opps, node) {
+		list_del(&opp->node);
+		devm_kfree(dev, opp->nodes);
+		devm_kfree(dev, opp->clks);
+		devm_kfree(dev, opp->rates);
+		devm_kfree(dev, opp);
+	}
+}
+
+int busfreq_register(struct platform_device *pdev,
+		     struct busfreq_icc_node *busfreq_nodes, int nodes_count,
+		     struct busfreq_plat_opp *busfreq_opps, int opps_count)
+{
+	struct device *dev = &pdev->dev;
+	struct busfreq_icc_desc *desc;
+	struct icc_provider *provider;
+	int ret;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	desc->dev = dev;
+	mutex_init(&desc->mutex);
+	INIT_LIST_HEAD(&desc->opps);
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+	provider->set = busfreq_icc_set;
+	provider->aggregate = busfreq_icc_aggregate;
+	provider->xlate = busfreq_icc_xlate;
+	provider->data = desc;
+	provider->dev = dev;
+	platform_set_drvdata(pdev, provider);
+
+	ret = icc_provider_add(provider);
+	if (ret) {
+		dev_err(dev, "error adding interconnect provider\n");
+		return ret;
+	}
+
+	ret = busfreq_register_nodes(provider, busfreq_nodes, nodes_count);
+	if (ret) {
+		dev_err(dev, "error adding interconnect nodes\n");
+		goto provider_del;
+	}
+
+	ret = busfreq_register_opps(dev, provider, busfreq_opps, opps_count);
+	if (ret) {
+		dev_err(dev, "error adding busfreq opp\n");
+		goto unregister_nodes;
+	}
+
+	ret = busfreq_set_opp(desc, desc->default_opp);
+	if (ret)
+		goto unregister_opps;
+
+	desc->pm_notifier.notifier_call = busfreq_pm_notify;
+	register_pm_notifier(&desc->pm_notifier);
+
+	desc->reboot_notifier.notifier_call = busfreq_reboot_notify;
+	register_reboot_notifier(&desc->reboot_notifier);
+
+	return 0;
+
+unregister_opps:
+	busfreq_unregister_opps(provider);
+unregister_nodes:
+	busfreq_unregister_nodes(provider);
+provider_del:
+	icc_provider_del(provider);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(busfreq_register);
+
+int busfreq_unregister(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+	struct busfreq_icc_desc *desc = provider->data;
+	int ret;
+
+	unregister_reboot_notifier(&desc->reboot_notifier);
+	unregister_pm_notifier(&desc->pm_notifier);
+
+	ret = busfreq_set_opp(desc, desc->default_opp);
+	if (ret)
+		return ret;
+
+	icc_provider_del(provider);
+
+	busfreq_unregister_opps(provider);
+	busfreq_unregister_nodes(provider);
+	devm_kfree(&pdev->dev, desc);
+	devm_kfree(&pdev->dev, provider);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(busfreq_unregister);
diff --git a/drivers/interconnect/imx/busfreq.h b/drivers/interconnect/imx/busfreq.h
new file mode 100644
index 000000000000..a60481f10500
--- /dev/null
+++ b/drivers/interconnect/imx/busfreq.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+#ifndef __BUSFREQ_H
+#define __BUSFREQ_H
+
+#include <linux/kernel.h>
+
+#define BUSFREQ_MAX_LINKS	32
+#define BUSFREQ_UNDEFINED_BW	0xffffffff
+
+/*
+ * struct busfreq_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 busfreq_icc_node {
+	char *name;
+	u16 id;
+	u16 links[BUSFREQ_MAX_LINKS];
+	u16 num_links;
+};
+
+/*
+ * struct busfreq_opp_clk - Clock name and rate to set for an opp
+ * @name: name of clock
+ * @rate: the rate to set when the opp is enabled
+ */
+struct busfreq_opp_clk {
+	char *name;
+	unsigned long rate;
+};
+
+/*
+ * struct busfreq_opp_bw - Describe a condition to meet to enable an opp
+ * @id: id of the node to test
+ * @avg_bw: minimum average bandwidth required to enable the opp, or
+ *          ignored if set to BUSFREQ_UNDEFINED_BW
+ * @peak_bw: minimum peak bandwidth required to enable the opp, or
+ *           ignored if set to BUSFREQ_UNDEFINED_BW
+ */
+struct busfreq_opp_bw {
+	u16 id;
+	u32 avg_bw;
+	u32 peak_bw;
+};
+
+/*
+ * struct busfreq_plat_opp - Describe an opp to register in busfreq
+ * @clks: array of clocks to configure when the opp is enable
+ * @clks_count: number of clocks
+ * @nodes: array of opp nodes (condition to meet for each node to enable opp)
+ * @nodes_count: number of nodes
+ * @default_opp: use this opp as default opp if true
+ */
+struct busfreq_plat_opp {
+	struct busfreq_opp_clk *clks;
+	int clks_count;
+	struct busfreq_opp_bw *nodes;
+	int nodes_count;
+	bool default_opp;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _numlinks, ...)	\
+	{							\
+		.id = _id,					\
+		.name = _name,					\
+		.num_links = _numlinks,				\
+		.links = { __VA_ARGS__ },			\
+	}
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id)			\
+	DEFINE_BUS_INTERCONNECT(_name, _id, 1, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, 0)
+
+#define DEFINE_OPP_CLOCK(_name, _rate)				\
+	{							\
+		.name = _name,					\
+		.rate = _rate,					\
+	}
+
+#define DEFINE_OPP_BW(_id, _avg, _peak)				\
+	{							\
+		.id = _id,					\
+		.avg_bw = _avg,					\
+		.peak_bw = _peak,				\
+	}
+
+#define DEFINE_OPP_NODE(_id)					\
+	DEFINE_OPP_BW(_id, BUSFREQ_UNDEFINED_BW, BUSFREQ_UNDEFINED_BW)
+
+#define DEFINE_OPP(_clks, _nodes, _default)				\
+	{							\
+		.clks = _clks,					\
+		.clks_count = ARRAY_SIZE(_clks),		\
+		.nodes = _nodes,				\
+		.nodes_count = ARRAY_SIZE(_nodes),		\
+		.default_opp = _default,			\
+	}
+
+#define DEFINE_OPP_NO_NODES(_clks, _default)				\
+	{							\
+		.clks = _clks,					\
+		.clks_count = ARRAY_SIZE(_clks),		\
+		.nodes = NULL,					\
+		.nodes_count = 0,				\
+		.default_opp = _default,			\
+	}
+
+int busfreq_register(struct platform_device *pdev,
+		     struct busfreq_icc_node *busfreq_nodes, int nodes_count,
+		     struct busfreq_plat_opp *busfreq_opps, int opps_count);
+int busfreq_unregister(struct platform_device *pdev);
+
+#endif /* __BUSFREQ_H */
-- 
2.17.1


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

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

[Leonard Crestez]

From: Alexandre Bailon <abailon@baylibre.com>

This adds a platform driver for the i.MX8MM SoC.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---

Changes by Leonard:
* Single DRAM clk
* Change compat string to imx8mm-interconnect
---
 drivers/interconnect/imx/Kconfig          |   4 +
 drivers/interconnect/imx/Makefile         |   1 +
 drivers/interconnect/imx/busfreq-imx8mm.c | 151 ++++++++++++++++++++++
 include/dt-bindings/interconnect/imx8mm.h |  49 +++++++
 4 files changed, 205 insertions(+)
 create mode 100644 drivers/interconnect/imx/busfreq-imx8mm.c
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index afd7b71bbd82..b569d40e5ca0 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -9,5 +9,9 @@ config BUSFREQ
 	depends on INTERCONNECT_IMX
 	help
 	  A generic interconnect driver that could be used for any i.MX.
 	  This provides a way to register master and slave and some opp
 	  to use when one or more master are in use.
+
+config BUSFREQ_IMX8MM
+	bool "i.MX8MM busfreq driver"
+	depends on BUSFREQ
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index fea647183815..a92fea6e042d 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_BUSFREQ) += busfreq.o
+obj-$(CONFIG_BUSFREQ_IMX8MM) += busfreq-imx8mm.o
diff --git a/drivers/interconnect/imx/busfreq-imx8mm.c b/drivers/interconnect/imx/busfreq-imx8mm.c
new file mode 100644
index 000000000000..49ffe230637d
--- /dev/null
+++ b/drivers/interconnect/imx/busfreq-imx8mm.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.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 "busfreq.h"
+
+/*
+ * 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 busfreq_icc_node imx8mm_icc_nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, 2,
+			IMX8MM_ICS_DRAM,
+			IMX8MM_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM),
+	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, 1, 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, 1, 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, 1, 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, 1, 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, 1, IMX8MM_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, 1, 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, 2,
+			IMX8MM_ICN_NOC,
+			IMX8MM_ICS_OCRAM),
+};
+
+static struct busfreq_opp_clk imx8mm_low_freq_clks[] = {
+	DEFINE_OPP_CLOCK("dram", 25000000),
+	DEFINE_OPP_CLOCK("noc", 150000000),
+	DEFINE_OPP_CLOCK("ahb", 22222222),
+	DEFINE_OPP_CLOCK("axi", 24000000),
+};
+
+static struct busfreq_opp_clk imx8mm_audio_freq_clks[] = {
+	DEFINE_OPP_CLOCK("dram", 100000000),
+	DEFINE_OPP_CLOCK("noc", 150000000),
+	DEFINE_OPP_CLOCK("ahb", 22222222),
+	DEFINE_OPP_CLOCK("axi", 24000000),
+};
+
+static struct busfreq_opp_bw imx8mm_audio_freq_nodes[] = {
+	DEFINE_OPP_NODE(IMX8MM_ICM_SDMA2),
+	DEFINE_OPP_NODE(IMX8MM_ICM_SDMA3),
+};
+
+static struct busfreq_opp_clk imx8mm_high_freq_clks[] = {
+	DEFINE_OPP_CLOCK("dram", 750000000),
+	DEFINE_OPP_CLOCK("noc", 750000000),
+	DEFINE_OPP_CLOCK("ahb", 133333333),
+	DEFINE_OPP_CLOCK("axi", 333333333),
+};
+
+static struct busfreq_opp_bw imx8mm_high_freq_nodes[] = {
+	DEFINE_OPP_NODE(IMX8MM_ICM_SDMA2),
+	DEFINE_OPP_NODE(IMX8MM_ICM_SDMA3),
+	DEFINE_OPP_NODE(IMX8MM_ICM_VPU_H1),
+	DEFINE_OPP_NODE(IMX8MM_ICM_VPU_G1),
+	DEFINE_OPP_NODE(IMX8MM_ICM_VPU_G2),
+	DEFINE_OPP_NODE(IMX8MM_ICM_GPU2D),
+	DEFINE_OPP_NODE(IMX8MM_ICM_GPU3D),
+	DEFINE_OPP_NODE(IMX8MM_ICM_CSI),
+	DEFINE_OPP_NODE(IMX8MM_ICM_LCDIF),
+};
+
+static struct busfreq_plat_opp imx8mm_opps[] = {
+	DEFINE_OPP_NO_NODES(imx8mm_low_freq_clks, false),
+	DEFINE_OPP(imx8mm_audio_freq_clks, imx8mm_audio_freq_nodes, false),
+	DEFINE_OPP(imx8mm_high_freq_clks, imx8mm_high_freq_nodes, true),
+};
+
+static int imx8mm_busfreq_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = busfreq_register(pdev, imx8mm_icc_nodes,
+			       ARRAY_SIZE(imx8mm_icc_nodes),
+			       imx8mm_opps, ARRAY_SIZE(imx8mm_opps));
+	return ret;
+}
+
+static int imx8mm_busfreq_remove(struct platform_device *pdev)
+{
+	return busfreq_unregister(pdev);
+}
+
+static const struct of_device_id busfreq_of_match[] = {
+	{ .compatible = "fsl,imx8mm-interconnect" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, busfreq_of_match);
+
+static struct platform_driver imx8mm_busfreq_driver = {
+	.probe = imx8mm_busfreq_probe,
+	.remove = imx8mm_busfreq_remove,
+	.driver = {
+		.name = "imx8mm-interconnect",
+		.of_match_table = busfreq_of_match,
+	},
+};
+
+builtin_platform_driver(imx8mm_busfreq_driver);
+MODULE_AUTHOR("Alexandre Bailon <abailon@baylibre.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/interconnect/imx8mm.h b/include/dt-bindings/interconnect/imx8mm.h
new file mode 100644
index 000000000000..293a99cbc1af
--- /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		11
+#define IMX8MM_ICM_USB2		12
+#define IMX8MM_ICM_PCIE		13
+#define IMX8MM_ICN_HSIO		14
+
+#define IMX8MM_ICM_SDMA2	15
+#define IMX8MM_ICM_SDMA3	16
+#define IMX8MM_ICN_AUDIO	17
+
+#define IMX8MM_ICM_ENET		18
+#define IMX8MM_ICN_ENET		19
+
+#define IMX8MM_ICN_MAIN		20
+#define IMX8MM_ICM_NAND		21
+#define IMX8MM_ICM_SDMA1	22
+#define IMX8MM_ICM_USDHC1	23
+#define IMX8MM_ICM_USDHC2	24
+#define IMX8MM_ICM_USDHC3	25
+
+#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

[RFCv2 6/8] devfreq: Add imx-devfreq driver

[Leonard Crestez]

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

Uses clk_set_min_rate so that other subsytems can also impose minimum
rate requests.

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

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index f3b242987fd9..3a7c6265ca2f 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -90,10 +90,20 @@ config ARM_EXYNOS_BUS_DEVFREQ
 	  Each memory bus group could contain many memoby bus block. It reads
 	  PPMU counters of memory controllers by using DEVFREQ-event device
 	  and adjusts the operating frequencies and voltages with OPP support.
 	  This does not yet operate with optimal voltages.
 
+config ARM_IMX_DEVFREQ
+	tristate "i.MX DEVFREQ Driver"
+	depends on ARCH_MXC
+	select DEVFREQ_GOV_USERSPACE
+	select PM_OPP
+	help
+	  This adds the DEVFREQ driver for the Tegra family of SoCs.
+	  It reads ACTMON counters of memory controllers and adjusts the
+	  operating frequencies and voltages with OPP support.
+
 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 338ae8440db6..c2463ed4c934 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_DEVFREQ)		+= imx-devfreq.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
 
 # DEVFREQ Event Drivers
diff --git a/drivers/devfreq/imx-devfreq.c b/drivers/devfreq/imx-devfreq.c
new file mode 100644
index 000000000000..aa8868d32f22
--- /dev/null
+++ b/drivers/devfreq/imx-devfreq.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.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_devfreq {
+	struct devfreq_dev_profile profile;
+	struct devfreq *devfreq;
+	struct clk *clk;
+};
+
+static int imx_devfreq_target(struct device *dev, unsigned long *freq, u32 flags)
+{
+	struct imx_devfreq *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);
+
+	ret = clk_set_min_rate(priv->clk, new_freq);
+        if (ret)
+		return ret;
+
+	ret = clk_set_rate(priv->clk, 0);
+	if (ret) {
+		clk_set_min_rate(priv->clk, priv->devfreq->previous_freq);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int imx_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+	struct imx_devfreq *priv = dev_get_drvdata(dev);
+
+	*freq = clk_get_rate(priv->clk);
+
+	return 0;
+}
+
+static int imx_devfreq_get_dev_status(struct device *dev,
+		struct devfreq_dev_status *stat)
+{
+	struct imx_devfreq *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_devfreq_exit(struct device *dev)
+{
+	return dev_pm_opp_of_remove_table(dev);
+}
+
+static int imx_devfreq_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_devfreq *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->devfreq)) {
+		ret = PTR_ERR(priv->devfreq);
+		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_devfreq_target;
+	priv->profile.get_dev_status = imx_devfreq_get_dev_status;
+	priv->profile.exit = imx_devfreq_exit;
+	priv->profile.get_cur_freq = imx_devfreq_get_cur_freq;
+	priv->profile.initial_freq = clk_get_rate(priv->clk);
+
+	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
+						DEVFREQ_GOV_USERSPACE,
+						NULL);
+	if (IS_ERR(priv->devfreq)) {
+		ret = PTR_ERR(priv->devfreq);
+		dev_err(dev, "failed to add devfreq device: %d\n", ret);
+		goto err_remove_table;
+	}
+
+	return 0;
+
+err_remove_table:
+	dev_pm_opp_of_remove_table(dev);
+	return ret;
+}
+
+static const struct of_device_id imx_devfreq_of_match[] = {
+	{ .compatible = "fsl,imx8mm-ddrc", },
+	{ .compatible = "fsl,imx8mm-noc", },
+	{ .compatible = "fsl,imx8mm-nic", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx_devfreq_of_match);
+
+static struct platform_driver imx_devfreq_platdrv = {
+	.probe		= imx_devfreq_probe,
+	.driver = {
+		.name	= "devfreq-imx",
+		.of_match_table = of_match_ptr(imx_devfreq_of_match),
+	},
+};
+module_platform_driver(imx_devfreq_platdrv);
+
+MODULE_DESCRIPTION("Generic i.MX bus frequency driver");
+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

Re: [RFCv2 6/8] devfreq: Add imx-devfreq driver

[Chanwoo Choi]

Hi Leonard,

On 19. 6. 28. 오후 4:39, Leonard Crestez wrote:
> Add initial support for frequency switches on pieces of the imx
> interconnect fabric.
> 
> Uses clk_set_min_rate so that other subsytems can also impose minimum
> rate requests.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/devfreq/Kconfig       |  10 +++
>  drivers/devfreq/Makefile      |   1 +
>  drivers/devfreq/imx-devfreq.c | 142 ++++++++++++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
>  create mode 100644 drivers/devfreq/imx-devfreq.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index f3b242987fd9..3a7c6265ca2f 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -90,10 +90,20 @@ config ARM_EXYNOS_BUS_DEVFREQ
>  	  Each memory bus group could contain many memoby bus block. It reads
>  	  PPMU counters of memory controllers by using DEVFREQ-event device
>  	  and adjusts the operating frequencies and voltages with OPP support.
>  	  This does not yet operate with optimal voltages.
>  
> +config ARM_IMX_DEVFREQ
> +	tristate "i.MX DEVFREQ Driver"
> +	depends on ARCH_MXC

You don't need to add 'COMPILE_TEST' as following?
	depends on ARCH_MXC || COMPILE_TEST

> +	select DEVFREQ_GOV_USERSPACE
> +	select PM_OPP
> +	help
> +	  This adds the DEVFREQ driver for the Tegra family of SoCs.
> +	  It reads ACTMON counters of memory controllers and adjusts the
> +	  operating frequencies and voltages with OPP support.

Maybe, you referred to the description of ARM_TEGRA_DEVFREQ.
You have to modify it according to the role of IMX_DEVFREQ.

> +
>  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 338ae8440db6..c2463ed4c934 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_DEVFREQ)		+= imx-devfreq.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
>  
>  # DEVFREQ Event Drivers
> diff --git a/drivers/devfreq/imx-devfreq.c b/drivers/devfreq/imx-devfreq.c
> new file mode 100644
> index 000000000000..aa8868d32f22
> --- /dev/null
> +++ b/drivers/devfreq/imx-devfreq.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 NXP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/devfreq-event.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_devfreq {
> +	struct devfreq_dev_profile profile;
> +	struct devfreq *devfreq;
> +	struct clk *clk;
> +};
> +
> +static int imx_devfreq_target(struct device *dev, unsigned long *freq, u32 flags)
> +{
> +	struct imx_devfreq *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);
> +
> +	ret = clk_set_min_rate(priv->clk, new_freq);
> +        if (ret)

Fix the indentation.

> +		return ret;
> +
> +	ret = clk_set_rate(priv->clk, 0);
> +	if (ret) {
> +		clk_set_min_rate(priv->clk, priv->devfreq->previous_freq);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> +	struct imx_devfreq *priv = dev_get_drvdata(dev);
> +
> +	*freq = clk_get_rate(priv->clk);
> +
> +	return 0;
> +}
> +
> +static int imx_devfreq_get_dev_status(struct device *dev,
> +		struct devfreq_dev_status *stat)
> +{
> +	struct imx_devfreq *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_devfreq_exit(struct device *dev)
> +{
> +	return dev_pm_opp_of_remove_table(dev);
> +}
> +
> +static int imx_devfreq_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx_devfreq *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->devfreq)) {
> +		ret = PTR_ERR(priv->devfreq);
> +		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_devfreq_target;
> +	priv->profile.get_dev_status = imx_devfreq_get_dev_status;
> +	priv->profile.exit = imx_devfreq_exit;
> +	priv->profile.get_cur_freq = imx_devfreq_get_cur_freq;
> +	priv->profile.initial_freq = clk_get_rate(priv->clk);
> +
> +	priv->devfreq = devm_devfreq_add_device(dev, &priv->profile,
> +						DEVFREQ_GOV_USERSPACE,
> +						NULL);
> +	if (IS_ERR(priv->devfreq)) {
> +		ret = PTR_ERR(priv->devfreq);
> +		dev_err(dev, "failed to add devfreq device: %d\n", ret);
> +		goto err_remove_table;
> +	}
> +
> +	return 0;
> +
> +err_remove_table:
> +	dev_pm_opp_of_remove_table(dev);
> +	return ret;
> +}
> +
> +static const struct of_device_id imx_devfreq_of_match[] = {
> +	{ .compatible = "fsl,imx8mm-ddrc", },
> +	{ .compatible = "fsl,imx8mm-noc", },
> +	{ .compatible = "fsl,imx8mm-nic", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, imx_devfreq_of_match);
> +
> +static struct platform_driver imx_devfreq_platdrv = {
> +	.probe		= imx_devfreq_probe,
> +	.driver = {
> +		.name	= "devfreq-imx",
> +		.of_match_table = of_match_ptr(imx_devfreq_of_match),
> +	},
> +};
> +module_platform_driver(imx_devfreq_platdrv);
> +
> +MODULE_DESCRIPTION("Generic i.MX bus frequency driver");
> +MODULE_LICENSE("GPL v2");
> 

Why don't you add the author information?

And when you are sending the patch, you better
to use get_maintainer.pl script to contain
the related all maintainers/reviewers.


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

[RFCv2 7/8] arm64: dts: imx8mm: Add interconnect node

[Leonard Crestez]

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 5da905c257ad..3b4b112814f7 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"
 
 / {
 	compatible = "fsl,imx8mm";
@@ -179,10 +180,20 @@
 		interrupts = <GIC_PPI 7
 			     (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
 		interrupt-affinity = <&A53_0>, <&A53_1>, <&A53_2>, <&A53_3>;
 	};
 
+	icc: interconnect {
+		compatible = "fsl,imx8mm-interconnect";
+		#interconnect-cells = <1>;
+		clocks = <&clk IMX8MM_CLK_DRAM>,
+			 <&clk IMX8MM_CLK_NOC>,
+			 <&clk IMX8MM_CLK_AHB>,
+			 <&clk IMX8MM_CLK_MAIN_AXI>;
+		clock-names = "dram", "noc", "ahb", "axi";
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Physical Secure */
 			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Physical Non-Secure */
 			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Virtual */
-- 
2.17.1


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

[RFCv2 8/8] arm64: dts: imx8mm: Add devfreq-imx nodes

[Leonard Crestez]

The imx8mm has multiple buses which can be scaled with some degree of
independence. Expose them as devfreq devices for userspace scaling.

It shouldn't be possible to get the system in a non-working state this
way. It is primarily aimed at testing and fine performance tuning.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 54 +++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 3b4b112814f7..aa9ed418652d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -121,10 +121,32 @@
 			opp-supported-hw = <0x8>, <0x7>;
 			clock-latency-ns = <150000>;
 		};
 	};
 
+	ddrc_opp_table: ddrc-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-25M {
+			opp-hz = /bits/ 64 <25000000>;
+		};
+		opp-750M {
+			opp-hz = /bits/ 64 <750000000>;
+		};
+	};
+
+	noc_opp_table: noc-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-150M {
+			opp-hz = /bits/ 64 <150000000>;
+		};
+		opp-750M {
+			opp-hz = /bits/ 64 <750000000>;
+		};
+	};
+
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0 0x80000000>;
 	};
 
@@ -748,10 +770,35 @@
 				status = "disabled";
 			};
 
 		};
 
+		pl301_main: nic@32000000 {
+			compatible = "fsl,imx8mm-nic";
+			reg = <0x32000000 0x100000>;
+			clocks = <&clk IMX8MM_CLK_MAIN_AXI>;
+		};
+
+		pl301_wakeup: nic@32100000 {
+			compatible = "fsl,imx8mm-nic";
+			reg = <0x32100000 0x100000>;
+			clocks = <&clk IMX8MM_CLK_AHB>;
+		};
+
+		pl301_enet: nic@32400000 {
+			compatible = "fsl,imx8mm-nic";
+			reg = <0x32400000 0x100000>;
+			clocks = <&clk IMX8MM_CLK_ENET_AXI>;
+		};
+
+		noc: noc@32700000 {
+			compatible = "fsl,imx8mm-noc";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MM_CLK_NOC>;
+			operating-points-v2 = <&noc_opp_table>;
+		};
+
 		aips4: bus@32c00000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x32c00000 0x32c00000 0x400000>;
@@ -835,7 +882,14 @@
 			      <0x38880000 0xc0000>; /* GICR (RD_base + SGI_base) */
 			#interrupt-cells = <3>;
 			interrupt-controller;
 			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		ddrc: dram-controller@3d400000 {
+			compatible = "fsl,imx8mm-ddrc";
+			reg = <0x3d400000 0x400000>;
+			clocks = <&clk IMX8MM_CLK_DRAM>;
+			operating-points-v2 = <&ddrc_opp_table>;
+		};
 	};
 };
-- 
2.17.1


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

Re: [RFCv2 0/8] Add imx8mm bus frequency switching

[Saravana Kannan]

Hi Leonard,

On Fri, Jun 28, 2019 at 12:40 AM Leonard Crestez
<leonard.crestez@nxp.com> wrote:
>
> This series attempts to add upstream DVFS support for imx8mm, covering dynamic
> scaling of internal buses and dram. It uses the interconnect framework for
> proactive scaling (in response to explicit bandwidth requests from devices) and
> devfreq in order expose the buses and eventually implement reactive scaling (in
> response to measuredtraffic).

I'm assuming you took a glance at my patch series [1] adding BW OPP
tables and adding devfreq support for scaling interconnect paths.

Based on looking at your patch series, I think [1] would allow you to
use ICC framework for both proactive and reactive scaling. Proactive
scaling would use the ICC framework directly. Reactive scaling would
go through devfreq (so that you can use various governors/adjust
policy) before it goes to ICC framework.

> Actual scaling is performed through the clk framework: The NOC and main NICs
> are driven by composite clks and a new 'imx8m-dram' clk is included for
> scaling dram using firmware calls.

Seems reasonable. All hardware block in the end run using a clock.

> The interconnect and devfreq parts do not communicate explicitly: they both
> just call clk_set_min_rate and the clk core picks the minimum value that can
> satisfy both. They are thus completely independent.

Two different parts not talking to each other and just setting min
rate can cause problems for some concurrency use cases. ICC framework
is explicitly designed to handle cases like this and aggregate the
needs correctly. You might want to look into that more closely.

> This is easily extensible to more members of the imx8m family, some of which
> expose more detailed controls over interconnect fabric frequencies.
>
> TODO:
> * Clarify DT bindings
> * Clarify interconnect OPP picking logic
> * Implement devfreq_event for imx8m ddrc
> * Expose more dram frequencies
>
> The clk_set_min_rate approach does not mesh very well with the OPP framework.
> Some of interconnect nodes on imx8m can run at different voltages: OPP can
> handle this well but not in response to a clk_set_min_rate from an unrelated
> subsystem. Maybe set voltage on a clk notifier?

I think if you design it something like below, it might make your life
a whole lot easier.
Hopefully the formatting looks okay on your end. The arrow going up is
just connecting devfreq to ICC.

                        Proactive -> ICC -> clk/OPP API to set freq/volt
                                      ^
                                      |
HW measure -> governor -> devfreq ----+

That way, all voltage/frequency requirements are managed cleanly by
clk/OPP frameworks. The ICC deals with aggregating all the
requirements and devfreq lets you handle reactive scaling and policy.

And in the beginning, while you get a governor going, you can use
"performance" governor for the "reactive scaling" users. That way,
your reactive paths will continue to have good performance while the
rest of the "proactive" clients change to use ICC APIs.

> Vendor tree does not support voltage switching, independent freqs for
> different parts of the fabric or any reactive scaling. I think it's important
> to pick an upstreaming approach which can support as much as possible.
>
> Feedback welcome.

If all of this makes sense, please take a look at [2] and provide your
thoughts. I've dropped a few patches from [1] to avoid confusion (too
much going on at once). I think BW OPP tables and having OPP tables
for interconnect paths will be something you'll need (if not now,
eventually) and something you can build on top of nicely.

Thanks,
Saravana

[1] - https://lore.kernel.org/lkml/20190614041733.120807-1-saravanak@google.com/
[2] - https://lore.kernel.org/lkml/20190703011020.151615-1-saravanak@google.com/



> Some objections were apparently raised to doing DRAM switch inside CLK:
> perhaps ICC should make min_freq requests to devfreq instead?
>
> Link to v1 (multiple chunks):
>  * https://patchwork.kernel.org/patch/10976897/
>  * https://patchwork.kernel.org/patch/10968303/
>  * https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=91251
>
> Also as a github branch (with few other changes):
>     https://github.com/cdleonard/linux/tree/next_imx8mm_busfreq
>
> Alexandre Bailon (2):
>   interconnect: Add generic driver for imx
>   interconnect: imx: Add platform driver for imx8mm
>
> Leonard Crestez (6):
>   clk: imx8mm: Add dram freq switch support
>   clk: imx8m-composite: Switch to determine_rate
>   arm64: dts: imx8mm: Add dram dvfs irqs to ccm node
>   devfreq: Add imx-devfreq driver
>   arm64: dts: imx8mm: Add interconnect node
>   arm64: dts: imx8mm: Add devfreq-imx nodes
>
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi |  73 +++
>  drivers/clk/imx/Makefile                  |   1 +
>  drivers/clk/imx/clk-composite-8m.c        |  34 +-
>  drivers/clk/imx/clk-imx8m-dram.c          | 357 ++++++++++++
>  drivers/clk/imx/clk-imx8mm.c              |  12 +
>  drivers/clk/imx/clk.h                     |  13 +
>  drivers/devfreq/Kconfig                   |  10 +
>  drivers/devfreq/Makefile                  |   1 +
>  drivers/devfreq/imx-devfreq.c             | 142 +++++
>  drivers/interconnect/Kconfig              |   1 +
>  drivers/interconnect/Makefile             |   1 +
>  drivers/interconnect/imx/Kconfig          |  17 +
>  drivers/interconnect/imx/Makefile         |   2 +
>  drivers/interconnect/imx/busfreq-imx8mm.c | 151 ++++++
>  drivers/interconnect/imx/busfreq.c        | 628 ++++++++++++++++++++++
>  drivers/interconnect/imx/busfreq.h        | 123 +++++
>  include/dt-bindings/clock/imx8mm-clock.h  |   4 +-
>  include/dt-bindings/interconnect/imx8mm.h |  49 ++
>  18 files changed, 1606 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/clk/imx/clk-imx8m-dram.c
>  create mode 100644 drivers/devfreq/imx-devfreq.c
>  create mode 100644 drivers/interconnect/imx/Kconfig
>  create mode 100644 drivers/interconnect/imx/Makefile
>  create mode 100644 drivers/interconnect/imx/busfreq-imx8mm.c
>  create mode 100644 drivers/interconnect/imx/busfreq.c
>  create mode 100644 drivers/interconnect/imx/busfreq.h
>  create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>
> --
> 2.17.1
>

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

Re: [RFCv2 0/8] Add imx8mm bus frequency switching

[Leonard Crestez]

On 7/4/2019 1:20 AM, Saravana Kannan wrote:

>> The interconnect and devfreq parts do not communicate explicitly: they both
>> just call clk_set_min_rate and the clk core picks the minimum value that can
>> satisfy both. They are thus completely independent.
> 
> Two different parts not talking to each other and just setting min
> rate can cause problems for some concurrency use cases. ICC framework
> is explicitly designed to handle cases like this and aggregate the
> needs correctly. You might want to look into that more closely.

The clk framework aggregates the min_rate requests from multiple 
consumers under a big "prepare_lock" so I expect it will deal with 
concurrent requests correctly. As for performance: frequency switching 
shouldn't be a fast path.

>> The clk_set_min_rate approach does not mesh very well with the OPP framework.
>> Some of interconnect nodes on imx8m can run at different voltages: OPP can
>> handle this well but not in response to a clk_set_min_rate from an unrelated
>> subsystem. Maybe set voltage on a clk notifier?
> 
> I think if you design it something like below, it might make your life
> a whole lot easier.
> Hopefully the formatting looks okay on your end. The arrow going up is
> just connecting devfreq to ICC.
> 
>                          Proactive -> ICC -> clk/OPP API to set freq/volt
>                                        ^
>                                        |
> HW measure -> governor -> devfreq ----+
> 
> That way, all voltage/frequency requirements are managed cleanly by
> clk/OPP frameworks. The ICC deals with aggregating all the
> requirements and devfreq lets you handle reactive scaling and policy.

If icc and devfreq are to directly communicate it makes more sense to do 
it backwards: ICC should set a min_freq on nodes which have a devfreq 
instance attached and devfreq should implement actual freq switching.

HW measurement is done on individual nodes while ICC deals with requests 
along a path. In particular on imx we have a performance monitor 
attached to the ddr controller and I doubt it can distinguish between 
masters so how could this be mapped usefully to an interconnect request?

As far as I understand with devfreq the ddrc node could use "ondemand" 
while the other nodes would default to the "passive" governor and run at 
predefined default ratios relative to DDRC.

> If all of this makes sense, please take a look at [2] and provide your
> thoughts. I've dropped a few patches from [1] to avoid confusion (too
> much going on at once). I think BW OPP tables and having OPP tables
> for interconnect paths will be something you'll need (if not now,
> eventually) and something you can build on top of nicely.

I found it very confusing that you're assigning BW OPP tables to 
devices. My initial understanding was that BW OPP would map "bandwidth" 
to "frequency" so BW OPPs should be assigned to links along the 
interconnect graph. But maybe what you want is to have OPPs for the BW 
values requested by devices?

Looking at the sdm845 icc provider source and it seems that those 
"peak/avg" values are actually parameters which go into a firmware 
command!? It makes sense that you want interconnect to be "below" 
devfreq since icc_provider.set maps very closely to what firmware exposes.

 > Interconnects and interconnect paths quantify their performance 
levels > in terms of bandwidth and not in terms of frequency.

On i.MX we just have a bunch of interconnect IPs for which frequencies 
can be adjusted (in hz) so the above statement doesn't really hold. It 
is up to an icc provider to convert aggregate bandwidth values to 
frequencies along the path.

--
Regards,
Leonard

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

Re: [RFCv2 0/8] Add imx8mm bus frequency switching

[Saravana Kannan]

On Wed, Jul 3, 2019 at 4:30 PM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 7/4/2019 1:20 AM, Saravana Kannan wrote:
>
> >> The interconnect and devfreq parts do not communicate explicitly: they both
> >> just call clk_set_min_rate and the clk core picks the minimum value that can
> >> satisfy both. They are thus completely independent.
> >
> > Two different parts not talking to each other and just setting min
> > rate can cause problems for some concurrency use cases. ICC framework
> > is explicitly designed to handle cases like this and aggregate the
> > needs correctly. You might want to look into that more closely.
>
> The clk framework aggregates the min_rate requests from multiple
> consumers under a big "prepare_lock" so I expect it will deal with
> concurrent requests correctly. As for performance: frequency switching
> shouldn't be a fast path.

Sorry I wasn't clear. I was not talking about locking issues or race
conditions when I said concurrency use cases. What I meant was if GPU
wants 5 GB/s and at the same time (concurrent) camera wants 5 GB/s
you'll need to configure the interconnect for 10 GB/s. If both of them
just try to set the min freq equivalent for 5 GB/s the performance
would be bad or functionality might break.

> >> The clk_set_min_rate approach does not mesh very well with the OPP framework.
> >> Some of interconnect nodes on imx8m can run at different voltages: OPP can
> >> handle this well but not in response to a clk_set_min_rate from an unrelated
> >> subsystem. Maybe set voltage on a clk notifier?
> >
> > I think if you design it something like below, it might make your life
> > a whole lot easier.
> > Hopefully the formatting looks okay on your end. The arrow going up is
> > just connecting devfreq to ICC.
> >
> >                          Proactive -> ICC -> clk/OPP API to set freq/volt
> >                                        ^
> >                                        |
> > HW measure -> governor -> devfreq ----+
> >
> > That way, all voltage/frequency requirements are managed cleanly by
> > clk/OPP frameworks. The ICC deals with aggregating all the
> > requirements and devfreq lets you handle reactive scaling and policy.
>
> If icc and devfreq are to directly communicate it makes more sense to do
> it backwards: ICC should set a min_freq on nodes which have a devfreq
> instance attached and devfreq should implement actual freq switching.
>
> HW measurement is done on individual nodes while ICC deals with requests
> along a path. In particular on imx we have a performance monitor
> attached to the ddr controller and I doubt it can distinguish between
> masters so how could this be mapped usefully to an interconnect request?

Ah, that was the missing piece for me -- you are trying to use a
central performance monitor. I see what you are trying to do.

So you are looking at system wide traffic at DDR and then using that
to scale the interconnect/DDRC. I don't know how complicated or not
the IMX interconnect topology is, so please pardon my questions. If
you are using a performance monitor at the DDR controller, why do you
need the "proactive" requests from other clients? Wouldn't the
performance monitor account for all the traffic to DDR?

> As far as I understand with devfreq the ddrc node could use "ondemand"
> while the other nodes would default to the "passive" governor and run at
> predefined default ratios relative to DDRC.

Yes, that approach would also work but I'm not sure why you need the
ICC framework in that case.

> > If all of this makes sense, please take a look at [2] and provide your
> > thoughts. I've dropped a few patches from [1] to avoid confusion (too
> > much going on at once). I think BW OPP tables and having OPP tables
> > for interconnect paths will be something you'll need (if not now,
> > eventually) and something you can build on top of nicely.
>
> I found it very confusing that you're assigning BW OPP tables to
> devices. My initial understanding was that BW OPP would map "bandwidth"
> to "frequency" so BW OPPs should be assigned to links along the
> interconnect graph. But maybe what you want is to have OPPs for the BW
> values requested by devices?

I want to have OPPs for bandwidths requested for paths. Each
interconnect node can also use BW OPPs if that makes sense for them,
but I think they'd be better served by using frequency OPPs.

> Looking at the sdm845 icc provider source and it seems that those
> "peak/avg" values are actually parameters which go into a firmware
> command!? It makes sense that you want interconnect to be "below"
> devfreq since icc_provider.set maps very closely to what firmware exposes.

Even without the firmware (it's mainly there to aggregate requests for
some system wide resources) or if interconnects are scaled directly
using clock APIs (older chips), sdm845 would still want ICC to be
below devfreq. It's because 845 doesn't try to do ICC scaling by
measuring at the DDR. Each master makes separate requests and then the
ICC aggregates and sets the frequency. They have their reasons (good
ones) for doing that.

>  > Interconnects and interconnect paths quantify their performance
> levels > in terms of bandwidth and not in terms of frequency.
>
> On i.MX we just have a bunch of interconnect IPs for which frequencies
> can be adjusted (in hz)

This is true for every chip. In the end, they all set the Hz of hardware IPs.

> so the above statement doesn't really hold. It
> is up to an icc provider to convert aggregate bandwidth values to
> frequencies along the path.

That's the job of every ICC provider. In the case of i.MX you use
clock APIs and in the case of sdm845 they use some complicated
firmware interface. But in both cases, when the system is tuned for
performance/power everyone in the end cares about frequency and
voltage. If the frequency goes too high, they might reduce the
bandwidth to make sure the frequency remains reasonable for whatever
use case they are profiling.

I think the main difference is where the performance monitoring or
performance decision is done. If you don't have a central performance
monitor or don't want to use one, then the policy decision (which is
where devfreq fits in) will have to happen at the bandwidth decision
level.

-Saravana

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

Re: [RFCv2 0/8] Add imx8mm bus frequency switching

[Leonard Crestez]

On 7/4/2019 6:03 AM, Saravana Kannan wrote:
> On Wed, Jul 3, 2019 at 4:30 PM Leonard Crestez <leonard.crestez@nxp.com> wrote:

>> On 7/4/2019 1:20 AM, Saravana Kannan wrote:
>>
>>>> The interconnect and devfreq parts do not communicate explicitly: they both
>>>> just call clk_set_min_rate and the clk core picks the minimum value that can
>>>> satisfy both. They are thus completely independent.
>>>
>>> Two different parts not talking to each other and just setting min
>>> rate can cause problems for some concurrency use cases. ICC framework
>>> is explicitly designed to handle cases like this and aggregate the
>>> needs correctly. You might want to look into that more closely.
>>
>> The clk framework aggregates the min_rate requests from multiple
>> consumers under a big "prepare_lock" so I expect it will deal with
>> concurrent requests correctly. As for performance: frequency switching
>> shouldn't be a fast path.
> 
> Sorry I wasn't clear. I was not talking about locking issues or race
> conditions when I said concurrency use cases. What I meant was if GPU
> wants 5 GB/s and at the same time (concurrent) camera wants 5 GB/s
> you'll need to configure the interconnect for 10 GB/s. If both of them
> just try to set the min freq equivalent for 5 GB/s the performance
> would be bad or functionality might break.

I'm not calling clk_set_min_rate independently for each icc path, that 
would be obviously broken. The interconnect framework is still used to 
aggregate bandwith requests and in your scenario clk_set_min_rate for 
the main NOC would be called in a way that meets the combined 10 GB/s 
requests.

It is devfreq which calls clk_set_min_rate independently of 
interconnect, this results in CLK performing the final aggregation 
between the "proactive" and "reactive" scaling.

>>>> The clk_set_min_rate approach does not mesh very well with the OPP framework.
>>>> Some of interconnect nodes on imx8m can run at different voltages: OPP can
>>>> handle this well but not in response to a clk_set_min_rate from an unrelated
>>>> subsystem. Maybe set voltage on a clk notifier?
>>>
>>> I think if you design it something like below, it might make your life
>>> a whole lot easier.
>>> Hopefully the formatting looks okay on your end. The arrow going up is
>>> just connecting devfreq to ICC.
>>>
>>>                           Proactive -> ICC -> clk/OPP API to set freq/volt
>>>                                         ^
>>>                                         |
>>> HW measure -> governor -> devfreq ----+
>>>
>>> That way, all voltage/frequency requirements are managed cleanly by
>>> clk/OPP frameworks. The ICC deals with aggregating all the
>>> requirements and devfreq lets you handle reactive scaling and policy.
>>
>> If icc and devfreq are to directly communicate it makes more sense to do
>> it backwards: ICC should set a min_freq on nodes which have a devfreq
>> instance attached and devfreq should implement actual freq switching.
>>
>> HW measurement is done on individual nodes while ICC deals with requests
>> along a path. In particular on imx we have a performance monitor
>> attached to the ddr controller and I doubt it can distinguish between
>> masters so how could this be mapped usefully to an interconnect request?
> 
> Ah, that was the missing piece for me -- you are trying to use a
> central performance monitor. I see what you are trying to do.
> 
> So you are looking at system wide traffic at DDR and then using that
> to scale the interconnect/DDRC. I don't know how complicated or not
> the IMX interconnect topology is, so please pardon my questions. If
> you are using a performance monitor at the DDR controller, why do you
> need the "proactive" requests from other clients? Wouldn't the
> performance monitor account for all the traffic to DDR?

Reactive scaling is too slow to ramp-up in media playback scenarios and 
first few frames would fail.

>> As far as I understand with devfreq the ddrc node could use "ondemand"
>> while the other nodes would default to the "passive" governor and run at
>> predefined default ratios relative to DDRC.
> 
> Yes, that approach would also work but I'm not sure why you need the
> ICC framework in that case.

For proactive scaling: to ensure bandwidth *before* traffic starts. In 
imx vendor tree that's all that's implemented; for reactive scaling we 
just set busfreq to high as soon as cpu leaves min opp.

IMX interconnect topology is not very complex so mechanisms other than 
interconnect could be used. But ICC is the most powerful and expressive 
subsystem for proactive requests.

>>> If all of this makes sense, please take a look at [2] and provide your
>>> thoughts. I've dropped a few patches from [1] to avoid confusion (too
>>> much going on at once). I think BW OPP tables and having OPP tables
>>> for interconnect paths will be something you'll need (if not now,
>>> eventually) and something you can build on top of nicely.
>>
>> I found it very confusing that you're assigning BW OPP tables to
>> devices. My initial understanding was that BW OPP would map "bandwidth"
>> to "frequency" so BW OPPs should be assigned to links along the
>> interconnect graph. But maybe what you want is to have OPPs for the BW
>> values requested by devices?
> 
> I want to have OPPs for bandwidths requested for paths.

Right, this was not very obvious.

> Each interconnect node can also use BW OPPs if that makes sense for them,
> but I think they'd be better served by using frequency OPPs.

Each interconnect node is asked by the framework to ensure a certain BW 
is available in "bytes". The nodes could use OPPs with BW values to map 
the icc request to a frequency in "hz".

>> Looking at the sdm845 icc provider source and it seems that those
>> "peak/avg" values are actually parameters which go into a firmware
>> command!? It makes sense that you want interconnect to be "below"
>> devfreq since icc_provider.set maps very closely to what firmware exposes.
> 
> Even without the firmware (it's mainly there to aggregate requests for
> some system wide resources) or if interconnects are scaled directly
> using clock APIs (older chips), sdm845 would still want ICC to be
> below devfreq. It's because 845 doesn't try to do ICC scaling by
> measuring at the DDR. Each master makes separate requests and then the
> ICC aggregates and sets the frequency. They have their reasons (good
> ones) for doing that.

Maybe I'm confused about how devfreq is used in your scenario: you have 
devices which have their own OPPs (like a GPU) and expose this via 
devfreq. Then for each GPU OPP you want to pick a the BW value to 
request from interconnect, right?

My idea was to use devfreq *after* icc so that you can do stuff like 
force a certain NOC to "max" via echo "performance" > $sysfs/governor. 
It also allows encapsulating complex freq switching (clk maintainers 
don't seem to like my dram clk).

On a second examination there is no actual incompatibility here, devfreq 
could be used both below and above ICC.

--
Regards,
Leonard

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