[SRU][Bionic][PATCH v2 0/5] Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 0/5] Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board

Shrirang Bagul
BugLink: https://bugs.launchpad.net/bugs/1840437

[Impact]
We ship the armhf flavour of the Bionic kernel which supports the iMX6Q
based Nitrogen6x board. The cpufreq, thermal sensors and cooling cells
should be enabled for this board.

[Fix]
Backport upstream device tree patches to define the device nodes properly.

[Test]
Tested and verified on the Nitrogen6x board.

[Regression Potential]
Low. These patches update DT nodes with required properties for the
imx6q_cpufreq and imx_thermal drivers function properly on the platform.

Anson Huang (2):
  ARM: dts: imx7d: use operating-points-v2 for cpu
  ARM: dts: imx: add cooling-cells for cpufreq cooling device

Lucas Stach (1):
  ARM: dts: imx6: add thermal sensor and cooling cells

Nicolas Chauvet (1):
  arm: imx: Add MODULE_ALIAS for cpufreq

Viresh Kumar (1):
  ARM: dts: imx: Add missing OPP properties for CPUs

 arch/arm/boot/dts/imx6dl.dtsi      | 24 ++++++++
 arch/arm/boot/dts/imx6q-cm-fx6.dts | 66 ++++++++++++++++++++++
 arch/arm/boot/dts/imx6q.dtsi       | 89 +++++++++++++++++++++++++++++-
 arch/arm/boot/dts/imx6qdl.dtsi     |  3 +
 arch/arm/boot/dts/imx6sl.dtsi      |  1 +
 arch/arm/boot/dts/imx6sx.dtsi      |  1 +
 arch/arm/boot/dts/imx6ul.dtsi      |  1 +
 arch/arm/boot/dts/imx7d.dtsi       | 31 +++++++++--
 drivers/cpufreq/imx6q-cpufreq.c    |  1 +
 9 files changed, 209 insertions(+), 8 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 1/5] arm: imx: Add MODULE_ALIAS for cpufreq

Shrirang Bagul
From: Nicolas Chauvet <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840437

Without this, the imx6q-cpufreq driver isn't loaded
automatically when built as a module

Tested on wandboard quad with a fedora 27 kernel rpm

Signed-off-by: Nicolas Chauvet <[hidden email]>
Acked-by: Viresh Kumar <[hidden email]>
Signed-off-by: Rafael J. Wysocki <[hidden email]>
(cherry picked from commit d0404738c687c0ecaa7d6b7c5c39e4c0dac791e6)
Signed-off-by: Shrirang Bagul <[hidden email]>
---
 drivers/cpufreq/imx6q-cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index a51eda85ad35..52d63f88dae6 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -478,6 +478,7 @@ static struct platform_driver imx6q_cpufreq_platdrv = {
 };
 module_platform_driver(imx6q_cpufreq_platdrv);
 
+MODULE_ALIAS("platform:imx6q-cpufreq");
 MODULE_AUTHOR("Shawn Guo <[hidden email]>");
 MODULE_DESCRIPTION("Freescale i.MX6Q cpufreq driver");
 MODULE_LICENSE("GPL");
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 2/5] ARM: dts: imx: Add missing OPP properties for CPUs

Shrirang Bagul
In reply to this post by Shrirang Bagul
From: Viresh Kumar <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840437

The OPP properties, like "operating-points", should either be present
for all the CPUs of a cluster or none. If these are present only for a
subset of CPUs of a cluster then things will start falling apart as soon
as the CPUs are brought online in a different order. For example, this
will happen because the operating system looks for such properties in
the CPU node it is trying to bring up, so that it can create an OPP
table.

Add such missing properties.

Fix other missing properties (like clocks, supply, clock latency) as
well to make it all work.

