devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode
@ 2024-12-19  7:27 Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties Ahmad Fatoum
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

Unlike the i.MX8MM and i.MX8MN SoCs added earlier, the imx8mp.dtsi
configures some clocks at frequencies that are only validated for
overdrive mode, i.e., when VDD_SOC is 950 mV.

For the Skov i.MX8MP board, we want to run the SoC at the lower voltage of
850 mV though to reduce heat generation and power usage. For this to work,
clock rates need to adhere to the limits of the nominal drive mode.

This is done by this series: A new imx8mp-nominal.dtsi reconfigures
the imx8mp.dtsi clock tree to be compatible with nominal mode, an adaptation
to the Linux clock driver makes it sanity check the actual clock rates against
the SoC operating mode's constraints and finally the Skov DT makes use
of it.

Actual configuration of the VDD_SOC rail continues to happen prior to Linux
as well as PLL configuration that needs to happen earlier than the kernel
running. See the corresponding barebox patch series[1] for details.
Note that the barebox series didn't yet include VDD_SOC reconfiguration
to 850mV, that would follow once the kernel changes have been merged.

[1]: https://lore.kernel.org/barebox/20240503103717.1370636-1-a.fatoum@pengutronix.de/

---
Ahmad Fatoum (6):
      dt-bindings: clock: imx8m: document nominal/overdrive properties
      arm64: dts: imx8mp: Add optional nominal drive mode DTSI
      arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi
      arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration
      arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode
      clk: imx8mp: inform CCF of maximum frequency of clocks

 .../devicetree/bindings/clock/imx8m-clock.yaml     |  14 ++
 arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi  |  64 +++++++++
 .../arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi |   5 +-
 .../freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts  |  19 +--
 drivers/clk/imx/clk-imx8mp.c                       | 147 +++++++++++++++++++++
 5 files changed, 233 insertions(+), 16 deletions(-)
---
base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
change-id: 20241217-imx8m-clk-9467763dfcd8

Best regards,
-- 
Ahmad Fatoum <a.fatoum@pengutronix.de>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2024-12-19 19:49   ` Conor Dooley
  2024-12-19  7:27 ` [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI Ahmad Fatoum
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

The imx8m-clock.yaml binding covers the clock controller inside all
of the i.MX8M Q/M/N/P SoCs. All of them have in common that they
support two operating modes: nominal and overdrive mode.

While the overdrive mode allows for higher frequencies for many IPs,
the nominal mode needs a lower SoC voltage, thereby reducing
heat generation and power usage.

In any case, software should respect the maximum clock rate limits
described in the datasheet for each of the two operating modes.

To allow device tree consumers to enforce these limits, document two new
optional properties that can be used to sanity check the clock tree.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
index c643d4a81478..a6ae5257ef53 100644
--- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
@@ -43,6 +43,14 @@ properties:
       ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
       for the full list of i.MX8M clock IDs.
 
+  fsl,nominal-mode:
+    description: Set if SoC is operated in nominal mode
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  fsl,overdrive-mode:
+    description: Set if SoC is operated in overdrive mode
+    $ref: /schemas/types.yaml#/definitions/flag
+
 required:
   - compatible
   - reg
@@ -95,6 +103,12 @@ allOf:
             - const: clk_ext2
             - const: clk_ext3
             - const: clk_ext4
+  - if:
+      required:
+        - fsl,overdrive-mode
+    then:
+      properties:
+        fsl,nominal-mode: false
 
 additionalProperties: false
 

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2025-01-06  9:21   ` Peng Fan
  2024-12-19  7:27 ` [PATCH 3/6] arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi Ahmad Fatoum
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

Unlike the i.MX8MM and i.MX8MN SoCs added earlier, the device tree for
the i.MX8MP configures some clocks at frequencies that are only validated
for overdrive mode, i.e. when VDD_SOC is 950 mV.

Boards may want to run their SoC at the lower voltage of 850 mV though
to reduce heat generation and power usage. For this to work, clock rates
need to adhere to the limits of the nominal drive mode.

Add an optional DTSI file which can be included by various boards to run
in this mode.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi | 63 +++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi
new file mode 100644
index 000000000000..f9a82a663033
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2024 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
+ */
+
+&clk {
+	assigned-clocks = <&clk IMX8MP_CLK_A53_SRC>,
+			  <&clk IMX8MP_CLK_A53_CORE>,
+			  <&clk IMX8MP_SYS_PLL3>,
+			  <&clk IMX8MP_CLK_NOC>,
+			  <&clk IMX8MP_CLK_NOC_IO>,
+			  <&clk IMX8MP_CLK_GIC>;
+	assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
+				 <&clk IMX8MP_ARM_PLL_OUT>,
+				 <0>,
+				 <&clk IMX8MP_SYS_PLL1_800M>,
+				 <&clk IMX8MP_SYS_PLL3_OUT>,
+				 <&clk IMX8MP_SYS_PLL1_800M>;
+	assigned-clock-rates = <0>, <0>,
+			       <600000000>,
+			       <800000000>,
+			       <600000000>,
+			       <400000000>;
+};
+
+&pgc_hdmimix {
+	assigned-clocks = <&clk IMX8MP_CLK_HDMI_AXI>,
+			  <&clk IMX8MP_CLK_HDMI_APB>;
+	assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
+				 <&clk IMX8MP_SYS_PLL1_133M>;
+	assigned-clock-rates = <400000000>, <133000000>;
+};
+
+&pgc_hsiomix {
+	assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
+	assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
+	assigned-clock-rates = <400000000>;
+};
+
+&pgc_gpumix {
+	assigned-clocks = <&clk IMX8MP_CLK_GPU_AXI>,
+			  <&clk IMX8MP_CLK_GPU_AHB>;
+	assigned-clock-parents = <&clk IMX8MP_SYS_PLL3_OUT>,
+				 <&clk IMX8MP_SYS_PLL3_OUT>;
+	assigned-clock-rates = <600000000>, <300000000>;
+};
+
+&media_blk_ctrl {
+	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_AXI>,
+			  <&clk IMX8MP_CLK_MEDIA_APB>,
+			  <&clk IMX8MP_CLK_MEDIA_DISP1_PIX>,
+			  <&clk IMX8MP_CLK_MEDIA_DISP2_PIX>,
+			  <&clk IMX8MP_CLK_MEDIA_ISP>,
+			  <&clk IMX8MP_VIDEO_PLL1>;
+	assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
+				 <&clk IMX8MP_SYS_PLL1_800M>,
+				 <&clk IMX8MP_VIDEO_PLL1_OUT>,
+				 <&clk IMX8MP_VIDEO_PLL1_OUT>,
+				 <&clk IMX8MP_SYS_PLL1_800M>;
+	assigned-clock-rates = <400000000>, <200000000>,
+			       <0>, <0>, <400000000>,
+			       <1039500000>;
+};

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/6] arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 4/6] arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration Ahmad Fatoum
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

