All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv3 0/3] interconnect: Add imx support via devfreq
@ 2019-08-06 10:55 ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

This series add imx support for interconnect via devfreq: the ICC
framework is used to aggregate requests from devices and then those are
converted to "min_freq" requests to devfreq.

The dev_pm_qos API is used to request min frequencies from devfreq, it
relies on this patch: https://patchwork.kernel.org/patch/11078475/

This is greatly reduced compared to previous submissions:

 * Relying on devfreq and dev_pm_qos instead of CLK allows a wider range
of implementations for scaling individual nodes. In particular it's no
longer required to force DDR scaling into CLK.
 * No more "platform opp" stuff: forcing everything to scale together
loses many of the benefits of ICC. The devfreq "passive" governor can
still be used to tie some pieces of the fabric together.
 * No more suspend/resume handling: devfreq uses OPP framework which
already supports "suspend-opp".
 * Replace all mentions of "busfreq" with "interconnect"

Link to v2: https://patchwork.kernel.org/patch/11056789/

The per-soc platform drivers still need to defined the interconnect
graph and links to devfreq (by name) as well as bus widths. For example
some of the 128-bit buses on imx8m mini are 64-bit on imx8m nano.

There was another recent series tying icc and devfreq, for exynos:
 * https://lkml.org/lkml/2019/7/23/394

That series defines the interconnect graph/tree entirely inside
devicetree but that seems limiting. I'd rather keep the graph in a SOC
driver and #define ids for all bus masters/slaves. This way if USB1 and
USB2 are on the same bus they can still be treated differently.

One interesting idea from there is to turn devfreq nodes into
interconnect providers, this can avoid the need for a "virtual" ICC node
in devicetree. I expect devicetree folks will object to that?

Leonard Crestez (3):
  dt-bindings: interconnect: Add bindings for imx
  interconnect: Add imx core driver
  interconnect: imx: Add platform driver for imx8mm

 .../devicetree/bindings/interconnect/imx.yaml |  38 +++
 drivers/interconnect/Kconfig                  |   1 +
 drivers/interconnect/Makefile                 |   1 +
 drivers/interconnect/imx/Kconfig              |   9 +
 drivers/interconnect/imx/Makefile             |   2 +
 drivers/interconnect/imx/imx.c                | 258 ++++++++++++++++++
 drivers/interconnect/imx/imx.h                |  62 +++++
 drivers/interconnect/imx/imx8mm.c             | 114 ++++++++
 include/dt-bindings/interconnect/imx8mm.h     |  49 ++++
 9 files changed, 534 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/imx.c
 create mode 100644 drivers/interconnect/imx/imx.h
 create mode 100644 drivers/interconnect/imx/imx8mm.c
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h

-- 
2.17.1

^ permalink raw reply	[flat|nested] 40+ messages in thread
* [RFCv3 0/3] interconnect: Add imx support via devfreq
@ 2019-08-06 10:55 ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

This series add imx support for interconnect via devfreq: the ICC
framework is used to aggregate requests from devices and then those are
converted to "min_freq" requests to devfreq.

The dev_pm_qos API is used to request min frequencies from devfreq, it
relies on this patch: https://patchwork.kernel.org/patch/11078475/

This is greatly reduced compared to previous submissions:

 * Relying on devfreq and dev_pm_qos instead of CLK allows a wider range
of implementations for scaling individual nodes. In particular it's no
longer required to force DDR scaling into CLK.
 * No more "platform opp" stuff: forcing everything to scale together
loses many of the benefits of ICC. The devfreq "passive" governor can
still be used to tie some pieces of the fabric together.
 * No more suspend/resume handling: devfreq uses OPP framework which
already supports "suspend-opp".
 * Replace all mentions of "busfreq" with "interconnect"

Link to v2: https://patchwork.kernel.org/patch/11056789/

The per-soc platform drivers still need to defined the interconnect
graph and links to devfreq (by name) as well as bus widths. For example
some of the 128-bit buses on imx8m mini are 64-bit on imx8m nano.

There was another recent series tying icc and devfreq, for exynos:
 * https://lkml.org/lkml/2019/7/23/394

That series defines the interconnect graph/tree entirely inside
devicetree but that seems limiting. I'd rather keep the graph in a SOC
driver and #define ids for all bus masters/slaves. This way if USB1 and
USB2 are on the same bus they can still be treated differently.

One interesting idea from there is to turn devfreq nodes into
interconnect providers, this can avoid the need for a "virtual" ICC node
in devicetree. I expect devicetree folks will object to that?

Leonard Crestez (3):
  dt-bindings: interconnect: Add bindings for imx
  interconnect: Add imx core driver
  interconnect: imx: Add platform driver for imx8mm

 .../devicetree/bindings/interconnect/imx.yaml |  38 +++
 drivers/interconnect/Kconfig                  |   1 +
 drivers/interconnect/Makefile                 |   1 +
 drivers/interconnect/imx/Kconfig              |   9 +
 drivers/interconnect/imx/Makefile             |   2 +
 drivers/interconnect/imx/imx.c                | 258 ++++++++++++++++++
 drivers/interconnect/imx/imx.h                |  62 +++++
 drivers/interconnect/imx/imx8mm.c             | 114 ++++++++
 include/dt-bindings/interconnect/imx8mm.h     |  49 ++++
 9 files changed, 534 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/imx.c
 create mode 100644 drivers/interconnect/imx/imx.h
 create mode 100644 drivers/interconnect/imx/imx8mm.c
 create mode 100644 include/dt-bindings/interconnect/imx8mm.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 40+ messages in thread
* [RFCv3 0/3] interconnect: Add imx support via devfreq
@ 2019-08-06 10:55 ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

This series add imx support for interconnect via devfreq: the ICC
framework is used to aggregate requests from devices and then those are
converted to "min_freq" requests to devfreq.

The dev_pm_qos API is used to request min frequencies from devfreq, it
relies on this patch: https://patchwork.kernel.org/patch/11078475/

This is greatly reduced compared to previous submissions:

 * Relying on devfreq and dev_pm_qos instead of CLK allows a wider range
of implementations for scaling individual nodes. In particular it's no
longer required to force DDR scaling into CLK.
 * No more "platform opp" stuff: forcing everything to scale together
loses many of the benefits of ICC. The devfreq "passive" governor can
still be used to tie some pieces of the fabric together.
 * No more suspend/resume handling: devfreq uses OPP framework which
already supports "suspend-opp".
 * Replace all mentions of "busfreq" with "interconnect"

Link to v2: https://patchwork.kernel.org/patch/11056789/

The per-soc platform drivers still need to defined the interconnect
graph and links to devfreq (by name) as well as bus widths. For example
some of the 128-bit buses on imx8m mini are 64-bit on imx8m nano.

There was another recent series tying icc and devfreq, for exynos:
 * https://lkml.org/lkml/2019/7/23/394

That series defines the interconnect graph/tree entirely inside
devicetree but that seems limiting. I'd rather keep the graph in a SOC
driver and #define ids for all bus masters/slaves. This way if USB1 and
USB2 are on the same bus they can still be treated differently.

One interesting idea from there is to turn devfreq nodes into
interconnect providers, this can avoid the need for a "virtual" ICC node
in devicetree. I expect devicetree folks will object to that?

Leonard Crestez (3):
  dt-bindings: interconnect: Add bindings for imx
  interconnect: Add imx core driver
  interconnect: imx: Add platform driver for imx8mm

 .../devicetree/bindings/interconnect/imx.yaml |  38 +++
 drivers/interconnect/Kconfig                  |   1 +
 drivers/interconnect/Makefile                 |   1 +
 drivers/interconnect/imx/Kconfig              |   9 +
 drivers/interconnect/imx/Makefile             |   2 +
 drivers/interconnect/imx/imx.c                | 258 ++++++++++++++++++
 drivers/interconnect/imx/imx.h                |  62 +++++
 drivers/interconnect/imx/imx8mm.c             | 114 ++++++++
 include/dt-bindings/interconnect/imx8mm.h     |  49 ++++
 9 files changed, 534 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
 create mode 100644 drivers/interconnect/imx/Kconfig
 create mode 100644 drivers/interconnect/imx/Makefile
 create mode 100644 drivers/interconnect/imx/imx.c
 create mode 100644 drivers/interconnect/imx/imx.h
 create mode 100644 drivers/interconnect/imx/imx8mm.c
 create mode 100644 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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
  2019-08-06 10:55 ` Leonard Crestez
  (?)
@ 2019-08-06 10:55   ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
new file mode 100644
index 000000000000..c6f173b38f4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/imx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX interconnect device
+
+maintainers:
+  - Leonard Crestez <leonard.crestez@nxp.com>
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - fsl,imx8mm-interconnect
+  "#interconnect-cells":
+    const: 1
+  devfreq-names:
+    description: Names of devfreq instances for adjustable nodes
+  devfreq:
+    description: List of phandle pointing to devfreq instances
+
+required:
+  - compatible
+  - "#interconnect-cells"
+  - "devfreq-names"
+  - "devfreq"
+
+examples:
+  - |
+    #include <dt-bindings/interconnect/imx8mm.h>
+    icc: interconnect {
+        compatible = "fsl,imx8mm-interconnect";
+        #interconnect-cells = <1>;
+        devfreq-names = "dram", "noc", "axi";
+        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
+    };
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

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

diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
new file mode 100644
index 000000000000..c6f173b38f4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/imx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX interconnect device
+
+maintainers:
+  - Leonard Crestez <leonard.crestez@nxp.com>
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - fsl,imx8mm-interconnect
+  "#interconnect-cells":
+    const: 1
+  devfreq-names:
+    description: Names of devfreq instances for adjustable nodes
+  devfreq:
+    description: List of phandle pointing to devfreq instances
+
+required:
+  - compatible
+  - "#interconnect-cells"
+  - "devfreq-names"
+  - "devfreq"
+
+examples:
+  - |
+    #include <dt-bindings/interconnect/imx8mm.h>
+    icc: interconnect {
+        compatible = "fsl,imx8mm-interconnect";
+        #interconnect-cells = <1>;
+        devfreq-names = "dram", "noc", "axi";
+        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
new file mode 100644
index 000000000000..c6f173b38f4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/imx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX interconnect device
+
+maintainers:
+  - Leonard Crestez <leonard.crestez@nxp.com>
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - fsl,imx8mm-interconnect
+  "#interconnect-cells":
+    const: 1
+  devfreq-names:
+    description: Names of devfreq instances for adjustable nodes
+  devfreq:
+    description: List of phandle pointing to devfreq instances
+
+required:
+  - compatible
+  - "#interconnect-cells"
+  - "devfreq-names"
+  - "devfreq"
+
+examples:
+  - |
+    #include <dt-bindings/interconnect/imx8mm.h>
+    icc: interconnect {
+        compatible = "fsl,imx8mm-interconnect";
+        #interconnect-cells = <1>;
+        devfreq-names = "dram", "noc", "axi";
+        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
+    };
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 2/3] interconnect: Add imx core driver
  2019-08-06 10:55 ` Leonard Crestez
  (?)
@ 2019-08-06 10:55   ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

Platform drivers can describe their interconnect graph and
several adjustment knobs where an icc node bandwith converted to a
clk_min_rate request.

All adjustable nodes are assumed to be independent.

Based on an earlier work by Alexandre Bailon but greatly reduced to drop
"platform opp" support.

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

diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..e61802230f90 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -10,7 +10,8 @@ menuconfig INTERCONNECT
 	  If unsure, say no.
 
 if INTERCONNECT
 
 source "drivers/interconnect/qcom/Kconfig"
+source "drivers/interconnect/imx/Kconfig"
 
 endif
diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 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..45fbae7007af
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,5 @@
+config INTERCONNECT_IMX
+	bool "i.MX interconnect drivers"
+	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
+	help
+	  Generic interconnect driver for i.MX SOCs
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..bb92fd9fe4a5
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
new file mode 100644
index 000000000000..cc838e40419e
--- /dev/null
+++ b/drivers/interconnect/imx/imx.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/devfreq.h>
+#include <linux/of.h>
+
+#include "imx.h"
+
+/* private icc_provider data */
+struct imx_icc_provider {
+	struct device *dev;
+};
+
+/* private icc_node data */
+struct imx_icc_node {
+	const struct imx_icc_node_desc *desc;
+	struct devfreq *devfreq;
+	struct dev_pm_qos_request qos_req;
+};
+
+static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
+{
+	struct device *dev = node->provider->dev;
+	struct imx_icc_node *node_data = node->data;
+	unsigned long freq;
+
+	if (!node_data->devfreq)
+		return 0;
+
+	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
+	do_div(freq, node_data->desc->adj->bw_div);
+	if (freq > INT_MAX) {
+		dev_err(dev, "%s can't request more INT_MAX freq\n",
+				node->name);
+		return -ERANGE;
+	}
+
+	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
+			node->name, node->avg_bw, node->peak_bw, freq);
+
+	dev_pm_qos_update_request(&node_data->qos_req, freq);
+
+	return 0;
+}
+
+static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	return imx_icc_node_set(dst);
+}
+
+static int imx_icc_node_init_devfreq(struct device *dev, 
+				     struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	const struct imx_icc_node_desc *node_desc = node_data->desc;
+	int index;
+	int ret;
+
+	index = of_property_match_string(dev->of_node,
+			"devfreq-names", node_desc->adj->devfreq_name);
+	if (index < 0) {
+		dev_err(dev, "failed to match devfreq-names %s: %d\n",
+				node_desc->adj->devfreq_name, index);
+		return index;
+	}
+
+	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
+	if (IS_ERR(node_data->devfreq)) {
+		ret = PTR_ERR(node_data->devfreq);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
+					index, node_desc->adj->devfreq_name, ret);
+		return ret;
+	}
+
+	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
+			&node_data->qos_req,
+			DEV_PM_QOS_MIN_FREQUENCY, 0);
+}
+
+static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+					 const struct imx_icc_node_desc *node_desc)
+{
+	struct imx_icc_provider *provider_data = provider->data;
+	struct device *dev = provider_data->dev;
+	struct imx_icc_node *node_data;
+	struct icc_node *node;
+	int ret;
+
+	node = icc_node_create(node_desc->id);
+	if (IS_ERR(node)) {
+		dev_err(dev, "failed to create node %d\n", node_desc->id);
+		return node;
+	}
+
+	if (node->data) {
+		dev_err(dev, "already created node %s id=%d\n",
+				node_desc->name, node_desc->id);
+		return ERR_PTR(-EEXIST);
+	}
+
+	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
+	if (!node_data) {
+		icc_node_destroy(node->id);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	node->name = node_desc->name;
+	node->data = node_data;
+	node_data->desc = node_desc;
+	if (node_desc->adj) {
+		ret = imx_icc_node_init_devfreq(dev, node);
+		if (ret < 0) {
+			icc_node_destroy(node->id);
+			return ERR_PTR(ret);
+		}
+	}
+
+	icc_node_add(node, provider);
+
+	return node;
+}
+
+static void imx_icc_unregister_nodes(struct icc_provider *provider)
+{
+	struct icc_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
+		struct imx_icc_node *node_data = node->data;
+
+		icc_node_del(node);
+		icc_node_destroy(node->id);
+		if (dev_pm_qos_request_active(&node_data->qos_req))
+			dev_pm_qos_remove_request(&node_data->qos_req);
+	}
+}
+
+static int imx_icc_register_nodes(struct icc_provider *provider,
+				  const struct imx_icc_node_desc *descs,
+				  int count)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct icc_node *node;
+		const struct imx_icc_node_desc *node_desc = &descs[i];
+		size_t j;
+
+		node = imx_icc_node_add(provider, node_desc);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			if (ret != -EPROBE_DEFER)
+				dev_err(provider->dev, "failed to add %s: %d\n",
+						node_desc->name, ret);
+			goto err;
+		}
+
+		for (j = 0; j < node_desc->num_links; j++)
+			icc_link_create(node, node_desc->links[j]);
+	}
+
+	return 0;
+
+err:
+	imx_icc_unregister_nodes(provider);
+
+	return ret;
+}
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes, int nodes_count)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_icc_provider *desc;
+	struct icc_provider *provider;
+	int ret;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	desc->dev = dev;
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+	provider->set = imx_icc_set;
+	provider->aggregate = imx_icc_aggregate;
+	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
+	if (ret) {
+		dev_err(dev, "error adding interconnect nodes\n");
+		goto provider_del;
+	}
+
+	return 0;
+
+provider_del:
+	icc_provider_del(provider);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_icc_register);
+
+int imx_icc_unregister(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+
+	icc_provider_del(provider);
+	imx_icc_unregister_nodes(provider);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(imx_icc_unregister);
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
new file mode 100644
index 000000000000..ab191eb89616
--- /dev/null
+++ b/drivers/interconnect/imx/imx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+#ifndef __BUSFREQ_H
+#define __BUSFREQ_H
+
+#include <linux/kernel.h>
+
+#define IMX_ICC_MAX_LINKS	32
+#define IMX_ICC_UNDEFINED_BW	0xffffffff
+
+/*
+ * struct imx_icc_node_adj - Describe an dynamic adjustment knob
+ */
+struct imx_icc_node_adj_desc {
+	const char *devfreq_name;
+	unsigned int bw_mul, bw_div;
+};
+
+/*
+ * struct imx_icc_node - Describe an interconnect node
+ * @name: name of the node
+ * @id: an unique id to identify the node
+ * @links: an array of slaves' node id
+ * @num_links: number of id defined in links
+ */
+struct imx_icc_node_desc {
+	const char *name;
+	u16 id;
+	u16 links[IMX_ICC_MAX_LINKS];
+	u16 num_links;
+
+	const struct imx_icc_node_adj_desc *adj;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\
+	{								\
+		.id = _id,						\
+		.name = _name,						\
+		.adj = _adj,						\
+		.num_links = _numlinks,					\
+		.links = { __VA_ARGS__ },				\
+	}
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes,
+		     int nodes_count);
+int imx_icc_unregister(struct platform_device *pdev);
+
+#endif /* __BUSFREQ_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 2/3] interconnect: Add imx core driver
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

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

Platform drivers can describe their interconnect graph and
several adjustment knobs where an icc node bandwith converted to a
clk_min_rate request.

All adjustable nodes are assumed to be independent.

Based on an earlier work by Alexandre Bailon but greatly reduced to drop
"platform opp" support.

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

diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..e61802230f90 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -10,7 +10,8 @@ menuconfig INTERCONNECT
 	  If unsure, say no.
 
 if INTERCONNECT
 
 source "drivers/interconnect/qcom/Kconfig"
+source "drivers/interconnect/imx/Kconfig"
 
 endif
diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 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..45fbae7007af
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,5 @@
+config INTERCONNECT_IMX
+	bool "i.MX interconnect drivers"
+	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
+	help
+	  Generic interconnect driver for i.MX SOCs
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..bb92fd9fe4a5
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
new file mode 100644
index 000000000000..cc838e40419e
--- /dev/null
+++ b/drivers/interconnect/imx/imx.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/devfreq.h>
+#include <linux/of.h>
+
+#include "imx.h"
+
+/* private icc_provider data */
+struct imx_icc_provider {
+	struct device *dev;
+};
+
+/* private icc_node data */
+struct imx_icc_node {
+	const struct imx_icc_node_desc *desc;
+	struct devfreq *devfreq;
+	struct dev_pm_qos_request qos_req;
+};
+
+static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
+{
+	struct device *dev = node->provider->dev;
+	struct imx_icc_node *node_data = node->data;
+	unsigned long freq;
+
+	if (!node_data->devfreq)
+		return 0;
+
+	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
+	do_div(freq, node_data->desc->adj->bw_div);
+	if (freq > INT_MAX) {
+		dev_err(dev, "%s can't request more INT_MAX freq\n",
+				node->name);
+		return -ERANGE;
+	}
+
+	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
+			node->name, node->avg_bw, node->peak_bw, freq);
+
+	dev_pm_qos_update_request(&node_data->qos_req, freq);
+
+	return 0;
+}
+
+static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	return imx_icc_node_set(dst);
+}
+
+static int imx_icc_node_init_devfreq(struct device *dev, 
+				     struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	const struct imx_icc_node_desc *node_desc = node_data->desc;
+	int index;
+	int ret;
+
+	index = of_property_match_string(dev->of_node,
+			"devfreq-names", node_desc->adj->devfreq_name);
+	if (index < 0) {
+		dev_err(dev, "failed to match devfreq-names %s: %d\n",
+				node_desc->adj->devfreq_name, index);
+		return index;
+	}
+
+	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
+	if (IS_ERR(node_data->devfreq)) {
+		ret = PTR_ERR(node_data->devfreq);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
+					index, node_desc->adj->devfreq_name, ret);
+		return ret;
+	}
+
+	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
+			&node_data->qos_req,
+			DEV_PM_QOS_MIN_FREQUENCY, 0);
+}
+
+static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+					 const struct imx_icc_node_desc *node_desc)
+{
+	struct imx_icc_provider *provider_data = provider->data;
+	struct device *dev = provider_data->dev;
+	struct imx_icc_node *node_data;
+	struct icc_node *node;
+	int ret;
+
+	node = icc_node_create(node_desc->id);
+	if (IS_ERR(node)) {
+		dev_err(dev, "failed to create node %d\n", node_desc->id);
+		return node;
+	}
+
+	if (node->data) {
+		dev_err(dev, "already created node %s id=%d\n",
+				node_desc->name, node_desc->id);
+		return ERR_PTR(-EEXIST);
+	}
+
+	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
+	if (!node_data) {
+		icc_node_destroy(node->id);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	node->name = node_desc->name;
+	node->data = node_data;
+	node_data->desc = node_desc;
+	if (node_desc->adj) {
+		ret = imx_icc_node_init_devfreq(dev, node);
+		if (ret < 0) {
+			icc_node_destroy(node->id);
+			return ERR_PTR(ret);
+		}
+	}
+
+	icc_node_add(node, provider);
+
+	return node;
+}
+
+static void imx_icc_unregister_nodes(struct icc_provider *provider)
+{
+	struct icc_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
+		struct imx_icc_node *node_data = node->data;
+
+		icc_node_del(node);
+		icc_node_destroy(node->id);
+		if (dev_pm_qos_request_active(&node_data->qos_req))
+			dev_pm_qos_remove_request(&node_data->qos_req);
+	}
+}
+
+static int imx_icc_register_nodes(struct icc_provider *provider,
+				  const struct imx_icc_node_desc *descs,
+				  int count)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct icc_node *node;
+		const struct imx_icc_node_desc *node_desc = &descs[i];
+		size_t j;
+
+		node = imx_icc_node_add(provider, node_desc);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			if (ret != -EPROBE_DEFER)
+				dev_err(provider->dev, "failed to add %s: %d\n",
+						node_desc->name, ret);
+			goto err;
+		}
+
+		for (j = 0; j < node_desc->num_links; j++)
+			icc_link_create(node, node_desc->links[j]);
+	}
+
+	return 0;
+
+err:
+	imx_icc_unregister_nodes(provider);
+
+	return ret;
+}
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes, int nodes_count)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_icc_provider *desc;
+	struct icc_provider *provider;
+	int ret;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	desc->dev = dev;
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+	provider->set = imx_icc_set;
+	provider->aggregate = imx_icc_aggregate;
+	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
+	if (ret) {
+		dev_err(dev, "error adding interconnect nodes\n");
+		goto provider_del;
+	}
+
+	return 0;
+
+provider_del:
+	icc_provider_del(provider);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_icc_register);
+
+int imx_icc_unregister(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+
+	icc_provider_del(provider);
+	imx_icc_unregister_nodes(provider);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(imx_icc_unregister);
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
new file mode 100644
index 000000000000..ab191eb89616
--- /dev/null
+++ b/drivers/interconnect/imx/imx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+#ifndef __BUSFREQ_H
+#define __BUSFREQ_H
+
+#include <linux/kernel.h>
+
+#define IMX_ICC_MAX_LINKS	32
+#define IMX_ICC_UNDEFINED_BW	0xffffffff
+
+/*
+ * struct imx_icc_node_adj - Describe an dynamic adjustment knob
+ */
+struct imx_icc_node_adj_desc {
+	const char *devfreq_name;
+	unsigned int bw_mul, bw_div;
+};
+
+/*
+ * struct imx_icc_node - Describe an interconnect node
+ * @name: name of the node
+ * @id: an unique id to identify the node
+ * @links: an array of slaves' node id
+ * @num_links: number of id defined in links
+ */
+struct imx_icc_node_desc {
+	const char *name;
+	u16 id;
+	u16 links[IMX_ICC_MAX_LINKS];
+	u16 num_links;
+
+	const struct imx_icc_node_adj_desc *adj;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\
+	{								\
+		.id = _id,						\
+		.name = _name,						\
+		.adj = _adj,						\
+		.num_links = _numlinks,					\
+		.links = { __VA_ARGS__ },				\
+	}
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes,
+		     int nodes_count);
+int imx_icc_unregister(struct platform_device *pdev);
+
+#endif /* __BUSFREQ_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 2/3] interconnect: Add imx core driver
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

Platform drivers can describe their interconnect graph and
several adjustment knobs where an icc node bandwith converted to a
clk_min_rate request.

All adjustable nodes are assumed to be independent.

Based on an earlier work by Alexandre Bailon but greatly reduced to drop
"platform opp" support.

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

diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..e61802230f90 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -10,7 +10,8 @@ menuconfig INTERCONNECT
 	  If unsure, say no.
 
 if INTERCONNECT
 
 source "drivers/interconnect/qcom/Kconfig"
+source "drivers/interconnect/imx/Kconfig"
 
 endif
diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
index 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..45fbae7007af
--- /dev/null
+++ b/drivers/interconnect/imx/Kconfig
@@ -0,0 +1,5 @@
+config INTERCONNECT_IMX
+	bool "i.MX interconnect drivers"
+	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
+	help
+	  Generic interconnect driver for i.MX SOCs
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
new file mode 100644
index 000000000000..bb92fd9fe4a5
--- /dev/null
+++ b/drivers/interconnect/imx/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
new file mode 100644
index 000000000000..cc838e40419e
--- /dev/null
+++ b/drivers/interconnect/imx/imx.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/interconnect-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/devfreq.h>
+#include <linux/of.h>
+
+#include "imx.h"
+
+/* private icc_provider data */
+struct imx_icc_provider {
+	struct device *dev;
+};
+
+/* private icc_node data */
+struct imx_icc_node {
+	const struct imx_icc_node_desc *desc;
+	struct devfreq *devfreq;
+	struct dev_pm_qos_request qos_req;
+};
+
+static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
+{
+	struct device *dev = node->provider->dev;
+	struct imx_icc_node *node_data = node->data;
+	unsigned long freq;
+
+	if (!node_data->devfreq)
+		return 0;
+
+	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
+	do_div(freq, node_data->desc->adj->bw_div);
+	if (freq > INT_MAX) {
+		dev_err(dev, "%s can't request more INT_MAX freq\n",
+				node->name);
+		return -ERANGE;
+	}
+
+	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
+			node->name, node->avg_bw, node->peak_bw, freq);
+
+	dev_pm_qos_update_request(&node_data->qos_req, freq);
+
+	return 0;
+}
+
+static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	return imx_icc_node_set(dst);
+}
+
+static int imx_icc_node_init_devfreq(struct device *dev, 
+				     struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	const struct imx_icc_node_desc *node_desc = node_data->desc;
+	int index;
+	int ret;
+
+	index = of_property_match_string(dev->of_node,
+			"devfreq-names", node_desc->adj->devfreq_name);
+	if (index < 0) {
+		dev_err(dev, "failed to match devfreq-names %s: %d\n",
+				node_desc->adj->devfreq_name, index);
+		return index;
+	}
+
+	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
+	if (IS_ERR(node_data->devfreq)) {
+		ret = PTR_ERR(node_data->devfreq);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
+					index, node_desc->adj->devfreq_name, ret);
+		return ret;
+	}
+
+	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
+			&node_data->qos_req,
+			DEV_PM_QOS_MIN_FREQUENCY, 0);
+}
+
+static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+					 const struct imx_icc_node_desc *node_desc)
+{
+	struct imx_icc_provider *provider_data = provider->data;
+	struct device *dev = provider_data->dev;
+	struct imx_icc_node *node_data;
+	struct icc_node *node;
+	int ret;
+
+	node = icc_node_create(node_desc->id);
+	if (IS_ERR(node)) {
+		dev_err(dev, "failed to create node %d\n", node_desc->id);
+		return node;
+	}
+
+	if (node->data) {
+		dev_err(dev, "already created node %s id=%d\n",
+				node_desc->name, node_desc->id);
+		return ERR_PTR(-EEXIST);
+	}
+
+	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
+	if (!node_data) {
+		icc_node_destroy(node->id);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	node->name = node_desc->name;
+	node->data = node_data;
+	node_data->desc = node_desc;
+	if (node_desc->adj) {
+		ret = imx_icc_node_init_devfreq(dev, node);
+		if (ret < 0) {
+			icc_node_destroy(node->id);
+			return ERR_PTR(ret);
+		}
+	}
+
+	icc_node_add(node, provider);
+
+	return node;
+}
+
+static void imx_icc_unregister_nodes(struct icc_provider *provider)
+{
+	struct icc_node *node, *tmp;
+
+	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
+		struct imx_icc_node *node_data = node->data;
+
+		icc_node_del(node);
+		icc_node_destroy(node->id);
+		if (dev_pm_qos_request_active(&node_data->qos_req))
+			dev_pm_qos_remove_request(&node_data->qos_req);
+	}
+}
+
+static int imx_icc_register_nodes(struct icc_provider *provider,
+				  const struct imx_icc_node_desc *descs,
+				  int count)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		struct icc_node *node;
+		const struct imx_icc_node_desc *node_desc = &descs[i];
+		size_t j;
+
+		node = imx_icc_node_add(provider, node_desc);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			if (ret != -EPROBE_DEFER)
+				dev_err(provider->dev, "failed to add %s: %d\n",
+						node_desc->name, ret);
+			goto err;
+		}
+
+		for (j = 0; j < node_desc->num_links; j++)
+			icc_link_create(node, node_desc->links[j]);
+	}
+
+	return 0;
+
+err:
+	imx_icc_unregister_nodes(provider);
+
+	return ret;
+}
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes, int nodes_count)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_icc_provider *desc;
+	struct icc_provider *provider;
+	int ret;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	desc->dev = dev;
+
+	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
+	if (!provider)
+		return -ENOMEM;
+	provider->set = imx_icc_set;
+	provider->aggregate = imx_icc_aggregate;
+	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
+	if (ret) {
+		dev_err(dev, "error adding interconnect nodes\n");
+		goto provider_del;
+	}
+
+	return 0;
+
+provider_del:
+	icc_provider_del(provider);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_icc_register);
+
+int imx_icc_unregister(struct platform_device *pdev)
+{
+	struct icc_provider *provider = platform_get_drvdata(pdev);
+
+	icc_provider_del(provider);
+	imx_icc_unregister_nodes(provider);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(imx_icc_unregister);
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
new file mode 100644
index 000000000000..ab191eb89616
--- /dev/null
+++ b/drivers/interconnect/imx/imx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+#ifndef __BUSFREQ_H
+#define __BUSFREQ_H
+
+#include <linux/kernel.h>
+
+#define IMX_ICC_MAX_LINKS	32
+#define IMX_ICC_UNDEFINED_BW	0xffffffff
+
+/*
+ * struct imx_icc_node_adj - Describe an dynamic adjustment knob
+ */
+struct imx_icc_node_adj_desc {
+	const char *devfreq_name;
+	unsigned int bw_mul, bw_div;
+};
+
+/*
+ * struct imx_icc_node - Describe an interconnect node
+ * @name: name of the node
+ * @id: an unique id to identify the node
+ * @links: an array of slaves' node id
+ * @num_links: number of id defined in links
+ */
+struct imx_icc_node_desc {
+	const char *name;
+	u16 id;
+	u16 links[IMX_ICC_MAX_LINKS];
+	u16 num_links;
+
+	const struct imx_icc_node_adj_desc *adj;
+};
+
+#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\
+	{								\
+		.id = _id,						\
+		.name = _name,						\
+		.adj = _adj,						\
+		.num_links = _numlinks,					\
+		.links = { __VA_ARGS__ },				\
+	}
+
+#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
+
+#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
+	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
+
+int imx_icc_register(struct platform_device *pdev,
+		     struct imx_icc_node_desc *nodes,
+		     int nodes_count);
+int imx_icc_unregister(struct platform_device *pdev);
+
+#endif /* __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

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-08-06 10:55 ` Leonard Crestez
  (?)
@ 2019-08-06 10:55   ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

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

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index 45fbae7007af..2f06cb1f81c3 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -1,5 +1,9 @@
 config INTERCONNECT_IMX
 	bool "i.MX interconnect drivers"
 	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
 	help
 	  Generic interconnect driver for i.MX SOCs
+
+config INTERCONNECT_IMX8MM
+	def_bool y
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index bb92fd9fe4a5..5f658c1608a6 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
+obj-$(CONFIG_INTERCONNECT_IMX8MM) += imx8mm.o
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
new file mode 100644
index 000000000000..5bed7babff96
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mm.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
+	.devfreq_name = "dram",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
+	.devfreq_name = "noc",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+			2, IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
+
+	/* VPUMIX */
+	DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, 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, NULL, 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, NULL, 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, NULL, 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, NULL, 1, IMX8MM_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, 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, NULL,
+			2, IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
+};
+
+static int imx8mm_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mm_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static const struct of_device_id imx_icc_of_match[] = {
+	{ .compatible = "fsl,imx8mm-interconnect" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_icc_of_match);
+
+static struct platform_driver imx8mm_icc_driver = {
+	.probe = imx8mm_icc_probe,
+	.remove = imx8mm_icc_remove,
+	.driver = {
+		.name = "imx8mm-interconnect",
+		.of_match_table = imx_icc_of_match,
+	},
+};
+
+builtin_platform_driver(imx8mm_icc_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..5404f2af15c3
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#ifndef __IMX8MM_ICM_INTERCONNECT_IDS_H
+#define __IMX8MM_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MM_ICN_NOC		1
+#define IMX8MM_ICS_DRAM		2
+#define IMX8MM_ICS_OCRAM	3
+#define IMX8MM_ICM_A53		4
+
+#define IMX8MM_ICM_VPU_H1	5
+#define IMX8MM_ICM_VPU_G1	6
+#define IMX8MM_ICM_VPU_G2	7
+#define IMX8MM_ICN_VIDEO	8
+
+#define IMX8MM_ICM_GPU2D	9
+#define IMX8MM_ICM_GPU3D	10
+#define IMX8MM_ICN_GPU		11
+
+#define IMX8MM_ICM_CSI		12
+#define IMX8MM_ICM_LCDIF	13
+#define IMX8MM_ICN_MIPI		14
+
+#define IMX8MM_ICM_USB1		15
+#define IMX8MM_ICM_USB2		16
+#define IMX8MM_ICM_PCIE		17
+#define IMX8MM_ICN_HSIO		18
+
+#define IMX8MM_ICM_SDMA2	19
+#define IMX8MM_ICM_SDMA3	20
+#define IMX8MM_ICN_AUDIO	21
+
+#define IMX8MM_ICN_ENET		22
+#define IMX8MM_ICM_ENET		23
+
+#define IMX8MM_ICN_MAIN		24
+#define IMX8MM_ICM_NAND		25
+#define IMX8MM_ICM_SDMA1	26
+#define IMX8MM_ICM_USDHC1	27
+#define IMX8MM_ICM_USDHC2	28
+#define IMX8MM_ICM_USDHC3	29
+
+#endif /* __IMX8MM_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

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

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

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index 45fbae7007af..2f06cb1f81c3 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -1,5 +1,9 @@
 config INTERCONNECT_IMX
 	bool "i.MX interconnect drivers"
 	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
 	help
 	  Generic interconnect driver for i.MX SOCs
+
+config INTERCONNECT_IMX8MM
+	def_bool y
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index bb92fd9fe4a5..5f658c1608a6 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
+obj-$(CONFIG_INTERCONNECT_IMX8MM) += imx8mm.o
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
new file mode 100644
index 000000000000..5bed7babff96
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mm.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
+	.devfreq_name = "dram",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
+	.devfreq_name = "noc",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+			2, IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
+
+	/* VPUMIX */
+	DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, 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, NULL, 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, NULL, 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, NULL, 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, NULL, 1, IMX8MM_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, 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, NULL,
+			2, IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
+};
+
+static int imx8mm_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mm_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static const struct of_device_id imx_icc_of_match[] = {
+	{ .compatible = "fsl,imx8mm-interconnect" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_icc_of_match);
+
+static struct platform_driver imx8mm_icc_driver = {
+	.probe = imx8mm_icc_probe,
+	.remove = imx8mm_icc_remove,
+	.driver = {
+		.name = "imx8mm-interconnect",
+		.of_match_table = imx_icc_of_match,
+	},
+};
+
+builtin_platform_driver(imx8mm_icc_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..5404f2af15c3
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#ifndef __IMX8MM_ICM_INTERCONNECT_IDS_H
+#define __IMX8MM_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MM_ICN_NOC		1
+#define IMX8MM_ICS_DRAM		2
+#define IMX8MM_ICS_OCRAM	3
+#define IMX8MM_ICM_A53		4
+
+#define IMX8MM_ICM_VPU_H1	5
+#define IMX8MM_ICM_VPU_G1	6
+#define IMX8MM_ICM_VPU_G2	7
+#define IMX8MM_ICN_VIDEO	8
+
+#define IMX8MM_ICM_GPU2D	9
+#define IMX8MM_ICM_GPU3D	10
+#define IMX8MM_ICN_GPU		11
+
+#define IMX8MM_ICM_CSI		12
+#define IMX8MM_ICM_LCDIF	13
+#define IMX8MM_ICN_MIPI		14
+
+#define IMX8MM_ICM_USB1		15
+#define IMX8MM_ICM_USB2		16
+#define IMX8MM_ICM_PCIE		17
+#define IMX8MM_ICN_HSIO		18
+
+#define IMX8MM_ICM_SDMA2	19
+#define IMX8MM_ICM_SDMA3	20
+#define IMX8MM_ICN_AUDIO	21
+
+#define IMX8MM_ICN_ENET		22
+#define IMX8MM_ICM_ENET		23
+
+#define IMX8MM_ICN_MAIN		24
+#define IMX8MM_ICM_NAND		25
+#define IMX8MM_ICM_SDMA1	26
+#define IMX8MM_ICM_USDHC1	27
+#define IMX8MM_ICM_USDHC2	28
+#define IMX8MM_ICM_USDHC3	29
+
+#endif /* __IMX8MM_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread
* [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-06 10:55   ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-06 10:55 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

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

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

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index 45fbae7007af..2f06cb1f81c3 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -1,5 +1,9 @@
 config INTERCONNECT_IMX
 	bool "i.MX interconnect drivers"
 	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
 	help
 	  Generic interconnect driver for i.MX SOCs
+
+config INTERCONNECT_IMX8MM
+	def_bool y
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index bb92fd9fe4a5..5f658c1608a6 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
+obj-$(CONFIG_INTERCONNECT_IMX8MM) += imx8mm.o
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
new file mode 100644
index 000000000000..5bed7babff96
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Copyright (c) 2019, NXP
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ * Author: Leonard Crestez <leonard.crestez@nxp.com>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/interconnect/imx8mm.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mm_dram_adj = {
+	.devfreq_name = "dram",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+static const struct imx_icc_node_adj_desc imx8mm_noc_adj = {
+	.devfreq_name = "noc",
+	.bw_mul = 1,
+	.bw_div = 16,
+};
+
+/*
+ * Describe bus masters, slaves and connections between them
+ *
+ * This is a simplified subset of the bus diagram, there are several other
+ * PL301 nics which are skipped/merged into PL301_MAIN
+ */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MM_ICN_NOC, &imx8mm_noc_adj,
+			2, IMX8MM_ICS_DRAM, IMX8MM_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("DRAM", IMX8MM_ICS_DRAM, &imx8mm_dram_adj),
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MM_ICS_OCRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MM_ICM_A53, IMX8MM_ICN_NOC),
+
+	/* VPUMIX */
+	DEFINE_BUS_MASTER("VPU H1", IMX8MM_ICM_VPU_H1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G1", IMX8MM_ICM_VPU_G1, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G2", IMX8MM_ICM_VPU_G2, IMX8MM_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_VIDEO", IMX8MM_ICN_VIDEO, NULL, 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, NULL, 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, NULL, 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, NULL, 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, NULL, 1, IMX8MM_ICN_MAIN),
+
+	/* Ethernet */
+	DEFINE_BUS_MASTER("ENET", IMX8MM_ICM_ENET, IMX8MM_ICN_ENET),
+	DEFINE_BUS_INTERCONNECT("PL301_ENET", IMX8MM_ICN_ENET, NULL, 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, NULL,
+			2, IMX8MM_ICN_NOC, IMX8MM_ICS_OCRAM),
+};
+
+static int imx8mm_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+}
+
+static int imx8mm_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static const struct of_device_id imx_icc_of_match[] = {
+	{ .compatible = "fsl,imx8mm-interconnect" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_icc_of_match);
+
+static struct platform_driver imx8mm_icc_driver = {
+	.probe = imx8mm_icc_probe,
+	.remove = imx8mm_icc_remove,
+	.driver = {
+		.name = "imx8mm-interconnect",
+		.of_match_table = imx_icc_of_match,
+	},
+};
+
+builtin_platform_driver(imx8mm_icc_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..5404f2af15c3
--- /dev/null
+++ b/include/dt-bindings/interconnect/imx8mm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright (c) 2019, BayLibre
+ * Author: Alexandre Bailon <abailon@baylibre.com>
+ */
+
+#ifndef __IMX8MM_ICM_INTERCONNECT_IDS_H
+#define __IMX8MM_ICM_INTERCONNECT_IDS_H
+
+#define IMX8MM_ICN_NOC		1
+#define IMX8MM_ICS_DRAM		2
+#define IMX8MM_ICS_OCRAM	3
+#define IMX8MM_ICM_A53		4
+
+#define IMX8MM_ICM_VPU_H1	5
+#define IMX8MM_ICM_VPU_G1	6
+#define IMX8MM_ICM_VPU_G2	7
+#define IMX8MM_ICN_VIDEO	8
+
+#define IMX8MM_ICM_GPU2D	9
+#define IMX8MM_ICM_GPU3D	10
+#define IMX8MM_ICN_GPU		11
+
+#define IMX8MM_ICM_CSI		12
+#define IMX8MM_ICM_LCDIF	13
+#define IMX8MM_ICN_MIPI		14
+
+#define IMX8MM_ICM_USB1		15
+#define IMX8MM_ICM_USB2		16
+#define IMX8MM_ICM_PCIE		17
+#define IMX8MM_ICN_HSIO		18
+
+#define IMX8MM_ICM_SDMA2	19
+#define IMX8MM_ICM_SDMA3	20
+#define IMX8MM_ICN_AUDIO	21
+
+#define IMX8MM_ICN_ENET		22
+#define IMX8MM_ICM_ENET		23
+
+#define IMX8MM_ICN_MAIN		24
+#define IMX8MM_ICM_NAND		25
+#define IMX8MM_ICM_SDMA1	26
+#define IMX8MM_ICM_USDHC1	27
+#define IMX8MM_ICM_USDHC2	28
+#define IMX8MM_ICM_USDHC3	29
+
+#endif /* __IMX8MM_ICM_INTERCONNECT_IDS_H */
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
  2019-08-06 10:55   ` Leonard Crestez
  (?)
@ 2019-08-07 15:16     ` Georgi Djakov
  -1 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---

Please put some commit text.

>  .../devicetree/bindings/interconnect/imx.yaml | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
> new file mode 100644
> index 000000000000..c6f173b38f4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX interconnect device
> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +properties:
> +  compatible:
> +    contains:
> +      enum:
> +        - fsl,imx8mm-interconnect

Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
up to you.

> +  "#interconnect-cells":
> +    const: 1
> +  devfreq-names:
> +    description: Names of devfreq instances for adjustable nodes
> +  devfreq:
> +    description: List of phandle pointing to devfreq instances
> +
> +required:
> +  - compatible
> +  - "#interconnect-cells"
> +  - "devfreq-names"
> +  - "devfreq"
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interconnect/imx8mm.h>
> +    icc: interconnect {
> +        compatible = "fsl,imx8mm-interconnect";
> +        #interconnect-cells = <1>;
> +        devfreq-names = "dram", "noc", "axi";
> +        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
> +    };
> 

Thanks,
Georgi

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-07 15:16     ` Georgi Djakov
  0 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---

Please put some commit text.

>  .../devicetree/bindings/interconnect/imx.yaml | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
> new file mode 100644
> index 000000000000..c6f173b38f4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX interconnect device
> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +properties:
> +  compatible:
> +    contains:
> +      enum:
> +        - fsl,imx8mm-interconnect

Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
up to you.

> +  "#interconnect-cells":
> +    const: 1
> +  devfreq-names:
> +    description: Names of devfreq instances for adjustable nodes
> +  devfreq:
> +    description: List of phandle pointing to devfreq instances
> +
> +required:
> +  - compatible
> +  - "#interconnect-cells"
> +  - "devfreq-names"
> +  - "devfreq"
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interconnect/imx8mm.h>
> +    icc: interconnect {
> +        compatible = "fsl,imx8mm-interconnect";
> +        #interconnect-cells = <1>;
> +        devfreq-names = "dram", "noc", "axi";
> +        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
> +    };
> 

Thanks,
Georgi

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-07 15:16     ` Georgi Djakov
  0 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---

Please put some commit text.

>  .../devicetree/bindings/interconnect/imx.yaml | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
> new file mode 100644
> index 000000000000..c6f173b38f4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX interconnect device
> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +properties:
> +  compatible:
> +    contains:
> +      enum:
> +        - fsl,imx8mm-interconnect

Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
up to you.

> +  "#interconnect-cells":
> +    const: 1
> +  devfreq-names:
> +    description: Names of devfreq instances for adjustable nodes
> +  devfreq:
> +    description: List of phandle pointing to devfreq instances
> +
> +required:
> +  - compatible
> +  - "#interconnect-cells"
> +  - "devfreq-names"
> +  - "devfreq"
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interconnect/imx8mm.h>
> +    icc: interconnect {
> +        compatible = "fsl,imx8mm-interconnect";
> +        #interconnect-cells = <1>;
> +        devfreq-names = "dram", "noc", "axi";
> +        devfreq = <&ddrc>, <&noc>, <&pl301_main>;
> +    };
> 

Thanks,
Georgi

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 2/3] interconnect: Add imx core driver
  2019-08-06 10:55   ` Leonard Crestez
  (?)
@ 2019-08-07 15:16     ` Georgi Djakov
  -1 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> This adds support for i.MX SoC family to interconnect framework.
> 
> Platform drivers can describe their interconnect graph and
> several adjustment knobs where an icc node bandwith converted to a

s/bandwith/bandwidth/

> clk_min_rate request.
> 
> All adjustable nodes are assumed to be independent.
> 
> Based on an earlier work by Alexandre Bailon but greatly reduced to drop
> "platform opp" support.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Your Signed-off-by should be below Alexandre's.

> ---
>  drivers/interconnect/Kconfig      |   1 +
>  drivers/interconnect/Makefile     |   1 +
>  drivers/interconnect/imx/Kconfig  |   5 +
>  drivers/interconnect/imx/Makefile |   1 +
>  drivers/interconnect/imx/imx.c    | 258 ++++++++++++++++++++++++++++++
>  drivers/interconnect/imx/imx.h    |  62 +++++++
>  6 files changed, 328 insertions(+)
>  create mode 100644 drivers/interconnect/imx/Kconfig
>  create mode 100644 drivers/interconnect/imx/Makefile
>  create mode 100644 drivers/interconnect/imx/imx.c
>  create mode 100644 drivers/interconnect/imx/imx.h
> 
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index bfa4ca3ab7a9..e61802230f90 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -10,7 +10,8 @@ menuconfig INTERCONNECT
>  	  If unsure, say no.
>  
>  if INTERCONNECT
>  
>  source "drivers/interconnect/qcom/Kconfig"
> +source "drivers/interconnect/imx/Kconfig"
>  
>  endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 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..45fbae7007af
> --- /dev/null
> +++ b/drivers/interconnect/imx/Kconfig
> @@ -0,0 +1,5 @@
> +config INTERCONNECT_IMX
> +	bool "i.MX interconnect drivers"
> +	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
> +	help
> +	  Generic interconnect driver for i.MX SOCs
> diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
> new file mode 100644
> index 000000000000..bb92fd9fe4a5
> --- /dev/null
> +++ b/drivers/interconnect/imx/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> new file mode 100644
> index 000000000000..cc838e40419e
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +#include <linux/devfreq.h>

Please sort alphabetically.

> +#include <linux/of.h>
> +
> +#include "imx.h"
> +
> +/* private icc_provider data */
> +struct imx_icc_provider {
> +	struct device *dev;
> +};
> +
> +/* private icc_node data */
> +struct imx_icc_node {
> +	const struct imx_icc_node_desc *desc;
> +	struct devfreq *devfreq;
> +	struct dev_pm_qos_request qos_req;
> +};
> +
> +static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
> +{
> +	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
> +{
> +	struct device *dev = node->provider->dev;
> +	struct imx_icc_node *node_data = node->data;
> +	unsigned long freq;
> +
> +	if (!node_data->devfreq)
> +		return 0;
> +
> +	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
> +	do_div(freq, node_data->desc->adj->bw_div);
> +	if (freq > INT_MAX) {
> +		dev_err(dev, "%s can't request more INT_MAX freq\n",
> +				node->name);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
> +			node->name, node->avg_bw, node->peak_bw, freq);
> +
> +	dev_pm_qos_update_request(&node_data->qos_req, freq);
> +
> +	return 0;
> +}
> +
> +static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +	return imx_icc_node_set(dst);
> +}
> +
> +static int imx_icc_node_init_devfreq(struct device *dev, 
> +				     struct icc_node *node)
> +{
> +	struct imx_icc_node *node_data = node->data;
> +	const struct imx_icc_node_desc *node_desc = node_data->desc;
> +	int index;
> +	int ret;
> +
> +	index = of_property_match_string(dev->of_node,
> +			"devfreq-names", node_desc->adj->devfreq_name);
> +	if (index < 0) {
> +		dev_err(dev, "failed to match devfreq-names %s: %d\n",
> +				node_desc->adj->devfreq_name, index);
> +		return index;
> +	}
> +
> +	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
> +	if (IS_ERR(node_data->devfreq)) {
> +		ret = PTR_ERR(node_data->devfreq);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
> +					index, node_desc->adj->devfreq_name, ret);
> +		return ret;
> +	}
> +
> +	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
> +			&node_data->qos_req,
> +			DEV_PM_QOS_MIN_FREQUENCY, 0);
> +}
> +
> +static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
> +					 const struct imx_icc_node_desc *node_desc)
> +{
> +	struct imx_icc_provider *provider_data = provider->data;
> +	struct device *dev = provider_data->dev;
> +	struct imx_icc_node *node_data;
> +	struct icc_node *node;
> +	int ret;
> +
> +	node = icc_node_create(node_desc->id);
> +	if (IS_ERR(node)) {
> +		dev_err(dev, "failed to create node %d\n", node_desc->id);
> +		return node;
> +	}
> +
> +	if (node->data) {
> +		dev_err(dev, "already created node %s id=%d\n",
> +				node_desc->name, node_desc->id);
> +		return ERR_PTR(-EEXIST);
> +	}
> +
> +	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
> +	if (!node_data) {
> +		icc_node_destroy(node->id);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	node->name = node_desc->name;
> +	node->data = node_data;
> +	node_data->desc = node_desc;
> +	if (node_desc->adj) {
> +		ret = imx_icc_node_init_devfreq(dev, node);
> +		if (ret < 0) {
> +			icc_node_destroy(node->id);
> +			return ERR_PTR(ret);
> +		}
> +	}
> +
> +	icc_node_add(node, provider);
> +
> +	return node;
> +}
> +
> +static void imx_icc_unregister_nodes(struct icc_provider *provider)
> +{
> +	struct icc_node *node, *tmp;
> +
> +	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
> +		struct imx_icc_node *node_data = node->data;
> +
> +		icc_node_del(node);
> +		icc_node_destroy(node->id);
> +		if (dev_pm_qos_request_active(&node_data->qos_req))
> +			dev_pm_qos_remove_request(&node_data->qos_req);
> +	}
> +}
> +
> +static int imx_icc_register_nodes(struct icc_provider *provider,
> +				  const struct imx_icc_node_desc *descs,
> +				  int count)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		struct icc_node *node;
> +		const struct imx_icc_node_desc *node_desc = &descs[i];
> +		size_t j;
> +
> +		node = imx_icc_node_add(provider, node_desc);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(provider->dev, "failed to add %s: %d\n",
> +						node_desc->name, ret);
> +			goto err;
> +		}
> +
> +		for (j = 0; j < node_desc->num_links; j++)
> +			icc_link_create(node, node_desc->links[j]);
> +	}
> +
> +	return 0;
> +
> +err:
> +	imx_icc_unregister_nodes(provider);
> +
> +	return ret;
> +}
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes, int nodes_count)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx_icc_provider *desc;
> +	struct icc_provider *provider;
> +	int ret;
> +
> +	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
> +	if (!desc)
> +		return -ENOMEM;
> +	desc->dev = dev;
> +
> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> +	if (!provider)
> +		return -ENOMEM;
> +	provider->set = imx_icc_set;
> +	provider->aggregate = imx_icc_aggregate;
> +	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
> +	if (ret) {
> +		dev_err(dev, "error adding interconnect nodes\n");
> +		goto provider_del;
> +	}
> +
> +	return 0;
> +
> +provider_del:
> +	icc_provider_del(provider);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_register);
> +
> +int imx_icc_unregister(struct platform_device *pdev)
> +{
> +	struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> +	icc_provider_del(provider);
> +	imx_icc_unregister_nodes(provider);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_unregister);
> diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> new file mode 100644
> index 000000000000..ab191eb89616
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +#ifndef __BUSFREQ_H
> +#define __BUSFREQ_H

Maybe __DRIVERS_INTERCONNECT_IMX_IMX_H to match with the path and filename.

> +
> +#include <linux/kernel.h>
> +
> +#define IMX_ICC_MAX_LINKS	32

2 seems enough?

> +#define IMX_ICC_UNDEFINED_BW	0xffffffff

Is this used?

> +
> +/*
> + * struct imx_icc_node_adj - Describe an dynamic adjustment knob

s/an/a/

> + */
> +struct imx_icc_node_adj_desc {
> +	const char *devfreq_name;
> +	unsigned int bw_mul, bw_div;
> +};
> +
> +/*
> + * struct imx_icc_node - Describe an interconnect node
> + * @name: name of the node
> + * @id: an unique id to identify the node
> + * @links: an array of slaves' node id
> + * @num_links: number of id defined in links
> + */
> +struct imx_icc_node_desc {
> +	const char *name;
> +	u16 id;
> +	u16 links[IMX_ICC_MAX_LINKS];
> +	u16 num_links;
> +
> +	const struct imx_icc_node_adj_desc *adj;
> +};
> +
> +#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\

You can remove the _numlinks...

> +	{								\
> +		.id = _id,						\
> +		.name = _name,						\
> +		.adj = _adj,						\
> +		.num_links = _numlinks,					\

...and calculate the number of links automatically with:
		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\

> +		.links = { __VA_ARGS__ },				\
> +	}
> +
> +#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
> +
> +#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes,
> +		     int nodes_count);
> +int imx_icc_unregister(struct platform_device *pdev);
> +
> +#endif /* __BUSFREQ_H */
Thanks,
Georgi

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 2/3] interconnect: Add imx core driver
@ 2019-08-07 15:16     ` Georgi Djakov
  0 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Saravana Kannan, Mark Rutland, Shawn Guo, Dong Aisheng,
	Fabio Estevam, Stephen Boyd, Michael Turquette, Jacky Bai,
	Anson Huang, kernel, linux-imx, linux-pm, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> This adds support for i.MX SoC family to interconnect framework.
> 
> Platform drivers can describe their interconnect graph and
> several adjustment knobs where an icc node bandwith converted to a

s/bandwith/bandwidth/

> clk_min_rate request.
> 
> All adjustable nodes are assumed to be independent.
> 
> Based on an earlier work by Alexandre Bailon but greatly reduced to drop
> "platform opp" support.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Your Signed-off-by should be below Alexandre's.

> ---
>  drivers/interconnect/Kconfig      |   1 +
>  drivers/interconnect/Makefile     |   1 +
>  drivers/interconnect/imx/Kconfig  |   5 +
>  drivers/interconnect/imx/Makefile |   1 +
>  drivers/interconnect/imx/imx.c    | 258 ++++++++++++++++++++++++++++++
>  drivers/interconnect/imx/imx.h    |  62 +++++++
>  6 files changed, 328 insertions(+)
>  create mode 100644 drivers/interconnect/imx/Kconfig
>  create mode 100644 drivers/interconnect/imx/Makefile
>  create mode 100644 drivers/interconnect/imx/imx.c
>  create mode 100644 drivers/interconnect/imx/imx.h
> 
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index bfa4ca3ab7a9..e61802230f90 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -10,7 +10,8 @@ menuconfig INTERCONNECT
>  	  If unsure, say no.
>  
>  if INTERCONNECT
>  
>  source "drivers/interconnect/qcom/Kconfig"
> +source "drivers/interconnect/imx/Kconfig"
>  
>  endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 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..45fbae7007af
> --- /dev/null
> +++ b/drivers/interconnect/imx/Kconfig
> @@ -0,0 +1,5 @@
> +config INTERCONNECT_IMX
> +	bool "i.MX interconnect drivers"
> +	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
> +	help
> +	  Generic interconnect driver for i.MX SOCs
> diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
> new file mode 100644
> index 000000000000..bb92fd9fe4a5
> --- /dev/null
> +++ b/drivers/interconnect/imx/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> new file mode 100644
> index 000000000000..cc838e40419e
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +#include <linux/devfreq.h>

Please sort alphabetically.

> +#include <linux/of.h>
> +
> +#include "imx.h"
> +
> +/* private icc_provider data */
> +struct imx_icc_provider {
> +	struct device *dev;
> +};
> +
> +/* private icc_node data */
> +struct imx_icc_node {
> +	const struct imx_icc_node_desc *desc;
> +	struct devfreq *devfreq;
> +	struct dev_pm_qos_request qos_req;
> +};
> +
> +static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
> +{
> +	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
> +{
> +	struct device *dev = node->provider->dev;
> +	struct imx_icc_node *node_data = node->data;
> +	unsigned long freq;
> +
> +	if (!node_data->devfreq)
> +		return 0;
> +
> +	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
> +	do_div(freq, node_data->desc->adj->bw_div);
> +	if (freq > INT_MAX) {
> +		dev_err(dev, "%s can't request more INT_MAX freq\n",
> +				node->name);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
> +			node->name, node->avg_bw, node->peak_bw, freq);
> +
> +	dev_pm_qos_update_request(&node_data->qos_req, freq);
> +
> +	return 0;
> +}
> +
> +static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +	return imx_icc_node_set(dst);
> +}
> +
> +static int imx_icc_node_init_devfreq(struct device *dev, 
> +				     struct icc_node *node)
> +{
> +	struct imx_icc_node *node_data = node->data;
> +	const struct imx_icc_node_desc *node_desc = node_data->desc;
> +	int index;
> +	int ret;
> +
> +	index = of_property_match_string(dev->of_node,
> +			"devfreq-names", node_desc->adj->devfreq_name);
> +	if (index < 0) {
> +		dev_err(dev, "failed to match devfreq-names %s: %d\n",
> +				node_desc->adj->devfreq_name, index);
> +		return index;
> +	}
> +
> +	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
> +	if (IS_ERR(node_data->devfreq)) {
> +		ret = PTR_ERR(node_data->devfreq);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
> +					index, node_desc->adj->devfreq_name, ret);
> +		return ret;
> +	}
> +
> +	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
> +			&node_data->qos_req,
> +			DEV_PM_QOS_MIN_FREQUENCY, 0);
> +}
> +
> +static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
> +					 const struct imx_icc_node_desc *node_desc)
> +{
> +	struct imx_icc_provider *provider_data = provider->data;
> +	struct device *dev = provider_data->dev;
> +	struct imx_icc_node *node_data;
> +	struct icc_node *node;
> +	int ret;
> +
> +	node = icc_node_create(node_desc->id);
> +	if (IS_ERR(node)) {
> +		dev_err(dev, "failed to create node %d\n", node_desc->id);
> +		return node;
> +	}
> +
> +	if (node->data) {
> +		dev_err(dev, "already created node %s id=%d\n",
> +				node_desc->name, node_desc->id);
> +		return ERR_PTR(-EEXIST);
> +	}
> +
> +	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
> +	if (!node_data) {
> +		icc_node_destroy(node->id);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	node->name = node_desc->name;
> +	node->data = node_data;
> +	node_data->desc = node_desc;
> +	if (node_desc->adj) {
> +		ret = imx_icc_node_init_devfreq(dev, node);
> +		if (ret < 0) {
> +			icc_node_destroy(node->id);
> +			return ERR_PTR(ret);
> +		}
> +	}
> +
> +	icc_node_add(node, provider);
> +
> +	return node;
> +}
> +
> +static void imx_icc_unregister_nodes(struct icc_provider *provider)
> +{
> +	struct icc_node *node, *tmp;
> +
> +	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
> +		struct imx_icc_node *node_data = node->data;
> +
> +		icc_node_del(node);
> +		icc_node_destroy(node->id);
> +		if (dev_pm_qos_request_active(&node_data->qos_req))
> +			dev_pm_qos_remove_request(&node_data->qos_req);
> +	}
> +}
> +
> +static int imx_icc_register_nodes(struct icc_provider *provider,
> +				  const struct imx_icc_node_desc *descs,
> +				  int count)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		struct icc_node *node;
> +		const struct imx_icc_node_desc *node_desc = &descs[i];
> +		size_t j;
> +
> +		node = imx_icc_node_add(provider, node_desc);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(provider->dev, "failed to add %s: %d\n",
> +						node_desc->name, ret);
> +			goto err;
> +		}
> +
> +		for (j = 0; j < node_desc->num_links; j++)
> +			icc_link_create(node, node_desc->links[j]);
> +	}
> +
> +	return 0;
> +
> +err:
> +	imx_icc_unregister_nodes(provider);
> +
> +	return ret;
> +}
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes, int nodes_count)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx_icc_provider *desc;
> +	struct icc_provider *provider;
> +	int ret;
> +
> +	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
> +	if (!desc)
> +		return -ENOMEM;
> +	desc->dev = dev;
> +
> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> +	if (!provider)
> +		return -ENOMEM;
> +	provider->set = imx_icc_set;
> +	provider->aggregate = imx_icc_aggregate;
> +	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
> +	if (ret) {
> +		dev_err(dev, "error adding interconnect nodes\n");
> +		goto provider_del;
> +	}
> +
> +	return 0;
> +
> +provider_del:
> +	icc_provider_del(provider);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_register);
> +
> +int imx_icc_unregister(struct platform_device *pdev)
> +{
> +	struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> +	icc_provider_del(provider);
> +	imx_icc_unregister_nodes(provider);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_unregister);
> diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> new file mode 100644
> index 000000000000..ab191eb89616
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +#ifndef __BUSFREQ_H
> +#define __BUSFREQ_H