Signed-off-by: Viresh Kumar <[hidden email]>
Signed-off-by: Shawn Guo <[hidden email]>
(cherry picked from commit b97872d4eb226fa853d0f69c72b24a711e388757)
Signed-off-by: Shrirang Bagul <[hidden email]>
---
 arch/arm/boot/dts/imx6dl.dtsi      | 23 ++++++++
 arch/arm/boot/dts/imx6q-cm-fx6.dts | 66 +++++++++++++++++++++++
 arch/arm/boot/dts/imx6q.dtsi       | 87 ++++++++++++++++++++++++++++--
 arch/arm/boot/dts/imx7d.dtsi       |  5 ++
 4 files changed, 178 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 4d693a75ce98..7b6886b64a1b 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -56,6 +56,29 @@
  device_type = "cpu";
  reg = <1>;
  next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz    uV */
+ 996000  1250000
+ 792000  1175000
+ 396000  1150000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz  SOC-PU uV */
+ 996000 1175000
+ 792000 1175000
+ 396000 1175000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ clocks = <&clks IMX6QDL_CLK_ARM>,
+ <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
+ <&clks IMX6QDL_CLK_STEP>,
+ <&clks IMX6QDL_CLK_PLL1_SW>,
+ <&clks IMX6QDL_CLK_PLL1_SYS>;
+ clock-names = "arm", "pll2_pfd2_396m", "step",
+      "pll1_sw", "pll1_sys";
+ arm-supply = <&reg_arm>;
+ pu-supply = <&reg_pu>;
+ soc-supply = <&reg_soc>;
  };
  };
 
diff --git a/arch/arm/boot/dts/imx6q-cm-fx6.dts b/arch/arm/boot/dts/imx6q-cm-fx6.dts
index bc7587c383f6..ef1350608bc3 100644
--- a/arch/arm/boot/dts/imx6q-cm-fx6.dts
+++ b/arch/arm/boot/dts/imx6q-cm-fx6.dts
@@ -187,6 +187,72 @@
  >;
 };
 
+&cpu1 {
+ /*
+ * Although the imx6q fuse indicates that 1.2GHz operation is possible,
+ * the module behaves unstable at this frequency. Hence, remove the
+ * 1.2GHz operation point here.
+ */
+ operating-points = <
+ /* kHz uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz SOC-PU uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+};
+
+&cpu2 {
+ /*
+ * Although the imx6q fuse indicates that 1.2GHz operation is possible,
+ * the module behaves unstable at this frequency. Hence, remove the
+ * 1.2GHz operation point here.
+ */
+ operating-points = <
+ /* kHz uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz SOC-PU uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+};
+
+&cpu3 {
+ /*
+ * Although the imx6q fuse indicates that 1.2GHz operation is possible,
+ * the module behaves unstable at this frequency. Hence, remove the
+ * 1.2GHz operation point here.
+ */
+ operating-points = <
+ /* kHz uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz SOC-PU uV */
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+};
+
 &ecspi1 {
  cs-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>, <&gpio3 19 GPIO_ACTIVE_HIGH>;
  pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 0d84d5131ef6..f1506c4b17da 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -56,25 +56,106 @@
  soc-supply = <&reg_soc>;
  };
 