The imx8mp-nominal.dtsi is meant to be included into boards that want to
override the default overdrive clock settings with settings suitable for
running in nominal drive mode at its lower required voltage.

Specifying fsl,nominal-mode informs drivers of this fact, so they can
sanity check runtime clock reconfiguration to observe the limits imposed
by nominal mode.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi
index f9a82a663033..532eb049c7e8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi
@@ -21,6 +21,7 @@ &clk {
 			       <800000000>,
 			       <600000000>,
 			       <400000000>;
+	fsl,nominal-mode;
 };
 
 &pgc_hdmimix {

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/6] arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2024-12-19  7:27 ` [PATCH 3/6] arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 5/6] arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode Ahmad Fatoum
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

When the imx8mp-skov-revb-mi1010ait-1cp1 device tree was first added, it
configured the minimum clock rate supported by the panel instead of
the typical clock rate:

  fsl-ldb 32ec0000.blk-ctrl:bridge@5c: Configured LDB clock
  (482300000 Hz) does not match requested LVDS clock: 490000000 Hz

Besides the warning, the display functioned normally though
This broke with commit ff06ea04e4cf ("clk: imx: clk-imx8mp: Allow
media_disp pixel clock reconfigure parent rate") as the reconfiguration
of the parent clocks removed setting the IMX8MP_VIDEO_PLL1 to 7 times
the display clock rate:

  fsl-ldb 32ec0000.blk-ctrl:bridge@5c: Configured LDB clock
  (70000000 Hz) does not match requested LVDS clock: 490000000 Hz

Fix this by configuring the typical rate instead and benefit from
the new commit by dropping the now unneeded assigned-clock-rates
in &media_blk_ctrl.

Fixes: ff06ea04e4cf ("clk: imx: clk-imx8mp: Allow media_disp pixel clock reconfigure parent rate")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../dts/freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts
index 30962922b361..a13f6d76a495 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts
@@ -51,8 +51,11 @@ &lcdif2 {
 };
 
 &lvds_bridge {
-	/* IMX8MP_CLK_MEDIA_LDB = IMX8MP_CLK_MEDIA_DISP2_PIX * 7 */
-	assigned-clock-rates = <482300000>;
+	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_LDB>,
+				 <&clk IMX8MP_VIDEO_PLL1>;
+	assigned-clock-parents = <&clk IMX8MP_VIDEO_PLL1_OUT>;
+	/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_DISP2_PIX * 2 * 7 */
+	assigned-clock-rates = <0>, <980000000>;
 	status = "okay";
 
 	ports {
@@ -64,18 +67,6 @@ ldb_lvds_ch0: endpoint {
 	};
 };
 
-&media_blk_ctrl {
-	/* currently it is not possible to let display clocks confugure
-	 * automatically, so we need to set them manually
-	 */
-	assigned-clock-rates = <500000000>, <200000000>, <0>,
-		/* IMX8MP_CLK_MEDIA_DISP2_PIX = pixelclk of lvds panel */
-		<68900000>,
-		<500000000>,
-		/* IMX8MP_VIDEO_PLL1 = IMX8MP_CLK_MEDIA_LDB * 2 */
-		<964600000>;
-};
-
 &pwm4 {
 	status = "okay";
 };

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/6] arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2024-12-19  7:27 ` [PATCH 4/6] arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
  2024-12-20  6:16 ` [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Peng Fan
  6 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

To reduce heat generation, the Skov i.MX8MP boards should run in nominal
drive mode with a VDD_SOC voltage of 850 mV.

At this operating point, not all frequencies that are achievable with
overdrive mode are possible, so import imx8mp-nominal.dtsi to clock down
the clocks.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
index 59813ef8e2bb..9dc36517e90c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 
 #include "imx8mp.dtsi"
+#include "imx8mp-nominal.dtsi"
 
 #include <dt-bindings/leds/common.h>
 
@@ -237,8 +238,8 @@ pmic@25 {
 		regulators {
 			reg_vdd_soc: BUCK1 {
 				regulator-name = "VDD_SOC";
-				regulator-min-microvolt = <600000>;
-				regulator-max-microvolt = <2187500>;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
 				vin-supply = <&reg_5v_p>;
 				regulator-boot-on;
 				regulator-always-on;

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2024-12-19  7:27 ` [PATCH 5/6] arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode Ahmad Fatoum
@ 2024-12-19  7:27 ` Ahmad Fatoum
  2024-12-20  6:18   ` Peng Fan
                     ` (2 more replies)
  2024-12-20  6:16 ` [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Peng Fan
  6 siblings, 3 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19  7:27 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut
  Cc: linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel,
	Ahmad Fatoum

The IMX8MPCEC datasheet lists maximum frequencies allowed for different
modules. Some of these limits are universal, but some depend on
whether the SoC is operating in nominal or in overdrive mode.

The imx8mp.dtsi currently assumes overdrive mode and configures some
clocks in accordance with this. Boards wishing to make use of nominal
mode will need to override some of the clock rates manually.

As operating the clocks outside of their allowed range can lead to
difficult to debug issues, it makes sense to register the maximum rates
allowed in the driver, so the CCF can take them into account.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk-imx8mp.c | 147 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 516dbd170c8a..3b06990b73ad 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/units.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -405,6 +406,145 @@ static const char * const imx8mp_clkout_sels[] = {"audio_pll1_out", "audio_pll2_
 static struct clk_hw **hws;
 static struct clk_hw_onecell_data *clk_hw_data;
 
+struct imx8mp_clock_constraints {
+	int clkid;
+	u32 maxrate;
+};
+
+/*
+ * Below tables are taken from IMX8MPCEC Rev. 2.1, 07/2023
+ * Table 13. Maximum frequency of modules.
+ * Probable typos fixed are marked with a comment.
+ */
+static const struct imx8mp_clock_constraints imx8mp_clock_common_constraints[] = {
+	{ IMX8MP_CLK_A53_DIV,             1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ENET_AXI,             266666667 }, /* Datasheet claims 266MHz */
+	{ IMX8MP_CLK_NAND_USDHC_BUS,       266666667 }, /* Datasheet claims 266MHz */
+	{ IMX8MP_CLK_MEDIA_APB,            200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HDMI_APB,             133333333 }, /* Datasheet claims 133MHz */
+	{ IMX8MP_CLK_ML_AXI,               800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_AHB,                  133333333 },
+	{ IMX8MP_CLK_IPG_ROOT,              66666667 },
+	{ IMX8MP_CLK_AUDIO_AHB,            400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_DISP2_PIX,      170 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_DRAM_ALT,             666666667 },
+	{ IMX8MP_CLK_DRAM_APB,             200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_CAN1,                  80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_CAN2,                  80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_PCIE_AUX,              10 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_I2C5,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_I2C6,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_SAI1,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_SAI2,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_SAI3,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_SAI5,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_SAI6,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_ENET_QOS,             125 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ENET_QOS_TIMER,       200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ENET_REF,             125 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ENET_TIMER,           125 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ENET_PHY_REF,         125 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_NAND,                 500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_QSPI,                 400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_USDHC1,               400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_USDHC2,               400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_I2C1,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_I2C2,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_I2C3,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_I2C4,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_UART1,                 80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_UART2,                 80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_UART3,                 80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_UART4,                 80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ECSPI1,                80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ECSPI2,                80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_PWM1,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_PWM2,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_PWM3,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_PWM4,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_GPT1,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPT2,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPT3,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPT4,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPT5,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPT6,                 100 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_WDOG,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_IPP_DO_CLKO1,         200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_IPP_DO_CLKO2,         200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HDMI_REF_266M,        266 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_USDHC3,               400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_MIPI_PHY1_REF,  300 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_DISP1_PIX,      250 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_CAM2_PIX,       277 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_LDB,            595 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_MIPI_TEST_BYTE, 200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ECSPI3,                80 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_PDM,                  200 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_SAI7,                  66666667 }, /* Datasheet claims 66MHz */
+	{ IMX8MP_CLK_MAIN_AXI,             400 * HZ_PER_MHZ },
+	{ /* sentinel */ },
+};
+
+static const struct imx8mp_clock_constraints imx8mp_clock_nominal_constraints[] = {
+	{ IMX8MP_CLK_M7_CORE,           600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ML_CORE,           800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU3D_CORE,        800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU3D_SHADER_CORE, 800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU2D_CORE,        800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_AUDIO_AXI_SRC,     600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HSIO_AXI,          400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_ISP,         400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_BUS,           600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_AXI,         400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HDMI_AXI,          400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU_AXI,           600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU_AHB,           300 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_NOC,               800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_NOC_IO,            600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ML_AHB,            300 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_G1,            600 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_G2,            500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_CAM1_PIX,    400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_VC8000E,       400 * HZ_PER_MHZ }, /* Datasheet claims 500MHz */
+	{ IMX8MP_CLK_DRAM_CORE,         800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GIC,               400 * HZ_PER_MHZ },
+	{ /* sentinel */ },
+};
+
+static const struct imx8mp_clock_constraints imx8mp_clock_overdrive_constraints[] = {
+	{ IMX8MP_CLK_M7_CORE,            800 * HZ_PER_MHZ},
+	{ IMX8MP_CLK_ML_CORE,           1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU3D_CORE,        1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU3D_SHADER_CORE, 1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU2D_CORE,        1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_AUDIO_AXI_SRC,      800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HSIO_AXI,           500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_ISP,          500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_BUS,            800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_AXI,          500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_HDMI_AXI,           500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU_AXI,            800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GPU_AHB,            400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_NOC,               1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_NOC_IO,             800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_ML_AHB,             400 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_G1,             800 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_G2,             700 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_MEDIA_CAM1_PIX,     500 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_VPU_VC8000E,        500 * HZ_PER_MHZ }, /* Datasheet claims 400MHz */
+	{ IMX8MP_CLK_DRAM_CORE,         1000 * HZ_PER_MHZ },
+	{ IMX8MP_CLK_GIC,                500 * HZ_PER_MHZ },
+	{ /* sentinel */ },
+};
+
+static void imx8mp_clocks_apply_constraints(const struct imx8mp_clock_constraints constraints[])
+{
+	const struct imx8mp_clock_constraints *constr;
+
+	for (constr = constraints; constr->clkid; constr++)
+		clk_hw_set_rate_range(hws[constr->clkid], 0, constr->maxrate);
+}
+
 static int imx8mp_clocks_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -714,6 +854,13 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
 
 	imx_check_clk_hws(hws, IMX8MP_CLK_END);
 
+	imx8mp_clocks_apply_constraints(imx8mp_clock_common_constraints);
+
+	if (of_property_read_bool(np, "fsl,nominal-mode"))
+		imx8mp_clocks_apply_constraints(imx8mp_clock_nominal_constraints);
+	else if (of_property_read_bool(np, "fsl,overdrive-mode"))
+		imx8mp_clocks_apply_constraints(imx8mp_clock_overdrive_constraints);
+
 	err = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data);
 	if (err < 0) {
 		dev_err(dev, "failed to register hws for i.MX8MP\n");

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-19  7:27 ` [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties Ahmad Fatoum
@ 2024-12-19 19:49   ` Conor Dooley
  2024-12-19 20:14     ` Ahmad Fatoum
  0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2024-12-19 19:49 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]

On Thu, Dec 19, 2024 at 08:27:32AM +0100, Ahmad Fatoum wrote:
> The imx8m-clock.yaml binding covers the clock controller inside all
> of the i.MX8M Q/M/N/P SoCs. All of them have in common that they
> support two operating modes: nominal and overdrive mode.

This implies that only the two modes you mention are possible, but you
leave the option open to a dts author to use either. How come?

Makes it seem like we only need one of these, for whatever the
non-default option is?

> 
> While the overdrive mode allows for higher frequencies for many IPs,
> the nominal mode needs a lower SoC voltage, thereby reducing
> heat generation and power usage.
> 
> In any case, software should respect the maximum clock rate limits
> described in the datasheet for each of the two operating modes.
> 
> To allow device tree consumers to enforce these limits, document two new
> optional properties that can be used to sanity check the clock tree.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> index c643d4a81478..a6ae5257ef53 100644
> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> @@ -43,6 +43,14 @@ properties:
>        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
>        for the full list of i.MX8M clock IDs.
>  
> +  fsl,nominal-mode:
> +    description: Set if SoC is operated in nominal mode
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  fsl,overdrive-mode:
> +    description: Set if SoC is operated in overdrive mode
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
>  required:
>    - compatible
>    - reg
> @@ -95,6 +103,12 @@ allOf:
>              - const: clk_ext2
>              - const: clk_ext3
>              - const: clk_ext4
> +  - if:
> +      required:
> +        - fsl,overdrive-mode
> +    then:
> +      properties:
> +        fsl,nominal-mode: false
>  
>  additionalProperties: false
>  
> 
> -- 
> 2.39.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-19 19:49   ` Conor Dooley
@ 2024-12-19 20:14     ` Ahmad Fatoum
  2024-12-25 14:20       ` Conor Dooley
  0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-19 20:14 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

Hello Conor,

On 19.12.24 20:49, Conor Dooley wrote:
> On Thu, Dec 19, 2024 at 08:27:32AM +0100, Ahmad Fatoum wrote:
>> The imx8m-clock.yaml binding covers the clock controller inside all
>> of the i.MX8M Q/M/N/P SoCs. All of them have in common that they
>> support two operating modes: nominal and overdrive mode.
> 
> This implies that only the two modes you mention are possible, but you
> leave the option open to a dts author to use either. How come?
> 
> Makes it seem like we only need one of these, for whatever the
> non-default option is?

There is no real default. The mode is configured implicitly by the
bootloader setting VDD_SOC and then kernel needs to adhere to the
limits that imposes.

For i.MX8M Mini and Nano, the kernel SoC DTSIs has assigned-clock-rates
that are all achievable in nominal mode. For i.MX8MP, there are some
rates only validated for overdrive mode.

But even for the i.MX8M Mini/Nano boards, we don't know what rates they
may configure at runtime, so it's not possible to infer from just the
device tree what the mode is, which is why I need to allow for absence
of either property. I can make it a single property with two possible
values though if that's preferable.

Theoretically, we could infer mode at runtime from VDD_SOC voltage,
but we need to set up clocks to read out the PMIC and I want to
apply the constraints as early as possible as I don't want the SoC
to run outside of spec even for a short while.

Thanks,
Ahmad

> 
>>
>> While the overdrive mode allows for higher frequencies for many IPs,
>> the nominal mode needs a lower SoC voltage, thereby reducing
>> heat generation and power usage.
>>
>> In any case, software should respect the maximum clock rate limits
>> described in the datasheet for each of the two operating modes.
>>
>> To allow device tree consumers to enforce these limits, document two new
>> optional properties that can be used to sanity check the clock tree.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>> index c643d4a81478..a6ae5257ef53 100644
>> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>> @@ -43,6 +43,14 @@ properties:
>>        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
>>        for the full list of i.MX8M clock IDs.
>>  
>> +  fsl,nominal-mode:
>> +    description: Set if SoC is operated in nominal mode
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +
>> +  fsl,overdrive-mode:
>> +    description: Set if SoC is operated in overdrive mode
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -95,6 +103,12 @@ allOf:
>>              - const: clk_ext2
>>              - const: clk_ext3
>>              - const: clk_ext4
>> +  - if:
>> +      required:
>> +        - fsl,overdrive-mode
>> +    then:
>> +      properties:
>> +        fsl,nominal-mode: false
>>  
>>  additionalProperties: false
>>  
>>
>> -- 
>> 2.39.5
>>


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode
  2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
@ 2024-12-20  6:16 ` Peng Fan
  2024-12-20  6:21   ` Ahmad Fatoum
  6 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2024-12-20  6:16 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

On Thu, Dec 19, 2024 at 08:27:31AM +0100, Ahmad Fatoum wrote:
>Unlike the i.MX8MM and i.MX8MN SoCs added earlier, the imx8mp.dtsi
>configures some clocks at frequencies that are only validated for
>overdrive mode, i.e., when VDD_SOC is 950 mV.
>
>For the Skov i.MX8MP board, we want to run the SoC at the lower voltage of
>850 mV though to reduce heat generation and power usage. For this to work,
>clock rates need to adhere to the limits of the nominal drive mode.
>
>This is done by this series: A new imx8mp-nominal.dtsi reconfigures
>the imx8mp.dtsi clock tree to be compatible with nominal mode, an adaptation
>to the Linux clock driver makes it sanity check the actual clock rates against
>the SoC operating mode's constraints and finally the Skov DT makes use
>of it.
>
>Actual configuration of the VDD_SOC rail continues to happen prior to Linux
>as well as PLL configuration that needs to happen earlier than the kernel
>running. See the corresponding barebox patch series[1] for details.
>Note that the barebox series didn't yet include VDD_SOC reconfiguration
>to 850mV, that would follow once the kernel changes have been merged.

Good to see this. I had same plan to support i.MX9.

I see you introduce a new property, how about using a boot pararmeter,
saying "mode=nominal" or "mode=overdrive". Then drivers could
act accordingly.

Regards,
Peng

>
>[1]: https://lore.kernel.org/barebox/20240503103717.1370636-1-a.fatoum@pengutronix.de/
>
>---
>Ahmad Fatoum (6):
>      dt-bindings: clock: imx8m: document nominal/overdrive properties
>      arm64: dts: imx8mp: Add optional nominal drive mode DTSI
>      arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi
>      arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration
>      arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode
>      clk: imx8mp: inform CCF of maximum frequency of clocks
>
> .../devicetree/bindings/clock/imx8m-clock.yaml     |  14 ++
> arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi  |  64 +++++++++
> .../arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi |   5 +-
> .../freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts  |  19 +--
> drivers/clk/imx/clk-imx8mp.c                       | 147 +++++++++++++++++++++
> 5 files changed, 233 insertions(+), 16 deletions(-)
>---
>base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
>change-id: 20241217-imx8m-clk-9467763dfcd8
>
>Best regards,
>-- 
>Ahmad Fatoum <a.fatoum@pengutronix.de>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks
  2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
@ 2024-12-20  6:18   ` Peng Fan
  2024-12-20  6:30     ` Ahmad Fatoum
  2024-12-30  1:38   ` Peng Fan
  2025-01-06  9:08   ` Peng Fan
  2 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2024-12-20  6:18 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

On Thu, Dec 19, 2024 at 08:27:37AM +0100, Ahmad Fatoum wrote:
>The IMX8MPCEC datasheet lists maximum frequencies allowed for different
>modules. Some of these limits are universal, but some depend on
>whether the SoC is operating in nominal or in overdrive mode.
>
>The imx8mp.dtsi currently assumes overdrive mode and configures some
>clocks in accordance with this. Boards wishing to make use of nominal
>mode will need to override some of the clock rates manually.
>
>As operating the clocks outside of their allowed range can lead to
>difficult to debug issues, it makes sense to register the maximum rates
>allowed in the driver, so the CCF can take them into account.
>
>Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>---
> drivers/clk/imx/clk-imx8mp.c | 147 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 147 insertions(+)
>
>diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
>index 516dbd170c8a..3b06990b73ad 100644
>--- a/drivers/clk/imx/clk-imx8mp.c
>+++ b/drivers/clk/imx/clk-imx8mp.c
>@@ -8,6 +8,7 @@
> #include <linux/err.h>
> #include <linux/io.h>
> #include <linux/module.h>
>+#include <linux/units.h>
> #include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>@@ -405,6 +406,145 @@ static const char * const imx8mp_clkout_sels[] = {"audio_pll1_out", "audio_pll2_
> static struct clk_hw **hws;
> static struct clk_hw_onecell_data *clk_hw_data;
> 
>+struct imx8mp_clock_constraints {
>+	int clkid;
>+	u32 maxrate;
>+};
>+
>+/*
>+ * Below tables are taken from IMX8MPCEC Rev. 2.1, 07/2023
>+ * Table 13. Maximum frequency of modules.
>+ * Probable typos fixed are marked with a comment.
>+ */
>+static const struct imx8mp_clock_constraints imx8mp_clock_common_constraints[] = {
>+	{ IMX8MP_CLK_A53_DIV,             1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ENET_AXI,             266666667 }, /* Datasheet claims 266MHz */
>+	{ IMX8MP_CLK_NAND_USDHC_BUS,       266666667 }, /* Datasheet claims 266MHz */
>+	{ IMX8MP_CLK_MEDIA_APB,            200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HDMI_APB,             133333333 }, /* Datasheet claims 133MHz */
>+	{ IMX8MP_CLK_ML_AXI,               800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_AHB,                  133333333 },
>+	{ IMX8MP_CLK_IPG_ROOT,              66666667 },
>+	{ IMX8MP_CLK_AUDIO_AHB,            400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_DISP2_PIX,      170 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_DRAM_ALT,             666666667 },
>+	{ IMX8MP_CLK_DRAM_APB,             200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_CAN1,                  80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_CAN2,                  80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_PCIE_AUX,              10 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_I2C5,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_I2C6,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_SAI1,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_SAI2,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_SAI3,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_SAI5,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_SAI6,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_ENET_QOS,             125 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ENET_QOS_TIMER,       200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ENET_REF,             125 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ENET_TIMER,           125 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ENET_PHY_REF,         125 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_NAND,                 500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_QSPI,                 400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_USDHC1,               400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_USDHC2,               400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_I2C1,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_I2C2,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_I2C3,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_I2C4,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_UART1,                 80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_UART2,                 80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_UART3,                 80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_UART4,                 80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ECSPI1,                80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ECSPI2,                80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_PWM1,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_PWM2,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_PWM3,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_PWM4,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_GPT1,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPT2,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPT3,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPT4,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPT5,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPT6,                 100 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_WDOG,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_IPP_DO_CLKO1,         200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_IPP_DO_CLKO2,         200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HDMI_REF_266M,        266 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_USDHC3,               400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_MIPI_PHY1_REF,  300 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_DISP1_PIX,      250 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_CAM2_PIX,       277 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_LDB,            595 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_MIPI_TEST_BYTE, 200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ECSPI3,                80 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_PDM,                  200 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_SAI7,                  66666667 }, /* Datasheet claims 66MHz */
>+	{ IMX8MP_CLK_MAIN_AXI,             400 * HZ_PER_MHZ },
>+	{ /* sentinel */ },
>+};
>+
>+static const struct imx8mp_clock_constraints imx8mp_clock_nominal_constraints[] = {
>+	{ IMX8MP_CLK_M7_CORE,           600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ML_CORE,           800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU3D_CORE,        800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU3D_SHADER_CORE, 800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU2D_CORE,        800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_AUDIO_AXI_SRC,     600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HSIO_AXI,          400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_ISP,         400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_BUS,           600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_AXI,         400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HDMI_AXI,          400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU_AXI,           600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU_AHB,           300 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_NOC,               800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_NOC_IO,            600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ML_AHB,            300 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_G1,            600 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_G2,            500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_CAM1_PIX,    400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_VC8000E,       400 * HZ_PER_MHZ }, /* Datasheet claims 500MHz */
>+	{ IMX8MP_CLK_DRAM_CORE,         800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GIC,               400 * HZ_PER_MHZ },
>+	{ /* sentinel */ },
>+};
>+
>+static const struct imx8mp_clock_constraints imx8mp_clock_overdrive_constraints[] = {
>+	{ IMX8MP_CLK_M7_CORE,            800 * HZ_PER_MHZ},
>+	{ IMX8MP_CLK_ML_CORE,           1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU3D_CORE,        1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU3D_SHADER_CORE, 1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU2D_CORE,        1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_AUDIO_AXI_SRC,      800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HSIO_AXI,           500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_ISP,          500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_BUS,            800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_AXI,          500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_HDMI_AXI,           500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU_AXI,            800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GPU_AHB,            400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_NOC,               1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_NOC_IO,             800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_ML_AHB,             400 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_G1,             800 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_G2,             700 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_MEDIA_CAM1_PIX,     500 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_VPU_VC8000E,        500 * HZ_PER_MHZ }, /* Datasheet claims 400MHz */
>+	{ IMX8MP_CLK_DRAM_CORE,         1000 * HZ_PER_MHZ },
>+	{ IMX8MP_CLK_GIC,                500 * HZ_PER_MHZ },
>+	{ /* sentinel */ },
>+};
>+
>+static void imx8mp_clocks_apply_constraints(const struct imx8mp_clock_constraints constraints[])
>+{
>+	const struct imx8mp_clock_constraints *constr;
>+
>+	for (constr = constraints; constr->clkid; constr++)
>+		clk_hw_set_rate_range(hws[constr->clkid], 0, constr->maxrate);
>+}
>+
> static int imx8mp_clocks_probe(struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
>@@ -714,6 +854,13 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
> 
> 	imx_check_clk_hws(hws, IMX8MP_CLK_END);
> 
>+	imx8mp_clocks_apply_constraints(imx8mp_clock_common_constraints);
>+
>+	if (of_property_read_bool(np, "fsl,nominal-mode"))
>+		imx8mp_clocks_apply_constraints(imx8mp_clock_nominal_constraints);
>+	else if (of_property_read_bool(np, "fsl,overdrive-mode"))
>+		imx8mp_clocks_apply_constraints(imx8mp_clock_overdrive_constraints);

As I replied, a boot parameter should be better? the mode is a soc level mode,
not just clock controller.

Thanks,
Peng

>+
> 	err = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data);
> 	if (err < 0) {
> 		dev_err(dev, "failed to register hws for i.MX8MP\n");
>
>-- 
>2.39.5
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode
  2024-12-20  6:16 ` [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Peng Fan
@ 2024-12-20  6:21   ` Ahmad Fatoum
  0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-20  6:21 UTC (permalink / raw)
  To: Peng Fan
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

Hello Peng,

On 20.12.24 07:16, Peng Fan wrote:
> On Thu, Dec 19, 2024 at 08:27:31AM +0100, Ahmad Fatoum wrote:
>> Unlike the i.MX8MM and i.MX8MN SoCs added earlier, the imx8mp.dtsi
>> configures some clocks at frequencies that are only validated for
>> overdrive mode, i.e., when VDD_SOC is 950 mV.
>>
>> For the Skov i.MX8MP board, we want to run the SoC at the lower voltage of
>> 850 mV though to reduce heat generation and power usage. For this to work,
>> clock rates need to adhere to the limits of the nominal drive mode.
>>
>> This is done by this series: A new imx8mp-nominal.dtsi reconfigures
>> the imx8mp.dtsi clock tree to be compatible with nominal mode, an adaptation
>> to the Linux clock driver makes it sanity check the actual clock rates against
>> the SoC operating mode's constraints and finally the Skov DT makes use
>> of it.
>>
>> Actual configuration of the VDD_SOC rail continues to happen prior to Linux
>> as well as PLL configuration that needs to happen earlier than the kernel
>> running. See the corresponding barebox patch series[1] for details.
>> Note that the barebox series didn't yet include VDD_SOC reconfiguration
>> to 850mV, that would follow once the kernel changes have been merged.
> 
> Good to see this. I had same plan to support i.MX9.
> 
> I see you introduce a new property, how about using a boot pararmeter,
> saying "mode=nominal" or "mode=overdrive". Then drivers could
> act accordingly.

I assume you would place that parameter in drivers/soc/imx/soc-imx8m.c?

That's certainly possible as well, but I think for such a hardware property,
the device tree is the natural place.

Thanks,
Ahmad

> 
> Regards,
> Peng
> 
>>
>> [1]: https://lore.kernel.org/barebox/20240503103717.1370636-1-a.fatoum@pengutronix.de/
>>
>> ---
>> Ahmad Fatoum (6):
>>      dt-bindings: clock: imx8m: document nominal/overdrive properties
>>      arm64: dts: imx8mp: Add optional nominal drive mode DTSI
>>      arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi
>>      arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration
>>      arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode
>>      clk: imx8mp: inform CCF of maximum frequency of clocks
>>
>> .../devicetree/bindings/clock/imx8m-clock.yaml     |  14 ++
>> arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi  |  64 +++++++++
>> .../arm64/boot/dts/freescale/imx8mp-skov-reva.dtsi |   5 +-
>> .../freescale/imx8mp-skov-revb-mi1010ait-1cp1.dts  |  19 +--
>> drivers/clk/imx/clk-imx8mp.c                       | 147 +++++++++++++++++++++
>> 5 files changed, 233 insertions(+), 16 deletions(-)
>> ---
>> base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
>> change-id: 20241217-imx8m-clk-9467763dfcd8
>>
>> Best regards,
>> -- 
>> Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks
  2024-12-20  6:18   ` Peng Fan
@ 2024-12-20  6:30     ` Ahmad Fatoum
  0 siblings, 0 replies; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-20  6:30 UTC (permalink / raw)
  To: Peng Fan
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

Hi Peng,

On 20.12.24 07:18, Peng Fan wrote:
> On Thu, Dec 19, 2024 at 08:27:37AM +0100, Ahmad Fatoum wrote:
>> The IMX8MPCEC datasheet lists maximum frequencies allowed for different
>> modules. Some of these limits are universal, but some depend on
>> whether the SoC is operating in nominal or in overdrive mode.
>>
>> The imx8mp.dtsi currently assumes overdrive mode and configures some
>> clocks in accordance with this. Boards wishing to make use of nominal
>> mode will need to override some of the clock rates manually.
>>
>> As operating the clocks outside of their allowed range can lead to
>> difficult to debug issues, it makes sense to register the maximum rates
>> allowed in the driver, so the CCF can take them into account.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

>> +	imx8mp_clocks_apply_constraints(imx8mp_clock_common_constraints);
>> +
>> +	if (of_property_read_bool(np, "fsl,nominal-mode"))
>> +		imx8mp_clocks_apply_constraints(imx8mp_clock_nominal_constraints);
>> +	else if (of_property_read_bool(np, "fsl,overdrive-mode"))
>> +		imx8mp_clocks_apply_constraints(imx8mp_clock_overdrive_constraints);
> 
> As I replied, a boot parameter should be better? the mode is a soc level mode,
> not just clock controller.

I think it's counterproductive for a sanity check to be enforced via kernel
command-line.

The Skov board shouldn't run with overdrive frequencies and I prefer to encode
that in the same device tree, where I define the permissible VDD_SOC range
and configure the initial clock rates.

The mode is selected by the VDD_SOC voltage, but affects AFAICS only the clock
tree. IMO, the clock controller module is thus a natural place for the property.

Cheers,
Ahmad

> 
> Thanks,
> Peng
> 
>> +
>> 	err = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data);
>> 	if (err < 0) {
>> 		dev_err(dev, "failed to register hws for i.MX8MP\n");
>>
>> -- 
>> 2.39.5
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-19 20:14     ` Ahmad Fatoum
@ 2024-12-25 14:20       ` Conor Dooley
  2024-12-26 16:54         ` Ahmad Fatoum
  0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2024-12-25 14:20 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4247 bytes --]

On Thu, Dec 19, 2024 at 09:14:10PM +0100, Ahmad Fatoum wrote:
> Hello Conor,
> 
> On 19.12.24 20:49, Conor Dooley wrote:
> > On Thu, Dec 19, 2024 at 08:27:32AM +0100, Ahmad Fatoum wrote:
> >> The imx8m-clock.yaml binding covers the clock controller inside all
> >> of the i.MX8M Q/M/N/P SoCs. All of them have in common that they
> >> support two operating modes: nominal and overdrive mode.
> > 
> > This implies that only the two modes you mention are possible, but you
> > leave the option open to a dts author to use either. How come?
> > 
> > Makes it seem like we only need one of these, for whatever the
> > non-default option is?
> 
> There is no real default. The mode is configured implicitly by the
> bootloader setting VDD_SOC and then kernel needs to adhere to the
> limits that imposes.
> 
> For i.MX8M Mini and Nano, the kernel SoC DTSIs has assigned-clock-rates
> that are all achievable in nominal mode. For i.MX8MP, there are some
> rates only validated for overdrive mode.
> 
> But even for the i.MX8M Mini/Nano boards, we don't know what rates they
> may configure at runtime, so it's not possible to infer from just the
> device tree what the mode is, which is why I need to allow for absence
> of either property. I can make it a single property with two possible
> values though if that's preferable.
> 
> Theoretically, we could infer mode at runtime from VDD_SOC voltage,
> but we need to set up clocks to read out the PMIC and I want to
> apply the constraints as early as possible as I don't want the SoC
> to run outside of spec even for a short while.

Apologies for the delay responding to you, doing it today cos I feel
guilty! I think what you've explained here is fine, but could you add a
bit more of that info to the commit message, explaining why one cannot
be default? With that,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> 
> Thanks,
> Ahmad
> 
> > 
> >>
> >> While the overdrive mode allows for higher frequencies for many IPs,
> >> the nominal mode needs a lower SoC voltage, thereby reducing
> >> heat generation and power usage.
> >>
> >> In any case, software should respect the maximum clock rate limits
> >> described in the datasheet for each of the two operating modes.
> >>
> >> To allow device tree consumers to enforce these limits, document two new
> >> optional properties that can be used to sanity check the clock tree.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 14 ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> index c643d4a81478..a6ae5257ef53 100644
> >> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> @@ -43,6 +43,14 @@ properties:
> >>        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
> >>        for the full list of i.MX8M clock IDs.
> >>  
> >> +  fsl,nominal-mode:
> >> +    description: Set if SoC is operated in nominal mode
> >> +    $ref: /schemas/types.yaml#/definitions/flag
> >> +
> >> +  fsl,overdrive-mode:
> >> +    description: Set if SoC is operated in overdrive mode
> >> +    $ref: /schemas/types.yaml#/definitions/flag
> >> +
> >>  required:
> >>    - compatible
> >>    - reg
> >> @@ -95,6 +103,12 @@ allOf:
> >>              - const: clk_ext2
> >>              - const: clk_ext3
> >>              - const: clk_ext4
> >> +  - if:
> >> +      required:
> >> +        - fsl,overdrive-mode
> >> +    then:
> >> +      properties:
> >> +        fsl,nominal-mode: false
> >>  
> >>  additionalProperties: false
> >>  
> >>
> >> -- 
> >> 2.39.5
> >>
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-25 14:20       ` Conor Dooley
@ 2024-12-26 16:54         ` Ahmad Fatoum
  2024-12-27 17:48           ` Conor Dooley
  0 siblings, 1 reply; 19+ messages in thread
From: Ahmad Fatoum @ 2024-12-26 16:54 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

Hi Conor,

On 25.12.24 15:20, Conor Dooley wrote:
> On Thu, Dec 19, 2024 at 09:14:10PM +0100, Ahmad Fatoum wrote:
>> On 19.12.24 20:49, Conor Dooley wrote:
>>> On Thu, Dec 19, 2024 at 08:27:32AM +0100, Ahmad Fatoum wrote:
>> Theoretically, we could infer mode at runtime from VDD_SOC voltage,
>> but we need to set up clocks to read out the PMIC and I want to
>> apply the constraints as early as possible as I don't want the SoC
>> to run outside of spec even for a short while.
> 
> Apologies for the delay responding to you, doing it today cos I feel
> guilty!

I am fully aware that I needn't expect prompt review feedback so late in
December. Thanks a lot for taking the time still.

> I think what you've explained here is fine, but could you add a
> bit more of that info to the commit message, explaining why one cannot
> be default? With that,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks. I will await further review feedback and adjust this commit's
message for v2 as requested.

Wishing you nice holidays,
Ahmad

> 
> Cheers,
> Conor.
> 
>>
>> Thanks,
>> Ahmad
>>
>>>
>>>>
>>>> While the overdrive mode allows for higher frequencies for many IPs,
>>>> the nominal mode needs a lower SoC voltage, thereby reducing
>>>> heat generation and power usage.
>>>>
>>>> In any case, software should respect the maximum clock rate limits
>>>> described in the datasheet for each of the two operating modes.
>>>>
>>>> To allow device tree consumers to enforce these limits, document two new
>>>> optional properties that can be used to sanity check the clock tree.
>>>>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>>  Documentation/devicetree/bindings/clock/imx8m-clock.yaml | 14 ++++++++++++++
>>>>  1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>>>> index c643d4a81478..a6ae5257ef53 100644
>>>> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>>>> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
>>>> @@ -43,6 +43,14 @@ properties:
>>>>        ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
>>>>        for the full list of i.MX8M clock IDs.
>>>>  
>>>> +  fsl,nominal-mode:
>>>> +    description: Set if SoC is operated in nominal mode
>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>> +
>>>> +  fsl,overdrive-mode:
>>>> +    description: Set if SoC is operated in overdrive mode
>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>> +
>>>>  required:
>>>>    - compatible
>>>>    - reg
>>>> @@ -95,6 +103,12 @@ allOf:
>>>>              - const: clk_ext2
>>>>              - const: clk_ext3
>>>>              - const: clk_ext4
>>>> +  - if:
>>>> +      required:
>>>> +        - fsl,overdrive-mode
>>>> +    then:
>>>> +      properties:
>>>> +        fsl,nominal-mode: false
>>>>  
>>>>  additionalProperties: false
>>>>  
>>>>
>>>> -- 
>>>> 2.39.5
>>>>
>>
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties
  2024-12-26 16:54         ` Ahmad Fatoum
@ 2024-12-27 17:48           ` Conor Dooley
  0 siblings, 0 replies; 19+ messages in thread
From: Conor Dooley @ 2024-12-27 17:48 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Thu, Dec 26, 2024 at 05:54:15PM +0100, Ahmad Fatoum wrote:
> Hi Conor,
> 
> On 25.12.24 15:20, Conor Dooley wrote:
> > On Thu, Dec 19, 2024 at 09:14:10PM +0100, Ahmad Fatoum wrote:
> >> On 19.12.24 20:49, Conor Dooley wrote:
> >>> On Thu, Dec 19, 2024 at 08:27:32AM +0100, Ahmad Fatoum wrote:
> >> Theoretically, we could infer mode at runtime from VDD_SOC voltage,
> >> but we need to set up clocks to read out the PMIC and I want to
> >> apply the constraints as early as possible as I don't want the SoC
> >> to run outside of spec even for a short while.
> > 
> > Apologies for the delay responding to you, doing it today cos I feel
> > guilty!
> 
> I am fully aware that I needn't expect prompt review feedback so late in
> December. Thanks a lot for taking the time still.

I guess, but I /had/ replied to other things that came in after your
mail, which is where my guilt comes from!

> > I think what you've explained here is fine, but could you add a
> > bit more of that info to the commit message, explaining why one cannot
> > be default? With that,
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks. I will await further review feedback and adjust this commit's
> message for v2 as requested.
> 
> Wishing you nice holidays,

You too (or what's left of em at least).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks
  2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
  2024-12-20  6:18   ` Peng Fan
@ 2024-12-30  1:38   ` Peng Fan
  2025-01-06  9:08   ` Peng Fan
  2 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2024-12-30  1:38 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

On Thu, Dec 19, 2024 at 08:27:37AM +0100, Ahmad Fatoum wrote:
>The IMX8MPCEC datasheet lists maximum frequencies allowed for different
>modules. Some of these limits are universal, but some depend on
>whether the SoC is operating in nominal or in overdrive mode.
>
>The imx8mp.dtsi currently assumes overdrive mode and configures some
>clocks in accordance with this. Boards wishing to make use of nominal
>mode will need to override some of the clock rates manually.
>
>As operating the clocks outside of their allowed range can lead to
>difficult to debug issues, it makes sense to register the maximum rates
>allowed in the driver, so the CCF can take them into account.
>
>Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks
  2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
  2024-12-20  6:18   ` Peng Fan
  2024-12-30  1:38   ` Peng Fan
@ 2025-01-06  9:08   ` Peng Fan
  2 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2025-01-06  9:08 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

On Thu, Dec 19, 2024 at 08:27:37AM +0100, Ahmad Fatoum wrote:
>The IMX8MPCEC datasheet lists maximum frequencies allowed for different
>modules. Some of these limits are universal, but some depend on
>whether the SoC is operating in nominal or in overdrive mode.
>
>The imx8mp.dtsi currently assumes overdrive mode and configures some
>clocks in accordance with this. Boards wishing to make use of nominal
>mode will need to override some of the clock rates manually.
>
>As operating the clocks outside of their allowed range can lead to
>difficult to debug issues, it makes sense to register the maximum rates
>allowed in the driver, so the CCF can take them into account.
>
>Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI
  2024-12-19  7:27 ` [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI Ahmad Fatoum
@ 2025-01-06  9:21   ` Peng Fan
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2025-01-06  9:21 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Abel Vesa, Marek Vasut,
	linux-clk, imx, devicetree, linux-arm-kernel, linux-kernel

On Thu, Dec 19, 2024 at 08:27:33AM +0100, Ahmad Fatoum wrote:
>Unlike the i.MX8MM and i.MX8MN SoCs added earlier, the device tree for
>the i.MX8MP configures some clocks at frequencies that are only validated
>for overdrive mode, i.e. when VDD_SOC is 950 mV.
>
>Boards may want to run their SoC at the lower voltage of 850 mV though
>to reduce heat generation and power usage. For this to work, clock rates
>need to adhere to the limits of the nominal drive mode.
>
>Add an optional DTSI file which can be included by various boards to run
>in this mode.
>
>Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-01-06  8:16 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-19  7:27 [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Ahmad Fatoum
2024-12-19  7:27 ` [PATCH 1/6] dt-bindings: clock: imx8m: document nominal/overdrive properties Ahmad Fatoum
2024-12-19 19:49   ` Conor Dooley
2024-12-19 20:14     ` Ahmad Fatoum
2024-12-25 14:20       ` Conor Dooley
2024-12-26 16:54         ` Ahmad Fatoum
2024-12-27 17:48           ` Conor Dooley
2024-12-19  7:27 ` [PATCH 2/6] arm64: dts: imx8mp: Add optional nominal drive mode DTSI Ahmad Fatoum
2025-01-06  9:21   ` Peng Fan
2024-12-19  7:27 ` [PATCH 3/6] arm64: dts: imx8mp: add fsl,nominal-mode property into nominal.dtsi Ahmad Fatoum
2024-12-19  7:27 ` [PATCH 4/6] arm64: dts: freescale: imx8mp-skov: fix LDB clock rate configuration Ahmad Fatoum
2024-12-19  7:27 ` [PATCH 5/6] arm64: dts: freescale: imx8mp-skov: operate SoC in nominal mode Ahmad Fatoum
2024-12-19  7:27 ` [PATCH 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks Ahmad Fatoum
2024-12-20  6:18   ` Peng Fan
2024-12-20  6:30     ` Ahmad Fatoum
2024-12-30  1:38   ` Peng Fan
2025-01-06  9:08   ` Peng Fan
2024-12-20  6:16 ` [PATCH 0/6] arm64: dts: freescale: imx8mp-skov: switch to nominal drive mode Peng Fan
2024-12-20  6:21   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).