Maybe __DRIVERS_INTERCONNECT_IMX_IMX_H to match with the path and filename.

> +
> +#include <linux/kernel.h>
> +
> +#define IMX_ICC_MAX_LINKS	32

2 seems enough?

> +#define IMX_ICC_UNDEFINED_BW	0xffffffff

Is this used?

> +
> +/*
> + * struct imx_icc_node_adj - Describe an dynamic adjustment knob

s/an/a/

> + */
> +struct imx_icc_node_adj_desc {
> +	const char *devfreq_name;
> +	unsigned int bw_mul, bw_div;
> +};
> +
> +/*
> + * struct imx_icc_node - Describe an interconnect node
> + * @name: name of the node
> + * @id: an unique id to identify the node
> + * @links: an array of slaves' node id
> + * @num_links: number of id defined in links
> + */
> +struct imx_icc_node_desc {
> +	const char *name;
> +	u16 id;
> +	u16 links[IMX_ICC_MAX_LINKS];
> +	u16 num_links;
> +
> +	const struct imx_icc_node_adj_desc *adj;
> +};
> +
> +#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\

You can remove the _numlinks...

> +	{								\
> +		.id = _id,						\
> +		.name = _name,						\
> +		.adj = _adj,						\
> +		.num_links = _numlinks,					\

...and calculate the number of links automatically with:
		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\

> +		.links = { __VA_ARGS__ },				\
> +	}
> +
> +#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
> +
> +#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes,
> +		     int nodes_count);
> +int imx_icc_unregister(struct platform_device *pdev);
> +
> +#endif /* __BUSFREQ_H */
Thanks,
Georgi