- cpu@1 {
+ cpu1: cpu@1 {
  compatible = "arm,cortex-a9";
  device_type = "cpu";
  reg = <1>;
  next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz    uV */
+ 1200000 1275000
+ 996000  1250000
+ 852000  1250000
+ 792000  1175000
+ 396000  975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz  SOC-PU uV */
+ 1200000 1275000
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ clocks = <&clks IMX6QDL_CLK_ARM>,
+ <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
+ <&clks IMX6QDL_CLK_STEP>,
+ <&clks IMX6QDL_CLK_PLL1_SW>,
+ <&clks IMX6QDL_CLK_PLL1_SYS>;
+ clock-names = "arm", "pll2_pfd2_396m", "step",
+      "pll1_sw", "pll1_sys";
+ arm-supply = <&reg_arm>;
+ pu-supply = <&reg_pu>;
+ soc-supply = <&reg_soc>;
  };
 
- cpu@2 {
+ cpu2: cpu@2 {
  compatible = "arm,cortex-a9";
  device_type = "cpu";
  reg = <2>;
  next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz    uV */
+ 1200000 1275000
+ 996000  1250000
+ 852000  1250000
+ 792000  1175000
+ 396000  975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz  SOC-PU uV */
+ 1200000 1275000
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ clocks = <&clks IMX6QDL_CLK_ARM>,
+ <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
+ <&clks IMX6QDL_CLK_STEP>,
+ <&clks IMX6QDL_CLK_PLL1_SW>,
+ <&clks IMX6QDL_CLK_PLL1_SYS>;
+ clock-names = "arm", "pll2_pfd2_396m", "step",
+      "pll1_sw", "pll1_sys";
+ arm-supply = <&reg_arm>;
+ pu-supply = <&reg_pu>;
+ soc-supply = <&reg_soc>;
  };
 
- cpu@3 {
+ cpu3: cpu@3 {
  compatible = "arm,cortex-a9";
  device_type = "cpu";
  reg = <3>;
  next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz    uV */
+ 1200000 1275000
+ 996000  1250000
+ 852000  1250000
+ 792000  1175000
+ 396000  975000
+ >;
+ fsl,soc-operating-points = <
+ /* ARM kHz  SOC-PU uV */
+ 1200000 1275000
+ 996000 1250000
+ 852000 1250000
+ 792000 1175000
+ 396000 1175000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ clocks = <&clks IMX6QDL_CLK_ARM>,
+ <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
+ <&clks IMX6QDL_CLK_STEP>,
+ <&clks IMX6QDL_CLK_PLL1_SW>,
+ <&clks IMX6QDL_CLK_PLL1_SYS>;
+ clock-names = "arm", "pll2_pfd2_396m", "step",
+      "pll1_sw", "pll1_sys";
+ arm-supply = <&reg_arm>;
+ pu-supply = <&reg_pu>;
+ soc-supply = <&reg_soc>;
  };
  };
 
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 119b63ffb0fe..fdf5076a60d4 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -59,6 +59,11 @@
  compatible = "arm,cortex-a7";
  device_type = "cpu";
  reg = <1>;
+ operating-points = <
+ /* KHz uV */
+ 996000 1075000
+ 792000 975000
+ >;
  clock-frequency = <996000000>;
  };
  };
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 3/5] ARM: dts: imx7d: use operating-points-v2 for cpu

Shrirang Bagul
In reply to this post by Shrirang Bagul
From: Anson Huang <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840437

This patch uses "operating-points-v2" instead of
"operating-points" to be more fit with cpufreq-dt
driver.

Signed-off-by: Anson Huang <[hidden email]>
Signed-off-by: Shawn Guo <[hidden email]>
(cherry picked from commit bce48c92a641c1f1a4795a37d57168fd08fecc3b)
Signed-off-by: Shrirang Bagul <[hidden email]>
---
 arch/arm/boot/dts/imx7d.dtsi | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index fdf5076a60d4..5534f1b880c1 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -47,12 +47,8 @@
 / {
  cpus {
  cpu0: cpu@0 {
- operating-points = <
- /* KHz uV */
- 996000 1075000
- 792000 975000
- >;
  clock-frequency = <996000000>;
+ operating-points-v2 = <&cpu0_opp_table>;
  };
 
  cpu1: cpu@1 {
@@ -65,6 +61,25 @@
  792000 975000
  >;
  clock-frequency = <996000000>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ };
+ };
+
+ cpu0_opp_table: opp-table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-792000000 {
+ opp-hz = /bits/ 64 <792000000>;
+ opp-microvolt = <975000>;
+ clock-latency-ns = <150000>;
+ };
+
+ opp-996000000 {
+ opp-hz = /bits/ 64 <996000000>;
+ opp-microvolt = <1075000>;
+ clock-latency-ns = <150000>;
+ opp-suspend;
  };
  };
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 4/5] ARM: dts: imx: add cooling-cells for cpufreq cooling device

Shrirang Bagul
In reply to this post by Shrirang Bagul
From: Anson Huang <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840437

Add #cooling-cells for i.MX6/7 SoCs for cpufreq cooling device usage.

Signed-off-by: Anson Huang <[hidden email]>
Signed-off-by: Bastian Stender <[hidden email]>
Signed-off-by: Shawn Guo <[hidden email]>
(cherry picked from commit f3d80deb080f422ff1df4a715444f156bb51adc1)
Signed-off-by: Shrirang Bagul <[hidden email]>
---
 arch/arm/boot/dts/imx6dl.dtsi | 1 +
 arch/arm/boot/dts/imx6q.dtsi  | 1 +
 arch/arm/boot/dts/imx6sl.dtsi | 1 +
 arch/arm/boot/dts/imx6sx.dtsi | 1 +
 arch/arm/boot/dts/imx6ul.dtsi | 1 +
 arch/arm/boot/dts/imx7d.dtsi  | 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 7b6886b64a1b..9eb21c3b9c3c 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -39,6 +39,7 @@
  396000 1175000
  >;
  clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
  clocks = <&clks IMX6QDL_CLK_ARM>,
  <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
  <&clks IMX6QDL_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f1506c4b17da..b049794467f8 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -44,6 +44,7 @@
  396000 1175000
  >;
  clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
  clocks = <&clks IMX6QDL_CLK_ARM>,
  <&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
  <&clks IMX6QDL_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 7eaac40b8664..d42c55f3f8a5 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -65,6 +65,7 @@
  396000          1175000
  >;
  clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
  clocks = <&clks IMX6SL_CLK_ARM>, <&clks IMX6SL_CLK_PLL2_PFD2>,
  <&clks IMX6SL_CLK_STEP>, <&clks IMX6SL_CLK_PLL1_SW>,
  <&clks IMX6SL_CLK_PLL1_SYS>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index ca4a7cd87db5..c9fead0ab233 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -83,6 +83,7 @@
  198000    1175000
  >;
  clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
  clocks = <&clks IMX6SX_CLK_ARM>,
  <&clks IMX6SX_CLK_PLL2_PFD2>,
  <&clks IMX6SX_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2f9585e25510..703d382c003e 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -66,6 +66,7 @@
  device_type = "cpu";
  reg = <0>;
  clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
  operating-points = <
  /* kHz uV */
  528000 1175000
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 5534f1b880c1..84e22e457c1d 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -49,6 +49,7 @@
  cpu0: cpu@0 {
  clock-frequency = <996000000>;
  operating-points-v2 = <&cpu0_opp_table>;
+ #cooling-cells = <2>;
  };
 
  cpu1: cpu@1 {
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH v2 5/5] ARM: dts: imx6: add thermal sensor and cooling cells

Shrirang Bagul
In reply to this post by Shrirang Bagul
From: Lucas Stach <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840437

This allows a board to specify a custom thermal zone configuration
involving the SoC internal sensor, CPU and GPU nodes without having
to change those nodes.

Signed-off-by: Lucas Stach <[hidden email]>
Signed-off-by: Shawn Guo <[hidden email]>
(backported from commit 4951c2da1a3a8b56d4ef0659d80938942307a8a3)
Signed-off-by: Shrirang Bagul <[hidden email]>
---
 arch/arm/boot/dts/imx6q.dtsi   | 1 +
 arch/arm/boot/dts/imx6qdl.dtsi | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index b049794467f8..6073d2039894 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -208,6 +208,7 @@
  <&clks IMX6QDL_CLK_GPU2D_CORE>;
  clock-names = "bus", "core";
  power-domains = <&pd_pu>;
+ #cooling-cells = <2>;
  };
 
  ipu2: ipu@2800000 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 3372308974bf..f85d7c2d87a6 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -157,6 +157,7 @@
  <&clks IMX6QDL_CLK_GPU3D_SHADER>;
  clock-names = "bus", "core", "shader";
  power-domains = <&pd_pu>;