^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 2/3] interconnect: Add imx core driver
@ 2019-08-07 15:16     ` Georgi Djakov
  0 siblings, 0 replies; 40+ messages in thread
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
  To: Leonard Crestez, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

Hi Leonard,

On 8/6/19 13:55, Leonard Crestez wrote:
> This adds support for i.MX SoC family to interconnect framework.
> 
> Platform drivers can describe their interconnect graph and
> several adjustment knobs where an icc node bandwith converted to a

s/bandwith/bandwidth/

> clk_min_rate request.
> 
> All adjustable nodes are assumed to be independent.
> 
> Based on an earlier work by Alexandre Bailon but greatly reduced to drop
> "platform opp" support.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Your Signed-off-by should be below Alexandre's.

> ---
>  drivers/interconnect/Kconfig      |   1 +
>  drivers/interconnect/Makefile     |   1 +
>  drivers/interconnect/imx/Kconfig  |   5 +
>  drivers/interconnect/imx/Makefile |   1 +
>  drivers/interconnect/imx/imx.c    | 258 ++++++++++++++++++++++++++++++
>  drivers/interconnect/imx/imx.h    |  62 +++++++
>  6 files changed, 328 insertions(+)
>  create mode 100644 drivers/interconnect/imx/Kconfig
>  create mode 100644 drivers/interconnect/imx/Makefile
>  create mode 100644 drivers/interconnect/imx/imx.c
>  create mode 100644 drivers/interconnect/imx/imx.h
> 
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index bfa4ca3ab7a9..e61802230f90 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -10,7 +10,8 @@ menuconfig INTERCONNECT
>  	  If unsure, say no.
>  
>  if INTERCONNECT
>  
>  source "drivers/interconnect/qcom/Kconfig"
> +source "drivers/interconnect/imx/Kconfig"
>  
>  endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 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..45fbae7007af
> --- /dev/null
> +++ b/drivers/interconnect/imx/Kconfig
> @@ -0,0 +1,5 @@
> +config INTERCONNECT_IMX
> +	bool "i.MX interconnect drivers"
> +	depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
> +	help
> +	  Generic interconnect driver for i.MX SOCs
> diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
> new file mode 100644
> index 000000000000..bb92fd9fe4a5
> --- /dev/null
> +++ b/drivers/interconnect/imx/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> new file mode 100644
> index 000000000000..cc838e40419e
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +#include <linux/devfreq.h>

Please sort alphabetically.

> +#include <linux/of.h>
> +
> +#include "imx.h"
> +
> +/* private icc_provider data */
> +struct imx_icc_provider {
> +	struct device *dev;
> +};
> +
> +/* private icc_node data */
> +struct imx_icc_node {
> +	const struct imx_icc_node_desc *desc;
> +	struct devfreq *devfreq;
> +	struct dev_pm_qos_request qos_req;
> +};
> +
> +static int imx_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* imx_icc_xlate(struct of_phandle_args *spec, void *data)
> +{
> +	struct imx_icc_provider *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 imx_icc_node_set(struct icc_node *node)
> +{
> +	struct device *dev = node->provider->dev;
> +	struct imx_icc_node *node_data = node->data;
> +	unsigned long freq;
> +
> +	if (!node_data->devfreq)
> +		return 0;
> +
> +	freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
> +	do_div(freq, node_data->desc->adj->bw_div);
> +	if (freq > INT_MAX) {
> +		dev_err(dev, "%s can't request more INT_MAX freq\n",
> +				node->name);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
> +			node->name, node->avg_bw, node->peak_bw, freq);
> +
> +	dev_pm_qos_update_request(&node_data->qos_req, freq);
> +
> +	return 0;
> +}
> +
> +static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +	return imx_icc_node_set(dst);
> +}
> +
> +static int imx_icc_node_init_devfreq(struct device *dev, 
> +				     struct icc_node *node)
> +{
> +	struct imx_icc_node *node_data = node->data;
> +	const struct imx_icc_node_desc *node_desc = node_data->desc;
> +	int index;
> +	int ret;
> +
> +	index = of_property_match_string(dev->of_node,
> +			"devfreq-names", node_desc->adj->devfreq_name);
> +	if (index < 0) {
> +		dev_err(dev, "failed to match devfreq-names %s: %d\n",
> +				node_desc->adj->devfreq_name, index);
> +		return index;
> +	}
> +
> +	node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
> +	if (IS_ERR(node_data->devfreq)) {
> +		ret = PTR_ERR(node_data->devfreq);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
> +					index, node_desc->adj->devfreq_name, ret);
> +		return ret;
> +	}
> +
> +	return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
> +			&node_data->qos_req,
> +			DEV_PM_QOS_MIN_FREQUENCY, 0);
> +}
> +
> +static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
> +					 const struct imx_icc_node_desc *node_desc)
> +{
> +	struct imx_icc_provider *provider_data = provider->data;
> +	struct device *dev = provider_data->dev;
> +	struct imx_icc_node *node_data;
> +	struct icc_node *node;
> +	int ret;
> +
> +	node = icc_node_create(node_desc->id);
> +	if (IS_ERR(node)) {
> +		dev_err(dev, "failed to create node %d\n", node_desc->id);
> +		return node;
> +	}
> +
> +	if (node->data) {
> +		dev_err(dev, "already created node %s id=%d\n",
> +				node_desc->name, node_desc->id);
> +		return ERR_PTR(-EEXIST);
> +	}
> +
> +	node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
> +	if (!node_data) {
> +		icc_node_destroy(node->id);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	node->name = node_desc->name;
> +	node->data = node_data;
> +	node_data->desc = node_desc;
> +	if (node_desc->adj) {
> +		ret = imx_icc_node_init_devfreq(dev, node);
> +		if (ret < 0) {
> +			icc_node_destroy(node->id);
> +			return ERR_PTR(ret);
> +		}
> +	}
> +
> +	icc_node_add(node, provider);
> +
> +	return node;
> +}
> +
> +static void imx_icc_unregister_nodes(struct icc_provider *provider)
> +{
> +	struct icc_node *node, *tmp;
> +
> +	list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
> +		struct imx_icc_node *node_data = node->data;
> +
> +		icc_node_del(node);
> +		icc_node_destroy(node->id);
> +		if (dev_pm_qos_request_active(&node_data->qos_req))
> +			dev_pm_qos_remove_request(&node_data->qos_req);
> +	}
> +}
> +
> +static int imx_icc_register_nodes(struct icc_provider *provider,
> +				  const struct imx_icc_node_desc *descs,
> +				  int count)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		struct icc_node *node;
> +		const struct imx_icc_node_desc *node_desc = &descs[i];
> +		size_t j;
> +
> +		node = imx_icc_node_add(provider, node_desc);
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(provider->dev, "failed to add %s: %d\n",
> +						node_desc->name, ret);
> +			goto err;
> +		}
> +
> +		for (j = 0; j < node_desc->num_links; j++)
> +			icc_link_create(node, node_desc->links[j]);
> +	}
> +
> +	return 0;
> +
> +err:
> +	imx_icc_unregister_nodes(provider);
> +
> +	return ret;
> +}
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes, int nodes_count)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx_icc_provider *desc;
> +	struct icc_provider *provider;
> +	int ret;
> +
> +	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
> +	if (!desc)
> +		return -ENOMEM;
> +	desc->dev = dev;
> +
> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> +	if (!provider)
> +		return -ENOMEM;
> +	provider->set = imx_icc_set;
> +	provider->aggregate = imx_icc_aggregate;
> +	provider->xlate = imx_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 = imx_icc_register_nodes(provider, nodes, nodes_count);
> +	if (ret) {
> +		dev_err(dev, "error adding interconnect nodes\n");
> +		goto provider_del;
> +	}
> +
> +	return 0;
> +
> +provider_del:
> +	icc_provider_del(provider);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_register);
> +
> +int imx_icc_unregister(struct platform_device *pdev)
> +{
> +	struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> +	icc_provider_del(provider);
> +	imx_icc_unregister_nodes(provider);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_unregister);
> diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> new file mode 100644
> index 000000000000..ab191eb89616
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +#ifndef __BUSFREQ_H
> +#define __BUSFREQ_H

Maybe __DRIVERS_INTERCONNECT_IMX_IMX_H to match with the path and filename.

> +
> +#include <linux/kernel.h>
> +
> +#define IMX_ICC_MAX_LINKS	32

2 seems enough?

> +#define IMX_ICC_UNDEFINED_BW	0xffffffff

Is this used?

> +
> +/*
> + * struct imx_icc_node_adj - Describe an dynamic adjustment knob

s/an/a/

> + */
> +struct imx_icc_node_adj_desc {
> +	const char *devfreq_name;
> +	unsigned int bw_mul, bw_div;
> +};
> +
> +/*
> + * struct imx_icc_node - Describe an interconnect node
> + * @name: name of the node
> + * @id: an unique id to identify the node
> + * @links: an array of slaves' node id
> + * @num_links: number of id defined in links
> + */
> +struct imx_icc_node_desc {
> +	const char *name;
> +	u16 id;
> +	u16 links[IMX_ICC_MAX_LINKS];
> +	u16 num_links;
> +
> +	const struct imx_icc_node_adj_desc *adj;
> +};
> +
> +#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...)	\

You can remove the _numlinks...

> +	{								\
> +		.id = _id,						\
> +		.name = _name,						\
> +		.adj = _adj,						\
> +		.num_links = _numlinks,					\

...and calculate the number of links automatically with:
		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\

> +		.links = { __VA_ARGS__ },				\
> +	}
> +
> +#define DEFINE_BUS_MASTER(_name, _id, _dest_id)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
> +
> +#define DEFINE_BUS_SLAVE(_name, _id, _adj)				\
> +	DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
> +
> +int imx_icc_register(struct platform_device *pdev,
> +		     struct imx_icc_node_desc *nodes,
> +		     int nodes_count);
> +int imx_icc_unregister(struct platform_device *pdev);
> +
> +#endif /* __BUSFREQ_H */
Thanks,
Georgi



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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
  2019-08-07 15:16     ` Georgi Djakov
  (?)
@ 2019-08-07 22:35       ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-07 22:35 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Saravana Kannan, Anson Huang, Stephen Boyd,
	Viresh Kumar, Michael Turquette, linux-pm, Krzysztof Kozlowski,
	Chanwoo Choi, Kyungmin Park, MyungJoo Ham, Alexandre Bailon,
	kernel, Fabio Estevam, linux-arm-kernel, Shawn Guo, Aisheng Dong,
	devicetree

On 8/7/2019 6:16 PM, Georgi Djakov wrote:

> Please put some commit text.

Will fix

>> +properties:
>> +  compatible:
>> +    contains:
>> +      enum:
>> +        - fsl,imx8mm-interconnect
> 
> Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
> up to you.

In the vendor tree busfreq is effectively its own subsystem with its own 
API calls used by imx drivers. As part of replacing this with standard 
devfreq + interconnect in upstream the old name should go away.

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-07 22:35       ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-07 22:35 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring
  Cc: Artur Świgoń,
	Alexandre Bailon, Viresh Kumar, Krzysztof Kozlowski,
	Chanwoo Choi, MyungJoo Ham, Kyungmin Park, Saravana Kannan,
	Mark Rutland, Shawn Guo, Aisheng Dong, Fabio Estevam,
	Stephen Boyd, Michael Turquette, Jacky Bai, Anson Huang, kernel,
	dl-linux-imx, linux-pm, devicetree, linux-arm-kernel

On 8/7/2019 6:16 PM, Georgi Djakov wrote:

> Please put some commit text.

Will fix

>> +properties:
>> +  compatible:
>> +    contains:
>> +      enum:
>> +        - fsl,imx8mm-interconnect
> 
> Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
> up to you.

In the vendor tree busfreq is effectively its own subsystem with its own 
API calls used by imx drivers. As part of replacing this with standard 
devfreq + interconnect in upstream the old name should go away.

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
@ 2019-08-07 22:35       ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-07 22:35 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Saravana Kannan, Anson Huang, Stephen Boyd,
	Viresh Kumar, Michael Turquette, linux-pm, Krzysztof Kozlowski,
	Chanwoo Choi, Kyungmin Park, MyungJoo Ham, Alexandre Bailon,
	kernel, Fabio Estevam, linux-arm-kernel, Shawn Guo, Aisheng Dong,
	devicetree, dl-linux-imx

On 8/7/2019 6:16 PM, Georgi Djakov wrote:

> Please put some commit text.

Will fix

>> +properties:
>> +  compatible:
>> +    contains:
>> +      enum:
>> +        - fsl,imx8mm-interconnect
> 
> Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
> up to you.

In the vendor tree busfreq is effectively its own subsystem with its own 
API calls used by imx drivers. As part of replacing this with standard 
devfreq + interconnect in upstream the old name should go away.

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-08-06 10:55   ` Leonard Crestez
@ 2019-08-21 14:02     ` Martin Kepplinger
  -1 siblings, 0 replies; 40+ messages in thread
From: Martin Kepplinger @ 2019-08-21 14:02 UTC (permalink / raw)
  To: Leonard Crestez, Georgi Djakov, Rob Herring,
	Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

On 06.08.19 12:55, Leonard Crestez wrote:
> This adds a platform driver for the i.MX8MM SoC.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/interconnect/imx/Kconfig          |   4 +
>  drivers/interconnect/imx/Makefile         |   1 +
>  drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>  include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>  4 files changed, 168 insertions(+)
>  create mode 100644 drivers/interconnect/imx/imx8mm.c
>  create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> 

Hi Leonard,

Do you plan to add such a driver for imx8mq too?

And I guess the commit message could be more descriptive here.

thanks for your work,

                      martin

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-21 14:02     ` Martin Kepplinger
  0 siblings, 0 replies; 40+ messages in thread
From: Martin Kepplinger @ 2019-08-21 14:02 UTC (permalink / raw)
  To: Leonard Crestez, Georgi Djakov, Rob Herring,
	Artur Świgoń,
	Alexandre Bailon, Viresh Kumar
  Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
	Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
	Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
	linux-arm-kernel

On 06.08.19 12:55, Leonard Crestez wrote:
> This adds a platform driver for the i.MX8MM SoC.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/interconnect/imx/Kconfig          |   4 +
>  drivers/interconnect/imx/Makefile         |   1 +
>  drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>  include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>  4 files changed, 168 insertions(+)
>  create mode 100644 drivers/interconnect/imx/imx8mm.c
>  create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> 

Hi Leonard,

Do you plan to add such a driver for imx8mq too?

And I guess the commit message could be more descriptive here.

thanks for your work,

                      martin

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-08-21 14:02     ` Martin Kepplinger
  (?)
@ 2019-08-21 14:21       ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-21 14:21 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel

On 21.08.2019 17:03, Martin Kepplinger wrote:
> On 06.08.19 12:55, Leonard Crestez wrote:
>> This adds a platform driver for the i.MX8MM SoC.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>   drivers/interconnect/imx/Kconfig          |   4 +
>>   drivers/interconnect/imx/Makefile         |   1 +
>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>   4 files changed, 168 insertions(+)
>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> 
> Do you plan to add such a driver for imx8mq too?

Yes! The topology is different (serving different IP blocks) but no 
functional code changes are required between 8mm 8mn 8mq.

I already wrote the code and tested it but didn't post because I want to 
get the core parts in first. I periodically push my "full" 
work-in-progress to github:

https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq

You need out-of-tree ATF changes or devfreq probe will fail:

https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-21 14:21       ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-21 14:21 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar, Mark Rutland, Aisheng Dong,
	Jacky Bai, Saravana Kannan, Anson Huang, Stephen Boyd,
	Michael Turquette, linux-pm, Krzysztof Kozlowski, Chanwoo Choi,
	Kyungmin Park, MyungJoo Ham, dl-linux-imx, kernel, Fabio Estevam,
	Shawn Guo, devicetree, linux-arm-kernel

On 21.08.2019 17:03, Martin Kepplinger wrote:
> On 06.08.19 12:55, Leonard Crestez wrote:
>> This adds a platform driver for the i.MX8MM SoC.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>   drivers/interconnect/imx/Kconfig          |   4 +
>>   drivers/interconnect/imx/Makefile         |   1 +
>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>   4 files changed, 168 insertions(+)
>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> 
> Do you plan to add such a driver for imx8mq too?

Yes! The topology is different (serving different IP blocks) but no 
functional code changes are required between 8mm 8mn 8mq.

I already wrote the code and tested it but didn't post because I want to 
get the core parts in first. I periodically push my "full" 
work-in-progress to github:

https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq

You need out-of-tree ATF changes or devfreq probe will fail:

https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-21 14:21       ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-08-21 14:21 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov

On 21.08.2019 17:03, Martin Kepplinger wrote:
> On 06.08.19 12:55, Leonard Crestez wrote:
>> This adds a platform driver for the i.MX8MM SoC.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>   drivers/interconnect/imx/Kconfig          |   4 +
>>   drivers/interconnect/imx/Makefile         |   1 +
>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>   4 files changed, 168 insertions(+)
>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
> 
> Do you plan to add such a driver for imx8mq too?

Yes! The topology is different (serving different IP blocks) but no 
functional code changes are required between 8mm 8mn 8mq.

I already wrote the code and tested it but didn't post because I want to 
get the core parts in first. I periodically push my "full" 
work-in-progress to github:

https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq

You need out-of-tree ATF changes or devfreq probe will fail:

https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-08-21 14:21       ` Leonard Crestez
  (?)
@ 2019-08-21 14:26         ` Martin Kepplinger
  -1 siblings, 0 replies; 40+ messages in thread
From: Martin Kepplinger @ 2019-08-21 14:26 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel

On 21.08.19 16:21, Leonard Crestez wrote:
> On 21.08.2019 17:03, Martin Kepplinger wrote:
>> On 06.08.19 12:55, Leonard Crestez wrote:
>>> This adds a platform driver for the i.MX8MM SoC.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>> ---
>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>   4 files changed, 168 insertions(+)
>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>
>> Do you plan to add such a driver for imx8mq too?
> 
> Yes! The topology is different (serving different IP blocks) but no 
> functional code changes are required between 8mm 8mn 8mq.
> 
> I already wrote the code and tested it but didn't post because I want to 
> get the core parts in first. I periodically push my "full" 
> work-in-progress to github:
> 
> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
> 
> You need out-of-tree ATF changes or devfreq probe will fail:
> 
> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
> 

Thanks for the update, that's good to hear. I'll get back to you when I
come around to test this and wish you good progress until then :)

                        martin

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-21 14:26         ` Martin Kepplinger
  0 siblings, 0 replies; 40+ messages in thread
From: Martin Kepplinger @ 2019-08-21 14:26 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Georgi Djakov, Rob Herring, Artur Świgoń,
	Alexandre Bailon, Viresh Kumar, Mark Rutland, Aisheng Dong,
	Jacky Bai, Saravana Kannan, Anson Huang, Stephen Boyd,
	Michael Turquette, linux-pm, Krzysztof Kozlowski, Chanwoo Choi,
	Kyungmin Park, MyungJoo Ham, dl-linux-imx, kernel, Fabio Estevam,
	Shawn Guo, devicetree, linux-arm-kernel

On 21.08.19 16:21, Leonard Crestez wrote:
> On 21.08.2019 17:03, Martin Kepplinger wrote:
>> On 06.08.19 12:55, Leonard Crestez wrote:
>>> This adds a platform driver for the i.MX8MM SoC.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>> ---
>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>   4 files changed, 168 insertions(+)
>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>
>> Do you plan to add such a driver for imx8mq too?
> 
> Yes! The topology is different (serving different IP blocks) but no 
> functional code changes are required between 8mm 8mn 8mq.
> 
> I already wrote the code and tested it but didn't post because I want to 
> get the core parts in first. I periodically push my "full" 
> work-in-progress to github:
> 
> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
> 
> You need out-of-tree ATF changes or devfreq probe will fail:
> 
> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
> 

Thanks for the update, that's good to hear. I'll get back to you when I
come around to test this and wish you good progress until then :)

                        martin



^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-08-21 14:26         ` Martin Kepplinger
  0 siblings, 0 replies; 40+ messages in thread
From: Martin Kepplinger @ 2019-08-21 14:26 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov

On 21.08.19 16:21, Leonard Crestez wrote:
> On 21.08.2019 17:03, Martin Kepplinger wrote:
>> On 06.08.19 12:55, Leonard Crestez wrote:
>>> This adds a platform driver for the i.MX8MM SoC.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>> ---
>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>   drivers/interconnect/imx/imx8mm.c         | 114 ++++++++++++++++++++++
>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>   4 files changed, 168 insertions(+)
>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>
>> Do you plan to add such a driver for imx8mq too?
> 
> Yes! The topology is different (serving different IP blocks) but no 
> functional code changes are required between 8mm 8mn 8mq.
> 
> I already wrote the code and tested it but didn't post because I want to 
> get the core parts in first. I periodically push my "full" 
> work-in-progress to github:
> 
> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
> 
> You need out-of-tree ATF changes or devfreq probe will fail:
> 
> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
> 

Thanks for the update, that's good to hear. I'll get back to you when I
come around to test this and wish you good progress until then :)

                        martin



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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-08-21 14:26         ` Martin Kepplinger
  (?)
@ 2019-10-10 14:43           ` Angus Ainslie
  -1 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-10 14:43 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Anson Huang, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	Martin Kepplinger, linux-arm-kernel, Aisheng Dong,
	Saravana Kannan, Stephen Boyd, Kyungmin Park, kernel,
	Fabio Estevam, Shawn Guo

Hi Leonard,

On 2019-08-21 07:26, Martin Kepplinger wrote:
> On 21.08.19 16:21, Leonard Crestez wrote:
>> On 21.08.2019 17:03, Martin Kepplinger wrote:
>>> On 06.08.19 12:55, Leonard Crestez wrote:
>>>> This adds a platform driver for the i.MX8MM SoC.
>>>> 
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>> ---
>>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>>   drivers/interconnect/imx/imx8mm.c         | 114 
>>>> ++++++++++++++++++++++
>>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>   4 files changed, 168 insertions(+)
>>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>> 
>>> Do you plan to add such a driver for imx8mq too?
>> 
>> Yes! The topology is different (serving different IP blocks) but no
>> functional code changes are required between 8mm 8mn 8mq.
>> 
>> I already wrote the code and tested it but didn't post because I want 
>> to
>> get the core parts in first. I periodically push my "full"
>> work-in-progress to github:
>> 
>> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
>> 
>> You need out-of-tree ATF changes or devfreq probe will fail:
>> 
>> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
>> 
> 
> Thanks for the update, that's good to hear. I'll get back to you when I
> come around to test this and wish you good progress until then :)
> 

I've taken up this work while Martin is on leave.

I've integrated your u-boot and ATF on our board and I have a couple of 
questions. Our board is running imx8mq B0 (Rev 2.0) silicon.

It looks like this line limits the training frequencies to 800 MHz and 
166 MHz

https://source.puri.sm/Librem5/uboot-imx/blob/busfreq/board/purism/librem5_devkit/lpddr4_timing_b0.c#L1365

Does 100 MHz and 25 MHz not work on B0 ?

I added the ddrc_and noc opp as well as the 166MHz opp

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L214

I also added the interconnects ( do we need them on imx8mq ? )

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L990

I had to add a hack as the PM QoS was limiting the bus speed to 399MHz , 
if you have any ideas why that would be appreciated.

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/drivers/devfreq/devfreq.c#L143

The driver is probing

[   12.136537] bus: 'platform': driver_probe_device: matched device 
3d400000.dram-controller with driver imx-ddrc-devfrq
[   12.147259] bus: 'platform': really_probe: probing driver 
imx-ddrc-devfreq with device 3d400000.dram-controller
[   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl 
handle
[   12.164197] arm_smcc rate 0 800000000
[   12.167880] arm_smcc rate 1 166750000
[   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from 
pmu imx8_ddr0
[   12.212403] Added freq 0 25000000
[   12.215742] Added freq 1 100000000
[   12.219177] Added freq 2 166750000
[   12.222648] Added freq 3 800000000
[   12.226105] device: 'devfreq0': device_add
[   12.230287] PM: Adding info for No Bus:devfreq0
[   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device 
'3d400000.dram-controller'
[   12.243699] bus: 'platform': really_probe: bound device 
3d400000.dram-controller to driver imx-ddrc-devfreq

Add seems to run correctly until it tries to adjust the clock to 166MHz

[   19.555482] ddrc checking rate 800000000 166750000
[   19.555489] ddrc checking rate 166750000 166750000
[   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to 
driver option1
[   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to 
change freq 800000000 to 166750000

And the board hangs there. Any ideas on how to get past this ?

Thanks
Angus

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-10 14:43           ` Angus Ainslie
  0 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-10 14:43 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov,
	Martin Kepplinger

Hi Leonard,

On 2019-08-21 07:26, Martin Kepplinger wrote:
> On 21.08.19 16:21, Leonard Crestez wrote:
>> On 21.08.2019 17:03, Martin Kepplinger wrote:
>>> On 06.08.19 12:55, Leonard Crestez wrote:
>>>> This adds a platform driver for the i.MX8MM SoC.
>>>> 
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>> ---
>>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>>   drivers/interconnect/imx/imx8mm.c         | 114 
>>>> ++++++++++++++++++++++
>>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>   4 files changed, 168 insertions(+)
>>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>> 
>>> Do you plan to add such a driver for imx8mq too?
>> 
>> Yes! The topology is different (serving different IP blocks) but no
>> functional code changes are required between 8mm 8mn 8mq.
>> 
>> I already wrote the code and tested it but didn't post because I want 
>> to
>> get the core parts in first. I periodically push my "full"
>> work-in-progress to github:
>> 
>> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
>> 
>> You need out-of-tree ATF changes or devfreq probe will fail:
>> 
>> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
>> 
> 
> Thanks for the update, that's good to hear. I'll get back to you when I
> come around to test this and wish you good progress until then :)
> 

I've taken up this work while Martin is on leave.

I've integrated your u-boot and ATF on our board and I have a couple of 
questions. Our board is running imx8mq B0 (Rev 2.0) silicon.

It looks like this line limits the training frequencies to 800 MHz and 
166 MHz

https://source.puri.sm/Librem5/uboot-imx/blob/busfreq/board/purism/librem5_devkit/lpddr4_timing_b0.c#L1365

Does 100 MHz and 25 MHz not work on B0 ?

I added the ddrc_and noc opp as well as the 166MHz opp

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L214

I also added the interconnects ( do we need them on imx8mq ? )

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L990

I had to add a hack as the PM QoS was limiting the bus speed to 399MHz , 
if you have any ideas why that would be appreciated.

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/drivers/devfreq/devfreq.c#L143

The driver is probing

[   12.136537] bus: 'platform': driver_probe_device: matched device 
3d400000.dram-controller with driver imx-ddrc-devfrq
[   12.147259] bus: 'platform': really_probe: probing driver 
imx-ddrc-devfreq with device 3d400000.dram-controller
[   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl 
handle
[   12.164197] arm_smcc rate 0 800000000
[   12.167880] arm_smcc rate 1 166750000
[   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from 
pmu imx8_ddr0
[   12.212403] Added freq 0 25000000
[   12.215742] Added freq 1 100000000
[   12.219177] Added freq 2 166750000
[   12.222648] Added freq 3 800000000
[   12.226105] device: 'devfreq0': device_add
[   12.230287] PM: Adding info for No Bus:devfreq0
[   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device 
'3d400000.dram-controller'
[   12.243699] bus: 'platform': really_probe: bound device 
3d400000.dram-controller to driver imx-ddrc-devfreq

Add seems to run correctly until it tries to adjust the clock to 166MHz

[   19.555482] ddrc checking rate 800000000 166750000
[   19.555489] ddrc checking rate 166750000 166750000
[   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to 
driver option1
[   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to 
change freq 800000000 to 166750000

And the board hangs there. Any ideas on how to get past this ?

Thanks
Angus

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-10 14:43           ` Angus Ainslie
  0 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-10 14:43 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Anson Huang, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	Martin Kepplinger, linux-arm-kernel, Aisheng Dong,
	Saravana Kannan, Stephen Boyd, Kyungmin Park, kernel,
	Fabio Estevam, Shawn Guo, Georgi Djakov

Hi Leonard,

On 2019-08-21 07:26, Martin Kepplinger wrote:
> On 21.08.19 16:21, Leonard Crestez wrote:
>> On 21.08.2019 17:03, Martin Kepplinger wrote:
>>> On 06.08.19 12:55, Leonard Crestez wrote:
>>>> This adds a platform driver for the i.MX8MM SoC.
>>>> 
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>> ---
>>>>   drivers/interconnect/imx/Kconfig          |   4 +
>>>>   drivers/interconnect/imx/Makefile         |   1 +
>>>>   drivers/interconnect/imx/imx8mm.c         | 114 
>>>> ++++++++++++++++++++++
>>>>   include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>   4 files changed, 168 insertions(+)
>>>>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>> 
>>> Do you plan to add such a driver for imx8mq too?
>> 
>> Yes! The topology is different (serving different IP blocks) but no
>> functional code changes are required between 8mm 8mn 8mq.
>> 
>> I already wrote the code and tested it but didn't post because I want 
>> to
>> get the core parts in first. I periodically push my "full"
>> work-in-progress to github:
>> 
>> https://github.com/cdleonard/linux/commits/next_imx8mm_busfreq
>> 
>> You need out-of-tree ATF changes or devfreq probe will fail:
>> 
>> https://github.com/cdleonard/arm-trusted-firmware/commits/imx_2.0.y_busfreq
>> 
> 
> Thanks for the update, that's good to hear. I'll get back to you when I
> come around to test this and wish you good progress until then :)
> 

I've taken up this work while Martin is on leave.

I've integrated your u-boot and ATF on our board and I have a couple of 
questions. Our board is running imx8mq B0 (Rev 2.0) silicon.

It looks like this line limits the training frequencies to 800 MHz and 
166 MHz

https://source.puri.sm/Librem5/uboot-imx/blob/busfreq/board/purism/librem5_devkit/lpddr4_timing_b0.c#L1365

Does 100 MHz and 25 MHz not work on B0 ?

I added the ddrc_and noc opp as well as the 166MHz opp

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L214

I also added the interconnects ( do we need them on imx8mq ? )

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/arch/arm64/boot/dts/freescale/imx8mq.dtsi#L990

I had to add a hack as the PM QoS was limiting the bus speed to 399MHz , 
if you have any ideas why that would be appreciated.

https://source.puri.sm/angus.ainslie/linux-next/blob/busfreq/drivers/devfreq/devfreq.c#L143

The driver is probing

[   12.136537] bus: 'platform': driver_probe_device: matched device 
3d400000.dram-controller with driver imx-ddrc-devfrq
[   12.147259] bus: 'platform': really_probe: probing driver 
imx-ddrc-devfreq with device 3d400000.dram-controller
[   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl 
handle
[   12.164197] arm_smcc rate 0 800000000
[   12.167880] arm_smcc rate 1 166750000
[   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0 
uvmin:0 uvmax:0 latency:0
[   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from 
pmu imx8_ddr0
[   12.212403] Added freq 0 25000000
[   12.215742] Added freq 1 100000000
[   12.219177] Added freq 2 166750000
[   12.222648] Added freq 3 800000000
[   12.226105] device: 'devfreq0': device_add
[   12.230287] PM: Adding info for No Bus:devfreq0
[   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device 
'3d400000.dram-controller'
[   12.243699] bus: 'platform': really_probe: bound device 
3d400000.dram-controller to driver imx-ddrc-devfreq

Add seems to run correctly until it tries to adjust the clock to 166MHz

[   19.555482] ddrc checking rate 800000000 166750000
[   19.555489] ddrc checking rate 166750000 166750000
[   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to 
driver option1
[   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to 
change freq 800000000 to 166750000

And the board hangs there. Any ideas on how to get past this ?

Thanks
Angus

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

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-10-10 14:43           ` Angus Ainslie
@ 2019-10-15 14:05             ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-10-15 14:05 UTC (permalink / raw)
  To: Angus Ainslie, Jacky Bai
  Cc: Mark Rutland, Artur Świgoń,
	Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov,
	Martin Kepplinger

On 10.10.2019 17:43, Angus Ainslie wrote:

>>>>> This adds a platform driver for the i.MX8MM SoC.
>>>>>
>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>>> ---
>>>>>    drivers/interconnect/imx/Kconfig          |   4 +
>>>>>    drivers/interconnect/imx/Makefile         |   1 +
>>>>>    drivers/interconnect/imx/imx8mm.c         | 114
>>>>> ++++++++++++++++++++++
>>>>>    include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>>    4 files changed, 168 insertions(+)
>>>>>    create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>>    create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>>>
>>>> Do you plan to add such a driver for imx8mq too?
>>>
>>> Yes! The topology is different (serving different IP blocks) but no
>>> functional code changes are required between 8mm 8mn 8mq.
>>
>> Thanks for the update, that's good to hear. I'll get back to you when I
>> come around to test this and wish you good progress until then :)
>>
> I've taken up this work while Martin is on leave.
> 
> I've integrated your u-boot and ATF on our board and I have a couple of
> questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
> 
> It looks like this line limits the training frequencies to 800 MHz and
> 166 MHz

Yes! This is due to a hardware errata which was solved in B1: DRAM pll 
can't be disabled. This means that instead of 25/100/800 freqs are 
166/800, and this requires code changes.

> Does 100 MHz and 25 MHz not work on B0 ?

No, lower rates require dram clk from a composite slice (dram_alt_root)

> I added the ddrc_and noc opp as well as the 166MHz opp
> 
> I also added the interconnects ( do we need them on imx8mq ? )

The interconnect stuff is not required to switch dram frequency, it's 
for device to make minimum bandwidth requests. It an additional feature 
on top.

As a hack I configured FEC to do so but a saner example would be to 
request bandwidth based on display resolution and color depth.

> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz ,
> if you have any ideas why that would be appreciated.

You probably need to set ethernet down (which is awkward) or better just 
drop the interconnect properties and test using the devfreq userspace 
governor.

> The driver is probing
> 
> [   12.136537] bus: 'platform': driver_probe_device: matched device
> 3d400000.dram-controller with driver imx-ddrc-devfrq
> [   12.147259] bus: 'platform': really_probe: probing driver
> imx-ddrc-devfreq with device 3d400000.dram-controller
> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
> handle
> [   12.164197] arm_smcc rate 0 800000000
> [   12.167880] arm_smcc rate 1 166750000
> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
> pmu imx8_ddr0
> [   12.212403] Added freq 0 25000000
> [   12.215742] Added freq 1 100000000
> [   12.219177] Added freq 2 166750000
> [   12.222648] Added freq 3 800000000
> [   12.226105] device: 'devfreq0': device_add
> [   12.230287] PM: Adding info for No Bus:devfreq0
> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device
> '3d400000.dram-controller'
> [   12.243699] bus: 'platform': really_probe: bound device
> 3d400000.dram-controller to driver imx-ddrc-devfreq
> 
> Add seems to run correctly until it tries to adjust the clock to 166MHz
> 
> [   19.555482] ddrc checking rate 800000000 166750000
> [   19.555489] ddrc checking rate 166750000 166750000
> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to
> driver option1
> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to
> change freq 800000000 to 166750000
> 
> And the board hangs there. Any ideas on how to get past this ?

Please try this ATF patch:

https://github.com/cdleonard/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9

I tested switching on imx8mq-evk with B0 SoC but a few additional 
changes are required in kernel to support switching between rates which 
are both backed by PLL:

* Mark the PLL CLK_GET_RATE_NOCACHE
* Set the rate to 166935483 exactly (to match clk_get_rate)
* Make the rounding in imx-ddrc more generous.

I will integrate these changes into the next version.

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-15 14:05             ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-10-15 14:05 UTC (permalink / raw)
  To: Angus Ainslie, Jacky Bai
  Cc: Mark Rutland, Artur Świgoń,
	Viresh Kumar, Michael Turquette, Alexandre Bailon, Anson Huang,
	Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham, dl-linux-imx,
	devicetree, linux-pm, Rob Herring, Martin Kepplinger,
	linux-arm-kernel, Aisheng Dong, Saravana Kannan, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov

On 10.10.2019 17:43, Angus Ainslie wrote:

>>>>> This adds a platform driver for the i.MX8MM SoC.
>>>>>
>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>>> ---
>>>>>    drivers/interconnect/imx/Kconfig          |   4 +
>>>>>    drivers/interconnect/imx/Makefile         |   1 +
>>>>>    drivers/interconnect/imx/imx8mm.c         | 114
>>>>> ++++++++++++++++++++++
>>>>>    include/dt-bindings/interconnect/imx8mm.h |  49 ++++++++++
>>>>>    4 files changed, 168 insertions(+)
>>>>>    create mode 100644 drivers/interconnect/imx/imx8mm.c
>>>>>    create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>>>>
>>>> Do you plan to add such a driver for imx8mq too?
>>>
>>> Yes! The topology is different (serving different IP blocks) but no
>>> functional code changes are required between 8mm 8mn 8mq.
>>
>> Thanks for the update, that's good to hear. I'll get back to you when I
>> come around to test this and wish you good progress until then :)
>>
> I've taken up this work while Martin is on leave.
> 
> I've integrated your u-boot and ATF on our board and I have a couple of
> questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
> 
> It looks like this line limits the training frequencies to 800 MHz and
> 166 MHz

Yes! This is due to a hardware errata which was solved in B1: DRAM pll 
can't be disabled. This means that instead of 25/100/800 freqs are 
166/800, and this requires code changes.

> Does 100 MHz and 25 MHz not work on B0 ?

No, lower rates require dram clk from a composite slice (dram_alt_root)

> I added the ddrc_and noc opp as well as the 166MHz opp
> 
> I also added the interconnects ( do we need them on imx8mq ? )

The interconnect stuff is not required to switch dram frequency, it's 
for device to make minimum bandwidth requests. It an additional feature 
on top.

As a hack I configured FEC to do so but a saner example would be to 
request bandwidth based on display resolution and color depth.

> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz ,
> if you have any ideas why that would be appreciated.

You probably need to set ethernet down (which is awkward) or better just 
drop the interconnect properties and test using the devfreq userspace 
governor.

> The driver is probing
> 
> [   12.136537] bus: 'platform': driver_probe_device: matched device
> 3d400000.dram-controller with driver imx-ddrc-devfrq
> [   12.147259] bus: 'platform': really_probe: probing driver
> imx-ddrc-devfreq with device 3d400000.dram-controller
> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
> handle
> [   12.164197] arm_smcc rate 0 800000000
> [   12.167880] arm_smcc rate 1 166750000
> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
> uvmin:0 uvmax:0 latency:0
> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
> pmu imx8_ddr0
> [   12.212403] Added freq 0 25000000
> [   12.215742] Added freq 1 100000000
> [   12.219177] Added freq 2 166750000
> [   12.222648] Added freq 3 800000000
> [   12.226105] device: 'devfreq0': device_add
> [   12.230287] PM: Adding info for No Bus:devfreq0
> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to device
> '3d400000.dram-controller'
> [   12.243699] bus: 'platform': really_probe: bound device
> 3d400000.dram-controller to driver imx-ddrc-devfreq
> 
> Add seems to run correctly until it tries to adjust the clock to 166MHz
> 
> [   19.555482] ddrc checking rate 800000000 166750000
> [   19.555489] ddrc checking rate 166750000 166750000
> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 to
> driver option1
> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to
> change freq 800000000 to 166750000
> 
> And the board hangs there. Any ideas on how to get past this ?

Please try this ATF patch:

https://github.com/cdleonard/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9

I tested switching on imx8mq-evk with B0 SoC but a few additional 
changes are required in kernel to support switching between rates which 
are both backed by PLL:

* Mark the PLL CLK_GET_RATE_NOCACHE
* Set the rate to 166935483 exactly (to match clk_get_rate)
* Make the rounding in imx-ddrc more generous.

I will integrate these changes into the next version.

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-10-15 14:05             ` Leonard Crestez
@ 2019-10-16 14:09               ` Angus Ainslie
  -1 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-16 14:09 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Jacky Bai, Mark Rutland, Artur Świgoń,
	Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov,
	Martin Kepplinger, linux-pm-owner

On 2019-10-15 07:05, Leonard Crestez wrote:
> On 10.10.2019 17:43, Angus Ainslie wrote:
>> 
>> I've integrated your u-boot and ATF on our board and I have a couple 
>> of
>> questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>> 
>> It looks like this line limits the training frequencies to 800 MHz and
>> 166 MHz
> 
> Yes! This is due to a hardware errata which was solved in B1: DRAM pll
> can't be disabled. This means that instead of 25/100/800 freqs are
> 166/800, and this requires code changes.
> 
>> Does 100 MHz and 25 MHz not work on B0 ?
> 
> No, lower rates require dram clk from a composite slice (dram_alt_root)
> 
>> I added the ddrc_and noc opp as well as the 166MHz opp
>> 
>> I also added the interconnects ( do we need them on imx8mq ? )
> 
> The interconnect stuff is not required to switch dram frequency, it's
> for device to make minimum bandwidth requests. It an additional feature
> on top.
> 
> As a hack I configured FEC to do so but a saner example would be to
> request bandwidth based on display resolution and color depth.
> 
>> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz 
>> ,
>> if you have any ideas why that would be appreciated.
> 
> You probably need to set ethernet down (which is awkward) or better 
> just
> drop the interconnect properties and test using the devfreq userspace
> governor.
> 
>> The driver is probing
>> 
>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>> [   12.147259] bus: 'platform': really_probe: probing driver
>> imx-ddrc-devfreq with device 3d400000.dram-controller
>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>> handle
>> [   12.164197] arm_smcc rate 0 800000000
>> [   12.167880] arm_smcc rate 1 166750000
>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
>> pmu imx8_ddr0
>> [   12.212403] Added freq 0 25000000
>> [   12.215742] Added freq 1 100000000
>> [   12.219177] Added freq 2 166750000
>> [   12.222648] Added freq 3 800000000
>> [   12.226105] device: 'devfreq0': device_add
>> [   12.230287] PM: Adding info for No Bus:devfreq0
>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to 
>> device
>> '3d400000.dram-controller'
>> [   12.243699] bus: 'platform': really_probe: bound device
>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>> 
>> Add seems to run correctly until it tries to adjust the clock to 
>> 166MHz
>> 
>> [   19.555482] ddrc checking rate 800000000 166750000
>> [   19.555489] ddrc checking rate 166750000 166750000
>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 
>> to
>> driver option1
>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about 
>> to
>> change freq 800000000 to 166750000
>> 
>> And the board hangs there. Any ideas on how to get past this ?
> 
> Please try this ATF patch:
> 
> https://github.com/cdleonard/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9
> 

Ok applied this to the tree we're using

https://source.puri.sm/Librem5/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9

> I tested switching on imx8mq-evk with B0 SoC but a few additional
> changes are required in kernel to support switching between rates which
> are both backed by PLL:
> 
> * Mark the PLL CLK_GET_RATE_NOCACHE

Is this what you meant ?

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 2813884f69c1..e5f50cf8a264 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct 
platform_device *pdev)
         clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out", 
sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 0x30, 
CLK_IS_CRITICAL);
         clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out", 
sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 0x3c, 
CLK_IS_CRITICAL);
         clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out", 
sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 0x48, 
CLK_IS_CRITICAL);
-       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out", 
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60, 
CLK_IS_CRITICAL);
+       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out", 
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60, 
CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);

         /* SYS PLL1 fixed output */
         clks[IMX8MQ_SYS1_PLL_40M_CG] = imx_clk_gate("sys1_pll_40m_cg", 
"sys1_pll_out", base + 0x30, 9);

> * Set the rate to 166935483 exactly (to match clk_get_rate)

Okay I hacked that in

diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
index 4eed6f50bb8d..a832768a865f 100644
--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device 
*dev)
                         return -ENODEV;
                 }

+               /* B0 hack */
+               if ( freq->rate == 166750000 )
+                       freq->rate = 166935483;
+
                 pr_err( "arm_smcc rate %d %lu\n", index, freq->rate );
         }

--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -211,7 +211,7 @@
                         opp-hz = /bits/ 64 <100000000>;
                 };
                 opp-166M {
-                       opp-hz = /bits/ 64 <166750000>;
+                       opp-hz = /bits/ 64 <166935483>;
                 };
                 opp-800M {
                         opp-hz = /bits/ 64 <800000000>;


> * Make the rounding in imx-ddrc more generous.

Sorry I don't understand what you mean by this.

Adding the other changes the board no longer hangs when trying to change 
frequencies but it also doesn't seem to actually change the frequency.

[    3.076426] ddrc checking rate 800000000 166935483
[    3.081290] ddrc checking rate 166935483 166935483
[    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to 
change freq 800000000 to 166935483
[    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed 
freq 800000000 to 166935483

root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
800000000
root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
166935483

Is this the missing rounding or is there something else missing ?

Thanks
Angus


> 
> I will integrate these changes into the next version.
> 
> --
> Regards,
> Leonard

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-16 14:09               ` Angus Ainslie
  0 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-16 14:09 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, linux-pm-owner,
	Alexandre Bailon, Anson Huang, Krzysztof Kozlowski, Chanwoo Choi,
	MyungJoo Ham, dl-linux-imx, devicetree, linux-pm, Rob Herring,
	Martin Kepplinger, linux-arm-kernel, Aisheng Dong,
	Saravana Kannan, Stephen Boyd, Kyungmin Park, kernel,
	Fabio Estevam, Shawn Guo, Georgi Djakov

On 2019-10-15 07:05, Leonard Crestez wrote:
> On 10.10.2019 17:43, Angus Ainslie wrote:
>> 
>> I've integrated your u-boot and ATF on our board and I have a couple 
>> of
>> questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>> 
>> It looks like this line limits the training frequencies to 800 MHz and
>> 166 MHz
> 
> Yes! This is due to a hardware errata which was solved in B1: DRAM pll
> can't be disabled. This means that instead of 25/100/800 freqs are
> 166/800, and this requires code changes.
> 
>> Does 100 MHz and 25 MHz not work on B0 ?
> 
> No, lower rates require dram clk from a composite slice (dram_alt_root)
> 
>> I added the ddrc_and noc opp as well as the 166MHz opp
>> 
>> I also added the interconnects ( do we need them on imx8mq ? )
> 
> The interconnect stuff is not required to switch dram frequency, it's
> for device to make minimum bandwidth requests. It an additional feature
> on top.
> 
> As a hack I configured FEC to do so but a saner example would be to
> request bandwidth based on display resolution and color depth.
> 
>> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz 
>> ,
>> if you have any ideas why that would be appreciated.
> 
> You probably need to set ethernet down (which is awkward) or better 
> just
> drop the interconnect properties and test using the devfreq userspace
> governor.
> 
>> The driver is probing
>> 
>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>> [   12.147259] bus: 'platform': really_probe: probing driver
>> imx-ddrc-devfreq with device 3d400000.dram-controller
>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>> handle
>> [   12.164197] arm_smcc rate 0 800000000
>> [   12.167880] arm_smcc rate 1 166750000
>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>> uvmin:0 uvmax:0 latency:0
>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
>> pmu imx8_ddr0
>> [   12.212403] Added freq 0 25000000
>> [   12.215742] Added freq 1 100000000
>> [   12.219177] Added freq 2 166750000
>> [   12.222648] Added freq 3 800000000
>> [   12.226105] device: 'devfreq0': device_add
>> [   12.230287] PM: Adding info for No Bus:devfreq0
>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to 
>> device
>> '3d400000.dram-controller'
>> [   12.243699] bus: 'platform': really_probe: bound device
>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>> 
>> Add seems to run correctly until it tries to adjust the clock to 
>> 166MHz
>> 
>> [   19.555482] ddrc checking rate 800000000 166750000
>> [   19.555489] ddrc checking rate 166750000 166750000
>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0 
>> to
>> driver option1
>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about 
>> to
>> change freq 800000000 to 166750000
>> 
>> And the board hangs there. Any ideas on how to get past this ?
> 
> Please try this ATF patch:
> 
> https://github.com/cdleonard/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9
> 

Ok applied this to the tree we're using

https://source.puri.sm/Librem5/arm-trusted-firmware/commit/783fc2b2c4266bfdb5218e4d9b6b2bc90849e0e9

> I tested switching on imx8mq-evk with B0 SoC but a few additional
> changes are required in kernel to support switching between rates which
> are both backed by PLL:
> 
> * Mark the PLL CLK_GET_RATE_NOCACHE

Is this what you meant ?

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 2813884f69c1..e5f50cf8a264 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct 
platform_device *pdev)
         clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out", 
sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 0x30, 
CLK_IS_CRITICAL);
         clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out", 
sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 0x3c, 
CLK_IS_CRITICAL);
         clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out", 
sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 0x48, 
CLK_IS_CRITICAL);
-       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out", 
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60, 
CLK_IS_CRITICAL);
+       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out", 
dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60, 
CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);

         /* SYS PLL1 fixed output */
         clks[IMX8MQ_SYS1_PLL_40M_CG] = imx_clk_gate("sys1_pll_40m_cg", 
"sys1_pll_out", base + 0x30, 9);

> * Set the rate to 166935483 exactly (to match clk_get_rate)

Okay I hacked that in

diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
index 4eed6f50bb8d..a832768a865f 100644
--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device 
*dev)
                         return -ENODEV;
                 }

+               /* B0 hack */
+               if ( freq->rate == 166750000 )
+                       freq->rate = 166935483;
+
                 pr_err( "arm_smcc rate %d %lu\n", index, freq->rate );
         }

--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -211,7 +211,7 @@
                         opp-hz = /bits/ 64 <100000000>;
                 };
                 opp-166M {
-                       opp-hz = /bits/ 64 <166750000>;
+                       opp-hz = /bits/ 64 <166935483>;
                 };
                 opp-800M {
                         opp-hz = /bits/ 64 <800000000>;


> * Make the rounding in imx-ddrc more generous.

Sorry I don't understand what you mean by this.

Adding the other changes the board no longer hangs when trying to change 
frequencies but it also doesn't seem to actually change the frequency.

[    3.076426] ddrc checking rate 800000000 166935483
[    3.081290] ddrc checking rate 166935483 166935483
[    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to 
change freq 800000000 to 166935483
[    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed 
freq 800000000 to 166935483

root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
800000000
root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
166935483

Is this the missing rounding or is there something else missing ?

Thanks
Angus


> 
> I will integrate these changes into the next version.
> 
> --
> Regards,
> Leonard

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

^ permalink raw reply related	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-10-16 14:09               ` Angus Ainslie
@ 2019-10-16 14:54                 ` Leonard Crestez
  -1 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-10-16 14:54 UTC (permalink / raw)
  To: Angus Ainslie
  Cc: Jacky Bai, Mark Rutland, Artur Świgoń,
	Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov,
	Martin Kepplinger, linux-pm-owner

On 16.10.2019 17:09, Angus Ainslie wrote:
> On 2019-10-15 07:05, Leonard Crestez wrote:
>> On 10.10.2019 17:43, Angus Ainslie wrote:
>>>
>>> I've integrated your u-boot and ATF on our board and I have a couple
>>> of questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>>>
>>> It looks like this line limits the training frequencies to 800 MHz and
>>> 166 MHz
>>
>> Yes! This is due to a hardware errata which was solved in B1: DRAM pll
>> can't be disabled. This means that instead of 25/100/800 freqs are
>> 166/800, and this requires code changes.
>>
>>> Does 100 MHz and 25 MHz not work on B0 ?
>>
>> No, lower rates require dram clk from a composite slice (dram_alt_root)
>>
>>> I added the ddrc_and noc opp as well as the 166MHz opp
>>>
>>> I also added the interconnects ( do we need them on imx8mq ? )
>>
>> The interconnect stuff is not required to switch dram frequency, it's
>> for device to make minimum bandwidth requests. It an additional feature
>> on top.
>>
>> As a hack I configured FEC to do so but a saner example would be to
>> request bandwidth based on display resolution and color depth.
>>
>>> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz,
>>> if you have any ideas why that would be appreciated.
>>
>> You probably need to set ethernet down (which is awkward) or better
>> just drop the interconnect properties and test using the devfreq userspace
>> governor.
>>
>>> The driver is probing
>>>
>>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>>> [   12.147259] bus: 'platform': really_probe: probing driver
>>> imx-ddrc-devfreq with device 3d400000.dram-controller
>>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>>> handle
>>> [   12.164197] arm_smcc rate 0 800000000
>>> [   12.167880] arm_smcc rate 1 166750000
>>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
>>> pmu imx8_ddr0
>>> [   12.212403] Added freq 0 25000000
>>> [   12.215742] Added freq 1 100000000
>>> [   12.219177] Added freq 2 166750000
>>> [   12.222648] Added freq 3 800000000
>>> [   12.226105] device: 'devfreq0': device_add
>>> [   12.230287] PM: Adding info for No Bus:devfreq0
>>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to
>>> device
>>> '3d400000.dram-controller'
>>> [   12.243699] bus: 'platform': really_probe: bound device
>>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>>>
>>> Add seems to run correctly until it tries to adjust the clock to
>>> 166MHz
>>>
>>> [   19.555482] ddrc checking rate 800000000 166750000
>>> [   19.555489] ddrc checking rate 166750000 166750000
>>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0
>>> to
>>> driver option1
>>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
>>> to
>>> change freq 800000000 to 166750000
>>>
>>> And the board hangs there. Any ideas on how to get past this ?
>>
>> Please try this ATF patch:
> 
> Ok applied this to the tree we're using
> 
>> I tested switching on imx8mq-evk with B0 SoC but a few additional
>> changes are required in kernel to support switching between rates which
>> are both backed by PLL:
>>
>> * Mark the PLL CLK_GET_RATE_NOCACHE
> 
> Is this what you meant ?
> 
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index 2813884f69c1..e5f50cf8a264 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct
> platform_device *pdev)
>           clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out",
> sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 0x30,
> CLK_IS_CRITICAL);
>           clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out",
> sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 0x3c,
> CLK_IS_CRITICAL);
>           clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out",
> sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 0x48,
> CLK_IS_CRITICAL);
> -       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60,
> CLK_IS_CRITICAL);
> +       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60,
> CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);

Yes.

>> * Set the rate to 166935483 exactly (to match clk_get_rate)
> 
> Okay I hacked that in
> 
> diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
> index 4eed6f50bb8d..a832768a865f 100644
> --- a/drivers/devfreq/imx-ddrc.c
> +++ b/drivers/devfreq/imx-ddrc.c
> @@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device
> *dev)
>                           return -ENODEV;
>                   }
> 
> +               /* B0 hack */
> +               if ( freq->rate == 166750000 )
> +                       freq->rate = 166935483;
> +inserting 
>                   pr_err( "arm_smcc rate %d %lu\n", index, freq->rate );
>           }

A nicer solution would be to keep imx_ddrc_freq.rate in MT/s as reported 
by firmware and divide by 25000 in imx_ddrc_find_freq instead.

> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -211,7 +211,7 @@
>                           opp-hz = /bits/ 64 <100000000>;
>                   };
>                   opp-166M {
> -                       opp-hz = /bits/ 64 <166750000>;
> +                       opp-hz = /bits/ 64 <166935483>;
>                   };
>                   opp-800M {
>                           opp-hz = /bits/ 64 <800000000>;

Yes, this is the precise clock rate in Hz.

>> * Make the rounding in imx-ddrc more generous.
> 
> Sorry I don't understand what you mean by this

I meant to make imx_ddrc_find_freq find 667 MT/s for an OPP of 166935483:

         /*
          * Firmware reports values in MT/s, so we round-down from Hz
          * Rounding is extra generous to ensure a match.
          */
         rate = DIV_ROUND_CLOSEST(rate, 250000);
         for (i = 0; i < priv->freq_count; ++i) {
                 struct imx_ddrc_freq *freq = &priv->freq_table[i];
                 if (freq->rate == rate ||
                                 freq->rate + 1 == rate ||
                                 freq->rate - 1 == rate)
                         return freq;
         }

But your B0 hack above should also work.

> Adding the other changes the board no longer hangs when trying to change
> frequencies but it also doesn't seem to actually change the frequency.
> 
> [    3.076426] ddrc checking rate 800000000 166935483
> [    3.081290] ddrc checking rate 166935483 166935483
> [    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to
> change freq 800000000 to 166935483
> [    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed
> freq 800000000 to 166935483
> 
> root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
> 800000000
> root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
> 166935483

The target_freq value is from clk_get_rate(dram_core) but it is 
dram_core's parent which gets updated. It seems that a clk mux ignores 
CLK_GET_RATE_NOCACHE on the parent.

An update can be forced by adding `clk_get_rate(new_dram_core_parent);` 
at the end of imx_ddrc_set_freq.

You should also be able to check by looking at clk_summary or
/sys/kernel/debug/clk/dram_core_clk/clk_rate
/sys/kernel/debug/clk/dram_pll_out/clk_rate

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-16 14:54                 ` Leonard Crestez
  0 siblings, 0 replies; 40+ messages in thread
From: Leonard Crestez @ 2019-10-16 14:54 UTC (permalink / raw)
  To: Angus Ainslie
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, linux-pm-owner,
	Alexandre Bailon, Anson Huang, Krzysztof Kozlowski, Chanwoo Choi,
	MyungJoo Ham, dl-linux-imx, devicetree, linux-pm, Rob Herring,
	Martin Kepplinger, linux-arm-kernel, Aisheng Dong,
	Saravana Kannan, Stephen Boyd, Kyungmin Park, kernel,
	Fabio Estevam, Shawn Guo, Georgi Djakov

On 16.10.2019 17:09, Angus Ainslie wrote:
> On 2019-10-15 07:05, Leonard Crestez wrote:
>> On 10.10.2019 17:43, Angus Ainslie wrote:
>>>
>>> I've integrated your u-boot and ATF on our board and I have a couple
>>> of questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>>>
>>> It looks like this line limits the training frequencies to 800 MHz and
>>> 166 MHz
>>
>> Yes! This is due to a hardware errata which was solved in B1: DRAM pll
>> can't be disabled. This means that instead of 25/100/800 freqs are
>> 166/800, and this requires code changes.
>>
>>> Does 100 MHz and 25 MHz not work on B0 ?
>>
>> No, lower rates require dram clk from a composite slice (dram_alt_root)
>>
>>> I added the ddrc_and noc opp as well as the 166MHz opp
>>>
>>> I also added the interconnects ( do we need them on imx8mq ? )
>>
>> The interconnect stuff is not required to switch dram frequency, it's
>> for device to make minimum bandwidth requests. It an additional feature
>> on top.
>>
>> As a hack I configured FEC to do so but a saner example would be to
>> request bandwidth based on display resolution and color depth.
>>
>>> I had to add a hack as the PM QoS was limiting the bus speed to 399MHz,
>>> if you have any ideas why that would be appreciated.
>>
>> You probably need to set ethernet down (which is awkward) or better
>> just drop the interconnect properties and test using the devfreq userspace
>> governor.
>>
>>> The driver is probing
>>>
>>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>>> [   12.147259] bus: 'platform': really_probe: probing driver
>>> imx-ddrc-devfreq with device 3d400000.dram-controller
>>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>>> handle
>>> [   12.164197] arm_smcc rate 0 800000000
>>> [   12.167880] arm_smcc rate 1 166750000
>>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>>> uvmin:0 uvmax:0 latency:0
>>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events from
>>> pmu imx8_ddr0
>>> [   12.212403] Added freq 0 25000000
>>> [   12.215742] Added freq 1 100000000
>>> [   12.219177] Added freq 2 166750000
>>> [   12.222648] Added freq 3 800000000
>>> [   12.226105] device: 'devfreq0': device_add
>>> [   12.230287] PM: Adding info for No Bus:devfreq0
>>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to
>>> device
>>> '3d400000.dram-controller'
>>> [   12.243699] bus: 'platform': really_probe: bound device
>>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>>>
>>> Add seems to run correctly until it tries to adjust the clock to
>>> 166MHz
>>>
>>> [   19.555482] ddrc checking rate 800000000 166750000
>>> [   19.555489] ddrc checking rate 166750000 166750000
>>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0
>>> to
>>> driver option1
>>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
>>> to
>>> change freq 800000000 to 166750000
>>>
>>> And the board hangs there. Any ideas on how to get past this ?
>>
>> Please try this ATF patch:
> 
> Ok applied this to the tree we're using
> 
>> I tested switching on imx8mq-evk with B0 SoC but a few additional
>> changes are required in kernel to support switching between rates which
>> are both backed by PLL:
>>
>> * Mark the PLL CLK_GET_RATE_NOCACHE
> 
> Is this what you meant ?
> 
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index 2813884f69c1..e5f50cf8a264 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct
> platform_device *pdev)
>           clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out",
> sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 0x30,
> CLK_IS_CRITICAL);
>           clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out",
> sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 0x3c,
> CLK_IS_CRITICAL);
>           clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out",
> sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 0x48,
> CLK_IS_CRITICAL);
> -       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60,
> CLK_IS_CRITICAL);
> +       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 0x60,
> CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);

Yes.

>> * Set the rate to 166935483 exactly (to match clk_get_rate)
> 
> Okay I hacked that in
> 
> diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
> index 4eed6f50bb8d..a832768a865f 100644
> --- a/drivers/devfreq/imx-ddrc.c
> +++ b/drivers/devfreq/imx-ddrc.c
> @@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device
> *dev)
>                           return -ENODEV;
>                   }
> 
> +               /* B0 hack */
> +               if ( freq->rate == 166750000 )
> +                       freq->rate = 166935483;
> +inserting 
>                   pr_err( "arm_smcc rate %d %lu\n", index, freq->rate );
>           }

A nicer solution would be to keep imx_ddrc_freq.rate in MT/s as reported 
by firmware and divide by 25000 in imx_ddrc_find_freq instead.

> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -211,7 +211,7 @@
>                           opp-hz = /bits/ 64 <100000000>;
>                   };
>                   opp-166M {
> -                       opp-hz = /bits/ 64 <166750000>;
> +                       opp-hz = /bits/ 64 <166935483>;
>                   };
>                   opp-800M {
>                           opp-hz = /bits/ 64 <800000000>;

Yes, this is the precise clock rate in Hz.

>> * Make the rounding in imx-ddrc more generous.
> 
> Sorry I don't understand what you mean by this

I meant to make imx_ddrc_find_freq find 667 MT/s for an OPP of 166935483:

         /*
          * Firmware reports values in MT/s, so we round-down from Hz
          * Rounding is extra generous to ensure a match.
          */
         rate = DIV_ROUND_CLOSEST(rate, 250000);
         for (i = 0; i < priv->freq_count; ++i) {
                 struct imx_ddrc_freq *freq = &priv->freq_table[i];
                 if (freq->rate == rate ||
                                 freq->rate + 1 == rate ||
                                 freq->rate - 1 == rate)
                         return freq;
         }

But your B0 hack above should also work.

> Adding the other changes the board no longer hangs when trying to change
> frequencies but it also doesn't seem to actually change the frequency.
> 
> [    3.076426] ddrc checking rate 800000000 166935483
> [    3.081290] ddrc checking rate 166935483 166935483
> [    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about to
> change freq 800000000 to 166935483
> [    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed
> freq 800000000 to 166935483
> 
> root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
> 800000000
> root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
> 166935483

The target_freq value is from clk_get_rate(dram_core) but it is 
dram_core's parent which gets updated. It seems that a clk mux ignores 
CLK_GET_RATE_NOCACHE on the parent.

An update can be forced by adding `clk_get_rate(new_dram_core_parent);` 
at the end of imx_ddrc_set_freq.

You should also be able to check by looking at clk_summary or
/sys/kernel/debug/clk/dram_core_clk/clk_rate
/sys/kernel/debug/clk/dram_pll_out/clk_rate

--
Regards,
Leonard

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
  2019-10-16 14:54                 ` Leonard Crestez
@ 2019-10-17 13:25                   ` Angus Ainslie
  -1 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-17 13:25 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Jacky Bai, Mark Rutland, Artur Świgoń,
	Viresh Kumar, Michael Turquette, Alexandre Bailon,
	Saravana Kannan, Krzysztof Kozlowski, Chanwoo Choi, MyungJoo Ham,
	dl-linux-imx, devicetree, linux-pm, Rob Herring,
	linux-arm-kernel, Aisheng Dong, Anson Huang, Stephen Boyd,
	Kyungmin Park, kernel, Fabio Estevam, Shawn Guo, Georgi Djakov,
	Martin Kepplinger, linux-pm-owner

On 2019-10-16 07:54, Leonard Crestez wrote:
> On 16.10.2019 17:09, Angus Ainslie wrote:
>> On 2019-10-15 07:05, Leonard Crestez wrote:
>>> On 10.10.2019 17:43, Angus Ainslie wrote:
>>>> 
>>>> I've integrated your u-boot and ATF on our board and I have a couple
>>>> of questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>>>> 
>>>> It looks like this line limits the training frequencies to 800 MHz 
>>>> and
>>>> 166 MHz
>>> 
>>> Yes! This is due to a hardware errata which was solved in B1: DRAM 
>>> pll
>>> can't be disabled. This means that instead of 25/100/800 freqs are
>>> 166/800, and this requires code changes.
>>> 
>>>> Does 100 MHz and 25 MHz not work on B0 ?
>>> 
>>> No, lower rates require dram clk from a composite slice 
>>> (dram_alt_root)
>>> 
>>>> I added the ddrc_and noc opp as well as the 166MHz opp
>>>> 
>>>> I also added the interconnects ( do we need them on imx8mq ? )
>>> 
>>> The interconnect stuff is not required to switch dram frequency, it's
>>> for device to make minimum bandwidth requests. It an additional 
>>> feature
>>> on top.
>>> 
>>> As a hack I configured FEC to do so but a saner example would be to
>>> request bandwidth based on display resolution and color depth.
>>> 
>>>> I had to add a hack as the PM QoS was limiting the bus speed to 
>>>> 399MHz,
>>>> if you have any ideas why that would be appreciated.
>>> 
>>> You probably need to set ethernet down (which is awkward) or better
>>> just drop the interconnect properties and test using the devfreq 
>>> userspace
>>> governor.
>>> 
>>>> The driver is probing
>>>> 
>>>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>>>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>>>> [   12.147259] bus: 'platform': really_probe: probing driver
>>>> imx-ddrc-devfreq with device 3d400000.dram-controller
>>>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>>>> handle
>>>> [   12.164197] arm_smcc rate 0 800000000
>>>> [   12.167880] arm_smcc rate 1 166750000
>>>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events 
>>>> from
>>>> pmu imx8_ddr0
>>>> [   12.212403] Added freq 0 25000000
>>>> [   12.215742] Added freq 1 100000000
>>>> [   12.219177] Added freq 2 166750000
>>>> [   12.222648] Added freq 3 800000000
>>>> [   12.226105] device: 'devfreq0': device_add
>>>> [   12.230287] PM: Adding info for No Bus:devfreq0
>>>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to
>>>> device
>>>> '3d400000.dram-controller'
>>>> [   12.243699] bus: 'platform': really_probe: bound device
>>>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>>>> 
>>>> Add seems to run correctly until it tries to adjust the clock to
>>>> 166MHz
>>>> 
>>>> [   19.555482] ddrc checking rate 800000000 166750000
>>>> [   19.555489] ddrc checking rate 166750000 166750000
>>>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0
>>>> to
>>>> driver option1
>>>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
>>>> to
>>>> change freq 800000000 to 166750000
>>>> 
>>>> And the board hangs there. Any ideas on how to get past this ?
>>> 
>>> Please try this ATF patch:
>> 
>> Ok applied this to the tree we're using
>> 
>>> I tested switching on imx8mq-evk with B0 SoC but a few additional
>>> changes are required in kernel to support switching between rates 
>>> which
>>> are both backed by PLL:
>>> 
>>> * Mark the PLL CLK_GET_RATE_NOCACHE
>> 
>> Is this what you meant ?
>> 
>> diff --git a/drivers/clk/imx/clk-imx8mq.c 
>> b/drivers/clk/imx/clk-imx8mq.c
>> index 2813884f69c1..e5f50cf8a264 100644
>> --- a/drivers/clk/imx/clk-imx8mq.c
>> +++ b/drivers/clk/imx/clk-imx8mq.c
>> @@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct
>> platform_device *pdev)
>>           clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out",
>> sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 
>> 0x30,
>> CLK_IS_CRITICAL);
>>           clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out",
>> sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 
>> 0x3c,
>> CLK_IS_CRITICAL);
>>           clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out",
>> sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 
>> 0x48,
>> CLK_IS_CRITICAL);
>> -       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
>> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 
>> 0x60,
>> CLK_IS_CRITICAL);
>> +       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
>> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 
>> 0x60,
>> CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);
> 
> Yes.
> 
>>> * Set the rate to 166935483 exactly (to match clk_get_rate)
>> 
>> Okay I hacked that in
>> 
>> diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
>> index 4eed6f50bb8d..a832768a865f 100644
>> --- a/drivers/devfreq/imx-ddrc.c
>> +++ b/drivers/devfreq/imx-ddrc.c
>> @@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device
>> *dev)
>>                           return -ENODEV;
>>                   }
>> 
>> +               /* B0 hack */
>> +               if ( freq->rate == 166750000 )
>> +                       freq->rate = 166935483;
>> +inserting
>>                   pr_err( "arm_smcc rate %d %lu\n", index, freq->rate 
>> );
>>           }
> 
> A nicer solution would be to keep imx_ddrc_freq.rate in MT/s as 
> reported
> by firmware and divide by 25000 in imx_ddrc_find_freq instead.
> 
>> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> @@ -211,7 +211,7 @@
>>                           opp-hz = /bits/ 64 <100000000>;
>>                   };
>>                   opp-166M {
>> -                       opp-hz = /bits/ 64 <166750000>;
>> +                       opp-hz = /bits/ 64 <166935483>;
>>                   };
>>                   opp-800M {
>>                           opp-hz = /bits/ 64 <800000000>;
> 
> Yes, this is the precise clock rate in Hz.
> 
>>> * Make the rounding in imx-ddrc more generous.
>> 
>> Sorry I don't understand what you mean by this
> 
> I meant to make imx_ddrc_find_freq find 667 MT/s for an OPP of 
> 166935483:
> 
>          /*
>           * Firmware reports values in MT/s, so we round-down from Hz
>           * Rounding is extra generous to ensure a match.
>           */
>          rate = DIV_ROUND_CLOSEST(rate, 250000);
>          for (i = 0; i < priv->freq_count; ++i) {
>                  struct imx_ddrc_freq *freq = &priv->freq_table[i];
>                  if (freq->rate == rate ||
>                                  freq->rate + 1 == rate ||
>                                  freq->rate - 1 == rate)
>                          return freq;
>          }
> 
> But your B0 hack above should also work.
> 

Thanks with those additions the driver is working on imx8mq B0.

There is a small adjustment to the code above because you're scaling the 
rate you also need to scale the matching rate.

--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -106,9 +106,17 @@ static struct imx_ddrc_freq 
*imx_ddrc_find_freq(struct imx_ddrc *priv,
  {
         int i;

+       /*
+       * Firmware reports values in MT/s, so we round-down from Hz
+       * Rounding is extra generous to ensure a match.
+       */
+       rate = DIV_ROUND_CLOSEST(rate, 250000);
         for (i = 0; i < priv->freq_count; ++i) {
-               if (priv->freq_table[i].rate == rate)
+               struct imx_ddrc_freq *freq = &priv->freq_table[i];
+               unsigned long match_rate = 
DIV_ROUND_CLOSEST(freq->rate,250000);
+               if (match_rate + 1 >= rate &&
+                   match_rate - 1 <= rate)
                         return &priv->freq_table[i];
         }

Cheers
Angus

>> Adding the other changes the board no longer hangs when trying to 
>> change
>> frequencies but it also doesn't seem to actually change the frequency.
>> 
>> [    3.076426] ddrc checking rate 800000000 166935483
>> [    3.081290] ddrc checking rate 166935483 166935483
>> [    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about 
>> to
>> change freq 800000000 to 166935483
>> [    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed
>> freq 800000000 to 166935483
>> 
>> root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
>> 800000000
>> root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
>> 166935483
> 
> The target_freq value is from clk_get_rate(dram_core) but it is
> dram_core's parent which gets updated. It seems that a clk mux ignores
> CLK_GET_RATE_NOCACHE on the parent.
> 
> An update can be forced by adding `clk_get_rate(new_dram_core_parent);`
> at the end of imx_ddrc_set_freq.
> 
> You should also be able to check by looking at clk_summary or
> /sys/kernel/debug/clk/dram_core_clk/clk_rate
> /sys/kernel/debug/clk/dram_pll_out/clk_rate
> 
> --
> Regards,
> Leonard

^ permalink raw reply	[flat|nested] 40+ messages in thread
* Re: [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm
@ 2019-10-17 13:25                   ` Angus Ainslie
  0 siblings, 0 replies; 40+ messages in thread
From: Angus Ainslie @ 2019-10-17 13:25 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Mark Rutland, Artur Świgoń,
	Jacky Bai, Viresh Kumar, Michael Turquette, linux-pm-owner,
	Alexandre Bailon, Anson Huang, Krzysztof Kozlowski, Chanwoo Choi,
	MyungJoo Ham, dl-linux-imx, devicetree, linux-pm, Rob Herring,
	Martin Kepplinger, linux-arm-kernel, Aisheng Dong,
	Saravana Kannan, Stephen Boyd, Kyungmin Park, kernel,
	Fabio Estevam, Shawn Guo, Georgi Djakov

On 2019-10-16 07:54, Leonard Crestez wrote:
> On 16.10.2019 17:09, Angus Ainslie wrote:
>> On 2019-10-15 07:05, Leonard Crestez wrote:
>>> On 10.10.2019 17:43, Angus Ainslie wrote:
>>>> 
>>>> I've integrated your u-boot and ATF on our board and I have a couple
>>>> of questions. Our board is running imx8mq B0 (Rev 2.0) silicon.
>>>> 
>>>> It looks like this line limits the training frequencies to 800 MHz 
>>>> and
>>>> 166 MHz
>>> 
>>> Yes! This is due to a hardware errata which was solved in B1: DRAM 
>>> pll
>>> can't be disabled. This means that instead of 25/100/800 freqs are
>>> 166/800, and this requires code changes.
>>> 
>>>> Does 100 MHz and 25 MHz not work on B0 ?
>>> 
>>> No, lower rates require dram clk from a composite slice 
>>> (dram_alt_root)
>>> 
>>>> I added the ddrc_and noc opp as well as the 166MHz opp
>>>> 
>>>> I also added the interconnects ( do we need them on imx8mq ? )
>>> 
>>> The interconnect stuff is not required to switch dram frequency, it's
>>> for device to make minimum bandwidth requests. It an additional 
>>> feature
>>> on top.
>>> 
>>> As a hack I configured FEC to do so but a saner example would be to
>>> request bandwidth based on display resolution and color depth.
>>> 
>>>> I had to add a hack as the PM QoS was limiting the bus speed to 
>>>> 399MHz,
>>>> if you have any ideas why that would be appreciated.
>>> 
>>> You probably need to set ethernet down (which is awkward) or better
>>> just drop the interconnect properties and test using the devfreq 
>>> userspace
>>> governor.
>>> 
>>>> The driver is probing
>>>> 
>>>> [   12.136537] bus: 'platform': driver_probe_device: matched device
>>>> 3d400000.dram-controller with driver imx-ddrc-devfrq
>>>> [   12.147259] bus: 'platform': really_probe: probing driver
>>>> imx-ddrc-devfreq with device 3d400000.dram-controller
>>>> [   12.157382] imx-ddrc-devfreq 3d400000.dram-controller: no pinctrl
>>>> handle
>>>> [   12.164197] arm_smcc rate 0 800000000
>>>> [   12.167880] arm_smcc rate 1 166750000
>>>> [   12.171778] of: _opp_add_static_v2: turbo:0 rate:25000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.179994] of: _opp_add_static_v2: turbo:0 rate:100000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.188311] of: _opp_add_static_v2: turbo:0 rate:166750000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.196606] of: _opp_add_static_v2: turbo:0 rate:800000000 uv:0
>>>> uvmin:0 uvmax:0 latency:0
>>>> [   12.204930] imx-ddrc-devfreq 3d400000.dram-controller: events 
>>>> from
>>>> pmu imx8_ddr0
>>>> [   12.212403] Added freq 0 25000000
>>>> [   12.215742] Added freq 1 100000000
>>>> [   12.219177] Added freq 2 166750000
>>>> [   12.222648] Added freq 3 800000000
>>>> [   12.226105] device: 'devfreq0': device_add
>>>> [   12.230287] PM: Adding info for No Bus:devfreq0
>>>> [   12.234864] driver: 'imx-ddrc-devfreq': driver_bound: bound to
>>>> device
>>>> '3d400000.dram-controller'
>>>> [   12.243699] bus: 'platform': really_probe: bound device
>>>> 3d400000.dram-controller to driver imx-ddrc-devfreq
>>>> 
>>>> Add seems to run correctly until it tries to adjust the clock to
>>>> 166MHz
>>>> 
>>>> [   19.555482] ddrc checking rate 800000000 166750000
>>>> [   19.555489] ddrc checking rate 166750000 166750000
>>>> [   19.560442] bus: 'usb-serial': really_probe: bound device ttyUSB0
>>>> to
>>>> driver option1
>>>> [   19.568751] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about
>>>> to
>>>> change freq 800000000 to 166750000
>>>> 
>>>> And the board hangs there. Any ideas on how to get past this ?
>>> 
>>> Please try this ATF patch:
>> 
>> Ok applied this to the tree we're using
>> 
>>> I tested switching on imx8mq-evk with B0 SoC but a few additional
>>> changes are required in kernel to support switching between rates 
>>> which
>>> are both backed by PLL:
>>> 
>>> * Mark the PLL CLK_GET_RATE_NOCACHE
>> 
>> Is this what you meant ?
>> 
>> diff --git a/drivers/clk/imx/clk-imx8mq.c 
>> b/drivers/clk/imx/clk-imx8mq.c
>> index 2813884f69c1..e5f50cf8a264 100644
>> --- a/drivers/clk/imx/clk-imx8mq.c
>> +++ b/drivers/clk/imx/clk-imx8mq.c
>> @@ -345,7 +345,7 @@ static int imx8mq_clocks_probe(struct
>> platform_device *pdev)
>>           clks[IMX8MQ_SYS1_PLL_OUT] = imx_clk_sccg_pll("sys1_pll_out",
>> sys1_pll_out_sels, ARRAY_SIZE(sys1_pll_out_sels), 0, 0, 0, base + 
>> 0x30,
>> CLK_IS_CRITICAL);
>>           clks[IMX8MQ_SYS2_PLL_OUT] = imx_clk_sccg_pll("sys2_pll_out",
>> sys2_pll_out_sels, ARRAY_SIZE(sys2_pll_out_sels), 0, 0, 1, base + 
>> 0x3c,
>> CLK_IS_CRITICAL);
>>           clks[IMX8MQ_SYS3_PLL_OUT] = imx_clk_sccg_pll("sys3_pll_out",
>> sys3_pll_out_sels, ARRAY_SIZE(sys3_pll_out_sels), 0, 0, 1, base + 
>> 0x48,
>> CLK_IS_CRITICAL);
>> -       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
>> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 
>> 0x60,
>> CLK_IS_CRITICAL);
>> +       clks[IMX8MQ_DRAM_PLL_OUT] = imx_clk_sccg_pll("dram_pll_out",
>> dram_pll_out_sels, ARRAY_SIZE(dram_pll_out_sels), 0, 0, 0, base + 
>> 0x60,
>> CLK_IS_CRITICAL|CLK_GET_RATE_NOCACHE);
> 
> Yes.
> 
>>> * Set the rate to 166935483 exactly (to match clk_get_rate)
>> 
>> Okay I hacked that in
>> 
>> diff --git a/drivers/devfreq/imx-ddrc.c b/drivers/devfreq/imx-ddrc.c
>> index 4eed6f50bb8d..a832768a865f 100644
>> --- a/drivers/devfreq/imx-ddrc.c
>> +++ b/drivers/devfreq/imx-ddrc.c
>> @@ -436,6 +436,10 @@ static int imx_ddrc_init_freq_info(struct device
>> *dev)
>>                           return -ENODEV;
>>                   }
>> 
>> +               /* B0 hack */
>> +               if ( freq->rate == 166750000 )
>> +                       freq->rate = 166935483;
>> +inserting
>>                   pr_err( "arm_smcc rate %d %lu\n", index, freq->rate 
>> );
>>           }
> 
> A nicer solution would be to keep imx_ddrc_freq.rate in MT/s as 
> reported
> by firmware and divide by 25000 in imx_ddrc_find_freq instead.
> 
>> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> @@ -211,7 +211,7 @@
>>                           opp-hz = /bits/ 64 <100000000>;
>>                   };
>>                   opp-166M {
>> -                       opp-hz = /bits/ 64 <166750000>;
>> +                       opp-hz = /bits/ 64 <166935483>;
>>                   };
>>                   opp-800M {
>>                           opp-hz = /bits/ 64 <800000000>;
> 
> Yes, this is the precise clock rate in Hz.
> 
>>> * Make the rounding in imx-ddrc more generous.
>> 
>> Sorry I don't understand what you mean by this
> 
> I meant to make imx_ddrc_find_freq find 667 MT/s for an OPP of 
> 166935483:
> 
>          /*
>           * Firmware reports values in MT/s, so we round-down from Hz
>           * Rounding is extra generous to ensure a match.
>           */
>          rate = DIV_ROUND_CLOSEST(rate, 250000);
>          for (i = 0; i < priv->freq_count; ++i) {
>                  struct imx_ddrc_freq *freq = &priv->freq_table[i];
>                  if (freq->rate == rate ||
>                                  freq->rate + 1 == rate ||
>                                  freq->rate - 1 == rate)
>                          return freq;
>          }
> 
> But your B0 hack above should also work.
> 

Thanks with those additions the driver is working on imx8mq B0.

There is a small adjustment to the code above because you're scaling the 
rate you also need to scale the matching rate.

--- a/drivers/devfreq/imx-ddrc.c
+++ b/drivers/devfreq/imx-ddrc.c
@@ -106,9 +106,17 @@ static struct imx_ddrc_freq 
*imx_ddrc_find_freq(struct imx_ddrc *priv,
  {
         int i;

+       /*
+       * Firmware reports values in MT/s, so we round-down from Hz
+       * Rounding is extra generous to ensure a match.
+       */
+       rate = DIV_ROUND_CLOSEST(rate, 250000);
         for (i = 0; i < priv->freq_count; ++i) {
-               if (priv->freq_table[i].rate == rate)
+               struct imx_ddrc_freq *freq = &priv->freq_table[i];
+               unsigned long match_rate = 
DIV_ROUND_CLOSEST(freq->rate,250000);
+               if (match_rate + 1 >= rate &&
+                   match_rate - 1 <= rate)
                         return &priv->freq_table[i];
         }

Cheers
Angus

>> Adding the other changes the board no longer hangs when trying to 
>> change
>> frequencies but it also doesn't seem to actually change the frequency.
>> 
>> [    3.076426] ddrc checking rate 800000000 166935483
>> [    3.081290] ddrc checking rate 166935483 166935483
>> [    3.086225] imx-ddrc-devfreq 3d400000.dram-controller: ddrc about 
>> to
>> change freq 800000000 to 166935483
>> [    3.086891] imx-ddrc-devfreq 3d400000.dram-controller: ddrc changed
>> freq 800000000 to 166935483
>> 
>> root@pureos:~# cat /sys/class/devfreq/devfreq0/cur_freq
>> 800000000
>> root@pureos:~# cat /sys/class/devfreq/devfreq0/target_freq
>> 166935483
> 
> The target_freq value is from clk_get_rate(dram_core) but it is
> dram_core's parent which gets updated. It seems that a clk mux ignores
> CLK_GET_RATE_NOCACHE on the parent.
> 
> An update can be forced by adding `clk_get_rate(new_dram_core_parent);`
> at the end of imx_ddrc_set_freq.
> 
> You should also be able to check by looking at clk_summary or
> /sys/kernel/debug/clk/dram_core_clk/clk_rate
> /sys/kernel/debug/clk/dram_pll_out/clk_rate
> 
> --
> Regards,
> Leonard

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

^ permalink raw reply	[flat|nested] 40+ messages in thread
end of thread, other threads:[~2019-10-17 13:25 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 10:55 [RFCv3 0/3] interconnect: Add imx support via devfreq Leonard Crestez
2019-08-06 10:55 ` Leonard Crestez
2019-08-06 10:55 ` Leonard Crestez
2019-08-06 10:55 ` [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-07 15:16   ` Georgi Djakov
2019-08-07 15:16     ` Georgi Djakov
2019-08-07 15:16     ` Georgi Djakov
2019-08-07 22:35     ` Leonard Crestez
2019-08-07 22:35       ` Leonard Crestez
2019-08-07 22:35       ` Leonard Crestez
2019-08-06 10:55 ` [RFCv3 2/3] interconnect: Add imx core driver Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-07 15:16   ` Georgi Djakov
2019-08-07 15:16     ` Georgi Djakov
2019-08-07 15:16     ` Georgi Djakov
2019-08-06 10:55 ` [RFCv3 3/3] interconnect: imx: Add platform driver for imx8mm Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-06 10:55   ` Leonard Crestez
2019-08-21 14:02   ` Martin Kepplinger
2019-08-21 14:02     ` Martin Kepplinger
2019-08-21 14:21     ` Leonard Crestez
2019-08-21 14:21       ` Leonard Crestez
2019-08-21 14:21       ` Leonard Crestez
2019-08-21 14:26       ` Martin Kepplinger
2019-08-21 14:26         ` Martin Kepplinger
2019-08-21 14:26         ` Martin Kepplinger
2019-10-10 14:43         ` Angus Ainslie
2019-10-10 14:43           ` Angus Ainslie
2019-10-10 14:43           ` Angus Ainslie
2019-10-15 14:05           ` Leonard Crestez
2019-10-15 14:05             ` Leonard Crestez
2019-10-16 14:09             ` Angus Ainslie
2019-10-16 14:09               ` Angus Ainslie
2019-10-16 14:54               ` Leonard Crestez
2019-10-16 14:54                 ` Leonard Crestez
2019-10-17 13:25                 ` Angus Ainslie
2019-10-17 13:25                   ` Angus Ainslie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.