+ #cooling-cells = <2>;
  };
 
  gpu_2d: gpu@134000 {
@@ -167,6 +168,7 @@
  <&clks IMX6QDL_CLK_GPU2D_CORE>;
  clock-names = "bus", "core";
  power-domains = <&pd_pu>;
+ #cooling-cells = <2>;
  };
 
  timer@a00600 {
@@ -735,6 +737,7 @@
  fsl,tempmon = <&anatop>;
  fsl,tempmon-data = <&ocotp>;
  clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
+ #thermal-sensor-cells = <0>;
  };
 
  usbphy1: usbphy@20c9000 {
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Bionic][PATCH v2 0/5] Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board

Paolo Pisati-5
In reply to this post by Shrirang Bagul
On Mon, Aug 19, 2019 at 5:43 AM Shrirang Bagul
<[hidden email]> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/1840437

There's something weird between patch 2 and 3.

Patch 2:
...
The OPP properties, like "operating-points", should either be present
for all the CPUs of a cluster or none. If these are present only for a
subset of CPUs of a cluster then things will start falling apart as soon
as the CPUs are brought online in a different order.
...

and adds such property to cpu1:

--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -59,6 +59,11 @@
                        compatible = "arm,cortex-a7";
                        device_type = "cpu";
                        reg = <1>;
+                       operating-points = <
+                               /* KHz  uV */
+                               996000  1075000
+                               792000  975000
+                       >;
...

afterward, in patch 3, "operating points" is replaced with
"operating-points-v2", but that is only done for cpu0:

--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -47,12 +47,8 @@
 / {
        cpus {
                cpu0: cpu@0 {
-                       operating-points = <
-                               /* KHz  uV */
-                               996000  1075000
-                               792000  975000
-                       >;
                        clock-frequency = <996000000>;
+                       operating-points-v2 = <&cpu0_opp_table>;
                };

                cpu1: cpu@1 {
@@ -65,6 +61,25 @@
                                792000  975000
                        >;
                        clock-frequency = <996000000>;
+                       operating-points-v2 = <&cpu0_opp_table>;
...

and this is the final result:

...
        cpus {
                cpu0: cpu@0 {
                        clock-frequency = <996000000>;
                        operating-points-v2 = <&cpu0_opp_table>;
                        #cooling-cells = <2>;
                };

                cpu1: cpu@1 {
                        compatible = "arm,cortex-a7";
                        device_type = "cpu";
                        reg = <1>;
                        operating-points = <
                                /* KHz  uV */
                                996000  1075000
                                792000  975000
                        >;
                        clock-frequency = <996000000>;
                        operating-points-v2 = <&cpu0_opp_table>;
                };
        };
...

Could you check that?

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Bionic][PATCH v2 0/5] Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board

Shrirang Bagul
On Mon, 2019-08-19 at 10:59 +0200, Paolo Pisati wrote:

> On Mon, Aug 19, 2019 at 5:43 AM Shrirang Bagul
> <[hidden email]> wrote:
> >
> > BugLink: https://bugs.launchpad.net/bugs/1840437
>
> There's something weird between patch 2 and 3.
>
> Patch 2:
> ...
> The OPP properties, like "operating-points", should either be present
> for all the CPUs of a cluster or none. If these are present only for a
> subset of CPUs of a cluster then things will start falling apart as soon
> as the CPUs are brought online in a different order.
> ...
>
> and adds such property to cpu1:
>
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -59,6 +59,11 @@
>                         compatible = "arm,cortex-a7";
>                         device_type = "cpu";
>                         reg = <1>;
> +                       operating-points = <
> +                               /* KHz  uV */
> +                               996000  1075000
> +                               792000  975000
> +                       >;
> ...
>
> afterward, in patch 3, "operating points" is replaced with
> "operating-points-v2", but that is only done for cpu0:
>
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -47,12 +47,8 @@
>  / {
>         cpus {
>                 cpu0: cpu@0 {
> -                       operating-points = <
> -                               /* KHz  uV */
> -                               996000  1075000
> -                               792000  975000
> -                       >;
>                         clock-frequency = <996000000>;
> +                       operating-points-v2 = <&cpu0_opp_table>;
>                 };
>
>                 cpu1: cpu@1 {
> @@ -65,6 +61,25 @@
>                                 792000  975000
>                         >;
>                         clock-frequency = <996000000>;
> +                       operating-points-v2 = <&cpu0_opp_table>;
> ...
>
> and this is the final result:
>
> ...
>         cpus {
>                 cpu0: cpu@0 {
>                         clock-frequency = <996000000>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                         #cooling-cells = <2>;
>                 };
>
>                 cpu1: cpu@1 {
>                         compatible = "arm,cortex-a7";
>                         device_type = "cpu";
>                         reg = <1>;
>                         operating-points = <
>                                 /* KHz  uV */
>                                 996000  1075000
>                                 792000  975000
>                         >;
>                         clock-frequency = <996000000>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                 };
>         };
> ...
>
> Could you check that?
Right, requires another upstream patch to fix this anomaly.

commit 33a8d5a595dd0f9b7f801c1cddb26dc05bc33a73
Author: Anson Huang <[hidden email]>
Date:   Thu Jul 19 16:24:19 2018 +0800

    ARM: dts: imx7d: remove "operating-points" property for cpu1
   
    Commit b97872d4eb22 ("ARM: dts: imx: Add missing OPP properties for CPUs")
    added "operating-points" property for all CPUs, but i.MX7D already has
    "operating-points-v2" property on both CPUs, so no need to add
    "operating-points" property again, this patch removes it.
   
    Fixes: b97872d4eb22 ("ARM: dts: imx: Add missing OPP properties for CPUs")
    Signed-off-by: Anson Huang <[hidden email]>
    Signed-off-by: Shawn Guo <[hidden email]>


Thank you for the review.
NAK'ing this series. Will resend with necessary updates.



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

NAK: [SRU][Bionic][PATCH v2 0/5] Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board

Shrirang Bagul
In reply to this post by Shrirang Bagul
Superseded by v3, with updates to imx7d cpufreq properties.
 
On Mon, 2019-08-19 at 11:42 +0800, Shrirang Bagul wrote:

> BugLink: https://bugs.launchpad.net/bugs/1840437
>
> [Impact]
> We ship the armhf flavour of the Bionic kernel which supports the iMX6Q
> based Nitrogen6x board. The cpufreq, thermal sensors and cooling cells
> should be enabled for this board.
>
> [Fix]
> Backport upstream device tree patches to define the device nodes properly.
>
> [Test]
> Tested and verified on the Nitrogen6x board.
>
> [Regression Potential]
> Low. These patches update DT nodes with required properties for the
> imx6q_cpufreq and imx_thermal drivers function properly on the platform.
>
> Anson Huang (2):
>   ARM: dts: imx7d: use operating-points-v2 for cpu
>   ARM: dts: imx: add cooling-cells for cpufreq cooling device
>
> Lucas Stach (1):
>   ARM: dts: imx6: add thermal sensor and cooling cells
>
> Nicolas Chauvet (1):
>   arm: imx: Add MODULE_ALIAS for cpufreq
>
> Viresh Kumar (1):
>   ARM: dts: imx: Add missing OPP properties for CPUs
>
>  arch/arm/boot/dts/imx6dl.dtsi      | 24 ++++++++
>  arch/arm/boot/dts/imx6q-cm-fx6.dts | 66 ++++++++++++++++++++++
>  arch/arm/boot/dts/imx6q.dtsi       | 89 +++++++++++++++++++++++++++++-
>  arch/arm/boot/dts/imx6qdl.dtsi     |  3 +
>  arch/arm/boot/dts/imx6sl.dtsi      |  1 +
>  arch/arm/boot/dts/imx6sx.dtsi      |  1 +
>  arch/arm/boot/dts/imx6ul.dtsi      |  1 +
>  arch/arm/boot/dts/imx7d.dtsi       | 31 +++++++++--
>  drivers/cpufreq/imx6q-cpufreq.c    |  1 +
>  9 files changed, 209 insertions(+), 8 deletions(-)
>
> --
> 2.17.1
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment